From 78260fbc424da748376224c70bd691da5758e760 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 15 Dec 2023 12:56:31 +0100 Subject: [PATCH 01/10] :bug: Fix no migration are applied on accessing to a file --- backend/src/app/rpc/commands/files.clj | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/backend/src/app/rpc/commands/files.clj b/backend/src/app/rpc/commands/files.clj index a0cc12e43..30dd81cae 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -250,11 +250,13 @@ file))) (defn get-file - [{:keys [::db/conn] :as cfg} id & {:keys [project-id migrate? + [{:keys [::db/conn] :as cfg} id & {:keys [project-id + migrate? include-deleted? lock-for-update?] :or {include-deleted? false - lock-for-update? false}}] + lock-for-update? false + migrate? true}}] (dm/assert! "expected cfg with valid connection" (db/connection-map? cfg)) From f01cad9ce712dc43351e6e925a594bbc091be3ba Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 15 Dec 2023 14:51:47 +0100 Subject: [PATCH 02/10] :bug: Fix incorrect error reporting on clone template error --- frontend/src/app/main/ui/dashboard/import.cljs | 12 +++++------- frontend/src/app/main/ui/dashboard/projects.cljs | 9 +++++---- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/frontend/src/app/main/ui/dashboard/import.cljs b/frontend/src/app/main/ui/dashboard/import.cljs index 426843edb..f85df2def 100644 --- a/frontend/src/app/main/ui/dashboard/import.cljs +++ b/frontend/src/app/main/ui/dashboard/import.cljs @@ -351,17 +351,15 @@ on-template-cloned-success (fn [] - (swap! state - (fn [state] - (-> state - (assoc :status :importing :importing-templates 0)))) + (swap! state assoc :status :importing :importing-templates 0) (st/emit! (dd/fetch-recent-files))) on-template-cloned-error (fn [cause] - (errors/print-cause! "Template Clone Error" cause) - (st/emit! (modal/hide) - (msg/error (tr "dashboard.libraries-and-templates.import-error")))) + (swap! state assoc :status :error :importing-templates 0) + (errors/print-error! cause) + (rx/of (modal/hide) + (msg/error (tr "dashboard.libraries-and-templates.import-error")))) continue-files (fn [] diff --git a/frontend/src/app/main/ui/dashboard/projects.cljs b/frontend/src/app/main/ui/dashboard/projects.cljs index 545d9c0aa..01538627e 100644 --- a/frontend/src/app/main/ui/dashboard/projects.cljs +++ b/frontend/src/app/main/ui/dashboard/projects.cljs @@ -15,6 +15,7 @@ [app.main.data.messages :as msg] [app.main.data.modal :as modal] [app.main.data.users :as du] + [app.main.errors :as errors] [app.main.refs :as refs] [app.main.store :as st] [app.main.ui.context :as ctx] @@ -138,10 +139,10 @@ on-template-cloned-error (mf/use-fn - (fn [] - (swap! state #(assoc % :status :waiting)) - (st/emit! - (msg/error (tr "dashboard.libraries-and-templates.import-error"))))) + (fn [cause] + (swap! state assoc :status :error) + (errors/print-error! cause) + (st/emit! (msg/error (tr "dashboard.libraries-and-templates.import-error"))))) download-tutorial (mf/use-fn From eabed4325a05a901c59d7cd32f41005369035372 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 15 Dec 2023 14:52:46 +0100 Subject: [PATCH 03/10] :sparkles: Integrate feature handling on file data migration Make it less error prone --- common/src/app/common/files/migrations.cljc | 23 ++++++++++++--------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index 9411566f1..f26a689f4 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -42,18 +42,21 @@ (reduce migrate-fn data (range (:version data 0) to-version)))))) (defn migrate-file - [{:keys [id data] :as file}] - (let [data (assoc data :id id)] - (-> file - (assoc ::orig-version (:version data)) - (assoc :data (migrate-data data))))) + [{:keys [id data features] :as file}] + (binding [cfeat/*new* (atom #{})] + (let [file (-> file + (update :data assoc :id id) + (update :data migrate-data) + (update :features (fnil into #{}) (deref cfeat/*new*)) + (update :features cfeat/migrate-legacy-features))] + (if (or (not= (:version data) (:version (:data file))) + (not= features (:features file))) + (vary-meta file assoc ::migrated true) + file)))) (defn migrated? - [{:keys [data] :as file}] - (or (::migrated file) - (let [version (:version data)] - (> version - (::orig-version file version))))) + [file] + (true? (-> file meta ::migrated))) ;; Default handler, noop (defmethod migrate :default [data] data) From da15924de0feeb3914d1f323e3bff8656dd60620 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 15 Dec 2023 14:53:46 +0100 Subject: [PATCH 04/10] :fire: Remove empty lines on ui.dashboard ns file --- frontend/src/app/main/ui/dashboard.cljs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frontend/src/app/main/ui/dashboard.cljs b/frontend/src/app/main/ui/dashboard.cljs index 54894422e..6905c68bc 100644 --- a/frontend/src/app/main/ui/dashboard.cljs +++ b/frontend/src/app/main/ui/dashboard.cljs @@ -65,7 +65,6 @@ (d/seek :is-default) (:id))) - on-resize (mf/use-fn (fn [_] @@ -256,7 +255,7 @@ :section section :search-term search-term :team team}])])]] - + [:& (mf/provider ctx/current-team-id) {:value team-id} [:& (mf/provider ctx/current-project-id) {:value project-id} ;; NOTE: dashboard events and other related functions assumes From eee28a57931c2fe32ba55271b8ae98ff49b46579 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 15 Dec 2023 14:59:00 +0100 Subject: [PATCH 05/10] :sparkles: Simplify feature handling on components-v2 migration functions --- backend/src/app/features/components_v2.clj | 26 ++++++++-------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 19b4f0b9d..99074dd59 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -9,7 +9,6 @@ [app.common.data :as d] [app.common.data.macros :as dm] [app.common.exceptions :as ex] - [app.common.features :as cfeat] [app.common.files.changes :as cp] [app.common.files.changes-builder :as fcb] [app.common.files.helpers :as cfh] @@ -769,12 +768,13 @@ fdata (migrate-graphics fdata)] (update fdata :options assoc :components-v2 true))))) -(defn- prepare-fdata - [fdata id] - (-> fdata - (assoc :id id) - (fdata/process-pointers deref) - (fmg/migrate-data))) +(defn- get-file + [system id] + (binding [pmap/*load-fn* (partial fdata/load-pointer system id)] + (-> (files/get-file system id :migrate? false) + (update :data assoc :id id) + (update :data fdata/process-pointers deref) + (fmg/migrate-file)))) (defn- validate-file! [file libs throw-on-validate?] @@ -788,18 +788,10 @@ (defn- process-file [{:keys [::db/conn] :as system} id & {:keys [validate? throw-on-validate?]}] - (let [file (binding [cfeat/*new* (atom #{}) - pmap/*load-fn* (partial fdata/load-pointer system id)] - (-> (files/get-file system id :migrate? false) - (update :data prepare-fdata id) - (update :features into (deref cfeat/*new*)) - (update :features cfeat/migrate-legacy-features))) + (let [file (get-file system id) libs (->> (files/get-file-libraries conn id) - (into [file] (map (fn [{:keys [id]}] - (binding [pmap/*load-fn* (partial fdata/load-pointer system id)] - (-> (files/get-file system id :migrate? false) - (update :data prepare-fdata id)))))) + (into [file] (comp (map :id) (map (partial get-file system)))) (d/index-by :id)) file (-> file From 47baa21d53ee59d45d1592cf76895088fd3f0f91 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 15 Dec 2023 15:00:59 +0100 Subject: [PATCH 06/10] :bug: Fix some edge cases on feature handling on binfile import process --- backend/src/app/rpc/commands/binfile.clj | 88 +++++++++++++----------- 1 file changed, 46 insertions(+), 42 deletions(-) diff --git a/backend/src/app/rpc/commands/binfile.clj b/backend/src/app/rpc/commands/binfile.clj index 13a16c277..253d77fba 100644 --- a/backend/src/app/rpc/commands/binfile.clj +++ b/backend/src/app/rpc/commands/binfile.clj @@ -11,7 +11,7 @@ [app.common.exceptions :as ex] [app.common.features :as cfeat] [app.common.files.defaults :as cfd] - [app.common.files.migrations :as pmg] + [app.common.files.migrations :as fmg] [app.common.files.validate :as fval] [app.common.fressian :as fres] [app.common.logging :as l] @@ -701,23 +701,28 @@ (update :object-id #(str/replace-first % #"^(.*?)/" (str file-id "/"))))) thumbnails)) -(defn- process-fdata - [fdata id] - (-> fdata - (dissoc :recent-colors) - (assoc :id id) - (cond-> (> (:version fdata) cfd/version) - (assoc :version cfd/version)) - ;; FIXME: We're temporarily activating all - ;; migrations because a problem in the - ;; environments messed up with the version - ;; numbers When this problem is fixed delete - ;; the following line - (assoc :version 22) - (pmg/migrate-data) - (update :pages-index relink-shapes) - (update :components relink-shapes) - (update :media relink-media))) +(defn- process-file + [{:keys [id] :as file}] + (-> file + (update :data (fn [fdata] + (-> fdata + (assoc :id id) + (dissoc :recent-colors) + (cond-> (> (:version fdata) cfd/version) + (assoc :version cfd/version)) + ;; FIXME: We're temporarily activating all + ;; migrations because a problem in the + ;; environments messed up with the version + ;; numbers When this problem is fixed delete + ;; the following line + (cond-> (> (:version fdata) 22) + (assoc :version 22))))) + (fmg/migrate-file) + (update :data (fn [fdata] + (-> fdata + (update :pages-index relink-shapes) + (update :components relink-shapes) + (update :media relink-media)))))) (defmethod read-section :v1/files @@ -730,19 +735,7 @@ file-id (:id file) file-id' (lookup-index file-id) - thumbnails (:thumbnails file) - file (update file :features cfeat/migrate-legacy-features) - - features (-> enabled-features - (set/difference cfeat/frontend-only-features) - (set/union (cfeat/check-supported-features! (:features file))))] - - ;; All features that are enabled and requires explicit migration - ;; are added to the state for a posterior migration step - (doseq [feature (-> enabled-features - (set/difference cfeat/no-migration-features) - (set/difference (:features file)))] - (vswap! *state* update :pending-to-migrate (fnil conj []) [feature file-id'])) + thumbnails (:thumbnails file)] (when (not= file-id expected-file-id) (ex/raise :type :validation @@ -773,16 +766,28 @@ (l/dbg :hint "update media references" ::l/sync? true) (vswap! *state* update :media into (map #(update % :id lookup-index)) media)) - (let [file (binding [cfeat/*new* (atom #{})] - (-> file - (assoc :id file-id') - (assoc :features features) - (assoc :project-id project-id) - (assoc :created-at timestamp) - (assoc :modified-at timestamp) - (dissoc :thumbnails) - (update :data process-fdata file-id') - (update :features into (deref cfeat/*new*)))) + (let [file (-> file + (assoc :id file-id') + (process-file)) + + ;; All features that are enabled and requires explicit migration are + ;; added to the state for a posterior migration step. + _ (doseq [feature (-> enabled-features + (set/difference cfeat/no-migration-features) + (set/difference (:features file)))] + (vswap! *state* update :pending-to-migrate (fnil conj []) [feature file-id'])) + + file (-> file + (assoc :project-id project-id) + (assoc :created-at timestamp) + (assoc :modified-at timestamp) + (dissoc :thumbnails) + (update :features + (fn [features] + (let [features (cfeat/check-supported-features! features)] + (-> enabled-features + (set/difference cfeat/frontend-only-features) + (set/union features)))))) _ (when (contains? cf/flags :file-schema-validation) (fval/validate-file-schema! file)) @@ -803,7 +808,6 @@ file)) file) - file (-> file (update :features #(db/create-array conn "text" %)) (update :data blob/encode))] From ac20451ae76e044343a3847e44e8802f9e50592a Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 15 Dec 2023 15:01:50 +0100 Subject: [PATCH 07/10] :sparkles: Simplify feature handling on get-file --- backend/src/app/rpc/commands/files.clj | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/backend/src/app/rpc/commands/files.clj b/backend/src/app/rpc/commands/files.clj index 30dd81cae..096e96195 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -224,11 +224,8 @@ (defn- migrate-file [{:keys [::db/conn] :as cfg} {:keys [id] :as file}] (binding [pmap/*load-fn* (partial feat.fdata/load-pointer cfg id) - pmap/*tracked* (pmap/create-tracked) - cfeat/*new* (atom #{})] - (let [file (-> (fmg/migrate-file file) - (update :features into (deref cfeat/*new*)) - (update :features cfeat/migrate-legacy-features))] + pmap/*tracked* (pmap/create-tracked)] + (let [file (fmg/migrate-file file)] ;; NOTE: when file is migrated, we break the rule of no perform ;; mutations on get operations and update the file with all From 0ad2e8a0f2167d2becb1e493cfb82ed65bfe0deb Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 15 Dec 2023 15:02:33 +0100 Subject: [PATCH 08/10] :sparkles: Make retrieving fdata for thumbnail to no modify the file This prevents that file to be considered opened just for creating the thumbnail for it. --- .../src/app/rpc/commands/files_thumbnails.clj | 69 ++++++++----------- 1 file changed, 30 insertions(+), 39 deletions(-) diff --git a/backend/src/app/rpc/commands/files_thumbnails.clj b/backend/src/app/rpc/commands/files_thumbnails.clj index c60338349..19f36072f 100644 --- a/backend/src/app/rpc/commands/files_thumbnails.clj +++ b/backend/src/app/rpc/commands/files_thumbnails.clj @@ -10,6 +10,7 @@ [app.common.data.macros :as dm] [app.common.features :as cfeat] [app.common.files.helpers :as cfh] + [app.common.files.migrations :as fmg] [app.common.geom.shapes :as gsh] [app.common.schema :as sm] [app.common.thumbnails :as thc] @@ -105,24 +106,12 @@ (letfn [;; function responsible on finding the frame marked to be ;; used as thumbnail; the returned frame always have ;; the :page-id set to the page that it belongs. - - (get-thumbnail-frame [file] - ;; NOTE: this is a hack for avoid perform blocking - ;; operation inside the for loop, clojure lazy-seq uses - ;; synchronized blocks that does not plays well with - ;; virtual threads where all rpc methods calls are - ;; dispatched, so we need to perform the load operation - ;; first. This operation forces all pointer maps load into - ;; the memory. - ;; - ;; FIXME: this is no longer true with clojure>=1.12 - (let [{:keys [data]} (update file :data feat.fdata/process-pointers pmap/load!)] - ;; Then proceed to find the frame set for thumbnail - (d/seek #(or (:use-for-thumbnail %) - (:use-for-thumbnail? %)) ; NOTE: backward comp (remove on v1.21) - (for [page (-> data :pages-index vals) - frame (-> page :objects ctt/get-frames)] - (assoc frame :page-id (:id page)))))) + (get-thumbnail-frame [{:keys [data]}] + (d/seek #(or (:use-for-thumbnail %) + (:use-for-thumbnail? %)) ; NOTE: backward comp (remove on v1.21) + (for [page (-> data :pages-index vals) + frame (-> page :objects ctt/get-frames)] + (assoc frame :page-id (:id page))))) ;; function responsible to filter objects data structure of ;; all unneeded shapes if a concrete frame is provided. If no @@ -166,30 +155,29 @@ objects)))] - (binding [pmap/*load-fn* (partial feat.fdata/load-pointer cfg id)] - (let [frame (get-thumbnail-frame file) - frame-id (:id frame) - page-id (or (:page-id frame) - (-> data :pages first)) + (let [frame (get-thumbnail-frame file) + frame-id (:id frame) + page-id (or (:page-id frame) + (-> data :pages first)) - page (dm/get-in data [:pages-index page-id]) - page (cond-> page (pmap/pointer-map? page) deref) - frame-ids (if (some? frame) (list frame-id) (map :id (ctt/get-frames (:objects page)))) + page (dm/get-in data [:pages-index page-id]) + page (cond-> page (pmap/pointer-map? page) deref) + frame-ids (if (some? frame) (list frame-id) (map :id (ctt/get-frames (:objects page)))) - obj-ids (map #(thc/fmt-object-id (:id file) page-id % "frame") frame-ids) - thumbs (get-object-thumbnails conn id obj-ids)] + obj-ids (map #(thc/fmt-object-id (:id file) page-id % "frame") frame-ids) + thumbs (get-object-thumbnails conn id obj-ids)] - (cond-> page - ;; If we have frame, we need to specify it on the page level - ;; and remove the all other unrelated objects. - (some? frame-id) - (-> (assoc :thumbnail-frame-id frame-id) - (update :objects filter-objects frame-id)) + (cond-> page + ;; If we have frame, we need to specify it on the page level + ;; and remove the all other unrelated objects. + (some? frame-id) + (-> (assoc :thumbnail-frame-id frame-id) + (update :objects filter-objects frame-id)) - ;; Assoc the available thumbnails and prune not visible shapes - ;; for avoid transfer unnecessary data. - :always - (update :objects assoc-thumbnails page-id thumbs)))))) + ;; Assoc the available thumbnails and prune not visible shapes + ;; for avoid transfer unnecessary data. + :always + (update :objects assoc-thumbnails page-id thumbs))))) (def ^:private schema:get-file-data-for-thumbnail @@ -221,7 +209,10 @@ :profile-id profile-id :file-id file-id) - file (files/get-file cfg file-id)] + file (binding [pmap/*load-fn* (partial feat.fdata/load-pointer cfg file-id)] + (-> (files/get-file cfg file-id :migrate? false) + (update :data feat.fdata/process-pointers deref) + (fmg/migrate-file)))] (-> (cfeat/get-team-enabled-features cf/flags team) (cfeat/check-client-features! (:features params)) From ca50486639cc6a649c1357b25dcd7a387f6bf6ac Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 15 Dec 2023 15:04:01 +0100 Subject: [PATCH 09/10] :sparkles: Simplify feature handling on duplicate-file --- backend/src/app/rpc/commands/management.clj | 57 +++++++++------------ 1 file changed, 25 insertions(+), 32 deletions(-) diff --git a/backend/src/app/rpc/commands/management.clj b/backend/src/app/rpc/commands/management.clj index 2928830ca..80287e9ae 100644 --- a/backend/src/app/rpc/commands/management.clj +++ b/backend/src/app/rpc/commands/management.clj @@ -106,41 +106,34 @@ media media)) - (update-fdata [fdata new-id] - (-> fdata - (assoc :id new-id) - (feat.fdata/process-pointers deref) - (pmg/migrate-data) - (update :pages-index relink-shapes) - (update :components relink-shapes) - (update :media relink-media) - (d/without-nils)))] + (process-file [{:keys [id] :as file}] + (-> file + (update :data assoc :id id) + (update :data feat.fdata/process-pointers deref) + (pmg/migrate-file) + (update :data (fn [data] + (-> data + (update :pages-index relink-shapes) + (update :components relink-shapes) + (update :media relink-media) + (d/without-nils))))))] - (binding [pmap/*load-fn* (partial feat.fdata/load-pointer cfg id) - pmap/*tracked* (pmap/create-tracked) - cfeat/*new* (atom #{})] + (let [new-id (get index id) + file (binding [pmap/*load-fn* (partial feat.fdata/load-pointer cfg id)] + (-> (assoc file :id new-id) + (process-file))) - (let [new-id (get index id) - file (binding [pmap/*load-fn* (partial feat.fdata/load-pointer cfg id) - cfeat/*new* (atom #{})] - (-> file - (assoc :id new-id) - (update :data update-fdata new-id) - (update :features into (deref cfeat/*new*)) - (update :features cfeat/migrate-legacy-features))) + file (if (contains? (:features file) "fdata/objects-map") + (feat.fdata/enable-objects-map file) + file) - file (if (contains? (:features file) "fdata/objects-map") - (feat.fdata/enable-objects-map file) - file) - - file (if (contains? (:features file) "fdata/pointer-map") - (binding [pmap/*tracked* (pmap/create-tracked)] - (let [file (feat.fdata/enable-pointer-map file)] - (feat.fdata/persist-pointers! cfg (:id file)) - file)) - file)] - - file)))) + file (if (contains? (:features file) "fdata/pointer-map") + (binding [pmap/*tracked* (pmap/create-tracked)] + (let [file (feat.fdata/enable-pointer-map file)] + (feat.fdata/persist-pointers! cfg (:id file)) + file)) + file)] + file))) (def sql:get-used-libraries "select flr.* From a9dd55b8d2556d6a2e2ab7fa4931346c588fb333 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 15 Dec 2023 16:03:30 +0100 Subject: [PATCH 10/10] :bug: Fix incorrect feature detection on frontend code --- frontend/src/app/main/features.cljs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/frontend/src/app/main/features.cljs b/frontend/src/app/main/features.cljs index 9c6ff683e..a23b2a27d 100644 --- a/frontend/src/app/main/features.cljs +++ b/frontend/src/app/main/features.cljs @@ -32,11 +32,11 @@ (defn get-team-enabled-features [state] - (let [runtime-features (set/intersection (:features/runtime state #{}) - cfeat/no-migration-features)] - (-> global-enabled-features - (set/union runtime-features) - (set/union (:features/team state #{}))))) + (-> global-enabled-features + (set/union (:features/runtime state #{})) + (set/intersection cfeat/no-migration-features) + (set/union cfeat/default-enabled-features) + (set/union (:features/team state #{})))) (def features-ref (l/derived get-team-enabled-features st/state =)) @@ -99,7 +99,7 @@ ptk/UpdateEvent (update [_ state] (let [runtime-features (get state :features/runtime #{}) - team-features (into cfeat/default-enabled-features + team-features (into #{} cfeat/xf-supported-features team-features)] (-> state