From bc97666ee39f01d8255ae7154415788d28512083 Mon Sep 17 00:00:00 2001 From: Tienson Qin Date: Fri, 10 Apr 2026 02:01:50 +0800 Subject: [PATCH] fix(db-sync): keep title-only raw txs through rebase --- src/main/frontend/worker/sync/apply_txs.cljs | 92 +++++++--- src/test/frontend/worker/db_sync_test.cljs | 184 +++++++++++++++++-- 2 files changed, 237 insertions(+), 39 deletions(-) diff --git a/src/main/frontend/worker/sync/apply_txs.cljs b/src/main/frontend/worker/sync/apply_txs.cljs index 266feb084d..3f7abb02ee 100644 --- a/src/main/frontend/worker/sync/apply_txs.cljs +++ b/src/main/frontend/worker/sync/apply_txs.cljs @@ -201,14 +201,32 @@ (seq (:inverse-outliner-ops tx-meta)))} (op-construct/derive-history-outliner-ops db-before db-after tx-data tx-meta))) +(defn- title-only-raw-tx? + [tx-data] + (let [tx-items (seq tx-data)] + (and tx-items + (every? + (fn [entry] + (and (vector? entry) + (>= (count entry) 4) + (= :db/add (first entry)) + (= :block/title (nth entry 2)) + (string? (nth entry 3)))) + tx-items)))) + (defn- rebase-history-ops [local-tx] (let [forward-outliner-ops (seq (:forward-outliner-ops local-tx)) inverse-outliner-ops (seq (:inverse-outliner-ops local-tx)) - ;; Fall back to raw tx replay for legacy rebase rows that explicitly - ;; persisted raw transact placeholders. + ;; Fall back to raw tx replay for legacy rebase rows that persisted without + ;; semantic history ops, and for direct title-only transact rows whose + ;; metadata doesn't carry semantic ops. Keep other non-rebase rows as-is + ;; to avoid replaying arbitrary local raw txs during remote rebase. fallback-forward-ops (when (and (nil? forward-outliner-ops) - (= :rebase (:outliner-op local-tx))) + (or (= :rebase (:outliner-op local-tx)) + (and (nil? (:outliner-op local-tx)) + (title-only-raw-tx? (:tx local-tx)))) + (seq (:tx local-tx))) canonical-transact-op) forward-ops (or forward-outliner-ops fallback-forward-ops) inverse-ops (or inverse-outliner-ops @@ -231,6 +249,7 @@ existing-ent (d/entity @conn [:db-sync/tx-id tx-id]) should-inc-pending? (not= true (:db-sync/pending? existing-ent)) now (.now js/Date) + created-at (or (:db-sync/created-at existing-ent) now) {:keys [forward-outliner-ops inverse-outliner-ops]} (derive-history-outliner-ops db-before db-after tx-data tx-meta) inferred-outliner-ops?' (inferred-outliner-ops? tx-meta)] @@ -254,7 +273,7 @@ :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}]) + :db-sync/created-at created-at}]) (worker-undo-redo/gen-undo-ops! repo tx-report tx-id {:apply-history-action! apply-history-action!}) (when should-inc-pending? @@ -550,27 +569,16 @@ :error error})))))) (flush-pending! repo client))) -(defn- missing-order-add-op? - [db item] - (and (vector? item) - (>= (count item) 4) - (= :db/add (first item)) - (= :block/order (nth item 2 nil)) - (nil? (d/entity db (second item))))) - (defn- reverse-history-action! [conn local-tx] - (let [db @conn] - (if-let [tx-data (->> (:reversed-tx local-tx) - (remove (fn [item] (missing-order-add-op? db item))) - seq)] - (ldb/transact! conn tx-data - {:outliner-op (:outliner-op local-tx) - :reverse? true}) - (invalid-rebase-op! :reverse-history-action - {:reason :missing-reversed-tx-data - :tx-id (:tx-id local-tx) - :outliner-op (:outliner-op local-tx)})))) + (if-let [tx-data (seq (:reversed-tx local-tx))] + (ldb/transact! conn tx-data + {:outliner-op (:outliner-op local-tx) + :reverse? true}) + (invalid-rebase-op! :reverse-history-action + {:reason :missing-reversed-tx-data + :tx-id (:tx-id local-tx) + :outliner-op (:outliner-op local-tx)}))) (defn- replace-uuid-str-with-eid [db v] @@ -964,7 +972,20 @@ (doseq [op forward-ops] (replay-canonical-outliner-op! conn op rebase-db-before)))))] {:tx-id (:tx-id local-tx) - :status (if rebase-tx-report :rebased :no-op)}) + :status (cond + rebase-tx-report + :rebased + + ;; Title-only raw tx replay can become empty after remote applies + ;; the same title; keep it pending instead of dropping as stale. + (and (= canonical-transact-op forward-ops) + (nil? (:forward-outliner-ops local-tx)) + (nil? (:outliner-op local-tx)) + (title-only-raw-tx? (:tx local-tx))) + :skipped + + :else + :no-op)}) (catch :default error (let [drop-log {:tx-id (:tx-id local-tx) :outliner-ops forward-ops @@ -972,8 +993,27 @@ (log/warn :db-sync/drop-op-driven-pending-tx drop-log) {:tx-id (:tx-id local-tx) :status :failed}))) - {:tx-id (:tx-id local-tx) - :status :skipped}))) + (let [tx-data (some-> (:tx local-tx) seq vec) + dry-run-tx-data (some->> tx-data + (mapv (fn [item] + (if (and (vector? item) (= 5 (count item))) + (let [[op e a v _t] item] + [op e a v]) + item))))] + (if (seq dry-run-tx-data) + (try + (d/with @conn dry-run-tx-data) + {:tx-id (:tx-id local-tx) + :status :skipped} + (catch :default error + (log/warn :db-sync/drop-skipped-pending-tx + {:tx-id (:tx-id local-tx) + :outliner-op (:outliner-op local-tx) + :error error}) + {:tx-id (:tx-id local-tx) + :status :failed})) + {:tx-id (:tx-id local-tx) + :status :skipped}))))) (defn- rebase-local-txs! [repo conn local-txs rebase-db-before] diff --git a/src/test/frontend/worker/db_sync_test.cljs b/src/test/frontend/worker/db_sync_test.cljs index c180ac2967..4c361c9afb 100644 --- a/src/test/frontend/worker/db_sync_test.cljs +++ b/src/test/frontend/worker/db_sync_test.cljs @@ -405,6 +405,33 @@ (is (= 0 @send-calls))) (#'sync-apply/set-upload-stopped! test-repo false)))))) +(deftest prepare-upload-tx-entries-drops-empty-txs-test + (testing "empty tx rows should be dropped from upload batch" + (let [{:keys [conn client-ops-conn child1]} (setup-parent-child) + empty-tx-id (random-uuid) + valid-tx-id (random-uuid)] + (with-datascript-conns conn client-ops-conn + (fn [] + (ldb/transact! client-ops-conn + [{:db-sync/tx-id empty-tx-id + :db-sync/pending? true + :db-sync/created-at 1 + :db-sync/outliner-op :transact + :db-sync/normalized-tx-data []} + {:db-sync/tx-id valid-tx-id + :db-sync/pending? true + :db-sync/created-at 2 + :db-sync/outliner-op :save-block + :db-sync/normalized-tx-data + [[:db/add [:block/uuid (:block/uuid child1)] + :block/title + "valid-title"]]}]) + (let [pending (#'sync-apply/pending-txs test-repo) + {:keys [tx-entries drop-tx-ids]} + (sync-apply/prepare-upload-tx-entries conn pending)] + (is (= [empty-tx-id] drop-tx-ids)) + (is (= [valid-tx-id] (mapv :tx-id tx-entries))))))))) + (deftest sync-counts-counts-only-true-pending-local-ops-test (testing "pending-local should count only rows with :db-sync/pending? true" (let [{:keys [conn client-ops-conn]} (setup-parent-child)] @@ -1504,24 +1531,31 @@ (is (= "raw reverse" (:block/title (d/entity @conn [:block/uuid child-uuid])))))))))) -(deftest reverse-local-txs-drops-missing-entity-order-adds-test - (testing "reverse should ignore :db/add :block/order for missing entities" - (let [{:keys [conn client-ops-conn child1]} (setup-parent-child) +(deftest reverse-local-txs-keeps-order-add-for-restored-entity-test + (testing "reverse should keep :db/add :block/order when restoring a deleted block" + (let [{:keys [conn client-ops-conn parent]} (setup-parent-child) tx-id (random-uuid) - child-id (:db/id child1) - child-uuid (:block/uuid child1) - missing-uuid (random-uuid) + restored-id 999999 + restored-uuid (random-uuid) + now (.now js/Date) + parent-id (:db/id parent) local-tx {:tx-id tx-id - :outliner-op :save-block - :reversed-tx [[:db/add [:block/uuid missing-uuid] :block/order "a0"] - [:db/add child-id :block/title "reverse-kept"]]}] + :outliner-op :delete-block + :reversed-tx [[:db/add restored-id :block/uuid restored-uuid] + [:db/add restored-id :block/title "reverse-restored"] + [:db/add restored-id :block/created-at now] + [:db/add restored-id :block/updated-at now] + [:db/add restored-id :block/page parent-id] + [:db/add restored-id :block/parent parent-id] + [:db/add restored-id :block/order "a0"]]}] (with-datascript-conns conn client-ops-conn (fn [] - (is (nil? (d/entity @conn [:block/uuid missing-uuid]))) - (let [reports (#'sync-apply/reverse-local-txs! conn [local-tx])] + (is (nil? (d/entity @conn [:block/uuid restored-uuid]))) + (let [reports (#'sync-apply/reverse-local-txs! conn [local-tx]) + restored (d/entity @conn [:block/uuid restored-uuid])] (is (= 1 (count reports))) - (is (= "reverse-kept" - (:block/title (d/entity @conn [:block/uuid child-uuid])))))))))) + (is (= "reverse-restored" (:block/title restored))) + (is (= "a0" (:block/order restored))))))))) (deftest enqueue-local-tx-keeps-mixed-semantic-forward-outliner-ops-test (testing "mixed semantic outliner ops stay semantic and preserve op ordering" @@ -3458,6 +3492,66 @@ (is (= tx-ids-before tx-ids-after)) (is (= 2 (count (distinct tx-ids-after)))))))))))) +(deftest rebase-keeps-original-created-at-for-pending-tx-test + (testing "rebasing a pending tx should keep its original created-at ordering key" + (let [{:keys [conn client-ops-conn parent child1]} (setup-parent-child)] + (with-redefs [db-sync/enqueue-local-tx! + (let [orig db-sync/enqueue-local-tx!] + (fn [repo tx-report] + (when-not (:rtc-tx? (:tx-meta tx-report)) + (orig repo tx-report))))] + (with-datascript-conns conn client-ops-conn + (fn [] + (d/transact! conn [[:db/add (:db/id child1) :block/title "child 1 local"]]) + (let [{:keys [tx-id]} (first (#'sync-apply/pending-txs test-repo)) + created-at-before (:db-sync/created-at + (d/entity @client-ops-conn [:db-sync/tx-id tx-id]))] + (is (number? created-at-before)) + (loop [] + (when (<= (.now js/Date) created-at-before) + (recur))) + (#'sync-apply/apply-remote-tx! + test-repo + nil + [[:db/add (:db/id parent) :block/title "parent remote"]]) + (let [created-at-after (:db-sync/created-at + (d/entity @client-ops-conn [:db-sync/tx-id tx-id]))] + (is (= created-at-before created-at-after)))))))))) + +(deftest persist-local-tx-keeps-created-at-for-existing-tx-id-test + (testing "persisting an existing tx-id should preserve created-at ordering" + (let [{:keys [conn client-ops-conn child1]} (setup-parent-child) + tx-id (random-uuid) + child-id (:db/id child1)] + (with-datascript-conns conn client-ops-conn + (fn [] + (with-redefs [undo-redo/gen-undo-ops! (fn [& _] nil)] + (let [tx-report-1 (d/with @conn + [[:db/add child-id :block/title "created-at-v1"]] + (assoc local-tx-meta + :db-sync/tx-id tx-id + :outliner-op :save-block)) + {:keys [normalized-tx-data reversed-datoms]} + (#'sync-apply/normalize-rebased-pending-tx tx-report-1)] + (#'sync-apply/persist-local-tx! test-repo tx-report-1 normalized-tx-data reversed-datoms) + (let [created-at-before (:db-sync/created-at + (d/entity @client-ops-conn [:db-sync/tx-id tx-id]))] + (is (number? created-at-before)) + (loop [] + (when (<= (.now js/Date) created-at-before) + (recur))) + (let [tx-report-2 (d/with @conn + [[:db/add child-id :block/title "created-at-v2"]] + (assoc local-tx-meta + :db-sync/tx-id tx-id + :outliner-op :rebase)) + {:keys [normalized-tx-data reversed-datoms]} + (#'sync-apply/normalize-rebased-pending-tx tx-report-2)] + (#'sync-apply/persist-local-tx! test-repo tx-report-2 normalized-tx-data reversed-datoms) + (let [created-at-after (:db-sync/created-at + (d/entity @client-ops-conn [:db-sync/tx-id tx-id]))] + (is (= created-at-before created-at-after)))))))))))) + (deftest rebase-keeps-pending-when-rebased-empty-test (testing "pending txs stay when rebased txs are empty" (let [{:keys [conn client-ops-conn child1]} (setup-parent-child)] @@ -3515,6 +3609,70 @@ (is (not-any? string? (keep second save-block-tx)))))))))))) +(deftest rebase-drops-stale-raw-pending-tx-with-missing-history-ops-test + (testing "legacy rebase rows without history ops should fallback to transact replay and be dropped when stale" + (let [{:keys [conn client-ops-conn child1]} (setup-parent-child) + block-uuid (:block/uuid child1) + previous-title (:block/title child1) + tx-id (random-uuid)] + (with-datascript-conns conn client-ops-conn + (fn [] + (ldb/transact! client-ops-conn + [{:db-sync/tx-id tx-id + :db-sync/pending? true + :db-sync/created-at 1 + :db-sync/outliner-op :rebase + :db-sync/forward-outliner-ops nil + :db-sync/inverse-outliner-ops nil + :db-sync/normalized-tx-data + [[:db/add [:block/uuid block-uuid] + :block/title + "stale raw value"]] + :db-sync/reversed-tx-data + [[:db/add [:block/uuid block-uuid] + :block/title + previous-title]]}]) + (is (= 1 (count (#'sync-apply/pending-txs test-repo)))) + (#'sync-apply/apply-remote-txs! + test-repo + nil + [{:tx-data [[:db/retractEntity [:block/uuid block-uuid]]]}]) + (is (empty? (#'sync-apply/pending-txs test-repo)))))))) + +(deftest rebase-replays-title-only-raw-pending-tx-without-history-ops-test + (testing "metadata-less title-only raw pending tx should replay during rebase" + (let [{:keys [conn client-ops-conn child1 parent]} (setup-parent-child) + block-uuid (:block/uuid child1) + previous-title (:block/title child1) + parent-uuid (:block/uuid parent) + tx-id (random-uuid) + local-title "local raw title"] + (with-datascript-conns conn client-ops-conn + (fn [] + (ldb/transact! client-ops-conn + [{:db-sync/tx-id tx-id + :db-sync/pending? true + :db-sync/created-at 1 + :db-sync/outliner-op nil + :db-sync/forward-outliner-ops nil + :db-sync/inverse-outliner-ops nil + :db-sync/normalized-tx-data + [[:db/add [:block/uuid block-uuid] + :block/title + local-title]] + :db-sync/reversed-tx-data + [[:db/add [:block/uuid block-uuid] + :block/title + previous-title]]}]) + (#'sync-apply/apply-remote-txs! + test-repo + nil + [{:tx-data [[:db/add [:block/uuid parent-uuid] :block/title "parent remote"]]}]) + (let [pending (#'sync-apply/pending-txs test-repo)] + (is (= local-title (:block/title (d/entity @conn [:block/uuid block-uuid])))) + (is (= 1 (count pending))) + (is (= tx-id (:tx-id (first pending)))))))))) + (deftest reverse-tx-data-create-property-text-block-restores-base-db-test (testing "reverse-tx-data for create-property-text-block should restore the base db" (let [conn (db-test/create-conn-with-blocks