🐛 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
This commit is contained in:
Andrey Antukh 2026-10-01 19:19:05 +00:00
parent 556d604e67
commit 48ab90b1ec
2 changed files with 136 additions and 8 deletions

View File

@ -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}]

View File

@ -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)))))))