From 9de8ebb52c54678ddd0b88767e05ed8e8f0d01a7 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 8 May 2025 19:12:50 +0200 Subject: [PATCH 1/2] :sparkles: Add read-only option for files/get-file --- backend/src/app/rpc/commands/files.clj | 66 +++++++++++++++----------- 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/backend/src/app/rpc/commands/files.clj b/backend/src/app/rpc/commands/files.clj index cbca9b3f54..a293348faa 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -208,7 +208,7 @@ [:project-id {:optional true} ::sm/uuid]]) (defn- migrate-file - [{:keys [::db/conn] :as cfg} {:keys [id] :as file}] + [{:keys [::db/conn] :as cfg} {:keys [id] :as file} {:keys [read-only?]}] (binding [pmap/*load-fn* (partial feat.fdata/load-pointer cfg id) pmap/*tracked* (pmap/create-tracked)] (let [;; For avoid unnecesary overhead of creating multiple pointers and @@ -219,43 +219,45 @@ file (-> file (update :data feat.fdata/process-pointers deref) (update :data feat.fdata/process-objects (partial into {})) - (fmg/migrate-file)) + (fmg/migrate-file))] - ;; When file is migrated, we break the rule of no perform - ;; mutations on get operations and update the file with all - ;; migrations applied - ;; - ;; WARN: he following code will not work on read-only mode, - ;; it is a known issue; we keep is not implemented until we - ;; really need this. - file (if (contains? (:features file) "fdata/objects-map") - (feat.fdata/enable-objects-map file) - file) - file (if (contains? (:features file) "fdata/pointer-map") - (feat.fdata/enable-pointer-map file) - file)] + (if (or read-only? (db/read-only? conn)) + file + (let [;; When file is migrated, we break the rule of no perform + ;; mutations on get operations and update the file with all + ;; migrations applied + file (if (contains? (:features file) "fdata/objects-map") + (feat.fdata/enable-objects-map file) + file) + file (if (contains? (:features file) "fdata/pointer-map") + (feat.fdata/enable-pointer-map file) + file)] - (db/update! conn :file - {:data (blob/encode (:data file)) - :version (:version file) - :features (db/create-array conn "text" (:features file))} - {:id id}) + (db/update! conn :file + {:data (blob/encode (:data file)) + :version (:version 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)) + (when (contains? (:features file) "fdata/pointer-map") + (feat.fdata/persist-pointers! cfg id)) - (feat.fmigr/upsert-migrations! conn file) - (feat.fmigr/resolve-applied-migrations cfg file)))) + (feat.fmigr/upsert-migrations! conn file) + (feat.fmigr/resolve-applied-migrations cfg file)))))) (defn get-file [{:keys [::db/conn ::wrk/executor] :as cfg} id & {:keys [project-id migrate? include-deleted? - lock-for-update?] + lock-for-update? + preload-pointers?] :or {include-deleted? false lock-for-update? false - migrate? true}}] + migrate? true + preload-pointers? false} + :as options}] (assert (db/connection? conn) "expected cfg with valid connection") @@ -273,10 +275,16 @@ ;; because it has heavy and synchronous operations for ;; decoding file body that are not very friendly with virtual ;; threads. - file (px/invoke! executor #(decode-row file))] + file (px/invoke! executor #(decode-row file)) + + file (if (and migrate? (fmg/need-migration? file)) + (migrate-file cfg file options) + file)] + + (if preload-pointers? + (binding [pmap/*load-fn* (partial feat.fdata/load-pointer cfg id)] + (update file :data feat.fdata/process-pointers deref)) - (if (and migrate? (fmg/need-migration? file)) - (migrate-file cfg file) file))) (defn get-minimal-file From bc20598b3d6817b5378aa65bd7aebf0423cdca0a Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 8 May 2025 19:15:28 +0200 Subject: [PATCH 2/2] :sparkles: Don't persist file on several read operations after applying migrations --- backend/src/app/rpc/commands/files.clj | 6 ++++-- backend/src/app/rpc/commands/files_thumbnails.clj | 15 ++++++--------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/backend/src/app/rpc/commands/files.clj b/backend/src/app/rpc/commands/files.clj index a293348faa..bdb2fcbc59 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -492,7 +492,7 @@ (let [perms (get-permissions conn profile-id file-id share-id) - file (get-file cfg file-id) + file (get-file cfg file-id :read-only? true) proj (db/get conn :project {:id (:project-id file)}) @@ -749,7 +749,9 @@ :project-id project-id :file-id id) - file (get-file cfg id :project-id project-id)] + file (get-file cfg id + :project-id project-id + :read-only? true)] (-> (cfeat/get-team-enabled-features cf/flags team) (cfeat/check-client-features! (:features params)) diff --git a/backend/src/app/rpc/commands/files_thumbnails.clj b/backend/src/app/rpc/commands/files_thumbnails.clj index 2455807dd8..9d64ec504d 100644 --- a/backend/src/app/rpc/commands/files_thumbnails.clj +++ b/backend/src/app/rpc/commands/files_thumbnails.clj @@ -10,7 +10,6 @@ [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] @@ -18,7 +17,6 @@ [app.config :as cf] [app.db :as db] [app.db.sql :as-alias sql] - [app.features.fdata :as feat.fdata] [app.loggers.audit :as-alias audit] [app.loggers.webhooks :as-alias webhooks] [app.media :as media] @@ -200,14 +198,13 @@ (db/run! cfg (fn [{:keys [::db/conn] :as cfg}] (files/check-read-permissions! conn profile-id file-id) - (let [team (teams/get-team conn - :profile-id profile-id - :file-id file-id) + (let [team (teams/get-team conn + :profile-id profile-id + :file-id 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)))] + file (files/get-file cfg file-id + :preload-pointers? true + :read-only? true)] (-> (cfeat/get-team-enabled-features cf/flags team) (cfeat/check-file-features! (:features file)))