From b0a640a10150713313af4f9dd9782b4b6f0dbf78 Mon Sep 17 00:00:00 2001 From: Sean Corfield Date: Wed, 20 Nov 2024 16:49:39 -0800 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + src/next/jdbc.clj | 11 +++++-- src/next/jdbc/transaction.clj | 60 ++++++++++++++++------------------- test/next/jdbc_test.clj | 6 ++++ 4 files changed, 43 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c49400e..5e7ff5a 100644 --- 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 index 8016544..8eabc2c 100644 --- 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 index d73ed68..1c6ce80 100644 --- 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 index f6eaab7..7843c5f 100644 --- 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")