🔧 Validate changes immediately after processing them

This commit is contained in:
Andrés Moya 2026-09-01 14:49:47 +02:00
parent eb2576b8c0
commit 5868ad0042
8 changed files with 206 additions and 65 deletions

View File

@ -6,6 +6,7 @@
(ns app.common.files.changes
(:require
#?(:cljs [app.common.files.validate :as val])
[app.common.data :as d]
[app.common.data.macros :as dm]
[app.common.exceptions :as ex]
@ -438,7 +439,14 @@
[:map {:title "SetTokensSource"}
[:type [:= :set-tokens-source]]
[:file-id ::sm/uuid]
[:library-id [:maybe ::sm/uuid]]]]])
[:library-id [:maybe ::sm/uuid]]]]
[:validate-shapes
[:map {:title "ValidateShapesChange"}
[:type [:= :validate-shapes]]
[:page-id ::sm/uuid]
[:shape-ids [:vector ::sm/uuid]]
[:context :string]]]])
(def schema:changes
[:sequential {:gen/max 5 :gen/min 1} schema:change])
@ -474,7 +482,7 @@
to the processor backend."
nil)
(defmulti process-change (fn [_ change] (:type change)))
(defmulti process-change (fn [_ change _] (:type change)))
(defmulti process-operation (fn [_ op] (:type op)))
;; Changes Processing Impl
@ -506,22 +514,25 @@
(defn process-changes
([data items]
(process-changes data items true))
(process-changes data items true {}))
([data items verify?]
(process-changes data items verify? {}))
([data items verify? libraries]
;; When verify? false we spec the schema validation. Currently used
;; to make just 1 validation even if the changes are applied twice
(when verify?
(check-changes items))
(binding [*touched-changes* (volatile! #{})]
(let [result (reduce #(or (process-change %1 %2) %1) data items)]
(let [result (reduce #(or (process-change %1 %2 libraries) %1) data items)]
(reduce process-touched-change result @*touched-changes*)))))
;; --- Comment Threads
(defmethod process-change :set-comment-thread-position
[data {:keys [page-id comment-thread-id position frame-id]}]
[data {:keys [page-id comment-thread-id position frame-id]} _]
(d/update-in-when data [:pages-index page-id]
(fn [page]
(if (and position frame-id)
@ -534,7 +545,7 @@
;; --- Guides
(defmethod process-change :set-guide
[data {:keys [page-id id params]}]
[data {:keys [page-id id params]} _]
(if (nil? params)
(d/update-in-when data [:pages-index page-id]
(fn [page]
@ -550,7 +561,7 @@
;; --- Flows
(defmethod process-change :set-flow
[data {:keys [page-id id params]}]
[data {:keys [page-id id params]} _]
(if (nil? params)
(d/update-in-when data [:pages-index page-id]
(fn [page]
@ -566,7 +577,7 @@
;; --- Grids
(defmethod process-change :set-default-grid
[data {:keys [page-id grid-type params]}]
[data {:keys [page-id grid-type params]} _]
(if (nil? params)
(d/update-in-when data [:pages-index page-id]
(fn [page]
@ -603,7 +614,7 @@
(update state :media-refs into xform media-refs)))
(defmethod process-change :add-obj
[data {:keys [id obj page-id component-id frame-id parent-id index ignore-touched]}]
[data {:keys [id obj page-id component-id frame-id parent-id index ignore-touched]} _]
;; NOTE: we only perform hard validation on backend
#?(:clj (validate-shape obj page-id))
@ -638,7 +649,7 @@
objects))
(defmethod process-change :mod-obj
[data {:keys [page-id component-id] :as change}]
[data {:keys [page-id component-id] :as change} _]
(if page-id
(d/update-in-when data [:pages-index page-id :objects] process-operations change)
(d/update-in-when data [:components component-id :objects] process-operations change)))
@ -668,19 +679,19 @@
objects))
(defmethod process-change :reorder-children
[data {:keys [page-id component-id] :as change}]
[data {:keys [page-id component-id] :as change} _]
(if page-id
(d/update-in-when data [:pages-index page-id :objects] process-children-reordering change)
(d/update-in-when data [:components component-id :objects] process-children-reordering change)))
(defmethod process-change :del-obj
[data {:keys [page-id component-id id ignore-touched]}]
[data {:keys [page-id component-id id ignore-touched]} _]
(if page-id
(d/update-in-when data [:pages-index page-id] ctst/delete-shape id ignore-touched)
(d/update-in-when data [:components component-id] ctst/delete-shape id ignore-touched)))
(defmethod process-change :fix-obj
[data {:keys [page-id component-id id] :as params}]
[data {:keys [page-id component-id id] :as params} _]
(letfn [(fix-container [container]
(case (:fix params :broken-children)
:broken-children (ctst/fix-broken-children container id)
@ -692,7 +703,7 @@
(d/update-in-when data [:components component-id] fix-container))))
(defmethod process-change :reg-objects
[data {:keys [page-id component-id shapes]}]
[data {:keys [page-id component-id shapes]} _]
;; FIXME: Improve performance
(letfn [(reg-objects [objects]
(let [lookup (d/getf objects)
@ -749,7 +760,7 @@
(defmethod process-change :mov-objects
;; FIXME: ignore-touched is no longer used, so we can consider it deprecated
[data {:keys [parent-id shapes index page-id component-id #_ignore-touched after-shape allow-altering-copies syncing]}]
[data {:keys [parent-id shapes index page-id component-id #_ignore-touched after-shape allow-altering-copies syncing]} _]
(letfn [(calculate-invalid-targets [objects shape-id]
(let [reduce-fn #(into %1 (calculate-invalid-targets objects %2))]
(->> (get-in objects [shape-id :shapes])
@ -864,7 +875,7 @@
(d/update-in-when data [:components component-id :objects] move-objects))))
(defmethod process-change :add-page
[data {:keys [id name page]}]
[data {:keys [id name page]} _]
(when (and id name page)
(ex/raise :type :conflict
:hint "id+name or page should be provided, never both"))
@ -874,7 +885,7 @@
(ctpl/add-page data page)))
(defmethod process-change :mod-page
[data {:keys [id] :as params}]
[data {:keys [id] :as params} _]
(d/update-in-when data [:pages-index id]
(fn [page]
(let [name (get params :name)
@ -911,7 +922,7 @@
(dissoc :pixel-grid-opacity))))))
(defmethod process-change :set-plugin-data
[data {:keys [object-type object-id page-id namespace key value]}]
[data {:keys [object-type object-id page-id namespace key value]} _]
(letfn [(update-fn [data]
(if (some? value)
(assoc-in data [:plugin-data namespace key] value)
@ -937,83 +948,83 @@
(d/update-in-when data [:components object-id] update-fn))))
(defmethod process-change :del-page
[data {:keys [id]}]
[data {:keys [id]} _]
(ctpl/delete-page data id))
(defmethod process-change :mov-page
[data {:keys [id index]}]
[data {:keys [id index]} _]
(update data :pages d/insert-at-index index [id]))
(defmethod process-change :add-color
[data {:keys [color]}]
[data {:keys [color]} _]
(ctl/add-color data color))
(defmethod process-change :mod-color
[data {:keys [color]}]
[data {:keys [color]} _]
(ctl/set-color data color))
(defmethod process-change :del-color
[data {:keys [id]}]
[data {:keys [id]} _]
(ctl/delete-color data id))
;; -- Media
(defmethod process-change :add-media
[data {:keys [object]}]
[data {:keys [object]} _]
(update data :media assoc (:id object) object))
(defmethod process-change :mod-media
[data {:keys [object]}]
[data {:keys [object]} _]
(d/update-in-when data [:media (:id object)] merge object))
(defmethod process-change :del-media
[data {:keys [id]}]
[data {:keys [id]} _]
(d/update-when data :media dissoc id))
;; -- Components
(defmethod process-change :add-component
[data params]
[data params _]
(ctkl/add-component data params))
(defmethod process-change :mod-component
[data params]
[data params _]
(ctkl/mod-component data params))
(defmethod process-change :del-component
[data {:keys [id skip-undelete? delta]}]
[data {:keys [id skip-undelete? delta]} _]
(ctf/delete-component data id skip-undelete? delta))
(defmethod process-change :restore-component
[data {:keys [id page-id]}]
[data {:keys [id page-id]} _]
(ctf/restore-component data id page-id))
(defmethod process-change :purge-component
[data {:keys [id]}]
[data {:keys [id]} _]
(ctf/purge-component data id))
;; -- Typography
(defmethod process-change :add-typography
[data {:keys [typography]}]
[data {:keys [typography]} _]
(ctyl/add-typography data typography))
(defmethod process-change :mod-typography
[data {:keys [typography]}]
[data {:keys [typography]} _]
(ctyl/update-typography data (:id typography) merge typography))
(defmethod process-change :del-typography
[data {:keys [id]}]
[data {:keys [id]} _]
(ctyl/delete-typography data id))
;; -- Design Tokens
(defmethod process-change :set-tokens-lib
[data {:keys [tokens-lib]}]
[data {:keys [tokens-lib]} _]
(assoc data :tokens-lib tokens-lib))
(defmethod process-change :set-token
[data {:keys [set-id token-id attrs]}]
[data {:keys [set-id token-id attrs]} _]
(-> (cfo/ensure-tokens-lib data)
(cfo/update-tokens-lib
(fn [lib']
@ -1030,7 +1041,7 @@
(ctob/make-token (merge prev-token attrs)))))))))
(defmethod process-change :set-token-set
[data {:keys [id attrs]}]
[data {:keys [id attrs]} _]
(-> (cfo/ensure-tokens-lib data)
(cfo/update-tokens-lib
(fn [lib']
@ -1045,7 +1056,7 @@
(ctob/update-set lib' id (fn [_] (ctob/make-token-set attrs))))))))
(defmethod process-change :set-token-theme
[data {:keys [id attrs]}]
[data {:keys [id attrs]} _]
(-> (cfo/ensure-tokens-lib data)
(cfo/update-tokens-lib
(fn [lib']
@ -1063,35 +1074,68 @@
(ctob/make-token-theme (merge prev-token-theme attrs)))))))))
(defmethod process-change :set-tokens-status
[data {:keys [theme-ids set-ids]}]
[data {:keys [theme-ids set-ids]} _]
(-> (cfo/ensure-tokens-lib data)
(cfo/update-tokens-status ctos/set-tokens-status theme-ids set-ids)))
(defmethod process-change :rename-token-set-group
[data {:keys [set-group-path set-group-fname]}]
[data {:keys [set-group-path set-group-fname]} _]
(-> (cfo/ensure-tokens-lib data)
(cfo/update-tokens-lib ctob/rename-set-group set-group-path set-group-fname)))
(defmethod process-change :move-token-set
[data {:keys [from-path to-path before-path before-group] :as changes}]
[data {:keys [from-path to-path before-path before-group] :as changes} _]
(-> (cfo/ensure-tokens-lib data)
(cfo/update-tokens-lib ctob/move-set from-path to-path before-path before-group)))
(defmethod process-change :move-token-set-group
[data {:keys [from-path to-path before-path before-group]}]
[data {:keys [from-path to-path before-path before-group]} _]
(-> (cfo/ensure-tokens-lib data)
(cfo/update-tokens-lib ctob/move-set-group from-path to-path before-path before-group)))
;; === Design Tokens configuration
;; --- Design Tokens configuration
(defmethod process-change :set-base-font-size
[data {:keys [base-font-size]}]
[data {:keys [base-font-size]} _]
(ctf/set-base-font-size data base-font-size))
(defmethod process-change :set-tokens-source
[data {:keys [library-id]}]
[data {:keys [library-id]} _]
(cfo/set-tokens-source data library-id))
;; --- Validate Shapes
#?(:clj
(defmethod process-change :validate-shapes
[data _ _]
data))
#?(:cljs
(defmethod process-change :validate-shapes
[data {:keys [page-id shape-ids context]} libraries]
(if libraries
(println "Validating shapes: \n"
" page-id:" (str page-id) "\n"
" shape-ids:" (str shape-ids) "\n"
" context:" context)
(let [file {:data data :id uuid/zero}
errors (reduce (fn [acc shape-id]
(if-let [page (ctpl/get-page data page-id)]
(let [page-errors (val/validate-shape shape-id file page libraries)]
(if (seq page-errors)
(into acc page-errors)
acc))
acc))
[]
shape-ids)]
(when (seq errors)
(ex/raise :type :validation
:code :referential-integrity
:hint (str "error on validating shapes: " context)
:details errors))
data))
data))
;; === Operations
(def decode-shape-attrs

View File

@ -1230,7 +1230,6 @@
[changes]
(::page-id (meta changes)))
(defn set-text-content
[changes id content prev-content]
(assert-page-id! changes)
@ -1251,3 +1250,12 @@
(-> changes
(update :redo-changes conj redo-change)
(update :undo-changes conj undo-change))))
;; Validate Shapes
(defn validate-shapes
[changes page-id shape-ids context]
(update changes :redo-changes conj {:type :validate-shapes
:page-id page-id
:shape-ids (vec shape-ids)
:context context}))

View File

@ -290,8 +290,11 @@
duplicated-parent?
(->> ids-map vals (some #(= % (:parent-id first-shape))))
grid-parent?
(and (ctsl/grid-layout? objects (:parent-id first-shape)) (not duplicated-parent?))
changes
(if (and (ctsl/grid-layout? objects (:parent-id first-shape)) (not duplicated-parent?))
(if grid-parent?
(let [target-cell (-> position meta :cell)
[row column]
@ -313,7 +316,19 @@
changes
(reduce #(pcb/add-object %1 %2 {:ignore-touched true})
changes
(rest new-shapes))]
(rest new-shapes))
ids-to-validate (cond-> [(:id first-shape)]
grid-parent?
(conj (:parent-id first-shape)))
changes (if (seq ids-to-validate)
(pcb/validate-shapes changes
(:id page)
ids-to-validate
(str "generate-instantiate-component: " component-id
" under parent-id" (or parent-id " root")))
changes)]
[new-shape changes])))
@ -3127,7 +3142,6 @@
;; we calculate a new one because the components will have created new shapes.
ids-map (into {} (map #(vector % (uuid/next))) all-ids)
;; If there is an alt-duplication we change to root
;; For variants so the copy is made as a child of root
;; This is because inside a variant-container can't be a copy
@ -3139,7 +3153,6 @@
(assoc :parent-id uuid/zero :frame-id uuid/zero)))
shapes)
changes (-> changes
(pcb/with-page page)
(pcb/with-objects all-objects)
@ -3169,7 +3182,20 @@
(comp
(filter #(= :add-obj (:type %)))
(map #(vector (:old-id %) (-> % :obj :id))))
(:redo-changes changes))]
(:redo-changes changes))
copied-components
(ctn/get-all-instance-roots (:objects page) ids)
ids-to-validate
(map #(get ids-map % %) copied-components)
changes (if (seq ids-to-validate)
(pcb/validate-shapes changes
(:id page)
ids-to-validate
(cond-> (str "generate-duplicate-changes: " ids)))
changes)]
(-> changes
(generate-duplicate-flows shapes page ids-map)

View File

@ -84,7 +84,7 @@
(reduce check-shape changes mod-obj-changes)))
(defn generate-update-shapes
[changes ids update-fn objects {:keys [attrs changed-sub-attr changed-item-index ignore-tree ignore-touched with-objects? translation? skip-grid-reassignment?]}]
[changes ids update-fn objects {:keys [attrs changed-sub-attr changed-item-index ignore-tree ignore-touched with-objects? translation? skip-grid-reassignment? extra-context]}]
(let [changes (reduce
(fn [changes id]
(let [opts {:attrs attrs
@ -105,7 +105,19 @@
(pcb/reorder-grid-children ids))
(not ignore-touched)
(generate-unapply-tokens objects changed-sub-attr changed-item-index))]
(generate-unapply-tokens objects changed-sub-attr changed-item-index))
page-id (pcb/get-page-id changes)
modified-components (ctn/get-all-instance-roots objects ids)
changes (if (and page-id (seq modified-components))
(pcb/validate-shapes changes
page-id
modified-components
(cond-> (str "generate-update-shapes: " ids " " attrs)
(some? extra-context)
(str " \n -> from " extra-context)))
changes)]
changes))
(defn- generate-update-shape-flags
@ -257,8 +269,8 @@
page-id (pcb/get-page-id changes)
page (or (pcb/get-page changes)
(ctpl/get-page data page-id))
ids (cfh/clean-loops objects ids)
in-component-copy?
(fn [shape-id]
;; Look for shapes that are inside a component copy, but are
@ -267,7 +279,7 @@
;; If we want to specifically allow altering the copies, this is
;; a special case, like a component swap, in which case we want
;; to delete the old shape
(let [shape (get objects shape-id)]
(let [shape (get objects shape-id)]
(and (ctn/has-any-copy-parent? objects shape)
(not allow-altering-copies))))
@ -446,7 +458,19 @@
(into []
(remove #(and (ctsi/has-destination %)
(id-to-delete? (:destination %))))
interactions))))))]
interactions))))))
modified-components (ctn/get-all-instance-roots objects (disj all-parents uuid/zero))
;; There is no need to validate deleted objects. Probably also no need to validate hidden or unmasked objects,
;; but we may think of it
changes (if (seq modified-components)
(pcb/validate-shapes changes
page-id
modified-components
(str "generate-delete-shapes: " ids))
changes)]
[all-parents changes])))

View File

@ -216,6 +216,41 @@
:else
(get-instance-root objects (get objects (:parent-id shape)))))
(defn get-all-instance-roots
"Given a list of shape ids and an objects tree, returns a set with the ids of
all instance roots that are at, above or below any of the shapes identified by
the given list. An instance root is a shape that has :component-root set to
true (checked by ctk/instance-root?). There is at most one instance root in
any subtree rooted at an instance root, so the downward search stops at the
first instance root found in each branch. Uses a visited set to avoid
reprocessing the same shapes."
[objects shape-ids]
(let [visited (atom #{})
result (atom #{})]
(letfn [(search-up [shape-id]
(when-not (contains? @visited shape-id)
(swap! visited conj shape-id)
(let [shape (get objects shape-id)]
(when-not (nil? shape)
(if (ctk/instance-root? shape)
(swap! result conj (:id shape))
(when-not (cfh/root? shape)
(when-let [parent-id (:parent-id shape)]
(search-up parent-id))))))))
(search-down [shape-id]
(when-not (contains? @visited shape-id)
(swap! visited conj shape-id)
(let [shape (get objects shape-id)]
(when-not (nil? shape)
(if (ctk/instance-root? shape)
(swap! result conj (:id shape))
(doseq [child-id (:shapes shape)]
(search-down child-id)))))))]
(doseq [shape-id shape-ids]
(search-up shape-id)
(search-down shape-id))
@result)))
(defn find-component-main
"If the shape is a component main instance or is inside one, return that instance.
Uses an iterative loop with cycle detection to prevent stack overflow on circular

View File

@ -103,12 +103,14 @@
(wasm.api/process-objects shapes)
(wasm.api/request-render "sync-wasm-structural-changes"))))))
(defn- apply-changes-localy
(defn- apply-changes-locally
[{:keys [file-id redo-changes ignore-wasm?] :as commit} pending]
(ptk/reify ::apply-changes-localy
(ptk/reify ::apply-changes-locally
ptk/UpdateEvent
(update [_ state]
(let [undo-changes
(let [libraries (dsh/lookup-libraries state)
undo-changes
(if pending
(->> pending
(map :undo-changes)
@ -126,8 +128,8 @@
apply-changes
(fn [fdata]
(let [fdata (cpc/process-changes fdata undo-changes false)
fdata (cpc/process-changes fdata redo-changes false)
(let [fdata (cpc/process-changes fdata undo-changes false libraries)
fdata (cpc/process-changes fdata redo-changes false libraries)
pids (into #{} xf:map-page-id redo-changes)]
(reduce #(ctst/update-object-indices %1 %2) fdata pids)))]
@ -200,7 +202,7 @@
(let [pending (when-not local?
(get-pending-commits state))]
(rx/concat
(rx/of (apply-changes-localy commit pending))
(rx/of (apply-changes-locally commit pending))
(if pending
(rx/concat
(->> (rx/from (reverse pending))

View File

@ -193,7 +193,8 @@
layout-initializer (get-layout-initializer type from-frame? calculate-params?)]
(rx/of (dwu/start-undo-transaction undo-id)
(dwsh/update-shapes [id] layout-initializer {:with-objects? true})
(dwsh/update-shapes [id] layout-initializer {:with-objects? true
:extra-context (str "create-layout-from-id: " id)})
(dwsh/update-shapes (dm/get-prop parent :shapes) #(dissoc % :constraints-h :constraints-v))
(ptk/data-event :layout/update {:ids [id]})
(dwu/commit-undo-transaction undo-id))))))

View File

@ -191,7 +191,7 @@
{:as props
:keys [reg-objects? save-undo? stack-undo? attrs ignore-tree page-id
ignore-touched undo-group with-objects? changed-sub-attr changed-item-index translation?
skip-grid-reassignment? skip-component-sync?]
skip-grid-reassignment? skip-component-sync? extra-context]
:or {reg-objects? false
save-undo? true
stack-undo? false
@ -226,7 +226,8 @@
:ignore-touched ignore-touched
:with-objects? with-objects?
:translation? translation?
:skip-grid-reassignment? skip-grid-reassignment?})
:skip-grid-reassignment? skip-grid-reassignment?
:extra-context extra-context})
(cond-> undo-group
(pcb/set-undo-group undo-group))
(pcb/set-translation? translation?)