diff --git a/common/src/app/common/files/repair.cljc b/common/src/app/common/files/repair.cljc index 6714bfce04..95f3fda22f 100644 --- a/common/src/app/common/files/repair.cljc +++ b/common/src/app/common/files/repair.cljc @@ -10,12 +10,14 @@ [app.common.files.changes-builder :as pcb] [app.common.files.helpers :as cfh] [app.common.logging :as log] + [app.common.path-names :as cpn] [app.common.types.component :as ctk] [app.common.types.components-list :as ctkl] [app.common.types.container :as ctn] [app.common.types.file :as ctf] [app.common.types.pages-list :as ctpl] [app.common.types.shape :as cts] + [app.common.types.variant :as ctv] [app.common.uuid :as uuid])) (log/set-level! :debug) @@ -35,7 +37,7 @@ (assoc :width 0.01) (assoc :height 0.01) (cts/setup-rect)))] - (log/dbg :hint "repairing shape :invalid-geometry" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :invalid-geometry" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -48,7 +50,7 @@ (log/debug :hint " -> set to " :parent-id uuid/zero) (assoc shape :parent-id uuid/zero))] - (log/dbg :hint "repairing shape :parent-not-found" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :parent-not-found" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -61,7 +63,7 @@ (log/debug :hint " -> add children to" :parent-id (:id parent-shape)) (update parent-shape :shapes conj (:id shape)))] - (log/dbg :hint "repairing shape :child-not-in-parent" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :child-not-in-parent" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:parent-id shape)] repair-shape)))) @@ -74,7 +76,7 @@ (log/debug :hint " -> remove duplicated children") (update shape :shapes distinct))] - (log/dbg :hint "repairing shape :duplicated-children" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :duplicated-children" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -86,14 +88,14 @@ (log/debug :hint " -> remove child" :child-id (:child-id args)) (update parent-shape :shapes (fn [shapes] (d/removev #(= (:child-id args) %) shapes))))] - (log/dbg :hint "repairing shape :child-not-found" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :child-not-found" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) (defmethod repair-error :invalid-parent [_ {:keys [shape page-id args] :as error} file-data _] - (log/dbg :hint "repairing shape :invalid-parent" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :invalid-parent" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/change-parent (:parent-id args) [shape] nil {:allow-altering-copies true}))) @@ -109,7 +111,7 @@ (log/debug :hint " -> set to " :frame-id frame-id) (assoc shape :frame-id frame-id)))] - (log/dbg :hint "repairing shape :frame-not-found" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :frame-not-found" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -125,7 +127,7 @@ (log/debug :hint " -> set to " :frame-id frame-id) (assoc shape :frame-id frame-id)))] - (log/dbg :hint "repairing shape :invalid-frame" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :invalid-frame" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -138,7 +140,7 @@ (log/debug :hint " -> set :main-instance") (assoc shape :main-instance true))] - (log/dbg :hint "repairing shape :component-not-main" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :component-not-main" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -155,7 +157,7 @@ ;; (log/warn :hint " -> CANNOT REPAIR THIS AUTOMATICALLY.") ;; shape)] - (log/dbg :hint "repairing shape :component-main-external" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :component-main-external" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -174,7 +176,7 @@ ;; (log/warn :hint " -> CANNOT REPAIR THIS AUTOMATICALLY.") ;; shape)] - (log/dbg :hint "repairing shape :component-not-found" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :component-not-found" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes shape-ids repair-shape)))) @@ -194,7 +196,7 @@ (log/debug :hint " -> detach shape" :shape-id (:id shape)) (ctk/detach-shape shape))] - (log/dbg :hint "repairing shape :invalid-main-instance-id" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :invalid-main-instance-id" :id (:id shape) :name (:name shape) :page-id page-id) (if (and (some? component) (not (:deleted component))) (-> (pcb/empty-changes nil page-id) (pcb/with-library-data file-data) @@ -211,7 +213,7 @@ ;; Assign main instance in the component to current shape (log/debug :hint " -> assign main-instance-page" :component-id (:id component)) (assoc component :main-instance-page page-id))] - (log/dbg :hint "repairing shape :invalid-main-instance-page" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :invalid-main-instance-page" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-library-data file-data) (pcb/update-component (:component-id shape) repair-component)))) @@ -224,7 +226,7 @@ (log/warn :hint " -> CANNOT REPAIR THIS AUTOMATICALLY.") shape)] - (log/dbg :hint "repairing shape :invalid-main-instance" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :invalid-main-instance" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -237,7 +239,7 @@ (log/debug :hint " -> unset :main-instance") (dissoc shape :main-instance))] - (log/dbg :hint "repairing shape :component-main" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :component-main" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -250,7 +252,7 @@ (log/debug :hint " -> set :component-root") (assoc shape :component-root true))] - (log/dbg :hint "repairing shape :should-be-component-root" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :should-be-component-root" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -263,7 +265,7 @@ (log/debug :hint " -> unset :component-root") (dissoc shape :component-root))] - (log/dbg :hint "repairing shape :should-not-be-component-root" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :should-not-be-component-root" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -310,7 +312,7 @@ ;; If the shape still refers to the remote component, try to find the corresponding near one ;; and link to it. If not, detach the shape. - (log/dbg :hint "repairing shape :ref-shape-not-found" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :ref-shape-not-found" :id (:id shape) :name (:name shape) :page-id page-id) (if (some? matching-shape) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) @@ -329,7 +331,7 @@ (log/debug :hint " -> unhead shape") (ctk/unhead-shape shape))] - (log/dbg :hint "repairing shape :shape-ref-is-not-head" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :shape-ref-is-not-head" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -354,7 +356,7 @@ (nil? (:component-file args)) (dissoc :component-file)))] - (log/dbg :hint "repairing shape :component-id-mismatch" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :component-id-mismatch" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -367,7 +369,7 @@ (log/debug :hint " -> reroot shape") (ctk/rehead-shape shape (:component-file args) (:component-id args)))] - (log/dbg :hint "repairing shape :shape-ref-is-head" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :shape-ref-is-head" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -396,7 +398,7 @@ (assoc acc k v))) {} objects)))))] - (log/dbg :hint "repairing component :shape-ref-cycle" :id (:id shape) :name (:name shape)) + (log/debug :hint "repairing component :shape-ref-cycle" :id (:id shape) :name (:name shape)) (-> (pcb/empty-changes nil nil) (pcb/with-library-data file-data) (pcb/update-component (:id shape) repair-component)))) @@ -409,7 +411,7 @@ (log/debug :hint " -> unset :shape-ref") (dissoc shape :shape-ref))] - (log/dbg :hint "repairing shape :shape-ref-in-main" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :shape-ref-in-main" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -422,7 +424,7 @@ (log/debug :hint " -> unset :component-root") (dissoc shape :component-root))] - (log/dbg :hint "repairing shape :root-main-not-allowed" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :root-main-not-allowed" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -435,7 +437,7 @@ (log/debug :hint " -> set :component-root") (assoc shape :component-root true))] - (log/dbg :hint "repairing shape :nested-main-not-allowed" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :nested-main-not-allowed" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape) @@ -449,7 +451,7 @@ (log/debug :hint " -> unset :component-root") (dissoc shape :component-root))] - (log/dbg :hint "repairing shape :root-copy-not-allowed" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :root-copy-not-allowed" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -462,7 +464,7 @@ (log/debug :hint " -> set :component-root") (assoc shape :component-root true))] - (log/dbg :hint "repairing shape :nested-copy-not-allowed" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :nested-copy-not-allowed" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -475,7 +477,7 @@ (log/debug :hint " -> detach shape" :shape-id (:id shape)) (ctk/detach-shape shape))] - (log/dbg :hint "repairing shape :not-head-main-not-allowed" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :not-head-main-not-allowed" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -488,7 +490,7 @@ (log/debug :hint " -> detach shape" :shape-id (:id shape)) (ctk/detach-shape shape))] - (log/dbg :hint "repairing shape :not-head-copy-not-allowed" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :not-head-copy-not-allowed" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -501,7 +503,7 @@ (log/warn :hint " -> CANNOT REPAIR THIS AUTOMATICALLY.") shape)] - (log/dbg :hint "repairing shape :not-component-not-allowed" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :not-component-not-allowed" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -520,7 +522,7 @@ :r3 0 :r4 0))] - (log/dbg :hint "repairing shape :instance-head-not-frame" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :instance-head-not-frame" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -538,7 +540,7 @@ (log/debug :hint " -> remove :objects") (dissoc component :objects))))] - (log/dbg :hint "repairing component :component-nil-objects-not-allowed" :id (:id component) :name (:name component)) + (log/debug :hint "repairing component :component-nil-objects-not-allowed" :id (:id component) :name (:name component)) (-> (pcb/empty-changes nil) (pcb/with-library-data file-data) (pcb/update-component (:id component) repair-component)))) @@ -554,7 +556,7 @@ (dissoc component :objects)) component))] - (log/dbg :hint "repairing component :non-deleted-component-cannot-have-objects" :id (:id component) :name (:name component)) + (log/debug :hint "repairing component :non-deleted-component-cannot-have-objects" :id (:id component) :name (:name component)) (-> (pcb/empty-changes nil) (pcb/with-library-data file-data) (pcb/update-component (:id component) repair-component)))) @@ -567,7 +569,7 @@ (log/debug :hint " -> add :content-group to :touched-groups") (update shape :touched ctk/set-touched-group :content-group))] - (log/dbg :hint "repairing shape :invalid-text-touched" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :invalid-text-touched" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -580,7 +582,7 @@ (log/debug :hint " -> remove swap-slot") (ctk/remove-swap-slot shape))] - (log/dbg :hint "repairing shape :misplaced-slot" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :misplaced-slot" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) @@ -603,13 +605,11 @@ (log/debug :hint " -> remove swap-slot" :child-id (:id shape)) (ctk/remove-swap-slot shape))] - (log/dbg :hint "repairing shape :duplicated-slot" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :duplicated-slot" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes (map :id child-with-duplicate) repair-shape)))) - - (defmethod repair-error :component-duplicate-slot [_ {:keys [shape] :as error} file-data _] (let [main-shape (get-in shape [:objects (:main-instance-id shape)]) @@ -633,7 +633,7 @@ (:objects component))] (assoc component :objects objects)))] - (log/dbg :hint "repairing component :component-duplicated-slot" :id (:id shape) :name (:name shape)) + (log/debug :hint "repairing component :component-duplicated-slot" :id (:id shape) :name (:name shape)) (-> (pcb/empty-changes nil) (pcb/with-library-data file-data) (pcb/update-component (:id shape) repair-component)))) @@ -649,50 +649,139 @@ (ctk/set-swap-slot shape slot)) shape)))] - (log/dbg :hint "repairing shape :missing-slot" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint "repairing shape :missing-slot" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) -(defmethod repair-error :not-a-variant - [_ error file _] - (log/error :hint "Variant error code, we don't want to auto repair it for now" :code (:code error)) - file) +(defmethod repair-error :main-instance-not-a-variant + [_ {:keys [shape page-id args]} file-data _] + (let [repair-shape + (fn [shape] + (let [variant-id (:variant-id args)] + ;; Set the desired variant-id + (log/debug :hint (str " -> set variant-id to " variant-id)) + (assoc shape :variant-id variant-id)))] -(defmethod repair-error :invalid-variant-id - [_ error file _] - (log/error :hint "Variant error code, we don't want to auto repair it for now" :code (:code error)) - file) + (log/debug :hint "repairing shape :main-instance-not-a-variant" :id (:id shape) :name (:name shape) :page-id page-id) + (-> (pcb/empty-changes nil page-id) + (pcb/with-file-data file-data) + (pcb/update-shapes [(:id shape)] repair-shape)))) + +(defmethod repair-error :main-instance-invalid-variant-id + [_ {:keys [shape page-id args]} file-data _] + (let [repair-shape + (fn [shape] + (let [variant-id (:variant-id args)] + ;; Set the desired variant-id + (log/debug :hint (str " -> set variant-id to " variant-id)) + (assoc shape + :variant-id variant-id)))] + + (log/debug :hint "repairing shape :main-instance-invalid-variant-id" :id (:id shape) :name (:name shape) :page-id page-id) + (-> (pcb/empty-changes nil page-id) + (pcb/with-file-data file-data) + (pcb/update-shapes [(:id shape)] repair-shape)))) (defmethod repair-error :invalid-variant-properties - [_ error file _] - (log/error :hint "Variant error code, we don't want to auto repair it for now" :code (:code error)) - file) + [_ {:keys [shape page-id args]} file-data _] + (let [prop-names (:prop-names args) + + component (get-in file-data [:components (:component-id shape)]) + prop-values (into {} (map (juxt :name :value)) (:variant-properties component)) + properties' (mapv (fn [name] {:name name :value (get prop-values name "")}) prop-names) + variant-name (ctv/properties-to-name properties') + + repair-component + (fn [component] + ;; Rebuild component properties, removing any extra ones and adding missing ones with empty value + (log/debug :hint " -> rebuild properties" :component-id (:id component) :prop-names (str prop-names)) + (assoc component :variant-properties properties')) + + repair-shape + (fn [shape] + (log/debug :hint " -> set variant-name" :variant-name variant-name) + (assoc shape :variant-name variant-name))] + + (log/debug :hint "repairing shape :invalid-variant-properties" :id (:id shape) :name (:name shape) :page-id page-id) + (-> (pcb/empty-changes nil page-id) + (pcb/with-file-data file-data) + (pcb/with-library-data file-data) + (pcb/update-component (:component-id shape) repair-component) + (pcb/update-shapes [(:id shape)] repair-shape)))) (defmethod repair-error :variant-not-main - [_ error file _] - (log/error :hint "Variant error code, we don't want to auto repair it for now" :code (:code error)) - file) + [_ {:keys [shape page-id]} file-data _] + (let [page (ctpl/get-page file-data page-id) + shape-ids (cfh/get-children-ids-with-self (:objects page) (:id shape))] + (log/debug :hint "repairing shape :variant-not-main" :id (:id shape) :name (:name shape) :page-id page-id) + (log/debug :hint " -> delete shapes" :shape-ids shape-ids) + (-> (pcb/empty-changes nil page-id) + (pcb/with-file-data file-data) + (pcb/remove-objects shape-ids)))) (defmethod repair-error :parent-not-variant - [_ error file _] - (log/error :hint "Variant error code, we don't want to auto repair it for now" :code (:code error)) - file) + [_ {:keys [shape page-id]} file-data _] + (let [parent-id (:parent-id shape) + repair-fn + (fn [parent] + (log/debug :hint " -> set :is-variant-container true") + (assoc parent :is-variant-container true))] + (log/debug :hint "repairing shape :parent-not-variant" :id (:id shape) :name (:name shape) :parent-id parent-id :page-id page-id) + (-> (pcb/empty-changes nil page-id) + (pcb/with-file-data file-data) + (pcb/update-shapes [parent-id] repair-fn)))) -(defmethod repair-error :variant-bad-name - [_ error file _] - (log/error :hint "Variant error code, we don't want to auto repair it for now" :code (:code error)) - file) +(defmethod repair-error :variant-main-bad-name + [_ {:keys [shape page-id args]} file-data _] + (let [repair-fn + (fn [shape] + (log/debug :hint " -> set :name" :name (:variant-name args)) + (assoc shape :name (:variant-name args)))] + (log/debug :hint "repairing shape :variant-main-bad-name" :id (:id shape) :name (:name shape) :page-id page-id) + (-> (pcb/empty-changes nil page-id) + (pcb/with-file-data file-data) + (pcb/update-shapes [(:id shape)] repair-fn)))) -(defmethod repair-error :variant-bad-variant-name - [_ error file _] - (log/error :hint "Variant error code, we don't want to auto repair it for now" :code (:code error)) - file) +(defmethod repair-error :variant-main-bad-variant-name + [_ {:keys [shape page-id]} file-data _] + (let [component (get-in file-data [:components (:component-id shape)]) + variant-name (ctv/properties-to-name (:variant-properties component)) + repair-fn + (fn [shape] + (log/debug :hint " -> set :variant-name" :variant-name variant-name) + (assoc shape :variant-name variant-name))] + (log/dbg :hint "repairing shape :variant-main-bad-variant-name" :id (:id shape) :name (:name shape) :page-id page-id) + (-> (pcb/empty-changes nil page-id) + (pcb/with-file-data file-data) + (pcb/update-shapes [(:id shape)] repair-fn)))) (defmethod repair-error :variant-component-bad-name - [_ error file _] - (log/error :hint "Variant error code, we don't want to auto repair it for now" :code (:code error)) - file) + [_ {:keys [shape page-id args]} file-data _] + (let [[path name] (cpn/split-group-name (:variant-container-name args)) + repair-fn + (fn [component] + (log/debug :hint " -> set :path and :name" :path path :name name) + (assoc component :path path :name name))] + (log/dbg :hint "repairing shape :variant-component-bad-name" :id (:id shape) :name (:name shape) :page-id page-id) + (-> (pcb/empty-changes nil page-id) + (pcb/with-library-data file-data) + (pcb/update-component (:component-id shape) repair-fn)))) + +(defmethod repair-error :variant-component-bad-id + [_ {:keys [shape page-id args]} file-data _] + (let [repair-shape + (fn [shape] + (let [variant-id (:variant-id args)] + ;; Set the desired variant-id + (log/debug :hint (str " -> set variant-id to " variant-id)) + (assoc shape + :variant-id variant-id)))] + + (log/debug :hint "repairing shape :variant-component-bad-id" :id (:id shape) :name (:name shape) :page-id page-id) + (-> (pcb/empty-changes nil page-id) + (pcb/with-file-data file-data) + (pcb/update-shapes [(:id shape)] repair-shape)))) (defmethod repair-error :default [_ error file _] @@ -701,7 +790,7 @@ (defn repair-file [{:keys [data id] :as file} libraries errors] - (log/dbg :hint "repairing file" :id (str id) :errors (count errors)) + (log/debug :hint "repairing file" :id (str id) :errors (count errors)) (let [{:keys [redo-changes]} (reduce (fn [changes error] (pcb/concat-changes changes diff --git a/common/src/app/common/files/validate.cljc b/common/src/app/common/files/validate.cljc index 82aeed3d9e..9cb2a5e060 100644 --- a/common/src/app/common/files/validate.cljc +++ b/common/src/app/common/files/validate.cljc @@ -65,13 +65,13 @@ :misplaced-slot :missing-slot :shape-ref-cycle - :not-a-variant - :invalid-variant-id + :main-instance-not-a-variant + :main-instance-invalid-variant-id :invalid-variant-properties :variant-not-main :parent-not-variant - :variant-bad-name - :variant-bad-variant-name + :variant-main-bad-name + :variant-main-bad-variant-name :variant-component-bad-name :variant-component-bad-id}) @@ -573,19 +573,23 @@ (run! (fn [child-id] (when-let [child (get objects child-id)] (if (not (ctk/is-variant? child)) - (report-error :not-a-variant - (str/ffmt "Shape % should be a variant" (:id child)) - child file page) + (report-error :main-instance-not-a-variant + (str/ffmt "Main instance shape % should be a variant" (:id child)) + child file page + :variant-id shape-id) (do (when (not= (:variant-id child) shape-id) - (report-error :invalid-variant-id - (str/ffmt "Variant % has invalid variant-id %" (:id child) (:variant-id child)) - child file page)) + (report-error :main-instance-invalid-variant-id + (str/ffmt "Main instance in variant % should have the variant-id of the container but has %" (:id child) (:variant-id child)) + child file page + :variant-id shape-id)) (when (not= prop-names (cfv/extract-properties-names child file-data)) (report-error :invalid-variant-properties (str/ffmt "Variant % has invalid properties %" (:id child) (vec prop-names)) - child file page)))))) + child file page + :prop-names prop-names)))))) shapes))) + (defn- check-variant "Shape is a variant, so -it should be a main component @@ -594,9 +598,9 @@ -its name should be the same as its parent's " [shape file page] - (let [parent (ctst/get-shape page (:parent-id shape)) - component (ctkl/get-component (:data file) (:component-id shape) true) - name (ctv/properties-to-name (:variant-properties component))] + (let [parent (ctst/get-shape page (:parent-id shape)) + component (ctkl/get-component (:data file) (:component-id shape) true) + variant-name (ctv/properties-to-name (:variant-properties component))] (when-not (ctk/main-instance? shape) (report-error :variant-not-main (str/ffmt "Variant % is not a main instance" (:id shape)) @@ -605,23 +609,26 @@ (report-error :parent-not-variant (str/ffmt "Variant % has an invalid parent" (:id shape)) shape file page)) - - (when-not (= name (:variant-name shape)) - (report-error :variant-bad-variant-name + (when-not (= variant-name (:variant-name shape)) + (report-error :variant-main-bad-variant-name (str/ffmt "Variant % has an invalid variant-name" (:id shape)) - shape file page)) + shape file page + :variant-name variant-name)) (when-not (= (:name parent) (:name shape)) - (report-error :variant-bad-name - (str/ffmt "Variant % has an invalid name" (:id shape)) - shape file page)) + (report-error :variant-main-bad-name + (str/ffmt "Main instance inside variant % has an invalid name" (:id shape)) + shape file page + :variant-name (:name parent))) (when-not (= (:name parent) (cpn/merge-path-item (:path component) (:name component))) (report-error :variant-component-bad-name (str/ffmt "Component % has an invalid name" (:id shape)) - shape file page)) + shape file page + :variant-container-name (:name parent))) (when-not (= (:variant-id component) (:variant-id shape)) (report-error :variant-component-bad-id (str/ffmt "Variant % has adifferent variant-id than its component" (:id shape)) - shape file page)))) + shape file page + :variant-id (:variant-id component))))) (defn- check-shape "Validate referential integrity and semantic coherence of @@ -740,14 +747,15 @@ -It should have at least one variant property" [component file] (let [component-page (ctf/get-component-page (:data file) component) - main-component (if (:deleted component) + main-instance (if (:deleted component) (dm/get-in component [:objects (:main-instance-id component)]) (ctst/get-shape component-page (:main-instance-id component)))] - (when (and main-component - (not (ctk/is-variant? main-component))) - (report-error :not-a-variant - (str/ffmt "Shape % should be a variant" (:id main-component)) - main-component file component-page)))) + (when (and main-instance + (not (ctk/is-variant? main-instance))) + (report-error :main-instance-not-a-variant + (str/ffmt "Main instance shape % should be a variant" (:id main-instance)) + main-instance file component-page + :variant-id (:variant-id component))))) (defn- check-main-inside-main [component file] diff --git a/common/src/app/common/test_helpers/variants.cljc b/common/src/app/common/test_helpers/variants.cljc index bf2e6fc973..07b4a58be7 100644 --- a/common/src/app/common/test_helpers/variants.cljc +++ b/common/src/app/common/test_helpers/variants.cljc @@ -13,6 +13,11 @@ [app.common.types.text :as txt])) (defn add-variant + "Add a variant component to a file with two variants, each with a root shape. + :variant-label [:name Board] + {:root2-label} [:name Board] # [Component :component2-label] + {:root1-label} [:name Board] # [Component :component1-label] + " [file variant-label component1-label root1-label component2-label root2-label & {:keys [variant1-params variant2-params] :or {variant1-params {} variant2-params {}}}] diff --git a/common/src/app/common/types/file.cljc b/common/src/app/common/types/file.cljc index 8c97f4cce0..fe91dca8ad 100644 --- a/common/src/app/common/types/file.cljc +++ b/common/src/app/common/types/file.cljc @@ -912,8 +912,10 @@ (let [shape (get objects shape-id)] (println (str/pad (str (str/repeat " " level) (when (:main-instance shape) "{") + (when (:is-variant-container shape) "{{") (:name shape) (when (:main-instance shape) "}") + (when (:is-variant-container shape) "}}") (when (seq (:touched shape)) "*") (when show-ids (str/format " %s" (:id shape)))) {:length 20 diff --git a/common/test/common_tests/files/repair_test.cljc b/common/test/common_tests/files/repair_test.cljc new file mode 100644 index 0000000000..a5021baf03 --- /dev/null +++ b/common/test/common_tests/files/repair_test.cljc @@ -0,0 +1,230 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC Sucursal en España SL + +(ns common-tests.files.repair-test + "Tests for the validate / repair functions in app.common.files.validate + and app.common.files.repair. + + The tests generate cases of broken files and check that the validation functions + generate accurate errors, and that the repair functions return the file to + a stable state." + (:require + [app.common.files.repair :as cfr] + [app.common.files.validate :as cfv] + [app.common.test-helpers.components :as thc] + [app.common.test-helpers.files :as thf] + [app.common.test-helpers.ids-map :as thi] + [app.common.test-helpers.shapes :as ths] + [app.common.test-helpers.variants :as thv] + [app.common.uuid :as uuid] + [clojure.test :as t])) + +(t/use-fixtures :each thi/test-fixture) + +(t/deftest repair-main-instance-not-a-variant + (t/testing "detect and repair a variant component whose root shape is not a variant" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thv/add-variant :variant1 :component1 :root1 :component2 :root2) + (ths/update-shape :root1 :variant-id nil)) + + errors (cfv/validate-file file {}) + changes (cfr/repair-file file {} errors) + file' (thf/apply-changes file {:redo-changes changes} :validate? false) + errors' (cfv/validate-file file' {}) + + root1' (ths/get-shape file' :root1 :page-label :page1)] + + (t/is (= 2 (count errors))) ;; There are two different checks that detect the same problem + (t/is (= :main-instance-not-a-variant (:code (first errors)))) + + (t/is (nil? errors')) + (t/is (= (thi/id :variant1) (:variant-id root1')))))) + +(t/deftest repair-invalid-variant-id-variant-component-bad-id + (t/testing "detect and repair a variant component whose variant id does not match the container's id" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thv/add-variant :variant1 :component1 :root1 :component2 :root2) + (ths/update-shape :root1 :variant-id (uuid/next))) + + errors (cfv/validate-file file {}) + changes (cfr/repair-file file {} errors) + file' (thf/apply-changes file {:redo-changes changes} :validate? false) + errors' (cfv/validate-file file' {}) + + root1' (ths/get-shape file' :root1 :page-label :page1)] + + (t/is (= 2 (count errors))) ;; There are two different validation that actually check the same problem + (t/is (= :main-instance-invalid-variant-id (:code (first errors)))) + (t/is (= :variant-component-bad-id (:code (second errors)))) + + (t/is (nil? errors')) + (t/is (= (thi/id :variant1) (:variant-id root1')))))) + +(t/deftest repair-invalid-variant-properties + (t/testing "detect and repair a second variant component whose properties do not match the first variant component's properties" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thv/add-variant :variant1 :component1 :root1 :component2 :root2) + ;; Component1 has ["Property 1", "Property 2"], component2 gets ["Property 1", "Property 3"] + ;; This breaks validation: prop-names mismatch (missing "Property 2", extra "Property 3") + (thc/update-component :component1 {:variant-properties [{:name "Property 1" :value "Value1"} + {:name "Property 2" :value "ValueA"}]}) + (thc/update-component :component2 {:variant-properties [{:name "Property 1" :value "Value2"} + {:name "Property 3" :value "ValueB"}]}) + (ths/update-shape :root1 :variant-name "Value1, ValueA") + (ths/update-shape :root2 :variant-name "Value2, ValueB")) + + errors (cfv/validate-file file {}) + changes (cfr/repair-file file {} errors) + file' (thf/apply-changes file {:redo-changes changes} :validate? false) + errors' (cfv/validate-file file' {}) + + comp1' (thc/get-component file' :component1) + comp2' (thc/get-component file' :component2) + root1' (ths/get-shape file' :root1) + root2' (ths/get-shape file' :root2)] + + (t/is (= 1 (count errors))) + (t/is (= :invalid-variant-properties (:code (first errors)))) + + (t/is (nil? errors')) + + ;; After repair, component1's properties are rebuilt to match component2's property names + ;; (the first child in the variant container is root2, so prop-names come from component2) + ;; "Property 1" keeps its value, "Property 3" is added with empty value, "Property 2" is removed + (t/is (= [{:name "Property 1" :value "Value1"} + {:name "Property 3" :value ""}] + (:variant-properties comp1'))) + + (t/is (= "Value1" (:variant-name root1'))) + + ;; Component2 is unchanged (it was the reference for the property names) + (t/is (= [{:name "Property 1" :value "Value2"} + {:name "Property 3" :value "ValueB"}] + (:variant-properties comp2'))) + + (t/is (= "Value2, ValueB" (:variant-name root2')))))) + +(t/deftest repair-variant-not-main + (t/testing "detect and repair a non-main-instance shape inside a variant container" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thv/add-variant :variant1 :component1 :root1 :component2 :root2) + ;; Add a third child to the variant container with :variant-id but NOT a main-instance + (ths/add-sample-shape :bad-shape + :type :frame + :parent-label :variant1 + :variant-id (thi/id :variant1) + :variant-name "") + ;; Add a child to the bad shape (to verify the repair deletes it too) + (ths/add-sample-shape :bad-child + :type :rect + :parent-label :bad-shape)) + + errors (cfv/validate-file file {}) + changes (cfr/repair-file file {} errors) + file' (thf/apply-changes file {:redo-changes changes} :validate? false) + errors' (cfv/validate-file file' {}) + + bad-shape' (ths/get-shape file' :bad-shape) + bad-child' (ths/get-shape file' :bad-child)] + + (t/is (= 4 (count errors))) ;; The bad container also triggers other errors + (t/is (= :invalid-variant-properties (:code (nth errors 0)))) + (t/is (= :variant-not-main (:code (nth errors 1)))) + (t/is (= :variant-component-bad-name (:code (nth errors 2)))) + (t/is (= :variant-component-bad-id (:code (nth errors 3)))) + (t/is (nil? errors')) + + (t/is (nil? bad-shape')) + (t/is (nil? bad-child'))))) + +(t/deftest repair-parent-not-variant + (t/testing "detect and repair a variant shape whose parent is not a variant-container" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thv/add-variant :variant1 :component1 :root1 :component2 :root2) + ;; Break the variant container + (ths/update-shape :variant1 :is-variant-container false)) + + errors (cfv/validate-file file {}) + changes (cfr/repair-file file {} errors) + file' (thf/apply-changes file {:redo-changes changes} :validate? false) + errors' (cfv/validate-file file' {}) + + container' (ths/get-shape file' :variant1)] + + (t/is (= 2 (count errors))) ;; The error is detected twice, once for each child of the variant container + (t/is (= :parent-not-variant (:code (first errors)))) + (t/is (= :parent-not-variant (:code (second errors)))) + (t/is (nil? errors')) + + (t/is (true? (:is-variant-container container')))))) + +(t/deftest repair-variant-main-bad-name + (t/testing "detect and repair a main instance whose name doesn't match the variant container's name" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thv/add-variant :variant1 :component1 :root1 :component2 :root2) + ;; Change root1's name so it doesn't match the container + (ths/update-shape :root1 :name "WrongName")) + + errors (cfv/validate-file file {}) + changes (cfr/repair-file file {} errors) + file' (thf/apply-changes file {:redo-changes changes} :validate? false) + errors' (cfv/validate-file file' {}) + root1' (ths/get-shape file' :root1)] + + (t/is (= 1 (count errors))) + (t/is (= :variant-main-bad-name (:code (first errors)))) + (t/is (nil? errors')) + (t/is (= "Board" (:name root1')))))) + +(t/deftest repair-variant-main-bad-variant-name + (t/testing "detect and repair a variant shape whose :variant-name doesn't match the component's properties" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thv/add-variant :variant1 :component1 :root1 :component2 :root2) + (thc/update-component :component1 {:variant-properties [{:name "Property 1" :value "Value1"} + {:name "Property 2" :value "ValueA"}]}) + (thc/update-component :component2 {:variant-properties [{:name "Property 1" :value "Value2"} + {:name "Property 2" :value "ValueB"}]}) + ;; Change root1's :variant-name to something wrong + (ths/update-shape :root1 :variant-name "WrongVariantName") + (ths/update-shape :root2 :variant-name "Value2, ValueB")) + + errors (cfv/validate-file file {}) + changes (cfr/repair-file file {} errors) + file' (thf/apply-changes file {:redo-changes changes} :validate? false) + errors' (cfv/validate-file file' {}) + + root1' (ths/get-shape file' :root1)] + + (t/is (= 1 (count errors))) + (t/is (= :variant-main-bad-variant-name (:code (first errors)))) + (t/is (nil? errors')) + (t/is (= "Value1, ValueA" (:variant-name root1')))))) + +(t/deftest repair-variant-component-bad-name + (t/testing "detect and repair a variant component whose path/name doesn't match the container name" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thv/add-variant :variant1 :component1 :root1 :component2 :root2) + ;; Update names to have path structure + (ths/update-shape :variant1 :name "Group / Subgroup / Component") + (ths/update-shape :root1 :name "Group / Subgroup / Component") + (ths/update-shape :root2 :name "Group / Subgroup / Component") + ;; Update component paths and names + (thc/update-component :component1 {:path "Group / Subgroup" :name "Component"}) + (thc/update-component :component2 {:path "Group / Subgroup" :name "Component"}) + ;; Break component1's name + (thc/update-component :component1 {:name "WrongName"})) + + errors (cfv/validate-file file {}) + changes (cfr/repair-file file {} errors) + file' (thf/apply-changes file {:redo-changes changes} :validate? false) + errors' (cfv/validate-file file' {}) + comp1' (thc/get-component file' :component1)] + + (t/is (= 1 (count errors))) + (t/is (= :variant-component-bad-name (:code (first errors)))) + (t/is (nil? errors')) + (t/is (= "Group / Subgroup" (:path comp1'))) + (t/is (= "Component" (:name comp1'))))))