From b6014e824a401af2c68e7b3c0698a16a38f5122a Mon Sep 17 00:00:00 2001 From: Sean Corfield Date: Wed, 11 Dec 2019 16:03:41 -0800 Subject: [PATCH] Fixes #80 by carefully avoiding the implicit commit when rollback fails --- CHANGELOG.md | 1 + src/next/jdbc/transaction.clj | 21 ++++++++--- test/next/jdbc_test.clj | 69 ++++++++++++++++++++++++++++++++--- 3 files changed, 81 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da45a32..9c06127 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Only accretive/fixative changes will be made from now on. The following changes have been committed to the **master** branch since the 1.0.11 release: +* Fix #80 by avoiding the auto-commit restore after a failed rollback in a failed transaction. * Address #78 by documenting the `:connectionInitSql` workaround for HikariCP/PostgreSQL and non-default schemas. ## Stable Builds diff --git a/src/next/jdbc/transaction.clj b/src/next/jdbc/transaction.clj index cdb94fb..a64b971 100644 --- a/src/next/jdbc/transaction.clj +++ b/src/next/jdbc/transaction.clj @@ -27,7 +27,8 @@ rollback-only (:rollback-only opts) old-autocommit (.getAutoCommit con) old-isolation (.getTransactionIsolation con) - old-readonly (.isReadOnly con)] + old-readonly (.isReadOnly con) + restore-ac? (volatile! true)] (io! (when isolation (.setTransactionIsolation con (isolation isolation-levels))) @@ -37,12 +38,21 @@ (try (let [result (f con)] (if rollback-only - (.rollback con) + (do + ;; per #80: if the rollback operation fails, we do not + ;; want to try to restore auto-commit... + (vreset! restore-ac? false) + (.rollback con) + (vreset! restore-ac? true)) (.commit con)) result) (catch Throwable t (try + ;; per #80: if the rollback operation fails, we do not + ;; want to try to restore auto-commit... + (vreset! restore-ac? false) (.rollback con) + (vreset! restore-ac? true) (catch Throwable rb ;; combine both exceptions (throw (ex-info (str "Rollback failed handling \"" @@ -56,9 +66,10 @@ ;; want those to replace any exception currently being ;; handled -- and if the connection got closed, we just ;; want to ignore exceptions here anyway - (try - (.setAutoCommit con old-autocommit) - (catch Exception _)) + (when @restore-ac? + (try ; only restore auto-commit if rollback did not fail + (.setAutoCommit con old-autocommit) + (catch Exception _))) (when isolation (try (.setTransactionIsolation con old-isolation) diff --git a/test/next/jdbc_test.clj b/test/next/jdbc_test.clj index ff63efc..ba3887f 100644 --- a/test/next/jdbc_test.clj +++ b/test/next/jdbc_test.clj @@ -171,7 +171,17 @@ VALUES ('Pear', 'green', 49, 47) INSERT INTO fruit (name, appearance, cost, grade) VALUES ('Pear', 'green', 49, 47) "])))) - (is (= 4 (count (jdbc/execute! (ds) ["select * from fruit"]))))) + (is (= 4 (count (jdbc/execute! (ds) ["select * from fruit"])))) + (with-open [con (jdbc/get-connection (ds))] + (let [ac (.getAutoCommit con)] + (is (= [{:next.jdbc/update-count 1}] + (jdbc/with-transaction [t con {:rollback-only true}] + (jdbc/execute! t [" +INSERT INTO fruit (name, appearance, cost, grade) +VALUES ('Pear', 'green', 49, 47) +"])))) + (is (= 4 (count (jdbc/execute! con ["select * from fruit"])))) + (is (= ac (.getAutoCommit con)))))) (testing "with-transaction exception" (is (thrown? Throwable (jdbc/with-transaction [t (ds)] @@ -180,7 +190,18 @@ INSERT INTO fruit (name, appearance, cost, grade) VALUES ('Pear', 'green', 49, 47) "]) (throw (ex-info "abort" {}))))) - (is (= 4 (count (jdbc/execute! (ds) ["select * from fruit"]))))) + (is (= 4 (count (jdbc/execute! (ds) ["select * from fruit"])))) + (with-open [con (jdbc/get-connection (ds))] + (let [ac (.getAutoCommit con)] + (is (thrown? Throwable + (jdbc/with-transaction [t con] + (jdbc/execute! t [" +INSERT INTO fruit (name, appearance, cost, grade) +VALUES ('Pear', 'green', 49, 47) +"]) + (throw (ex-info "abort" {}))))) + (is (= 4 (count (jdbc/execute! con ["select * from fruit"])))) + (is (= ac (.getAutoCommit con)))))) (testing "with-transaction call rollback" (is (= [{:next.jdbc/update-count 1}] (jdbc/with-transaction [t (ds)] @@ -190,7 +211,19 @@ VALUES ('Pear', 'green', 49, 47) "])] (.rollback t) result)))) - (is (= 4 (count (jdbc/execute! (ds) ["select * from fruit"]))))) + (is (= 4 (count (jdbc/execute! (ds) ["select * from fruit"])))) + (with-open [con (jdbc/get-connection (ds))] + (let [ac (.getAutoCommit con)] + (is (= [{:next.jdbc/update-count 1}] + (jdbc/with-transaction [t con] + (let [result (jdbc/execute! t [" +INSERT INTO fruit (name, appearance, cost, grade) +VALUES ('Pear', 'green', 49, 47) +"])] + (.rollback t) + result)))) + (is (= 4 (count (jdbc/execute! con ["select * from fruit"])))) + (is (= ac (.getAutoCommit con)))))) (testing "with-transaction with unnamed save point" (is (= [{:next.jdbc/update-count 1}] (jdbc/with-transaction [t (ds)] @@ -201,7 +234,20 @@ VALUES ('Pear', 'green', 49, 47) "])] (.rollback t save-point) result)))) - (is (= 4 (count (jdbc/execute! (ds) ["select * from fruit"]))))) + (is (= 4 (count (jdbc/execute! (ds) ["select * from fruit"])))) + (with-open [con (jdbc/get-connection (ds))] + (let [ac (.getAutoCommit con)] + (is (= [{:next.jdbc/update-count 1}] + (jdbc/with-transaction [t con] + (let [save-point (.setSavepoint t) + result (jdbc/execute! t [" +INSERT INTO fruit (name, appearance, cost, grade) +VALUES ('Pear', 'green', 49, 47) +"])] + (.rollback t save-point) + result)))) + (is (= 4 (count (jdbc/execute! con ["select * from fruit"])))) + (is (= ac (.getAutoCommit con)))))) (testing "with-transaction with named save point" (is (= [{:next.jdbc/update-count 1}] (jdbc/with-transaction [t (ds)] @@ -212,7 +258,20 @@ VALUES ('Pear', 'green', 49, 47) "])] (.rollback t save-point) result)))) - (is (= 4 (count (jdbc/execute! (ds) ["select * from fruit"])))))) + (is (= 4 (count (jdbc/execute! (ds) ["select * from fruit"])))) + (with-open [con (jdbc/get-connection (ds))] + (let [ac (.getAutoCommit con)] + (is (= [{:next.jdbc/update-count 1}] + (jdbc/with-transaction [t con] + (let [save-point (.setSavepoint t (name (gensym))) + result (jdbc/execute! t [" +INSERT INTO fruit (name, appearance, cost, grade) +VALUES ('Pear', 'green', 49, 47) +"])] + (.rollback t save-point) + result)))) + (is (= 4 (count (jdbc/execute! con ["select * from fruit"])))) + (is (= ac (.getAutoCommit con))))))) (deftest connection-tests (testing "datasource via jdbcUrl"