diff --git a/lib/ecto_libsql.ex b/lib/ecto_libsql.ex index 94f49fc..b8c3aba 100644 --- a/lib/ecto_libsql.ex +++ b/lib/ecto_libsql.ex @@ -302,6 +302,12 @@ defmodule EctoLibSql do |> Enum.map(fn [_full, name] -> name end) end + # DBConnection drives nested transactions with `mode: :savepoint`. SQLite allows + # duplicate savepoint names and resolves RELEASE/ROLLBACK TO against the most + # recent match, so one fixed name nests correctly - the same approach postgrex + # takes with "postgrex_savepoint". + @savepoint "ecto_libsql_savepoint" + @impl true @doc """ Begins a new database transaction. @@ -309,10 +315,29 @@ defmodule EctoLibSql do The transaction behaviour (deferred/immediate/exclusive) can be controlled via options passed to the Native module. """ - def handle_begin(_opts, state) do - case EctoLibSql.Native.begin(state) do - {:ok, new_state} -> {:ok, :begin, new_state} - {:error, reason} -> {:error, reason, state} + def handle_begin(opts, state) do + # `:mode` is overloaded here: DBConnection sets `:transaction` or `:savepoint`, + # while this adapter's own API uses it for the libSQL transaction mode + # (`:deferred`, `:immediate`, `:exclusive`, `:read_only`). Only `:savepoint` is + # intercepted; everything else, including an absent mode, begins as before so + # the transaction modes keep working. + case {Keyword.get(opts, :mode), state.trx_id} do + {:savepoint, trx_id} when is_binary(trx_id) -> + case EctoLibSql.Native.create_savepoint(state, @savepoint) do + :ok -> {:ok, %EctoLibSql.Result{}, state} + {:error, reason} -> {:error, savepoint_error(reason), state} + end + + {:savepoint, nil} -> + # A savepoint outside a transaction is not something SQLite can honour. + # Report status and let DBConnection decide rather than emitting bad SQL. + {:idle, state} + + _other -> + case EctoLibSql.Native.begin(state) do + {:ok, new_state} -> {:ok, :begin, new_state} + {:error, reason} -> {:error, reason, state} + end end end @@ -328,15 +353,24 @@ defmodule EctoLibSql do The state must contain a valid transaction ID. For embedded replicas with auto-sync enabled, this will also trigger a sync to the remote database. """ - def handle_commit(_opts, state) do - case EctoLibSql.Native.commit( - %EctoLibSql.State{conn_id: conn_id, trx_id: _trx_id, mode: mode} = state - ) do - {:ok, _} -> - {:ok, %EctoLibSql.Result{}, %EctoLibSql.State{conn_id: conn_id, mode: mode}} - - {:error, reason} -> - {:disconnect, reason, state} + def handle_commit(opts, state) do + if Keyword.get(opts, :mode, :transaction) == :savepoint do + # Releasing a savepoint leaves the enclosing transaction open, so the + # state - trx_id included - carries through untouched. + case EctoLibSql.Native.release_savepoint_by_name(state, @savepoint) do + :ok -> {:ok, %EctoLibSql.Result{}, state} + {:error, reason} -> {:error, savepoint_error(reason), state} + end + else + case EctoLibSql.Native.commit( + %EctoLibSql.State{conn_id: conn_id, trx_id: _trx_id, mode: mode} = state + ) do + {:ok, _} -> + {:ok, %EctoLibSql.Result{}, %EctoLibSql.State{conn_id: conn_id, mode: mode}} + + {:error, reason} -> + {:disconnect, reason, state} + end end end @@ -347,16 +381,32 @@ defmodule EctoLibSql do Discards all changes made within the transaction and returns the connection to autocommit mode. """ - def handle_rollback(_opts, %EctoLibSql.State{conn_id: conn_id, trx_id: _trx_id} = state) do - case EctoLibSql.Native.rollback(state) do - {:ok, _} -> - {:ok, %EctoLibSql.Result{}, %EctoLibSql.State{conn_id: conn_id, trx_id: nil}} + def handle_rollback(opts, %EctoLibSql.State{conn_id: conn_id, trx_id: _trx_id} = state) do + if Keyword.get(opts, :mode, :transaction) == :savepoint do + # ROLLBACK TO leaves the savepoint itself active (SQLite keeps it so it can + # be rolled back to again), so it has to be released explicitly afterwards + # or the next savepoint of the same name nests inside this one. + with :ok <- EctoLibSql.Native.rollback_to_savepoint_by_name(state, @savepoint), + :ok <- EctoLibSql.Native.release_savepoint_by_name(state, @savepoint) do + {:ok, %EctoLibSql.Result{}, state} + else + {:error, reason} -> {:error, savepoint_error(reason), state} + end + else + case EctoLibSql.Native.rollback(state) do + {:ok, _} -> + {:ok, %EctoLibSql.Result{}, %EctoLibSql.State{conn_id: conn_id, trx_id: nil}} - {:error, reason} -> - {:disconnect, reason, state} + {:error, reason} -> + {:disconnect, reason, state} + end end end + defp savepoint_error(reason) when is_binary(reason), do: %RuntimeError{message: reason} + defp savepoint_error(%{__exception__: true} = reason), do: reason + defp savepoint_error(reason), do: %RuntimeError{message: inspect(reason)} + @impl true @doc """ Closes the query. Currently a no-op as queries are stateless. diff --git a/test/ecto_sandbox_savepoint_test.exs b/test/ecto_sandbox_savepoint_test.exs new file mode 100644 index 0000000..999eedb --- /dev/null +++ b/test/ecto_sandbox_savepoint_test.exs @@ -0,0 +1,115 @@ +defmodule EctoLibSql.EctoSandboxSavepointTest do + @moduledoc """ + Coverage for `Ecto.Adapters.SQL.Sandbox`, which is how most Ecto applications run + their entire test suite. + + The sandbox holds one transaction open for the duration of each test and rolls it + back afterwards, so every `Repo.transaction/1` in application code runs nested and + is issued by DBConnection with `mode: :savepoint`. Without SAVEPOINT support in + `handle_begin/2` that mode is discarded, a plain BEGIN is issued inside the open + transaction, and the test fails with "cannot start a transaction within a + transaction" - meaning no transactional application code can be tested at all. + """ + use ExUnit.Case, async: false + + import Ecto.Query + + alias Ecto.Adapters.SQL.Sandbox + + defmodule SandboxRepo do + use Ecto.Repo, otp_app: :ecto_libsql, adapter: Ecto.Adapters.LibSql + end + + defmodule Item do + use Ecto.Schema + + schema "sandbox_items" do + field(:num, :integer) + end + end + + setup do + unique_id = :erlang.unique_integer([:positive]) + test_db = "z_ecto_libsql_test-sandbox_savepoint-#{unique_id}.db" + + {:ok, pid} = + SandboxRepo.start_link( + database: test_db, + pool_size: 1, + pool: Sandbox, + name: SandboxRepo + ) + + Sandbox.mode(SandboxRepo, :manual) + owner = Sandbox.start_owner!(SandboxRepo, shared: true) + + Ecto.Adapters.SQL.query!(SandboxRepo, """ + CREATE TABLE IF NOT EXISTS sandbox_items ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + num INTEGER + ) + """) + + on_exit(fn -> + Sandbox.stop_owner(owner) + + # Process.alive?/1 is a point-in-time check: stopping the owner above can take + # the repo supervisor down while we are still on this line, and Supervisor.stop/1 + # then exits with :noproc. Catch it rather than failing an otherwise passing test. + if Process.alive?(pid) do + try do + :ok = Supervisor.stop(pid) + catch + :exit, _ -> :ok + end + end + + Process.sleep(50) + for f <- Path.wildcard(test_db <> "*"), do: File.rm(f) + end) + + :ok + end + + defp nums, do: SandboxRepo.all(from(i in Item, order_by: i.num, select: i.num)) + + test "a transaction can be opened inside the sandbox transaction at all" do + assert {:ok, :done} = + SandboxRepo.transaction(fn -> + SandboxRepo.insert!(%Item{num: 1}) + :done + end) + + assert nums() == [1] + end + + test "rolling back only discards work done inside the savepoint" do + SandboxRepo.insert!(%Item{num: 1}) + + assert {:error, :nope} = + SandboxRepo.transaction(fn -> + SandboxRepo.insert!(%Item{num: 2}) + SandboxRepo.rollback(:nope) + end) + + # The write made before the savepoint survives, and the sandbox transaction is + # still usable afterwards - which is the whole point of rolling back to a + # savepoint rather than aborting the enclosing transaction. + assert nums() == [1] + SandboxRepo.insert!(%Item{num: 3}) + assert nums() == [1, 3] + end + + test "an exception inside the transaction is contained the same way" do + SandboxRepo.insert!(%Item{num: 1}) + + assert_raise RuntimeError, "boom", fn -> + SandboxRepo.transaction(fn -> + SandboxRepo.insert!(%Item{num: 2}) + raise "boom" + end) + end + + assert nums() == [1] + end +end