fix: offline parent cycle

deps/db-sync/src/logseq/db_sync/cycle.cljs

@@ -1,136 +1,226 @@
 (ns logseq.db-sync.cycle
-  (:require [datascript.core :as d]
-            [datascript.impl.entity :as de :refer [Entity]]))
+  "Generic cycle / bad-ref repair utilities for DataScript graphs.
 
-(def special-attrs
+  Goal:
+  - Support multiple ref attributes that can form chains/cycles, e.g.
+    :block/parent, :logseq.property.class/extends, etc.
+    * Cycle repair (after rebase): detect & break cycles, preferably breaking edges
+      introduced by local rebase (if available).
+
+  Notes:
+  - We assume attributes are single-valued refs (cardinality-one).
+  - We intentionally keep repairs as simple datoms (db/retract + db/add) to avoid
+    triggering complex outliner logic."
+
+  (:refer-clojure :exclude [cycle])
+  (:require
+   [datascript.core :as d]
+   [logseq.db :as ldb]))
+
+;; FIXME: `extends` cardinality-many
+
+;; -----------------------------------------------------------------------------
+;; Configure which ref attributes should be repaired, and how to find a safe target
+;; -----------------------------------------------------------------------------
+
+(def ^:private repair-attrs
+  "Ref attributes that can form chains/cycles and should be repaired client-side."
   #{:block/parent
     :logseq.property.class/extends})
 
-(defn- ref->eid [db ref]
-  (cond
-    (nil? ref) nil
-    (number? ref) (when (pos? ref) ref)
-    (vector? ref) (d/entid db ref)
-    (keyword? ref) (d/entid db [:db/ident ref])
-    :else nil))
+(defn- safe-target-for-block-parent
+  "Default safe target for :block/parent.
+   We attach to the page entity by default. If your tree requires a page-root BLOCK
+   instead of the page entity, replace this to return that block eid."
+  [db e _attr _bad-v]
+  (some-> (d/entity db e) :block/page :db/id))
 
-(defn- attr-updates-from-tx [tx-data attr]
+(defn- safe-target-for-class-extends
+  "Default safe target for :logseq.property.class/extends."
+  [_db _e _attr _bad-v]
+  :logseq.class/Root)
+
+(def ^:private default-attr-opts
+  {;; Keep blocks inside a sane container
+   :block/parent
+   {:safe-target-fn safe-target-for-block-parent}
+
+   ;; For class inheritance cycles, safest default is to retract the edge.
+   :logseq.property.class/extends
+   {:safe-target-fn safe-target-for-class-extends}})
+
+;; -----------------------------------------------------------------------------
+;; Basics
+;; -----------------------------------------------------------------------------
+
+(defn ref-eid
+  "Read a cardinality-one ref attribute as eid."
+  [db e attr]
+  (some-> (d/entity db e) (get attr) :db/id))
+
+(defn touched-eids
+  "Collect entity ids whose `attr` was added/changed (added=true) in tx-data."
+  [tx-data attr]
+  (->> tx-data
+       (keep (fn [[e a _v _t added]]
+               (when (and added (= a attr)) e)))
+       distinct))
+
+(defn touched-eids-many
+  "Collect touched entity ids for repair attrs.
+   Returns {attr #{eid ...}}"
+  [tx-data]
+  (reduce (fn [m attr]
+            (let [xs (touched-eids tx-data attr)]
+              (if (seq xs) (assoc m attr (set xs)) m)))
+          {}
+          repair-attrs))
+
+;; -----------------------------------------------------------------------------
+;; Cycle detection
+;; -----------------------------------------------------------------------------
+
+(defn reachable-cycle
+  "Detect a ref-cycle reachable by repeatedly following (e --attr--> v).
+
+  Returns a vector like [a b c a] or nil.
+  Only follows `attr` edges.
+
+  `skip?` can be used to ignore certain edges in traversal."
+  [db start-eid attr {:keys [skip?] :as _attr-opts}]
+  (let [visited  (volatile! #{})
+        stack    (volatile! [])
+        in-stack (volatile! #{})
+        cycle    (volatile! nil)]
+    (letfn [(next-eid [e]
+              (let [v (ref-eid db e attr)]
+                (when (and v (not (and skip? (skip? db e attr v))))
+                  v)))
+            (dfs! [e]
+              (when-not @cycle
+                (cond
+                  (contains? @in-stack e)
+                  (let [stk @stack
+                        idx (.indexOf stk e)]
+                    (when (>= idx 0)
+                      (vreset! cycle (conj (subvec stk idx) e))))
+
+                  (contains? @visited e)
+                  nil
+
+                  :else
+                  (do
+                    (vswap! visited conj e)
+                    (vswap! in-stack conj e)
+                    (vswap! stack conj e)
+                    (when-let [n (next-eid e)]
+                      (dfs! n))
+                    (vswap! stack pop)
+                    (vswap! in-stack disj e)))))]
+      (dfs! start-eid)
+      @cycle)))
+
+(defn- pick-victim
+  "Pick which node in the cycle to detach.
+
+  Inputs:
+  - cycle: [a b c a]
+  - local-touched?: (fn [eid] -> boolean) ; edge likely introduced by local rebase
+  - remote-touched?: (fn [eid] -> boolean) ; edge likely introduced by remote tx
+
+  Strategy:
+  1) Prefer nodes touched by local rebase
+  2) else nodes touched by remote
+  3) else first node"
+  [cycle local-touched? remote-touched?]
+  (let [nodes (vec (distinct (butlast cycle)))]
+    (or (some (fn [e] (when (local-touched? e) e)) nodes)
+        (some (fn [e] (when (remote-touched? e) e)) nodes)
+        (first nodes))))
+
+(defn break-cycle-tx
+  "Generate tx to break one cycle for one attr.
+
+  We detach victim by retracting its current (e attr v) and optionally add a safe
+  target from `safe-target-fn`. If safe-target-fn returns nil, we just retract.
+
+  touched-info:
+  - {:local-touched #{...} :remote-touched #{...}} ; per attr
+  "
+  [db cycle attr {:keys [safe-target-fn skip?] :as _attr-opts} {:keys [local-touched remote-touched]}]
+  (when (seq cycle)
+    (let [local-touched?  (fn [e] (contains? (or local-touched #{}) e))
+          remote-touched? (fn [e] (contains? (or remote-touched #{}) e))
+          victim          (pick-victim cycle local-touched? remote-touched?)]
+      (when victim
+        (let [bad-v (ref-eid db victim attr)]
+          (when (and bad-v (not (and skip? (skip? db victim attr bad-v))))
+            (let [safe (when safe-target-fn (safe-target-fn db victim attr bad-v))]
+              (cond
+                (and safe (not= safe bad-v))
+                [[:db/retract victim attr bad-v]
+                 [:db/add     victim attr safe]]
+
+                :else
+                [[:db/retract victim attr bad-v]]))))))))
+
+(defn apply-cycle-repairs!
+  "Detect & break cycles AFTER rebase.
+
+  Inputs:
+  - candidates-by-attr: {attr #{eid ...}}  (usually union of remote+local touched)
+  - touched-by-attr: {attr {:local-touched #{...} :remote-touched #{...}}}
+  - attr-opts: {attr {:safe-target-fn ... :skip? ...}}
+
+  We de-dup repairs by `distinct` tx vectors to reduce repeated work."
+  [transact! temp-conn candidates-by-attr touched-by-attr attr-opts]
+  (let [db @temp-conn
+        tx (->> candidates-by-attr
+                (mapcat (fn [[attr es]]
+                          (let [opts (get attr-opts attr {})
+                                touched (get touched-by-attr attr {})]
+                            (keep (fn [e]
+                                    (when-let [cycle (reachable-cycle db e attr opts)]
+                                      (prn :debug :detected-cycle cycle)
+                                      (break-cycle-tx db cycle attr opts touched)))
+                                  es))))
+                distinct
+                (apply concat))]
+    (when (seq tx)
+      (prn :debug :tx tx)
+      (transact! temp-conn tx {:outliner-op :fix-cycle :gen-undo-ops? false}))))
+
+(defn union-candidates
+  "Union remote + local candidates: {attr #{...}}"
+  [remote-by-attr local-by-attr]
   (reduce
-   (fn [acc tx]
-     (cond
-       (and (vector? tx)
-            (= :db/add (first tx))
-            (= attr (nth tx 2)))
-       (conj acc {:entity (nth tx 1)
-                  :value (nth tx 3)})
+   (fn [m attr]
+     (let [r (get remote-by-attr attr #{})
+           l (get local-by-attr attr #{})
+           u (into (set r) l)]
+       (if (seq u) (assoc m attr u) m)))
+   {}
+   (distinct (concat (keys remote-by-attr) (keys local-by-attr)))))
 
-       (and (map? tx) (contains? tx attr))
-       (let [entity (or (:db/id tx)
-                        (:block/uuid tx)
-                        (:db/ident tx))
-             value (get tx attr)]
-         (if (some? entity)
-           (conj acc {:entity entity
-                      :value value})
-           acc))
-
-       :else acc))
-   []
-   tx-data))
-
-(defn- normalize-entity-ref [entity]
-  (cond
-    (vector? entity) entity
-    (uuid? entity) [:block/uuid entity]
-    (keyword? entity) [:db/ident entity]
-    :else entity))
-
-(defn- next-parent-eid [db attr eid updates-by-eid]
-  (if (contains? updates-by-eid eid)
-    (get updates-by-eid eid)
-    (when-let [entity (d/entity db eid)]
-      (let [value (get entity attr)]
-        (cond
-          (instance? Entity value) (:db/id value)
-          :else (ref->eid db (normalize-entity-ref value)))))))
-
-(defn- cycle-from-eid? [db attr start-eid target-eid updates-by-eid]
-  (loop [seen #{target-eid}
-         current start-eid]
-    (cond
-      (nil? current) false
-      (contains? seen current) true
-      :else (recur (conj seen current)
-                   (next-parent-eid db attr current updates-by-eid)))))
-
-(defn- normalize-entity-ref-for-result [db entity-ref]
-  (if (number? entity-ref)
-    (when-let [ent (d/entity db entity-ref)]
-      [:block/uuid (:block/uuid ent)])
-    entity-ref))
-
-(defn detect-cycle
-  "Returns a map with cycle details when applying tx-data would introduce a cycle.
-  Otherwise returns nil."
-  [db tx-data]
+(defn touched-info-by-attr
+  "Build {attr {:remote-touched #{...} :local-touched #{...}}}."
+  [remote-by-attr local-by-attr]
   (reduce
-   (fn [_ attr]
-     (let [updates (attr-updates-from-tx tx-data attr)
-           updates-by-eid
-           (reduce
-            (fn [acc {:keys [entity value]}]
-              (let [entity-ref (normalize-entity-ref entity)
-                    eid (ref->eid db entity-ref)
-                    value-ref (normalize-entity-ref value)
-                    value-eid (ref->eid db value-ref)]
-                (if eid
-                  (assoc acc eid value-eid)
-                  acc)))
-            {}
-            updates)
-           result
-           (reduce
-            (fn [_ {:keys [entity value]}]
-              (if (nil? value)
-                nil
-                (let [entity-ref (normalize-entity-ref entity)
-                      eid (ref->eid db entity-ref)
-                      value-ref (normalize-entity-ref value)
-                      value-eid (ref->eid db value-ref)]
-                  (when (and eid value-eid
-                             (cycle-from-eid? db attr value-eid eid updates-by-eid))
-                    {:attr attr
-                     :entity (normalize-entity-ref-for-result db entity-ref)}))))
-            nil
-            updates)]
-       (when result
-         (reduced result))))
-   nil
-   special-attrs))
+   (fn [m attr]
+     (let [r (get remote-by-attr attr #{})
+           l (get local-by-attr attr #{})]
+       (assoc m attr {:remote-touched r :local-touched l})))
+   {}
+   (distinct (concat (keys remote-by-attr) (keys local-by-attr)))))
 
-(defn server-values-for
-  "Returns a map of entity refs to the server's current value for attr."
-  [db tx-data attr]
-  (let [updates (attr-updates-from-tx tx-data attr)]
-    (reduce
-     (fn [acc {:keys [entity]}]
-       (let [entity-ref (normalize-entity-ref entity)
-             eid (ref->eid db entity-ref)
-             current-raw (when eid (get (d/entity db eid) attr))
-             current (cond
-                       (nil? current-raw) nil
-                       (= attr :logseq.property.class/extends)
-                       (if (instance? Entity current-raw)
-                         (:db/ident current-raw)
-                         current-raw)
-                       (= attr :block/parent)
-                       (let [parent-uuid (cond
-                                           (instance? Entity current-raw) (:block/uuid current-raw)
-                                           (number? current-raw) (:block/uuid (d/entity db current-raw))
-                                           :else nil)]
-                         (when parent-uuid
-                           [:block/uuid parent-uuid]))
-                       :else current-raw)]
-         (assoc acc [:block/uuid (:block/uuid (d/entity db eid))] current)))
-     {}
-     updates)))
+(defn fix-cycle!
+  [temp-conn remote-tx-report rebase-tx-report]
+  (let [remote-touched-by-attr (touched-eids-many (:tx-data remote-tx-report))
+        local-touched-by-attr (touched-eids-many (:tx-data rebase-tx-report))
+        ;; Union candidates (remote + local) for cycle detection
+        candidates-by-attr (union-candidates remote-touched-by-attr local-touched-by-attr)
+
+        ;; Per-attr touched info to prefer breaking local edges first
+        touched-info (touched-info-by-attr remote-touched-by-attr local-touched-by-attr)]
+    (when (seq candidates-by-attr)
+      (apply-cycle-repairs! ldb/transact! temp-conn candidates-by-attr touched-info default-attr-opts))))

src/main/frontend/worker/db_sync.cljs

@@ -10,8 +10,8 @@
             [logseq.common.path :as path]
             [logseq.common.util :as common-util]
             [logseq.db :as ldb]
-            [logseq.db-sync.checksum :as db-sync-checksum]
-            [logseq.db-sync.cycle :as db-sync-cycle]
+            [logseq.db-sync.checksum :as sync-checksum]
+            [logseq.db-sync.cycle :as sync-cycle]
             [logseq.db-sync.malli-schema :as db-sync-schema]
             [logseq.db-sync.order :as sync-order]
             [logseq.db.common.normalize :as db-normalize]
@@ -321,43 +321,6 @@
                      (contains? deleted-ids id)))))
     tx-data))
 
-(defn- remove-attr-updates
-  [db tx-data attr entity-ref]
-  (remove (fn [tx]
-            (and (vector? tx)
-                 (= attr (nth tx 2 nil))
-                 (entity-ref-matches? db entity-ref (nth tx 1 nil))))
-          tx-data))
-
-(defn- tx-entity-ref
-  [db entity-ref]
-  (or (entity-ref->entid db entity-ref)
-      (normalize-entity-ref entity-ref)))
-
-(defn- replace-parent-with-page-root
-  [db tx-data entity-ref]
-  (let [entity-entid (entity-ref->entid db entity-ref)
-        page-uuid (when entity-entid
-                    (some-> (d/entity db entity-entid) :block/page :block/uuid))
-        page-ref (when page-uuid [:block/uuid page-uuid])]
-    (cond-> (remove-attr-updates db tx-data :block/parent entity-ref)
-      page-ref
-      (conj [:db/add (tx-entity-ref db entity-ref) :block/parent page-ref]))))
-
-(defn- fix-cycle-updates
-  [db tx-data]
-  (loop [tx-data tx-data
-         attempt 0]
-    (if (>= attempt 4)
-      tx-data
-      (if-let [{:keys [attr entity]} (db-sync-cycle/detect-cycle db tx-data)]
-        (let [tx-data' (case attr
-                         :block/parent (replace-parent-with-page-root db tx-data entity)
-                         :logseq.property.class/extends (remove-attr-updates db tx-data attr entity)
-                         (remove-attr-updates db tx-data attr entity))]
-          (recur tx-data' (inc attempt)))
-        tx-data))))
-
 (defn- keep-last-update
   [db tx-data]
   (let [properties (->> (distinct (map :a tx-data))
@@ -378,8 +341,7 @@
         sanitized-tx-data (->> tx-data
                                db-normalize/replace-attr-retract-with-retract-entity-v2
                                (keep-last-update db)
-                               (drop-invalid-refs deleted-ids)
-                               (fix-cycle-updates db))]
+                               (drop-invalid-refs deleted-ids))]
     (when (not= tx-data sanitized-tx-data)
       (log/info :db-sync/tx-sanitized
                 {:diff (data/diff tx-data sanitized-tx-data)}))
@@ -656,12 +618,12 @@
                    reversed-tx-report (when has-local-changes?
                                         (ldb/transact! temp-conn reversed-tx-data tx-meta))
                    ;; 2. transact remote tx-data
-                   tx-report (ldb/transact! temp-conn tx-data tx-meta)
-                   _ (reset! *remote-tx-report tx-report)
+                   remote-tx-report (ldb/transact! temp-conn tx-data tx-meta)
+                   _ (reset! *remote-tx-report remote-tx-report)
                    computed-checksum (when expected-checksum
-                                       (db-sync-checksum/next-checksum
+                                       (sync-checksum/next-checksum
                                         (client-op/get-local-checksum repo)
-                                        (db-sync-checksum/filter-tx-data tx-report)))]
+                                        (sync-checksum/filter-tx-data remote-tx-report)))]
 
                (reset! *computed-checksum computed-checksum)
                ;; (when (and expected-checksum (not= expected-checksum computed-checksum))
@@ -679,7 +641,7 @@
                                                             (nil? (d/entity db e)))
                                                    (d/entity (:db-after reversed-tx-report) e)))
                                                (:tx-data reversed-tx-report)))
-                       remote-deleted-blocks (->> (outliner-pipeline/filter-deleted-blocks (:tx-data tx-report))
+                       remote-deleted-blocks (->> (outliner-pipeline/filter-deleted-blocks (:tx-data remote-tx-report))
                                                   (map #(d/entity db (:db/id %))))
                        deleted-nodes (concat local-deleted-blocks remote-deleted-blocks)
                        deleted-ids (set (keep :block/uuid deleted-nodes))
@@ -708,9 +670,12 @@
                                               (prn :debug :pending-tx-data pending-tx-data
                                                    :rebased-tx-data rebased-tx-data)
                                               (when (seq rebased-tx-data)
-                                                (ldb/transact! temp-conn rebased-tx-data {:gen-undo-ops? false}))))]
-                     (sync-order/fix-duplicate-orders! temp-conn (concat (:tx-data tx-report)
-                                                                         (:tx-data rebase-tx-report))))))))
+                                                (ldb/transact! temp-conn rebased-tx-data {:gen-undo-ops? false}))))
+                         fix-cycle-tx-report (sync-cycle/fix-cycle! temp-conn remote-tx-report rebase-tx-report)]
+                     (sync-order/fix-duplicate-orders! temp-conn
+                                                       (mapcat :tx-data [remote-tx-report
+                                                                         rebase-tx-report
+                                                                         fix-cycle-tx-report])))))))
            {:listen-db (fn [{:keys [tx-data tx-meta]}]
                          (when (and has-local-changes? (not (:rtc-tx? tx-meta)))
                            (swap! *rebased-tx-data into tx-data)))})