From 48ab90b1ec1fa0da4cf6cc04f027f2e98ec728a4 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 1 Oct 2026 19:19:05 +0000 Subject: [PATCH] :bug: Skip circular library links on binfile import File import recreated circular library references that the link-file-to-library command already rejects. The v3 importer now skips manifest relations that would close a cycle, or link a file to itself, and logs each skipped link. Closes #12052. AI-assisted-by: muse-spark-1.3-contributor-free --- backend/src/app/binfile/v3.clj | 31 ++++-- backend/test/backend_tests/binfile_test.clj | 113 ++++++++++++++++++++ 2 files changed, 136 insertions(+), 8 deletions(-) diff --git a/backend/src/app/binfile/v3.clj b/backend/src/app/binfile/v3.clj index c2fa61b686..e8f56f3779 100644 --- a/backend/src/app/binfile/v3.clj +++ b/backend/src/app/binfile/v3.clj @@ -994,14 +994,29 @@ libr-id (bfc/lookup-index libr-id)] (when (and file-id libr-id) - (l/dbg :hint "create file library link" - :file-id (str file-id) - :lib-id (str libr-id) - ::l/sync? true) - (let [rel-params {:file-id file-id - :library-file-id libr-id}] - (db/insert! conn :file-library-rel rel-params) - (bfc/upsert-file-library-sync! conn (assoc rel-params :synced-at timestamp))))))) + (cond + (= file-id libr-id) + (l/warn :hint "ignoring self-referencing library link" + :file-id (str file-id) + :lib-id (str libr-id) + ::l/sync? true) + + (contains? (bfc/get-libraries cfg [libr-id]) file-id) + (l/warn :hint "ignoring circular library link" + :file-id (str file-id) + :lib-id (str libr-id) + ::l/sync? true) + + :else + (do + (l/dbg :hint "create file library link" + :file-id (str file-id) + :lib-id (str libr-id) + ::l/sync? true) + (let [rel-params {:file-id file-id + :library-file-id libr-id}] + (db/insert! conn :file-library-rel rel-params) + (bfc/upsert-file-library-sync! conn (assoc rel-params :synced-at timestamp))))))))) (defn- import-storage-objects [{:keys [::bfc/input ::entries-index ::bfc/timestamp] :as cfg}] diff --git a/backend/test/backend_tests/binfile_test.clj b/backend/test/backend_tests/binfile_test.clj index 1b75b3a6cd..e2c7fa17e5 100644 --- a/backend/test/backend_tests/binfile_test.clj +++ b/backend/test/backend_tests/binfile_test.clj @@ -2894,3 +2894,116 @@ (t/is (= (str (first result) "/" thumb-page-id "/" thumb-frame-id "/" thumb-tag) (:object-id thumb))) (t/is (some? (:media-id thumb))))))) + +(t/deftest import-skips-circular-library-relations + ;; https://github.com/penpot/penpot/issues/12052: two libraries that use + ;; each other (possible before the 2.17.1 guard) export both relations in + ;; the manifest; the import must skip the one closing the cycle instead + ;; of recreating it. The cycle is built with direct inserts because the + ;; UI command `link-file-to-library` already rejects it. + (let [profile (th/create-profile* 1) + lib-a (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared true + :name "Library A"}) + lib-b (th/create-file* 2 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared true + :name "Library B"})] + (db/insert! th/*system* :file-library-rel + {:file-id (:id lib-a) + :library-file-id (:id lib-b)}) + (db/insert! th/*system* :file-library-rel + {:file-id (:id lib-b) + :library-file-id (:id lib-a)}) + + (let [output (tmp/tempfile :suffix ".zip")] + (v3/export-files! + (-> th/*system* + (assoc ::bfc/ids #{(:id lib-a) (:id lib-b)}) + (assoc ::bfc/export-type :include-libraries)) + (io/output-stream output)) + + (let [result (:file-ids (-> th/*system* + (assoc ::bfc/project-id (:default-project-id profile)) + (assoc ::bfc/profile-id (:id profile)) + (assoc ::bfc/input output) + (v3/import-files!))) + rels (mapcat #(db/query th/*system* :file-library-rel {:file-id %}) + result)] + (t/is (= 2 (count result))) + ;; only one of the two cycle edges survives the import + (t/is (= 1 (count rels))) + ;; no imported file lists itself among its transitive libraries + (doseq [file-id result] + (t/is (not (contains? (bfc/get-libraries th/*system* [file-id]) + file-id)))))))) + +(t/deftest import-skips-self-referencing-library-relation + ;; A file linked to itself must not abort the import; the edge is + ;; skipped the same way `link-file-to-library` rejects it. + (let [profile (th/create-profile* 1) + lib-a (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared true + :name "Library A"})] + (db/insert! th/*system* :file-library-rel + {:file-id (:id lib-a) + :library-file-id (:id lib-a)}) + + (let [output (tmp/tempfile :suffix ".zip")] + (v3/export-files! + (-> th/*system* + (assoc ::bfc/ids #{(:id lib-a)}) + (assoc ::bfc/export-type :include-libraries)) + (io/output-stream output)) + + (let [result (:file-ids (-> th/*system* + (assoc ::bfc/project-id (:default-project-id profile)) + (assoc ::bfc/profile-id (:id profile)) + (assoc ::bfc/input output) + (v3/import-files!))) + rels (mapcat #(db/query th/*system* :file-library-rel {:file-id %}) + result)] + (t/is (= 1 (count result))) + (t/is (= 0 (count rels))))))) + +(t/deftest import-keeps-acyclic-library-relations + ;; Control test: a chain without cycles must be imported untouched, so + ;; the new guard only skips edges that would close a cycle. + (let [profile (th/create-profile* 1) + lib-a (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared true + :name "Library A"}) + lib-b (th/create-file* 2 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared true + :name "Library B"}) + lib-c (th/create-file* 3 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared true + :name "Library C"})] + (db/insert! th/*system* :file-library-rel + {:file-id (:id lib-a) + :library-file-id (:id lib-b)}) + (db/insert! th/*system* :file-library-rel + {:file-id (:id lib-b) + :library-file-id (:id lib-c)}) + + (let [output (tmp/tempfile :suffix ".zip")] + (v3/export-files! + (-> th/*system* + (assoc ::bfc/ids #{(:id lib-a) (:id lib-b) (:id lib-c)}) + (assoc ::bfc/export-type :include-libraries)) + (io/output-stream output)) + + (let [result (:file-ids (-> th/*system* + (assoc ::bfc/project-id (:default-project-id profile)) + (assoc ::bfc/profile-id (:id profile)) + (assoc ::bfc/input output) + (v3/import-files!))) + rels (mapcat #(db/query th/*system* :file-library-rel {:file-id %}) + result)] + (t/is (= 3 (count result))) + (t/is (= 2 (count rels)))))))