From b538fe2571032600215c40f2bcfab261a7c08dcc Mon Sep 17 00:00:00 2001 From: Tienson Qin Date: Mon, 13 Oct 2025 22:57:21 +0800 Subject: [PATCH] enhance: run db validate before writes --- deps.edn | 3 +- deps/db/deps.edn | 3 +- deps/db/src/logseq/db.cljs | 41 ++++++++- deps/db/src/logseq/db/frontend/validate.cljs | 6 +- deps/outliner/deps.edn | 3 +- deps/outliner/src/logseq/outliner/core.cljs | 24 ++--- src/main/frontend/worker/db_worker.cljs | 10 ++ src/main/frontend/worker/pipeline.cljs | 91 ++++--------------- .../rtc/full_upload_download_graph.cljs | 4 +- 9 files changed, 90 insertions(+), 95 deletions(-) diff --git a/deps.edn b/deps.edn index 7fa7e674e6..8ced3bd842 100644 --- a/deps.edn +++ b/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"} diff --git a/deps/db/deps.edn b/deps/db/deps.edn index 8e609173ff..ea051a1195 100644 --- a/deps/db/deps.edn +++ b/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"} diff --git a/deps/db/src/logseq/db.cljs b/deps/db/src/logseq/db.cljs index c35d7781eb..4e040ecad2 100644 --- a/deps/db/src/logseq/db.cljs +++ b/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) diff --git a/deps/db/src/logseq/db/frontend/validate.cljs b/deps/db/src/logseq/db/frontend/validate.cljs index cab0540220..0591a9a235 100644 --- a/deps/db/src/logseq/db/frontend/validate.cljs +++ b/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) diff --git a/deps/outliner/deps.edn b/deps/outliner/deps.edn index 1e3c1eb693..7c33a1f5ba 100644 --- a/deps/outliner/deps.edn +++ b/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 diff --git a/deps/outliner/src/logseq/outliner/core.cljs b/deps/outliner/src/logseq/outliner/core.cljs index 47e4f46529..8653976045 100644 --- a/deps/outliner/src/logseq/outliner/core.cljs +++ b/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)) diff --git a/src/main/frontend/worker/db_worker.cljs b/src/main/frontend/worker/db_worker.cljs index ae528b852a..e8a0886019 100644 --- a/src/main/frontend/worker/db_worker.cljs +++ b/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 diff --git a/src/main/frontend/worker/pipeline.cljs b/src/main/frontend/worker/pipeline.cljs index cbf2964509..78d76c2c07 100644 --- a/src/main/frontend/worker/pipeline.cljs +++ b/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 diff --git a/src/main/frontend/worker/rtc/full_upload_download_graph.cljs b/src/main/frontend/worker/rtc/full_upload_download_graph.cljs index 53165c3b47..d23a610b6d 100644 --- a/src/main/frontend/worker/rtc/full_upload_download_graph.cljs +++ b/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.))