From b718a282e0f8e7310c212ae0adfdd632bad26313 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 14 Feb 2024 11:37:27 +0100 Subject: [PATCH 1/5] :recycle: Add minor refactor to file migrations Relevant changes: - Add the ability to create migration in both directions, defaulting to identity if not provided - Move the version attribute to file table column for to make it more accessible (previously it was on data blob) - Reduce db update operations on file-update rpc method --- backend/resources/app/templates/debug.tmpl | 2 +- backend/resources/log4j2-devenv.xml | 2 +- backend/src/app/binfile/common.clj | 32 +-- backend/src/app/features/components_v2.clj | 12 +- backend/src/app/http/debug.clj | 8 +- backend/src/app/migrations.clj | 5 +- .../migrations/sql/0119-mod-file-table.sql | 2 + backend/src/app/rpc/commands/files.clj | 10 +- backend/src/app/rpc/commands/files_create.clj | 2 + backend/src/app/rpc/commands/files_update.clj | 15 +- backend/src/app/srepl/helpers.clj | 1 + backend/src/app/tasks/file_gc.clj | 5 +- .../rpc_file_thumbnails_test.clj | 1 - common/src/app/common/files/migrations.cljc | 234 +++++++++++------- common/src/app/common/types/file.cljc | 6 +- .../common_tests/files_migrations_test.cljc | 54 ++-- 16 files changed, 240 insertions(+), 151 deletions(-) create mode 100644 backend/src/app/migrations/sql/0119-mod-file-table.sql diff --git a/backend/resources/app/templates/debug.tmpl b/backend/resources/app/templates/debug.tmpl index caede1af8..d408d5d9d 100644 --- a/backend/resources/app/templates/debug.tmpl +++ b/backend/resources/app/templates/debug.tmpl @@ -157,7 +157,7 @@ Debug Main Page Reset file version Allows reset file data version to a specific number/ -
+
diff --git a/backend/resources/log4j2-devenv.xml b/backend/resources/log4j2-devenv.xml index 46c0ae150..62c15a0e4 100644 --- a/backend/resources/log4j2-devenv.xml +++ b/backend/resources/log4j2-devenv.xml @@ -31,7 +31,7 @@ - + diff --git a/backend/src/app/binfile/common.clj b/backend/src/app/binfile/common.clj index 4f9af0e7f..d5a4241e2 100644 --- a/backend/src/app/binfile/common.clj +++ b/backend/src/app/binfile/common.clj @@ -12,7 +12,6 @@ [app.common.data.macros :as dm] [app.common.exceptions :as ex] [app.common.features :as cfeat] - [app.common.files.defaults :as cfd] [app.common.files.migrations :as fmg] [app.common.files.validate :as fval] [app.common.logging :as l] @@ -381,23 +380,24 @@ (d/group-by first rest) (reduce (partial process-group-of-assets) data))))) +(defn- fix-version + [file] + (let [file (fmg/fix-version file)] + ;; 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 + (if (> (:version file) 22) + (assoc file :version 22) + file))) (defn process-file [{:keys [id] :as file}] (-> file + (fix-version) (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))))) + (dissoc :recent-colors)))) (fmg/migrate-file) (update :data (fn [fdata] (-> fdata @@ -409,19 +409,21 @@ (defn- upsert-file! [conn file] - (let [sql (str "INSERT INTO file (id, project_id, name, revn, is_shared, data, created_at, modified_at) " - "VALUES (?, ?, ?, ?, ?, ?, ?, ?) " - "ON CONFLICT (id) DO UPDATE SET data=?")] + (let [sql (str "INSERT INTO file (id, project_id, name, revn, version, is_shared, data, created_at, modified_at) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) " + "ON CONFLICT (id) DO UPDATE SET data=?, version=?")] (db/exec-one! conn [sql (:id file) (:project-id file) (:name file) (:revn file) + (:version file) (:is-shared file) (:data file) (:created-at file) (:modified-at file) - (:data file)]))) + (:data file) + (:version file)]))) (defn persist-file! "Applies all the final validations and perist the file." diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 6ae8d82e1..8c7933160 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -1496,6 +1496,13 @@ fdata (migrate-graphics fdata)] (update fdata :options assoc :components-v2 true))))) +(defn- fix-version + [file] + (let [file (fmg/fix-version file)] + (if (> (:version file) 22) + (assoc file :version 22) + file))) + (defn- get-file [system id] (binding [pmap/*load-fn* (partial fdata/load-pointer system id)] @@ -1506,10 +1513,7 @@ (update :data assoc :id id) (update :data fdata/process-pointers deref) (update :data fdata/process-objects (partial into {})) - (update :data (fn [data] - (if (> (:version data) 22) - (assoc data :version 22) - data))) + (fix-version) (fmg/migrate-file)))) (defn get-team diff --git a/backend/src/app/http/debug.clj b/backend/src/app/http/debug.clj index e7763fce6..1d2a129cf 100644 --- a/backend/src/app/http/debug.clj +++ b/backend/src/app/http/debug.clj @@ -393,7 +393,7 @@ ::rres/body (str/ffmt "PROFILE '%' ACTIVATED" (:email profile))})))) -(defn- reset-file-data-version +(defn- reset-file-version [cfg {:keys [params] :as request}] (let [file-id (some-> params :file-id d/parse-uuid) version (some-> params :version d/parse-integer)] @@ -413,7 +413,7 @@ :code :invalid-version :hint "provided invalid version")) - (db/tx-run! cfg srepl/process-file! file-id #(update % :data assoc :version version)) + (db/tx-run! cfg srepl/process-file! file-id #(assoc % :version version)) {::rres/status 200 ::rres/headers {"content-type" "text/plain"} @@ -486,8 +486,8 @@ ["/error" {:handler (partial error-list-handler cfg)}] ["/actions/resend-email-verification" {:handler (partial resend-email-notification cfg)}] - ["/actions/reset-file-data-version" - {:handler (partial reset-file-data-version cfg)}] + ["/actions/reset-file-version" + {:handler (partial reset-file-version cfg)}] ["/file/export" {:handler (partial export-handler cfg)}] ["/file/import" {:handler (partial import-handler cfg)}] ["/file/data" {:handler (partial file-data-handler cfg)}] diff --git a/backend/src/app/migrations.clj b/backend/src/app/migrations.clj index 9cfcf9fe2..87f3d90b9 100644 --- a/backend/src/app/migrations.clj +++ b/backend/src/app/migrations.clj @@ -373,7 +373,10 @@ :fn (mg/resource "app/migrations/sql/0117-mod-file-object-thumbnail-table.sql")} {:name "0118-mod-task-table" - :fn (mg/resource "app/migrations/sql/0118-mod-task-table.sql")}]) + :fn (mg/resource "app/migrations/sql/0118-mod-task-table.sql")} + + {:name "0119-mod-file-table" + :fn (mg/resource "app/migrations/sql/0119-mod-file-table.sql")}]) (defn apply-migrations! [pool name migrations] diff --git a/backend/src/app/migrations/sql/0119-mod-file-table.sql b/backend/src/app/migrations/sql/0119-mod-file-table.sql new file mode 100644 index 000000000..28a296a91 --- /dev/null +++ b/backend/src/app/migrations/sql/0119-mod-file-table.sql @@ -0,0 +1,2 @@ +ALTER TABLE file + ADD COLUMN version integer NULL; diff --git a/backend/src/app/rpc/commands/files.clj b/backend/src/app/rpc/commands/files.clj index 3f7313291..368760f9e 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -71,19 +71,14 @@ data (assoc :data (blob/decode data))))) (defn check-version! - [{:keys [data] :as file}] - (dm/assert! - "expect data to be decoded" - (map? data)) - - (let [version (:version data 0)] + [file] + (let [version (:version file)] (when (> version fmg/version) (ex/raise :type :restriction :code :file-version-not-supported :hint "file version is greated that the maximum" :file-version version :max-version fmg/version)) - file)) ;; --- FILE PERMISSIONS @@ -252,6 +247,7 @@ (db/update! conn :file {:data (blob/encode (:data file)) + :version (:version file) :features (db/create-array conn "text" (:features file))} {:id id}) diff --git a/backend/src/app/rpc/commands/files_create.clj b/backend/src/app/rpc/commands/files_create.clj index dbbd1c04d..cc15830d4 100644 --- a/backend/src/app/rpc/commands/files_create.clj +++ b/backend/src/app/rpc/commands/files_create.clj @@ -9,6 +9,7 @@ [app.common.data :as d] [app.common.data.macros :as dm] [app.common.features :as cfeat] + [app.common.files.defaults :refer [version]] [app.common.schema :as sm] [app.common.types.file :as ctf] [app.common.uuid :as uuid] @@ -61,6 +62,7 @@ :name name :revn revn :is-shared is-shared + :version version :data data :features features :ignore-sync-until ignore-sync-until diff --git a/backend/src/app/rpc/commands/files_update.clj b/backend/src/app/rpc/commands/files_update.clj index 9c7c5fb4b..cf9e9b590 100644 --- a/backend/src/app/rpc/commands/files_update.clj +++ b/backend/src/app/rpc/commands/files_update.clj @@ -183,8 +183,8 @@ (l/trace :hint "update-file" :time (dt/format-duration elapsed)))))))))) (defn update-file - [{:keys [::db/conn ::mtx/metrics] :as cfg} - {:keys [id file features changes changes-with-metadata] :as params}] + [{:keys [::mtx/metrics] :as cfg} + {:keys [file features changes changes-with-metadata] :as params}] (let [features (-> features (set/difference cfeat/frontend-only-features) (set/union (:features file))) @@ -207,12 +207,6 @@ (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}))) - (binding [cfeat/*current* features cfeat/*previous* (:features file)] (let [file (assoc file :features features) @@ -234,7 +228,8 @@ {:keys [profile-id file changes session-id ::created-at skip-validate] :as params}] (let [;; Process the file data on separated thread for avoid to do ;; the CPU intensive operation on vthread. - file (px/invoke! executor (partial update-file-data cfg file changes skip-validate))] + file (px/invoke! executor (partial update-file-data cfg file changes skip-validate)) + features (db/create-array conn "text" (:features file))] (db/insert! conn :file-change {:id (uuid/next) @@ -252,6 +247,8 @@ (db/update! conn :file {:revn (:revn file) :data (:data file) + :version (:version file) + :features features :data-backend nil :modified-at created-at :has-media-trimmed false} diff --git a/backend/src/app/srepl/helpers.clj b/backend/src/app/srepl/helpers.clj index 07b89ab7c..010a0aa7a 100644 --- a/backend/src/app/srepl/helpers.clj +++ b/backend/src/app/srepl/helpers.clj @@ -66,6 +66,7 @@ (db/update! conn :file {:revn (:revn file) :data (:data file) + :version (:version file) :features (:features file) :deleted-at (:deleted-at file) :created-at (:created-at file) diff --git a/backend/src/app/tasks/file_gc.clj b/backend/src/app/tasks/file_gc.clj index 031bf357a..29c07f78b 100644 --- a/backend/src/app/tasks/file_gc.clj +++ b/backend/src/app/tasks/file_gc.clj @@ -62,6 +62,8 @@ (db/update! conn :file {:has-media-trimmed true + :features (:features file) + :version (:version file) :data (:data file)} {:id id}))) @@ -82,6 +84,7 @@ "SELECT f.id, f.data, f.revn, + f.version, f.features, f.modified_at FROM file AS f @@ -175,7 +178,7 @@ (def ^:private sql:get-files-for-library - "SELECT f.id, f.data, f.modified_at, f.features + "SELECT f.id, f.data, f.modified_at, f.features, f.version FROM file AS f LEFT JOIN file_library_rel AS fl ON (fl.file_id = f.id) WHERE fl.library_file_id = ? diff --git a/backend/test/backend_tests/rpc_file_thumbnails_test.clj b/backend/test/backend_tests/rpc_file_thumbnails_test.clj index 5ae85b8a1..f0cfc9637 100644 --- a/backend/test/backend_tests/rpc_file_thumbnails_test.clj +++ b/backend/test/backend_tests/rpc_file_thumbnails_test.clj @@ -82,7 +82,6 @@ (t/is (nil? (:error out))) (t/is (map? (:result out)))) - ;; run the task again (let [res (th/run-task! "storage-gc-touched" {:min-age 0})] (t/is (= 2 (:freeze res)))) diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index ccb2560f5..2823bf3e2 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -29,33 +29,74 @@ #?(:cljs (l/set-level! :info)) +(declare ^:private migrations) + (def version cfd/version) -(defmulti migrate :version) - (defn need-migration? - [{:keys [data]}] - (> cfd/version (:version data 0))) + [file] + (or (nil? (:version file)) + (not= cfd/version (:version file)))) + +(defn- apply-migrations + [data migrations from-version] + + (loop [migrations migrations + data data] + (if-let [[to-version migrate-fn] (first migrations)] + (let [migrate-fn (or migrate-fn identity)] + (l/inf :hint "migrate file" + :op (if (>= from-version to-version) "down" "up") + :file-id (str (:id data)) + :version to-version) + (recur (rest migrations) + (migrate-fn data))) + data))) (defn migrate-data - ([data] (migrate-data data version)) - ([data to-version] - (if (= (:version data) to-version) - data - (let [migrate-fn #(do - (l/dbg :hint "migrate file" :id (:id %) :version-from %2 :version-to (inc %2)) - (migrate (assoc %1 :version (inc %2))))] - (reduce migrate-fn data (range (:version data 0) to-version)))))) + [data migrations from-version to-version] + (if (= from-version to-version) + data + (let [migrations (if (< from-version to-version) + (->> migrations + (drop-while #(<= (get % :id) from-version)) + (take-while #(<= (get % :id) to-version)) + (map (juxt :id :migrate-up))) + (->> (reverse migrations) + (drop-while #(> (get % :id) from-version)) + (take-while #(> (get % :id) to-version)) + (map (juxt :id :migrate-down))))] + (apply-migrations data migrations from-version)))) + +(defn fix-version + "Fixes the file versioning numbering" + [{:keys [version] :as file}] + (if (int? version) + file + (let [version (or version (-> file :data :version))] + (-> file + (assoc :version version) + (update :data dissoc :version))))) (defn migrate-file - [{:keys [id data features] :as file}] + [{:keys [id data features version] :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))) + (let [version (or version (:version data)) + file (-> file + (assoc :version cfd/version) + (update :data (fn [data] + (-> data + (assoc :id id) + (dissoc :version) + (migrate-data migrations version cfd/version)))) + (update :features (fnil into #{}) (deref cfeat/*new*)) + ;; NOTE: in some future we can consider to apply + ;; a migration to the whole database and remove + ;; this code from this function that executes on + ;; each file migration operation + (update :features cfeat/migrate-legacy-features))] + + (if (or (not= version (:version file)) (not= features (:features file))) (vary-meta file assoc ::migrated true) file)))) @@ -64,13 +105,10 @@ [file] (true? (-> file meta ::migrated))) -;; Default handler, noop -(defmethod migrate :default [data] data) - ;; -- MIGRATIONS -- -;; Ensure that all :shape attributes on shapes are vectors. -(defmethod migrate 2 +(defn migrate-up-2 + "Ensure that all :shape attributes on shapes are vectors" [data] (letfn [(update-object [object] (d/update-when object :shapes @@ -83,8 +121,8 @@ (update data :pages-index update-vals update-page))) -;; Changes paths formats -(defmethod migrate 3 +(defn migrate-up-3 + "Changes paths formats" [data] (letfn [(migrate-path [shape] (if-not (contains? shape :content) @@ -137,12 +175,9 @@ (update data :pages-index update-vals update-page))) -;; We did rollback version 4 migration. -;; Keep this in order to remember the next version to be 5 -(defmethod migrate 4 [data] data) - -;; Put the id of the local file in :component-file in instances of local components -(defmethod migrate 5 +(defn migrate-up-5 + "Put the id of the local file in :component-file in instances of + local components" [data] (letfn [(update-object [object] (if (and (some? (:component-id object)) @@ -155,9 +190,9 @@ (update data :pages-index update-vals update-page))) -(defmethod migrate 6 +(defn migrate-up-6 + "Fixes issues with selrect/points for shapes with width/height = 0 (line-like paths)" [data] - ;; Fixes issues with selrect/points for shapes with width/height = 0 (line-like paths)" (letfn [(fix-line-paths [shape] (if (= (:type shape) :path) (let [{:keys [width height]} (grc/points->rect (:points shape))] @@ -181,8 +216,8 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -;; Remove interactions pointing to deleted frames -(defmethod migrate 7 +(defn migrate-up-7 + "Remove interactions pointing to deleted frames" [data] (letfn [(update-object [page object] (d/update-when object :interactions @@ -194,9 +229,8 @@ (update data :pages-index update-vals update-page))) -;; Remove groups without any shape, both in pages and components - -(defmethod migrate 8 +(defn migrate-up-8 + "Remove groups without any shape, both in pages and components" [data] (letfn [(clean-parents [obj deleted?] (d/update-when obj :shapes @@ -236,7 +270,7 @@ (update :pages-index update-vals clean-container) (update :components update-vals clean-container)))) -(defmethod migrate 9 +(defn migrate-up-9 [data] (letfn [(find-empty-groups [objects] (->> (vals objects) @@ -264,13 +298,13 @@ (recur (cpc/process-changes data changes)) data))))) -(defmethod migrate 10 +(defn migrate-up-10 [data] (letfn [(update-page [page] (d/update-in-when page [:objects uuid/zero] dissoc :points :selrect))] (update data :pages-index update-vals update-page))) -(defmethod migrate 11 +(defn migrate-up-11 [data] (letfn [(update-object [objects shape] (if (cfh/frame-shape? shape) @@ -284,7 +318,7 @@ (update data :pages-index update-vals update-page))) -(defmethod migrate 12 +(defn migrate-up-12 [data] (letfn [(update-grid [grid] (cond-> grid @@ -296,8 +330,8 @@ (update data :pages-index update-vals update-page))) -;; Add rx and ry to images -(defmethod migrate 13 +(defn migrate-up-13 + "Add rx and ry to images" [data] (letfn [(fix-radius [shape] (if-not (or (contains? shape :rx) (contains? shape :r1)) @@ -316,7 +350,7 @@ (update data :pages-index update-vals update-page))) -(defmethod migrate 14 +(defn migrate-up-14 [data] (letfn [(process-shape [shape] (let [fill-color (str/upper (:fill-color shape)) @@ -347,11 +381,8 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) - -(defmethod migrate 15 [data] data) - -;; Add fills and strokes -(defmethod migrate 16 +(defn migrate-up-16 + "Add fills and strokes" [data] (letfn [(assign-fills [shape] (let [attrs {:fill-color (:fill-color shape) @@ -397,7 +428,7 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -(defmethod migrate 17 +(defn migrate-up-17 [data] (letfn [(affected-object? [object] (and (cfh/image-shape? object) @@ -426,8 +457,8 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -;;Remove position-data to solve a bug with the text positioning -(defmethod migrate 18 +(defn migrate-up-18 + "Remove position-data to solve a bug with the text positioning" [data] (letfn [(update-object [object] (cond-> object @@ -441,7 +472,7 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -(defmethod migrate 19 +(defn migrate-up-19 [data] (letfn [(update-object [object] (cond-> object @@ -457,7 +488,7 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -(defmethod migrate 25 +(defn migrate-up-25 [data] (some-> cfeat/*new* (swap! conj "fdata/shape-data-type")) (letfn [(update-object [object] @@ -470,7 +501,7 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -(defmethod migrate 26 +(defn migrate-up-26 [data] (letfn [(update-object [object] (cond-> object @@ -487,7 +518,7 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -(defmethod migrate 27 +(defn migrate-up-27 [data] (letfn [(update-object [object] (cond-> object @@ -518,7 +549,7 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -(defmethod migrate 28 +(defn migrate-up-28 [data] (letfn [(update-object [objects object] (let [frame-id (:frame-id object) @@ -544,10 +575,7 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -;; TODO: pending to do a migration for delete already not used fill -;; and stroke props. This should be done for >1.14.x version. - -(defmethod migrate 29 +(defn migrate-up-29 [data] (letfn [(valid-ref? [ref] (or (uuid? ref) @@ -582,7 +610,7 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -(defmethod migrate 31 +(defn migrate-up-31 [data] (letfn [(update-object [object] (cond-> object @@ -596,7 +624,7 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -(defmethod migrate 32 +(defn migrate-up-32 [data] (some-> cfeat/*new* (swap! conj "fdata/shape-data-type")) (letfn [(update-object [object] @@ -615,7 +643,7 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -(defmethod migrate 33 +(defn migrate-up-33 [data] (letfn [(update-object [object] ;; Ensure all root objects are well formed shapes. @@ -635,7 +663,7 @@ (-> data (update :pages-index update-vals update-container)))) -(defmethod migrate 34 +(defn migrate-up-34 [data] (letfn [(update-object [object] (if (or (cfh/path-shape? object) @@ -648,17 +676,7 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) - -;; NOTE: We need to repeat this migration for correct feature handling - -(defmethod migrate 35 - [data] - (-> data - (assoc :version 25) - (migrate) - (assoc :version 35))) - -(defmethod migrate 36 +(defn migrate-up-36 [data] (letfn [(update-container [container] (d/update-when container :objects (fn [objects] @@ -669,11 +687,12 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -(defmethod migrate 37 +(defn migrate-up-37 + "Clean nil values on data" [data] (d/without-nils data)) -(defmethod migrate 38 +(defn migrate-up-38 [data] (letfn [(fix-gradient [{:keys [type] :as gradient}] (if (string? type) @@ -701,7 +720,7 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -(defmethod migrate 39 +(defn migrate-up-39 [data] (letfn [(update-shape [shape] (cond @@ -723,7 +742,7 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -(defmethod migrate 40 +(defn migrate-up-40 [data] (letfn [(update-shape [{:keys [content shapes] :as shape}] ;; Fix frame shape that in reallity is a path shape @@ -747,7 +766,7 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -(defmethod migrate 41 +(defn migrate-up-41 [data] (letfn [(update-shape [shape] (cond @@ -780,7 +799,7 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -(defmethod migrate 42 +(defn migrate-up-42 [data] (letfn [(update-object [object] (if (and (or (cfh/frame-shape? object) @@ -800,7 +819,7 @@ (def ^:private valid-fill? (sm/lazy-validator ::cts/fill)) -(defmethod migrate 43 +(defn migrate-up-43 [data] (letfn [(number->string [v] (if (number? v) @@ -829,7 +848,7 @@ (def ^:private valid-shadow? (sm/lazy-validator ::ctss/shadow)) -(defmethod migrate 44 +(defn migrate-up-44 [data] (letfn [(fix-shadow [shadow] (if (string? (:color shadow)) @@ -851,7 +870,7 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -(defmethod migrate 45 +(defn migrate-up-45 [data] (letfn [(fix-shape [shape] (let [frame-id (or (:frame-id shape) @@ -866,7 +885,7 @@ (-> data (update :pages-index update-vals update-container)))) -(defmethod migrate 46 +(defn migrate-up-46 [data] (letfn [(update-object [object] (dissoc object :thumbnail)) @@ -876,3 +895,42 @@ (-> data (update :pages-index update-vals update-container) (update :components update-vals update-container)))) + +(def migrations + "A vector of all applicable migrations" + [{:id 2 :migrate-up migrate-up-2} + {:id 3 :migrate-up migrate-up-3} + {:id 5 :migrate-up migrate-up-5} + {:id 6 :migrate-up migrate-up-6} + {:id 7 :migrate-up migrate-up-7} + {:id 8 :migrate-up migrate-up-8} + {:id 9 :migrate-up migrate-up-9} + {:id 10 :migrate-up migrate-up-10} + {:id 11 :migrate-up migrate-up-11} + {:id 12 :migrate-up migrate-up-12} + {:id 13 :migrate-up migrate-up-13} + {:id 14 :migrate-up migrate-up-14} + {:id 16 :migrate-up migrate-up-16} + {:id 17 :migrate-up migrate-up-17} + {:id 18 :migrate-up migrate-up-18} + {:id 19 :migrate-up migrate-up-19} + {:id 25 :migrate-up migrate-up-25} + {:id 26 :migrate-up migrate-up-26} + {:id 27 :migrate-up migrate-up-27} + {:id 28 :migrate-up migrate-up-28} + {:id 29 :migrate-up migrate-up-29} + {:id 31 :migrate-up migrate-up-31} + {:id 32 :migrate-up migrate-up-32} + {:id 33 :migrate-up migrate-up-33} + {:id 34 :migrate-up migrate-up-34} + {:id 36 :migrate-up migrate-up-36} + {:id 37 :migrate-up migrate-up-37} + {:id 38 :migrate-up migrate-up-38} + {:id 39 :migrate-up migrate-up-39} + {:id 40 :migrate-up migrate-up-40} + {:id 41 :migrate-up migrate-up-41} + {:id 42 :migrate-up migrate-up-42} + {:id 43 :migrate-up migrate-up-43} + {:id 44 :migrate-up migrate-up-44} + {:id 45 :migrate-up migrate-up-45} + {:id 46 :migrate-up migrate-up-46}]) diff --git a/common/src/app/common/types/file.cljc b/common/src/app/common/types/file.cljc index 1fd8b1f00..5281e78d9 100644 --- a/common/src/app/common/types/file.cljc +++ b/common/src/app/common/types/file.cljc @@ -9,7 +9,6 @@ [app.common.data :as d] [app.common.data.macros :as dm] [app.common.features :as cfeat] - [app.common.files.defaults :refer [version]] [app.common.files.helpers :as cfh] [app.common.geom.point :as gpt] [app.common.geom.shapes :as gsh] @@ -70,8 +69,7 @@ ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (def empty-file-data - {:version version - :pages [] + {:pages [] :pages-index {}}) (defn make-file-data @@ -82,7 +80,7 @@ (let [page (when (some? page-id) (ctp/make-empty-page page-id "Page 1"))] - (cond-> (assoc empty-file-data :id file-id :version version) + (cond-> (assoc empty-file-data :id file-id) (some? page-id) (ctpl/add-page page) diff --git a/common/test/common_tests/files_migrations_test.cljc b/common/test/common_tests/files_migrations_test.cljc index 3ae1d5fd0..b012cb0ea 100644 --- a/common/test/common_tests/files_migrations_test.cljc +++ b/common/test/common_tests/files_migrations_test.cljc @@ -7,11 +7,42 @@ (ns common-tests.files-migrations-test (:require [app.common.data :as d] - [app.common.files.migrations :as cpm] + [app.common.files.migrations :as cfm] + [app.common.pprint :as pp] [app.common.uuid :as uuid] - [clojure.pprint :refer [pprint]] [clojure.test :as t])) +(t/deftest test-generic-migration-subsystem-1 + (let [migrations [{:id 1 :migrate-up (comp inc inc) :migrate-down (comp dec dec)} + {:id 2 :migrate-up (comp inc inc) :migrate-down (comp dec dec)} + {:id 3 :migrate-up (comp inc inc) :migrate-down (comp dec dec)} + {:id 4 :migrate-up (comp inc inc) :migrate-down (comp dec dec)} + {:id 5 :migrate-up (comp inc inc) :migrate-down (comp dec dec)} + {:id 6 :migrate-up (comp inc inc) :migrate-down (comp dec dec)} + {:id 7 :migrate-up (comp inc inc) :migrate-down (comp dec dec)} + {:id 8 :migrate-up (comp inc inc) :migrate-down (comp dec dec)} + {:id 9 :migrate-up (comp inc inc) :migrate-down (comp dec dec)} + {:id 10 :migrate-up (comp inc inc) :migrate-down (comp dec dec)} + {:id 11 :migrate-up (comp inc inc) :migrate-down (comp dec dec)} + {:id 12 :migrate-up (comp inc inc) :migrate-down (comp dec dec)} + {:id 13 :migrate-up (comp inc inc) :migrate-down (comp dec dec)}]] + + (t/testing "migrate up 1" + (let [result (cfm/migrate-data 0 migrations 0 2)] + (t/is (= result 4)))) + + (t/testing "migrate up 2" + (let [result (cfm/migrate-data 0 migrations 0 20)] + (t/is (= result 26)))) + + (t/testing "migrate down 1" + (let [result (cfm/migrate-data 12 migrations 6 3)] + (t/is (= result 6)))) + + (t/testing "migrate down 2" + (let [result (cfm/migrate-data 12 migrations 6 0)] + (t/is (= result 0)))))) + (t/deftest test-migration-8-1 (let [page-id (uuid/custom 0 0) objects [{:type :rect :id (uuid/custom 1 0)} @@ -34,16 +65,11 @@ {:type :path :id (uuid/custom 1 5)}] data {:pages-index {page-id {:objects (d/index-by :id objects)}} - :components {} - :version 7} + :components {}} - res (cpm/migrate-data data 8)] + res (cfm/migrate-data data cfm/migrations 7 8)] - ;; (pprint data) - ;; (pprint res) - - (t/is (= (dissoc data :version) - (dissoc res :version))))) + (t/is (= data res)))) (t/deftest test-migration-8-2 (let [page-id (uuid/custom 0 0) @@ -67,8 +93,7 @@ {:type :path :id (uuid/custom 1 5)}] data {:pages-index {page-id {:objects (d/index-by :id objects)}} - :components {} - :version 7} + :components {}} expect (-> data (update-in [:pages-index page-id :objects] dissoc @@ -80,10 +105,9 @@ (let [id (uuid/custom 1 2)] (into [] (remove #(= id %)) shapes))))) - res (cpm/migrate-data data 8)] + res (cfm/migrate-data data cfm/migrations 7 8)] ;; (pprint res) ;; (pprint expect) - (t/is (= (dissoc expect :version) - (dissoc res :version))))) + (t/is (= expect res)))) From a89f16e5945f84ceef12608a1f5185e609ecd4d3 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 14 Feb 2024 15:16:05 +0100 Subject: [PATCH 2/5] :sparkles: Add better logging config for devenv --- backend/resources/log4j2-devenv-repl.xml | 52 ++++++++++++++++++++++++ backend/resources/log4j2-devenv.xml | 10 +++-- backend/resources/log4j2.xml | 9 +--- backend/scripts/repl | 2 +- backend/scripts/start-dev | 2 +- 5 files changed, 62 insertions(+), 13 deletions(-) create mode 100644 backend/resources/log4j2-devenv-repl.xml diff --git a/backend/resources/log4j2-devenv-repl.xml b/backend/resources/log4j2-devenv-repl.xml new file mode 100644 index 000000000..07a84f2fd --- /dev/null +++ b/backend/resources/log4j2-devenv-repl.xml @@ -0,0 +1,52 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/backend/resources/log4j2-devenv.xml b/backend/resources/log4j2-devenv.xml index 62c15a0e4..31e196829 100644 --- a/backend/resources/log4j2-devenv.xml +++ b/backend/resources/log4j2-devenv.xml @@ -6,13 +6,13 @@ alwaysWriteExceptions="true" /> - + - + @@ -31,22 +31,26 @@ - + + + + + diff --git a/backend/resources/log4j2.xml b/backend/resources/log4j2.xml index 9041556c4..591883e4a 100644 --- a/backend/resources/log4j2.xml +++ b/backend/resources/log4j2.xml @@ -11,16 +11,9 @@ - - - - - - + - - diff --git a/backend/scripts/repl b/backend/scripts/repl index 2a89163a5..f521f2bff 100755 --- a/backend/scripts/repl +++ b/backend/scripts/repl @@ -68,7 +68,7 @@ export OPTIONS=" -J-Djava.util.logging.manager=org.apache.logging.log4j.jul.LogManager \ -J-Djdk.attach.allowAttachSelf \ -J-Dpolyglot.engine.WarnInterpreterOnly=false \ - -J-Dlog4j2.configurationFile=log4j2-devenv.xml \ + -J-Dlog4j2.configurationFile=log4j2-devenv-repl.xml \ -J-XX:+EnableDynamicAgentLoading \ -J-XX:-OmitStackTraceInFastThrow \ -J-XX:+UnlockDiagnosticVMOptions \ diff --git a/backend/scripts/start-dev b/backend/scripts/start-dev index 8dafad250..8fecd79af 100755 --- a/backend/scripts/start-dev +++ b/backend/scripts/start-dev @@ -28,7 +28,7 @@ export OPTIONS=" -J-Djava.util.logging.manager=org.apache.logging.log4j.jul.LogManager \ -J-Djdk.attach.allowAttachSelf \ -J-Dpolyglot.engine.WarnInterpreterOnly=false \ - -J-Dlog4j2.configurationFile=log4j2.xml \ + -J-Dlog4j2.configurationFile=log4j2-devenv.xml \ -J-XX:+EnableDynamicAgentLoading \ -J-XX:-OmitStackTraceInFastThrow \ -J-XX:+UnlockDiagnosticVMOptions \ From 757291644bcca9e4a3a561cf13a634af484aad68 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 14 Feb 2024 15:16:37 +0100 Subject: [PATCH 3/5] :bug: Fix incorrect warning on climit initialization when disabled --- backend/src/app/rpc/climit.clj | 59 +++++++++++++++++----------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/backend/src/app/rpc/climit.clj b/backend/src/app/rpc/climit.clj index 2b83d5c22..988fe29ad 100644 --- a/backend/src/app/rpc/climit.clj +++ b/backend/src/app/rpc/climit.clj @@ -197,40 +197,41 @@ config (::config climit) label (::sv/name mdata)] - (reduce (fn [handler [limit-id key-fn]] - (if-let [config (get config limit-id)] - (let [key-fn (or key-fn noop-fn)] - (l/dbg :hint "instrumenting method" - :method label - :limit (id->str limit-id) - :timeout (:timeout config) - :permits (:permits config) - :queue (:queue config) - :keyed (not= key-fn noop-fn)) + (if climit + (reduce (fn [handler [limit-id key-fn]] + (if-let [config (get config limit-id)] + (let [key-fn (or key-fn noop-fn)] + (l/dbg :hint "instrumenting method" + :method label + :limit (id->str limit-id) + :timeout (:timeout config) + :permits (:permits config) + :queue (:queue config) + :keyed (not= key-fn noop-fn)) + (if (and (= key-fn ::rpc/profile-id) + (false? (::rpc/auth mdata true))) - (if (and (= key-fn ::rpc/profile-id) - (false? (::rpc/auth mdata true))) + ;; We don't enforce by-profile limit on methods that does + ;; not require authentication + handler - ;; We don't enforce by-profile limit on methods that does - ;; not require authentication - handler + (fn [cfg params] + (let [limit-key (key-fn params) + cache-key [limit-id limit-key] + limiter (cache/get cache cache-key (partial create-limiter config)) + profile-id (if (= key-fn ::rpc/profile-id) + limit-key + (get params ::rpc/profile-id))] + (invoke limiter metrics limit-id limit-key label profile-id handler [cfg params]))))) - (fn [cfg params] - (let [limit-key (key-fn params) - cache-key [limit-id limit-key] - limiter (cache/get cache cache-key (partial create-limiter config)) - profile-id (if (= key-fn ::rpc/profile-id) - limit-key - (get params ::rpc/profile-id))] - (invoke limiter metrics limit-id limit-key label profile-id handler [cfg params]))))) + (do + (l/wrn :hint "no config found for specified queue" :id (id->str limit-id)) + handler))) - (do - (l/wrn :hint "no config found for specified queue" :id (id->str limit-id)) - handler))) - - handler - (concat global-limits (get-limits mdata))))) + handler + (concat global-limits (get-limits mdata))) + handler))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; PUBLIC API From 41794c5f5e16b27df5673b82913c75da882cb875 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 15 Feb 2024 12:08:40 +0100 Subject: [PATCH 4/5] :sparkles: Simplify fdata feature helpers --- backend/src/app/features/fdata.clj | 57 +++++++++++++----------------- 1 file changed, 24 insertions(+), 33 deletions(-) diff --git a/backend/src/app/features/fdata.clj b/backend/src/app/features/fdata.clj index 4715c5bfb..3ec5fdfb5 100644 --- a/backend/src/app/features/fdata.clj +++ b/backend/src/app/features/fdata.clj @@ -22,18 +22,16 @@ (defn enable-objects-map [file] - (let [update-container - (fn [container] - (if (and (pmap/pointer-map? container) - (not (pmap/loaded? container))) - container - (d/update-when container :objects omap/wrap))) + (let [update-page + (fn [page] + (if (and (pmap/pointer-map? page) + (not (pmap/loaded? page))) + page + (update page :objects omap/wrap))) update-data (fn [fdata] - (-> fdata - (update :pages-index d/update-vals update-container) - (d/update-when :components d/update-vals update-container)))] + (update fdata :pages-index d/update-vals update-page))] (-> file (update :data update-data) @@ -43,19 +41,15 @@ "Apply a function to all objects-map on the file. Usualy used for convert the objects-map instances to plain maps" [fdata update-fn] - (let [update-container - (fn [container] - (d/update-when container :objects - (fn [objects] - (if (omap/objects-map? objects) - (update-fn objects) - objects))))] - (cond-> fdata - (contains? fdata :pages-index) - (update :pages-index update-vals update-container) - - (contains? fdata :components) - (update :components update-vals update-container)))) + (if (contains? fdata :pages-index) + (update fdata :pages-index d/update-vals + (fn [page] + (update page :objects + (fn [objects] + (if (omap/objects-map? objects) + (update-fn objects) + objects))))) + fdata)) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; POINTER-MAP @@ -95,15 +89,13 @@ "Apply a function to all pointers on the file. Usuly used for dereference the pointer to a plain value before some processing." [fdata update-fn] - (cond-> fdata - (contains? fdata :pages-index) - (update :pages-index process-pointers update-fn) - - :always - (update-vals (fn [val] - (if (pmap/pointer-map? val) - (update-fn val) - val))))) + (let [update-fn' (fn [val] + (if (pmap/pointer-map? val) + (update-fn val) + val))] + (-> fdata + (d/update-vals update-fn') + (update :pages-index d/update-vals update-fn')))) (defn get-used-pointer-ids "Given a file, return all pointer ids used in the data." @@ -119,7 +111,6 @@ (-> file (update :data (fn [fdata] (-> fdata - (update :pages-index update-vals pmap/wrap) + (update :pages-index d/update-vals pmap/wrap) (d/update-when :components pmap/wrap)))) - (update :features conj "fdata/pointer-map"))) From 90cb2c4518b2664a89fbe1ab8f8ca853fadc9a04 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 15 Feb 2024 14:57:50 +0100 Subject: [PATCH 5/5] :bug: Fix incorrect redirect on login with different user after logout --- frontend/src/app/main/data/users.cljs | 12 +++++++++--- frontend/src/app/main/ui/auth/verify_token.cljs | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/frontend/src/app/main/data/users.cljs b/frontend/src/app/main/data/users.cljs index 50cd0e584..08beb78f1 100644 --- a/frontend/src/app/main/data/users.cljs +++ b/frontend/src/app/main/data/users.cljs @@ -227,9 +227,14 @@ (ptk/reify ::login-from-token ptk/WatchEvent (watch [_ _ _] - (rx/of (logged-in - (with-meta profile - {::ev/source "login-with-token"})))))) + (->> (rx/of (logged-in (with-meta profile {::ev/source "login-with-token"}))) + ;; NOTE: we need this to be asynchronous because the effect + ;; should be called before proceed with the login process + (rx/observe-on :async))) + + ptk/EffectEvent + (effect [_ _ _] + (set-current-team! nil)))) (defn login-from-register "Event used mainly for mark current session as logged-in in after the @@ -274,6 +279,7 @@ (effect [_ _ _] ;; We prefer to keek some stuff in the storage like the current-team-id and the profile (swap! storage dissoc :redirect-url) + (set-current-team! nil) (i18n/reset-locale))))) (defn logout diff --git a/frontend/src/app/main/ui/auth/verify_token.cljs b/frontend/src/app/main/ui/auth/verify_token.cljs index e0dcd7cfe..827e37f0e 100644 --- a/frontend/src/app/main/ui/auth/verify_token.cljs +++ b/frontend/src/app/main/ui/auth/verify_token.cljs @@ -25,7 +25,7 @@ (defmethod handle-token :verify-email [data] (let [msg (tr "dashboard.notifications.email-verified-successfully")] - (ts/schedule 100 #(st/emit! (dm/success msg))) + (ts/schedule 1000 #(st/emit! (dm/success msg))) (st/emit! (du/login-from-token data)))) (defmethod handle-token :change-email