diff --git a/.gitignore b/.gitignore index b6012c7..9239edb 100644 --- a/.gitignore +++ b/.gitignore @@ -18,3 +18,5 @@ erl_crash.dump # Also ignore archive artifacts (built via "mix archive.build"). *.ez + +.elixir_ls diff --git a/README.md b/README.md index 9c815cc..dd6f425 100644 --- a/README.md +++ b/README.md @@ -1,10 +1,10 @@ -# CockroachDBSandbox +# EctoReplaySandbox -This is an override of Ecto.Adapters.SQL.Sandbox designed to work for CockroachDB by not leveraging savepoints. -Each test runs inside a transaction managed by the Sandbox, just like with regular Ecto Sandbox. +This is a custom implementation of Ecto.Adapters.SQL.Sandbox designed to work with CockroachDB by not leveraging savepoints. +Each test runs inside a transaction managed by the Sandbox, just like with default Ecto Sandbox. Inside your test, when your code opens a transaction block, given that CockroachDB does not support nested transactions or savepoints, no actual database transaction is created. -Instead the Sandbox is using a log approach described below and such transaction are called pseudo transaction. +Instead the sandbox is using a log approach described below and such transaction are called pseudo transaction. The sandbox maintains 2 logs for a given managed transaction: - Sandbox log @@ -21,15 +21,36 @@ Once the test finishes, the managed transaction is being rollbacked to restore t ## Installation If [available in Hex](https://hex.pm/docs/publish), the package can be installed -by adding `cockroachdb_sandbox` to your list of dependencies in `mix.exs`: +by adding `ecto_replay_sandbox` to your list of dependencies in `mix.exs`: ```elixir def deps do - [{:cockroachdb_sandbox, "~> 0.1.0", only: :test}] + [{:ecto_replay_sandbox, "~> 1.0.0", only: :test}] end ``` -> You need to declare this dependency after Ecto in order to make sure that we override the default Ecto Sandbox. +## Usage + +In your `config/test.ex` +```elixir +config :my_app, MyApp.Repo, + pool: EctoReplaySandbox +``` + +Then in your `test/test_helper.ex` + +Replace the following line: +```elixir +Ecto.Adapters.SQL.Sandbox.mode(MyApp.Repo, :manual) +``` + +with: +```elixir +sandbox = Application.get_env(:my_app, MyApp.Repo)[:pool] +sandbox.mode(MyApp.Repo, :manual) +``` + +It effectively removes the hardcoded usage of `Ecto.Adapters.SQL.Sandbox` with a dynamic lookup of the configured pool. ## Credits diff --git a/lib/cockroachdb_sandbox.ex b/lib/ecto_replay_sandbox.ex similarity index 93% rename from lib/cockroachdb_sandbox.ex rename to lib/ecto_replay_sandbox.ex index c473b25..ac5066d 100644 --- a/lib/cockroachdb_sandbox.ex +++ b/lib/ecto_replay_sandbox.ex @@ -1,4 +1,4 @@ -defmodule Ecto.Adapters.SQL.Sandbox do +defmodule EctoReplaySandbox do @moduledoc ~S""" A pool for concurrent transactional tests. @@ -322,7 +322,7 @@ defmodule Ecto.Adapters.SQL.Sandbox do {state, sandbox_log} = case sandbox_log do [:error_detected | tail] -> restart_result = restart_sandbox_tx(conn_mod, state) - {elem(restart_result, 2),[:replay_needed] ++ tail} + {elem(restart_result, 2),[:replay_needed | tail]} _ -> {state, sandbox_log ++ tx_log} end @@ -330,9 +330,13 @@ defmodule Ecto.Adapters.SQL.Sandbox do {:ok, @commit_result, {conn_mod, state, false, {sandbox_log, []}}} end def handle_rollback(opts, {conn_mod, state, true, {sandbox_log, _}}) do + sandbox_log = case sandbox_log do + [:error_detected | tail] -> tail + _ -> sandbox_log + end case restart_sandbox_tx(conn_mod, state, opts) do {:ok, _, conn_state} -> - {:ok, @rollback_result, {conn_mod, conn_state, false, {[:replay_needed] ++ sandbox_log, []}}} + {:ok, @rollback_result, {conn_mod, conn_state, false, {[:replay_needed | sandbox_log], []}}} error -> pos = :erlang.tuple_size(error) :erlang.setelement(pos, error, {conn_mod, :erlang.element(pos, error), false, {sandbox_log, []}}) @@ -359,10 +363,10 @@ defmodule Ecto.Adapters.SQL.Sandbox do defp proxy(fun, {conn_mod, state, in_transaction?, {sandbox_log, _tx_log} = log_state}, args) do # Handle replay {state, log_state} = case sandbox_log do - [head | tail] when head == :replay_needed -> + [:replay_needed | tail] -> state = tail |> Enum.reduce(state, fn {replay_fun, replay_args}, state -> - {status, _, state} = apply(conn_mod, replay_fun, replay_args ++ [state]) + {_status, _, state} = apply(conn_mod, replay_fun, replay_args ++ [state]) state end) {state, {tail, []}} @@ -379,10 +383,14 @@ defmodule Ecto.Adapters.SQL.Sandbox do {state, log_command(fun, args, in_transaction?, log_state)} :error -> if(in_transaction?) do - {state, {[:error_detected] ++ elem(log_state, 0), []}} + log_state = case sandbox_log do + [:error_detected | _tail] -> {elem(log_state, 0), []} + _ -> {[:error_detected | elem(log_state, 0)], []} + end + {state, log_state} else restart_result = restart_sandbox_tx(conn_mod, state) - {elem(restart_result, 2),{[:replay_needed] ++ elem(log_state, 0), []}} + {elem(restart_result, 2),{[:replay_needed | elem(log_state, 0)], []}} end end @@ -472,7 +480,7 @@ defmodule Ecto.Adapters.SQL.Sandbox do def mode(repo, mode) when mode in [:auto, :manual] when elem(mode, 0) == :shared and is_pid(elem(mode, 1)) do - {name, opts} = repo.__pool__ + {_repo_mod, name, opts} = Ecto.Registry.lookup(repo) if opts[:pool] != DBConnection.Ownership do raise """ @@ -513,11 +521,11 @@ defmodule Ecto.Adapters.SQL.Sandbox do 15000 ms if not set. """ def checkout(repo, opts \\ []) do - {name, pool_opts} = + {_repo_mod, name, pool_opts} = if Keyword.get(opts, :sandbox, true) do proxy_pool(repo) else - repo.__pool__ + Ecto.Registry.lookup(repo) end pool_opts_overrides = Keyword.take(opts, [:ownership_timeout]) @@ -549,7 +557,7 @@ defmodule Ecto.Adapters.SQL.Sandbox do Checks in the connection back into the sandbox pool. """ def checkin(repo, _opts \\ []) do - {name, opts} = repo.__pool__ + {_repo_mod, name, opts} = Ecto.Registry.lookup(repo) DBConnection.Ownership.ownership_checkin(name, opts) end @@ -557,13 +565,34 @@ defmodule Ecto.Adapters.SQL.Sandbox do Allows the `allow` process to use the same connection as `parent`. """ def allow(repo, parent, allow, _opts \\ []) do - {name, opts} = repo.__pool__ + {_repo_mod, name, opts} = Ecto.Registry.lookup(repo) DBConnection.Ownership.ownership_allow(name, parent, allow, opts) end + def ensure_all_started(app, type \\ :temporary) do + DBConnection.Ownership.ensure_all_started(app, type) + end + + def child_spec(module, opts, child_opts) do + DBConnection.Ownership.child_spec(module, opts, child_opts) + end + + @doc """ + Runs a function outside of the sandbox. + """ + def unboxed_run(repo, fun) do + checkin(repo) + checkout(repo, sandbox: false) + try do + fun.() + after + checkin(repo) + end + end + defp proxy_pool(repo) do - {name, opts} = repo.__pool__ + {repo_mod, name, opts} = Ecto.Registry.lookup(repo) {pool, opts} = Keyword.pop(opts, :ownership_pool, DBConnection.Poolboy) - {name, [repo: repo, sandbox_pool: pool, ownership_pool: Pool] ++ opts} + {repo_mod, name, [repo: repo, sandbox_pool: pool, ownership_pool: Pool] ++ opts} end end \ No newline at end of file diff --git a/mix.exs b/mix.exs index 27ab58e..c6f8503 100644 --- a/mix.exs +++ b/mix.exs @@ -1,15 +1,28 @@ -defmodule CockroachDBSandbox.Mixfile do +defmodule EctoReplaySandbox.Mixfile do use Mix.Project + @version "1.0.0" + def project do - [app: :cockroachdb_sandbox, - version: "0.1.0", + [app: :ecto_replay_sandbox, + version: @version, elixir: "~> 1.4", build_embedded: Mix.env == :prod, start_permanent: Mix.env == :prod, + package: package(), + source_url: "https://github.com/jumpn/ecto_replay_sandbox", + docs: [source_ref: "v#{@version}", main: "EctoReplaySandbox"], deps: deps()] end + defp package do + [description: "Log replay based sandbox for Ecto, compatible with CockroachDB", + files: ["lib", "mix.exs", "README*"], + maintainers: ["Christian Meunier"], + licenses: ["MIT"], + links: %{github: "https://github.com/jumpn/ecto_replay_sandbox"}] + end + def application do [ extra_applications: [:logger], @@ -18,7 +31,7 @@ defmodule CockroachDBSandbox.Mixfile do defp deps do [ - {:ecto, "~> 2.1"}, + {:ecto, "~> 2.2"}, {:db_connection, "~> 1.1"}, {:postgrex, git: "git@github.com:jumpn/postgrex.git", override: true}, ] diff --git a/mix.lock b/mix.lock index 6610159..21f0f68 100644 --- a/mix.lock +++ b/mix.lock @@ -1,6 +1,6 @@ %{"connection": {:hex, :connection, "1.0.4", "a1cae72211f0eef17705aaededacac3eb30e6625b04a6117c1b2db6ace7d5976", [:mix], []}, - "db_connection": {:hex, :db_connection, "1.1.1", "f9d246e8f65b9490945cf7360875eee18fcec9a0115207603215eb1fd94c39ef", [:mix], [{:connection, "~> 1.0.2", [hex: :connection, optional: false]}, {:poolboy, "~> 1.5", [hex: :poolboy, optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, optional: true]}]}, - "decimal": {:hex, :decimal, "1.3.1", "157b3cedb2bfcb5359372a7766dd7a41091ad34578296e951f58a946fcab49c6", [:mix], []}, - "ecto": {:hex, :ecto, "2.1.3", "ffb24e150b519a4c0e4c84f9eabc8587199389bc499195d5d1a93cd3b2d9a045", [:mix], [{:db_connection, "~> 1.1", [hex: :db_connection, optional: true]}, {:decimal, "~> 1.2", [hex: :decimal, optional: false]}, {:mariaex, "~> 0.8.0", [hex: :mariaex, optional: true]}, {:poison, "~> 2.2 or ~> 3.0", [hex: :poison, optional: true]}, {:poolboy, "~> 1.5", [hex: :poolboy, optional: false]}, {:postgrex, "~> 0.13.0", [hex: :postgrex, optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, optional: true]}]}, + "db_connection": {:hex, :db_connection, "1.1.2", "2865c2a4bae0714e2213a0ce60a1b12d76a6efba0c51fbda59c9ab8d1accc7a8", [:mix], [{:connection, "~> 1.0.2", [hex: :connection, optional: false]}, {:poolboy, "~> 1.5", [hex: :poolboy, optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, optional: true]}]}, + "decimal": {:hex, :decimal, "1.4.0", "fac965ce71a46aab53d3a6ce45662806bdd708a4a95a65cde8a12eb0124a1333", [:mix], []}, + "ecto": {:hex, :ecto, "2.2.4", "defde3c8eca385bd86466d2e1491d19e77f9b79ad996dc8e89e4e107f3942f40", [:mix], [{:db_connection, "~> 1.1", [hex: :db_connection, optional: true]}, {:decimal, "~> 1.2", [hex: :decimal, optional: false]}, {:mariaex, "~> 0.8.0", [hex: :mariaex, optional: true]}, {:poison, "~> 2.2 or ~> 3.0", [hex: :poison, optional: true]}, {:poolboy, "~> 1.5", [hex: :poolboy, optional: false]}, {:postgrex, "~> 0.13.0", [hex: :postgrex, optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, optional: true]}]}, "poolboy": {:hex, :poolboy, "1.5.1", "6b46163901cfd0a1b43d692657ed9d7e599853b3b21b95ae5ae0a777cf9b6ca8", [:rebar], []}, - "postgrex": {:git, "git@github.com:jumpn/postgrex.git", "977367cd1c96a52e09992282e22ecec9f4622f49", []}} + "postgrex": {:git, "git@github.com:jumpn/postgrex.git", "cce1d0ac21f5cf200a0848d8a3efd7b7f8d07bbc", []}} diff --git a/test/cockroachdb_sandbox_test.exs b/test/ecto_replay_sandbox_test.exs similarity index 81% rename from test/cockroachdb_sandbox_test.exs rename to test/ecto_replay_sandbox_test.exs index b01f53f..8b63501 100644 --- a/test/cockroachdb_sandbox_test.exs +++ b/test/ecto_replay_sandbox_test.exs @@ -1,10 +1,9 @@ -defmodule CockroachDBSandboxTest do +defmodule EctoReplaySandboxTest do use ExUnit.Case - #alias CockroachDBSandbox, as: Sandbox - alias Ecto.Adapters.SQL.Sandbox - alias CockroachDBSandbox.Integration.TestRepo - alias CockroachDBSandbox.Integration.Post + alias EctoReplaySandbox, as: Sandbox + alias EctoReplaySandbox.Integration.TestRepo + alias EctoReplaySandbox.Integration.Post import ExUnit.CaptureLog @@ -85,22 +84,7 @@ defmodule CockroachDBSandboxTest do Sandbox.checkin(TestRepo) end - @tag :pending - test "disconnects sandbox on transaction timeouts" do - Sandbox.checkout(TestRepo) - - assert capture_log(fn -> - catch_error( - TestRepo.transaction fn -> - :timer.sleep(1000) - end, timeout: 0 - ) - end) =~ "timed out" - - Sandbox.checkin(TestRepo) - end - - test "runs inside a sandbox even with failed queries" do + test "works even with failed queries" do Sandbox.checkout(TestRepo) {:ok, _} = TestRepo.insert(%Post{}, skip_transaction: true) @@ -112,7 +96,7 @@ defmodule CockroachDBSandboxTest do Sandbox.checkin(TestRepo) end - test "works inside failed transaction is rollbacked" do + test "the failed transaction is properly rollbacked" do Sandbox.checkout(TestRepo) TestRepo.transaction fn -> @@ -138,16 +122,34 @@ defmodule CockroachDBSandboxTest do Sandbox.checkin(TestRepo) end - test "work inside failed transaction is rollbacked" do + test "sanbox still works once a transaction with a failed changeset is rollbacked" do Sandbox.checkout(TestRepo) - {:ok, _} = TestRepo.insert(%Post{}, skip_transaction: true) + {:ok, _} = TestRepo.insert(%Post{id: 1}, skip_transaction: true) + TestRepo.transaction fn -> - TestRepo.insert(%Post{}) - # This is a failed query to trigger a rollback - {:error, _} = TestRepo.query("INVALID") + %Post{} + |> Post.changeset(%{id: 1}) + |> TestRepo.insert end - assert 1 == TestRepo.all(Post) |> Enum.count + + TestRepo.all(Post) + + Sandbox.checkin(TestRepo) + end + + test "sanbox replays log in correct order" do + Sandbox.checkout(TestRepo) + + {:ok, _post} = TestRepo.insert(%Post{}, skip_transaction: true) + TestRepo.update_all(Post, set: [title: "New title"]) + TestRepo.update_all(Post, set: [title: "New title2"]) + + # This is a failed query but it should not taint the sandbox transaction + {:error, _} = TestRepo.query("INVALID") + + assert [post] = TestRepo.all(Post) + assert post.title == "New title2" Sandbox.checkin(TestRepo) end diff --git a/test/migration.exs b/test/migration.exs index 612a97b..4864109 100644 --- a/test/migration.exs +++ b/test/migration.exs @@ -1,4 +1,4 @@ -defmodule CockroachDBSandbox.Integration.Migration do +defmodule EctoReplaySandbox.Integration.Migration do use Ecto.Migration def change do diff --git a/test/repo.exs b/test/repo.exs index 693e9b5..7161ff2 100644 --- a/test/repo.exs +++ b/test/repo.exs @@ -1,10 +1,10 @@ -defmodule CockroachDBSandbox.Integration.Repo do +defmodule EctoReplaySandbox.Integration.Repo do defmacro __using__(opts) do quote do - config = Application.get_env(:cockroachdb_sandbox, __MODULE__) + config = Application.get_env(:ecto_replay_sandbox, __MODULE__) config = Keyword.put(config, :loggers, [Ecto.LogEntry, - {CockroachDBSandbox.Integration.Repo, :log, [:on_log]}]) - Application.put_env(:cockroachdb_sandbox, __MODULE__, config) + {EctoReplaySandbox.Integration.Repo, :log, [:on_log]}]) + Application.put_env(:ecto_replay_sandbox, __MODULE__, config) use Ecto.Repo, unquote(opts) end end diff --git a/test/schema.exs b/test/schema.exs index 9a1e3f8..8a02007 100644 --- a/test/schema.exs +++ b/test/schema.exs @@ -1,4 +1,4 @@ -defmodule CockroachDBSandbox.Integration.Schema do +defmodule EctoReplaySandbox.Integration.Schema do defmacro __using__(_) do quote do use Ecto.Schema @@ -13,7 +13,7 @@ defmodule CockroachDBSandbox.Integration.Schema do end end -defmodule CockroachDBSandbox.Integration.User do +defmodule EctoReplaySandbox.Integration.User do @moduledoc """ This module is used to test: @@ -21,17 +21,17 @@ defmodule CockroachDBSandbox.Integration.User do * Relationships """ - use CockroachDBSandbox.Integration.Schema + use EctoReplaySandbox.Integration.Schema schema "users" do field :name, :string - has_many :comments, CockroachDBSandbox.Integration.Comment, foreign_key: :author_id, on_delete: :nilify_all, on_replace: :nilify - has_many :posts, CockroachDBSandbox.Integration.Post, foreign_key: :author_id, on_delete: :nothing, on_replace: :delete + has_many :comments, EctoReplaySandbox.Integration.Comment, foreign_key: :author_id, on_delete: :nilify_all, on_replace: :nilify + has_many :posts, EctoReplaySandbox.Integration.Post, foreign_key: :author_id, on_delete: :nothing, on_replace: :delete timestamps(type: :utc_datetime) end end -defmodule CockroachDBSandbox.Integration.Post do +defmodule EctoReplaySandbox.Integration.Post do @moduledoc """ This module is used to test: @@ -42,7 +42,7 @@ defmodule CockroachDBSandbox.Integration.Post do * Dependent callbacks """ - use CockroachDBSandbox.Integration.Schema + use EctoReplaySandbox.Integration.Schema import Ecto.Changeset schema "posts" do @@ -55,8 +55,8 @@ defmodule CockroachDBSandbox.Integration.Post do field :visits, :integer field :intensity, :float field :posted, :date - has_many :comments, CockroachDBSandbox.Integration.Comment, on_delete: :delete_all, on_replace: :delete - belongs_to :author, CockroachDBSandbox.Integration.User + has_many :comments, EctoReplaySandbox.Integration.Comment, on_delete: :delete_all, on_replace: :delete + belongs_to :author, EctoReplaySandbox.Integration.User timestamps() end @@ -66,7 +66,7 @@ defmodule CockroachDBSandbox.Integration.Post do end end -defmodule CockroachDBSandbox.Integration.Comment do +defmodule EctoReplaySandbox.Integration.Comment do @moduledoc """ This module is used to test: @@ -74,12 +74,12 @@ defmodule CockroachDBSandbox.Integration.Comment do * Dependent callbacks """ - use CockroachDBSandbox.Integration.Schema + use EctoReplaySandbox.Integration.Schema schema "comments" do field :text, :string - belongs_to :post, CockroachDBSandbox.Integration.Post - belongs_to :author, CockroachDBSandbox.Integration.User + belongs_to :post, EctoReplaySandbox.Integration.Post + belongs_to :author, EctoReplaySandbox.Integration.User end def changeset(schema, params) do diff --git a/test/test_helper.exs b/test/test_helper.exs index 8a9cb2b..6a0ad18 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -9,9 +9,8 @@ Application.put_env(:ecto, :lock_for_update, "FOR UPDATE") Application.put_env(:ecto, :primary_key_type, :id) # Configure CockroachDB connection -Application.put_env(:cockroachdb_sandbox, :cdb_test_url, +Application.put_env(:ecto_replay_sandbox, :cdb_test_url, "ecto://" <> (System.get_env("CDB_URL") || "root@localhost:26257") - #"ecto://" <> (System.get_env("CDB_URL") || "postgres@localhost:5432") ) # Load support files @@ -26,16 +25,16 @@ pool = end # Pool repo for async, safe tests -alias CockroachDBSandbox.Integration.TestRepo +alias EctoReplaySandbox.Integration.TestRepo -Application.put_env(:cockroachdb_sandbox, TestRepo, +Application.put_env(:ecto_replay_sandbox, TestRepo, adapter: Ecto.Adapters.Postgres, - url: Application.get_env(:cockroachdb_sandbox, :cdb_test_url) <> "/cockroachdb_sandbox_test", - pool: Ecto.Adapters.SQL.Sandbox, + url: Application.get_env(:ecto_replay_sandbox, :cdb_test_url) <> "/ecto_replay_sandbox_test", + pool: EctoReplaySandbox, ownership_pool: pool) -defmodule CockroachDBSandbox.Integration.TestRepo do - use CockroachDBSandbox.Integration.Repo, otp_app: :cockroachdb_sandbox +defmodule EctoReplaySandbox.Integration.TestRepo do + use EctoReplaySandbox.Integration.Repo, otp_app: :ecto_replay_sandbox end {:ok, _} = Ecto.Adapters.Postgres.ensure_all_started(TestRepo, :temporary) @@ -46,6 +45,6 @@ _ = Ecto.Adapters.Postgres.storage_down(TestRepo.config) {:ok, _pid} = TestRepo.start_link -:ok = Ecto.Migrator.up(TestRepo, 0, CockroachDBSandbox.Integration.Migration, log: false) -Ecto.Adapters.SQL.Sandbox.mode(TestRepo, :manual) +:ok = Ecto.Migrator.up(TestRepo, 0, EctoReplaySandbox.Integration.Migration, log: false) +EctoReplaySandbox.mode(TestRepo, :manual) Process.flag(:trap_exit, true)