🐛 Fix nil path content crash by exposing safe public API (#8806)

* 🐛 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.

* ♻️ Clean up path segment dead code and add missing tests

Remove dead code from segment.cljc: opposite-handler (duplicate of
calculate-opposite-handler) and path-closest-point-accuracy (unused
constant). Make update-handler and calculate-extremities private as
they are only used internally within segment.cljc.

Add missing tests for path/handler-indices, path/closest-point,
path/make-curve-point and path/merge-nodes. Update extremities tests
to use the local reference implementation instead of the now-private
calculate-extremities. Remove tests for deleted/privatized functions.

Add empty-content guard in path/closest-point wrapper to prevent
ArityException when reducing over zero segments.
This commit is contained in:
Andrey Antukh 2026-04-07 18:54:14 +02:00 committed by GitHub
parent e10bd6a8d3
commit f8c04949e1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 233 additions and 93 deletions

View File

@ -191,19 +191,129 @@
(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)]
(when (pos? (count 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

View File

@ -19,7 +19,7 @@
#?(:clj (set! *warn-on-reflection* true))
(defn update-handler
(defn- update-handler
[command prefix point]
(let [[cox coy] (if (= prefix :c1) [:c1x :c1y] [:c2x :c2y])]
(-> command
@ -127,11 +127,6 @@
(let [handler-vector (gpt/to-vec point handler)]
(gpt/add point (gpt/negate handler-vector))))
(defn opposite-handler
"Calculates the coordinates of the opposite handler"
[point handler]
(let [phv (gpt/to-vec point handler)]
(gpt/add point (gpt/negate phv))))
(defn get-points
"Returns points for the given segment, faster version of
@ -178,8 +173,6 @@
(conj result [prev-point last-start]))))
(def ^:const path-closest-point-accuracy 0.01)
;; FIXME: move to helpers?, this function need performance review, it
;; is executed so many times on path edition
(defn- curve-closest-point
@ -787,7 +780,7 @@
(let [transform (gmt/translate-matrix move-vec)]
(transform-content content transform)))
(defn calculate-extremities
(defn- calculate-extremities
"Calculate extremities for the provided content"
[content]
(loop [points (transient #{})

View File

@ -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)]
@ -325,18 +325,12 @@
(let [pdata (path/content sample-content)
result1 (calculate-extremities sample-content)
result2 (calculate-extremities pdata)
result3 (path.segment/calculate-extremities sample-content)
result4 (path.segment/calculate-extremities pdata)
expect #{(gpt/point 480.0 839.0)
(gpt/point 439.0 802.0)
(gpt/point 264.0 634.0)}
n-iter 100000]
(gpt/point 264.0 634.0)}]
(t/is (= result1 result3))
(t/is (= result1 expect))
(t/is (= result2 expect))
(t/is (= result3 expect))
(t/is (= result4 expect))))
(t/is (= result2 expect))))
(def sample-content-2
[{:command :move-to, :params {:x 480.0, :y 839.0}}
@ -346,21 +340,17 @@
{:command :close-path :params {}}])
(t/deftest extremities-2
(let [result1 (path.segment/calculate-extremities sample-content-2)
result2 (calculate-extremities sample-content-2)]
(t/is (= result1 result2))))
(let [result1 (calculate-extremities sample-content-2)]
(t/is (some? result1))))
(t/deftest extremities-3
(let [segments [{:command :move-to, :params {:x -310.5355224609375, :y 452.62115478515625}}]
content (path/content segments)
result1 (calculate-extremities segments)
result2 (path.segment/calculate-extremities segments)
result3 (path.segment/calculate-extremities content)
result2 (calculate-extremities content)
expect #{}]
(t/is (= result1 expect))
(t/is (= result1 expect))
(t/is (= result2 expect))
(t/is (= result3 expect))))
(t/is (= result2 expect))))
(t/deftest points-to-content
(let [initial [(gpt/point 0.0 0.0)
@ -926,17 +916,9 @@
(t/is (some? e))))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; SEGMENT UNTESTED FUNCTIONS
;; SEGMENT FUNCTIONS
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(t/deftest segment-update-handler
(let [cmd {:command :curve-to
:params {:x 10.0 :y 0.0 :c1x 0.0 :c1y 0.0 :c2x 0.0 :c2y 0.0}}
pt (gpt/point 3.0 5.0)
r (path.segment/update-handler cmd :c1 pt)]
(t/is (= 3.0 (get-in r [:params :c1x])))
(t/is (= 5.0 (get-in r [:params :c1y])))))
(t/deftest segment-get-handler
(let [cmd {:command :curve-to
:params {:x 10.0 :y 0.0 :c1x 3.0 :c1y 5.0 :c2x 7.0 :c2y 2.0}}]
@ -960,13 +942,6 @@
(t/is (mth/close? 2.0 (:x opp)))
(t/is (mth/close? 5.0 (:y opp)))))
(t/deftest segment-opposite-handler
(let [pt (gpt/point 5.0 5.0)
h (gpt/point 8.0 5.0)
opp (path.segment/opposite-handler pt h)]
(t/is (mth/close? 2.0 (:x opp)))
(t/is (mth/close? 5.0 (:y opp)))))
(t/deftest segment-point-indices
(let [content (path/content sample-content-2)
pt (gpt/point 480.0 839.0)
@ -1139,6 +1114,74 @@
(t/is (mth/close? (+ 480.0 5.0) (get-in first-seg [:params :x])))
(t/is (mth/close? (+ 839.0 3.0) (get-in first-seg [:params :y])))))
(t/deftest path-handler-indices
(t/testing "handler-indices returns expected handlers for a curve point"
(let [content (path/content sample-content-2)
;; point at index 2 is (4.0, 4.0), which is a curve-to endpoint
pt (gpt/point 4.0 4.0)
result (path/handler-indices content pt)]
;; The :c2 handler of index 2 belongs to point (4.0, 4.0)
;; The :c1 handler of index 3 also belongs to point (4.0, 4.0)
(t/is (some? result))
(t/is (pos? (count result)))
(t/is (every? (fn [[idx prefix]]
(and (number? idx)
(#{:c1 :c2} prefix)))
result))))
(t/testing "handler-indices returns empty for a point with no associated handlers"
(let [content (path/content sample-content-2)
;; (480.0, 839.0) is the move-to at index 0; since index 1
;; is a line-to (not a curve-to), there is no :c1 handler
;; for this point.
pt (gpt/point 480.0 839.0)
result (path/handler-indices content pt)]
(t/is (empty? result))))
(t/testing "handler-indices with nil content returns empty"
(let [result (path/handler-indices nil (gpt/point 0 0))]
(t/is (empty? result)))))
(t/deftest path-closest-point
(t/testing "closest-point on a line segment"
(let [content (path/content simple-open-content)
;; simple-open-content: (0,0)->(10,0)->(10,10)
;; Query a point near the first segment
pos (gpt/point 5.0 1.0)
result (path/closest-point content pos 0.01)]
(t/is (some? result))
;; Closest point on line (0,0)->(10,0) to (5,1) should be near (5,0)
(t/is (mth/close? 5.0 (:x result) 0.5))
(t/is (mth/close? 0.0 (:y result) 0.5))))
(t/testing "closest-point on nil content returns nil"
(let [result (path/closest-point nil (gpt/point 5.0 5.0) 0.01)]
(t/is (nil? result)))))
(t/deftest path-make-curve-point
(t/testing "make-curve-point converts a line-to point into a curve"
(let [content (path/content simple-open-content)
;; The midpoint (10,0) is reached via :line-to
pt (gpt/point 10.0 0.0)
result (path/make-curve-point content pt)
segs (vec result)]
(t/is (some? result))
;; After making (10,0) a curve, we expect at least one :curve-to
(t/is (some #(= :curve-to (:command %)) segs)))))
(t/deftest path-merge-nodes
(t/testing "merge-nodes reduces segments at contiguous points"
(let [content (path/content simple-open-content)
;; Merge the midpoint (10,0) — should reduce segment count
pts #{(gpt/point 10.0 0.0)}
result (path/merge-nodes content pts)]
(t/is (some? result))
(t/is (<= (count result) (count simple-open-content)))))
(t/testing "merge-nodes with empty points returns same content"
(let [content (path/content simple-open-content)
result (path/merge-nodes content #{})]
(t/is (= (count result) (count simple-open-content)))))
(t/testing "merge-nodes with nil content does not throw"
(let [result (path/merge-nodes nil #{(gpt/point 0 0)})]
(t/is (some? result)))))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; BOOL OPERATIONS — INTERSECTION / DIFFERENCE / EXCLUSION
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

View File

@ -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

View File

@ -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)

View File

@ -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

View File

@ -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)

View File

@ -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

View File

@ -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)

View File

@ -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)]