github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-11-01 15:40:41 +00:00
Code Issues Packages Projects Releases Wiki Activity
penpot/common/test/common_tests/logic/comp_sync_test.cljc
alonso.torres 98ae356fac ♻️ Simplify the component sync walk and its changes 
- The sync walk took its direction as a map of seven closures
  (direct-sync, inverse-sync). The two directions are fixed, so each
  is now a plain recursive function: sync-copy-recursive (main to
  copy, a sync or a reset) and update-main-recursive (copy to main),
  with sync-copy-shape and update-main-shape for the pair itself.
  Both share compare-children, which takes the children, a pair?
  predicate and four callbacks instead of 13 arguments; the inverse
  branch's call of the swapped callback, which could never run, goes.
- The container of the main shapes is found once, at the start of the
  walk. The walk holds the :remote-synced a reset or an update main
  gives the copy shapes, instead of initial-root? and two branches.
  add-shape-to-instance and add-shape-to-main take the parent from the
  pair being walked instead of searching the whole subtree for it.
- pcb/add-changes takes a container and the redo and undo changes, and
  applies them locally. Eight sync functions and the library
  typography sync each wrote that block; make-change goes from both
  namespaces. One list holds the attributes that link a copy shape
  with its main, and one shape-label prints shapes in the log lines.
- change-touched chose what to write from three flags. It is now
  set-touched, which takes the value each caller computes. The swap's
  restore of the kept properties passes ignore-touched instead of
  calling it afterwards to put the groups back.
- generate-sync-shape was a multimethod over the three asset types; it
  is a case in generate-sync-container. generate-sync-file loses its
  unused arity. The 100-line comment that opened the sync helpers
  described an older model; a short note points at the component sync
  contract. text-content-with-overrides only called
  txt/merge-overrides, which sync and switch now call.
- update-flex-child-copy-attrs and update-flex-child-main-attrs are one
  function, update-flex-children, and the omit-touched? argument that
  was always false goes.
- In the variant switch, sized-geometry-value builds the resized
  :selrect and :points that two geometry policies each built, and a
  binding of skip? shadowed by a second one is skip-attr?.

add-shape-to-main keeps two changes that look wrong, now marked F102:
its :reg-objects names the component, so it only reaches a deleted
component's objects, and the undo of its new shapes deletes them from
the copy's page.

No behavior change: the golden masters and the suites are unchanged.
libraries.cljc: 1816 -> 1284 lines.

Benchmark (lower quantile, ms; baseline / now / ratio):
  deepest-edit-through-three-levels    35.2 /  15.2 / 0.43
  edit-through-a-chain-of-dependents   68.9 /  35.7 / 0.52
  library-update-into-copies           83.1 /  40.5 / 0.49
  main-edit-among-many-shapes           4.8 /   2.4 / 0.51
  main-edit-into-10-copies              1.8 /   0.8 / 0.46
  main-edit-into-100-copies            15.4 /   7.8 / 0.51
  main-edit-into-1000-copies          153.1 /  97.4 / 0.64
  variant-switch-into-copies           32.1 /  17.6 / 0.55
(The machine was busy for the 1000-copies world: its mean was 119 ms,
against about 83 in the runs of the earlier steps, at 0.49-0.52.)

AI-assisted-by: claude-opus-5-5

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-10-15 00:35:54 +02:00

583 lines
28 KiB
Clojure
Raw Permalink Blame History

;; This Source Code Form is subject to the terms of the Mozilla Public
;; License, v. 2.0. If a copy of the MPL was not distributed with this
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
;;
;; Copyright (c) KALEIDOS SUBSIDIARY SL
(ns common-tests.logic.comp-sync-test
(:require
[app.common.data :as d]
[app.common.files.changes-builder :as pcb]
[app.common.geom.point :as gpt]
[app.common.geom.shapes :as gsh]
[app.common.logic.libraries :as cll]
[app.common.logic.shapes :as cls]
[app.common.test-helpers.components :as thc]
[app.common.test-helpers.compositions :as tho]
[app.common.test-helpers.files :as thf]
[app.common.test-helpers.ids-map :as thi]
[app.common.test-helpers.shapes :as ths]
[app.common.types.component :as ctk]
[app.common.types.container :as ctn]
[app.common.types.shape-tree :as ctst]
[clojure.test :as t]))
(t/use-fixtures :each thi/test-fixture)
(t/deftest test-sync-when-changing-attribute
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(tho/add-simple-component-with-copy :component1
:main-root
:main-child
:copy-root
:main-child-params {:fills (ths/sample-fills-color
:fill-color "#abcdef")}
:copy-root-params {:children-labels [:copy-child]}))
page (thf/current-page file)
main-child (ths/get-shape file :main-child)
;; ==== Action
changes1 (cls/generate-update-shapes (pcb/empty-changes nil (:id page))
#{(:id main-child)}
(fn [shape]
(assoc shape :fills (ths/sample-fills-color :fill-color "#fabada")))
(:objects page)
{})
updated-file (thf/apply-changes file changes1)
changes2 (cll/generate-sync-file-changes (pcb/empty-changes)
nil
:components
(:id updated-file)
(thi/id :component1)
(:id updated-file)
{(:id updated-file) updated-file}
(:id updated-file))
file' (thf/apply-changes updated-file changes2)
;; ==== Get
copy-root' (ths/get-shape file' :copy-root)
copy-child' (ths/get-shape file' :copy-child)
fills' (:fills copy-child')
fill' (first fills')]
;; ==== Check
(t/is (some? copy-root'))
(t/is (some? copy-child'))
(t/is (= (count fills') 1))
(t/is (= (:fill-color fill') "#fabada"))
(t/is (= (:fill-opacity fill') 1))
(t/is (= (:touched copy-root') nil))
(t/is (= (:touched copy-child') nil))))
(t/deftest test-sync-when-changing-attribute-from-library
(let [;; ==== Setup
library (-> (thf/sample-file :file1)
(tho/add-simple-component :component1
:main-root
:main-child
:copy-root
:main-child-params {:fills (ths/sample-fills-color
:fill-color "#abcdef")}))
file (-> (thf/sample-file :file)
(thc/instantiate-component :component1 :copy-root
:library library
:children-labels [:copy-child]))
page (thf/current-page library)
main-child (ths/get-shape library :main-child)
;; ==== Action
changes1 (cls/generate-update-shapes (pcb/empty-changes nil (:id page))
#{(:id main-child)}
(fn [shape]
(assoc shape :fills (ths/sample-fills-color :fill-color "#fabada")))
(:objects page)
{})
updated-library (thf/apply-changes library changes1)
changes2 (cll/generate-sync-file-changes (pcb/empty-changes)
nil
:components
(:id file)
(thi/id :component1)
(:id updated-library)
{(:id updated-library) updated-library
(:id file) file}
(:id file))
file' (thf/apply-changes file changes2)
;; ==== Get
copy-root' (ths/get-shape file' :copy-root)
copy-child' (ths/get-shape file' :copy-child)
fills' (:fills copy-child')
fill' (first fills')]
;; ==== Check
(t/is (some? copy-root'))
(t/is (some? copy-child'))
(t/is (= (count fills') 1))
(t/is (= (:fill-color fill') "#fabada"))
(t/is (= (:fill-opacity fill') 1))
(t/is (= (:touched copy-root') nil))
(t/is (= (:touched copy-child') nil))))
(t/deftest test-sync-when-changing-attribute-preserve-touched
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(tho/add-simple-component-with-copy :component1
:main-root
:main-child
:copy-root
:main-child-params {:fills (ths/sample-fills-color
:fill-color "#abcdef")}
:copy-root-params {:children-labels [:copy-child]}))
page (thf/current-page file)
main-child (ths/get-shape file :main-child)
copy-child (ths/get-shape file :copy-child)
;; ==== Action
changes1 (-> (pcb/empty-changes nil (:id page))
(cls/generate-update-shapes
#{(:id copy-child)}
(fn [shape]
(assoc shape :fills (ths/sample-fills-color :fill-color "#aaaaaa")))
(:objects page)
{})
(cls/generate-update-shapes
#{(:id main-child)}
(fn [shape]
(assoc shape :fills (ths/sample-fills-color :fill-color "#fabada")))
(:objects page)
{}))
updated-file (thf/apply-changes file changes1)
changes2 (cll/generate-sync-file-changes (pcb/empty-changes)
nil
:components
(:id updated-file)
(thi/id :component1)
(:id updated-file)
{(:id updated-file) updated-file}
(:id updated-file))
file' (thf/apply-changes updated-file changes2)
;; ==== Get
copy-root' (ths/get-shape file' :copy-root)
copy-child' (ths/get-shape file' :copy-child)
fills' (:fills copy-child')
fill' (first fills')]
;; ==== Check
(t/is (some? copy-root'))
(t/is (some? copy-child'))
(t/is (= (count fills') 1))
(t/is (= (:fill-color fill') "#aaaaaa"))
(t/is (= (:fill-opacity fill') 1))
(t/is (= (:touched copy-root') nil))
(t/is (= (:touched copy-child') #{:fill-group}))))
(t/deftest test-sync-when-adding-shape
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(tho/add-simple-component-with-copy :component1
:main-root
:main-child
:copy-root
:copy-root-params {:children-labels [:copy-child]})
(ths/add-sample-shape :free-shape))
page (thf/current-page file)
;; ==== Action
changes1 (cls/generate-relocate (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-objects (:objects page)))
(thi/id :main-root) ; parent-id
0 ; to-index
#{(thi/id :free-shape)}) ; ids
updated-file (thf/apply-changes file changes1)
changes2 (cll/generate-sync-file-changes (pcb/empty-changes)
nil
:components
(:id updated-file)
(thi/id :component1)
(:id updated-file)
{(:id updated-file) updated-file}
(:id updated-file))
file' (thf/apply-changes updated-file changes2)
;; ==== Get
main-free-shape' (ths/get-shape file' :free-shape)
copy-root' (ths/get-shape file' :copy-root)
copy-new-child-id' (d/seek #(not= % (thi/id :copy-child)) (:shapes copy-root'))
copy-new-child' (ths/get-shape-by-id file' copy-new-child-id')]
;; ==== Check
(t/is (some? copy-root'))
(t/is (some? copy-new-child'))
(t/is (= (:touched copy-root') nil))
(t/is (= (:touched copy-new-child') nil))
(t/is (ctst/parent-of? copy-root' copy-new-child'))
(t/is (ctk/is-main-of? main-free-shape' copy-new-child'))))
(t/deftest test-sync-when-deleting-shape
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(tho/add-simple-component-with-copy :component1
:main-root
:main-child
:copy-root
:copy-root-params {:children-labels [:copy-child]}))
page (thf/current-page file)
main-child (ths/get-shape file :main-child)
;; ==== Action
;; IMPORTANT: as modifying copies structure is now forbidden, this action will not
;; delete the child shape, but hide it (thus setting the visibility group).
[_all-parents changes1]
(cls/generate-delete-shapes (pcb/empty-changes)
file
page
(:objects page)
#{(:id main-child)}
{})
updated-file (thf/apply-changes file changes1)
changes2 (cll/generate-sync-file-changes (pcb/empty-changes)
nil
:components
(:id updated-file)
(thi/id :component1)
(:id updated-file)
{(:id updated-file) updated-file}
(:id updated-file))
file' (thf/apply-changes updated-file changes2)
;; ==== Get
copy-root' (ths/get-shape file' :copy-root)
copy-child' (ths/get-shape file' :copy-child)]
;; ==== Check
(t/is (some? copy-root'))
(t/is (nil? copy-child'))
(t/is (= (:touched copy-root') nil))
(t/is (empty? (:shapes copy-root')))))
(t/deftest test-sync-when-moving-shape
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(tho/add-component-with-many-children-and-copy :component1
:main-root
[:main-child1 :main-child2 :main-child3]
:copy-root
:copy-root-params {:children-labels [:copy-child1
:copy-child2
:copy-child3]})
(ths/add-sample-shape :free-shape))
page (thf/current-page file)
main-child1 (ths/get-shape file :main-child1)
;; ==== Action
changes1 (cls/generate-relocate (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-objects (:objects page)))
(thi/id :main-root) ; parent-id
2 ; to-index
#{(:id main-child1)}) ; ids
updated-file (thf/apply-changes file changes1)
changes2 (cll/generate-sync-file-changes (pcb/empty-changes)
nil
:components
(:id updated-file)
(thi/id :component1)
(:id updated-file)
{(:id updated-file) updated-file}
(:id updated-file))
file' (thf/apply-changes updated-file changes2)
;; ==== Get
copy-root' (ths/get-shape file' :copy-root)
copy-child1' (ths/get-shape file' :copy-child1)
copy-child2' (ths/get-shape file' :copy-child2)
copy-child3' (ths/get-shape file' :copy-child3)]
;; ==== Check
(t/is (some? copy-root'))
(t/is (some? copy-child1'))
(t/is (= (:touched copy-root') nil))
(t/is (= (:touched copy-child1') nil))
(t/is (= (:touched copy-child2') nil))
(t/is (= (:touched copy-child3') nil))
(t/is (= (second (:shapes copy-root')) (:id copy-child1')))
(t/is (= (first (:shapes copy-root')) (:id copy-child2')))
(t/is (= (nth (:shapes copy-root') 2) (:id copy-child3')))))
(t/deftest test-sync-when-changing-upper
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(tho/add-nested-component-with-copy :component1
:main1-root
:main1-child
:component2
:main2-root
:main2-nested-head
:copy2-root
:main2-root-params {:fills (ths/sample-fills-color
:fill-color "#abcdef")}))
page (thf/current-page file)
main2-root (ths/get-shape file :main2-root)
;; ==== Action
changes1 (cls/generate-update-shapes (pcb/empty-changes nil (:id page))
#{(:id main2-root)}
(fn [shape]
(assoc shape :fills (ths/sample-fills-color :fill-color "#fabada")))
(:objects page)
{})
updated-file (thf/apply-changes file changes1)
changes2 (cll/generate-sync-file-changes (pcb/empty-changes)
nil
:components
(:id updated-file)
(thi/id :component2)
(:id updated-file)
{(:id updated-file) updated-file}
(:id updated-file))
file' (thf/apply-changes updated-file changes2)
;; ==== Get
copy2-root' (ths/get-shape file' :copy2-root)
fills' (:fills copy2-root')
fill' (first fills')]
;; ==== Check
(t/is (some? copy2-root'))
(t/is (= (count fills') 1))
(t/is (= (:fill-color fill') "#fabada"))
(t/is (= (:fill-opacity fill') 1))
(t/is (= (:touched copy2-root') nil))))
(t/deftest test-sync-when-changing-lower-near
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(tho/add-nested-component-with-copy :component1
:main1-root
:main1-child
:component2
:main2-root
:main2-nested-head
:copy2-root
:copy2-root-params {:children-labels [:copy2-child]}))
page (thf/current-page file)
main2-nested-head (ths/get-shape file :main2-nested-head)
;; ==== Action
changes1 (cls/generate-update-shapes (pcb/empty-changes nil (:id page))
#{(:id main2-nested-head)}
(fn [shape]
(assoc shape :fills (ths/sample-fills-color :fill-color "#fabada")))
(:objects page)
{})
updated-file (thf/apply-changes file changes1)
changes2 (cll/generate-sync-file-changes (pcb/empty-changes)
nil
:components
(:id updated-file)
(thi/id :component2)
(:id updated-file)
{(:id updated-file) updated-file}
(:id updated-file))
file' (thf/apply-changes updated-file changes2)
;; ==== Get
copy2-root' (ths/get-shape file' :copy2-root)
copy2-child' (ths/get-shape file' :copy2-child)
fills' (:fills copy2-child')
fill' (first fills')]
;; ==== Check
(t/is (some? copy2-root'))
(t/is (some? copy2-child'))
(t/is (= (count fills') 1))
(t/is (= (:fill-color fill') "#fabada"))
(t/is (= (:fill-opacity fill') 1))
(t/is (= (:touched copy2-root') nil))
(t/is (= (:touched copy2-child') nil))))
(t/deftest test-sync-when-changing-lower-remote
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(tho/add-nested-component-with-copy :component1
:main1-root
:main1-child
:component2
:main2-root
:main2-nested-head
:copy2-root
:copy2-root-params {:children-labels [:copy2-child]}))
page (thf/current-page file)
main1-root (ths/get-shape file :main1-root)
;; ==== Action
changes1 (cls/generate-update-shapes (pcb/empty-changes nil (:id page))
#{(:id main1-root)}
(fn [shape]
(assoc shape :fills (ths/sample-fills-color :fill-color "#fabada")))
(:objects page)
{})
updated-file (thf/apply-changes file changes1)
changes2 (cll/generate-sync-file-changes (pcb/empty-changes)
nil
:components
(:id updated-file)
(thi/id :component1)
(:id updated-file)
{(:id updated-file) updated-file}
(:id updated-file))
synced-file (thf/apply-changes updated-file changes2)
changes3 (cll/generate-sync-file-changes (pcb/empty-changes)
nil
:components
(:id synced-file)
(thi/id :component2)
(:id synced-file)
{(:id synced-file) synced-file}
(:id synced-file))
file' (thf/apply-changes synced-file changes3)
;; ==== Get
copy2-root' (ths/get-shape file' :copy2-root)
copy2-child' (ths/get-shape file' :copy2-child)
fills' (:fills copy2-child')
fill' (first fills')]
;; ==== Check
(t/is (some? copy2-root'))
(t/is (some? copy2-child'))
(t/is (= (count fills') 1))
(t/is (= (:fill-color fill') "#fabada"))
(t/is (= (:fill-opacity fill') 1))
(t/is (= (:touched copy2-root') nil))
(t/is (= (:touched copy2-child') nil))))
(t/deftest test-no-sync-changes-when-only-position-changes
;; Regression: the library sync dialog was shown even when a library component
;; was only moved (x/y changed). Position changes are normalised by
;; reposition-shape during sync and never propagate to copies, so
;; generate-sync-file-changes must return empty :redo-changes in this case.
(let [;; ==== Setup
;; Use integer width/height so that floating-point arithmetic of
;; move(+delta) followed by reposition(-delta) cancels out exactly.
file (-> (thf/sample-file :file1)
(tho/add-simple-component-with-copy :component1
:main-root
:main-child
:copy-root
:main-root-params {:width 100 :height 100}
:main-child-params {:width 50 :height 50}))
page (thf/current-page file)
main-root (ths/get-shape file :main-root)
main-child (ths/get-shape file :main-child)
;; ==== Action
;; Move the entire main component (root + child) by a non-zero integer delta.
;; This is a position-only change: no fills, strokes or other
;; attributes are modified.
delta (gpt/point 100 150)
changes1 (cls/generate-update-shapes (pcb/empty-changes nil (:id page))
#{(:id main-root) (:id main-child)}
(fn [shape] (gsh/move shape delta))
(:objects page)
{})
updated-file (thf/apply-changes file changes1)
;; Run the full sync to check whether any real redo-changes are produced.
;; The fixed frontend code filters out libraries whose sync produces no
;; :redo-changes before showing the "library updated" notification.
sync-changes (cll/generate-sync-file-changes (pcb/empty-changes)
nil
:components
(:id updated-file)
(thi/id :component1)
(:id updated-file)
{(:id updated-file) updated-file}
(:id updated-file))]
;; ==== Check
;; A position-only change in the main component must not propagate to copies
;; and therefore must produce no redo-changes.
(t/is (empty? (:redo-changes sync-changes)))))
(t/deftest test-update-main-from-a-copy-with-reordered-children
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(tho/add-component-with-many-children-and-copy :component1
:main-root
[:main-child1 :main-child2]
:copy-root
:copy-root-params {:children-labels [:copy-child1 :copy-child2]}))
page (thf/current-page file)
;; The UI never reorders the children of a copy, so the order is
;; set directly: the copy holds them in the reverse order of the main.
file (update-in file [:data :pages-index (:id page) :objects (thi/id :copy-root) :shapes]
(comp vec reverse))
changes (cls/generate-update-shapes (pcb/empty-changes nil (:id page))
#{(thi/id :copy-child2)}
#(assoc % :hidden true)
(:objects page)
{})
file (thf/apply-changes file changes)
;; ==== Action
page (thf/current-page file)
container (ctn/make-container page :page)
changes (-> (pcb/empty-changes)
(pcb/with-container container)
(cll/generate-sync-shape-inverse {(:id file) file}
container
(thi/id :copy-root)))
file' (thf/apply-changes file changes)
;; ==== Get
main-root' (ths/get-shape file' :main-root)
main-child2' (ths/get-shape file' :main-child2)]
;; ==== Check
(t/is (true? (:hidden main-child2')))
(t/is (= [(thi/id :main-child2) (thi/id :main-child1)] (:shapes main-root')))))
Reference in New Issue View Git Blame Copy Permalink