From fadb3124a0051b76a47a5025f2a3f7d1dea2fc7b Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 30 Jul 2026 12:49:00 +0200 Subject: [PATCH 1/8] :bug: Clamp gradient stop offsets to valid range (#10881) * :bug: Clamp gradient stop offsets to valid [0, 1] range Fixed a bug where gradient stop offsets outside the valid [0, 1] range were being sent to the server, causing schema validation errors ('invalid shape found'). Changes: - Viewport gradient handler: clamp offset in points-on-pointer-down before creating new stops - Colorpicker gradient preview: clamp offset in handle-preview-down before adding stops - Data layer: clamp offset parameter in update-colorpicker-add-stop and all stop offsets in update-colorpicker-stops; added app.common.math require All clamping follows the existing pattern used in handle-marker-pointer-move. AI-assisted-by: deepseek-v4-flash * :lipstick: Fix formatting in gradient handlers Fix cljfmt formatting issues in gradient handler functions. AI-assisted-by: qwen3.7-plus --- frontend/src/app/main/data/workspace/colors.cljs | 7 +++++-- frontend/src/app/main/store.cljs | 2 +- .../src/app/main/ui/workspace/colorpicker/gradients.cljs | 1 + frontend/src/app/main/ui/workspace/viewport/gradients.cljs | 1 + 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/frontend/src/app/main/data/workspace/colors.cljs b/frontend/src/app/main/data/workspace/colors.cljs index c145a203b7..c137936650 100644 --- a/frontend/src/app/main/data/workspace/colors.cljs +++ b/frontend/src/app/main/data/workspace/colors.cljs @@ -9,6 +9,7 @@ [app.common.data :as d] [app.common.data.macros :as dm] [app.common.files.helpers :as cfh] + [app.common.math :as mth] [app.common.schema :as sm] [app.common.types.color :as clr] [app.common.types.fills :as types.fills] @@ -950,7 +951,8 @@ (or (not cap-stops?) (< (count stops) types.fills/MAX-GRADIENT-STOPS))] (if can-add-stop? - (let [new-stop (-> (clr/interpolate-gradient stops offset) + (let [offset (mth/clamp offset 0 1) + new-stop (-> (clr/interpolate-gradient stops offset) (split-color-components)) stops (conj stops new-stop) stops (into [] (sort-by :offset stops)) @@ -973,7 +975,8 @@ stops (mapv split-color-components (if cap-stops? (take types.fills/MAX-GRADIENT-STOPS stops) - stops))] + stops)) + stops (mapv #(update % :offset (fn [o] (mth/clamp o 0 1))) stops)] (-> state (assoc :current-color (get stops stop)) (assoc :stops stops)))))))) diff --git a/frontend/src/app/main/store.cljs b/frontend/src/app/main/store.cljs index dc389033ae..8bdff34a65 100644 --- a/frontend/src/app/main/store.cljs +++ b/frontend/src/app/main/store.cljs @@ -121,7 +121,7 @@ delta (if prev-t (str "(+" (ct/diff-ms prev-t t) "ms)") "(+0ms)") - delta-pad (str/pad delta {:length 10 :type :right})] + delta-pad (str/pad delta {:length 10 :type :right})] (recur t (next xs) (conj! out (str iso " " delta-pad " " name)))) diff --git a/frontend/src/app/main/ui/workspace/colorpicker/gradients.cljs b/frontend/src/app/main/ui/workspace/colorpicker/gradients.cljs index 7010e84371..93648bc80d 100644 --- a/frontend/src/app/main/ui/workspace/colorpicker/gradients.cljs +++ b/frontend/src/app/main/ui/workspace/colorpicker/gradients.cljs @@ -233,6 +233,7 @@ (mf/deps on-add-stop-preview) (fn [^js e] (let [offset (-> (event->offset e) + (mth/clamp 0 1) (mth/precision 2))] (when on-add-stop-preview (on-add-stop-preview offset))))) diff --git a/frontend/src/app/main/ui/workspace/viewport/gradients.cljs b/frontend/src/app/main/ui/workspace/viewport/gradients.cljs index bc16b89045..c135d11e7e 100644 --- a/frontend/src/app/main/ui/workspace/viewport/gradients.cljs +++ b/frontend/src/app/main/ui/workspace/viewport/gradients.cljs @@ -189,6 +189,7 @@ lv (-> (gpt/to-vec from-p to-p) (gpt/unit)) nv (gpt/normal-left lv) offset (-> (gsp/project-t position [from-p to-p] nv) + (mth/clamp 0 1) (mth/precision 2)) new-stop (cc/interpolate-gradient stops offset) stops (conj stops new-stop) From 040080749b94bb3ba466d937ce3eefb7697b8432 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 30 Jul 2026 12:49:19 +0200 Subject: [PATCH 2/8] :bug: Fix shape export failures when export name is nil or empty (#10852) Use cuerdas blank-name handling directly when normalizing frontend export payloads. Replace nil or blank export names with the object-id string in request-simple-export, request-multiple-export, clipboard export, and plugin direct export payloads so that the backend always receives a valid name. Add focused frontend tests for nil/blank name normalization and normalized request params. AI-assisted-by: nex-n2-pro --- .../src/app/main/data/exports/assets.cljs | 187 ++++++++++-------- .../app/main/data/workspace/clipboard.cljs | 21 +- frontend/src/app/plugins/shape.cljs | 15 +- .../data/exports_assets_test.cljs | 99 ++++++++++ frontend/test/frontend_tests/runner.cljs | 2 + 5 files changed, 223 insertions(+), 101 deletions(-) create mode 100644 frontend/test/frontend_tests/data/exports_assets_test.cljs diff --git a/frontend/src/app/main/data/exports/assets.cljs b/frontend/src/app/main/data/exports/assets.cljs index 24d72e5e92..b3adaebade 100644 --- a/frontend/src/app/main/data/exports/assets.cljs +++ b/frontend/src/app/main/data/exports/assets.cljs @@ -20,10 +20,27 @@ [app.util.dom :as dom] [app.util.websocket :as ws] [beicon.v2.core :as rx] + [cuerdas.core :as str] [potok.v2.core :as ptk])) (def default-timeout 5000) +(defn normalize-export + [{:keys [object-id name] :as export}] + (assoc export :name (if (str/blank? name) + (str object-id) + name))) + +(defn- normalize-exports + [exports] + (mapv normalize-export exports)) + +(defn- normalize-export-shapes-params + [{:keys [exports] :as params}] + (cond-> params + (seq exports) + (assoc :exports (normalize-exports exports)))) + (defn toggle-detail-visibililty [] (ptk/reify ::toggle-detail-visibililty @@ -181,104 +198,106 @@ (defn request-simple-export [{:keys [export]}] - (ptk/reify ::request-simple-export - ptk/UpdateEvent - (update [_ state] - (cond-> state - (not (use-wasm-export? state export)) - (update :export assoc :in-progress true :id uuid/zero))) + (let [export (normalize-export export)] + (ptk/reify ::request-simple-export + ptk/UpdateEvent + (update [_ state] + (cond-> state + (not (use-wasm-export? state export)) + (update :export assoc :in-progress true :id uuid/zero))) - ptk/WatchEvent - (watch [_ state _] - (if (use-wasm-export? state export) - (do - (case (:type export) - :pdf (wasm.exports/export-pdf export) - (wasm.exports/export-image export)) - (rx/empty)) - (let [profile-id (:profile-id state) - params {:exports [export] - :profile-id profile-id - :cmd :export-shapes - :wait true - :is-wasm (wasm-export-enabled? state)}] - (rx/concat - (dwp/force-persist-and-wait 400) + ptk/WatchEvent + (watch [_ state _] + (if (use-wasm-export? state export) + (do + (case (:type export) + :pdf (wasm.exports/export-pdf export) + (wasm.exports/export-image export)) + (rx/empty)) + (let [profile-id (:profile-id state) + params (normalize-export-shapes-params {:exports [export] + :profile-id profile-id + :cmd :export-shapes + :wait true + :is-wasm (wasm-export-enabled? state)})] + (rx/concat + (dwp/force-persist-and-wait 400) - (->> (rp/cmd! :export params) - (rx/map (fn [{:keys [filename mtype uri]}] - (dom/trigger-download-uri filename mtype uri) - (clear-export-state uuid/zero))) - (rx/catch (fn [cause] - (rx/concat - (rx/of (clear-export-state uuid/zero)) - (rx/throw cause))))))))))) + (->> (rp/cmd! :export params) + (rx/map (fn [{:keys [filename mtype uri]}] + (dom/trigger-download-uri filename mtype uri) + (clear-export-state uuid/zero))) + (rx/catch (fn [cause] + (rx/concat + (rx/of (clear-export-state uuid/zero)) + (rx/throw cause)))))))))))) (defn request-multiple-export [{:keys [exports cmd name] :or {cmd :export-shapes} :as params}] - (ptk/reify ::request-multiple-export - ptk/WatchEvent - (watch [_ state _] - (let [resource-id (volatile! nil) - profile-id (:profile-id state) - ws-conn (:ws-conn state) - params (cond-> - {:exports exports - :cmd cmd - :profile-id profile-id - :force-multiple true - :is-wasm (wasm-export-enabled? state)} - (some? name) - (assoc :name name)) + (let [exports (normalize-exports exports)] + (ptk/reify ::request-multiple-export + ptk/WatchEvent + (watch [_ state _] + (let [resource-id (volatile! nil) + profile-id (:profile-id state) + ws-conn (:ws-conn state) + params (cond-> + {:exports exports + :cmd cmd + :profile-id profile-id + :force-multiple true + :is-wasm (wasm-export-enabled? state)} + (some? name) + (assoc :name name)) - progress-stream - (->> (ws/get-rcv-stream ws-conn) - (rx/filter ws/message-event?) - (rx/map :payload) - (rx/filter #(= :export-update (:type %))) - (rx/filter #(= @resource-id (:resource-id %))) - (rx/share)) + progress-stream + (->> (ws/get-rcv-stream ws-conn) + (rx/filter ws/message-event?) + (rx/map :payload) + (rx/filter #(= :export-update (:type %))) + (rx/filter #(= @resource-id (:resource-id %))) + (rx/share)) - stopper - (rx/filter #(or (= "ended" (:status %)) - (= "error" (:status %))) - progress-stream)] + stopper + (rx/filter #(or (= "ended" (:status %)) + (= "error" (:status %))) + progress-stream)] - (swap! st/ongoing-tasks conj :export) + (swap! st/ongoing-tasks conj :export) - (rx/merge - ;; Force that all data is persisted; best effort. - (rx/of ::dwp/force-persist) + (rx/merge + ;; Force that all data is persisted; best effort. + (rx/of ::dwp/force-persist) - ;; Launch the exportation process and stores the resource id - ;; locally. - (->> (rp/cmd! :export params) - (rx/map (fn [{:keys [id] :as resource}] - (vreset! resource-id id) - (initialize-export-status exports cmd resource)))) + ;; Launch the exportation process and stores the resource id + ;; locally. + (->> (rp/cmd! :export params) + (rx/map (fn [{:keys [id] :as resource}] + (vreset! resource-id id) + (initialize-export-status exports cmd resource)))) - ;; We proceed to update the export state with incoming - ;; progress updates. We delay the stopper for give some time - ;; to update the status with ended or errored status before - ;; close the stream. - (->> progress-stream - (rx/map update-export-status) - (rx/take-until (rx/delay 500 stopper)) - (rx/finalize (fn [] - (swap! st/ongoing-tasks disj :export)))) + ;; We proceed to update the export state with incoming + ;; progress updates. We delay the stopper for give some time + ;; to update the status with ended or errored status before + ;; close the stream. + (->> progress-stream + (rx/map update-export-status) + (rx/take-until (rx/delay 500 stopper)) + (rx/finalize (fn [] + (swap! st/ongoing-tasks disj :export)))) - ;; We hide need to hide the ui elements of the export after - ;; some interval. We also delay a little bit more the stopper - ;; for ensure that after some security time, the stream is - ;; completely closed. - (->> progress-stream - (rx/filter #(= "ended" (:status %))) - (rx/take 1) - (rx/delay default-timeout) - (rx/map #(clear-export-state @resource-id)) - (rx/take-until (rx/delay 6000 stopper)))))))) + ;; We hide need to hide the ui elements of the export after + ;; some interval. We also delay a little bit more the stopper + ;; for ensure that after some security time, the stream is + ;; completely closed. + (->> progress-stream + (rx/filter #(= "ended" (:status %))) + (rx/take 1) + (rx/delay default-timeout) + (rx/map #(clear-export-state @resource-id)) + (rx/take-until (rx/delay 6000 stopper))))))))) (defn request-export [{:keys [exports] :as params}] diff --git a/frontend/src/app/main/data/workspace/clipboard.cljs b/frontend/src/app/main/data/workspace/clipboard.cljs index c87a5707f9..eca273832a 100644 --- a/frontend/src/app/main/data/workspace/clipboard.cljs +++ b/frontend/src/app/main/data/workspace/clipboard.cljs @@ -34,6 +34,7 @@ [app.config :as cf] [app.main.data.changes :as dch] [app.main.data.event :as ev] + [app.main.data.exports.assets :as de] [app.main.data.exports.wasm :as wasm.exports] [app.main.data.helpers :as dsh] [app.main.data.notifications :as ntf] @@ -1147,16 +1148,16 @@ page-id (:current-page-id state) selected (first (dsh/lookup-selected state)) - export {:file-id file-id - :page-id page-id - :object-id selected - ;; webp would be preferrable, but PNG is the most supported image MIME type by clipboard APIs. - :type :png - ;; Always use 2 to ensure good enough quality for wireframes. - :scale 2 - :suffix "" - :enabled true - :name ""} + export (de/normalize-export {:file-id file-id + :page-id page-id + :object-id selected + ;; webp would be preferrable, but PNG is the most supported image MIME type by clipboard APIs. + :type :png + ;; Always use 2 to ensure good enough quality for wireframes. + :scale 2 + :suffix "" + :enabled true + :name ""}) ;; Create a deferred promise immediately, before any async operations. ;; Registering the clipboard write NOW preserves the user-gesture security diff --git a/frontend/src/app/plugins/shape.cljs b/frontend/src/app/plugins/shape.cljs index 4a1ea9bdf9..725f69c3c8 100644 --- a/frontend/src/app/plugins/shape.cljs +++ b/frontend/src/app/plugins/shape.cljs @@ -34,6 +34,7 @@ [app.common.types.text :as txt] [app.common.uuid :as uuid] [app.config :as cf] + [app.main.data.exports.assets :as de] [app.main.data.exports.wasm :as wasm.exports] [app.main.data.persistence :as dwp] [app.main.data.plugins :as dp] @@ -1547,13 +1548,13 @@ :profile-id (:profile-id @st/state) :wait true :is-wasm false - :exports [{:file-id file-id - :page-id page-id - :object-id id - :name (:name shape) - :type (:type value :png) - :suffix (:suffix value "") - :scale (:scale value 1)}]}] + :exports [(de/normalize-export {:file-id file-id + :page-id page-id + :object-id id + :name (:name shape) + :type (:type value :png) + :suffix (:suffix value "") + :scale (:scale value 1)})]}] (js/Promise. (fn [resolve reject] ;; The exporter renders the file from its persisted diff --git a/frontend/test/frontend_tests/data/exports_assets_test.cljs b/frontend/test/frontend_tests/data/exports_assets_test.cljs new file mode 100644 index 0000000000..d4ae9edea8 --- /dev/null +++ b/frontend/test/frontend_tests/data/exports_assets_test.cljs @@ -0,0 +1,99 @@ +;; 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 INC Sucursal en España SL + +(ns frontend-tests.data.exports-assets-test + (:require + [app.common.uuid :as uuid] + [app.main.data.exports.assets :as de] + [app.main.data.persistence :as dwp] + [app.main.repo :as repo] + [app.main.store :as st] + [app.util.dom :as dom] + [app.util.websocket :as ws] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.events :as the] + [frontend-tests.helpers.mock :as mock] + [potok.v2.core :as ptk])) + +(def ^:private export {:id (uuid/next) + :object-id (uuid/next) + :type :png + :suffix "" + :scale 1}) + +(defn- export-with-name + [name] + (merge export {:name name})) + +(defn- test-state + [] + {:profile-id (:id export) + :ws-conn nil}) + +(t/deftest normalize-export-preserves-existing-name + (t/is (= (export-with-name "Layer 1") + (de/normalize-export (export-with-name "Layer 1"))))) + +(t/deftest normalize-export-replaces-nil-name-with-object-id + (t/is (= (export-with-name (str (:object-id export))) + (de/normalize-export (assoc export :name nil))))) + +(t/deftest normalize-export-replaces-empty-name-with-object-id + (t/is (= (export-with-name (str (:object-id export))) + (de/normalize-export (assoc export :name ""))))) + +(t/deftest request-simple-export-sends-normalized-export + (t/async done + (let [export (export-with-name "") + observed (atom nil)] + (mock/with-mocks {repo/cmd! (mock/stub (fn [_ params] + (reset! observed params) + (rx/of {:filename "export.png" + :mtype "image/png" + :uri "blob:export"}))) + dwp/force-persist-and-wait (mock/stub (fn [_] (rx/of ::force-persisted))) + dom/trigger-download-uri (mock/stub (fn [& _] nil))} + (fn [done'] + (let [completed (fn [_state] + (t/is (= (export-with-name (str (:object-id export))) + (-> @observed :exports first))))] + (ptk/emit! (the/prepare-store (test-state) done' completed) + (de/request-simple-export {:export export}) + :the/end))) + done)))) + +(t/deftest request-multiple-export-sends-normalized-enabled-exports + (t/async done + (let [exports [{:id "enabled-1" + :object-id "enabled-1" + :shape {:id "enabled-1"} + :type :png + :suffix "" + :scale 1 + :enabled true + :name ""}] + observed (atom nil)] + (mock/with-mocks {repo/cmd! (mock/stub (fn [_ params] + (reset! observed params) + (rx/of {:id (:id export)}))) + ws/get-rcv-stream (mock/stub (fn [_] (rx/empty))) + st/ongoing-tasks (atom #{})} + (fn [done'] + (let [completed (fn [_state] + (t/is (= [{:id "enabled-1" + :object-id "enabled-1" + :shape {:id "enabled-1"} + :type :png + :suffix "" + :scale 1 + :enabled true + :name "enabled-1"}] + (:exports @observed))))] + (ptk/emit! (the/prepare-store (test-state) done' completed) + (de/request-multiple-export {:exports exports}) + :the/end))) + done)))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index 450f6978bd..7972b4e039 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -7,6 +7,7 @@ [frontend-tests.basic-shapes-test] [frontend-tests.code-gen-style-test] [frontend-tests.copy-as-svg-test] + [frontend-tests.data.exports-assets-test] [frontend-tests.data.nitrate-test] [frontend-tests.data.repo-test] [frontend-tests.data.store-test] @@ -88,6 +89,7 @@ 'frontend-tests.data.nitrate-test 'frontend-tests.data.repo-test 'frontend-tests.data.store-test + 'frontend-tests.data.exports-assets-test 'frontend-tests.errors-test 'frontend-tests.main-errors-test 'frontend-tests.data.uploads-test From ef593514f231a91e206a067122b0325df5d3438d Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 30 Jul 2026 12:50:46 +0200 Subject: [PATCH 3/8] :bug: Add nil guards on viewport-node in pixel overlay component (#10812) * :bug: Add nil guards on viewport-node in pixel overlay component Add nil checks for viewport-node in process-pointer-move, viewport->canvas-coords, process-pointer-move-wasm, pick-color-at-wasm, and handle-draw-picker-canvas. Fixes a crash ("can't access property 'getBoundingClientRect', ... is null") when the viewport DOM node is unmounted while the color picker eyedropper is active and pointer move events are still firing. Fixes #10811 AI-assisted-by: mimo-v2.5 * :bug: Remove unused app.common.pprint require from errors.cljs Fixes clj-kondo warning: namespace app.common.pprint is required but never used. AI-assisted-by: mimo-v2.5 --- .../ui/workspace/viewport/pixel_overlay.cljs | 98 ++++++++++--------- 1 file changed, 50 insertions(+), 48 deletions(-) diff --git a/frontend/src/app/main/ui/workspace/viewport/pixel_overlay.cljs b/frontend/src/app/main/ui/workspace/viewport/pixel_overlay.cljs index afc461b4c6..c756f06e52 100644 --- a/frontend/src/app/main/ui/workspace/viewport/pixel_overlay.cljs +++ b/frontend/src/app/main/ui/workspace/viewport/pixel_overlay.cljs @@ -56,51 +56,52 @@ (defn process-pointer-move [viewport-node canvas canvas-image-data zoom-view-context last-picked-color client-x client-y] - (when-let [image-data (mf/ref-val canvas-image-data)] - (when-let [zoom-view-node (dom/get-element "picker-detail")] - (when-not (mf/ref-val zoom-view-context) - (mf/set-ref-val! zoom-view-context (.getContext zoom-view-node "2d"))) - (let [canvas-width 260 - canvas-height 140 - {brx :left bry :top} (dom/get-bounding-rect viewport-node) + (when viewport-node + (when-let [image-data (mf/ref-val canvas-image-data)] + (when-let [zoom-view-node (dom/get-element "picker-detail")] + (when-not (mf/ref-val zoom-view-context) + (mf/set-ref-val! zoom-view-context (.getContext zoom-view-node "2d"))) + (let [canvas-width 260 + canvas-height 140 + {brx :left bry :top} (dom/get-bounding-rect viewport-node) - x (mth/floor (- client-x brx)) - y (mth/floor (- client-y bry)) + x (mth/floor (- client-x brx)) + y (mth/floor (- client-y bry)) - img-width (unchecked-get image-data "width") - img-height (unchecked-get image-data "height") + img-width (unchecked-get image-data "width") + img-height (unchecked-get image-data "height") - zoom-context (mf/ref-val zoom-view-context) + zoom-context (mf/ref-val zoom-view-context) - sx (- x 32) - sy (if (cfg/check-browser? :safari) y (- y 17)) - sw 65 - sh 35 - dx 0 - dy 0 - dw canvas-width - dh canvas-height] + sx (- x 32) + sy (if (cfg/check-browser? :safari) y (- y 17)) + sw 65 + sh 35 + dx 0 + dy 0 + dw canvas-width + dh canvas-height] - (when (obj/get zoom-context "imageSmoothingEnabled") - (obj/set! zoom-context "imageSmoothingEnabled" false)) - (.clearRect zoom-context 0 0 canvas-width canvas-height) - (.drawImage zoom-context canvas sx sy sw sh dx dy dw dh) + (when (obj/get zoom-context "imageSmoothingEnabled") + (obj/set! zoom-context "imageSmoothingEnabled" false)) + (.clearRect zoom-context 0 0 canvas-width canvas-height) + (.drawImage zoom-context canvas sx sy sw sh dx dy dw dh) - ;; Only pick color when cursor is within canvas bounds to avoid garbage pixels - (when (and (>= x 0) (< x img-width) (>= y 0) (< y img-height)) - (let [offset (* (+ (* y img-width) x) 4) - rgba (unchecked-get image-data "data") - r (d/check-num (obj/get rgba (+ 0 offset)) 255) - g (d/check-num (obj/get rgba (+ 1 offset)) 255) - b (d/check-num (obj/get rgba (+ 2 offset)) 255) - a (d/check-num (obj/get rgba (+ 3 offset)) 255) - color [r g b a]] - ;; Store latest color synchronously so the click handler always reads - ;; the correct pixel even before the rAF fires (fixes race condition) - (mf/set-ref-val! last-picked-color color) - (timers/raf - (fn [] - (st/emit! (dwc/pick-color color)))))))))) + ;; Only pick color when cursor is within canvas bounds to avoid garbage pixels + (when (and (>= x 0) (< x img-width) (>= y 0) (< y img-height)) + (let [offset (* (+ (* y img-width) x) 4) + rgba (unchecked-get image-data "data") + r (d/check-num (obj/get rgba (+ 0 offset)) 255) + g (d/check-num (obj/get rgba (+ 1 offset)) 255) + b (d/check-num (obj/get rgba (+ 2 offset)) 255) + a (d/check-num (obj/get rgba (+ 3 offset)) 255) + color [r g b a]] + ;; Store latest color synchronously so the click handler always reads + ;; the correct pixel even before the rAF fires (fixes race condition) + (mf/set-ref-val! last-picked-color color) + (timers/raf + (fn [] + (st/emit! (dwc/pick-color color))))))))))) (mf/defc pixel-overlay* @@ -260,18 +261,19 @@ (defn- viewport->canvas-coords "Maps client (viewport) coordinates to device-pixel canvas coordinates." [viewport-node client-x client-y] - (let [{brx :left bry :top} (dom/get-bounding-rect viewport-node) - dpr (wasm.api/get-dpr) - x (mth/floor (- client-x brx)) - y (mth/floor (- client-y bry))] - [(mth/floor (* x dpr)) - (mth/floor (* y dpr))])) + (when viewport-node + (let [{brx :left bry :top} (dom/get-bounding-rect viewport-node) + dpr (wasm.api/get-dpr) + x (mth/floor (- client-x brx)) + y (mth/floor (- client-y bry))] + [(mth/floor (* x dpr)) + (mth/floor (* y dpr))]))) (defn process-pointer-move-wasm "Updates the magnifier loupe with the canvas region under the cursor. The actual color is only read on click (see `pick-color-at-wasm`)." [viewport-node canvas zoom-view-context client-x client-y] - (when canvas + (when (and canvas viewport-node) (when-let [zoom-view-node (dom/get-element "picker-detail")] (when-not (mf/ref-val zoom-view-context) (mf/set-ref-val! zoom-view-context (.getContext zoom-view-node "2d"))) @@ -309,7 +311,7 @@ the correct color even on GPUs where a raw WebGL `readPixels` returned values with their byte order swapped." [viewport-node canvas client-x client-y] - (when canvas + (when (and canvas viewport-node) (let [[canvas-x canvas-y] (viewport->canvas-coords viewport-node client-x client-y) img-width (.-width canvas) img-height (.-height canvas)] @@ -370,7 +372,7 @@ handle-draw-picker-canvas (mf/use-callback (fn [] - (when canvas + (when (and canvas viewport-node) ;; Read current mouse position from ref so the loupe refreshes on ;; each render even without a mouse-move. (let [{mx :x my :y} (mf/ref-val initial-mouse-pos)] From 1ae9334064aba44d3765bc6ed09c6409cc630c08 Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Thu, 30 Jul 2026 10:22:58 +0000 Subject: [PATCH 4/8] :bug: Fail fast on Redis task dispatch when no instance is connected #10958 In multi-user mode, plugin task requests are published to a Redis channel keyed by user token. When no MCP server instance held a plugin connection for that token (e.g. after the user navigated away from the workspace), the publish reached zero subscribers and the request was silently dropped, so every tool call stalled until the 30-second task timeout instead of failing with a meaningful error. RedisBridge.sendTaskRequest now returns the PUBLISH receiver count and releases its response-channel subscription when the request reached no receiver (or publishing failed), since no response can arrive. PluginBridge uses the count to reject the pending task immediately with the multi-user connection error message; publish failures likewise reject the task instead of surfacing as an unhandled rejection followed by a timeout. The pending- task settlement logic shared with the timeout handler is extracted into a rejectPendingTask helper. AI-assisted-by: claude-fable-5 --- mcp/packages/server/src/PluginBridge.ts | 66 ++++++++++++++++++++----- mcp/packages/server/src/RedisBridge.ts | 20 +++++++- 2 files changed, 71 insertions(+), 15 deletions(-) diff --git a/mcp/packages/server/src/PluginBridge.ts b/mcp/packages/server/src/PluginBridge.ts index 1c24547b8f..23f6d09e15 100644 --- a/mcp/packages/server/src/PluginBridge.ts +++ b/mcp/packages/server/src/PluginBridge.ts @@ -20,6 +20,8 @@ interface ClientConnection { * over these connections. */ export class PluginBridge { + public static readonly MULTIUSER_CONNECTION_ERROR_MESSAGE = `No Penpot instance connected for user token. Please ensure that Penpot is connected and that the MCP client connection is using the correct token.`; + private readonly logger = createLogger("PluginBridge"); private readonly wsServer: WebSocketServer; private readonly connectedClients: Map = new Map(); @@ -189,6 +191,35 @@ export class PluginBridge { this.logger.info(`Task ${response.id} completed: success=${response.success}`); } + /** + * Rejects a still-pending task with the given error, releasing its correlation state. + * + * Clears the task's timeout (if armed) and removes the task from the pending-task + * index before rejecting its promise. Safe to call for a task that has already been + * settled (e.g. by a response or a timeout), in which case nothing happens. + * + * @param taskId - The ID of the task to reject + * @param error - The error with which to reject the task + * @returns Whether the task was still pending and has been rejected + */ + private rejectPendingTask(taskId: string, error: Error): boolean { + const pendingTask = this.pendingTasks.get(taskId); + if (!pendingTask) { + return false; + } + + const timeoutHandle = this.taskTimeouts.get(taskId); + if (timeoutHandle) { + clearTimeout(timeoutHandle); + this.taskTimeouts.delete(taskId); + } + this.pendingTasks.delete(taskId); + + pendingTask.rejectWithError(error); + this.logger.info(`Task ${taskId} rejected: ${error.message}`); + return true; + } + /** * Determines the client connection to use for executing a task. * @@ -207,9 +238,7 @@ export class PluginBridge { const connection = this.clientsByToken.get(sessionContext.userToken); if (!connection) { - throw new Error( - `No plugin instance connected for user token. Please ensure the plugin is running and connected with the correct token.` - ); + throw new Error(PluginBridge.MULTIUSER_CONNECTION_ERROR_MESSAGE); } return connection; @@ -257,6 +286,10 @@ export class PluginBridge { * `resolveWithResult`/`rejectWithError` methods. The same correlation and timeout * handling therefore applies regardless of the transport. * + * When routing via Redis, the task is rejected immediately (rather than timing out) + * if the published request reached no instance, i.e. if no instance holds a plugin + * connection for the session's user token, or if publishing fails outright. + * * @param task - The task to dispatch * @param useRedis - Whether to route the request via Redis (multi-instance) rather * than directly over the local WebSocket connection @@ -279,9 +312,17 @@ export class PluginBridge { // register the task for result correlation, then publish the request via Redis this.pendingTasks.set(task.id, task); - void redisBridge.sendTaskRequest(userToken, task.toRequest(), (response) => - this.handlePluginTaskResponse(response) - ); + void redisBridge + .sendTaskRequest(userToken, task.toRequest(), (response) => this.handlePluginTaskResponse(response)) + .then((receiverCount) => { + // fail fast when no instance received the request (no connection with matching user token in any instance) + if (receiverCount === 0) { + this.rejectPendingTask(task.id, new Error(PluginBridge.MULTIUSER_CONNECTION_ERROR_MESSAGE)); + } + }) + .catch((error) => { + this.rejectPendingTask(task.id, error instanceof Error ? error : new Error(String(error))); + }); // on timeout, release the response-channel subscription, since no response // will arrive to trigger its self-unsubscribe. @@ -300,14 +341,13 @@ export class PluginBridge { // Set up a timeout to reject the task if no response is received const timeoutHandle = setTimeout(() => { - const pendingTask = this.pendingTasks.get(task.id); - if (pendingTask) { - this.pendingTasks.delete(task.id); - this.taskTimeouts.delete(task.id); - onTimeout?.(); - pendingTask.rejectWithError( + if ( + this.rejectPendingTask( + task.id, new Error(`Task ${task.id} timed out after ${this.taskTimeoutSecs} seconds`) - ); + ) + ) { + onTimeout?.(); } }, this.taskTimeoutSecs * 1000); diff --git a/mcp/packages/server/src/RedisBridge.ts b/mcp/packages/server/src/RedisBridge.ts index d8117d9242..89c01a4871 100644 --- a/mcp/packages/server/src/RedisBridge.ts +++ b/mcp/packages/server/src/RedisBridge.ts @@ -91,12 +91,16 @@ export class RedisBridge { * @param userToken - The user token identifying the target plugin's request channel * @param request - The serialized plugin task request, passed through verbatim * @param onResponse - Handler invoked with the response when it arrives + * @returns The number of instances that received the request. A count of 0 means no + * instance is subscribed to the token's request channel (i.e. the plugin is not + * connected anywhere); the request was dropped, no response will ever arrive, and + * the response subscription has already been released. */ async sendTaskRequest( userToken: string, request: PluginTaskRequest, onResponse: TaskResponseHandler - ): Promise { + ): Promise { const responseChannel = this.responseChannel(request.id); const requestChannel = this.requestChannel(userToken); @@ -113,7 +117,19 @@ export class RedisBridge { await this.subscriber.subscribe(responseChannel); // publish only once the response subscription is confirmed - await this.publisher.publish(requestChannel, JSON.stringify(request)); + let receiverCount: number; + try { + receiverCount = await this.publisher.publish(requestChannel, JSON.stringify(request)); + } catch (error) { + // the request was never delivered, so no response can arrive + await this.unsubscribeFromResponse(request.id); + throw error; + } + if (receiverCount === 0) { + // no subscriber received the request, so no response can arrive + await this.unsubscribeFromResponse(request.id); + } + return receiverCount; } /** From b4659df5b28b473f8191a3f93748ca7a5dfd1adf Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Thu, 30 Jul 2026 10:40:33 +0000 Subject: [PATCH 5/8] :bug: Preserve established plugin connection when rejecting a duplicate #10961 In multi-user mode, rejecting a second plugin WebSocket connection for an already-registered user token performed the full removeConnection cleanup for the newcomer. Since the token-keyed cleanup is keyed by token rather than by socket, this deleted the clientsByToken entry and the Redis request-channel subscription of the established, healthy connection. That connection then remained open and heartbeating but was unroutable, so every subsequent MCP tool call for the user failed although a valid plugin connection existed. removeConnection now performs the token-keyed cleanup only if the removed connection actually owns the token registration, so rejecting a duplicate releases only the resources the newcomer itself registered. AI-assisted-by: claude-fable-5 --- mcp/packages/server/src/PluginBridge.ts | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/mcp/packages/server/src/PluginBridge.ts b/mcp/packages/server/src/PluginBridge.ts index 23f6d09e15..e8dda27ac3 100644 --- a/mcp/packages/server/src/PluginBridge.ts +++ b/mcp/packages/server/src/PluginBridge.ts @@ -133,9 +133,10 @@ export class PluginBridge { /** * Removes a client connection and releases all resources associated with it. * - * Clears the per-connection keep-alive interval and removes the connection - * from both the socket-keyed and token-keyed indexes. Safe to call with a - * socket that is not (or no longer) registered. + * Clears the per-connection keep-alive interval and removes the connection from the + * socket-keyed index. The token-keyed index entry (and, in multi-instance mode, the + * token's Redis task subscription) is removed only if it is owned by the given + * connection. Safe to call with a socket that is not (or no longer) registered. * * @param ws - The WebSocket whose connection state should be removed */ @@ -147,12 +148,18 @@ export class PluginBridge { clearInterval(connection.pingInterval); this.connectedClients.delete(ws); if (connection.userToken) { - this.clientsByToken.delete(connection.userToken); + // Perform the token-keyed cleanup only if this connection owns the token registration. + // A connection rejected as a duplicate carries the same token but must not remove token associations. + if (this.clientsByToken.get(connection.userToken) !== connection) { + this.logger.debug("Removed connection does not own its token registration; skipping token cleanup"); + } else { + this.clientsByToken.delete(connection.userToken); - if (this.redisBridge) { - this.redisBridge - .unsubscribeFromTasks(connection.userToken) - .catch((error) => this.logger.error(error, "Failed to unsubscribe from Redis task channel")); + if (this.redisBridge) { + this.redisBridge + .unsubscribeFromTasks(connection.userToken) + .catch((error) => this.logger.error(error, "Failed to unsubscribe from Redis task channel")); + } } } } From 30943f1074515a7184f20d29e9f9e40b787e5025 Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Thu, 30 Jul 2026 10:38:29 +0200 Subject: [PATCH 6/8] :sparkles: Make MCP tool call timeout configurable, raising default The timeout for tool calls (which is trictly relevant for plugin tasks only) is now configurable via env. var PENPOT_MCP_TOOL_TIMEOUT_S. The default was raised from 30 to 120, because 30 seconds was not enough for some calls, especially in larger Penpot files. #10953 --- mcp/README.md | 1 + mcp/packages/server/src/PenpotMcpServer.ts | 3 ++- mcp/packages/server/src/PluginBridge.ts | 6 ++++-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/mcp/README.md b/mcp/README.md index 3efc6255e7..846ee4d128 100644 --- a/mcp/README.md +++ b/mcp/README.md @@ -272,6 +272,7 @@ The Penpot MCP server can be configured using environment variables. | `PENPOT_MCP_REPL_PORT` | Port for the REPL server (development/debugging) | `4403` | | `PENPOT_MCP_REMOTE_MODE` | Enable remote mode (disables file system access). Set to `true` to enable. | `false` | | `PENPOT_MCP_DEVENV` | Enable Penpot development environment tools. Set to `true` to enable. | `false` | +| `PENPOT_MCP_TOOL_TIMEOUT_S` | Timeout, in seconds, for tool calls dispatched to the Penpot plugin | `120` | | `PENPOT_MCP_EXPORT_SHAPE_MAX_PARALLEL_REQUESTS` | Maximum number of parallel export shape requests (multi-user mode only). | `0` (no limit) | | `PENPOT_MCP_REDIS_URI` | Redis connection URI (e.g. `redis://host:6379`) enabling multi-instance horizontal scaling via Redis pub/sub task routing (multi-user mode only). When unset, the server runs in single-instance mode, requiring the plugin and MCP client to connect to the same instance. | (unset) | diff --git a/mcp/packages/server/src/PenpotMcpServer.ts b/mcp/packages/server/src/PenpotMcpServer.ts index 8a4ee30f25..bd992ec108 100644 --- a/mcp/packages/server/src/PenpotMcpServer.ts +++ b/mcp/packages/server/src/PenpotMcpServer.ts @@ -127,6 +127,7 @@ export class PenpotMcpServer { this.webSocketPort = parseInt(process.env.PENPOT_MCP_WEBSOCKET_PORT ?? "4402", 10); this.replPort = parseInt(process.env.PENPOT_MCP_REPL_PORT ?? "4403", 10); this.tenant = process.env.PENPOT_TENANT ?? "default"; + const toolTimeoutSecs = parseInt(process.env.PENPOT_MCP_TOOL_TIMEOUT_S ?? "120", 10); this.configLoader = new ConfigurationLoader(process.cwd()); this.apiDocs = new ApiDocs(); @@ -147,7 +148,7 @@ export class PenpotMcpServer { this.redisBridge = new RedisBridge(redisUri, this.tenant); } - this.pluginBridge = new PluginBridge(this, this.webSocketPort, this.redisBridge); + this.pluginBridge = new PluginBridge(this, this.webSocketPort, toolTimeoutSecs, this.redisBridge); this.replServer = new ReplServer(this.pluginBridge, this.replPort, this.host); } diff --git a/mcp/packages/server/src/PluginBridge.ts b/mcp/packages/server/src/PluginBridge.ts index e8dda27ac3..ac7a2c005c 100644 --- a/mcp/packages/server/src/PluginBridge.ts +++ b/mcp/packages/server/src/PluginBridge.ts @@ -24,6 +24,7 @@ export class PluginBridge { private readonly logger = createLogger("PluginBridge"); private readonly wsServer: WebSocketServer; + private readonly connectedClients: Map = new Map(); private readonly clientsByToken: Map = new Map(); private readonly pendingTasks: Map> = new Map(); @@ -39,12 +40,13 @@ export class PluginBridge { * holding the relevant plugin's WebSocket connection (which may be this same * instance) via Redis, rather than dispatched directly over a local socket. * @param taskTimeoutSecs - Timeout, in seconds, for plugin task execution + * (defaults to {@link DEFAULT_TASK_TIMEOUT_SECS}) */ constructor( public readonly mcpServer: PenpotMcpServer, private port: number, - private readonly redisBridge?: RedisBridge, - private taskTimeoutSecs: number = 30 + private readonly taskTimeoutSecs: number, + private readonly redisBridge?: RedisBridge ) { this.wsServer = new WebSocketServer({ port: port }); this.setupWebSocketHandlers(); From 764b62906b868f2d860b54fd73ea6b31f2624b33 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 31 Jul 2026 12:06:19 +0200 Subject: [PATCH 7/8] :bug: Handle unrecognized JSON escape sequences as malformed-json (#10808) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * :paperclip: Update serena documentation about creating-prs workflow * :bug: Handle unrecognized JSON escape sequences as malformed-json When clojure.data.json's read-escaped-char encounters an unrecognized escape sequence (e.g. a backslash followed by '}', or other case fall-throughs in the parser) in a JSON request body, it throws a bare IllegalArgumentException. Previously this fell through to the generic RuntimeException branch in wrap-parse-request's handle-error, which unwrapped and recurred without matching, eventually reaching the internal-error handler and producing HTTP 500 + an error report — even though the root cause was malformed client input, not a server bug. The fix converts any IllegalArgumentException raised in the JSON parse path into a `:validation`/`:malformed-json` error by raising a new ex-info (which is caught by the top-level error handler in `app.http/router-handler`). The result is an HTTP 400 response with a descriptive hint, and no error report is generated. This addresses ~10% of all error reports received. The new IAE branch is placed before the RuntimeException branch in the cond (since IllegalArgumentException IS-A RuntimeException) and uses the throw-style (ex/raise) to match the existing RequestTooBigException / EOFException branches. A comment above the handle-error cond documents why raising is intentional and is caught by the top-level app.http error handler, not by the per-route wrap-errors middleware. Test suite changes: - Extend the existing `DummyRequest` defrecord in `http_middleware_test.clj` from 2 fields to 12 fields, implementing every IRequest method, and add a private `make-dummy-request` constructor that accepts an options map with every key optional and sensible `:or` defaults. Future fields added to DummyRequest won't break existing call sites as long as the `:or` defaults are kept in sync. - Remove the now-redundant `JsonRequest` defrecord and migrate all 11 `->DummyRequest` call sites to `make-dummy-request`. - Add 6 new deftest cases: - parse-request-illegal-argument-exception: malformed JSON body (containing `\}`) is converted to `:malformed-json`. - parse-request-request-too-big-exception: RequestTooBigException is converted to `:request-body-too-large`. - parse-request-eof-exception: java.io.EOFException is converted to `:malformed-json`. - parse-request-runtime-exception-with-cause: a wrapped RuntimeException recurses on ex-cause and dispatches to the matching specific branch. - parse-request-runtime-exception-without-cause: a bare RuntimeException falls through to errors/handle, returning 500 with :type :server-error :code :unexpected. - parse-request-non-runtime-throwable: java.io.IOException (a non-RuntimeException Throwable) is handled by the dedicated handle-exception method, returning 500 with :code :io-exception. Together, the new tests cover all 6 branches of wrap-parse-request's handle-error cond. Refs #10804. AI-assisted-by: minimax-m3 --- .serena/memories/workflow/creating-prs.md | 2 +- backend/src/app/http/middleware.clj | 26 ++- .../backend_tests/http_middleware_test.clj | 206 ++++++++++++++++-- 3 files changed, 215 insertions(+), 19 deletions(-) diff --git a/.serena/memories/workflow/creating-prs.md b/.serena/memories/workflow/creating-prs.md index 00252a7661..13bde56149 100644 --- a/.serena/memories/workflow/creating-prs.md +++ b/.serena/memories/workflow/creating-prs.md @@ -30,7 +30,7 @@ See `mem:workflow/creating-commits` for emoji codes. Squash merge uses the PR ti Include concise sections covering: - what changed and why; -- related GitHub issues or Taiga stories (`Fixes #NNNN`, `Relates to #NNNN`, `Taiga #NNNN`); +- related GitHub issues or Taiga stories (`Closes #NNNN`, `Relates to #NNNN`, `Taiga #NNNN`); - screenshots or recordings for UI-visible changes; - testing performed and residual risk; - breaking changes or migration notes, if any. diff --git a/backend/src/app/http/middleware.clj b/backend/src/app/http/middleware.clj index 81b1ef13ec..fa2faa8a55 100644 --- a/backend/src/app/http/middleware.clj +++ b/backend/src/app/http/middleware.clj @@ -65,12 +65,25 @@ :else request))) + ;; The specific-exception branches below (IAE, + ;; RequestTooBigException, EOFException) raise with + ;; `ex/raise` rather than calling `errors/handle` directly. + ;; This is intentional: the throw is caught by the + ;; top-level error handler in `app.http/router-handler` + ;; (`backend/src/app/http.clj`), which routes every + ;; uncaught exception through `errors/handle`. The + ;; per-route `wrap-errors` middleware in the route list + ;; is a defensive layer; correctness does not depend on + ;; it. Raising here keeps the cond uniform with the + ;; existing RequestTooBigException / EOFException + ;; branches. (handle-error [cause request] (cond - (instance? RuntimeException cause) - (if-let [cause (ex-cause cause)] - (handle-error cause request) - (errors/handle cause request)) + (instance? IllegalArgumentException cause) + (ex/raise :type :validation + :code :malformed-json + :hint (ex-message cause) + :cause cause) (instance? RequestTooBigException cause) (ex/raise :type :validation @@ -83,6 +96,11 @@ :hint (ex-message cause) :cause cause) + (instance? RuntimeException cause) + (if-let [cause (ex-cause cause)] + (handle-error cause request) + (errors/handle cause request)) + :else (errors/handle cause request)))] diff --git a/backend/test/backend_tests/http_middleware_test.clj b/backend/test/backend_tests/http_middleware_test.clj index f751d8aca6..bd986fc031 100644 --- a/backend/test/backend_tests/http_middleware_test.clj +++ b/backend/test/backend_tests/http_middleware_test.clj @@ -21,19 +21,73 @@ [clojure.test :as t] [mockery.core :refer [with-mocks]] [yetti.request :as yreq] - [yetti.response :as yres])) + [yetti.response :as yres]) + (:import + io.undertow.server.RequestTooBigException)) (t/use-fixtures :once th/state-init) (t/use-fixtures :each th/database-reset) -(defrecord DummyRequest [headers cookies] +(defrecord DummyRequest [headers cookies method body-stream + remote-addr server-name server-port + scheme protocol path query ssl-client-cert] yreq/IRequestCookies (get-cookie [_ name] {:value (get cookies name)}) yreq/IRequest (get-header [_ name] - (get headers name))) + (get headers name)) + (method [_] method) + (body [_] body-stream) + (path [_] path) + (query [_] query) + (server-port [_] server-port) + (server-name [_] server-name) + (remote-addr [_] remote-addr) + (ssl-client-cert [_] ssl-client-cert) + (scheme [_] scheme) + (protocol [_] protocol)) + +(defn- make-dummy-request + "Constructs a DummyRequest from an options map. Every key is + optional; missing values fall back to sensible defaults. New + fields added to DummyRequest won't break existing call sites + as long as this constructor keeps its `:or` defaults in sync. + + Recognized keys: + :headers — map of header name → value + :cookies — map of cookie name → value + :method — HTTP method keyword (default :get) + :body-stream — InputStream for the body (used directly) + :body-bytes — bytes or string for the body; wrapped in a + ByteArrayInputStream if :body-stream is not + given + :remote-addr — string (default \"127.0.0.1\") + :server-name — string (default \"test\") + :server-port — long (default 0) + :scheme — keyword (default :http) + :protocol — string (default \"HTTP/1.1\") + :path — string (default \"/test\") + :query — string or nil (default nil) + :ssl-client-cert — X509Certificate or nil (default nil)" + [{:keys [headers cookies method body-stream body-bytes + remote-addr server-name server-port scheme protocol + path query ssl-client-cert] + :or {headers {} cookies {} method :get + body-stream nil + remote-addr "127.0.0.1" server-name "test" server-port 0 + scheme :http protocol "HTTP/1.1" path "/test" query nil + ssl-client-cert nil}}] + (let [body-stream (or body-stream + (when body-bytes + (java.io.ByteArrayInputStream. + (if (string? body-bytes) + (.getBytes ^String body-bytes "UTF-8") + body-bytes))))] + (->DummyRequest headers cookies method body-stream + remote-addr server-name server-port + scheme protocol path query ssl-client-cert))) (t/deftest auth-middleware-1 (let [request (volatile! nil) @@ -41,11 +95,11 @@ (fn [req] (vreset! request req)) {})] - (handler (->DummyRequest {} {})) + (handler (make-dummy-request {})) (t/is (nil? (::http/auth-data @request))) - (handler (->DummyRequest {"authorization" "Token aaaa"} {})) + (handler (make-dummy-request {:headers {"authorization" "Token aaaa"}})) (let [{:keys [token claims] token-type :type} (get @request ::http/auth-data)] (t/is (= :token token-type)) @@ -58,10 +112,10 @@ (fn [req] (vreset! request req)) {})] - (handler (->DummyRequest {} {})) + (handler (make-dummy-request {})) (t/is (nil? (::http/auth-data @request))) - (handler (->DummyRequest {"authorization" "Bearer aaaa"} {})) + (handler (make-dummy-request {:headers {"authorization" "Bearer aaaa"}})) (let [{:keys [token claims] token-type :type} (get @request ::http/auth-data)] (t/is (= :bearer token-type)) @@ -74,10 +128,10 @@ (fn [req] (vreset! request req)) {})] - (handler (->DummyRequest {} {})) + (handler (make-dummy-request {})) (t/is (nil? (::http/auth-data @request))) - (handler (->DummyRequest {} {"auth-token" "foobar"})) + (handler (make-dummy-request {:cookies {"auth-token" "foobar"}})) (let [{:keys [token claims] token-type :type} (get @request ::http/auth-data)] (t/is (= :cookie token-type)) @@ -89,16 +143,16 @@ (fn [req] {::yres/status 200}) {:test1 "secret-key"})] - (let [response (handler (->DummyRequest {} {}))] + (let [response (handler (make-dummy-request {}))] (t/is (= 403 (::yres/status response)))) - (let [response (handler (->DummyRequest {"x-shared-key" "secret-key2"} {}))] + (let [response (handler (make-dummy-request {:headers {"x-shared-key" "secret-key2"}}))] (t/is (= 403 (::yres/status response)))) - (let [response (handler (->DummyRequest {"x-shared-key" "secret-key"} {}))] + (let [response (handler (make-dummy-request {:headers {"x-shared-key" "secret-key"}}))] (t/is (= 403 (::yres/status response)))) - (let [response (handler (->DummyRequest {"x-shared-key" "test1 secret-key"} {}))] + (let [response (handler (make-dummy-request {:headers {"x-shared-key" "test1 secret-key"}}))] (t/is (= 200 (::yres/status response)))))) (t/deftest access-token-authz @@ -209,7 +263,7 @@ :user-agent "user agent"}) (#'session/assign-token cfg)) - response (handler (->DummyRequest {} {"auth-token" (:token session)})) + response (handler (make-dummy-request {:cookies {"auth-token" (:token session)}})) {:keys [token claims] token-type :type} (get response ::http/auth-data)] @@ -220,3 +274,127 @@ (t/is (= "penpot" (:aud claims))) (t/is (= (:id session) (:sid claims))) (t/is (= (:id profile) (:uid claims))))) + +(t/deftest parse-request-illegal-argument-exception + ;; clojure.data.json raises IllegalArgumentException (case + ;; fall-through) on several kinds of malformed input. The + ;; parse-request middleware should convert any such IAE into a + ;; 400 :malformed-json validation error rather than letting it + ;; surface as a 500 internal error. Because the conversion is + ;; done by raising an ex-info (caught by the top-level error + ;; handler in app.http/router-handler), this test asserts on + ;; the ex-info thrown by wrap-parse-request directly. + (let [handler (#'app.http.middleware/wrap-parse-request + (fn [_] {::yres/status 200 ::yres/body :ok})) + ;; Body contains the bytes for: {"x": "\}"} -- a string + ;; value with a backslash followed by '}', which + ;; clojure.data.json v0.5.x cannot handle. + body (.getBytes "{\"x\": \"\\}\"}" "UTF-8") + request (make-dummy-request + {:method :post + :headers {"content-type" "application/json"} + :body-bytes body}) + ex (try + (handler request) + (catch clojure.lang.ExceptionInfo e e))] + (t/is (instance? clojure.lang.ExceptionInfo ex)) + (t/is (= :validation (-> ex ex-data :type))) + (t/is (= :malformed-json (-> ex ex-data :code))) + (t/is (string? (-> ex ex-data :hint))))) + +(t/deftest parse-request-request-too-big-exception + ;; When RequestTooBigException is raised (e.g. the request body + ;; exceeded the configured size limit), the middleware should + ;; convert it to a 413 :request-body-too-large validation + ;; error. + (let [handler (#'app.http.middleware/wrap-parse-request + (fn [_] (throw (RequestTooBigException. "too large")))) + request (make-dummy-request + {:method :post + :headers {"content-type" "application/json"} + :body-bytes (.getBytes "{}" "UTF-8")}) + ex (try + (handler request) + (catch clojure.lang.ExceptionInfo e e))] + (t/is (instance? clojure.lang.ExceptionInfo ex)) + (t/is (= :validation (-> ex ex-data :type))) + (t/is (= :request-body-too-large (-> ex ex-data :code))) + (t/is (string? (-> ex ex-data :hint))))) + +(t/deftest parse-request-eof-exception + ;; When java.io.EOFException is raised (e.g. the body stream + ;; was closed before the parser could read it), the middleware + ;; should convert it to a 400 :malformed-json validation error. + (let [handler (#'app.http.middleware/wrap-parse-request + (fn [_] (throw (java.io.EOFException. "stream closed")))) + request (make-dummy-request + {:method :post + :headers {"content-type" "application/json"} + :body-bytes (.getBytes "{}" "UTF-8")}) + ex (try + (handler request) + (catch clojure.lang.ExceptionInfo e e))] + (t/is (instance? clojure.lang.ExceptionInfo ex)) + (t/is (= :validation (-> ex ex-data :type))) + (t/is (= :malformed-json (-> ex ex-data :code))) + (t/is (string? (-> ex ex-data :hint))))) + +(t/deftest parse-request-runtime-exception-with-cause + ;; When a RuntimeException with a non-nil ex-cause is raised, + ;; the middleware should recurse on the cause and dispatch + ;; through the specific-exception branches. Here we wrap an + ;; IllegalArgumentException in a RuntimeException and verify + ;; it surfaces as :malformed-json. + (let [iae (IllegalArgumentException. "No matching clause: 99") + wrapped (doto (RuntimeException. "wrapped") + (.initCause iae)) + handler (#'app.http.middleware/wrap-parse-request + (fn [_] (throw wrapped))) + request (make-dummy-request + {:method :post + :headers {"content-type" "application/json"} + :body-bytes (.getBytes "{}" "UTF-8")}) + ex (try + (handler request) + (catch clojure.lang.ExceptionInfo e e))] + (t/is (instance? clojure.lang.ExceptionInfo ex)) + (t/is (= :validation (-> ex ex-data :type))) + (t/is (= :malformed-json (-> ex ex-data :code))))) + +(t/deftest parse-request-runtime-exception-without-cause + ;; When a bare RuntimeException (no ex-cause) is raised, the + ;; middleware should fall through to errors/handle's :default + ;; path and return a 500 with :type :server-error :code + ;; :unexpected. This is the "true internal error" path. + (let [handler (#'app.http.middleware/wrap-parse-request + (fn [_] (throw (RuntimeException. "boom")))) + request (make-dummy-request + {:method :post + :headers {"content-type" "application/json"} + :body-bytes (.getBytes "{}" "UTF-8")}) + response (handler request) + body (::yres/body response)] + (t/is (= 500 (::yres/status response))) + (t/is (= :server-error (:type body))) + (t/is (= :unexpected (:code body))) + (t/is (= "boom" (:hint body))))) + +(t/deftest parse-request-non-runtime-throwable + ;; When a non-RuntimeException Throwable is raised (e.g. an + ;; Error subclass or a non-RuntimeException checked-style + ;; exception), the middleware should fall through to the + ;; :else branch and call errors/handle. java.io.IOException + ;; has a dedicated handle-exception method that returns 500 + ;; with :code :io-exception. + (let [handler (#'app.http.middleware/wrap-parse-request + (fn [_] (throw (java.io.IOException. "network gone")))) + request (make-dummy-request + {:method :post + :headers {"content-type" "application/json"} + :body-bytes (.getBytes "{}" "UTF-8")}) + response (handler request) + body (::yres/body response)] + (t/is (= 500 (::yres/status response))) + (t/is (= :server-error (:type body))) + (t/is (= :io-exception (:code body))) + (t/is (= "network gone" (:hint body))))) From d04cbf175e0a28ae74f7b960d31d8ece5e2b971c Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 31 Jul 2026 12:51:35 +0200 Subject: [PATCH 8/8] :bug: Fix nil dereference crash during flex layout drag operations (#10845) Production crash where @(get bounds id) threw "No protocol method IDeref.-deref defined for type null" when a shape ID had no corresponding entry in the bounds map during layout calculations. Added defensive nil guards (when-let / when) to all unprotected bounds dereference sites: - flex_layout/bounds.cljc: layout-content-points (parent + child) and layout-content-bounds - grid_layout/bounds.cljc: layout-content-points and layout-content-bounds - min_size_layout.cljc: child-min-width grid branch (3 sites) and child-min-height grid branch Added 7 new tests in geom_bounds_layout_nil_test.cljc covering all nil-bounds edge cases for flex, grid, and min-size layout paths. Registered in runner.cljc. Closes #10843 AI-assisted-by: qwen3.7-plus --- .../geom/shapes/flex_layout/bounds.cljc | 117 +++++----- .../geom/shapes/grid_layout/bounds.cljc | 54 ++--- .../common/geom/shapes/min_size_layout.cljc | 28 ++- .../geom_bounds_layout_nil_test.cljc | 213 ++++++++++++++++++ common/test/common_tests/runner.cljc | 2 + 5 files changed, 321 insertions(+), 93 deletions(-) create mode 100644 common/test/common_tests/geom_bounds_layout_nil_test.cljc diff --git a/common/src/app/common/geom/shapes/flex_layout/bounds.cljc b/common/src/app/common/geom/shapes/flex_layout/bounds.cljc index e6aef398f0..b434d99fc9 100644 --- a/common/src/app/common/geom/shapes/flex_layout/bounds.cljc +++ b/common/src/app/common/geom/shapes/flex_layout/bounds.cljc @@ -121,74 +121,79 @@ (defn layout-content-points [bounds parent children objects] - (let [parent-id (dm/get-prop parent :id) - parent-bounds @(get bounds parent-id) - reverse? (ctl/reverse? parent) - children (cond->> children (not reverse?) reverse)] + (let [parent-id (dm/get-prop parent :id) + parent-bounds (get bounds parent-id)] + (when-let [parent-bounds (some-> parent-bounds deref)] + (let [reverse? (ctl/reverse? parent) + children (cond->> children (not reverse?) reverse)] - (loop [children (seq children) - result (transient []) - correct-v (gpt/point 0)] + (loop [children (seq children) + result (transient []) + correct-v (gpt/point 0)] - (if (not children) - (persistent! result) + (if (not children) + (persistent! result) - (let [child (first children) - child-id (dm/get-prop child :id) - child-bounds @(get bounds child-id) - [margin-top margin-right margin-bottom margin-left] (ctl/child-margins child) + (let [child (first children) + child-id (dm/get-prop child :id) + child-bounds-ref (get bounds child-id) + child-bounds (some-> child-bounds-ref deref) + [margin-top margin-right margin-bottom margin-left] (ctl/child-margins child) - [child-bounds correct-v] - (if (or (ctl/fill-width? child) (ctl/fill-height? child)) - (child-layout-bound-points parent child parent-bounds child-bounds correct-v bounds objects) - [(->> child-bounds (map #(gpt/add % correct-v))) correct-v]) + [child-bounds correct-v] + (if (and child-bounds + (or (ctl/fill-width? child) (ctl/fill-height? child))) + (child-layout-bound-points parent child parent-bounds child-bounds correct-v bounds objects) + [(when child-bounds + (->> child-bounds (map #(gpt/add % correct-v)))) + correct-v]) - child-bounds - (when (d/not-empty? child-bounds) - (-> (gpo/parent-coords-bounds child-bounds parent-bounds) - (gpo/pad-points (- margin-top) (- margin-right) (- margin-bottom) (- margin-left))))] + child-bounds + (when (d/not-empty? child-bounds) + (-> (gpo/parent-coords-bounds child-bounds parent-bounds) + (gpo/pad-points (- margin-top) (- margin-right) (- margin-bottom) (- margin-left))))] - (recur (next children) - (cond-> result (some? child-bounds) (conj! child-bounds)) - correct-v)))))) + (recur (next children) + (cond-> result (some? child-bounds) (conj! child-bounds)) + correct-v)))))))) (defn layout-content-bounds [bounds {:keys [layout-padding] :as parent} children objects] - (let [parent-id (:id parent) - parent-bounds @(get bounds parent-id) + (let [parent-id (:id parent) + parent-bounds (get bounds parent-id)] + (when-let [parent-bounds (some-> parent-bounds deref)] + (let [row? (ctl/row? parent) + col? (ctl/col? parent) + space-around? (ctl/space-around? parent) + space-evenly? (ctl/space-evenly? parent) + content-evenly? (ctl/content-evenly? parent) + [layout-gap-row layout-gap-col] (ctl/gaps parent) - row? (ctl/row? parent) - col? (ctl/col? parent) - space-around? (ctl/space-around? parent) - space-evenly? (ctl/space-evenly? parent) - content-evenly? (ctl/content-evenly? parent) - [layout-gap-row layout-gap-col] (ctl/gaps parent) + row-pad (if (or (and col? space-evenly?) + (and col? space-around?) + (and row? content-evenly?)) + layout-gap-row + 0) - row-pad (if (or (and col? space-evenly?) - (and col? space-around?) - (and row? content-evenly?)) - layout-gap-row - 0) + col-pad (if (or (and row? space-evenly?) + (and row? space-around?) + (and col? content-evenly?)) + layout-gap-col + 0) - col-pad (if (or (and row? space-evenly?) - (and row? space-around?) - (and col? content-evenly?)) - layout-gap-col - 0) + {pad-top :p1 pad-right :p2 pad-bottom :p3 pad-left :p4} layout-padding + pad-top (+ (or pad-top 0) row-pad) + pad-right (+ (or pad-right 0) col-pad) + pad-bottom (+ (or pad-bottom 0) row-pad) + pad-left (+ (or pad-left 0) col-pad) - {pad-top :p1 pad-right :p2 pad-bottom :p3 pad-left :p4} layout-padding - pad-top (+ (or pad-top 0) row-pad) - pad-right (+ (or pad-right 0) col-pad) - pad-bottom (+ (or pad-bottom 0) row-pad) - pad-left (+ (or pad-left 0) col-pad) + layout-points + (layout-content-points bounds parent children objects)] - layout-points - (layout-content-points bounds parent children objects)] - - (if (d/not-empty? layout-points) - (-> layout-points - (gpo/merge-parent-coords-bounds parent-bounds) - (gpo/pad-points (- pad-top) (- pad-right) (- pad-bottom) (- pad-left))) - ;; Cannot create some bounds from the children so we return the parent's - parent-bounds))) + (if (d/not-empty? layout-points) + (-> layout-points + (gpo/merge-parent-coords-bounds parent-bounds) + (gpo/pad-points (- pad-top) (- pad-right) (- pad-bottom) (- pad-left))) + ;; Cannot create some bounds from the children so we return the parent's + parent-bounds))))) diff --git a/common/src/app/common/geom/shapes/grid_layout/bounds.cljc b/common/src/app/common/geom/shapes/grid_layout/bounds.cljc index 0628cf2060..caadff4766 100644 --- a/common/src/app/common/geom/shapes/grid_layout/bounds.cljc +++ b/common/src/app/common/geom/shapes/grid_layout/bounds.cljc @@ -12,36 +12,36 @@ (defn layout-content-points [bounds parent {:keys [row-tracks column-tracks]}] - (let [parent-id (:id parent) - parent-bounds @(get bounds parent-id) - - hv #(gpo/start-hv parent-bounds %) - vv #(gpo/start-vv parent-bounds %)] - (d/concat-vec - (->> row-tracks - (mapcat #(vector (:start-p %) - (gpt/add (:start-p %) (vv (:size %)))))) - (->> column-tracks - (mapcat #(vector (:start-p %) - (gpt/add (:start-p %) (hv (:size %))))))))) + (let [parent-id (:id parent) + parent-bounds (get bounds parent-id)] + (when-let [parent-bounds (some-> parent-bounds deref)] + (let [hv #(gpo/start-hv parent-bounds %) + vv #(gpo/start-vv parent-bounds %)] + (d/concat-vec + (->> row-tracks + (mapcat #(vector (:start-p %) + (gpt/add (:start-p %) (vv (:size %)))))) + (->> column-tracks + (mapcat #(vector (:start-p %) + (gpt/add (:start-p %) (hv (:size %))))))))))) (defn layout-content-bounds [bounds {:keys [layout-padding] :as parent} layout-data] - (let [parent-id (:id parent) - parent-bounds @(get bounds parent-id) + (let [parent-id (:id parent) + parent-bounds (get bounds parent-id)] + (when-let [parent-bounds (some-> parent-bounds deref)] + (let [{pad-top :p1 pad-right :p2 pad-bottom :p3 pad-left :p4} layout-padding + pad-top (or pad-top 0) + pad-right (or pad-right 0) + pad-bottom (or pad-bottom 0) + pad-left (or pad-left 0) - {pad-top :p1 pad-right :p2 pad-bottom :p3 pad-left :p4} layout-padding - pad-top (or pad-top 0) - pad-right (or pad-right 0) - pad-bottom (or pad-bottom 0) - pad-left (or pad-left 0) + layout-points (layout-content-points bounds parent layout-data)] - layout-points (layout-content-points bounds parent layout-data)] - - (if (d/not-empty? layout-points) - (-> layout-points - (gpo/merge-parent-coords-bounds parent-bounds) - (gpo/pad-points (- pad-top) (- pad-right) (- pad-bottom) (- pad-left))) - ;; Cannot create some bounds from the children so we return the parent's - parent-bounds))) + (if (d/not-empty? layout-points) + (-> layout-points + (gpo/merge-parent-coords-bounds parent-bounds) + (gpo/pad-points (- pad-top) (- pad-right) (- pad-bottom) (- pad-left))) + ;; Cannot create some bounds from the children so we return the parent's + parent-bounds))))) diff --git a/common/src/app/common/geom/shapes/min_size_layout.cljc b/common/src/app/common/geom/shapes/min_size_layout.cljc index 89f17871e3..57375098e9 100644 --- a/common/src/app/common/geom/shapes/min_size_layout.cljc +++ b/common/src/app/common/geom/shapes/min_size_layout.cljc @@ -31,13 +31,17 @@ (and (ctl/fill-width? child) (ctl/grid-layout? child)) - (let [children - (->> (cfh/get-immediate-children objects (:id child)) - (remove ctl/position-absolute?) - (map #(vector @(get bounds (:id %)) %))) - layout-data (gd/calc-layout-data child @(get bounds (:id child)) children bounds objects true)] - (max (ctl/child-min-width child) - (gpo/width-points (gb/layout-content-bounds bounds child layout-data)))) + (let [child-bounds-ref (get bounds (:id child))] + (if child-bounds-ref + (let [children + (->> (cfh/get-immediate-children objects (:id child)) + (remove ctl/position-absolute?) + (keep #(when-let [b (get bounds (:id %))] + [@b %]))) + layout-data (gd/calc-layout-data child @child-bounds-ref children bounds objects true)] + (max (ctl/child-min-width child) + (gpo/width-points (gb/layout-content-bounds bounds child layout-data)))) + (ctl/child-min-width child))) (ctl/fill-width? child) (ctl/child-min-width child) @@ -63,11 +67,15 @@ (let [children (->> (cfh/get-immediate-children objects (dm/get-prop child :id)) (remove ctl/position-absolute?) - (map (fn [child] [@(get bounds (:id child)) child]))) + (keep (fn [c] + (when-let [b (get bounds (:id c))] + [@b c])))) layout-data (gd/calc-layout-data child (:points child) children bounds objects true) auto-bounds (gb/layout-content-bounds bounds child layout-data)] - (max (ctl/child-min-height child) - (gpo/height-points auto-bounds))) + (if auto-bounds + (max (ctl/child-min-height child) + (gpo/height-points auto-bounds)) + (ctl/child-min-height child))) (ctl/fill-height? child) (ctl/child-min-height child) diff --git a/common/test/common_tests/geom_bounds_layout_nil_test.cljc b/common/test/common_tests/geom_bounds_layout_nil_test.cljc new file mode 100644 index 0000000000..db070ce4f4 --- /dev/null +++ b/common/test/common_tests/geom_bounds_layout_nil_test.cljc @@ -0,0 +1,213 @@ +;; 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 INC Sucursal en España SL + +(ns common-tests.geom-bounds-layout-nil-test + (:require + [app.common.data :as d] + [app.common.geom.bounds-map :as gbm] + [app.common.geom.point :as gpt] + [app.common.geom.rect :as grc] + [app.common.geom.shapes.flex-layout.bounds :as fb] + [app.common.geom.shapes.grid-layout.bounds :as gb] + [app.common.geom.shapes.min-size-layout :as msl] + [app.common.types.shape :as cts] + [app.common.types.shape.layout :as ctl] + [app.common.uuid :as uuid] + [clojure.test :as t])) + +;; ---- Helpers ---- + +(defn- make-rect + [id x y w h] + (-> (cts/setup-shape {:id id + :type :rect + :name (str "rect-" id) + :x x :y y :width w :height h}) + (assoc :parent-id uuid/zero + :frame-id uuid/zero))) + +(defn- make-flex-frame + [id child-ids & {:keys [x y w h dir] + :or {x 0 y 0 w 200 h 200 dir :row}}] + (-> (cts/setup-shape {:id id + :type :frame + :name (str "flex-" id) + :layout :flex + :layout-flex-dir dir + :x x :y y :width w :height h}) + (assoc :parent-id uuid/zero + :frame-id uuid/zero + :shapes (vec child-ids)))) + +(defn- make-grid-frame + [id child-ids & {:keys [x y w h dir] + :or {x 0 y 0 w 200 h 200 dir :row}}] + (let [cell-id (uuid/next)] + (-> (cts/setup-shape {:id id + :type :frame + :name (str "grid-" id) + :layout :grid + :layout-grid-dir dir + :layout-grid-columns [{:type :flex :value 1}] + :layout-grid-rows [{:type :flex :value 1}] + :layout-grid-cells + {cell-id {:id cell-id + :row 1 + :row-span 1 + :column 1 + :column-span 1 + :shapes (vec child-ids)}} + :layout-padding-type :multiple + :layout-padding {:p1 0 :p2 0 :p3 0 :p4 0} + :layout-gap {:column-gap 0 :row-gap 0} + :x x :y y :width w :height h}) + (assoc :parent-id uuid/zero + :frame-id uuid/zero + :shapes (vec child-ids))))) + +(defn- make-objects + [shapes] + (let [shape-map (into {} (map (fn [s] [(:id s) s]) shapes))] + (reduce-kv (fn [m _id shape] + (if (contains? shape :shapes) + (reduce (fn [m' child-id] + (assoc-in m' [child-id :parent-id] (:id shape))) + m + (:shapes shape)) + m)) + shape-map + shape-map))) + +(defn- bounds-map-from-objects + "Build a bounds map from objects, optionally excluding some IDs." + [objects & {:keys [exclude-ids]}] + (let [full (gbm/objects->bounds-map objects)] + (if (seq exclude-ids) + (apply dissoc full exclude-ids) + full))) + +;; ---- Tests for flex layout bounds with nil bounds ---- + +(t/deftest layout-content-points-with-missing-parent-bounds + (t/testing "layout-content-points returns nil when parent is not in bounds map" + (let [child-id (uuid/next) + parent-id (uuid/next) + child (make-rect child-id 10 10 50 50) + parent (make-flex-frame parent-id [child-id]) + objects (make-objects [parent child]) + bounds (bounds-map-from-objects objects :exclude-ids #{parent-id})] + + (t/is (nil? (fb/layout-content-points bounds parent [child] objects)))))) + +(t/deftest layout-content-points-with-missing-child-bounds + (t/testing "layout-content-points skips children with missing bounds" + (let [child1-id (uuid/next) + child2-id (uuid/next) + parent-id (uuid/next) + child1 (make-rect child1-id 10 10 50 50) + child2 (make-rect child2-id 70 10 50 50) + parent (make-flex-frame parent-id [child1-id child2-id]) + objects (make-objects [parent child1 child2]) + bounds (bounds-map-from-objects objects :exclude-ids #{child1-id})] + + (let [result (fb/layout-content-points bounds parent [child1 child2] objects)] + (t/is (some? result)) + ;; Only child2's bounds should be in the result + (t/is (pos? (count result))))))) + +(t/deftest layout-content-bounds-with-missing-parent-bounds + (t/testing "layout-content-bounds returns nil when parent is not in bounds map" + (let [child-id (uuid/next) + parent-id (uuid/next) + child (make-rect child-id 10 10 50 50) + parent (make-flex-frame parent-id [child-id]) + objects (make-objects [parent child]) + bounds (bounds-map-from-objects objects :exclude-ids #{parent-id})] + + (t/is (nil? (fb/layout-content-bounds bounds parent [child] objects)))))) + +;; ---- Tests for grid layout bounds with nil bounds ---- + +(t/deftest grid-layout-content-points-with-missing-parent-bounds + (t/testing "grid layout-content-points returns nil when parent is not in bounds map" + (let [parent-id (uuid/next) + parent (make-grid-frame parent-id []) + objects (make-objects [parent]) + bounds (bounds-map-from-objects objects :exclude-ids #{parent-id}) + layout-data {:row-tracks [{:start-p (gpt/point 0 0) :size 100}] + :column-tracks [{:start-p (gpt/point 0 0) :size 100}]}] + + (t/is (nil? (gb/layout-content-points bounds parent layout-data)))))) + +(t/deftest grid-layout-content-bounds-with-missing-parent-bounds + (t/testing "grid layout-content-bounds returns nil when parent is not in bounds map" + (let [parent-id (uuid/next) + parent (make-grid-frame parent-id []) + objects (make-objects [parent]) + bounds (bounds-map-from-objects objects :exclude-ids #{parent-id}) + layout-data {:row-tracks [{:start-p (gpt/point 0 0) :size 100}] + :column-tracks [{:start-p (gpt/point 0 0) :size 100}]}] + + (t/is (nil? (gb/layout-content-bounds bounds parent layout-data)))))) + +;; ---- Tests for min-size-layout with nil bounds ---- + +(t/deftest child-min-width-grid-with-missing-child-bounds + (t/testing "child-min-width falls back when grid layout child bounds are missing" + (let [grandchild-id (uuid/next) + child-id (uuid/next) + grandchild (make-rect grandchild-id 0 0 30 30) + child (-> (make-grid-frame child-id [grandchild-id] :w 100 :h 100) + (assoc :layout-grid-dir :row + :layout-item-h-sizing :fill)) + objects (make-objects [child grandchild]) + ;; Exclude grandchild from bounds to simulate missing entry + bounds (bounds-map-from-objects objects :exclude-ids #{grandchild-id}) + child-bounds (grc/rect->points (grc/make-rect 0 0 100 100))] + + (let [result (msl/child-min-width child child-bounds bounds objects)] + (t/is (= (ctl/child-min-width child) result)))))) + +(t/deftest child-min-height-grid-with-missing-child-bounds + (t/testing "child-min-height falls back when grid layout child bounds are missing" + (let [grandchild-id (uuid/next) + child-id (uuid/next) + grandchild (make-rect grandchild-id 0 0 30 30) + child (-> (make-grid-frame child-id [grandchild-id] :w 100 :h 100) + (assoc :layout-grid-dir :column + :layout-item-v-sizing :fill)) + objects (make-objects [child grandchild]) + bounds (bounds-map-from-objects objects :exclude-ids #{grandchild-id}) + child-bounds (grc/rect->points (grc/make-rect 0 0 100 100))] + + (let [result (msl/child-min-height child child-bounds bounds objects)] + (t/is (= (ctl/child-min-height child) result)))))) + +(t/deftest child-min-width-grid-with-present-child-bounds + (t/testing "child-min-width handles bounded children in a fill-width grid" + (let [grandchild-id (uuid/next) + child-id (uuid/next) + grandchild (make-rect grandchild-id 0 0 30 30) + child (-> (make-grid-frame child-id [grandchild-id] :w 100 :h 100) + (assoc :layout-item-h-sizing :fill)) + objects (make-objects [child grandchild]) + bounds (bounds-map-from-objects objects) + child-bounds (grc/rect->points (grc/make-rect 0 0 100 100))] + + (t/is (number? (msl/child-min-width child child-bounds bounds objects)))))) + +(t/deftest child-min-height-grid-with-present-child-bounds + (t/testing "child-min-height handles bounded children in a fill-height grid" + (let [grandchild-id (uuid/next) + child-id (uuid/next) + grandchild (make-rect grandchild-id 0 0 30 30) + child (-> (make-grid-frame child-id [grandchild-id] :w 100 :h 100) + (assoc :layout-item-v-sizing :fill)) + objects (make-objects [child grandchild]) + bounds (bounds-map-from-objects objects) + child-bounds (grc/rect->points (grc/make-rect 0 0 100 100))] + + (t/is (number? (msl/child-min-height child child-bounds bounds objects)))))) diff --git a/common/test/common_tests/runner.cljc b/common/test/common_tests/runner.cljc index 7738c5dd9e..d3d2f7d48e 100644 --- a/common/test/common_tests/runner.cljc +++ b/common/test/common_tests/runner.cljc @@ -22,6 +22,7 @@ [common-tests.files.shapes-builder-test] [common-tests.files.validate-test] [common-tests.geom-align-test] + [common-tests.geom-bounds-layout-nil-test] [common-tests.geom-bounds-map-test] [common-tests.geom-flex-layout-test] [common-tests.geom-grid-layout-test] @@ -95,6 +96,7 @@ 'common-tests.files-migrations-test 'common-tests.files.validate-test 'common-tests.geom-align-test + 'common-tests.geom-bounds-layout-nil-test 'common-tests.geom-bounds-map-test 'common-tests.geom-flex-layout-test 'common-tests.geom-grid-layout-test