Add validation for property values

Reuse schema for property types and values from frontend. Moved into db
deps as it will soon be needed for a db namespace. Also tweaked schema
for :template type

.clj-kondo/config.edn

@@ -118,6 +118,7 @@
              logseq.common.graph common-graph
              logseq.common.config common-config
              logseq.db.property db-property
+             logseq.db.property.type db-property-type
              logseq.db.rules rules
              logseq.db.schema db-schema
              logseq.db.sqlite.db sqlite-db

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

@@ -2,14 +2,6 @@
   "Property related fns for DB graphs and frontend/datascript usage"
   (:require [clojure.set :as set]))
 
-(def internal-builtin-schema-types
-  "Valid schema :type only to be used by built-in-properties"
-  #{:keyword :map :coll :any})
-
-(def user-builtin-schema-types
-  "Valid schema :type for users in order they appear in the UI"
-  [:default :number :date :checkbox :url :page :template :enum])
-
 ;; FIXME: no support for built-in-extended-properties
 (def ^:large-vars/data-var built-in-properties
   "Map of built in properties for db graphs. Each property has a config map with

deps/db/src/logseq/db/property/type.cljs

@@ -0,0 +1,75 @@
+(ns logseq.db.property.type
+  "Provides property types including fns to validate them"
+  (:require [datascript.core :as d]))
+
+(def internal-builtin-schema-types
+  "Valid schema :type only to be used by built-in-properties"
+  #{:keyword :map :coll :any})
+
+(def user-builtin-schema-types
+  "Valid schema :type for users in order they appear in the UI"
+  [:default :number :date :checkbox :url :page :template :enum])
+
+;; TODO:
+;; Validate && list fixes for non-validated values when updating property schema
+
+(defn url?
+  "Test if it is a `protocol://`-style URL.
+   Originally from gp-util/url? but does not need to be the same"
+  [s]
+  (and (string? s)
+       (try
+         (not (contains? #{nil "null"} (.-origin (js/URL. s))))
+         (catch :default _e
+           false))))
+
+(defn- logseq-page?
+  [db id]
+  (and (uuid? id)
+       (when-let [e (d/entity db [:block/uuid id])]
+         (nil? (:block/page e)))))
+
+;; FIXME: template instance check
+(defn- logseq-template?
+  [db id]
+  (and (uuid? id)
+       (some? (d/entity db [:block/uuid id]))))
+
+(defn- text-or-uuid?
+  [value]
+  (or (string? value) (uuid? value)))
+
+(def builtin-schema-types
+  {:default  [:fn
+              {:error/message "should be a text or UUID"}
+              text-or-uuid?]                     ; refs/tags will not be extracted
+   :number   number?
+   :date     [:fn
+              {:error/message "should be a journal date"}
+              logseq-page?]
+   :checkbox boolean?
+   :url      [:fn
+              {:error/message "should be a URL"}
+              url?]
+   :page     [:fn
+              {:error/message "should be a page"}
+              logseq-page?]
+   :template [:fn
+              {:error/message "should has #template"}
+              logseq-template?]
+   :enum     some?                      ; the value could be anything such as number, text, url, date, page, image, video, etc.
+   ;; internal usage
+   :keyword  keyword?
+   :map      map?
+   ;; coll elements are ordered as it's saved as a vec
+   :coll     coll?
+   :any      some?})
+
+(def property-types-with-db
+  "Property types whose validation fn requires a datascript db"
+  #{:date :page :template})
+
+(assert (= (set (keys builtin-schema-types))
+           (into internal-builtin-schema-types
+                 user-builtin-schema-types))
+        "Built-in schema types must be equal")

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

@@ -4,6 +4,7 @@
             [logseq.db.sqlite.db :as sqlite-db]
             [logseq.db.schema :as db-schema]
             [logseq.db.property :as db-property]
+            [logseq.db.property.type :as db-property-type]
             [datascript.core :as d]
             [clojure.string :as string]
             [nbb.core :as nbb]
@@ -15,24 +16,39 @@
             ["os" :as os]
             [cljs.pprint :as pprint]))
 
+(defn- validate-property-value
+  "Validates the value in a property tuple. The property value can be one or
+  many of a value to validated"
+  [prop-type schema-fn val]
+  (if (and (or (sequential? val) (set? val))
+           (not= :coll prop-type))
+    (every? schema-fn val)
+    (schema-fn val)))
+
+(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 [:block/uuid (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/builtin-schema-types)))
+
+(def block-properties
+  "Validates a slightly modified verson 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])
+
 (def page-or-block-attrs
   "Common attributes for page and normal blocks"
   [[:block/uuid :uuid]
    [:block/created-at :int]
    [:block/updated-at :int]
    [:block/properties {:optional true}
-    [:map-of :uuid [:or
-                    :string
-                    :int
-                    :boolean
-                    :uuid
-                    :map
-                    [:vector [:or :keyword :uuid]]
-                    [:set :uuid]
-                     ;; TODO: Remove when bug is fixed
-                    [:sequential :uuid]
-                    [:set :string]
-                    [:set :int]]]]
+    block-properties]
    [:block/refs {:optional true} [:set :int]]
    [:block/tags {:optional true} [:set :int]]
    [:block/tx-id {:optional true} :int]])
@@ -88,8 +104,8 @@
     [[:block/schema
       [:map
        {:closed false}
-       [:type (apply vector :enum (into db-property/internal-builtin-schema-types
-                                        db-property/user-builtin-schema-types))]
+       [:type (apply vector :enum (into db-property-type/internal-builtin-schema-types
+                                        db-property-type/user-builtin-schema-types))]
        [:hide? {:optional true} :boolean]
        [:cardinality {:optional true} [:enum :one :many]]]]]
     page-attrs
@@ -102,15 +118,18 @@
     [[:block/schema
       [:map
        {:closed false}
-       [:type (apply vector :enum db-property/user-builtin-schema-types)]
+       [:type (apply vector :enum db-property-type/user-builtin-schema-types)]
        [:hide? {:optional true} :boolean]
        [:description {:optional true} :string]
        ;; For any types except for :checkbox :default :template :enum
        [:cardinality {:optional true} [:enum :one :many]]
        ;; Just for :enum type
        [:enum-config {:optional true} :map]
-       ;; Just for :page and :template types
-       [:classes {:optional true} [:set :uuid]]]]]
+       ;; :template uses :sequential and :page uses :set.
+       ;; Should :template should use :set?
+       [:classes {:optional true} [:or
+                                   [:set :uuid]
+                                   [:sequential :uuid]]]]]]
     page-attrs
     page-or-block-attrs)))
 
@@ -135,7 +154,8 @@
    [:block/metadata {:optional true}
     [:map {:closed false}
      [:created-from-block :uuid]
-     [:created-from-property :uuid]]]
+     [:created-from-property :uuid]
+     [:created-from-template {:optional true} :uuid]]]
     ;; refs
    [:block/page :int]
    [:block/path-refs {:optional true} [:set :int]]
@@ -217,25 +237,48 @@
                                                    vec)}]))
                     (into {}))}))))
 
+(defn- update-schema
+  "Updates the db schema to add a datascript db for property validations
+   and to optionally close maps"
+  [db-schema db {:keys [closed-maps]}]
+  (let [db-schema-with-property-vals
+        (walk/postwalk (fn [e]
+                         (let [meta' (meta e)]
+                           (cond
+                             (:add-db meta')
+                             (partial e db)
+                             (:property-value meta')
+                             (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 :uuid [:fn validation-fn]]])
+                             :else
+                             e)))
+                       db-schema)]
+    (if closed-maps
+      (walk/postwalk (fn [e]
+                       (if (and (vector? e)
+                                (= :map (first e))
+                                (contains? (second e) :closed))
+                         (assoc e 1 (assoc (second e) :closed true))
+                         e))
+                     db-schema-with-property-vals)
+      db-schema-with-property-vals)))
+
 (defn validate-client-db
   "Validate datascript db as a vec of entity maps"
-  [db ent-maps {:keys [closed-maps verbose group-errors]}]
-  (let [schema (if closed-maps
-                 (walk/postwalk (fn [e]
-                                  (if (and (vector? e)
-                                           (= :map (first e))
-                                           (contains? (second e) :closed))
-                                    (assoc e 1 (assoc (second e) :closed true))
-                                    e))
-                                client-db-schema)
-                 client-db-schema)]
+  [db ent-maps* {:keys [verbose group-errors] :as options}]
+  (let [ent-maps (vec (map #(if (:block/properties %)
+                              (update % :block/properties (fn [x] (mapv identity x)))
+                              %)
+                           (vals ent-maps*)))
+        schema (update-schema client-db-schema db options)]
     (if-let [errors (->> ent-maps
-                         vals
                          (m/explain schema)
                          :errors)]
       (do
         (if group-errors
-          (let [ent-errors (build-grouped-errors db (vec (vals ent-maps)) errors)]
+          (let [ent-errors (build-grouped-errors db ent-maps errors)]
             (println "Found" (count ent-errors) "entities in errors:")
             (if verbose
               (pprint/pprint ent-errors)
@@ -243,12 +286,11 @@
           (do
             (println "Found" (count errors) "errors:")
             (if verbose
-              (let [full-maps (vec (vals ent-maps))]
-                (pprint/pprint
-                 (map #(assoc %
-                              :entity (get full-maps (-> % :in first))
-                              :schema (m/form (:schema %)))
-                      errors)))
+              (pprint/pprint
+               (map #(assoc %
+                            :entity (get ent-maps (-> % :in first))
+                            :schema (m/form (:schema %)))
+                    errors))
               (pprint/pprint errors))))
         (js/process.exit 1))
       (println "Valid!"))))
@@ -285,7 +327,9 @@
         conn (sqlite-cli/read-graph db-name)
         datoms (d/datoms @conn :eavt)
         ent-maps (datoms->entity-maps datoms)]
-    (println "Read graph" (str db-name " with " (count datoms) " datoms!"))
+    (println "Read graph" (str db-name " with " (count datoms) " datoms, "
+                               (count ent-maps) " entities and "
+                               (count (mapcat :block/properties (vals ent-maps))) " properties"))
     (validate-client-db @conn ent-maps options)))
 
 (defn -main [argv]

src/main/frontend/components/property.cljs

@@ -19,6 +19,7 @@
             [frontend.ui :as ui]
             [frontend.util :as util]
             [logseq.db.property :as db-property]
+            [logseq.db.property.type :as db-property-type]
             [rum.core :as rum]
             [frontend.handler.route :as route-handler]
             [frontend.components.icon :as icon-component]
@@ -327,9 +328,9 @@
 
       [:div.grid.grid-cols-4.gap-1.items-center.leading-8
        [:label.col-span-1 "Schema type:"]
-       (let [schema-types (->> (concat db-property/user-builtin-schema-types
+       (let [schema-types (->> (concat db-property-type/user-builtin-schema-types
                                        (when built-in-property?
-                                         db-property/internal-builtin-schema-types))
+                                         db-property-type/internal-builtin-schema-types))
                                (map (fn [type]
                                       {:label (property-type-label type)
                                        :disabled disabled?

src/main/frontend/handler/db_based/property.cljs

@@ -9,63 +9,24 @@
             [frontend.util :as util]
             [logseq.graph-parser.util :as gp-util]
             [logseq.db.sqlite.util :as sqlite-util]
-            [logseq.db.property :as db-property]
+            [logseq.db.property.type :as db-property-type]
             [malli.util :as mu]
             [malli.error :as me]))
 
-;; TODO:
-;; Validate && list fixes for non-validated values when updating property schema
-
-(defn- logseq-page?
-  [id]
-  (and (uuid? id)
-       (when-let [e (db/entity [:block/uuid id])]
-         (nil? (:block/page e)))))
-
-;; FIXME: template instance check
-(defn- logseq-template?
-  [id]
-  (and (uuid? id)
-       (some? (db/entity [:block/uuid id]))))
-
-(defn- text-or-uuid?
-  [value]
-  (or (string? value) (uuid? value)))
-
-(def builtin-schema-types
-  {:default  [:fn
-              {:error/message "should be a text or UUID"}
-              text-or-uuid?]                     ; refs/tags will not be extracted
-   :number   number?
-   :date     [:fn
-              {:error/message "should be a journal date"}
-              logseq-page?]
-   :checkbox boolean?
-   :url      [:fn
-              {:error/message "should be a URL"}
-              gp-util/url?]
-   :page     [:fn
-              {:error/message "should be a page"}
-              logseq-page?]
-   :template [:fn
-              {:error/message "should has #template"}
-              logseq-template?]
-   :enum     some?                      ; the value could be anything such as number, text, url, date, page, image, video, etc.
-   ;; internal usage
-   :keyword  keyword?
-   :map      map?
-   ;; coll elements are ordered as it's saved as a vec
-   :coll     coll?
-   :any      some?})
-
-(assert (= (set (keys builtin-schema-types))
-           (into db-property/internal-builtin-schema-types
-                 db-property/user-builtin-schema-types))
-        "Built-in schema types must be equal")
-
 ;; schema -> type, cardinality, object's class
 ;;           min, max -> string length, number range, cardinality size limit
 
+(def builtin-schema-types
+  "A frontend version of builtin-schema-types that adds the current database to
+   schema fns"
+  (into {}
+        (map (fn [[property-type property-val-schema]]
+               (if (db-property-type/property-types-with-db property-type)
+                 (let [[_ schema-opts schema-fn] property-val-schema]
+                   [property-type [:fn schema-opts #(schema-fn (db/get-db) %)]])
+                 [property-type property-val-schema]))
+             db-property-type/builtin-schema-types)))
+
 (defn- infer-schema-from-input-string
   [v-str]
   (try