fixes #282 by tracking raw Connection objects for TXs.

this no longer checks TX nesting for DataSource-based TXs, but instead uses the Connection-based implementation directly. raw Connection objects are tracked in a dynamic set. thanks to [mbezjak](https://github.com/mbezjak) for the core of the implementation. Signed-off-by: Sean Corfield <[email protected]>
seancorfield · Nov 21, 2024 · b0a640a · b0a640a
1 parent a754681
commit b0a640a
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 35 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@ Only accretive/fixative changes will be made from now on.
 
 * 1.3.next in progress
   * Fix [#287](https://github.com/seancorfield/next-jdbc/issues/287) by merging user-supplied options over `:return-keys true`.
+  * Fix [#282](https://github.com/seancorfield/next-jdbc/issues/282) by tracking raw `Connection` objects for active TXs, which relaxes several of the conditions around nested transactions.
 
 * 1.3.955 -- 2024-10-06
   * Address [#285](https://github.com/seancorfield/next-jdbc/issues/285) by setting the default Clojure version to the earliest supported (1.10.3) to give a better hint to users.

diff --git a/src/next/jdbc.clj b/src/next/jdbc.clj
@@ -447,12 +447,19 @@
   "Returns true if `next.jdbc` has a currently active transaction in the
   current thread, else false.
 
+  With no arguments, tells you if any transaction is currently active.
+
+  With a `Connection` argument, tells you if a transaction is currently
+  active on that specific connection.
+
   Note: transactions are a convention of operations on a `Connection` so
   this predicate only reflects `next.jdbc/transact` and `next.jdbc/with-transaction`
   operations -- it does not reflect any other operations on a `Connection`,
   performed via JDBC interop directly."
-  []
-  @#'tx/*active-tx*)
+  ([]
+   (boolean (seq @#'tx/*active-tx*)))
+  ([con]
+   (contains? @#'tx/*active-tx* con)))
 
 (defn with-options
   "Given a connectable/transactable object and a set of (default) options

diff --git a/src/next/jdbc/transaction.clj b/src/next/jdbc/transaction.clj
@@ -38,7 +38,7 @@
   :allow)
 
 (defonce ^:private ^:dynamic ^{:doc "Used to detect nested transactions."}
-  *active-tx* false)
+  *active-tx* #{})
 
 (def ^:private isolation-levels
   "Transaction isolation levels."
@@ -112,43 +112,37 @@
              (.setReadOnly con old-readonly)
              (catch Exception _))))))))
 
+(defn- raw-connection ^Connection [^Connection con]
+  (if (.isWrapperFor con Connection)
+    (.unwrap con Connection)
+    con))
+
 (extend-protocol p/Transactable
   java.sql.Connection
   (-transact [this body-fn opts]
-    (cond
-      (and (not *active-tx*) (= :ignore *nested-tx*))
-      ;; #245 do not lock when in c.j.j compatibility mode:
-      (binding [*active-tx* true]
-        (transact* this body-fn opts))
-      (or (not *active-tx*) (= :allow *nested-tx*))
-      (locking this
-        (binding [*active-tx* true]
-          (transact* this body-fn opts)))
-      (= :ignore *nested-tx*)
-      (body-fn this)
-      (= :prohibit *nested-tx*)
-      (throw (IllegalStateException. "Nested transactions are prohibited"))
-      :else
-      (throw (IllegalArgumentException.
-              (str "*nested-tx* ("
-                   *nested-tx*
-                   ") was not :allow, :ignore, or :prohibit")))))
+    (let [raw (raw-connection this)]
+      (cond
+        (and (not (contains? *active-tx* raw)) (= :ignore *nested-tx*))
+        ;; #245 do not lock when in c.j.j compatibility mode:
+        (binding [*active-tx* (conj *active-tx* raw)]
+          (transact* this body-fn opts))
+        (or (not (contains? *active-tx* raw)) (= :allow *nested-tx*))
+        (locking this
+          (binding [*active-tx* (conj *active-tx* raw)]
+            (transact* this body-fn opts)))
+        (= :ignore *nested-tx*)
+        (body-fn this)
+        (= :prohibit *nested-tx*)
+        (throw (IllegalStateException. "Nested transactions are prohibited"))
+        :else
+        (throw (IllegalArgumentException.
+                (str "*nested-tx* ("
+                     *nested-tx*
+                     ") was not :allow, :ignore, or :prohibit"))))))
   javax.sql.DataSource
   (-transact [this body-fn opts]
-    (cond (or (not *active-tx*) (= :allow *nested-tx*))
-      (binding [*active-tx* true]
-        (with-open [con (p/get-connection this opts)]
-          (transact* con body-fn opts)))
-      (= :ignore *nested-tx*)
-      (with-open [con (p/get-connection this opts)]
-        (body-fn con))
-      (= :prohibit *nested-tx*)
-      (throw (IllegalStateException. "Nested transactions are prohibited"))
-      :else
-      (throw (IllegalArgumentException.
-              (str "*nested-tx* ("
-                   *nested-tx*
-                   ") was not :allow, :ignore, or :prohibit")))))
+    (with-open [con (p/get-connection this opts)]
+      (p/-transact con body-fn opts)))
   Object
   (-transact [this body-fn opts]
     (p/-transact (p/get-datasource this) body-fn opts)))
diff --git a/test/next/jdbc_test.clj b/test/next/jdbc_test.clj
@@ -233,6 +233,7 @@ VALUES ('Pear', 'green', 49, 47)
     (is (= [{:next.jdbc/update-count 1}]
            (jdbc/with-transaction [t (ds) {:rollback-only true}]
              (is (jdbc/active-tx?) "should be in a transaction")
+             (is (jdbc/active-tx? t) "connection should be in a transaction")
              (jdbc/execute! t ["
 INSERT INTO fruit (name, appearance, cost, grade)
 VALUES ('Pear', 'green', 49, 47)
@@ -244,6 +245,7 @@ VALUES ('Pear', 'green', 49, 47)
         (is (= [{:next.jdbc/update-count 1}]
                (jdbc/with-transaction [t con {:rollback-only true}]
                  (is (jdbc/active-tx?) "should be in a transaction")
+                 (is (jdbc/active-tx? t) "connection should be in a transaction")
                  (jdbc/execute! t ["
 INSERT INTO fruit (name, appearance, cost, grade)
 VALUES ('Pear', 'green', 49, 47)
@@ -258,6 +260,7 @@ INSERT INTO fruit (name, appearance, cost, grade)
 VALUES ('Pear', 'green', 49, 47)
 "])
              (is (jdbc/active-tx?) "should be in a transaction")
+             (is (jdbc/active-tx? t) "connection should be in a transaction")
              (throw (ex-info "abort" {})))))
     (is (= 4 (count (jdbc/execute! (ds) ["select * from fruit"]))))
     (is (not (jdbc/active-tx?)) "should not be in a transaction")
@@ -270,6 +273,7 @@ INSERT INTO fruit (name, appearance, cost, grade)
 VALUES ('Pear', 'green', 49, 47)
 "])
                  (is (jdbc/active-tx?) "should be in a transaction")
+                 (is (jdbc/active-tx? t) "connection should be in a transaction")
                  (throw (ex-info "abort" {})))))
         (is (= 4 (count (jdbc/execute! con ["select * from fruit"]))))
         (is (= ac (.getAutoCommit con))))))
@@ -283,6 +287,7 @@ VALUES ('Pear', 'green', 49, 47)
                (.rollback t)
                ;; still in a next.jdbc TX even tho' we rolled back!
                (is (jdbc/active-tx?) "should be in a transaction")
+               (is (jdbc/active-tx? t) "connection should be in a transaction")
                result))))
     (is (= 4 (count (jdbc/execute! (ds) ["select * from fruit"]))))
     (is (not (jdbc/active-tx?)) "should not be in a transaction")
@@ -309,6 +314,7 @@ VALUES ('Pear', 'green', 49, 47)
                (.rollback t save-point)
                ;; still in a next.jdbc TX even tho' we rolled back to a save point!
                (is (jdbc/active-tx?) "should be in a transaction")
+               (is (jdbc/active-tx? t) "connection should be in a transaction")
                result))))
     (is (= 4 (count (jdbc/execute! (ds) ["select * from fruit"]))))
     (is (not (jdbc/active-tx?)) "should not be in a transaction")