From 3512a57df7b9a550965d617d5b7761352fd6369d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bel=C3=A9n=20Albeza?= Date: Thu, 21 May 2026 11:18:13 +0200 Subject: [PATCH] :bug: Fix referential integrity data in old files (#9771) --- .../src/app/common/files/comp_processors.cljc | 18 +++++ common/src/app/common/files/migrations.cljc | 16 +++- .../files/comp_processors_test.cljc | 78 +++++++++++++++++++ 3 files changed, 111 insertions(+), 1 deletion(-) diff --git a/common/src/app/common/files/comp_processors.cljc b/common/src/app/common/files/comp_processors.cljc index 80e782e7cc..b757427ae6 100644 --- a/common/src/app/common/files/comp_processors.cljc +++ b/common/src/app/common/files/comp_processors.cljc @@ -38,6 +38,24 @@ (dissoc component :objects)) component))))) +(defn normalize-component-root + "Some old files have shapes with an explicit :component-root false. This is semantically + equivalent to the attribute being absent (instance-root? only treats true as root), but + breaks the subcopy-head? predicate, which expects nil. Remove the explicit false so the + downstream fixers can recognize these shapes as nested copy heads." + [file-data] + (ctf/update-all-shapes + file-data + (fn [shape] + (if (false? (:component-root shape)) + (do + (log/warn :msg "Normalizing :component-root false on shape" + :shape-id (:id shape) + :shape-name (:name shape) + :file-id (:id file-data)) + {:result :update :updated-shape (dissoc shape :component-root)}) + {:result :keep})))) + (defn fix-missing-swap-slots "Locate shapes that have been swapped (i.e. their shape-ref does not point to the near match) but they don't have a swap slot. In this case, add one pointing to the near match." diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index 5721910890..d4d848e3cf 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -1820,6 +1820,19 @@ (update :pages-index d/update-vals update-container) (d/update-when :components d/update-vals update-container)))) +;; Re-run the 0019 and 0020 fixers after normalizing :component-root. +;; Migrations 0019 and 0020 missed shapes with an explicit :component-root +;; false because subcopy-head? expects nil. Normalize first, then re-run. +(defmethod migrate-data "0022-normalize-component-root-and-resync" + [data _] + (let [libraries (if (:libs data) + (deref (:libs data)) + {})] + (-> data + (cfcp/normalize-component-root) + (cfcp/fix-missing-swap-slots libraries) + (cfcp/sync-component-id-with-ref-shape libraries)))) + (def available-migrations (into (d/ordered-set) ["legacy-2" @@ -1898,4 +1911,5 @@ "0018-remove-unneeded-objects-from-components" "0019-fix-missing-swap-slots" "0020-sync-component-id-with-near-main" - "0021-fix-shape-svg-attrs"])) + "0021-fix-shape-svg-attrs" + "0022-normalize-component-root-and-resync"])) diff --git a/common/test/common_tests/files/comp_processors_test.cljc b/common/test/common_tests/files/comp_processors_test.cljc index c1cabbbb72..412986ede4 100644 --- a/common/test/common_tests/files/comp_processors_test.cljc +++ b/common/test/common_tests/files/comp_processors_test.cljc @@ -114,6 +114,84 @@ (t/is (= expected-diff diff))))) +(t/deftest test-normalize-component-root + + (t/testing "nil file should return nil" + (let [file nil + file' (ctf/update-file-data file cfcp/normalize-component-root)] + (t/is (nil? file')))) + + (t/testing "empty file should not need any action" + (let [file (thf/sample-file :file1) + file' (ctf/update-file-data file cfcp/normalize-component-root)] + (t/is (empty? (d/map-diff file file'))))) + + (t/testing "shape with :component-root true should not be modified" + (let [file + (-> (thf/sample-file :file1) + (tho/add-simple-component :component1 :main1-root :main1-child) + (thc/instantiate-component :component1 :copy1-root)) + + file' (ctf/update-file-data file cfcp/normalize-component-root)] + + (t/is (empty? (d/map-diff file file'))))) + + (t/testing "shape with :component-root false should have it removed" + (let [file + (-> (thf/sample-file :file1) + (tho/add-nested-component-with-copy :component1 :main1-root :main1-child + :component2 :main2-root :nested-head + :copy2-root) + (ths/update-shape :nested-head :component-root false)) + + file' (ctf/update-file-data file cfcp/normalize-component-root) + + shape' (ths/get-shape file' :nested-head)] + + (t/is (not (contains? shape' :component-root)))))) + +(t/deftest test-migration-0022-fixes-component-root-false-mismatch + + (t/testing "a nested copy with :component-root false and a stale :component-id is fixed by the full migration sequence" + (let [file + ;; Reproduce the legacy corruption: a nested copy head has + ;; :component-root false (instead of absent) AND a stale + ;; component-id. Migrations 0019/0020 alone don't catch it + ;; because subcopy-head? expects nil. After normalize-component-root, + ;; sync-component-id-with-ref-shape can repair it. + (-> (thf/sample-file :file1) + (tho/add-nested-component :component1 :main1-root :main1-child + :component2 :main2-root :nested-head) + (thc/instantiate-component :component2 :copy2-root :children-labels [:copy2-nested-head]) + (ths/update-shape :copy2-nested-head :component-root false) + (ths/update-shape :copy2-nested-head :component-id (thi/new-id! :wrong-id))) + + ;; Run the old sequence (0019/0020 without normalization): the bad shape is skipped. + old-sequence + (-> file + (ctf/update-file-data #(cfcp/fix-missing-swap-slots % {})) + (ctf/update-file-data #(cfcp/sync-component-id-with-ref-shape % {}))) + + old-shape (ths/get-shape old-sequence :copy2-nested-head) + + ;; Run the new sequence (0022): normalize first, then 0019/0020 logic. + new-sequence + (-> file + (ctf/update-file-data cfcp/normalize-component-root) + (ctf/update-file-data #(cfcp/fix-missing-swap-slots % {})) + (ctf/update-file-data #(cfcp/sync-component-id-with-ref-shape % {}))) + + new-shape (ths/get-shape new-sequence :copy2-nested-head)] + + ;; Confirm the old sequence fails to repair (this guards against a future + ;; subcopy-head? change accidentally fixing it and masking the regression). + (t/is (= false (:component-root old-shape))) + (t/is (= (thi/id :wrong-id) (:component-id old-shape))) + + ;; Confirm the new sequence repairs both the explicit false and the bad ref. + (t/is (not (contains? new-shape :component-root))) + (t/is (= (thi/id :component1) (:component-id new-shape)))))) + (t/deftest test-fix-missing-swap-slots (t/testing "nil file should return nil"