From ed418d3fc9548f3cd8a219d02e92caedc2cc93d4 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 26 Mar 2026 13:40:43 +0000 Subject: [PATCH] :bug: Fix nil path content crash by exposing safe public API Move nil-safety for path segment helpers to the public API layer (app.common.types.path) rather than the low-level segment namespace. Add nil-safe wrappers for get-handlers, opposite-index, get-handler-point, get-handler, handler->node, point-indices, handler-indices, next-node, append-segment, points->content, closest-point, make-corner-point, make-curve-point, split-segments, remove-nodes, merge-nodes, join-nodes, and separate-nodes. Update all frontend callers to use path/ instead of path.segment/ for these functions, removing the path.segment require from helpers, drawing, edition, tools, curve, editor and debug. Replace ad-hoc nil checks with impl/path-data coercion in all public wrapper functions in app.common.types.path. The path-data helper already handles nil by returning an empty PathData instance, which provides uniform nil-safety across all content-accepting functions. Update the path-get-points-nil-safe test to expect empty collection instead of nil, matching the new coercion behavior. --- common/src/app/common/types/path.cljc | 123 +++++++++++++++++- .../common_tests/types/path_data_test.cljc | 4 +- .../main/data/workspace/drawing/curve.cljs | 12 +- .../app/main/data/workspace/path/drawing.cljs | 7 +- .../app/main/data/workspace/path/edition.cljs | 13 +- .../app/main/data/workspace/path/helpers.cljs | 15 +-- .../app/main/data/workspace/path/tools.cljs | 15 +-- .../app/main/ui/workspace/shapes/debug.cljs | 5 +- .../main/ui/workspace/shapes/path/editor.cljs | 13 +- 9 files changed, 155 insertions(+), 52 deletions(-) diff --git a/common/src/app/common/types/path.cljc b/common/src/app/common/types/path.cljc index 67d8031efb..84e4756f0b 100644 --- a/common/src/app/common/types/path.cljc +++ b/common/src/app/common/types/path.cljc @@ -191,19 +191,128 @@ (defn get-points "Returns points for the given content. Accepts PathData instances or - plain segment vectors. Returns nil for nil content." + plain segment vectors." [content] - (when (some? content) - (let [content (if (impl/path-data? content) - content - (impl/path-data content))] - (segment/get-points content)))) + (let [content (impl/path-data content)] + (segment/get-points content))) (defn calc-selrect "Calculate selrect from a content. The content can be in a PathData instance or plain vector of segments." [content] - (segment/content->selrect content)) + (let [content (impl/path-data content)] + (segment/content->selrect content))) + +(defn get-handlers + "Retrieve a map where for every point will retrieve a list of the + handlers that are associated with that point. + point -> [[index, prefix]]" + [content] + (let [content (impl/path-data content)] + (segment/get-handlers content))) + +(defn get-handler-point + "Given a content, segment index and prefix, get a handler point." + [content index prefix] + (let [content (impl/path-data content)] + (segment/get-handler-point content index prefix))) + +(defn get-handler + "Given a segment (command map) and a prefix, returns the handler + coordinate map {:x ... :y ...} from its params, or nil when absent." + [command prefix] + (segment/get-handler command prefix)) + +(defn handler->node + "Given a content, index and prefix, returns the path node (anchor + point) that the handler belongs to." + [content index prefix] + (let [content (impl/path-data content)] + (segment/handler->node content index prefix))) + +(defn opposite-index + "Calculates the opposite handler index given a content, index and + prefix." + [content index prefix] + (let [content (impl/path-data content)] + (segment/opposite-index content index prefix))) + +(defn point-indices + "Returns the indices of all segments whose endpoint matches point." + [content point] + (let [content (impl/path-data content)] + (segment/point-indices content point))) + +(defn handler-indices + "Returns [[index prefix] ...] of all handlers associated with point." + [content point] + (let [content (impl/path-data content)] + (segment/handler-indices content point))) + +(defn next-node + "Calculates the next node segment to be inserted when drawing." + [content position prev-point prev-handler] + (let [content (impl/path-data content)] + (segment/next-node content position prev-point prev-handler))) + +(defn append-segment + "Appends a segment to content, accepting PathData or plain vector." + [content segment] + (let [content (impl/path-data content)] + (segment/append-segment content segment))) + +(defn points->content + "Given a vector of points generate a path content." + [points & {:keys [close]}] + (segment/points->content points :close close)) + +(defn closest-point + "Returns the closest point in the path to position, at a given precision." + [content position precision] + (let [content (impl/path-data content)] + (segment/closest-point content position precision))) + +(defn make-corner-point + "Changes the content to make a point a corner." + [content point] + (let [content (impl/path-data content)] + (segment/make-corner-point content point))) + +(defn make-curve-point + "Changes the content to make a point a curve." + [content point] + (let [content (impl/path-data content)] + (segment/make-curve-point content point))) + +(defn split-segments + "Given a content, splits segments between points with new segments." + [content points value] + (let [content (impl/path-data content)] + (segment/split-segments content points value))) + +(defn remove-nodes + "Removes the given points from content, reconstructing paths as needed." + [content points] + (let [content (impl/path-data content)] + (segment/remove-nodes content points))) + +(defn merge-nodes + "Reduces contiguous segments at the given points to a single point." + [content points] + (let [content (impl/path-data content)] + (segment/merge-nodes content points))) + +(defn join-nodes + "Creates new segments between points that weren't previously connected." + [content points] + (let [content (impl/path-data content)] + (segment/join-nodes content points))) + +(defn separate-nodes + "Removes the segments between the given points." + [content points] + (let [content (impl/path-data content)] + (segment/separate-nodes content points))) (defn- calc-bool-content* "Calculate the boolean content from shape and objects. Returns plain diff --git a/common/test/common_tests/types/path_data_test.cljc b/common/test/common_tests/types/path_data_test.cljc index d1a70707b2..9cfe37fd11 100644 --- a/common/test/common_tests/types/path_data_test.cljc +++ b/common/test/common_tests/types/path_data_test.cljc @@ -273,8 +273,8 @@ (t/is (= result2 result3)))) (t/deftest path-get-points-nil-safe - (t/testing "path/get-points returns nil for nil content without throwing" - (t/is (nil? (path/get-points nil)))) + (t/testing "path/get-points returns empty for nil content without throwing" + (t/is (empty? (path/get-points nil)))) (t/testing "path/get-points returns correct points for valid content" (let [content (path/content sample-content) points (path/get-points content)] diff --git a/frontend/src/app/main/data/workspace/drawing/curve.cljs b/frontend/src/app/main/data/workspace/drawing/curve.cljs index 9846a05ccf..0b554b5797 100644 --- a/frontend/src/app/main/data/workspace/drawing/curve.cljs +++ b/frontend/src/app/main/data/workspace/drawing/curve.cljs @@ -11,7 +11,7 @@ [app.common.geom.shapes.flex-layout :as gslf] [app.common.geom.shapes.grid-layout :as gslg] [app.common.types.container :as ctn] - [app.common.types.path.segment :as path.segment] + [app.common.types.path :as path] [app.common.types.shape :as cts] [app.common.types.shape-tree :as ctst] [app.common.types.shape.layout :as ctl] @@ -33,7 +33,7 @@ (update [_ state] (let [objects (dsh/lookup-page-objects state) content (dm/get-in state [:workspace-drawing :object :content]) - position (path.segment/get-handler-point content 0 nil) + position (path/get-handler-point content 0 nil) frame-id (->> (ctst/top-nested-frame objects position) (ctn/get-first-valid-parent objects) ;; We don't want to change the structure of component copies @@ -65,8 +65,8 @@ (fn [object] (let [points (-> (::points object) (conj point)) - content (path.segment/points->content points) - selrect (path.segment/content->selrect content) + content (path/points->content points) + selrect (path/calc-selrect content) points' (grc/rect->points selrect)] (-> object (assoc ::points points) @@ -82,8 +82,8 @@ (update-in state [:workspace-drawing :object] (fn [{:keys [::points] :as shape}] (let [points (ups/simplify points simplify-tolerance) - content (path.segment/points->content points) - selrect (path.segment/content->selrect content) + content (path/points->content points) + selrect (path/calc-selrect content) points (grc/rect->points selrect)] (-> shape diff --git a/frontend/src/app/main/data/workspace/path/drawing.cljs b/frontend/src/app/main/data/workspace/path/drawing.cljs index d7a5409f1b..19923b5264 100644 --- a/frontend/src/app/main/data/workspace/path/drawing.cljs +++ b/frontend/src/app/main/data/workspace/path/drawing.cljs @@ -13,7 +13,6 @@ [app.common.types.container :as ctn] [app.common.types.path :as path] [app.common.types.path.helpers :as path.helpers] - [app.common.types.path.segment :as path.segment] [app.common.types.shape :as cts] [app.common.types.shape-tree :as ctst] [app.common.types.shape.layout :as ctl] @@ -64,7 +63,7 @@ {:keys [last-point prev-handler]} (get-in state [:workspace-local :edit-path id]) - segment (path.segment/next-node shape position last-point prev-handler)] + segment (path/next-node shape position last-point prev-handler)] (assoc-in state [:workspace-local :edit-path id :preview] segment))))) (defn add-node @@ -99,7 +98,7 @@ prefix (or prefix :c1) position (or position (path.helpers/segment->point (nth content (dec index)))) - old-handler (path.segment/get-handler-point content index prefix) + old-handler (path/get-handler-point content index prefix) handler-position (cond-> (gpt/point x y) shift? (path.helpers/position-fixed-angle position)) @@ -148,7 +147,7 @@ ptk/WatchEvent (watch [_ state stream] (let [content (st/get-path state :content) - handlers (-> (path.segment/get-handlers content) + handlers (-> (path/get-handlers content) (get position)) [idx prefix] (when (= (count handlers) 1) diff --git a/frontend/src/app/main/data/workspace/path/edition.cljs b/frontend/src/app/main/data/workspace/path/edition.cljs index f5118f16ab..eade6bcb1e 100644 --- a/frontend/src/app/main/data/workspace/path/edition.cljs +++ b/frontend/src/app/main/data/workspace/path/edition.cljs @@ -11,7 +11,6 @@ [app.common.geom.point :as gpt] [app.common.types.path :as path] [app.common.types.path.helpers :as path.helpers] - [app.common.types.path.segment :as path.segment] [app.main.data.changes :as dch] [app.main.data.helpers :as dsh] [app.main.data.workspace.edition :as dwe] @@ -74,8 +73,8 @@ (defn modify-content-point [content {dx :x dy :y} modifiers point] - (let [point-indices (path.segment/point-indices content point) ;; [indices] - handler-indices (path.segment/handler-indices content point) ;; [[index prefix]] + (let [point-indices (path/point-indices content point) ;; [indices] + handler-indices (path/handler-indices content point) ;; [[index prefix]] modify-point (fn [modifiers index] @@ -258,10 +257,10 @@ points (path/get-points content) point (-> content (nth (if (= prefix :c1) (dec index) index)) (path.helpers/segment->point)) - handler (-> content (nth index) (path.segment/get-handler prefix)) + handler (-> content (nth index) (path/get-handler prefix)) - [op-idx op-prefix] (path.segment/opposite-index content index prefix) - opposite (path.segment/get-handler-point content op-idx op-prefix)] + [op-idx op-prefix] (path/opposite-index content index prefix) + opposite (path/get-handler-point content op-idx op-prefix)] (streams/drag-stream (rx/concat @@ -344,7 +343,7 @@ (-> state (assoc-in [:workspace-local :edit-path id :old-content] content) (st/set-content (-> content - (path.segment/split-segments #{from-p to-p} t) + (path/split-segments #{from-p to-p} t) (path/content)))))) ptk/WatchEvent diff --git a/frontend/src/app/main/data/workspace/path/helpers.cljs b/frontend/src/app/main/data/workspace/path/helpers.cljs index c904e388f5..a403671f15 100644 --- a/frontend/src/app/main/data/workspace/path/helpers.cljs +++ b/frontend/src/app/main/data/workspace/path/helpers.cljs @@ -9,15 +9,14 @@ [app.common.geom.point :as gpt] [app.common.math :as mth] [app.common.types.path :as path] - [app.common.types.path.helpers :as path.helpers] - [app.common.types.path.segment :as path.segment])) + [app.common.types.path.helpers :as path.helpers])) (defn append-node "Creates a new node in the path. Usually used when drawing." [shape position prev-point prev-handler] - (let [segment (path.segment/next-node (:content shape) position prev-point prev-handler)] + (let [segment (path/next-node (:content shape) position prev-point prev-handler)] (-> shape - (update :content path.segment/append-segment segment) + (update :content path/append-segment segment) (path/update-geometry)))) (defn angle-points [common p1 p2] @@ -61,11 +60,11 @@ [content index prefix match-distance? match-angle? dx dy] (let [[cx cy] (path.helpers/prefix->coords prefix) - [op-idx op-prefix] (path.segment/opposite-index content index prefix) + [op-idx op-prefix] (path/opposite-index content index prefix) - node (path.segment/handler->node content index prefix) - handler (path.segment/get-handler-point content index prefix) - opposite (path.segment/get-handler-point content op-idx op-prefix) + node (path/handler->node content index prefix) + handler (path/get-handler-point content index prefix) + opposite (path/get-handler-point content op-idx op-prefix) [ocx ocy] (path.helpers/prefix->coords op-prefix) [odx ody] (calculate-opposite-delta node handler opposite match-angle? match-distance? dx dy) diff --git a/frontend/src/app/main/data/workspace/path/tools.cljs b/frontend/src/app/main/data/workspace/path/tools.cljs index 0fd108f41c..76429452db 100644 --- a/frontend/src/app/main/data/workspace/path/tools.cljs +++ b/frontend/src/app/main/data/workspace/path/tools.cljs @@ -8,7 +8,6 @@ (:require [app.common.data.macros :as dm] [app.common.types.path :as path] - [app.common.types.path.segment :as path.segment] [app.main.data.changes :as dch] [app.main.data.helpers :as dsh] [app.main.data.workspace.edition :as dwe] @@ -59,7 +58,7 @@ (process-path-tool (when point #{point}) (fn [content points] - (reduce path.segment/make-corner-point content points))))) + (reduce path/make-corner-point content points))))) (defn make-curve ([] @@ -68,22 +67,22 @@ (process-path-tool (when point #{point}) (fn [content points] - (reduce path.segment/make-curve-point content points))))) + (reduce path/make-curve-point content points))))) (defn add-node [] - (process-path-tool (fn [content points] (path.segment/split-segments content points 0.5)))) + (process-path-tool (fn [content points] (path/split-segments content points 0.5)))) (defn remove-node [] - (process-path-tool path.segment/remove-nodes)) + (process-path-tool path/remove-nodes)) (defn merge-nodes [] - (process-path-tool path.segment/merge-nodes)) + (process-path-tool path/merge-nodes)) (defn join-nodes [] - (process-path-tool path.segment/join-nodes)) + (process-path-tool path/join-nodes)) (defn separate-nodes [] - (process-path-tool path.segment/separate-nodes)) + (process-path-tool path/separate-nodes)) (defn toggle-snap [] (ptk/reify ::toggle-snap diff --git a/frontend/src/app/main/ui/workspace/shapes/debug.cljs b/frontend/src/app/main/ui/workspace/shapes/debug.cljs index ce981a86a5..a1a97e65dc 100644 --- a/frontend/src/app/main/ui/workspace/shapes/debug.cljs +++ b/frontend/src/app/main/ui/workspace/shapes/debug.cljs @@ -15,7 +15,6 @@ [app.common.types.path :as path] [app.common.types.path.bool :as path.bool] [app.common.types.path.helpers :as path.helpers] - [app.common.types.path.segment :as path.segment] [app.common.types.path.subpath :as path.subpath] [app.main.refs :as refs] [app.util.color :as uc] @@ -124,8 +123,8 @@ (path.bool/add-previous)) - sr-a (path.segment/content->selrect content-a) - sr-b (path.segment/content->selrect content-b) + sr-a (path/calc-selrect content-a) + sr-b (path/calc-selrect content-b) [content-a-split content-b-split] (path.bool/content-intersect-split content-a content-b sr-a sr-b) diff --git a/frontend/src/app/main/ui/workspace/shapes/path/editor.cljs b/frontend/src/app/main/ui/workspace/shapes/path/editor.cljs index 24996169ac..8251f0daf4 100644 --- a/frontend/src/app/main/ui/workspace/shapes/path/editor.cljs +++ b/frontend/src/app/main/ui/workspace/shapes/path/editor.cljs @@ -11,7 +11,6 @@ [app.common.geom.point :as gpt] [app.common.types.path :as path] [app.common.types.path.helpers :as path.helpers] - [app.common.types.path.segment :as path.segment] [app.main.data.workspace.path :as drp] [app.main.snap :as snap] [app.main.store :as st] @@ -251,8 +250,8 @@ (defn- matching-handler? [content node handlers] (when (= 2 (count handlers)) (let [[[i1 p1] [i2 p2]] handlers - p1 (path.segment/get-handler-point content i1 p1) - p2 (path.segment/get-handler-point content i2 p2) + p1 (path/get-handler-point content i1 p1) + p2 (path/get-handler-point content i2 p2) v1 (gpt/to-vec node p1) v2 (gpt/to-vec node p2) @@ -309,7 +308,7 @@ handlers (mf/with-memo [content] - (path.segment/get-handlers content)) + (path/get-handlers content)) is-path-start (not (some? last-point)) @@ -331,7 +330,7 @@ ms/mouse-position (mf/deps base-content zoom) (fn [position] - (when-let [point (path.segment/closest-point base-content position (/ 0.01 zoom))] + (when-let [point (path/closest-point base-content position (/ 0.01 zoom))] (reset! hover-point (when (< (gpt/distance position point) (/ 10 zoom)) point))))) [:g.path-editor {:ref editor-ref} @@ -367,7 +366,7 @@ (fn [[index prefix]] ;; FIXME: get-handler-point is executed twice for each ;; render, this can be optimized - (let [handler-position (path.segment/get-handler-point content index prefix)] + (let [handler-position (path/get-handler-point content index prefix)] (not= position handler-position))) position-handlers @@ -390,7 +389,7 @@ [:g.path-node {:key (dm/str pos-x "-" pos-y)} [:g.point-handlers {:pointer-events (when (= edit-mode :draw) "none")} (for [[hindex prefix] position-handlers] - (let [handler-position (path.segment/get-handler-point content hindex prefix) + (let [handler-position (path/get-handler-point content hindex prefix) handler-hover? (contains? hover-handlers [hindex prefix]) moving-handler? (= handler-position moving-handler) matching-handler? (matching-handler? content position position-handlers)]