feat: fix EctoAdapter transaction compatibility with Ecto.Adapters.SQL.Sandbox

README.md

@@ -336,6 +336,17 @@ Implement all [matchers' functions](https://casbin.org/docs/function):
 - [ ] ipMatch
 - [ ] globMatch
 
+## Testing
+
+### Using with Ecto.Adapters.SQL.Sandbox
+
+If you're using Casbin-Ex with Ecto and need to wrap operations in database transactions during testing, see our guide on [Testing with Ecto.Adapters.SQL.Sandbox and Transactions](guides/sandbox_testing.md).
+
+Key points:
+- Use `Ecto.Adapters.SQL.Sandbox.mode(Repo, {:shared, self()})` for tests with transactions
+- Allow the EnforcerServer process to access your test's database connection
+- See the guide for complete examples and best practices
+
 ## License
 
 This project is licensed under the [Apache 2.0 license](LICENSE).

guides/sandbox_testing.md

@@ -0,0 +1,169 @@
+# Testing with Ecto.Adapters.SQL.Sandbox and Transactions
+
+This guide explains how to use Casbin-Ex with `Ecto.Adapters.SQL.Sandbox` when you need to wrap Casbin operations in database transactions.
+
+## The Problem
+
+When using `Ecto.Adapters.SQL.Sandbox` in checkout mode (the default), database connections are restricted to the process that checked them out. The Casbin `EnforcerServer` runs in a separate process, which causes issues when:
+
+1. Your test wraps Casbin operations in a `Repo.transaction`
+2. The transaction locks the connection to the test process
+3. The `EnforcerServer` process cannot access the locked connection, even with `Sandbox.allow/3`
+
+This results in the error:
+```
+** (DBConnection.ConnectionError) could not checkout the connection owned by #PID<...>
+```
+
+## Solution: Use Shared Mode
+
+The recommended solution is to use Ecto's **shared mode** for tests that need to call Casbin within transactions:
+
+```elixir
+defmodule MyApp.RolesTest do
+  use MyApp.DataCase
+  use MyApp.CasbinCase
+
+  setup do
+    # Check out a connection
+    :ok = Ecto.Adapters.SQL.Sandbox.checkout(MyApp.Repo)
+
+    # Enable shared mode - this allows the EnforcerServer to use the connection
+    Ecto.Adapters.SQL.Sandbox.mode(MyApp.Repo, {:shared, self()})
+
+    # Allow the EnforcerServer process to access the connection
+    case Registry.lookup(Acx.EnforcerRegistry, "my_enforcer") do
+      [{enforcer_pid, _}] ->
+        Ecto.Adapters.SQL.Sandbox.allow(MyApp.Repo, self(), enforcer_pid)
+      [] ->
+        :ok
+    end
+
+    :ok
+  end
+
+  test "create role with permissions in transaction" do
+    permissions = [
+      %{resource: "orgs", action: "read"},
+      %{resource: "users", action: "read"}
+    ]
+
+    # This now works because of shared mode
+    assert {:ok, :created} = 
+      MyApp.Repo.transaction(fn ->
+        Enum.each(permissions, fn %{resource: resource, action: action} ->
+          case Acx.EnforcerServer.add_policy("my_enforcer", {:p, ["analyst", resource, action]}) do
+            :ok -> :ok
+            {:error, reason} -> MyApp.Repo.rollback(reason)
+          end
+        end)
+
+        {:ok, :created}
+      end)
+  end
+end
+```
+
+## Important Notes
+
+### About Shared Mode
+
+- **Shared mode** allows multiple processes to access the same database connection
+- All tests in a module using shared mode will share the connection
+- This may reduce test isolation compared to the default checkout mode
+- It's still safe because each test gets a clean transaction that's rolled back
+
+### When to Use Shared Mode
+
+Use shared mode when you need to:
+- Wrap Casbin operations in application-level transactions
+- Test rollback behavior with Casbin
+- Test atomic operations that involve both Casbin and other database changes
+
+### Alternative Approaches
+
+If you don't need transactions in your tests, you can:
+
+1. **Avoid wrapping in transactions**: Call Casbin operations directly without `Repo.transaction`
+2. **Test transactions separately**: Test transaction logic separately from Casbin operations
+3. **Use async: false**: Use `async: false` to run tests serially with shared connections
+
+## Example Test Module
+
+Here's a complete example of a test module using shared mode:
+
+```elixir
+defmodule MyApp.Authorization.RolesTest do
+  use MyApp.DataCase, async: false  # async: false for shared mode
+
+  alias MyApp.Repo
+  alias MyApp.Authorization.Roles
+
+  setup do
+    # Set up sandbox in shared mode
+    :ok = Ecto.Adapters.SQL.Sandbox.checkout(Repo)
+    Ecto.Adapters.SQL.Sandbox.mode(Repo, {:shared, self()})
+
+    # Allow EnforcerServer to access the connection
+    case Registry.lookup(Acx.EnforcerRegistry, "my_enforcer") do
+      [{pid, _}] -> Ecto.Adapters.SQL.Sandbox.allow(Repo, self(), pid)
+      [] -> :ok
+    end
+
+    :ok
+  end
+
+  test "creates role with permissions atomically" do
+    permissions = [
+      %{resource: "users", action: "read"},
+      %{resource: "users", action: "write"}
+    ]
+
+    # This transaction includes Casbin operations
+    result = Repo.transaction(fn ->
+      # Create database record
+      {:ok, role} = Roles.create_role_record("admin")
+
+      # Add Casbin policies
+      Enum.each(permissions, fn perm ->
+        case Acx.EnforcerServer.add_policy(
+          "my_enforcer",
+          {:p, ["admin", perm.resource, perm.action]}
+        ) do
+          :ok -> :ok
+          {:error, reason} -> Repo.rollback(reason)
+        end
+      end)
+
+      {:ok, role}
+    end)
+
+    assert {:ok, _role} = result
+  end
+
+  test "rolls back Casbin operations on error" do
+    result = Repo.transaction(fn ->
+      # Add a policy
+      :ok = Acx.EnforcerServer.add_policy(
+        "my_enforcer",
+        {:p, ["temp_role", "resource", "action"]}
+      )
+
+      # Simulate an error
+      Repo.rollback(:simulated_error)
+    end)
+
+    assert {:error, :simulated_error} = result
+
+    # Verify the policy was not persisted
+    policies = Acx.EnforcerServer.list_policies("my_enforcer", %{sub: "temp_role"})
+    assert policies == []
+  end
+end
+```
+
+## Further Reading
+
+- [Ecto.Adapters.SQL.Sandbox Documentation](https://hexdocs.pm/ecto_sql/Ecto.Adapters.SQL.Sandbox.html)
+- [Ecto.Adapters.SQL.Sandbox Shared Mode](https://hexdocs.pm/ecto_sql/Ecto.Adapters.SQL.Sandbox.html#module-shared-mode)
+- [Testing with Ecto](https://hexdocs.pm/ecto/testing-with-ecto.html)

lib/acx/persist/ecto_adapter.ex

@@ -2,11 +2,45 @@ defmodule Acx.Persist.EctoAdapter do
   @moduledoc """
   This module defines an adapter for persisting the list of policies
   to a database.
+
+  ## Ecto.Adapters.SQL.Sandbox Compatibility
+
+  When using this adapter with `Ecto.Adapters.SQL.Sandbox` in tests, especially
+  with nested transactions, you need to ensure proper connection handling.
+
+  ### Recommended: Use Shared Mode
+
+  In your test setup, use shared mode for tests that wrap Casbin operations in transactions:
+
+      setup do
+        :ok = Ecto.Adapters.SQL.Sandbox.checkout(MyApp.Repo)
+        Ecto.Adapters.SQL.Sandbox.mode(MyApp.Repo, {:shared, self()})
+        :ok
+      end
+
+  This allows the EnforcerServer process to access the database connection
+  during transactions. Note that this means all tests in the module will
+  share the same connection, which may affect test isolation.
+
+  ### Alternative: Avoid Transactions in Tests
+
+  If you need better test isolation, consider structuring your tests to avoid
+  wrapping Casbin operations in explicit transactions, or handle rollback differently.
+
+  ### Advanced: Dynamic Repo (Limited Use)
+
+  For advanced use cases, you can configure the adapter with a function that
+  returns the repo, though this alone doesn't solve the transaction isolation issue:
+
+      # In your application setup
+      adapter = EctoAdapter.new(fn -> MyApp.Repo end)
+
+  See `Ecto.Adapters.SQL.Sandbox` documentation for more details on connection handling.
   """
   import Ecto.Changeset
   use Ecto.Schema
 
-  defstruct repo: nil
+  defstruct repo: nil, get_dynamic_repo: nil
 
   defmodule CasbinRule do
     @moduledoc """
@@ -109,8 +143,38 @@ defmodule Acx.Persist.EctoAdapter do
     end
   end
 
-  def new(repo) do
-    %__MODULE__{repo: repo}
+  @doc """
+  Creates a new EctoAdapter with the given repo.
+
+  ## Parameters
+  - `repo`: An Ecto.Repo module or a function that returns one.
+
+  ## Examples
+      # Static repo (standard usage)
+      adapter = EctoAdapter.new(MyApp.Repo)
+
+      # Dynamic repo (for Sandbox testing with transactions)
+      adapter = EctoAdapter.new(fn -> Ecto.Repo.get_dynamic_repo() || MyApp.Repo end)
+  """
+  def new(repo) when is_atom(repo) do
+    %__MODULE__{repo: repo, get_dynamic_repo: nil}
+  end
+
+  def new(repo_fn) when is_function(repo_fn, 0) do
+    %__MODULE__{repo: nil, get_dynamic_repo: repo_fn}
+  end
+
+  @doc """
+  Gets the repo to use for the current operation.
+  If get_dynamic_repo is set, calls it to get the dynamic repo.
+  Otherwise returns the static repo.
+  """
+  def get_repo(%__MODULE__{get_dynamic_repo: get_fn}) when is_function(get_fn, 0) do
+    get_fn.()
+  end
+
+  def get_repo(%__MODULE__{repo: repo}) when is_atom(repo) do
+    repo
   end
 
   defimpl Acx.Persist.PersistAdapter, for: Acx.Persist.EctoAdapter do
@@ -124,13 +188,15 @@ defmodule Acx.Persist.EctoAdapter do
         ...> {:error, "repo is not set"}
     """
     @spec load_policies(EctoAdapter.t()) :: [Model.Policy.t()]
-    def load_policies(%Acx.Persist.EctoAdapter{repo: nil}) do
+    def load_policies(%Acx.Persist.EctoAdapter{repo: nil, get_dynamic_repo: nil}) do
       {:error, "repo is not set"}
     end
 
     def load_policies(adapter) do
+      repo = Acx.Persist.EctoAdapter.get_repo(adapter)
+
       policies =
-        adapter.repo.all(CasbinRule)
+        repo.all(CasbinRule)
         |> Enum.map(&CasbinRule.changeset_to_list(&1))
 
       {:ok, policies}
@@ -156,15 +222,16 @@ defmodule Acx.Persist.EctoAdapter do
         ...> {:error, "repo is not set"}
     """
     @spec load_filtered_policy(EctoAdapter.t(), map()) :: {:ok, [list()]} | {:error, String.t()}
-    def load_filtered_policy(%Acx.Persist.EctoAdapter{repo: nil}, _filter) do
+    def load_filtered_policy(%Acx.Persist.EctoAdapter{repo: nil, get_dynamic_repo: nil}, _filter) do
       {:error, "repo is not set"}
     end
 
     def load_filtered_policy(adapter, filter) when is_map(filter) do
+      repo = Acx.Persist.EctoAdapter.get_repo(adapter)
       query = build_filtered_query(filter)
 
       policies =
-        adapter.repo.all(query)
+        repo.all(query)
         |> Enum.map(&CasbinRule.changeset_to_list(&1))
 
       {:ok, policies}
@@ -224,14 +291,12 @@ defmodule Acx.Persist.EctoAdapter do
         ...>    {:p, ["user", "file", "read"]})
         ...> {:error, "repo is not set"}
     """
-    def add_policy(%Acx.Persist.EctoAdapter{repo: nil}, _) do
+    def add_policy(%Acx.Persist.EctoAdapter{repo: nil, get_dynamic_repo: nil}, _) do
       {:error, "repo is not set"}
     end
 
-    def add_policy(
-          %Acx.Persist.EctoAdapter{repo: repo} = adapter,
-          {_key, _attrs} = policy
-        ) do
+    def add_policy(adapter, {_key, _attrs} = policy) do
+      repo = Acx.Persist.EctoAdapter.get_repo(adapter)
       changeset = CasbinRule.create_changeset(policy)
 
       case repo.insert(changeset) do
@@ -254,14 +319,12 @@ defmodule Acx.Persist.EctoAdapter do
         ...>    {:p, ["user", "file", "read"]})
         ...> {:error, "repo is not set"}
     """
-    def remove_policy(%Acx.Persist.EctoAdapter{repo: nil}, _) do
+    def remove_policy(%Acx.Persist.EctoAdapter{repo: nil, get_dynamic_repo: nil}, _) do
       {:error, "repo is not set"}
     end
 
-    def remove_policy(
-          %Acx.Persist.EctoAdapter{repo: repo} = adapter,
-          {_key, _attr} = policy
-        ) do
+    def remove_policy(adapter, {_key, _attr} = policy) do
+      repo = Acx.Persist.EctoAdapter.get_repo(adapter)
       f = CasbinRule.changeset_to_queryable(policy)
 
       case repo.delete_all(f) do
@@ -270,12 +333,8 @@ defmodule Acx.Persist.EctoAdapter do
       end
     end
 
-    def remove_filtered_policy(
-          %Acx.Persist.EctoAdapter{repo: repo} = adapter,
-          key,
-          idx,
-          attrs
-        ) do
+    def remove_filtered_policy(adapter, key, idx, attrs) do
+      repo = Acx.Persist.EctoAdapter.get_repo(adapter)
       f = CasbinRule.changeset_to_queryable({key, attrs}, idx)
 
       case repo.delete_all(f) do
@@ -296,14 +355,12 @@ defmodule Acx.Persist.EctoAdapter do
         ...>    [])
         ...> {:error, "repo is not set"}
     """
-    def save_policies(%Acx.Persist.EctoAdapter{repo: nil}, _) do
+    def save_policies(%Acx.Persist.EctoAdapter{repo: nil, get_dynamic_repo: nil}, _) do
       {:error, "repo is not set"}
     end
 
-    def save_policies(
-          %Acx.Persist.EctoAdapter{repo: repo} = adapter,
-          policies
-        ) do
+    def save_policies(adapter, policies) do
+      repo = Acx.Persist.EctoAdapter.get_repo(adapter)
       repo.transaction(fn -> insert_policies(repo, adapter, policies) end)
     end