From b2c3fd872a707ac30a1f3707f73645df82df6ce7 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 1 Oct 2026 13:16:12 +0200 Subject: [PATCH 1/4] :recycle: Scope organization notifications to specific WebSocket topics (#11460) * :recycle: Scope organization notifications to specific WebSocket topics Publish team/org notifications to team-id and organization-id topics instead of broadcasting to all connections via uuid/zero. Dashboard and workspace now subscribe to their current team and organization on initialization, receiving only relevant events. Backend: added subscribe-organization/unsubscribe-organization WebSocket handlers and updated :close to clean up organization subscriptions. Modified notify-team-change, notify-organization-deletion, and notify-organization-change-sso to publish to specific topics. Frontend: dashboard and workspace now subscribe to team-id and organization-id (when applicable) on initialization, with nil-guard in topic filters. Closes #11455 AI-assisted-by: longcat-2.0 * :wrench: Remove unused session-id binding in unsubscribe-organization Clj-kondo lint fix. AI-assisted-by: longcat-2.0 * :bug: Fix permission check on team ws connection --------- Co-authored-by: alonso.torres --- backend/src/app/http/websocket.clj | 57 +++++++++++++------ backend/src/app/main.clj | 11 ++-- backend/src/app/rpc/notifications.clj | 12 ++-- .../backend_tests/http_websocket_test.clj | 45 +++++++++++++++ .../rpc_management_nitrate_test.clj | 26 +++++++-- frontend/src/app/main/data/dashboard.cljs | 18 ++++-- .../main/data/workspace/notifications.cljs | 5 +- .../frontend_tests/data/dashboard_test.cljs | 48 ++++++++++++++++ 8 files changed, 185 insertions(+), 37 deletions(-) diff --git a/backend/src/app/http/websocket.clj b/backend/src/app/http/websocket.clj index 18bf25fd8a..50bd64f115 100644 --- a/backend/src/app/http/websocket.clj +++ b/backend/src/app/http/websocket.clj @@ -8,6 +8,7 @@ "A penpot notification service for file cooperative edition." (:require [app.binfile.common :as bfc] + [app.common.data.macros :as dm] [app.common.exceptions :as ex] [app.common.logging :as l] [app.common.pprint :as pp] @@ -18,6 +19,7 @@ [app.http.session :as session] [app.metrics :as mtx] [app.msgbus :as mbus] + [app.nitrate :as nitrate] [app.rpc.commands.files :as files] [app.rpc.commands.teams :as teams] [app.util.websocket :as ws] @@ -42,13 +44,13 @@ (defn repl-get-connections-for-file [file-id] (->> (vals @state) - (filter #(= file-id (-> % deref ::file-subscription :file-id))) + (filter #(= file-id (-> % ::ws/state deref ::file-subscription :file-id))) (map ::ws/id))) (defn repl-get-connections-for-team [team-id] (->> (vals @state) - (filter #(= team-id (-> % deref ::team-subscription :team-id))) + (filter #(= team-id (-> % ::ws/state deref ::team-subscription :team-id))) (map ::ws/id))) (defn repl-close-connection @@ -60,15 +62,17 @@ (defn repl-get-connection-info [id] (when-let [wsp (get @state id)] - {:id id - :created-at (::created-at wsp) - :profile-id (::profile-id wsp) - :session-id (::session-id wsp) - :user-agent (::ws/user-agent wsp) - :ip-addr (::ws/remote-addr wsp) - :last-activity-at (::ws/last-activity-at wsp) - :subscribed-file (-> wsp ::file-subscription :file-id) - :subscribed-team (-> wsp ::team-subscription :team-id)})) + (let [subs (some-> wsp ::ws/state deref)] + {:id id + :created-at (::created-at wsp) + :profile-id (::profile-id wsp) + :session-id (::session-id wsp) + :user-agent (::ws/user-agent wsp) + :ip-addr (::ws/remote-addr wsp) + :last-activity-at (::ws/last-activity-at wsp) + :subscribed-file (-> subs ::file-subscription :file-id) + :subscribed-team (-> subs ::team-subscription :team-id) + :subscribed-org (-> subs ::team-subscription :organization-id)}))) (defn repl-print-connection-info [id] @@ -133,18 +137,39 @@ (mbus/purge! msgbus [channel]) (mbus/pub! msgbus :topic topic :message msg)))) +(defn- get-team-organization-id + "Returns the id of the organization that owns `team-id`, or nil when + the team has no organization or nitrate cannot be reached." + [cfg team-id] + (try + (-> (nitrate/call cfg :get-team-organization {:team-id team-id}) + (dm/get-in [:organization :id])) + (catch Throwable cause + (l/warn :hint "unable to resolve team organization" + :team-id team-id + :cause cause) + nil))) + (defmethod handle-message :subscribe-team [cfg {:keys [::ws/id ::ws/state ::ws/output-ch ::session-id ::profile-id]} {:keys [team-id] :as params}] (l/trace :fn "handle-message" :event "subscribe-team" :team-id team-id :conn-id id) (teams/check-read-permissions! cfg profile-id team-id) - (let [prev-subs (get @state ::team-subscription) - channel (sp/chan :buf (sp/dropping-buffer 64) - :xf (remove #(= (:session-id %) session-id)))] + (let [prev-subs (get @state ::team-subscription) + organization-id (get-team-organization-id cfg team-id) + ;; Resolved server-side so a client only hears its readable team's org + topics (cond-> [team-id] + (some? organization-id) + (conj organization-id)) + channel (sp/chan :buf (sp/dropping-buffer 64) + :xf (remove #(= (:session-id %) session-id)))] (sp/pipe channel output-ch false) - (mbus/sub! (::mbus/msgbus cfg) :topic team-id :chan channel) + (mbus/sub! (::mbus/msgbus cfg) :topics topics :chan channel) - (let [subs {:team-id team-id :channel channel :topic team-id}] + (let [subs {:team-id team-id + :organization-id organization-id + :channel channel + :topic team-id}] (swap! state assoc ::team-subscription subs)) ;; Close previous subscription if exists diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index de72b9a0ef..d9ce4bf7c2 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -379,11 +379,12 @@ ::setup/props (ig/ref ::setup/props)} ::http.ws/routes - {::db/pool (ig/ref ::db/pool) - ::mtx/metrics (ig/ref ::mtx/metrics) - ::mbus/msgbus (ig/ref ::mbus/msgbus) - ::setup/props (ig/ref ::setup/props) - ::session/manager (ig/ref ::session/manager)} + {::db/pool (ig/ref ::db/pool) + ::mtx/metrics (ig/ref ::mtx/metrics) + ::mbus/msgbus (ig/ref ::mbus/msgbus) + ::setup/props (ig/ref ::setup/props) + ::session/manager (ig/ref ::session/manager) + :app.nitrate/client (ig/ref :app.nitrate/client)} :app.http.assets/routes {::http.assets/path (cf/get :assets-path) diff --git a/backend/src/app/rpc/notifications.clj b/backend/src/app/rpc/notifications.clj index ad68556493..00d6b8548e 100644 --- a/backend/src/app/rpc/notifications.clj +++ b/backend/src/app/rpc/notifications.clj @@ -6,16 +6,14 @@ (ns app.rpc.notifications (:require - [app.common.uuid :as uuid] [app.msgbus :as mbus])) (defn notify-team-change [cfg team notification] - (let [msgbus (::mbus/msgbus cfg)] + (let [msgbus (::mbus/msgbus cfg) + team-id (:id team)] (mbus/pub! msgbus - ;;TODO There is a bug on dashboard with teams notifications. - ;;For now we send it to uuid/zero instead of team-id - :topic uuid/zero + :topic team-id :message {:type :team-organization-change :team team :notification notification}))) @@ -37,7 +35,7 @@ [cfg organization-id organization-name teams deleted-teams] (let [msgbus (::mbus/msgbus cfg)] (mbus/pub! msgbus - :topic uuid/zero + :topic organization-id :message {:type :organization-deleted :organization-id organization-id :organization-name organization-name @@ -48,6 +46,6 @@ [cfg organization-id] (let [msgbus (::mbus/msgbus cfg)] (mbus/pub! msgbus - :topic uuid/zero + :topic organization-id :message {:type :organization-change-sso :organization-id organization-id}))) diff --git a/backend/test/backend_tests/http_websocket_test.clj b/backend/test/backend_tests/http_websocket_test.clj index 22683e93c2..6ffc10e744 100644 --- a/backend/test/backend_tests/http_websocket_test.clj +++ b/backend/test/backend_tests/http_websocket_test.clj @@ -10,6 +10,7 @@ [app.db :as db] [app.http.websocket :as ws] [app.msgbus :as mbus] + [app.nitrate :as nitrate] [app.rpc :as-alias rpc] [app.rpc.commands.files :as files] [app.rpc.commands.teams :as teams] @@ -68,6 +69,50 @@ (t/testing "permission check passes for authorized user" (t/is (nil? (teams/check-read-permissions! cfg (:id profile1) (:id team))))))) +(defn- subscribed-topics + "Runs :subscribe-team for `profile-id` on `team-id` with `nitrate-call` + standing in for nitrate; returns the topics and the stored subscription." + [profile-id team-id nitrate-call] + (let [state (atom {}) + output-ch (sp/chan :buf (sp/dropping-buffer 64)) + wsp (make-wsp profile-id state output-ch) + calls (atom [])] + (with-redefs [nitrate/call nitrate-call + mbus/sub! (fn [_ & {:keys [topics]}] + (swap! calls conj topics))] + ((get-method ws/handle-message :subscribe-team) + th/*system* wsp {:team-id team-id})) + (some-> @state ::ws/team-subscription :channel sp/close!) + {:topics @calls + :subscription (::ws/team-subscription @state)})) + +(t/deftest subscribe-team-subscribes-to-team-organization + (let [profile (th/create-profile* 1 {:is-active true}) + team-id (:id (th/create-team* 1 {:profile-id (:id profile)})) + org-id (uuid/next)] + + (t/testing "adds the organization topic when the team has one" + (let [{:keys [topics subscription]} + (subscribed-topics (:id profile) team-id + (fn [_ method params] + (when (= :get-team-organization method) + {:id (:team-id params) + :organization {:id org-id}})))] + (t/is (= [[team-id org-id]] topics)) + (t/is (= org-id (:organization-id subscription))))) + + (t/testing "subscribes only to the team when it has no organization" + (let [{:keys [topics subscription]} + (subscribed-topics (:id profile) team-id (constantly nil))] + (t/is (= [[team-id]] topics)) + (t/is (nil? (:organization-id subscription))))) + + (t/testing "subscribes to the team when nitrate fails" + (let [{:keys [topics]} + (subscribed-topics (:id profile) team-id + (fn [& _] (throw (ex-info "nitrate down" {}))))] + (t/is (= [[team-id]] topics)))))) + (t/deftest pointer-update-validates-file-id (let [profile (th/create-profile* 1 {:is-active true}) file (th/create-file* 1 {:profile-id (:id profile) diff --git a/backend/test/backend_tests/rpc_management_nitrate_test.clj b/backend/test/backend_tests/rpc_management_nitrate_test.clj index 3aa81b7a2b..a754e45b1c 100644 --- a/backend/test/backend_tests/rpc_management_nitrate_test.clj +++ b/backend/test/backend_tests/rpc_management_nitrate_test.clj @@ -243,7 +243,7 @@ :organization organization}))] (t/is (th/success? out)) (t/is (= 1 (count @calls))) - (t/is (= uuid/zero (-> @calls first :topic))) + (t/is (= team-id (-> @calls first :topic))) (let [msg (-> @calls first :message)] (t/is (= :team-organization-change (:type msg))) (t/is (= nil (:notification msg))) @@ -267,7 +267,7 @@ :organization {:name organization-name}}))] (t/is (th/success? out)) (t/is (= 1 (count @calls))) - (t/is (= uuid/zero (-> @calls first :topic))) + (t/is (= team-id (-> @calls first :topic))) (let [msg (-> @calls first :message)] (t/is (= :team-organization-change (:type msg))) (t/is (= "dashboard.team-no-longer-belong-organization" (:notification msg))) @@ -475,7 +475,7 @@ ;; --- Verify: exactly one organization-deleted event is published on the message bus --- (t/is (:called? @mbus-mock)) (let [msg (apply hash-map (rest (:call-args @mbus-mock)))] - (t/is (= uuid/zero (:topic msg))) + (t/is (= organization-id (:topic msg))) (t/is (= :organization-deleted (:type (:message msg)))) (t/is (= organization-id (:organization-id (:message msg)))) (t/is (= organization-name (:organization-name (:message msg)))) @@ -484,6 +484,24 @@ (t/is (= #{(:id empty-team)} (set (:deleted-teams (:message msg)))))))))) +(t/deftest notify-organization-change-sso-publishes-event + (let [organization-id (uuid/random) + calls (atom []) + out (with-redefs [mbus/pub! (fn [_cfg & {:keys [topic message]}] + (swap! calls conj {:topic topic + :message message}))] + (th/management-command! {::th/type :notify-organization-sso-change + ::rpc/profile-id (uuid/random) + :organization-id organization-id + :updated-props true + :announce-activation false}))] + (t/is (th/success? out)) + (t/is (= 1 (count @calls))) + (t/is (= organization-id (-> @calls first :topic))) + (let [msg (-> @calls first :message)] + (t/is (= :organization-change-sso (:type msg))) + (t/is (= organization-id (:organization-id msg)))))) + (t/deftest notify-user-organizations-deletion-renames-or-deletes-teams-and-publishes-per-organization-events ;; --- Deferred owned-organizations: nil during setup, filled before RPC --- (let [owned-organizations-ref (atom nil)] @@ -591,7 +609,7 @@ ;; --- Verify: one organization-deleted event per organization, all on correct topic --- (t/is (= 2 (count msgs))) - (t/is (every? #(= uuid/zero (:topic %)) + (t/is (every? #(contains? #{organization-1-id organization-2-id} (:topic %)) (->> (:call-args-list @mbus-mock) (map #(apply hash-map (rest %)))))) (t/is (= #{:organization-deleted} (set (map :type msgs)))) diff --git a/frontend/src/app/main/data/dashboard.cljs b/frontend/src/app/main/data/dashboard.cljs index 37d9f9fc44..b0e47eafea 100644 --- a/frontend/src/app/main/data/dashboard.cljs +++ b/frontend/src/app/main/data/dashboard.cljs @@ -50,18 +50,28 @@ (ptk/reify ::initialize ptk/WatchEvent (watch [_ state stream] - (let [stopper (rx/filter (ptk/type? ::finalize) stream) - profile-id (:profile-id state)] + (let [stopper (rx/filter (ptk/type? ::finalize) stream) + profile-id (:profile-id state) + organization-id (dm/get-in state [:teams team-id :organization :id]) + initmsg {:type :subscribe-team :team-id team-id}] (->> (rx/merge (rx/of (fetch-projects team-id) - (df/fetch-fonts team-id)) + (df/fetch-fonts team-id) + (dws/send initmsg)) + ;; On reconnect, send again the subscription message + (->> stream + (rx/filter (ptk/type? ::dws/opened)) + (rx/map #(dws/send initmsg))) (->> stream (rx/filter (ptk/type? ::dws/message)) (rx/map deref) (rx/filter (fn [{:keys [topic] :as msg}] (or (= topic uuid/zero) - (= topic profile-id)))) + (= topic profile-id) + (= topic team-id) + (when (some? organization-id) + (= topic organization-id))))) (rx/map process-message))) (rx/take-until stopper)))))) diff --git a/frontend/src/app/main/data/workspace/notifications.cljs b/frontend/src/app/main/data/workspace/notifications.cljs index 55f9735814..b3eead45ef 100644 --- a/frontend/src/app/main/data/workspace/notifications.cljs +++ b/frontend/src/app/main/data/workspace/notifications.cljs @@ -52,6 +52,7 @@ (watch [_ state stream] (let [stopper (rx/filter (ptk/type? ::finalize) stream) profile-id (:profile-id state) + organization-id (dm/get-in state [:teams team-id :organization :id]) initmsg [{:type :subscribe-file :file-id file-id @@ -75,7 +76,9 @@ (or (= topic uuid/zero) (= topic profile-id) (= topic team-id) - (= topic file-id)))) + (= topic file-id) + (when (some? organization-id) + (= topic organization-id))))) (rx/map process-message)) ;; On reconnect, send again the subscription messages diff --git a/frontend/test/frontend_tests/data/dashboard_test.cljs b/frontend/test/frontend_tests/data/dashboard_test.cljs index 001ea2f924..12fefdb476 100644 --- a/frontend/test/frontend_tests/data/dashboard_test.cljs +++ b/frontend/test/frontend_tests/data/dashboard_test.cljs @@ -9,10 +9,13 @@ [app.common.uuid :as uuid] [app.config :as cf] [app.main.data.common :as dcm] + [app.main.data.dashboard :as dd] + [app.main.data.websocket :as dws] [app.main.repo :as rp] [app.main.router :as rt] [beicon.v2.core :as rx] [cljs.test :as t :include-macros true] + [frontend-tests.helpers.async :as async] [frontend-tests.helpers.mock :as mock] [potok.v2.core :as ptk])) @@ -82,3 +85,48 @@ (fn [] (done'))))) done)))) + +(defn- sent-messages + "Runs the watch of `event` against `stream` with `dws/send` stubbed + and resolves to the messages it sends over the websocket." + [event state stream] + (let [sent (atom [])] + (-> (mock/with-mocks* + {dws/send (mock/stub (fn [msg] (swap! sent conj msg) msg))} + (await (async/observe (ptk/watch event state stream)))) + (.then (fn [_] @sent))))) + +(t/deftest ^:async dashboard-initialize-subscribes-to-team + (let [team-id (uuid/next) + state {:profile-id (uuid/next) + :teams {team-id {:id team-id :organization {:id (uuid/next)}}}} + sent (await (sent-messages (dd/initialize team-id) state (rx/empty)))] + (t/is (= [{:type :subscribe-team :team-id team-id}] sent)))) + +(t/deftest ^:async dashboard-initialize-resubscribes-on-reconnect + (let [team-id (uuid/next) + state {:profile-id (uuid/next) + :teams {team-id {:id team-id}}} + stream (rx/of (ptk/data-event ::dws/opened {}) + (ptk/data-event ::dws/opened {})) + sent (await (sent-messages (dd/initialize team-id) state stream))] + (t/is (= 3 (count sent))) + (t/is (every? #(= {:type :subscribe-team :team-id team-id} %) sent)))) + +(t/deftest ^:async dashboard-initialize-accepts-team-organization-messages + (let [team-id (uuid/next) + org-id (uuid/next) + state {:profile-id (uuid/next) + :teams {team-id {:id team-id :organization {:id org-id}}}} + message (fn [topic] + (ptk/data-event ::dws/message + {:type :organization-change-sso + :topic topic + :organization-id org-id})) + stream (rx/of (message org-id) (message (uuid/next))) + processed (atom [])] + (await (mock/with-mocks* + {dws/send (mock/stub identity)} + (await (async/observe (ptk/watch (dd/initialize team-id) state stream) + :on-next #(swap! processed conj (ptk/type %)))))) + (t/is (= 1 (count (filter #{::dcm/handle-organization-change-sso} @processed)))))) From b5fbc4fd8cab8654e3d6102e7f57a4b80660b0a8 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 1 Oct 2026 14:31:30 +0200 Subject: [PATCH 2/4] :sparkles: Add size limits to profile props and plugin registry (#11596) * :sparkles: Add size limits to profile props and plugin registry Bound the total serialized size of profile settings to 2 MiB (:profile-props-max-size), checked on the merged result before persisting, with a controlled :props-too-large error. Profiles that already exceed the limit can still shrink but cannot grow. Cap plugin registry entries in the shared schema (code 1 MiB, 50 plugins max, bounded name/host/description/icon) and restore rate limiting on the plugin RPCs (profile-mutations bucket, one write at a time per profile). The plugin manager now asks for confirmation before removal and ignores repeated clicks while a persist request is in flight. Closes #11592 AI-assisted-by: muse-spark-1.3-contributor * :bug: Enforce plugin count cap, byte sizes and removal guard Enforce the declared 50-plugin cap in add-profile-plugin with a specific :too-many-plugins error (updates of existing entries still pass); the cap lives in a shared max-plugins constant. Measure profile props size in UTF-8 bytes instead of chars so multibyte content cannot slip past the limit. Cover install/remove persist logic with mocked-RPC frontend tests (release semantics, in-flight dedupe, validation vs rollback split) and add the missing boundary tests in common. Expose the in-flight persist set from the plugin registry and disable the remove button of entries being saved. Closes #11592 AI-assisted-by: muse-spark-1.3-contributor * :bug: Fix rollback loops and restore paths in plugin registry Restore the previous plugin version instead of dropping the entry when a validation error rejects an update of an installed plugin. Make compensating writes one-shot with terminal callbacks so a persistent failure cannot ping-pong between install and remove. Restores keep the original list position; the unused public plugin-persisting? predicate is removed. Pin count-before-size precedence with a dedicated test and fix translation source refs to their canonical lines. Closes #11592 AI-assisted-by: muse-spark-1.3-contributor * :bug: Guard notifications write and fix restore ordering Route update-profile-notifications through check-props-size! so oversized profiles cannot grow through that path; document the exempt system writers. Remove the duplicated stale entries in en.po, keeping the canonical translation refs. Restore rejected plugin updates at their original list position instead of leaving the optimistic move in place. Closes #11592 AI-assisted-by: muse-spark-1.3-contributor * :bug: Skip no-op plugin removal and clarify size comments Return early from remove-profile-plugin when the id is absent: no wasted write, no size check, and no manufactured :plugins key that could spuriously fail on oversized profiles. Clarify that per-field string caps count chars while the byte budget is enforced by profile-props-max-size. Closes #11592 AI-assisted-by: muse-spark-1.3-contributor * :paperclip: Fix formatting in rlimit.edn for profile operations Signed-off-by: Andrey Antukh * :paperclip: Fix formatting of import-binfile/global entry Signed-off-by: Andrey Antukh * :recycle: Simplify props size check and tighten plugin entry caps Measure props with transit bytes directly instead of the PGobject string roundtrip. Rename check-props-size! to check-props-size: single hard limit on the merged props, no growth comparison, and return props so writers thread the check into the update. Move the 2 MiB default into default-props-max-size on the profile namespace, still overridable with the optional :profile-props-max-size config entry. Tighten registry-entry :code and :icon to 500 chars: they hold manifest paths, not content. Closes #11592 AI-assisted-by: muse-spark-1.3-contributor * :bug: Fix compatibility problems --------- Signed-off-by: Andrey Antukh Co-authored-by: alonso.torres --- backend/resources/climit.edn | 6 + backend/resources/rlimit.edn | 2 + backend/src/app/config.clj | 3 + backend/src/app/rpc/commands/plugins.clj | 55 ++- backend/src/app/rpc/commands/profile.clj | 86 ++++- .../test/backend_tests/rpc_plugins_test.clj | 221 ++++++++++++ .../test/backend_tests/rpc_profile_test.clj | 105 ++++++ common/src/app/common/types/plugins.cljc | 27 +- common/test/common_tests/runner.cljc | 2 + .../test/common_tests/types/plugins_test.cljc | 64 ++++ .../scripts/check-translations/words.es.txt | 1 + frontend/src/app/main/errors.cljs | 9 + frontend/src/app/main/ui/confirm.scss | 1 + .../src/app/main/ui/workspace/main_menu.cljs | 2 +- .../src/app/main/ui/workspace/main_menu.scss | 8 + .../src/app/main/ui/workspace/plugins.cljs | 48 ++- .../src/app/main/ui/workspace/plugins.scss | 6 + frontend/src/app/plugins/register.cljs | 156 +++++++-- .../frontend_tests/plugins/register_test.cljs | 322 ++++++++++++++++++ frontend/test/frontend_tests/runner.cljs | 2 + frontend/translations/en.po | 110 +++--- frontend/translations/es.po | 110 +++--- 22 files changed, 1181 insertions(+), 165 deletions(-) create mode 100644 common/test/common_tests/types/plugins_test.cljc create mode 100644 frontend/test/frontend_tests/plugins/register_test.cljs diff --git a/backend/resources/climit.edn b/backend/resources/climit.edn index ded9b5c9b8..c109b2afdb 100644 --- a/backend/resources/climit.edn +++ b/backend/resources/climit.edn @@ -51,4 +51,10 @@ {:permits 4} :import-binfile/by-profile + {:permits 1 :queue 2} + + :profile-plugin-ops/global + {:permits 4} + + :profile-plugin-ops/by-profile {:permits 1 :queue 2}} diff --git a/backend/resources/rlimit.edn b/backend/resources/rlimit.edn index 86247690f1..246992fa8e 100644 --- a/backend/resources/rlimit.edn +++ b/backend/resources/rlimit.edn @@ -221,6 +221,8 @@ ;; ═══════════════════════════════════════════════ #{:main/update-profile :main/update-profile-props + :main/add-profile-plugin + :main/remove-profile-plugin :main/update-profile-photo :main/update-profile-password :main/update-profile-notifications diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index a9bc88e7eb..c4f097c373 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -165,6 +165,9 @@ [:binfile-import-max-text-total-size {:optional true} ::sm/int] [:binfile-import-max-zip-entries {:optional true} ::sm/int] + ;; Max serialized size of profile props in bytes (default 2 MiB) + [:profile-props-max-size {:optional true} ::sm/int] + [:login-lockout-max-attempts {:optional true} ::sm/int] [:login-lockout-window {:optional true} ::ct/duration] diff --git a/backend/src/app/rpc/commands/plugins.clj b/backend/src/app/rpc/commands/plugins.clj index 4d6b8eb8c6..8f01b3b521 100644 --- a/backend/src/app/rpc/commands/plugins.clj +++ b/backend/src/app/rpc/commands/plugins.clj @@ -11,6 +11,7 @@ [app.common.types.plugins :as ctp] [app.db :as db] [app.rpc :as-alias rpc] + [app.rpc.climit :as-alias climit] [app.rpc.commands.profile :as profile] [app.rpc.doc :as-alias doc] [app.util.services :as sv])) @@ -33,6 +34,8 @@ (sv/defmethod ::add-profile-plugin {::doc/added "2.18" + ::climit/id [[:profile-plugin-ops/by-profile ::rpc/profile-id] + [:profile-plugin-ops/global]] ::sm/params schema:add-profile-plugin ::sm/result ctp/schema:registry-entry ::db/transaction true} @@ -42,14 +45,22 @@ (let [profile (profile/get-profile conn profile-id ::db/for-update true) plugins (get-in profile [:props :plugins] {:ids [] :data {}}) plugin-id (:plugin-id plugin) - plugins (-> plugins - (update :ids #(vec (distinct (conj % plugin-id)))) - (assoc-in [:data plugin-id] plugin))] - (db/update! conn :profile - {:props (db/tjson (assoc (:props profile) :plugins plugins))} - {:id profile-id} - {::db/return-keys false}) - plugin)) + exists? (contains? (set (:ids plugins)) plugin-id)] + (when (and (not exists?) (>= (count (:ids plugins)) ctp/max-plugins)) + (ex/raise :type :validation + :code :too-many-plugins + :hint "plugin registry exceeds maximum size")) + (let [plugins (-> plugins + (update :ids #(vec (distinct (conj % plugin-id)))) + (assoc-in [:data plugin-id] plugin)) + props (-> (:props profile) + (assoc :plugins plugins) + (profile/check-props-size))] + (db/update! conn :profile + {:props (db/tjson props)} + {:id profile-id} + {::db/return-keys false}) + plugin))) (def ^:private schema:remove-profile-plugin @@ -58,18 +69,28 @@ (sv/defmethod ::remove-profile-plugin {::doc/added "2.18" + ::climit/id [[:profile-plugin-ops/by-profile ::rpc/profile-id] + [:profile-plugin-ops/global]] ::sm/params schema:remove-profile-plugin ::sm/result :nil ::db/transaction true} [{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id plugin-id]}] (let [profile (profile/get-profile conn profile-id ::db/for-update true) plugins (get-in profile [:props :plugins] {:ids [] :data {}}) - plugin-id-str (str plugin-id) - plugins (-> plugins - (update :ids #(vec (remove (partial = plugin-id-str) %))) - (update :data dissoc plugin-id-str))] - (db/update! conn :profile - {:props (db/tjson (assoc (:props profile) :plugins plugins))} - {:id profile-id} - {::db/return-keys false}) - nil)) + plugin-id-str (str plugin-id)] + (if-not (or (some #(= % plugin-id-str) (:ids plugins)) + (contains? (:data plugins) plugin-id-str)) + ;; Nothing to remove: no write + nil + (let [plugins (-> plugins + (update :ids #(vec (remove (partial = plugin-id-str) %))) + (update :data dissoc plugin-id-str)) + ;; Raises when the remaining props still exceed the size limit + props (-> (:props profile) + (assoc :plugins plugins) + (profile/check-props-size))] + (db/update! conn :profile + {:props (db/tjson props)} + {:id profile-id} + {::db/return-keys false}) + nil)))) diff --git a/backend/src/app/rpc/commands/profile.clj b/backend/src/app/rpc/commands/profile.clj index 68e93231cd..686884aeed 100644 --- a/backend/src/app/rpc/commands/profile.clj +++ b/backend/src/app/rpc/commands/profile.clj @@ -12,6 +12,7 @@ [app.common.exceptions :as ex] [app.common.schema :as sm] [app.common.time :as ct] + [app.common.transit :as t] [app.common.types.plugins :as ctp] [app.common.uuid :as uuid] [app.config :as cf] @@ -36,6 +37,7 @@ [cuerdas.core :as str])) (declare check-profile-existence!) +(declare check-props-size) (declare decode-row) (declare filter-props) (declare get-profile) @@ -283,7 +285,8 @@ props (-> (get profile :props) - (assoc :notifications notifications))] + (assoc :notifications notifications) + (check-props-size))] (db/update! conn :profile {:props (db/tjson props)} @@ -474,18 +477,46 @@ [:map {:title "update-profile-props"} [:props schema:props-writeable]]) +(def default-props-max-size + "Default total serialized size limit (in bytes) for profile props. + Overridable with the :profile-props-max-size config entry." + (* 1024 1024 2)) ;; 2 MiB + +(defn- props-size + "Returns the serialized size in UTF-8 bytes of the props map." + [props] + (if props + (alength ^bytes (t/encode props {:type :json-verbose})) + 0)) + +(defn check-props-size + "Raises :props-too-large when props exceed the total size limit. + Returns props unchanged so it can be threaded into the write. + + Used by the user-facing props writers; system writers (OIDC login, + management subscription) write fixed-key props and skip it." + [props] + (let [limit (cf/get :profile-props-max-size default-props-max-size) + size (props-size props)] + (when (> size limit) + (ex/raise :type :validation + :code :props-too-large + :hint "profile props exceed maximum size")) + props)) + (defn update-profile-props [{:keys [::db/conn] :as cfg} profile-id props] (let [profile (get-profile conn profile-id ::db/for-update true) - props (reduce-kv (fn [props k v] - ;; We don't accept namespaced keys - (if (simple-ident? k) - (if (nil? v) - (dissoc props k) - (assoc props k v)) - props)) - (:props profile) - (apply dissoc props system-managed-props))] + props (->> (apply dissoc props system-managed-props) + (reduce-kv (fn [props k v] + ;; We don't accept namespaced keys + (if (simple-ident? k) + (if (nil? v) + (dissoc props k) + (assoc props k v)) + props)) + (:props profile)) + (check-props-size))] (db/update! conn :profile {:props (db/tjson props)} @@ -658,8 +689,41 @@ [props] (into {} (filter (fn [[k _]] (simple-ident? k))) props)) +(defn- truncate-string + "Cuts s to at most n chars without splitting a surrogate pair." + [^String s n] + (if (> (count s) n) + (let [n (if (Character/isHighSurrogate (.charAt s (dec n))) (dec n) n)] + (subs s 0 n)) + s)) + +(defn- clamp-plugin-entry + [entry] + (reduce-kv (fn [entry k n] + (let [v (get entry k)] + (if (and (string? v) (> (count v) n)) + (assoc entry k (truncate-string v n)) + entry))) + entry + ctp/registry-entry-max-lengths)) + +(defn clamp-plugins-registry + "Fits a plugin registry into the registry schema caps: keeps the + first `ctp/max-plugins` plugins with data and truncates the bounded + strings. Applied on read, so a stored registry over the caps cannot + keep the profile over the props size limit." + [{:keys [ids data] :as plugins}] + (let [ids (->> ids + (filter #(contains? data %)) + (distinct) + (take ctp/max-plugins) + (vec)) + data (update-vals (select-keys data ids) clamp-plugin-entry)] + (assoc plugins :ids ids :data data))) + (defn decode-row [{:keys [props] :as row}] (cond-> row (db/pgobject? props "jsonb") - (assoc :props (db/decode-transit-pgobject props)))) + (assoc :props (-> (db/decode-transit-pgobject props) + (d/update-when :plugins #(cond-> % (map? %) (clamp-plugins-registry))))))) diff --git a/backend/test/backend_tests/rpc_plugins_test.clj b/backend/test/backend_tests/rpc_plugins_test.clj index cb895d4395..8c57931844 100644 --- a/backend/test/backend_tests/rpc_plugins_test.clj +++ b/backend/test/backend_tests/rpc_plugins_test.clj @@ -6,7 +6,11 @@ (ns backend-tests.rpc-plugins-test (:require + [app.common.schema :as sm] + [app.common.types.plugins :as ctp] [app.common.uuid :as uuid] + [app.config :as cf] + [app.db :as db] [app.rpc :as-alias rpc] [app.rpc.commands.profile :as profile] [backend-tests.helpers :as th] @@ -145,6 +149,140 @@ (t/is (= "Test Plugin" (get-in plugins [:data plugin-id-1 :name]))) (t/is (= "Second Plugin" (get-in plugins [:data plugin-id-2 :name])))))) +(t/deftest add-profile-plugin-rejects-oversized-code + ;; The merged props must not exceed :profile-props-max-size + (let [profile (th/create-profile* 1) + plugin (assoc valid-plugin :code (apply str (repeat 200 "x"))) + data {::th/type :add-profile-plugin + ::rpc/profile-id (:id profile) + :plugin plugin}] + (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100})] + (let [out (th/command! data)] + (t/is (th/ex-info? (:error out))) + (t/is (th/ex-of-type? (:error out) :validation)) + (t/is (th/ex-of-code? (:error out) :props-too-large)))))) + +(t/deftest add-profile-plugin-rejects-oversized-code-path + ;; :code holds a manifest path, not content: overlong values are + ;; rejected by the entry schema before the props size check runs + (let [profile (th/create-profile* 1) + plugin (assoc valid-plugin :code (apply str (repeat 501 "x"))) + data {::th/type :add-profile-plugin + ::rpc/profile-id (:id profile) + :plugin plugin} + out (th/command! data)] + (t/is (th/ex-info? (:error out))) + (t/is (th/ex-of-type? (:error out) :validation)) + (t/is (th/ex-of-code? (:error out) :params-validation)))) + +(t/deftest remove-profile-plugin-allowed-on-oversized-profile + ;; Removal shrinks props, so it passes even under a tight limit + (let [profile (th/create-profile* 1) + plugin (assoc valid-plugin :code (apply str (repeat 200 "x")))] + ;; Seed an oversized registry while the limit is high + (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100000})] + (let [out (th/command! {::th/type :add-profile-plugin + ::rpc/profile-id (:id profile) + :plugin plugin})] + (t/is (nil? (:error out))))) + ;; Removal under a tighter limit still passes: the seeded registry + ;; is oversized against it, but the remaining props fit + (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 300})] + (let [out (th/command! {::th/type :remove-profile-plugin + ::rpc/profile-id (:id profile) + :plugin-id (uuid/uuid plugin-id-1)})] + (t/is (nil? (:error out))))))) + +(t/deftest add-profile-plugin-rejects-51st-plugin + ;; The registry holds at most 50 plugins; the 51st (new id) must fail + (let [profile (th/create-profile* 1)] + ;; Seed 50 plugins + (doseq [i (range 50)] + (let [plugin (assoc valid-plugin + :plugin-id (str (uuid/next)) + :name (str "Plugin " i)) + out (th/command! {::th/type :add-profile-plugin + ::rpc/profile-id (:id profile) + :plugin plugin})] + (t/is (nil? (:error out)) (str "seed plugin " i " should install")))) + ;; The 51st must fail with a specific error + (let [extra (assoc valid-plugin + :plugin-id (str (uuid/next)) + :name "One Too Many") + out (th/command! {::th/type :add-profile-plugin + ::rpc/profile-id (:id profile) + :plugin extra})] + (t/is (th/ex-info? (:error out))) + (t/is (th/ex-of-type? (:error out) :validation)) + (t/is (th/ex-of-code? (:error out) :too-many-plugins))) + ;; And nothing extra was persisted + (let [saved (th/db-get :profile {:id (:id profile)}) + props (profile/decode-row saved)] + (t/is (= 50 (count (get-in props [:props :plugins :ids]))))))) + +(t/deftest add-profile-plugin-updates-existing-at-limit + ;; Re-adding an existing id at the limit is an update, not a new entry + (let [profile (th/create-profile* 1) + ids (mapv (fn [_] (str (uuid/next))) (range 50))] + (doseq [[i pid] (map-indexed vector ids)] + (th/command! {::th/type :add-profile-plugin + ::rpc/profile-id (:id profile) + :plugin (assoc valid-plugin :plugin-id pid :name (str "Plugin " i))})) + (let [out (th/command! {::th/type :add-profile-plugin + ::rpc/profile-id (:id profile) + :plugin (assoc valid-plugin :plugin-id (first ids) :name "Renamed")})] + (t/is (nil? (:error out))) + (let [saved (th/db-get :profile {:id (:id profile)}) + props (profile/decode-row saved)] + (t/is (= 50 (count (get-in props [:props :plugins :ids])))) + (t/is (= "Renamed" (get-in props [:props :plugins :data (first ids) :name]))))))) + +(t/deftest add-profile-plugin-full-registry-reports-too-many-before-size + ;; A full registry plus oversized content reports the count guard, + ;; which runs before the size check + (let [profile (th/create-profile* 1)] + ;; Seed 50 plugins under a generous limit + (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 1000000})] + (doseq [i (range 50)] + (let [plugin (assoc valid-plugin + :plugin-id (str (uuid/next)) + :name (str "Plugin " i)) + out (th/command! {::th/type :add-profile-plugin + ::rpc/profile-id (:id profile) + :plugin plugin})] + (t/is (nil? (:error out)) (str "seed plugin " i " should install"))))) + ;; Tight limit + 51st small plugin: count wins over size + (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100})] + (let [extra (assoc valid-plugin + :plugin-id (str (uuid/next)) + :name "One Too Many") + out (th/command! {::th/type :add-profile-plugin + ::rpc/profile-id (:id profile) + :plugin extra})] + (t/is (th/ex-info? (:error out))) + (t/is (th/ex-of-type? (:error out) :validation)) + (t/is (th/ex-of-code? (:error out) :too-many-plugins)))) + ;; And nothing extra was persisted + (let [saved (th/db-get :profile {:id (:id profile)}) + props (profile/decode-row saved)] + (t/is (= 50 (count (get-in props [:props :plugins :ids]))))))) + +(t/deftest remove-profile-plugin-noop-on-oversized-profile-without-plugins + ;; Removing an absent id changes nothing: no write, no size failure, + ;; and no :plugins key is manufactured + (let [profile (th/create-profile* 1) + big {:onboarding-questions {:big-blob (apply str (repeat 200 "x"))}}] + (th/db-update! :profile {:props (db/tjson big)} {:id (:id profile)}) + (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100})] + (let [out (th/command! {::th/type :remove-profile-plugin + ::rpc/profile-id (:id profile) + :plugin-id (uuid/next)})] + (t/is (nil? (:error out))))) + (let [saved (th/db-get :profile {:id (:id profile)}) + props (profile/decode-row saved)] + (t/is (nil? (get-in props [:props :plugins]))) + (t/is (= big (get props :props)))))) + (t/deftest update-profile-props-rejects-plugins (let [profile (th/create-profile* 1) data {::th/type :update-profile-props @@ -160,3 +298,86 @@ props (profile/decode-row saved)] (t/is (nil? (get-in props [:props :plugins])) ":plugins must not be writable via update-profile-props")))) + +;; --- Stored registries over the caps + +(defn- legacy-entry + [plugin-id & {:as attrs}] + (merge valid-plugin {:plugin-id plugin-id :code "plugin.js"} attrs)) + +(defn- legacy-registry + [entries] + {:ids (mapv :plugin-id entries) + :data (into {} (map (juxt :plugin-id identity)) entries)}) + +(defn- stored-props + "Props as stored in the database, without the read-time clamping." + [profile-id] + (db/decode-transit-pgobject (:props (th/db-get :profile {:id profile-id})))) + +(t/deftest get-profile-clamps-legacy-registry + (let [profile (th/create-profile* 1) + big (apply str (repeat 10000 "x")) + ;; "😀" is two chars, so char 500 is a high surrogate + emoji (str (apply str (repeat 499 "n")) "😀") + entries (into [(legacy-entry plugin-id-1 + :name emoji + :description big + :host big + :code big + :icon big)] + (map #(legacy-entry (str "extra-" %))) + (range 60)) + stored (-> (legacy-registry entries) + (update :ids conj "dangling"))] + (th/db-update! :profile {:props (db/tjson {:plugins stored :renderer :wasm})} + {:id (:id profile)}) + (let [out (th/command! {::th/type :get-profile ::rpc/profile-id (:id profile)}) + props (get-in out [:result :props]) + plugins (:plugins props) + entry (get-in plugins [:data plugin-id-1])] + (t/is (nil? (:error out))) + (t/is (= :wasm (:renderer props)) "other props are kept") + (t/is (= (mapv :plugin-id (take ctp/max-plugins entries)) (:ids plugins)) + "keeps the first plugins in registry order, drops ids without data") + (t/is (= (set (:ids plugins)) (set (keys (:data plugins))))) + (t/is (= (apply str (repeat 499 "n")) (:name entry)) "does not split a surrogate pair") + (doseq [k [:description :host :code :icon]] + (t/is (= (get ctp/registry-entry-max-lengths k) (count (get entry k))) + (str k " truncated to its cap"))) + (t/is (sm/validate ctp/schema:plugin-registry plugins))) + (t/is (= stored (:plugins (stored-props (:id profile)))) + "reading does not write"))) + +(t/deftest get-profile-keeps-valid-registry + (let [profile (th/create-profile* 1) + stored (legacy-registry [(legacy-entry plugin-id-1)])] + (th/db-update! :profile {:props (db/tjson {:plugins stored})} {:id (:id profile)}) + (let [out (th/command! {::th/type :get-profile ::rpc/profile-id (:id profile)})] + (t/is (= stored (get-in out [:result :props :plugins])))))) + +(t/deftest oversized-legacy-profile-recovers-on-write + ;; A stored registry over the caps pushes props past the size limit; + ;; the clamped read lets writes through and saves the clamped registry + (let [profile (th/create-profile* 1) + code (apply str (repeat 3000 "c")) + entries [(legacy-entry plugin-id-1 :code code) + (legacy-entry plugin-id-2 :code code)]] + (th/db-update! :profile {:props (db/tjson {:plugins (legacy-registry entries)})} + {:id (:id profile)}) + (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 5000})] + (t/is (thrown? Exception (profile/check-props-size (stored-props (:id profile)))) + "the stored props exceed the limit") + (let [out (th/command! {::th/type :update-profile-props + ::rpc/profile-id (:id profile) + :props {:workspace-visited true}})] + (t/is (nil? (:error out)))) + (let [out (th/command! {::th/type :remove-profile-plugin + ::rpc/profile-id (:id profile) + :plugin-id (uuid/uuid plugin-id-1)})] + (t/is (nil? (:error out))))) + (let [props (stored-props (:id profile))] + (t/is (true? (:workspace-visited props))) + (t/is (= [plugin-id-2] (get-in props [:plugins :ids]))) + (t/is (= 500 (count (get-in props [:plugins :data plugin-id-2 :code]))) + "the write saved the clamped registry")))) diff --git a/backend/test/backend_tests/rpc_profile_test.clj b/backend/test/backend_tests/rpc_profile_test.clj index a2750beb12..1bfb54aeee 100644 --- a/backend/test/backend_tests/rpc_profile_test.clj +++ b/backend/test/backend_tests/rpc_profile_test.clj @@ -1549,6 +1549,111 @@ (t/is (th/ex-of-code? (:error out) :params-validation)))) +(t/deftest update-profile-props-rejects-oversized-props + ;; The merged props must not exceed :profile-props-max-size + (let [profile (th/create-profile* 1)] + (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100})] + (let [data {::th/type :update-profile-props + ::rpc/profile-id (:id profile) + :props {:onboarding-questions {:big-blob (apply str (repeat 200 "x"))}}} + out (th/command! data)] + (t/is (th/ex-info? (:error out))) + (t/is (th/ex-of-type? (:error out) :validation)) + (t/is (th/ex-of-code? (:error out) :props-too-large)))))) + + +(t/deftest update-profile-props-enforces-hard-limit-on-oversized-profile + ;; An already-oversized profile can only write back under the limit: + ;; shrinking below it passes, staying above it fails + (let [profile (th/create-profile* 1) + big {:onboarding-questions {:big-blob (apply str (repeat 200 "x"))}}] + ;; Seed an already-oversized profile directly in DB (bypasses RPC validation) + (th/db-update! :profile {:props (db/tjson big)} {:id (:id profile)}) + (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100})] + ;; Shrinking below the limit passes + (let [data {::th/type :update-profile-props + ::rpc/profile-id (:id profile) + :props {:onboarding-questions {:big-blob "small"}}} + out (th/command! data)] + (t/is (nil? (:error out)))) + ;; Staying above the limit fails + (let [data {::th/type :update-profile-props + ::rpc/profile-id (:id profile) + :props {:onboarding-questions {:big-blob (apply str (repeat 300 "x"))}}} + out (th/command! data)] + (t/is (th/ex-info? (:error out))) + (t/is (th/ex-of-type? (:error out) :validation)) + (t/is (th/ex-of-code? (:error out) :props-too-large)))))) + + +(t/deftest check-props-size-measures-bytes-not-chars + ;; The limit is in UTF-8 bytes: multibyte content that fits in chars + ;; but exceeds the byte limit must be rejected + (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 200})] + ;; 70 ASCII chars (~87 bytes serialized) passes and returns props unchanged + (let [props {:blob (apply str (repeat 70 "x"))}] + (t/is (= props (profile/check-props-size props)))) + ;; 70 CJK chars (~227 bytes serialized, still 70 chars) raises + (try + (profile/check-props-size {:blob (apply str (repeat 70 "日"))}) + (t/is false "should have thrown") + (catch clojure.lang.ExceptionInfo e + (t/is (= :validation (:type (ex-data e)))) + (t/is (= :props-too-large (:code (ex-data e)))))))) + +(t/deftest update-profile-props-rejects-steady-size-on-oversized-profile + ;; Same size (not smaller) on an oversized profile still exceeds + ;; the limit, so it fails + (let [profile (th/create-profile* 1) + big {:onboarding-questions {:big-blob (apply str (repeat 200 "x"))}}] + (th/db-update! :profile {:props (db/tjson big)} {:id (:id profile)}) + (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100})] + (let [data {::th/type :update-profile-props + ::rpc/profile-id (:id profile) + :props {:onboarding-questions {:big-blob (apply str (repeat 200 "y"))}}} + out (th/command! data)] + (t/is (th/ex-info? (:error out))) + (t/is (th/ex-of-type? (:error out) :validation)) + (t/is (th/ex-of-code? (:error out) :props-too-large)))))) + + +(t/deftest update-profile-notifications-rejects-growth-on-oversized-profile + ;; The notifications write path goes through the same size check: + ;; growing an oversized profile fails + (let [profile (th/create-profile* 1) + big {:onboarding-questions {:big-blob (apply str (repeat 200 "x"))}}] + (th/db-update! :profile {:props (db/tjson big)} {:id (:id profile)}) + (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100})] + (let [data {::th/type :update-profile-notifications + ::rpc/profile-id (:id profile) + :dashboard-comments :all + :email-comments :all + :email-invites :all} + out (th/command! data)] + (t/is (th/ex-info? (:error out))) + (t/is (th/ex-of-type? (:error out) :validation)) + (t/is (th/ex-of-code? (:error out) :props-too-large)))))) + +(t/deftest update-profile-notifications-rejects-steady-on-oversized-profile + ;; Same-size notifications write on an oversized profile still exceeds + ;; the limit, so it fails + (let [profile (th/create-profile* 1) + notifications {:dashboard-comments :all + :email-comments :all + :email-invites :all} + big {:onboarding-questions {:big-blob (apply str (repeat 200 "x"))} + :notifications notifications}] + (th/db-update! :profile {:props (db/tjson big)} {:id (:id profile)}) + (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100})] + (let [data (merge {::th/type :update-profile-notifications + ::rpc/profile-id (:id profile)} + notifications) + out (th/command! data)] + (t/is (th/ex-info? (:error out))) + (t/is (th/ex-of-type? (:error out) :validation)) + (t/is (th/ex-of-code? (:error out) :props-too-large)))))) + + (t/deftest prepare-register-profile-password-too-short (let [data {::th/type :prepare-register-profile :email "user@example.com" diff --git a/common/src/app/common/types/plugins.cljc b/common/src/app/common/types/plugins.cljc index bb74bfcc09..035be0efb3 100644 --- a/common/src/app/common/types/plugins.cljc +++ b/common/src/app/common/types/plugins.cljc @@ -41,21 +41,34 @@ "Schema for plugin permissions - a set of valid permission strings." [:set {:gen/max 11} (into [:enum] (sort valid-permissions))]) +(def max-plugins + "Maximum number of plugins a profile can hold." + 50) + +(def registry-entry-max-lengths + "Maximum length (in chars) of the bounded registry entry strings. + `:code` and `:icon` hold manifest paths, not content." + {:name 500 + :description 4096 + :host 500 + :code 500 + :icon 500}) + (def schema:registry-entry [:map [:plugin-id :string] [:version {:optional true} :int] - [:name :string] - [:description {:optional true} :string] - [:host :string] - [:code :string] - [:icon {:optional true} :string] + [:name [:string {:max (:name registry-entry-max-lengths)}]] + [:description {:optional true} [:string {:max (:description registry-entry-max-lengths)}]] + [:host [:string {:max (:host registry-entry-max-lengths)}]] + [:code [:string {:max (:code registry-entry-max-lengths)}]] + [:icon {:optional true} [:string {:max (:icon registry-entry-max-lengths)}]] [:permissions schema:permissions]]) (def schema:plugin-registry [:map - [:ids [:vector :string]] + [:ids [:vector {:max max-plugins} :string]] [:data - [:map-of {:gen/max 5} + [:map-of {:gen/max 5 :max max-plugins} :string schema:registry-entry]]]) diff --git a/common/test/common_tests/runner.cljc b/common/test/common_tests/runner.cljc index 8cab260e59..728fafda6d 100644 --- a/common/test/common_tests/runner.cljc +++ b/common/test/common_tests/runner.cljc @@ -84,6 +84,7 @@ [common-tests.types.objects-map-test] [common-tests.types.organization-test] [common-tests.types.path-data-test] + [common-tests.types.plugins-test] [common-tests.types.shape-decode-encode-test] [common-tests.types.shape-interactions-test] [common-tests.types.shape-layout-test] @@ -167,6 +168,7 @@ 'common-tests.types.objects-map-test 'common-tests.types.organization-test 'common-tests.types.path-data-test + 'common-tests.types.plugins-test 'common-tests.types.shape-decode-encode-test 'common-tests.types.shape-interactions-test 'common-tests.types.shape-layout-test diff --git a/common/test/common_tests/types/plugins_test.cljc b/common/test/common_tests/types/plugins_test.cljc new file mode 100644 index 0000000000..ff4f2b65cf --- /dev/null +++ b/common/test/common_tests/types/plugins_test.cljc @@ -0,0 +1,64 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS SUBSIDIARY SL + +(ns common-tests.types.plugins-test + (:require + [app.common.schema :as sm] + [app.common.types.plugins :as ctp] + [clojure.test :as t])) + +(def ^:private valid-entry + {:plugin-id "plugin-1" + :name "Test Plugin" + :description "A test plugin" + :host "https://example.com" + :code "(function() {})()" + :permissions #{"content:read"}}) + +(t/deftest registry-entry-accepts-valid-plugin + (t/is (true? (sm/validate ctp/schema:registry-entry valid-entry)))) + +(t/deftest registry-entry-rejects-oversized-code + (t/is (false? (sm/validate ctp/schema:registry-entry + (assoc valid-entry + :code (apply str (repeat 501 "x"))))))) + +(t/deftest registry-entry-rejects-oversized-name + (t/is (false? (sm/validate ctp/schema:registry-entry + (assoc valid-entry :name (apply str (repeat 501 "x"))))))) + +(t/deftest registry-entry-rejects-oversized-host + (t/is (false? (sm/validate ctp/schema:registry-entry + (assoc valid-entry :host (apply str (repeat 501 "x"))))))) + +(t/deftest registry-entry-rejects-oversized-description + (t/is (false? (sm/validate ctp/schema:registry-entry + (assoc valid-entry :description (apply str (repeat 4097 "x"))))))) + +(t/deftest registry-entry-rejects-oversized-icon + (t/is (false? (sm/validate ctp/schema:registry-entry + (assoc valid-entry :icon (apply str (repeat 501 "x"))))))) + +(t/deftest registry-entry-accepts-values-at-max + (t/is (true? (sm/validate ctp/schema:registry-entry + (assoc valid-entry + :name (apply str (repeat 500 "x")) + :host (apply str (repeat 500 "x")) + :description (apply str (repeat 4096 "x")) + :icon (apply str (repeat 500 "x")) + :code (apply str (repeat 500 "x"))))))) + +(defn- make-registry + [n] + (let [ids (mapv #(str "plugin-" %) (range n))] + {:ids ids + :data (into {} (map (fn [id] [id (assoc valid-entry :plugin-id id)]) ids))})) + +(t/deftest plugin-registry-accepts-fifty-plugins + (t/is (true? (sm/validate ctp/schema:plugin-registry (make-registry 50))))) + +(t/deftest plugin-registry-rejects-more-than-fifty-plugins + (t/is (false? (sm/validate ctp/schema:plugin-registry (make-registry 51))))) diff --git a/frontend/scripts/check-translations/words.es.txt b/frontend/scripts/check-translations/words.es.txt index 0fa7fe5ffa..009f4c1c4e 100644 --- a/frontend/scripts/check-translations/words.es.txt +++ b/frontend/scripts/check-translations/words.es.txt @@ -274,6 +274,7 @@ hay hubo importada iniciarse +instalarla invitaciones lectores letras diff --git a/frontend/src/app/main/errors.cljs b/frontend/src/app/main/errors.cljs index 056e10eecf..928722f889 100644 --- a/frontend/src/app/main/errors.cljs +++ b/frontend/src/app/main/errors.cljs @@ -719,6 +719,15 @@ :level :error :timeout 3000}))) + ;; Rejected profile write: notify and keep the app running + (= code :props-too-large) + (let [message (tr "errors.profile-props-too-large")] + (st/async-emit! + (ntf/show {:content message + :type :toast + :level :error + :timeout 5000}))) + (= code :snapshot-already-locked) (let [message (tr "errors.version-already-locked")] (st/async-emit! diff --git a/frontend/src/app/main/ui/confirm.scss b/frontend/src/app/main/ui/confirm.scss index 2f61a5bb16..d3662c3659 100644 --- a/frontend/src/app/main/ui/confirm.scss +++ b/frontend/src/app/main/ui/confirm.scss @@ -79,6 +79,7 @@ .modal-msg { @include use-typography("body-large"); + overflow-wrap: anywhere; color: var(--color-foreground-secondary); } diff --git a/frontend/src/app/main/ui/workspace/main_menu.cljs b/frontend/src/app/main/ui/workspace/main_menu.cljs index f1340e9ca4..5a64cf76c9 100644 --- a/frontend/src/app/main/ui/workspace/main_menu.cljs +++ b/frontend/src/app/main/ui/workspace/main_menu.cljs @@ -806,7 +806,7 @@ :submenu-item true :disabled (not can-open?)) :on-key-down on-key-down} - [:span {:class (stl/css :item-name)} name] + [:span {:class (stl/css :item-name :plugin-name) :title name} name] (when-not can-open? [:span {:title (tr "workspace.plugins.error.need-editor")} [:> icon* {:icon-id i/help diff --git a/frontend/src/app/main/ui/workspace/main_menu.scss b/frontend/src/app/main/ui/workspace/main_menu.scss index 1ff9364c6c..591e96b9c7 100644 --- a/frontend/src/app/main/ui/workspace/main_menu.scss +++ b/frontend/src/app/main/ui/workspace/main_menu.scss @@ -9,6 +9,7 @@ @use "ds/_borders.scss" as *; @use "ds/_sizes.scss" as *; @use "ds/_utils.scss" as *; +@use "ds/mixins.scss" as *; .base-menu { position: absolute; @@ -70,6 +71,7 @@ } &.plugins { + max-width: $sz-364; max-height: calc(100vh - $sz-200); overflow: hidden auto; } @@ -120,6 +122,12 @@ grid-area: name; } +.plugin-name { + @include text-ellipsis; + + min-width: 0; +} + .item-indicator { --menu-indicator-color: var(--color-foreground-secondary); diff --git a/frontend/src/app/main/ui/workspace/plugins.cljs b/frontend/src/app/main/ui/workspace/plugins.cljs index 41cf242d8d..a6ff1b34f0 100644 --- a/frontend/src/app/main/ui/workspace/plugins.cljs +++ b/frontend/src/app/main/ui/workspace/plugins.cljs @@ -56,7 +56,7 @@ icon)) (mf/defc plugin-entry* - [{:keys [index manifest user-can-edit on-open-plugin on-remove-plugin]}] + [{:keys [index manifest user-can-edit on-open-plugin on-remove-plugin remove-disabled]}] (let [{:keys [plugin-id host icon name description permissions]} manifest plugins-permissions-peek (deref refs/plugins-permissions-peek) @@ -86,8 +86,8 @@ (icon-url host icon) (avatars/generate {:name name}))}]] [:div {:class (stl/css :plugin-description)} - [:div {:class (stl/css :plugin-title)} name] - [:div {:class (stl/css :plugin-summary)} (d/nilv description "")]] + [:div {:class (stl/css :plugin-title) :title name} name] + [:div {:class (stl/css :plugin-summary) :title description} (d/nilv description "")]] [:> button* {:class (stl/css :open-button) @@ -100,6 +100,7 @@ [:> icon-button* {:variant "ghost" :aria-label (tr "workspace.plugins.remove-plugin") :on-click handle-delete-click + :disabled remove-disabled :icon i/delete}]])) (mf/defc plugin-management-dialog @@ -126,6 +127,10 @@ fetching-manifest? (mf/use-state false) + ;; Ids with a persist in flight; their remove button is disabled + in-flight* + (mf/use-state #{}) + on-url-change (mf/use-fn (fn [value] @@ -174,13 +179,35 @@ (mf/deps plugins-state) (fn [plugin-index] (let [plugins-list (preg/plugins-list) - plugin (nth plugins-list plugin-index)] - (st/emit! (ev/event {::ev/name "remove-plugin" - :name (:name plugin) - :host (:host plugin)})) - (dp/close-plugin! plugin) - (preg/remove-plugin! plugin) - (reset! plugins-state* (preg/plugins-list)))))] + plugin (nth plugins-list plugin-index) + plugin-name (:name plugin) + ;; Truncated so long names fit the confirm dialog + plugin-name (if (> (count plugin-name) 60) + (str (subs plugin-name 0 60) "…") + plugin-name)] + (modal/show! + {:type :confirm + :title (tr "workspace.plugins.remove-confirmation.title") + :message (tr "workspace.plugins.remove-confirmation.message" plugin-name) + :accept-label (tr "workspace.plugins.remove-plugin") + :on-accept (fn [_] + (st/emit! (ev/event {::ev/name "remove-plugin" + :name (:name plugin) + :host (:host plugin)})) + (dp/close-plugin! plugin) + (preg/remove-plugin! plugin) + (modal/show! :plugin-management {})) + :on-cancel (fn [_] + (modal/show! :plugin-management {}))}))))] + + (mf/with-effect [] + (let [listener (preg/subscribe-in-flight! #(reset! in-flight* %))] + (partial preg/unsubscribe-in-flight! listener))) + + ;; Re-reads the list on every registry change, including rollbacks + (mf/with-effect [] + (let [listener (preg/subscribe-registry! #(reset! plugins-state* (preg/plugins-list)))] + (partial preg/unsubscribe-registry! listener))) [:div {:class (stl/css :modal-overlay)} [:div {:class (stl/css :modal-dialog :plugin-management)} @@ -236,6 +263,7 @@ :index idx :manifest manifest :user-can-edit user-can-edit? + :remove-disabled (contains? @in-flight* (:plugin-id manifest)) :on-open-plugin on-open-plugin :on-remove-plugin on-remove-plugin}])]])]]])) diff --git a/frontend/src/app/main/ui/workspace/plugins.scss b/frontend/src/app/main/ui/workspace/plugins.scss index b1aa38ff8e..c67d5ea498 100644 --- a/frontend/src/app/main/ui/workspace/plugins.scss +++ b/frontend/src/app/main/ui/workspace/plugins.scss @@ -5,6 +5,7 @@ // Copyright (c) KALEIDOS SUBSIDIARY SL @use "refactor/common-refactor.scss" as deprecated; +@use "ds/mixins.scss" as *; .modal-overlay { @extend %modal-overlay-base; @@ -15,6 +16,7 @@ display: grid; grid-template-rows: auto 1fr auto; + grid-template-columns: minmax(0, 1fr); max-height: initial; &.plugin-permissions { @@ -160,17 +162,21 @@ flex-direction: column; gap: deprecated.$s-8; width: 100%; + min-width: 0; } .plugin-title { @include deprecated.body-medium-typography; + @include text-ellipsis; color: var(--color-foreground-primary); } .plugin-summary { @include deprecated.body-small-typography; + @include two-line-text-ellipsis; + overflow-wrap: anywhere; color: var(--color-foreground-secondary); } diff --git a/frontend/src/app/plugins/register.cljs b/frontend/src/app/plugins/register.cljs index 2a3720c65e..c70b4f9e7a 100644 --- a/frontend/src/app/plugins/register.cljs +++ b/frontend/src/app/plugins/register.cljs @@ -112,6 +112,20 @@ manifest (.error js/console (clj->js (sm/explain ctp/schema:registry-entry manifest)))))) +(defn subscribe-registry! + "Subscribes f, called with no arguments on every registry change. + Returns f." + [f] + (add-watch registry f (fn [_ _ old new] + (when-not (identical? old new) + (f)))) + f) + +(defn unsubscribe-registry! + [f] + (remove-watch registry f) + nil) + (defn load-from-store [] (reset! registry (get-in @st/state [:profile :props :plugins] {}))) @@ -122,36 +136,128 @@ (declare remove-plugin!) +;; Plugin ids with a persist in flight; install/remove calls on them are skipped +(defonce ^:private in-flight (atom #{})) + +(defonce ^:private in-flight-listeners (atom #{})) + +(defn subscribe-in-flight! + "Subscribes f, called with the in-flight id set on every change. + Calls f immediately with the current set. Returns f." + [f] + (swap! in-flight-listeners conj f) + (f @in-flight) + f) + +(defn unsubscribe-in-flight! + [f] + (swap! in-flight-listeners disj f) + nil) + +(defn- notify-in-flight! + [] + (let [ids @in-flight] + (doseq [f @in-flight-listeners] + (f ids)))) + +(defn- track! + [plugin-id] + (swap! in-flight conj plugin-id) + (notify-in-flight!)) + +(defn- release! + [plugin-id] + (swap! in-flight disj plugin-id) + (notify-in-flight!)) + +(defn- validation-error? + [err] + (= :validation (:type (ex-data err)))) + +(defn- drop-local! + [{:keys [plugin-id]}] + (swap! registry #(-> % + (update :ids (fn [ids] (vec (remove (partial = plugin-id) ids)))) + (update :data dissoc plugin-id)))) + +(defn- insert-at + [ids idx id] + (let [v (vec ids) + idx (max 0 (min idx (count v)))] + (vec (concat (subvec v 0 idx) [id] (subvec v idx))))) + +(defn- restore-local! + "Puts the stored plugin back into the registry at position idx." + [{:keys [plugin-id] :as plugin} idx] + (swap! registry #(-> % + (update :ids (fn [ids] + (insert-at (remove (partial = plugin-id) ids) + idx + plugin-id))) + (assoc-in [:data plugin-id] plugin)))) + (defn install-plugin! [plugin] - (letfn [(update-ids [ids] - (conj - (->> ids (remove #(= % (:plugin-id plugin)))) - (:plugin-id plugin)))] - (swap! registry #(-> % - (update :ids update-ids) - (update :data assoc (:plugin-id plugin) plugin))) - (->> (rp/cmd! :add-profile-plugin {:plugin plugin}) - (rx/subs! identity - (fn [err] - (remove-plugin! plugin) - (.error js/console "Failed to install plugin:" err)))))) + (let [plugin-id (:plugin-id plugin) + previous (get-plugin plugin-id) + prev-idx (.indexOf (vec (:ids @registry)) plugin-id)] + (when-not (contains? @in-flight plugin-id) + (track! plugin-id) + (letfn [(update-ids [ids] + (conj + (->> ids (remove #(= % (:plugin-id plugin)))) + (:plugin-id plugin)))] + (swap! registry #(-> % + (update :ids update-ids) + (update :data assoc (:plugin-id plugin) plugin))) + (->> (rp/cmd! :add-profile-plugin {:plugin plugin}) + (rx/subs! (fn [_] + (release! plugin-id)) + (fn [err] + (release! plugin-id) + ;; Restore the previous version in place, else drop it + (if previous + (restore-local! previous prev-idx) + (drop-local! plugin)) + ;; Other failures may have reached the server: undo it + ;; once by re-saving the previous version or removing + ;; the new entry, without further rollback. + (when-not (validation-error? err) + (->> (if previous + (rp/cmd! :add-profile-plugin {:plugin previous}) + (rp/cmd! :remove-profile-plugin {:plugin-id plugin-id})) + (rx/subs! (fn [_] nil) + (fn [err2] + (.error js/console "Rollback failed:" err2))))) + (.error js/console "Failed to install plugin:" err)))))))) (defn remove-plugin! [{:keys [plugin-id]}] - (let [plugin (get-plugin plugin-id)] - (letfn [(update-ids [ids] - (->> ids - (remove #(= % plugin-id))))] - (swap! registry #(-> % - (update :ids update-ids) - (update :data dissoc plugin-id))) - (->> (rp/cmd! :remove-profile-plugin {:plugin-id plugin-id}) - (rx/subs! identity - (fn [err] - (when plugin - (install-plugin! plugin)) - (.error js/console "Failed to remove plugin:" err))))))) + (let [stored (get-plugin plugin-id) + prev-idx (.indexOf (vec (:ids @registry)) plugin-id)] + (when-not (contains? @in-flight plugin-id) + (track! plugin-id) + (letfn [(update-ids [ids] + (->> ids + (remove #(= % plugin-id))))] + (swap! registry #(-> % + (update :ids update-ids) + (update :data dissoc plugin-id))) + (->> (rp/cmd! :remove-profile-plugin {:plugin-id plugin-id}) + (rx/subs! (fn [_] + (release! plugin-id)) + (fn [err] + (release! plugin-id) + (when stored + ;; Restore in place; validation errors keep it server-side + (restore-local! stored prev-idx) + ;; Other failures: re-save it once, without further rollback + (when-not (validation-error? err) + (->> (rp/cmd! :add-profile-plugin {:plugin stored}) + (rx/subs! (fn [_] nil) + (fn [err2] + (.error js/console "Rollback install failed:" err2)))))) + (.error js/console "Failed to remove plugin:" err)))))))) (defn check-permission [plugin-id permission] diff --git a/frontend/test/frontend_tests/plugins/register_test.cljs b/frontend/test/frontend_tests/plugins/register_test.cljs new file mode 100644 index 0000000000..55ddd1c4a6 --- /dev/null +++ b/frontend/test/frontend_tests/plugins/register_test.cljs @@ -0,0 +1,322 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS SUBSIDIARY SL + +(ns frontend-tests.plugins.register-test + (:require + [app.main.repo :as rp] + [app.plugins.register :as preg] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.mock :as mock])) + +(defn- record-cmd-mock + "Mock rp/cmd! that records calls in mock/rpc-calls and answers + with (response-fn cmd params)." + [response-fn] + (mock/stub + (fn [cmd params] + (swap! mock/rpc-calls conj {:cmd cmd :params params}) + (response-fn cmd params)))) + +(defn- cmds + [] + (mapv :cmd @mock/rpc-calls)) + +;; --- install-plugin! --- + +(t/deftest install-success-releases-id + (t/async done + (mock/with-mocks + {rp/cmd! (record-cmd-mock (fn [_ _] (rx/of {:ok true})))} + (fn [done'] + (let [plugin {:plugin-id "reg-install-ok"}] + (preg/install-plugin! plugin) + (t/is (= [:add-profile-plugin] (cmds))) + (t/is (= plugin (preg/get-plugin "reg-install-ok"))) + ;; id released: a second install issues a second RPC + (preg/install-plugin! plugin) + (t/is (= 2 (count @mock/rpc-calls))) + (done'))) + done))) + +(t/deftest install-while-in-flight-issues-no-second-rpc + (t/async done + (let [subjects (atom [])] + (mock/with-mocks + {rp/cmd! (record-cmd-mock + (fn [_ _] + (let [sb (rx/subject)] + (swap! subjects conj sb) + sb)))} + (fn [done'] + (let [plugin {:plugin-id "reg-install-dedupe"}] + (preg/install-plugin! plugin) + (preg/install-plugin! plugin) + (t/is (= 1 (count @mock/rpc-calls)) "second install while in flight is skipped") + ;; complete the pending call; the id is released afterwards + (rx/push! (first @subjects) {:ok true}) + (preg/install-plugin! plugin) + (t/is (= 2 (count @mock/rpc-calls))) + (done'))) + done)))) + +(t/deftest install-validation-error-cleans-local-only + (t/async done + (mock/with-mocks + {rp/cmd! (record-cmd-mock + (fn [_ _] (rx/throw (ex-info "rejected" {:type :validation :code :props-too-large}))))} + (fn [done'] + (let [plugin {:plugin-id "reg-install-validation"}] + (preg/install-plugin! plugin) + (t/is (= 1 (count @mock/rpc-calls)) "no rollback write after validation rejection") + (t/is (nil? (preg/get-plugin "reg-install-validation"))) + ;; id released: the next install retries the RPC + (preg/install-plugin! plugin) + (t/is (= 2 (count @mock/rpc-calls))) + (done'))) + done))) + +(t/deftest install-validation-error-restores-previous-version + (t/async done + (mock/with-mocks + {rp/cmd! (record-cmd-mock + (fn [_ params] + (if (= "v2" (get-in params [:plugin :name])) + (rx/throw (ex-info "rejected" {:type :validation})) + (rx/of {:ok true}))))} + (fn [done'] + (let [v1 {:plugin-id "reg-install-prev" :name "v1"} + v2 {:plugin-id "reg-install-prev" :name "v2"}] + (preg/install-plugin! v1) + (preg/install-plugin! v2) + (t/is (= 2 (count @mock/rpc-calls))) + (t/is (= v1 (preg/get-plugin "reg-install-prev")) + "the server-kept version is restored, not dropped") + (done'))) + done))) + +(t/deftest install-validation-error-keeps-original-position + (t/async done + (mock/with-mocks + {rp/cmd! (record-cmd-mock + (fn [_ params] + (if (= "v2" (get-in params [:plugin :name])) + (rx/throw (ex-info "rejected" {:type :validation})) + (rx/of {:ok true}))))} + (fn [done'] + (let [own #{"reg-pos-a" "reg-pos-b" "reg-pos-c"} + v1 {:plugin-id "reg-pos-b" :name "v1"} + v2 {:plugin-id "reg-pos-b" :name "v2"}] + (preg/install-plugin! {:plugin-id "reg-pos-a"}) + (preg/install-plugin! v1) + (preg/install-plugin! {:plugin-id "reg-pos-c"}) + (preg/install-plugin! v2) + ;; installs prepend, so newest-first; the rejected update + ;; must preserve order and restore v1 + (t/is (= ["reg-pos-c" "reg-pos-b" "reg-pos-a"] + (filterv own (mapv :plugin-id (preg/plugins-list))))) + (t/is (= v1 (preg/get-plugin "reg-pos-b"))) + (done'))) + done))) + +(t/deftest install-persistent-failure-terminates + (t/async done + (mock/with-mocks + {rp/cmd! (record-cmd-mock + (fn [_ _] (rx/throw (ex-info "boom" {:type :other}))))} + (fn [done'] + (let [plugin {:plugin-id "reg-install-hang"}] + (preg/install-plugin! plugin) + (t/is (= [:add-profile-plugin :remove-profile-plugin] (cmds)) + "one-shot rollback: no further calls") + (t/is (nil? (preg/get-plugin "reg-install-hang"))) + (done'))) + done))) + +(t/deftest install-non-validation-error-rolls-back-via-rpc + (t/async done + (mock/with-mocks + {rp/cmd! (record-cmd-mock + (fn [cmd _] + (if (= cmd :add-profile-plugin) + (rx/throw (ex-info "boom" {:type :other})) + (rx/of nil))))} + (fn [done'] + (let [plugin {:plugin-id "reg-install-rollback"}] + (preg/install-plugin! plugin) + (t/is (= [:add-profile-plugin :remove-profile-plugin] (cmds))) + (t/is (nil? (preg/get-plugin "reg-install-rollback"))) + (done'))) + done))) + +(t/deftest install-non-validation-error-on-update-resaves-previous + (t/async done + (mock/with-mocks + {rp/cmd! (record-cmd-mock + (fn [_ params] + (if (= "v2" (get-in params [:plugin :name])) + (rx/throw (ex-info "busy" {:type :concurrency-limit})) + (rx/of {:ok true}))))} + (fn [done'] + (let [own #{"reg-upd-a" "reg-upd-b" "reg-upd-c"} + v1 {:plugin-id "reg-upd-b" :name "v1"} + v2 {:plugin-id "reg-upd-b" :name "v2"}] + (preg/install-plugin! {:plugin-id "reg-upd-a"}) + (preg/install-plugin! v1) + (preg/install-plugin! {:plugin-id "reg-upd-c"}) + (reset! mock/rpc-calls []) + (preg/install-plugin! v2) + (t/is (= [:add-profile-plugin :add-profile-plugin] (cmds)) + "the compensating write re-saves v1, never removes it") + (t/is (= v1 (get-in (second @mock/rpc-calls) [:params :plugin]))) + (t/is (= v1 (preg/get-plugin "reg-upd-b"))) + (t/is (= ["reg-upd-c" "reg-upd-b" "reg-upd-a"] + (filterv own (mapv :plugin-id (preg/plugins-list))))) + (done'))) + done))) + +;; --- remove-plugin! --- + +(t/deftest remove-success-releases-id + (t/async done + (mock/with-mocks + {rp/cmd! (record-cmd-mock (fn [_ _] (rx/of {:ok true})))} + (fn [done'] + (let [plugin {:plugin-id "reg-remove-ok"}] + (preg/install-plugin! plugin) + (preg/remove-plugin! plugin) + (t/is (= [:add-profile-plugin :remove-profile-plugin] (cmds))) + (t/is (nil? (preg/get-plugin "reg-remove-ok"))) + ;; id released: a second remove issues another RPC + (preg/remove-plugin! plugin) + (t/is (= 3 (count @mock/rpc-calls))) + (done'))) + done))) + +(t/deftest remove-while-in-flight-issues-no-second-rpc + (t/async done + (let [subjects (atom [])] + (mock/with-mocks + {rp/cmd! (record-cmd-mock + (fn [cmd _] + (if (= cmd :add-profile-plugin) + (rx/of {:ok true}) + (let [sb (rx/subject)] + (swap! subjects conj sb) + sb))))} + (fn [done'] + (let [plugin {:plugin-id "reg-remove-dedupe"}] + (preg/install-plugin! plugin) + (preg/remove-plugin! plugin) + (preg/remove-plugin! plugin) + (t/is (= 2 (count @mock/rpc-calls)) "second remove while in flight is skipped") + ;; complete the pending call; the id is released afterwards + (rx/push! (first @subjects) nil) + (preg/remove-plugin! plugin) + (t/is (= 3 (count @mock/rpc-calls))) + (done'))) + done)))) + +(t/deftest remove-validation-error-restores-local-only + (t/async done + (mock/with-mocks + {rp/cmd! (record-cmd-mock + (fn [cmd _] + (if (= cmd :add-profile-plugin) + (rx/of {:ok true}) + (rx/throw (ex-info "rejected" {:type :validation})))))} + (fn [done'] + (let [plugin {:plugin-id "reg-remove-validation"}] + (preg/install-plugin! plugin) + (preg/remove-plugin! plugin) + (t/is (= [:add-profile-plugin :remove-profile-plugin] (cmds)) + "no reinstall write after validation rejection") + (t/is (= plugin (preg/get-plugin "reg-remove-validation")) + "local entry is restored") + (done'))) + done))) + +(t/deftest remove-non-validation-error-reinstalls-via-rpc + (t/async done + (mock/with-mocks + {rp/cmd! (record-cmd-mock + (fn [cmd _] + (if (= cmd :remove-profile-plugin) + (rx/throw (ex-info "boom" {:type :other})) + (rx/of {:ok true}))))} + (fn [done'] + (let [plugin {:plugin-id "reg-remove-rollback"}] + (preg/install-plugin! plugin) + (preg/remove-plugin! plugin) + (t/is (= [:add-profile-plugin :remove-profile-plugin :add-profile-plugin] (cmds))) + (t/is (= plugin (preg/get-plugin "reg-remove-rollback"))) + (done'))) + done))) + +(t/deftest remove-persistent-failure-terminates + (t/async done + (mock/with-mocks + {rp/cmd! (record-cmd-mock + (fn [cmd _] + (if (= cmd :add-profile-plugin) + (rx/of {:ok true}) + (rx/throw (ex-info "boom" {:type :other})))))} + (fn [done'] + (let [plugin {:plugin-id "reg-remove-hang"}] + (preg/install-plugin! plugin) + (preg/remove-plugin! plugin) + (t/is (= [:add-profile-plugin :remove-profile-plugin :add-profile-plugin] (cmds)) + "one-shot rollback: no further calls") + (t/is (= plugin (preg/get-plugin "reg-remove-hang"))) + (done'))) + done))) + +(t/deftest remove-validation-error-keeps-original-position + (t/async done + (mock/with-mocks + {rp/cmd! (record-cmd-mock + (fn [cmd _] + (if (= cmd :add-profile-plugin) + (rx/of {:ok true}) + (rx/throw (ex-info "rejected" {:type :validation})))))} + (fn [done'] + (let [own #{"reg-idx-a" "reg-idx-b" "reg-idx-c"} + plugin {:plugin-id "reg-idx-b"}] + (preg/install-plugin! {:plugin-id "reg-idx-a"}) + (preg/install-plugin! plugin) + (preg/install-plugin! {:plugin-id "reg-idx-c"}) + (preg/remove-plugin! plugin) + ;; installs prepend, so the order is newest-first; + ;; the failed removal must preserve it exactly + (t/is (= ["reg-idx-c" "reg-idx-b" "reg-idx-a"] + (filterv own (mapv :plugin-id (preg/plugins-list))))) + (done'))) + done))) + +;; --- subscribe-registry! --- + +(t/deftest registry-subscriber-sees-rollback + (t/async done + (let [subjects (atom []) + seen (atom []) + listener (fn [] (swap! seen conj (some? (preg/get-plugin "reg-watch"))))] + (mock/with-mocks + {rp/cmd! (record-cmd-mock + (fn [_ _] + (let [sb (rx/subject)] + (swap! subjects conj sb) + sb)))} + (fn [done'] + (preg/subscribe-registry! listener) + (preg/install-plugin! {:plugin-id "reg-watch"}) + (rx/error! (first @subjects) (ex-info "rejected" {:type :validation})) + (t/is (= [true false] @seen) + "notified on the optimistic add and on the rollback") + (preg/unsubscribe-registry! listener) + (preg/install-plugin! {:plugin-id "reg-watch"}) + (t/is (= [true false] @seen) "no calls after unsubscribing") + (done')) + done)))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index b3764d977b..0937f7b732 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -72,6 +72,7 @@ [frontend-tests.plugins.page-active-validation-test] [frontend-tests.plugins.page-test] [frontend-tests.plugins.parser-test] + [frontend-tests.plugins.register-test] [frontend-tests.plugins.shape-bugfixes-test] [frontend-tests.plugins.text-test] [frontend-tests.plugins.tokens-test] @@ -207,6 +208,7 @@ 'frontend-tests.plugins.page-active-validation-test 'frontend-tests.plugins.page-test 'frontend-tests.plugins.parser-test + 'frontend-tests.plugins.register-test 'frontend-tests.plugins.shape-bugfixes-test 'frontend-tests.plugins.text-test 'frontend-tests.plugins.tokens-test diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 815de4addb..b25cf8ed43 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -1438,11 +1438,11 @@ msgstr "Attention" msgid "ds.component-subtitle" msgstr "Components to update:" -#: src/app/main/ui/workspace/plugins.cljs:379, src/app/main/ui/workspace/plugins.cljs:442 +#: src/app/main/ui/workspace/plugins.cljs:407, src/app/main/ui/workspace/plugins.cljs:470 msgid "ds.confirm-allow" msgstr "Allow" -#: src/app/main/ui/comments.cljs:719, src/app/main/ui/confirm.cljs:43, src/app/main/ui/settings/subscription.cljs:327, src/app/main/ui/settings/subscription.cljs:360, src/app/main/ui/settings/subscription.cljs:844, src/app/main/ui/settings/subscription.cljs:893, src/app/main/ui/settings/subscription.cljs:903, src/app/main/ui/workspace/plugins.cljs:373, src/app/main/ui/workspace/plugins.cljs:436 +#: src/app/main/ui/comments.cljs:719, src/app/main/ui/confirm.cljs:43, src/app/main/ui/settings/subscription.cljs:327, src/app/main/ui/settings/subscription.cljs:360, src/app/main/ui/settings/subscription.cljs:844, src/app/main/ui/settings/subscription.cljs:893, src/app/main/ui/settings/subscription.cljs:903, src/app/main/ui/workspace/plugins.cljs:401, src/app/main/ui/workspace/plugins.cljs:464 msgid "ds.confirm-cancel" msgstr "Cancel" @@ -1524,7 +1524,7 @@ msgstr "" "Clipboard access denied. Please allow clipboard permissions in your browser " "to paste content" -#: src/app/main/errors.cljs:773 +#: src/app/main/errors.cljs:782 msgid "errors.comment-error" msgstr "There was an error with the comment" @@ -1532,21 +1532,21 @@ msgstr "There was an error with the comment" msgid "errors.connection-error" msgstr "Cannot reach the server. Check your connection and try again." -#: src/app/main/errors.cljs:842 +#: src/app/main/errors.cljs:851 msgid "errors.deprecated" msgstr "" "Sorry! This is an old file that uses a deprecated type of Penpot assets and " "you can't open it." -#: src/app/main/errors.cljs:845 +#: src/app/main/errors.cljs:854 msgid "errors.deprecated.contact.after" msgstr "so we can help you." -#: src/app/main/errors.cljs:843 +#: src/app/main/errors.cljs:852 msgid "errors.deprecated.contact.before" msgstr "Although Penpot no longer support this type of Assets, you can" -#: src/app/main/errors.cljs:844 +#: src/app/main/errors.cljs:853 msgid "errors.deprecated.contact.text" msgstr "contact us" @@ -1582,13 +1582,13 @@ msgstr "The email «%s» has many permanent bounce reports." msgid "errors.email-spam-or-permanent-bounces" msgstr "The email «%s» has been reported as spam or permanently bounce." -#: src/app/main/errors.cljs:819 +#: src/app/main/errors.cljs:828 msgid "errors.feature-mismatch" msgstr "" "Looks like you are opening a file that has the feature '%s' enabled but the " "current penpot version does not supports it or has it disabled." -#: src/app/main/errors.cljs:823, src/app/main/errors.cljs:837 +#: src/app/main/errors.cljs:832, src/app/main/errors.cljs:846 msgid "errors.feature-not-supported" msgstr "Feature '%s' is not supported." @@ -1608,7 +1608,7 @@ msgstr "Empty field" msgid "errors.field-not-all-whitespace" msgstr "The name must contain some character other than space." -#: src/app/main/errors.cljs:815 +#: src/app/main/errors.cljs:824 msgid "errors.file-feature-mismatch" msgstr "" "It seems that there is a mismatch between the enabled features and the " @@ -1621,11 +1621,11 @@ msgstr "" "The font family name can only contain letters, numbers, spaces, hyphens, " "underscores, and dots." -#: src/app/main/data/auth.cljs:352, src/app/main/errors.cljs:479, src/app/main/ui/auth/login.cljs:116, src/app/main/ui/auth/register.cljs:131, src/app/main/ui/auth/register.cljs:317, src/app/main/ui/auth/verify_token.cljs:158, src/app/main/ui/dashboard/fonts.cljs:386, src/app/main/ui/dashboard/team.cljs:250, src/app/main/ui/dashboard/team.cljs:1131, src/app/main/ui/onboarding/team_choice.cljs:125, src/app/main/ui/settings/feedback.cljs:84, src/app/main/ui/settings/integrations.cljs:143, src/app/main/ui/workspace/main_menu.cljs:966 +#: src/app/main/data/auth.cljs:352, src/app/main/errors.cljs:479, src/app/main/ui/auth/login.cljs:116, src/app/main/ui/auth/register.cljs:131, src/app/main/ui/auth/register.cljs:317, src/app/main/ui/auth/verify_token.cljs:131, src/app/main/ui/dashboard/fonts.cljs:386, src/app/main/ui/dashboard/team.cljs:250, src/app/main/ui/dashboard/team.cljs:1131, src/app/main/ui/onboarding/team_choice.cljs:125, src/app/main/ui/settings/feedback.cljs:84, src/app/main/ui/settings/integrations.cljs:143, src/app/main/ui/workspace/main_menu.cljs:966 msgid "errors.generic" msgstr "Something wrong has happened." -#: src/app/main/errors.cljs:752 +#: src/app/main/errors.cljs:761 msgid "errors.internal-worker-error" msgstr "Something wrong has happened with the web worker." @@ -1675,7 +1675,7 @@ msgstr "This invite might be canceled or may be expired." msgid "errors.ldap-disabled" msgstr "LDAP authentication is disabled." -#: src/app/main/errors.cljs:831, src/app/main/ui/dashboard/team.cljs:234, src/app/main/ui/dashboard/team.cljs:1123, src/app/main/ui/onboarding/team_choice.cljs:117 +#: src/app/main/errors.cljs:840, src/app/main/ui/dashboard/team.cljs:234, src/app/main/ui/dashboard/team.cljs:1123, src/app/main/ui/onboarding/team_choice.cljs:117 msgid "errors.max-quota-reached" msgstr "You have reached the '%s' quota. Contact support." @@ -1701,7 +1701,7 @@ msgstr "Seems that this is not a valid image." msgid "errors.member-is-muted" msgstr "The profile you inviting has emails muted (spam reports or high bounces)." -#: src/app/main/errors.cljs:805 +#: src/app/main/errors.cljs:814 msgid "errors.migration-in-progress" msgstr "Migration in progress" @@ -1745,6 +1745,12 @@ msgstr "The profile is blocked" msgid "errors.profile-is-muted" msgstr "Your profile has emails muted (spam reports or high bounces)." +#: src/app/main/errors.cljs:724 +msgid "errors.profile-props-too-large" +msgstr "" +"Your profile settings are too large to save. Remove some plugins and try " +"again." + #: src/app/main/data/auth.cljs:340, src/app/main/ui/auth/register.cljs:96 msgid "errors.registration-disabled" msgstr "The registration is currently disabled." @@ -1759,11 +1765,11 @@ msgstr "" msgid "errors.save-retrying" msgstr "Connection lost. Retrying to save your changes." -#: src/app/main/errors.cljs:764 +#: src/app/main/errors.cljs:773 msgid "errors.svg-parser.invalid-svg" msgstr "SVG is invalid or malformed" -#: src/app/main/errors.cljs:810 +#: src/app/main/errors.cljs:819 msgid "errors.team-feature-mismatch" msgstr "Detected incompatible feature '%s'" @@ -1936,7 +1942,7 @@ msgstr "An unexpected error occurred." msgid "errors.unexpected-token" msgstr "Unknown token" -#: src/app/main/errors.cljs:723 +#: src/app/main/errors.cljs:732 msgid "errors.version-already-locked" msgstr "This version is already locked" @@ -1944,7 +1950,7 @@ msgstr "This version is already locked" msgid "errors.version-locked" msgstr "This version is locked and cannot be deleted by others" -#: src/app/main/errors.cljs:827 +#: src/app/main/errors.cljs:836 msgid "errors.version-not-supported" msgstr "File has an incompatible version number" @@ -8099,16 +8105,16 @@ msgstr "Snap nodes (%s)" msgid "workspace.plugins.button-open" msgstr "Open" -#: src/app/main/ui/workspace/plugins.cljs:215 +#: src/app/main/ui/workspace/plugins.cljs:242 #, markdown msgid "workspace.plugins.discover" msgstr "Discover [more plugins](%s)" -#: src/app/main/ui/workspace/plugins.cljs:222 +#: src/app/main/ui/workspace/plugins.cljs:249 msgid "workspace.plugins.empty-plugins" msgstr "No plugins installed yet" -#: src/app/main/ui/workspace/plugins.cljs:209 +#: src/app/main/ui/workspace/plugins.cljs:236 msgid "workspace.plugins.error.manifest" msgstr "The plugin manifest is incorrect." @@ -8120,15 +8126,15 @@ msgstr "You need to be an editor to use this plugin" msgid "workspace.plugins.error.unreachable" msgstr "The plugin URL could not be reached." -#: src/app/main/ui/workspace/plugins.cljs:205 +#: src/app/main/ui/workspace/plugins.cljs:232 msgid "workspace.plugins.error.url" msgstr "The plugin doesn't exist or the URL is not correct." -#: src/app/main/ui/workspace/plugins.cljs:201 +#: src/app/main/ui/workspace/plugins.cljs:228 msgid "workspace.plugins.install" msgstr "Install" -#: src/app/main/ui/workspace/plugins.cljs:231 +#: src/app/main/ui/workspace/plugins.cljs:258 msgid "workspace.plugins.installed-plugins" msgstr "Installed plugins" @@ -8140,102 +8146,112 @@ msgstr "Plugins manager" msgid "workspace.plugins.menu.title" msgstr "Plugins" -#: src/app/main/ui/workspace/plugins.cljs:424 +#: src/app/main/ui/workspace/plugins.cljs:452 msgid "workspace.plugins.permissions-update.title" msgstr "UPDATE THIS PLUGIN" -#: src/app/main/ui/workspace/plugins.cljs:428 +#: src/app/main/ui/workspace/plugins.cljs:456 msgid "workspace.plugins.permissions-update.warning" msgstr "" "The plugin has been modified since you last opened it. It now also wants to " "access:" -#: src/app/main/ui/workspace/plugins.cljs:297 +#: src/app/main/ui/workspace/plugins.cljs:325 msgid "workspace.plugins.permissions.allow-download" msgstr "Start file downloads." -#: src/app/main/ui/workspace/plugins.cljs:304 +#: src/app/main/ui/workspace/plugins.cljs:332 msgid "workspace.plugins.permissions.allow-localstorage" msgstr "Store data in the browser." -#: src/app/main/ui/workspace/plugins.cljs:317 +#: src/app/main/ui/workspace/plugins.cljs:345 msgid "workspace.plugins.permissions.clipboard-read" msgstr "Read the contents of your clipboard." -#: src/app/main/ui/workspace/plugins.cljs:311 +#: src/app/main/ui/workspace/plugins.cljs:339 msgid "workspace.plugins.permissions.clipboard-write" msgstr "Read and write to your clipboard." -#: src/app/main/ui/workspace/plugins.cljs:290 +#: src/app/main/ui/workspace/plugins.cljs:318 msgid "workspace.plugins.permissions.comment-read" msgstr "Read your comments and replies." -#: src/app/main/ui/workspace/plugins.cljs:284 +#: src/app/main/ui/workspace/plugins.cljs:312 msgid "workspace.plugins.permissions.comment-write" msgstr "Read and modify your comments and reply in your name." -#: src/app/main/ui/workspace/plugins.cljs:257 +#: src/app/main/ui/workspace/plugins.cljs:285 msgid "workspace.plugins.permissions.content-read" msgstr "Read the content of files that users have access to." -#: src/app/main/ui/workspace/plugins.cljs:251 +#: src/app/main/ui/workspace/plugins.cljs:279 msgid "workspace.plugins.permissions.content-write" msgstr "Read and modify the content of files that users have access to." -#: src/app/main/ui/workspace/plugins.cljs:366 +#: src/app/main/ui/workspace/plugins.cljs:394 msgid "workspace.plugins.permissions.disclaimer" msgstr "" "Please note that this plugin is created by an external party, so ensure you " "trust it before granting access. Your data privacy and security are " "important to us. If you have any concerns, please contact support." -#: src/app/main/ui/workspace/plugins.cljs:277 +#: src/app/main/ui/workspace/plugins.cljs:305 msgid "workspace.plugins.permissions.library-read" msgstr "Read your libraries and assets." -#: src/app/main/ui/workspace/plugins.cljs:271 +#: src/app/main/ui/workspace/plugins.cljs:299 msgid "workspace.plugins.permissions.library-write" msgstr "Read and modify your libraries and assets." -#: src/app/main/ui/workspace/plugins.cljs:359 +#: src/app/main/ui/workspace/plugins.cljs:387 msgid "workspace.plugins.permissions.title" msgstr "'%s' PLUGIN WANTS ACCESS TO:" -#: src/app/main/ui/workspace/plugins.cljs:264 +#: src/app/main/ui/workspace/plugins.cljs:292 msgid "workspace.plugins.permissions.user-read" msgstr "Read the profile information of the current user." -#: src/app/main/ui/workspace/plugins.cljs:227 +#: src/app/main/ui/workspace/plugins.cljs:254 msgid "workspace.plugins.plugin-list-link" msgstr "Plugins List" -#: src/app/main/ui/workspace/plugins.cljs:101 +#: src/app/main/ui/workspace/plugins.cljs:191 +msgid "workspace.plugins.remove-confirmation.message" +msgstr "" +"Are you sure you want to remove the plugin %s? You can install it again at " +"any time." + +#: src/app/main/ui/workspace/plugins.cljs:190 +msgid "workspace.plugins.remove-confirmation.title" +msgstr "Remove plugin" + +#: src/app/main/ui/workspace/plugins.cljs:101, src/app/main/ui/workspace/plugins.cljs:192 msgid "workspace.plugins.remove-plugin" msgstr "Remove plugin" -#: src/app/main/ui/workspace/plugins.cljs:195 +#: src/app/main/ui/workspace/plugins.cljs:222 msgid "workspace.plugins.search-placeholder" msgstr "Write a plugin URL" -#: src/app/main/ui/workspace/plugins.cljs:188 +#: src/app/main/ui/workspace/plugins.cljs:215 msgid "workspace.plugins.title" msgstr "Plugins" -#: src/app/main/ui/workspace/plugins.cljs:494 +#: src/app/main/ui/workspace/plugins.cljs:522 msgid "workspace.plugins.try-out.cancel" msgstr "NOT NOW" -#: src/app/main/ui/workspace/plugins.cljs:487 +#: src/app/main/ui/workspace/plugins.cljs:515 msgid "workspace.plugins.try-out.message" msgstr "" "Want to take a look? It will open in a new draft for your current team. (If " "not, you can always find it in the installed plugins of any file.)" -#: src/app/main/ui/workspace/plugins.cljs:483 +#: src/app/main/ui/workspace/plugins.cljs:511 msgid "workspace.plugins.try-out.title" msgstr "'%s' PLUGIN IS INSTALLED FOR YOUR USER!" -#: src/app/main/ui/workspace/plugins.cljs:500 +#: src/app/main/ui/workspace/plugins.cljs:528 msgid "workspace.plugins.try-out.try" msgstr "TRY PLUGIN" diff --git a/frontend/translations/es.po b/frontend/translations/es.po index 2d22dedfdf..a45e2512dd 100644 --- a/frontend/translations/es.po +++ b/frontend/translations/es.po @@ -1452,11 +1452,11 @@ msgstr "Atención" msgid "ds.component-subtitle" msgstr "Componentes a actualizar:" -#: src/app/main/ui/workspace/plugins.cljs:379, src/app/main/ui/workspace/plugins.cljs:442 +#: src/app/main/ui/workspace/plugins.cljs:407, src/app/main/ui/workspace/plugins.cljs:470 msgid "ds.confirm-allow" msgstr "Permitir" -#: src/app/main/ui/comments.cljs:719, src/app/main/ui/confirm.cljs:43, src/app/main/ui/settings/subscription.cljs:327, src/app/main/ui/settings/subscription.cljs:360, src/app/main/ui/settings/subscription.cljs:844, src/app/main/ui/settings/subscription.cljs:893, src/app/main/ui/settings/subscription.cljs:903, src/app/main/ui/workspace/plugins.cljs:373, src/app/main/ui/workspace/plugins.cljs:436 +#: src/app/main/ui/comments.cljs:719, src/app/main/ui/confirm.cljs:43, src/app/main/ui/settings/subscription.cljs:327, src/app/main/ui/settings/subscription.cljs:360, src/app/main/ui/settings/subscription.cljs:844, src/app/main/ui/settings/subscription.cljs:893, src/app/main/ui/settings/subscription.cljs:903, src/app/main/ui/workspace/plugins.cljs:401, src/app/main/ui/workspace/plugins.cljs:464 msgid "ds.confirm-cancel" msgstr "Cancelar" @@ -1538,25 +1538,25 @@ msgstr "" "Acceso al portapapeles denegado. Permite el acceso al portapapeles en tu " "navegador para pegar contenido." -#: src/app/main/errors.cljs:773 +#: src/app/main/errors.cljs:782 msgid "errors.comment-error" msgstr "Ha habido un error con el comentario" -#: src/app/main/errors.cljs:842 +#: src/app/main/errors.cljs:851 msgid "errors.deprecated" msgstr "" "¡Lo sentimos! Este es un fichero antiguo que utiliza un tipo de recurso de " "Penpot obsoleto, y no puedes abrirlo." -#: src/app/main/errors.cljs:845 +#: src/app/main/errors.cljs:854 msgid "errors.deprecated.contact.after" msgstr "para que podamos ayudarte." -#: src/app/main/errors.cljs:843 +#: src/app/main/errors.cljs:852 msgid "errors.deprecated.contact.before" msgstr "Aunque Penpot ya no da soporte a este tipo de Recursos, puedes" -#: src/app/main/errors.cljs:844 +#: src/app/main/errors.cljs:853 msgid "errors.deprecated.contact.text" msgstr "contactar con nosotros" @@ -1592,13 +1592,13 @@ msgstr "El correo electrónico «%s» tiene varios reportes de rebote permanente msgid "errors.email-spam-or-permanent-bounces" msgstr "El email «%s» tiene reportes de spam o de rebote permanente." -#: src/app/main/errors.cljs:819 +#: src/app/main/errors.cljs:828 msgid "errors.feature-mismatch" msgstr "" "Parece que está abriendo un archivo que tiene la función '%s' habilitada, " "pero la versión actual de penpot no la admite o la tiene deshabilitada." -#: src/app/main/errors.cljs:823, src/app/main/errors.cljs:837 +#: src/app/main/errors.cljs:832, src/app/main/errors.cljs:846 msgid "errors.feature-not-supported" msgstr "Caracteristica no soportada: '%s'." @@ -1618,7 +1618,7 @@ msgstr "Campo vacio" msgid "errors.field-not-all-whitespace" msgstr "Debe contener algún carácter diferente de espacio." -#: src/app/main/errors.cljs:815 +#: src/app/main/errors.cljs:824 msgid "errors.file-feature-mismatch" msgstr "" "Parece que hay discordancia entre las features habilitadas y las features " @@ -1631,11 +1631,11 @@ msgstr "" "El nombre de la familia tipográfica solo puede contener letras, números, " "espacios, guiones, guiones bajos y puntos." -#: src/app/main/data/auth.cljs:352, src/app/main/errors.cljs:479, src/app/main/ui/auth/login.cljs:116, src/app/main/ui/auth/register.cljs:131, src/app/main/ui/auth/register.cljs:317, src/app/main/ui/auth/verify_token.cljs:158, src/app/main/ui/dashboard/fonts.cljs:386, src/app/main/ui/dashboard/team.cljs:250, src/app/main/ui/dashboard/team.cljs:1131, src/app/main/ui/onboarding/team_choice.cljs:125, src/app/main/ui/settings/feedback.cljs:84, src/app/main/ui/settings/integrations.cljs:143, src/app/main/ui/workspace/main_menu.cljs:966 +#: src/app/main/data/auth.cljs:352, src/app/main/errors.cljs:479, src/app/main/ui/auth/login.cljs:116, src/app/main/ui/auth/register.cljs:131, src/app/main/ui/auth/register.cljs:317, src/app/main/ui/auth/verify_token.cljs:131, src/app/main/ui/dashboard/fonts.cljs:386, src/app/main/ui/dashboard/team.cljs:250, src/app/main/ui/dashboard/team.cljs:1131, src/app/main/ui/onboarding/team_choice.cljs:125, src/app/main/ui/settings/feedback.cljs:84, src/app/main/ui/settings/integrations.cljs:143, src/app/main/ui/workspace/main_menu.cljs:966 msgid "errors.generic" msgstr "Ha ocurrido algún error." -#: src/app/main/errors.cljs:752 +#: src/app/main/errors.cljs:761 msgid "errors.internal-worker-error" msgstr "Ha ocurrido un problema con el web worker." @@ -1687,7 +1687,7 @@ msgstr "Esta invitación puede haber sido cancelada o ha expirado." msgid "errors.ldap-disabled" msgstr "La autheticacion via LDAP esta deshabilitada." -#: src/app/main/errors.cljs:831, src/app/main/ui/dashboard/team.cljs:234, src/app/main/ui/dashboard/team.cljs:1123, src/app/main/ui/onboarding/team_choice.cljs:117 +#: src/app/main/errors.cljs:840, src/app/main/ui/dashboard/team.cljs:234, src/app/main/ui/dashboard/team.cljs:1123, src/app/main/ui/onboarding/team_choice.cljs:117 msgid "errors.max-quota-reached" msgstr "Ha alcalzando el maximo de la quota '%s'. Contacte con soporte tecnico." @@ -1717,7 +1717,7 @@ msgstr "" "El perfil que esta invitando tiene los emails silenciados (por reportes de " "spam o alto índice de rebote)." -#: src/app/main/errors.cljs:805 +#: src/app/main/errors.cljs:814 msgid "errors.migration-in-progress" msgstr "Migración en proceso" @@ -1765,6 +1765,12 @@ msgstr "" "Tu perfil tiene los emails silenciados (por reportes de spam o alto índice " "de rebote)." +#: src/app/main/errors.cljs:724 +msgid "errors.profile-props-too-large" +msgstr "" +"La configuración de tu perfil es demasiado grande y no se puede guardar. " +"Elimina algunas extensiones e inténtalo de nuevo." + #: src/app/main/data/auth.cljs:340, src/app/main/ui/auth/register.cljs:96 msgid "errors.registration-disabled" msgstr "El registro está actualmente desactivado." @@ -1776,11 +1782,11 @@ msgstr "" "informar del error y recarga para continuar desde los últimos cambios " "guardados." -#: src/app/main/errors.cljs:764 +#: src/app/main/errors.cljs:773 msgid "errors.svg-parser.invalid-svg" msgstr "El SVG no es válido o está mal formado" -#: src/app/main/errors.cljs:810 +#: src/app/main/errors.cljs:819 msgid "errors.team-feature-mismatch" msgstr "Detectada funcionalidad incompatible '%s'" @@ -1964,7 +1970,7 @@ msgstr "Ha ocurrido un error inesperado." msgid "errors.unexpected-token" msgstr "Token desconocido" -#: src/app/main/errors.cljs:723 +#: src/app/main/errors.cljs:732 msgid "errors.version-already-locked" msgstr "Esta versión ya está bloqueada" @@ -1972,7 +1978,7 @@ msgstr "Esta versión ya está bloqueada" msgid "errors.version-locked" msgstr "Esta versión está bloqueada y otras personas no pueden eliminarla" -#: src/app/main/errors.cljs:827 +#: src/app/main/errors.cljs:836 msgid "errors.version-not-supported" msgstr "El fichero tiene un número de versión incompatible" @@ -8169,16 +8175,16 @@ msgstr "Alinear nodos (%s)" msgid "workspace.plugins.button-open" msgstr "Abrir" -#: src/app/main/ui/workspace/plugins.cljs:215 +#: src/app/main/ui/workspace/plugins.cljs:242 #, markdown msgid "workspace.plugins.discover" msgstr "Descubre [más extensiones](%s)" -#: src/app/main/ui/workspace/plugins.cljs:222 +#: src/app/main/ui/workspace/plugins.cljs:249 msgid "workspace.plugins.empty-plugins" msgstr "No se encuentran extensiones" -#: src/app/main/ui/workspace/plugins.cljs:209 +#: src/app/main/ui/workspace/plugins.cljs:236 msgid "workspace.plugins.error.manifest" msgstr "El manifiesto de la expansión es incorrecto." @@ -8190,15 +8196,15 @@ msgstr "Debes ser un editor para usar este plugin" msgid "workspace.plugins.error.unreachable" msgstr "No se ha podido acceder a la URL de la extensión." -#: src/app/main/ui/workspace/plugins.cljs:205 +#: src/app/main/ui/workspace/plugins.cljs:232 msgid "workspace.plugins.error.url" msgstr "La extensión no existe o la url no es correcta." -#: src/app/main/ui/workspace/plugins.cljs:201 +#: src/app/main/ui/workspace/plugins.cljs:228 msgid "workspace.plugins.install" msgstr "Instalar" -#: src/app/main/ui/workspace/plugins.cljs:231 +#: src/app/main/ui/workspace/plugins.cljs:258 msgid "workspace.plugins.installed-plugins" msgstr "Extensiones instaladas" @@ -8210,49 +8216,49 @@ msgstr "Gestor de extensiones" msgid "workspace.plugins.menu.title" msgstr "Extensiones" -#: src/app/main/ui/workspace/plugins.cljs:424 +#: src/app/main/ui/workspace/plugins.cljs:452 msgid "workspace.plugins.permissions-update.title" msgstr "EXTENSIÓN ACTUALIZADA" -#: src/app/main/ui/workspace/plugins.cljs:428 +#: src/app/main/ui/workspace/plugins.cljs:456 msgid "workspace.plugins.permissions-update.warning" msgstr "" "La extensión ha cambiado desde la última vez que la abriste. Ahora quiere " "acceder a:" -#: src/app/main/ui/workspace/plugins.cljs:297 +#: src/app/main/ui/workspace/plugins.cljs:325 msgid "workspace.plugins.permissions.allow-download" msgstr "Comenzar descargas de ficheros." -#: src/app/main/ui/workspace/plugins.cljs:304 +#: src/app/main/ui/workspace/plugins.cljs:332 msgid "workspace.plugins.permissions.allow-localstorage" msgstr "Guardar datos en el navegador." -#: src/app/main/ui/workspace/plugins.cljs:317 +#: src/app/main/ui/workspace/plugins.cljs:345 msgid "workspace.plugins.permissions.clipboard-read" msgstr "Leer el contenido de tu portapapeles." -#: src/app/main/ui/workspace/plugins.cljs:311 +#: src/app/main/ui/workspace/plugins.cljs:339 msgid "workspace.plugins.permissions.clipboard-write" msgstr "Leer y escribir en tu portapapeles." -#: src/app/main/ui/workspace/plugins.cljs:290 +#: src/app/main/ui/workspace/plugins.cljs:318 msgid "workspace.plugins.permissions.comment-read" msgstr "Leer tus comentarios y respuestas." -#: src/app/main/ui/workspace/plugins.cljs:284 +#: src/app/main/ui/workspace/plugins.cljs:312 msgid "workspace.plugins.permissions.comment-write" msgstr "Leer y modificar tus comentarios y responder en tu nombre." -#: src/app/main/ui/workspace/plugins.cljs:257 +#: src/app/main/ui/workspace/plugins.cljs:285 msgid "workspace.plugins.permissions.content-read" msgstr "Leer el contenido de sus archivos." -#: src/app/main/ui/workspace/plugins.cljs:251 +#: src/app/main/ui/workspace/plugins.cljs:279 msgid "workspace.plugins.permissions.content-write" msgstr "Leer y modificar el contenido de sus archivos." -#: src/app/main/ui/workspace/plugins.cljs:366 +#: src/app/main/ui/workspace/plugins.cljs:394 msgid "workspace.plugins.permissions.disclaimer" msgstr "" "Ten en cuenta que las extensiones están desarrolladas por terceros, " @@ -8260,54 +8266,64 @@ msgstr "" "seguridad es importante para nosotros. Si tienes cualquier duda, contacta " "con soporte." -#: src/app/main/ui/workspace/plugins.cljs:277 +#: src/app/main/ui/workspace/plugins.cljs:305 msgid "workspace.plugins.permissions.library-read" msgstr "Leer la información de sus bibliotecas y recursos." -#: src/app/main/ui/workspace/plugins.cljs:271 +#: src/app/main/ui/workspace/plugins.cljs:299 msgid "workspace.plugins.permissions.library-write" msgstr "Leer y modificar la información de sus bibliotecas y recursos." -#: src/app/main/ui/workspace/plugins.cljs:359 +#: src/app/main/ui/workspace/plugins.cljs:387 msgid "workspace.plugins.permissions.title" msgstr "LA EXTENSIÓN '%s' SOLICITA PERMISO PARA ACCEDER:" -#: src/app/main/ui/workspace/plugins.cljs:264 +#: src/app/main/ui/workspace/plugins.cljs:292 msgid "workspace.plugins.permissions.user-read" msgstr "Leer la información del usuario actual." -#: src/app/main/ui/workspace/plugins.cljs:227 +#: src/app/main/ui/workspace/plugins.cljs:254 msgid "workspace.plugins.plugin-list-link" msgstr "Lista de extensiones" -#: src/app/main/ui/workspace/plugins.cljs:101 +#: src/app/main/ui/workspace/plugins.cljs:191 +msgid "workspace.plugins.remove-confirmation.message" +msgstr "" +"¿Seguro que quieres eliminar la extensión %s? Puedes instalarla de nuevo en " +"cualquier momento." + +#: src/app/main/ui/workspace/plugins.cljs:190 +msgid "workspace.plugins.remove-confirmation.title" +msgstr "Eliminar extensión" + +#: src/app/main/ui/workspace/plugins.cljs:101, src/app/main/ui/workspace/plugins.cljs:192 msgid "workspace.plugins.remove-plugin" msgstr "Eliminar extensión" -#: src/app/main/ui/workspace/plugins.cljs:195 +#: src/app/main/ui/workspace/plugins.cljs:222 msgid "workspace.plugins.search-placeholder" msgstr "Intruduzca URL de la extensión" -#: src/app/main/ui/workspace/plugins.cljs:188 +#: src/app/main/ui/workspace/plugins.cljs:215 msgid "workspace.plugins.title" msgstr "Extensiones" -#: src/app/main/ui/workspace/plugins.cljs:494 +#: src/app/main/ui/workspace/plugins.cljs:522 msgid "workspace.plugins.try-out.cancel" msgstr "AHORA NO" -#: src/app/main/ui/workspace/plugins.cljs:487 +#: src/app/main/ui/workspace/plugins.cljs:515 msgid "workspace.plugins.try-out.message" msgstr "" "¿Quieres echar un vistazo?. Crearemos un nuevo borrador en tu equipo " "actual. (Si no, puedes encontrar los plugins instalados en cualquier " "fichero.)" -#: src/app/main/ui/workspace/plugins.cljs:483 +#: src/app/main/ui/workspace/plugins.cljs:511 msgid "workspace.plugins.try-out.title" msgstr "¡LA EXTENSIÓN '%s' HA SIDO INSTALADA PARA TU USUARIO!" -#: src/app/main/ui/workspace/plugins.cljs:500 +#: src/app/main/ui/workspace/plugins.cljs:528 msgid "workspace.plugins.try-out.try" msgstr "PROBAR PLUGIN" From 60679bbbcd26b35aae9f28558dd0cc565cd9e66f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bel=C3=A9n=20Albeza?= Date: Thu, 1 Oct 2026 18:29:07 +0200 Subject: [PATCH 3/4] :bug: Revert RTL auto-width text growing from its right edge (#12051) Revert #11775 (commit 142f3d9de8). We are putting this fix on hold until we settle the UX for RTL auto-width text. Relates to #11523 --- common/src/app/common/types/text.cljc | 8 - .../app/main/data/workspace/wasm_text.cljs | 9 +- .../shapes/text/text_edition_outline.cljs | 13 +- .../ui/workspace/shapes/text/v3_editor.cljs | 11 +- .../frontend_tests/data/wasm_text_test.cljs | 131 ----------- .../data/workspace_texts_test.cljs | 50 ----- frontend/test/frontend_tests/runner.cljs | 2 - render-wasm/src/math.rs | 58 +---- render-wasm/src/render/text_editor.rs | 149 ++----------- render-wasm/src/shapes/modifiers.rs | 9 +- render-wasm/src/shapes/text.rs | 207 +----------------- 11 files changed, 29 insertions(+), 618 deletions(-) delete mode 100644 frontend/test/frontend_tests/data/wasm_text_test.cljs diff --git a/common/src/app/common/types/text.cljc b/common/src/app/common/types/text.cljc index 82c4a20f82..4e53c9b753 100644 --- a/common/src/app/common/types/text.cljc +++ b/common/src/app/common/types/text.cljc @@ -209,14 +209,6 @@ [node] (= "root" (:type node))) -(defn rtl-content? - "True when the content has paragraphs and all of them are `\"rtl\"`; picks the - growth anchor of auto-width text. Mixed, \"none\" and empty content are ltr." - [content] - (boolean - (when-let [paragraphs (node-seq is-paragraph-node? content)] - (every? #(= "rtl" (:text-direction %)) paragraphs)))) - (defn is-node? [node] (or ^boolean (is-text-node? node) diff --git a/frontend/src/app/main/data/workspace/wasm_text.cljs b/frontend/src/app/main/data/workspace/wasm_text.cljs index ddd4f81400..435c8082c4 100644 --- a/frontend/src/app/main/data/workspace/wasm_text.cljs +++ b/frontend/src/app/main/data/workspace/wasm_text.cljs @@ -15,7 +15,6 @@ [app.common.geom.matrix :as gmt] [app.common.geom.point :as gpt] [app.common.types.modifiers :as ctm] - [app.common.types.text :as ctt] [app.main.data.helpers :as dsh] [app.main.data.workspace :as-alias dw] [app.main.data.workspace.modifiers :as dwm] @@ -64,16 +63,12 @@ ([shape] (resize-wasm-text-modifiers shape (:content shape))) - ([{:keys [id points selrect grow-type] :as shape} content] + ([{:keys [id points selrect] :as shape} content] (when-let [new-size (get-wasm-text-new-size shape content)] (let [width-scale (/ (:width new-size) (:width selrect)) height-scale (/ (:height new-size) (:height selrect)) resize-v (gpt/point width-scale height-scale) - ;; Rtl text grows leftward, so anchor it on the top-right. - origin (if (and (= :auto-width grow-type) - (ctt/rtl-content? content)) - (second points) - (first points))] + origin (first points)] {id {:modifiers (ctm/resize-modifiers diff --git a/frontend/src/app/main/ui/workspace/shapes/text/text_edition_outline.cljs b/frontend/src/app/main/ui/workspace/shapes/text/text_edition_outline.cljs index ca4c74632e..93e7562438 100644 --- a/frontend/src/app/main/ui/workspace/shapes/text/text_edition_outline.cljs +++ b/frontend/src/app/main/ui/workspace/shapes/text/text_edition_outline.cljs @@ -25,18 +25,11 @@ ;; finalize-only), so measure the live WASM text for the growing axes: ;; width grows on auto-width, height on auto-width/auto-height. grow-type (:grow-type shape) - {live-x :x live-width :width live-height :height} (wasm.api/get-text-dimensions (:id shape)) + {live-width :width live-height :height} (wasm.api/get-text-dimensions (:id shape)) sr-width (if (= grow-type :auto-width) live-width (:width selrect)) - sr-height (if (= grow-type :fixed) (:height selrect) live-height) - ;; Rtl auto-width text is anchored on its right edge, so use the origin - ;; wasm reports. A zero measurement means it has no layout yet. - sr-x (if (and (= grow-type :auto-width) - (some? live-x) - (pos? live-width)) - live-x - (:x selrect))] + sr-height (if (= grow-type :fixed) (:height selrect) live-height)] [:rect.main.viewport-selrect - {:x sr-x + {:x (:x selrect) :y (:y selrect) :width sr-width :height sr-height diff --git a/frontend/src/app/main/ui/workspace/shapes/text/v3_editor.cljs b/frontend/src/app/main/ui/workspace/shapes/text/v3_editor.cljs index 832f1f1b07..1c95406ef5 100644 --- a/frontend/src/app/main/ui/workspace/shapes/text/v3_editor.cljs +++ b/frontend/src/app/main/ui/workspace/shapes/text/v3_editor.cljs @@ -214,7 +214,7 @@ (font-family-from-font-id (:font-id font))) fallback-fonts) [{:keys [x y width height]} transform] - (let [{text-x :x :keys [width height]} (wasm.api/get-text-dimensions shape-id) + (let [{:keys [width height]} (wasm.api/get-text-dimensions shape-id) selrect-transform (mf/deref refs/workspace-selrect) vbox (mf/deref refs/vbox) [selrect transform] (dsh/get-selrect selrect-transform shape) @@ -231,20 +231,13 @@ overlay-width (if (= (:grow-type shape) :auto-width) (+ max-width viewport-width) max-width) - ;; `on-pointer-down` feeds offsets within this element to wasm as - ;; paragraph-local coords, so this edge must sit on the text's. - x (if (and (= (:grow-type shape) :auto-width) - (some? text-x) - (pos? width)) - text-x - (:x selrect)) valign (-> shape :content :vertical-align) y (:y selrect) y (case valign "bottom" (+ y (- selrect-height height)) "center" (+ y (/ (- selrect-height height) 2)) y)] - [(assoc selrect :x x :y y :width overlay-width :height max-height) transform]) + [(assoc selrect :y y :width overlay-width :height max-height) transform]) on-composition-start (mf/use-fn diff --git a/frontend/test/frontend_tests/data/wasm_text_test.cljs b/frontend/test/frontend_tests/data/wasm_text_test.cljs deleted file mode 100644 index 5624e40c04..0000000000 --- a/frontend/test/frontend_tests/data/wasm_text_test.cljs +++ /dev/null @@ -1,131 +0,0 @@ -;; This Source Code Form is subject to the terms of the Mozilla Public -;; License, v. 2.0. If a copy of the MPL was not distributed with this -;; file, You can obtain one at http://mozilla.org/MPL/2.0/. -;; -;; Copyright (c) KALEIDOS SUBSIDIARY SL - -(ns frontend-tests.data.wasm-text-test - "Growth anchor of auto-grow wasm text shapes (see `resize-wasm-text-modifiers`). - Tests stub the wasm bridge and assert on the shape the modifiers produce." - (:require - [app.common.geom.shapes :as gsh] - [app.common.types.modifiers :as ctm] - [app.common.types.shape :as cts] - [app.main.data.workspace.wasm-text :as dwwt] - [cljs.test :as t :include-macros true])) - -;; --------------------------------------------------------------------------- -;; Helpers -;; --------------------------------------------------------------------------- - -(defn- make-content - "Text content whose paragraphs carry the given `:text-direction` values." - [& directions] - {:type "root" - :children [{:type "paragraph-set" - :children (vec (for [direction directions] - {:type "paragraph" - :text-direction direction - :children [{:text "hello"}]}))}]}) - -(defn- make-text-shape - [& {:keys [x y width height grow-type rotation] - :or {x 100 y 50 width 60 height 20 grow-type :auto-width}}] - (let [shape (-> (cts/setup-shape {:type :text - :x x - :y y - :width width - :height height}) - (assoc :grow-type grow-type))] - (if (some? rotation) - (gsh/transform-shape - shape - (ctm/rotation-modifiers shape (gsh/shape->center shape) rotation)) - shape))) - -(defn- resized - "Shape that results from `resize-wasm-text-modifiers` when the renderer - measures `new-size`." - [shape content new-size] - ;; The stub needs the real fn's arities: a variadic `fn` has no `arity$2` - ;; dispatch, so the 2-arity call site blows up. - (with-redefs [dwwt/get-wasm-text-new-size (fn ([_] new-size) ([_ _] new-size))] - (let [modifiers (dwwt/resize-wasm-text-modifiers shape content)] - (gsh/transform-shape shape (get-in modifiers [(:id shape) :modifiers]))))) - -(defn- close? [a b] - (< (abs (- a b)) 0.01)) - -;; --------------------------------------------------------------------------- -;; Growth anchor -;; --------------------------------------------------------------------------- - -(t/deftest rtl-auto-width-keeps-its-right-edge-when-growing - (t/testing "an rtl auto-width shape extends leftward: the right and top edges - stay put and x moves left" - (let [shape (make-text-shape) - content (make-content "rtl") - before (:selrect shape) - after (:selrect (resized shape content {:width 120 :height 20}))] - (t/is (close? (+ (:x after) (:width after)) - (+ (:x before) (:width before))) - "right edge preserved") - (t/is (close? (:y after) (:y before)) "top edge preserved") - (t/is (close? (:x after) 40) "x moved left by the growth") - (t/is (close? (:width after) 120))))) - -(t/deftest rtl-auto-width-keeps-its-right-edge-when-shrinking - (t/testing "deleting text shrinks the box from the left, right edge preserved" - (let [shape (make-text-shape) - content (make-content "rtl") - before (:selrect shape) - after (:selrect (resized shape content {:width 30 :height 20}))] - (t/is (close? (+ (:x after) (:width after)) - (+ (:x before) (:width before))) - "right edge preserved") - (t/is (close? (:x after) 130) "x moved right as the box narrowed")))) - -(t/deftest ltr-auto-width-keeps-its-left-edge - (t/testing "ltr content is untouched: the left edge stays anchored" - (let [shape (make-text-shape) - content (make-content "ltr") - after (:selrect (resized shape content {:width 120 :height 20}))] - (t/is (close? (:x after) 100) "left edge preserved") - (t/is (close? (:width after) 120))))) - -(t/deftest mixed-direction-auto-width-keeps-its-left-edge - (t/testing "a box with one rtl and one ltr paragraph keeps the previous - left-anchored growth" - (let [shape (make-text-shape) - content (make-content "rtl" "ltr") - after (:selrect (resized shape content {:width 120 :height 20}))] - (t/is (close? (:x after) 100) "left edge preserved")))) - -(t/deftest rtl-auto-height-keeps-its-left-edge - (t/testing "auto-height only ever changes height, so the anchor is irrelevant - and x must not move" - (let [shape (make-text-shape :grow-type :auto-height) - content (make-content "rtl") - after (:selrect (resized shape content {:width 60 :height 80}))] - (t/is (close? (:x after) 100) "x preserved") - (t/is (close? (:width after) 60) "width preserved") - (t/is (close? (:height after) 80) "height grew")))) - -(t/deftest rotated-rtl-auto-width-grows-along-its-own-axis - (t/testing "a rotated rtl shape keeps its own top-right corner, not the - axis-aligned one" - (let [shape (make-text-shape :rotation 30) - content (make-content "rtl") - before (:points shape) - after (:points (resized shape content {:width 120 :height 20}))] - ;; points are [top-left top-right bottom-right bottom-left] - (t/is (close? (:x (second after)) (:x (second before))) - "top-right x preserved") - (t/is (close? (:y (second after)) (:y (second before))) - "top-right y preserved")))) - -(t/deftest no-modifiers-when-the-renderer-has-no-size - (t/testing "a nil measurement (shape absent from wasm state) skips the resize" - (let [shape (make-text-shape)] - (with-redefs [dwwt/get-wasm-text-new-size (fn ([_] nil) ([_ _] nil))] - (t/is (nil? (dwwt/resize-wasm-text-modifiers shape (make-content "rtl")))))))) diff --git a/frontend/test/frontend_tests/data/workspace_texts_test.cljs b/frontend/test/frontend_tests/data/workspace_texts_test.cljs index 6994f1a219..14eef12d50 100644 --- a/frontend/test/frontend_tests/data/workspace_texts_test.cljs +++ b/frontend/test/frontend_tests/data/workspace_texts_test.cljs @@ -581,53 +581,3 @@ (t/testing "a non-root content (e.g. paragraph) is left alone" (let [node {:type "paragraph" :children []}] (t/is (= node (dwt/ensure-valid-text-content node)))))) - -;; --------------------------------------------------------------------------- -;; txt/rtl-content? -;; --------------------------------------------------------------------------- - -(defn- make-content - "Text content whose paragraphs carry the given `:text-direction` values; a - `nil` entry leaves the attribute out entirely." - [& directions] - {:type "root" - :children [{:type "paragraph-set" - :children (vec (for [direction directions] - (cond-> {:type "paragraph" - :children [{:text "hello"}]} - (some? direction) - (assoc :text-direction direction))))}]}) - -(t/deftest rtl-content-single-rtl-paragraph - (t/testing "a lone rtl paragraph makes the content rtl" - (t/is (true? (txt/rtl-content? (make-content "rtl")))))) - -(t/deftest rtl-content-every-paragraph-rtl - (t/testing "several paragraphs, all rtl" - (t/is (true? (txt/rtl-content? (make-content "rtl" "rtl" "rtl")))))) - -(t/deftest rtl-content-mixed-directions - (t/testing "a mix of rtl and ltr is not rtl" - (t/is (false? (txt/rtl-content? (make-content "rtl" "ltr")))))) - -(t/deftest rtl-content-missing-direction-on-one-paragraph - (t/testing "a paragraph without :text-direction defaults to ltr, so the - content is not rtl" - (t/is (false? (txt/rtl-content? (make-content "rtl" nil)))))) - -(t/deftest rtl-content-all-ltr - (t/testing "all-ltr content is not rtl" - (t/is (false? (txt/rtl-content? (make-content "ltr" "ltr")))))) - -(t/deftest rtl-content-none-direction-is-ltr - (t/testing "\"none\" (the sidebar's un-toggled value) is ltr, matching what - translate-text-direction sends to the renderer" - (t/is (false? (txt/rtl-content? (make-content "none")))))) - -(t/deftest rtl-content-without-paragraphs - (t/testing "a root with no paragraph nodes is not rtl" - (t/is (false? (txt/rtl-content? {:type "root" :children []}))))) - -(t/deftest rtl-content-nil - (t/testing "nil content is not rtl and does not throw" - (t/is (false? (txt/rtl-content? nil))))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index 0937f7b732..1f78325a70 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -21,7 +21,6 @@ [frontend-tests.data.svg-upload-test] [frontend-tests.data.uploads-test] [frontend-tests.data.viewer-test] - [frontend-tests.data.wasm-text-test] [frontend-tests.data.workspace-colors-test] [frontend-tests.data.workspace-comments-test] [frontend-tests.data.workspace-context-menu-test] @@ -157,7 +156,6 @@ 'frontend-tests.data.svg-upload-test 'frontend-tests.data.uploads-test 'frontend-tests.data.viewer-test - 'frontend-tests.data.wasm-text-test 'frontend-tests.data.workspace-colors-test 'frontend-tests.data.workspace-comments-test 'frontend-tests.data.workspace-context-menu-test diff --git a/render-wasm/src/math.rs b/render-wasm/src/math.rs index de5d425fb5..0b8feeed04 100644 --- a/render-wasm/src/math.rs +++ b/render-wasm/src/math.rs @@ -434,26 +434,6 @@ pub fn resize_matrix( child_bounds: &Bounds, new_width: f32, new_height: f32, -) -> Matrix { - resize_matrix_from( - parent_bounds, - child_bounds, - new_width, - new_height, - child_bounds.nw, - ) -} - -/* - * Like `resize_matrix`, but scaling about an explicit corner instead of `nw`. - * Rtl auto-width text anchors on `ne` so it extends leftward as it grows. - */ -pub fn resize_matrix_from( - parent_bounds: &Bounds, - child_bounds: &Bounds, - new_width: f32, - new_height: f32, - anchor: Point, ) -> Matrix { let mut result = Matrix::default(); @@ -475,7 +455,7 @@ pub fn resize_matrix_from( parent_transform.pre_translate(-center); let parent_transform_inv = &parent_transform.invert().unwrap_or_default(); - let origin = parent_transform_inv.map_point(anchor); + let origin = parent_transform_inv.map_point(child_bounds.nw); let mut scale = Matrix::scale((scale_width, scale_height)); scale.post_translate(origin); @@ -617,40 +597,4 @@ mod tests { assert!((m.translate_x() - 0.0).abs() <= 0.1); assert!((m.translate_y() - 0.0).abs() <= 0.1); } - - fn axis_aligned_bounds(x: f32, y: f32, w: f32, h: f32) -> Bounds { - Bounds::new( - Point::new(x, y), - Point::new(x + w, y), - Point::new(x + w, y + h), - Point::new(x, y + h), - ) - } - - #[test] - fn resize_matrix_keeps_the_north_west_corner() { - let bounds = axis_aligned_bounds(100.0, 50.0, 60.0, 20.0); - let matrix = resize_matrix(&bounds, &bounds, 120.0, 20.0); - let resized = bounds.transform(&matrix); - assert!(is_close_to(resized.nw.x, 100.0)); - assert!(is_close_to(resized.ne.x, 220.0)); - } - - #[test] - fn resize_matrix_from_north_east_keeps_the_right_edge() { - let bounds = axis_aligned_bounds(100.0, 50.0, 60.0, 20.0); - let matrix = resize_matrix_from(&bounds, &bounds, 120.0, 20.0, bounds.ne); - let resized = bounds.transform(&matrix); - assert!(is_close_to(resized.ne.x, 160.0), "right edge preserved"); - assert!(is_close_to(resized.nw.x, 40.0), "grew leftward"); - assert!(is_close_to(resized.nw.y, 50.0), "top edge preserved"); - } - - #[test] - fn resize_matrix_from_north_west_matches_the_default() { - let bounds = axis_aligned_bounds(100.0, 50.0, 60.0, 20.0); - let default = resize_matrix(&bounds, &bounds, 120.0, 40.0); - let explicit = resize_matrix_from(&bounds, &bounds, 120.0, 40.0, bounds.nw); - assert_eq!(default, explicit); - } } diff --git a/render-wasm/src/render/text_editor.rs b/render-wasm/src/render/text_editor.rs index b894d9f233..d1825aeca8 100644 --- a/render-wasm/src/render/text_editor.rs +++ b/render-wasm/src/render/text_editor.rs @@ -2,25 +2,9 @@ use crate::render::options::RenderOptions; use crate::shapes::{vertical_align_offset, Shape, TextContent, Type}; use crate::state::{TextEditorState, TextSelection}; use crate::view::Viewbox; -use skia_safe::textlayout::{RectHeightStyle, RectWidthStyle, TextBox, TextDirection}; +use skia_safe::textlayout::{RectHeightStyle, RectWidthStyle}; use skia_safe::{BlendMode, Canvas, Color, Paint, Rect}; -/// Caret x where a character typed *before* this glyph would land. -fn leading_edge(text_box: &TextBox) -> f32 { - match text_box.direct { - TextDirection::RTL => text_box.rect.right(), - _ => text_box.rect.left(), - } -} - -/// Caret x where a character typed *after* this glyph would land. -fn trailing_edge(text_box: &TextBox) -> f32 { - match text_box.direct { - TextDirection::RTL => text_box.rect.left(), - _ => text_box.rect.right(), - } -} - pub fn render_overlay( canvas: &Canvas, viewbox: &Viewbox, @@ -128,14 +112,6 @@ fn render_selection( canvas.restore(); } -/// Caret and selection rects are drawn through `shape.get_matrix()`, which -/// translates by the *selrect's* `left_top()`; this shifts them onto the text. -fn paragraphs_horizontal_offset(shape: &Shape, text_content: &TextContent) -> f32 { - let selrect = shape.selrect(); - let width = text_content.get_width(selrect.width()); - text_content.layout_origin_x(&selrect, width) - selrect.x() -} - fn paragraphs_vertical_offset( shape: &Shape, layout_paragraphs: &[&skia_safe::textlayout::Paragraph], @@ -165,7 +141,6 @@ fn calculate_cursor_rect( return None; } - let x_offset = paragraphs_horizontal_offset(shape, text_content); let mut y_offset = paragraphs_vertical_offset(shape, &layout_paragraphs); for (idx, laid_out_para) in layout_paragraphs.iter().enumerate() { if idx == cursor.paragraph { @@ -183,13 +158,8 @@ fn calculate_cursor_rect( // Skia ranges are UTF-16 code units, not characters. let (cursor_x, cursor_y, cursor_width, cursor_height) = if para_char_count == 0 { - // No glyph to anchor to: sit where the first character will appear. - let empty_x = if para.text_direction() == TextDirection::RTL { - laid_out_para.max_width() - } else { - 0.0 - }; - (empty_x, 0.0, 1.0, laid_out_para.height()) + // Empty paragraph - use default height + (0.0, 0.0, 1.0, laid_out_para.height()) } else if char_pos == 0 { let rects = laid_out_para.get_rects_for_range( 0..para.char_utf16_len_at(0), @@ -198,7 +168,7 @@ fn calculate_cursor_rect( ); if !rects.is_empty() { let r = &rects[0].rect; - (leading_edge(&rects[0]), r.top(), r.width(), r.height()) + (r.left(), r.top(), r.width(), r.height()) } else { (0.0, 0.0, 1.0, laid_out_para.height()) } @@ -212,15 +182,14 @@ fn calculate_cursor_rect( ); if !rects.is_empty() { let r = &rects[0].rect; - (trailing_edge(&rects[0]), r.top(), r.width(), r.height()) + (r.right(), r.top(), r.width(), r.height()) } else if let Some(line) = laid_out_para.get_line_metrics().last() { - // No glyph box to measure: use the end of the line. - let line_end = if para.text_direction() == TextDirection::RTL { - line.left as f32 - } else { - line.left as f32 + line.width as f32 - }; - (line_end, 0.0, 1.0, laid_out_para.height()) + ( + line.left as f32 + line.width as f32, + 0.0, + 1.0, + laid_out_para.height(), + ) } else { (0.0, 0.0, 1.0, laid_out_para.height()) } @@ -233,7 +202,7 @@ fn calculate_cursor_rect( ); if !rects.is_empty() { let r = &rects[0].rect; - (leading_edge(&rects[0]), r.top(), r.width(), r.height()) + (r.left(), r.top(), r.width(), r.height()) } else { // Fallback: use glyph position let pos = laid_out_para.get_glyph_position_at_coordinate((0.0, 0.0)); @@ -242,7 +211,7 @@ fn calculate_cursor_rect( }; return Some(Rect::from_xywh( - x_offset + cursor_x, + cursor_x, y_offset + cursor_y, cursor_width, // cursor_width cursor_height, @@ -267,7 +236,6 @@ fn calculate_selection_rects( let paragraphs = text_content.paragraphs(); let layout_paragraphs: Vec<_> = text_content.layout.paragraphs.iter().flatten().collect(); - let x_offset = paragraphs_horizontal_offset(shape, text_content); let mut y_offset = paragraphs_vertical_offset(shape, &layout_paragraphs); for (para_idx, laid_out_para) in layout_paragraphs.iter().enumerate() { @@ -310,7 +278,7 @@ fn calculate_selection_rects( for text_box in text_boxes { let r = text_box.rect; rects.push(Rect::from_xywh( - x_offset + r.left(), + r.left(), y_offset + r.top(), r.width(), r.height(), @@ -323,92 +291,3 @@ fn calculate_selection_rects( rects } - -#[cfg(test)] -mod tests { - use super::*; - use crate::shapes::{FontFamily, FontStyle, GrowType, Paragraph, TextAlign, TextSpan}; - use crate::uuid::Uuid; - - fn rtl_span() -> TextSpan { - TextSpan::new( - "نام".to_string(), - FontFamily::new(Uuid::nil(), 400, FontStyle::Normal), - 14.0, - 1.2, - 0.0, - None, - None, - TextDirection::RTL, - 400, - Uuid::nil(), - vec![], - ) - } - - /// Auto-width rtl content measured wider than its stale selrect: mid-edit, - /// where the overlay origin and the selrect diverge. - fn grown_rtl_shape(measured_width: f32) -> Shape { - let selrect = Rect::from_xywh(100.0, 50.0, 60.0, 20.0); - let mut content = TextContent::new(selrect, GrowType::AutoWidth); - content.add_paragraph(Paragraph::new( - TextAlign::Right, - TextDirection::RTL, - None, - None, - 1.2, - 0.0, - vec![rtl_span()], - )); - content.size.width = measured_width; - content.size.height = 20.0; - - let mut shape = Shape::new(Uuid::nil()); - shape.set_selrect(selrect.left, selrect.top, selrect.right, selrect.bottom); - shape.set_shape_type(Type::Text(content)); - shape - } - - fn text_content_of(shape: &Shape) -> &TextContent { - match &shape.shape_type { - Type::Text(content) => content, - _ => unreachable!(), - } - } - - #[test] - fn horizontal_offset_shifts_the_overlay_onto_grown_rtl_text() { - let shape = grown_rtl_shape(120.0); - // right edge 160 - measured 120 = 40, i.e. 60 left of selrect.x - assert_eq!( - paragraphs_horizontal_offset(&shape, text_content_of(&shape)), - -60.0 - ); - } - - #[test] - fn horizontal_offset_is_zero_once_the_selrect_is_committed() { - let shape = grown_rtl_shape(60.0); - assert_eq!( - paragraphs_horizontal_offset(&shape, text_content_of(&shape)), - 0.0 - ); - } - - #[test] - fn leading_and_trailing_edges_follow_the_run_direction() { - let rect = Rect::from_ltrb(10.0, 0.0, 30.0, 12.0); - let ltr = TextBox { - rect, - direct: TextDirection::LTR, - }; - let rtl = TextBox { - rect, - direct: TextDirection::RTL, - }; - assert_eq!(leading_edge(<r), 10.0); - assert_eq!(trailing_edge(<r), 30.0); - assert_eq!(leading_edge(&rtl), 30.0); - assert_eq!(trailing_edge(&rtl), 10.0); - } -} diff --git a/render-wasm/src/shapes/modifiers.rs b/render-wasm/src/shapes/modifiers.rs index 1bb6b9d3cb..1898ed7276 100644 --- a/render-wasm/src/shapes/modifiers.rs +++ b/render-wasm/src/shapes/modifiers.rs @@ -320,18 +320,11 @@ fn propagate_transform( } } } - // Rtl text keeps its right edge, so scale about the ne corner. - let anchor = if text_content.is_rtl() { - shape_bounds_after.ne - } else { - shape_bounds_after.nw - }; - let resize_transform = math::resize_matrix_from( + let resize_transform = math::resize_matrix( &shape_bounds_after, &shape_bounds_after, new_width, new_height, - anchor, ); shape_bounds_after = shape_bounds_after.transform(&resize_transform); transform.post_concat(&resize_transform); diff --git a/render-wasm/src/shapes/text.rs b/render-wasm/src/shapes/text.rs index a13b4c02e7..8122ee21be 100644 --- a/render-wasm/src/shapes/text.rs +++ b/render-wasm/src/shapes/text.rs @@ -476,26 +476,18 @@ impl TextContent { /// [`cached_layout_paint_offset`] on the canvas so both move together. pub fn cached_layout_paint_anchor(&self, selrect: &Rect) -> Point { self.layout_paint_origin - .unwrap_or_else(|| Point::new(self.selrect_origin_x(selrect), selrect.y())) + .unwrap_or_else(|| Point::new(selrect.x(), selrect.y())) } /// Canvas translation from the baked paint origin to the current selrect. /// Zero when there is no recorded origin (fall back to painting at selrect). pub fn cached_layout_paint_offset(&self, selrect: &Rect) -> Point { match self.layout_paint_origin { - Some(origin) => Point::new( - self.selrect_origin_x(selrect) - origin.x, - selrect.y() - origin.y, - ), + Some(origin) => Point::new(selrect.x() - origin.x, selrect.y() - origin.y), None => Point::new(0.0, 0.0), } } - /// [`layout_origin_x`] for this content's own measured width. - fn selrect_origin_x(&self, selrect: &Rect) -> f32 { - self.layout_origin_x(selrect, self.size.width) - } - /// Text content for paint when [`Rect`] size may differ from stored bounds /// (e.g. modifier transform). Reuses `self` when width/height match; otherwise /// clones paragraphs into a rebound copy with an empty layout cache. @@ -576,26 +568,6 @@ impl TextContent { } } - /// Non-empty and every paragraph RTL. Mixed content counts as LTR, mirroring - /// `app.common.types.text/rtl-content?` on the ClojureScript side. - pub fn is_rtl(&self) -> bool { - !self.paragraphs.is_empty() - && self - .paragraphs - .iter() - .all(|paragraph| paragraph.text_direction() == TextDirection::RTL) - } - - /// Left edge of the laid-out text of `width` inside `selrect`. RTL auto-width - /// text anchors right so the box extends leftward as it grows. - pub fn layout_origin_x(&self, selrect: &Rect, width: f32) -> f32 { - if self.grow_type() == GrowType::AutoWidth && self.is_rtl() { - selrect.right() - width - } else { - selrect.x() - } - } - /// Compute a tight text rect from laid-out Skia paragraphs using glyph /// metrics (fm.top for overshoot, line descent for bottom, line left/width /// for horizontal extent). @@ -748,6 +720,7 @@ impl TextContent { } pub fn content_rect(&self, selrect: &Rect, valign: VerticalAlign) -> Rect { + let x = selrect.x(); let mut y = selrect.y(); let width = if self.grow_type() == GrowType::AutoWidth { @@ -756,8 +729,6 @@ impl TextContent { selrect.width() }; - let x = self.layout_origin_x(selrect, width); - let height = if self.size.width.round() != width.round() { self.get_height(width) } else { @@ -1182,8 +1153,8 @@ impl TextContent { self.layout.set(result.0, result.1); self.size .copy_finite_size(result.2, default_width, default_height); - // Absolute image/gradient shaders are built against `self.bounds()`, so the - // origin matches them and `cached_layout_paint_offset` carries any rtl shift. + // Paragraph paints (incl. absolute image/gradient shaders) were built + // against `self.bounds()` in `paragraph_builder_group_from_text`. self.layout_paint_origin = Some(Point::new(self.bounds.x(), self.bounds.y())); } @@ -1853,7 +1824,7 @@ pub fn calculate_text_layout_data( let selrect_width = shape.selrect().width(); let text_width = text_content.get_width(selrect_width); let selrect_height = shape.selrect().height(); - let x = text_content.layout_origin_x(&shape.selrect, text_width); + let x = shape.selrect.x(); let base_y = shape.selrect.y(); let mut position_data: Vec = Vec::new(); let mut previous_line_height = text_content.normalized_line_height(); @@ -2412,170 +2383,4 @@ mod tests { layout.clear(); assert!(layout.needs_update()); } - - // ----------------------------------------------------------------------- - // RTL auto-width growth anchor - // ----------------------------------------------------------------------- - - fn directed_paragraph(direction: TextDirection) -> Paragraph { - let span = TextSpan::new( - "hello".to_string(), - FontFamily::new(Uuid::nil(), 400, crate::shapes::FontStyle::Normal), - 14.0, - 1.2, - 0.0, - None, - None, - direction, - 400, - Uuid::nil(), - vec![], - ); - Paragraph::new(TextAlign::Left, direction, None, None, 1.2, 0.0, vec![span]) - } - - /// Auto-width content measured wider than its stale selrect: mid-edit, where - /// the anchor is observable. - fn grown_content(directions: &[TextDirection], measured_width: f32) -> TextContent { - let bounds = Rect::from_xywh(100.0, 50.0, 60.0, 20.0); - let mut content = TextContent::new(bounds, GrowType::AutoWidth); - for direction in directions { - content.add_paragraph(directed_paragraph(*direction)); - } - content.size.width = measured_width; - content.size.height = 20.0; - content - } - - #[test] - fn is_rtl_false_when_no_paragraphs() { - let content = TextContent::new(Rect::from_xywh(0.0, 0.0, 10.0, 10.0), GrowType::AutoWidth); - assert!(!content.is_rtl()); - } - - #[test] - fn is_rtl_true_when_every_paragraph_is_rtl() { - let content = grown_content(&[TextDirection::RTL, TextDirection::RTL], 120.0); - assert!(content.is_rtl()); - } - - #[test] - fn is_rtl_false_when_directions_are_mixed() { - let content = grown_content(&[TextDirection::RTL, TextDirection::LTR], 120.0); - assert!(!content.is_rtl()); - } - - #[test] - fn is_rtl_false_when_every_paragraph_is_ltr() { - let content = grown_content(&[TextDirection::LTR], 120.0); - assert!(!content.is_rtl()); - } - - #[test] - fn layout_origin_x_right_anchors_rtl_auto_width() { - let content = grown_content(&[TextDirection::RTL], 120.0); - let selrect = Rect::from_xywh(100.0, 50.0, 60.0, 20.0); - // right edge (160) minus the measured width (120) - assert_eq!(content.layout_origin_x(&selrect, 120.0), 40.0); - } - - #[test] - fn layout_origin_x_left_anchors_ltr_auto_width() { - let content = grown_content(&[TextDirection::LTR], 120.0); - let selrect = Rect::from_xywh(100.0, 50.0, 60.0, 20.0); - assert_eq!(content.layout_origin_x(&selrect, 120.0), 100.0); - } - - #[test] - fn layout_origin_x_left_anchors_mixed_direction_auto_width() { - let content = grown_content(&[TextDirection::RTL, TextDirection::LTR], 120.0); - let selrect = Rect::from_xywh(100.0, 50.0, 60.0, 20.0); - assert_eq!(content.layout_origin_x(&selrect, 120.0), 100.0); - } - - #[test] - fn layout_origin_x_ignores_direction_for_fixed_and_auto_height() { - let selrect = Rect::from_xywh(100.0, 50.0, 60.0, 20.0); - for grow_type in [GrowType::Fixed, GrowType::AutoHeight] { - let mut content = grown_content(&[TextDirection::RTL], 120.0); - content.set_grow_type(grow_type); - assert_eq!(content.layout_origin_x(&selrect, 120.0), 100.0); - } - } - - #[test] - fn layout_origin_x_is_selrect_x_once_the_selrect_is_committed() { - let content = grown_content(&[TextDirection::RTL], 60.0); - let selrect = Rect::from_xywh(100.0, 50.0, 60.0, 20.0); - assert_eq!(content.layout_origin_x(&selrect, 60.0), selrect.x()); - } - - #[test] - fn content_rect_right_anchors_grown_rtl_auto_width() { - let content = grown_content(&[TextDirection::RTL], 120.0); - let selrect = Rect::from_xywh(100.0, 50.0, 60.0, 20.0); - let rect = content.content_rect(&selrect, VerticalAlign::Top); - assert_eq!(rect.x(), 40.0); - assert_eq!(rect.right(), selrect.right()); - assert_eq!(rect.width(), 120.0); - assert_eq!(rect.y(), selrect.y()); - } - - #[test] - fn content_rect_left_anchors_grown_ltr_auto_width() { - let content = grown_content(&[TextDirection::LTR], 120.0); - let selrect = Rect::from_xywh(100.0, 50.0, 60.0, 20.0); - let rect = content.content_rect(&selrect, VerticalAlign::Top); - assert_eq!(rect.x(), selrect.x()); - assert_eq!(rect.width(), 120.0); - } - - #[test] - fn content_rect_unchanged_for_fixed_rtl_content() { - // A mismatch sends `content_rect` into `get_height`, which needs global - // font state tests do not have. - let mut content = grown_content(&[TextDirection::RTL], 60.0); - content.set_grow_type(GrowType::Fixed); - let selrect = Rect::from_xywh(100.0, 50.0, 60.0, 20.0); - let rect = content.content_rect(&selrect, VerticalAlign::Top); - assert_eq!(rect.x(), selrect.x()); - assert_eq!(rect.width(), selrect.width()); - } - - #[test] - fn cached_paint_anchor_plus_offset_lands_on_the_layout_origin() { - let mut content = grown_content(&[TextDirection::RTL], 120.0); - content.layout_paint_origin = Some(Point::new(100.0, 50.0)); - let selrect = Rect::from_xywh(100.0, 50.0, 60.0, 20.0); - let anchor = content.cached_layout_paint_anchor(&selrect); - let offset = content.cached_layout_paint_offset(&selrect); - assert_eq!( - anchor.x + offset.x, - content.layout_origin_x(&selrect, 120.0) - ); - } - - #[test] - fn cached_paint_offset_stays_a_pure_translation_when_the_shape_moves() { - let mut content = grown_content(&[TextDirection::RTL], 120.0); - content.layout_paint_origin = Some(Point::new(100.0, 50.0)); - let before = Rect::from_xywh(100.0, 50.0, 60.0, 20.0); - let after = Rect::from_xywh(130.0, 70.0, 60.0, 20.0); - let moved_by = Point::new( - content.cached_layout_paint_offset(&after).x - - content.cached_layout_paint_offset(&before).x, - content.cached_layout_paint_offset(&after).y - - content.cached_layout_paint_offset(&before).y, - ); - assert_eq!(moved_by, Point::new(30.0, 20.0)); - } - - #[test] - fn cached_paint_anchor_right_anchors_when_no_origin_was_baked() { - let content = grown_content(&[TextDirection::RTL], 120.0); - let selrect = Rect::from_xywh(100.0, 50.0, 60.0, 20.0); - assert_eq!(content.layout_paint_origin, None); - assert_eq!(content.cached_layout_paint_anchor(&selrect).x, 40.0); - assert_eq!(content.cached_layout_paint_offset(&selrect).x, 0.0); - } } From 3f610ad793815de401354b2175d0700d2bf8d88e Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 1 Oct 2026 19:22:01 +0200 Subject: [PATCH 4/4] :sparkles: Isolate backend-test database per devenv instance (#12048) * :sparkles: Isolate backend-test database per devenv instance Each wsN now gets its own backend-test database (penpot_test_wsN) and Valkey DB (6+N) on the shared infra, so parallel test runs no longer wipe each other. The main database stays shared on purpose. manage.sh creates the test database on bring-up and passes PENPOT_TEST_* into the main container. Test helpers already read those vars, so no runtime code changes. scripts/psql and db-schema gain --ws. AI-assisted-by: muse-spark-1.3-contributor-free * :bug: Fix per-instance test database creation on bring-up psql -c does not reliably mix SQL with psql-only commands, so the CREATE DATABASE ... WHERE NOT EXISTS ... \gexec one-liner never ran. Use two plain SQL round-trips instead (check pg_database, then CREATE DATABASE) and print what happens, so a future failure shows up in the bring-up output. AI-assisted-by: muse-spark-1.3-contributor-free --- .serena/memories/devenv/core.md | 5 +- .serena/memories/scripts/psql.md | 9 +++- backend/test/backend_tests/helpers.clj | 7 +++ docker/devenv/docker-compose.main.yml | 5 ++ manage.sh | 73 ++++++++++++++++++++++++++ scripts/db-schema | 31 +++++++++++ scripts/psql | 31 +++++++++++ 7 files changed, 158 insertions(+), 3 deletions(-) diff --git a/.serena/memories/devenv/core.md b/.serena/memories/devenv/core.md index ddc83b1845..7bd436221a 100644 --- a/.serena/memories/devenv/core.md +++ b/.serena/memories/devenv/core.md @@ -1,6 +1,6 @@ # Devenv startup and configuration -Compose-based dev environment under `docker/devenv/`, driven by `manage.sh`. Parallel instances share infra + Postgres + RustFS; each instance has its own `main` container, Valkey, source checkout, tmux session. +Compose-based dev environment under `docker/devenv/`, driven by `manage.sh`. Parallel instances share infra + Postgres + RustFS; each instance has its own `main` container, Valkey, source checkout, tmux session. The main database (`penpot`) and the S3 bucket (`penpot`) are shared by all instances on purpose; backend-test databases are isolated per instance (`penpot_test` on ws0, `penpot_test_wsN` on wsN) with per-instance Valkey DB numbers (`6+N`, since runtime backends occupy `0..5`). ## Compose project layout @@ -12,7 +12,8 @@ Compose-based dev environment under `docker/devenv/`, driven by `manage.sh`. Par ## Source-of-truth files - `docker/devenv/defaults.env`: ws0 baseline — container/volume names, runtime env, published host ports, tmux defaults. `manage.sh` aborts if unreadable. -- For ws1+, `instance-env-overrides` computes the per-instance overrides (container/volume names, host ports offset `10000·N`, `PENPOT_PUBLIC_URI`, `PENPOT_REDIS_URI`, `PENPOT_BACKEND_WORKER=false`) and `instance-compose` injects them as env vars at compose time — never written to disk, recomputed each call so they can't drift. ws0 uses `defaults.env` as-is. +- For ws1+, `instance-env-overrides` computes the per-instance overrides (container/volume names, host ports offset `10000·N`, `PENPOT_PUBLIC_URI`, `PENPOT_REDIS_URI`, `PENPOT_TEST_DATABASE_URI`, `PENPOT_TEST_REDIS_URI`) and `instance-compose` injects them as env vars at compose time — never written to disk, recomputed each call so they can't drift. ws0 uses `defaults.env` as-is. +- Per-instance test databases are created by `ensure-instance-test-database` (called from `start-instance`): `CREATE DATABASE` guarded by a `pg_database` existence check, so it is idempotent. Needed because `postgresql_init.sql` only runs on first volume creation. The backend applies migrations itself on first use, so an empty database is enough. - `backend/scripts/_env`: backend-internal only — secret keys, `PENPOT_FLAGS` (with `enable-backend-worker` gated on `PENPOT_BACKEND_WORKER`), `JAVA_OPTS`, `setup_s3_bucket()`. Never duplicates `defaults.env`. - Compose files use pure `${VAR}` substitution; missing var = compose fails. diff --git a/.serena/memories/scripts/psql.md b/.serena/memories/scripts/psql.md index 810905f7c6..8b527cb0c6 100644 --- a/.serena/memories/scripts/psql.md +++ b/.serena/memories/scripts/psql.md @@ -16,13 +16,20 @@ development. # Default connection (penpot db, localhost) scripts/psql -c "SELECT version();" -# Test database +# Test database (ws0) scripts/psql --test -c "SELECT * FROM migrations;" +# Isolated test database of another instance (ws1+) +scripts/psql --test --ws 1 -c "SELECT * FROM migrations;" + # Custom host/user/database scripts/psql --host myhost --user myuser --db mydb ``` +`--ws N` only applies together with `--test` (the main database is +shared by all instances): it selects `penpot_test` on `--ws 0` and +`penpot_test_wsN` on `--ws N`. `scripts/db-schema` accepts the same flags. + `scripts/psql` must be invoked from the repo root so the path resolves. ## Native Tool Available (OpenCode V2) diff --git a/backend/test/backend_tests/helpers.clj b/backend/test/backend_tests/helpers.clj index b41bde7770..1ee446af6a 100644 --- a/backend/test/backend_tests/helpers.clj +++ b/backend/test/backend_tests/helpers.clj @@ -60,6 +60,13 @@ (def ^:dynamic *system* nil) (def ^:dynamic *pool* nil) +;; Fallback values used when no PENPOT_TEST_* env vars are set (e.g. CI sets +;; them explicitly, see .github/workflows/tests-backend.yml). Inside the +;; devenv each wsN container receives per-instance values via +;; manage.sh (PENPOT_TEST_DATABASE_URI=postgresql://postgres/penpot_test_wsN, +;; PENPOT_TEST_REDIS_URI=redis://valkey/<6+N>), so parallel test runs on +;; different workspaces never share a database. cf/read-config picks the env +;; vars up through the "penpot-test" prefix. (def default {:database-uri "postgresql://postgres/penpot_test" :redis-uri "redis://valkey/1" diff --git a/docker/devenv/docker-compose.main.yml b/docker/devenv/docker-compose.main.yml index e593ad1ba7..491c76b729 100644 --- a/docker/devenv/docker-compose.main.yml +++ b/docker/devenv/docker-compose.main.yml @@ -81,6 +81,11 @@ services: - PENPOT_DATABASE_PASSWORD=${PENPOT_DATABASE_PASSWORD} - PENPOT_DATABASE_MAX_POOL_SIZE=${PENPOT_DATABASE_MAX_POOL_SIZE} - PENPOT_REDIS_URI=${PENPOT_REDIS_URI} + # Per-instance backend-test config. Values come from + # instance-env-overrides in manage.sh; the main database above stays + # shared by all instances on purpose. + - PENPOT_TEST_DATABASE_URI=${PENPOT_TEST_DATABASE_URI} + - PENPOT_TEST_REDIS_URI=${PENPOT_TEST_REDIS_URI} - PENPOT_OBJECTS_STORAGE_BACKEND=${PENPOT_OBJECTS_STORAGE_BACKEND} - PENPOT_OBJECTS_STORAGE_S3_ENDPOINT=${PENPOT_OBJECTS_STORAGE_S3_ENDPOINT} - PENPOT_OBJECTS_STORAGE_S3_BUCKET=${PENPOT_OBJECTS_STORAGE_S3_BUCKET} diff --git a/manage.sh b/manage.sh index 0cbc4e7cd7..408792d781 100755 --- a/manage.sh +++ b/manage.sh @@ -94,6 +94,9 @@ set -e # instance-compose wrap 'docker compose' for one instance's main # project, injecting that instance's overrides # instance-env-overrides the per-instance KEY=VALUE overrides +# instance-test-db-name test database name for one instance +# instance-test-redis-db Valkey DB number for one instance's tests +# ensure-instance-test-database create one instance's test database # devenv-main-container resolve the 'main' container id via compose ps # devenv-main-running true if 'main' is up # @@ -301,6 +304,43 @@ function ensure-infra-up { infra-compose up -d --wait --wait-timeout 60 --remove-orphans } +# Create the backend-test database for on the shared Postgres +# if it does not exist yet. Idempotent: safe to run on every bring-up. +# Needed because postgresql_init.sql only runs when the Postgres volume is +# created for the first time, so per-instance test databases (which did not +# exist back then) would otherwise never be created on existing volumes. +# The backend applies migrations itself on first use, so an empty database +# is enough here. (Two plain SQL round-trips on purpose: psql -c does not +# reliably mix SQL with psql-only commands like \gexec.) +function ensure-instance-test-database { + local instance="$1" + local dbname + dbname=$(instance-test-db-name "$instance") + + local pg_container + pg_container=$(infra-compose ps -q postgres 2>/dev/null) + if [[ -z "$pg_container" ]]; then + echo "[${instance}] postgres container not found; is shared infra up?" >&2 + return 1 + fi + + local -a psql_base=(docker exec + -e "PGPASSWORD=${PENPOT_DATABASE_PASSWORD:-penpot}" + "$pg_container" + psql -h 127.0.0.1 -U "${PENPOT_DATABASE_USERNAME:-penpot}" + -d postgres -v ON_ERROR_STOP=1 -tA) + + local exists + exists=$("${psql_base[@]}" -c "SELECT 1 FROM pg_database WHERE datname = '${dbname}'") + if [[ "$exists" == "1" ]]; then + echo "[${instance}] test database ${dbname} already exists." + return 0 + fi + + echo "[${instance}] creating test database ${dbname} ..." + "${psql_base[@]}" -c "CREATE DATABASE \"${dbname}\"" +} + # Refuse to sync workspaces if the live repo is in a fragile Git state. # Copying a partial rebase/merge/cherry-pick into all workspaces would leave # every instance in the same broken state. @@ -333,6 +373,33 @@ function instance-port { echo $(( base + n * PENPOT_INSTANCE_PORT_STRIDE )) } +# Echo the backend-test database name for . ws0 keeps the +# historical name so existing flows stay untouched; ws1+ get an isolated +# database each on the shared Postgres server. The main database stays +# shared by all instances on purpose. +function instance-test-db-name { + local instance="$1" + local n=0 + [[ "$instance" =~ ^ws([0-9]+)$ ]] && n="${BASH_REMATCH[1]}" + if (( n == 0 )); then + echo "penpot_test" + else + echo "penpot_test_ws${n}" + fi +} + +# Echo the Valkey DB number reserved for the backend tests of . +# Runtime backends already occupy 0..PENPOT_MAX_WS_INDEX on the shared +# Valkey, so test DBs start right after to never share a DB with a +# running backend. (Resolved at call time; PENPOT_MAX_WS_INDEX is defined +# further below but always set before any call.) +function instance-test-redis-db { + local instance="$1" + local n=0 + [[ "$instance" =~ ^ws([0-9]+)$ ]] && n="${BASH_REMATCH[1]}" + echo $(( PENPOT_MAX_WS_INDEX + 1 + n )) +} + # Echo the per-instance Compose variable overrides for a workspace, one # KEY=VALUE per line, for instance-compose to inject into its `env -i` line. # Compose gives shell-env precedence over --env-file, so these override the @@ -358,11 +425,15 @@ function instance-env-overrides { opencode=$(instance-port "$instance" "$PENPOT_PORT_BASE_OPENCODE") mdts=$(instance-port "$instance" "$PENPOT_PORT_BASE_MDTS") storybook=$(instance-port "$instance" "$PENPOT_PORT_BASE_STORYBOOK") + test_db=$(instance-test-db-name "$instance") + test_redis_db=$(instance-test-redis-db "$instance") printf '%s\n' \ "PENPOT_MAIN_CONTAINER_NAME=penpot-devenv-${instance}-main" \ "PENPOT_USER_DATA_VOLUME=penpotdev_${instance}_user_data" \ "PENPOT_PUBLIC_URI=https://localhost:${public_https}" \ "PENPOT_REDIS_URI=redis://valkey/${n}" \ + "PENPOT_TEST_DATABASE_URI=postgresql://postgres/${test_db}" \ + "PENPOT_TEST_REDIS_URI=redis://valkey/${test_redis_db}" \ "PENPOT_PUBLIC_HTTPS_PORT=${public_https}" \ "PENPOT_PUBLIC_HTTP_PORT=${public}" \ "PENPOT_MCP_SERVER_PORT=${mcp}" \ @@ -709,6 +780,8 @@ function start-instance { local git_user_email="${4:-}" local agentic="${5:-true}" + ensure-instance-test-database "$instance" + instance-compose "$instance" up -d main # Wait briefly for main to be reachable; the tmux session lives inside. diff --git a/scripts/db-schema b/scripts/db-schema index 6cf2a8c710..3557cc859e 100755 --- a/scripts/db-schema +++ b/scripts/db-schema @@ -5,11 +5,22 @@ HOST="${PENPOT_DB_HOST:-postgres}" USER="${PENPOT_DB_USER:-penpot}" PASSWORD="${PENPOT_DB_PASSWORD:-penpot}" DB="${PENPOT_DB_NAME:-penpot}" +TEST=false +WS="" while [[ $# -gt 0 ]]; do case "$1" in --test) DB="penpot_test" + TEST=true + shift + ;; + --ws) + WS="$2" + shift 2 + ;; + --ws*) + WS="${1#--ws}" shift ;; --host|-h) @@ -30,5 +41,25 @@ while [[ $# -gt 0 ]]; do esac done +# --ws selects one instance's isolated backend-test database +# (penpot_test on ws0, penpot_test_wsN on wsN). It only applies together +# with --test because the main database is shared by all instances. +if [[ -n "$WS" ]]; then + if [[ "$WS" =~ ^ws([0-9]+)$ ]]; then + WS="${BASH_REMATCH[1]}" + fi + if [[ ! "$WS" =~ ^[0-9]+$ ]]; then + echo "Invalid --ws value: '$WS' (expected a non-negative integer, e.g. --ws 1)" >&2 + exit 1 + fi + if [[ "$TEST" != "true" ]]; then + echo "--ws only applies together with --test (the main database is shared by all instances)." >&2 + exit 1 + fi + if [[ "$WS" != "0" ]]; then + DB="penpot_test_ws${WS}" + fi +fi + export PGPASSWORD="$PASSWORD" exec pg_dump -h "$HOST" -U "$USER" -d "$DB" --schema-only --no-owner --no-privileges "$@" diff --git a/scripts/psql b/scripts/psql index 03dee3be19..3efac9c591 100755 --- a/scripts/psql +++ b/scripts/psql @@ -5,11 +5,22 @@ HOST="${PENPOT_DB_HOST:-postgres}" USER="${PENPOT_DB_USER:-penpot}" PASSWORD="${PENPOT_DB_PASSWORD:-penpot}" DB="${PENPOT_DB_NAME:-penpot}" +TEST=false +WS="" while [[ $# -gt 0 ]]; do case "$1" in --test) DB="penpot_test" + TEST=true + shift + ;; + --ws) + WS="$2" + shift 2 + ;; + --ws*) + WS="${1#--ws}" shift ;; --host|-h) @@ -30,5 +41,25 @@ while [[ $# -gt 0 ]]; do esac done +# --ws selects one instance's isolated backend-test database +# (penpot_test on ws0, penpot_test_wsN on wsN). It only applies together +# with --test because the main database is shared by all instances. +if [[ -n "$WS" ]]; then + if [[ "$WS" =~ ^ws([0-9]+)$ ]]; then + WS="${BASH_REMATCH[1]}" + fi + if [[ ! "$WS" =~ ^[0-9]+$ ]]; then + echo "Invalid --ws value: '$WS' (expected a non-negative integer, e.g. --ws 1)" >&2 + exit 1 + fi + if [[ "$TEST" != "true" ]]; then + echo "--ws only applies together with --test (the main database is shared by all instances)." >&2 + exit 1 + fi + if [[ "$WS" != "0" ]]; then + DB="penpot_test_ws${WS}" + fi +fi + export PGPASSWORD="$PASSWORD" exec psql -h "$HOST" -U "$USER" -d "$DB" "$@"