Skip to content

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

Merged
hsluoyz merged 5 commits into
masterfrom
copilot/fix-ecto-adapter-transaction-issue
Nov 9, 2025
Merged

feat: fix EctoAdapter transaction compatibility with Ecto.Adapters.SQL.Sandbox#44
hsluoyz merged 5 commits into
masterfrom
copilot/fix-ecto-adapter-transaction-issue

Conversation

Copilot AI commented Nov 8, 2025

Copy link
Copy Markdown
Contributor

Fix EctoAdapter to work with Ecto.Adapters.SQL.Sandbox in nested transactions

Problem Summary

The EctoAdapter fails when used within Repo.transaction blocks in tests using SQL.Sandbox because:

  1. The EnforcerServer runs in a separate GenServer process
  2. Database connections are locked to the process that starts the transaction
  3. Even with Sandbox.allow/3, the EnforcerServer cannot access a connection locked by another process's transaction

Solution Implemented ✅

Code Changes

  • Enhanced EctoAdapter with dynamic repo support
  • Added get_dynamic_repo field to EctoAdapter struct
  • Implemented get_repo/1 helper for runtime repo determination
  • Updated all PersistAdapter protocol implementations to use get_repo/1
  • Fixed module references to use fully qualified name Acx.Persist.EctoAdapter.get_repo/1
  • Maintained full backward compatibility with existing code
  • Added comprehensive inline documentation
  • Fixed code formatting issues (removed trailing whitespace)

Documentation Created

  • guides/sandbox_testing.md - Complete guide covering problem explanation, solution, examples, and best practices
  • test/persist/ecto_sandbox_transaction_test.exs - Example test file showing proper setup patterns
  • Updated README.md - Added Testing section with link to guide
  • Updated EctoAdapter moduledoc - Clear Sandbox compatibility documentation

How Users Should Use This ✅

The recommended solution follows Ecto's official best practices:

setup do
  :ok = Ecto.Adapters.SQL.Sandbox.checkout(MyApp.Repo)
  # Enable shared mode (KEY FIX!)
  Ecto.Adapters.SQL.Sandbox.mode(MyApp.Repo, {:shared, self()})
  
  # Allow EnforcerServer process
  case Registry.lookup(Acx.EnforcerRegistry, "my_enforcer") do
    [{pid, _}] -> Ecto.Adapters.SQL.Sandbox.allow(MyApp.Repo, self(), pid)
    [] -> :ok
  end
  :ok
end

Fixes #43

Original prompt

This section details on the original issue you should resolve

<issue_title>EctoAdapter doesn't work with Ecto.Adapters.SQL.Sandbox in nested transactions</issue_title>
<issue_description>## Problem

The EctoAdapter fails when used within a Repo.transaction block in test environments using Ecto.Adapters.SQL.Sandbox. The EnforcerServer runs in a separate process and cannot access the database connection owned by the test process, even when using Sandbox.allow/3.

This is a specific case of the broader issue described in #31, where the problem manifests when Casbin operations are wrapped in application-level transactions.

Error Message

** (DBConnection.ConnectionError) could not checkout the connection owned by #PID<0.462.0>. 
When using the sandbox, connections are shared, so this may imply another process is using a connection. 
Reason: connection not available and request was dropped from queue after 973ms.

Reproduction Steps

1. Setup
# config/test.exs
config :my_app, MyApp.Repo,
  pool: Ecto.Adapters.SQL.Sandbox

# test/support/casbin_case.ex
defmodule MyApp.CasbinCase do
  use ExUnit.CaseTemplate

  setup do
    :ok = Ecto.Adapters.SQL.Sandbox.checkout(MyApp.Repo)
    
    # This doesn't work because EnforcerServer runs in separate process
    case Registry.lookup(Acx.EnforcerRegistry, "my_enforcer") do
      [{enforcer_pid, _}] ->
        Ecto.Adapters.SQL.Sandbox.allow(MyApp.Repo, self(), enforcer_pid)
      [] ->
        :ok
    end
    
    :ok
  end
end


2. Application Code (with transaction)
defmodule MyApp.Authorization.Roles do
  def create_role(role_name, domain, permissions) do
    # Wrapping in transaction for rollback safety
    Repo.transaction(fn ->
      Enum.each(permissions, fn %{resource: resource, action: action} ->
        case Policy.add(role_name, domain, resource, action) do
          :ok -> :ok
          {:error, reason} -> Repo.rollback(reason)
        end
      end)
      
      {:ok, :created}
    end)
  end
end


3. Test (fails)
defmodule MyApp.RolesTest do
  use MyApp.DataCase
  use MyApp.CasbinCase

  test "create role" do
    permissions = [
      %{resource: "orgs", action: "read"},
      %{resource: "users", action: "read"}
    ]

    # This fails with connection error
    assert {:ok, :created} = Roles.create_role("analyst", "*", permissions)
  end
end

When Repo.transaction is called in the test process, it acquires the sandbox connection. When the transaction then calls Policy.add(), which sends a message to the EnforcerServer (a separate process), that server tries to insert into the database using repo.insert(). Even though we've called Sandbox.allow(Repo, test_pid, enforcer_pid), the enforcer cannot access the connection because:

  1. The test process owns the connection in a transaction
  2. The transaction has locked the connection
  3. The EnforcerServer cannot access a connection locked by another process's transaction</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copilot AI and others added 2 commits November 8, 2025 17:51
Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
…tion usage

Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix EctoAdapter not working with Ecto.Adapters.SQL.Sandbox Fix EctoAdapter transaction compatibility with Ecto.Adapters.SQL.Sandbox Nov 8, 2025
Copilot AI requested a review from hsluoyz November 8, 2025 18:02
@hsluoyz
hsluoyz marked this pull request as ready for review November 8, 2025 18:10
@hsluoyz

hsluoyz commented Nov 9, 2025

Copy link
Copy Markdown
Member

@copilot Fix failed CI check:

1s
Run mix format --check-formatted
** (Mix) mix format failed due to --check-formatted.
The following files are not formatted:

  * lib/acx/persist/ecto_adapter.ex
  * test/persist/ecto_sandbox_transaction_test.exs

Error: Process completed with exit code 1.

Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>

Copilot AI commented Nov 9, 2025

Copy link
Copy Markdown
Contributor Author

@copilot Fix failed CI check:

...

Fixed in commit 552788d. Ran mix format on both files to remove trailing whitespace.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • esm.ubuntu.com
    • Triggering command: /usr/lib/apt/methods/https (dns block)
  • repo.hex.pm
    • Triggering command: /usr/lib/erlang/erts-13.2.2.5/bin/inet_gethost 4 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@hsluoyz

hsluoyz commented Nov 9, 2025

Copy link
Copy Markdown
Member

@copilot fix failed compile error:

Run mix compile --warnings-as-errors
Compiling 5 files (.ex)
warning: EctoAdapter.get_repo/1 is undefined (module EctoAdapter is not available or is yet to be defined)
Invalid call found at 6 locations:
  lib/acx/persist/ecto_adapter.ex:196: Acx.Persist.PersistAdapter.Acx.Persist.EctoAdapter.load_policies/1
  lib/acx/persist/ecto_adapter.ex:230: Acx.Persist.PersistAdapter.Acx.Persist.EctoAdapter.load_filtered_policy/2
  lib/acx/persist/ecto_adapter.ex:299: Acx.Persist.PersistAdapter.Acx.Persist.EctoAdapter.add_policy/2
  lib/acx/persist/ecto_adapter.ex:327: Acx.Persist.PersistAdapter.Acx.Persist.EctoAdapter.remove_policy/2
  lib/acx/persist/ecto_adapter.ex:337: Acx.Persist.PersistAdapter.Acx.Persist.EctoAdapter.remove_filtered_policy/4
  lib/acx/persist/ecto_adapter.ex:363: Acx.Persist.PersistAdapter.Acx.Persist.EctoAdapter.save_policies/2

Compilation failed due to warnings while using the --warnings-as-errors option
Error: Process completed with exit code 1.

…name)

Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>

Copilot AI commented Nov 9, 2025

Copy link
Copy Markdown
Contributor Author

@copilot fix failed compile error:

...

Fixed in commit 7809be8. Changed all EctoAdapter.get_repo(adapter) calls within the protocol implementation to use the fully qualified module name Acx.Persist.EctoAdapter.get_repo(adapter) to resolve the compilation error.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • esm.ubuntu.com
    • Triggering command: /usr/lib/apt/methods/https (dns block)
  • repo.hex.pm
    • Triggering command: /usr/lib/erlang/erts-13.2.2.5/bin/inet_gethost 4 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@hsluoyz hsluoyz changed the title Fix EctoAdapter transaction compatibility with Ecto.Adapters.SQL.Sandbox feat: fix EctoAdapter transaction compatibility with Ecto.Adapters.SQL.Sandbox Nov 9, 2025
@hsluoyz
hsluoyz merged commit faac648 into master Nov 9, 2025
3 of 4 checks passed
github-actions Bot pushed a commit that referenced this pull request Nov 9, 2025
# [1.6.0](v1.5.0...v1.6.0) (2025-11-09)

### Features

* fix EctoAdapter transaction compatibility with Ecto.Adapters.SQL.Sandbox ([#44](#44)) ([faac648](faac648))
@github-actions

github-actions Bot commented Nov 9, 2025

Copy link
Copy Markdown

🎉 This PR is included in version 1.6.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

hsluoyz added a commit that referenced this pull request Nov 9, 2025
github-actions Bot pushed a commit that referenced this pull request Nov 9, 2025
# [1.6.0](v1.5.0...v1.6.0) (2025-11-09)

### Features

* fix EctoAdapter transaction compatibility with Ecto.Adapters.SQL.Sandbox ([#44](#44)) ([d827793](d827793))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EctoAdapter doesn't work with Ecto.Adapters.SQL.Sandbox in nested transactions

3 participants