From 6c1c7807587fe8fdee251d188a7c6ac10940c3ba Mon Sep 17 00:00:00 2001 From: Pablo Alba Date: Fri, 17 Nov 2023 09:33:37 +0100 Subject: [PATCH] :bug: Add validation and repair for files with nil in component :objects --- backend/src/app/features/components_v2.clj | 20 ++++++++++++++--- .../src/app/common/files/changes_builder.cljc | 2 +- common/src/app/common/files/repair.cljc | 18 +++++++++++++++ common/src/app/common/files/validate.cljc | 22 ++++++++++++++----- 4 files changed, 53 insertions(+), 9 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 2a5f3eee4..0d28fe6d3 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -271,11 +271,24 @@ (let [nearest-frame (cph/get-frame objects (:parent-id shape)) frame-id (or (:id nearest-frame) uuid/zero)] (update objects (:id shape) assoc :frame-id frame-id)) - objects)))] + objects)))] (-> file-data (update :pages-index update-vals fix-container) - (update :components update-vals fix-container))))] + (update :components update-vals fix-container)))) + + fix-component-nil-objects + (fn [file-data] + ;; Ensure that objects of all components is not null + (letfn [(fix-component + [component] + (if (and (contains? component :objects) (nil? (:objects component))) + (if (:deleted component) + (assoc component :objects []) + (dissoc component :objects)) + component))] + (-> file-data + (update :components update-vals fix-component))))] (-> file-data (fix-orphan-shapes) @@ -286,7 +299,8 @@ (fix-copies-of-detached) (transform-to-frames) (remap-frame-ids) - (fix-frame-ids)))) + (fix-frame-ids) + (fix-component-nil-objects)))) (defn- migrate-components "If there is any component in the file library, add a new 'Library diff --git a/common/src/app/common/files/changes_builder.cljc b/common/src/app/common/files/changes_builder.cljc index ad7d14609..be371314a 100644 --- a/common/src/app/common/files/changes_builder.cljc +++ b/common/src/app/common/files/changes_builder.cljc @@ -728,7 +728,7 @@ :main-instance-id (:main-instance-id new-component) :main-instance-page (:main-instance-page new-component) :annotation (:annotation new-component) - :objects (:objects new-component)}) ;; this won't exist in components-v2 + :objects (:objects new-component)}) ;; this won't exist in components-v2 (except for deleted components) (update :undo-changes conj {:type :mod-component :id id :name (:name prev-component) diff --git a/common/src/app/common/files/repair.cljc b/common/src/app/common/files/repair.cljc index ff21efe2e..da366a0ef 100644 --- a/common/src/app/common/files/repair.cljc +++ b/common/src/app/common/files/repair.cljc @@ -404,6 +404,24 @@ (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) +(defmethod repair-error :component-nil-objects-not-allowed + [_ {:keys [shape] :as error} file-data _] + (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/info :hint "Repairing component :component-nil-objects-not-allowed" :id (:id shape) :name (:name shape)) + (-> (pcb/empty-changes nil) + (pcb/with-library-data file-data) + (pcb/update-component (:id shape) repair-component)))) + (defmethod repair-error :default [_ error file _] (log/error :hint "Unknown error code, don't know how to repair" :code (:code error)) diff --git a/common/src/app/common/files/validate.cljc b/common/src/app/common/files/validate.cljc index 5989b0616..30e14acf5 100644 --- a/common/src/app/common/files/validate.cljc +++ b/common/src/app/common/files/validate.cljc @@ -413,6 +413,15 @@ (validate-shape! shape-id file page libraries) (deref *errors*))) +(defn validate-component! + " Validate semantic coherence of a component. Raises an exception + on first error found." + [component file] + (when (and (contains? component :objects) (nil? (:objects component))) + (report-error! :component-nil-objects-not-allowed + "Objects list cannot be nil" + component file nil))) + (def valid-fdata? "Structural validation of file data using defined schema" (sm/lazy-validator ::ctf/data)) @@ -439,14 +448,17 @@ :file-id id ::sm/explain (get-fdata-explain data))) - ;; If `libraries` is provided, this means the fill file + ;; If `libraries` is provided, this means the full file ;; validation is activated so we proceed to execute the ;; validation - (when (seq libraries) - (doseq [page (filter :id (ctpl/pages-seq data))] - (validate-shape! uuid/zero file page libraries))) + (when (some? libraries) + (when (:components data) + (doseq [component (vals (:components data))] + (validate-component! component file))) + (doseq [page (filter :id (ctpl/pages-seq data))] + (validate-shape! uuid/zero file page libraries))) - file)) + file)) (defn validate-file "Validate referencial integrity and semantic coherence of