Merge pull request #1 from jumpn/no-override

Don't override the sandbox, leverage pluggable pool introduced with Ecto 2.2

.gitignore

@@ -18,3 +18,5 @@ erl_crash.dump
 
 # Also ignore archive artifacts (built via "mix archive.build").
 *.ez
+
+.elixir_ls

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
 

lib/cockroachdb_sandbox.ex → 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,17 +322,21 @@ 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
 
       {: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,21 +557,42 @@ 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
 
   @doc """
   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

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: "[email protected]:jumpn/postgrex.git", override: true},
     ]

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, "[email protected]:jumpn/postgrex.git", "977367cd1c96a52e09992282e22ecec9f4622f49", []}}
+  "postgrex": {:git, "[email protected]:jumpn/postgrex.git", "cce1d0ac21f5cf200a0848d8a3efd7b7f8d07bbc", []}}

test/cockroachdb_sandbox_test.exs → 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

test/migration.exs

@@ -1,4 +1,4 @@
-defmodule CockroachDBSandbox.Integration.Migration do
+defmodule EctoReplaySandbox.Integration.Migration do
   use Ecto.Migration
 
   def change do

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