Further evolution of middleware/hooks

Removed the `IDeref` approach and plumbed `:post-execute-fn` directly 
into a new `WrappedExecutable` protocol that re-implements the 
`Executable` protocol but with hooks in place.

CHANGELOG.md

@@ -7,7 +7,6 @@ Only accretive/fixative changes will be made from now on.
 The following changes have been committed to the **master** branch since the 1.0.10 release:
 
 * Add `next.jdbc.middleware` containing a `wrapper` for connectable objects that can offer default options, as well as four "hooks" for pre- and post-processing functions that make it easier to add logging and timing code to your `next.jdbc`-based application.
-* Make the "mapified" result set object implement `clojure.lang.IDeref` so you can "force" the result set builder to be constructed so that the `:post-execute-fn` hook will run in middleware.
 * Add testing against Microsoft SQL Server (run tests with environment variables `NEXT_JDBC_TEST_MSSQL=yes` and `MSSQL_SA_PASSWORD` set to your local -- `127.0.0.1:1433` -- SQL Server `sa` user password; assumes that it can create and drop `fruit` and `fruit_time` tables in the `model` database).
 * Add testing against MySQL (run tests with environment variables `NEXT_JDBC_TEST_MYSQL=yes` and `MYSQL_ROOT_PASSWORD` set to your local -- `127.0.0.1:3306` -- MySQL `root` user password; assumes you have already created an empty database called `clojure_test`).
 * Bump several JDBC driver versions for up-to-date testing.

doc/middleware.md

@@ -26,9 +26,10 @@ The `next.jdbc.middleware/wrapper` function accepts a connectable and an optiona
 In addition to providing default options, the middleware wrapper also provides a number of "hooks" around SQL execution and result set building that you can tap into by providing any of the following options:
 
 * `:pre-process-fn` -- `(fn [sql-params opts] ,,, [sql-params' opts'])` -- this function is called on the SQL & parameters and the options hash map, prior to executing the SQL, and can pre-process them, returning a vector pair of (possibly updated) SQL & parameters and options,
-* `:post-process-fn` -- `(fn [rs opts] ,,, [rs' opts'])` -- this function is called on the `ResultSet` object and the options hash map, after executing the SQL, and can post-process them, returning a vector pair of (possibly updated) `ResultSet` object and options,
+* `:post-process-fn` -- `(fn [rs opts] ,,, [rs' opts'])` -- this function is called on the `ResultSet` object and the options hash map, after executing the SQL, and can post-process them, returning a vector pair of (possibly updated) `ResultSet` object and options;
+if the SQL operation does not return a `ResultSet` then this function is called on the update count and the options hash map, and should return a vector pair of the update count and options (unchanged),
 * `:row!-fn` -- `(fn [row opts] ,,, row')` -- this function is called on each row as it is realized (and also passed the options hash map) and can post-process the row, returning a  (possibly updated) row; it is named for the `row!` function in the result set builder that it wraps,
-* `:rs!-fn` -- `(fn [sql-params opts] ,,, [sql-params' opts'])` -- this function is called on the result set once it is realized (and also passed the options hash map) and can post-process the result set, returning a (possibly updated) result set; it is named for the `rs!` function in the result set builder that it wraps.
+* `:rs!-fn` -- `(fn [sql-params opts] ,,, [sql-params' opts'])` -- this function is called, for `execute!` only, on the full result set once it is realized (and also passed the options hash map) and can post-process the result set, returning a (possibly updated) result set; it is named for the `rs!` function in the result set builder that it wraps.
 
 Here's the data flow of middleware:
 
@@ -48,9 +49,11 @@ Here's the data flow of middleware:
 ;; 2. pre-process the SQL, parameters, and options:
 ;;    [sql-params' opts''] <- (A ["select..." 4] opts')
 ;; 3. execute sql-params' with the opts'' hash map
-;; 4. create the result set builder from the ResultSet rs and options opts''
+;; 4. post-process the ResultSet (or update count) and options:
-;; 5. inside that builder, post-process the ResultSet and options:
+;;    [rs' opts'''] <- (B rs opts'') or
-;;    [rs' opts'''] <- (B rs opts'')
+;;    [count' opts'''] <- (B count opts'')
+;; if a result set was produced then:
+;; 5. create the result set builder from the ResultSet rs' and options opts'''
 ;; 6. post-process each row as row! is called:
 ;;    row' <- (C (row! builder row) opts''')
 ;; 7. add row' into the result set being built
@@ -63,27 +66,24 @@ As you can see, both `:pre-process-fn` and `:post-process-fn` can return updated
 
 Any of the hook functions may execute side-effects (such as logging) but must still return the expected data.
 
-## Middleware and `plan`
+### Middleware and `plan`
 
-Because `next.jdbc/plan` tries to avoid realizing a result set, it is possible to perform reductions that do not even cause the result set builder to be constructed -- the `:post-execute-fn` hook will not executed in such cases. For example:
+Because `plan` tries to avoid realizing a result set, it is possible to perform reductions that do not even cause the result set builder to be constructed -- the `:post-execute-fn` hook will still be executed in such cases, but the `:row!-fn` hook may not be executed and the `:rs!-fn` hook will definitely not be executed. For example:
 
 ```clojure
 user=> (into [] (map :name) ; does not construct the builder!
-             (jdbc/plan db-spec ["select * from fruit"]))
+             (jdbc/plan db-spec ["select id, name from fruit"]))
 ["Apple" "Banana" "Peach" "Orange"]
+user=> (into [] (map #(dissoc % :id)) ; constructs builder, calls row!-fn
+             (jdbc/plan db-spec ["select id, name from fruit"]))
+[{:name "Apple"} {:name "Banana"} {:name "Peach"} {:name "Orange"}]
 ```
 
-You can force the result set builder to be constructed by calling `deref` on any row:
+### Middleware and `execute-one!`
 
-```clojure
+Because `execute-one!` only realizes at most one row from a result set, it never calls `rs!-fn`. It will call `row!-fn` at most once (zero times if no rows are returned from the SQL operation, exactly once if any rows are returned).
-user=> (into [] (map (comp :name deref))
-             (jdbc/plan db-spec ["select * from fruit"]))
-["Apple" "Banana" "Peach" "Orange"]
-```
 
-That will ensure that the result set builder _is_ constructed and it will execute the `:post-execute-fn` hook, but it will not cause rows (or the overall result set) to be realized. Thus, the only overhead of calling `deref` on each row is the one-off cost of constructing the result set builder for the first row, and the cost of derefencing a realized delay object for each row (and throwing that value away).
+Both of the `:pre-execute-fn` and `:post-execute-fn` hooks are always called for `execute-one!`.
-
-*Note: If your SQL operation produces no rows, or no `ResultSet` at all (only an update count), then there is no way to force the result set builder to be constructed and no way for the `:post-execute-fn` hook to be executed.*
 
 ## Examples of Middleware Usage
 

src/next/jdbc/middleware.clj

@@ -29,6 +29,11 @@
   For `:rs!-fn`, that means returning the result set data unchanged (and
   ignoring the options).
 
+  For SQL operations that do not produce a `ResultSet`, the post-process
+  hook (`:post-execute-fn`) is called with the update count and options
+  instead of the result set (and options) and should return a pair of the
+  update count and the options (unchanged).
+
   For timing middleware, you can pass per-operation timing data through the
   options hash map, so you can measure the timing for the SQL execution, and
   also the time taken to build the full result set (if it is built).
@@ -38,8 +43,10 @@
 
   You can also transform the SQL & parameters prior to execution and transform
   the rows and/or result set after each is built."
-  (:require [next.jdbc.protocols :as p]
+  (:require [next.jdbc.prepare :as prepare]
-            [next.jdbc.result-set :as rs]))
+            [next.jdbc.protocols :as p]
+            [next.jdbc.result-set :as rs])
+  (:import (java.sql PreparedStatement Statement)))
 
 (defn post-processing-adapter
   "Given a builder function (e.g., `as-lower-maps`), return a new builder
@@ -49,10 +56,6 @@
   functions, which should include `:next.jdbc/sql-params` (the vector of SQL
   and parameters, in case post-processing needs it):
 
-  * `:post-execute-fn` -- called on the `ResultSet` object and the options
-                          immediately after the SQL operation completes
-                          returns a pair of a (possibly updated) `ResultSet`
-                          object and (possibly updated) options
   * `:row!-fn`         -- called on each row and the options, as the row is
                           fully-realized and returns the (possiblly updated)
                           row data
@@ -63,10 +66,7 @@
   The results of these functions are returned as the rows/result set."
   [builder-fn]
   (fn [rs opts]
-    (let [exec-fn   (get opts :post-execute-fn vector)
+    (let [mrsb      (builder-fn rs opts)
-          ;; rebind both the ResultSet object and the options
-          [rs opts] (exec-fn rs opts)
-          mrsb      (builder-fn rs opts)
           row!-fn   (get opts :row!-fn (comp first vector))
           rs!-fn    (get opts :rs!-fn  (comp first vector))]
       (reify
@@ -80,43 +80,186 @@
         (with-row [this mrs row] (rs/with-row mrsb mrs row))
         (rs! [this mrs] (rs!-fn (rs/rs! mrsb mrs) opts))))))
 
+(defprotocol WrappedExecutable
+  "This is an implementation detail for the middleware wrapper."
+  (wrapped-execute ^clojure.lang.IReduceInit [this sql-params opts])
+  (wrapped-execute-one [this sql-params opts])
+  (wrapped-execute-all [this sql-params opts]))
+
+(defn- reduce-stmt
+  "Variant of `next.jdbc.result-set/reduce-stmt` that calls the
+  `:post-execute-fn` hook on results sets and update counts."
+  [^PreparedStatement stmt f init opts]
+  (if-let [rs (#'rs/stmt->result-set stmt opts)]
+    (let [[rs opts] ((:post-execute-fn opts) rs opts)
+          rs-map    (#'rs/mapify-result-set rs opts)]
+      (loop [init' init]
+        (if (.next rs)
+          (let [result (f init' rs-map)]
+            (if (reduced? result)
+              @result
+              (recur result)))
+          init')))
+    (let [[n _] ((:post-execute-fn opts) (.getUpdateCount stmt) opts)]
+      (f init {:next.jdbc/update-count n}))))
+
+(defn- reduce-stmt-sql
+  "Variant of `next.jdbc.result-set/reduce-stmt-sql` that calls the
+  `:post-execute-fn` hook on results sets and update counts."
+  [^Statement stmt sql f init opts]
+  (if-let [rs (#'rs/stmt-sql->result-set stmt sql opts)]
+    (let [[rs opts] ((:post-execute-fn opts) rs opts)
+          rs-map    (#'rs/mapify-result-set rs opts)]
+      (loop [init' init]
+        (if (.next rs)
+          (let [result (f init' rs-map)]
+            (if (reduced? result)
+              @result
+              (recur result)))
+          init')))
+    (let [[n _] ((:post-execute-fn opts) (.getUpdateCount stmt) opts)]
+      (f init {:next.jdbc/update-count n}))))
+
+;; this duplicates the Executable implementations from next.jdbc.result-set
+;; but with hooks for calling :post-execute-fn and being able to rely on
+;; :builder-fn always being present
+(extend-protocol WrappedExecutable
+  java.sql.Connection
+  (wrapped-execute [this sql-params opts]
+    (reify clojure.lang.IReduceInit
+      (reduce [_ f init]
+              (with-open [stmt (prepare/create this
+                                               (first sql-params)
+                                               (rest sql-params)
+                                               opts)]
+                (reduce-stmt stmt f init opts)))
+      (toString [_] "`IReduceInit` from `plan` -- missing reduction?")))
+  (wrapped-execute-one [this sql-params opts]
+    (with-open [stmt (prepare/create this
+                                     (first sql-params)
+                                     (rest sql-params)
+                                     opts)]
+      (wrapped-execute-one stmt nil opts)))
+  (wrapped-execute-all [this sql-params opts]
+    (with-open [stmt (prepare/create this
+                                     (first sql-params)
+                                     (rest sql-params)
+                                     opts)]
+      (wrapped-execute-all stmt nil opts)))
+
+  javax.sql.DataSource
+  (wrapped-execute [this sql-params opts]
+    (reify clojure.lang.IReduceInit
+      (reduce [_ f init]
+              (with-open [con  (p/get-connection this opts)
+                          stmt (prepare/create con
+                                               (first sql-params)
+                                               (rest sql-params)
+                                               opts)]
+                (reduce-stmt stmt f init opts)))
+      (toString [_] "`IReduceInit` from `plan` -- missing reduction?")))
+  (wrapped-execute-one [this sql-params opts]
+    (with-open [con  (p/get-connection this opts)
+                stmt (prepare/create con
+                                     (first sql-params)
+                                     (rest sql-params)
+                                     opts)]
+      (wrapped-execute-one stmt nil opts)))
+  (wrapped-execute-all [this sql-params opts]
+    (with-open [con  (p/get-connection this opts)
+                stmt (prepare/create con
+                                     (first sql-params)
+                                     (rest sql-params)
+                                     opts)]
+      (wrapped-execute-all stmt nil opts)))
+
+  java.sql.PreparedStatement
+  ;; we can't tell if this PreparedStatement will return generated
+  ;; keys so we pass a truthy value to at least attempt it if we
+  ;; do not get a ResultSet back from the execute call
+  (wrapped-execute [this _ opts]
+    (reify clojure.lang.IReduceInit
+      (reduce [_ f init]
+              (reduce-stmt this f init (assoc opts :return-keys true)))
+      (toString [_] "`IReduceInit` from `plan` -- missing reduction?")))
+  (wrapped-execute-one [this _ opts]
+    (if-let [rs (#'rs/stmt->result-set this (assoc opts :return-keys true))]
+      (let [[rs opts] ((:post-execute-fn opts) rs opts)
+            builder   ((:builder-fn opts) rs opts)]
+        (when (.next rs)
+          (rs/datafiable-row (#'rs/row-builder builder)
+                             (.getConnection this) opts)))
+      (let [[n _] ((:post-execute-fn opts) (.getUpdateCount this) opts)]
+        {:next.jdbc/update-count n})))
+  (wrapped-execute-all [this _ opts]
+    (if-let [rs (#'rs/stmt->result-set this opts)]
+      (let [[rs opts] ((:post-execute-fn opts) rs opts)]
+        (rs/datafiable-result-set rs (.getConnection this) opts))
+      (let [[n _] ((:post-execute-fn opts) (.getUpdateCount this) opts)]
+        [{:next.jdbc/update-count n}])))
+
+  java.sql.Statement
+  ;; we can't tell if this Statement will return generated
+  ;; keys so we pass a truthy value to at least attempt it if we
+  ;; do not get a ResultSet back from the execute call
+  (wrapped-execute [this sql-params opts]
+    (assert (= 1 (count sql-params))
+            "Parameters cannot be provided when executing a non-prepared Statement")
+    (reify clojure.lang.IReduceInit
+      (reduce [_ f init]
+              (reduce-stmt-sql this (first sql-params) f init (assoc opts :return-keys true)))
+      (toString [_] "`IReduceInit` from `plan` -- missing reduction?")))
+  (wrapped-execute-one [this sql-params opts]
+    (assert (= 1 (count sql-params))
+            "Parameters cannot be provided when executing a non-prepared Statement")
+    (if-let [rs (#'rs/stmt-sql->result-set this (first sql-params) (assoc opts :return-keys true))]
+      (let [[rs opts] ((:post-execute-fn opts) rs opts)
+            builder   ((:builder-fn opts) rs opts)]
+        (when (.next rs)
+          (rs/datafiable-row (#'rs/row-builder builder)
+                             (.getConnection this) opts)))
+      (let [[n _] ((:post-execute-fn opts) (.getUpdateCount this) opts)]
+        {:next.jdbc/update-count n})))
+  (wrapped-execute-all [this sql-params opts]
+    (assert (= 1 (count sql-params))
+            "Parameters cannot be provided when executing a non-prepared Statement")
+    (if-let [rs (#'rs/stmt-sql->result-set this (first sql-params) opts)]
+      (let [[rs opts] ((:post-execute-fn opts) rs opts)]
+        (rs/datafiable-result-set rs (.getConnection this) opts))
+      (let [[n _] ((:post-execute-fn opts) (.getUpdateCount this) opts)]
+        [{:next.jdbc/update-count n}])))
+
+  Object
+  (wrapped-execute [this sql-params opts]
+    (wrapped-execute (p/get-datasource this) sql-params opts))
+  (wrapped-execute-one [this sql-params opts]
+    (wrapped-execute-one (p/get-datasource this) sql-params opts))
+  (wrapped-execute-all [this sql-params opts]
+    (wrapped-execute-all (p/get-datasource this) sql-params opts)))
+
+(defn- execute-wrapper
+  [f db global-opts sql-params opts]
+  (let [opts (merge {:pre-execute-fn vector :post-execute-fn vector
+                     :builder-fn rs/as-maps}
+                    global-opts opts)
+        ;; rebind both the SQL & parameters and the options
+        [sql-params opts] ((:pre-execute-fn opts) sql-params opts)]
+    (f db sql-params
+       (assoc opts
+              :builder-fn (post-processing-adapter (:builder-fn opts))
+              :next.jdbc/sql-params sql-params))))
+
 (defrecord JdbcMiddleware [db global-opts]
   p/Executable
   (-execute [this sql-params opts]
-    (let [opts              (merge global-opts opts)
+    (execute-wrapper wrapped-execute db global-opts sql-params opts))
-          pre-execute-fn    (get opts :pre-execute-fn vector)
-          ;; rebind both the SQL & parameters and the options
-          [sql-params opts] (pre-execute-fn sql-params opts)
-          builder-fn        (get opts :builder-fn rs/as-maps)]
-      (p/-execute db sql-params
-                  (assoc opts
-                         :builder-fn (post-processing-adapter builder-fn)
-                         :next.jdbc/sql-params sql-params))))
   (-execute-one [this sql-params opts]
-    (let [opts              (merge global-opts opts)
+    (execute-wrapper wrapped-execute-one db global-opts sql-params opts))
-          pre-execute-fn    (get opts :pre-execute-fn vector)
-          ;; rebind both the SQL & parameters and the options
-          [sql-params opts] (pre-execute-fn sql-params opts)
-          builder-fn        (get opts :builder-fn rs/as-maps)]
-      (p/-execute-one db sql-params
-                      (assoc opts
-                             :builder-fn (post-processing-adapter builder-fn)
-                             :next.jdbc/sql-params sql-params))))
   (-execute-all [this sql-params opts]
-    (let [opts              (merge global-opts opts)
+    (execute-wrapper wrapped-execute-all db global-opts sql-params opts)))
-          pre-execute-fn    (get opts :pre-execute-fn vector)
-          ;; rebind both the SQL & parameters and the options
-          [sql-params opts] (pre-execute-fn sql-params opts)
-          builder-fn        (get opts :builder-fn rs/as-maps)]
-      (p/-execute-all db sql-params
-                      (assoc opts
-                             :builder-fn (post-processing-adapter builder-fn)
-                             :next.jdbc/sql-params sql-params)))))
 
 (defn wrapper
-  "Given a connectable, return a wrapped connectable that will run hooks.
+  "Given a connectable and a hash map of options, return a wrapped connectable
-
-  Given a connectable and a hash map of options, return a wrapped connectable
   that will use those options as defaults for any SQL operations and will
   run hooks.
 
@@ -133,7 +276,12 @@
                         passed options)
                         returns (possibly updated) result set data
 
-  Uses `next.jdbc.middleware/post-processing-adapter for the last three,
+  For SQL operations that do not produce a `ResultSet`, the post-process
+  hook (`:post-execute-fn`) is called with the update count and options
+  instead of the result set (and options) and should return a pair of the
+  update count and the options (unchanged).
+
+  Uses `next.jdbc.middleware/post-processing-adapter for the last two,
   wrapped around whatever `:builder-fn` you supply for each SQL operation."
   ([db]      (JdbcMiddleware. db {}))
   ([db opts] (JdbcMiddleware. db opts)))

src/next/jdbc/result_set.clj

@@ -460,13 +460,6 @@
           (row-builder @builder)
           {`core-p/datafy (navize-row connectable opts)}))
 
-      clojure.lang.IDeref
-      (deref [this]
-        ;; force the builder to be created but return the row
-        ;; without actually building anything
-        (deref builder)
-        this)
-
       ;; from java.lang.Object:
       (toString [_]
         (try
@@ -573,6 +566,11 @@
           init')))
     (f init {:next.jdbc/update-count (.getUpdateCount stmt)})))
 
+;; the Connection and DataSource implementations could simply delegate to
+;; the PreparedStatement implementation which would reduce the amount of
+;; code here -- but they are unrolled for performance, to avoid the extra
+;; function call through the protocol that would be involved in reuse
+;; note that middleware duplicates this code but does delegate and reuse
 (extend-protocol p/Executable
   java.sql.Connection
   (-execute [this sql-params opts]

test/next/jdbc/middleware_test.clj

@@ -8,7 +8,7 @@
                                               default-options]]
             [next.jdbc.result-set :as rs]
             [next.jdbc.specs :as specs])
-  (:import (java.sql ResultSet ResultSetMetaData)))
+  (:import (java.sql ResultSet)))
 
 (set! *warn-on-reflection* true)
 
@@ -64,14 +64,49 @@
                      (swap! (::timing opts) update ::total + (- end (::start opts)))
                      [rs opts]))
         timing   (atom {::calls 0 ::total 0.0})
+        mw-ds    (mw/wrapper (ds) {::timing         timing
+                                   :pre-execute-fn  start-fn
+                                   :post-execute-fn end-fn})
         sql-p    ["select * from fruit where id in (?,?) order by id desc" 1 4]]
-    (jdbc/execute! (mw/wrapper (ds) {::timing         timing
+    (jdbc/execute! mw-ds sql-p)
-                                     :pre-execute-fn  start-fn
+    (jdbc/execute! mw-ds sql-p)
-                                     :post-execute-fn end-fn})
-                   sql-p)
-    (jdbc/execute! (mw/wrapper (ds) {::timing         timing
-                                     :pre-execute-fn  start-fn
-                                     :post-execute-fn end-fn})
-                   sql-p)
     (printf "%20s - %d calls took %,10d nanoseconds\n"
             (:dbtype (db)) (::calls @timing) (long (::total @timing)))))
+
+(deftest post-execute-tests
+  (let [calls   (atom 0)
+        seen-rs (atom 0)
+        rows    (atom 0)
+        rss     (atom 0)
+        post-fn (fn [x opts]
+                  (swap! calls inc)
+                  (when (instance? ResultSet x)
+                    (swap! seen-rs inc))
+                  [x opts])
+        mw-ds   (mw/wrapper (ds) {:post-execute-fn post-fn
+                                  :row!-fn (fn [row _] (swap! rows inc) row)
+                                  :rs!-fn  (fn [rs  _] (swap! rss  inc) rs)})]
+    ;; first call, four rows, one result set
+    (jdbc/execute! mw-ds ["select * from fruit"])
+    (is (= 1 @calls))
+    (is (= 1 @seen-rs))
+    (is (= 4 @rows))
+    (is (= 1 @rss))
+    ;; second call, no rows, one more result set
+    (jdbc/execute! mw-ds ["select * from fruit where id < 0"])
+    (is (= 2 @calls))
+    (is (= 2 @seen-rs))
+    (is (= 4 @rows))
+    (is (= 2 @rss))
+    ;; third call, no result set
+    (jdbc/execute! mw-ds ["update fruit set name = ? where id < 0" "Plum"])
+    (is (= 3 @calls))
+    (is (= 2 @seen-rs))
+    (is (= 4 @rows))
+    (is (= 2 @rss))
+    ;; fourth call, one row, one result set (but no rs!-fn)
+    (jdbc/execute-one! mw-ds ["select * from fruit"])
+    (is (= 4 @calls))
+    (is (= 3 @seen-rs))
+    (is (= 5 @rows))
+    (is (= 2 @rss))))