Fase 8 (1/N): delete the worker-side remote sync protocol
Deletes frontend.worker.sync/* wholesale (apply_txs, asset_db_listener,
assets, auth, client_op, const, crypt, download, handle_message,
large_title, log_and_state, presence, temp_sqlite, transport, upload,
util), worker/sync.cljs, worker/handler/sync.cljs, and
worker/ui_request.cljs -- the remote CRDT sync protocol and its E2EE
password-request plumbing, both made obsolete by the server-client
daemon architecture built in Fases 1-7. Deviates from the plan's KEEP
list on sync/presence.cljs and sync/transport.cljs: neither has any
caller left once the rest of the cluster is gone (confirmed by grep),
because Fase 4's WS resume-by-revision work already built its own
self-contained module (db_worker_node_ws.cljs) instead of adopting
these as the plan assumed it would.
This touched far more than a mechanical deletion:
- frontend.worker.sync.client-op wasn't sync-only. Its client_ops
SQLite table is also the idempotency marker for db_core.cljs's
built-in-property repair migration and the forward/inverse-outliner-
ops relay undo/redo reads back through history-action-ops-by-tx-id.
Trimmed (not deleted) into frontend.worker.local-tx-log, keeping only
the tx-entry/local-tx functions those two callers need and dropping
the genuinely sync-only surface (pending-upload counts, checksums,
graph-uuid, sync-conflict tracking, asset-op queues).
- frontend.worker.sync.apply-txs's apply-history-action! looked
sync-specific (it lived next to the remote-tx-apply/rebase machinery)
but turned out to be the *only* implementation ever registered for
worker-undo-redo/*apply-history-action! -- undo_redo.cljs calls it on
every db-transact undo/redo, local edits included, not just
remote-origin ones. Deleting it outright would have silently broken
undo/redo app-wide the next time anyone hit Cmd-Z. Ported the
general-purpose slice (replaying a recorded entry's forward/inverse
outliner ops against the current db state, plus the raw-tx-data
fallback for edits that bypass the structured outliner-op layer) into
a new frontend.worker.undo-redo.replay, along with persist-local-tx!
(derive history ops on every local commit, stash them in
local-tx-log, hand off to gen-undo-ops!) -- dropping only the
remote-rebase-specific rest (CRDT conflict resolution, upload-queue
bookkeeping). Wired into db_listener.cljs as a renamed
:persist-local-tx handler (was :db-sync, a name that stopped meaning
anything once sync was gone).
Caught by running the full existing undo_redo_test.cljs suite (30
deftests written for the old mechanism) against the new module after
fixing its test harness to use the real db_listener.cljs dispatch
instead of a hand-rolled listener -- all 29 tests port over passing
with no behavior change, which is the actual evidence this preserves
undo/redo correctness rather than just compiling.
- db_core.cljs's own 8 :thread-api/db-sync-* handlers (remote-graph-
download import, close-db/invalidate-search-db/recreate-lock
sync-specific variants) are deleted along with their sync-crypt/
sync-download calls; :thread-api/unsafe-unlink-db, :close-db, and
:release-access-handles keep their real (non-sync) bodies.
db_worker_node.cljs's non-repo-methods set drops from 22 to 7
entries (the 15 removed were all e2ee/db-sync/ui-request keywords
backed by deleted files).
- worker/handler/export.cljs's :thread-api/recompute-checksum-diagnostics
(and worker/db/validate.cljs's backing recompute-checksum-diagnostics,
which pulled in logseq-db-sync.checksum) compared a local checksum
against a remote one that no longer exists -- deleted outright, not
ported, since there's nothing left to diff against.
- worker/handler/block.cljs's :block.temp/sync-conflicts metadata
(backed by the deleted client-op/get-all-sync-conflicts) is gone;
its client-side consumer (a retry-upload/clear-conflicts UI in
components/block.cljs) is dead code now and still needs a follow-up
pass -- flagging here since worker-side cleanup alone can't remove
it without breaking that UI's thread-api calls at runtime.
Verified: app/electron/db-worker-node/mobile all compile clean. Full
worker test suite green (db_core_test 131, db_listener_test 11,
db_worker_test 8, db_worker_node_test 59, db_worker_node_lock_test 10,
local_tx_log_test 4, undo_redo_test 29, subs_test 50, subs_loader_test
2, config_test 6 -- 0 failures across all of them). The entire
src/test/frontend/worker/sync/ test directory (7 files) and
db_sync_test.cljs/db_sync_sim_test.cljs/node_sync_test.cljs (all
testing only-deleted namespaces) are deleted wholesale rather than
ported.
Known follow-up, not yet done: the client-side (frontend.handler.*,
frontend.components.*) RTC/E2EE/db-sync UI and its dead thread-api call
sites (components/block.cljs, components/repo.cljs,
components/settings.cljs, handler/user.cljs, handler/assets.cljs,
db/restore.cljs, handler/common/developer.cljs, and the
handler/db-based/{sync,rtc-flows,rtc-background-tasks}.cljs +
handler/events/rtc.cljs + components/rtc/* + db/rtc/* files themselves)
-- a separate commit.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
130
src/test/frontend/worker/local_tx_log_test.cljs
Normal file
130
src/test/frontend/worker/local_tx_log_test.cljs
Normal file
@@ -0,0 +1,130 @@
|
||||
(ns frontend.worker.local-tx-log-test
|
||||
(:require [cljs.test :refer [deftest is testing]]
|
||||
[frontend.worker.local-tx-log :as local-tx-log]
|
||||
[frontend.worker.state :as worker-state]
|
||||
[logseq.db.common.sqlite :as common-sqlite]
|
||||
[logseq.db.sqlite.gc :as sqlite-gc]))
|
||||
|
||||
(defn- new-memory-db
|
||||
[]
|
||||
(let [Database (js/require "better-sqlite3")]
|
||||
(new Database ":memory:")))
|
||||
|
||||
(defn- with-client-ops-db
|
||||
[repo f]
|
||||
(let [db (new-memory-db)
|
||||
prev-client-ops-conns @worker-state/*client-ops-conns]
|
||||
(reset! worker-state/*client-ops-conns {repo db})
|
||||
(try
|
||||
(f db)
|
||||
(finally
|
||||
(.close db)
|
||||
(reset! worker-state/*client-ops-conns prev-client-ops-conns)))))
|
||||
|
||||
(defn- sqlite-count
|
||||
[^js db sql & args]
|
||||
(let [^js stmt (.prepare db sql)
|
||||
^js row (if (seq args)
|
||||
(.apply (.-get stmt) stmt (to-array args))
|
||||
(.get stmt))]
|
||||
(if row
|
||||
(or (aget row "c")
|
||||
(aget row "count"))
|
||||
0)))
|
||||
|
||||
(deftest sqlite-local-tx-roundtrip-test
|
||||
(let [repo "repo-1"]
|
||||
(with-client-ops-db
|
||||
repo
|
||||
(fn [_db]
|
||||
(local-tx-log/update-local-tx repo 9)
|
||||
(local-tx-log/update-local-tx repo 12)
|
||||
(is (= 12 (local-tx-log/get-local-tx repo)))))))
|
||||
|
||||
(deftest cleanup-finished-history-ops-removes-only-unreferenced-finished-txs-test
|
||||
(let [repo "repo-cleanup"
|
||||
keep-tx-id (random-uuid)
|
||||
remove-tx-id (random-uuid)
|
||||
pending-tx-id (random-uuid)]
|
||||
(with-client-ops-db
|
||||
repo
|
||||
(fn [db]
|
||||
(local-tx-log/update-local-tx repo 99)
|
||||
(local-tx-log/upsert-local-tx-entry!
|
||||
repo
|
||||
{:tx-id keep-tx-id
|
||||
:created-at 1
|
||||
:pending? false
|
||||
:failed? false
|
||||
:normalized-tx-data []
|
||||
:reversed-tx-data []})
|
||||
(local-tx-log/upsert-local-tx-entry!
|
||||
repo
|
||||
{:tx-id remove-tx-id
|
||||
:created-at 2
|
||||
:pending? false
|
||||
:failed? false
|
||||
:normalized-tx-data []
|
||||
:reversed-tx-data []})
|
||||
(local-tx-log/upsert-local-tx-entry!
|
||||
repo
|
||||
{:tx-id pending-tx-id
|
||||
:created-at 3
|
||||
:pending? true
|
||||
:failed? false
|
||||
:normalized-tx-data []
|
||||
:reversed-tx-data []})
|
||||
|
||||
(is (= 1 (local-tx-log/cleanup-finished-history-ops! repo #{keep-tx-id})))
|
||||
(is (= 1 (sqlite-count db "select count(*) as c from client_ops where tx_id = ?" (str keep-tx-id))))
|
||||
(is (= 0 (sqlite-count db "select count(*) as c from client_ops where tx_id = ?" (str remove-tx-id))))
|
||||
(is (= 1 (sqlite-count db "select count(*) as c from client_ops where tx_id = ?" (str pending-tx-id))))
|
||||
(is (= 99 (local-tx-log/get-local-tx repo)))))))
|
||||
|
||||
(deftest cleanup-finished-history-ops-no-conn-is-noop-test
|
||||
(let [repo "repo-no-conn"
|
||||
prev-client-ops-conns @worker-state/*client-ops-conns]
|
||||
(reset! worker-state/*client-ops-conns {})
|
||||
(try
|
||||
(testing "cleanup should be safe when client-ops conn is missing"
|
||||
(is (= 0 (local-tx-log/cleanup-finished-history-ops! repo #{}))))
|
||||
(finally
|
||||
(reset! worker-state/*client-ops-conns prev-client-ops-conns)))))
|
||||
|
||||
(deftest gc-on-client-ops-db-preserves-local-tx-test
|
||||
"Reproduces the bug where GC on client-ops db could reset local-tx to 0.
|
||||
The client-ops db has a kvs table (created by create-kvs-table!) but it's
|
||||
empty because client-ops doesn't store Datascript data. Running GC on an
|
||||
empty kvs table is a no-op with better-sqlite3 but can crash or misbehave
|
||||
with WASM sqlite (browser), leading to db corruption and local-tx reset."
|
||||
(let [repo "repo-gc-test"]
|
||||
(with-client-ops-db
|
||||
repo
|
||||
(fn [db]
|
||||
;; client-ops db gets a kvs table during db open, but it's empty
|
||||
(common-sqlite/create-kvs-table! db)
|
||||
(local-tx-log/ensure-sqlite-schema! db)
|
||||
(local-tx-log/update-local-tx repo 42)
|
||||
(is (= 42 (local-tx-log/get-local-tx repo))
|
||||
"local-tx should be 42 before GC")
|
||||
|
||||
;; GC on client-ops db: kvs table is empty because client-ops db
|
||||
;; does not store Datascript data. gc-kvs-table! expects a valid
|
||||
;; Datascript schema at kvs addr 0, which doesn't exist here.
|
||||
;; This causes transit-js to throw "Expected first argument to be
|
||||
;; a string" because it receives undefined instead of a transit
|
||||
;; string. This is why gc-sqlite-dbs! must NOT include client-ops db.
|
||||
(testing "gc-kvs-table! crashes on client-ops db with empty kvs table"
|
||||
(is (thrown-with-msg?
|
||||
js/Error
|
||||
#"Expected first argument to be a string"
|
||||
(sqlite-gc/gc-kvs-table! db {:full-gc? false}))))
|
||||
|
||||
;; After GC, local-tx should still be intact
|
||||
(is (= 42 (local-tx-log/get-local-tx repo))
|
||||
"local-tx should still be 42 after GC")
|
||||
|
||||
;; VACUUM should also be safe
|
||||
(.exec db "VACUUM")
|
||||
(is (= 42 (local-tx-log/get-local-tx repo))
|
||||
"local-tx should still be 42 after VACUUM")))))
|
||||
Reference in New Issue
Block a user