diff --git a/backend/src/app/rpc/commands/files_branch.clj b/backend/src/app/rpc/commands/files_branch.clj index c645ab31aa..e9d8ddbb0e 100644 --- a/backend/src/app/rpc/commands/files_branch.clj +++ b/backend/src/app/rpc/commands/files_branch.clj @@ -1106,7 +1106,13 @@ takes a safety snapshot of main, applies the merge to main through the production change pipeline, records the merge with its `comment` (`file_branch_merge`), deletes the branch and its file and notifies - open clients via msgbus." + open clients via msgbus. + + Copies follow the merged components: a copy attr no side overrode is + no conflict, and after the merge (and its repair) applies, the copies + of every local component whose main it changed are synced on main + (`bm/sync-copies`). The sync changes go into the same `file_change` + row and the same validation as the merge." {::doc/added "2.16" ::webhooks/event? true ::sm/params schema:merge-file-branch @@ -1283,24 +1289,39 @@ ;; under the pre-allocated ids the changes reference (copy-media-rows! conn main-id fresh-map new-media) - (let [merged (-> main-file - (update :revn inc) - (update :data #(cpc/process-changes % changes))) - libs (bfc/get-resolved-file-libraries cfg merged) - errors (not-empty (cfv/validate-file merged libs)) - repair (if errors - (cfr/repair-file merged libs errors) - []) - merged (cond-> merged - (seq repair) - (update :data cpc/process-changes repair)) + (let [applied (-> main-file + (update :revn inc) + (update :data #(cpc/process-changes % changes))) + libs (bfc/get-resolved-file-libraries cfg applied) + errors (not-empty (cfv/validate-file applied libs)) + repair (if errors + (cfr/repair-file applied libs errors) + []) + merged (cond-> applied + (seq repair) + (update :data cpc/process-changes repair)) + + ;; copies follow the integrated components: + ;; the copies of every local component whose + ;; main the merge (or the repair) changed are + ;; synced, as the workspace syncs them after + ;; an edit (`bm/sync-copies`) + {synced :data sync :changes} + (bm/sync-copies main-id (:data merged) + (into (bm/changed-components (:data main-file) changes) + (bm/changed-components (:data applied) repair))) + merged (assoc merged :data synced) + libs (if (seq sync) + (bfc/get-resolved-file-libraries cfg merged) + libs) ;; the validator, not the merge, decides - ;; validity: the repaired result goes back - ;; to it, and whatever stays broken refuses - ;; the merge and rolls the transaction back - errors (when errors - (not-empty (cfv/validate-file merged libs)))] + ;; validity: the repaired and synced result + ;; goes back to it, and whatever stays broken + ;; refuses the merge and rolls the + ;; transaction back + errors (when (or errors (seq sync)) + (not-empty (cfv/validate-file merged libs)))] (when errors (ex/raise :type :validation :code :merge-result-invalid @@ -1319,7 +1340,9 @@ :revn (:revn merged) :version (:version merged) :features (into-array (:features merged)) - :changes (blob/encode (into (vec changes) repair))} + :changes (blob/encode (-> (vec changes) + (into repair) + (into sync)))} {::db/return-keys false}) ;; a page the merge left as it was keeps its @@ -1408,7 +1431,12 @@ `{:status :unsupported}` for change kinds not yet translatable. On success it applies main's changes (and any conflicts resolved to main) to the branch, takes a safety snapshot, and repositions the merge base - to the current state of main." + to the current state of main. + + Copies follow the integrated components as in a merge: after main's + changes apply, the copies of every local component whose main they + changed are synced on the branch, in the branch's id space + (`bm/sync-copies`), before the squash records the branch's net." {::doc/added "2.16" ::webhooks/event? true ::sm/params schema:update-branch-from-main @@ -1649,25 +1677,41 @@ ;; rows — or the remapped changes would name ids ;; it does not hold and apply to nothing (a ;; `:del-media` of a paired copy would keep it) - updated (-> branch-file + before (bm/remap-refs (:data branch-file) media-map) + written (-> branch-file (update :revn inc) - (update :data #(cpc/process-changes - (bm/remap-refs % media-map) - applied))) - libs (bfc/get-resolved-file-libraries cfg updated) - errors (not-empty (cfv/validate-file updated libs)) + (assoc :data (cpc/process-changes before applied))) + libs (bfc/get-resolved-file-libraries cfg written) + errors (not-empty (cfv/validate-file written libs)) repair (if errors - (cfr/repair-file updated libs errors) + (cfr/repair-file written libs errors) []) - updated (cond-> updated + updated (cond-> written (seq repair) (update :data cpc/process-changes repair)) + ;; copies follow the integrated components: the + ;; copies of every local component whose main + ;; the update (or the repair) changed are + ;; synced, in the branch's id space the write + ;; runs in (`bm/sync-copies`). The squash below + ;; diffs `updated`, so it records the sync as + ;; branch work + {synced :data sync :changes} + (bm/sync-copies branch-file-id (:data updated) + (into (bm/changed-components before applied) + (bm/changed-components (:data written) repair))) + updated (assoc updated :data synced) + libs (if (seq sync) + (bfc/get-resolved-file-libraries cfg updated) + libs) + ;; the validator, not the update, decides - ;; validity: the repaired result goes back to - ;; it, and whatever stays broken refuses the - ;; update and rolls the transaction back - errors (when errors + ;; validity: the repaired and synced result + ;; goes back to it, and whatever stays broken + ;; refuses the update and rolls the + ;; transaction back + errors (when (or errors (seq sync)) (not-empty (cfv/validate-file updated libs)))] (when errors (ex/raise :type :validation @@ -1686,7 +1730,9 @@ :revn (:revn updated) :version (:version updated) :features (into-array (:features updated)) - :changes (blob/encode (into (vec applied) repair))} + :changes (blob/encode (-> (vec applied) + (into repair) + (into sync)))} {::db/return-keys false}) ;; SQUASH: the new base (main's current state) diff --git a/backend/test/backend_tests/branch_copies_follow_component_test.clj b/backend/test/backend_tests/branch_copies_follow_component_test.clj new file mode 100644 index 0000000000..b192105544 --- /dev/null +++ b/backend/test/backend_tests/branch_copies_follow_component_test.clj @@ -0,0 +1,304 @@ +;; 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 Sucursal en España SL + +(ns backend-tests.branch-copies-follow-component-test + "Copies follow their merged component. When a branch is merged into + main, or main's changes are brought into the branch, the copies of a + local component end up as Penpot's component sync leaves them after + the integrated component: an attribute no side overrode by hand follows + the component, an attribute one side overrode keeps that side's value, + and only an attribute both sides overrode differently is a conflict. + + Each test builds main with a local component (main `:mi-id`) and one + copy of it (`:copy-id`), branches it, edits both sides and integrates. + A component edit writes the main and, as the workspace's sync does, + the untouched copy with `:ignore-touched`; an override writes the copy + alone, which marks it touched." + (:require + [app.binfile.common :as bfc] + [app.common.features :as cfeat] + [app.common.files.validate :as cfv] + [app.common.types.shape :as cts] + [app.common.uuid :as uuid] + [app.config :as cf] + [app.db :as db] + [app.rpc :as-alias rpc] + [backend-tests.helpers :as th] + [clojure.test :as t])) + +(t/use-fixtures :once th/state-init) +(t/use-fixtures :each th/database-reset) + +(defn- save + "Apply `changes` to `file-id` through the RPC, as the workspace does." + [profile file-id changes] + (let [file (th/db-get :file {:id file-id}) + out (th/command! {::th/type :update-file + ::rpc/profile-id (:id profile) + :id file-id + :session-id (uuid/random) + :revn (:revn file) + :vern (:vern file) + :features cfeat/supported-features + :changes changes})] + (t/is (nil? (:error out))))) + +(defn- get-file + [profile file-id] + (:result (th/command! {::th/type :get-file + ::rpc/profile-id (:id profile) + :id file-id}))) + +(defn- component-frame + [id attrs] + (merge (cts/setup-shape {:id id :name "Button" :type :frame + :x 0 :y 0 :width 100 :height 100 + :parent-id uuid/zero :frame-id uuid/zero}) + attrs)) + +(defn- add-root + [page-id id obj] + {:type :add-obj :page-id page-id :id id + :parent-id uuid/zero :frame-id uuid/zero :obj obj}) + +(defn- fills + [color] + [{:fill-color color :fill-opacity 1}]) + +(defn- set-fill + "A hand edit of the fill of `id`: on a copy it marks the fill + overridden." + [page-id id color] + {:type :mod-obj :page-id page-id :id id + :operations [{:type :set :attr :fills :val (fills color)}]}) + +(defn- sync-fill + "The fill the workspace's component sync writes on an untouched copy." + [page-id id color] + {:type :mod-obj :page-id page-id :id id + :operations [{:type :set :attr :fills :val (fills color) :ignore-touched true}]}) + +(defn- copy-of + [page-id id component-id main-id component-file] + (add-root page-id id (component-frame id {:component-root true + :component-id component-id + :component-file component-file + :shape-ref main-id}))) + +(defn- edit-component + "Paint the component's main `color` and sync the copy `copy-id`." + [page-id mi-id copy-id color] + [(set-fill page-id mi-id color) + (sync-fill page-id copy-id color)]) + +(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." + [n] + (let [profile (th/create-profile* n {:is-active true}) + file (th/create-file* n {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + main-id (:id file) + page-id (-> (get-file profile main-id) :data :pages first) + comp-id (uuid/random) + mi-id (uuid/random) + copy-id (uuid/random)] + (save profile main-id + [(add-root page-id mi-id (component-frame mi-id {:component-root true + :main-instance true + :component-id comp-id + :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)]) + (let [branch (:result (th/command! {::th/type :create-file-branch + ::rpc/profile-id (:id profile) + :file-id main-id + :name "copies"}))] + {:profile profile + :main-id main-id + :page-id page-id + :comp-id comp-id + :mi-id mi-id + :copy-id copy-id + :branch-id (:id branch) + :branch-file-id (:branch-file-id branch)}))) + +(defn- merge-branch + [{:keys [profile branch-id]} & {:as params}] + (th/command! (merge {::th/type :merge-file-branch + ::rpc/profile-id (:id profile) + :branch-id branch-id} + params))) + +(defn- update-branch + [{:keys [profile branch-id]} & {:as params}] + (th/command! (merge {::th/type :update-branch-from-main + ::rpc/profile-id (:id profile) + :branch-id branch-id} + params))) + +(defn- shape + [file page-id id] + (get-in file [:data :pages-index page-id :objects id])) + +(defn- fill + [file page-id id] + (-> (shape file page-id id) :fills first :fill-color)) + +(defn- validation-codes + [file] + (let [libs (db/run! th/*system* #(bfc/get-resolved-file-libraries % file))] + (into #{} (map :code) (cfv/validate-file file libs)))) + +(defn- conflict-ids + [out] + (into #{} (map :id) (-> out :result :conflicts))) + +;; --- a: one side edits the component, the other adds a copy + +(t/deftest merge-syncs-a-copy-main-added-to-the-branch-component + (with-redefs [cf/flags (conj cf/flags :branching)] + (let [{:keys [profile main-id branch-file-id page-id comp-id mi-id copy-id] :as ctx} (setup 1) + new-copy (uuid/random)] + (save profile branch-file-id (edit-component page-id mi-id copy-id "#00ff00")) + (save profile main-id [(copy-of page-id new-copy comp-id mi-id main-id)]) + + (let [out (merge-branch ctx)] + (t/is (nil? (:error out))) + (t/is (= :merged (-> out :result :status)))) + + (let [main (get-file profile main-id)] + (t/is (= "#00ff00" (fill main page-id mi-id))) + (t/is (= "#00ff00" (fill main page-id new-copy))) + (t/is (= "#00ff00" (fill main page-id copy-id))) + (t/is (empty? (:touched (shape main page-id copy-id)))) + (t/is (empty? (validation-codes main))))))) + +(t/deftest update-syncs-a-copy-the-branch-added-to-main-component + (with-redefs [cf/flags (conj cf/flags :branching)] + (let [{:keys [profile main-id branch-file-id page-id comp-id mi-id copy-id] :as ctx} (setup 1) + new-copy (uuid/random)] + (save profile main-id (edit-component page-id mi-id copy-id "#0000ff")) + (save profile branch-file-id [(copy-of page-id new-copy comp-id mi-id branch-file-id)]) + + (let [out (update-branch ctx)] + (t/is (nil? (:error out))) + (t/is (= :updated (-> out :result :status)))) + + (let [branch (get-file profile branch-file-id)] + (t/is (= "#0000ff" (fill branch page-id mi-id))) + (t/is (= "#0000ff" (fill branch page-id new-copy))) + (t/is (= "#0000ff" (fill branch page-id copy-id))) + (t/is (empty? (:touched (shape branch page-id copy-id)))) + (t/is (empty? (validation-codes branch))))))) + +;; --- b: both sides edit the component, each syncs its copy + +(defn- both-edit-component + [{:keys [profile main-id branch-file-id page-id mi-id copy-id]}] + (save profile branch-file-id (edit-component page-id mi-id copy-id "#00ff00")) + (save profile main-id (edit-component page-id mi-id copy-id "#0000ff"))) + +(t/deftest merge-of-a-component-both-sides-edited-conflicts-on-the-component-only + (with-redefs [cf/flags (conj cf/flags :branching)] + (doseq [[side color] [[:branch "#00ff00"] [:main "#0000ff"]]] + (t/testing (str "resolved to " side) + (let [{:keys [profile main-id page-id mi-id copy-id] :as ctx} (setup (if (= :branch side) 1 2))] + (both-edit-component ctx) + + (let [out (merge-branch ctx)] + (t/is (= :conflicts (-> out :result :status))) + (t/is (= #{mi-id} (conflict-ids out)))) + + (let [out (merge-branch ctx :resolutions {mi-id side})] + (t/is (nil? (:error out))) + (t/is (= :merged (-> out :result :status)))) + + (let [main (get-file profile main-id)] + (t/is (= color (fill main page-id mi-id))) + (t/is (= color (fill main page-id copy-id))) + (t/is (empty? (:touched (shape main page-id copy-id)))) + (t/is (empty? (validation-codes main))))))))) + +(t/deftest update-of-a-component-both-sides-edited-conflicts-on-the-component-only + (with-redefs [cf/flags (conj cf/flags :branching)] + (doseq [[side color] [[:branch "#00ff00"] [:main "#0000ff"]]] + (t/testing (str "resolved to " side) + (let [{:keys [profile branch-file-id page-id mi-id copy-id] :as ctx} (setup (if (= :branch side) 1 2))] + (both-edit-component ctx) + + (let [out (update-branch ctx)] + (t/is (= :conflicts (-> out :result :status))) + (t/is (= #{mi-id} (conflict-ids out)))) + + (let [out (update-branch ctx :resolutions {mi-id side})] + (t/is (nil? (:error out))) + (t/is (= :updated (-> out :result :status)))) + + (let [branch (get-file profile branch-file-id)] + (t/is (= color (fill branch page-id mi-id))) + (t/is (= color (fill branch page-id copy-id))) + (t/is (empty? (:touched (shape branch page-id copy-id)))) + (t/is (empty? (validation-codes branch))))))))) + +;; --- c: one side overrides the copy, the other edits the component + +(t/deftest merge-keeps-the-branch-override-of-a-copy + (with-redefs [cf/flags (conj cf/flags :branching)] + (let [{:keys [profile main-id branch-file-id page-id mi-id copy-id] :as ctx} (setup 1)] + (save profile branch-file-id [(set-fill page-id copy-id "#ff0000")]) + (save profile main-id (edit-component page-id mi-id copy-id "#0000ff")) + + (let [out (merge-branch ctx)] + (t/is (nil? (:error out))) + (t/is (= :merged (-> out :result :status)))) + + (let [main (get-file profile main-id)] + (t/is (= "#0000ff" (fill main page-id mi-id))) + (t/is (= "#ff0000" (fill main page-id copy-id))) + (t/is (contains? (:touched (shape main page-id copy-id)) :fill-group)) + (t/is (empty? (validation-codes main))))))) + +(t/deftest update-keeps-main-override-of-a-copy + (with-redefs [cf/flags (conj cf/flags :branching)] + (let [{:keys [profile main-id branch-file-id page-id mi-id copy-id] :as ctx} (setup 1)] + (save profile main-id [(set-fill page-id copy-id "#ff0000")]) + (save profile branch-file-id (edit-component page-id mi-id copy-id "#00ff00")) + + (let [out (update-branch ctx)] + (t/is (nil? (:error out))) + (t/is (= :updated (-> out :result :status)))) + + (let [branch (get-file profile branch-file-id)] + (t/is (= "#00ff00" (fill branch page-id mi-id))) + (t/is (= "#ff0000" (fill branch page-id copy-id))) + (t/is (contains? (:touched (shape branch page-id copy-id)) :fill-group)) + (t/is (empty? (validation-codes branch))))))) + +;; --- d: both sides override the copy differently + +(t/deftest merge-of-a-copy-both-sides-overrode-is-a-conflict + (with-redefs [cf/flags (conj cf/flags :branching)] + (let [{:keys [profile main-id branch-file-id page-id copy-id] :as ctx} (setup 1)] + (save profile branch-file-id [(set-fill page-id copy-id "#ff0000")]) + (save profile main-id [(set-fill page-id copy-id "#ffff00")]) + + (let [out (merge-branch ctx)] + (t/is (= :conflicts (-> out :result :status))) + (t/is (= #{copy-id} (conflict-ids out))))))) + +(t/deftest update-of-a-copy-both-sides-overrode-is-a-conflict + (with-redefs [cf/flags (conj cf/flags :branching)] + (let [{:keys [profile main-id branch-file-id page-id copy-id] :as ctx} (setup 1)] + (save profile branch-file-id [(set-fill page-id copy-id "#ff0000")]) + (save profile main-id [(set-fill page-id copy-id "#ffff00")]) + + (let [out (update-branch ctx)] + (t/is (= :conflicts (-> out :result :status))) + (t/is (= #{copy-id} (conflict-ids out))))))) diff --git a/common/src/app/common/files/branch_merge.cljc b/common/src/app/common/files/branch_merge.cljc index 230f4b2edb..7c149d7374 100644 --- a/common/src/app/common/files/branch_merge.cljc +++ b/common/src/app/common/files/branch_merge.cljc @@ -21,8 +21,12 @@ reported payload names the DOCUMENTS either way (see `compute-merge`)." (:require [app.common.data :as d] + [app.common.files.changes :as cfc] + [app.common.files.changes-builder :as pcb] [app.common.files.helpers :as cfh] + [app.common.logic.libraries :as cll] [app.common.types.component :as ctk] + [app.common.types.components-list :as ctkl] [app.common.types.file :as ctf] [app.common.types.pages-list :as ctpl] [app.common.types.tokens-lib :as ctob] @@ -334,6 +338,138 @@ {:changes (into [] (mapcat :changes) results) :conflicts (into [] (mapcat :conflicts) results)}) +;; --- Component copies --- +;; +;; A copy's attributes follow its component through Penpot's component +;; sync unless the copy overrides them by hand, which the copy records as +;; a sync group in its `:touched` set (`app.common.types.component`). The +;; merge honours that per attribute of a copy both sides changed: +;; - no side overrode it: it follows the merged component, so the +;; target keeps its value and the sync run after the integration +;; (`sync-copies`) sets it; +;; - one side overrode it: that side's value wins; +;; - both sides overrode it: a real conflict. +;; The sides are the comparison roles: `:main` is `theirs` (the target), +;; `:branch` is `ours` (the source). + +(def ^:private text-content-groups + "The groups a text `:content` change marks on top of `:content-group` + (`app.common.types.text/get-diff-type`)." + #{:text-content-text :text-content-attribute :text-content-structure}) + +(defn- overrode? + "True when `touched` holds the sync `group` of `attr` and `shape` moved + the attr off its `base` value: the side overrode it by hand." + [group touched base shape attr] + (and (contains? touched group) + (not= (get base attr) (get shape attr)))) + +(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." + [sb st so t o] + (when (and (some? sb) (some? st) (some? so) + (ctk/in-component-copy? t) + (ctk/in-component-copy? o) + (not= sb st) (not= sb so)) + (let [sb (strip-nil-attrs sb) + st (strip-nil-attrs st) + so (strip-nil-attrs so) + tt (set (:touched t)) + ot (set (:touched o))] + (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 #{}} + (shallow-attr-diff st so)))))) + +(defn- settle-copy-conflicts + "Apply the copy rule to the `:modify-modify` shape conflicts of one + page's comparison `res`. A conflict whose attrs the rule settles + entirely leaves `:conflicts`; it comes back as a `:modified` change + over the attrs the source wins, if any, since the merge applies them. + A conflict with open attrs stays, its `:changed-attrs` narrowed to + them: the merge applies the settled ones whatever the user picks." + [{:keys [conflicts] :as res} sbo sto soo to oo] + (reduce + (fn [acc {:keys [id reason changed-attrs] :as c}] + (if-let [{:keys [auto open]} (when (= :modify-modify reason) + (copy-conflict-sides (get sbo id) (get sto id) (get soo id) + (get to id) (get oo id)))] + (let [won (into [] (keep (fn [[attr side]] (when (= :branch side) attr))) auto)] + (cond + (seq open) + (update acc :conflicts conj (assoc c :changed-attrs (select-keys changed-attrs open))) + + (seq won) + (update acc :changes conj (-> c + (dissoc :reason :base :main :branch) + (assoc :status :modified + :changed-attrs (select-keys changed-attrs won)))) + + :else + acc)) + (update acc :conflicts conj c))) + (assoc res :conflicts []) + conflicts)) + +(defn- copy-resolution + "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)) + +(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?] + (let [ops (into [] + (comp (remove #(= :touched (:attr %))) + (map #(assoc % :ignore-touched true))) + ops) + taken (into #{} + (mapcat (fn [{:keys [attr]}] + (when-let [group (ctk/resolve-sync-group (:type o) attr)] + (if (and (= :content attr) (= :text (:type o))) + (conj text-content-groups group) + [group])))) + ops) + own (set (:touched t)) + touched (if whole? + (set (:touched o)) + (into (into #{} (remove taken) own) + (filter taken) + (:touched o)))] + (cond-> ops + (and (seq ops) (not= touched own)) + (conj {:type :set-touched :touched (not-empty touched)})))) + (defn- presence-only "Keep only the existence-level results of a CONTENT-valued presence diff: added/deleted changes and delete conflicts. Content modifications of @@ -881,7 +1017,11 @@ :deleted (assoc e :shape (or (get bo (:id e)) (get to (:id e)))) e)) - passed (tree-policy-pass res bo to oo sbo sto reorders pid modes)] + passed (-> (tree-policy-pass res bo to oo sbo sto reorders pid modes) + ;; a copy's attrs no more than one + ;; side overrode are not conflicts + ;; (see `copy-conflict-sides`) + (settle-copy-conflicts sbo sto soo to oo))] {:changes (mapv (fn [e] ;; a `:refuse` entry names the ;; container it could not place @@ -1409,7 +1549,14 @@ The `:tokens-lib` is diffed as well (`diff-tokens`), so token changes are reported here with their own kinds. The file's tokens source is - reported under kind `:tokens-source`, entity id `:tokens-source`." + reported under kind `:tokens-source`, entity id `:tokens-source`. + + 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." ([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))) @@ -1771,20 +1918,39 @@ own-drop) keep? (complement drop-ids) + ;; a copy both sides changed takes, per attr, the side the copy + ;; rule picks (`copy-conflict-sides`) and the user's resolution + ;; only over the attrs the rule leaves open. Only the attr pass + ;; reads these; moves and restores keep the user's entity choice + shape-res (reduce (fn [acc id] + (if-let [sides (copy-conflict-sides (get sbo id) (get sto id) (get soo id) + (get to id) (get oo id))] + (assoc acc id (copy-resolution sides (get resolutions id))) + acc)) + resolutions + (filter keep? (keys sbo))) + ;; modifications + deletions (additions handled below, ordered) mod-del (->> (flat-changes (into {} (filter (fn [[id _]] (keep? id))) sbo) (into {} (filter (fn [[id _]] (keep? id))) sto) (into {} (filter (fn [[id _]] (keep? id))) soo) - resolutions + shape-res (fn [_ _] nil) (fn [id _] - (let [res (get resolutions id) - ops (if (and (map? res) - (not= (get sto id) (get sbo id))) ; genuine conflict -> per-attr - (shape-resolved-ops (get sto id) (get soo id) - (get to id) (get oo id) res) - (shape-set-ops (get to id) (get oo id)))] + (let [res (get shape-res id) + t (get to id) + o (get oo id) + whole? (not (and (map? res) + (not= (get sto id) (get sbo id)))) ; genuine conflict -> per-attr + ops (if whole? + (shape-set-ops t o) + (shape-resolved-ops (get sto id) (get soo id) t o res)) + ;; 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?) + ops)] (when (seq ops) {:type :mod-obj :page-id page-id :id id :operations ops}))) (fn [id] @@ -1992,7 +2158,15 @@ shape it reparented there), so no emitted change can reference a parent the merge does not create; the alternative policy, refusing the exclusion and naming those shapes instead, stays documented and is not - the working default." + the working default. + + 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." ([base main branch] (compute-changes base main branch {})) ([base main branch resolutions] @@ -2335,3 +2509,57 @@ set-presence set-rename-changes set-order-changes token-vals themes active-changes))}))) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; COPIES FOLLOW THE INTEGRATED COMPONENTS +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(defn changed-components + "Ids of the local components whose main instance `changes` modify, + read against `data`, the document before they apply: the detection the + workspace runs on every commit to sync copies + (`app.common.files.changes/components-changed`). A deleted component + has no copies left to follow it and is left out." + [data changes] + (into #{} + (comp (mapcat #(cfc/components-changed data %)) + (filter #(some? (ctkl/get-component data %)))) + changes)) + +(defn sync-copies + "Sync the copies of the local components `component-ids` in the + document `data` of the file `file-id`, with Penpot's component sync + (`app.common.logic.libraries/generate-sync-file-changes`): every attr a + copy does not override takes its component's value. A sync that writes + inside the main of another local component (a nested copy) makes that + component sync in turn, as the workspace does, until no sync changes + anything. + + Returns `{:data :changes []}`. The + merge and the update run it after integrating, so the copies the + comparison left with the target's value, or never saw (one added on + the other side), match the integrated components." + [file-id data component-ids] + (let [limit (inc (count (:components data)))] + (loop [pending (set component-ids) + data data + out [] + round 0] + (if (or (empty? pending) (> round limit)) + {:data data :changes out} + (let [[data out queued] + (reduce (fn [[data out queued] component-id] + (let [libraries {file-id {:id file-id :data data}} + redo (-> (pcb/empty-changes) + (cll/generate-sync-file-changes nil :components file-id + component-id file-id + libraries file-id) + :redo-changes)] + (if (empty? redo) + [data out queued] + [(cfc/process-changes data redo) + (into out redo) + (into queued (changed-components data redo))]))) + [data out #{}] + pending)] + (recur queued data out (inc round))))))) diff --git a/common/test/common_tests/files_branch_merge_test.cljc b/common/test/common_tests/files_branch_merge_test.cljc index a59d05defd..f66bc5504f 100644 --- a/common/test/common_tests/files_branch_merge_test.cljc +++ b/common/test/common_tests/files_branch_merge_test.cljc @@ -8,7 +8,13 @@ (:require [app.common.files.branch-merge :as bm] [app.common.files.changes :as cfc] + [app.common.files.changes-builder :as pcb] [app.common.files.tokens :as cfo] + [app.common.logic.shapes :as cls] + [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.page :as ctp] [app.common.types.shape :as cts] [app.common.types.shape-tree :as ctst] @@ -1897,3 +1903,125 @@ (t/testing "replaying the squashed net over the new base reproduces the branch" (t/is (= (tree-objects updated) (tree-objects (cfc/process-changes new-base net))))))) + +;; --- copies follow their component (the copy rule) --- + +(defn- copy-shape + "A copy rect of the main `:m`: `fill` and an optional `:touched` set." + [fill & {:keys [touched opacity]}] + (cond-> {:id :k :name "K" :type :rect :shape-ref :m :fills [{:fill-color fill}]} + (some? touched) (assoc :touched touched) + (some? opacity) (assoc :opacity opacity))) + +(defn- copy-ops + "The operations `compute-changes` writes on the copy `:k`." + [changes] + (->> changes + (filter #(and (= :mod-obj (:type %)) (= :k (:id %)))) + (mapcat :operations) + (vec))) + +(t/deftest copy-attr-no-side-overrode-follows-the-component + ;; both sides synced K from their own edit of the component: the attr + ;; follows the merged component, so the target keeps its value until + ;; the sync after the merge sets it + (let [base (mkdata {:k (copy-shape "red")}) + main (mkdata {:k (copy-shape "blue")}) + branch (mkdata {:k (copy-shape "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-attr-the-source-overrode-wins + (let [base (mkdata {:k (copy-shape "red")}) + main (mkdata {:k (copy-shape "blue")}) + branch (mkdata {:k (copy-shape "green" :touched #{:fill-group})}) + r (bm/compute-merge base main branch :branch->main) + ops (copy-ops (:changes (bm/compute-changes base main branch {} r)))] + (t/is (empty? (:conflicts r))) + (t/testing "the summary reports the override as a change the merge applies" + (t/is (= [[:k :modified #{:fills}]] + (mapv (juxt :id :status (comp set keys :changed-attrs)) (:changes r))))) + (t/testing "the copy takes the value and the override mark" + (t/is (= [{:type :set :attr :fills :val [{:fill-color "green"}] :ignore-touched true} + {:type :set-touched :touched #{:fill-group}}] + ops))))) + +(t/deftest copy-attr-the-target-overrode-stays + (let [base (mkdata {:k (copy-shape "red")}) + main (mkdata {:k (copy-shape "blue" :touched #{:fill-group})}) + branch (mkdata {:k (copy-shape "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-attr-both-sides-overrode-is-a-conflict + (let [base (mkdata {:k (copy-shape "red")}) + main (mkdata {:k (copy-shape "blue" :touched #{:fill-group})}) + branch (mkdata {:k (copy-shape "green" :touched #{:fill-group})}) + r (bm/compute-merge base main branch :branch->main)] + (t/is (= [:k] (mapv :id (:conflicts r)))) + (t/is (= #{:fills} (set (keys (:changed-attrs (first (:conflicts r))))))) + (t/testing "resolved to the branch, the copy takes its value and override" + (t/is (= [{:type :set :attr :fills :val [{:fill-color "green"}] :ignore-touched true}] + (copy-ops (:changes (bm/compute-changes base main branch {:k :branch} r)))))))) + +(t/deftest copy-conflict-narrows-to-the-attrs-both-sides-overrode + ;; both overrode the fill; only the branch overrode the opacity: the + ;; conflict asks about the fill, and the opacity comes from the branch + ;; whatever the user picks + (let [base (mkdata {:k (copy-shape "red" :opacity 1)}) + main (mkdata {:k (copy-shape "blue" :opacity 1 :touched #{:fill-group})}) + branch (mkdata {:k (copy-shape "green" :opacity 0.5 + :touched #{:fill-group :layer-effects-group})}) + r (bm/compute-merge base main branch :branch->main) + ops (copy-ops (:changes (bm/compute-changes base main branch {:k :main} r)))] + (t/is (= #{:fills} (set (keys (:changed-attrs (first (:conflicts r))))))) + (t/is (= [{:type :set :attr :opacity :val 0.5 :ignore-touched true} + {:type :set-touched :touched #{:fill-group :layer-effects-group}}] + ops)))) + +(t/deftest copy-rule-holds-updating-from-main + ;; the update direction: main is the source, the branch the target + (let [base (mkdata {:k (copy-shape "red")}) + main (mkdata {:k (copy-shape "blue" :touched #{:fill-group})}) + branch (mkdata {:k (copy-shape "green")}) + r (bm/compute-merge base main branch :main->branch) + ops (copy-ops (:changes (bm/compute-changes base branch main {} r)))] + (t/is (empty? (:conflicts r))) + (t/is (= [{:type :set :attr :fills :val [{:fill-color "blue"}] :ignore-touched true} + {:type :set-touched :touched #{:fill-group}}] + ops)))) + +(t/deftest clean-copy-change-marks-no-override + ;; the branch synced K from its component and main left K alone: the + ;; merge writes the value without marking it overridden + (let [base (mkdata {:k (copy-shape "red")}) + branch (mkdata {:k (copy-shape "green")})] + (t/is (= [{:type :set :attr :fills :val [{:fill-color "green"}] :ignore-touched true}] + (copy-ops (:changes (bm/compute-changes base base branch))))))) + +(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 + (thi/test-fixture + (fn [] + (let [file (-> (thf/sample-file :file1) + (tho/add-nested-component-with-copy :component1 :main1-root :main1-child + :component2 :main2-root :nested2-head + :copy2-root)) + page (thf/current-page file) + changes (cls/generate-update-shapes (pcb/empty-changes nil (:id page)) + #{(:id (ths/get-shape file :main1-child))} + #(assoc % :fills (ths/sample-fills-color :fill-color "#fabada")) + (:objects page) + {}) + edited (thf/apply-changes file changes) + scope (bm/changed-components (:data file) (:redo-changes changes)) + {:keys [data]} (bm/sync-copies (:id edited) (:data edited) scope) + synced (assoc edited :data data)] + (t/is (= #{(thi/id :component1)} scope)) + (t/is (= "#fabada" (tho/bottom-fill-color synced :nested2-head))) + (t/is (= "#fabada" (tho/bottom-fill-color synced :copy2-root)))))))