diff --git a/CHANGELOG.md b/CHANGELOG.md index bec8f359..013f5c79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Support for nested maps in `transact!` (also fixes #38) - [ BREAKING ] Custom aggregate fns must be called via special syntax (`aggregate` keyword): `(q '[:find (aggregate ?myfn ?e) :in $ ?myfn ...])`. Built-in aggregates work as before: `(q '[:find (count ?e) ...]` - Return nil from `entity` when passed nil eid (issue #47) +- Transaction data is now validated, with proper error messages (also fixes #48) # 0.7.2 diff --git a/src/datascript/core.cljs b/src/datascript/core.cljs index 7b7da85a..0ad8e8c5 100644 --- a/src/datascript/core.cljs +++ b/src/datascript/core.cljs @@ -298,11 +298,25 @@ (keyword (namespace attr) (subs (name attr) 1)) (keyword (namespace attr) (str "_" (name attr))))) +(defn- validate-eid [eid at] + (when-not (number? eid) + (throw (js/Error. (str "Bad entity id " eid " at " at ", expected number"))))) + +(defn- validate-attr [attr at] + (when-not (or (keyword? attr) (string? attr)) + (throw (js/Error. (str "Bad entity attribute " attr " at " at ", expected keyword or string"))))) + +(defn- validate-val [v at] + (when (nil? v) + (throw (js/Error. (str "Cannot store nil as a value at " at))))) + (defn- explode [db entity] (let [eid (:db/id entity)] (for [[a vs] entity :when (not= a :db/id) - :let [reverse? (reverse-ref? a) + :let [_ (when-not (or (keyword? a) (string? a)) + (throw (js/Error. (str "Bad entity attribute at {:db/id " eid ", " a " " vs "}, expected keyword or string")))) + reverse? (reverse-ref? a) straight-a (if reverse? (reverse-ref a) a) _ (when (and reverse? (not (ref? db straight-a))) (throw (js/Error. (str "Bad attribute " a ": reverse attribute name requires {:db/valueType :db.type/ref} in schema"))))] @@ -316,10 +330,15 @@ [:db/add v straight-a eid] [:db/add eid straight-a v]))))) -(defn- transact-add [report [_ e a v]] - (let [tx (current-tx report) - db (:db-after report) - datom (Datom. e a v tx true)] +(defn- transact-add [report [_ e a v :as ent]] + (validate-eid e ent) + (validate-attr a ent) + (validate-val v ent) + (let [tx (current-tx report) + db (:db-after report) + datom (Datom. e a v tx true)] + (when (and (ref? db a) (not (number? v))) + (throw (js/Error. (str "Bad value at " ent ", expected number due to {:db/valueType :db.type/ref}")))) (if (multival? db a) (if (empty? (-search db [e a v])) (transact-report report datom) @@ -345,8 +364,11 @@ (filter #(component? db (.-a %))) (map #(vector :db.fn/retractEntity (.-v %)))) datoms)) -(defn- transact-tx-data [report [entity & entities :as es]] - (let [db (:db-after report)] +(defn- transact-tx-data [report es] + (when-not (or (nil? es) (sequential? es)) + (throw (js/Error. (str "Bad transaction data " es ", expected sequential collection")))) + (let [[entity & entities] es + db (:db-after report)] (cond (nil? entity) (-> report @@ -366,7 +388,7 @@ :else (recur report (concat (explode db entity) entities))) - :else + (sequential? entity) (let [[op e a v] entity] (cond (= op :db.fn/call) @@ -375,6 +397,10 @@ (= op :db.fn/cas) (let [[_ e a ov nv] entity + _ (validate-eid e entity) + _ (validate-attr a entity) + _ (validate-val ov entity) + _ (validate-val nv entity) datoms (-search db [e a])] (if (multival? db a) (if (some #(= (.-v %) ov) datoms) @@ -405,17 +431,32 @@ (recur (transact-add report entity) entities) (= op :db/retract) - (if-let [old-datom (first (-search db [e a v]))] - (recur (transact-retract-datom report old-datom) entities) - (recur report entities)) + (do + (validate-eid e entity) + (validate-attr a entity) + (validate-val v entity) + (if-let [old-datom (first (-search db [e a v]))] + (recur (transact-retract-datom report old-datom) entities) + (recur report entities))) (= op :db.fn/retractAttribute) - (let [datoms (-search db [e a])] - (recur (reduce transact-retract-datom report datoms) - (concat (retract-components db datoms) entities))) + (do + (validate-eid e entity) + (validate-attr a entity) + (let [datoms (-search db [e a])] + (recur (reduce transact-retract-datom report datoms) + (concat (retract-components db datoms) entities)))) (= op :db.fn/retractEntity) - (let [e-datoms (-search db [e]) - v-datoms (mapcat (fn [a] (-search db [nil a e])) (-refs db))] - (recur (reduce transact-retract-datom report (concat e-datoms v-datoms)) - (concat (retract-components db e-datoms) entities)))))))) + (do + (validate-eid e entity) + (let [e-datoms (-search db [e]) + v-datoms (mapcat (fn [a] (-search db [nil a e])) (-refs db))] + (recur (reduce transact-retract-datom report (concat e-datoms v-datoms)) + (concat (retract-components db e-datoms) entities)))) + + :else + (throw (js/Error. (str "Unknown operation at " entity ", expected :db/add, :db/retract, :db.fn/call, :db.fn/retractAttribute or :db.fn/retractEntity"))))) + :else + (throw (js/Error. (str "Bad entity type at " entity ", expected map or vector"))) + ))) diff --git a/test/test/datascript.cljs b/test/test/datascript.cljs index f3dbdc40..86fda3c9 100644 --- a/test/test/datascript.cljs +++ b/test/test/datascript.cljs @@ -44,6 +44,30 @@ :where [1 :name ?v]] db) #{["Petr"]})))))) +(deftest test-with-validation + (let [db (d/empty-db {:profile { :db/valueType :db.type/ref }})] + (are [tx] (thrown-with-msg? js/Error #"Bad entity id" (d/db-with db tx)) + [[:db/add nil :name "Ivan"]] + [[:db/add "aaa" :name "Ivan"]] + [{:db/id "aaa" :name "Ivan"}]) + + (are [tx] (thrown-with-msg? js/Error #"Bad entity attribute" (d/db-with db tx)) + [[:db/add -1 nil "Ivan"]] + [[:db/add -1 17 "Ivan"]] + [{:db/id -1 17 "Ivan"}]) + + (are [tx] (thrown-with-msg? js/Error #"Cannot store nil as a value" (d/db-with db tx)) + [[:db/add -1 :name nil]] + [{:db/id -1 :name nil}]) + + (are [tx] (thrown-with-msg? js/Error #"Bad value" (d/db-with db tx)) + [[:db/add -1 :profile "aaa"]] + [{:db/id -1 :profile "aaa"}]) + + (is (thrown-with-msg? js/Error #"Unknown operation" (d/db-with db [["aaa" :name "Ivan"]]))) + (is (thrown-with-msg? js/Error #"Bad entity type at" (d/db-with db [:db/add "aaa" :name "Ivan"]))) + (is (thrown-with-msg? js/Error #"Bad transaction data" (d/db-with db {:profile "aaa"}))))) + (deftest test-retract-fns (let [db (-> (d/empty-db {:aka { :db/cardinality :db.cardinality/many } :friend { :db/valueType :db.type/ref }}) @@ -647,10 +671,10 @@ :where [(> 2 1)]] [:a :b :c]) #{[:a] [:b] [:c]}))) - (let [db (-> (d/empty-db {:parent {:parent {:db/valueType :db.valueType/ref}}}) + (let [db (-> (d/empty-db {:parent {:db/valueType :db.type/ref}}) (d/db-with [ { :db/id 1, :name "Ivan", :age 15 } - { :db/id 2, :name "Petr", :age 22, :height 240 :parent 1} - { :db/id 3, :name "Slava", :age 37 :parent 2}]))] + { :db/id 2, :name "Petr", :age 22, :height 240, :parent 1} + { :db/id 3, :name "Slava", :age 37, :parent 2}]))] (testing "get-else" (is (= (d/q '[:find ?e ?age ?height