Fix broken page after transaction

Tests has been added

src/main/frontend/db/fix.cljs

@@ -0,0 +1,173 @@
+(ns frontend.db.fix
+  "DB validation and fix.
+  For pages:
+  1. Each block should has a unique [:block/parent :block/left] position.
+  2. For any block, its children should be connected by :block/left (no broken chain, no circle, no left to self)."
+  (:require [datascript.core :as d]
+            [frontend.db :as db]
+            [frontend.db.model :as db-model]
+            [frontend.util :as util]
+            [frontend.state :as state]
+            [frontend.handler.notification :as notification]
+            [clojure.set :as set]))
+
+(defn- fix-parent-broken-chain
+  [db parent-id]
+  (let [parent (db/entity parent-id)
+        parent-id (:db/id parent)
+        blocks (:block/_parent parent)]
+    (when (seq blocks)
+      (let [children-ids (set (map :db/id blocks))
+            full-ids (conj children-ids parent-id)
+            left-ids (set (keep (fn [block]
+                                  (let [left-id (:db/id (:block/left block))]
+                                    (when (and (not= left-id (:db/id block))
+                                               (contains? full-ids left-id))
+                                      left-id))) blocks))
+            broken-chain? (not (and (set/subset? left-ids full-ids)
+                                    (= 1 (- (count full-ids) (count left-ids)))))
+            first-child-id (:db/id (db-model/get-by-parent-&-left db parent-id parent-id))
+            *ids (atom children-ids)
+            sections (loop [sections []]
+                       (if (seq @*ids)
+                         (let [last-section (last sections)
+                               current-section (if (seq (last sections))
+                                                 last-section
+                                                 (if (and (empty? sections) first-child-id)
+                                                   (do
+                                                     (swap! *ids disj first-child-id)
+                                                     [first-child-id])
+                                                   (do
+                                                     (let [id (first @*ids)]
+                                                       (swap! *ids disj id)
+                                                       [id]))))
+                               section-with-left (or
+                                                  (when-let [left-id (:db/id (:block/left (db/entity (first current-section))))]
+                                                    (swap! *ids disj left-id)
+                                                    (when (and
+                                                           (not (contains? (set current-section) left-id)) ; circle
+                                                           (contains? children-ids left-id))
+                                                      (vec (cons left-id current-section))))
+                                                  current-section)
+                               section-with-right (or
+                                                   (when-let [right-id (:db/id (db-model/get-right-sibling db (last section-with-left)))]
+                                                     (swap! *ids disj right-id)
+                                                     (when (and (not (contains? (set section-with-left) right-id)) ; circle
+                                                                (contains? children-ids right-id))
+                                                       (conj section-with-left right-id)))
+                                                   section-with-left)
+                               new-sections (cond
+                                              (empty? last-section)
+                                              (conj (vec (butlast sections)) section-with-right)
+
+                                              (= section-with-right current-section)
+                                              (conj sections [])
+
+                                              :else
+                                              (conj (vec (butlast sections)) section-with-right))]
+                           (recur new-sections))
+                         sections))
+            tx-data (->>
+                     (map-indexed
+                      (fn [idx section]
+                        (map-indexed
+                         (fn [idx' item]
+                           (let [m {:db/id item}
+                                 left (cond
+                                        (and (zero? idx) (zero? idx'))
+                                        parent-id
+
+                                        (and (not (zero? idx)) (zero? idx')) ; first one need to connected to the last section
+                                        (last (nth sections (dec idx)))
+
+                                        (> idx' 0)
+                                        (nth section (dec idx')))]
+                             (assoc m :block/left left)))
+                         section))
+                      sections)
+                     (apply concat))]
+        (when broken-chain?
+          (let [error-data {:parent-id parent-id}]
+            (prn :debug "Broken chain:")
+            (notification/show!
+             [:div
+              (str "Broken chain detected:\n" error-data)]
+             :error
+             false)))
+        tx-data))))
+
+(defn- fix-broken-chain
+  [db parent-left->es]
+  (let [parents (distinct (map first (keys parent-left->es)))]
+    (mapcat #(fix-parent-broken-chain db %) parents)))
+
+(defn- build-parent-left->es
+  [db page-id]
+  (let [parent-left-f (fn [b]
+                        [(get-in b [:block/parent :db/id])
+                         (get-in b [:block/left :db/id])])
+        page (d/entity db page-id)
+        blocks (:block/_page page)]
+    (->> (group-by parent-left-f blocks)
+         (remove (fn [[k _v]] (= k [nil nil])))
+         (into {}))))
+
+(defn- fix-parent-left-conflicts
+  [conflicts]
+  (when (seq conflicts)
+    (prn :debug "Parent left id conflicts:")
+    (notification/show!
+     [:div
+      (str "Parent-left conflicts detected:\n"
+           conflicts)]
+     :error
+     false))
+  (mapcat
+   (fn [[_parent-left blocks]]
+     (let [items (sort-by :block/created-at blocks)
+           [first-item & others] items
+           tx (map-indexed
+               (fn [idx other]
+                 {:db/id (:db/id other)
+                  :block/left (:db/id (nth items (if (zero? idx) idx (dec idx))))
+                  :block/parent (:db/id (:block/parent first-item))})
+               others)
+           right-tx (when-let [right (db-model/get-right-sibling (db/get-db) (:db/id first-item))]
+                      [{:db/id (:db/id right)
+                        :block/left (:db/id (last items))}])]
+       (concat tx right-tx)))
+   conflicts))
+
+(defn loop-fix-conflicts
+  [repo db page-id transact-opts]
+  (let [get-conflicts (fn [db]
+                        (let [parent-left->es (build-parent-left->es db page-id)]
+                          (filter #(> (count (second %)) 1) parent-left->es)))
+        conflicts (get-conflicts db)
+        fix-conflicts-tx (when (seq conflicts)
+                           (fix-parent-left-conflicts conflicts))]
+    (when (seq fix-conflicts-tx)
+      (prn :debug :conflicts-tx)
+      (util/pprint fix-conflicts-tx)
+      (db/transact! repo fix-conflicts-tx transact-opts)
+      (let [db (db/get-db repo)]
+        (when (seq (get-conflicts db))
+          (loop-fix-conflicts repo db page-id transact-opts))))))
+
+(defn fix-page-if-broken!
+  "Fix the page if it has either parent-left conflicts or broken chains."
+  [db page-id {:keys [fix-parent-left? fix-broken-chain? replace-tx?]
+            :or {fix-parent-left? true
+                 fix-broken-chain? true
+                 replace-tx? true}
+            :as _opts}]
+  (let [repo (state/get-current-repo)
+        transact-opts (if replace-tx? {:replace? true} {})]
+    (when fix-parent-left?
+      (loop-fix-conflicts repo db page-id transact-opts))
+    (when fix-broken-chain?
+      (let [db' (db/get-db)
+            parent-left->es' (build-parent-left->es (db/get-db) page-id)
+            fix-broken-chain-tx (fix-broken-chain db' parent-left->es')]
+        (when (seq fix-broken-chain-tx)
+          (db/transact! repo fix-broken-chain-tx transact-opts))))))

src/main/frontend/db/validate.cljs

@@ -1,51 +0,0 @@
-(ns frontend.db.validate
-  "DB validation.
-  For pages:
-  1. Each block should has a unique [:block/parent :block/left] position.
-  2. For any block, its children should be connected by :block/left."
-  (:require [datascript.core :as d]
-            [medley.core :as medley]))
-
-(defn- broken-chain?
-  [page parent-left->eid]
-  (let [parents (->> (:block/_page page)
-                     (filter #(seq (:block/_parent %)))
-                     (cons page))]
-    (some
-     (fn [parent]
-       (let [parent-id (:db/id parent)
-             blocks (:block/_parent parent)]
-         (when (seq blocks)
-           (when-let [start (parent-left->eid [parent-id parent-id])]
-             (let [chain (loop [current start
-                                chain [start]]
-                           (let [next (parent-left->eid [parent-id current])]
-                             (if next
-                               (recur next (conj chain next))
-                               chain)))]
-               (when (not= (count chain) (count blocks))
-                 {:parent parent
-                  :chain chain
-                  :broken-blocks (remove (set chain) (map :db/id blocks))
-                  :blocks blocks}))))))
-     parents)))
-
-(defn broken-page?
-  "Whether `page` is broken."
-  [db page-id]
-  (let [parent-left-f (fn [b]
-                        [(get-in b [:block/parent :db/id])
-                         (get-in b [:block/left :db/id])])
-        page (d/entity db page-id)
-        blocks (:block/_page page)
-        parent-left->es (->> (group-by parent-left-f blocks)
-                             (remove (fn [[k _v]] (= k [nil nil])))
-                             (into {}))
-        conflicted (filter #(> (count (second %)) 1) parent-left->es)]
-    (if (seq conflicted)
-      [:conflict-parent-left conflicted]
-
-      (let [parent-left->eid (medley/map-vals (fn [c] (:db/id (first c))) parent-left->es)]
-        (if-let [result (broken-chain? page parent-left->eid)]
-          [:broken-chain result]
-          false)))))

src/main/frontend/modules/outliner/datascript.cljs

@@ -11,7 +11,7 @@
             [clojure.string :as string]
             [frontend.util :as util]
             [logseq.graph-parser.util.block-ref :as block-ref]
-            [frontend.db.validate :as db-validate]
+            [frontend.db.fix :as db-fix]
             [frontend.handler.file-based.property.util :as property-util]))
 
 (defn new-outliner-txs-state [] (atom []))
@@ -129,16 +129,8 @@
                                       :db/id)))
                            (remove nil?)
                            (distinct))]
-    (reduce
-     (fn [_ page-id]
-       (if-let [result (db-validate/broken-page? db-after page-id)]
-         (do
-           ;; TODO: revert db changes
-           (assert (false? result) (str "Broken page: " result))
-           (reduced false))
-         true))
-     true
-     changed-pages)))
+    (doseq [changed-page-id changed-pages]
+      (db-fix/fix-page-if-broken! db-after changed-page-id {}))))
 
 (defn transact!
   [txs opts before-editor-cursor]

src/test/frontend/db/fix_test.cljs

@@ -0,0 +1,211 @@
+(ns frontend.db.fix-test
+  (:require [cljs.test :refer [deftest is use-fixtures]]
+            [datascript.core :as d]
+            [frontend.core-test :as core-test]
+            [frontend.test.fixtures :as fixtures]
+            [frontend.db.fix :as db-fix]
+            [frontend.test.helper :as test-helper]))
+
+(use-fixtures :each fixtures/reset-db)
+
+(def test-db test-helper/test-db)
+
+(defonce init-conflicts
+  [{:block/uuid "1"}
+   {:block/uuid "2"
+    :block/page [:block/uuid "1"]
+    :block/parent [:block/uuid "1"]
+    :block/left [:block/uuid "1"]}
+   {:block/uuid "3"
+    :block/page [:block/uuid "1"]
+    :block/parent [:block/uuid "1"]
+    :block/left [:block/uuid "1"]}])
+
+(deftest test-conflicts
+  (let [conn (core-test/get-current-conn)
+        _ (d/transact! conn init-conflicts)
+        page-id (:db/id (d/entity @conn 1))
+        _ (db-fix/fix-page-if-broken! @conn page-id {})]
+    (is (= 2 (:db/id (:block/left (d/entity @conn 3)))))))
+
+(deftest test-conflicts-with-right
+  (let [conn (core-test/get-current-conn)
+        data (concat init-conflicts
+                     [{:block/uuid "4"
+                       :block/page [:block/uuid "1"]
+                       :block/parent [:block/uuid "1"]
+                       :block/left [:block/uuid "2"]}])
+        _ (d/transact! conn data)
+        page-id (:db/id (d/entity @conn 1))
+        _ (db-fix/fix-page-if-broken! @conn page-id {})]
+    (is (= 3 (:db/id (:block/left (d/entity @conn 4)))))))
+
+(def init-broken-chain
+  [{:block/uuid "1"}
+   {:block/uuid "2"
+    :block/page [:block/uuid "1"]
+    :block/parent [:block/uuid "1"]
+    :block/left [:block/uuid "1"]}
+   {:block/uuid "3"
+    :block/page [:block/uuid "1"]
+    :block/parent [:block/uuid "1"]
+    :block/left [:block/uuid "2"]}
+   {:block/uuid "4"}
+   {:block/uuid "5"
+    :block/page [:block/uuid "1"]
+    :block/parent [:block/uuid "1"]
+    :block/left [:block/uuid "4"]}])
+
+(deftest test-broken-chain
+  (let [conn (core-test/get-current-conn)
+        data init-broken-chain
+        _ (d/transact! conn data)
+        page-id (:db/id (d/entity @conn 1))
+        _ (db-fix/fix-page-if-broken! @conn page-id {})]
+    (is
+     (=
+      (set [{:db/id 2, :block/left 1}
+            {:db/id 3, :block/left 2}
+            {:db/id 5, :block/left 3}])
+      (set
+       (map (fn [b]
+              {:db/id (:db/id b)
+               :block/left (:db/id (:block/left b))})
+            (:block/_parent (d/entity @conn 1))))))))
+
+(deftest test-broken-chain-with-no-start
+  (let [conn (core-test/get-current-conn)
+        data [{:block/uuid "1"}
+              {:block/uuid "5"}
+              {:block/uuid "2"
+               :block/page [:block/uuid "1"]
+               :block/parent [:block/uuid "1"]
+               :block/left [:block/uuid "5"]}
+              {:block/uuid "3"
+               :block/page [:block/uuid "1"]
+               :block/parent [:block/uuid "1"]
+               :block/left [:block/uuid "2"]}]
+        _ (d/transact! conn data)
+        page-id (:db/id (d/entity @conn 1))
+        _ (db-fix/fix-page-if-broken! @conn page-id {})]
+    (is
+     (=
+      (set [{:db/id 3, :block/left 1}
+            {:db/id 4, :block/left 3}])
+      (set (map (fn [b]
+                  {:db/id (:db/id b)
+                   :block/left (:db/id (:block/left b))})
+                (:block/_parent (d/entity @conn 1))))))))
+
+(deftest test-broken-chain-with-circle
+  (let [conn (core-test/get-current-conn)
+        data [{:block/uuid "1"}
+              {:block/uuid "2"
+               :block/page [:block/uuid "1"]
+               :block/parent [:block/uuid "1"]
+               :block/left [:block/uuid "1"]}
+              {:block/uuid "4"}
+              {:block/uuid "3"
+               :block/page [:block/uuid "1"]
+               :block/parent [:block/uuid "1"]
+               :block/left [:block/uuid "4"]}
+              {:block/uuid "4"
+               :block/page [:block/uuid "1"]
+               :block/parent [:block/uuid "1"]
+               :block/left [:block/uuid "3"]}]
+        _ (d/transact! conn data)
+        page-id (:db/id (d/entity @conn 1))
+        _ (db-fix/fix-page-if-broken! @conn page-id {})]
+    (is
+     (=
+      (set [{:db/id 2, :block/left 1}
+            {:db/id 4, :block/left 2}
+            {:db/id 3, :block/left 4}])
+      (set (map (fn [b]
+                  {:db/id (:db/id b)
+                   :block/left (:db/id (:block/left b))})
+             (:block/_parent (d/entity @conn 1))))))))
+
+(deftest test-broken-chain-with-no-start-and-circle
+  (let [conn (core-test/get-current-conn)
+        data [{:block/uuid "1"
+               :db/id 1}
+              {:block/uuid "5"
+               :db/id 5}
+              {:block/uuid "2"
+               :db/id 2
+               :block/page [:block/uuid "1"]
+               :block/parent [:block/uuid "1"]
+               :block/left [:block/uuid "5"]}
+              {:block/uuid "3"
+               :db/id 3
+               :block/page [:block/uuid "1"]
+               :block/parent [:block/uuid "1"]
+               :block/left [:block/uuid "2"]}
+              {:block/uuid "4"
+               :db/id 4
+               :block/page [:block/uuid "1"]
+               :block/parent [:block/uuid "1"]
+               :block/left [:block/uuid "3"]}
+              {:block/uuid "5"
+               :block/page [:block/uuid "1"]
+               :block/parent [:block/uuid "1"]
+               :block/left [:block/uuid "2"]}]
+        _ (d/transact! conn data)
+        page-id (:db/id (d/entity @conn 1))
+        _ (db-fix/fix-page-if-broken! @conn page-id {})]
+    (is
+     (=
+      #{{:db/id 3, :block/left 1}
+        {:db/id 5, :block/left 3}
+        {:db/id 2, :block/left 5}
+        {:db/id 4, :block/left 2}}
+      (set (map (fn [b]
+                  {:db/id (:db/id b)
+                   :block/left (:db/id (:block/left b))})
+             (:block/_parent (d/entity @conn 1))))))))
+
+(deftest test-multiple-broken-chains
+  (let [conn (core-test/get-current-conn)
+        data [{:block/uuid "1"
+               :db/id 1}
+              {:block/uuid "2"
+               :db/id 2
+               :block/page [:block/uuid "1"]
+               :block/parent [:block/uuid "1"]
+               :block/left [:block/uuid "1"]}
+              {:block/uuid "4"
+               :db/id 4}
+              {:block/uuid "3"
+               :db/id 3
+               :block/page [:block/uuid "1"]
+               :block/parent [:block/uuid "1"]
+               :block/left [:block/uuid "4"]}
+              {:block/uuid "5"
+               :block/page [:block/uuid "1"]
+               :block/parent [:block/uuid "1"]
+               :block/left [:block/uuid "3"]}
+              {:block/uuid "6"
+               :db/id 6}
+              {:block/uuid "7"
+               :block/page [:block/uuid "1"]
+               :block/parent [:block/uuid "1"]
+               :block/left [:block/uuid "5"]}]
+        _ (d/transact! conn data)
+        page-id (:db/id (d/entity @conn 1))
+        _ (db-fix/fix-page-if-broken! @conn page-id {})]
+    (is
+     (=
+      #{{:db/id 2, :block/left 1}
+        {:db/id 3, :block/left 2}
+        {:db/id 5, :block/left 3}
+        {:db/id 7, :block/left 5}}
+      (set (map (fn [b]
+                  {:db/id (:db/id b)
+                   :block/left (:db/id (:block/left b))})
+                (:block/_parent (d/entity @conn 1))))))))
+
+(comment
+  (do
+    (frontend.test.fixtures/reset-datascript test-db)
+    nil))