diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index b9554c4aa4..7e2422844f 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -1401,11 +1401,13 @@ (rx/map first)) ;; Barriers open before async inspection and close after detection. + ;; Every local commit opens one, so a wait covers the watcher's + ;; decision for any commit: sync, only touch, or drop. pending-sync-barriers* (atom #{}) start-sync-barrier - (fn [{:keys [file-id save-undo?] :as event}] - (let [task (when (and save-undo? (uuid? file-id)) + (fn [{:keys [file-id] :as event}] + (let [task (when (uuid? file-id) (wrf/start! :sync-file [file-id]))] (when task (swap! pending-sync-barriers* conj task)) @@ -1423,18 +1425,31 @@ (doseq [[[_ task] _] batch] (finish-sync-barrier! task))) - commits-s + local-commits-s (->> stream (rx/filter dch/commit?) (rx/map deref) (rx/filter #(= :local (:source %))) - ;; Translation commits never propagate component changes. - (rx/filter (complement :translation?)) - ;; Derived / corrective commits (font-load selrect fix, - ;; position-data regen) are not user component edits. - (rx/filter (complement :skip-component-sync?)) ;; Keep waits pending while component changes are checked. (rx/map start-sync-barrier) + (rx/share)) + + ;; Translation commits never propagate component changes. Derived / + ;; corrective commits (font-load selrect fix, position-data regen) + ;; are not user component edits. + dropped-commit? + (fn [[{:keys [translation? skip-component-sync?]} _]] + (or translation? skip-component-sync?)) + + dropped-commits-s + (->> local-commits-s + (rx/filter dropped-commit?) + (rx/tap (comp finish-sync-barrier! second)) + (rx/ignore)) + + commits-s + (->> local-commits-s + (rx/filter (complement dropped-commit?)) ;; Paired as it arrives, so later commits in the same batch ;; cannot hide the shapes this one moved or deleted. (rx/with-latest-from workspace-buffer-s) @@ -1502,6 +1517,7 @@ (when (or (contains? cf/flags :component-thumbnails) (features/active-feature? state "render-wasm/v1")) (->> (rx/merge + dropped-commits-s component-events-s ;; WASM only: render the thumbnail on every component diff --git a/frontend/src/app/main/data/workspace/reflow.cljs b/frontend/src/app/main/data/workspace/reflow.cljs index b6326d0e14..18f1deffd2 100644 --- a/frontend/src/app/main/data/workspace/reflow.cljs +++ b/frontend/src/app/main/data/workspace/reflow.cljs @@ -229,6 +229,11 @@ (finish-tasks! (map tasks-by-id @remaining)) (reset! remaining #{}))))))))) +(defn pending + "The work pending right now, as `{id -> {kind -> #{task-id}}}`." + [] + (.getValue ^js pending-shapes)) + (defn settled "Observable that emits once every id in `ids` has drained from the pending map, then completes. A nil `ids` waits for every pending id; an empty one has diff --git a/frontend/test/frontend_tests/composable_tests/interpreter.cljs b/frontend/test/frontend_tests/composable_tests/interpreter.cljs index 4c1ea73af8..92f92f4edd 100644 --- a/frontend/test/frontend_tests/composable_tests/interpreter.cljs +++ b/frontend/test/frontend_tests/composable_tests/interpreter.cljs @@ -245,7 +245,7 @@ (map (fn [[lib-id lib]] [lib-id (assoc lib :library-of (:id file))])) aux)))))) -(defn- current-file +(defn current-file "Read the live workspace file out of the global store." [] (let [st @st/state] @@ -380,23 +380,25 @@ (set! st/state (:state original-store)) (set! st/stream (:stream original-store))) +(defn install! + "Install `situation` in the global store and start the watchers the workspace + starts, replacing the previous variant's. Restores the global store first (a + preceding namespace may have swapped it). Returns the situation." + [situation] + (restore-global-store!) + (st/emit! (install-situation-event situation) + (dw/finalize-edit-watchers) + (dw/initialize-edit-watchers) + (dwl/watch-library-changes)) + situation) + (defn- run-variant "Set up one variant on the global store and run its ops. `setup` returns a - situation (file + roles). Restores the global store (a preceding namespace may - have swapped it), installs the file + starts the watcher, then folds the ops. - Calls `k` with the final situation." + situation (file + roles). Calls `k` with the final situation." [setup ops k] ;; fresh label space per variant (mirrors the pure `tm/run-all`) (cthi/reset-idmap!) - (restore-global-store!) - (let [situation (setup)] - (st/emit! (install-situation-event situation)) - ;; The same watchers the workspace starts; re-emitting them replaces the - ;; previous variant's. - (st/emit! (dw/finalize-edit-watchers) - (dw/initialize-edit-watchers) - (dwl/watch-library-changes)) - (run-ops situation ops k))) + (run-ops (install! (setup)) ops k)) ;; -------------------------------------------------------------------------- ;; (3) Test-facing check diff --git a/frontend/test/frontend_tests/data/workspace_sync_barrier_test.cljs b/frontend/test/frontend_tests/data/workspace_sync_barrier_test.cljs new file mode 100644 index 0000000000..a610a5b32d --- /dev/null +++ b/frontend/test/frontend_tests/data/workspace_sync_barrier_test.cljs @@ -0,0 +1,101 @@ +;; 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 frontend-tests.data.workspace-sync-barrier-test + "Tests the `:sync-file` wait the component watcher holds for each local + commit until it has decided what to do with it: sync, only touch the + component, or drop the commit. Plugin `waitForLayoutUpdate` and the + composable interpreter rely on it." + (:require + [app.common.files.changes-builder :as pcb] + [app.common.logic.shapes :as cls] + [app.common.test-helpers.composable.comp.setups :as setup] + [app.common.test-helpers.files :as thf] + [app.common.test-helpers.ids-map :as thi] + [app.common.test-helpers.shapes :as ths] + [app.main.data.changes :as dch] + [app.main.data.workspace.reflow :as wrf] + [app.main.store :as st] + [app.plugins.reflow :as pwrf] + [cljs.test :as t :include-macros true] + [frontend-tests.composable-tests.interpreter :as ftm])) + +;; The watcher schedules thumbnail renders that reach `window`, absent +;; headless (see `ftm/install-thumbnail-noop!`). +(t/use-fixtures :each + {:before (fn [] + (thi/reset-idmap!) + (ftm/install-thumbnail-noop!)) + :after ftm/restore-thumbnail!}) + +(def ^:private red "#ff0000") + +(defn- main-child-fill-commit + "A commit that sets the main child's fill to red, with `opts` merged into the + commit params." + [opts] + (let [file (ftm/current-file) + page (thf/current-page file) + changes (cls/generate-update-shapes + (pcb/empty-changes nil (:id page)) + #{(thi/id :main-child)} + #(assoc % :fills (ths/sample-fills-color :fill-color red)) + (:objects page) + {})] + (dch/commit-changes (merge {:redo-changes (:redo-changes changes) + :undo-changes (:undo-changes changes)} + opts)))) + +(defn- sync-pending? + [file-id] + (contains? (get (wrf/pending) file-id) :sync-file)) + +(defn- fill-of + [label] + (-> (ths/get-shape (ftm/current-file) label) :fills first :fill-color)) + +(defn- run-commit + "Install a component with a copy and emit the main child fill commit with + `commit-opts`. Once the wait settles, call `check` with whether the sync + wait was open right after the commit. Fails the test when the wait does not + settle." + [done commit-opts check] + (ftm/install! (setup/simple-component-with-labeled-copy)) + (let [file-id (:id (ftm/current-file))] + (st/emit! (main-child-fill-commit commit-opts)) + (let [open? (sync-pending? file-id)] + (-> (pwrf/wait-for-layout-update nil 2000) + (.then #(check open?) + #(t/is false "the sync wait did not settle")) + (.catch #(t/is false (str "the check threw: " %))) + (.finally done))))) + +(t/deftest user-edit-holds-the-wait-until-copies-sync + (t/async done + (run-commit done {} + (fn [open?] + (t/is open?) + (t/is (= red (fill-of :copy-child))))))) + +(t/deftest undo-commit-holds-the-wait-until-the-watcher-handles-it + ;; Undo and redo commit with `save-undo? false`. The watcher only touches + ;; the component for them, after a timer turn; the wait must cover that. + (t/async done + (run-commit done {:save-undo? false} + (fn [open?] + (t/is open?))))) + +(t/deftest translation-commit-leaves-nothing-pending + (t/async done + (run-commit done {:translation? true} + (fn [open?] + (t/is (not open?)))))) + +(t/deftest derived-commit-leaves-nothing-pending + (t/async done + (run-commit done {:skip-component-sync? true} + (fn [open?] + (t/is (not open?)))))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index e23f915520..80b98597a3 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -37,6 +37,7 @@ [frontend-tests.data.workspace-path-edition-test] [frontend-tests.data.workspace-reflow-test] [frontend-tests.data.workspace-shortcuts-test] + [frontend-tests.data.workspace-sync-barrier-test] [frontend-tests.data.workspace-texts-test] [frontend-tests.data.workspace-thumbnails-test] [frontend-tests.data.workspace-variant-rename-test] @@ -204,6 +205,7 @@ 'frontend-tests.data.workspace-pages-test 'frontend-tests.data.workspace-path-edition-test 'frontend-tests.data.workspace-reflow-test + 'frontend-tests.data.workspace-sync-barrier-test 'frontend-tests.data.workspace-shortcuts-test 'frontend-tests.data.workspace-texts-test 'frontend-tests.data.workspace-thumbnails-test