From 7b746adbcbdc644cd231cc77fc9712fb1edd36d2 Mon Sep 17 00:00:00 2001 From: Tienson Qin Date: Tue, 24 Mar 2026 13:10:35 +0800 Subject: [PATCH] fix(db-sync): rebind redo history tx-id for undo replay --- .../src/logseq/outliner/op/construct.cljc | 111 +++++--------- .../frontend/modules/outliner/pipeline.cljs | 3 +- src/main/frontend/worker/db_listener.cljs | 5 - src/main/frontend/worker/db_worker.cljs | 7 - src/main/frontend/worker/sync/apply_txs.cljs | 89 +++++------ src/main/frontend/worker/undo_redo.cljs | 105 ++++++------- .../frontend/worker/db_sync_sim_test.cljs | 18 +-- src/test/frontend/worker/db_sync_test.cljs | 139 ++++++++---------- src/test/frontend/worker/undo_redo_test.cljs | 131 ++++++++++++++--- 9 files changed, 314 insertions(+), 294 deletions(-) diff --git a/deps/outliner/src/logseq/outliner/op/construct.cljc b/deps/outliner/src/logseq/outliner/op/construct.cljc index 62b517df52..860ae1f63a 100644 --- a/deps/outliner/src/logseq/outliner/op/construct.cljc +++ b/deps/outliner/src/logseq/outliner/op/construct.cljc @@ -345,11 +345,10 @@ (let [[template-id target-id opts] args template-ref (stable-entity-ref db template-id) target-ref (stable-entity-ref db target-id) - opts' (assoc (dissoc opts - :template-blocks - :template-id - :outliner-op) - :keep-uuid? true)] + opts' (dissoc opts + :template-blocks + :template-id + :outliner-op)] (when-not (and template-ref target-ref) (throw (ex-info "Invalid apply-template args" {:args args}))) @@ -702,45 +701,38 @@ ;; Use restore semantics instead of save-block to retract recycle markers. [:restore-recycled [page-uuid]])))) -(defn- insert-like-delete-ids - [db-before db-after blocks] - (->> blocks - (keep (fn [block] - (when-let [u (:block/uuid block)] - [:block/uuid u]))) - (filter (fn [eid] - (and - (nil? (d/entity db-before eid)) - (d/entity db-after eid)))) - vec)) - (defn- restore-target-insert-op [db-before db-after target-id opts] (when (:replace-empty-target? opts) (when-let [target-ref (stable-entity-ref db-before target-id)] (when (d/entity db-after target-ref) (when-let [target (d/entity db-before target-ref)] - (build-inverse-save-block db-before target opts)))))) + [[:delete-blocks [[target-ref] {}]] + (build-inverse-save-block db-before target opts)]))))) (defn- build-inverse-insert-like - [db-before db-after args] - (let [[blocks target-id opts] args - delete-ids (insert-like-delete-ids db-before db-after blocks) + [db-before db-after tx-data args] + (let [[_blocks target-id opts] args + new-block-eids (keep + (fn [d] + (when (and (= :block/uuid (:a d)) + (:added d) + (nil? (d/entity db-before (:e d)))) + [:block/uuid (:v d)])) + tx-data) restore-op (restore-target-insert-op db-before db-after target-id opts)] - (prn :debug :delete-ids delete-ids - :restore-op restore-op) (cond-> [] - (seq delete-ids) - (conj [:delete-blocks [delete-ids {}]]) + (seq new-block-eids) + (conj [:delete-blocks [new-block-eids {}]]) restore-op - (conj restore-op) + (into restore-op) :always seq))) (defn- build-strict-inverse-outliner-ops - [db-before db-after forward-ops] + [db-before db-after tx-data forward-ops] (when (seq forward-ops) (let [inverse-entries (mapv (fn [[op args]] @@ -751,10 +743,10 @@ (build-inverse-save-block db-before block opts)) :insert-blocks - (build-inverse-insert-like db-before db-after args) + (build-inverse-insert-like db-before db-after tx-data args) :apply-template - (build-inverse-insert-like db-before db-after args) + (build-inverse-insert-like db-before db-after tx-data args) :move-blocks (let [[ids _target-id _opts] args] @@ -944,60 +936,35 @@ (some (fn [[op]] (= :transact op)) forward-outliner-ops)) canonical-transact-op forward-outliner-ops)) - built-inverse-outliner-ops (some-> (build-strict-inverse-outliner-ops db-before db-after forward-outliner-ops) + built-inverse-outliner-ops (some-> (build-strict-inverse-outliner-ops db-before db-after tx-data forward-outliner-ops) seq vec) explicit-inverse-outliner-ops (some-> (canonicalize-explicit-outliner-ops db-after tx-data (:db-sync/inverse-outliner-ops tx-meta)) (patch-inverse-delete-block-ops forward-outliner-ops) seq vec) - inverse-outliner-ops (if (has-replace-empty-target-insert-op? forward-outliner-ops) - built-inverse-outliner-ops - (cond - (seq built-inverse-outliner-ops) - built-inverse-outliner-ops + inverse-outliner-ops (cond + (and (= :apply-template (:outliner-op tx-meta)) + (:undo? tx-meta) + (seq (:db-sync/inverse-outliner-ops tx-meta))) + (:db-sync/inverse-outliner-ops tx-meta) - (nil? explicit-inverse-outliner-ops) - nil + (has-replace-empty-target-insert-op? forward-outliner-ops) + built-inverse-outliner-ops + + (seq built-inverse-outliner-ops) + built-inverse-outliner-ops + + (nil? explicit-inverse-outliner-ops) + nil ;; Treat explicit transact placeholder as "no semantic inverse". ;; Keep nil so semantic replay must fail-fast when required. - (= canonical-transact-op explicit-inverse-outliner-ops) - nil + (= canonical-transact-op explicit-inverse-outliner-ops) + nil - :else - explicit-inverse-outliner-ops)) + :else + explicit-inverse-outliner-ops) inverse-outliner-ops (some-> inverse-outliner-ops seq vec)] {:forward-outliner-ops forward-outliner-ops :inverse-outliner-ops inverse-outliner-ops})) - -(defn build-history-action-metadata - [{:keys [db-before db-after tx-data tx-meta] :as data}] - (let [{:keys [forward-outliner-ops inverse-outliner-ops]} - (derive-history-outliner-ops db-before db-after tx-data tx-meta) - semantic-ops-explicit? - (or (seq (:outliner-ops tx-meta)) - (seq (:db-sync/forward-outliner-ops tx-meta)) - (seq (:db-sync/inverse-outliner-ops tx-meta)))] - (when (and semantic-ops-explicit? - (contains? semantic-outliner-ops (:outliner-op tx-meta)) - (not= :restore-recycled (:outliner-op tx-meta)) - (or - (empty? forward-outliner-ops) - (empty? inverse-outliner-ops))) - (log/error ::invalid-outliner-ops {:tx-meta tx-meta - :forward-outliner-ops forward-outliner-ops - :inverse-outliner-ops inverse-outliner-ops}) - (throw (ex-info "Invalid outliner-ops" {:tx-meta tx-meta}))) - ;; (pprint/pprint - ;; {:forward-outliner-ops forward-outliner-ops - ;; :inverse-outliner-ops inverse-outliner-ops}) - - (cond-> (-> data - (dissoc :db-before :db-after) - (assoc :db-sync/tx-id (or (:db-sync/tx-id tx-meta) (random-uuid)))) - (seq forward-outliner-ops) - (assoc :db-sync/forward-outliner-ops forward-outliner-ops) - - (seq inverse-outliner-ops) - (assoc :db-sync/inverse-outliner-ops inverse-outliner-ops)))) diff --git a/src/main/frontend/modules/outliner/pipeline.cljs b/src/main/frontend/modules/outliner/pipeline.cljs index ca34727bbb..115398dc31 100644 --- a/src/main/frontend/modules/outliner/pipeline.cljs +++ b/src/main/frontend/modules/outliner/pipeline.cljs @@ -65,7 +65,8 @@ tx-data))] (d/transact! conn tx-data' tx-meta)) - (when-not (= (:client-id tx-meta) (:client-id @state/state)) + (when (or (not= (:client-id tx-meta) (:client-id @state/state)) + (= :apply-template (:outliner-op tx-meta))) (update-editing-block-title-if-changed! tx-data)) ;; (when (seq deleted-assets) diff --git a/src/main/frontend/worker/db_listener.cljs b/src/main/frontend/worker/db_listener.cljs index 45c57252c5..3ba9c319fc 100644 --- a/src/main/frontend/worker/db_listener.cljs +++ b/src/main/frontend/worker/db_listener.cljs @@ -8,7 +8,6 @@ [frontend.worker.shared-service :as shared-service] [frontend.worker.state :as worker-state] [frontend.worker.sync :as db-sync] - [frontend.worker.undo-redo :as worker-undo-redo] [logseq.db :as ldb] [promesa.core :as p])) @@ -63,10 +62,6 @@ [_ {:keys [repo]} tx-report] (db-sync/handle-local-tx! repo tx-report)) -(defmethod listen-db-changes :undo-redo - [_ {:keys [repo]} tx-report] - (worker-undo-redo/gen-undo-ops! repo tx-report)) - (defn- remove-old-embeddings-and-reset-new-updates! [conn tx-data tx-meta] (let [;; Remove old :logseq.property.embedding/hnsw-label-updated-at when importing a graph diff --git a/src/main/frontend/worker/db_worker.cljs b/src/main/frontend/worker/db_worker.cljs index e67e83c253..64d1cd6606 100644 --- a/src/main/frontend/worker/db_worker.cljs +++ b/src/main/frontend/worker/db_worker.cljs @@ -25,7 +25,6 @@ [frontend.worker.shared-service :as shared-service] [frontend.worker.state :as worker-state] [frontend.worker.sync :as db-sync] - [frontend.worker.sync.apply-txs :as sync-apply] [frontend.worker.sync.asset-db-listener] [frontend.worker.sync.client-op :as client-op] [frontend.worker.sync.crypt :as sync-crypt] @@ -636,12 +635,6 @@ (log/error ::worker-transact-failed e) (throw e))))) -(def-thread-api :thread-api/apply-history-action - [repo tx-id undo? tx-meta] - (assert (some? repo)) - (worker-state/set-db-latest-tx-time! repo) - (sync-apply/apply-history-action! repo tx-id undo? tx-meta)) - (def-thread-api :thread-api/undo-redo-set-pending-editor-info [repo editor-info] (worker-undo-redo/set-pending-editor-info! repo editor-info) diff --git a/src/main/frontend/worker/sync/apply_txs.cljs b/src/main/frontend/worker/sync/apply_txs.cljs index aab40b3b2a..09bcdeecb3 100644 --- a/src/main/frontend/worker/sync/apply_txs.cljs +++ b/src/main/frontend/worker/sync/apply_txs.cljs @@ -1,6 +1,7 @@ (ns frontend.worker.sync.apply-txs "Pending tx and remote tx application helpers for db sync." - (:require [clojure.set :as set] + (:require [cljs.pprint :as pprint] + [clojure.set :as set] [clojure.string :as string] [datascript.core :as d] [frontend.worker-common.util :as worker-util] @@ -15,6 +16,7 @@ [frontend.worker.sync.large-title :as sync-large-title] [frontend.worker.sync.presence :as sync-presence] [frontend.worker.sync.transport :as sync-transport] + [frontend.worker.undo-redo :as worker-undo-redo] [lambdaisland.glogi :as log] [logseq.db :as ldb] [logseq.db-sync.order :as sync-order] @@ -170,14 +172,6 @@ [db block] (op-construct/rewrite-block-title-with-retracted-refs db block)) -(defn- derive-history-outliner-ops - [db-before db-after tx-data tx-meta] - (op-construct/derive-history-outliner-ops db-before db-after tx-data tx-meta)) - -(defn build-history-action-metadata - [data] - (op-construct/build-history-action-metadata data)) - (defn- inferred-outliner-ops? [tx-meta] (and (nil? (:outliner-ops tx-meta)) @@ -185,7 +179,8 @@ (not (:redo? tx-meta)) (not= :batch-import-edn (:outliner-op tx-meta)))) -(defn- persist-local-tx! [repo db-before db-after tx-data normalized-tx-data reversed-datoms tx-meta] +(declare apply-history-action!) +(defn- persist-local-tx! [repo {:keys [db-before db-after tx-data tx-meta] :as tx-report} normalized-tx-data reversed-datoms] (worker-util/profile "persist-local-tx!" (when-let [conn (client-ops-conn repo)] @@ -194,23 +189,29 @@ should-inc-pending? (not= true (:db-sync/pending? existing-ent)) now (.now js/Date) {:keys [forward-outliner-ops inverse-outliner-ops]} - (worker-util/profile "derive-history-outliner-ops" (derive-history-outliner-ops db-before db-after tx-data tx-meta)) - outliner-ops forward-outliner-ops + (op-construct/derive-history-outliner-ops db-before db-after tx-data tx-meta) inferred-outliner-ops?' (inferred-outliner-ops? tx-meta)] + ;; (pprint/pprint + ;; {:undo? (:undo? tx-meta) + ;; :forward-outliner-ops forward-outliner-ops + ;; :inverse-outliner-ops inverse-outliner-ops + ;; :tx-id tx-id + ;; :existing-action? (some? existing-ent)}) (ldb/transact! conn [{:db-sync/tx-id tx-id :db-sync/normalized-tx-data normalized-tx-data :db-sync/reversed-tx-data reversed-datoms :db-sync/pending? true :db-sync/outliner-op (:outliner-op tx-meta) - :db-sync/outliner-ops outliner-ops - :db-sync/forward-outliner-ops outliner-ops + :db-sync/forward-outliner-ops forward-outliner-ops :db-sync/inverse-outliner-ops inverse-outliner-ops :db-sync/inferred-outliner-ops? inferred-outliner-ops?' :db-sync/created-at now}]) + (worker-undo-redo/gen-undo-ops! repo tx-report tx-id + {:apply-history-action! apply-history-action!}) (when should-inc-pending? - (client-op/adjust-pending-local-tx-count! repo 1)) - (when-let [client (current-client repo)] - (broadcast-rtc-state! client)) + (client-op/adjust-pending-local-tx-count! repo 1) + (when-let [client (current-client repo)] + (broadcast-rtc-state! client))) tx-id)))) (defn pending-txs @@ -231,7 +232,6 @@ reversed-tx' (:db-sync/reversed-tx-data ent)] {:tx-id tx-id :outliner-op (:db-sync/outliner-op ent) - :outliner-ops (:db-sync/outliner-ops ent) :forward-outliner-ops (:db-sync/forward-outliner-ops ent) :inverse-outliner-ops (:db-sync/inverse-outliner-ops ent) :inferred-outliner-ops? (:db-sync/inferred-outliner-ops? ent) @@ -245,7 +245,6 @@ (when-let [ent (d/entity @conn [:db-sync/tx-id tx-id])] {:tx-id (:db-sync/tx-id ent) :outliner-op (:db-sync/outliner-op ent) - :outliner-ops (:db-sync/outliner-ops ent) :forward-outliner-ops (:db-sync/forward-outliner-ops ent) :inverse-outliner-ops (:db-sync/inverse-outliner-ops ent) :tx (:db-sync/normalized-tx-data ent) @@ -297,18 +296,19 @@ (declare precreate-missing-save-blocks! replay-canonical-outliner-op!) (defn- apply-history-action-tx! - [conn tx-data tx-meta] + [conn tx-data tx-meta history-tx-id] (try (let [tx-meta' (-> tx-meta (assoc :outliner-op :transact) - (dissoc :outliner-ops - :real-outliner-op + (dissoc :real-outliner-op :db-sync/forward-outliner-ops :db-sync/inverse-outliner-ops))] (d/with @conn tx-data {:outliner-op :transact :persist-op? false}) (ldb/transact! conn tx-data tx-meta') - {:applied? true :source :raw-tx}) + {:applied? true + :source :raw-tx + :history-tx-id history-tx-id}) (catch :default error (log/debug :db-sync/drop-history-action-raw-tx {:reason :invalid-history-action-tx @@ -328,12 +328,18 @@ (let [semantic-forward? (semantic-op-stream? (:forward-outliner-ops action)) ops (history-action-ops action undo?) tx-data (history-action-tx-data action undo?) - tx-meta' (cond-> (merge {:local-tx? true - :gen-undo-ops? false - :persist-op? true} - (dissoc tx-meta :db-sync/tx-id)) - (seq ops) - (assoc :outliner-ops (vec ops)) + history-tx-id (let [provided-history-tx-id (:db-sync/tx-id tx-meta)] + (if (and (uuid? provided-history-tx-id) + (not= provided-history-tx-id tx-id)) + provided-history-tx-id + (random-uuid))) + tx-meta' (cond-> {:local-tx? true + :gen-undo-ops? false + :persist-op? true + :undo? undo? + :db-sync/tx-id history-tx-id + :db-sync/source-tx-id (or (:db-sync/source-tx-id tx-meta) + tx-id)} (:outliner-op action) (assoc :outliner-op (:outliner-op action)) @@ -349,10 +355,10 @@ (assoc :db-sync/inverse-outliner-ops (vec (if undo? (:forward-outliner-ops action) (:inverse-outliner-ops action)))))] - ;; (prn :debug :outliner-ops) - ;; (pprint/pprint (select-keys action [:tx-id :outliner-op :forward-outliner-ops :inverse-outliner-ops])) - ;; (prn :debug :tx-meta) - ;; (pprint/pprint tx-meta) + ;; (prn :debug :outliner-ops) + ;; (pprint/pprint (select-keys action [:tx-id :outliner-op :forward-outliner-ops :inverse-outliner-ops])) + ;; (prn :debug :tx-meta) + ;; (pprint/pprint tx-meta) (cond (and semantic-forward? (not (seq ops))) @@ -383,7 +389,9 @@ (precreate-missing-save-blocks! row-conn ops) (doseq [op ops] (replay-canonical-outliner-op! row-conn op)))) - {:applied? true :source :semantic-ops} + {:applied? true + :source :semantic-ops + :history-tx-id history-tx-id} (catch :default error (log/error ::db-transact-failed error) (if semantic-forward? @@ -415,7 +423,7 @@ :tx-data tx-data}) (seq tx-data) - (apply-history-action-tx! conn tx-data tx-meta') + (apply-history-action-tx! conn tx-data tx-meta' history-tx-id) :else {:applied? false :reason :unsupported-history-action @@ -870,12 +878,11 @@ (defn- rebase-op-driven-local-tx! [conn local-txs index local-tx tx-meta] - (let [outliner-ops (:outliner-ops local-tx) - replay-meta (assoc (local-tx-debug-meta tx-meta local-txs index local-tx :rebase) + (let [replay-meta (assoc (local-tx-debug-meta tx-meta local-txs index local-tx :rebase) :db-sync/tx-id (:tx-id local-tx) :db-sync/forward-outliner-ops (:forward-outliner-ops local-tx) - :db-sync/inverse-outliner-ops (:inverse-outliner-ops local-tx) - :outliner-ops outliner-ops)] + :db-sync/inverse-outliner-ops (:inverse-outliner-ops local-tx)) + outliner-ops (:forward-outliner-ops local-tx)] (try (ldb/batch-transact! conn @@ -1013,7 +1020,7 @@ (apply-remote-txs! repo client [{:tx-data tx-data}])) (defn enqueue-local-tx! - [repo {:keys [tx-meta tx-data db-after db-before]}] + [repo {:keys [tx-meta tx-data db-after db-before] :as tx-report}] (worker-util/profile "enqueue-local-tx!" (when-let [conn (worker-state/get-datascript-conn repo)] @@ -1024,7 +1031,7 @@ (let [normalized (normalize-tx-data db-after db-before tx-data) reversed-datoms (reverse-tx-data db-before db-after tx-data)] (when (seq normalized) - (persist-local-tx! repo db-before db-after tx-data normalized reversed-datoms tx-meta) + (persist-local-tx! repo tx-report normalized reversed-datoms) (worker-util/profile "flush pending" (when-let [client @worker-state/*db-sync-client] diff --git a/src/main/frontend/worker/undo_redo.cljs b/src/main/frontend/worker/undo_redo.cljs index cbd3003578..6fccd8e394 100644 --- a/src/main/frontend/worker/undo_redo.cljs +++ b/src/main/frontend/worker/undo_redo.cljs @@ -2,7 +2,6 @@ "Undo redo new implementation" (:require [datascript.core :as d] [frontend.worker.state :as worker-state] - [frontend.worker.sync.apply-txs :as sync-apply] [lambdaisland.glogi :as log] [logseq.common.defkeywords :refer [defkeywords]] [logseq.db :as ldb] @@ -16,6 +15,8 @@ ::db-transact {:doc "db tx"} ::ui-state {:doc "ui state such as route && sidebar blocks"}) +(defonce *apply-history-action! (atom nil)) + ;; TODO: add other UI states such as `::ui-updates`. (comment ;; TODO: convert it to a qualified-keyword @@ -35,9 +36,7 @@ [:outliner-op :keyword]]] [:added-ids [:set :int]] [:retracted-ids [:set :int]] - [:db-sync/tx-id {:optional true} :uuid] - [:db-sync/forward-outliner-ops {:optional true} [:sequential :any]] - [:db-sync/inverse-outliner-ops {:optional true} [:sequential :any]]]]] + [:db-sync/tx-id {:optional true} :uuid]]]] [::record-editor-info [:cat :keyword @@ -167,37 +166,17 @@ [repo] (empty? (get @*redo-ops repo))) -(defn- ensure-history-action-metadata - [{:keys [tx-meta] :as data}] - (cond-> (sync-apply/build-history-action-metadata data) - (nil? (:db-sync/tx-id tx-meta)) - (dissoc :db-sync/tx-id))) - (defn- undo-redo-action-meta [{:keys [tx-meta] - source-tx-id :db-sync/tx-id - forward-outliner-ops :db-sync/forward-outliner-ops - inverse-outliner-ops :db-sync/inverse-outliner-ops} + source-tx-id :db-sync/tx-id} undo?] - (let [forward-outliner-ops' (if undo? inverse-outliner-ops forward-outliner-ops) - inverse-outliner-ops' (if undo? forward-outliner-ops inverse-outliner-ops)] - (cond-> (-> tx-meta - (dissoc :db-sync/tx-id) - (assoc - :gen-undo-ops? false - :undo? undo? - :redo? (not undo?) - :db-sync/source-tx-id source-tx-id)) - (seq forward-outliner-ops') - (assoc :db-sync/forward-outliner-ops (vec forward-outliner-ops')) - - (seq inverse-outliner-ops') - (assoc :db-sync/inverse-outliner-ops (vec inverse-outliner-ops'))))) - -(defn- apply-history-action! - [repo data undo? tx-meta] - (when-let [tx-id (:db-sync/tx-id data)] - (sync-apply/apply-history-action! repo tx-id undo? tx-meta))) + (-> tx-meta + (dissoc :db-sync/tx-id) + (assoc + :gen-undo-ops? false + :undo? undo? + :redo? (not undo?) + :db-sync/source-tx-id source-tx-id))) (defn- reverse-datoms [conn datoms schema added-ids retracted-ids undo? redo?] @@ -296,6 +275,16 @@ (remove nil?))))] reversed-tx-data)) +(defn- rebind-op-db-sync-tx-id + [op history-tx-id] + (if (uuid? history-tx-id) + (mapv (fn [item] + (if (= ::db-transact (first item)) + [::db-transact (assoc (second item) :db-sync/tx-id history-tx-id)] + item)) + op) + op)) + (defn- undo-redo-aux [repo undo?] (if-let [op (not-empty ((if undo? pop-undo-op pop-redo-op) repo))] @@ -314,8 +303,8 @@ (when (seq tx-data) (let [tx-meta' (undo-redo-action-meta data undo?) tx-id (:db-sync/tx-id data) - handler (fn handler [] - ((if undo? push-redo-op push-undo-op) repo op) + handler (fn handler [op'] + ((if undo? push-redo-op push-undo-op) repo op') (let [editor-cursors (->> (filter #(= ::record-editor-info (first %)) op) (map second)) block-content (:block/title (d/entity @conn [:block/uuid (:block-uuid @@ -333,7 +322,7 @@ (if (undo-validate/valid-undo-redo-tx? conn reversed-tx-data) (try (ldb/transact! conn reversed-tx-data tx-meta') - (handler) + (handler op) (catch :default e (log/error ::undo-redo-failed e) (clear-history! repo) @@ -350,17 +339,20 @@ (undo-redo-aux repo undo?)))))] (if tx-id (try - (let [worker-result (apply-history-action! repo data undo? tx-meta')] - (if (:applied? worker-result) - (handler) - (do - (log/error ::undo-redo-worker-action-unavailable - {:undo? undo? - :repo repo - :tx-id tx-id - :result worker-result}) - (clear-history! repo) - (if undo? ::empty-undo-stack ::empty-redo-stack)))) + (when-let [apply-action @*apply-history-action!] + (let [worker-result (apply-action repo tx-id undo? tx-meta')] + (if (:applied? worker-result) + (handler (if undo? + op + (rebind-op-db-sync-tx-id op (:history-tx-id worker-result)))) + (do + (log/error ::undo-redo-worker-action-unavailable + {:undo? undo? + :repo repo + :tx-id tx-id + :result worker-result}) + (clear-history! repo) + (if undo? ::empty-undo-stack ::empty-redo-stack))))) (catch :default e (log/error ::undo-redo-worker-failed e) (throw e) @@ -397,7 +389,10 @@ (push-undo-op repo [[::ui-state ui-state-str]]))) (defn gen-undo-ops! - [repo {:keys [tx-data tx-meta db-after db-before]}] + [repo {:keys [tx-data tx-meta db-after db-before]} tx-id + {:keys [apply-history-action!]}] + (when (nil? @*apply-history-action!) + (reset! *apply-history-action! apply-history-action!)) (let [{:keys [outliner-op local-tx?]} tx-meta] (when (and (true? local-tx?) @@ -416,16 +411,14 @@ tx-data' (vec tx-data) editor-info (or (:undo-redo/editor-info tx-meta) (take-pending-editor-info! repo)) - history-data (ensure-history-action-metadata - {:tx-data tx-data' - :tx-meta tx-meta - :added-ids added-ids - :retracted-ids retracted-ids - :db-after db-after - :db-before db-before}) + data (cond-> + {:db-sync/tx-id tx-id + :tx-meta (dissoc tx-meta :outliner-ops) + :added-ids added-ids + :retracted-ids retracted-ids + :tx-data tx-data'}) op (->> [(when editor-info [::record-editor-info editor-info]) - [::db-transact - history-data]] + [::db-transact data]] (remove nil?) vec)] (push-undo-op repo op))))) diff --git a/src/test/frontend/worker/db_sync_sim_test.cljs b/src/test/frontend/worker/db_sync_sim_test.cljs index cb37552489..3796c5adfd 100644 --- a/src/test/frontend/worker/db_sync_sim_test.cljs +++ b/src/test/frontend/worker/db_sync_sim_test.cljs @@ -143,16 +143,14 @@ (let [key (keyword "db-sync-sim" repo)] (d/listen! conn key (fn [tx-report] - (db-sync/enqueue-local-tx! repo tx-report) - (undo-redo/gen-undo-ops! - repo - (-> tx-report - (assoc-in [:tx-meta :client-id] (:client-id @state/state)) - (update-in [:tx-meta :local-tx?] - (fn [local-tx?] - (if (nil? local-tx?) - true - local-tx?))))))) + (let [tx-report' (-> tx-report + (assoc-in [:tx-meta :client-id] (:client-id @state/state)) + (update-in [:tx-meta :local-tx?] + (fn [local-tx?] + (if (nil? local-tx?) + true + local-tx?))))] + (db-sync/enqueue-local-tx! repo tx-report')))) (swap! listeners conj [conn key])))) (try (f) diff --git a/src/test/frontend/worker/db_sync_test.cljs b/src/test/frontend/worker/db_sync_test.cljs index 4f1a314410..a1099ebc29 100644 --- a/src/test/frontend/worker/db_sync_test.cljs +++ b/src/test/frontend/worker/db_sync_test.cljs @@ -1010,80 +1010,6 @@ (finally (reset! ldb/*transact-invalid-callback prev-invalid-callback)))))))) -(deftest undo-redo-insert-save-insert-save-indent-sequence-keeps-block-valid-test - (testing "insert/save/insert/save/indent then undo-all/redo-all/undo keeps block 2 valid" - (let [conn (db-test/create-conn-with-blocks - {:pages-and-blocks [{:page {:block/title "page 1"} - :blocks []}]}) - client-ops-conn (d/create-conn client-op/schema-in-db) - page-1 (db-test/find-page-by-title @conn "page 1") - page-id (:db/id page-1) - block-1-uuid (random-uuid) - block-2-uuid (random-uuid) - prev-invalid-callback @ldb/*transact-invalid-callback - invalid-payload* (atom nil)] - (with-datascript-conns conn client-ops-conn - (fn [] - (d/listen! conn ::worker-undo-listener - (fn [tx-report] - (worker-undo-redo/gen-undo-ops! test-repo tx-report))) - (reset! ldb/*transact-invalid-callback - (fn [tx-report errors] - (reset! invalid-payload* {:tx-meta (:tx-meta tx-report) - :errors errors}))) - (worker-undo-redo/clear-history! test-repo) - (try - (outliner-op/apply-ops! conn - [[:insert-blocks [[{:block/uuid block-1-uuid - :block/title ""}] - page-id - {:sibling? false - :keep-uuid? true}]]] - local-tx-meta) - (outliner-op/apply-ops! conn - [[:save-block [{:block/uuid block-1-uuid - :block/title "1"} - nil]]] - local-tx-meta) - (let [block-1 (d/entity @conn [:block/uuid block-1-uuid])] - (outliner-op/apply-ops! conn - [[:insert-blocks [[{:block/uuid block-2-uuid - :block/title ""}] - (:db/id block-1) - {:sibling? true - :keep-uuid? true}]]] - local-tx-meta)) - (outliner-op/apply-ops! conn - [[:save-block [{:block/uuid block-2-uuid - :block/title "2"} - nil]]] - local-tx-meta) - (let [block-2 (d/entity @conn [:block/uuid block-2-uuid])] - (outliner-op/apply-ops! conn - [[:indent-outdent-blocks [[(:db/id block-2)] true {}]]] - local-tx-meta)) - - (loop [] - (when-not (= :frontend.worker.undo-redo/empty-undo-stack - (worker-undo-redo/undo test-repo)) - (recur))) - (loop [] - (when-not (= :frontend.worker.undo-redo/empty-redo-stack - (worker-undo-redo/redo test-repo)) - (recur))) - (is (not= :frontend.worker.undo-redo/empty-undo-stack - (worker-undo-redo/undo test-repo))) - (let [block-2 (d/entity @conn [:block/uuid block-2-uuid])] - (is (some? block-2)) - (is (= "2" (:block/title block-2))) - (is (= (:block/uuid page-1) (-> block-2 :block/page :block/uuid))) - (is (= (:block/uuid page-1) (-> block-2 :block/parent :block/uuid)))) - (is (nil? @invalid-payload*)) - (finally - (d/unlisten! conn ::worker-undo-listener) - (worker-undo-redo/clear-history! test-repo) - (reset! ldb/*transact-invalid-callback prev-invalid-callback)))))))) - (deftest enqueue-local-tx-canonicalizes-batch-import-to-transact-test (testing "batch-import-edn local tx persists as canonical transact op" (let [{:keys [conn client-ops-conn]} (setup-parent-child) @@ -1126,11 +1052,13 @@ :block/title "hello"} nil]]] local-tx-meta) (let [{:keys [tx-id]} (first (#'sync-apply/pending-txs test-repo))] - (is (= true - (:applied? (#'sync-apply/apply-history-action! test-repo - tx-id - true - {:db-sync/tx-id tx-id})))) + (let [{:keys [applied? history-tx-id]} (#'sync-apply/apply-history-action! test-repo + tx-id + true + {:db-sync/tx-id tx-id})] + (is (= true applied?)) + (is (uuid? history-tx-id)) + (is (not= tx-id history-tx-id))) (let [pending (#'sync-apply/pending-txs test-repo)] (is (= 2 (count pending))) (is (= 2 (count (distinct (map :tx-id pending))))) @@ -1138,6 +1066,59 @@ (get-in (#'sync-apply/pending-tx-by-id test-repo tx-id) [:forward-outliner-ops 0 1 0 :block/title])))))))))) +(deftest apply-history-action-preserves-source-forward-inverse-ops-test + (testing "undo/redo history actions should preserve source forward/inverse ops and create new tx rows" + (let [{:keys [conn client-ops-conn child1]} (setup-parent-child) + child-uuid (:block/uuid child1)] + (with-datascript-conns conn client-ops-conn + (fn [] + (outliner-op/apply-ops! conn + [[:save-block [{:block/uuid child-uuid + :block/title "hello"} nil]]] + local-tx-meta) + (let [{source-tx-id :tx-id} (first (#'sync-apply/pending-txs test-repo))] + (let [{undo-applied? :applied? + undo-history-tx-id :history-tx-id} + (#'sync-apply/apply-history-action! test-repo + source-tx-id + true + {})] + (is (= true undo-applied?)) + (is (uuid? undo-history-tx-id)) + (is (not= source-tx-id undo-history-tx-id))) + (let [source-pending (#'sync-apply/pending-tx-by-id test-repo source-tx-id) + pending-after-undo (#'sync-apply/pending-txs test-repo) + undo-pending (first (filter #(not= source-tx-id (:tx-id %)) pending-after-undo))] + (is (= 2 (count pending-after-undo))) + (is (some? undo-pending)) + (is (= "hello" + (get-in source-pending [:forward-outliner-ops 0 1 0 :block/title]))) + (is (= "child 1" + (get-in source-pending [:inverse-outliner-ops 0 1 0 :block/title]))) + (is (= "child 1" + (get-in undo-pending [:forward-outliner-ops 0 1 0 :block/title]))) + (is (= "hello" + (get-in undo-pending [:inverse-outliner-ops 0 1 0 :block/title])))) + (let [{redo-applied? :applied? + redo-history-tx-id :history-tx-id} + (#'sync-apply/apply-history-action! test-repo + source-tx-id + false + {})] + (is (= true redo-applied?)) + (is (uuid? redo-history-tx-id)) + (is (not= source-tx-id redo-history-tx-id))) + (let [source-pending (#'sync-apply/pending-tx-by-id test-repo source-tx-id) + pending-after-redo (#'sync-apply/pending-txs test-repo) + new-tx-ids (set (map :tx-id pending-after-redo))] + (is (= 3 (count pending-after-redo))) + (is (= 3 (count new-tx-ids))) + (is (contains? new-tx-ids source-tx-id)) + (is (= "hello" + (get-in source-pending [:forward-outliner-ops 0 1 0 :block/title]))) + (is (= "child 1" + (get-in source-pending [:inverse-outliner-ops 0 1 0 :block/title])))))))))) + (deftest apply-history-action-semantic-op-must-not-fallback-to-raw-tx-test (testing "semantic history action should not fallback to raw tx replay" (let [{:keys [conn client-ops-conn child1]} (setup-parent-child) diff --git a/src/test/frontend/worker/undo_redo_test.cljs b/src/test/frontend/worker/undo_redo_test.cljs index d698976dc4..8a1a489661 100644 --- a/src/test/frontend/worker/undo_redo_test.cljs +++ b/src/test/frontend/worker/undo_redo_test.cljs @@ -34,8 +34,7 @@ (reset! worker-state/*client-ops-conns {test-repo client-ops-conn}) (d/listen! conn ::gen-undo-ops (fn [tx-report] - (db-sync/enqueue-local-tx! test-repo tx-report) - (worker-undo-redo/gen-undo-ops! test-repo tx-report))) + (db-sync/enqueue-local-tx! test-repo tx-report))) (worker-undo-redo/clear-history! test-repo) (try (f) @@ -47,27 +46,6 @@ (use-fixtures :each with-worker-conns) -(deftest gen-undo-ops-consumes-pending-editor-info-test - (let [conn (worker-state/get-datascript-conn test-repo) - block (db-test/find-block-by-content @conn "task") - block-uuid (:block/uuid block) - tx-report (d/with @conn - [[:db/add (:db/id block) :block/title "updated task"]] - (local-tx-meta - {:outliner-op :save-block - :outliner-ops [[:save-block [{:block/uuid block-uuid - :block/title "updated task"} nil]]]})) - editor-info {:block-uuid block-uuid - :container-id 1 - :start-pos 0 - :end-pos 7}] - (worker-undo-redo/set-pending-editor-info! test-repo editor-info) - (worker-undo-redo/gen-undo-ops! test-repo tx-report) - (let [op (last (get @worker-undo-redo/*undo-ops test-repo))] - (is (= [::worker-undo-redo/record-editor-info editor-info] - (first op))) - (is (nil? (get @worker-undo-redo/*pending-editor-info test-repo)))))) - (deftest worker-ui-state-roundtrip-test (let [ui-state-str "{:old-state {}, :new-state {:route-data {:to :page}}}"] (worker-undo-redo/record-ui-state! test-repo ui-state-str) @@ -130,6 +108,13 @@ (second %)) undo-op))) +(defn- latest-redo-history-data + [] + (let [redo-op (last (get @worker-undo-redo/*redo-ops test-repo))] + (some #(when (= ::worker-undo-redo/db-transact (first %)) + (second %)) + redo-op))) + (deftest undo-missing-history-action-row-clears-history-test (testing "worker undo treats missing tx-id action row as unavailable and clears history" (worker-undo-redo/clear-history! test-repo) @@ -153,6 +138,27 @@ (is (= ::worker-undo-redo/empty-redo-stack redo-result)) (is (= "v2" (:block/title (d/entity @conn [:block/uuid child-uuid])))))))) +(deftest undo-redo-rebinds-stack-to-latest-history-tx-id-test + (testing "undo/redo pushes stack op with latest persisted history tx id" + (worker-undo-redo/clear-history! test-repo) + (let [conn (worker-state/get-datascript-conn test-repo) + client-ops-conn (get @worker-state/*client-ops-conns test-repo) + {:keys [child-uuid]} (seed-page-parent-child!)] + (save-block-title! conn child-uuid "v1") + (let [source-tx-id (:db-sync/tx-id (latest-undo-history-data))] + (is (uuid? source-tx-id)) + (is (not= ::worker-undo-redo/empty-undo-stack + (worker-undo-redo/undo test-repo))) + (let [redo-tx-id (:db-sync/tx-id (latest-redo-history-data))] + (is (uuid? redo-tx-id)) + (is (= source-tx-id redo-tx-id)) + (is (not= ::worker-undo-redo/empty-redo-stack + (worker-undo-redo/redo test-repo))) + (let [undo-tx-id (:db-sync/tx-id (latest-undo-history-data))] + (is (uuid? undo-tx-id)) + (is (not= source-tx-id undo-tx-id)) + (is (some? (d/entity @client-ops-conn [:db-sync/tx-id undo-tx-id]))))))))) + (deftest undo-records-only-local-txs-test (testing "undo history records only local txs" (worker-undo-redo/clear-history! test-repo) @@ -550,6 +556,85 @@ (is (some? inserted-a)) (is (some? inserted-b)))))) +(deftest apply-template-repeated-undo-redo-uses-latest-history-tx-id-test + (testing ":apply-template repeated undo/redo should always undo latest recreated blocks" + (worker-undo-redo/clear-history! test-repo) + (let [conn (worker-state/get-datascript-conn test-repo) + {:keys [page-uuid]} (seed-page-parent-child!) + page-id (:db/id (d/entity @conn [:block/uuid page-uuid])) + template-root-uuid (random-uuid) + template-a-uuid (random-uuid) + template-b-uuid (random-uuid) + empty-target-uuid (random-uuid)] + (outliner-op/apply-ops! + conn + [[:insert-blocks [[{:block/uuid template-root-uuid + :block/title "template 1" + :block/tags #{:logseq.class/Template}} + {:block/uuid template-a-uuid + :block/title "a" + :block/parent [:block/uuid template-root-uuid]} + {:block/uuid template-b-uuid + :block/title "b" + :block/parent [:block/uuid template-a-uuid]}] + page-id + {:sibling? false + :keep-uuid? true}]]] + (local-tx-meta {:client-id "test-client"})) + (outliner-op/apply-ops! + conn + [[:insert-blocks [[{:block/uuid empty-target-uuid + :block/title ""}] + page-id + {:sibling? false + :keep-uuid? true}]]] + (local-tx-meta {:client-id "test-client"})) + (worker-undo-redo/clear-history! test-repo) + (let [template-root (d/entity @conn [:block/uuid template-root-uuid]) + empty-target (d/entity @conn [:block/uuid empty-target-uuid]) + template-blocks (->> (ldb/get-block-and-children @conn template-root-uuid + {:include-property-block? true}) + rest) + blocks-to-insert (cons (assoc (first template-blocks) + :logseq.property/used-template (:db/id template-root)) + (rest template-blocks)) + find-inserted-a-id (fn [] + (d/q '[:find ?b . + :in $ ?template-uuid + :where + [?template :block/uuid ?template-uuid] + [?b :logseq.property/used-template ?template] + [?b :block/title "a"]] + @conn + template-root-uuid))] + (outliner-op/apply-ops! + conn + [[:apply-template [(:db/id template-root) + (:db/id empty-target) + {:sibling? true + :replace-empty-target? true + :template-blocks blocks-to-insert}]]] + (local-tx-meta {:client-id "test-client"})) + (is (some? (find-inserted-a-id))) + (is (not= ::worker-undo-redo/empty-undo-stack + (worker-undo-redo/undo test-repo))) + (is (nil? (find-inserted-a-id))) + (is (not= ::worker-undo-redo/empty-redo-stack + (worker-undo-redo/redo test-repo))) + (let [redo-1-a-id (find-inserted-a-id)] + (is (some? redo-1-a-id)) + (is (not= ::worker-undo-redo/empty-undo-stack + (worker-undo-redo/undo test-repo))) + (is (nil? (find-inserted-a-id))) + (is (not= ::worker-undo-redo/empty-redo-stack + (worker-undo-redo/redo test-repo))) + (let [redo-2-a-id (find-inserted-a-id)] + (is (some? redo-2-a-id)) + (is (not= redo-1-a-id redo-2-a-id)) + (is (not= ::worker-undo-redo/empty-undo-stack + (worker-undo-redo/undo test-repo))) + (is (nil? (find-inserted-a-id))))))))) + (deftest undo-history-records-forward-ops-for-save-block-test (testing "worker save-block history keeps semantic forward ops for redo replay" (worker-undo-redo/clear-history! test-repo)