瀏覽代碼

fix: validation of single value for a :many property

Single value of a :many property wasn't being rolled up as a set,
in CLI or in app validation
Gabriel Horner 1 年之前
父節點
當前提交
1e1a17aaf4

+ 30 - 15
deps/db/src/logseq/db/frontend/malli_schema.cljs

@@ -85,21 +85,36 @@
    ents))
 
 (defn datoms->entity-maps
-  "Returns entity maps for given :eavt datoms indexed by db/id"
-  [datoms]
-  (->> datoms
-       (reduce (fn [acc {:keys [a e v]}]
-                 (if (contains? db-schema/card-many-attributes a)
-                   (update acc e update a (fnil conj #{}) v)
-                   ;; TODO: Only do this for property pair ents. Is there a way to
-                   ;; confirm property's cardinality w/o more ent lookups?
-                   ;; If there's already a val, automatically start collecting it as a :many
-                   (if-let [existing-val (get-in acc [e a])]
-                     (if (set? existing-val)
-                       (update acc e assoc a (conj existing-val v))
-                       (update acc e assoc a #{existing-val v}))
-                     (update acc e assoc a v))))
-               {})))
+  "Returns entity maps for given :eavt datoms indexed by db/id. Optional keys:
+   * :entity-fn - Optional fn that given an entity id, returns entity. Defaults
+     to just doing a lookup within entity-maps to be as performant as possible"
+  [datoms & {:keys [entity-fn]}]
+  (let [ent-maps
+        (reduce (fn [acc {:keys [a e v]}]
+                  (if (contains? db-schema/card-many-attributes a)
+                    (update acc e update a (fnil conj #{}) v)
+                    ;; If there's already a val, don't clobber it and automatically start collecting it as a :many
+                    (if-let [existing-val (get-in acc [e a])]
+                      (if (set? existing-val)
+                        (update acc e assoc a (conj existing-val v))
+                        (update acc e assoc a #{existing-val v}))
+                      (update acc e assoc a v))))
+                {}
+                datoms)
+        entity-fn' (or entity-fn #(get ent-maps %))]
+    (-> ent-maps
+        (update-vals
+         (fn [v]
+           (let [pair-ent (when (:property/pair-property v) (entity-fn' (:property/pair-property v)))]
+             (if-let [prop-value
+                      (and pair-ent
+                           (= :db.cardinality/many (:db/cardinality pair-ent))
+                           (get v (:db/ident pair-ent)))]
+               (if-not (set? prop-value)
+                 ;; Fix :many property values that only had one value
+                 (assoc v (:db/ident pair-ent) #{prop-value})
+                 v)
+               v)))))))
 
 (defn datoms->entities
   "Returns a vec of entity maps given :eavt datoms"

+ 3 - 1
deps/db/src/logseq/db/frontend/validate.cljs

@@ -22,7 +22,9 @@
   boolean indicating if db is valid"
   [{: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* (-> (mapcat #(d/datoms db-after :eavt %) changed-ids)
+                      (db-malli-schema/datoms->entity-maps {:entity-fn #(d/entity db-after %)})
+                      vals)
         ent-maps (db-malli-schema/update-properties-in-ents db-after ent-maps*)
         db-schema (update-schema db-malli-schema/DB db-after validate-options)
         invalid-ent-maps (remove #(m/validate db-schema [%]) ent-maps)]

+ 1 - 1
src/main/frontend/handler/db_based/property.cljs

@@ -217,7 +217,7 @@
               (upsert-property! repo property-id (assoc property-schema :type property-type) {})
               (let [pair-id (:db/id (db-property/get-pair-e block property-id))
                     tx-data (concat
-                             [(when pair-id [:db/retract pair-id property-id])]
+                             (when pair-id [[:db/retract pair-id property-id]])
                              (build-property-value-tx-data block property-id values' false))]
                 (db/transact! repo tx-data {:outliner-op :save-block})))))))))