From 12b4c3399d417fc0d3db3b11104d99631e734ba6 Mon Sep 17 00:00:00 2001 From: rcmerci Date: Thu, 30 Apr 2026 00:17:27 +0800 Subject: [PATCH] fix(cli,db-worker): not keep empty new graph when sync download failed --- cli-e2e/spec/non_sync_cases.edn | 10 +- src/main/logseq/cli/command/graph.cljs | 2 +- src/main/logseq/cli/command/sync.cljs | 124 ++++++++++++----- src/main/logseq/cli/common.cljs | 30 ++-- src/test/logseq/cli/command/sync_test.cljs | 153 ++++++++++++++++++++- src/test/logseq/cli/integration_test.cljs | 76 +++++++++- 6 files changed, 336 insertions(+), 59 deletions(-) diff --git a/cli-e2e/spec/non_sync_cases.edn b/cli-e2e/spec/non_sync_cases.edn index 7a78f64e7d..130892edf0 100644 --- a/cli-e2e/spec/non_sync_cases.edn +++ b/cli-e2e/spec/non_sync_cases.edn @@ -894,10 +894,14 @@ ["{{cli}} --root-dir {{root-dir-arg}} --config {{config-path-arg}} --output json graph create --graph {{graph-arg}} >/dev/null" "{{cli}} --root-dir {{root-dir-arg}} --config {{config-path-arg}} --output json server stop --graph {{graph-arg}} >/dev/null"], :cmds - ["{{cli}} --root-dir {{root-dir-arg}} --config {{config-path-arg}} --output json graph remove --graph {{graph-arg}}"], - :expect {:exit 1, :stdout-contains ["graph-not-removed"]}, + ["{{cli}} --root-dir {{root-dir-arg}} --config {{config-path-arg}} --output json graph remove --graph {{graph-arg}}" + "{{cli}} --root-dir {{root-dir-arg}} --config {{config-path-arg}} --output json graph list"], + :expect + {:exit 0, + :stdout-json-paths {[:status] "ok"}, + :stdout-not-contains ["{{graph}}"]}, :covers - {:commands ["graph remove"], + {:commands ["graph remove" "graph list"], :options {:global ["--config" "--graph" "--root-dir" "--output"]}}, :tags [:graph :remove]} {:id "remove-block-uuid-human", diff --git a/src/main/logseq/cli/command/graph.cljs b/src/main/logseq/cli/command/graph.cljs index ff9acd31a1..36ac594787 100644 --- a/src/main/logseq/cli/command/graph.cljs +++ b/src/main/logseq/cli/command/graph.cljs @@ -464,7 +464,7 @@ (= :server-not-found (get-in stop-result [:error :code]))) (throw (ex-info (get-in stop-result [:error :message] "failed to stop server") {:code (get-in stop-result [:error :code])}))) - unlinked-dir (cli-common/unlink-graph! (:repo action))] + unlinked-dir (cli-common/unlink-graph! (cli-server/graphs-dir config) (:repo action))] (if unlinked-dir {:status :ok :data {:result nil}} diff --git a/src/main/logseq/cli/command/sync.cljs b/src/main/logseq/cli/command/sync.cljs index 012fcabb62..ee76a3a0cb 100644 --- a/src/main/logseq/cli/command/sync.cljs +++ b/src/main/logseq/cli/command/sync.cljs @@ -1,8 +1,10 @@ (ns logseq.cli.command.sync "Sync-related CLI commands." (:require [clojure.string :as string] + [lambdaisland.glogi :as log] [logseq.cli.auth :as cli-auth] [logseq.cli.command.core :as core] + [logseq.cli.common :as cli-common] [logseq.cli.config :as cli-config] [logseq.cli.output-mode :as output-mode] [logseq.cli.server :as cli-server] @@ -602,48 +604,94 @@ (= graph-id (:graph-uuid payload))) (:message payload))) +(defn- cleanup-error-details + [error] + (let [data (ex-data error)] + (cond-> {:code (or (:code data) :exception) + :message (or (ex-message error) (str error))} + (seq data) (assoc :context data)))) + +(defn- (p/let [stop-result (cli-server/stop-server! config repo) + _ (when-not (or (:ok? stop-result) + (= :server-not-found (get-in stop-result [:error :code]))) + (throw (ex-info (get-in stop-result [:error :message] "failed to stop server") + {:code (get-in stop-result [:error :code]) + :repo repo + :stage :stop-server + :stop-result stop-result}))) + graphs-after-stop (cli-server/list-graphs config) + graph-exists? (some #(= (core/repo->graph repo) %) graphs-after-stop) + unlinked-dir (when graph-exists? + (cli-common/unlink-graph! (cli-server/graphs-dir config) repo)) + _ (when (and graph-exists? (not unlinked-dir)) + (throw (ex-info "unable to remove graph" + {:code :graph-not-removed + :repo repo + :stage :unlink-graph})))] + {:status :ok + :data {:repo repo + :unlinked-dir unlinked-dir}}) + (p/catch (fn [error] + (log/warn :cli-sync-download-cleanup-failed + {:repo repo + :error (cleanup-error-details error)}) + {:status :error + :error (cleanup-error-details error)})))) + (defn- execute-sync-download [action config] (let [config' (download-config config) progress-enabled? (sync-download-progress-enabled? action config')] - (-> (p/let [remote-graphs (invoke-global config' - :thread-api/db-sync-list-remote-graphs - []) - remote-graph (some (fn [graph] - (when (= (:graph action) (:graph-name graph)) - graph)) - remote-graphs)] - (if-not remote-graph - {:status :error - :error {:code :remote-graph-not-found - :message (str "remote graph not found: " (:graph action)) - :graph (:graph action)}} - (p/let [cfg (cli-server/ensure-server! config' (:repo action)) - _ ( (transport/invoke download-cfg :thread-api/db-sync-download-graph-by-id false - [(:repo action) graph-id (:graph-e2ee? remote-graph)]) - (p/finally (fn [] - (when-let [close! (:close! events-sub)] - (close!)))))] - {:status :ok - :data (if (map? result) - result - {:result result})}))) - (p/catch (fn [error] - (if (= :e2ee-password-not-found (:code (ex-data error))) - (e2ee-password-not-found-error :sync-download (:repo action)) - (exception->error error {:repo (:repo action) - :graph (:graph action)}))))))) + (p/let [local-graphs-before (cli-server/list-graphs config') + graph-existed-before? (some #(= (:graph action) %) local-graphs-before)] + (-> (p/let [remote-graphs (invoke-global config' + :thread-api/db-sync-list-remote-graphs + []) + remote-graph (some (fn [graph] + (when (= (:graph action) (:graph-name graph)) + graph)) + remote-graphs)] + (if-not remote-graph + {:status :error + :error {:code :remote-graph-not-found + :message (str "remote graph not found: " (:graph action)) + :graph (:graph action)}} + (p/let [cfg (cli-server/ensure-server! config' (:repo action)) + _ ( (transport/invoke download-cfg :thread-api/db-sync-download-graph-by-id false + [(:repo action) graph-id (:graph-e2ee? remote-graph)]) + (p/finally (fn [] + (when-let [close! (:close! events-sub)] + (close!)))))] + {:status :ok + :data (if (map? result) + result + {:result result})}))) + (p/then (fn [result] + (if (and (not graph-existed-before?) + (= :error (:status result))) + (p/let [_ (error error {:repo (:repo action) + :graph (:graph action)}))))))))) (defn- run-sync-status [action config] diff --git a/src/main/logseq/cli/common.cljs b/src/main/logseq/cli/common.cljs index a7cd739ad8..34028c6da9 100644 --- a/src/main/logseq/cli/common.cljs +++ b/src/main/logseq/cli/common.cljs @@ -9,17 +9,19 @@ (defn unlink-graph! "Unlinks the given repo by moving it to the 'Unlinked graphs' dir. Returns path of unlinked dir if move is successful or nil if not" - [repo] - (let [graph-dir-name (graph-dir/repo->encoded-graph-dir-name repo) - graphs-dir (common-graph/expand-home (common-graph/get-default-graphs-dir)) - path (node-path/join graphs-dir graph-dir-name) - unlinked (node-path/join graphs-dir common-config/unlinked-graphs-dir) - new-path (node-path/join unlinked graph-dir-name) - new-path-exists? (fs/existsSync new-path) - new-path' (if new-path-exists? - (node-path/join unlinked (str graph-dir-name "-" (random-uuid))) - new-path)] - (when (fs/existsSync path) - (fs/ensureDirSync unlinked) - (fs/moveSync path new-path') - new-path'))) + ([repo] + (unlink-graph! (common-graph/expand-home (common-graph/get-default-graphs-dir)) repo)) + ([graphs-dir repo] + (let [graph-dir-name (graph-dir/repo->encoded-graph-dir-name repo) + graphs-dir (common-graph/expand-home graphs-dir) + path (node-path/join graphs-dir graph-dir-name) + unlinked (node-path/join graphs-dir common-config/unlinked-graphs-dir) + new-path (node-path/join unlinked graph-dir-name) + new-path-exists? (fs/existsSync new-path) + new-path' (if new-path-exists? + (node-path/join unlinked (str graph-dir-name "-" (random-uuid))) + new-path)] + (when (fs/existsSync path) + (fs/ensureDirSync unlinked) + (fs/moveSync path new-path') + new-path')))) diff --git a/src/test/logseq/cli/command/sync_test.cljs b/src/test/logseq/cli/command/sync_test.cljs index 27fad96035..ead623fc16 100644 --- a/src/test/logseq/cli/command/sync_test.cljs +++ b/src/test/logseq/cli/command/sync_test.cljs @@ -2,6 +2,7 @@ (:require [cljs.test :refer [async deftest is testing]] [logseq.cli.auth :as cli-auth] [logseq.cli.command.sync :as sync-command] + [logseq.cli.common :as cli-common] [logseq.cli.config :as cli-config] [logseq.cli.server :as cli-server] [logseq.cli.transport :as transport] @@ -347,10 +348,24 @@ (deftest test-execute-sync-download-missing-e2ee-password-for-e2ee-graph-is-error (async done (let [ensure-calls (atom []) - invoke-calls (atom [])] - (-> (p/with-redefs [cli-server/ensure-server! (fn [config repo] + invoke-calls (atom []) + stop-calls (atom []) + unlink-calls (atom []) + list-graphs-calls (atom 0)] + (-> (p/with-redefs [cli-server/list-graphs (fn [_config] + (let [idx (swap! list-graphs-calls inc)] + (if (= idx 1) + [] + ["demo"]))) + cli-server/ensure-server! (fn [config repo] (swap! ensure-calls conj [config repo]) (p/resolved (assoc config :base-url "http://example"))) + cli-server/stop-server! (fn [config repo] + (swap! stop-calls conj [config repo]) + (p/resolved {:ok? true})) + cli-common/unlink-graph! (fn [& args] + (swap! unlink-calls conj args) + "/tmp/unlinked-demo") transport/invoke (fn [_ method direct-pass? args] (swap! invoke-calls conj [method direct-pass? args]) (case method @@ -382,6 +397,8 @@ (is (= :error (:status result))) (is (= :e2ee-password-not-found (get-in result [:error :code]))) (is (= "logseq_db_demo" (get-in result [:error :repo]))) + (is (= ["logseq_db_demo"] (mapv second @stop-calls))) + (is (= ["logseq_db_demo"] (mapv last @unlink-calls))) (is (not-any? (fn [[method _ _]] (= :thread-api/db-sync-download-graph-by-id method)) @invoke-calls)))) @@ -389,6 +406,138 @@ (is false (str "unexpected error: " e)))) (p/finally done))))) +(deftest test-execute-sync-download-preserves-e2ee-error-when-cleanup-fails + (async done + (let [stop-calls (atom []) + unlink-calls (atom [])] + (-> (p/with-redefs [cli-server/list-graphs (fn [_config] + []) + cli-server/ensure-server! (fn [config _repo] + (p/resolved (assoc config :base-url "http://example"))) + cli-server/stop-server! (fn [config repo] + (swap! stop-calls conj [config repo]) + (p/rejected (ex-info "stop failed" + {:code :stop-failed}))) + cli-common/unlink-graph! (fn [& args] + (swap! unlink-calls conj args) + "/tmp/unlinked-demo") + transport/invoke (fn [_ method _direct-pass? _args] + (case method + :thread-api/db-sync-list-remote-graphs + (p/resolved [{:graph-id "remote-graph-id" + :graph-name "demo" + :graph-e2ee? true}]) + + :thread-api/get-e2ee-password + (p/rejected (ex-info "missing-e2ee-password" + {:code :db-sync/missing-e2ee-password})) + + :thread-api/db-sync-download-graph-by-id + (p/resolved {:ok true}) + + (p/resolved nil)))] + (p/let [result (sync-command/execute {:type :sync-download + :repo "logseq_db_demo" + :graph "demo"} + {:base-url "http://example" + :root-dir "/tmp" + :http-base "https://api.logseq.io" + :id-token "runtime-token" + :refresh-token "refresh-token"})] + (is (= :error (:status result))) + (is (= :e2ee-password-not-found (get-in result [:error :code]))) + (is (= ["logseq_db_demo"] (mapv second @stop-calls))) + (is (= [] @unlink-calls)))) + (p/catch (fn [e] + (is false (str "unexpected error: " e)))) + (p/finally done))))) + +(deftest test-execute-sync-download-uses-persisted-e2ee-password-when-option-missing + (async done + (let [invoke-calls (atom [])] + (-> (p/with-redefs [cli-server/list-graphs (fn [_config] + []) + cli-server/ensure-server! (fn [config _repo] + (p/resolved (assoc config :base-url "http://example"))) + transport/invoke (fn [_ method direct-pass? args] + (swap! invoke-calls conj [method direct-pass? args]) + (case method + :thread-api/db-sync-list-remote-graphs + (p/resolved [{:graph-id "remote-graph-id" + :graph-name "demo" + :graph-e2ee? true}]) + :thread-api/get-e2ee-password + (p/resolved "persisted-password") + :thread-api/q + (p/resolved 0) + :thread-api/db-sync-download-graph-by-id + (p/resolved {:graph-id "remote-graph-id" + :remote-tx 22}) + (p/resolved nil)))] + (p/let [result (sync-command/execute {:type :sync-download + :repo "logseq_db_demo" + :graph "demo"} + {:base-url "http://example" + :root-dir "/tmp" + :http-base "https://api.logseq.io" + :id-token "runtime-token" + :refresh-token "refresh-token"})] + (is (= :ok (:status result))) + (is (= "remote-graph-id" (get-in result [:data :graph-id]))) + (is (some #(= [:thread-api/get-e2ee-password false ["refresh-token"]] + %) + @invoke-calls)) + (is (not-any? #(= :thread-api/verify-and-save-e2ee-password (first %)) + @invoke-calls)) + (is (some #(= [:thread-api/db-sync-download-graph-by-id false ["logseq_db_demo" "remote-graph-id" true]] + %) + @invoke-calls)))) + (p/catch (fn [e] + (is false (str "unexpected error: " e)))) + (p/finally done))))) + +(deftest test-execute-sync-download-skips-cleanup-for-preexisting-graph + (async done + (let [stop-calls (atom []) + unlink-calls (atom [])] + (-> (p/with-redefs [cli-server/list-graphs (fn [_config] + ["demo"]) + cli-server/ensure-server! (fn [config _repo] + (p/resolved (assoc config :base-url "http://example"))) + cli-server/stop-server! (fn [config repo] + (swap! stop-calls conj [config repo]) + (p/resolved {:ok? true})) + cli-common/unlink-graph! (fn [& args] + (swap! unlink-calls conj args) + "/tmp/unlinked-demo") + transport/invoke (fn [_ method _direct-pass? _args] + (case method + :thread-api/db-sync-list-remote-graphs + (p/resolved [{:graph-id "remote-graph-id" + :graph-name "demo" + :graph-e2ee? true}]) + + :thread-api/get-e2ee-password + (p/rejected (ex-info "missing-e2ee-password" + {:code :db-sync/missing-e2ee-password})) + + (p/resolved nil)))] + (p/let [result (sync-command/execute {:type :sync-download + :repo "logseq_db_demo" + :graph "demo"} + {:base-url "http://example" + :root-dir "/tmp" + :http-base "https://api.logseq.io" + :id-token "runtime-token" + :refresh-token "refresh-token"})] + (is (= :error (:status result))) + (is (= :e2ee-password-not-found (get-in result [:error :code]))) + (is (= [] @stop-calls)) + (is (= [] @unlink-calls)))) + (p/catch (fn [e] + (is false (str "unexpected error: " e)))) + (p/finally done))))) + (deftest test-execute-sync-start-runtime-error-after-open (async done (let [status-calls (atom 0)] diff --git a/src/test/logseq/cli/integration_test.cljs b/src/test/logseq/cli/integration_test.cljs index cdab23e85e..dfe42f88b0 100644 --- a/src/test/logseq/cli/integration_test.cljs +++ b/src/test/logseq/cli/integration_test.cljs @@ -269,6 +269,80 @@ (is false (str "unexpected error: " e)) (done))))))) +(deftest ^:long test-cli-sync-download-missing-e2ee-password-cleans-created-graph-and-allows-retry + (async done + (let [root-dir (node-helper/create-tmp-dir "db-worker-sync-download-cleanup-cli") + download-graph "sync-download-cleanup-graph" + download-calls (atom 0) + invoke-calls (atom [])] + (-> (p/let [cfg-path (node-path/join (node-helper/create-tmp-dir "cli") "cli.edn") + _ (fs/writeFileSync cfg-path "{:output-format :json}") + _ (p/with-redefs [cli-auth/resolve-auth! (fn [_config] + (p/resolved {:id-token "runtime-token" + :refresh-token "refresh-token"})) + transport/invoke (fn [_ method direct-pass? args] + (swap! invoke-calls conj [method direct-pass? args]) + (case method + :thread-api/set-db-sync-config + (p/resolved nil) + + :thread-api/db-sync-list-remote-graphs + (p/resolved [{:graph-id "remote-graph-id" + :graph-name download-graph + :graph-e2ee? true}]) + + :thread-api/get-e2ee-password + (p/rejected (ex-info "missing-e2ee-password" + {:code :db-sync/missing-e2ee-password + :field :e2ee-password + :reason :missing-persisted-password})) + + :thread-api/verify-and-save-e2ee-password + (p/resolved nil) + + :thread-api/q + (p/resolved 0) + + :thread-api/db-sync-download-graph-by-id + (do + (swap! download-calls inc) + (p/resolved {:repo download-graph + :graph-id "remote-graph-id" + :remote-tx 22 + :graph-e2ee? true})) + + (p/resolved nil)))] + (p/let [failed-result (run-cli ["--graph" download-graph "sync" "download"] root-dir cfg-path) + failed-payload (parse-json-output-safe failed-result "failed encrypted sync download") + list-after-fail (run-cli ["graph" "list"] root-dir cfg-path) + list-payload (parse-json-output-safe list-after-fail "graph list after failed sync download") + servers-after-fail (run-cli ["server" "list"] root-dir cfg-path) + servers-payload (parse-json-output-safe servers-after-fail "server list after failed sync download") + _ (is (= 1 (:exit-code failed-result))) + _ (is (= "error" (:status failed-payload))) + _ (is (= "e2ee-password-not-found" (get-in failed-payload [:error :code]))) + _ (is (not (some #(= download-graph %) (get-in list-payload [:data :graphs])))) + _ (is (not (some #(= download-graph (:repo %)) (get-in servers-payload [:data :servers])))) + _ (is (= 0 @download-calls)) + retry-result (run-cli ["--graph" download-graph "sync" "download" "--e2ee-password" "pw"] root-dir cfg-path) + retry-payload (parse-json-output-safe retry-result "retry encrypted sync download") + _ (is (= 0 (:exit-code retry-result))) + _ (is (= "ok" (:status retry-payload)) + (pr-str retry-payload)) + _ (is (= "remote-graph-id" (get-in retry-payload [:data :graph-id]))) + _ (is (= 1 @download-calls))] + nil)) + _ (stop-repo! root-dir cfg-path download-graph)] + nil) + (p/catch (fn [e] + (is false (str "unexpected error: " e)))) + (p/finally (fn [] + (try + (fs/rmSync root-dir #js {:recursive true :force true}) + (catch :default _ + nil)) + (done))))))) + (deftest ^:long test-cli-sync-upload-with-mocked-worker-bootstrap (async done (let [root-dir (node-helper/create-tmp-dir "db-worker-sync-upload-cli") @@ -295,7 +369,7 @@ (p/resolved {:graph-id "created-graph-id"}) (p/resolved nil)))] - (run-cli ["--graph" upload-repo "sync" "upload"] root-dir cfg-path)) + (run-cli ["--graph" upload-repo "sync" "upload"] root-dir cfg-path)) upload-payload (parse-json-output-safe upload-result "sync upload")] (is (= 0 (:exit-code upload-result))) (is (= "ok" (:status upload-payload)))