From 611594a3929915602124481f6fbf8fb8f577e205 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 13 Dec 2023 21:17:28 +0100 Subject: [PATCH] :sparkles: Add general features handling improvements --- backend/src/app/features/components_v2.clj | 65 +++++++------- backend/src/app/rpc/commands/binfile.clj | 85 +++++++++---------- backend/src/app/rpc/commands/files.clj | 27 +++--- backend/src/app/rpc/commands/files_update.clj | 53 ++++++------ backend/src/app/rpc/commands/management.clj | 40 ++++++--- common/src/app/common/features.cljc | 5 +- 6 files changed, 149 insertions(+), 126 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 974f56434..19b4f0b9d 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -769,7 +769,7 @@ fdata (migrate-graphics fdata)] (update fdata :options assoc :components-v2 true))))) -(defn- process-fdata +(defn- prepare-fdata [fdata id] (-> fdata (assoc :id id) @@ -788,43 +788,46 @@ (defn- process-file [{:keys [::db/conn] :as system} id & {:keys [validate? throw-on-validate?]}] - (binding [pmap/*tracked* (pmap/create-tracked) - pmap/*load-fn* (partial fdata/load-pointer *system* id)] + (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 (binding [cfeat/*new* (atom #{})] - (-> (files/get-file system id :migrate? false) - (update :data process-fdata id) - (update :features into (deref cfeat/*new*)) - (update :features cfeat/migrate-legacy-features))) + 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)))))) + (d/index-by :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 process-fdata id)))))) - (d/index-by :id)) + file (-> file + (update :data migrate-fdata libs) + (update :features conj "components/v2")) - pmap? (contains? (:features file) "fdata/pointer-map") + _ (when validate? + (validate-file! file libs throw-on-validate?)) - file (-> file - (update :data migrate-fdata libs) - (update :features conj "components/v2") - (cond-> pmap? (fdata/enable-pointer-map))) - ] + file (if (contains? (:features file) "fdata/objects-map") + (fdata/enable-objects-map file) + file) - (when validate? - (validate-file! file libs throw-on-validate?)) + file (if (contains? (:features file) "fdata/pointer-map") + (binding [pmap/*tracked* (pmap/create-tracked)] + (let [file (fdata/enable-pointer-map file)] + (fdata/persist-pointers! system id) + file)) + file)] - (db/update! conn :file - {:data (blob/encode (:data file)) - :features (db/create-array conn "text" (:features file)) - :revn (:revn file)} - {:id (:id file)}) + (db/update! conn :file + {:data (blob/encode (:data file)) + :features (db/create-array conn "text" (:features file)) + :revn (:revn file)} + {:id (:id file)} + {::db/return-keys? false}) - (when pmap? - (fdata/persist-pointers! system id)) - - (dissoc file :data)))) + (dissoc file :data))) (defn migrate-file! [system file-id & {:keys [validate? throw-on-validate?]}] diff --git a/backend/src/app/rpc/commands/binfile.clj b/backend/src/app/rpc/commands/binfile.clj index 2dc76bf96..13a16c277 100644 --- a/backend/src/app/rpc/commands/binfile.clj +++ b/backend/src/app/rpc/commands/binfile.clj @@ -701,6 +701,25 @@ (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))) + + (defmethod read-section :v1/files [{:keys [::db/conn ::input ::project-id ::enabled-features ::timestamp ::overwrite?] :as system}] @@ -754,58 +773,40 @@ (l/dbg :hint "update media references" ::l/sync? true) (vswap! *state* update :media into (map #(update % :id lookup-index)) media)) - (binding [pmap/*tracked* (pmap/create-tracked) - cfeat/*new* (atom #{})] - (let [file (-> file + (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 (fn [data] - (-> data - (dissoc :recent-colors) - (assoc :id file-id') - (cond-> (> (:version data) cfd/version) - (assoc :version cfd/version)) + (update :data process-fdata file-id') + (update :features into (deref cfeat/*new*)))) - ;; 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) - (update :pages-index relink-shapes) - (update :components relink-shapes) - (update :media relink-media) - (pmg/migrate-data) - (d/without-nils))))) + _ (when (contains? cf/flags :file-schema-validation) + (fval/validate-file-schema! file)) - ;; Add to file features all possible features added on - ;; migration process. - file (update file :features into (deref cfeat/*new*)) + _ (when (contains? cf/flags :soft-file-schema-validation) + (let [result (ex/try! (fval/validate-file-schema! file))] + (when (ex/exception? result) + (l/error :hint "file schema validation error" :cause result)))) - file (if (contains? cf/flags :file-schema-validation) - (fval/validate-file-schema! file) - file) + file (if (contains? (:features file) "fdata/objects-map") + (feat.fdata/enable-objects-map file) + file) - _ (when (contains? cf/flags :soft-file-schema-validation) - (try - (fval/validate-file-schema! file) - (catch Throwable cause - (l/error :hint "file schema validation error" :cause cause)))) + 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! system file-id') + file)) + file) - file (cond-> file - (contains? (:features file) "fdata/objects-map") - (feat.fdata/enable-objects-map) - (contains? (:features file) "fdata/pointer-map") - (feat.fdata/enable-pointer-map)) - - file (-> file - (update :features #(db/create-array conn "text" %)) - (update :data blob/encode))] + file (-> file + (update :features #(db/create-array conn "text" %)) + (update :data blob/encode))] (l/dbg :hint "create file" :id (str file-id') ::l/sync? true) @@ -813,12 +814,10 @@ (create-or-update-file! conn file) (db/insert! conn :file file)) - (feat.fdata/persist-pointers! system file-id') - (when overwrite? (db/delete! conn :file-thumbnail {:file-id file-id'})) - file-id'))))) + file-id')))) (defmethod read-section :v1/rels [{:keys [::db/conn ::input ::timestamp]}] diff --git a/backend/src/app/rpc/commands/files.clj b/backend/src/app/rpc/commands/files.clj index 4b56df77d..a0cc12e43 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -34,7 +34,6 @@ [app.util.pointer-map :as pmap] [app.util.services :as sv] [app.util.time :as dt] - [clojure.set :as set] [clojure.spec.alpha :as s] [cuerdas.core :as str])) @@ -227,7 +226,10 @@ (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)] + (let [file (-> (fmg/migrate-file file) + (update :features into (deref cfeat/*new*)) + (update :features cfeat/migrate-legacy-features))] + ;; NOTE: when file is migrated, we break the rule of no perform ;; mutations on get operations and update the file with all ;; migrations applied @@ -235,16 +237,17 @@ ;; NOTE: the following code will not work on read-only mode, it ;; is a known issue; we keep is not implemented until we really ;; need this - (if (fmg/migrated? file) - (let [file (update file :features cfeat/migrate-legacy-features) - features (set/union (deref cfeat/*new*) (:features file))] - (db/update! conn :file - {:data (blob/encode (:data file)) - :features (db/create-array conn "text" features)} - {:id id}) - (feat.fdata/persist-pointers! cfg id) - (assoc file :features features)) - file)))) + (when (fmg/migrated? file) + (db/update! conn :file + {:data (blob/encode (:data file)) + :features (db/create-array conn "text" (:features file))} + {:id id} + {::db/return-keys? false}) + + (when (contains? (:features file) "fdata/pointer-map") + (feat.fdata/persist-pointers! cfg id))) + + file))) (defn get-file [{:keys [::db/conn] :as cfg} id & {:keys [project-id migrate? diff --git a/backend/src/app/rpc/commands/files_update.clj b/backend/src/app/rpc/commands/files_update.clj index 8843463b1..7a27a834e 100644 --- a/backend/src/app/rpc/commands/files_update.clj +++ b/backend/src/app/rpc/commands/files_update.clj @@ -182,40 +182,39 @@ (defn update-file [{:keys [::db/conn ::mtx/metrics] :as cfg} {:keys [id file features changes changes-with-metadata] :as params}] - (binding [cfeat/*current* features - cfeat/*previous* (:features file)] + (let [features (-> features + (set/difference cfeat/frontend-only-features) + (set/union (:features file))) - (let [features (-> features - (set/difference cfeat/frontend-only-features) - (set/union (:features file))) + update-fn (cond-> update-file* + (contains? features "fdata/pointer-map") + (wrap-with-pointer-map-context) - update-fn (cond-> update-file* - (contains? features "fdata/pointer-map") - (wrap-with-pointer-map-context) + (contains? features "fdata/objects-map") + (wrap-with-objects-map-context)) - (contains? features "fdata/objects-map") - (wrap-with-objects-map-context)) + changes (if changes-with-metadata + (->> changes-with-metadata (mapcat :changes) vec) + (vec changes))] - changes (if changes-with-metadata - (->> changes-with-metadata (mapcat :changes) vec) - (vec changes))] + (when (> (:revn params) + (:revn file)) + (ex/raise :type :validation + :code :revn-conflict + :hint "The incoming revision number is greater that stored version." + :context {:incoming-revn (:revn params) + :stored-revn (:revn file)})) - (when (> (:revn params) - (:revn file)) - (ex/raise :type :validation - :code :revn-conflict - :hint "The incoming revision number is greater that stored version." - :context {:incoming-revn (:revn params) - :stored-revn (:revn file)})) + (mtx/run! metrics {:id :update-file-changes :inc (count changes)}) - (mtx/run! metrics {:id :update-file-changes :inc (count changes)}) - - (when (not= features (:features file)) - (let [features (db/create-array conn "text" features)] - (db/update! conn :file - {:features features} - {:id id}))) + (when (not= features (:features file)) + (let [features (db/create-array conn "text" features)] + (db/update! conn :file + {:features features} + {:id id}))) + (binding [cfeat/*current* features + cfeat/*previous* (:features file)] (let [file (assoc file :features features) params (-> params (assoc :file file) diff --git a/backend/src/app/rpc/commands/management.clj b/backend/src/app/rpc/commands/management.clj index 7b8a03bc3..2928830ca 100644 --- a/backend/src/app/rpc/commands/management.clj +++ b/backend/src/app/rpc/commands/management.clj @@ -109,23 +109,37 @@ (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) - (feat.fdata/process-pointers pmap/clone)))] + (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 (-> file - (assoc :id new-id) - (update :data update-fdata new-id) - (update :features into (deref cfeat/*new*)) - (update :features cfeat/migrate-legacy-features))] - (feat.fdata/persist-pointers! cfg new-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/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 @@ -191,20 +205,22 @@ (db/insert! conn :file (-> file (update :features #(db/create-array conn "text" %)) - (update :data blob/encode))) + (update :data blob/encode)) + {::db/return-keys? false}) (db/insert! conn :file-profile-rel {:file-id (:id file) :profile-id profile-id :is-owner true :is-admin true - :can-edit true}) + :can-edit true} + {::db/return-keys? false}) (doseq [params flibs] - (db/insert! conn :file-library-rel params)) + (db/insert! conn :file-library-rel params ::db/return-keys? false)) (doseq [params fmeds] - (db/insert! conn :file-media-object params)) + (db/insert! conn :file-media-object params ::db/return-keys? false)) file)) diff --git a/common/src/app/common/features.cljc b/common/src/app/common/features.cljc index 9004375b8..d627d1255 100644 --- a/common/src/app/common/features.cljc +++ b/common/src/app/common/features.cljc @@ -51,7 +51,10 @@ "layout/grid"}) ;; A set of features enabled by default for each file, they are -;; implicit and are enabled by default and can't be disabled +;; implicit and are enabled by default and can't be disabled. The +;; features listed in this set are mainly freatures addedby file +;; migrations process, so all features referenced in migrations should +;; be here. (def default-enabled-features #{"fdata/shape-data-type"})