From d37df950fafda9792bcd2dd589a75c696d577bad Mon Sep 17 00:00:00 2001 From: Gabriel Horner Date: Thu, 16 May 2024 15:05:37 -0400 Subject: [PATCH] fix: property rules now work with property ref values Also remove use of :string properties in tests --- .../logseq/db/frontend/property/build.cljs | 24 ++++++++++ deps/db/src/logseq/db/frontend/rules.cljc | 8 +++- .../test/logseq/db/frontend/rules_test.cljs | 47 ++++++++++++------- .../logseq/tasks/db_graph/create_graph.cljs | 1 + src/main/frontend/worker/handler/page.cljs | 18 +++---- 5 files changed, 69 insertions(+), 29 deletions(-) diff --git a/deps/db/src/logseq/db/frontend/property/build.cljs b/deps/db/src/logseq/db/frontend/property/build.cljs index 87e1e2b574..01cf415143 100644 --- a/deps/db/src/logseq/db/frontend/property/build.cljs +++ b/deps/db/src/logseq/db/frontend/property/build.cljs @@ -74,3 +74,27 @@ (when (keyword? property) {:db/ident property})) :block/order (db-order/gen-key)} sqlite-util/block-with-timestamps)) + +(defn build-property-values-tx-m + "Builds a map of property names to their property value blocks to be transacted, given a block + and a properties map with raw property values" + [block properties] + ;; Build :db/id out of uuid if block doesn't have one for tx purposes + (let [block' (if (:db/id block) block (assoc block :db/id [:block/uuid (:block/uuid block)]))] + (->> properties + (map (fn [[k v]] + [k + (if (set? v) + (set (map #(build-property-value-block block' k %) v)) + (build-property-value-block block' k v))])) + (into {})))) + +(defn build-properties-with-ref-values + "Given a properties map with property values to be transacted e.g. from + build-property-values-tx-m, build a properties map to be transacted with the block" + [prop-vals-tx-m] + (update-vals prop-vals-tx-m + (fn [v] + (if (set? v) + (set (map #(vector :block/uuid (:block/uuid %)) v)) + (vector :block/uuid (:block/uuid v)))))) diff --git a/deps/db/src/logseq/db/frontend/rules.cljc b/deps/db/src/logseq/db/frontend/rules.cljc index 7c37b560b1..1dd4915da0 100644 --- a/deps/db/src/logseq/db/frontend/rules.cljc +++ b/deps/db/src/logseq/db/frontend/rules.cljc @@ -183,7 +183,9 @@ :page-property '[[(page-property ?p ?prop ?val) [?p :block/name] - [?p ?prop ?val] + [?p ?prop ?pv] + (or [?pv :block/content ?val] + [?pv :block/original-name ?val]) [?prop-e :db/ident ?prop] [?prop-e :block/type "property"]] ;; TODO: Delete when DB_GRAPH query-dsl tests are passing @@ -226,7 +228,9 @@ :property '[(property ?b ?prop ?val) - [?b ?prop ?val] + [?b ?prop ?pv] + (or [?pv :block/content ?val] + [?pv :block/original-name ?val]) [(missing? $ ?b :block/name)] [?prop-e :db/ident ?prop] [?prop-e :block/type "property"]] diff --git a/deps/db/test/logseq/db/frontend/rules_test.cljs b/deps/db/test/logseq/db/frontend/rules_test.cljs index 2536314d62..09249a9635 100644 --- a/deps/db/test/logseq/db/frontend/rules_test.cljs +++ b/deps/db/test/logseq/db/frontend/rules_test.cljs @@ -5,7 +5,7 @@ [logseq.db.frontend.rules :as rules] [logseq.db.sqlite.create-graph :as sqlite-create-graph] [logseq.db.sqlite.util :as sqlite-util] - [logseq.db :as ldb])) + [logseq.db.frontend.property.build :as db-property-build])) (defn- new-db-conn [] (let [conn (d/create-conn db-schema/schema-for-db-based-graph) @@ -21,10 +21,15 @@ (deftest has-page-property-rule (let [conn (new-db-conn) page (sqlite-util/build-new-page "Page") - _ (d/transact! conn [(sqlite-util/build-new-property :user.property/foo {:type :string}) - (sqlite-util/build-new-property :user.property/foo2 {:type :string}) - page - {:block/uuid (:block/uuid page) :user.property/foo "bar"}])] + prop-vals-tx-m (db-property-build/build-property-values-tx-m page {:user.property/foo "bar"}) + _ (d/transact! conn + (concat + [(sqlite-util/build-new-property :user.property/foo {}) + (sqlite-util/build-new-property :user.property/foo2 {}) + page] + (vals prop-vals-tx-m) + [(merge {:block/uuid (:block/uuid page)} + (db-property-build/build-properties-with-ref-values prop-vals-tx-m))]))] (is (= ["Page"] (->> (q-with-rules '[:find (pull ?b [:block/original-name]) :where (has-page-property ?b :user.property/foo)] @conn) @@ -45,16 +50,22 @@ (let [conn (new-db-conn) page (sqlite-util/build-new-page "Page") page-a (sqlite-util/build-new-page "Page A") - _ (d/transact! conn [(sqlite-util/build-new-property :user.property/foo {:type :string}) - (sqlite-util/build-new-property :user.property/foo2 {:type :string}) - (sqlite-util/build-new-property :user.property/number-many {:type :number :cardinality :many}) - (sqlite-util/build-new-property :user.property/page-many {:type :page :cardinality :many}) - page - page-a - {:block/uuid (:block/uuid page) :user.property/foo "bar"} - {:block/uuid (:block/uuid page) :user.property/number-many #{5 10}} - {:block/uuid (:block/uuid page) :user.property/page-many #{[:block/uuid (:block/uuid page-a)]}} - {:block/uuid (:block/uuid page-a) :user.property/foo "bar A"}])] + page-prop-vals-tx-m (db-property-build/build-property-values-tx-m page {:user.property/foo "bar" :user.property/number-many #{5 10}}) + page-a-prop-vals-tx-m (db-property-build/build-property-values-tx-m page-a {:user.property/foo "bar A"}) + _ (d/transact! conn + (concat [(sqlite-util/build-new-property :user.property/foo {}) + (sqlite-util/build-new-property :user.property/foo2 {}) + (sqlite-util/build-new-property :user.property/number-many {:type :number :cardinality :many}) + (sqlite-util/build-new-property :user.property/page-many {:type :page :cardinality :many}) + page + page-a] + (mapcat #(if (set? %) % [%]) (vals page-prop-vals-tx-m)) + (mapcat #(if (set? %) % [%]) (vals page-a-prop-vals-tx-m)) + [(merge {:block/uuid (:block/uuid page)} + (db-property-build/build-properties-with-ref-values page-prop-vals-tx-m)) + {:block/uuid (:block/uuid page) :user.property/page-many #{[:block/uuid (:block/uuid page-a)]}} + (merge {:block/uuid (:block/uuid page-a)} + (db-property-build/build-properties-with-ref-values page-a-prop-vals-tx-m))]))] (testing "cardinality :one property" (is (= ["Page"] @@ -96,7 +107,7 @@ (testing ":ref property" (is (= ["Page"] (->> (q-with-rules '[:find (pull ?b [:block/original-name]) - :where (page-property ?b :user.property/page-many ?pv) [?pv :block/original-name "Page A"]] + :where (page-property ?b :user.property/page-many "Page A")] @conn) (map (comp :block/original-name first)))) "page-property returns result when page has property") @@ -117,7 +128,7 @@ (is (= #{[:user.property/number-many 10] [:user.property/number-many 5] [:user.property/foo "bar"] - [:user.property/page-many (:db/id (ldb/get-page @conn "Page A"))]} + [:user.property/page-many "Page A"]} (->> (q-with-rules '[:find ?p ?v :where (page-property ?b ?p ?v) [?b :block/original-name "Page"]] @conn) @@ -126,7 +137,7 @@ (is (= #{"Page"} (->> (q-with-rules '[:find (pull ?b [:block/original-name]) :where - (page-property ?b :user.property/page-many ?pv) + [?b :user.property/page-many ?pv] (page-property ?pv :user.property/foo "bar A")] @conn) (map (comp :block/original-name first)) diff --git a/scripts/src/logseq/tasks/db_graph/create_graph.cljs b/scripts/src/logseq/tasks/db_graph/create_graph.cljs index ebbb674d23..1a7e11909f 100644 --- a/scripts/src/logseq/tasks/db_graph/create_graph.cljs +++ b/scripts/src/logseq/tasks/db_graph/create_graph.cljs @@ -109,6 +109,7 @@ ;; FIXME: Remove when fixed in UI (str value))) +;; TODO: Use build-property-values-tx-m (defn- ->property-value-tx-m "Given a new block and its properties, creates a map of properties which have values of property value tx. This map is used for both creating the new property values and then adding them to a block" diff --git a/src/main/frontend/worker/handler/page.cljs b/src/main/frontend/worker/handler/page.cljs index d8b3b888af..b8186cccc3 100644 --- a/src/main/frontend/worker/handler/page.cljs +++ b/src/main/frontend/worker/handler/page.cljs @@ -42,13 +42,14 @@ tags)}))] (if (sqlite-util/db-based-graph? repo) (let [property-vals-tx-m - ;; Builds property values for built-in :one properties like logseq.property.pdf/file - (->> properties - (keep (fn [[k v]] - (when (db-property-util/built-in-has-ref-value? k) - [k - (db-property-build/build-property-value-block {:db/id page-entity} k v)]))) - (into {}))] + ;; Builds property values for built-in properties like logseq.property.pdf/file + (db-property-build/build-property-values-tx-m + page' + (->> properties + (keep (fn [[k v]] + (when (db-property-util/built-in-has-ref-value? k) + [k v]))) + (into {})))] (cond-> [(merge page' (when class? {:block/type "class"}))] (seq property-vals-tx-m) @@ -57,8 +58,7 @@ (conj (merge {:block/uuid (:block/uuid page)} ;; FIXME: Add refs for properties? properties - ;; Replace property values with their refs - (update-vals property-vals-tx-m #(vector :block/uuid (:block/uuid %))))))) + (db-property-build/build-properties-with-ref-values property-vals-tx-m))))) (let [file-page (merge page' (when (seq properties) {:block/properties properties}))] (if (and (seq properties)