diff --git a/common/src/app/common/files/changes.cljc b/common/src/app/common/files/changes.cljc index ce45edd485..e99dbe5c02 100644 --- a/common/src/app/common/files/changes.cljc +++ b/common/src/app/common/files/changes.cljc @@ -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 diff --git a/common/src/app/common/files/changes_builder.cljc b/common/src/app/common/files/changes_builder.cljc index 1e71d05b4d..06924ab4c7 100644 --- a/common/src/app/common/files/changes_builder.cljc +++ b/common/src/app/common/files/changes_builder.cljc @@ -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})) diff --git a/common/src/app/common/logic/libraries.cljc b/common/src/app/common/logic/libraries.cljc index fc3c4d00de..aca26744e5 100644 --- a/common/src/app/common/logic/libraries.cljc +++ b/common/src/app/common/logic/libraries.cljc @@ -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) diff --git a/common/src/app/common/logic/shapes.cljc b/common/src/app/common/logic/shapes.cljc index 30d6067889..020f92d0a8 100644 --- a/common/src/app/common/logic/shapes.cljc +++ b/common/src/app/common/logic/shapes.cljc @@ -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]))) diff --git a/common/src/app/common/types/container.cljc b/common/src/app/common/types/container.cljc index cc7ea9bd07..3978a804de 100644 --- a/common/src/app/common/types/container.cljc +++ b/common/src/app/common/types/container.cljc @@ -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 diff --git a/frontend/src/app/main/data/changes.cljs b/frontend/src/app/main/data/changes.cljs index 865779e5dc..48edebd9fd 100644 --- a/frontend/src/app/main/data/changes.cljs +++ b/frontend/src/app/main/data/changes.cljs @@ -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)) diff --git a/frontend/src/app/main/data/workspace/shape_layout.cljs b/frontend/src/app/main/data/workspace/shape_layout.cljs index df614dfe7a..dee6bb19e3 100644 --- a/frontend/src/app/main/data/workspace/shape_layout.cljs +++ b/frontend/src/app/main/data/workspace/shape_layout.cljs @@ -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)))))) diff --git a/frontend/src/app/main/data/workspace/shapes.cljs b/frontend/src/app/main/data/workspace/shapes.cljs index 0a170541cc..a39d019834 100644 --- a/frontend/src/app/main/data/workspace/shapes.cljs +++ b/frontend/src/app/main/data/workspace/shapes.cljs @@ -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?)