From 746d89824522ae74a38fc8d94852b04d0688fe3f Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 2 Jan 2024 21:41:49 +0100 Subject: [PATCH] :sparkles: Improve the db api efficiency Mainly setup proper defaults and reduce unnecesary allocations on every db api call. --- backend/deps.edn | 2 +- backend/src/app/db.clj | 144 +++++++++++++----- backend/src/app/db/sql.clj | 33 ++-- backend/src/app/features/components_v2.clj | 7 +- backend/src/app/features/fdata.clj | 5 +- backend/src/app/loggers/webhooks.clj | 6 +- backend/src/app/rpc/commands/audit.clj | 2 +- backend/src/app/rpc/commands/auth.clj | 6 +- backend/src/app/rpc/commands/binfile.clj | 2 +- backend/src/app/rpc/commands/comments.clj | 27 ++-- backend/src/app/rpc/commands/files.clj | 29 ++-- .../src/app/rpc/commands/files_thumbnails.clj | 14 +- backend/src/app/rpc/commands/files_update.clj | 3 +- backend/src/app/rpc/commands/fonts.clj | 15 +- backend/src/app/rpc/commands/management.clj | 6 +- backend/src/app/rpc/commands/media.clj | 3 +- backend/src/app/rpc/commands/profile.clj | 12 +- backend/src/app/rpc/commands/projects.clj | 6 +- backend/src/app/rpc/commands/teams.clj | 3 +- backend/src/app/rpc/commands/webhooks.clj | 3 +- backend/src/app/srepl/cli.clj | 13 +- backend/src/app/storage.clj | 6 +- backend/src/app/storage/gc_deleted.clj | 5 +- backend/src/app/tasks/file_gc.clj | 6 +- backend/src/app/tasks/objects_gc.clj | 40 ++--- backend/src/app/tasks/orphan_teams_gc.clj | 3 +- .../rpc_file_thumbnails_test.clj | 9 +- .../test/backend_tests/rpc_profile_test.clj | 4 +- 28 files changed, 228 insertions(+), 186 deletions(-) diff --git a/backend/deps.edn b/backend/deps.edn index b3cacd663..d3c989318 100644 --- a/backend/deps.edn +++ b/backend/deps.edn @@ -26,7 +26,7 @@ :git/url "https://github.com/funcool/yetti.git" :exclusions [org.slf4j/slf4j-api]} - com.github.seancorfield/next.jdbc {:mvn/version "1.3.894"} + com.github.seancorfield/next.jdbc {:mvn/version "1.3.909"} metosin/reitit-core {:mvn/version "0.6.0"} nrepl/nrepl {:mvn/version "1.1.0"} cider/cider-nrepl {:mvn/version "0.43.1"} diff --git a/backend/src/app/db.clj b/backend/src/app/db.clj index 357737022..b1c8476a7 100644 --- a/backend/src/app/db.clj +++ b/backend/src/app/db.clj @@ -19,6 +19,7 @@ [app.util.json :as json] [app.util.time :as dt] [clojure.java.io :as io] + [clojure.set :as set] [clojure.spec.alpha :as s] [integrant.core :as ig] [next.jdbc :as jdbc] @@ -239,6 +240,10 @@ (ex/raise :type :internal :code :unable-resolve-pool)))) +(defn get-update-count + [result] + (:next.jdbc/update-count result)) + (defn get-connection [cfg-or-conn] (if (connection? cfg-or-conn) @@ -265,48 +270,120 @@ :code :unable-resolve-connectable :hint "expected conn, pool or system"))) +(def ^:private params-mapping + {::return-keys? :return-keys + ::return-keys :return-keys}) + +(defn rename-opts + [opts] + (set/rename-keys opts params-mapping)) + +(def ^:private default-insert-opts + {:builder-fn sql/as-kebab-maps + :return-keys true}) + (def ^:private default-opts {:builder-fn sql/as-kebab-maps}) (defn exec! - ([ds sv] - (-> (get-connectable ds) - (jdbc/execute! sv default-opts))) + ([ds sv] (exec! ds sv nil)) ([ds sv opts] - (-> (get-connectable ds) - (jdbc/execute! sv (into default-opts (sql/adapt-opts opts)))))) + (let [conn (get-connectable ds) + opts (if (empty? opts) + default-opts + (into default-opts (rename-opts opts)))] + (jdbc/execute! conn sv opts)))) (defn exec-one! - ([ds sv] - (-> (get-connectable ds) - (jdbc/execute-one! sv default-opts))) + ([ds sv] (exec-one! ds sv nil)) ([ds sv opts] - (-> (get-connectable ds) - (jdbc/execute-one! sv (into default-opts (sql/adapt-opts opts)))))) + (let [conn (get-connectable ds) + opts (if (empty? opts) + default-opts + (into default-opts (rename-opts opts)))] + (jdbc/execute-one! conn sv opts)))) (defn insert! - [ds table params & {:as opts :keys [::return-keys?] :or {return-keys? true}}] - (-> (get-connectable ds) - (exec-one! (sql/insert table params opts) - (assoc opts ::return-keys? return-keys?)))) + "A helper that builds an insert sql statement and executes it. By + default returns the inserted row with all the field; you can delimit + the returned columns with the `::columns` option." + [ds table params & {:as opts}] + (let [conn (get-connectable ds) + sql (sql/insert table params opts) + opts (if (empty? opts) + default-insert-opts + (into default-insert-opts (rename-opts opts)))] + (jdbc/execute-one! conn sql opts))) -(defn insert-multi! - [ds table cols rows & {:as opts :keys [::return-keys?] :or {return-keys? true}}] - (-> (get-connectable ds) - (exec! (sql/insert-multi table cols rows opts) - (assoc opts ::return-keys? return-keys?)))) +(defn insert-many! + "An optimized version of `insert!` that perform insertion of multiple + values at once. + + This expands to a single SQL statement with placeholders for every + value being inserted. For large data sets, this may exceed the limit + of sql string size and/or number of parameters." + [ds table cols rows & {:as opts}] + (let [conn (get-connectable ds) + sql (sql/insert-many table cols rows opts) + opts (if (empty? opts) + default-insert-opts + (into default-insert-opts (rename-opts opts))) + opts (update opts :return-keys boolean)] + (jdbc/execute! conn sql opts))) (defn update! - [ds table params where & {:as opts :keys [::return-keys?] :or {return-keys? true}}] - (-> (get-connectable ds) - (exec-one! (sql/update table params where opts) - (assoc opts ::return-keys? return-keys?)))) + "A helper that build an UPDATE SQL statement and executes it. + + Given a connectable object, a table name, a hash map of columns and + values to set, and either a hash map of columns and values to search + on or a vector of a SQL where clause and parameters, perform an + update on the table. + + By default returns an object with the number of affected rows; a + complete row can be returned if you pass `::return-keys` with `true` + or with a vector of columns. + + Also it can be combined with the `::many` option if you perform an + update to multiple rows and you want all the affected rows to be + returned." + [ds table params where & {:as opts}] + (let [conn (get-connectable ds) + sql (sql/update table params where opts) + opts (if (empty? opts) + default-opts + (into default-opts (rename-opts opts))) + opts (update opts :return-keys boolean)] + (if (::many opts) + (jdbc/execute! conn sql opts) + (jdbc/execute-one! conn sql opts)))) (defn delete! - [ds table params & {:as opts :keys [::return-keys?] :or {return-keys? true}}] - (-> (get-connectable ds) - (exec-one! (sql/delete table params opts) - (assoc opts ::return-keys? return-keys?)))) + "A helper that builds an DELETE SQL statement and executes it. + + Given a connectable object, a table name, and either a hash map of columns + and values to search on or a vector of a SQL where clause and parameters, + perform a delete on the table. + + By default returns an object with the number of affected rows; a + complete row can be returned if you pass `::return-keys` with `true` + or with a vector of columns. + + Also it can be combined with the `::many` option if you perform an + update to multiple rows and you want all the affected rows to be + returned." + [ds table params & {:as opts}] + (let [conn (get-connectable ds) + sql (sql/delete table params opts) + opts (if (empty? opts) + default-opts + (into default-opts (rename-opts opts)))] + (if (::many opts) + (jdbc/execute! conn sql opts) + (jdbc/execute-one! conn sql opts)))) + +(defn query + [ds table params & {:as opts}] + (exec! ds (sql/select table params opts) opts)) (defn is-row-deleted? [{:keys [deleted-at]}] @@ -320,7 +397,7 @@ [ds table params & {:as opts}] (let [rows (exec! ds (sql/select table params opts)) rows (cond->> rows - (::remove-deleted? opts true) + (::remove-deleted opts true) (remove is-row-deleted?))] (first rows))) @@ -329,7 +406,7 @@ filters. Raises :not-found exception if no object is found." [ds table params & {:as opts}] (let [row (get* ds table params opts)] - (when (and (not row) (::check-deleted? opts true)) + (when (and (not row) (::check-deleted opts true)) (ex/raise :type :not-found :code :object-not-found :table table @@ -364,10 +441,6 @@ [ds table id & {:as opts}] (get ds table {:id id} opts)) -(defn query - [ds table params & {:as opts}] - (exec! ds (sql/select table params opts))) - (defn pgobject? ([v] (instance? PGobject v)) @@ -567,11 +640,6 @@ (.setType "jsonb") (.setValue (json/encode-str data))))) -(defn get-update-count - [result] - (:next.jdbc/update-count result)) - - ;; --- Locks (def ^:private siphash-state diff --git a/backend/src/app/db/sql.clj b/backend/src/app/db/sql.clj index 4b002f41b..37814733d 100644 --- a/backend/src/app/db/sql.clj +++ b/backend/src/app/db/sql.clj @@ -8,7 +8,6 @@ (:refer-clojure :exclude [update]) (:require [app.db :as-alias db] - [clojure.set :as set] [clojure.string :as str] [next.jdbc.optional :as jdbc-opt] [next.jdbc.sql.builder :as sql])) @@ -20,14 +19,6 @@ {:table-fn snake-case :column-fn snake-case}) -(def params-mapping - {::db/return-keys? :return-keys - ::db/columns :columns}) - -(defn adapt-opts - [opts] - (set/rename-keys opts params-mapping)) - (defn as-kebab-maps [rs opts] (jdbc-opt/as-unqualified-modified-maps rs (assoc opts :label-fn kebab-case))) @@ -42,7 +33,7 @@ (assoc :suffix "ON CONFLICT DO NOTHING"))] (sql/for-insert table key-map opts)))) -(defn insert-multi +(defn insert-many [table cols rows opts] (let [opts (merge default-opts opts)] (sql/for-insert-multi table cols rows opts))) @@ -53,11 +44,9 @@ ([table where-params opts] (let [opts (merge default-opts opts) opts (cond-> opts - (::db/columns opts) (assoc :columns (::db/columns opts)) - (::db/for-update? opts) (assoc :suffix "FOR UPDATE") - (::db/for-share? opts) (assoc :suffix "FOR KEY SHARE") - (:for-update opts) (assoc :suffix "FOR UPDATE") - (:for-key-share opts) (assoc :suffix "FOR KEY SHARE"))] + (::columns opts) (assoc :columns (::columns opts)) + (::for-update opts) (assoc :suffix "FOR UPDATE") + (::for-share opts) (assoc :suffix "FOR KEY SHARE"))] (sql/for-query table where-params opts)))) (defn update @@ -65,11 +54,9 @@ (update table key-map where-params nil)) ([table key-map where-params opts] (let [opts (into default-opts opts) - opts (if-let [columns (::db/columns opts)] - (let [columns (if (seq columns) - (sql/as-cols columns opts) - "*")] - (assoc opts :suffix (str "RETURNING " columns))) + keys (::db/return-keys opts) + opts (if (vector? keys) + (assoc opts :suffix (str "RETURNING " (sql/as-cols keys opts))) opts)] (sql/for-update table key-map where-params opts)))) @@ -77,5 +64,9 @@ ([table where-params] (delete table where-params nil)) ([table where-params opts] - (let [opts (merge default-opts opts)] + (let [opts (merge default-opts opts) + keys (::db/return-keys opts) + opts (if (vector? keys) + (assoc opts :suffix (str "RETURNING " (sql/as-cols keys opts))) + opts)] (sql/for-delete table where-params opts)))) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 99074dd59..1dc9f8325 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -816,8 +816,7 @@ {:data (blob/encode (:data file)) :features (db/create-array conn "text" (:features file)) :revn (:revn file)} - {:id (:id file)} - {::db/return-keys? false}) + {:id (:id file)}) (dissoc file :data))) @@ -900,7 +899,9 @@ (conj "styles/v2"))] (db/update! conn :team {:features (db/create-array conn "text" features)} - {:id team-id}))))))) + {:id team-id}) + + nil)))))) (finally (some-> *semaphore* ps/release!) (let [elapsed (tpoint)] diff --git a/backend/src/app/features/fdata.clj b/backend/src/app/features/fdata.clj index 832d3360d..d04369d5d 100644 --- a/backend/src/app/features/fdata.clj +++ b/backend/src/app/features/fdata.clj @@ -11,6 +11,7 @@ [app.common.exceptions :as ex] [app.common.logging :as l] [app.db :as db] + [app.db.sql :as-alias sql] [app.util.blob :as blob] [app.util.objects-map :as omap] [app.util.pointer-map :as pmap])) @@ -38,8 +39,8 @@ [system file-id id] (let [{:keys [content]} (db/get system :file-data-fragment {:id id :file-id file-id} - {::db/columns [:content] - ::db/check-deleted? false})] + {::sql/columns [:content] + ::db/check-deleted false})] (when-not content (ex/raise :type :internal :code :fragment-not-found diff --git a/backend/src/app/loggers/webhooks.clj b/backend/src/app/loggers/webhooks.clj index eb0f14fc8..00ebd3f38 100644 --- a/backend/src/app/loggers/webhooks.clj +++ b/backend/src/app/loggers/webhooks.clj @@ -111,9 +111,11 @@ " where id=?") err (:id whook)] - res (db/exec-one! pool sql {::db/return-keys? true})] + res (db/exec-one! pool sql {::db/return-keys true})] (when (>= (:error-count res) max-errors) - (db/update! pool :webhook {:is-active false} {:id (:id whook)}))) + (db/update! pool :webhook + {:is-active false} + {:id (:id whook)}))) (db/update! pool :webhook {:updated-at (dt/now) diff --git a/backend/src/app/rpc/commands/audit.clj b/backend/src/app/rpc/commands/audit.clj index fa5608721..76bd6e188 100644 --- a/backend/src/app/rpc/commands/audit.clj +++ b/backend/src/app/rpc/commands/audit.clj @@ -48,7 +48,7 @@ (map event->row)) events (sequence xform events)] (when (seq events) - (db/insert-multi! pool :audit-log event-columns events)))) + (db/insert-many! pool :audit-log event-columns events)))) (def schema:event [:map {:title "Event"} diff --git a/backend/src/app/rpc/commands/auth.clj b/backend/src/app/rpc/commands/auth.clj index d3be9ac1b..949d528ac 100644 --- a/backend/src/app/rpc/commands/auth.clj +++ b/backend/src/app/rpc/commands/auth.clj @@ -133,7 +133,8 @@ (update-password [conn profile-id] (let [pwd (profile/derive-password cfg password)] - (db/update! conn :profile {:password pwd} {:id profile-id})))] + (db/update! conn :profile {:password pwd} {:id profile-id}) + nil))] (db/with-atomic [conn pool] (->> (validate-token token) @@ -303,7 +304,8 @@ (-> (db/update! conn :profile {:default-team-id (:id team) :default-project-id (:default-project-id team)} - {:id id}) + {:id id} + {::db/return-keys true}) (profile/decode-row)))) diff --git a/backend/src/app/rpc/commands/binfile.clj b/backend/src/app/rpc/commands/binfile.clj index e3015c023..3ccb22a0f 100644 --- a/backend/src/app/rpc/commands/binfile.clj +++ b/backend/src/app/rpc/commands/binfile.clj @@ -317,7 +317,7 @@ [cfg file-id] (db/run! cfg (fn [{:keys [::db/conn] :as cfg}] (binding [pmap/*load-fn* (partial feat.fdata/load-pointer cfg file-id)] - (some-> (db/get* conn :file {:id file-id} {::db/remove-deleted? false}) + (some-> (db/get* conn :file {:id file-id} {::db/remove-deleted false}) (files/decode-row) (update :data feat.fdata/process-pointers deref)))))) diff --git a/backend/src/app/rpc/commands/comments.clj b/backend/src/app/rpc/commands/comments.clj index 99f6094b4..9e1a9d436 100644 --- a/backend/src/app/rpc/commands/comments.clj +++ b/backend/src/app/rpc/commands/comments.clj @@ -12,6 +12,7 @@ [app.common.spec :as us] [app.common.uuid :as uuid] [app.db :as db] + [app.db.sql :as sql] [app.features.fdata :as feat.fdata] [app.loggers.audit :as-alias audit] [app.loggers.webhooks :as-alias webhooks] @@ -62,8 +63,8 @@ (decode-row))) (defn- get-comment - [conn comment-id & {:keys [for-update?]}] - (db/get-by-id conn :comment comment-id {:for-update for-update?})) + [conn comment-id & {:as opts}] + (db/get-by-id conn :comment comment-id opts)) (defn- get-next-seqn [conn file-id] @@ -375,7 +376,7 @@ {::doc/added "1.15"} [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id id share-id] :as params}] (db/with-atomic [conn pool] - (let [{:keys [file-id] :as thread} (get-comment-thread conn id ::db/for-update? true)] + (let [{:keys [file-id] :as thread} (get-comment-thread conn id ::sql/for-update true)] (files/check-comment-permissions! conn profile-id file-id share-id) (upsert-comment-thread-status! conn profile-id id)))) @@ -392,7 +393,7 @@ {::doc/added "1.15"} [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id id is-resolved share-id] :as params}] (db/with-atomic [conn pool] - (let [{:keys [file-id] :as thread} (get-comment-thread conn id ::db/for-update? true)] + (let [{:keys [file-id] :as thread} (get-comment-thread conn id ::sql/for-update true)] (files/check-comment-permissions! conn profile-id file-id share-id) (db/update! conn :comment-thread {:is-resolved is-resolved} @@ -415,7 +416,7 @@ [cfg {:keys [::rpc/profile-id ::rpc/request-at thread-id share-id content]}] (db/tx-run! cfg (fn [{:keys [::db/conn] :as cfg}] - (let [{:keys [file-id page-id] :as thread} (get-comment-thread conn thread-id ::db/for-update? true) + (let [{:keys [file-id page-id] :as thread} (get-comment-thread conn thread-id ::sql/for-update true) {:keys [team-id project-id page-name] :as file} (get-file cfg file-id page-id)] (files/check-comment-permissions! conn profile-id (:id file) share-id) @@ -471,8 +472,8 @@ (db/tx-run! cfg (fn [{:keys [::db/conn] :as cfg}] - (let [{:keys [thread-id owner-id] :as comment} (get-comment conn id ::db/for-update? true) - {:keys [file-id page-id] :as thread} (get-comment-thread conn thread-id ::db/for-update? true)] + (let [{:keys [thread-id owner-id] :as comment} (get-comment conn id ::sql/for-update true) + {:keys [file-id page-id] :as thread} (get-comment-thread conn thread-id ::sql/for-update true)] (files/check-comment-permissions! conn profile-id file-id share-id) @@ -504,7 +505,7 @@ {::doc/added "1.15"} [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id id share-id]}] (db/with-atomic [conn pool] - (let [{:keys [owner-id file-id] :as thread} (get-comment-thread conn id ::db/for-update? true)] + (let [{:keys [owner-id file-id] :as thread} (get-comment-thread conn id ::sql/for-update true)] (files/check-comment-permissions! conn profile-id file-id share-id) (when-not (= owner-id profile-id) (ex/raise :type :validation @@ -524,14 +525,14 @@ {::doc/added "1.15"} [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id id share-id] :as params}] (db/with-atomic [conn pool] - (let [{:keys [owner-id thread-id] :as comment} (get-comment conn id ::db/for-update? true) + (let [{:keys [owner-id thread-id] :as comment} (get-comment conn id ::sql/for-update true) {:keys [file-id] :as thread} (get-comment-thread conn thread-id)] (files/check-comment-permissions! conn profile-id file-id share-id) (when-not (= owner-id profile-id) (ex/raise :type :validation :code :not-allowed)) - (db/delete! conn :comment {:id id})))) - + (db/delete! conn :comment {:id id}) + nil))) ;; --- COMMAND: Update comment thread position @@ -544,7 +545,7 @@ {::doc/added "1.15"} [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id ::rpc/request-at id position frame-id share-id]}] (db/with-atomic [conn pool] - (let [{:keys [file-id] :as thread} (get-comment-thread conn id ::db/for-update? true)] + (let [{:keys [file-id] :as thread} (get-comment-thread conn id ::sql/for-update true)] (files/check-comment-permissions! conn profile-id file-id share-id) (db/update! conn :comment-thread {:modified-at request-at @@ -564,7 +565,7 @@ {::doc/added "1.15"} [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id ::rpc/request-at id frame-id share-id]}] (db/with-atomic [conn pool] - (let [{:keys [file-id] :as thread} (get-comment-thread conn id ::db/for-update? true)] + (let [{:keys [file-id] :as thread} (get-comment-thread conn id ::sql/for-update true)] (files/check-comment-permissions! conn profile-id file-id share-id) (db/update! conn :comment-thread {:modified-at request-at diff --git a/backend/src/app/rpc/commands/files.clj b/backend/src/app/rpc/commands/files.clj index 2bb6cd9b6..58b10742d 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -20,6 +20,7 @@ [app.common.types.file :as ctf] [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] @@ -238,8 +239,7 @@ (db/update! conn :file {:data (blob/encode (:data file)) :features (db/create-array conn "text" (:features file))} - {:id id} - {::db/return-keys? false}) + {:id id}) (when (contains? (:features file) "fdata/pointer-map") (feat.fdata/persist-pointers! cfg id))) @@ -262,9 +262,9 @@ (when (some? project-id) {:project-id project-id})) file (-> (db/get conn :file params - {::db/check-deleted? (not include-deleted?) - ::db/remove-deleted? (not include-deleted?) - ::db/for-update? lock-for-update?}) + {::db/check-deleted (not include-deleted?) + ::db/remove-deleted (not include-deleted?) + ::sql/for-update lock-for-update?}) (decode-row))] (if migrate? (migrate-file cfg file) @@ -733,7 +733,8 @@ (db/update! conn :file {:name name :modified-at (dt/now)} - {:id id})) + {:id id} + {::db/return-keys true})) (sv/defmethod ::rename-file {::doc/added "1.17" @@ -860,9 +861,7 @@ (let [file (assoc file :is-shared true)] (db/update! conn :file {:is-shared true} - {:id id} - ::db/return-keys? false) - + {:id id}) file) :else @@ -899,7 +898,7 @@ (db/update! conn :file {:deleted-at (dt/now)} {:id file-id} - {::db/columns [:id :name :is-shared :project-id :created-at :modified-at]})) + {::db/return-keys [:id :name :is-shared :project-id :created-at :modified-at]})) (def ^:private schema:delete-file @@ -998,8 +997,8 @@ [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id file-id] :as params}] (db/with-atomic [conn pool] (check-edition-permissions! conn profile-id file-id) - (unlink-file-from-library conn params))) - + (unlink-file-from-library conn params) + nil)) ;; --- MUTATION COMMAND: update-sync @@ -1008,7 +1007,8 @@ (db/update! conn :file-library-rel {:synced-at (dt/now)} {:file-id file-id - :library-file-id library-id})) + :library-file-id library-id} + {::db/return-keys true})) (def ^:private schema:update-file-library-sync-status [:map {:title "update-file-library-sync-status"} @@ -1031,7 +1031,8 @@ [conn {:keys [file-id date] :as params}] (db/update! conn :file {:ignore-sync-until date} - {:id file-id})) + {:id file-id} + {::db/return-keys true})) (s/def ::ignore-file-library-sync-status (s/keys :req [::rpc/profile-id] diff --git a/backend/src/app/rpc/commands/files_thumbnails.clj b/backend/src/app/rpc/commands/files_thumbnails.clj index c34fedaff..712c21204 100644 --- a/backend/src/app/rpc/commands/files_thumbnails.clj +++ b/backend/src/app/rpc/commands/files_thumbnails.clj @@ -17,6 +17,7 @@ [app.common.types.shape-tree :as ctt] [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] @@ -236,8 +237,8 @@ {:file-id file-id :object-id object-id :tag tag} - {::db/remove-deleted? false - ::db/for-update? true}) + {::db/remove-deleted false + ::sql/for-update true}) path (:path media) mtype (:mtype media) @@ -312,14 +313,13 @@ (when-let [{:keys [media-id tag]} (db/get* conn :file-tagged-object-thumbnail {:file-id file-id :object-id object-id} - {::db/for-update? true})] + {::sql/for-update true})] (sto/touch-object! storage media-id) (db/update! conn :file-tagged-object-thumbnail {:deleted-at (dt/now)} {:file-id file-id :object-id object-id - :tag tag} - {::db/return-keys? false}))) + :tag tag}))) (s/def ::delete-file-object-thumbnail (s/keys :req [::rpc/profile-id] @@ -365,8 +365,8 @@ thumb (db/get* conn :file-thumbnail {:file-id file-id :revn revn} - {::db/remove-deleted? false - ::db/for-update? true})] + {::db/remove-deleted false + ::sql/for-update true})] (if (some? thumb) (do diff --git a/backend/src/app/rpc/commands/files_update.clj b/backend/src/app/rpc/commands/files_update.clj index 7a27a834e..96b92c0d6 100644 --- a/backend/src/app/rpc/commands/files_update.clj +++ b/backend/src/app/rpc/commands/files_update.clj @@ -250,7 +250,8 @@ :features (db/create-array conn "text" (:features file)) :data (when (take-snapshot? file) (:data file)) - :changes (blob/encode changes)}) + :changes (blob/encode changes)} + {::db/return-keys false}) (db/update! conn :file {:revn (:revn file) diff --git a/backend/src/app/rpc/commands/fonts.clj b/backend/src/app/rpc/commands/fonts.clj index 830efe3e5..c19b8a285 100644 --- a/backend/src/app/rpc/commands/fonts.clj +++ b/backend/src/app/rpc/commands/fonts.clj @@ -11,6 +11,7 @@ [app.common.schema :as sm] [app.common.uuid :as uuid] [app.db :as db] + [app.db.sql :as-alias sql] [app.loggers.audit :as-alias audit] [app.loggers.webhooks :as-alias webhooks] [app.media :as media] @@ -179,8 +180,7 @@ (db/update! conn :team-font-variant {:font-family name} {:font-id id - :team-id team-id} - {::db/return-keys? false}) + :team-id team-id}) (rph/with-meta (rph/wrap nil) {::audit/replace-props {:id id @@ -201,7 +201,6 @@ ::webhooks/event? true ::sm/params schema:delete-font} [cfg {:keys [::rpc/profile-id id team-id]}] - (db/tx-run! cfg (fn [{:keys [::db/conn ::sto/storage] :as cfg}] (teams/check-edition-permissions! conn profile-id team-id) @@ -209,7 +208,7 @@ {:team-id team-id :font-id id :deleted-at nil} - {::db/for-update? true}) + {::sql/for-update true}) storage (media/configure-assets-storage storage conn) tnow (dt/now)] @@ -220,8 +219,7 @@ (doseq [font fonts] (db/update! conn :team-font-variant {:deleted-at tnow} - {:id (:id font)} - {::db/return-keys? false}) + {:id (:id font)}) (some->> (:woff1-file-id font) (sto/touch-object! storage)) (some->> (:woff2-file-id font) (sto/touch-object! storage)) (some->> (:ttf-file-id font) (sto/touch-object! storage)) @@ -250,13 +248,12 @@ (teams/check-edition-permissions! conn profile-id team-id) (let [variant (db/get conn :team-font-variant {:id id :team-id team-id} - {::db/for-update? true}) + {::sql/for-update true}) storage (media/configure-assets-storage storage conn)] (db/update! conn :team-font-variant {:deleted-at (dt/now)} - {:id (:id variant)} - {::db/return-keys? false}) + {:id (:id variant)}) (some->> (:woff1-file-id variant) (sto/touch-object! storage)) (some->> (:woff2-file-id variant) (sto/touch-object! storage)) diff --git a/backend/src/app/rpc/commands/management.clj b/backend/src/app/rpc/commands/management.clj index 98f08a867..aaf51c3a0 100644 --- a/backend/src/app/rpc/commands/management.clj +++ b/backend/src/app/rpc/commands/management.clj @@ -215,7 +215,7 @@ (-> file (update :features #(db/create-array conn "text" %)) (update :data blob/encode)) - {::db/return-keys? false}) + {::db/return-keys false}) ;; The file profile creation is optional, so when no profile is ;; present (when this function is called from profile less @@ -231,10 +231,10 @@ {::db/return-keys? false})) (doseq [params flibs] - (db/insert! conn :file-library-rel params ::db/return-keys? false)) + (db/insert! conn :file-library-rel params ::db/return-keys false)) (doseq [params fmeds] - (db/insert! conn :file-media-object params ::db/return-keys? false)) + (db/insert! conn :file-media-object params ::db/return-keys false)) file)) diff --git a/backend/src/app/rpc/commands/media.clj b/backend/src/app/rpc/commands/media.clj index a357c109c..a3dc357db 100644 --- a/backend/src/app/rpc/commands/media.clj +++ b/backend/src/app/rpc/commands/media.clj @@ -157,8 +157,7 @@ (db/update! conn :file {:modified-at (dt/now) :has-media-trimmed false} - {:id file-id} - {::db/return-keys? false}) + {:id file-id}) (db/exec-one! conn [sql:create-file-media-object (or id (uuid/next)) diff --git a/backend/src/app/rpc/commands/profile.clj b/backend/src/app/rpc/commands/profile.clj index 1deacf14a..5b814abe6 100644 --- a/backend/src/app/rpc/commands/profile.clj +++ b/backend/src/app/rpc/commands/profile.clj @@ -13,6 +13,7 @@ [app.common.uuid :as uuid] [app.config :as cf] [app.db :as db] + [app.db.sql :as-alias sql] [app.email :as eml] [app.http.session :as session] [app.loggers.audit :as audit] @@ -99,7 +100,7 @@ ;; NOTE: we need to retrieve the profile independently if we use ;; it or not for explicit locking and avoid concurrent updates of ;; the same row/object. - (let [profile (-> (db/get-by-id conn :profile profile-id ::db/for-update? true) + (let [profile (-> (db/get-by-id conn :profile profile-id ::sql/for-update true) (decode-row)) ;; Update the profile map with direct params @@ -164,7 +165,7 @@ (defn- validate-password! [{:keys [::db/conn] :as cfg} {:keys [profile-id old-password] :as params}] - (let [profile (db/get-by-id conn :profile profile-id ::db/for-update? true)] + (let [profile (db/get-by-id conn :profile profile-id ::sql/for-update true)] (when (and (not= (:password profile) "!") (not (:valid (verify-password cfg old-password (:password profile))))) (ex/raise :type :validation @@ -176,7 +177,8 @@ (when-not (db/read-only? conn) (db/update! conn :profile {:password (auth/derive-password password)} - {:id id}))) + {:id id}) + nil)) ;; --- MUTATION: Update Photo @@ -202,7 +204,7 @@ (defn update-profile-photo [{:keys [::db/pool ::sto/storage] :as cfg} {:keys [profile-id file] :as params}] (let [photo (upload-photo cfg params) - profile (db/get-by-id pool :profile profile-id ::db/for-update? true)] + profile (db/get-by-id pool :profile profile-id ::sql/for-update true)] ;; Schedule deletion of old photo (when-let [id (:photo-id profile)] @@ -329,7 +331,7 @@ ::sm/params schema:update-profile-props} [{:keys [::db/pool]} {:keys [::rpc/profile-id props]}] (db/with-atomic [conn pool] - (let [profile (get-profile conn profile-id ::db/for-update? true) + (let [profile (get-profile conn profile-id ::sql/for-update true) props (reduce-kv (fn [props k v] ;; We don't accept namespaced keys (if (simple-ident? k) diff --git a/backend/src/app/rpc/commands/projects.clj b/backend/src/app/rpc/commands/projects.clj index 6b4e72091..b8a555f44 100644 --- a/backend/src/app/rpc/commands/projects.clj +++ b/backend/src/app/rpc/commands/projects.clj @@ -9,6 +9,7 @@ [app.common.data.macros :as dm] [app.common.spec :as us] [app.db :as db] + [app.db.sql :as-alias sql] [app.loggers.audit :as-alias audit] [app.loggers.webhooks :as webhooks] [app.rpc :as-alias rpc] @@ -233,7 +234,7 @@ [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id id name] :as params}] (db/with-atomic [conn pool] (check-edition-permissions! conn profile-id id) - (let [project (db/get-by-id conn :project id ::db/for-update? true)] + (let [project (db/get-by-id conn :project id ::sql/for-update true)] (db/update! conn :project {:name name} {:id id}) @@ -259,7 +260,8 @@ (check-edition-permissions! conn profile-id id) (let [project (db/update! conn :project {:deleted-at (dt/now)} - {:id id :is-default false})] + {:id id :is-default false} + {::db/return-keys true})] (rph/with-meta (rph/wrap) {::audit/props {:team-id (:team-id project) :name (:name project) diff --git a/backend/src/app/rpc/commands/teams.clj b/backend/src/app/rpc/commands/teams.clj index 791bce1d6..264fca2a1 100644 --- a/backend/src/app/rpc/commands/teams.clj +++ b/backend/src/app/rpc/commands/teams.clj @@ -963,5 +963,6 @@ (let [invitation (db/delete! conn :team-invitation {:team-id team-id - :email-to (str/lower email)})] + :email-to (str/lower email)} + {::db/return-keys true})] (rph/wrap nil {::audit/props {:invitation-id (:id invitation)}}))))) diff --git a/backend/src/app/rpc/commands/webhooks.clj b/backend/src/app/rpc/commands/webhooks.clj index 1c6b812c5..13a5d0210 100644 --- a/backend/src/app/rpc/commands/webhooks.clj +++ b/backend/src/app/rpc/commands/webhooks.clj @@ -95,7 +95,8 @@ :mtype mtype :error-code nil :error-count 0} - {:id id}) + {:id id} + {::db/return-keys true}) (decode-row))) (sv/defmethod ::create-webhook diff --git a/backend/src/app/srepl/cli.clj b/backend/src/app/srepl/cli.clj index c3a4bd0c1..9b4943bdc 100644 --- a/backend/src/app/srepl/cli.clj +++ b/backend/src/app/srepl/cli.clj @@ -65,9 +65,8 @@ (let [res (db/update! conn :profile params {:email email - :deleted-at nil} - {::db/return-keys? false})] - (pos? (:next.jdbc/update-count res)))))))) + :deleted-at nil})] + (pos? (db/get-update-count res)))))))) (defmethod exec-command :delete-profile [{:keys [email soft]}] @@ -82,12 +81,10 @@ (let [res (if soft (db/update! conn :profile {:deleted-at (dt/now)} - {:email email :deleted-at nil} - {::db/return-keys? false}) + {:email email :deleted-at nil}) (db/delete! conn :profile - {:email email} - {::db/return-keys? false}))] - (pos? (:next.jdbc/update-count res)))))) + {:email email}))] + (pos? (db/get-update-count res)))))) (defmethod exec-command :search-profile [{:keys [email]}] diff --git a/backend/src/app/storage.clj b/backend/src/app/storage.clj index 5d24f8e68..c27672e2a 100644 --- a/backend/src/app/storage.clj +++ b/backend/src/app/storage.clj @@ -170,8 +170,7 @@ (let [id (if (impl/object? object-or-id) (:id object-or-id) object-or-id) rs (db/update! pool-or-conn :storage-object {:touched-at (dt/now)} - {:id id} - {::db/return-keys? false})] + {:id id})] (pos? (db/get-update-count rs)))) (defn get-object-data @@ -220,8 +219,7 @@ (let [id (if (impl/object? object-or-id) (:id object-or-id) object-or-id) res (db/update! pool-or-conn :storage-object {:deleted-at (dt/now)} - {:id id} - {::db/return-keys? false})] + {:id id})] (pos? (db/get-update-count res)))) (dm/export impl/resolve-backend) diff --git a/backend/src/app/storage/gc_deleted.clj b/backend/src/app/storage/gc_deleted.clj index ec90d483f..2852c8fe6 100644 --- a/backend/src/app/storage/gc_deleted.clj +++ b/backend/src/app/storage/gc_deleted.clj @@ -47,8 +47,7 @@ [conn ids] (let [ids (db/create-array conn "uuid" ids)] (-> (db/exec-one! conn [sql:delete-sobjects ids]) - (db/get-update-count)))) - + (db/get-update-count)))) (defn- delete-in-bulk! [cfg backend-id ids] @@ -60,7 +59,6 @@ (fn [{:keys [::db/conn ::sto/storage]}] (when-let [ids (lock-ids conn ids)] (let [total (delete-sobjects! conn ids)] - (-> (impl/resolve-backend storage backend-id) (impl/del-objects-in-bulk ids)) @@ -68,7 +66,6 @@ (l/dbg :hint "permanently delete storage object" :id (str id) :backend (name backend-id))) - total)))) (catch Throwable cause (l/err :hint "unexpected error on bulk deletion" diff --git a/backend/src/app/tasks/file_gc.clj b/backend/src/app/tasks/file_gc.clj index 3e75cb728..dcb92a457 100644 --- a/backend/src/app/tasks/file_gc.clj +++ b/backend/src/app/tasks/file_gc.clj @@ -249,8 +249,7 @@ (blob/encode))] (db/update! conn :file {:data data} - {:id file-id} - {::db/return-keys? false})) + {:id file-id})) (count unused)))) @@ -315,7 +314,6 @@ ;; Mark file as trimmed (db/update! conn :file {:has-media-trimmed true} - {:id id} - {::db/return-keys? false}) + {:id id}) (feat.fdata/persist-pointers! cfg id)))) diff --git a/backend/src/app/tasks/objects_gc.clj b/backend/src/app/tasks/objects_gc.clj index 4dec5fa0d..c5e74ce3a 100644 --- a/backend/src/app/tasks/objects_gc.clj +++ b/backend/src/app/tasks/objects_gc.clj @@ -89,9 +89,7 @@ ;; CASCADE database triggers. This may leave orphan ;; teams, but there is a special task for deleting ;; orphaned teams. - (db/delete! conn :profile - {:id id} - {::db/return-keys? false}) + (db/delete! conn :profile {:id id}) (inc total)) 0))) @@ -118,20 +116,16 @@ (some->> photo-id (sto/touch-object! storage)) ;; And finally, permanently delete the team. - (db/delete! conn :team - {:id id} - {::db/return-keys? false}) + (db/delete! conn :team {:id id}) ;; Mark for deletion in cascade (db/update! conn :team-font-variant {:deleted-at deleted-at} - {:team-id id} - {::db/return-keys? false}) + {:team-id id}) (db/update! conn :project {:deleted-at deleted-at} - {:team-id id} - {::db/return-keys? false}) + {:team-id id}) (inc total)) 0))) @@ -162,9 +156,7 @@ (some->> (:ttf-file-id font) (sto/touch-object! storage)) ;; And finally, permanently delete the team font variant - (db/delete! conn :team-font-variant - {:id id} - {::db/return-keys? false}) + (db/delete! conn :team-font-variant {:id id}) (inc total)) 0))) @@ -187,16 +179,14 @@ :id (str id) :team-id (str team-id) :deleted-at (dt/format-instant deleted-at)) + ;; And finally, permanently delete the project. - (db/delete! conn :project - {:id id} - {::db/return-keys? false}) + (db/delete! conn :project {:id id}) ;; Mark files to be deleted (db/update! conn :file {:deleted-at deleted-at} - {:project-id id} - {::db/return-keys? false}) + {:project-id id}) (inc total)) 0))) @@ -221,25 +211,21 @@ :deleted-at (dt/format-instant deleted-at)) ;; And finally, permanently delete the file. - (db/delete! conn :file - {:id id} - {::db/return-keys? false}) + (db/delete! conn :file {:id id}) ;; Mark file media objects to be deleted (db/update! conn :file-media-object {:deleted-at deleted-at} - {:file-id id} - {::db/return-keys? false}) + {:file-id id}) ;; Mark thumbnails to be deleted (db/update! conn :file-thumbnail {:deleted-at deleted-at} - {:file-id id} - {::db/return-keys? false}) + {:file-id id}) + (db/update! conn :file-tagged-object-thumbnail {:deleted-at deleted-at} - {:file-id id} - {::db/return-keys? false}) + {:file-id id}) (inc total)) 0))) diff --git a/backend/src/app/tasks/orphan_teams_gc.clj b/backend/src/app/tasks/orphan_teams_gc.clj index f7f1daedf..c04123a83 100644 --- a/backend/src/app/tasks/orphan_teams_gc.clj +++ b/backend/src/app/tasks/orphan_teams_gc.clj @@ -54,7 +54,6 @@ (l/trc :hint "mark orphan team for deletion" :id (str team-id)) (db/update! conn :team {:deleted-at (dt/now)} - {:id team-id} - {::db/return-keys? false}) + {:id team-id}) (inc total)) 0))) diff --git a/backend/test/backend_tests/rpc_file_thumbnails_test.clj b/backend/test/backend_tests/rpc_file_thumbnails_test.clj index d88b5ed9f..12e5d7158 100644 --- a/backend/test/backend_tests/rpc_file_thumbnails_test.clj +++ b/backend/test/backend_tests/rpc_file_thumbnails_test.clj @@ -140,7 +140,7 @@ (t/is (= 0 (:freeze res)))) ;; check that storage object is still exists but is marked as deleted - (let [row (th/db-get :storage-object {:id (:media-id row1)} {::db/remove-deleted? false})] + (let [row (th/db-get :storage-object {:id (:media-id row1)} {::db/remove-deleted false})] (t/is (some? (:deleted-at row)))) ;; Run the storage gc deleted task, it should permanently delete @@ -152,7 +152,7 @@ (t/is (some? (sto/get-object storage (:media-id row2)))) ;; check that storage object is still exists but is marked as deleted - (let [row (th/db-get :storage-object {:id (:media-id row1)} {::db/remove-deleted? false})] + (let [row (th/db-get :storage-object {:id (:media-id row1)} {::db/remove-deleted false})] (t/is (nil? row)))))) (t/deftest create-file-thumbnail @@ -240,7 +240,7 @@ (t/is (nil? (sto/get-object storage (:media-id row1)))) (t/is (some? (sto/get-object storage (:media-id row2)))) - (let [row (th/db-get :storage-object {:id (:media-id row1)} {::db/remove-deleted? false})] + (let [row (th/db-get :storage-object {:id (:media-id row1)} {::db/remove-deleted false})] (t/is (some? (:deleted-at row)))) ;; Run the storage gc deleted task, it should permanently delete @@ -320,6 +320,3 @@ (let [result (:result out)] (t/is (contains? result "test-key-2")))))) - - - diff --git a/backend/test/backend_tests/rpc_profile_test.clj b/backend/test/backend_tests/rpc_profile_test.clj index 0bfc01fc7..f21dc17ec 100644 --- a/backend/test/backend_tests/rpc_profile_test.clj +++ b/backend/test/backend_tests/rpc_profile_test.clj @@ -149,7 +149,7 @@ (let [row (th/db-get :team {:id (:default-team-id prof)} - {::db/remove-deleted? false})] + {::db/remove-deleted false})] (t/is (nil? (:deleted-at row)))) (let [result (th/run-task! :orphan-teams-gc {:min-age 0})] @@ -157,7 +157,7 @@ (let [row (th/db-get :team {:id (:default-team-id prof)} - {::db/remove-deleted? false})] + {::db/remove-deleted false})] (t/is (dt/instant? (:deleted-at row)))) ;; query profile after delete