Make sure before-update doesn't break update! with a conditions map (…

…#44)

src/toucan2/map_backend/honeysql2.clj

@@ -104,16 +104,28 @@
 (m/defmethod pipeline/transduce-build [#_query-type :toucan.query-type/update.*
                                        #_model      :default
                                        #_query      :toucan.map-backend/honeysql2]
-  [rf query-type model {:keys [kv-args changes], :as parsed-args} query]
+  [rf query-type model {:keys [kv-args changes], :as parsed-args} conditions-map]
   (log/debugf :compile "Building UPDATE query for %s" model)
-  (let [parsed-args (assoc parsed-args :kv-args (merge kv-args query))
+  (let [parsed-args (assoc parsed-args :kv-args (merge kv-args conditions-map))
         built-query (-> {:update (table-and-alias model)
                          :set    changes}
-                        (with-meta (meta query)))]
+                        (with-meta (meta conditions-map)))]
     (log/debugf :compile "=> %s" built-query)
     ;; `:changes` are added to `parsed-args` so we can get the no-op behavior in the default method.
     (next-method rf query-type model (assoc parsed-args :changes changes) built-query)))
 
+;;; For building a SELECT query using the args passed to something like [[toucan2.update/update!]]. This is needed to
+;;; implement [[toucan2.tools.before-update]]. The main syntax difference is a map 'resolved-query' is supposed to be
+;;; treated as a conditions map for update instead of as a raw Honey SQL query.
+
+(m/defmethod pipeline/transduce-build [#_query-type :toucan.query-type/select.instances.from-update
+                                       #_model      :default
+                                       #_query      :toucan.map-backend/honeysql2]
+  ;; treat the resolved query as a conditions map but otherwise behave the same as the
+  ;; `:toucan.query-type/select.instances` impl.
+  [rf query-type model parsed-args conditions-map]
+  (next-method rf query-type model (update parsed-args :kv-args #(merge % conditions-map)) {}))
+
 (m/defmethod pipeline/transduce-build [#_query-type :toucan.query-type/delete.*
                                        #_model      :default
                                        #_query      :toucan.map-backend/honeysql2]

src/toucan2/pipeline.clj

@@ -62,6 +62,13 @@
 ;;; transform to the results, so we don't want to apply a default fields transform or other stuff like that.
 (derive :toucan.query-type/select.instances.fns :toucan.query-type/select.instances)
 
+;;; A special subtype of a SELECT query that should use the syntax of update. Used to power `before-update` and the
+;;; like.
+;;;
+;;; The difference is that update is supposed to treat a resolved query map as a conditions map rather than a Honey SQL
+;;; form.
+(derive :toucan.query-type/select.instances.from-update :toucan.query-type/select.instances)
+
 (doto :toucan.query-type/insert.update-count
   (derive :toucan.query-type/insert.*)
   (derive :toucan.result-type/update-count))

src/toucan2/tools/before_update.clj

@@ -66,7 +66,7 @@
     (when-let [changes->pk-maps (not-empty (let [rf (changes->affected-pk-maps-rf model changes)]
                                              (pipeline/transduce-with-model
                                               rf
-                                              :toucan.query-type/select.instances
+                                              :toucan.query-type/select.instances.from-update
                                               model
                                               parsed-args)))]
       (log/tracef :compile "changes->pk-maps = %s" changes->pk-maps)

test/toucan2/tools/before_update_test.clj

@@ -160,6 +160,25 @@
             (is (= [{:category "not-bar"}]
                    (map protocols/changes @*updated-venues*)))))))))
 
+(deftest ^:synchronized conditions-map-test
+  (testing "Should work with conditions maps"
+    (test/with-discarded-table-changes :venues
+      (binding [*updated-venues* (atom [])]
+        (is (= 1
+               (update/update! ::venues.before-update {:id 3, :category [:not= "bar"]} {:category "not-bar"})))
+        (is (= "not-bar"
+               (select/select-one-fn :category ::venues.before-update 3)))
+        (testing "current"
+          (is (= [{:id         3
+                   :name       "BevMo"
+                   :category   "not-bar"
+                   :created-at (LocalDateTime/parse "2017-01-01T00:00")
+                   :updated-at (LocalDateTime/parse "2017-01-01T00:00")}]
+                 @*updated-venues*)))
+        (testing "changes"
+          (is (= [{:category "not-bar"}]
+                 (map protocols/changes @*updated-venues*))))))))
+
 (deftest ^:synchronized no-changes-test
   (testing "No changes passed to update!* itself -- should no-op"
     (test/with-discarded-table-changes :venues

toucan1/test/toucan/db_test.clj

@@ -217,20 +217,31 @@
              (t1.db/select-one PhoneNumber :number id))))))
 
 (deftest ^:synchronized update-where!-test
-  (test/with-discarded-table-changes User
-    (t1.db/update-where! User {:first-name [:not= "Cam"]}
-                         :first-name "Cam")
-    (is (= [{:id 1, :first-name "Cam", :last-name "Saul"}
-            {:id 2, :first-name "Cam", :last-name "Toucan"}
-            {:id 3, :first-name "Cam", :last-name "Bird"}]
-           (t1.db/select User {:order-by [:id]}))))
-  (test/with-discarded-table-changes User
-    (t1.db/update-where! User {:first-name "Cam"}
-                         :first-name "Not Cam")
-    (is (= [{:id 1, :first-name "Not Cam", :last-name "Saul"}
-            {:id 2, :first-name "Rasta", :last-name "Toucan"}
-            {:id 3, :first-name "Lucky", :last-name "Bird"}]
-           (t1.db/select User {:order-by [:id]})))))
+  (testing :not=
+    (test/with-discarded-table-changes User
+      (t1.db/update-where! User {:first-name [:not= "Cam"]}
+                           :first-name "Cam")
+      (is (= [{:id 1, :first-name "Cam", :last-name "Saul"}
+              {:id 2, :first-name "Cam", :last-name "Toucan"}
+              {:id 3, :first-name "Cam", :last-name "Bird"}]
+             (t1.db/select User {:order-by [:id]})))))
+  (testing "simple condition"
+    (test/with-discarded-table-changes User
+      (t1.db/update-where! User {:first-name "Cam"}
+                           :first-name "Not Cam")
+      (is (= [{:id 1, :first-name "Not Cam", :last-name "Saul"}
+              {:id 2, :first-name "Rasta", :last-name "Toucan"}
+              {:id 3, :first-name "Lucky", :last-name "Bird"}]
+             (t1.db/select User {:order-by [:id]})))))
+  (testing "multiple conditions"
+    (test/with-discarded-table-changes User
+      (t1.db/update-where! User {:first-name "Cam"
+                                 :id         [:in #{1 2 3}]}
+                           :first-name "Not Cam")
+      (is (= [{:id 1, :first-name "Not Cam", :last-name "Saul"}
+              {:id 2, :first-name "Rasta", :last-name "Toucan"}
+              {:id 3, :first-name "Lucky", :last-name "Bird"}]
+             (t1.db/select User {:order-by [:id]}))))))
 
 (deftest ^:synchronized update-non-nil-keys!-test
   (test/with-discarded-table-changes User