Merge branch 'refactor/db-properties-schema' into refactor/db-remove-block-name-unique

deps/db/script/validate_client_db.cljs

@@ -16,8 +16,9 @@
 
 (defn validate-client-db
   "Validate datascript db as a vec of entity maps"
-  [db ent-maps {:keys [verbose group-errors closed-maps]}]
-  (let [schema (db-validate/update-schema db-malli-schema/DB db {:closed-schema? closed-maps})]
+  [db ent-maps* {:keys [verbose group-errors closed-maps]}]
+  (let [ent-maps (db-malli-schema/update-properties-in-ents ent-maps*)
+        schema (db-validate/update-schema db-malli-schema/DB db {:closed-schema? closed-maps})]
     (if-let [errors (->> ent-maps
                          (m/explain schema)
                          :errors)]

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

@@ -3,7 +3,58 @@
   (:require [clojure.walk :as walk]
             [clojure.string :as string]
             [logseq.db.frontend.schema :as db-schema]
-            [logseq.db.frontend.property.type :as db-property-type]))
+            [logseq.db.frontend.property.type :as db-property-type]
+            [datascript.core :as d]
+            [logseq.db.frontend.property :as db-property]))
+
+;; :db/ident malli schemas
+;; =======================
+
+(def logseq-property-namespaces
+  #{"logseq.property" "logseq.property.table" "logseq.property.tldraw"
+    "logseq.task"})
+
+(def db-attribute-properties
+  "Internal properties that are also db attributes"
+  #{:block/alias :block/tags})
+
+(def db-attribute-ident
+  (into [:enum] db-attribute-properties))
+
+(def logseq-property-ident
+  [:and :keyword [:fn
+                  {:error/message "should be a valid logseq property namespace"}
+                  (fn logseq-namespace? [k]
+                    (contains? logseq-property-namespaces (namespace k)))]])
+
+(def internal-property-ident
+  [:or logseq-property-ident db-attribute-ident])
+
+(defn user-property-namespace?
+  "Determines if keyword is a user property"
+  [kw]
+  (contains? #{"user.property"} (namespace kw)))
+
+(def user-property-ident
+  [:and :keyword [:fn
+                  {:error/message "should be a valid user property namespace"}
+                  user-property-namespace?]])
+
+(def property-ident
+  [:or internal-property-ident user-property-ident])
+
+(def logseq-ident-namespaces
+  "Set of all namespaces Logseq uses for :db/ident except for
+  db-attribute-ident. It's important to grow this list purposefully and have it
+  start with 'logseq' to allow for users and 3rd party plugins to provide their
+  own namespaces to core concepts."
+  (into logseq-property-namespaces #{"logseq.class" "logseq.kv"}))
+
+(def logseq-ident
+  [:and :keyword [:fn
+                  {:error/message "should be a valid :db/ident namespace"}
+                  (fn logseq-namespace? [k]
+                    (contains? logseq-ident-namespaces (namespace k)))]])
 
 ;; Helper fns
 ;; ==========
@@ -28,11 +79,24 @@
                        (let [[property-type schema-fn] e
                              schema-fn' (if (db-property-type/property-types-with-db property-type) (partial schema-fn db) schema-fn)
                              validation-fn #(validate-property-value property-type schema-fn' %)]
-                         [property-type [:tuple :entity [:fn validation-fn]]])
+                         [property-type [:tuple property-ident [:fn validation-fn]]])
                        :else
                        e)))
                  db-schema))
 
+(defn update-properties-in-ents
+  "Prepares properties in entities to be validated by DB schema"
+  [ents]
+  (mapv
+   (fn [ent]
+     (reduce (fn [m [k v]]
+               (if (db-property/property? k)
+                 (update m :block/properties (fnil conj []) [k v])
+                 (assoc m k v)))
+             {}
+             ent))
+   ents))
+
 (defn datoms->entity-maps
   "Returns entity maps for given :eavt datoms"
   [datoms]
@@ -43,17 +107,33 @@
                    (update acc (:e m) assoc (:a m) (:v m))))
                {})))
 
-;; Malli schemas
-;; =============
+;; Main malli schemas
+;; ==================
 ;; These schemas should be data vars to remain as simple and reusable as possible
+(def property-tuple
+  "Represents a tuple of a property and its property value. This schema
+   has 2 metadata hooks which are used to inject a datascript db later"
+  (into
+   [:multi {:dispatch ^:add-db (fn [db property-tuple]
+                                 (get-in (d/entity db (first property-tuple))
+                                         [:block/schema :type]))}]
+   (map (fn [[prop-type value-schema]]
+          ^:property-value [prop-type (if (vector? value-schema) (last value-schema) value-schema)])
+        db-property-type/built-in-validation-schemas)))
+
+(def block-properties
+  "Validates a slightly modified version of :block/properties. Properties are
+  expected to be a vector of tuples instead of a map in order to validate each
+  property with its property value that is valid for its type"
+  [:sequential property-tuple])
 
-;; FIXME: validate properties
 (def page-or-block-attrs
   "Common attributes for page and normal blocks"
   [[:block/uuid :uuid]
    [:block/created-at :int]
    [:block/updated-at :int]
    [:block/format [:enum :markdown]]
+   [:block/properties {:optional true} block-properties]
    [:block/refs {:optional true} [:set :int]]
    [:block/tags {:optional true} [:set :int]]
    [:block/collapsed-properties {:optional true} [:set :int]]
@@ -69,37 +149,6 @@
     ;; TODO: Should this be here or in common?
    [:block/path-refs {:optional true} [:set :int]]])
 
-(def logseq-ident-namespaces
-  "Set of all namespaces Logseq uses for :db/ident. It's important to grow this
-  list purposefully and have it start with 'logseq' to allow for users and 3rd
-  party plugins to provide their own namespaces to core concepts."
-  #{"logseq.property" "logseq.property.table" "logseq.property.tldraw"
-    "logseq.class" "logseq.task" "logseq.kv"})
-
-(def user-ident-namespaces
-  "Set of all namespaces Logseq uses for :db/ident. It's important to grow this
-  list purposefully and have it start with 'logseq' to allow for users and 3rd
-  party plugins to provide their own namespaces to core concepts."
-  #{"user.property"})
-
-(def db-attribute
-  [:and :keyword [:fn
-                  {:error/message "should be a valid db attribute"}
-                  (fn db-attribute? [k]
-                    (contains? #{"block"} (namespace k)))]])
-
-(def logseq-ident
-  [:and :keyword [:fn
-                  {:error/message "should be a valid :db/ident namespace"}
-                  (fn logseq-namespace? [k]
-                    (contains? logseq-ident-namespaces (namespace k)))]])
-
-(def user-ident
-  [:and :keyword [:fn
-                  {:error/message "should be a valid :db/ident namespace"}
-                  (fn user-namespace? [k]
-                    (contains? user-ident-namespaces (namespace k)))]])
-
 (def property-attrs
   "Common attributes for properties"
   [[:db/index {:optional true} :boolean]
@@ -118,7 +167,7 @@
     page-or-block-attrs)))
 
 (def class-attrs
-  [[:db/ident {:optional true} :keyword]
+  [[:db/ident {:optional true} logseq-ident]
    [:class/parent {:optional true} :int]
    [:class/schema.properties {:optional true} [:set :int]]])
 
@@ -154,7 +203,7 @@
   (vec
    (concat
     [:map
-     [:db/ident [:or logseq-ident db-attribute]]
+     [:db/ident internal-property-ident]
      [:block/schema
       (vec
        (concat
@@ -189,7 +238,7 @@
   (vec
    (concat
     [:map
-     [:db/ident user-ident]
+     [:db/ident user-property-ident]
      [:block/schema {:optional true} user-property-schema]]
     property-attrs
     page-attrs
@@ -197,8 +246,8 @@
 
 (def property-page
   [:multi {:dispatch (fn [m]
-                       (or (string/starts-with? (namespace (m :db/ident)) "logseq.")
-                           (= "block" (namespace (m :db/ident)))))}
+                       (or (contains? logseq-property-namespaces (namespace (m :db/ident)))
+                           (contains? db-attribute-properties (m :db/ident))))}
    [true internal-property]
    [:malli.core/default user-property]])
 
@@ -252,7 +301,7 @@
     [:map]
     [[:block/type [:= #{"closed value"}]]
      ;; for built-in properties
-     [:db/ident {:optional true} logseq-ident]
+     [:db/ident {:optional true} logseq-property-ident]
      [:block/schema {:optional true}
       [:map
        [:value [:or :string :double]]

deps/db/src/logseq/db/frontend/property/util.cljs

@@ -42,8 +42,7 @@
   [property]
   (let [page-name (str hidden-page-name-prefix (:block/uuid property))]
     (-> (sqlite-util/build-new-page page-name)
-        (assoc :block/type #{"hidden"}
-               :block/format :markdown))))
+        (assoc :block/type #{"hidden"}))))
 
 (defn build-closed-values
   "Builds all the tx needed for property with closed values including

deps/db/src/logseq/db/frontend/rules.cljc

@@ -169,13 +169,13 @@
         [(not= #{} ?v)]]
 
     :page-property
-    '[;; Clause 1: Match non-ref values
-      [(page-property ?p ?prop ?val)
+    '[[(page-property ?p ?prop ?val)
        [?p :block/name]
        [?p ?prop ?val]
        [?prop-e :db/ident ?prop]
        [?prop-e :block/type "property"]]
       ;; TODO: Delete when DB_GRAPH query-dsl tests are passing
+      ;; Clause 1: Match non-ref values
       #_[(page-property ?p ?key ?val)
          [?p :block/name]
          [?p :block/properties ?prop]

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

@@ -23,7 +23,7 @@
   [{:keys [db-after tx-data tx-meta]} validate-options]
   (let [changed-ids (->> tx-data (map :e) distinct)
         ent-maps* (->> changed-ids (mapcat #(d/datoms db-after :eavt %)) db-malli-schema/datoms->entity-maps vals)
-        ent-maps (vec ent-maps*)
+        ent-maps (db-malli-schema/update-properties-in-ents ent-maps*)
         db-schema (update-schema db-malli-schema/DB db-after validate-options)
         explain-result (m/explain db-schema ent-maps)]
     (js/console.log "changed eids:" changed-ids tx-meta)
@@ -68,9 +68,8 @@
   [db]
   (let [datoms (d/datoms db :eavt)
         ent-maps* (db-malli-schema/datoms->entity-maps datoms)
-        ent-maps (vec (vals ent-maps*))
-        ;; FIXME: Fix validation for closed-schema? true
-        schema (update-schema db-malli-schema/DB db {:closed-schema? false})
+        ent-maps (db-malli-schema/update-properties-in-ents (vals ent-maps*))
+        schema (update-schema db-malli-schema/DB db {:closed-schema? true})
         errors (->> ent-maps (m/explain schema) :errors)]
     (cond-> {:datom-count (count datoms)
              :entities ent-maps}

deps/db/src/logseq/db/sqlite/create_graph.cljs

@@ -63,8 +63,7 @@
                         :file/content ""
                         :file/last-modified-at (js/Date.)}]
         default-pages (->> (map sqlite-util/build-new-page built-in-pages-names)
-                           (map #(assoc % :block/format :markdown))
-                           (map #(default-db/mark-block-as-built-in %)))
+                           (map default-db/mark-block-as-built-in))
         default-properties (build-initial-properties)
         db-ident->properties (zipmap
                               (map :db/ident default-properties)

deps/db/src/logseq/db/sqlite/util.cljs

@@ -111,4 +111,5 @@
    {:block/name (common-util/page-name-sanity-lc page-name)
     :block/original-name page-name
     :block/journal? false
-    :block/uuid (d/squuid)}))
+    :block/uuid (d/squuid)
+    :block/format :markdown}))

deps/db/test/logseq/db/frontend/rules_test.cljs

@@ -21,7 +21,7 @@
   (let [conn (new-db-conn)
         _ (d/transact! conn [(sqlite-util/build-new-property :user.property/foo "foo" {})
                              (sqlite-util/build-new-property :user.property/foo2 "foo2" {})
-                             (assoc (sqlite-util/build-new-page "Page") :block/format :markdown)
+                             (sqlite-util/build-new-page "Page")
                              {:block/original-name "Page" :user.property/foo "bar"}])]
     (is (= ["Page"]
            (->> (q-with-rules '[:find (pull ?b [:block/original-name]) :where (has-page-property ?b :user.property/foo)]
@@ -44,9 +44,14 @@
         _ (d/transact! conn [(sqlite-util/build-new-property :user.property/foo "foo" {})
                              (sqlite-util/build-new-property :user.property/foo2 "foo2" {})
                              (sqlite-util/build-new-property :user.property/number-many "number-many" {:type :number :cardinality :many})
-                             (assoc (sqlite-util/build-new-page "Page") :block/format :markdown)
+                             (sqlite-util/build-new-property :user.property/page-many "page-many" {:type :page :cardinality :many})
+                             (sqlite-util/build-new-page "Page")
+                             (sqlite-util/build-new-page "Page A")
                              {:block/original-name "Page" :user.property/foo "bar"}
-                             {:block/original-name "Page" :user.property/number-many #{5 10}}])]
+                             {:block/original-name "Page" :user.property/number-many #{5 10}}
+                             {:block/original-name "Page" :user.property/page-many #{[:block/original-name "Page A"]}}
+                             {:block/original-name "Page A" :user.property/foo "bar A"}])]
+
     (testing "cardinality :one property"
       (is (= ["Page"]
              (->> (q-with-rules '[:find (pull ?b [:block/original-name]) :where (page-property ?b :user.property/foo "bar")]
@@ -83,8 +88,23 @@
                   set))
           "page-property can bind to property arg with bound property value"))
 
-    (testing "binding when property value is unspecified"
-      (is (= #{:user.property/foo :user.property/number-many}
+    ;; NOTE: Querying a ref's name is different than before and requires more than just the rule
+    (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"]]
+                                @conn)
+                  (map (comp :block/original-name first))))
+          "page-property returns result when page has property")
+      (is (= []
+             (->> (q-with-rules '[:find (pull ?b [:block/original-name])
+                                  :where (page-property ?b :user.property/page-many ?pv) [?pv :block/original-name "Page B"]]
+                                @conn)
+                  (map (comp :block/original-name first))))
+          "page-property returns no result when page doesn't have property value"))
+
+    (testing "bindings with property value"
+      (is (= #{:user.property/foo :user.property/number-many :user.property/page-many}
              (->> (q-with-rules '[:find [?p ...]
                                   :where (page-property ?b ?p _) [?b :block/original-name "Page"]]
                                 @conn)
@@ -92,9 +112,19 @@
           "page-property can bind to property arg with unbound property value")
       (is (= #{[:user.property/number-many 10]
                [:user.property/number-many 5]
-               [:user.property/foo "bar"]}
+               [:user.property/foo "bar"]
+               [:user.property/page-many (:db/id (d/entity @conn [:block/original-name "Page A"]))]}
              (->> (q-with-rules '[:find ?p ?v
                                   :where (page-property ?b ?p ?v) [?b :block/original-name "Page"]]
                                 @conn)
                   set))
-          "page-property can bind to property and property value args"))))
+          "page-property can bind to property and property value args")
+      (is (= #{"Page"}
+             (->> (q-with-rules '[:find (pull ?b [:block/original-name])
+                                  :where
+                                  (page-property ?b :user.property/page-many ?pv)
+                                  (page-property ?pv :user.property/foo "bar A")]
+                                @conn)
+                  (map (comp :block/original-name first))
+                  set))
+          "page-property can be used multiple times to query a property value's property"))))

deps/graph-parser/src/logseq/graph_parser/exporter.cljs

@@ -183,8 +183,7 @@
                             (assoc (sqlite-util/build-new-page
                                     (date-time-util/int->journal-title deadline (common-config/get-date-formatter user-config)))
                                    :block/journal? true
-                                   :block/journal-day deadline
-                                   :block/format :markdown))]
+                                   :block/journal-day deadline))]
       (-> block
           (update :block/properties assoc deadline-prop (:block/uuid deadline-page))
           (update :block/refs (fnil into []) [:logseq.task/deadline deadline-page])
@@ -204,8 +203,7 @@
                             (assoc (sqlite-util/build-new-page
                                     (date-time-util/int->journal-title scheduled (common-config/get-date-formatter user-config)))
                                    :block/journal? true
-                                   :block/journal-day scheduled
-                                   :block/format :markdown))]
+                                   :block/journal-day scheduled))]
       (-> block
           (update :block/properties assoc scheduled-prop (:block/uuid scheduled-page))
           (update :block/refs (fnil into []) [:logseq.task/scheduled scheduled-page])

scripts/src/logseq/tasks/db_graph/create_graph_with_properties.cljs

@@ -60,7 +60,8 @@
         random-page-closed-value #(let [val (-> closed-values-config % rand-nth :value)]
                                     (swap! closed-values assoc % (second val))
                                     val)
-        get-closed-value #(get @closed-values %)]
+        ;; TODO: Remove default when page-closed/date-closed re-enabled
+        get-closed-value #(get @closed-values % "")]
     {:pages-and-blocks
      ;; Journals
      [{:page
@@ -88,11 +89,12 @@
         {:block/content "number-closed property block" :properties {:number-closed (random-closed-value :number-closed)}}
         {:block/content "page property block" :properties {:page [:page "page 1"]}}
         {:block/content "page-many property block" :properties {:page-many #{[:page "page 1"] [:page "page 2"]}}}
-        {:block/content "page-closed property block" :properties {:page-closed (random-page-closed-value :page-closed)}}
+        ;; TODO: Fix page-closed and date-closed
+        #_{:block/content "page-closed property block" :properties {:page-closed (random-page-closed-value :page-closed)}}
         {:block/content "date property block" :properties {:date [:page (date-journal-title today)]}}
         {:block/content "date-many property block" :properties {:date-many #{[:page (date-journal-title today)]
                                                                              [:page (date-journal-title yesterday)]}}}
-        {:block/content "date-closed property block" :properties {:date-closed (random-page-closed-value :date-closed)}}]}
+        #_{:block/content "date-closed property block" :properties {:date-closed (random-page-closed-value :date-closed)}}]}
       {:page {:block/original-name "Block Property Queries"}
        :blocks
        [{:block/content "{{query (property :default \"haha\")}}"}
@@ -132,7 +134,7 @@
       {:page {:block/name "number-closed page" :properties {:number-closed (random-closed-value :number-closed)}}}
       {:page {:block/name "page page" :properties {:page [:page "page 1"]}}}
       {:page {:block/name "page-many page" :properties {:page-many #{[:page "page 1"] [:page "page 2"]}}}}
-      {:page {:block/name "page-closed page" :properties {:page-closed (random-page-closed-value :page-closed)}}}
+      #_{:page {:block/name "page-closed page" :properties {:page-closed (random-page-closed-value :page-closed)}}}
       {:page {:block/name "date page" :properties {:date [:page (date-journal-title today)]}}}
       {:page {:block/name "date-many page" :properties {:date-many #{[:page (date-journal-title today)]
                                                                      [:page (date-journal-title yesterday)]}}}}