From d0ffa6e411f88848052d6216fe98f1eee5a850ff Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Wed, 14 Oct 2026 15:51:33 +0200 Subject: [PATCH] :recycle: Simplify component validation and repair - Most repairs change one shape, or the shape and its subtree, with a function, and each one wrote the same 12 to 15 lines around it: the log line, the empty changes of the page, the file data and the update of the shape. repair-shapes and repair-component hold that, so each method keeps its own name and says only what it changes. The functions several repairs repeated go once: detach-shape, the frame lookup of :frame-not-found and :invalid-frame, the no-op of the repairs that cannot be done, and the variant id of three variant repairs. The log lines now name the code that was repaired, which for :ref-shape-is-not-head and :ref-shape-is-head they misspelled. - Three error codes could never be reported, because each check runs only on shapes that already pass it: :component-not-main (in the check of a main head, reached only when :main-instance is true, and of a copy head, reached only when it is not), :should-be-component-root (reached only when :component-root is true) and :not-component-not-allowed (a main context counts as inside a main). They go, with their repairs and their place in error-codes. The check that a shape inside a component is no head also tested :component-id, which such a shape never has. The branch of check-shape for a shape that is neither a head nor a copy shape is a cond. No behavior change: the repair tests and the suites are unchanged. repair.cljc: 815 -> 534 lines. AI-assisted-by: claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 (1M context) --- common/src/app/common/files/repair.cljc | 845 ++++++------------ common/src/app/common/files/validate.cljc | 64 +- .../files/repair_components_test.cljc | 6 +- 3 files changed, 299 insertions(+), 616 deletions(-) diff --git a/common/src/app/common/files/repair.cljc b/common/src/app/common/files/repair.cljc index 14b8554493..3b0583942f 100644 --- a/common/src/app/common/files/repair.cljc +++ b/common/src/app/common/files/repair.cljc @@ -28,73 +28,80 @@ (defmulti repair-error (fn [code _error _file-data _libraries] code)) +(defn- repair-shapes + "The changes that apply `repair-fn` to the shapes `ids` of the page of + `error` (its shape, by default)." + ([code error file-data repair-fn] + (repair-shapes code error file-data [(:id (:shape error))] repair-fn)) + ([code {:keys [shape page-id]} file-data ids repair-fn] + (log/debug :hint (str "repairing shape " code) :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 ids repair-fn)))) + +(defn- repair-component + "The changes that apply `repair-fn` to the component `component-id`." + [code {:keys [shape page-id]} file-data component-id repair-fn] + (log/debug :hint (str "repairing component " code) :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 repair-fn))) + +(defn- detach-shape + [shape] + (log/debug :hint " -> detach shape" :shape-id (:id shape)) + (ctk/detach-shape shape)) + +(defn- set-frame-id + "Set the frame of `shape` to the first frame among its parents." + [page shape] + (let [frame-id (or (:id (cfh/get-frame (:objects page) (:parent-id shape))) uuid/zero)] + (log/debug :hint " -> set to " :frame-id frame-id) + (assoc shape :frame-id frame-id))) + +(defn- cannot-repair + [shape] + ;; There is no solution that may recover it with confidence + (log/warn :hint " -> CANNOT REPAIR THIS AUTOMATICALLY.") + shape) + (defmethod repair-error :invalid-geometry - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ; Reset geometry to minimal - (log/debug :hint " -> reset geometry") - (-> shape - (assoc :x 0) - (assoc :y 0) - (assoc :width 0.01) - (assoc :height 0.01) - (cts/setup-rect)))] - (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)))) + [code error file-data _] + (repair-shapes code error file-data + (fn [shape] + (log/debug :hint " -> reset geometry") + (-> shape + (assoc :x 0 :y 0 :width 0.01 :height 0.01) + (cts/setup-rect))))) (defmethod repair-error :parent-not-found - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Set parent to root frame. - (log/debug :hint " -> set to " :parent-id uuid/zero) - (assoc shape :parent-id uuid/zero))] - - (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)))) + [code error file-data _] + (repair-shapes code error file-data + (fn [shape] + (log/debug :hint " -> set to " :parent-id uuid/zero) + (assoc shape :parent-id uuid/zero)))) (defmethod repair-error :child-not-in-parent - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [parent-shape] - ;; Add shape to parent's children list - (log/debug :hint " -> add children to" :parent-id (:id parent-shape)) - (update parent-shape :shapes conj (:id shape)))] - - (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)))) + [code {:keys [shape] :as error} file-data _] + (repair-shapes code error file-data [(:parent-id shape)] + (fn [parent-shape] + (log/debug :hint " -> add children to" :parent-id (:id parent-shape)) + (update parent-shape :shapes conj (:id shape))))) (defmethod repair-error :duplicated-children - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Remove duplicated - (log/debug :hint " -> remove duplicated children") - (update shape :shapes distinct))] - - (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)))) + [code error file-data _] + (repair-shapes code error file-data + (fn [shape] + (log/debug :hint " -> remove duplicated children") + (update shape :shapes distinct)))) (defmethod repair-error :child-not-found - [_ {:keys [shape page-id args] :as error} file-data _] - (let [repair-shape - (fn [parent-shape] - (log/debug :hint " -> remove child" :child-id (:child-id args)) - (update parent-shape :shapes (fn [shapes] - (d/removev #(= (:child-id args) %) shapes))))] - (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)))) + [code {:keys [args] :as error} file-data _] + (repair-shapes code error file-data + (fn [parent-shape] + (log/debug :hint " -> remove child" :child-id (:child-id args)) + (update parent-shape :shapes (fn [shapes] + (d/removev #(= (:child-id args) %) shapes)))))) (defmethod repair-error :invalid-parent [_ {:keys [shape page-id args] :as error} file-data _] @@ -104,179 +111,66 @@ (pcb/change-parent (:parent-id args) [shape] nil {:allow-altering-copies true}))) (defmethod repair-error :frame-not-found - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Locate the first frame in parents and set frame-id to it. - (let [page (ctpl/get-page file-data page-id) - frame (cfh/get-frame (:objects page) (:parent-id shape)) - frame-id (or (:id frame) uuid/zero)] - (log/debug :hint " -> set to " :frame-id frame-id) - (assoc shape :frame-id frame-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)))) + [code {:keys [page-id] :as error} file-data _] + (repair-shapes code error file-data #(set-frame-id (ctpl/get-page file-data page-id) %))) (defmethod repair-error :invalid-frame - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Locate the first frame in parents and set frame-id to it. - (let [page (ctpl/get-page file-data page-id) - frame (cfh/get-frame (:objects page) (:parent-id shape)) - frame-id (or (:id frame) uuid/zero)] - (log/debug :hint " -> set to " :frame-id frame-id) - (assoc shape :frame-id frame-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)))) - -(defmethod repair-error :component-not-main - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Set the :shape as main instance root - (log/debug :hint " -> set :main-instance") - (assoc shape :main-instance true))] - - (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)))) + [code {:keys [page-id] :as error} file-data _] + (repair-shapes code error file-data #(set-frame-id (ctpl/get-page file-data page-id) %))) (defmethod repair-error :component-main-external - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Set :component-file to local file - (log/debug :hint " -> set :component-file to local file") - (assoc shape :component-file (:id file-data)))] - - ;; There is no solution that may recover it with confidence - ;; (log/warn :hint " -> CANNOT REPAIR THIS AUTOMATICALLY.") - ;; shape)] - - (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)))) + [code error file-data _] + (repair-shapes code error file-data + (fn [shape] + (log/debug :hint " -> set :component-file to local file") + (assoc shape :component-file (:id file-data))))) (defmethod repair-error :component-not-found - [_ {:keys [shape page-id] :as error} file-data _] - (let [page (ctpl/get-page file-data page-id) - shape-ids (cfh/get-children-ids-with-self (:objects page) (:id shape)) - - repair-shape - (fn [shape] - ;; Detach the shape and convert it to non instance. - (log/debug :hint " -> detach shape" :shape-id (:id shape)) - (ctk/detach-shape shape))] - ;; There is no solution that may recover it with confidence - ;; (log/warn :hint " -> CANNOT REPAIR THIS AUTOMATICALLY.") - ;; shape)] - - (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)))) + [code {:keys [shape page-id] :as error} file-data _] + (let [page (ctpl/get-page file-data page-id)] + (repair-shapes code error file-data + (cfh/get-children-ids-with-self (:objects page) (:id shape)) + detach-shape))) (defmethod repair-error :invalid-main-instance-id - [_ {:keys [shape page-id] :as error} file-data _] - (let [component (ctkl/get-component file-data (:component-id shape)) - - repair-component - (fn [component] - ;; Assign main instance in the component to current shape - (log/debug :hint " -> assign main-instance-id" :component-id (:id component)) - (assoc component :main-instance-id (:id shape))) - - detach-shape - (fn [shape] - (log/debug :hint " -> detach shape" :shape-id (:id shape)) - (ctk/detach-shape shape))] - - (log/debug :hint "repairing shape :invalid-main-instance-id" :id (:id shape) :name (:name shape) :page-id page-id) + [code {:keys [shape] :as error} file-data _] + (let [component (ctkl/get-component file-data (:component-id shape))] (if (and (some? component) (not (:deleted component))) - (-> (pcb/empty-changes nil page-id) - (pcb/with-library-data file-data) - (pcb/update-component (:component-id shape) repair-component)) - - (-> (pcb/empty-changes nil page-id) - (pcb/with-file-data file-data) - (pcb/update-shapes [(:id shape)] detach-shape))))) + (repair-component code error file-data (:component-id shape) + (fn [component] + (log/debug :hint " -> assign main-instance-id" :component-id (:id component)) + (assoc component :main-instance-id (:id shape)))) + (repair-shapes code error file-data detach-shape)))) (defmethod repair-error :invalid-main-instance-page - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-component - (fn [component] - ;; 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/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)))) + [code {:keys [shape page-id] :as error} file-data _] + (repair-component code error file-data (:component-id shape) + (fn [component] + (log/debug :hint " -> assign main-instance-page" :component-id (:id component)) + (assoc component :main-instance-page page-id)))) (defmethod repair-error :invalid-main-instance - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; There is no solution that may recover it with confidence - (log/warn :hint " -> CANNOT REPAIR THIS AUTOMATICALLY.") - shape)] - - (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)))) + [code error file-data _] + (repair-shapes code error file-data cannot-repair)) (defmethod repair-error :component-main - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Unset the :shape as main instance root - (log/debug :hint " -> unset :main-instance") - (dissoc shape :main-instance))] - - (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)))) - -(defmethod repair-error :should-be-component-root - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Convert the shape in a top copy root. - (log/debug :hint " -> set :component-root") - (assoc shape :component-root true))] - - (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)))) + [code error file-data _] + (repair-shapes code error file-data + (fn [shape] + (log/debug :hint " -> unset :main-instance") + (dissoc shape :main-instance)))) (defmethod repair-error :should-not-be-component-root - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Convert the shape in a nested copy root. - (log/debug :hint " -> unset :component-root") - (dissoc shape :component-root))] - - (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)))) + [code error file-data _] + (repair-shapes code error file-data + (fn [shape] + (log/debug :hint " -> unset :component-root") + (dissoc shape :component-root)))) (defmethod repair-error :ref-shape-not-found - [_ {:keys [shape page-id] :as error} file-data libraries] - (let [matching-shape (let [page (ctpl/get-page file-data page-id) - root-shape (ctn/get-component-shape (:objects page) shape) + [code {:keys [shape page-id] :as error} file-data libraries] + (let [page (ctpl/get-page file-data page-id) + matching-shape (let [root-shape (ctn/get-component-shape (:objects page) shape) component-file (if (= (:component-file root-shape) (:id file-data)) file-data (-> (get libraries (:component-file root-shape)) :data)) @@ -302,337 +196,206 @@ component-shapes (ctf/get-component-shapes file-data component)] (if-let [near-shape-3 (d/seek #(= (:id %) (:shape-ref random-shape)) component-shapes)] near-shape-3 - nil)))))) - reassign-shape - (fn [shape] - (log/debug :hint " -> reassign shape-ref to" :shape-ref (:id matching-shape)) - (assoc shape :shape-ref (:id matching-shape))) - - detach-shape - (fn [shape] - (log/debug :hint " -> detach shape" :shape-id (:id shape)) - (ctk/detach-shape shape))] + nil))))))] ;; 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/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) - (pcb/update-shapes [(:id shape)] reassign-shape)) - (let [page (ctpl/get-page file-data page-id) - shape-ids (cfh/get-children-ids-with-self (:objects page) (:id shape))] - (-> (pcb/empty-changes nil page-id) - (pcb/with-file-data file-data) - (pcb/update-shapes shape-ids detach-shape)))))) + (repair-shapes code error file-data + (fn [shape] + (log/debug :hint " -> reassign shape-ref to" :shape-ref (:id matching-shape)) + (assoc shape :shape-ref (:id matching-shape)))) + (repair-shapes code error file-data + (cfh/get-children-ids-with-self (:objects page) (:id shape)) + detach-shape)))) (defmethod repair-error :ref-shape-is-not-head - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Convert shape in a normal copy, removing nested copy status - (log/debug :hint " -> unhead shape") - (ctk/unhead-shape shape))] - - (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)))) + [code error file-data _] + (repair-shapes code error file-data + (fn [shape] + (log/debug :hint " -> unhead shape") + (ctk/unhead-shape shape)))) (defmethod repair-error :component-id-mismatch - [_ {:keys [shape page-id args] :as error} file-data _] - (let [repair-shape - (fn [shape] - ; Set the component-id and component-file to the ones of the near main - (log/debug :hint (str " -> set component-id to " (:component-id args))) - (log/debug :hint (str " -> set component-file to " (:component-file args))) - (cond-> shape - (some? (:component-id args)) - (assoc :component-id (:component-id args)) + [code {:keys [args] :as error} file-data _] + (repair-shapes code error file-data + (fn [shape] + (log/debug :hint (str " -> set component-id to " (:component-id args))) + (log/debug :hint (str " -> set component-file to " (:component-file args))) + (cond-> shape + (some? (:component-id args)) + (assoc :component-id (:component-id args)) - (nil? (:component-id args)) - (dissoc :component-id) + (nil? (:component-id args)) + (dissoc :component-id) - (some? (:component-file args)) - (assoc :component-file (:component-file args)) + (some? (:component-file args)) + (assoc :component-file (:component-file args)) - (nil? (:component-file args)) - (dissoc :component-file)))] - - (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)))) + (nil? (:component-file args)) + (dissoc :component-file))))) (defmethod repair-error :ref-shape-is-head - [_ {:keys [shape page-id args] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Convert shape in a nested head, adding component info - (log/debug :hint " -> reroot shape") - (ctk/rehead-shape shape (:component-file args) (:component-id args)))] - - (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)))) + [code {:keys [args] :as error} file-data _] + (repair-shapes code error file-data + (fn [shape] + (log/debug :hint " -> reroot shape") + (ctk/rehead-shape shape (:component-file args) (:component-id args))))) (defmethod repair-error :shape-ref-cycle - [_ {:keys [shape args] :as error} file-data _] - (let [repair-component - (fn [component] - (let [objects (:objects component) - ;; we only have encounter this on deleted components, - ;; so the relevant objects are inside the component - to-detach (->> (:cycles-ids args) - (map #(get objects %)) - (map #(ctn/get-head-shape objects %)) - (map :id) - distinct - (mapcat #(ctn/get-children-in-instance objects %)) - (map :id) - set)] + [code {:keys [shape args] :as error} file-data _] + ;; the :shape of this error is the component + (repair-component code (dissoc error :page-id) file-data (:id shape) + (fn [component] + (let [objects (:objects component) + ;; we only have encounter this on deleted components, + ;; so the relevant objects are inside the component + to-detach (->> (:cycles-ids args) + (map #(get objects %)) + (map #(ctn/get-head-shape objects %)) + (map :id) + distinct + (mapcat #(ctn/get-children-in-instance objects %)) + (map :id) + set)] - (update component :objects - (fn [objects] - (reduce-kv (fn [acc k v] - (if (contains? to-detach k) - (assoc acc k (ctk/detach-shape v)) - (assoc acc k v))) - {} - objects)))))] - (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)))) + (update component :objects + (fn [objects] + (reduce-kv (fn [acc k v] + (if (contains? to-detach k) + (assoc acc k (ctk/detach-shape v)) + (assoc acc k v))) + {} + objects))))))) (defmethod repair-error :shape-ref-in-main - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Remove shape-ref - (log/debug :hint " -> unset :shape-ref") - (dissoc shape :shape-ref))] - - (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)))) + [code error file-data _] + (repair-shapes code error file-data + (fn [shape] + (log/debug :hint " -> unset :shape-ref") + (dissoc shape :shape-ref)))) (defmethod repair-error :root-main-not-allowed - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Convert the shape in a nested main head. - (log/debug :hint " -> unset :component-root") - (dissoc shape :component-root))] - - (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)))) + [code error file-data _] + (repair-shapes code error file-data + (fn [shape] + ;; Convert the shape in a nested main head. + (log/debug :hint " -> unset :component-root") + (dissoc shape :component-root)))) (defmethod repair-error :nested-main-not-allowed - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Convert the shape in a top main head. - (log/debug :hint " -> set :component-root") - (assoc shape :component-root true))] - - (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) - (pcb/change-parent uuid/zero [shape] nil {:allow-altering-copies true})))) + [code {:keys [shape] :as error} file-data _] + (-> (repair-shapes code error file-data + (fn [shape] + ;; Convert the shape in a top main head. + (log/debug :hint " -> set :component-root") + (assoc shape :component-root true))) + (pcb/change-parent uuid/zero [shape] nil {:allow-altering-copies true}))) (defmethod repair-error :root-copy-not-allowed - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Convert the shape in a nested copy head. - (log/debug :hint " -> unset :component-root") - (dissoc shape :component-root))] - - (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)))) + [code error file-data _] + (repair-shapes code error file-data + (fn [shape] + ;; Convert the shape in a nested copy head. + (log/debug :hint " -> unset :component-root") + (dissoc shape :component-root)))) (defmethod repair-error :nested-copy-not-allowed - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Convert the shape in a top copy root. - (log/debug :hint " -> set :component-root") - (assoc shape :component-root true))] - - (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)))) + [code error file-data _] + (repair-shapes code error file-data + (fn [shape] + ;; Convert the shape in a top copy root. + (log/debug :hint " -> set :component-root") + (assoc shape :component-root true)))) (defmethod repair-error :not-head-main-not-allowed - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Detach the shape and convert it to non instance. - (log/debug :hint " -> detach shape" :shape-id (:id shape)) - (ctk/detach-shape shape))] - - (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)))) + [code error file-data _] + (repair-shapes code error file-data detach-shape)) (defmethod repair-error :not-head-copy-not-allowed - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Detach the shape and convert it to non instance. - (log/debug :hint " -> detach shape" :shape-id (:id shape)) - (ctk/detach-shape shape))] - - (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)))) - -(defmethod repair-error :not-component-not-allowed - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; There is no solution that may recover it with confidence - (log/warn :hint " -> CANNOT REPAIR THIS AUTOMATICALLY.") - shape)] - - (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)))) + [code error file-data _] + (repair-shapes code error file-data detach-shape)) (defmethod repair-error :instance-head-not-frame - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Convert the shape in a frame. - (log/debug :hint " -> set :type :frame") - (assoc shape :type :frame - :fills [] - :hide-in-viewer true - :r1 0 - :r2 0 - :r3 0 - :r4 0))] - - (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)))) + [code error file-data _] + (repair-shapes code error file-data + (fn [shape] + (log/debug :hint " -> set :type :frame") + (assoc shape :type :frame + :fills [] + :hide-in-viewer true + :r1 0 + :r2 0 + :r3 0 + :r4 0)))) (defmethod repair-error :component-nil-objects-not-allowed - [_ {component :shape} file-data _] ; in this error the :shape argument is the component - (let [repair-component - (fn [component] - ;; Remove the objects key, or set it to {} if the component is deleted - (if (:deleted component) - (do - (log/debug :hint " -> set :objects {}") - (assoc component :objects {})) - (do - (log/debug :hint " -> remove :objects") - (dissoc component :objects))))] - - (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)))) + [code {component :shape :as error} file-data _] ; in this error the :shape argument is the component + (repair-component code (dissoc error :page-id) file-data (:id component) + (fn [component] + ;; Remove the objects key, or set it to {} if the component is deleted + (if (:deleted component) + (do + (log/debug :hint " -> set :objects {}") + (assoc component :objects {})) + (do + (log/debug :hint " -> remove :objects") + (dissoc component :objects)))))) (defmethod repair-error :non-deleted-component-cannot-have-objects - [_ {component :shape} file-data _] ; in this error the :shape argument is the component - (let [repair-component - (fn [component] - ; Remove the :objects field - (if-not (:deleted component) - (do - (log/debug :hint " -> remove :objects") - (dissoc component :objects)) - 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)))) + [code {component :shape :as error} file-data _] ; in this error the :shape argument is the component + (repair-component code (dissoc error :page-id) file-data (:id component) + (fn [component] + (if-not (:deleted component) + (do + (log/debug :hint " -> remove :objects") + (dissoc component :objects)) + component)))) (defmethod repair-error :invalid-text-touched - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Add content group - (log/debug :hint " -> add :content-group to :touched-groups") - (update shape :touched ctk/set-touched-group :content-group))] - - (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)))) + [code error file-data _] + (repair-shapes code error file-data + (fn [shape] + (log/debug :hint " -> add :content-group to :touched-groups") + (update shape :touched ctk/set-touched-group :content-group)))) (defmethod repair-error :legacy-text-touched - [_ {:keys [shape page-id] :as error} file-data libraries] + [code {:keys [shape page-id] :as error} file-data libraries] (let [page (ctpl/get-page file-data page-id) file {:id (:id file-data) :data file-data} ref (when (ctk/in-component-copy? shape) (ctf/find-ref-shape file (ctn/make-container page :page) libraries shape :include-deleted? true))] - (log/debug :hint "repairing shape :legacy-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)] #(assoc % :touched (cfcp/recompute-text-touched % ref)))))) + (repair-shapes code error file-data + #(assoc % :touched (cfcp/recompute-text-touched % ref))))) (defmethod repair-error :misplaced-slot - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - ;; Remove the swap slot - (log/debug :hint " -> remove swap-slot") - (cfss/remove-swap-slot shape))] - - (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)))) + [code error file-data _] + (repair-shapes code error file-data + (fn [shape] + (log/debug :hint " -> remove swap-slot") + (cfss/remove-swap-slot shape)))) (defmethod repair-error :misplaced-swap-group - [_ {:keys [shape page-id] :as error} file-data _] - (let [repair-shape - (fn [shape] - (log/debug :hint " -> remove :swap-group") - (ctk/remove-swap-group shape))] - - (log/debug :hint "repairing shape :misplaced-swap-group" :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)))) + [code error file-data _] + (repair-shapes code error file-data + (fn [shape] + (log/debug :hint " -> remove :swap-group") + (ctk/remove-swap-group shape)))) (defmethod repair-error :ref-not-in-near-main - [_ {:keys [shape page-id] :as error} file-data libraries] + [code {:keys [shape page-id] :as error} file-data libraries] (let [page (ctpl/get-page file-data page-id) file {:id (:id file-data) :data file-data} objects (:objects page) - replaced (cfcp/find-replaced-shape file (ctn/make-container page :page) libraries shape)] - - (log/debug :hint "repairing shape :ref-not-in-near-main" :id (:id shape) :name (:name shape) :page-id page-id) + replaced (cfcp/find-replaced-shape file (ctn/make-container page :page) libraries shape) + ids (cfh/get-children-ids-with-self objects (:id shape))] (if (some? replaced) (do (log/debug :hint " -> take the position of" :shape-ref (:id replaced)) - (-> (pcb/empty-changes nil page-id) - (pcb/with-file-data file-data) - (pcb/update-shapes (cfh/get-children-ids-with-self objects (:id shape)) - (ctf/take-position file libraries objects shape (:id replaced) replaced)))) + (repair-shapes code error file-data ids + (ctf/take-position file libraries objects shape (:id replaced) replaced))) ;; no position to take: the copy keeps its content, detached - (-> (pcb/empty-changes nil page-id) - (pcb/with-file-data file-data) - (pcb/update-shapes (cfh/get-children-ids-with-self objects (:id shape)) - ctk/detach-shape))))) + (repair-shapes code error file-data ids ctk/detach-shape)))) (defmethod repair-error :duplicate-ref [_ {:keys [shape page-id] :as error} file-data _] @@ -656,34 +419,18 @@ (pcb/with-file-data file-data) (pcb/remove-objects ids)))) -(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)))] +(defn- set-variant-id + [variant-id shape] + (log/debug :hint (str " -> set variant-id to " variant-id)) + (assoc shape :variant-id variant-id)) - (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-not-a-variant + [code {:keys [args] :as error} file-data _] + (repair-shapes code error file-data #(set-variant-id (:variant-id args) %))) (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)))) + [code {:keys [args] :as error} file-data _] + (repair-shapes code error file-data #(set-variant-id (:variant-id args) %))) (defmethod repair-error :invalid-variant-properties [_ {:keys [shape page-id args]} file-data _] @@ -723,67 +470,39 @@ (pcb/remove-objects shape-ids)))) (defmethod repair-error :parent-not-variant - [_ {: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)))) + [code {:keys [shape] :as error} file-data _] + (repair-shapes code error file-data [(:parent-id shape)] + (fn [parent] + (log/debug :hint " -> set :is-variant-container true") + (assoc parent :is-variant-container true)))) (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)))) + [code {:keys [args] :as error} file-data _] + (repair-shapes code error file-data + (fn [shape] + (log/debug :hint " -> set :name" :name (:variant-name args)) + (assoc shape :name (:variant-name args))))) (defmethod repair-error :variant-main-bad-variant-name - [_ {:keys [shape page-id]} file-data _] + [code {:keys [shape] :as error} 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)))) + variant-name (ctv/properties-to-name (:variant-properties component))] + (repair-shapes code error file-data + (fn [shape] + (log/debug :hint " -> set :variant-name" :variant-name variant-name) + (assoc shape :variant-name variant-name))))) (defmethod repair-error :variant-component-bad-name - [_ {: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)))) + [code {:keys [shape args] :as error} file-data _] + (let [[path name] (cpn/split-group-name (:variant-container-name args))] + (repair-component code error file-data (:component-id shape) + (fn [component] + (log/debug :hint " -> set :path and :name" :path path :name name) + (assoc component :path path :name name))))) (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)))) + [code {:keys [args] :as error} file-data _] + (repair-shapes code error file-data #(set-variant-id (:variant-id args) %))) (defmethod repair-error :missing-tokens-status [_ _ file-data _] diff --git a/common/src/app/common/files/validate.cljc b/common/src/app/common/files/validate.cljc index 4edefcc7e4..ada143cdfb 100644 --- a/common/src/app/common/files/validate.cljc +++ b/common/src/app/common/files/validate.cljc @@ -37,7 +37,6 @@ :child-not-found :frame-not-found :invalid-frame - :component-not-main :component-main-external :component-not-found :invalid-main-instance-id @@ -45,7 +44,6 @@ :invalid-main-instance :invalid-parent :component-main - :should-be-component-root :should-not-be-component-root :ref-shape-not-found :ref-shape-is-head @@ -58,7 +56,6 @@ :nested-copy-not-allowed :not-head-main-not-allowed :not-head-copy-not-allowed - :not-component-not-allowed :component-nil-objects-not-allowed :non-deleted-component-cannot-have-objects :instance-head-not-frame @@ -267,13 +264,9 @@ shape file page))))))))) (defn- check-component-main-head - "Validate shape is a main instance head, component exists - and its main-instance points to this shape." + "Validate that the component of the main head `shape` is in this file, + exists and has `shape` as its main instance." [shape file page libraries] - (when (nil? (:main-instance shape)) - (report-error :component-not-main - "Shape expected to be main instance" - shape file page)) (when-not (= (:component-file shape) (:id file)) (report-error :component-main-external "Main instance should refer to a component in the same file" @@ -302,15 +295,9 @@ shape file page)))))))) (defn- check-component-not-main-head - "Validate shape is a not-main instance head, component - exists and its main-instance does not point to this - shape." + "Validate that the component of the copy head `shape` exists and has + another shape as its main instance." [shape file page libraries] - (when (true? (:main-instance shape)) - (report-error :component-not-main - "Shape not expected to be main instance" - shape file page)) - (let [library-exists (library-exists? file libraries shape) component (when library-exists (ctf/resolve-component shape file libraries {:include-deleted? true}))] @@ -332,20 +319,11 @@ (report-error :component-main "Shape not expected to be main instance" shape file page)) - (when (or (some? (:component-id shape)) - (some? (:component-file shape))) + (when (some? (:component-file shape)) (report-error :component-main "Shape not expected to be component head" shape file page))) -(defn- check-component-root - "Validate that this shape is an instance root." - [shape file page] - (when (nil? (:component-root shape)) - (report-error :should-be-component-root - "Shape should be component root" - shape file page))) - (defn- check-component-not-root "Validate that this shape is not an instance root." [shape file page] @@ -485,7 +463,6 @@ - :component-root" [shape file page libraries] (check-component-main-head shape file page libraries) - (check-component-root shape file page) (check-component-not-ref shape file page) (check-no-swap-slot shape file page) (check-not-swapped shape file page) @@ -514,7 +491,6 @@ ;; We propagate have to propagate to nested shapes if library is valid or not (let [library-exists (library-exists? file libraries shape)] (check-component-not-main-head shape file page libraries) - (check-component-root shape file page) (check-component-ref shape file page libraries) (check-ref-is-head shape file page libraries) (check-no-swap-slot shape file page) @@ -715,26 +691,18 @@ shape file page) (check-shape-copy-not-root shape file page libraries)) - ;; Short-circuit `inside-component-main?` when the propagated - ;; `context` already classifies this sub-tree as belonging to a - ;; main component. `inside-component-main?` performs an O(depth) - ;; upward ancestor walk; skipping it for every non-head shape that - ;; sits inside a known-main context avoids redundant tree traversals - ;; that would otherwise dominate validation time on deep hierarchies. - (let [in-main? (or (#{:main-top :main-nested :main-any} context) - (ctn/inside-component-main? (:objects page) shape))] - (if in-main? - (if-not (#{:main-top :main-nested :main-any} context) - (report-error :not-head-main-not-allowed - "Non-root main only allowed inside a main component" - shape file page) - (check-shape-main-not-root shape file page libraries)) + (cond + (#{:main-top :main-nested :main-any} context) + (check-shape-main-not-root shape file page libraries) - (if (#{:main-top :main-nested :main-any} context) - (report-error :not-component-not-allowed - "Not components are not allowed inside a main" - shape file page) - (check-shape-not-component shape file page libraries))))))))) + ;; an O(depth) walk up, so only when the context does not answer + (ctn/inside-component-main? (:objects page) shape) + (report-error :not-head-main-not-allowed + "Non-root main only allowed inside a main component" + shape file page) + + :else + (check-shape-not-component shape file page libraries))))))) (defn check-ref-cycles [component file] diff --git a/common/test/common_tests/files/repair_components_test.cljc b/common/test/common_tests/files/repair_components_test.cljc index 113ba138f0..68b9f4c4ee 100644 --- a/common/test/common_tests/files/repair_components_test.cljc +++ b/common/test/common_tests/files/repair_components_test.cljc @@ -8,11 +8,7 @@ "One test per component error code of `app.common.files.validate`: each builds a file broken in one way, checks the codes validation reports, repairs it as the backend does (validate and repair until valid, at most - ten passes) and checks the result. - - Three codes have no test because no file can produce them: the checks that - report `:component-not-main`, `:should-be-component-root` and - `:not-component-not-allowed` only run on shapes that already satisfy them." + ten passes) and checks the result." (:require [app.common.files.changes-builder :as pcb] [app.common.files.repair :as cfr]