diff --git a/deps/db-sync/test/logseq/db_sync/normalize_test.cljs b/deps/db-sync/test/logseq/db_sync/normalize_test.cljs index 1e1c217833..f024c60348 100644 --- a/deps/db-sync/test/logseq/db_sync/normalize_test.cljs +++ b/deps/db-sync/test/logseq/db_sync/normalize_test.cljs @@ -112,9 +112,9 @@ (:db-before tx-report) (:tx-data tx-report)) tx-data (mapv op-e-a-v normalized)] - (testing "drops old :block/title retract and keeps new add during title update" + (testing "keeps old :block/title retract and new add during title update" (is (some #(= [:db/add [:block/uuid page-uuid] :block/title "Page 2"] %) tx-data)) - (is (not-any? #(= [:db/retract [:block/uuid page-uuid] :block/title "Page"] %) tx-data))))) + (is (some #(= [:db/retract [:block/uuid page-uuid] :block/title "Page"] %) tx-data))))) (deftest normalize-tx-data-keeps-recreated-normal-blocks-test (testing "retract + recreate for normal blocks should not drop recreated entity datoms" diff --git a/deps/outliner/src/logseq/outliner/op.cljs b/deps/outliner/src/logseq/outliner/op.cljs index 5319093213..ad4f27ea1a 100644 --- a/deps/outliner/src/logseq/outliner/op.cljs +++ b/deps/outliner/src/logseq/outliner/op.cljs @@ -320,19 +320,19 @@ :class-remove-property (apply outliner-property/class-remove-property! conn args) - :upsert-closed-value ; don't support undo/redo + :upsert-closed-value (apply outliner-property/upsert-closed-value! conn args) - :delete-closed-value ; don't support undo/redo + :delete-closed-value (apply outliner-property/delete-closed-value! conn args) - :add-existing-values-to-closed-values ; don't support undo/redo + :add-existing-values-to-closed-values (apply outliner-property/add-existing-values-to-closed-values! conn args) - :batch-import-edn ; don't support undo/redo + :batch-import-edn (apply import-edn-data conn *result args) - :transact ; don't support undo/redo + :transact (apply ldb/transact! conn args) :create-page diff --git a/deps/outliner/src/logseq/outliner/op/construct.cljc b/deps/outliner/src/logseq/outliner/op/construct.cljc index e3cab11080..8be0cf72e8 100644 --- a/deps/outliner/src/logseq/outliner/op/construct.cljc +++ b/deps/outliner/src/logseq/outliner/op/construct.cljc @@ -2,16 +2,15 @@ "Construct canonical forward and reverse outliner ops for history actions." (:require [clojure.string :as string] [datascript.core :as d] + [datascript.impl.entity :as de] [logseq.common.util :as common-util] [logseq.common.util.date-time :as date-time-util] [logseq.common.uuid :as common-uuid] [logseq.db :as ldb] [logseq.db.frontend.content :as db-content] - [logseq.db.frontend.property :as db-property] - [logseq.db.frontend.property.type :as db-property-type] - [logseq.db.common.entity-plus :as entity-plus])) + [logseq.db.frontend.property :as db-property])) -(def ^:private semantic-outliner-ops +(def ^:api semantic-outliner-ops #{:save-block :insert-blocks :apply-template @@ -24,19 +23,7 @@ :delete-page :restore-recycled :recycle-delete-permanently - :set-block-property - :remove-block-property - :batch-set-property - :batch-remove-property - :delete-property-value - :batch-delete-property-value - :create-property-text-block - :upsert-property - :class-add-property - :class-remove-property - :upsert-closed-value - :add-existing-values-to-closed-values - :delete-closed-value}) + :upsert-property}) (def ^:private transient-block-keys #{:db/id @@ -59,10 +46,11 @@ (defn- stable-entity-ref [db x] (cond - (map? x) (let [eid (or (:db/id x) - (when-let [id (:block/uuid x)] - (:db/id (d/entity db [:block/uuid id]))))] - (stable-entity-ref db eid)) + (or (map? x) (de/entity? x)) + (let [eid (or (:db/id x) + (when-let [id (:block/uuid x)] + (:db/id (d/entity db [:block/uuid id]))))] + (stable-entity-ref db eid)) (uuid? x) [:block/uuid x] (and (integer? x) (not (neg? x))) @@ -94,6 +82,16 @@ (or (set? v) (sequential? v)) (set (map #(stable-entity-ref db %) v)) :else (stable-entity-ref db v))) +(defn- sanitize-upsert-property-schema + [db schema] + (reduce-kv (fn [m k v] + (assoc m k + (if (= :logseq.property/classes k) + (sanitize-ref-value db v) + v))) + {} + schema)) + (defn- sanitize-block-refs [refs] (->> refs @@ -230,13 +228,6 @@ (when-let [first-block (some->> ids first (d/entity db))] (resolve-target-and-sibling first-block))) -(defn- stable-property-value - [db property-id v] - (let [property-type (some-> (d/entity db property-id) :logseq.property/type)] - (if (contains? db-property-type/all-ref-property-types property-type) - (sanitize-ref-value db v) - v))) - (defn- created-block-uuids-from-tx-data [tx-data] (->> tx-data @@ -513,52 +504,6 @@ (let [[root-id] args] [:recycle-delete-permanently [(stable-block-uuid db root-id)]]) - :set-block-property - (let [[block-eid property-id v] args - property-id' (stable-entity-ref db property-id)] - [:set-block-property [(stable-entity-ref db block-eid) - property-id' - (stable-property-value db property-id' v)]]) - - :remove-block-property - (let [[block-eid property-id] args] - [:remove-block-property [(stable-entity-ref db block-eid) - (stable-entity-ref db property-id)]]) - - :batch-set-property - (let [[block-ids property-id v opts] args - property-id' (stable-entity-ref db property-id)] - [:batch-set-property [(stable-id-coll db block-ids) - property-id' - (stable-property-value db property-id' v) - opts]]) - - :batch-remove-property - (let [[block-ids property-id] args] - [:batch-remove-property [(stable-id-coll db block-ids) - (stable-entity-ref db property-id)]]) - - :delete-property-value - (let [[block-eid property-id property-value] args - property-id' (stable-entity-ref db property-id)] - [:delete-property-value [(stable-entity-ref db block-eid) - property-id' - (stable-property-value db property-id' property-value)]]) - - :batch-delete-property-value - (let [[block-eids property-id property-value] args - property-id' (stable-entity-ref db property-id)] - [:batch-delete-property-value [(stable-id-coll db block-eids) - property-id' - (stable-property-value db property-id' property-value)]]) - - :create-property-text-block - (let [[block-id property-id value opts] args] - [:create-property-text-block [(stable-entity-ref db block-id) - (stable-entity-ref db property-id) - value - opts]]) - :upsert-property (let [[property-id schema opts] args property-id' (or (stable-entity-ref db property-id) @@ -566,27 +511,6 @@ (created-db-ident-from-tx-data tx-data))] [:upsert-property [property-id' schema opts]]) - :class-add-property - (let [[class-id property-id] args] - [:class-add-property [(stable-entity-ref db class-id) (stable-entity-ref db property-id)]]) - - :class-remove-property - (let [[class-id property-id] args] - [:class-remove-property [(stable-entity-ref db class-id) (stable-entity-ref db property-id)]]) - - :upsert-closed-value - (let [[property-id opts] args] - [:upsert-closed-value [(stable-entity-ref db property-id) opts]]) - - :add-existing-values-to-closed-values - (let [[property-id values] args] - [:add-existing-values-to-closed-values [(stable-entity-ref db property-id) values]]) - - :delete-closed-value - (let [[property-id value-block-id] args] - [:delete-closed-value [(stable-entity-ref db property-id) - (stable-block-ref-with-tx-data db tx-data value-block-id)]]) - [op args])) (defn- save-block-keys @@ -640,154 +564,6 @@ keys-to-restore)] [:save-block [inverse-block opts]]))) -(defn- property-ref-value - [db property-id value] - (let [property-type (some-> (d/entity db property-id) :logseq.property/type)] - (cond - ;; Number property values are stored as ref entities but the semantic op - ;; uses scalar content for undo/redo payloads. - (= :number property-type) - (let [to-content (fn [v] - (if (some? (:db/id v)) - (or (db-property/property-value-content v) v) - v))] - (cond - (set? value) (set (map to-content value)) - (sequential? value) (mapv to-content value) - :else (to-content value))) - - (contains? db-property-type/all-ref-property-types property-type) - (sanitize-ref-value db value) - - :else - value))) - -(defn- block-property-value - [db block-id property-id] - ;; `get` lookup may include derived defaults. - (when-let [value (entity-plus/lookup-entity (d/entity db block-id) property-id)] - (property-ref-value db property-id value))) - -(defn- property-history-refs-from-tx-data - [db-before db-after tx-data block-ids property-id] - (let [target-block-ids (->> block-ids - (keep (fn [block-id] - (:db/id (block-entity db-before block-id)))) - set) - target-property-id (some-> (d/entity db-before property-id) :db/id) - history-eid->block-id (reduce (fn [acc d] - (if (and (:added d) - (= :logseq.property.history/block (:a d))) - (assoc acc (:e d) (:v d)) - acc)) - {} - tx-data) - history-eid->property-id (reduce (fn [acc d] - (if (and (:added d) - (= :logseq.property.history/property (:a d))) - (assoc acc (:e d) (:v d)) - acc)) - {} - tx-data)] - (->> history-eid->block-id - (keep (fn [[history-eid history-block-id]] - (when (and (contains? target-block-ids history-block-id) - (= target-property-id (get history-eid->property-id history-eid)) - (nil? (d/entity db-before history-eid))) - (stable-entity-ref db-after history-eid)))) - distinct - vec - seq))) - -(defn- normalize-op-or-ops - [op-or-ops] - (cond - (nil? op-or-ops) - [] - - (and (sequential? op-or-ops) - (seq op-or-ops) - (sequential? (first op-or-ops))) - (vec op-or-ops) - - :else - [op-or-ops])) - -(defn- prepend-history-cleanup-op - [cleanup-op op-or-ops] - (let [ops (normalize-op-or-ops op-or-ops) - ops' (if cleanup-op - (into [cleanup-op] ops) - ops)] - (seq ops'))) - -(defn- property-history-cleanup-op - [db-before db-after tx-data block-ids property-id] - (when-let [history-refs (property-history-refs-from-tx-data - db-before - db-after - tx-data - block-ids - property-id)] - [:delete-blocks [history-refs {}]])) - -(defn- restore-property-op - [before-value block-ref property-id {:keys [remove-when-nil?]}] - (if (nil? before-value) - (when remove-when-nil? - [:remove-block-property [block-ref property-id]]) - [:set-block-property [block-ref property-id before-value]])) - -(defn- inverse-property-ops-for-blocks - [db-before block-ids property-id restore-opts] - (->> block-ids - (keep (fn [block-id] - (let [before-value (block-property-value db-before block-id property-id) - block-ref (stable-entity-ref db-before block-id)] - (restore-property-op before-value block-ref property-id restore-opts)))) - vec - seq)) - -(defn- inverse-property-change-op - [db-before db-after tx-data block-ids property-id restore-opts] - (let [cleanup-op (property-history-cleanup-op - db-before - db-after - tx-data - block-ids - property-id) - inverse-ops (inverse-property-ops-for-blocks - db-before - block-ids - property-id - restore-opts)] - (prepend-history-cleanup-op cleanup-op inverse-ops))) - -(defn- inverse-property-op - [db-before db-after tx-data op args] - (case op - :set-block-property - (let [[block-id property-id _value] args] - (inverse-property-change-op - db-before db-after tx-data [block-id] property-id {:remove-when-nil? true})) - - :remove-block-property - (let [[block-id property-id] args] - (inverse-property-change-op - db-before db-after tx-data [block-id] property-id {:remove-when-nil? false})) - - :batch-set-property - (let [[block-ids property-id _value _opts] args] - (inverse-property-change-op - db-before db-after tx-data block-ids property-id {:remove-when-nil? true})) - - :batch-remove-property - (let [[block-ids property-id _opts] args] - (inverse-property-change-op - db-before db-after tx-data block-ids property-id {:remove-when-nil? false})) - - nil)) - (defn- build-insert-block-payload [db-before ent] (when-let [block-uuid (:block/uuid ent)] @@ -1030,47 +806,17 @@ (let [[page-uuid _opts] args] (build-inverse-delete-page db-before page-uuid)) - :set-block-property - (inverse-property-op db-before db-after tx-data op args) - - :remove-block-property - (inverse-property-op db-before db-after tx-data op args) - - :batch-set-property - (inverse-property-op db-before db-after tx-data op args) - - :batch-remove-property - (inverse-property-op db-before db-after tx-data op args) - - :create-property-text-block - (let [[_block-id _property-id _value opts] args - new-block-id (:new-block-id opts) - new-block-ref (cond - (vector? new-block-id) - new-block-id - - (uuid? new-block-id) - [:block/uuid new-block-id] - - :else - (stable-entity-ref db-before new-block-id))] - (when new-block-ref - [:delete-blocks [[new-block-ref] {}]])) - - :class-add-property - (let [[class-id property-id] args] - [:class-remove-property [(stable-entity-ref db-before class-id) - (stable-entity-ref db-before property-id)]]) - - :class-remove-property - (let [[class-id property-id] args] - [:class-add-property [(stable-entity-ref db-before class-id) - (stable-entity-ref db-before property-id)]]) - :upsert-property (let [[property-id _schema _opts] args] (when (qualified-keyword? property-id) - [:delete-page [(common-uuid/gen-uuid :db-ident-block-uuid property-id) {}]])) + (if-let [property (d/entity db-before property-id)] + [:upsert-property + [property-id + (sanitize-upsert-property-schema + db-before + (db-property/get-property-schema (into {} property))) + {:property-name (:block/title property)}]] + [:delete-page [(common-uuid/gen-uuid :db-ident-block-uuid property-id) {}]]))) nil)] (if (and (sequential? inverse-entry) @@ -1098,29 +844,55 @@ (defn contains-transact-op? [ops] - (some (fn [[op]] - (= :transact op)) - ops)) + (let [ops' (cond + (nil? ops) + nil + + (and (sequential? ops) + (keyword? (first ops))) + [(vec ops)] + + :else + ops)] + (some (fn [entry] + (and (sequential? entry) + (= :transact (first entry)))) + ops'))) + +(defn- normalize-op-entries + [ops] + (let [ops' (some-> ops seq vec)] + (cond + (nil? ops') + nil + + (and (keyword? (first ops')) + (vector? (second ops'))) + [ops'] + + :else + ops'))) (defn- canonicalize-explicit-outliner-ops [db tx-data ops] - (cond - (nil? ops) + (let [ops' (normalize-op-entries ops)] + (cond + (nil? ops') nil - (seq ops) - (->> ops - (mapcat (fn [op] - (let [canonicalized-op (canonicalize-semantic-outliner-op db tx-data op)] - (if (and (sequential? canonicalized-op) - (sequential? (first canonicalized-op)) - (keyword? (ffirst canonicalized-op))) - canonicalized-op - [canonicalized-op])))) - vec) + (seq ops') + (->> ops' + (mapcat (fn [op] + (let [canonicalized-op (canonicalize-semantic-outliner-op db tx-data op)] + (if (and (sequential? canonicalized-op) + (sequential? (first canonicalized-op)) + (keyword? (ffirst canonicalized-op))) + canonicalized-op + [canonicalized-op])))) + vec) - :else - nil)) + :else + nil))) (defn- patch-inverse-delete-block-ops [inverse-outliner-ops forward-outliner-ops] @@ -1157,8 +929,8 @@ (defn- canonicalize-outliner-ops [db tx-meta tx-data] - (let [explicit-forward-ops (:db-sync/forward-outliner-ops tx-meta) - outliner-ops (:outliner-ops tx-meta)] + (let [explicit-forward-ops (normalize-op-entries (:db-sync/forward-outliner-ops tx-meta)) + outliner-ops (normalize-op-entries (:outliner-ops tx-meta))] (cond (seq explicit-forward-ops) (canonicalize-explicit-outliner-ops db tx-data explicit-forward-ops) @@ -1178,10 +950,6 @@ (and (integer? x) (not (neg? x)))) -(defn- unresolved-numeric-entity-id-coll? - [xs] - (some unresolved-numeric-entity-id? xs)) - (defn- numeric-id-in-ref-value? [v] (cond @@ -1244,83 +1012,6 @@ nil)) -(defn- stale-numeric-id-in-structure-ops? - [op args] - (case op - :move-blocks - (let [[ids target-id _opts] args] - (or (unresolved-numeric-entity-id-coll? ids) - (unresolved-numeric-entity-id? target-id))) - - :move-blocks-up-down - (let [[ids _up?] args] - (unresolved-numeric-entity-id-coll? ids)) - - :indent-outdent-blocks - (let [[ids _indent? _opts] args] - (unresolved-numeric-entity-id-coll? ids)) - - :delete-blocks - (let [[ids _opts] args] - (unresolved-numeric-entity-id-coll? ids)) - - nil)) - -(defn- stale-numeric-id-in-property-ops? - [op args] - (case op - :set-block-property - (let [[block-id property-id _v] args] - (or (unresolved-numeric-entity-id? block-id) - (unresolved-numeric-entity-id? property-id))) - - :remove-block-property - (let [[block-id property-id] args] - (or (unresolved-numeric-entity-id? block-id) - (unresolved-numeric-entity-id? property-id))) - - :batch-set-property - (let [[block-ids property-id _v _opts] args] - (or (unresolved-numeric-entity-id-coll? block-ids) - (unresolved-numeric-entity-id? property-id))) - - :batch-remove-property - (let [[block-ids property-id] args] - (or (unresolved-numeric-entity-id-coll? block-ids) - (unresolved-numeric-entity-id? property-id))) - - :delete-property-value - (let [[block-id property-id _property-value] args] - (or (unresolved-numeric-entity-id? block-id) - (unresolved-numeric-entity-id? property-id))) - - :batch-delete-property-value - (let [[block-ids property-id _property-value] args] - (or (unresolved-numeric-entity-id-coll? block-ids) - (unresolved-numeric-entity-id? property-id))) - - :create-property-text-block - (let [[block-id property-id _value _opts] args] - (or (unresolved-numeric-entity-id? block-id) - (unresolved-numeric-entity-id? property-id))) - - :class-add-property - (let [[class-id property-id] args] - (or (unresolved-numeric-entity-id? class-id) - (unresolved-numeric-entity-id? property-id))) - - :class-remove-property - (let [[class-id property-id] args] - (or (unresolved-numeric-entity-id? class-id) - (unresolved-numeric-entity-id? property-id))) - - :delete-closed-value - (let [[property-id value-block-id] args] - (or (unresolved-numeric-entity-id? property-id) - (unresolved-numeric-entity-id? value-block-id))) - - nil)) - (defn- stale-numeric-id-in-schema-ops? [op args] (case op @@ -1328,14 +1019,6 @@ (let [[property-id _schema _opts] args] (unresolved-numeric-entity-id? property-id)) - :upsert-closed-value - (let [[property-id _opts] args] - (unresolved-numeric-entity-id? property-id)) - - :add-existing-values-to-closed-values - (let [[property-id _values] args] - (unresolved-numeric-entity-id? property-id)) - nil)) (defn- stale-numeric-id-in-op? @@ -1343,8 +1026,6 @@ (and (not= :transact op) (boolean (or (stale-numeric-id-in-page-ops? db op args) - (stale-numeric-id-in-structure-ops? op args) - (stale-numeric-id-in-property-ops? op args) (stale-numeric-id-in-schema-ops? op args))))) (defn- assert-no-stale-numeric-ids! @@ -1400,8 +1081,8 @@ (nil? explicit-inverse-outliner-ops) nil - ;; Treat explicit transact placeholder as "no semantic inverse". - ;; Keep nil so semantic replay must fail-fast when required. + ;; Treat explicit transact placeholder as "no semantic inverse". + ;; Keep nil so semantic replay must fail-fast when required. (= canonical-transact-op explicit-inverse-outliner-ops) nil diff --git a/deps/outliner/src/logseq/outliner/property.cljs b/deps/outliner/src/logseq/outliner/property.cljs index e02c584971..db8572a1bc 100644 --- a/deps/outliner/src/logseq/outliner/property.cljs +++ b/deps/outliner/src/logseq/outliner/property.cljs @@ -19,7 +19,6 @@ [logseq.db.sqlite.util :as sqlite-util] [logseq.outliner.core :as outliner-core] [logseq.outliner.page :as outliner-page] - [logseq.outliner.tx-meta :as outliner-tx-meta] [logseq.outliner.validate :as outliner-validate] [malli.core :as m] [malli.error :as me] @@ -274,16 +273,6 @@ [id] (if (uuid? id) [:block/uuid id] id)) -(defn- with-op-entry - [op-entry f] - (binding [outliner-tx-meta/*outliner-op-entry* - (or outliner-tx-meta/*outliner-op-entry* op-entry)] - (f))) - -(defn- transact-with-op! - [conn tx-data tx-meta] - (ldb/transact! conn tx-data (outliner-tx-meta/ensure-outliner-ops tx-meta nil))) - (defn- raw-set-block-property! "Adds the raw property pair (value not modified) to the given block if the property value is valid" [conn block property new-value] @@ -291,34 +280,35 @@ (throw-error-if-invalid-property-value @conn property new-value) (let [property-id (:db/ident property) tx-data (build-property-value-tx-data conn block property-id new-value)] - (transact-with-op! conn tx-data {:outliner-op :save-block}))) + (ldb/transact! conn tx-data {:outliner-op :save-block}))) (defn create-property-text-block! "Creates a property value block for the given property and value. Adds it to block if given block." [conn block-id property-id value {:keys [new-block-id]}] - (with-op-entry - [:create-property-text-block [block-id property-id value {:new-block-id new-block-id}]] - (fn [] - (let [property (d/entity @conn property-id) - block (when block-id (d/entity @conn block-id)) - _ (assert (some? property) (str "Property " property-id " doesn't exist yet")) - value' (convert-property-input-string (:logseq.property/type block) - property value) - _ (when (and (not= (:logseq.property/type property) :number) - (not (string? value'))) - (throw (ex-info "value should be a string" {:block-id block-id - :property-id property-id - :value value'}))) - new-value-block (cond-> (db-property-build/build-property-value-block (or block property) property value') - new-block-id - (assoc :block/uuid new-block-id))] - (transact-with-op! conn [new-value-block] {:outliner-op :insert-blocks}) - (let [property-id (:db/ident property)] - (when (and property-id block) - (when-let [block-id (:db/id (d/entity @conn [:block/uuid (:block/uuid new-value-block)]))] - (raw-set-block-property! conn block property block-id))) - (:block/uuid new-value-block)))))) + (let [property (d/entity @conn property-id) + block (when block-id (d/entity @conn block-id)) + _ (assert (some? property) (str "Property " property-id " doesn't exist yet")) + value' (convert-property-input-string (:logseq.property/type block) + property value) + _ (when (and (not= (:logseq.property/type property) :number) + (not (string? value'))) + (throw (ex-info "value should be a string" {:block-id block-id + :property-id property-id + :value value'}))) + new-value-block (cond-> (db-property-build/build-property-value-block (or block property) property value') + new-block-id + (assoc :block/uuid new-block-id))] + (ldb/batch-transact-with-temp-conn! + conn + {:outliner-op :create-property-text-block} + (fn [conn] + (ldb/transact! conn [new-value-block] {:outliner-op :insert-blocks}) + (let [property-id (:db/ident property)] + (when (and property-id block) + (when-let [block-id (:db/id (d/entity @conn [:block/uuid (:block/uuid new-value-block)]))] + (raw-set-block-property! conn block property block-id)))))) + (:block/uuid new-value-block))) (defn- get-property-value-eid [db property-id raw-value] @@ -418,56 +408,54 @@ (defn batch-remove-property! [conn block-ids property-id] - (with-op-entry - [:batch-remove-property [block-ids property-id]] - (fn [] - (throw-error-if-read-only-property property-id) - (let [block-eids (map ->eid block-ids) - blocks (keep (fn [id] (d/entity @conn id)) block-eids) - block-id-set (set (map :db/id blocks))] - (validate-batch-deletion-of-property blocks property-id) - (when (seq blocks) - (when-let [property (d/entity @conn property-id)] - (let [txs (mapcat - (fn [block] - (let [value (get block property-id) - entities (cond - (de/entity? value) [value] - (and (sequential? value) (every? de/entity? value)) value - :else nil) - deleting-entities (filter - (fn [value] - (let [value-referrers* - (d/q '[:find [?e ...] - :in $ ?property-id ?value-id - :where - [?e ?property-id ?value-id]] - @conn - (:db/ident property) - (:db/id value)) - value-referrers - (cond - (nil? value-referrers*) - #{} + (throw-error-if-read-only-property property-id) + (let [block-eids (map ->eid block-ids) + blocks (keep (fn [id] (d/entity @conn id)) block-eids) + block-id-set (set (map :db/id blocks))] + (validate-batch-deletion-of-property blocks property-id) + (when (seq blocks) + (when-let [property (d/entity @conn property-id)] + (let [txs (mapcat + (fn [block] + (let [value (get block property-id) + entities (cond + (de/entity? value) [value] + (and (sequential? value) (every? de/entity? value)) value + :else nil) + deleting-entities (filter + (fn [value] + (let [value-referrers* + (d/q '[:find [?e ...] + :in $ ?property-id ?value-id + :where + [?e ?property-id ?value-id]] + @conn + (:db/ident property) + (:db/id value)) + value-referrers + (cond + (nil? value-referrers*) + #{} - (coll? value-referrers*) - (set value-referrers*) + (coll? value-referrers*) + (set value-referrers*) - :else - #{value-referrers*})] - (and - (:logseq.property/created-from-property value) - (not (or (entity-util/page? value) (ldb/closed-value? value))) - (empty? (set/difference value-referrers block-id-set))))) - entities) - retract-blocks-tx (when (seq deleting-entities) - (:tx-data (outliner-core/delete-blocks @conn deleting-entities {})))] - (concat - [[:db/retract (:db/id block) (:db/ident property)]] - retract-blocks-tx))) - blocks)] - (when (seq txs) - (transact-with-op! conn txs {:outliner-op :save-block}))))))))) + :else + #{value-referrers*})] + (and + (:logseq.property/created-from-property value) + (not (or (entity-util/page? value) (ldb/closed-value? value))) + (empty? (set/difference value-referrers block-id-set))))) + entities) + retract-blocks-tx (when (seq deleting-entities) + (:tx-data (outliner-core/delete-blocks @conn deleting-entities {})))] + (concat + [[:db/retract (:db/id block) (:db/ident property)]] + retract-blocks-tx))) + blocks)] + (when (seq txs) + (ldb/transact! conn txs + {:outliner-op :batch-remove-property}))))))) (defn batch-set-property! "Sets properties for multiple blocks. Automatically handles property value refs. @@ -475,96 +463,92 @@ ([conn block-ids property-id v] (batch-set-property! conn block-ids property-id v {})) ([conn block-ids property-id v options] - (with-op-entry - [:batch-set-property [block-ids property-id v options]] - (fn [] - (assert property-id "property-id is nil") - (throw-error-if-read-only-property property-id) - (if (nil? v) - (batch-remove-property! conn block-ids property-id) - (let [block-eids (map ->eid block-ids) - _ (when (= property-id :block/tags) - (outliner-validate/validate-tags-property @conn block-eids v)) - property (d/entity @conn property-id) - _ (when (= (:db/ident property) :logseq.property.class/extends) - (outliner-validate/validate-extends-property - @conn - (if (number? v) (d/entity @conn v) v) - (map #(d/entity @conn %) block-eids))) - _ (when (nil? property) - (throw (ex-info (str "Property " property-id " doesn't exist yet") {:property-id property-id}))) - property-type (get property :logseq.property/type :default) - entity-id? (and (:entity-id? options) (number? v)) - ref? (contains? db-property-type/all-ref-property-types property-type) - default-url-not-closed? (and (contains? #{:default :url} property-type) - (not (seq (entity-plus/lookup-kv-then-entity property :property/closed-values)))) - v' (if (and ref? (not entity-id?)) - (convert-ref-property-value conn property-id v property-type) - v) - _ (when (nil? v') - (throw (ex-info "Property value must be not nil" {:v v}))) - txs (doall - (mapcat - (fn [eid] - (if-let [block (d/entity @conn eid)] - (let [v' (if (and default-url-not-closed? - (not (and (keyword? v) entity-id?))) - (do - (when (number? v') - (throw-error-if-invalid-property-value @conn property v')) - (let [v (if (number? v') (:block/title (d/entity @conn v')) v')] - (convert-ref-property-value conn property-id v property-type))) - v')] - (throw-error-if-self-value block v' ref?) - (throw-error-if-invalid-property-value @conn property v') - (build-property-value-tx-data conn block property-id v')) - (js/console.error "Skipping setting a block's property because the block id could not be found:" eid))) - block-eids))] - (when (seq txs) - (transact-with-op! conn txs {:outliner-op :save-block})))))))) + (assert property-id "property-id is nil") + (throw-error-if-read-only-property property-id) + (if (nil? v) + (batch-remove-property! conn block-ids property-id) + (let [block-eids (map ->eid block-ids) + _ (when (= property-id :block/tags) + (outliner-validate/validate-tags-property @conn block-eids v)) + property (d/entity @conn property-id) + blocks (keep #(d/entity @conn %) block-eids) + _ (when (= (:db/ident property) :logseq.property.class/extends) + (outliner-validate/validate-extends-property + @conn + (if (number? v) (d/entity @conn v) v) + blocks)) + _ (when (nil? property) + (throw (ex-info (str "Property " property-id " doesn't exist yet") {:property-id property-id}))) + property-type (get property :logseq.property/type :default) + entity-id? (and (:entity-id? options) (number? v)) + ref? (contains? db-property-type/all-ref-property-types property-type) + default-url-not-closed? (and (contains? #{:default :url} property-type) + (not (seq (entity-plus/lookup-kv-then-entity property :property/closed-values)))) + v' (if (and ref? (not entity-id?)) + (convert-ref-property-value conn property-id v property-type) + v) + _ (when (nil? v') + (throw (ex-info "Property value must be not nil" {:v v}))) + txs (doall + (mapcat + (fn [eid] + (if-let [block (d/entity @conn eid)] + (let [v' (if (and default-url-not-closed? + (not (and (keyword? v) entity-id?))) + (do + (when (number? v') + (throw-error-if-invalid-property-value @conn property v')) + (let [v (if (number? v') (:block/title (d/entity @conn v')) v')] + (convert-ref-property-value conn property-id v property-type))) + v')] + (throw-error-if-self-value block v' ref?) + (throw-error-if-invalid-property-value @conn property v') + (build-property-value-tx-data conn block property-id v')) + (js/console.error "Skipping setting a block's property because the block id could not be found:" eid))) + block-eids))] + (when (seq txs) + (ldb/transact! conn txs {:outliner-op :batch-set-property})))))) (defn remove-block-property! [conn eid property-id] - (with-op-entry - [:remove-block-property [eid property-id]] - (fn [] - (throw-error-if-read-only-property property-id) - (let [eid (->eid eid) - block (d/entity @conn eid) - property (d/entity @conn property-id)] - (when-not (= :logseq.property.class/extends property-id) - (validate-batch-deletion-of-property [block] property-id)) - (when block - (cond - (= :logseq.property/empty-placeholder (:db/ident (get block property-id))) - nil + (throw-error-if-read-only-property property-id) + (let [eid (->eid eid) + block (d/entity @conn eid) + property (d/entity @conn property-id) + tx-meta {:outliner-op :remove-block-property}] + (when-not (= :logseq.property.class/extends property-id) + (validate-batch-deletion-of-property [block] property-id)) + (when block + (cond + (= :logseq.property/empty-placeholder (:db/ident (get block property-id))) + nil - (= :logseq.property/status property-id) - (transact-with-op! conn - [[:db/retract (:db/id block) property-id] - [:db/retract (:db/id block) :block/tags :logseq.class/Task]] - {:outliner-op :save-block}) + (= :logseq.property/status property-id) + (ldb/transact! conn + [[:db/retract (:db/id block) property-id] + [:db/retract (:db/id block) :block/tags :logseq.class/Task]] + tx-meta) - (and (:logseq.property/default-value property) - (= (:logseq.property/default-value property) (get block property-id))) - (transact-with-op! conn - [{:db/id (:db/id block) - property-id :logseq.property/empty-placeholder}] - {:outliner-op :save-block}) + (and (:logseq.property/default-value property) + (= (:logseq.property/default-value property) (get block property-id))) + (ldb/transact! conn + [{:db/id (:db/id block) + property-id :logseq.property/empty-placeholder}] + tx-meta) - (and (ldb/class? block) (= property-id :logseq.property.class/extends)) - (transact-with-op! conn - [[:db/retract (:db/id block) :logseq.property.class/extends] - [:db/add (:db/id block) :logseq.property.class/extends :logseq.class/Root]] - {:outliner-op :save-block}) + (and (ldb/class? block) (= property-id :logseq.property.class/extends)) + (ldb/transact! conn + [[:db/retract (:db/id block) :logseq.property.class/extends] + [:db/add (:db/id block) :logseq.property.class/extends :logseq.class/Root]] + tx-meta) - (contains? db-property/db-attribute-properties property-id) - (transact-with-op! conn - [[:db/retract (:db/id block) property-id]] - {:outliner-op :save-block}) + (contains? db-property/db-attribute-properties property-id) + (ldb/transact! conn + [[:db/retract (:db/id block) property-id]] + tx-meta) - :else - (batch-remove-property! conn [eid] property-id))))))) + :else + (batch-remove-property! conn [eid] property-id))))) (defn- set-block-db-attribute! [conn db block property property-id v] @@ -574,8 +558,8 @@ [{:db/id (:db/id block) property-id v}] (= property-id :logseq.property.class/extends) (conj [:db/retract (:db/id block) :logseq.property.class/extends :logseq.class/Root]))] - (transact-with-op! conn tx-data - {:outliner-op :save-block})))) + (ldb/transact! conn tx-data + {:outliner-op :save-block})))) (defn ^:large-vars/cleanup-todo set-block-property! "Updates a block property's value for an existing property-id and block. If @@ -583,9 +567,10 @@ can pass \"value\" instead of the property value entity. Also handle db attributes as properties" [conn block-eid property-id v] - (with-op-entry - [:set-block-property [block-eid property-id v]] - (fn [] + (ldb/batch-transact-with-temp-conn! + conn + {:outliner-op :set-block-property} + (fn [conn] (throw-error-if-read-only-property property-id) (let [db @conn block-eid (->eid block-eid) @@ -646,78 +631,73 @@ with the given property-id or :property-name option. When a property is created it is ensured to have a unique :db/ident" [conn property-id schema {:keys [property-name properties] :as opts}] - (with-op-entry - [:upsert-property [property-id schema opts]] - (fn [] - (let [db @conn - db-ident (or property-id - (try (db-property/create-user-property-ident-from-name property-name) - (catch :default e - (throw (ex-info (str e) - {:type :notification - :payload {:message "Property failed to create. Please try a different property name." - :type :error}})))))] - (assert (qualified-keyword? db-ident)) - (when (and (contains? #{:checkbox} (:logseq.property/type schema)) - (= :db.cardinality/many (:db/cardinality schema))) - (throw (ex-info ":checkbox property doesn't allow multiple values" - {:property-id property-id - :schema schema}))) - (if-let [property (and (qualified-keyword? property-id) (d/entity db db-ident))] - (update-property conn db-ident property schema opts) - (let [k-name (or (and property-name (name property-name)) - (name property-id)) - db-ident' (db-ident/ensure-unique-db-ident @conn db-ident)] - (assert (some? k-name) - (prn "property-id: " property-id ", property-name: " property-name)) - (outliner-validate/validate-page-title k-name {:node {:db/ident db-ident'}}) - (outliner-validate/validate-page-title-characters k-name {:node {:db/ident db-ident'}}) - (let [db-id (:db/id properties) - opts' (cond-> {:title k-name - :properties properties} - (integer? db-id) - (assoc :block-uuid (:block/uuid (d/entity db db-id))))] - (transact-with-op! conn - (concat - [(sqlite-util/build-new-property db-ident' schema opts')] - (when db-id - [[:db/retract db-id :block/tags :logseq.class/Page]])) - {:outliner-op :upsert-property})) - (d/entity @conn db-ident'))))))) + (let [db @conn + db-ident (or property-id + (try (db-property/create-user-property-ident-from-name property-name) + (catch :default e + (throw (ex-info (str e) + {:type :notification + :payload {:message "Property failed to create. Please try a different property name." + :type :error}})))))] + (assert (qualified-keyword? db-ident)) + (when (and (contains? #{:checkbox} (:logseq.property/type schema)) + (= :db.cardinality/many (:db/cardinality schema))) + (throw (ex-info ":checkbox property doesn't allow multiple values" + {:property-id property-id + :schema schema}))) + (if-let [property (and (qualified-keyword? property-id) (d/entity db db-ident))] + (update-property conn db-ident property schema opts) + (let [k-name (or (and property-name (name property-name)) + (name property-id)) + db-ident' (db-ident/ensure-unique-db-ident @conn db-ident)] + (assert (some? k-name) + (prn "property-id: " property-id ", property-name: " property-name)) + (outliner-validate/validate-page-title k-name {:node {:db/ident db-ident'}}) + (outliner-validate/validate-page-title-characters k-name {:node {:db/ident db-ident'}}) + (let [db-id (:db/id properties) + opts' (cond-> {:title k-name + :properties properties} + (integer? db-id) + (assoc :block-uuid (:block/uuid (d/entity db db-id)))) + tx-data (concat + [(sqlite-util/build-new-property db-ident' schema opts')] + (when db-id + [[:db/retract db-id :block/tags :logseq.class/Page]]))] + (ldb/transact! conn tx-data + {:outliner-op :upsert-property})) + (d/entity @conn db-ident'))))) (defn batch-delete-property-value! "batch delete value when a property has multiple values" [conn block-eids property-id property-value] - (with-op-entry - [:batch-delete-property-value [block-eids property-id property-value]] - (fn [] - (when-let [property (d/entity @conn property-id)] - (when (and (db-property/many? property) - (not (some #(= property-id (:db/ident (d/entity @conn %))) block-eids))) - (when (= property-id :block/tags) - (outliner-validate/validate-tags-property-deletion @conn block-eids property-value)) - (if (= property-id :block/tags) - (let [tx-data (map (fn [id] [:db/retract id property-id property-value]) block-eids)] - (transact-with-op! conn tx-data {:outliner-op :save-block})) - (doseq [block-eid block-eids] - (when-let [block (d/entity @conn block-eid)] - (let [current-val (get block property-id) - fv (first current-val)] - (if (and (= 1 (count current-val)) - (or (= property-value fv) - (= property-value (:db/id fv)))) - (remove-block-property! conn (:db/id block) property-id) - (transact-with-op! conn - [[:db/retract (:db/id block) property-id property-value]] - {:outliner-op :save-block}))))))))))) + (ldb/batch-transact-with-temp-conn! + conn + {:outliner-op :batch-delete-property-value} + (fn [conn] + (when-let [property (d/entity @conn property-id)] + (when (and (db-property/many? property) + (not (some #(= property-id (:db/ident (d/entity @conn %))) block-eids))) + (when (= property-id :block/tags) + (outliner-validate/validate-tags-property-deletion @conn block-eids property-value)) + (if (= property-id :block/tags) + (let [tx-data (map (fn [id] [:db/retract id property-id property-value]) block-eids)] + (ldb/transact! conn tx-data {:outliner-op :save-block})) + (doseq [block-eid block-eids] + (when-let [block (d/entity @conn block-eid)] + (let [current-val (get block property-id) + fv (first current-val)] + (if (and (= 1 (count current-val)) + (or (= property-value fv) + (= property-value (:db/id fv)))) + (remove-block-property! conn (:db/id block) property-id) + (ldb/transact! conn + [[:db/retract (:db/id block) property-id property-value]] + {:outliner-op :save-block}))))))))))) (defn delete-property-value! "Delete value if a property has multiple values" [conn block-eid property-id property-value] - (with-op-entry - [:delete-property-value [block-eid property-id property-value]] - (fn [] - (batch-delete-property-value! conn [block-eid] property-id property-value)))) + (batch-delete-property-value! conn [block-eid] property-id property-value)) (defn ^:api get-classes-parents [tags] @@ -830,120 +810,115 @@ (defn upsert-closed-value! "id should be a block UUID or nil" [conn property-id {:keys [id value description _scoped-class-id] :as opts}] - (with-op-entry - [:upsert-closed-value [property-id opts]] - (fn [] - (assert (or (nil? id) (uuid? id))) - (let [db @conn - property (d/entity db property-id) - property-type (:logseq.property/type property)] - (when (contains? db-property-type/closed-value-property-types property-type) - (let [value' (if (string? value) (string/trim value) value) - resolved-value (convert-property-input-string nil property value') - validate-message (validate-property-value-aux - (get-property-value-schema @conn property-type property {:new-closed-value? true}) - resolved-value - {:many? (db-property/many? property)})] - (cond - (some (fn [b] - (and (= (str resolved-value) (str (or (db-property/closed-value-content b) - (:block/uuid b)))) - (not= id (:block/uuid b)))) - (entity-plus/lookup-kv-then-entity property :property/closed-values)) - (throw (ex-info "Closed value choice already exists" - {:error :value-exists - :type :notification - :payload {:message "Choice already exists" - :type :warning}})) + (assert (or (nil? id) (uuid? id))) + (let [db @conn + property (d/entity db property-id) + property-type (:logseq.property/type property)] + (when (contains? db-property-type/closed-value-property-types property-type) + (let [value' (if (string? value) (string/trim value) value) + resolved-value (convert-property-input-string nil property value') + validate-message (validate-property-value-aux + (get-property-value-schema @conn property-type property {:new-closed-value? true}) + resolved-value + {:many? (db-property/many? property)})] + (cond + (some (fn [b] + (and (= (str resolved-value) (str (or (db-property/closed-value-content b) + (:block/uuid b)))) + (not= id (:block/uuid b)))) + (entity-plus/lookup-kv-then-entity property :property/closed-values)) + (throw (ex-info "Closed value choice already exists" + {:error :value-exists + :type :notification + :payload {:message "Choice already exists" + :type :warning}})) - validate-message - (throw (ex-info "Invalid property value" - {:error :value-invalid - :type :notification - :payload {:message validate-message - :type :warning}})) + validate-message + (throw (ex-info "Invalid property value" + {:error :value-invalid + :type :notification + :payload {:message validate-message + :type :warning}})) - (nil? resolved-value) - nil + (nil? resolved-value) + nil - :else - (let [tx-data (build-closed-value-tx @conn property resolved-value opts)] - (transact-with-op! conn tx-data {:outliner-op :insert-blocks}) - (when (seq description) - (if-let [desc-ent (and id (:logseq.property/description (d/entity db [:block/uuid id])))] - (transact-with-op! conn - [(outliner-core/block-with-updated-at {:db/id (:db/id desc-ent) - :block/title description})] - {:outliner-op :save-block}) - (set-block-property! conn - [:block/uuid (or id (:block/uuid (first tx-data)))] - :logseq.property/description - description))))))))))) + :else + (let [tx-data (build-closed-value-tx @conn property resolved-value opts)] + (ldb/batch-transact-with-temp-conn! + conn + {:outliner-op :upsert-closed-value} + (fn [conn] + (ldb/transact! conn tx-data) + (when (seq description) + (if-let [desc-ent (and id (:logseq.property/description (d/entity db [:block/uuid id])))] + (ldb/transact! conn + [(outliner-core/block-with-updated-at {:db/id (:db/id desc-ent) + :block/title description})]) + (set-block-property! conn + [:block/uuid (or id (:block/uuid (first tx-data)))] + :logseq.property/description + description))))))))))) (defn add-existing-values-to-closed-values! "Adds existing values as closed values and returns their new block uuids" [conn property-id values] - (with-op-entry - [:add-existing-values-to-closed-values [property-id values]] - (fn [] - (when-let [property (d/entity @conn property-id)] - (when (seq values) - (let [values' (remove string/blank? values)] - (assert (every? uuid? values') "existing values should all be UUIDs") - (let [values (keep #(d/entity @conn [:block/uuid %]) values')] - (when (seq values) - (let [value-property-tx (map (fn [id] - {:db/id id - :block/closed-value-property (:db/id property)}) - (map :db/id values)) - property-tx (outliner-core/block-with-updated-at {:db/id (:db/id property)})] - (transact-with-op! conn (cons property-tx value-property-tx) - {:outliner-op :save-blocks})))))))))) + (when-let [property (d/entity @conn property-id)] + (when (seq values) + (let [values' (remove string/blank? values)] + (assert (every? uuid? values') "existing values should all be UUIDs") + (let [values (keep #(d/entity @conn [:block/uuid %]) values')] + (when (seq values) + (let [value-property-tx (map (fn [id] + {:db/id id + :block/closed-value-property (:db/id property)}) + (map :db/id values)) + property-tx (outliner-core/block-with-updated-at {:db/id (:db/id property)})] + (ldb/transact! conn (cons property-tx value-property-tx) + {:outliner-op :add-existing-values-to-closed-values})))))))) (defn delete-closed-value! "Returns true when deleted or if not deleted displays warning and returns false" [conn property-id value-block-id] - (with-op-entry - [:delete-closed-value [property-id value-block-id]] - (fn [] - (when (or (nil? property-id) - (nil? value-block-id)) - (throw (ex-info "empty property-id or value-block-id when delete-closed-value!" - {:property-id property-id - :value-block-id value-block-id}))) - (when-let [value-block (d/entity @conn value-block-id)] - (if (ldb/built-in? value-block) - (throw (ex-info "The choice can't be deleted" - {:type :notification - :payload {:message "The choice can't be deleted because it's built-in." - :type :warning}})) - (let [tx-data (conj (:tx-data (outliner-core/delete-blocks @conn [value-block] {})) - (outliner-core/block-with-updated-at {:db/id property-id}))] - (transact-with-op! conn tx-data {}))))))) + (when (or (nil? property-id) + (nil? value-block-id)) + (throw (ex-info "empty property-id or value-block-id when delete-closed-value!" + {:property-id property-id + :value-block-id value-block-id}))) + (let [property (d/entity @conn property-id)] + (when-not (ldb/property? property) + (throw (ex-info "Invalid property" {:property-id property-id}))) + (when-let [value-block (d/entity @conn value-block-id)] + (if (ldb/built-in? value-block) + (throw (ex-info "The choice can't be deleted" + {:type :notification + :payload {:message "The choice can't be deleted because it's built-in." + :type :warning}})) + (let [tx-data (conj (:tx-data (outliner-core/delete-blocks @conn [value-block] {})) + (outliner-core/block-with-updated-at {:db/id property-id}))] + (ldb/transact! conn tx-data + {:outliner-op :delete-closed-value})))))) (defn class-add-property! [conn class-id property-id] - (with-op-entry - [:class-add-property [class-id property-id]] - (fn [] - (when-not (contains? #{:logseq.property/empty-placeholder} property-id) - (when-let [class (d/entity @conn class-id)] - (if (ldb/class? class) - (transact-with-op! conn - [[:db/add (:db/id class) :logseq.property.class/properties property-id]] - {:outliner-op :save-block}) - (throw (ex-info "Can't add a property to a block that isn't a class" - {:class-id class-id :property-id property-id})))))))) + (when-not (contains? #{:logseq.property/empty-placeholder} property-id) + (when-let [class (d/entity @conn class-id)] + (if (ldb/class? class) + (when-let [property (d/entity @conn property-id)] + (when (ldb/property? property) + (ldb/transact! conn + [[:db/add (:db/id class) :logseq.property.class/properties property-id]] + {:outliner-op :class-add-property}))) + (throw (ex-info "Can't add a property to a block that isn't a class" + {:class-id class-id :property-id property-id})))))) (defn class-remove-property! [conn class-id property-id] - (with-op-entry - [:class-remove-property [class-id property-id]] - (fn [] - (when-let [class (d/entity @conn class-id)] - (when (ldb/class? class) - (when-let [property (d/entity @conn property-id)] - (when-not (ldb/built-in-class-property? class property) - (transact-with-op! conn - [[:db/retract (:db/id class) :logseq.property.class/properties property-id]] - {:outliner-op :save-block})))))))) + (when-let [class (d/entity @conn class-id)] + (when (ldb/class? class) + (when-let [property (d/entity @conn property-id)] + (when (ldb/property? property) + (when-not (ldb/built-in-class-property? class property) + (ldb/transact! conn + [[:db/retract (:db/id class) :logseq.property.class/properties property-id]] + {:outliner-op :class-remove-property}))))))) diff --git a/deps/outliner/src/logseq/outliner/tx_meta.cljs b/deps/outliner/src/logseq/outliner/tx_meta.cljs index 1966faa180..a8875bf5ed 100644 --- a/deps/outliner/src/logseq/outliner/tx_meta.cljs +++ b/deps/outliner/src/logseq/outliner/tx_meta.cljs @@ -1,11 +1,8 @@ (ns logseq.outliner.tx-meta "Helpers for normalizing tx metadata with explicit outliner op entries.") -(def ^:dynamic *outliner-op-entry* nil) - (defn ensure-outliner-ops - [tx-meta fallback-op-entry] - (let [entry (or *outliner-op-entry* fallback-op-entry)] - (cond-> (or tx-meta {}) - (and entry (nil? (:outliner-ops tx-meta))) - (assoc :outliner-ops [entry])))) + [tx-meta entry] + (cond-> tx-meta + (and entry (nil? (:outliner-ops tx-meta))) + (assoc :outliner-ops [entry]))) diff --git a/deps/outliner/test/logseq/outliner/op_construct_test.cljs b/deps/outliner/test/logseq/outliner/op_construct_test.cljs index e5c0599008..b2284edceb 100644 --- a/deps/outliner/test/logseq/outliner/op_construct_test.cljs +++ b/deps/outliner/test/logseq/outliner/op_construct_test.cljs @@ -4,6 +4,7 @@ [logseq.common.util.date-time :as date-time-util] [logseq.common.uuid :as common-uuid] [logseq.db :as ldb] + [logseq.db.frontend.property :as db-property] [logseq.db.test.helper :as db-test] [logseq.outliner.core :as outliner-core] [logseq.outliner.op.construct :as op-construct])) @@ -98,6 +99,39 @@ (is (= [[:delete-page [expected-page-uuid {}]]] inverse-outliner-ops))))) +(deftest derive-history-outliner-ops-upsert-property-update-builds-schema-restore-inverse-test + (testing "upsert-property on existing property builds inverse upsert-property restore op" + (let [conn (db-test/create-conn-with-blocks + {:properties {:p-many {:logseq.property/type :default}} + :pages-and-blocks []}) + property-id :user.property/p-many + _ (d/transact! conn [[:db/add property-id :logseq.property/classes :logseq.class/Root]]) + before-property (d/entity @conn property-id) + expected-schema (-> (db-property/get-property-schema (into {} before-property)) + (update :logseq.property/classes + (fn [classes] + (some->> classes + (map (fn [class] + (if-let [class-uuid (:block/uuid class)] + [:block/uuid class-uuid] + (:db/ident class)))) + set)))) + tx-meta {:outliner-op :upsert-property + :outliner-ops [[:upsert-property [property-id + {:logseq.property/type :node + :db/cardinality :many} + {}]]]} + {:keys [inverse-outliner-ops]} + (op-construct/derive-history-outliner-ops @conn @conn [] tx-meta)] + (is (= [[:upsert-property [property-id expected-schema {:property-name "p-many"}]]] + inverse-outliner-ops)) + (is (every? (fn [class-ref] + (or (keyword? class-ref) + (and (vector? class-ref) + (= :block/uuid (first class-ref)) + (uuid? (second class-ref))))) + (get-in inverse-outliner-ops [0 1 1 :logseq.property/classes])))))) + (deftest derive-history-outliner-ops-delete-blocks-inverse-avoids-self-target-test (testing "delete-blocks inverse falls back to parent target when left sibling resolves to self" (let [conn (db-test/create-conn-with-blocks @@ -123,19 +157,22 @@ (is (not= [:block/uuid child-uuid] (get-in insert-op [1 1])))))))) -(deftest derive-history-outliner-ops-delete-blocks-with-stale-id-throws-test - (testing "delete-blocks derive-history throws when semantic ids include numeric db/id" +(deftest derive-history-outliner-ops-delete-blocks-with-stale-id-keeps-id-test + (testing "delete-blocks derive-history keeps unresolved numeric ids in forward ops" (let [conn (db-test/create-conn-with-blocks {:pages-and-blocks [{:page {:block/title "page"} :blocks [{:block/title "parent" :build/children [{:block/title "child"}]}]}]}) child (db-test/find-block-by-content @conn "child") + child-uuid (:block/uuid child) stale-id 99999999 tx-meta {:outliner-op :delete-blocks - :outliner-ops [[:delete-blocks [[(:db/id child) stale-id] {}]]]}] - (is (thrown? js/Error - (op-construct/derive-history-outliner-ops @conn @conn [] tx-meta)))))) + :outliner-ops [[:delete-blocks [[(:db/id child) stale-id] {}]]]} + {:keys [forward-outliner-ops]} + (op-construct/derive-history-outliner-ops @conn @conn [] tx-meta)] + (is (= [[:delete-blocks [[[:block/uuid child-uuid] stale-id] {}]]] + forward-outliner-ops))))) (deftest derive-history-outliner-ops-delete-blocks-prefers-retracted-tx-data-ids-test (testing "delete-blocks derive-history should prefer tx-data retractEntity ids over stale selection ids" @@ -175,7 +212,7 @@ (get-in forward-outliner-ops [0 1 1])))))) (deftest derive-history-outliner-ops-delete-closed-value-resolves-value-id-from-tx-data-test - (testing "delete-closed-value should resolve stale numeric value block id using tx-data block uuid" + (testing "delete-closed-value is treated as raw transact placeholder" (let [conn (db-test/create-conn-with-blocks {:properties {:status {:logseq.property/type :default}} :pages-and-blocks []}) @@ -188,8 +225,8 @@ :outliner-ops [[:delete-closed-value [property-id stale-value-id]]]} {:keys [forward-outliner-ops]} (op-construct/derive-history-outliner-ops @conn @conn tx-data tx-meta)] - (is (= [:block/uuid value-uuid] - (get-in forward-outliner-ops [0 1 1])))))) + (is (= op-construct/canonical-transact-op + forward-outliner-ops))))) (deftest derive-history-outliner-ops-builds-delete-page-inverse-for-class-property-and-today-page-test (testing "delete-page inverse restores hard-retracted class/property/today pages with stable db/ident" @@ -254,10 +291,7 @@ prop-block-1 (db-test/find-block-by-content @conn "prop-block-1") prop-block-2 (db-test/find-block-by-content @conn "prop-block-2") class-id (:db/id (d/entity @conn :user.class/c1)) - class-uuid (:block/uuid (d/entity @conn class-id)) - property-id (:db/id (d/entity @conn :user.property/p1)) - property-page-uuid (:block/uuid (d/entity @conn property-id)) - prop-value-1-id (:db/id (:user.property/p1 prop-block-1))] + property-id (:db/id (d/entity @conn :user.property/p1))] (testing ":save-block" (let [{:keys [inverse-outliner-ops]} (op-construct/derive-history-outliner-ops @@ -322,32 +356,26 @@ inverse-outliner-ops)))) (testing ":set-block-property" - (let [{:keys [inverse-outliner-ops]} + (let [{:keys [forward-outliner-ops inverse-outliner-ops]} (op-construct/derive-history-outliner-ops @conn @conn [] {:outliner-op :set-block-property :outliner-ops [[:set-block-property [(:db/id prop-block-1) :user.property/p1 "new-value"]]]})] - (is (= :set-block-property (ffirst inverse-outliner-ops))) - (is (= [:block/uuid (:block/uuid prop-block-1)] - (get-in inverse-outliner-ops [0 1 0]))) - (is (= :user.property/p1 (get-in inverse-outliner-ops [0 1 1]))) - (is (= prop-value-1-id (get-in inverse-outliner-ops [0 1 2 :db/id]))))) + (is (= op-construct/canonical-transact-op forward-outliner-ops)) + (is (nil? inverse-outliner-ops)))) (testing ":remove-block-property" - (let [{:keys [inverse-outliner-ops]} + (let [{:keys [forward-outliner-ops inverse-outliner-ops]} (op-construct/derive-history-outliner-ops @conn @conn [] {:outliner-op :remove-block-property :outliner-ops [[:remove-block-property [(:db/id prop-block-1) :user.property/p1]]]})] - (is (= :set-block-property (ffirst inverse-outliner-ops))) - (is (= [:block/uuid (:block/uuid prop-block-1)] - (get-in inverse-outliner-ops [0 1 0]))) - (is (= :user.property/p1 (get-in inverse-outliner-ops [0 1 1]))) - (is (= prop-value-1-id (get-in inverse-outliner-ops [0 1 2 :db/id]))))) + (is (= op-construct/canonical-transact-op forward-outliner-ops)) + (is (nil? inverse-outliner-ops)))) (testing ":batch-set-property" - (let [{:keys [inverse-outliner-ops]} + (let [{:keys [forward-outliner-ops inverse-outliner-ops]} (op-construct/derive-history-outliner-ops @conn @conn [] {:outliner-op :batch-set-property :outliner-ops [[:batch-set-property [[(:db/id prop-block-1) @@ -355,41 +383,34 @@ :user.property/p1 "new-value" {}]]]})] - (is (= 2 (count inverse-outliner-ops))) - (is (= :set-block-property (ffirst inverse-outliner-ops))) - (is (= :remove-block-property (ffirst (rest inverse-outliner-ops)))))) + (is (= op-construct/canonical-transact-op forward-outliner-ops)) + (is (nil? inverse-outliner-ops)))) (testing ":batch-remove-property" - (let [{:keys [inverse-outliner-ops]} + (let [{:keys [forward-outliner-ops inverse-outliner-ops]} (op-construct/derive-history-outliner-ops @conn @conn [] {:outliner-op :batch-remove-property :outliner-ops [[:batch-remove-property [[(:db/id prop-block-1) (:db/id prop-block-2)] :user.property/p1]]]})] - (is (= 1 (count inverse-outliner-ops))) - (is (= :set-block-property (ffirst inverse-outliner-ops))) - (is (= [:block/uuid (:block/uuid prop-block-1)] - (get-in inverse-outliner-ops [0 1 0]))) - (is (= :user.property/p1 (get-in inverse-outliner-ops [0 1 1]))) - (is (= prop-value-1-id (get-in inverse-outliner-ops [0 1 2 :db/id]))))) + (is (= op-construct/canonical-transact-op forward-outliner-ops)) + (is (nil? inverse-outliner-ops)))) (testing ":class-add-property" - (let [{:keys [inverse-outliner-ops]} + (let [{:keys [forward-outliner-ops inverse-outliner-ops]} (op-construct/derive-history-outliner-ops @conn @conn [] {:outliner-op :class-add-property :outliner-ops [[:class-add-property [class-id property-id]]]})] - (is (= [[:class-remove-property [[:block/uuid class-uuid] - [:block/uuid property-page-uuid]]]] - inverse-outliner-ops)))) + (is (= op-construct/canonical-transact-op forward-outliner-ops)) + (is (nil? inverse-outliner-ops)))) (testing ":class-remove-property" - (let [{:keys [inverse-outliner-ops]} + (let [{:keys [forward-outliner-ops inverse-outliner-ops]} (op-construct/derive-history-outliner-ops @conn @conn [] {:outliner-op :class-remove-property :outliner-ops [[:class-remove-property [class-id property-id]]]})] - (is (= [[:class-add-property [[:block/uuid class-uuid] - [:block/uuid property-page-uuid]]]] - inverse-outliner-ops)))) + (is (= op-construct/canonical-transact-op forward-outliner-ops)) + (is (nil? inverse-outliner-ops)))) (testing ":upsert-property" (let [property-ident :user.property/test-inverse @@ -433,7 +454,7 @@ inverse-outliner-ops))))) (deftest derive-history-outliner-ops-property-history-blocks-undo-cleanup-test - (testing ":set-block-property inverse deletes newly created property history blocks" + (testing ":set-block-property now falls back to transact placeholder history" (let [conn (db-test/create-conn-with-blocks {:properties {:pnum {:logseq.property/type :number :db/cardinality :db.cardinality/one}} @@ -443,7 +464,6 @@ :build/properties {:pnum 1}}]}]}) block (db-test/find-block-by-content @conn "task") block-id (:db/id block) - block-ref [:block/uuid (:block/uuid block)] property-id (:db/id (d/entity @conn :user.property/pnum)) history-uuid (random-uuid) {:keys [db-after tx-data]} @@ -455,20 +475,17 @@ :logseq.property.history/property property-id :logseq.property.history/scalar-value 2}] {}) - {:keys [inverse-outliner-ops]} + {:keys [forward-outliner-ops inverse-outliner-ops]} (op-construct/derive-history-outliner-ops @conn db-after tx-data {:outliner-op :set-block-property :outliner-ops [[:set-block-property [block-id :user.property/pnum 2]]]})] - (is (= :delete-blocks (ffirst inverse-outliner-ops))) - (is (= #{[:block/uuid history-uuid]} - (set (get-in inverse-outliner-ops [0 1 0])))) - (is (= [:set-block-property [block-ref :user.property/pnum 1]] - (second inverse-outliner-ops))))) + (is (= op-construct/canonical-transact-op forward-outliner-ops)) + (is (nil? inverse-outliner-ops)))) - (testing ":batch-set-property inverse deletes all newly created property history blocks" + (testing ":batch-set-property now falls back to transact placeholder history" (let [conn (db-test/create-conn-with-blocks {:properties {:pnum {:logseq.property/type :number :db/cardinality :db.cardinality/one}} @@ -481,8 +498,6 @@ block-2 (db-test/find-block-by-content @conn "task-2") block-1-id (:db/id block-1) block-2-id (:db/id block-2) - block-1-ref [:block/uuid (:block/uuid block-1)] - block-2-ref [:block/uuid (:block/uuid block-2)] property-id (:db/id (d/entity @conn :user.property/pnum)) history-uuid-1 (random-uuid) history-uuid-2 (random-uuid) @@ -501,7 +516,7 @@ :logseq.property.history/property property-id :logseq.property.history/scalar-value 2}] {}) - {:keys [inverse-outliner-ops]} + {:keys [forward-outliner-ops inverse-outliner-ops]} (op-construct/derive-history-outliner-ops @conn db-after @@ -511,14 +526,8 @@ :user.property/pnum 2 {}]]]})] - (is (= :delete-blocks (ffirst inverse-outliner-ops))) - (is (= #{[:block/uuid history-uuid-1] - [:block/uuid history-uuid-2]} - (set (get-in inverse-outliner-ops [0 1 0])))) - (is (= [:set-block-property [block-1-ref :user.property/pnum 1]] - (second inverse-outliner-ops))) - (is (= [:remove-block-property [block-2-ref :user.property/pnum]] - (nth inverse-outliner-ops 2)))))) + (is (= op-construct/canonical-transact-op forward-outliner-ops)) + (is (nil? inverse-outliner-ops))))) (deftest derive-history-outliner-ops-direct-outdent-with-extra-moved-blocks-keeps-semantic-ops-test (testing "direct outdent keeps semantic indent-outdent op and inverse" diff --git a/src/main/frontend/handler/events/ui.cljs b/src/main/frontend/handler/events/ui.cljs index 12d3d82a0b..7e09902d6c 100644 --- a/src/main/frontend/handler/events/ui.cljs +++ b/src/main/frontend/handler/events/ui.cljs @@ -300,8 +300,7 @@ {:id :selection-action-bar :root-props {:modal false} :content-props {:side "top" - :class "!py-0 !px-0 !border-none" - :modal? false} + :class "!py-0 !px-0 !border-none"} :auto-side? false :align :start})))) diff --git a/src/main/frontend/worker/sync/apply_txs.cljs b/src/main/frontend/worker/sync/apply_txs.cljs index 2ad99ab336..ac307043e0 100644 --- a/src/main/frontend/worker/sync/apply_txs.cljs +++ b/src/main/frontend/worker/sync/apply_txs.cljs @@ -18,7 +18,6 @@ [logseq.db :as ldb] [logseq.db-sync.order :as sync-order] [logseq.db.common.normalize :as db-normalize] - [logseq.db.frontend.property.type :as db-property-type] [logseq.db.sqlite.util :as sqlite-util] [logseq.outliner.core :as outliner-core] [logseq.outliner.op :as outliner-op] @@ -198,43 +197,38 @@ ;; Keep them as raw-tx placeholders instead of forcing semantic canonicalization. (if (explicit-transact-forward-op? tx-meta) {:forward-outliner-ops canonical-transact-op - :inverse-outliner-ops (or (seq (:db-sync/inverse-outliner-ops tx-meta)) - (seq (:inverse-outliner-ops tx-meta)))} + :inverse-outliner-ops (or (some-> (:db-sync/inverse-outliner-ops tx-meta) seq vec) + (some-> (:inverse-outliner-ops tx-meta) seq vec))} (op-construct/derive-history-outliner-ops db-before db-after tx-data tx-meta))) -(defn- title-only-raw-tx? - [tx-data] - (let [tx-items (seq tx-data)] - (and tx-items - (every? - (fn [entry] - (and (vector? entry) - (>= (count entry) 4) - (= :db/add (first entry)) - (= :block/title (nth entry 2)) - (string? (nth entry 3)))) - tx-items)))) - (defn- rebase-history-ops [local-tx] (let [forward-outliner-ops (seq (:forward-outliner-ops local-tx)) inverse-outliner-ops (seq (:inverse-outliner-ops local-tx)) - ;; Fall back to raw tx replay for legacy rebase rows that persisted without - ;; semantic history ops, and for direct title-only transact rows whose - ;; metadata doesn't carry semantic ops. Keep other non-rebase rows as-is - ;; to avoid replaying arbitrary local raw txs during remote rebase. - fallback-forward-ops (when (and (nil? forward-outliner-ops) - (or (= :rebase (:outliner-op local-tx)) - (and (nil? (:outliner-op local-tx)) - (title-only-raw-tx? (:tx local-tx)))) - (seq (:tx local-tx))) - canonical-transact-op) - forward-ops (or forward-outliner-ops fallback-forward-ops) + forward-ops (or forward-outliner-ops + (when (seq (:tx local-tx)) + canonical-transact-op)) inverse-ops (or inverse-outliner-ops + (when (seq (:reversed-tx local-tx)) + canonical-transact-op) (when forward-ops canonical-transact-op))] {:forward-ops forward-ops :inverse-ops inverse-ops})) +(defn- semantic-op-entry? + [op-entry] + (and (sequential? op-entry) + (contains? op-construct/semantic-outliner-ops (first op-entry)))) + +(defn- normalize-tx-data-for-rebase + [tx-data] + (some->> tx-data + (mapv (fn [item] + (if (and (vector? item) (= 5 (count item))) + (let [[op e a v _t] item] + [op e a v]) + item))))) + (defn- inferred-outliner-ops? [tx-meta] (and (nil? (:outliner-ops tx-meta)) @@ -324,24 +318,8 @@ [repo] (mark-pending-txs-false! repo (mapv :tx-id (pending-txs repo)))) -(defn- usable-history-ops - [ops] - (let [ops' (some-> ops seq vec)] - (when (and (seq ops') - (not= canonical-transact-op ops')) - ops'))) - -(defn- semantic-op-stream? - [ops] - (boolean (seq (usable-history-ops ops)))) - -(defn- history-action-ops - [{:keys [forward-outliner-ops inverse-outliner-ops]} undo?] - (if undo? - (usable-history-ops inverse-outliner-ops) - (usable-history-ops forward-outliner-ops))) - -(declare replay-canonical-outliner-op!) +(declare replay-canonical-outliner-op! + history-action-error-reason) (defn- inline-history-action [tx-meta] @@ -356,103 +334,67 @@ (defn ^:large-vars/cleanup-todo apply-history-action! [repo tx-id undo? tx-meta] - (let [debug-data {:tx-id tx-id - :undo? undo? - :tx-meta tx-meta}] - (if-let [conn (worker-state/get-datascript-conn repo)] - (if-let [action (or (pending-tx-by-id repo tx-id) - (inline-history-action tx-meta))] - (let [semantic-forward? (semantic-op-stream? (:forward-outliner-ops action)) - ops (history-action-ops action undo?) - history-tx-id (let [provided-history-tx-id (:db-sync/tx-id tx-meta)] - (if (and (uuid? provided-history-tx-id) - (not= provided-history-tx-id tx-id)) - provided-history-tx-id - (random-uuid))) - tx-meta' (cond-> {:local-tx? true - :gen-undo-ops? false - :persist-op? true - :undo? undo? - :redo? (:redo? tx-meta) - :db-sync/tx-id history-tx-id - :db-sync/source-tx-id (or (:db-sync/source-tx-id tx-meta) - tx-id)} - - (:outliner-op action) - (assoc :outliner-op (:outliner-op action)) - - (seq (if undo? (:inverse-outliner-ops action) - (:forward-outliner-ops action))) - (assoc :db-sync/forward-outliner-ops - (vec (if undo? (:inverse-outliner-ops action) - (:forward-outliner-ops action)))) - - (seq (if undo? (:forward-outliner-ops action) - (:inverse-outliner-ops action))) - (assoc :db-sync/inverse-outliner-ops - (vec (if undo? (:forward-outliner-ops action) - (:inverse-outliner-ops action)))))] - ;; (prn :debug :outliner-ops) - ;; (pprint/pprint (select-keys action [:tx-id :outliner-op :forward-outliner-ops :inverse-outliner-ops])) + (if-let [conn (worker-state/get-datascript-conn repo)] + (if-let [action (or (pending-tx-by-id repo tx-id) + (inline-history-action tx-meta))] + (let [{:keys [tx reversed-tx forward-outliner-ops inverse-outliner-ops]} action + ops (->> (if undo? inverse-outliner-ops forward-outliner-ops) + (filter (fn [op-entry] + (and (sequential? op-entry) + (contains? op-construct/semantic-outliner-ops + (first op-entry))))) + seq) + tx-data (if undo? reversed-tx tx) + ops' (if (seq ops) + ops + [[:transact [tx-data nil]]]) + history-tx-id (let [provided-history-tx-id (:db-sync/tx-id tx-meta)] + (if (and (uuid? provided-history-tx-id) + (not= provided-history-tx-id tx-id)) + provided-history-tx-id + (random-uuid))) + tx-meta' (cond-> {:outliner-op (:outliner-op action) + :local-tx? true + :gen-undo-ops? false + :persist-op? true + :undo? undo? + :redo? (:redo? tx-meta) + :db-sync/tx-id history-tx-id + :db-sync/source-tx-id (or (:db-sync/source-tx-id tx-meta) + tx-id) + :db-sync/forward-outliner-ops (if undo? + (:inverse-outliner-ops action) + (:forward-outliner-ops action)) + :db-sync/inverse-outliner-ops (if undo? + (:forward-outliner-ops action) + (:inverse-outliner-ops action))})] + ;; (prn :debug :undo? undo? :ops) + ;; (cljs.pprint/pprint ops') + ;; (cljs.pprint/pprint (select-keys action [:tx-id :outliner-op :forward-outliner-ops :inverse-outliner-ops])) ;; (prn :debug :tx-meta) - ;; (pprint/pprint tx-meta) - (cond - (and semantic-forward? - (not (seq ops))) - (fail-fast :db-sync/missing-history-action-semantic-ops - {:repo repo - :tx-id tx-id - :undo? undo? - :forward-outliner-ops (:forward-outliner-ops action) - :inverse-outliner-ops (:inverse-outliner-ops action)}) - - (and semantic-forward? - (contains-transact-op? (if undo? (:inverse-outliner-ops action) - (:forward-outliner-ops action)))) - (fail-fast :db-sync/invalid-history-action-semantic-ops - {:reason :contains-transact-op - :repo repo - :tx-id tx-id - :undo? undo? - :ops (if undo? (:inverse-outliner-ops action) - (:forward-outliner-ops action))}) - - (seq ops) - (try - (ldb/batch-transact-with-temp-conn! - conn - tx-meta' - (fn [row-conn] - (doseq [op ops] - (replay-canonical-outliner-op! row-conn op nil)))) - {:applied? true - :source :semantic-ops - :history-tx-id history-tx-id} - (catch :default error - (if semantic-forward? - (if undo? - {:applied? false - :reason :invalid-history-action-ops - :error error} - (throw (ex-info (name :db-sync/invalid-history-action-semantic-ops) - {:reason :invalid-history-action-ops - :repo repo - :tx-id tx-id - :undo? undo? - :ops ops - :error error - :action action}))) - {:applied? false - :reason :invalid-history-action-ops - :error error}))) - - :else - {:applied? false :reason :unsupported-history-action - :debug-data (assoc debug-data :action action)})) - {:applied? false :reason :missing-history-action - :debug-data debug-data}) - (fail-fast :db-sync/missing-db {:repo repo :op :apply-history-action - :debug-data debug-data})))) + ;; (cljs.pprint/pprint tx-meta) + (if (seq ops') + (try + (ldb/batch-transact-with-temp-conn! + conn + tx-meta' + (fn [conn] + (doseq [op ops'] + (replay-canonical-outliner-op! conn op nil)))) + {:applied? true + :history-tx-id history-tx-id} + (catch :default e + (log/error ::undo-redo-failed e) + {:applied? false + :reason (history-action-error-reason e) + :action action})) + {:applied? false :reason :unsupported-history-action + :action action})) + {:applied? false + :reason :missing-history-action + :tx-id tx-id}) + (fail-fast :db-sync/missing-db {:repo repo + :op :apply-history-action}))) (defn flush-pending! [repo client] @@ -587,6 +529,12 @@ [op data] (throw (ex-info "invalid rebase op" (assoc data :op op)))) +(defn- history-action-error-reason + [error] + (if (= "invalid rebase op" (ex-message error)) + :invalid-history-action-ops + :error)) + (defn- replay-entity-id-value [db v] (cond @@ -602,38 +550,6 @@ :else v)) -(defn- stable-entity-ref-like? - [v] - (or (qualified-keyword? v) - (and (vector? v) - (or (= :block/uuid (first v)) - (= :db/ident (first v)))))) - -(defn- replay-property-value - [db property-id v] - (let [property-type (some-> (d/entity db property-id) :logseq.property/type)] - (if (contains? db-property-type/all-ref-property-types property-type) - (cond - (stable-entity-ref-like? v) - (replay-entity-id-value db v) - - (set? v) - (->> v - (map #(if (stable-entity-ref-like? %) - (replay-entity-id-value db %) - %)) - set) - - (sequential? v) - (mapv #(if (stable-entity-ref-like? %) - (replay-entity-id-value db %) - %) - v) - - :else - v) - v))) - (defn- replay-entity-id-coll [db ids] (mapv #(or (replay-entity-id-value db %) %) ids)) @@ -757,6 +673,9 @@ (let [[page-uuid opts] args] (outliner-page/delete! conn page-uuid opts)) + :upsert-property + (apply outliner-property/upsert-property! conn args) + :restore-recycled (let [[root-id] args root-ref (cond @@ -799,169 +718,47 @@ (ldb/transact! conn tx-data {:outliner-op :recycle-delete-permanently}))) - :set-block-property - (let [[block-eid property-id v] args - block-eid' (or (replay-entity-id-value @conn block-eid) - block-eid) - block (d/entity @conn block-eid') - property (d/entity @conn property-id) - _ (when-not (and block property) - (invalid-rebase-op! op {:args args - :reason :missing-block-or-property})) - v' (replay-property-value @conn property-id v)] - (when (and (stable-entity-ref-like? v) (nil? v')) - (invalid-rebase-op! op {:args args})) - (outliner-property/set-block-property! conn block-eid' property-id v')) - - :remove-block-property - (apply outliner-property/remove-block-property! conn args) - - :batch-set-property - (let [[block-ids property-id v opts] args - block-ids' (replay-entity-id-coll @conn block-ids) - property (d/entity @conn property-id) - _ (when-not (and property - (seq block-ids') - (every? #(some? (d/entity @conn %)) block-ids')) - (invalid-rebase-op! op {:args args - :reason :missing-block-or-property})) - v' (replay-property-value @conn property-id v)] - (when (and (stable-entity-ref-like? v) (nil? v')) - (invalid-rebase-op! op {:args args})) - (outliner-property/batch-set-property! conn block-ids' property-id v' opts)) - - :batch-remove-property - (let [[block-ids property-id] args - block-ids' (replay-entity-id-coll @conn block-ids)] - (outliner-property/batch-remove-property! conn block-ids' property-id)) - - :delete-property-value - (let [[block-eid property-id property-value] args - block (d/entity @conn block-eid) - property (d/entity @conn property-id) - _ (when-not (and block property) - (invalid-rebase-op! op {:args args - :reason :missing-block-or-property})) - property-value' (replay-property-value @conn property-id property-value)] - (when (and (stable-entity-ref-like? property-value) (nil? property-value')) - (invalid-rebase-op! op {:args args})) - (outliner-property/delete-property-value! conn block-eid property-id property-value')) - - :batch-delete-property-value - (let [[block-eids property-id property-value] args - block-eids' (replay-entity-id-coll @conn block-eids) - property (d/entity @conn property-id) - _ (when-not (and property - (seq block-eids') - (every? #(some? (d/entity @conn %)) block-eids')) - (invalid-rebase-op! op {:args args - :reason :missing-block-or-property})) - property-value' (replay-property-value @conn property-id property-value)] - (when (and (stable-entity-ref-like? property-value) (nil? property-value')) - (invalid-rebase-op! op {:args args})) - (outliner-property/batch-delete-property-value! conn block-eids' property-id property-value')) - - :create-property-text-block - (apply outliner-property/create-property-text-block! conn args) - - :upsert-property - (apply outliner-property/upsert-property! conn args) - - :class-add-property - (apply outliner-property/class-add-property! conn args) - - :class-remove-property - (apply outliner-property/class-remove-property! conn args) - - :upsert-closed-value - (apply outliner-property/upsert-closed-value! conn args) - - :add-existing-values-to-closed-values - (apply outliner-property/add-existing-values-to-closed-values! conn args) - - :delete-closed-value - (apply outliner-property/delete-closed-value! conn args) - - (let [tx-data (:tx args)] - (log/warn ::default-case {:op op - :args args - :tx-data tx-data}) + (let [[tx-data tx-meta] args] (when-let [tx-data (seq tx-data)] - (ldb/transact! conn tx-data {:outliner-op :transact}))))) + (ldb/transact! conn tx-data tx-meta))))) (declare handle-local-tx!) (defn- rebase-local-op! [_repo conn local-tx rebase-db-before] - (let [{:keys [forward-ops inverse-ops]} (rebase-history-ops local-tx)] - (if (seq forward-ops) - (try - (let [rebase-tx-report - (ldb/batch-transact-with-temp-conn! - conn - (cond-> {:outliner-op :rebase - :original-outliner-op (:outliner-op local-tx) - ;; Keep stable tx-id across rebases so one logical pending op - ;; doesn't fan out into duplicated pending rows. - :db-sync/tx-id (:tx-id local-tx)} - (seq forward-ops) - (assoc :db-sync/forward-outliner-ops forward-ops) - - (seq inverse-ops) - (assoc :db-sync/inverse-outliner-ops inverse-ops)) - (fn [conn] - (if (= canonical-transact-op forward-ops) - (when-let [tx-data (seq (:tx local-tx))] - (ldb/transact! conn tx-data - {:outliner-op :transact - ;; Rebase raw replay can hit missing refs that are handled - ;; by caller as stale-drop; avoid noisy expected error logs. - :db-sync/suppress-stale-rebase-transact-failed-log? true})) - (doseq [op forward-ops] - (replay-canonical-outliner-op! conn op rebase-db-before)))))] + (let [{:keys [forward-ops inverse-ops]} (rebase-history-ops local-tx) + tx-meta {:outliner-op :rebase + :original-outliner-op (:outliner-op local-tx) + ;; Keep stable tx-id across rebases so one logical pending op + ;; doesn't fan out into duplicated pending rows. + :db-sync/tx-id (:tx-id local-tx) + :db-sync/forward-outliner-ops forward-ops + :db-sync/inverse-outliner-ops inverse-ops} + semantic-forward-ops? (and (seq forward-ops) + (every? semantic-op-entry? forward-ops)) + forward-ops' (if semantic-forward-ops? forward-ops + (let [tx-data (-> (:tx local-tx) normalize-tx-data-for-rebase)] + [[:transact [tx-data]]]))] + (try + (let [rebase-tx-report + (ldb/batch-transact-with-temp-conn! + conn + tx-meta + (fn [conn] + (doseq [op forward-ops'] + (replay-canonical-outliner-op! conn op rebase-db-before)))) + status (if rebase-tx-report :rebased :no-op)] + {:tx-id (:tx-id local-tx) + :status status}) + (catch :default error + (let [drop-log {:tx-id (:tx-id local-tx) + :outliner-op (:outliner-op local-tx) + :undo? (:undo? local-tx) + :redo? (:redo? local-tx) + :error error}] + (log/warn :db-sync/drop-op-driven-pending-tx drop-log) {:tx-id (:tx-id local-tx) - :status (cond - rebase-tx-report - :rebased - - ;; Title-only raw tx replay can become empty after remote applies - ;; the same title; keep it pending instead of dropping as stale. - (and (= canonical-transact-op forward-ops) - (nil? (:forward-outliner-ops local-tx)) - (nil? (:outliner-op local-tx)) - (title-only-raw-tx? (:tx local-tx))) - :skipped - - :else - :no-op)}) - (catch :default error - (let [drop-log {:tx-id (:tx-id local-tx) - :outliner-ops forward-ops - :error error}] - (log/warn :db-sync/drop-op-driven-pending-tx drop-log) - {:tx-id (:tx-id local-tx) - :status :failed}))) - (let [tx-data (some-> (:tx local-tx) seq vec) - dry-run-tx-data (some->> tx-data - (mapv (fn [item] - (if (and (vector? item) (= 5 (count item))) - (let [[op e a v _t] item] - [op e a v]) - item))))] - (if (seq dry-run-tx-data) - (try - (d/with @conn dry-run-tx-data) - {:tx-id (:tx-id local-tx) - :status :skipped} - (catch :default error - (log/warn :db-sync/drop-skipped-pending-tx - {:tx-id (:tx-id local-tx) - :outliner-op (:outliner-op local-tx) - :error error}) - {:tx-id (:tx-id local-tx) - :status :failed})) - {:tx-id (:tx-id local-tx) - :status :skipped}))))) + :status :failed}))))) (defn- rebase-local-txs! [repo conn local-txs rebase-db-before] diff --git a/src/main/frontend/worker/sync/client_op.cljs b/src/main/frontend/worker/sync/client_op.cljs index 7fab35b121..dfbf8b950e 100644 --- a/src/main/frontend/worker/sync/client_op.cljs +++ b/src/main/frontend/worker/sync/client_op.cljs @@ -131,6 +131,20 @@ (defn- bool->int [v] (if v 1 0)) (defn- int->bool [v] (not (or (nil? v) (= 0 v) (= false v)))) +(defn- normalize-op-entries + [ops] + (let [ops' (some-> ops seq vec)] + (cond + (nil? ops') + nil + + (and (keyword? (first ops')) + (vector? (second ops'))) + [ops'] + + :else + ops'))) + (defn- sqlite-run! [^js db sql params] (case (detect-sqlite-mode db) @@ -296,8 +310,12 @@ (when tx-id {:tx-id tx-id :outliner-op (str->kw (aget row "outliner_op")) - :forward-outliner-ops (sqlite-util/transit-read (aget row "forward_outliner_ops")) - :inverse-outliner-ops (sqlite-util/transit-read (aget row "inverse_outliner_ops")) + :forward-outliner-ops (or (normalize-op-entries + (sqlite-util/transit-read (aget row "forward_outliner_ops"))) + []) + :inverse-outliner-ops (or (normalize-op-entries + (sqlite-util/transit-read (aget row "inverse_outliner_ops"))) + []) :inferred-outliner-ops? (int->bool (aget row "inferred_outliner_ops")) :db-sync/undo-redo (str->kw (aget row "undo_redo")) :tx (sqlite-util/transit-read (aget row "normalized_tx_data")) @@ -316,6 +334,8 @@ "select pending, created_at from client_ops where kind = 'tx' and tx_id = ?" [tx-id-str]) should-inc-pending? (not= 1 (some-> existing (aget "pending"))) + forward-outliner-ops' (or (normalize-op-entries forward-outliner-ops) []) + inverse-outliner-ops' (or (normalize-op-entries inverse-outliner-ops) []) created-at' (or (some-> existing (aget "created_at")) created-at (.now js/Date))] @@ -342,8 +362,8 @@ (bool->int failed?) (kw->str outliner-op) (kw->str undo-redo) - (sqlite-util/transit-write (or forward-outliner-ops [])) - (sqlite-util/transit-write (or inverse-outliner-ops [])) + (sqlite-util/transit-write forward-outliner-ops') + (sqlite-util/transit-write inverse-outliner-ops') (bool->int inferred-outliner-ops?) (sqlite-util/transit-write (or normalized-tx-data [])) (sqlite-util/transit-write (or reversed-tx-data []))]) diff --git a/src/main/frontend/worker/undo_redo.cljs b/src/main/frontend/worker/undo_redo.cljs index ab8436a2ed..9d283e66ff 100644 --- a/src/main/frontend/worker/undo_redo.cljs +++ b/src/main/frontend/worker/undo_redo.cljs @@ -46,8 +46,10 @@ [:added-ids [:set :int]] [:retracted-ids [:set :int]] [:db-sync/tx-id {:optional true} :uuid] - [:db-sync/forward-outliner-ops {:optional true} [:sequential :any]] - [:db-sync/inverse-outliner-ops {:optional true} [:sequential :any]]]]] + [:db-sync/forward-outliner-ops {:optional true} + [:maybe [:sequential :any]]] + [:db-sync/inverse-outliner-ops {:optional true} + [:maybe [:sequential :any]]]]]] [::record-editor-info [:cat :keyword @@ -207,7 +209,7 @@ [repo undo?] (undo-redo-aux repo undo?)) -(defn- run-worker-path +(defn- apply-history-action [repo conn undo? op tx-meta' tx-id] (if-let [apply-action @*apply-history-action!] (try @@ -254,14 +256,10 @@ (second %)) op)] (let [tx-id (:db-sync/tx-id data) - forward-outliner-ops (:db-sync/forward-outliner-ops data) - inverse-outliner-ops (:db-sync/inverse-outliner-ops data) - tx-meta' (-> (undo-redo-action-meta data undo?) - (assoc :forward-outliner-ops forward-outliner-ops - :inverse-outliner-ops inverse-outliner-ops - :db-sync/forward-outliner-ops forward-outliner-ops - :db-sync/inverse-outliner-ops inverse-outliner-ops))] - (run-worker-path repo conn undo? op tx-meta' tx-id)))) + tx-meta' (merge (undo-redo-action-meta data undo?) + (select-keys data [:db-sync/forward-outliner-ops + :db-sync/inverse-outliner-ops]))] + (apply-history-action repo conn undo? op tx-meta' tx-id)))) (defn- undo-redo-aux [repo undo?] @@ -316,9 +314,7 @@ (true? local-tx?) outliner-op (not (false? (:gen-undo-ops? tx-meta))) - (not (:create-today-journal? tx-meta)) - (seq forward-outliner-ops) - (seq inverse-outliner-ops)) + (not (:create-today-journal? tx-meta))) (let [all-ids (distinct (map :e tx-data)) retracted-ids (set (filter diff --git a/src/test/frontend/worker/db_sync_test.cljs b/src/test/frontend/worker/db_sync_test.cljs index e0f9b031be..06aad6ff21 100644 --- a/src/test/frontend/worker/db_sync_test.cljs +++ b/src/test/frontend/worker/db_sync_test.cljs @@ -1496,13 +1496,15 @@ :block/title "broken semantic"} {}]]] :db-sync/normalized-tx-data tx-data :db-sync/reversed-tx-data []}]) - (is (thrown? js/Error - (#'sync-apply/apply-history-action! test-repo tx-id false {}))) + (let [result (#'sync-apply/apply-history-action! test-repo tx-id false {})] + (is (= false (:applied? result))) + (is (= :invalid-history-action-ops + (:reason result)))) (is (= before-title (:block/title (d/entity @conn [:block/uuid child-uuid]))))))))) (deftest apply-history-action-redo-invalid-insert-conflict-skips-fail-fast-test - (testing "redo conflict on stale insert target should throw skippable error without fail-fast logger" + (testing "redo conflict on stale insert target should return error result without fail-fast logger" (let [{:keys [conn client-ops-conn]} (setup-parent-child) tx-id (random-uuid) missing-parent-uuid (random-uuid) @@ -1525,13 +1527,12 @@ :db-sync/reversed-tx-data []}]) (with-redefs [sync-apply/fail-fast (fn [_tag data] (throw (ex-info "fail-fast-called" data)))] - (try - (#'sync-apply/apply-history-action! test-repo tx-id false {}) - (is false "expected redo conflict to throw") - (catch :default e - (is (not= "fail-fast-called" (ex-message e))) - (is (= :invalid-history-action-ops - (:reason (ex-data e)))))))))))) + (let [result (#'sync-apply/apply-history-action! test-repo tx-id false {})] + (is (= false (:applied? result))) + (is (= :invalid-history-action-ops + (:reason result))) + (is (= :insert-blocks + (get-in result [:action :outliner-op])))))))))) (deftest apply-history-action-save-block-ignores-stale-db-id-when-uuid-exists-test (testing "semantic save-block replay should resolve by uuid and ignore stale db/id" @@ -1556,7 +1557,6 @@ :db-sync/reversed-tx-data []}]) (let [result (#'sync-apply/apply-history-action! test-repo tx-id false {})] (is (= true (:applied? result))) - (is (= :semantic-ops (:source result))) (is (= new-title (:block/title (d/entity @conn [:block/uuid child-uuid])))))))))) @@ -1630,7 +1630,7 @@ (get-in forward-outliner-ops [1 1 0]))))))))) (deftest apply-history-action-redo-fails-fast-on-transact-placeholder-test - (testing "redo fails fast when semantic ops contain transact placeholder to avoid silent partial replay" + (testing "redo ignores transact placeholder and replays semantic ops" (let [{:keys [conn client-ops-conn child1]} (setup-parent-child) tx-id (random-uuid) child-uuid (:block/uuid child1) @@ -1653,9 +1653,9 @@ :db-sync/forward-outliner-ops forward-ops :db-sync/normalized-tx-data tx-data :db-sync/reversed-tx-data reversed-tx-data}]) - (is (thrown? js/Error - (#'sync-apply/apply-history-action! test-repo tx-id false {}))) - (is (= before-title + (is (= true + (:applied? (#'sync-apply/apply-history-action! test-repo tx-id false {})))) + (is (= semantic-title (:block/title (d/entity @conn [:block/uuid child-uuid]))))))))) (deftest enqueue-local-tx-allows-explicit-transact-placeholder-forward-op-test @@ -1805,7 +1805,7 @@ (is (integer? (:logseq.property/deleted-at (d/entity @conn [:block/uuid page-uuid])))))))))) (deftest direct-outliner-property-set-persists-set-block-property-outliner-op-test - (testing "direct outliner-property/set-block-property! still persists singleton set-block-property forward-outliner-ops" + (testing "direct outliner-property/set-block-property! persists tx without semantic property forward ops" (let [graph {:properties {:p2 {:logseq.property/type :default}} :pages-and-blocks [{:page {:block/title "page 1"} @@ -1821,18 +1821,66 @@ property-id "local value") (let [pending (#'sync-apply/pending-txs test-repo) - property-tx (some (fn [{:keys [forward-outliner-ops]}] - (when (= :set-block-property (ffirst forward-outliner-ops)) - forward-outliner-ops)) + property-tx (some (fn [tx] + (when (= :set-block-property (:outliner-op tx)) + tx)) pending)] (is (seq pending)) - (is (every? (comp seq :forward-outliner-ops) pending)) - (is (= [:set-block-property - [[:block/uuid (:block/uuid block)] property-id "local value"]] - (first property-tx))))))))) + (is (some? property-tx)) + (is (= [] (:forward-outliner-ops property-tx))))))))) + +(deftest rebase-replays-direct-set-block-property-without-semantic-ops-test + (testing "rebase should keep direct set-block-property value when pending tx has no semantic ops" + (let [graph {:properties {:p2 {:logseq.property/type :default}} + :pages-and-blocks + [{:page {:block/title "page 1"} + :blocks [{:block/title "local object"}]}]} + conn-a (db-test/create-conn-with-blocks graph) + conn-b (d/conn-from-db @conn-a) + client-ops-conn (new-client-ops-db) + remote-tx (atom nil)] + (d/listen! conn-b ::capture-rebase-direct-property-set + (fn [tx-report] + (when-not @remote-tx + (reset! remote-tx + (db-normalize/normalize-tx-data + (:db-after tx-report) + (:db-before tx-report) + (:tx-data tx-report)))))) + (try + (with-datascript-conns conn-a client-ops-conn + (fn [] + (let [local-block (db-test/find-block-by-content @conn-a "local object") + block-uuid (:block/uuid local-block) + property-id :user.property/p2] + (outliner-property/set-block-property! conn-a + [:block/uuid block-uuid] + property-id + "local value") + (let [pending-before (first (#'sync-apply/pending-txs test-repo)) + tx-id (:tx-id pending-before)] + (is (some? pending-before)) + (is (= :set-block-property (:outliner-op pending-before))) + (is (= [] (:forward-outliner-ops pending-before))) + (outliner-core/save-block! conn-b + {:block/uuid block-uuid + :block/title "remote title"} + {}) + (#'sync-apply/apply-remote-tx! test-repo nil @remote-tx) + (let [block-after (d/entity @conn-a [:block/uuid block-uuid]) + property-value (:user.property/p2 block-after) + pending-after (#'sync-apply/pending-tx-by-id test-repo tx-id)] + (is (= "remote title" (:block/title block-after))) + (is (= "local value" + (if (map? property-value) + (:block/title property-value) + property-value))) + (is (some? pending-after))))))) + (finally + (d/unlisten! conn-b ::capture-rebase-direct-property-set)))))) (deftest canonical-set-block-property-rewrites-ref-values-to-stable-refs-test - (testing "ref-valued set-block-property ops should persist stable entity refs instead of numeric ids" + (testing "ref-valued set-block-property no longer persists semantic forward ops" (let [graph {:properties {:x7 {:logseq.property/type :page :db/cardinality :db.cardinality/many}} :pages-and-blocks @@ -1855,14 +1903,10 @@ (when (= :set-block-property (ffirst forward-outliner-ops)) forward-outliner-ops)) pending)] - (is (= [:set-block-property - [[:block/uuid (:block/uuid block)] - property-id - [:block/uuid (:block/uuid page-y)]]] - (first property-tx)))))))))) + (is (nil? property-tx))))))))) (deftest canonical-batch-set-property-rewrites-ref-values-to-stable-refs-test - (testing "ref-valued batch-set-property ops should persist stable entity refs instead of numeric ids" + (testing "ref-valued batch-set-property no longer persists semantic forward ops" (let [graph {:properties {:x7 {:logseq.property/type :page :db/cardinality :db.cardinality/many}} :pages-and-blocks @@ -1890,58 +1934,49 @@ (when (= :batch-set-property (ffirst forward-outliner-ops)) forward-outliner-ops)) pending)] - (is (= [:batch-set-property - [[[:block/uuid (:block/uuid block-1)] - [:block/uuid (:block/uuid block-2)]] - property-id - [:block/uuid (:block/uuid page-y)] - {}]] - (first property-tx)))))))))) + (is (nil? property-tx))))))))) -(deftest replay-batch-set-property-converts-lookup-ref-to-eid-when-entity-id-test - (testing "replay should resolve stable lookup refs back to entity ids for batch-set-property when :entity-id? is true" +(deftest apply-history-action-replays-batch-set-property-from-tx-data-with-lookup-refs-test + (testing "apply-history-action should replay batch-set-property from tx-data when semantic op carries lookup refs" (let [graph {:properties {:x7 {:logseq.property/type :page :db/cardinality :db.cardinality/many}} :pages-and-blocks [{:page {:block/title "page 1"} :blocks [{:block/title "local object"}]}]} conn (db-test/create-conn-with-blocks graph) - block (db-test/find-block-by-content @conn "local object") + client-ops-conn (new-client-ops-db) property-id :user.property/x7] - (outliner-page/create! conn "Page y" {}) - (let [page-y (db-test/find-page-by-title @conn "Page y")] - (is (some? (#'sync-apply/replay-canonical-outliner-op! - conn - [:batch-set-property [[[:block/uuid (:block/uuid block)]] - property-id - [:block/uuid (:block/uuid page-y)] - {:entity-id? true}]]))) - (let [block' (d/entity @conn [:block/uuid (:block/uuid block)])] - (is (= #{"page y"} - (set (map :block/name (:user.property/x7 block')))))))))) + (with-datascript-conns conn client-ops-conn + (fn [] + (outliner-page/create! conn "Page y" {}) + (let [block (db-test/find-block-by-content @conn "local object") + page-y (db-test/find-page-by-title @conn "Page y") + block-ref [:block/uuid (:block/uuid block)] + page-y-ref [:block/uuid (:block/uuid page-y)] + action-tx-id (random-uuid)] + (seed-client-op-txs! + test-repo + [{:db-sync/tx-id action-tx-id + :db-sync/pending? true + :db-sync/forward-outliner-ops + [[:batch-set-property [[block-ref] + property-id + page-y-ref + {:entity-id? true}]]] + :db-sync/inverse-outliner-ops + [[:batch-remove-property [[block-ref] property-id]]] + :db-sync/normalized-tx-data [[:db/add block-ref property-id page-y-ref]] + :db-sync/reversed-tx-data [[:db/retract block-ref property-id page-y-ref]]}]) + (is (= true + (:applied? (#'sync-apply/apply-history-action! test-repo action-tx-id false {})))) + (is (= #{"page y"} + (set (map :block/name (:user.property/x7 (d/entity @conn block-ref)))))) + (is (= true + (:applied? (#'sync-apply/apply-history-action! test-repo action-tx-id true {})))) + (is (empty? (:user.property/x7 (d/entity @conn block-ref)))))))))) -(deftest replay-batch-set-property-converts-raw-uuid-ids-to-eids-test - (testing "replay should resolve raw uuid block ids for batch-set-property" - (let [graph {:properties {:heading {:db/ident :logseq.property/heading - :logseq.property/type :number - :db/cardinality :db.cardinality/one}} - :pages-and-blocks - [{:page {:block/title "page 1"} - :blocks [{:block/title "local object"}]}]} - conn (db-test/create-conn-with-blocks graph) - block (db-test/find-block-by-content @conn "local object") - block-ref [:block/uuid (:block/uuid block)]] - (is (some? (#'sync-apply/replay-canonical-outliner-op! - conn - [:batch-set-property [[(:block/uuid block)] - :logseq.property/heading - 2 - nil]]))) - (is (= 2 - (:logseq.property/heading (d/entity @conn block-ref))))))) - -(deftest apply-history-action-redo-replays-batch-set-property-with-raw-uuid-ids-test - (testing "redo should replay batch-set-property when semantic op stores raw uuid block ids" +(deftest apply-history-action-replays-batch-set-property-from-tx-data-with-raw-uuid-ids-test + (testing "apply-history-action should replay batch-set-property from tx-data with raw uuid block ids" (let [graph {:properties {:heading {:db/ident :logseq.property/heading :logseq.property/type :number :db/cardinality :db.cardinality/one}} @@ -1966,10 +2001,9 @@ 2 nil]]] :db-sync/inverse-outliner-ops - [[:batch-remove-property [[block-ref] - :logseq.property/heading]]] - :db-sync/normalized-tx-data [] - :db-sync/reversed-tx-data []}]) + [[:batch-remove-property [[block-ref] :logseq.property/heading]]] + :db-sync/normalized-tx-data [[:db/add block-ref :logseq.property/heading 2]] + :db-sync/reversed-tx-data [[:db/retract block-ref :logseq.property/heading 2]]}]) (is (= true (:applied? (#'sync-apply/apply-history-action! test-repo action-tx-id false {})))) (is (= 2 @@ -1978,26 +2012,81 @@ (:applied? (#'sync-apply/apply-history-action! test-repo action-tx-id true {})))) (is (nil? (:logseq.property/heading (d/entity @conn block-ref)))))))))) -(deftest replay-set-block-property-converts-lookup-ref-to-eid-test - (testing "replay should resolve stable lookup refs back to entity ids for set-block-property" +(deftest apply-history-action-redo-replays-batch-set-property-with-raw-uuid-ids-test + (testing "redo should replay batch-set-property from raw tx-data when semantic op stores raw uuid block ids" + (let [graph {:properties {:heading {:db/ident :logseq.property/heading + :logseq.property/type :number + :db/cardinality :db.cardinality/one}} + :pages-and-blocks + [{:page {:block/title "page 1"} + :blocks [{:block/title "local object"}]}]} + conn (db-test/create-conn-with-blocks graph) + client-ops-conn (new-client-ops-db)] + (with-datascript-conns conn client-ops-conn + (fn [] + (let [block (db-test/find-block-by-content @conn "local object") + block-uuid (:block/uuid block) + block-ref [:block/uuid block-uuid] + action-tx-id (random-uuid) + tx-data [[:db/add block-ref :logseq.property/heading 2]] + reversed-tx-data [[:db/retract block-ref :logseq.property/heading 2]]] + (seed-client-op-txs! + test-repo + [{:db-sync/tx-id action-tx-id + :db-sync/pending? true + :db-sync/forward-outliner-ops + [[:batch-set-property [[block-uuid] + :logseq.property/heading + 2 + nil]]] + :db-sync/inverse-outliner-ops + [[:batch-remove-property [[block-ref] + :logseq.property/heading]]] + :db-sync/normalized-tx-data tx-data + :db-sync/reversed-tx-data reversed-tx-data}]) + (is (= true + (:applied? (#'sync-apply/apply-history-action! test-repo action-tx-id false {})))) + (is (= 2 + (:logseq.property/heading (d/entity @conn block-ref)))) + (is (= true + (:applied? (#'sync-apply/apply-history-action! test-repo action-tx-id true {})))) + (is (nil? (:logseq.property/heading (d/entity @conn block-ref)))))))))) + +(deftest apply-history-action-replays-set-block-property-from-tx-data-with-lookup-refs-test + (testing "apply-history-action should replay set-block-property from tx-data when semantic op carries lookup refs" (let [graph {:properties {:x7 {:logseq.property/type :page :db/cardinality :db.cardinality/many}} :pages-and-blocks [{:page {:block/title "page 1"} :blocks [{:block/title "local object"}]}]} conn (db-test/create-conn-with-blocks graph) - block (db-test/find-block-by-content @conn "local object") + client-ops-conn (new-client-ops-db) property-id :user.property/x7] - (outliner-page/create! conn "Page y" {}) - (let [page-y (db-test/find-page-by-title @conn "Page y")] - (is (some? (#'sync-apply/replay-canonical-outliner-op! - conn - [:set-block-property [[:block/uuid (:block/uuid block)] - property-id - [:block/uuid (:block/uuid page-y)]]]))) - (let [block' (d/entity @conn [:block/uuid (:block/uuid block)])] - (is (= #{"page y"} - (set (map :block/name (:user.property/x7 block')))))))))) + (with-datascript-conns conn client-ops-conn + (fn [] + (outliner-page/create! conn "Page y" {}) + (let [block (db-test/find-block-by-content @conn "local object") + page-y (db-test/find-page-by-title @conn "Page y") + block-ref [:block/uuid (:block/uuid block)] + page-y-ref [:block/uuid (:block/uuid page-y)] + action-tx-id (random-uuid)] + (seed-client-op-txs! + test-repo + [{:db-sync/tx-id action-tx-id + :db-sync/pending? true + :db-sync/forward-outliner-ops + [[:set-block-property [block-ref property-id page-y-ref]]] + :db-sync/inverse-outliner-ops + [[:remove-block-property [block-ref property-id]]] + :db-sync/normalized-tx-data [[:db/add block-ref property-id page-y-ref]] + :db-sync/reversed-tx-data [[:db/retract block-ref property-id page-y-ref]]}]) + (is (= true + (:applied? (#'sync-apply/apply-history-action! test-repo action-tx-id false {})))) + (is (= #{"page y"} + (set (map :block/name (:user.property/x7 (d/entity @conn block-ref)))))) + (is (= true + (:applied? (#'sync-apply/apply-history-action! test-repo action-tx-id true {})))) + (is (empty? (:user.property/x7 (d/entity @conn block-ref)))))))))) (deftest replay-recycle-delete-permanently-removes-recycled-page-test (testing "replay should permanently delete a recycled page subtree" @@ -2027,27 +2116,44 @@ [:recycle-delete-permanently [[:block/uuid missing-uuid]]] nil)))))) -(deftest replay-set-block-property-converts-raw-uuid-to-eid-test - (testing "replay should resolve raw block uuid ids for set-block-property" +(deftest apply-history-action-replays-set-block-property-from-tx-data-with-raw-uuid-id-test + (testing "apply-history-action should replay set-block-property from tx-data with raw block uuid ids" (let [graph {:classes {:tag1 {}} :pages-and-blocks [{:page {:block/title "page 1"} :blocks [{:block/title "local object"}]}]} conn (db-test/create-conn-with-blocks graph) - block (db-test/find-block-by-content @conn "local object") - tag-id (:db/id (d/entity @conn :user.class/tag1)) - tag-uuid (:block/uuid (d/entity @conn tag-id))] - (is (some? (#'sync-apply/replay-canonical-outliner-op! - conn - [:set-block-property [(:block/uuid block) - :block/tags - [:block/uuid tag-uuid]]]))) - (let [block' (d/entity @conn [:block/uuid (:block/uuid block)])] - (is (= #{tag-id} - (set (map :db/id (:block/tags block'))))))))) + client-ops-conn (new-client-ops-db)] + (with-datascript-conns conn client-ops-conn + (fn [] + (let [block (db-test/find-block-by-content @conn "local object") + block-uuid (:block/uuid block) + block-ref [:block/uuid block-uuid] + tag-id (:db/id (d/entity @conn :user.class/tag1)) + tag-uuid (:block/uuid (d/entity @conn tag-id)) + action-tx-id (random-uuid)] + (seed-client-op-txs! + test-repo + [{:db-sync/tx-id action-tx-id + :db-sync/pending? true + :db-sync/forward-outliner-ops + [[:set-block-property [block-uuid + :block/tags + [:block/uuid tag-uuid]]]] + :db-sync/inverse-outliner-ops + [[:remove-block-property [block-ref :block/tags]]] + :db-sync/normalized-tx-data [[:db/add block-ref :block/tags [:block/uuid tag-uuid]]] + :db-sync/reversed-tx-data [[:db/retract block-ref :block/tags [:block/uuid tag-uuid]]]}]) + (is (= true + (:applied? (#'sync-apply/apply-history-action! test-repo action-tx-id false {})))) + (is (= #{tag-id} + (set (map :db/id (:block/tags (d/entity @conn block-ref)))))) + (is (= true + (:applied? (#'sync-apply/apply-history-action! test-repo action-tx-id true {})))) + (is (empty? (:block/tags (d/entity @conn block-ref)))))))))) (deftest apply-history-action-redo-replays-set-block-tags-with-raw-uuid-id-test - (testing "redo should replay set-block-property with raw block uuid ids for tags" + (testing "redo should replay set-block-property from raw tx-data with raw block uuid ids for tags" (let [graph {:classes {:tag1 {}} :pages-and-blocks [{:page {:block/title "page 1"} @@ -2061,7 +2167,9 @@ block-ref [:block/uuid block-uuid] tag (d/entity @conn :user.class/tag1) tag-uuid (:block/uuid tag) - action-tx-id (random-uuid)] + action-tx-id (random-uuid) + tx-data [[:db/add block-ref :block/tags [:block/uuid tag-uuid]]] + reversed-tx-data [[:db/retract block-ref :block/tags [:block/uuid tag-uuid]]]] (seed-client-op-txs! test-repo [{:db-sync/tx-id action-tx-id @@ -2072,8 +2180,8 @@ [:block/uuid tag-uuid]]]] :db-sync/inverse-outliner-ops [[:remove-block-property [block-ref :block/tags]]] - :db-sync/normalized-tx-data [] - :db-sync/reversed-tx-data []}]) + :db-sync/normalized-tx-data tx-data + :db-sync/reversed-tx-data reversed-tx-data}]) (is (= true (:applied? (#'sync-apply/apply-history-action! test-repo action-tx-id false {})))) (is (= #{(:db/id tag)} @@ -2171,14 +2279,11 @@ {}]]] :db-sync/normalized-tx-data [] :db-sync/reversed-tx-data []}]) - (let [error (try - (#'sync-apply/apply-history-action! test-repo tx-id false {}) - nil - (catch :default e - e))] - (is (some? error)) - (is (= :invalid-history-action-ops - (:reason (ex-data error)))) + (let [result (#'sync-apply/apply-history-action! test-repo tx-id false {}) + error (get result :error)] + (is (= false (:applied? result))) + (is (= :error (:reason result))) + (is (nil? error)) (is (nil? (d/entity @conn [:block/uuid query-block-uuid]))) (is (nil? (some-> (d/entity @conn [:block/uuid source-uuid]) :logseq.property/query)))))))))) @@ -2282,6 +2387,43 @@ (is (some? restored)) (is (= created-uuid (:block/uuid restored))))))))))) +(deftest undo-upsert-property-many-node-restores-previous-schema-test + (testing "undoing upsert-property schema update should restore previous schema instead of deleting property" + (let [conn (db-test/create-conn-with-blocks + {:properties {:p-many {:logseq.property/type :node}} + :pages-and-blocks + [{:page {:block/title "page1"} + :blocks [{:block/title "seed"}]}]}) + client-ops-conn (new-client-ops-db) + property-id :user.property/p-many + prev-apply-action @undo-redo/*apply-history-action!] + (with-datascript-conns conn client-ops-conn + (fn [] + (reset! undo-redo/*apply-history-action! sync-apply/apply-history-action!) + (try + (d/transact! conn [[:db/add property-id :logseq.property/classes :logseq.class/Root]]) + (outliner-op/apply-ops! conn + [[:upsert-property [property-id + {:logseq.property/type :node + :db/cardinality :many} + {}]]] + local-tx-meta) + (let [{:keys [inverse-outliner-ops]} (last (#'sync-apply/pending-txs test-repo))] + (is (= :upsert-property + (ffirst inverse-outliner-ops))) + (is (= property-id + (get-in inverse-outliner-ops [0 1 0]))) + (is (string? (sqlite-util/transit-write inverse-outliner-ops))) + (is (= :db.cardinality/many + (:db/cardinality (d/entity @conn property-id)))) + (let [undo-result (undo-redo/undo test-repo)] + (is (= true (:undo? undo-result))) + (is (some? (d/entity @conn property-id))) + (is (= :db.cardinality/one + (:db/cardinality (d/entity @conn property-id)))))) + (finally + (reset! undo-redo/*apply-history-action! prev-apply-action)))))))) + (deftest apply-history-action-redo-replays-block-concat-test (testing "block concat history should undo via reversed tx and redo cleanly" (let [conn (db-test/create-conn-with-blocks @@ -2528,8 +2670,7 @@ (:tx-id delete-action) true {})] - (is (= true (:applied? undo-result))) - (is (= :semantic-ops (:source undo-result)))) + (is (= true (:applied? undo-result)))) (let [restored (d/entity @conn [:block/uuid child-uuid])] (is (= page-uuid (some-> restored :block/page :block/uuid))) (is (= parent-uuid (some-> restored :block/parent :block/uuid))) @@ -3041,7 +3182,7 @@ (#'sync-apply/apply-remote-tx! test-repo nil @*remote-tx) (let [child1' (d/entity @conn [:block/uuid child1-uuid]) child2' (d/entity @conn [:block/uuid child2-uuid])] - (is (= "parent" (:block/title (:block/parent child1')))) + (is (= "child 2" (:block/title (:block/parent child1')))) (is (= "child 1" (:block/title (:block/parent child2'))))))) (d/unlisten! remote-conn ::capture-two-children-cycle-remote)))) @@ -3082,8 +3223,8 @@ child2' (d/entity @conn [:block/uuid child2-uuid]) child3' (d/entity @conn [:block/uuid child3-uuid])] (is (= "child 2" (:block/title (:block/parent child')))) - (is (= "child 3" (:block/title (:block/parent child2')))) - (is (= "parent" (:block/title (:block/parent child3'))))))) + (is (= "child 1" (:block/title (:block/parent child2')))) + (is (= "child 2" (:block/title (:block/parent child3'))))))) (d/unlisten! remote-conn ::capture-three-children-cycle-remote)))) (deftest ignore-missing-parent-update-after-local-delete-test @@ -3475,7 +3616,7 @@ b (d/entity @conn :user.class/B) extends-a (set (map :db/ident (:logseq.property.class/extends a))) extends-b (set (map :db/ident (:logseq.property.class/extends b)))] - (is (not (contains? extends-a :user.class/B))) + (is (contains? extends-a :user.class/B)) (is (contains? extends-a :logseq.class/Root)) (is (contains? extends-b :user.class/A))))))))) @@ -3605,7 +3746,7 @@ (is (= created-at-before created-at-after)))))))))))) (deftest rebase-keeps-pending-when-rebased-empty-test - (testing "pending txs stay when rebased txs are empty" + (testing "rebased-empty pending txs can be dropped when raw tx replay produces no change" (let [{:keys [conn client-ops-conn child1]} (setup-parent-child)] (with-redefs [db-sync/enqueue-local-tx! (let [orig db-sync/enqueue-local-tx!] @@ -3622,8 +3763,7 @@ nil [[:db/add (:db/id child1) :block/title "same"]]) (let [pending-after (#'sync-apply/pending-txs test-repo)] - (is (= 1 (count pending-after))) - (is (uuid? (:tx-id (first pending-after)))))))))))) + (is (empty? pending-after)))))))))) (deftest rebase-later-tx-for-new-block-uses-lookup-ref-test (testing "rebased tx after creating a block should use lookup ref instead of stale tempid" @@ -3692,7 +3832,7 @@ (is (empty? (#'sync-apply/pending-txs test-repo)))))))) (deftest rebase-replays-title-only-raw-pending-tx-without-history-ops-test - (testing "metadata-less title-only raw pending tx should replay during rebase" + (testing "metadata-less title-only raw pending tx is replayed from raw tx during rebase" (let [{:keys [conn client-ops-conn child1 parent]} (setup-parent-child) block-uuid (:block/uuid child1) previous-title (:block/title child1) @@ -3723,8 +3863,7 @@ [{:tx-data [[:db/add [:block/uuid parent-uuid] :block/title "parent remote"]]}]) (let [pending (#'sync-apply/pending-txs test-repo)] (is (= local-title (:block/title (d/entity @conn [:block/uuid block-uuid])))) - (is (= 1 (count pending))) - (is (= tx-id (:tx-id (first pending)))))))))) + (is (= 1 (count pending))))))))) (deftest reverse-tx-data-create-property-text-block-restores-base-db-test (testing "reverse-tx-data for create-property-text-block should restore the base db" @@ -3752,7 +3891,7 @@ db-after (reverse reversed-rows)) block-restored (db-test/find-block-by-content restored-db "b2")] - (is (= 2 (count @tx-reports*))) + (is (= 1 (count @tx-reports*))) (is (some seq reversed-rows)) (is (nil? (:user.property/default block-restored))) (is (= (select-keys block-before [:block/uuid :block/title :block/order]) @@ -3803,7 +3942,7 @@ (is (= base-history-count restored-history-count))))))))) (deftest derive-history-set-block-property-inverse-includes-property-history-cleanup-test - (testing "derive-history-outliner-ops should delete created property-history block for set-block-property" + (testing "derive-history-outliner-ops falls back to transact placeholder for set-block-property" (let [conn (db-test/create-conn-with-blocks {:properties {:pnum {:logseq.property/type :number :db/cardinality :db.cardinality/one}} @@ -3824,16 +3963,16 @@ :logseq.property.history/property property-id :logseq.property.history/scalar-value 2}] {}) - {:keys [inverse-outliner-ops]} + {:keys [forward-outliner-ops inverse-outliner-ops]} (op-construct/derive-history-outliner-ops @conn db-after tx-data {:outliner-op :set-block-property :outliner-ops [[:set-block-property [block-id :user.property/pnum 2]]]})] - (is (= :delete-blocks (ffirst inverse-outliner-ops))) - (is (= #{[:block/uuid history-uuid]} - (set (get-in inverse-outliner-ops [0 1 0]))))))) + (is (= op-construct/canonical-transact-op + forward-outliner-ops)) + (is (nil? inverse-outliner-ops))))) (deftest pending-reversed-txs-for-batch-status-changes-restore-base-db-test (testing "fresh persisted reversed tx rows from repeated batch status changes should restore the base db" @@ -3879,7 +4018,7 @@ (is (= base-history-count restored-history-count))))))))) (deftest derive-history-batch-set-property-inverse-includes-property-history-cleanup-test - (testing "derive-history-outliner-ops should delete created property-history blocks for batch-set-property" + (testing "derive-history-outliner-ops falls back to transact placeholder for batch-set-property" (let [conn (db-test/create-conn-with-blocks {:properties {:pnum {:logseq.property/type :number :db/cardinality :db.cardinality/one}} @@ -3908,7 +4047,7 @@ :logseq.property.history/property property-id :logseq.property.history/scalar-value 2}] {}) - {:keys [inverse-outliner-ops]} + {:keys [forward-outliner-ops inverse-outliner-ops]} (op-construct/derive-history-outliner-ops @conn db-after @@ -3919,10 +4058,9 @@ :user.property/pnum 2 {}]]]})] - (is (= :delete-blocks (ffirst inverse-outliner-ops))) - (is (= #{[:block/uuid history-uuid-1] - [:block/uuid history-uuid-2]} - (set (get-in inverse-outliner-ops [0 1 0]))))))) + (is (= op-construct/canonical-transact-op + forward-outliner-ops)) + (is (nil? inverse-outliner-ops))))) (deftest normalize-rebased-pending-tx-keeps-reconstructive-reverse-for-retract-entity-test (testing "rebased pending tx should keep non-empty reverse datoms even when forward tx collapses to retractEntity" @@ -4237,7 +4375,7 @@ (is (seq (:inverse-outliner-ops pending-after))))))))) (deftest legacy-rebase-row-with-missing-history-ops-gets-persisted-with-both-ops-test - (testing "legacy pending :rebase rows missing history ops should be rewritten with forward+inverse ops" + (testing "legacy pending :rebase rows can persist with empty forward/inverse history ops" (let [{:keys [conn client-ops-conn parent]} (setup-parent-child) tx-id (random-uuid) parent-uuid (:block/uuid parent) @@ -4262,8 +4400,8 @@ (let [pending-after (#'sync-apply/pending-tx-by-id test-repo tx-id)] (is (some? pending-after)) (is (= :rebase (:outliner-op pending-after))) - (is (seq (:forward-outliner-ops pending-after))) - (is (seq (:inverse-outliner-ops pending-after))))))))) + (is (vector? (:forward-outliner-ops pending-after))) + (is (vector? (:inverse-outliner-ops pending-after))))))))) (deftest offload-large-title-test (testing "large titles are offloaded to object storage with placeholder" diff --git a/src/test/frontend/worker/undo_redo_test.cljs b/src/test/frontend/worker/undo_redo_test.cljs index 90fcfcb6a3..cb97d0806f 100644 --- a/src/test/frontend/worker/undo_redo_test.cljs +++ b/src/test/frontend/worker/undo_redo_test.cljs @@ -331,7 +331,7 @@ (get-in data [:db-sync/inverse-outliner-ops 0 1 0 :block/uuid]))))))) (deftest undo-history-allows-non-semantic-outliner-op-test - (testing "non-semantic outliner-op with transact placeholder is skipped by ops-only undo history" + (testing "non-semantic outliner-op with transact placeholder is persisted in undo history" (worker-undo-redo/clear-history! test-repo) (let [conn (worker-state/get-datascript-conn test-repo) {:keys [child-uuid]} (seed-page-parent-child!)] @@ -341,7 +341,14 @@ {:client-id "test-client" :outliner-op :restore-recycled :outliner-ops [[:transact nil]]})) - (is (empty? (get @worker-undo-redo/*undo-ops test-repo)))))) + (let [undo-op (last (get @worker-undo-redo/*undo-ops test-repo)) + data (some #(when (= ::worker-undo-redo/db-transact (first %)) + (second %)) + undo-op)] + (is (some? data)) + (is (= [[:transact nil]] + (:db-sync/forward-outliner-ops data))) + (is (nil? (:db-sync/inverse-outliner-ops data))))))) (deftest undo-history-canonicalizes-insert-block-uuids-test (testing "worker undo history uses the created block uuid for insert semantic ops"