Introduce begin, rollback and commit (#88)

* Add begin(!)/1,2 to checkout and begin
* Add rollback(!)/1,2 to rollback and checkin
* Add commit(!)/1,2 to commit (or rollback) and checkin

rollback/2 is overloaded and has old behaviour inside transaction/3 for
backwards compatibility. However transaction/3 is deprecated because is
makes assumptions about database status that may not hold.

New return values from handle_begin/2, handle_rollback/2 and
handle_commit/2 allow to signal mistaken database status.

Author: fishcakez

integration_test/cases/transaction_execute_test.exs

@@ -263,7 +263,8 @@ defmodule TransactionExecuteTest do
         P.rollback(conn2, :oops)
       end) == {:error, :oops}
 
-      assert_raise DBConnection.ConnectionError, "transaction rolling back",
+      assert_raise DBConnection.ConnectionError,
+        "legacy transaction rolling back",
         fn() -> P.execute!(conn, %Q{}, [:param]) end
     end) == {:error, :rollback}
 
@@ -273,7 +274,7 @@ defmodule TransactionExecuteTest do
       handle_rollback: [_, :new_state]] = A.record(agent)
   end
 
-  test "transaction does not log commit if closed" do
+  test "transaction logs rollback if closed" do
    stack = [
       {:ok, :state},
       {:ok, :began, :new_state},
@@ -290,13 +291,12 @@ defmodule TransactionExecuteTest do
     log = &send(parent, &1)
 
     P.transaction(pool, fn(conn) ->
-      assert_received %DBConnection.LogEntry{call: :transaction,
-        query: :begin}
+      assert_received %DBConnection.LogEntry{call: :begin, query: :begin}
       assert_raise DBConnection.ConnectionError,
         fn() -> P.execute(conn, %Q{}, [:param]) end
     end, [log: log])
 
-    refute_received %DBConnection.LogEntry{call: :transaction}
+    assert_received %DBConnection.LogEntry{call: :rollback, query: :rollback}
 
     assert [
       {:connect, [_]},

integration_test/cases/transaction_test.exs

@@ -52,8 +52,8 @@ defmodule TransactionTest do
     log = &send(parent, &1)
 
     assert P.transaction(pool, fn(_) ->
-      assert_received %DBConnection.LogEntry{call: :transaction} = entry
-      assert %{query: :begin, params: nil, result: {:ok, :began}} = entry
+      assert_received %DBConnection.LogEntry{call: :begin} = entry
+      assert %{query: :begin, params: nil, result: {:ok, _, :began}} = entry
       assert is_integer(entry.pool_time)
       assert entry.pool_time >= 0
       assert is_integer(entry.connection_time)
@@ -63,19 +63,19 @@ defmodule TransactionTest do
       :result
     end, [log: log]) == {:ok, :result}
 
-    assert_received %DBConnection.LogEntry{call: :transaction} = entry
+    assert_received %DBConnection.LogEntry{call: :commit} = entry
     assert %{query: :commit, params: nil, result: {:ok, :committed}} = entry
     assert is_nil(entry.pool_time)
     assert is_integer(entry.connection_time)
     assert entry.connection_time >= 0
     assert is_nil(entry.decode_time)
 
     assert P.transaction(pool, fn(conn) ->
-      assert_received %DBConnection.LogEntry{}
+      assert_received %DBConnection.LogEntry{call: :begin}
       P.rollback(conn, :result)
     end, [log: log]) == {:error, :result}
 
-    assert_received %DBConnection.LogEntry{call: :transaction} = entry
+    assert_received %DBConnection.LogEntry{call: :rollback} = entry
     assert %{query: :rollback, params: nil, result: {:ok, :rolledback}} = entry
     assert is_nil(entry.pool_time)
     assert is_integer(entry.connection_time)
@@ -304,7 +304,7 @@ defmodule TransactionTest do
         P.transaction(pool, fn(_) -> flunk("transaction ran") end, [log: log])
       end
 
-    assert_received %DBConnection.LogEntry{call: :transaction} = entry
+    assert_received %DBConnection.LogEntry{call: :begin} = entry
     assert %{query: :begin, params: nil, result: {:error, ^err}} = entry
     assert is_integer(entry.pool_time)
     assert entry.pool_time >= 0
@@ -341,7 +341,7 @@ defmodule TransactionTest do
         P.transaction(pool, fn(_) -> flunk("transaction ran") end, [log: log])
       end
 
-    assert_received %DBConnection.LogEntry{call: :transaction} = entry
+    assert_received %DBConnection.LogEntry{call: :begin} = entry
     assert %{query: :begin, params: nil, result: {:error, err}} = entry
     assert %DBConnection.ConnectionError{message: "an exception was raised: ** (RuntimeError) oops" <> _} = err
     assert is_integer(entry.pool_time)
@@ -494,11 +494,11 @@ defmodule TransactionTest do
     assert_raise RuntimeError, "oops",
       fn() ->
         P.transaction(pool, fn(_) ->
-          assert_received %DBConnection.LogEntry{}
+          assert_received %DBConnection.LogEntry{call: :begin}
         end, [log: log])
       end
 
-    assert_received %DBConnection.LogEntry{call: :transaction} = entry
+    assert_received %DBConnection.LogEntry{call: :commit} = entry
     assert %{query: :commit, params: nil, result: {:error, ^err}} = entry
     assert is_nil(entry.pool_time)
     assert is_integer(entry.connection_time)
@@ -635,10 +635,10 @@ defmodule TransactionTest do
         P.transaction(pool, fn(_) -> :ok end, [log: log])
       end
 
-    assert_received %DBConnection.LogEntry{call: :transaction} = entry
-    assert %{query: :begin, params: nil, result: {:ok, :began}} = entry
+    assert_received %DBConnection.LogEntry{call: :begin} = entry
+    assert %{query: :begin, params: nil, result: {:ok, _, :began}} = entry
 
-    assert_received %DBConnection.LogEntry{call: :transaction} = entry
+    assert_received %DBConnection.LogEntry{call: :commit} = entry
     assert %{query: :commit, params: nil, result: {:error, err}} = entry
     assert %DBConnection.ConnectionError{message: "an exception was raised: ** (RuntimeError) oops" <> _} = err
     assert is_nil(entry.pool_time)
@@ -725,10 +725,10 @@ defmodule TransactionTest do
         P.transaction(pool, &P.rollback(&1, :oops), [log: log])
       end
 
-    assert_received %DBConnection.LogEntry{call: :transaction} = entry
-    assert %{query: :begin, params: nil, result: {:ok, :began}} = entry
+    assert_received %DBConnection.LogEntry{call: :begin} = entry
+    assert %{query: :begin, params: nil, result: {:ok, _, :began}} = entry
 
-    assert_received %DBConnection.LogEntry{call: :transaction} = entry
+    assert_received %DBConnection.LogEntry{call: :rollback} = entry
     assert %{query: :rollback, params: nil, result: {:error, err}} = entry
     assert %DBConnection.ConnectionError{message: "an exception was raised: ** (RuntimeError) oops" <> _} = err
     assert is_nil(entry.pool_time)
@@ -760,17 +760,197 @@ defmodule TransactionTest do
     assert_raise RuntimeError, "oops",
       fn() ->
         P.transaction(pool, fn(_) ->
-          assert_received %DBConnection.LogEntry{call: :transaction,
-            query: :begin}
+          assert_received %DBConnection.LogEntry{call: :begin, query: :begin}
           raise "oops"
         end, [log: log])
       end
 
-    assert_received %DBConnection.LogEntry{call: :transaction, query: :rollback}
+    assert_received %DBConnection.LogEntry{call: :rollback, query: :rollback}
 
     assert [
       connect: [_],
       handle_begin: [_, :state],
       handle_rollback: [_, :new_state]] = A.record(agent)
   end
+
+  test "transaction logs begin :commit and :rollback" do
+    stack = [
+      {:ok, :state},
+      {:commit, :new_state},
+      {:rollback, :newer_state},
+      ]
+    {:ok, agent} = A.start_link(stack)
+
+    parent = self()
+    opts = [agent: agent, parent: parent]
+    {:ok, pool} = P.start_link(opts)
+
+    log = &send(parent, &1)
+
+    assert P.transaction(pool, fn(_) -> flunk("transaction ran") end,
+      [log: log]) == {:error, :rollback}
+
+    assert_received %DBConnection.LogEntry{call: :begin} = entry
+    err = DBConnection.TransactionError.exception(:commit)
+    assert %{query: :begin, params: nil, result: {:error, ^err}} = entry
+    assert is_integer(entry.pool_time)
+    assert entry.pool_time >= 0
+    assert is_integer(entry.connection_time)
+    assert entry.connection_time >= 0
+    assert is_nil(entry.decode_time)
+
+    refute_received %DBConnection.LogEntry{}
+
+    assert P.transaction(pool, fn(_) -> flunk("transaction ran") end,
+      [log: log]) == {:error, :rollback}
+
+    assert_received %DBConnection.LogEntry{call: :begin} = entry
+    err = DBConnection.TransactionError.exception(:rollback)
+    assert %{query: :begin, params: nil, result: {:error, ^err}} = entry
+    assert is_integer(entry.pool_time)
+    assert entry.pool_time >= 0
+    assert is_integer(entry.connection_time)
+    assert entry.connection_time >= 0
+    assert is_nil(entry.decode_time)
+
+    refute_received %DBConnection.LogEntry{}
+
+    assert [
+      connect: [_],
+      handle_begin: [ _, :state],
+      handle_begin: [_, :new_state]] = A.record(agent)
+  end
+
+  test "transaction logs commit :begin and :rollback" do
+    stack = [
+      {:ok, :state},
+      {:ok, :began, :new_state},
+      {:begin, :newer_state},
+      {:ok, :began, :newest_state},
+      {:rollback, :state2},
+      {:ok, :rolledback, :new_state2}
+      ]
+    {:ok, agent} = A.start_link(stack)
+
+    parent = self()
+    opts = [agent: agent, parent: parent]
+    {:ok, pool} = P.start_link(opts)
+
+    log = &send(parent, &1)
+
+    assert P.transaction(pool,
+      fn(_) -> assert_receive %DBConnection.LogEntry{call: :begin} end,
+      [log: log]) == {:error, :rollback}
+
+    assert_received %DBConnection.LogEntry{call: :commit} = entry
+    err = DBConnection.TransactionError.exception(:begin)
+    assert %{query: :commit, params: nil, result: {:error, ^err}} = entry
+    assert is_nil(entry.pool_time)
+    assert is_integer(entry.connection_time)
+    assert entry.connection_time >= 0
+    assert is_nil(entry.decode_time)
+
+    refute_received %DBConnection.LogEntry{}
+
+    assert P.transaction(pool,
+      fn(_) -> assert_receive %DBConnection.LogEntry{call: :begin} end,
+      [log: log]) == {:error, :rollback}
+
+    assert_received %DBConnection.LogEntry{call: :commit} = entry
+    assert %{query: :rollback, params: nil, result: {:ok, :rolledback}} = entry
+    assert is_nil(entry.pool_time)
+    assert is_integer(entry.connection_time)
+    assert entry.connection_time >= 0
+    assert is_nil(entry.decode_time)
+
+    refute_received %DBConnection.LogEntry{}
+
+    assert [
+      connect: [_],
+      handle_begin: [ _, :state],
+      handle_commit: [_, :new_state],
+      handle_begin: [_, :newer_state],
+      handle_commit: [_, :newest_state],
+      handle_rollback: [_, :state2]] = A.record(agent)
+  end
+
+  test "transaction logs rollback :begin" do
+    stack = [
+      {:ok, :state},
+      {:ok, :began, :new_state},
+      {:begin, :newer_state}
+      ]
+    {:ok, agent} = A.start_link(stack)
+
+    parent = self()
+    opts = [agent: agent, parent: parent]
+    {:ok, pool} = P.start_link(opts)
+
+    log = &send(parent, &1)
+
+    assert P.transaction(pool,
+      fn(conn) ->
+        assert_receive %DBConnection.LogEntry{call: :begin}
+        P.rollback(conn, :oops)
+      end,
+      [log: log]) == {:error, :oops}
+
+    assert_received %DBConnection.LogEntry{call: :rollback} = entry
+    err = DBConnection.TransactionError.exception(:begin)
+    assert %{query: :rollback, params: nil, result: {:error, ^err}} = entry
+    assert is_nil(entry.pool_time)
+    assert is_integer(entry.connection_time)
+    assert entry.connection_time >= 0
+    assert is_nil(entry.decode_time)
+
+    refute_received %DBConnection.LogEntry{}
+
+    assert [
+      connect: [_],
+      handle_begin: [ _, :state],
+      handle_rollback: [_, :new_state]] = A.record(agent)
+  end
+
+  test "transaction with explicit begin/rollback/commit call causes rollback" do
+    stack = [
+      {:ok, :state},
+      {:ok, :began, :new_state},
+      {:ok, :rolledback, :newer_state},
+      {:ok, :began, :newest_state},
+      {:ok, :rolledback, :state2},
+      {:ok, :began, :new_state2},
+      {:ok, :rolledback, :newer_state2}
+      ]
+    {:ok, agent} = A.start_link(stack)
+
+    opts = [agent: agent, parent: self()]
+    {:ok, pool} = P.start_link(opts)
+
+    assert P.transaction(pool, fn(conn) ->
+      assert_raise DBConnection.ConnectionError,
+        "can not begin inside legacy transaction",
+        fn -> P.begin!(conn) end
+    end) == {:error, :rollback}
+
+    assert P.transaction(pool, fn(conn) ->
+      assert_raise DBConnection.ConnectionError,
+        "can not rollback inside legacy transaction",
+        fn -> P.rollback!(conn) end
+    end) == {:error, :rollback}
+
+    assert P.transaction(pool, fn(conn) ->
+      assert_raise DBConnection.ConnectionError,
+        "can not commit inside legacy transaction",
+        fn -> P.commit!(conn) end
+    end) == {:error, :rollback}
+
+    assert [
+      connect: [_],
+      handle_begin: [ _, :state],
+      handle_rollback: [_, :new_state],
+      handle_begin: [ _, :newer_state],
+      handle_rollback: [_, :newest_state],
+      handle_begin: [_, :state2],
+      handle_rollback: [_, :new_state2]] = A.record(agent)
+  end
 end