Browse Source

enhance: closed value membership validated everywhere

Frontend and lower-level validation now validate the same.
Also simplified metadata injection
Gabriel Horner 1 year ago
parent
commit
311cd13e1f

+ 26 - 17
deps/db/src/logseq/db/frontend/malli_schema.cljs

@@ -50,27 +50,34 @@
 
 ;; Helper fns
 ;; ==========
-(defn- validate-property-value
-  "Validates the value in a property tuple. The property value can be one or
-  many of a value to validated"
-  [schema-fn [property property-val]]
+(defn validate-property-value
+  "Validates the property value in a property tuple. The property value can be
+  one or many of a value to validated. validate-fn is a fn that is called
+  directly on each value to return a truthy value. validate-fn varies by
+  property type"
+  [db validate-fn [{:block/keys [schema] :as property} property-val] & {:keys [new-closed-value?]}]
   ;; For debugging
   ;; (when (not= "logseq.property" (namespace (:db/ident property))) (prn :validate-val property property-val))
-  (if (= (:cardinality property) :many)
-    (every? schema-fn property-val)
-    (or (schema-fn property-val) (= :logseq.property/empty-placeholder property-val))))
+  (let [validate-fn' (if (db-property-type/property-types-with-db (:type schema)) (partial validate-fn db) validate-fn)
+        validate-fn'' (if (and (db-property-type/closed-value-property-types (:type schema))
+                               ;; new closed values aren't associated with the property yet
+                               (not new-closed-value?)
+                               (seq (:values schema)))
+                        (fn closed-value-valid? [val]
+                          (and (validate-fn' val)
+                               (contains? (set (:values schema))
+                                          (:block/uuid (d/entity db val)))))
+                        validate-fn')]
+    (if (= (get-in property [:block/schema :cardinality]) :many)
+      (every? validate-fn'' property-val)
+      (or (validate-fn'' property-val) (= :logseq.property/empty-placeholder property-val)))))
 
 (defn update-properties-in-schema
   "Needs to be called on the DB schema to add the datascript db to it"
   [db-schema db]
   (walk/postwalk (fn [e]
                    (let [meta' (meta e)]
-                     (if (: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 schema-fn' %)]
-                         [property-type [:fn validation-fn]])
-                       e)))
+                     (if (:add-db meta') (partial e db) e)))
                  db-schema))
 
 (defn update-properties-in-ents
@@ -79,8 +86,8 @@
   (mapv
    #(if-let [pair (some->> (:property/pair-property %) (d/entity db))]
       (assoc % :property-tuple
-             [(assoc (select-keys (:block/schema pair) [:type :cardinality])
-                     :db/ident (:db/ident pair))
+             [(hash-map :block/schema (select-keys (:block/schema pair) [:type :cardinality :values])
+                        :db/ident (:db/ident pair))
               (get % (:db/ident pair))])
       %)
    ents))
@@ -346,9 +353,11 @@
   "A tuple of a property map and a property value. This schema
    has 1 metadata hook which is used to inject a datascript db later"
   (into
-   [:multi {:dispatch (comp :type first)}]
+   [:multi {:dispatch #(-> % first :block/schema :type)}]
    (map (fn [[prop-type value-schema]]
-          ^:property-value [prop-type (if (vector? value-schema) (last value-schema) value-schema)])
+          [prop-type
+           (let [schema-fn (if (vector? value-schema) (last value-schema) value-schema)]
+             [:fn (with-meta #(validate-property-value %1 schema-fn %2) {:add-db true})])])
         db-property-type/built-in-validation-schemas)))
 
 (def property-pair

+ 2 - 2
deps/db/src/logseq/db/frontend/property/type.cljs

@@ -100,10 +100,10 @@
 (def built-in-validation-schemas
   "Map of types to malli validation schemas that validate a property value for that type"
   {:default  [:fn
-              {:error/message "should be a entity"}
+              {:error/message "should be a string or an entity"}
               hidden-or-closed-block?]
    :number   [:fn
-              {:error/message "should be a number"}
+              {:error/message "should be a number or an entity"}
               ;; Also handles entity? so no need to use it
               number?]
    :date     [:fn

+ 12 - 19
src/main/frontend/handler/db_based/property.cljs

@@ -22,7 +22,8 @@
             [frontend.handler.property.util :as pu]
             [promesa.core :as p]
             [frontend.db.async :as db-async]
-            [logseq.db :as ldb]))
+            [logseq.db :as ldb]
+            [logseq.db.frontend.malli-schema :as db-malli-schema]))
 
 ;; schema -> type, cardinality, object's class
 ;;           min, max -> string length, number range, cardinality size limit
@@ -45,27 +46,19 @@
        [property-tx-data block-tx-data]))))
 
 (defn- get-property-value-schema
-  "Gets a malli schema to validate the property value for the given property type. Optionally
-   wraps the validation with db and property as needed"
+  "Gets a malli schema to validate the property value for the given property type and builds
+   it with additional args like datascript db"
   [property-type property & {:keys [new-closed-value?]
                              :or {new-closed-value? false}}]
   (let [property-val-schema (or (get db-property-type/built-in-validation-schemas property-type)
-                                (throw (ex-info (str "No validation for property type " (pr-str property-type)) {})))]
-    (if (vector? property-val-schema)
-      (let [[_ schema-opts schema-fn] property-val-schema
-            schema-fn' (if (db-property-type/property-types-with-db property-type)
-                         (partial schema-fn (db/get-db)) schema-fn)
-            ;; TODO: Move this validation into malli-schema
-            schema-fn'' (if (and (db-property-type/closed-value-property-types property-type)
-                                 (not new-closed-value?)
-                                 (seq (get-in property [:block/schema :values])))
-                          (fn closed-value-validate [val]
-                            (and (schema-fn' val)
-                                 (contains? (set (get-in property [:block/schema :values]))
-                                            (:block/uuid (db/entity val)))))
-                          schema-fn')]
-        [:fn schema-opts schema-fn''])
-      property-val-schema)))
+                                (throw (ex-info (str "No validation for property type " (pr-str property-type)) {})))
+        [schema-opts schema-fn] (if (vector? property-val-schema)
+                                  (rest property-val-schema)
+                                  [{} property-val-schema])]
+    [:fn
+     schema-opts
+     (fn validate-property-value [property-val]
+       (db-malli-schema/validate-property-value (db/get-db) schema-fn [property property-val] {:new-closed-value? new-closed-value?}))]))
 
 (defn- fail-parse-long
   [v-str]