enhance: run db validate before writes

deps.edn

@@ -5,7 +5,8 @@
                                          :sha     "5d672bf84ed944414b9f61eeb83808ead7be9127"}
 
   datascript/datascript                 {:git/url "https://github.com/logseq/datascript" ;; fork
-                                         :sha     "45f6721bf2038c24eb9fe3afb422322ab3f473b5"}
+                                         :sha     "3971e2d43bd93d89f42191dc7b4b092989e0cc61"}
+  ;; datascript/datascript                 {:local/root "../../datascript"}
 
   datascript-transit/datascript-transit {:mvn/version "0.3.0"}
   borkdude/rewrite-edn                  {:mvn/version "0.4.9"}

deps/db/deps.edn

@@ -1,7 +1,8 @@
 {:deps
  ;; These nbb-logseq deps are kept in sync with https://github.com/logseq/nbb-logseq/blob/main/bb.edn
  {datascript/datascript {:git/url "https://github.com/logseq/datascript" ;; fork
-                         :sha     "45f6721bf2038c24eb9fe3afb422322ab3f473b5"}
+                         :sha     "3971e2d43bd93d89f42191dc7b4b092989e0cc61"}
+  ;; datascript/datascript                 {:local/root "../../../../datascript"}
   datascript-transit/datascript-transit {:mvn/version "0.3.0"
                                          :exclusions [datascript/datascript]}
   cljs-bean/cljs-bean         {:mvn/version "1.5.0"}

deps/db/src/logseq/db.cljs

@@ -5,6 +5,7 @@
   (:require [clojure.set :as set]
             [clojure.string :as string]
             [clojure.walk :as walk]
+            [datascript.conn :as dc]
             [datascript.core :as d]
             [datascript.impl.entity :as de]
             [logseq.common.config :as common-config]
@@ -20,6 +21,7 @@
             [logseq.db.frontend.entity-util :as entity-util]
             [logseq.db.frontend.property :as db-property]
             [logseq.db.frontend.schema :as db-schema]
+            [logseq.db.frontend.validate :as db-validate]
             [logseq.db.sqlite.util :as sqlite-util])
   (:refer-clojure :exclude [object?]))
 
@@ -32,9 +34,14 @@
 (def build-favorite-tx db-db/build-favorite-tx)
 
 (defonce *transact-fn (atom nil))
+(defonce *transact-invalid-callback (atom nil))
+
 (defn register-transact-fn!
   [f]
   (when f (reset! *transact-fn f)))
+(defn register-transact-invalid-callback-fn!
+  [f]
+  (when f (reset! *transact-invalid-callback f)))
 
 (defn- remove-temp-block-data
   [tx-data]
@@ -70,6 +77,15 @@
        f))
    tx-data))
 
+(comment
+  (defn- skip-db-validate?
+    [datoms]
+    (every?
+     (fn [d]
+       (contains? #{:logseq.property/created-by-ref :block/refs :block/tx-id}
+                  (:a d)))
+     datoms)))
+
 (defn transact!
   "`repo-or-conn`: repo for UI thread and conn for worker/node"
   ([repo-or-conn tx-data]
@@ -105,7 +121,30 @@
        (if-let [transact-fn @*transact-fn]
          (transact-fn repo-or-conn tx-data tx-meta)
          (try
-           (d/transact! repo-or-conn tx-data tx-meta)
+           (let [conn repo-or-conn
+                 db @conn
+                 skip-validate? (:skip-validate-db? tx-meta false)
+                 db-based? (entity-plus/db-based-graph? db)
+                 [validate-result tx-report] (if (or (:reset-conn! tx-meta)
+                                                     (not db-based?)
+                                                     skip-validate?
+                                                     (:pipeline-replace? tx-meta))
+                                               [true nil]
+                                               (let [tx-report (d/with db tx-data tx-meta)]
+                                                 [(db-validate/validate-tx-report tx-report nil) tx-report]))]
+             (if validate-result
+               (if (and tx-report (seq (:tx-data tx-report)))
+                 ;; perf enhancement: avoid repeated call on `d/with`
+                 (do
+                   (reset! conn (:db-after tx-report))
+                   (dc/store-after-transact! conn tx-report)
+                   (dc/run-callbacks conn tx-report))
+                 (d/transact! conn tx-data tx-meta))
+               (do
+                 ;; notify ui
+                 (when-let [f @*transact-invalid-callback]
+                   (f tx-report))
+                 (throw (ex-info "DB write with invalid data" {:tx-data tx-data})))))
            (catch :default e
              (js/console.trace)
              (prn :debug :transact-failed :tx-meta tx-meta :tx-data tx-data)

deps/db/src/logseq/db/frontend/validate.cljs

@@ -21,10 +21,10 @@
   [closed-schema?]
   (if closed-schema? closed-db-schema-explainer db-schema-explainer))
 
-(defn validate-tx-report!
+(defn validate-tx-report
   "Validates the datascript tx-report for entities that have changed. Returns
   boolean indicating if db is valid"
-  [{:keys [db-after tx-data tx-meta]} validate-options]
+  [{:keys [db-after tx-data _tx-meta]} validate-options]
   (let [changed-ids (->> tx-data (keep :e) distinct)
         tx-datoms (mapcat #(d/datoms db-after :eavt %) changed-ids)
         ent-maps* (map (fn [[db-id m]]
@@ -38,7 +38,7 @@
                               ;; remove :db/id as it adds needless declarations to schema
                               #(validator [(dissoc % :db/id)])
                               ent-maps)]
-        (prn "changed eids:" changed-ids :tx-meta tx-meta)
+        ;; (prn "changed eids:" changed-ids :tx-meta tx-meta)
         (if (seq invalid-ent-maps)
           (let [explainer (get-schema-explainer (:closed-schema? validate-options))]
             (prn "Invalid datascript entities detected amongst changed entity ids:" changed-ids)

deps/outliner/deps.edn

@@ -1,7 +1,8 @@
 {:deps
  ;; These nbb-logseq deps are kept in sync with https://github.com/logseq/nbb-logseq/blob/main/bb.edn
  {datascript/datascript {:git/url "https://github.com/logseq/datascript" ;; fork
-                         :sha     "45f6721bf2038c24eb9fe3afb422322ab3f473b5"}
+                         :sha     "3971e2d43bd93d89f42191dc7b4b092989e0cc61"}
+  ;; datascript/datascript {:local/root "../../../../datascript"}
   com.cognitect/transit-cljs {:mvn/version "0.8.280"}
 
   ;; Any other deps should be added here and to nbb.edn

deps/outliner/src/logseq/outliner/core.cljs

@@ -1103,12 +1103,14 @@
 ;;; ### write-operations have side-effects (do transactions) ;;;;;;;;;;;;;;;;
 
 (defn- op-transact!
-  [f & args]
+  [outliner-op f & args]
   {:pre [(fn? f)]}
   (try
     (let [result (apply f args)]
       (when result
-        (let [tx-meta (assoc (:tx-meta result) :skip-store? true)]
+        (let [tx-meta (assoc (:tx-meta result)
+                             :outliner-op outliner-op
+                             :skip-store? true)]
           (ldb/transact! (second args) (:tx-data result) tx-meta)))
       result)
     (catch :default e
@@ -1119,31 +1121,29 @@
           (save-block repo @conn date-formatter block opts))]
   (defn save-block!
     [repo conn date-formatter block & {:as opts}]
-    (op-transact! f repo conn date-formatter block opts)))
+    (op-transact! :save-block f repo conn date-formatter block opts)))
 
 (let [f (fn [repo conn blocks target-block opts]
           (insert-blocks repo @conn blocks target-block opts))]
   (defn insert-blocks!
     [repo conn blocks target-block opts]
-    (op-transact! f repo conn blocks target-block (assoc opts :outliner-op :insert-blocks))))
+    (op-transact! :insert-blocks f repo conn blocks target-block (assoc opts :outliner-op :insert-blocks))))
 
-(let [f (fn [_repo conn blocks opts]
-          (let [{:keys [tx-data]} (delete-blocks @conn blocks)]
-            {:tx-data tx-data
-             :tx-meta (select-keys opts [:outliner-op])}))]
+(let [f (fn [_repo conn blocks _opts]
+          (delete-blocks @conn blocks))]
   (defn delete-blocks!
     [repo conn _date-formatter blocks opts]
-    (op-transact! f repo conn blocks opts)))
+    (op-transact! :delete-blocks f repo conn blocks opts)))
 
 (defn move-blocks!
   [repo conn blocks target-block opts]
-  (op-transact! move-blocks repo conn blocks target-block
+  (op-transact! :move-blocks move-blocks repo conn blocks target-block
                 (assoc opts :outliner-op :move-blocks)))
 
 (defn move-blocks-up-down!
   [repo conn blocks up?]
-  (op-transact! move-blocks-up-down repo conn blocks up?))
+  (op-transact! :move-blocks-up-down move-blocks-up-down repo conn blocks up?))
 
 (defn indent-outdent-blocks!
   [repo conn blocks indent? & {:as opts}]
-  (op-transact! indent-outdent-blocks repo conn blocks indent? opts))
+  (op-transact! :indent-outdent-blocks indent-outdent-blocks repo conn blocks indent? opts))

src/main/frontend/worker/db_worker.cljs

@@ -883,9 +883,19 @@
           (reset! *service [graph service])
           service)))))
 
+(defn- notify-invalid-data
+  [{:keys [tx-meta]}]
+  ;; don't notify on production when undo/redo failed
+  (when-not (and (or (:undo? tx-meta) (:redo? tx-meta))
+                 (not worker-util/dev?))
+    (shared-service/broadcast-to-clients! :notification
+                                          [["Invalid DB!"] :error])))
+
 (defn init
   "web worker entry"
   []
+  (ldb/register-transact-invalid-callback-fn! notify-invalid-data)
+
   (let [proxy-object (->>
                       fns
                       (map

src/main/frontend/worker/pipeline.cljs

@@ -6,9 +6,7 @@
             [frontend.worker.commands :as commands]
             [frontend.worker.file :as file]
             [frontend.worker.react :as worker-react]
-            [frontend.worker.shared-service :as shared-service]
             [frontend.worker.state :as worker-state]
-            [logseq.common.defkeywords :refer [defkeywords]]
             [logseq.common.util :as common-util]
             [logseq.common.util.page-ref :as page-ref]
             [logseq.common.uuid :as common-uuid]
@@ -16,7 +14,6 @@
             [logseq.db.common.order :as db-order]
             [logseq.db.common.sqlite :as common-sqlite]
             [logseq.db.frontend.class :as db-class]
-            [logseq.db.frontend.validate :as db-validate]
             [logseq.db.sqlite.export :as sqlite-export]
             [logseq.db.sqlite.util :as sqlite-util]
             [logseq.graph-parser.exporter :as gp-exporter]
@@ -97,44 +94,6 @@
                                          (:tx-data result)))))))]
     tx-data))
 
-(defkeywords
-  ::skip-validate-db? {:doc "tx-meta option, default = false"}
-  ::skip-store-conn {:doc "tx-meta option, skip `d/store` on conn. default = false"})
-
-(defn validate-db!
-  "Validate db is slow, we probably don't want to enable it for production."
-  [repo conn tx-report tx-meta context]
-  (when (and (not (::skip-validate-db? tx-meta false))
-             (or (:dev? context) (:undo? tx-meta) (:redo? tx-meta))
-             (not (:importing? context)) (sqlite-util/db-based-graph? repo))
-    (let [valid? (if (get-in tx-report [:tx-meta :reset-conn!])
-                   true
-                   (db-validate/validate-tx-report! tx-report (:validate-db-options context)))]
-      (when-not valid?
-        (when (and (or (get-in context [:validate-db-options :fail-invalid?]) worker-util/dev?)
-                   ;; don't notify on production when undo/redo failed
-                   (not (and (not (:dev? context)) (or (:undo? tx-meta) (:redo? tx-meta)))))
-          (shared-service/broadcast-to-clients! :notification
-                                                [["Invalid DB!"] :error]))
-        (throw (ex-info "Invalid data" {:graph repo})))))
-
-  ;; Ensure :block/order is unique for any block that has :block/parent
-  (when false;; (:dev? context)
-    (let [order-datoms (filter (fn [d] (= :block/order (:a d)))
-                               (:tx-data tx-report))]
-      (doseq [datom order-datoms]
-        (let [entity (d/entity @conn (:e datom))
-              parent (:block/parent entity)]
-          (when parent
-            (let [children (:block/_parent parent)
-                  order-different? (= (count (distinct (map :block/order children))) (count children))]
-              (when-not order-different?
-                (throw (ex-info (str ":block/order is not unique for children blocks, parent id: " (:db/id parent))
-                                {:children (->> (map (fn [b] (select-keys b [:db/id :block/title :block/order])) children)
-                                                (sort-by :block/order))
-                                 :tx-meta tx-meta
-                                 :tx-data (:tx-data tx-report)}))))))))))
-
 (defn- fix-page-tags
   "Add missing attributes and remove #Page when inserting or updating block/title with inline tags"
   [{:keys [db-after tx-data tx-meta]}]
@@ -158,7 +117,7 @@
                    [:db/add eid :logseq.property.class/extends :logseq.class/Root]
                    [:db/retract eid :block/tags :logseq.class/Page]])))
 
-            ;; remove #Page from tags/journals/whitebaords, etc.
+            ;; remove #Page from tags/journals/whiteboards, etc.
             (and (= :block/tags (:a datom))
                  (:added datom)
                  (= (:db/id page-tag) (:v datom)))
@@ -415,7 +374,6 @@
                           (compute-extra-tx-data repo conn tx-report))
           tx-report* (if (seq extra-tx-data)
                        (let [result (ldb/transact! conn extra-tx-data {:pipeline-replace? true
-                                                                       :outliner-op :pre-hook-invoke
                                                                        :skip-store? true})]
                          (assoc tx-report
                                 :tx-data (concat (:tx-data tx-report) (:tx-data result))
@@ -442,38 +400,23 @@
           blocks' (remove (fn [b] (deleted-block-ids (:db/id b))) blocks)
           block-refs (when (seq blocks')
                        (rebuild-block-refs repo tx-report* blocks'))
-          refs-tx-report (when (seq block-refs)
-                           (ldb/transact! conn block-refs {:pipeline-replace? true
-                                                           :skip-store? true}))
-          replace-tx (let [db-after (or (:db-after refs-tx-report) (:db-after tx-report*))]
-                       (concat
-                        ;; update block/tx-id
-                        (let [updated-blocks (remove (fn [b] (contains? deleted-block-ids (:db/id b)))
-                                                     (concat pages blocks))
-                              tx-id (get-in (or refs-tx-report tx-report*) [:tempids :db/current-tx])]
-                          (keep (fn [b]
-                                  (when-let [db-id (:db/id b)]
-                                    (when (:block/uuid (d/entity db-after db-id))
-                                      {:db/id db-id
-                                       :block/tx-id tx-id}))) updated-blocks))))
-          tx-report' (ldb/transact! conn replace-tx {:pipeline-replace? true
-                                                     ;; Ensure db persisted
-                                                     :db-persist? true})
-          _ (when-not (:revert-tx-data? tx-meta)
-              (try
-                (validate-db! repo conn tx-report* tx-meta context)
-                (catch :default e
-                  (when-not (rtc-tx-or-download-graph? tx-meta)
-                    (prn :debug :revert-invalid-tx
-                         :tx-meta
-                         tx-meta
-                         :tx-data
-                         (:tx-data tx-report*))
-                    (reverse-tx! conn (:tx-data tx-report*)))
-                  (throw e))))
+          tx-id-data (let [db-after (:db-after tx-report*)
+                           updated-blocks (remove (fn [b] (contains? deleted-block-ids (:db/id b)))
+                                                  (concat pages blocks))
+                           tx-id (get-in tx-report* [:tempids :db/current-tx])]
+                       (keep (fn [b]
+                               (when-let [db-id (:db/id b)]
+                                 (when (:block/uuid (d/entity db-after db-id))
+                                   {:db/id db-id
+                                    :block/tx-id tx-id}))) updated-blocks))
+          block-refs-tx-id-data (concat block-refs tx-id-data)
+          replace-tx-report (when (seq block-refs-tx-id-data)
+                              (ldb/transact! conn block-refs-tx-id-data {:pipeline-replace? true
+                                                                         ;; Ensure db persisted
+                                                                         :db-persist? true}))
+          tx-report' (or replace-tx-report tx-report*)
           full-tx-data (concat (:tx-data tx-report*)
-                               (:tx-data refs-tx-report)
-                               (:tx-data tx-report'))
+                               (:tx-data replace-tx-report))
           final-tx-report (assoc tx-report'
                                  :tx-data full-tx-data
                                  :tx-meta tx-meta

src/main/frontend/worker/rtc/full_upload_download_graph.cljs

@@ -376,7 +376,7 @@
          {:rtc-download-graph? true
           :gen-undo-ops? false
             ;; only transact db schema, skip validation to avoid warning
-          :frontend.worker.pipeline/skip-validate-db? true
+          :skip-validate-db? true
           :persist-op? false}
          (worker-state/get-context))
         (rtc-log-and-state/rtc-log :rtc.log/download {:sub-type :transact-graph-data-to-db-2
@@ -534,7 +534,7 @@
     {:rtc-download-graph? true
      :gen-undo-ops? false
       ;; only transact db schema, skip validation to avoid warning
-     :frontend.worker.pipeline/skip-validate-db? true
+     :skip-validate-db? true
      :persist-op? false}
     (worker-state/get-context))
    (prn :xxx3 (js/Date.))