mirror of
https://github.com/penpot/penpot.git
synced 2026-05-30 04:08:08 +00:00
⚡ Improve performance and fix orphan detection in validate-file (#9789)
* ⚡ Improve performance and fix orphan detection in validate-file - Add `*ref-shape-cache*` dynamic var to memoize `find-ref-shape` lookups per page, avoiding repeated O(depth) ancestor walks. - Add `*children-sets*` pre-computed maps for O(1) parent-child containment checks, replacing linear `some` scans. - Short-circuit `inside-component-main?` when the shape context already implies a main component. - Use single-pass reduce with early exit for duplicate detection (children, swap slots) instead of count/distinct or frequencies. - Guard `check-missing-slot` to skip expensive `find-near-match` when the shape already has a swap slot. - Refactor variant-set validation to use `run!` with direct `get`. - Refactor `check-ref-cycles` to use a single `reduce-kv` pass. - Fix `get-orphan-shapes`: the original `map` pipeline produced nils so orphan shapes were never validated; rewrite with `reduce-kv` for correct results. - Add `validate-file-affected!` for change-scoped validation, replacing full file validation in `process-changes-and-validate` to only validate pages and components touched by the changes. Signed-off-by: Andrey Antukh <niwi@niwi.nz> * ✨ Improved validation --------- Signed-off-by: Andrey Antukh <niwi@niwi.nz> Co-authored-by: alonso.torres <alonso.torres@kaleidos.net>
This commit is contained in:
Andrey Antukh
GitHub
parent
30bba7cd38
commit
40ce360c99
@ -379,19 +379,16 @@
|
||||
(l/error :hint "file schema validation error" :cause cause))))
|
||||
|
||||
(defn- soft-validate-file!
|
||||
[file libs]
|
||||
[file libs changes]
|
||||
(try
|
||||
(val/validate-file! file libs)
|
||||
(val/validate-file-affected! file libs changes)
|
||||
(catch Throwable cause
|
||||
(l/error :hint "file validation error"
|
||||
:cause cause))))
|
||||
|
||||
|
||||
(defn- process-changes-and-validate
|
||||
[cfg file changes skip-validate]
|
||||
(let [;; WARNING: this ruins performance; maybe we need to find
|
||||
;; some other way to do general validation
|
||||
libs
|
||||
(let [libs
|
||||
(when (and (or (contains? cf/flags :file-validation)
|
||||
(contains? cf/flags :soft-file-validation))
|
||||
(not skip-validate))
|
||||
@ -419,14 +416,14 @@
|
||||
|
||||
(binding [pmap/*tracked* nil]
|
||||
(when (contains? cf/flags :soft-file-validation)
|
||||
(soft-validate-file! file libs))
|
||||
(soft-validate-file! file libs changes))
|
||||
|
||||
(when (contains? cf/flags :soft-file-schema-validation)
|
||||
(soft-validate-file-schema! file))
|
||||
|
||||
(when (and (contains? cf/flags :file-validation)
|
||||
(not skip-validate))
|
||||
(val/validate-file! file libs))
|
||||
(val/validate-file-affected! file libs changes))
|
||||
|
||||
(when (and (contains? cf/flags :file-schema-validation)
|
||||
(not skip-validate))
|
||||
|
||||
@ -644,9 +644,10 @@
|
||||
(fn [shape]
|
||||
;; Set the desired swap slot
|
||||
(let [slot (:swap-slot args)]
|
||||
(when (some? slot)
|
||||
(log/debug :hint (str " -> set swap-slot to " slot))
|
||||
(ctk/set-swap-slot shape slot))))]
|
||||
(if (some? slot)
|
||||
(do (log/debug :hint (str " -> set swap-slot to " slot))
|
||||
(ctk/set-swap-slot shape slot))
|
||||
shape)))]
|
||||
|
||||
(log/dbg :hint "repairing shape :missing-slot" :id (:id shape) :name (:name shape) :page-id page-id)
|
||||
(-> (pcb/empty-changes nil page-id)
|
||||
|
||||
@ -17,7 +17,6 @@
|
||||
[app.common.types.components-list :as ctkl]
|
||||
[app.common.types.container :as ctn]
|
||||
[app.common.types.file :as ctf]
|
||||
[app.common.types.pages-list :as ctpl]
|
||||
[app.common.types.shape-tree :as ctst]
|
||||
[app.common.types.variant :as ctv]
|
||||
[app.common.uuid :as uuid]
|
||||
@ -94,6 +93,52 @@
|
||||
|
||||
(def ^:dynamic ^:private *errors* nil)
|
||||
|
||||
;; Per-page volatile map used to memoize `ctf/find-ref-shape` calls during a
|
||||
;; single validation pass. Keys are shape-ids; values are the returned ref-shape
|
||||
;; (or `nil` when the shape has no ref). The cache is reset once per page so
|
||||
;; that stale results never cross page boundaries.
|
||||
(def ^:dynamic ^:private *ref-shape-cache* nil)
|
||||
|
||||
;; Per-page pre-computed map from parent-id to the set of its children ids.
|
||||
;; Enables O(1) containment checks in `check-parent-children` instead of the
|
||||
;; default O(k) linear `(some #(= shape-id %) (:shapes parent))` scan.
|
||||
;; Bound as a plain immutable map (not a volatile) since it is read-only during
|
||||
;; a page's validation pass.
|
||||
(def ^:dynamic ^:private *children-sets* nil)
|
||||
|
||||
(defn- build-children-sets
|
||||
"Return a {parent-id → #{child-ids}} map built from `objects` in a single
|
||||
`reduce-kv` pass. Only shapes that have at least one child get an entry."
|
||||
[objects]
|
||||
(reduce-kv (fn [m _ shape]
|
||||
(if-let [kids (not-empty (:shapes shape))]
|
||||
(assoc m (:id shape) (set kids))
|
||||
m))
|
||||
{}
|
||||
objects))
|
||||
|
||||
(defn- find-ref-shape*
|
||||
"Cached wrapper around `ctf/find-ref-shape` with `:include-deleted? true`.
|
||||
When `*ref-shape-cache*` is bound, each shape-id is resolved at most once
|
||||
per validation pass regardless of how many check functions request it.
|
||||
|
||||
Cache miss detection uses `contains?` rather than a sentinel default
|
||||
(e.g. `(get cache id ::miss)` + `identical?`). The reason: in
|
||||
ClojureScript, `identical?` on namespace-qualified keywords is not
|
||||
reliable across compilation units because keyword interning is not
|
||||
guaranteed — a sentinel retrieved from `get` may not be `===` to the
|
||||
same literal written in code, causing every lookup to appear as a miss
|
||||
and breaking the whole optimisation."
|
||||
[file page libraries shape]
|
||||
(if-let [cache *ref-shape-cache*]
|
||||
(let [id (:id shape)]
|
||||
(if (contains? @cache id)
|
||||
(get @cache id)
|
||||
(let [result (ctf/find-ref-shape file page libraries shape :include-deleted? true)]
|
||||
(vswap! cache assoc id result)
|
||||
result)))
|
||||
(ctf/find-ref-shape file page libraries shape :include-deleted? true)))
|
||||
|
||||
(defn- library-exists?
|
||||
[file libraries shape]
|
||||
(or (= (:component-file shape) (:id file))
|
||||
@ -149,15 +194,32 @@
|
||||
shape file page)
|
||||
(do
|
||||
(when-not (cfh/root? shape)
|
||||
(when-not (some #(= shape-id %) (:shapes parent))
|
||||
;; Fast path: O(1) set lookup when `*children-sets*` is pre-computed
|
||||
;; for the current page. Falls back to an O(k) linear scan via
|
||||
;; `some` when the var is unbound (e.g. in isolated single-shape
|
||||
;; validation calls outside a full page pass).
|
||||
(when-not (if-let [cs *children-sets*]
|
||||
(contains? (get cs (:id parent)) shape-id)
|
||||
(some #(= shape-id %) (:shapes parent)))
|
||||
(report-error :child-not-in-parent
|
||||
(str/ffmt "Shape % not in parent's children list" shape-id)
|
||||
shape file page)))
|
||||
|
||||
(when-not (= (count shapes) (count (distinct shapes)))
|
||||
(report-error :duplicated-children
|
||||
(str/ffmt "Shape % has duplicated children" shape-id)
|
||||
shape file page))
|
||||
;; Single-pass duplicate detection: walk the children list once,
|
||||
;; accumulating seen IDs into a set and short-circuiting on the
|
||||
;; first duplicate. This replaces the previous two-pass
|
||||
;; `(not= (count shapes) (count (distinct shapes)))` approach
|
||||
;; which allocated an intermediate sequence and scanned twice.
|
||||
(let [dup? (reduce (fn [seen id]
|
||||
(if (contains? seen id)
|
||||
(reduced true)
|
||||
(conj seen id)))
|
||||
#{}
|
||||
shapes)]
|
||||
(when (true? dup?)
|
||||
(report-error :duplicated-children
|
||||
(str/ffmt "Shape % has duplicated children" shape-id)
|
||||
shape file page)))
|
||||
|
||||
(doseq [child-id shapes]
|
||||
(let [child (ctst/get-shape page child-id)]
|
||||
@ -292,7 +354,7 @@
|
||||
[shape file page libraries]
|
||||
(let [library-exists (library-exists? file libraries shape)
|
||||
ref-shape (when library-exists
|
||||
(ctf/find-ref-shape file page libraries shape :include-deleted? true))]
|
||||
(find-ref-shape* file page libraries shape))]
|
||||
(when (and library-exists (nil? ref-shape))
|
||||
(report-error :ref-shape-not-found
|
||||
(str/ffmt "Referenced shape % not found in near component" (:shape-ref shape))
|
||||
@ -309,7 +371,7 @@
|
||||
(defn- check-ref-is-not-head
|
||||
"Validate that the referenced shape is not a nested copy root."
|
||||
[shape file page libraries]
|
||||
(let [ref-shape (ctf/find-ref-shape file page libraries shape :include-deleted? true)]
|
||||
(let [ref-shape (find-ref-shape* file page libraries shape)]
|
||||
(when (and (some? ref-shape)
|
||||
(ctk/instance-head? ref-shape))
|
||||
(report-error :ref-shape-is-head
|
||||
@ -319,7 +381,7 @@
|
||||
(defn- check-ref-is-head
|
||||
"Validate that the referenced shape is a nested copy root."
|
||||
[shape file page libraries]
|
||||
(let [ref-shape (ctf/find-ref-shape file page libraries shape :include-deleted? true)]
|
||||
(let [ref-shape (find-ref-shape* file page libraries shape)]
|
||||
(when (and (some? ref-shape)
|
||||
(not (ctk/instance-head? ref-shape)))
|
||||
(report-error :ref-shape-is-not-head
|
||||
@ -333,7 +395,7 @@
|
||||
the same as in the referenced shape in the near main."
|
||||
[shape file page libraries]
|
||||
(when (nil? (ctk/get-swap-slot shape))
|
||||
(when-let [ref-shape (ctf/find-ref-shape file page libraries shape :include-deleted? true)]
|
||||
(when-let [ref-shape (find-ref-shape* file page libraries shape)]
|
||||
(when (or (not= (:component-id shape) (:component-id ref-shape))
|
||||
(not= (:component-file shape) (:component-file ref-shape)))
|
||||
(report-error :component-id-mismatch
|
||||
@ -351,12 +413,21 @@
|
||||
shape file page)))
|
||||
|
||||
(defn- has-duplicate-swap-slot?
|
||||
"Returns true if any two children of `shape` share the same swap slot.
|
||||
Uses a single pass with early exit on the first duplicate, avoiding
|
||||
the intermediate `frequencies` map allocation."
|
||||
[shape container]
|
||||
(let [shapes (map #(get (:objects container) %) (:shapes shape))
|
||||
slots (->> (map #(ctk/get-swap-slot %) shapes)
|
||||
(remove nil?))
|
||||
counts (frequencies slots)]
|
||||
(some (fn [[_ count]] (> count 1)) counts)))
|
||||
(let [objects (:objects container)
|
||||
result (reduce (fn [seen child-id]
|
||||
(let [slot (ctk/get-swap-slot (get objects child-id))]
|
||||
(if (nil? slot)
|
||||
seen
|
||||
(if (contains? seen slot)
|
||||
(reduced true)
|
||||
(conj seen slot)))))
|
||||
#{}
|
||||
(:shapes shape))]
|
||||
(true? result)))
|
||||
|
||||
(defn- check-duplicate-swap-slot
|
||||
"Validate that the children of this shape does not have duplicated slots."
|
||||
@ -370,14 +441,16 @@
|
||||
"Validate that the shape has swap-slot if it's a subinstance head and the ref shape is not the
|
||||
matching shape by position in the near main."
|
||||
[shape file page libraries]
|
||||
(let [near-match (ctf/find-near-match file page libraries shape :include-deleted? true :with-context? false)]
|
||||
(when (and (some? near-match)
|
||||
(not= (:shape-ref shape) (:id near-match))
|
||||
(nil? (ctk/get-swap-slot shape)))
|
||||
(report-error :missing-slot
|
||||
"Shape has been swapped, should have swap slot"
|
||||
shape file page
|
||||
:swap-slot (or (ctk/get-swap-slot near-match) (:id near-match))))))
|
||||
;; Guard first: if the shape already has a swap slot the invariant is satisfied
|
||||
;; and we can avoid the expensive `find-near-match` call entirely.
|
||||
(when (nil? (ctk/get-swap-slot shape))
|
||||
(let [near-match (ctf/find-near-match file page libraries shape :include-deleted? true :with-context? false)]
|
||||
(when (and (some? near-match)
|
||||
(not= (:shape-ref shape) (:id near-match)))
|
||||
(report-error :missing-slot
|
||||
"Shape has been swapped, should have swap slot"
|
||||
shape file page
|
||||
:swap-slot (or (ctk/get-swap-slot near-match) (:id near-match)))))))
|
||||
|
||||
(defn- check-valid-touched
|
||||
"Validate that the text touched flags are coherent."
|
||||
@ -491,26 +564,28 @@
|
||||
-all its children should be variants with variant-id equals to the shape-id
|
||||
-all the components should have the same properties"
|
||||
[shape file page]
|
||||
(let [shape-id (:id shape)
|
||||
shapes (:shapes shape)
|
||||
children (map #(ctst/get-shape page %) shapes)
|
||||
prop-names (cfv/extract-properties-names (first children) (:data file))]
|
||||
(doseq [child children]
|
||||
(when child
|
||||
(if (not (ctk/is-variant? child))
|
||||
(report-error :not-a-variant
|
||||
(str/ffmt "Shape % should be a variant" (:id child))
|
||||
child file page)
|
||||
(do
|
||||
(when (not= (:variant-id child) shape-id)
|
||||
(report-error :invalid-variant-id
|
||||
(str/ffmt "Variant % has invalid variant-id %" (:id child) (:variant-id child))
|
||||
child file page))
|
||||
(when (not= prop-names (cfv/extract-properties-names child (:data file)))
|
||||
(report-error :invalid-variant-properties
|
||||
(str/ffmt "Variant % has invalid properties %" (:id child) (vec prop-names))
|
||||
child file page))))))))
|
||||
|
||||
(let [shape-id (:id shape)
|
||||
shapes (:shapes shape)
|
||||
objects (:objects page)
|
||||
file-data (:data file)
|
||||
first-child (get objects (first shapes))
|
||||
prop-names (cfv/extract-properties-names first-child file-data)]
|
||||
(run! (fn [child-id]
|
||||
(when-let [child (get objects child-id)]
|
||||
(if (not (ctk/is-variant? child))
|
||||
(report-error :not-a-variant
|
||||
(str/ffmt "Shape % should be a variant" (:id child))
|
||||
child file page)
|
||||
(do
|
||||
(when (not= (:variant-id child) shape-id)
|
||||
(report-error :invalid-variant-id
|
||||
(str/ffmt "Variant % has invalid variant-id %" (:id child) (:variant-id child))
|
||||
child file page))
|
||||
(when (not= prop-names (cfv/extract-properties-names child file-data))
|
||||
(report-error :invalid-variant-properties
|
||||
(str/ffmt "Variant % has invalid properties %" (:id child) (vec prop-names))
|
||||
child file page))))))
|
||||
shapes)))
|
||||
(defn- check-variant
|
||||
"Shape is a variant, so
|
||||
-it should be a main component
|
||||
@ -615,18 +690,26 @@
|
||||
shape file page)
|
||||
(check-shape-copy-not-root shape file page libraries))
|
||||
|
||||
(if (ctn/inside-component-main? (:objects page) shape)
|
||||
(if-not (#{:main-top :main-nested :main-any} context)
|
||||
(report-error :not-head-main-not-allowed
|
||||
"Non-root main only allowed inside a main component"
|
||||
shape file page)
|
||||
(check-shape-main-not-root shape file page libraries))
|
||||
;; Short-circuit `inside-component-main?` when the propagated
|
||||
;; `context` already classifies this sub-tree as belonging to a
|
||||
;; main component. `inside-component-main?` performs an O(depth)
|
||||
;; upward ancestor walk; skipping it for every non-head shape that
|
||||
;; sits inside a known-main context avoids redundant tree traversals
|
||||
;; that would otherwise dominate validation time on deep hierarchies.
|
||||
(let [in-main? (or (#{:main-top :main-nested :main-any} context)
|
||||
(ctn/inside-component-main? (:objects page) shape))]
|
||||
(if in-main?
|
||||
(if-not (#{:main-top :main-nested :main-any} context)
|
||||
(report-error :not-head-main-not-allowed
|
||||
"Non-root main only allowed inside a main component"
|
||||
shape file page)
|
||||
(check-shape-main-not-root shape file page libraries))
|
||||
|
||||
(if (#{:main-top :main-nested :main-any} context)
|
||||
(report-error :not-component-not-allowed
|
||||
"Not compoments are not allowed inside a main"
|
||||
shape file page)
|
||||
(check-shape-not-component shape file page libraries))))))))
|
||||
(if (#{:main-top :main-nested :main-any} context)
|
||||
(report-error :not-component-not-allowed
|
||||
"Not compoments are not allowed inside a main"
|
||||
shape file page)
|
||||
(check-shape-not-component shape file page libraries)))))))))
|
||||
|
||||
(defn check-component-duplicate-swap-slot
|
||||
[component file]
|
||||
@ -638,11 +721,13 @@
|
||||
|
||||
(defn check-ref-cycles
|
||||
[component file]
|
||||
(let [cycles-ids (->> component
|
||||
:objects
|
||||
vals
|
||||
(filter #(= (:id %) (:shape-ref %)))
|
||||
(map :id))]
|
||||
(let [cycles-ids (-> (reduce-kv (fn [acc id shape]
|
||||
(if (= id (:shape-ref shape))
|
||||
(conj! acc id)
|
||||
acc))
|
||||
(transient [])
|
||||
(:objects component))
|
||||
(persistent!))]
|
||||
|
||||
(when (seq cycles-ids)
|
||||
(report-error :shape-ref-cycle
|
||||
@ -703,10 +788,25 @@
|
||||
(check-variant-component component file)))
|
||||
|
||||
(defn- get-orphan-shapes
|
||||
[{:keys [objects] :as page}]
|
||||
(let [xf (comp (map #(contains? objects (:parent-id %)))
|
||||
(map :id))]
|
||||
(into [] xf (vals objects))))
|
||||
"Return the ids of shapes whose parent does not exist in the objects
|
||||
map (i.e. shapes unreachable from the root traversal). The root
|
||||
shape itself is excluded since it is always validated separately.
|
||||
|
||||
Implemented with `reduce-kv` rather than a `map`/`filter` pipeline.
|
||||
The previous implementation mapped over `objects` and returned a
|
||||
lazy seq that could contain `nil` entries (for shapes that were
|
||||
*not* orphans), meaning `check-shape` was called with `nil` IDs and
|
||||
orphaned shapes were silently skipped. The `reduce-kv` approach
|
||||
builds a plain vector of IDs and never yields `nil` entries."
|
||||
[{:keys [objects] :as _page}]
|
||||
(persistent!
|
||||
(reduce-kv (fn [result id shape]
|
||||
(if (and (not (cfh/root? shape))
|
||||
(not (contains? objects (:parent-id shape))))
|
||||
(conj! result id)
|
||||
result))
|
||||
(transient [])
|
||||
objects)))
|
||||
|
||||
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
|
||||
;; PUBLIC API: VALIDATION FUNCTIONS
|
||||
@ -720,20 +820,39 @@
|
||||
(when (contains? features "components/v2")
|
||||
(binding [*errors* (volatile! [])]
|
||||
|
||||
(doseq [page (filter :id (ctpl/pages-seq data))]
|
||||
(check-shape uuid/zero file page libraries)
|
||||
(->> (get-orphan-shapes page)
|
||||
(run! #(check-shape % file page libraries))))
|
||||
;; `reduce-kv` is used throughout this function (and `validate-file-affected`)
|
||||
;; instead of `(run! f (vals m))` or `(doseq [[k v] m])` because persistent
|
||||
;; maps implement `IKVReduce`, which drives iteration internally without
|
||||
;; first materialising an intermediate sequence of key-value pairs.
|
||||
(reduce-kv
|
||||
(fn [_ _ page]
|
||||
(when (some? page)
|
||||
;; Both performance caches are scoped to a single page: ref-shape
|
||||
;; lookups and parent→children sets are page-local and must never
|
||||
;; carry over to the next page. A fresh volatile is created for
|
||||
;; each page so stale entries cannot corrupt subsequent pages.
|
||||
(binding [*ref-shape-cache* (volatile! {})
|
||||
*children-sets* (build-children-sets (:objects page))]
|
||||
(check-shape uuid/zero file page libraries)
|
||||
(run! #(check-shape % file page libraries)
|
||||
(get-orphan-shapes page)))))
|
||||
nil
|
||||
(:pages-index data))
|
||||
|
||||
(->> (vals (:components data))
|
||||
(run! #(check-component % file)))
|
||||
(reduce-kv
|
||||
(fn [_ _ component]
|
||||
(check-component component file))
|
||||
nil
|
||||
(:components data))
|
||||
|
||||
(-> *errors* deref not-empty))))
|
||||
|
||||
(defn validate-shape
|
||||
"Validate a shape and all its children. Returns a list of errors."
|
||||
[shape-id file page libraries]
|
||||
(binding [*errors* (volatile! [])]
|
||||
(binding [*errors* (volatile! [])
|
||||
*ref-shape-cache* (volatile! {})
|
||||
*children-sets* (build-children-sets (:objects page))]
|
||||
(check-shape shape-id file page libraries)
|
||||
(deref *errors*)))
|
||||
|
||||
@ -744,6 +863,107 @@
|
||||
(check-component component file)
|
||||
(deref *errors*)))
|
||||
|
||||
(defn- extract-affected-ids
|
||||
"Single reduce pass over a changes batch.
|
||||
Returns {:page-ids #{uuid …} :component-ids #{uuid …}}.
|
||||
|
||||
Only entities that need re-validation after applying `changes` are
|
||||
included. Deleted entities and pure file-level changes (colors,
|
||||
tokens, typography…) produce no entries because there is nothing left
|
||||
to check."
|
||||
[changes]
|
||||
|
||||
(loop [changes (seq changes)
|
||||
page-ids #{}
|
||||
component-ids #{}]
|
||||
(if-let [change (first changes)]
|
||||
(let [{:keys [type page-id component-id id]} change
|
||||
page-ids
|
||||
(case type
|
||||
;; Shape-level ops are scoped to either a page or a component
|
||||
(:add-obj :mod-obj :del-obj :fix-obj :mov-objects :reorder-children :reg-objects)
|
||||
(cond-> page-ids
|
||||
page-id (conj page-id))
|
||||
|
||||
;; A new or modified page needs a full page sweep
|
||||
(:add-page :mod-page)
|
||||
(conj page-ids id)
|
||||
|
||||
;; restore-component resurrects a deleted component (touches the
|
||||
;; component definition) and places its main instance on a page
|
||||
;; (touches that page's shape tree)
|
||||
:restore-component
|
||||
(conj page-ids page-id)
|
||||
|
||||
;; Otherwise don't change the ids
|
||||
page-ids)
|
||||
|
||||
component-ids
|
||||
(case type
|
||||
;; Shape-level ops are scoped to either a page or a component
|
||||
(:add-obj :mod-obj :del-obj :fix-obj :mov-objects :reorder-children :reg-objects)
|
||||
(cond-> component-ids
|
||||
component-id (conj component-id))
|
||||
|
||||
;; A new, modified, restored component needs component-level checking
|
||||
(:add-component :mod-component :restore-component)
|
||||
(conj component-ids id)
|
||||
|
||||
;; Otherwise don't change the ids
|
||||
component-ids)]
|
||||
(recur (rest changes) page-ids component-ids))
|
||||
|
||||
;; Return result of accumulated ids
|
||||
{:page-ids page-ids :component-ids component-ids})))
|
||||
|
||||
(defn validate-file-affected
|
||||
"Validate only the pages and components touched by `changes`.
|
||||
|
||||
Semantics are identical to `validate-file` but the work is bounded
|
||||
to the entities that were actually mutated, making it safe and cheap
|
||||
to call on every incremental file update.
|
||||
|
||||
Returns a list of errors or `nil`."
|
||||
[{:keys [data features] :as file} libraries changes]
|
||||
(when (contains? features "components/v2")
|
||||
(let [{:keys [page-ids component-ids]} (extract-affected-ids changes)]
|
||||
(binding [*errors* (volatile! [])]
|
||||
|
||||
(reduce-kv
|
||||
(fn [_ page-id page]
|
||||
(when (and (some? page) (contains? page-ids page-id))
|
||||
;; Both performance caches are scoped to a single page: ref-shape
|
||||
;; lookups and parent→children sets are page-local and must never
|
||||
;; carry over to the next page. A fresh volatile is created for
|
||||
;; each page so stale entries cannot corrupt subsequent pages.
|
||||
(binding [*ref-shape-cache* (volatile! {})
|
||||
*children-sets* (build-children-sets (:objects page))]
|
||||
(check-shape uuid/zero file page libraries)
|
||||
(run! #(check-shape % file page libraries)
|
||||
(get-orphan-shapes page)))))
|
||||
nil
|
||||
(:pages-index data))
|
||||
|
||||
(reduce-kv
|
||||
(fn [_ id component]
|
||||
(when (contains? component-ids id)
|
||||
(check-component component file)))
|
||||
nil
|
||||
(:components data))
|
||||
|
||||
(-> *errors* deref not-empty)))))
|
||||
|
||||
(defn validate-file-affected!
|
||||
"Like `validate-file-affected` but raises on the first non-empty
|
||||
error list instead of returning it."
|
||||
[file libraries changes]
|
||||
(when-let [errors (validate-file-affected file libraries changes)]
|
||||
(ex/raise :type :validation
|
||||
:code :referential-integrity
|
||||
:hint "error on validating file referential integrity"
|
||||
:file-id (:id file)
|
||||
:details errors)))
|
||||
|
||||
(defn validate-file-schema!
|
||||
"Validates the file itself, without external dependencies, it
|
||||
performs the schema checking and some semantical validation of the
|
||||
|
||||
@ -269,10 +269,19 @@
|
||||
[group]
|
||||
(str/starts-with? (name group) "swap-slot-"))
|
||||
|
||||
(def ^:private xf:normal-touched
|
||||
"Transducer that removes swap-slot touched groups."
|
||||
(remove swap-slot?))
|
||||
|
||||
(defn normal-touched-groups
|
||||
"Gets all touched groups that are not swap slots."
|
||||
"Gets all touched groups that are not swap slots.
|
||||
Returns an empty set immediately when `:touched` is nil or empty,
|
||||
avoiding an unnecessary `into #{}` allocation for the common case."
|
||||
[shape]
|
||||
(into #{} (remove swap-slot? (:touched shape))))
|
||||
(let [touched (:touched shape)]
|
||||
(if (empty? touched)
|
||||
#{}
|
||||
(into #{} xf:normal-touched touched))))
|
||||
|
||||
(defn group->swap-slot
|
||||
[group]
|
||||
|
||||
449
common/test/common_tests/files/validate_test.cljc
Normal file
449
common/test/common_tests/files/validate_test.cljc
Normal file
@ -0,0 +1,449 @@
|
||||
;; 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
|
||||
|
||||
(ns common-tests.files.validate-test
|
||||
"Exhaustive tests for the change-scoped partial validation functions in
|
||||
app.common.files.validate:
|
||||
|
||||
- validate-file-affected – returns nil or list of errors
|
||||
- validate-file-affected! – same but raises on non-empty errors
|
||||
|
||||
The tests verify the scoping logic implemented by extract-affected-ids
|
||||
(tested indirectly) by injecting controlled broken states into specific
|
||||
pages / components and confirming that only the expected entities are
|
||||
validated."
|
||||
(:require
|
||||
[app.common.files.validate :as cfv]
|
||||
[app.common.test-helpers.files :as thf]
|
||||
[app.common.test-helpers.ids-map :as thi]
|
||||
[app.common.types.file :as ctf]
|
||||
[app.common.types.pages-list :as ctpl]
|
||||
[app.common.types.shape-tree :as ctst]
|
||||
[app.common.uuid :as uuid]
|
||||
[clojure.test :as t]))
|
||||
|
||||
(t/use-fixtures :each thi/test-fixture)
|
||||
|
||||
;; ----------------------------------------------------------------
|
||||
;; Test-local helpers
|
||||
;; ----------------------------------------------------------------
|
||||
|
||||
(defn- inject-broken-child
|
||||
"Add a reference to a non-existent shape ID to the root shape's
|
||||
children list on the page identified by `page-label`.
|
||||
|
||||
This causes `check-parent-children` to report a :child-not-found
|
||||
error whenever that page is validated."
|
||||
[file page-label]
|
||||
(let [page-id (thi/id page-label)
|
||||
missing-id (uuid/next)]
|
||||
(ctf/update-file-data
|
||||
file
|
||||
(fn [file-data]
|
||||
(ctpl/update-page
|
||||
file-data
|
||||
page-id
|
||||
(fn [page]
|
||||
(let [root (ctst/get-shape page uuid/zero)]
|
||||
(ctst/set-shape page (update root :shapes conj missing-id)))))))))
|
||||
|
||||
(defn- inject-broken-component
|
||||
"Add a deleted component with `:objects nil` to the file.
|
||||
|
||||
This causes `check-component` to report a
|
||||
:component-nil-objects-not-allowed error whenever that component is
|
||||
validated. The component id is registered under `comp-label` in the
|
||||
ids-map so callers can look it up with `(thi/id comp-label)`."
|
||||
[file comp-label]
|
||||
(let [comp-id (thi/new-id! comp-label)]
|
||||
(ctf/update-file-data
|
||||
file
|
||||
(fn [file-data]
|
||||
(assoc-in file-data [:components comp-id]
|
||||
{:id comp-id
|
||||
:name "broken-component"
|
||||
:objects nil
|
||||
:deleted true
|
||||
:main-instance-id (uuid/next)
|
||||
:main-instance-page (uuid/next)})))))
|
||||
|
||||
;; ----------------------------------------------------------------
|
||||
;; 1. Feature gate
|
||||
;; ----------------------------------------------------------------
|
||||
|
||||
(t/deftest validate-file-affected-no-feature
|
||||
(t/testing "returns nil when file does not have the components/v2 feature"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(assoc :features #{}))
|
||||
page-id (thi/id :page1)
|
||||
changes [{:type :add-obj :page-id page-id :id (uuid/next)}]]
|
||||
(t/is (nil? (cfv/validate-file-affected file {} changes))))))
|
||||
|
||||
;; ----------------------------------------------------------------
|
||||
;; 2. Empty changes – nothing to validate
|
||||
;; ----------------------------------------------------------------
|
||||
|
||||
(t/deftest validate-file-affected-empty-changes
|
||||
(t/testing "returns nil when the changes list is empty"
|
||||
(let [file (thf/sample-file :file1 :page-label :page1)]
|
||||
(t/is (nil? (cfv/validate-file-affected file {} []))))))
|
||||
|
||||
;; ----------------------------------------------------------------
|
||||
;; 3. Page-level scoping – add-obj / mod-obj / fix-obj key: :page-id
|
||||
;; ----------------------------------------------------------------
|
||||
|
||||
(t/deftest validate-file-affected-page-scoping-misses-untouched-page
|
||||
(t/testing "add-obj on page1 does not validate broken page2"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(thf/add-sample-page :page2))
|
||||
file' (inject-broken-child file :page2)
|
||||
page1-id (thi/id :page1)
|
||||
;; The change only touches page1
|
||||
changes [{:type :add-obj :page-id page1-id :id (uuid/next)}]]
|
||||
(t/is (nil? (cfv/validate-file-affected file' {} changes)))))
|
||||
|
||||
(t/testing "mod-obj on page1 does not validate broken page2"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(thf/add-sample-page :page2))
|
||||
file' (inject-broken-child file :page2)
|
||||
page1-id (thi/id :page1)
|
||||
changes [{:type :mod-obj :page-id page1-id :id uuid/zero
|
||||
:operations [{:type :set :attr :name :val "root"}]}]]
|
||||
(t/is (nil? (cfv/validate-file-affected file' {} changes)))))
|
||||
|
||||
(t/testing "fix-obj on page1 does not validate broken page2"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(thf/add-sample-page :page2))
|
||||
file' (inject-broken-child file :page2)
|
||||
page1-id (thi/id :page1)
|
||||
changes [{:type :fix-obj :page-id page1-id :id (uuid/next)
|
||||
:operations []}]]
|
||||
(t/is (nil? (cfv/validate-file-affected file' {} changes))))))
|
||||
|
||||
(t/deftest validate-file-affected-page-scoping-catches-error-on-touched-page
|
||||
(t/testing "add-obj on broken page1 surfaces the error"
|
||||
(let [file (thf/sample-file :file1 :page-label :page1)
|
||||
file' (inject-broken-child file :page1)
|
||||
page1-id (thi/id :page1)
|
||||
changes [{:type :add-obj :page-id page1-id :id (uuid/next)}]]
|
||||
(let [errors (cfv/validate-file-affected file' {} changes)]
|
||||
(t/is (seq errors))
|
||||
(t/is (some #(= :child-not-found (:code %)) errors)))))
|
||||
|
||||
(t/testing "mod-obj on broken page1 surfaces the error"
|
||||
(let [file (thf/sample-file :file1 :page-label :page1)
|
||||
file' (inject-broken-child file :page1)
|
||||
page1-id (thi/id :page1)
|
||||
changes [{:type :mod-obj :page-id page1-id :id uuid/zero
|
||||
:operations [{:type :set :attr :name :val "root"}]}]]
|
||||
(let [errors (cfv/validate-file-affected file' {} changes)]
|
||||
(t/is (seq errors))
|
||||
(t/is (some #(= :child-not-found (:code %)) errors)))))
|
||||
|
||||
(t/testing "reg-objects on broken page1 surfaces the error"
|
||||
(let [file (thf/sample-file :file1 :page-label :page1)
|
||||
file' (inject-broken-child file :page1)
|
||||
page1-id (thi/id :page1)
|
||||
changes [{:type :reg-objects :page-id page1-id :shapes []}]]
|
||||
(let [errors (cfv/validate-file-affected file' {} changes)]
|
||||
(t/is (seq errors))
|
||||
(t/is (some #(= :child-not-found (:code %)) errors)))))
|
||||
|
||||
(t/testing "mov-objects on broken page1 surfaces the error"
|
||||
(let [file (thf/sample-file :file1 :page-label :page1)
|
||||
file' (inject-broken-child file :page1)
|
||||
page1-id (thi/id :page1)
|
||||
changes [{:type :mov-objects :page-id page1-id :parent-id uuid/zero :shapes []}]]
|
||||
(let [errors (cfv/validate-file-affected file' {} changes)]
|
||||
(t/is (seq errors))
|
||||
(t/is (some #(= :child-not-found (:code %)) errors))))))
|
||||
|
||||
;; ----------------------------------------------------------------
|
||||
;; 4. add-page / mod-page – scoped by :id (not :page-id)
|
||||
;; ----------------------------------------------------------------
|
||||
|
||||
(t/deftest validate-file-affected-add-page-scoped-by-id
|
||||
(t/testing "add-page with :id=page2 validates page2 (broken → errors)"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(thf/add-sample-page :page2))
|
||||
file' (inject-broken-child file :page2)
|
||||
page2-id (thi/id :page2)
|
||||
changes [{:type :add-page :id page2-id}]]
|
||||
(let [errors (cfv/validate-file-affected file' {} changes)]
|
||||
(t/is (seq errors))
|
||||
(t/is (some #(= :child-not-found (:code %)) errors)))))
|
||||
|
||||
(t/testing "add-page with :id=page1 does not validate broken page2"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(thf/add-sample-page :page2))
|
||||
file' (inject-broken-child file :page2)
|
||||
page1-id (thi/id :page1)
|
||||
changes [{:type :add-page :id page1-id}]]
|
||||
(t/is (nil? (cfv/validate-file-affected file' {} changes)))))
|
||||
|
||||
(t/testing "mod-page with :id=page2 validates page2 (broken → errors)"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(thf/add-sample-page :page2))
|
||||
file' (inject-broken-child file :page2)
|
||||
page2-id (thi/id :page2)
|
||||
changes [{:type :mod-page :id page2-id}]]
|
||||
(let [errors (cfv/validate-file-affected file' {} changes)]
|
||||
(t/is (seq errors))
|
||||
(t/is (some #(= :child-not-found (:code %)) errors))))))
|
||||
|
||||
;; ----------------------------------------------------------------
|
||||
;; 5. del-obj – shape-level op scoped by :page-id
|
||||
;; del-page / del-component / mov-page / purge-component – no-ops
|
||||
;; ----------------------------------------------------------------
|
||||
|
||||
(t/deftest validate-file-affected-del-obj-scopes-page
|
||||
(t/testing "del-obj scopes its :page-id just like add-obj / mod-obj"
|
||||
;; del-obj is in the same shape-level ops bucket and scopes by :page-id.
|
||||
(let [file (thf/sample-file :file1 :page-label :page1)
|
||||
file' (inject-broken-child file :page1)
|
||||
page1-id (thi/id :page1)
|
||||
changes [{:type :del-obj :page-id page1-id :id (uuid/next)}]]
|
||||
;; page1 has an error and del-obj touches page1 → error surfaced
|
||||
(let [errors (cfv/validate-file-affected file' {} changes)]
|
||||
(t/is (seq errors))
|
||||
(t/is (some #(= :child-not-found (:code %)) errors)))))
|
||||
|
||||
(t/testing "del-obj on page1 does not validate broken page2"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(thf/add-sample-page :page2))
|
||||
file' (inject-broken-child file :page2)
|
||||
page1-id (thi/id :page1)
|
||||
changes [{:type :del-obj :page-id page1-id :id (uuid/next)}]]
|
||||
(t/is (nil? (cfv/validate-file-affected file' {} changes))))))
|
||||
|
||||
(t/deftest validate-file-affected-noop-change-types
|
||||
(t/testing "del-page produces no affected entries"
|
||||
(let [file (thf/sample-file :file1 :page-label :page1)
|
||||
file' (inject-broken-child file :page1)
|
||||
page1-id (thi/id :page1)
|
||||
changes [{:type :del-page :id page1-id}]]
|
||||
(t/is (nil? (cfv/validate-file-affected file' {} changes)))))
|
||||
|
||||
(t/testing "del-component produces no affected entries"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(inject-broken-component :comp1))
|
||||
comp-id (thi/id :comp1)
|
||||
changes [{:type :del-component :id comp-id}]]
|
||||
(t/is (nil? (cfv/validate-file-affected file {} changes)))))
|
||||
|
||||
(t/testing "mov-page produces no affected entries"
|
||||
(let [file (thf/sample-file :file1 :page-label :page1)
|
||||
file' (inject-broken-child file :page1)
|
||||
page1-id (thi/id :page1)
|
||||
changes [{:type :mov-page :id page1-id}]]
|
||||
(t/is (nil? (cfv/validate-file-affected file' {} changes)))))
|
||||
|
||||
(t/testing "purge-component produces no affected entries"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(inject-broken-component :comp1))
|
||||
comp-id (thi/id :comp1)
|
||||
changes [{:type :purge-component :id comp-id}]]
|
||||
(t/is (nil? (cfv/validate-file-affected file {} changes)))))
|
||||
|
||||
(t/testing "add-color (library change) produces no affected entries"
|
||||
(let [file (thf/sample-file :file1 :page-label :page1)
|
||||
file' (inject-broken-child file :page1)
|
||||
changes [{:type :add-color :id (uuid/next) :color {}}]]
|
||||
(t/is (nil? (cfv/validate-file-affected file' {} changes)))))
|
||||
|
||||
(t/testing "mod-color (library change) produces no affected entries"
|
||||
(let [file (thf/sample-file :file1 :page-label :page1)
|
||||
file' (inject-broken-child file :page1)
|
||||
changes [{:type :mod-color :id (uuid/next) :color {}}]]
|
||||
(t/is (nil? (cfv/validate-file-affected file' {} changes))))))
|
||||
|
||||
;; ----------------------------------------------------------------
|
||||
;; 6. add-component / mod-component – scoped by :id (component-id)
|
||||
;; ----------------------------------------------------------------
|
||||
|
||||
(t/deftest validate-file-affected-add-component-scoped-by-id
|
||||
(t/testing "add-component :id=comp1 validates comp1 (broken → errors)"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(inject-broken-component :comp1))
|
||||
comp-id (thi/id :comp1)
|
||||
changes [{:type :add-component :id comp-id}]]
|
||||
(let [errors (cfv/validate-file-affected file {} changes)]
|
||||
(t/is (seq errors))
|
||||
(t/is (some #(= :component-nil-objects-not-allowed (:code %)) errors)))))
|
||||
|
||||
(t/testing "add-component :id=other-id does not validate broken comp1"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(inject-broken-component :comp1))
|
||||
_ (thi/id :comp1)
|
||||
changes [{:type :add-component :id (uuid/next)}]]
|
||||
(t/is (nil? (cfv/validate-file-affected file {} changes)))))
|
||||
|
||||
(t/testing "mod-component :id=comp1 validates comp1 (broken → errors)"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(inject-broken-component :comp1))
|
||||
comp-id (thi/id :comp1)
|
||||
changes [{:type :mod-component :id comp-id}]]
|
||||
(let [errors (cfv/validate-file-affected file {} changes)]
|
||||
(t/is (seq errors))
|
||||
(t/is (some #(= :component-nil-objects-not-allowed (:code %)) errors))))))
|
||||
|
||||
;; ----------------------------------------------------------------
|
||||
;; 7. restore-component – scopes BOTH :id (component) and :page-id
|
||||
;; ----------------------------------------------------------------
|
||||
|
||||
(t/deftest validate-file-affected-restore-component-scopes-page
|
||||
(t/testing "restore-component touches its :page-id (broken page → errors)"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(thf/add-sample-page :page2))
|
||||
file' (inject-broken-child file :page1)
|
||||
page1-id (thi/id :page1)
|
||||
changes [{:type :restore-component :id (uuid/next) :page-id page1-id}]]
|
||||
(let [errors (cfv/validate-file-affected file' {} changes)]
|
||||
(t/is (seq errors))
|
||||
(t/is (some #(= :child-not-found (:code %)) errors)))))
|
||||
|
||||
(t/testing "restore-component does not validate a page it does not reference"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(thf/add-sample-page :page2))
|
||||
file' (inject-broken-child file :page2)
|
||||
page1-id (thi/id :page1)
|
||||
changes [{:type :restore-component :id (uuid/next) :page-id page1-id}]]
|
||||
;; page2 has an error but the change only touches page1
|
||||
(t/is (nil? (cfv/validate-file-affected file' {} changes))))))
|
||||
|
||||
(t/deftest validate-file-affected-restore-component-scopes-component
|
||||
(t/testing "restore-component touches its component :id (broken component → errors)"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(inject-broken-component :comp1))
|
||||
comp-id (thi/id :comp1)
|
||||
page1-id (thi/id :page1)
|
||||
changes [{:type :restore-component :id comp-id :page-id page1-id}]]
|
||||
(let [errors (cfv/validate-file-affected file {} changes)]
|
||||
(t/is (seq errors))
|
||||
(t/is (some #(= :component-nil-objects-not-allowed (:code %)) errors))))))
|
||||
|
||||
;; ----------------------------------------------------------------
|
||||
;; 8. Mixed changes – union of affected entities
|
||||
;; ----------------------------------------------------------------
|
||||
|
||||
(t/deftest validate-file-affected-mixed-changes-union
|
||||
(t/testing "two changes on different pages: both pages are validated"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(thf/add-sample-page :page2))
|
||||
;; page2 is broken; page1 is clean
|
||||
file' (inject-broken-child file :page2)
|
||||
page1-id (thi/id :page1)
|
||||
page2-id (thi/id :page2)
|
||||
;; Both pages are touched
|
||||
changes [{:type :add-obj :page-id page1-id :id (uuid/next)}
|
||||
{:type :mod-obj :page-id page2-id :id uuid/zero
|
||||
:operations []}]]
|
||||
(let [errors (cfv/validate-file-affected file' {} changes)]
|
||||
;; page2 is validated → error surfaced
|
||||
(t/is (seq errors))
|
||||
(t/is (some #(= :child-not-found (:code %)) errors)))))
|
||||
|
||||
(t/testing "del-page (true no-op) mixed with add-obj on page1: only page1 is validated"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(thf/add-sample-page :page2))
|
||||
;; page2 is broken; page1 is clean
|
||||
file' (inject-broken-child file :page2)
|
||||
page1-id (thi/id :page1)
|
||||
page2-id (thi/id :page2)
|
||||
;; del-page on page2 is a no-op; add-obj on page1 scopes page1 only
|
||||
changes [{:type :del-page :id page2-id}
|
||||
{:type :add-obj :page-id page1-id :id (uuid/next)}]]
|
||||
;; page2's error is NOT surfaced because del-page produces no scope
|
||||
(t/is (nil? (cfv/validate-file-affected file' {} changes)))))
|
||||
|
||||
(t/testing "duplicate page-ids in changes are deduplicated (page validated once)"
|
||||
(let [file (thf/sample-file :file1 :page-label :page1)
|
||||
file' (inject-broken-child file :page1)
|
||||
page1-id (thi/id :page1)
|
||||
;; Three changes all touching the same page
|
||||
changes [{:type :add-obj :page-id page1-id :id (uuid/next)}
|
||||
{:type :mod-obj :page-id page1-id :id uuid/zero :operations []}
|
||||
{:type :del-obj :page-id page1-id :id (uuid/next)}]]
|
||||
;; del-obj excluded, add-obj + mod-obj both scope page1; error surfaced once
|
||||
(let [errors (cfv/validate-file-affected file' {} changes)]
|
||||
(t/is (seq errors))
|
||||
;; There should be exactly one :child-not-found (not duplicated)
|
||||
(t/is (= 1 (count (filter #(= :child-not-found (:code %)) errors))))))))
|
||||
|
||||
;; ----------------------------------------------------------------
|
||||
;; 9. reorder-children and component-context changes
|
||||
;; ----------------------------------------------------------------
|
||||
|
||||
(t/deftest validate-file-affected-reorder-children
|
||||
(t/testing "reorder-children on broken page surfaces the error"
|
||||
(let [file (thf/sample-file :file1 :page-label :page1)
|
||||
file' (inject-broken-child file :page1)
|
||||
page1-id (thi/id :page1)
|
||||
changes [{:type :reorder-children :page-id page1-id :id uuid/zero :shapes []}]]
|
||||
(let [errors (cfv/validate-file-affected file' {} changes)]
|
||||
(t/is (seq errors))
|
||||
(t/is (some #(= :child-not-found (:code %)) errors))))))
|
||||
|
||||
(t/deftest validate-file-affected-obj-in-component
|
||||
(t/testing "add-obj with :component-id (not :page-id) scopes a component"
|
||||
;; When a shape change carries :component-id instead of :page-id it
|
||||
;; means the change happened inside a deleted component's object tree.
|
||||
;; extract-affected-ids routes it to :component-ids.
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(inject-broken-component :comp1))
|
||||
comp-id (thi/id :comp1)
|
||||
changes [{:type :add-obj :component-id comp-id :id (uuid/next)}]]
|
||||
(let [errors (cfv/validate-file-affected file {} changes)]
|
||||
(t/is (seq errors))
|
||||
(t/is (some #(= :component-nil-objects-not-allowed (:code %)) errors)))))
|
||||
|
||||
(t/testing "add-obj with :component-id does not scope an unrelated component"
|
||||
(let [file (-> (thf/sample-file :file1 :page-label :page1)
|
||||
(inject-broken-component :comp1))
|
||||
_ (thi/id :comp1)
|
||||
changes [{:type :add-obj :component-id (uuid/next) :id (uuid/next)}]]
|
||||
(t/is (nil? (cfv/validate-file-affected file {} changes))))))
|
||||
|
||||
;; ----------------------------------------------------------------
|
||||
;; 10. validate-file-affected! – raises vs returns nil
|
||||
;; ----------------------------------------------------------------
|
||||
|
||||
(t/deftest validate-file-affected!-returns-nil-on-clean-file
|
||||
(t/testing "returns nil when the file has no errors in the touched page"
|
||||
(let [file (thf/sample-file :file1 :page-label :page1)
|
||||
page1-id (thi/id :page1)
|
||||
changes [{:type :add-obj :page-id page1-id :id (uuid/next)}]]
|
||||
(t/is (nil? (cfv/validate-file-affected! file {} changes))))))
|
||||
|
||||
(t/deftest validate-file-affected!-returns-nil-empty-changes
|
||||
(t/testing "returns nil when there are no changes (no pages validated)"
|
||||
(let [file (thf/sample-file :file1 :page-label :page1)]
|
||||
(t/is (nil? (cfv/validate-file-affected! file {} []))))))
|
||||
|
||||
(t/deftest validate-file-affected!-raises-on-error
|
||||
(t/testing "raises an exception when the touched page has validation errors"
|
||||
(let [file (thf/sample-file :file1 :page-label :page1)
|
||||
file' (inject-broken-child file :page1)
|
||||
page1-id (thi/id :page1)
|
||||
changes [{:type :mod-obj :page-id page1-id :id uuid/zero
|
||||
:operations []}]]
|
||||
(t/is (thrown? #?(:clj Exception :cljs js/Error)
|
||||
(cfv/validate-file-affected! file' {} changes)))))
|
||||
|
||||
(t/testing "raised exception is of :validation type with :referential-integrity code"
|
||||
(let [file (thf/sample-file :file1 :page-label :page1)
|
||||
file' (inject-broken-child file :page1)
|
||||
page1-id (thi/id :page1)
|
||||
changes [{:type :mod-obj :page-id page1-id :id uuid/zero
|
||||
:operations []}]]
|
||||
(try
|
||||
(cfv/validate-file-affected! file' {} changes)
|
||||
(t/is false "expected exception to be thrown")
|
||||
(catch #?(:clj clojure.lang.ExceptionInfo :cljs cljs.core.ExceptionInfo) e
|
||||
(let [data (ex-data e)]
|
||||
(t/is (= :validation (:type data)))
|
||||
(t/is (= :referential-integrity (:code data)))
|
||||
(t/is (seq (:details data)))))))))
|
||||
@ -16,6 +16,7 @@
|
||||
[common-tests.files-changes-test]
|
||||
[common-tests.files-migrations-test]
|
||||
[common-tests.files.shapes-builder-test]
|
||||
[common-tests.files.validate-test]
|
||||
[common-tests.geom-align-test]
|
||||
[common-tests.geom-bounds-map-test]
|
||||
[common-tests.geom-flex-layout-test]
|
||||
@ -97,6 +98,7 @@
|
||||
'common-tests.files-changes-test
|
||||
'common-tests.files-builder-test
|
||||
'common-tests.files-migrations-test
|
||||
'common-tests.files.validate-test
|
||||
'common-tests.geom-align-test
|
||||
'common-tests.geom-bounds-map-test
|
||||
'common-tests.geom-flex-layout-test
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user