From a1f5bcae80d36fd32c6cdedf0976933cd48ef361 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 3 Oct 2024 15:56:53 +0200 Subject: [PATCH] :recycle: Add better ergonomics for the internal quotes API --- backend/scripts/repl | 1 + backend/scripts/start-dev | 1 + backend/src/app/rpc/commands/access_token.clj | 37 ++- backend/src/app/rpc/commands/comments.clj | 59 ++-- backend/src/app/rpc/commands/files_create.clj | 77 ++--- backend/src/app/rpc/commands/fonts.clj | 9 +- backend/src/app/rpc/commands/projects.clj | 33 +- backend/src/app/rpc/commands/teams.clj | 217 +++++++------ backend/src/app/rpc/commands/verify_token.clj | 25 +- backend/src/app/rpc/quotes.clj | 289 +++++++++++------- backend/test/backend_tests/rpc_font_test.clj | 2 +- frontend/src/app/main/ui/dashboard/team.cljs | 4 + .../app/main/ui/onboarding/team_choice.cljs | 4 + 13 files changed, 412 insertions(+), 346 deletions(-) diff --git a/backend/scripts/repl b/backend/scripts/repl index 9d3a5a808..7170dbee4 100755 --- a/backend/scripts/repl +++ b/backend/scripts/repl @@ -23,6 +23,7 @@ export PENPOT_FLAGS="\ enable-urepl-server \ enable-rpc-climit \ enable-rpc-rlimit \ + enable-quotes \ enable-soft-rpc-rlimit \ enable-auto-file-snapshot \ enable-webhooks \ diff --git a/backend/scripts/start-dev b/backend/scripts/start-dev index b137af101..65ccbc9c1 100755 --- a/backend/scripts/start-dev +++ b/backend/scripts/start-dev @@ -17,6 +17,7 @@ export PENPOT_FLAGS="\ disable-secure-session-cookies \ enable-rpc-climit \ enable-smtp \ + enable-quotes \ enable-file-snapshot \ enable-access-tokens \ enable-tiered-file-data-storage \ diff --git a/backend/src/app/rpc/commands/access_token.clj b/backend/src/app/rpc/commands/access_token.clj index e8d9675f9..f1cb1d425 100644 --- a/backend/src/app/rpc/commands/access_token.clj +++ b/backend/src/app/rpc/commands/access_token.clj @@ -30,18 +30,17 @@ :tid token-id :iat created-at}) - expires-at (some-> expiration dt/in-future)] - - (db/insert! conn :access-token - {:id token-id - :name name - :token token - :profile-id profile-id - :created-at created-at - :updated-at created-at - :expires-at expires-at - :perms (db/create-array conn "text" [])}))) - + expires-at (some-> expiration dt/in-future) + token (db/insert! conn :access-token + {:id token-id + :name name + :token token + :profile-id profile-id + :created-at created-at + :updated-at created-at + :expires-at expires-at + :perms (db/create-array conn "text" [])})] + (decode-row token))) (defn repl:create-access-token [{:keys [::db/pool] :as system} profile-id name expiration] @@ -60,14 +59,12 @@ (sv/defmethod ::create-access-token {::doc/added "1.18" ::sm/params schema:create-access-token} - [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id name expiration]}] - (db/with-atomic [conn pool] - (let [cfg (assoc cfg ::db/conn conn)] - (quotes/check-quote! conn - {::quotes/id ::quotes/access-tokens-per-profile - ::quotes/profile-id profile-id}) - (-> (create-access-token cfg profile-id name expiration) - (decode-row))))) + [cfg {:keys [::rpc/profile-id name expiration]}] + + (quotes/check! cfg {::quotes/id ::quotes/access-tokens-per-profile + ::quotes/profile-id profile-id}) + + (db/tx-run! cfg create-access-token profile-id name expiration)) (def ^:private schema:delete-access-token [:map {:title "delete-access-token"} diff --git a/backend/src/app/rpc/commands/comments.clj b/backend/src/app/rpc/commands/comments.clj index cbc214262..f98099461 100644 --- a/backend/src/app/rpc/commands/comments.clj +++ b/backend/src/app/rpc/commands/comments.clj @@ -297,38 +297,38 @@ [:frame-id ::sm/uuid] [:share-id {:optional true} [:maybe ::sm/uuid]]]) +;; FIXME: relax transaction requirements + (sv/defmethod ::create-comment-thread {::doc/added "1.15" ::webhooks/event? true ::rtry/enabled true ::rtry/when rtry/conflict-exception? - ::sm/params schema:create-comment-thread} - [cfg {:keys [::rpc/profile-id ::rpc/request-at file-id page-id share-id position content frame-id]}] + ::sm/params schema:create-comment-thread + ::db/transaction true} + [{:keys [::db/conn] :as cfg} + {:keys [::rpc/profile-id ::rpc/request-at file-id page-id share-id position content frame-id]}] - (db/tx-run! cfg (fn [{:keys [::db/conn] :as cfg}] - (files/check-comment-permissions! cfg profile-id file-id share-id) - (let [{:keys [team-id project-id page-name]} (get-file conn file-id page-id)] + (files/check-comment-permissions! cfg profile-id file-id share-id) - (run! (partial quotes/check-quote! cfg) - (list {::quotes/id ::quotes/comment-threads-per-file - ::quotes/profile-id profile-id - ::quotes/team-id team-id - ::quotes/project-id project-id - ::quotes/file-id file-id} - {::quotes/id ::quotes/comments-per-file - ::quotes/profile-id profile-id - ::quotes/team-id team-id - ::quotes/project-id project-id - ::quotes/file-id file-id})) + (let [{:keys [team-id project-id page-name]} (get-file conn file-id page-id)] - (create-comment-thread conn {:created-at request-at - :profile-id profile-id - :file-id file-id - :page-id page-id - :page-name page-name - :position position - :content content - :frame-id frame-id}))))) + (-> cfg + (assoc ::quotes/profile-id profile-id) + (assoc ::quotes/team-id team-id) + (assoc ::quotes/project-id project-id) + (assoc ::quotes/file-id file-id) + (quotes/check! {::quotes/id ::quotes/comment-threads-per-file} + {::quotes/id ::quotes/comments-per-file})) + + (create-comment-thread conn {:created-at request-at + :profile-id profile-id + :file-id file-id + :page-id page-id + :page-name page-name + :position position + :content content + :frame-id frame-id}))) (defn- create-comment-thread [conn {:keys [profile-id file-id page-id page-name created-at position content frame-id]}] @@ -432,12 +432,11 @@ {:keys [team-id project-id page-name] :as file} (get-file cfg file-id page-id)] (files/check-comment-permissions! conn profile-id file-id share-id) - (quotes/check-quote! conn - {::quotes/id ::quotes/comments-per-file - ::quotes/profile-id profile-id - ::quotes/team-id team-id - ::quotes/project-id project-id - ::quotes/file-id file-id}) + (quotes/check! cfg {::quotes/id ::quotes/comments-per-file + ::quotes/profile-id profile-id + ::quotes/team-id team-id + ::quotes/project-id project-id + ::quotes/file-id file-id}) ;; Update the page-name cached attribute on comment thread table. (when (not= page-name (:page-name thread)) diff --git a/backend/src/app/rpc/commands/files_create.clj b/backend/src/app/rpc/commands/files_create.clj index bbff2a173..72c3ab884 100644 --- a/backend/src/app/rpc/commands/files_create.clj +++ b/backend/src/app/rpc/commands/files_create.clj @@ -98,46 +98,49 @@ {::doc/added "1.17" ::doc/module :files ::webhooks/event? true - ::sm/params schema:create-file} - [cfg {:keys [::rpc/profile-id project-id] :as params}] - (db/tx-run! cfg - (fn [{:keys [::db/conn] :as cfg}] - (projects/check-edition-permissions! conn profile-id project-id) - (let [team (teams/get-team conn - :profile-id profile-id - :project-id project-id) - team-id (:id team) + ::sm/params schema:create-file + ::db/transaction true} + [{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id project-id] :as params}] + (projects/check-edition-permissions! conn profile-id project-id) + (let [team (teams/get-team conn + :profile-id profile-id + :project-id project-id) + team-id (:id team) - ;; When we create files, we only need to respect the team - ;; features, because some features can be enabled - ;; globally, but the team is still not migrated properly. - features (-> (cfeat/get-team-enabled-features cf/flags team) - (cfeat/check-client-features! (:features params))) + ;; When we create files, we only need to respect the team + ;; features, because some features can be enabled + ;; globally, but the team is still not migrated properly. + features (-> (cfeat/get-team-enabled-features cf/flags team) + (cfeat/check-client-features! (:features params))) - ;; We also include all no migration features declared by - ;; client; that enables the ability to enable a runtime - ;; feature on frontend and make it permanent on file - features (-> (:features params #{}) - (set/intersection cfeat/no-migration-features) - (set/union features)) + ;; We also include all no migration features declared by + ;; client; that enables the ability to enable a runtime + ;; feature on frontend and make it permanent on file + features (-> (:features params #{}) + (set/intersection cfeat/no-migration-features) + (set/union features)) - params (-> params - (assoc :profile-id profile-id) - (assoc :features features))] + params (-> params + (assoc :profile-id profile-id) + (assoc :features features))] - (run! (partial quotes/check-quote! conn) - (list {::quotes/id ::quotes/files-per-project - ::quotes/team-id team-id - ::quotes/profile-id profile-id - ::quotes/project-id project-id})) + (quotes/check! cfg {::quotes/id ::quotes/files-per-project + ::quotes/team-id team-id + ::quotes/profile-id profile-id + ::quotes/project-id project-id}) - ;; When newly computed features does not match exactly with - ;; the features defined on team row, we update it. - (when (not= features (:features team)) - (let [features (db/create-array conn "text" features)] - (db/update! conn :team - {:features features} - {:id team-id}))) + ;; FIXME: IMPORTANT: this code can have race + ;; conditions, because we have no locks for updating + ;; team so, creating two files concurrently can lead + ;; to lost team features updating - (-> (create-file cfg params) - (vary-meta assoc ::audit/props {:team-id team-id})))))) + ;; When newly computed features does not match exactly with + ;; the features defined on team row, we update it. + (when (not= features (:features team)) + (let [features (db/create-array conn "text" features)] + (db/update! conn :team + {:features features} + {:id team-id}))) + + (-> (create-file cfg params) + (vary-meta assoc ::audit/props {:team-id team-id})))) diff --git a/backend/src/app/rpc/commands/fonts.clj b/backend/src/app/rpc/commands/fonts.clj index 51081eb19..43b90305e 100644 --- a/backend/src/app/rpc/commands/fonts.clj +++ b/backend/src/app/rpc/commands/fonts.clj @@ -86,6 +86,9 @@ [:font-weight [::sm/one-of {:format "number"} valid-weight]] [:font-style [::sm/one-of {:format "string"} valid-style]]]) +;; FIXME: IMPORTANT: refactor this, we should not hold a whole db +;; connection around the font creation + (sv/defmethod ::create-font-variant {::doc/added "1.18" ::climit/id [[:process-font/by-profile ::rpc/profile-id] @@ -96,9 +99,9 @@ (db/tx-run! cfg (fn [{:keys [::db/conn] :as cfg}] (teams/check-edition-permissions! conn profile-id team-id) - (quotes/check-quote! conn {::quotes/id ::quotes/font-variants-per-team - ::quotes/profile-id profile-id - ::quotes/team-id team-id}) + (quotes/check! cfg {::quotes/id ::quotes/font-variants-per-team + ::quotes/profile-id profile-id + ::quotes/team-id team-id}) (create-font-variant cfg (assoc params :profile-id profile-id))))) (defn create-font-variant diff --git a/backend/src/app/rpc/commands/projects.clj b/backend/src/app/rpc/commands/projects.clj index 3442fe80f..1b8310232 100644 --- a/backend/src/app/rpc/commands/projects.clj +++ b/backend/src/app/rpc/commands/projects.clj @@ -168,6 +168,17 @@ ;; --- MUTATION: Create Project +(defn- create-project + [{:keys [::db/conn] :as cfg} {:keys [profile-id team-id] :as params}] + (let [project (teams/create-project conn params)] + (teams/create-project-role conn profile-id (:id project) :owner) + (db/insert! conn :team-project-profile-rel + {:project-id (:id project) + :profile-id profile-id + :team-id team-id + :is-pinned false}) + (assoc project :is-pinned false))) + (def ^:private schema:create-project [:map {:title "create-project"} [:team-id ::sm/uuid] @@ -178,23 +189,15 @@ {::doc/added "1.18" ::webhooks/event? true ::sm/params schema:create-project} - [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id team-id] :as params}] - (db/with-atomic [conn pool] - (teams/check-edition-permissions! conn profile-id team-id) - (quotes/check-quote! conn {::quotes/id ::quotes/projects-per-team - ::quotes/profile-id profile-id - ::quotes/team-id team-id}) + [cfg {:keys [::rpc/profile-id team-id] :as params}] - (let [params (assoc params :profile-id profile-id) - project (teams/create-project conn params)] - (teams/create-project-role conn profile-id (:id project) :owner) - (db/insert! conn :team-project-profile-rel - {:project-id (:id project) - :profile-id profile-id - :team-id team-id - :is-pinned false}) - (assoc project :is-pinned false)))) + (teams/check-edition-permissions! cfg profile-id team-id) + (quotes/check! cfg {::quotes/id ::quotes/projects-per-team + ::quotes/profile-id profile-id + ::quotes/team-id team-id}) + (let [params (assoc params :profile-id profile-id)] + (db/tx-run! cfg create-project params))) ;; --- MUTATION: Toggle Project Pin diff --git a/backend/src/app/rpc/commands/teams.clj b/backend/src/app/rpc/commands/teams.clj index 1d9cf98f7..ca1cd40fb 100644 --- a/backend/src/app/rpc/commands/teams.clj +++ b/backend/src/app/rpc/commands/teams.clj @@ -401,19 +401,22 @@ (sv/defmethod ::create-team {::doc/added "1.17" - ::sm/params schema:create-team} + ::sm/params schema:create-team + ::db/transaction true} [cfg {:keys [::rpc/profile-id] :as params}] - (db/tx-run! cfg (fn [{:keys [::db/conn] :as cfg}] - (quotes/check-quote! conn {::quotes/id ::quotes/teams-per-profile - ::quotes/profile-id profile-id}) - (let [features (-> (cfeat/get-enabled-features cf/flags) - (cfeat/check-client-features! (:features params))) - team (create-team cfg (assoc params - :profile-id profile-id - :features features))] - (with-meta team - {::audit/props {:id (:id team)}}))))) + (quotes/check! cfg {::quotes/id ::quotes/teams-per-profile + ::quotes/profile-id profile-id}) + + (let [features (-> (cfeat/get-enabled-features cf/flags) + (cfeat/check-client-features! (:features params))) + params (-> params + (assoc :profile-id profile-id) + (assoc :features features)) + team (create-team cfg params)] + + (with-meta team + {::audit/props {:id (:id team)}}))) (defn create-team "This is a complete team creation process, it creates the team @@ -867,10 +870,11 @@ (ex/raise :type :restriction :code :profile-blocked)) - (quotes/check-quote! conn - {::quotes/id ::quotes/profiles-per-team - ::quotes/profile-id (:id member) - ::quotes/team-id team-id}) + (quotes/check! + {::db/conn conn + ::quotes/id ::quotes/profiles-per-team + ::quotes/profile-id (:id member) + ::quotes/team-id team-id}) ;; Insert the member to the team (db/insert! conn :team-profile-rel params {::db/on-conflict-do-nothing? true}) @@ -916,64 +920,62 @@ "A rpc call that allow to send a single or multiple invitations to join the team." {::doc/added "1.17" - ::sm/params schema:create-team-invitations} - [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id team-id emails role] :as params}] - (db/with-atomic [conn pool] - (let [perms (get-permissions conn profile-id team-id) - profile (db/get-by-id conn :profile profile-id) - team (db/get-by-id conn :team team-id) - emails (into #{} (map profile/clean-email) emails)] + ::sm/params schema:create-team-invitations + ::db/transaction true} + [{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id team-id emails role] :as params}] + (let [perms (get-permissions conn profile-id team-id) + profile (db/get-by-id conn :profile profile-id) + team (db/get-by-id conn :team team-id) + emails (into #{} (map profile/clean-email) emails)] - (when (> (count emails) max-invitations-by-request-threshold) - (ex/raise :type :validation - :code :max-invitations-by-request - :hint "the maximum of invitation on single request is reached" - :threshold max-invitations-by-request-threshold)) + (when-not (:is-admin perms) + (ex/raise :type :validation + :code :insufficient-permissions)) - (run! (partial quotes/check-quote! conn) - (list {::quotes/id ::quotes/invitations-per-team - ::quotes/profile-id profile-id - ::quotes/team-id (:id team) - ::quotes/incr (count emails)} - {::quotes/id ::quotes/profiles-per-team - ::quotes/profile-id profile-id - ::quotes/team-id (:id team) - ::quotes/incr (count emails)})) + (when (> (count emails) max-invitations-by-request-threshold) + (ex/raise :type :validation + :code :max-invitations-by-request + :hint "the maximum of invitation on single request is reached" + :threshold max-invitations-by-request-threshold)) - (when-not (:is-admin perms) - (ex/raise :type :validation - :code :insufficient-permissions)) + (-> cfg + (assoc ::quotes/profile-id profile-id) + (assoc ::quotes/team-id team-id) + (assoc ::quotes/incr (count emails)) + (quotes/check! {::quotes/id ::quotes/invitations-per-team} + {::quotes/id ::quotes/profiles-per-team})) - ;; Check if the current profile is allowed to send emails. - (check-valid-email-muted conn profile) + ;; Check if the current profile is allowed to send emails + (check-valid-email-muted conn profile) + (let [requested (into #{} (map :email) (get-valid-requests-email conn team-id)) + emails-to-add (filter #(contains? requested %) emails) + emails (remove #(contains? requested %) emails) + cfg (assoc cfg ::db/conn conn) + members (->> (db/exec! conn [sql:team-members team-id]) + (into #{} (map :email))) - (let [requested (into #{} (map :email) (get-valid-requests-email conn team-id)) - emails-to-add (filter #(contains? requested %) emails) - emails (remove #(contains? requested %) emails) - cfg (assoc cfg ::db/conn conn) - members (->> (db/exec! conn [sql:team-members team-id]) - (into #{} (map :email))) + invitations (into #{} + (comp + ;; We don't re-send inviation to already existing members + (remove (partial contains? members)) + (map (fn [email] + (-> params + (assoc :email email) + (assoc :team team) + (assoc :profile profile) + (assoc :role role)))) + (keep (partial create-invitation cfg))) + emails)] + ;; For requested invitations, do not send + ;; invitation emails, add the user directly to + ;; the team + (doseq [email emails-to-add] + (add-user-to-team conn profile team email role)) - invitations (into #{} - (comp - ;; We don't re-send inviation to already existing members - (remove (partial contains? members)) - (map (fn [email] - (-> params - (assoc :email email) - (assoc :team team) - (assoc :profile profile) - (assoc :role role)))) - (keep (partial create-invitation cfg))) - emails)] - ;; For requested invitations, do not send invitation emails, add the user directly to the team - (doseq [email emails-to-add] - (add-user-to-team conn profile team email role)) - - (with-meta {:total (count invitations) - :invitations invitations} - {::audit/props {:invitations (count invitations)}}))))) + (with-meta {:total (count invitations) + :invitations invitations} + {::audit/props {:invitations (count invitations)}})))) ;; --- Mutation: Create Team & Invite Members @@ -987,58 +989,51 @@ (sv/defmethod ::create-team-with-invitations {::doc/added "1.17" - ::sm/params schema:create-team-with-invitations} - [cfg {:keys [::rpc/profile-id emails role name] :as params}] + ::sm/params schema:create-team-with-invitations + ::db/transaction true} + [{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id emails role name] :as params}] + (let [features (-> (cfeat/get-enabled-features cf/flags) + (cfeat/check-client-features! (:features params))) - (db/tx-run! cfg - (fn [{:keys [::db/conn] :as cfg}] - (let [features (-> (cfeat/get-enabled-features cf/flags) - (cfeat/check-client-features! (:features params))) + params (-> params + (assoc :profile-id profile-id) + (assoc :features features)) - params (-> params - (assoc :profile-id profile-id) - (assoc :features features)) + team (create-team cfg params) + emails (into #{} (map profile/clean-email) emails)] - cfg (assoc cfg ::db/conn conn) - team (create-team cfg params) - profile (db/get-by-id conn :profile profile-id) - emails (into #{} (map profile/clean-email) emails)] + (-> cfg + (assoc ::quotes/profile-id profile-id) + (assoc ::quotes/team-id (:id team)) + (assoc ::quotes/incr (count emails)) + (quotes/check! {::quotes/id ::quotes/teams-per-profile} + {::quotes/id ::quotes/invitations-per-team} + {::quotes/id ::quotes/profiles-per-team})) - (when (> (count emails) max-invitations-by-request-threshold) - (ex/raise :type :validation - :code :max-invitations-by-request - :hint "the maximum of invitation on single request is reached" - :threshold max-invitations-by-request-threshold)) + (when (> (count emails) max-invitations-by-request-threshold) + (ex/raise :type :validation + :code :max-invitations-by-request + :hint "the maximum of invitation on single request is reached" + :threshold max-invitations-by-request-threshold)) - (let [props {:name name :features features} - event (-> (audit/event-from-rpc-params params) - (assoc ::audit/name "create-team") - (assoc ::audit/props props))] - (audit/submit! cfg event)) + (let [props {:name name :features features} + event (-> (audit/event-from-rpc-params params) + (assoc ::audit/name "create-team") + (assoc ::audit/props props))] + (audit/submit! cfg event)) - ;; Create invitations for all provided emails. - (->> emails - (map (fn [email] - (-> params - (assoc :team team) - (assoc :profile profile) - (assoc :email email) - (assoc :role role)))) - (run! (partial create-invitation cfg))) + ;; Create invitations for all provided emails. + (let [profile (db/get-by-id conn :profile profile-id)] + (->> emails + (map (fn [email] + (-> params + (assoc :team team) + (assoc :profile profile) + (assoc :email email) + (assoc :role role)))) + (run! (partial create-invitation cfg)))) - (run! (partial quotes/check-quote! conn) - (list {::quotes/id ::quotes/teams-per-profile - ::quotes/profile-id profile-id} - {::quotes/id ::quotes/invitations-per-team - ::quotes/profile-id profile-id - ::quotes/team-id (:id team) - ::quotes/incr (count emails)} - {::quotes/id ::quotes/profiles-per-team - ::quotes/profile-id profile-id - ::quotes/team-id (:id team) - ::quotes/incr (count emails)})) - - (vary-meta team assoc ::audit/props {:invitations (count emails)}))))) + (vary-meta team assoc ::audit/props {:invitations (count emails)}))) ;; --- Query: get-team-invitation-token diff --git a/backend/src/app/rpc/commands/verify_token.clj b/backend/src/app/rpc/commands/verify_token.clj index 6ed6d5814..7f0dd6b5f 100644 --- a/backend/src/app/rpc/commands/verify_token.clj +++ b/backend/src/app/rpc/commands/verify_token.clj @@ -37,14 +37,12 @@ ::doc/added "1.15" ::doc/module :auth ::sm/params schema:verify-token} - [{:keys [::db/pool] :as cfg} {:keys [token] :as params}] - (db/with-atomic [conn pool] - (let [claims (tokens/verify (::setup/props cfg) {:token token}) - cfg (assoc cfg :conn conn)] - (process-token cfg params claims)))) + [cfg {:keys [token] :as params}] + (let [claims (tokens/verify (::setup/props cfg) {:token token})] + (db/tx-run! cfg process-token params claims))) (defmethod process-token :change-email - [{:keys [conn] :as cfg} _params {:keys [profile-id email] :as claims}] + [{:keys [::db/conn] :as cfg} _params {:keys [profile-id email] :as claims}] (let [email (profile/clean-email email)] (when (profile/get-profile-by-email conn email) (ex/raise :type :validation @@ -60,7 +58,7 @@ ::audit/profile-id profile-id}))) (defmethod process-token :verify-email - [{:keys [conn] :as cfg} _ {:keys [profile-id] :as claims}] + [{:keys [::db/conn] :as cfg} _ {:keys [profile-id] :as claims}] (let [profile (profile/get-profile conn profile-id) claims (assoc claims :profile profile)] @@ -81,14 +79,14 @@ ::audit/profile-id (:id profile)})))) (defmethod process-token :auth - [{:keys [conn] :as cfg} _params {:keys [profile-id] :as claims}] + [{:keys [::db/conn] :as cfg} _params {:keys [profile-id] :as claims}] (let [profile (profile/get-profile conn profile-id)] (assoc claims :profile profile))) ;; --- Team Invitation (defn- accept-invitation - [{:keys [conn] :as cfg} {:keys [team-id role member-email] :as claims} invitation member] + [{:keys [::db/conn] :as cfg} {:keys [team-id role member-email] :as claims} invitation member] (let [;; Update the role if there is an invitation role (or (some-> invitation :role keyword) role) params (merge @@ -101,10 +99,9 @@ (ex/raise :type :restriction :code :profile-blocked)) - (quotes/check-quote! conn - {::quotes/id ::quotes/profiles-per-team - ::quotes/profile-id (:id member) - ::quotes/team-id team-id}) + (quotes/check! cfg {::quotes/id ::quotes/profiles-per-team + ::quotes/profile-id (:id member) + ::quotes/team-id team-id}) ;; Insert the invited member to the team (db/insert! conn :team-profile-rel params {::db/on-conflict-do-nothing? true}) @@ -140,7 +137,7 @@ (sm/lazy-validator schema:team-invitation-claims)) (defmethod process-token :team-invitation - [{:keys [conn] :as cfg} + [{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id token] :as params} {:keys [member-id team-id member-email] :as claims}] diff --git a/backend/src/app/rpc/quotes.clj b/backend/src/app/rpc/quotes.clj index bdcab5df3..888a5e9ab 100644 --- a/backend/src/app/rpc/quotes.clj +++ b/backend/src/app/rpc/quotes.clj @@ -7,16 +7,13 @@ (ns app.rpc.quotes "Penpot resource usage quotes." (:require - [app.common.data.macros :as dm] [app.common.exceptions :as ex] [app.common.logging :as l] [app.common.schema :as sm] - [app.common.spec :as us] [app.config :as cf] [app.db :as db] [app.util.time :as dt] [app.worker :as wrk] - [clojure.spec.alpha :as s] [cuerdas.core :as str])) (defmulti check-quote ::id) @@ -34,6 +31,9 @@ [::id :keyword] [::profile-id ::sm/uuid]]) +(def valid-quote? + (sm/lazy-validator schema:quote)) + (def ^:private enabled (volatile! true)) (defn enable! @@ -46,20 +46,31 @@ [] (vswap! enabled (constantly false))) -(defn check-quote! - [ds quote] - (dm/assert! - "expected valid quote map" - (sm/validate schema:quote quote)) +(defn- check + [cfg quote] + (let [quote (merge cfg quote) + id (::id quote)] - (when (contains? cf/flags :quotes) - (when @enabled - ;; This approach add flexibility on how and where the - ;; check-quote! can be called (in or out of transaction) - (db/run! ds (fn [cfg] - (-> (merge cfg quote) - (assoc ::target (name (::id quote))) - (check-quote))))))) + (when-not (valid-quote? quote) + (ex/raise :type :internal + :code :invalid-quote-definition + :hint "found invalid data for quote schema" + :quote (name id))) + + (-> (assoc quote ::target (name id)) + (check-quote)))) + +(defn check! + ([cfg] + (when (contains? cf/flags :quotes) + (when @enabled + (db/run! cfg check {})))) + + ([cfg & others] + (when (contains? cf/flags :quotes) + (when @enabled + (db/run! cfg (fn [cfg] + (run! (partial check cfg) others))))))) (defn- send-notification! [{:keys [::db/conn] :as params}] @@ -100,7 +111,7 @@ (map :quote) (reduce max (- Integer/MAX_VALUE))) quote (if (pos? quote) quote default) - total (->> (db/exec! conn count-sql) first :total)] + total (:total (db/exec-one! conn count-sql))] (when (> (+ total incr) quote) (if (contains? cf/flags :soft-quotes) @@ -112,72 +123,81 @@ :count total))))) (def ^:private sql:get-quotes-1 - "select id, quote from usage_quote - where target = ? - and profile_id = ? - and team_id is null - and project_id is null - and file_id is null;") + "SELECT id, quote + FROM usage_quote + WHERE target = ? + AND profile_id = ? + AND team_id IS NULL + AND project_id IS NULL + AND file_id IS NULL;") (def ^:private sql:get-quotes-2 - "select id, quote from usage_quote - where target = ? - and ((team_id = ? and (profile_id = ? or profile_id is null)) or - (profile_id = ? and team_id is null and project_id is null and file_id is null));") + "SELECT id, quote + FROM usage_quote + WHERE target = ? + AND ((team_id = ? AND (profile_id = ? OR profile_id IS NULL)) OR + (profile_id = ? AND team_id IS NULL AND project_id IS NULL AND file_id IS NULL));") (def ^:private sql:get-quotes-3 - "select id, quote from usage_quote - where target = ? - and ((project_id = ? and (profile_id = ? or profile_id is null)) or - (team_id = ? and (profile_id = ? or profile_id is null)) or - (profile_id = ? and team_id is null and project_id is null and file_id is null));") + "SELECT id, quote + FROM usage_quote + WHERE target = ? + AND ((project_id = ? AND (profile_id = ? OR profile_id IS NULL)) OR + (team_id = ? AND (profile_id = ? OR profile_id IS NULL)) OR + (profile_id = ? AND team_id IS NULL AND project_id IS NULL AND file_id IS NULL));") (def ^:private sql:get-quotes-4 - "select id, quote from usage_quote - where target = ? - and ((file_id = ? and (profile_id = ? or profile_id is null)) or - (project_id = ? and (profile_id = ? or profile_id is null)) or - (team_id = ? and (profile_id = ? or profile_id is null)) or - (profile_id = ? and team_id is null and project_id is null and file_id is null));") + "SELECT id, quote + FROM usage_quote + WHERE target = ? + AND ((file_id = ? AND (profile_id = ? OR profile_id IS NULL)) OR + (project_id = ? AND (profile_id = ? OR profile_id IS NULL)) OR + (team_id = ? AND (profile_id = ? OR profile_id IS NULL)) OR + (profile_id = ? AND team_id IS NULL AND project_id IS NULL AND file_id IS NULL));") ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; QUOTE: TEAMS-PER-PROFILE ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(def ^:private sql:get-teams-per-profile - "select count(*) as total - from team_profile_rel - where profile_id = ?") +(def ^:private schema:teams-per-profile + [:map [::profile-id ::sm/uuid]]) -(s/def ::profile-id ::us/uuid) -(s/def ::teams-per-profile - (s/keys :req [::profile-id ::target])) +(def ^:private valid-teams-per-profile-quote? + (sm/lazy-validator schema:teams-per-profile)) + +(def ^:private sql:get-teams-per-profile + "SELECT count(*) AS total + FROM team_profile_rel + WHERE profile_id = ?") (defmethod check-quote ::teams-per-profile [{:keys [::profile-id ::target] :as quote}] - (us/assert! ::teams-per-profile quote) + (assert (valid-teams-per-profile-quote? quote) "invalid quote parameters") (-> quote (assoc ::default (cf/get :quotes-teams-per-profile Integer/MAX_VALUE)) (assoc ::quote-sql [sql:get-quotes-1 target profile-id]) (assoc ::count-sql [sql:get-teams-per-profile profile-id]) (generic-check!))) - ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; QUOTE: ACCESS-TOKENS-PER-PROFILE ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(def ^:private sql:get-access-tokens-per-profile - "select count(*) as total - from access_token - where profile_id = ?") +(def ^:private schema:access-tokens-per-profile + [:map [::profile-id ::sm/uuid]]) -(s/def ::access-tokens-per-profile - (s/keys :req [::profile-id ::target])) +(def ^:private valid-access-tokens-per-profile-quote? + (sm/lazy-validator schema:access-tokens-per-profile)) + +(def ^:private sql:get-access-tokens-per-profile + "SELECT count(*) AS total + FROM access_token + WHERE profile_id = ?") (defmethod check-quote ::access-tokens-per-profile [{:keys [::profile-id ::target] :as quote}] - (us/assert! ::access-tokens-per-profile quote) + (assert (valid-access-tokens-per-profile-quote? quote) "invalid quote parameters") + (-> quote (assoc ::default (cf/get :quotes-access-tokens-per-profile Integer/MAX_VALUE)) (assoc ::quote-sql [sql:get-quotes-1 target profile-id]) @@ -188,40 +208,51 @@ ;; QUOTE: PROJECTS-PER-TEAM ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(def ^:private sql:get-projects-per-team - "select count(*) as total - from project as p - where p.team_id = ? - and p.deleted_at is null") +(def ^:private schema:projects-per-team + [:map + [::profile-id ::sm/uuid] + [::team-id ::sm/uuid]]) -(s/def ::team-id ::us/uuid) -(s/def ::projects-per-team - (s/keys :req [::profile-id ::team-id ::target])) +(def ^:private valid-projects-per-team-quote? + (sm/lazy-validator schema:projects-per-team)) + +(def ^:private sql:get-projects-per-team + "SELECT count(*) AS total + FROM project AS p + WHERE p.team_id = ? + AND p.deleted_at IS NULL") (defmethod check-quote ::projects-per-team [{:keys [::profile-id ::team-id ::target] :as quote}] + (assert (valid-projects-per-team-quote? quote) "invalid quote parameters") + (-> quote (assoc ::default (cf/get :quotes-projects-per-team Integer/MAX_VALUE)) (assoc ::quote-sql [sql:get-quotes-2 target team-id profile-id profile-id]) (assoc ::count-sql [sql:get-projects-per-team team-id]) (generic-check!))) - ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; QUOTE: FONT-VARIANTS-PER-TEAM ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(def ^:private sql:get-font-variants-per-team - "select count(*) as total - from team_font_variant as v - where v.team_id = ?") +(def ^:private schema:font-variants-per-team + [:map + [::profile-id ::sm/uuid] + [::team-id ::sm/uuid]]) -(s/def ::font-variants-per-team - (s/keys :req [::profile-id ::team-id ::target])) +(def ^:private valid-font-variant-per-team-quote? + (sm/lazy-validator schema:font-variants-per-team)) + +(def ^:private sql:get-font-variants-per-team + "SELECT count(*) AS total + FROM team_font_variant AS v + WHERE v.team_id = ?") (defmethod check-quote ::font-variants-per-team [{:keys [::profile-id ::team-id ::target] :as quote}] - (us/assert! ::font-variants-per-team quote) + (assert (valid-font-variant-per-team-quote? quote) "invalid quote parameters") + (-> quote (assoc ::default (cf/get :quotes-font-variants-per-team Integer/MAX_VALUE)) (assoc ::quote-sql [sql:get-quotes-2 target team-id profile-id profile-id]) @@ -233,70 +264,86 @@ ;; QUOTE: INVITATIONS-PER-TEAM ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(def ^:private sql:get-invitations-per-team - "select count(*) as total - from team_invitation - where team_id = ?") +(def ^:private schema:invitations-per-team + [:map + [::profile-id ::sm/uuid] + [::team-id ::sm/uuid]]) -(s/def ::invitations-per-team - (s/keys :req [::profile-id ::team-id ::target])) +(def ^:private valid-invitations-per-team-quote? + (sm/lazy-validator schema:invitations-per-team)) + +(def ^:private sql:get-invitations-per-team + "SELECT count(*) AS total + FROM team_invitation + WHERE team_id = ?") (defmethod check-quote ::invitations-per-team [{:keys [::profile-id ::team-id ::target] :as quote}] - (us/assert! ::invitations-per-team quote) + (assert (valid-invitations-per-team-quote? quote) "invalid quote parameters") + (-> quote (assoc ::default (cf/get :quotes-invitations-per-team Integer/MAX_VALUE)) (assoc ::quote-sql [sql:get-quotes-2 target team-id profile-id profile-id]) (assoc ::count-sql [sql:get-invitations-per-team team-id]) (generic-check!))) - ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; QUOTE: PROFILES-PER-TEAM ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +(def ^:private schema:profiles-per-team + [:map + [::profile-id ::sm/uuid] + [::team-id ::sm/uuid]]) + +(def ^:private valid-profiles-per-team-quote? + (sm/lazy-validator schema:profiles-per-team)) + (def ^:private sql:get-profiles-per-team - "select (select count(*) - from team_profile_rel - where team_id = ?) + - (select count(*) - from team_invitation - where team_id = ? - and valid_until > now()) as total;") + "SELECT (SELECT count(*) + FROM team_profile_rel + WHERE team_id = ?) + + (SELECT count(*) + FROM team_invitation + WHERE team_id = ? + AND valid_until > now()) AS total;") ;; NOTE: the total number of profiles is determined by the number of ;; effective members plus ongoing valid invitations. -(s/def ::profiles-per-team - (s/keys :req [::profile-id ::team-id ::target])) - (defmethod check-quote ::profiles-per-team [{:keys [::profile-id ::team-id ::target] :as quote}] - (us/assert! ::profiles-per-team quote) + (assert (valid-profiles-per-team-quote? quote) "invalid quote parameters") + (-> quote (assoc ::default (cf/get :quotes-profiles-per-team Integer/MAX_VALUE)) (assoc ::quote-sql [sql:get-quotes-2 target team-id profile-id profile-id]) (assoc ::count-sql [sql:get-profiles-per-team team-id team-id]) (generic-check!))) - ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; QUOTE: FILES-PER-PROJECT ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(def ^:private sql:get-files-per-project - "select count(*) as total - from file as f - where f.project_id = ? - and f.deleted_at is null") +(def ^:private schema:files-per-project + [:map + [::profile-id ::sm/uuid] + [::project-id ::sm/uuid] + [::team-id ::sm/uuid]]) -(s/def ::project-id ::us/uuid) -(s/def ::files-per-project - (s/keys :req [::profile-id ::project-id ::team-id ::target])) +(def ^:private valid-files-per-project-quote? + (sm/lazy-validator schema:files-per-project)) + +(def ^:private sql:get-files-per-project + "SELECT count(*) AS total + FROM file AS f + WHERE f.project_id = ? + AND f.deleted_at IS NULL") (defmethod check-quote ::files-per-project [{:keys [::profile-id ::project-id ::team-id ::target] :as quote}] - (us/assert! ::files-per-project quote) + (assert (valid-files-per-project-quote? quote) "invalid quote parameters") + (-> quote (assoc ::default (cf/get :quotes-files-per-project Integer/MAX_VALUE)) (assoc ::quote-sql [sql:get-quotes-3 target project-id profile-id team-id profile-id profile-id]) @@ -307,17 +354,24 @@ ;; QUOTE: COMMENT-THREADS-PER-FILE ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(def ^:private sql:get-comment-threads-per-file - "select count(*) as total - from comment_thread as ct - where ct.file_id = ?") +(def ^:private schema:comment-threads-per-file + [:map + [::profile-id ::sm/uuid] + [::project-id ::sm/uuid] + [::team-id ::sm/uuid]]) -(s/def ::comment-threads-per-file - (s/keys :req [::profile-id ::project-id ::team-id ::target])) +(def ^:private valid-comment-threads-per-file-quote? + (sm/lazy-validator schema:comment-threads-per-file)) + +(def ^:private sql:get-comment-threads-per-file + "SELECT count(*) AS total + FROM comment_thread AS ct + WHERE ct.file_id = ?") (defmethod check-quote ::comment-threads-per-file [{:keys [::profile-id ::file-id ::team-id ::project-id ::target] :as quote}] - (us/assert! ::files-per-project quote) + (assert (valid-comment-threads-per-file-quote? quote) "invalid quote parameters") + (-> quote (assoc ::default (cf/get :quotes-comment-threads-per-file Integer/MAX_VALUE)) (assoc ::quote-sql [sql:get-quotes-4 target file-id profile-id project-id @@ -325,23 +379,28 @@ (assoc ::count-sql [sql:get-comment-threads-per-file file-id]) (generic-check!))) - ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; QUOTE: COMMENTS-PER-FILE ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(def ^:private sql:get-comments-per-file - "select count(*) as total - from comment as c - join comment_thread as ct on (ct.id = c.thread_id) - where ct.file_id = ?") +(def ^:private schema:comments-per-file + [:map + [::profile-id ::sm/uuid] + [::project-id ::sm/uuid] + [::team-id ::sm/uuid]]) -(s/def ::comments-per-file - (s/keys :req [::profile-id ::project-id ::team-id ::target])) +(def ^:private valid-comments-per-file-quote? + (sm/lazy-validator schema:comments-per-file)) + +(def ^:private sql:get-comments-per-file + "SELECT count(*) AS total + FROM comment AS c + JOIN comment_thread AS ct ON (ct.id = c.thread_id) + WHERE ct.file_id = ?") (defmethod check-quote ::comments-per-file [{:keys [::profile-id ::file-id ::team-id ::project-id ::target] :as quote}] - (us/assert! ::files-per-project quote) + (assert (valid-comments-per-file-quote? quote) "invalid quote parameters") (-> quote (assoc ::default (cf/get :quotes-comments-per-file Integer/MAX_VALUE)) (assoc ::quote-sql [sql:get-quotes-4 target file-id profile-id project-id diff --git a/backend/test/backend_tests/rpc_font_test.clj b/backend/test/backend_tests/rpc_font_test.clj index ab9b57f4b..f20796943 100644 --- a/backend/test/backend_tests/rpc_font_test.clj +++ b/backend/test/backend_tests/rpc_font_test.clj @@ -21,7 +21,7 @@ (t/use-fixtures :each th/database-reset) (t/deftest ttf-font-upload-1 - (with-mocks [mock {:target 'app.rpc.quotes/check-quote! :return nil}] + (with-mocks [mock {:target 'app.rpc.quotes/check! :return nil}] (let [prof (th/create-profile* 1 {:is-active true}) team-id (:default-team-id prof) proj-id (:default-project-id prof) diff --git a/frontend/src/app/main/ui/dashboard/team.cljs b/frontend/src/app/main/ui/dashboard/team.cljs index 8cd0bc99f..3770fb568 100644 --- a/frontend/src/app/main/ui/dashboard/team.cljs +++ b/frontend/src/app/main/ui/dashboard/team.cljs @@ -176,6 +176,10 @@ (= :max-invitations-by-request code)) (swap! error-text (tr "errors.maximum-invitations-by-request-reached" (:threshold error))) + (and (= :restriction type) + (= :max-quote-reached code)) + (swap! error-text (tr "errors.max-quote-reached" (:target error))) + (or (= :member-is-muted code) (= :email-has-permanent-bounces code) (= :email-has-complaints code)) diff --git a/frontend/src/app/main/ui/onboarding/team_choice.cljs b/frontend/src/app/main/ui/onboarding/team_choice.cljs index 03b01a257..50ee2fba8 100644 --- a/frontend/src/app/main/ui/onboarding/team_choice.cljs +++ b/frontend/src/app/main/ui/onboarding/team_choice.cljs @@ -98,6 +98,10 @@ (= :max-invitations-by-request code)) (swap! error* (tr "errors.maximum-invitations-by-request-reached" (:threshold error))) + (and (= :restriction type) + (= :max-quote-reached code)) + (swap! error* (tr "errors.max-quote-reached" (:target error))) + (or (= :member-is-muted code) (= :email-has-permanent-bounces code) (= :email-has-complaints code))