diff --git a/backend/test/backend_tests/branch_copies_follow_component_test.clj b/backend/test/backend_tests/branch_copies_follow_component_test.clj index bc242ec026..48e52b1185 100644 --- a/backend/test/backend_tests/branch_copies_follow_component_test.clj +++ b/backend/test/backend_tests/branch_copies_follow_component_test.clj @@ -97,12 +97,43 @@ [(set-fill page-id mi-id color) (sync-fill page-id copy-id color)]) +(def ^:private token-colors + "The colour tokens `setup` adds to main, by name." + {"green" "#00ff00" "blue" "#0000ff" "red" "#ff0000" "yellow" "#ffff00"}) + +(defn- apply-fill-token + "Apply the colour token `token-name` to the fill of `id` by hand, as + the workspace's token application does: the token goes into + `:applied-tokens` (on a copy this marks the fill overridden) and its + colour into the fill without marking anything." + [page-id id token-name] + {:type :mod-obj :page-id page-id :id id + :operations [{:type :set :attr :applied-tokens :val {:fill token-name}} + {:type :set :attr :fills :val (fills (get token-colors token-name)) + :ignore-touched true}]}) + +(defn- sync-fill-token + "The token and fill the workspace's component sync writes on an + untouched copy." + [page-id id token-name] + {:type :mod-obj :page-id page-id :id id + :operations [{:type :set :attr :applied-tokens :val {:fill token-name} :ignore-touched true} + {:type :set :attr :fills :val (fills (get token-colors token-name)) + :ignore-touched true}]}) + +(defn- tokenize-component + "Apply the token `token-name` to the component's main fill and sync the + copy `copy-id`." + [page-id mi-id copy-id token-name] + [(apply-fill-token page-id mi-id token-name) + (sync-fill-token page-id copy-id token-name)]) + (defn- setup "Main holding a local component (main `:mi-id`) with one copy - (`:copy-id`), branched once. `n` numbers the profile and the file, so - one test can build more than one. With `:override`, the copy's fill is - overridden to that colour before branching, so the base holds the - override." + (`:copy-id`) and a token set with the colour tokens `token-colors`, + branched once. `n` numbers the profile and the file, so one test can + build more than one. With `:override`, the copy's fill is overridden + to that colour before branching, so the base holds the override." [n & {:keys [override]}] (let [profile (th/create-profile* n {:is-active true}) file (th/create-file* n {:profile-id (:id profile) @@ -112,7 +143,8 @@ page-id (-> (get-file profile main-id) :data :pages first) comp-id (uuid/random) mi-id (uuid/random) - copy-id (uuid/random)] + copy-id (uuid/random) + set-id (uuid/random)] (save profile main-id [(add-root page-id mi-id (component-frame mi-id {:component-root true :main-instance true @@ -120,7 +152,14 @@ :component-file main-id})) {:type :add-component :id comp-id :name "Button" :path "" :main-instance-id mi-id :main-instance-page page-id} - (copy-of page-id copy-id comp-id mi-id main-id)]) + (copy-of page-id copy-id comp-id mi-id main-id) + {:type :set-token-set :id set-id :attrs {:id set-id :name "core"}}]) + (save profile main-id + (mapv (fn [[token-name color]] + (let [token-id (uuid/random)] + {:type :set-token :set-id set-id :token-id token-id + :attrs {:id token-id :name token-name :type :color :value color}})) + token-colors)) (when override (save profile main-id [(set-fill page-id copy-id override)])) (let [branch (:result (th/command! {::th/type :create-file-branch @@ -373,3 +412,96 @@ (t/is (= (fill branch page-id mi-id) (fill branch page-id copy-id))) (t/is (empty? (validation-codes branch)))) (t/is (follows-component? profile branch-file-id page-id comp-id mi-id copy-id))))) + +;; --- tokens: the rule per applied-token slot + +(defn- fill-token + [file page-id id] + (get-in (shape file page-id id) [:applied-tokens :fill])) + +(defn- integrate + "Merge the branch (`:merge`) or update it from main (`:update`), and + return the result and the file the integration wrote to." + [dir {:keys [profile main-id branch-file-id] :as ctx} & {:as params}] + (case dir + :merge [(merge-branch ctx params) (get-file profile main-id)] + :update [(update-branch ctx params) (get-file profile branch-file-id)])) + +(def ^:private ok-status + {:merge :merged :update :updated}) + +(t/deftest a-token-both-sides-applied-to-the-component-conflicts-on-it-only + (with-redefs [cf/flags (conj cf/flags :branching)] + (doseq [[n dir side token] [[1 :merge :branch "green"] [2 :merge :main "blue"] + [3 :update :branch "green"] [4 :update :main "blue"]]] + (t/testing (str dir " resolved to " side) + (let [{:keys [profile main-id branch-file-id page-id mi-id copy-id] :as ctx} (setup n)] + (save profile branch-file-id (tokenize-component page-id mi-id copy-id "green")) + (save profile main-id (tokenize-component page-id mi-id copy-id "blue")) + + (let [[out] (integrate dir ctx)] + (t/is (= :conflicts (-> out :result :status))) + (t/is (= #{mi-id} (conflict-ids out)))) + + (let [[out file] (integrate dir ctx :resolutions {mi-id side})] + (t/is (nil? (:error out))) + (t/is (= (ok-status dir) (-> out :result :status))) + (t/is (= token (fill-token file page-id mi-id))) + (t/is (= token (fill-token file page-id copy-id))) + (t/is (= (get token-colors token) (fill file page-id copy-id))) + (t/is (empty? (:touched (shape file page-id copy-id)))) + (t/is (empty? (validation-codes file))))))))) + +(t/deftest a-token-one-side-applied-to-a-copy-by-hand-stays + (with-redefs [cf/flags (conj cf/flags :branching)] + (doseq [[n dir] [[1 :merge] [2 :update]]] + (t/testing (str dir) + (let [{:keys [profile main-id branch-file-id page-id mi-id copy-id] :as ctx} (setup n) + ;; the source overrides the copy, the target edits the component + [source target] (if (= :merge dir) + [branch-file-id main-id] + [main-id branch-file-id])] + (save profile source [(apply-fill-token page-id copy-id "red")]) + (save profile target (tokenize-component page-id mi-id copy-id "blue")) + + (let [[out file] (integrate dir ctx)] + (t/is (nil? (:error out))) + (t/is (= (ok-status dir) (-> out :result :status))) + (t/is (= "blue" (fill-token file page-id mi-id))) + (t/is (= "red" (fill-token file page-id copy-id))) + (t/is (= "#ff0000" (fill file page-id copy-id))) + (t/is (contains? (:touched (shape file page-id copy-id)) :fill-group)) + (t/is (empty? (validation-codes file))))))))) + +(t/deftest a-token-both-sides-applied-to-a-copy-by-hand-is-a-conflict + (with-redefs [cf/flags (conj cf/flags :branching)] + (doseq [[n dir] [[1 :merge] [2 :update]]] + (t/testing (str dir) + (let [{:keys [profile main-id branch-file-id page-id copy-id] :as ctx} (setup n)] + (save profile branch-file-id [(apply-fill-token page-id copy-id "red")]) + (save profile main-id [(apply-fill-token page-id copy-id "yellow")]) + + (let [[out] (integrate dir ctx)] + (t/is (= :conflicts (-> out :result :status))) + (t/is (= #{copy-id} (conflict-ids out))))))))) + +(t/deftest a-token-applied-to-the-component-reaches-a-copy-the-other-side-added + (with-redefs [cf/flags (conj cf/flags :branching)] + (doseq [[n dir] [[1 :merge] [2 :update]]] + (t/testing (str dir) + (let [{:keys [profile main-id branch-file-id page-id comp-id mi-id copy-id] :as ctx} (setup n) + new-copy (uuid/random) + ;; the source tokenizes the component, the target adds a copy + [source target] (if (= :merge dir) + [branch-file-id main-id] + [main-id branch-file-id])] + (save profile source (tokenize-component page-id mi-id copy-id "green")) + (save profile target [(copy-of page-id new-copy comp-id mi-id target)]) + + (let [[out file] (integrate dir ctx)] + (t/is (nil? (:error out))) + (t/is (= (ok-status dir) (-> out :result :status))) + (t/is (= "green" (fill-token file page-id new-copy))) + (t/is (= "#00ff00" (fill file page-id new-copy))) + (t/is (empty? (:touched (shape file page-id new-copy)))) + (t/is (empty? (validation-codes file))))))))) diff --git a/common/src/app/common/files/branch_merge.cljc b/common/src/app/common/files/branch_merge.cljc index 7c149d7374..e95575517d 100644 --- a/common/src/app/common/files/branch_merge.cljc +++ b/common/src/app/common/files/branch_merge.cljc @@ -29,6 +29,7 @@ [app.common.types.components-list :as ctkl] [app.common.types.file :as ctf] [app.common.types.pages-list :as ctpl] + [app.common.types.token :as ctt] [app.common.types.tokens-lib :as ctob] [app.common.types.tokens-status :as ctos] [app.common.uuid :as uuid] @@ -349,6 +350,10 @@ ;; (`sync-copies`) sets it; ;; - one side overrode it: that side's value wins; ;; - both sides overrode it: a real conflict. +;; A copy's `:applied-tokens` follows the same rule one token attr (slot) +;; at a time, as Penpot's own sync does (`generate-update-tokens` in +;; `app.common.logic.libraries`): a slot follows the component unless +;; the sync group of the shape attr it drives is touched. ;; The sides are the comparison roles: `:main` is `theirs` (the target), ;; `:branch` is `ours` (the source). @@ -364,17 +369,57 @@ (and (contains? touched group) (not= (get base attr) (get shape attr)))) +(defn- token-slot-group + "The sync group a copy marks touched when a token is applied by hand + to its `token-attr` (`app.common.types.container/set-shape-attr`): + the group of the shape attr the token drives, or `:content-group` for + a token a text shape keeps in its content (the typography ones)." + [type token-attr] + (or (ctk/resolve-sync-group type (ctt/token-attr->shape-attr token-attr)) + (when (and (= :text type) (contains? ctt/attrs-in-text-content token-attr)) + :content-group))) + +(defn- token-slot-sides + "The copy rule over the token attrs (slots) of `:applied-tokens` whose + token differs between `st` and `so`. A side overrode a slot when the + slot's group (`token-slot-group`) is in its `touched` set and its token + moved off the base one. Returns `{:slots {token-attr side} + :open-slots #{token-attr}}`: the rule's side for each slot no more than + one side overrode, and the slots both overrode or that drive no synced + attr." + [type sb st so tt ot] + (let [base-tokens (:applied-tokens sb) + theirs-tokens (:applied-tokens st) + ours-tokens (:applied-tokens so)] + (reduce + (fn [acc slot] + (let [group (token-slot-group type slot) + base (get base-tokens slot) + t? (and (some? group) (contains? tt group) (not= base (get theirs-tokens slot))) + o? (and (some? group) (contains? ot group) (not= base (get ours-tokens slot)))] + (cond + (or (nil? group) (and t? o?)) (update acc :open-slots conj slot) + o? (assoc-in acc [:slots slot] :branch) + :else (assoc-in acc [:slots slot] :main)))) + {:slots {} :open-slots #{}} + (sort-by name (filter #(not= (get theirs-tokens %) (get ours-tokens %)) + (set/union (set (keys theirs-tokens)) (set (keys ours-tokens)))))))) + (defn- copy-conflict-sides "How the copy rule settles a shape both sides changed differently, or nil when the shape is not such a copy. `sb`/`st`/`so` are the classification shapes (stripped, see `strip-shapes`) of the base, `theirs` and `ours`; `t`/`o` the full ones, which carry `:touched`. - Returns `{:auto {attr side} :open #{attr}}` over the attrs that differ - between the two sides: `:auto` holds the side the rule picks for each - sync attr no more than one side overrode (`:main` when none did), and - `:open` the attrs left to the user, the ones both sides overrode and - the ones outside every sync group." + Returns `{:auto {attr side} :open #{attr} :slots {token-attr side} + :open-slots #{token-attr}}` over the attrs that differ between the two + sides: `:auto` holds the side the rule picks for each sync attr no + more than one side overrode (`:main` when none did), and `:open` the + attrs left to the user, the ones both sides overrode and the ones + outside every sync group. `:applied-tokens` is settled per slot + (`token-slot-sides`): it is open while any slot is, the user's side + then deciding the open slots only, and otherwise auto, on `:branch` + when the source wins any slot." [sb st so t o] (when (and (some? sb) (some? st) (some? so) (ctk/in-component-copy? t) @@ -388,14 +433,22 @@ (when (and (not= sb st) (not= sb so) (not= st so)) (reduce-kv (fn [acc attr _] - (let [group (ctk/resolve-sync-group (:type so) attr) - t? (and (some? group) (overrode? group tt sb st attr)) - o? (and (some? group) (overrode? group ot sb so attr))] - (cond - (or (nil? group) (and t? o?)) (update acc :open conj attr) - o? (assoc-in acc [:auto attr] :branch) - :else (assoc-in acc [:auto attr] :main)))) - {:auto {} :open #{}} + (if (= :applied-tokens attr) + (let [{:keys [slots open-slots]} (token-slot-sides (:type so) sb st so tt ot)] + (cond-> (assoc acc :slots slots :open-slots open-slots) + (seq open-slots) + (update :open conj attr) + + (empty? open-slots) + (assoc-in [:auto attr] (if (some #{:branch} (vals slots)) :branch :main)))) + (let [group (ctk/resolve-sync-group (:type so) attr) + t? (and (some? group) (overrode? group tt sb st attr)) + o? (and (some? group) (overrode? group ot sb so attr))] + (cond + (or (nil? group) (and t? o?)) (update acc :open conj attr) + o? (assoc-in acc [:auto attr] :branch) + :else (assoc-in acc [:auto attr] :main))))) + {:auto {} :open #{} :slots {} :open-slots #{}} (shallow-attr-diff st so)))))) (defn- settle-copy-conflicts @@ -432,23 +485,49 @@ "The per-attr resolution of a copy the rule settled (`sides`, see `copy-conflict-sides`): the user's `res` over the open attrs, a side keyword standing for every one of them, and the rule's side for the - rest." - [{:keys [auto open]} res] - (merge (cond - (keyword? res) (zipmap open (repeat res)) - (map? res) res - :else {}) - auto)) + rest. Token slots ride under `::token-slots` (`{token-attr side}`): + the rule's side for the settled ones and the user's `:applied-tokens` + side for the open ones; `:applied-tokens` itself is `:branch` exactly + when some slot comes from the source, so the attr pass writes it." + [{:keys [auto open slots open-slots]} res] + (let [res (merge (cond + (keyword? res) (zipmap open (repeat res)) + (map? res) res + :else {}) + auto)] + (if (or (seq slots) (seq open-slots)) + (let [user (if (= :branch (get res :applied-tokens)) :branch :main) + slots (merge (zipmap open-slots (repeat user)) slots)] + (assoc res + :applied-tokens (if (some #{:branch} (vals slots)) :branch :main) + ::token-slots slots)) + res))) + +(defn- merged-tokens + "The `:applied-tokens` of the copy: the target's tokens `tt`, with each + slot `slots` resolves to `:branch` taken from the source's `ot`." + [tt ot slots] + (not-empty + (reduce-kv (fn [acc slot side] + (if (= :branch side) + (if-some [token (get ot slot)] + (assoc acc slot token) + (dissoc acc slot)) + acc)) + (or tt {}) + slots))) (defn- copy-touched-ops "Operations that write the copy `t` from the source shape `o` without marking overrides the source does not have: every `:set` carries `:ignore-touched`, and one `:set-touched` gives the copy the source's - sync groups for the attrs the operations take (all of them under - `whole?`, when the source shape wins entirely) and its own for the - rest. A plain `:set` on a copy marks the attr's group touched, which - would freeze an attr the source merely synced from its component." - [ops t o whole?] + sync groups for the attrs the operations take, and for the token + slots `slots` takes from the source (see `copy-resolution`), all of + them under `whole?`, when the source shape wins entirely, and its own + for the rest. A plain `:set` on a copy marks the attr's group touched, + which would freeze an attr the source merely synced from its + component." + [ops t o whole? slots] (let [ops (into [] (comp (remove #(= :touched (:attr %))) (map #(assoc % :ignore-touched true))) @@ -460,6 +539,10 @@ (conj text-content-groups group) [group])))) ops) + taken (into taken + (keep (fn [[slot side]] + (when (= :branch side) (token-slot-group (:type o) slot)))) + slots) own (set (:touched t)) touched (if whole? (set (:touched o)) @@ -1554,9 +1637,11 @@ A component copy both sides changed is settled per attr by the copy rule (see `copy-conflict-sides`): only the attrs both sides overrode by hand, and the ones outside every sync group, stay in its conflict. - A copy the rule settles entirely leaves `:conflicts`, and comes back - as a `:modified` change over the attrs the source overrode, which the - merge takes from it." + Its `:applied-tokens` is settled one token attr at a time, and stays + in the conflict, whole, while any token attr is open. A copy the rule + settles entirely leaves `:conflicts`, and comes back as a `:modified` + change over the attrs the source overrode, which the merge takes from + it." ([base main branch dir] (compute-merge* base main branch dir nil)) ([base main branch dir {:keys [only-pages]}] (compute-merge* base main branch dir only-pages))) @@ -1946,10 +2031,22 @@ ops (if whole? (shape-set-ops t o) (shape-resolved-ops (get sto id) (get soo id) t o res)) + ;; a copy's tokens are merged slot by slot + ;; (see `copy-resolution`) + slots (when-not whole? (::token-slots res)) + ops (if (seq slots) + (mapv (fn [op] + (if (= :applied-tokens (:attr op)) + (assoc op :val (merged-tokens (:applied-tokens t) + (:applied-tokens o) + slots)) + op)) + ops) + ops) ;; a copy keeps the overrides of the side ;; each attr comes from, never new ones ops (if (or (ctk/in-component-copy? t) (ctk/in-component-copy? o)) - (copy-touched-ops ops t o whole?) + (copy-touched-ops ops t o whole? slots) ops)] (when (seq ops) {:type :mod-obj :page-id page-id :id id :operations ops}))) @@ -2162,11 +2259,13 @@ A component copy takes, per attr, the side the copy rule picks (`copy-conflict-sides`) whatever `resolutions` says, which only - decides the attrs the rule leaves open. The operations written on a - copy never mark an override the source does not have - (`copy-touched-ops`). The attrs the rule leaves at the target's value - follow the component through `sync-copies`, which the caller runs - after applying these changes." + decides the attrs the rule leaves open. A copy's `:applied-tokens` is + written as one map merged token attr by token attr: the rule's side + for the settled ones, the user's side for the open ones + (`copy-resolution`). The operations written on a copy never mark an + override the source does not have (`copy-touched-ops`). The attrs the + rule leaves at the target's value follow the component through + `sync-copies`, which the caller runs after applying these changes." ([base main branch] (compute-changes base main branch {})) ([base main branch resolutions] diff --git a/common/test/common_tests/files_branch_merge_test.cljc b/common/test/common_tests/files_branch_merge_test.cljc index 2221aa5cd0..ff2e09a594 100644 --- a/common/test/common_tests/files_branch_merge_test.cljc +++ b/common/test/common_tests/files_branch_merge_test.cljc @@ -2019,6 +2019,77 @@ (t/is (= [{:type :set :attr :fills :val [{:fill-color "green"}] :ignore-touched true}] (copy-ops (:changes (bm/compute-changes base base branch))))))) +;; --- the copy rule per applied-token slot --- + +(defn- tokens-copy + "A copy rect of the main `:m` with `applied-tokens`, an optional + `:touched` set, and the fill a token on `:fill` resolves to." + [applied-tokens & {:keys [touched]}] + (cond-> {:id :k :name "K" :type :rect :shape-ref :m + :fills [{:fill-color (get applied-tokens :fill "none")}]} + (seq applied-tokens) (assoc :applied-tokens applied-tokens) + (some? touched) (assoc :touched touched))) + +(t/deftest copy-token-no-side-overrode-follows-the-component + (let [base (mkdata {:k (tokens-copy {})}) + main (mkdata {:k (tokens-copy {:fill "blue"})}) + branch (mkdata {:k (tokens-copy {:fill "green"})}) + r (bm/compute-merge base main branch :branch->main)] + (t/is (empty? (:conflicts r))) + (t/is (empty? (:changes r))) + (t/is (empty? (copy-ops (:changes (bm/compute-changes base main branch {} r))))))) + +(t/deftest copy-token-slots-settle-one-at-a-time + ;; main applied a fill token by hand, the branch a radius token by hand: + ;; each side's slot wins, and the copy carries both + (let [base (mkdata {:k (tokens-copy {})}) + main (mkdata {:k (tokens-copy {:fill "yellow"} :touched #{:fill-group})}) + branch (mkdata {:k (tokens-copy {:r1 "round"} :touched #{:radius-group})}) + r (bm/compute-merge base main branch :branch->main)] + (t/is (empty? (:conflicts r))) + (t/is (= [[:k :modified #{:applied-tokens}]] + (mapv (juxt :id :status (comp set keys :changed-attrs)) (:changes r)))) + (t/is (= [{:type :set :attr :applied-tokens :val {:fill "yellow" :r1 "round"} :ignore-touched true} + {:type :set-touched :touched #{:fill-group :radius-group}}] + (copy-ops (:changes (bm/compute-changes base main branch {} r))))))) + +(t/deftest copy-token-slot-both-sides-overrode-is-a-conflict + ;; both applied a fill token by hand; only the branch a radius token: + ;; the conflict asks about `:applied-tokens`, and the user's side + ;; decides the fill slot only + (let [base (mkdata {:k (tokens-copy {})}) + main (mkdata {:k (tokens-copy {:fill "yellow"} :touched #{:fill-group})}) + branch (mkdata {:k (tokens-copy {:fill "red" :r1 "round"} + :touched #{:fill-group :radius-group})}) + r (bm/compute-merge base main branch :branch->main)] + (t/is (= [:k] (mapv :id (:conflicts r)))) + (t/is (contains? (:changed-attrs (first (:conflicts r))) :applied-tokens)) + (t/testing "resolved to main, the radius slot still comes from the branch" + (t/is (= {:fill "yellow" :r1 "round"} + (->> (copy-ops (:changes (bm/compute-changes base main branch + {:k {:applied-tokens :main :fills :main}} + r))) + (filter #(= :applied-tokens (:attr %))) + first :val)))) + (t/testing "resolved to the branch, both slots come from it" + (t/is (= {:fill "red" :r1 "round"} + (->> (copy-ops (:changes (bm/compute-changes base main branch {:k :branch} r))) + (filter #(= :applied-tokens (:attr %))) + first :val)))))) + +(t/deftest copy-token-rule-holds-updating-from-main + ;; main (the source) applied a fill token to the copy by hand; the + ;; branch synced its own component edit + (let [base (mkdata {:k (tokens-copy {})}) + main (mkdata {:k (tokens-copy {:fill "red"} :touched #{:fill-group})}) + branch (mkdata {:k (tokens-copy {:fill "green"})}) + r (bm/compute-merge base main branch :main->branch)] + (t/is (empty? (:conflicts r))) + (t/is (= {:fill "red"} + (->> (copy-ops (:changes (bm/compute-changes base branch main {} r))) + (filter #(= :applied-tokens (:attr %))) + first :val))))) + (t/deftest sync-copies-follows-nested-components ;; a change to component1's main syncs its nested copy inside ;; component2's main, which in turn syncs component2's copy