diff --git a/bb.edn b/bb.edn index 2c15f12349..b664ae06a5 100644 --- a/bb.edn +++ b/bb.edn @@ -159,6 +159,9 @@ dev:test logseq.tasks.dev/test + dev:test-no-worker + logseq.tasks.dev/test-no-worker + dev:lint-and-test logseq.tasks.dev/lint-and-test diff --git a/deps/db-sync/src/logseq/db_sync/cycle.cljs b/deps/db-sync/src/logseq/db_sync/cycle.cljs index c3e8ed9726..1912a23c1f 100644 --- a/deps/db-sync/src/logseq/db_sync/cycle.cljs +++ b/deps/db-sync/src/logseq/db_sync/cycle.cljs @@ -46,6 +46,19 @@ [db _e _attr _bad-v] (d/entid db :logseq.class/Root)) +(defn- ancestor? + "Return true if `ancestor-eid` appears in the parent chain of `start-eid`." + [db ancestor-eid start-eid] + (loop [current start-eid + seen #{}] + (cond + (nil? current) false + (= current ancestor-eid) true + (contains? seen current) false + :else + (let [parent (some-> (d/entity db current) :block/parent :db/id)] + (recur parent (conj seen current)))))) + (def ^:private default-attr-opts {;; Cardinality-one :block/parent @@ -180,16 +193,41 @@ Returns a tx vector (possibly with an add), or nil." [db cycle attr {:keys [safe-target-fn skip?] :as attr-opts} touched] (let [edges (cycle-edges cycle) - victim (pick-victim cycle touched) - [_from bad-v] (some (fn [[from _to :as e]] - (when (= from victim) e)) - edges) + cycle-nodes (set (distinct (butlast cycle))) + remote-parent-fn (:remote-parent-fn attr-opts) + remote-candidates (when (and (= :block/parent attr) remote-parent-fn) + (keep (fn [[from to]] + (let [remote-parent (remote-parent-fn db from to) + remote-parent (entid db remote-parent) + to-id (entid db to)] + (when (and remote-parent + (not= remote-parent to-id)) + {:victim from + :bad-v to-id + :safe remote-parent + :outside? (not (contains? cycle-nodes remote-parent))}))) + edges)) + remote-choice (or (some #(when (:outside? %) %) remote-candidates) + (first remote-candidates)) + victim (or (:victim remote-choice) (pick-victim cycle touched)) + [_from bad-v] (or (when (and remote-choice (:bad-v remote-choice)) + [victim (:bad-v remote-choice)]) + (some (fn [[from _to :as e]] + (when (= from victim) e)) + edges)) bad-v (entid db bad-v)] (when (and victim bad-v) (when-not (and skip? (skip? db victim attr bad-v)) ;; Ensure the edge still exists in current db. (when (contains? (ref-eids db victim attr attr-opts) bad-v) - (let [safe (when safe-target-fn (safe-target-fn db victim attr bad-v))] + (let [safe (or (:safe remote-choice) + (when safe-target-fn (safe-target-fn db victim attr bad-v))) + safe (if (and (= :block/parent attr) + (number? safe) + (nil? remote-choice) + (ancestor? db victim safe)) + (safe-target-for-block-parent db victim attr bad-v) + safe)] (prn :debug :victim victim :page-id (:db/id (:block/page (d/entity db victim))) @@ -215,6 +253,11 @@ ;; Iterative repair (THIS is the key fix) ;; ----------------------------------------------------------------------------- +(def ^:private fix-cycle-tx-meta + {:outliner-op :fix-cycle + :gen-undo-ops? false + :persist-op? false}) + (defn- apply-cycle-repairs! "Detect & break cycles AFTER rebase, iterating until stable. @@ -267,8 +310,7 @@ (if (seq tx) (do - (transact! temp-conn tx (merge tx-meta - {:outliner-op :fix-cycle :gen-undo-ops? false})) + (transact! temp-conn tx (merge tx-meta fix-cycle-tx-meta)) (recur (inc it) seen-edges')) ;; No more cycles detected from these candidates => done nil)))))) @@ -303,9 +345,21 @@ - Iteratively breaks cycles until stable." [temp-conn remote-tx-report rebase-tx-report & {:keys [transact! tx-meta] :or {transact! ldb/transact!}}] - (let [remote-touched-by-attr (touched-eids-many (:tx-data remote-tx-report)) + (let [remote-db (:db-after remote-tx-report) + attr-opts (cond-> default-attr-opts + remote-db + (assoc-in [:block/parent :remote-parent-fn] + (fn [_db e _bad-v] + (some-> (d/entity remote-db e) :block/parent :db/id))) + remote-db + (assoc-in [:block/parent :safe-target-fn] + (fn [db e attr bad-v] + (let [remote-parent (some-> (d/entity remote-db e) :block/parent :db/id) + remote-parent (when (and remote-parent (not= remote-parent bad-v)) remote-parent)] + (or remote-parent (safe-target-for-block-parent db e attr bad-v)))))) + remote-touched-by-attr (touched-eids-many (:tx-data remote-tx-report)) local-touched-by-attr (touched-eids-many (:tx-data rebase-tx-report)) candidates-by-attr (union-candidates remote-touched-by-attr local-touched-by-attr) touched-info (touched-info-by-attr remote-touched-by-attr local-touched-by-attr)] (when (seq candidates-by-attr) - (apply-cycle-repairs! transact! temp-conn candidates-by-attr touched-info default-attr-opts tx-meta)))) + (apply-cycle-repairs! transact! temp-conn candidates-by-attr touched-info attr-opts tx-meta)))) diff --git a/deps/db-sync/test/logseq/db_sync/cycle_test.cljs b/deps/db-sync/test/logseq/db_sync/cycle_test.cljs index 5dcdefd632..c866c474bf 100644 --- a/deps/db-sync/test/logseq/db_sync/cycle_test.cljs +++ b/deps/db-sync/test/logseq/db_sync/cycle_test.cljs @@ -9,7 +9,12 @@ (defn- fix-cycle! [temp-conn remote-tx-report rebase-tx-report] - (cycle/fix-cycle! temp-conn remote-tx-report rebase-tx-report {:transact! d/transact!})) + (let [tx-metas (atom [])] + (cycle/fix-cycle! temp-conn remote-tx-report rebase-tx-report + {:transact! (fn [conn tx-data tx-meta] + (swap! tx-metas conj tx-meta) + (d/transact! conn tx-data tx-meta))}) + @tx-metas)) (defn- create-page! [conn title] @@ -35,12 +40,15 @@ :block/page [:block/uuid page-uuid] :block/parent [:block/uuid a]}]) (testing "breaks a 2-node :block/parent cycle and reparents to the page" - (let [remote-report (d/transact! conn [{:block/uuid a :block/parent [:block/uuid b]}])] - (fix-cycle! conn remote-report nil) - (let [a' (d/entity @conn [:block/uuid a]) - b' (d/entity @conn [:block/uuid b])] - (is (= (block-eid @conn page-uuid) (:db/id (:block/parent a')))) - (is (= (block-eid @conn a) (:db/id (:block/parent b'))))))))) + (let [remote-report (d/transact! conn [{:block/uuid a :block/parent [:block/uuid b]}]) + tx-metas (fix-cycle! conn remote-report nil) + a' (d/entity @conn [:block/uuid a]) + b' (d/entity @conn [:block/uuid b])] + (is (= (block-eid @conn page-uuid) (:db/id (:block/parent a')))) + (is (= (block-eid @conn a) (:db/id (:block/parent b')))) + (is (some #(= :fix-cycle (:outliner-op %)) tx-metas)) + (is (every? #(false? (:gen-undo-ops? %)) tx-metas)) + (is (every? #(false? (:persist-op? %)) tx-metas)))))) (deftest block-parent-3-node-cycle-test (let [conn (new-conn) @@ -58,14 +66,108 @@ :block/page [:block/uuid page-uuid] :block/parent [:block/uuid b]}]) (testing "breaks a 3-node :block/parent cycle and reparents to the page" - (let [remote-report (d/transact! conn [{:block/uuid a :block/parent [:block/uuid c]}])] - (fix-cycle! conn remote-report nil) + (let [remote-report (d/transact! conn [{:block/uuid a :block/parent [:block/uuid c]}]) + tx-metas (fix-cycle! conn remote-report nil)] (let [a' (d/entity @conn [:block/uuid a]) b' (d/entity @conn [:block/uuid b]) c' (d/entity @conn [:block/uuid c])] (is (= (block-eid @conn page-uuid) (:db/id (:block/parent a')))) (is (= (block-eid @conn a) (:db/id (:block/parent b')))) - (is (= (block-eid @conn b) (:db/id (:block/parent c'))))))))) + (is (= (block-eid @conn b) (:db/id (:block/parent c'))))) + (is (some #(= :fix-cycle (:outliner-op %)) tx-metas)) + (is (every? #(false? (:gen-undo-ops? %)) tx-metas)) + (is (every? #(false? (:persist-op? %)) tx-metas)))))) + +(deftest block-parent-cycle-prefers-remote-parent-test + (let [conn (new-conn) + page-uuid (create-page! conn "page") + a (random-uuid) + b (random-uuid) + c (random-uuid)] + (d/transact! conn [{:block/uuid a + :block/page [:block/uuid page-uuid] + :block/parent [:block/uuid page-uuid]} + {:block/uuid b + :block/page [:block/uuid page-uuid] + :block/parent [:block/uuid page-uuid]} + {:block/uuid c + :block/page [:block/uuid page-uuid] + :block/parent [:block/uuid page-uuid]}]) + (testing "prefers remote parent when breaking local cycle" + (let [remote-report (d/transact! conn [{:block/uuid b :block/parent [:block/uuid c]} + {:block/uuid a :block/parent [:block/uuid b]}]) + rebase-report (d/transact! conn [{:block/uuid b :block/parent [:block/uuid a]} + {:block/uuid c :block/parent [:block/uuid b]}]) + tx-metas (fix-cycle! conn remote-report rebase-report) + a' (d/entity @conn [:block/uuid a]) + b' (d/entity @conn [:block/uuid b]) + c' (d/entity @conn [:block/uuid c])] + (is (= (block-eid @conn b) (:db/id (:block/parent a')))) + (is (= (block-eid @conn c) (:db/id (:block/parent b')))) + (is (= (block-eid @conn page-uuid) (:db/id (:block/parent c')))) + (is (some #(= :fix-cycle (:outliner-op %)) tx-metas)) + (is (every? #(false? (:gen-undo-ops? %)) tx-metas)) + (is (every? #(false? (:persist-op? %)) tx-metas)))))) + +(deftest block-parent-cycle-avoids-descendant-remote-parent-test + (let [conn (new-conn) + page-uuid (create-page! conn "page") + a (random-uuid) + b (random-uuid) + c (random-uuid)] + (d/transact! conn [{:block/uuid a + :block/page [:block/uuid page-uuid] + :block/parent [:block/uuid page-uuid]} + {:block/uuid b + :block/page [:block/uuid page-uuid] + :block/parent [:block/uuid a]} + {:block/uuid c + :block/page [:block/uuid page-uuid] + :block/parent [:block/uuid b]}]) + (testing "falls back to page when remote parent is a descendant" + (let [remote-report (d/transact! conn [{:block/uuid b :block/parent [:block/uuid c]}]) + rebase-report (d/transact! conn [{:block/uuid a :block/parent [:block/uuid b]}]) + tx-metas (fix-cycle! conn remote-report rebase-report) + a' (d/entity @conn [:block/uuid a]) + b' (d/entity @conn [:block/uuid b]) + c' (d/entity @conn [:block/uuid c])] + (is (= (block-eid @conn page-uuid) (:db/id (:block/parent b')))) + (is (= (block-eid @conn b) (:db/id (:block/parent a')))) + (is (= (block-eid @conn b) (:db/id (:block/parent c')))) + (is (some #(= :fix-cycle (:outliner-op %)) tx-metas)) + (is (every? #(false? (:gen-undo-ops? %)) tx-metas)) + (is (every? #(false? (:persist-op? %)) tx-metas)))))) + +(deftest block-parent-cycle-preserves-safe-remote-edges-test + (let [conn (new-conn) + page-uuid (create-page! conn "page") + a (random-uuid) + b (random-uuid) + c (random-uuid)] + (d/transact! conn [{:block/uuid a + :block/page [:block/uuid page-uuid] + :block/parent [:block/uuid page-uuid]} + {:block/uuid b + :block/page [:block/uuid page-uuid] + :block/parent [:block/uuid a]} + {:block/uuid c + :block/page [:block/uuid page-uuid] + :block/parent [:block/uuid b]}]) + (testing "preserves remote parent when it doesn't reintroduce a cycle" + (let [remote-report (d/transact! conn [{:block/uuid b :block/parent [:block/uuid c]} + {:block/uuid a :block/parent [:block/uuid b]}]) + rebase-report (d/transact! conn [{:block/uuid b :block/parent [:block/uuid a]} + {:block/uuid c :block/parent [:block/uuid b]}]) + tx-metas (fix-cycle! conn remote-report rebase-report) + a' (d/entity @conn [:block/uuid a]) + b' (d/entity @conn [:block/uuid b]) + c' (d/entity @conn [:block/uuid c])] + (is (= (block-eid @conn b) (:db/id (:block/parent a')))) + (is (= (block-eid @conn page-uuid) (:db/id (:block/parent b')))) + (is (= (block-eid @conn b) (:db/id (:block/parent c')))) + (is (some #(= :fix-cycle (:outliner-op %)) tx-metas)) + (is (every? #(false? (:gen-undo-ops? %)) tx-metas)) + (is (every? #(false? (:persist-op? %)) tx-metas)))))) (deftest class-extends-2-node-cycle-test (let [conn (new-conn)] @@ -74,13 +176,16 @@ {:db/ident :user.class/A :logseq.property.class/extends :user.class/B}]) (testing "breaks a 2-node :logseq.property.class/extends cycle" (let [remote-report (d/transact! conn [{:db/ident :user.class/B - :logseq.property.class/extends :user.class/A}])] - (fix-cycle! conn remote-report nil) + :logseq.property.class/extends :user.class/A}]) + tx-metas (fix-cycle! conn remote-report nil)] (let [b (d/entity @conn :user.class/B) _ (prn :debug :extends (:logseq.property.class/extends b)) extends (set (map :db/ident (:logseq.property.class/extends b)))] (is (not (contains? extends :user.class/A))) - (is (contains? extends :logseq.class/Root))))))) + (is (contains? extends :logseq.class/Root))) + (is (some #(= :fix-cycle (:outliner-op %)) tx-metas)) + (is (every? #(false? (:gen-undo-ops? %)) tx-metas)) + (is (every? #(false? (:persist-op? %)) tx-metas)))))) (deftest class-extends-3-node-cycle-with-multiple-values-test (let [conn (new-conn)] @@ -91,10 +196,13 @@ {:db/ident :user.class/A :logseq.property.class/extends :user.class/B}]) (testing "breaks a 3-node :logseq.property.class/extends cycle while preserving other extends" (let [remote-report (d/transact! conn [{:db/ident :user.class/C - :logseq.property.class/extends #{:user.class/A :user.class/D}}])] - (fix-cycle! conn remote-report nil) + :logseq.property.class/extends #{:user.class/A :user.class/D}}]) + tx-metas (fix-cycle! conn remote-report nil)] (let [c (d/entity @conn :user.class/C) extends (set (map :db/ident (:logseq.property.class/extends c)))] (is (not (contains? extends :user.class/A))) (is (contains? extends :user.class/D)) - (is (contains? extends :logseq.class/Root))))))) + (is (contains? extends :logseq.class/Root))) + (is (some #(= :fix-cycle (:outliner-op %)) tx-metas)) + (is (every? #(false? (:gen-undo-ops? %)) tx-metas)) + (is (every? #(false? (:persist-op? %)) tx-metas)))))) diff --git a/deps/db-sync/test/logseq/db_sync/snapshot_import_test.cljs b/deps/db-sync/test/logseq/db_sync/snapshot_import_test.cljs deleted file mode 100644 index 1cd460d3c4..0000000000 --- a/deps/db-sync/test/logseq/db_sync/snapshot_import_test.cljs +++ /dev/null @@ -1,40 +0,0 @@ -(ns logseq.db-sync.snapshot-import-test - (:require [cljs.test :refer [deftest is async]] - [clojure.string :as string] - [logseq.db-sync.snapshot :as snapshot] - [logseq.db-sync.worker :as worker] - [promesa.core :as p])) - -(defn- make-sql [state] - #js {:exec (fn [sql & _args] - (swap! state update :executed conj sql) - nil)}) - -(defn- make-stream [chunk] - (js/ReadableStream. - #js {:start (fn [controller] - (.enqueue controller chunk) - (.close controller))})) - -(deftest snapshot-import-failure-does-not-touch-kvs-test - (async done - (let [state (atom {:executed []}) - sql (make-sql state) - self (doto (js-obj) - (aset "sql" sql))] - (-> (with-redefs [snapshot/parse-framed-chunk (fn [_ _] - (throw (ex-info "boom" {})))] - (-> (p/then (#'worker/import-snapshot-stream! - self - (make-stream (js/Uint8Array. #js [1 2 3])) - true) - (fn [_] - (is false "expected import to fail") - nil)) - (p/catch (fn [_] - (let [sqls (:executed @state)] - (is (some #(string/includes? % "drop table if exists kvs_import") sqls)) - (is (not-any? #(string/includes? % "insert into kvs ") sqls)) - (is (not-any? #(string/includes? % "delete from kvs") sqls))) - nil)))) - (p/finally (fn [] (done))))))) diff --git a/deps/db-sync/test/logseq/db_sync/worker_members_test.cljs b/deps/db-sync/test/logseq/db_sync/worker_members_test.cljs deleted file mode 100644 index 7a1c98b2b4..0000000000 --- a/deps/db-sync/test/logseq/db_sync/worker_members_test.cljs +++ /dev/null @@ -1,227 +0,0 @@ -(ns logseq.db-sync.worker-members-test - (:require [cljs.test :refer [deftest is async]] - [clojure.string :as string] - [logseq.db-sync.index :as index] - [logseq.db-sync.worker :as worker] - [promesa.core :as p])) - -(defn- js-key [k] - (cond - (keyword? k) (string/replace (name k) "-" "_") - (string? k) k - :else (str k))) - -(defn- js-row [m] - (let [o (js-obj)] - (doseq [[k v] m] - (aset o (js-key k) v)) - o)) - -(defn- js-rows [rows] - (into-array (map js-row rows))) - -(defn- record-exec! [state sql] - (swap! state update :executed conj sql)) - -(defn- run-sql! [state sql args] - (record-exec! state sql) - (cond - (string/includes? sql "insert into graph_members") - (let [[user-id graph-id role invited-by created-at] args] - (swap! state update :graph-members - (fn [members] - (let [k [user-id graph-id] - existing (get members k) - created-at (or (:created-at existing) created-at)] - (assoc members k {:user-id user-id - :graph-id graph-id - :role role - :invited-by invited-by - :created-at created-at}))))) - - (string/includes? sql "insert into graphs") - (let [[graph-id graph-name user-id schema-version created-at updated-at] args] - (swap! state update :graphs assoc graph-id {:graph-id graph-id - :graph-name graph-name - :user-id user-id - :schema-version schema-version - :created-at created-at - :updated-at updated-at})) - - (string/includes? sql "delete from graph_members") - (let [[graph-id user-id] args] - (swap! state update :graph-members dissoc [user-id graph-id])) - - :else - nil)) - -(defn- union-access-rows [state sql args] - (let [[graph-id user-id] args - graph-owner-id (get-in @state [:graphs graph-id :user-id]) - member (get-in @state [:graph-members [user-id graph-id]]) - manager-required? (string/includes? sql "role = 'manager'") - has-access? (or (= graph-owner-id user-id) - (and member - (or (not manager-required?) - (= "manager" (:role member)))))] - (if has-access? - (js-rows [{:graph-id graph-id}]) - (js-rows [])))) - -(defn- all-sql [state sql args] - (record-exec! state sql) - (cond - (string/includes? sql "select role from graph_members") - (let [[graph-id user-id] args - role (get-in @state [:graph-members [user-id graph-id] :role])] - (if role - (js-rows [{:role role}]) - (js-rows []))) - - (string/includes? sql "union select graph_id from graph_members") - (union-access-rows state sql args) - - :else - (js-rows []))) - -(defn- make-d1 [state] - #js {:prepare (fn [sql] - (let [stmt #js {}] - (set! (.-_sql stmt) sql) - (set! (.-_args stmt) []) - (set! (.-bind stmt) - (fn [& args] - (set! (.-_args stmt) (vec args)) - stmt)) - (set! (.-run stmt) - (fn [] - (run-sql! state (.-_sql stmt) (.-_args stmt)) - #js {})) - (set! (.-all stmt) - (fn [] - #js {:results (all-sql state (.-_sql stmt) (.-_args stmt))})) - stmt))}) - -(defn- request-delete [graph-id member-id] - (js/Request. (str "http://localhost/graphs/" graph-id "/members/" member-id) - #js {:method "DELETE"})) - -(defn- response-status [response] - (.-status response)) - -(defn- setup-graph! [db graph-id owner-id] - (index/ (p/do! - (setup-graph! db graph-id manager-id) - (index/ (p/do! - (setup-graph! db graph-id manager-id) - (index/ (p/do! - (setup-graph! db graph-id manager-id) - (index/ (p/do! - (setup-graph! db graph-id manager-id) - (index/ (.text resp) - (.then (fn [text] - (is (= "ok" text)) - (is (some? @called)) - (done))) - (.catch (fn [e] - (is false (str e)) - (done))))))))) diff --git a/package.json b/package.json index 0ee4089e00..535be92226 100644 --- a/package.json +++ b/package.json @@ -94,6 +94,8 @@ "cljs:release-publishing": "clojure -M:cljs release app publishing", "cljs:test": "clojure -M:test compile test", "cljs:run-test": "node static/tests.js", + "cljs:test-no-worker": "clojure -M:test compile test-no-worker", + "cljs:run-test-no-worker": "node static/tests-no-worker.js", "cljs:dev-release-app": "clojure -M:cljs release app db-worker inference-worker --config-merge \"{:closure-defines {frontend.config/DEV-RELEASE true}}\"", "cljs:dev-release-electron": "clojure -M:cljs release app db-worker inference-worker electron --debug --config-merge \"{:closure-defines {frontend.config/DEV-RELEASE true}}\" && clojure -M:cljs release publishing", "cljs:debug": "clojure -M:cljs release app db-worker inference-worker --debug", diff --git a/scripts/src/logseq/tasks/dev.clj b/scripts/src/logseq/tasks/dev.clj index b8fa77226c..2dc44b4e51 100644 --- a/scripts/src/logseq/tasks/dev.clj +++ b/scripts/src/logseq/tasks/dev.clj @@ -20,6 +20,12 @@ (shell "yarn cljs:test") (apply shell "yarn cljs:run-test" args)) +(defn test-no-worker + "Run tests without compiling worker namespaces. Pass args through to cmd 'yarn cljs:run-test-no-worker'" + [& args] + (shell "yarn cljs:test-no-worker") + (apply shell "yarn cljs:run-test-no-worker" args)) + (defn lint-and-test "Run all lint tasks, then run tests(exclude testcases tagged by :long). pass args through to cmd 'yarn cljs:run-test'" diff --git a/shadow-cljs.edn b/shadow-cljs.edn index 01b619b24b..2363755950 100644 --- a/shadow-cljs.edn +++ b/shadow-cljs.edn @@ -182,6 +182,16 @@ :compiler-options {:static-fns false} :main frontend.test.frontend-node-test-runner/main} + :test-no-worker {:target :node-test + :output-to "static/tests-no-worker.js" + :closure-defines {frontend.util/NODETEST true + logseq.shui.util/NODETEST true} + :devtools {:enabled false} + :build-options {:ns-exclude-regexp "^(frontend\\.worker|logseq\\.db-sync\\.worker)"} + ;; disable :static-fns to allow for with-redefs and repl development + :compiler-options {:static-fns false} + :main frontend.test.frontend-node-test-runner/main} + :gen-malli-kondo-config {:target :node-script :closure-defines {frontend.util/NODETEST true} :devtools {:enabled false}