diff --git a/backend/src/app/binfile/common.clj b/backend/src/app/binfile/common.clj index 746dd3e55a..8a9106d591 100644 --- a/backend/src/app/binfile/common.clj +++ b/backend/src/app/binfile/common.clj @@ -362,6 +362,37 @@ (set/difference cfeat/backend-only-features)) #{})))) +(defn- component-shape? + [shape] + (or (some? (:component-id shape)) + (some? (:shape-ref shape)) + (true? (:main-instance shape)))) + +(defn- has-component-data? + [{:keys [components pages-index]}] + (or (seq components) + (some (fn [page] (some component-shape? (vals (:objects page)))) + (vals pages-index)))) + +(defn check-components-v2 + "Check that an imported file can be stored as a components v2 file. + A file without the `components/v2` feature is in the deprecated + components v1 format: it is rejected when it has components or shapes + linked to them, and otherwise takes the feature as it is. Returns the + file." + [{:keys [features data] :as file}] + (cond + (contains? features "components/v2") + file + + (has-component-data? data) + (ex/raise :type :restriction + :code :file-in-components-v1 + :hint "components v1 is deprecated") + + :else + (update file :features (fnil conj #{}) "components/v2"))) + (defn check-file-exists [cfg id & {:keys [include-deleted?] :or {include-deleted? false} diff --git a/backend/src/app/binfile/migrations.clj b/backend/src/app/binfile/migrations.clj index 9fa5a4b4e9..35feb93c13 100644 --- a/backend/src/app/binfile/migrations.clj +++ b/backend/src/app/binfile/migrations.clj @@ -30,9 +30,6 @@ [_cfg] (doseq [[feature _file-id] (-> bfc/*state* deref :pending-to-migrate)] (case feature - "components/v2" - nil - "fdata/shape-data-type" nil diff --git a/backend/src/app/binfile/v1.clj b/backend/src/app/binfile/v1.clj index 649494a2c6..4480d6fece 100644 --- a/backend/src/app/binfile/v1.clj +++ b/backend/src/app/binfile/v1.clj @@ -556,7 +556,8 @@ (cond-> (and (= idx 0) (some? name)) (assoc :name name)) (assoc :project-id project-id) - (dissoc :thumbnails)) + (dissoc :thumbnails) + (bfc/check-components-v2)) file (bfc/process-file system file)] ;; All features that are enabled and requires explicit migration are diff --git a/backend/src/app/binfile/v3.clj b/backend/src/app/binfile/v3.clj index d599f766ed..85484cbbb0 100644 --- a/backend/src/app/binfile/v3.clj +++ b/backend/src/app/binfile/v3.clj @@ -1027,7 +1027,8 @@ {:generated-by (get manifest :generated-by) :referer (or (get manifest :referer) (get manifest :refer))})) (dissoc :options) - (dissoc :tokens-source)) + (dissoc :tokens-source) + (bfc/check-components-v2)) file (bfc/process-file cfg file) file (ctf/check-file file)] diff --git a/backend/test/backend_tests/binfile_test.clj b/backend/test/backend_tests/binfile_test.clj index 2043ad6a19..f72fc93438 100644 --- a/backend/test/backend_tests/binfile_test.clj +++ b/backend/test/backend_tests/binfile_test.clj @@ -2903,3 +2903,138 @@ (t/is (= (str (first result) "/" thumb-page-id "/" thumb-frame-id "/" thumb-tag) (:object-id thumb))) (t/is (some? (:media-id thumb))))))) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; FILES FROM BEFORE COMPONENTS V2 +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(defn- prepare-file-with-component + "A simple file whose library holds a component, with the file's shape + as its main." + [profile idx] + (let [file (prepare-simple-file profile idx)] + (update-file! + :file-id (:id file) + :profile-id (:id profile) + :revn 0 + :vern 0 + :changes + [{:type :add-component + :id (uuid/custom 5 3) + :name "component" + :path "" + :main-instance-id (uuid/custom 2 1) + :main-instance-page (uuid/custom 1 1)}]) + file)) + +(defn- drop-components-v2! + "Store the file without the `components/v2` feature, as a file from + before components v2." + [file-id] + (th/db-exec! ["UPDATE file SET features = array_remove(features, 'components/v2') WHERE id = ?" + file-id])) + +(defn- export-file + "A package of one file in the given binfile version." + [version file-id] + (let [output (tmp/tempfile :suffix ".penpot") + cfg (-> th/*system* + (assoc ::bfc/ids #{file-id}) + (assoc ::bfc/embed-assets false) + (assoc ::bfc/include-libraries false))] + (case version + 1 (v1/export-files! cfg output) + 3 (v3/export-files! cfg (io/output-stream output))) + output)) + +(defn- import-file + "Import a package of the given binfile version into the default + project of the profile; returns the ids of the imported files." + [version profile input] + (let [cfg (-> th/*system* + (assoc ::bfc/project-id (:default-project-id profile)) + (assoc ::bfc/profile-id (:id profile)) + (assoc ::bfc/team-id (:default-team-id profile)) + (assoc ::bfc/input input))] + (case version + 1 (vec (v1/import-files! cfg)) + 3 (:file-ids (v3/import-files! cfg))))) + +(defn- import-error + "The data of the error an import raises, or nil when it succeeds." + [version profile input] + (try + (import-file version profile input) + nil + (catch clojure.lang.ExceptionInfo cause + (ex-data cause)))) + +(defn- imported-features + [file-id] + (:features (bfc/get-file th/*system* file-id))) + +(defn- check-import-of-components-v1-files + [version] + (let [profile (th/create-profile* 1)] + + (t/testing "a file without components/v2 that has a component is rejected" + (let [file (prepare-file-with-component profile 1) + _ (drop-components-v2! (:id file)) + package (export-file version (:id file)) + before (count (db/query th/*system* :file {:project-id (:default-project-id profile)})) + error (import-error version profile package)] + (t/is (= :restriction (:type error))) + (t/is (= :file-in-components-v1 (:code error))) + (t/is (= before (count (db/query th/*system* :file {:project-id (:default-project-id profile)})))))) + + (t/testing "a file without components/v2 and without components is imported with it" + (let [file (prepare-simple-file profile 2) + _ (drop-components-v2! (:id file)) + package (export-file version (:id file)) + [id] (import-file version profile package)] + (t/is (contains? (imported-features id) "components/v2")))) + + (t/testing "a file with components/v2 and a component is imported" + (let [file (prepare-file-with-component profile 3) + package (export-file version (:id file)) + [id] (import-file version profile package)] + (t/is (contains? (imported-features id) "components/v2")) + (t/is (some? (get-in (bfc/get-file th/*system* id :realize? true) + [:data :components (uuid/custom 5 3)]))))))) + +(t/deftest import-binfile-v1-rejects-components-v1-files + (check-import-of-components-v1-files 1)) + +(t/deftest import-binfile-v3-rejects-components-v1-files + (check-import-of-components-v1-files 3)) + +(t/deftest check-components-v2-looks-at-components-and-component-shapes + (let [page-id (uuid/custom 1 1) + shape-id (uuid/custom 2 1) + file (fn [features shape components] + {:features features + :data {:pages [page-id] + :pages-index {page-id {:id page-id + :objects {shape-id (merge {:id shape-id} shape)}}} + :components components}}) + rejected? (fn [file] + (= :file-in-components-v1 + (try + (bfc/check-components-v2 file) + nil + (catch clojure.lang.ExceptionInfo cause + (:code (ex-data cause))))))] + + (t/testing "a file without components/v2 is rejected when it has component data" + (t/is (rejected? (file #{} {} {(uuid/custom 5 3) {:id (uuid/custom 5 3)}}))) + (t/is (rejected? (file #{} {:component-id (uuid/custom 5 3)} nil))) + (t/is (rejected? (file #{} {:shape-ref (uuid/custom 2 2)} nil))) + (t/is (rejected? (file #{} {:main-instance true} nil)))) + + (t/testing "a file without components/v2 and without component data takes the feature" + (t/is (= #{"components/v2"} + (:features (bfc/check-components-v2 (file #{} {} nil)))))) + + (t/testing "a file with components/v2 is left as it is" + (let [file (file #{"components/v2"} {:shape-ref (uuid/custom 2 2)} nil)] + (t/is (= file (bfc/check-components-v2 file))))))) diff --git a/frontend/src/app/main/data/imports.cljs b/frontend/src/app/main/data/imports.cljs index 44cdd681d3..59e1dace7f 100644 --- a/frontend/src/app/main/data/imports.cljs +++ b/frontend/src/app/main/data/imports.cljs @@ -53,7 +53,9 @@ (defn- job-message "The message of a job that failed." [error] - (or (resolve-hint error) (tr "labels.error"))) + (if (= :file-in-components-v1 (:code error)) + (tr "dashboard.import.components-v1-error") + (or (resolve-hint error) (tr "labels.error")))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; READING A PACKAGE diff --git a/frontend/test/frontend_tests/data/imports_test.cljs b/frontend/test/frontend_tests/data/imports_test.cljs index e73802fe38..e62f3b5691 100644 --- a/frontend/test/frontend_tests/data/imports_test.cljs +++ b/frontend/test/frontend_tests/data/imports_test.cljs @@ -307,6 +307,39 @@ {:libraries-resolution {}}] @seen)))))))) +(t/deftest ^:async a-components-v1-file-fails-with-its-own-message + (let [ws-stream (rx/subject) + calls (atom []) + rows (atom {}) + seen (atom []) + target (entry :name "Old")] + + (await + (mock/with-mocks* + {st/state (atom {:ws-conn ws-stream}) + http/send! (slurp-stub ::blob) + uploads/upload-blob-chunked (upload-stub (uuid/next)) + rp/cmd! (fake-server calls rows (fn [job-id] (job job-id "running"))) + ws/get-rcv-stream (ws-stub ws-stream)} + + (let [done (hva/observe (imp/import-files {:project-id project-id + :entries [target]}) + {:on-next #(swap! seen conj %)}) + job-id (:job-id (first @calls))] + + (swap! rows assoc job-id + (job job-id "failed" :error {:type :restriction + :code :file-in-components-v1 + :hint "components v1 is deprecated"})) + (rx/push! ws-stream (message (event job-id :end {:outcome "failed"}))) + (await done) + + (t/testing "the entry explains that the file is too old to import" + (t/is (= {:status :error + :file-id (:file-id target) + :error (tr "dashboard.import.components-v1-error")} + (second @seen))))))))) + (t/deftest ^:async a-package-that-cannot-be-created-fails-its-entries (let [ws-stream (rx/subject) seen (atom []) diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 3fa9a763f3..be1d2eec24 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -665,6 +665,12 @@ msgstr[1] "%s libraries were automatically linked by name." msgid "dashboard.import.bad-url" msgstr "Import failed. The template URL is incorrect" +#: src/app/main/data/imports.cljs:57 +msgid "dashboard.import.components-v1-error" +msgstr "" +"This file uses an old type of Penpot components that is no longer " +"supported, so it can't be imported." + #: src/app/main/ui/dashboard/import.cljs:733 msgid "dashboard.import.confirm-library-links" msgstr "Confirm library links" diff --git a/frontend/translations/es.po b/frontend/translations/es.po index 431886f0d1..f4be7e417f 100644 --- a/frontend/translations/es.po +++ b/frontend/translations/es.po @@ -676,6 +676,12 @@ msgstr[1] "%s bibliotecas fueron vinculadas automáticamente por nombre." msgid "dashboard.import.bad-url" msgstr "La importación ha fallado. La URL de la plantilla es incorrecta" +#: src/app/main/data/imports.cljs:57 +msgid "dashboard.import.components-v1-error" +msgstr "" +"Este fichero usa un tipo antiguo de componentes de Penpot que ya no tiene " +"soporte, así que no se puede importar." + #: src/app/main/ui/dashboard/import.cljs:733 msgid "dashboard.import.confirm-library-links" msgstr "Confirmar vínculos de biblioteca"