diff --git a/deps/db-sync/src/logseq/db_sync/checksum.cljs b/deps/db-sync/src/logseq/db_sync/checksum.cljs index 7acf3d18ab..8adca4e153 100644 --- a/deps/db-sync/src/logseq/db_sync/checksum.cljs +++ b/deps/db-sync/src/logseq/db_sync/checksum.cljs @@ -154,11 +154,6 @@ attr (normalize-checksum-value db attr (:v datom))])))) -(defn- existing-entity-in-db? - [db eid] - (and (number? eid) - (some? (d/entity db eid)))) - (defn recompute-checksum [db] (let [e2ee? (ldb/get-graph-rtc-e2ee? db) @@ -206,21 +201,15 @@ initial-state (if (valid-checksum? checksum) (checksum->state checksum) (checksum->state (recompute-checksum db-before))) - ;; UUID mutation on an existing entity can implicitly affect - ;; normalized parent/page tuples of referencing entities. - ;; Keep incremental logic simple and robust by full recompute. - existing-uuid-mutation? - (some (fn [{:keys [a e]}] - (and (= :block/uuid a) - (existing-entity-in-db? db-before e))) - tx-data) attrs (relevant-attrs after-e2ee?) - removed-tuples (keep #(when (false? (:added %)) - (datom->checksum-tuple db-before attrs %)) - tx-data) - added-tuples (keep #(when (:added %) - (datom->checksum-tuple db-after attrs %)) - tx-data) + removed-tuples (->> tx-data + (keep #(when (false? (:added %)) + (datom->checksum-tuple db-before attrs %))) + set) + added-tuples (->> tx-data + (keep #(when (:added %) + (datom->checksum-tuple db-after attrs %))) + set) state-after-removals (reduce (fn [checksum-state tuple] (subtract-digest checksum-state (tuple-digest tuple))) initial-state @@ -229,6 +218,4 @@ (add-digest checksum-state (tuple-digest tuple))) state-after-removals added-tuples)] - (if existing-uuid-mutation? - (recompute-checksum db-after) - (state->checksum state-after-additions)))))) + (state->checksum state-after-additions))))) diff --git a/deps/db-sync/test/logseq/db_sync/checksum_test.cljs b/deps/db-sync/test/logseq/db_sync/checksum_test.cljs index e0dabc919e..dce70177d0 100644 --- a/deps/db-sync/test/logseq/db_sync/checksum_test.cljs +++ b/deps/db-sync/test/logseq/db_sync/checksum_test.cljs @@ -123,24 +123,6 @@ (is (not= before-checksum full)) (is (= full incremental))))) -(deftest incremental-checksum-matches-recompute-when-referenced-uuid-is-retracted-test - (testing "incremental checksum updates dependents when a referenced entity loses its block UUID" - (let [db-before (sample-db) - parent-uuid (:block/uuid (d/entity db-before 3)) - tx-report (d/with db-before [[:db/retract 3 :block/uuid parent-uuid]]) - full (checksum/recompute-checksum (:db-after tx-report)) - incremental (checksum/update-checksum (checksum/recompute-checksum db-before) tx-report)] - (is (= full incremental))))) - -(deftest incremental-checksum-matches-recompute-when-block-uuid-changes-test - (testing "incremental checksum matches full recompute when an existing block UUID changes" - (let [db-before (sample-db) - new-parent-uuid (random-uuid) - tx-report (d/with db-before [[:db/add 3 :block/uuid new-parent-uuid]]) - full (checksum/recompute-checksum (:db-after tx-report)) - incremental (checksum/update-checksum (checksum/recompute-checksum db-before) tx-report)] - (is (= full incremental))))) - (deftest incremental-checksum-matches-recompute-when-block-is-readded-test (testing "incremental checksum remains equal to recompute when a block is deleted and re-added with the same UUID" (let [db0 (sample-db) diff --git a/deps/db/src/logseq/db/frontend/validate.cljs b/deps/db/src/logseq/db/frontend/validate.cljs index 7d2eb3e111..0a68b2f1a2 100644 --- a/deps/db/src/logseq/db/frontend/validate.cljs +++ b/deps/db/src/logseq/db/frontend/validate.cljs @@ -21,10 +21,35 @@ [closed-schema?] (if closed-schema? closed-db-schema-explainer db-schema-explainer)) +(defn- block-uuid-immutability-errors + [{:keys [db-before db-after tx-data tx-meta]}] + (let [uuid-touched-existing-eids + (->> tx-data + (keep (fn [{:keys [e a]}] + (when (and (= :block/uuid a) + (number? e) + (some? (:block/uuid (d/entity db-before e)))) + e))) + distinct)] + (->> uuid-touched-existing-eids + (keep (fn [eid] + (let [before-uuid (:block/uuid (d/entity db-before eid)) + after-ent (d/entity db-after eid) + after-uuid (:block/uuid after-ent) + deleted? (nil? after-ent)] + (when (and (not deleted?) + (not= before-uuid after-uuid)) + {:entity-map {:db/id eid + :block/uuid before-uuid + :block/uuid-after after-uuid} + :errors {:block/uuid ["immutable for existing entities; use :db/retractEntity to delete entities"]} + :tx-meta tx-meta})))) + vec))) + (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]} {:keys [closed-schema?]}] + [{:keys [db-before db-after tx-data tx-meta] :as tx-report} {:keys [closed-schema?]}] (binding [db-malli-schema/*skip-strict-url-validate?* true] (let [changed-ids (->> tx-data (keep :e) distinct) datoms (d/datoms db-after :eavt) @@ -37,30 +62,34 @@ validator (get-schema-validator closed-schema?)] (binding [db-malli-schema/*db-for-validate-fns* db-after] (let [invalid-ent-maps (remove - ;; remove :db/id as it adds needless declarations to schema - #(validator [(dissoc % :db/id)]) - ent-maps)] - (if (seq invalid-ent-maps) - (do - (prn "Invalid datascript entities detected amongst changed entity ids:" changed-ids :tx-meta tx-meta) - (let [explainer (get-schema-explainer closed-schema?) - errors (doall - (map - (fn [m] - (let [m' (update m :block/properties (fn [properties] - (map (fn [[p v]] - [(:db/ident p) v]) - properties))) - data {:entity-map m' - :errors (me/humanize (explainer [(dissoc m :db/id)]))}] - (try - (pprint/pprint data) - (catch :default _e - (prn data))) - data)) - invalid-ent-maps))] - - [false errors])) + ;; remove :db/id as it adds needless declarations to schema + #(validator [(dissoc % :db/id)]) + ent-maps) + schema-errors + (when (seq invalid-ent-maps) + (prn "Invalid datascript entities detected amongst changed entity ids:" changed-ids :tx-meta tx-meta) + (let [explainer (get-schema-explainer closed-schema?)] + (doall + (map + (fn [m] + (let [m' (update m :block/properties (fn [properties] + (map (fn [[p v]] + [(:db/ident p) v]) + properties))) + data {:entity-map m' + :errors (me/humanize (explainer [(dissoc m :db/id)]))}] + (try + (pprint/pprint data) + (catch :default _e + (prn data))) + data)) + invalid-ent-maps)))) + uuid-errors (block-uuid-immutability-errors tx-report) + errors (->> (concat schema-errors uuid-errors) + (remove nil?) + vec)] + (if (seq errors) + [false errors] [true nil])))))) (defn- group-errors-by-entity diff --git a/deps/db/test/logseq/db_test.cljs b/deps/db/test/logseq/db_test.cljs index 2a64920a63..cf0ed9bec7 100644 --- a/deps/db/test/logseq/db_test.cljs +++ b/deps/db/test/logseq/db_test.cljs @@ -112,6 +112,34 @@ :block/tags :logseq.class/Property}]) (ldb/transact! temp-conn [[:db/retract :logseq.class/Task :block/tags :logseq.class/Property]])))))) +(deftest block-uuid-is-immutable-for-existing-entity-test + (let [conn (db-test/create-conn-with-blocks + {:pages-and-blocks + [{:page {:block/title "page1"}}]}) + page (db-test/find-page-by-title @conn "page1") + db-id (:db/id page) + old-uuid (:block/uuid page) + new-uuid (random-uuid)] + (testing "cannot replace :block/uuid on existing entity" + (is (thrown? js/Error + (db-test/silence-stderr + (ldb/transact! conn [[:db/add db-id :block/uuid new-uuid]])))) + (is (= old-uuid (:block/uuid (d/entity @conn db-id))))) + (testing "cannot retract :block/uuid on existing entity" + (is (thrown? js/Error + (db-test/silence-stderr + (ldb/transact! conn [[:db/retract db-id :block/uuid old-uuid]])))) + (is (= old-uuid (:block/uuid (d/entity @conn db-id))))))) + +(deftest block-uuid-immutability-allows-retract-entity-test + (let [conn (db-test/create-conn-with-blocks + {:pages-and-blocks + [{:page {:block/title "page1"}}]}) + page (db-test/find-page-by-title @conn "page1") + db-id (:db/id page)] + (ldb/transact! conn [[:db/retractEntity db-id]]) + (is (nil? (d/entity @conn db-id))))) + (deftest get-bidirectional-properties (testing "disabled by default" (let [conn (db-test/create-conn-with-blocks