You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@casbin.apache.org by hs...@apache.org on 2026/08/06 16:18:34 UTC
(casbin-ex) 01/02: feat: add EnforcerSupervisor.stop_enforcer to isolate enforcers in async tests (#31)
This is an automated email from the ASF dual-hosted git repository.
hsluoyz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/casbin-ex.git
commit 8ce8df347e7ffa8302188684c8a957a7f609cecb
Author: Yang Luo <hs...@gmail.com>
AuthorDate: Thu Aug 6 23:59:21 2026 +0800
feat: add EnforcerSupervisor.stop_enforcer to isolate enforcers in async tests (#31)
---
README.md | 16 ++++++
guides/test_isolation.md | 86 ++++++++++++++++++++++++++++
lib/casbin/enforcer_server.ex | 45 +++++++++++++++
lib/casbin/enforcer_supervisor.ex | 40 ++++++++++++-
test/enforcer_server_test.exs | 100 +++++++++++++++++++++++++++++++++
test/persist/ecto_server_load_test.exs | 26 ++++++---
6 files changed, 303 insertions(+), 10 deletions(-)
diff --git a/README.md b/README.md
index 1622a47..a5a24cc 100644
--- a/README.md
+++ b/README.md
@@ -458,6 +458,22 @@ Key topics covered:
## Testing
+### Isolating enforcers in async tests
+
+An enforcer name is a global handle: every test using the same name talks to the
+same process and the same policies, so `async: true` tests interfere with each
+other. Give each test its own enforcer name and stop it when the test is over —
+see [Isolating enforcers in tests](guides/test_isolation.md).
+
+```elixir
+setup do
+ ename = "acl_test_#{:erlang.unique_integer([:positive])}"
+ {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile)
+ on_exit(fn -> EnforcerSupervisor.stop_enforcer(ename) end)
+ {:ok, ename: ename}
+end
+```
+
### 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).
diff --git a/guides/test_isolation.md b/guides/test_isolation.md
new file mode 100644
index 0000000..b6a3797
--- /dev/null
+++ b/guides/test_isolation.md
@@ -0,0 +1,86 @@
+# Isolating enforcers in tests (`async: true`)
+
+An enforcer is a named, long-lived process. Every caller that passes the same
+name talks to the same process and therefore to the same set of policies:
+
+```elixir
+EnforcerServer.add_policy("my_enforcer", {:p, ["alice", "blog_post", "read"]})
+```
+
+That is what you want in production, where one enforcer serves the whole
+application. It is not what you want in a test suite: with `async: true`,
+ExUnit runs test modules concurrently, so one test adding policies to
+`"my_enforcer"` and another test removing them from `"my_enforcer"` will step
+on each other. Typical symptoms are policies disappearing mid-test,
+`list_policies/2` returning `[]` right after a successful `add_policy/2`, and
+tests that pass on their own but fail when the suite runs.
+
+The enforcer name is the isolation boundary. Give each test its own name and
+the tests stop interfering, no matter how many run at once.
+
+## The pattern
+
+```elixir
+defmodule MyApp.AclTest do
+ use ExUnit.Case, async: true
+
+ alias Casbin.{EnforcerServer, EnforcerSupervisor}
+
+ @cfile "priv/casbin/model.conf"
+
+ setup do
+ # Unique per test, so concurrent tests never share an enforcer.
+ ename = "acl_test_#{:erlang.unique_integer([:positive])}"
+
+ {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile)
+ on_exit(fn -> EnforcerSupervisor.stop_enforcer(ename) end)
+
+ {:ok, ename: ename}
+ end
+
+ test "admin can read", %{ename: ename} do
+ :ok = EnforcerServer.add_policy(ename, {:p, ["admin", "blog_post", "read"]})
+ assert EnforcerServer.allow?(ename, ["admin", "blog_post", "read"])
+ end
+end
+```
+
+## Why `stop_enforcer/1` matters
+
+Enforcer state is cached in an ETS table so that a *crashed* enforcer is
+restarted with the policies it had before the crash. The cache is keyed by
+enforcer name and outlives the process.
+
+`EnforcerSupervisor.stop_enforcer/1` shuts the process down and drops that
+cached state, which gives you two things:
+
+- **No leak.** Without it, every unique name you use adds a process and an ETS
+ entry that live until the VM stops. Over a large suite that adds up.
+- **A clean slate.** Starting an enforcer under a name that was stopped builds
+ it fresh from its configuration file. Without the stop, a new enforcer under
+ a previously used name silently inherits the old policies and ignores the
+ config file you passed.
+
+It is safe to call whether or not an enforcer is running, so it needs no guard
+in `on_exit/1`.
+
+## If you must share one enforcer
+
+Sometimes the enforcer is started once by your application supervision tree and
+the name is baked into the code under test. In that case the tests touching it
+cannot be async:
+
+```elixir
+use ExUnit.Case, async: false
+```
+
+`async: false` only serializes against *other* modules that are also
+`async: false`, so it is a real constraint, not a formality: a shared enforcer
+means the whole suite must agree not to run those tests concurrently. Prefer
+making the enforcer name a parameter of the code under test so each test can
+supply its own.
+
+## Related
+
+- [Testing with Ecto.Adapters.SQL.Sandbox and Transactions](sandbox_testing.md) —
+ isolating the *database* connection an enforcer uses.
diff --git a/lib/casbin/enforcer_server.ex b/lib/casbin/enforcer_server.ex
index 6073199..07b778a 100644
--- a/lib/casbin/enforcer_server.ex
+++ b/lib/casbin/enforcer_server.ex
@@ -239,11 +239,33 @@ defmodule Casbin.EnforcerServer do
GenServer.call(via_tuple(ename), {:set_persist_adapter, adapter})
end
+ @doc """
+ Returns the pid of the enforcer process registered under `ename`, or
+ `nil` when no enforcer is running under that name.
+ """
+ def whereis(ename) do
+ # The registry unregisters a dead process asynchronously, so a lookup
+ # right after a shutdown can still return the pid that just went away.
+ case Registry.lookup(Casbin.EnforcerRegistry, ename) do
+ [{pid, _}] -> if Process.alive?(pid), do: pid, else: nil
+ [] -> nil
+ end
+ end
+
+ @doc """
+ Returns `true` if an enforcer process is running under `ename`.
+ """
+ def running?(ename), do: whereis(ename) !== nil
+
#
# Server Callbacks
#
def init({ename, cfile}) do
+ # Needed so `terminate/2` runs on a supervised shutdown and can drop
+ # the cached state of this enforcer.
+ Process.flag(:trap_exit, true)
+
case create_new_or_lookup_enforcer(ename, cfile) do
{:error, reason} ->
{:stop, reason}
@@ -254,6 +276,21 @@ defmodule Casbin.EnforcerServer do
end
end
+ # The ets cache exists so a crashed enforcer is restarted with the state
+ # it had before the crash. A deliberate shutdown is not a crash: dropping
+ # the entry keeps the state from leaking into the next enforcer that is
+ # started under the same name.
+ def terminate(reason, _enforcer) do
+ if graceful_shutdown?(reason) do
+ case self_name() do
+ nil -> :ok
+ ename -> :ets.delete(:enforcers_table, ename)
+ end
+ end
+
+ :ok
+ end
+
def handle_call({:allow?, req}, _from, enforcer) do
allowed = enforcer |> Enforcer.allow?(req)
{:reply, allowed, enforcer}
@@ -314,6 +351,7 @@ defmodule Casbin.EnforcerServer do
def handle_call({:save_policies}, _from, enforcer) do
new_enforcer = enforcer |> Enforcer.save_policies()
+ :ets.insert(:enforcers_table, {self_name(), new_enforcer})
{:reply, :ok, new_enforcer}
end
@@ -408,6 +446,13 @@ defmodule Casbin.EnforcerServer do
Registry.keys(Casbin.EnforcerRegistry, self()) |> List.first()
end
+ # Returns `true` when the process was stopped on purpose rather than
+ # having crashed.
+ defp graceful_shutdown?(:normal), do: true
+ defp graceful_shutdown?(:shutdown), do: true
+ defp graceful_shutdown?({:shutdown, _reason}), do: true
+ defp graceful_shutdown?(_reason), do: false
+
# Creates a new enforcer or lookups existing one in the ets table.
defp create_new_or_lookup_enforcer(ename, cfile) do
case :ets.lookup(:enforcers_table, ename) do
diff --git a/lib/casbin/enforcer_supervisor.ex b/lib/casbin/enforcer_supervisor.ex
index 2670452..5212782 100644
--- a/lib/casbin/enforcer_supervisor.ex
+++ b/lib/casbin/enforcer_supervisor.ex
@@ -14,15 +14,51 @@ defmodule Casbin.EnforcerSupervisor do
end
@doc """
- Starts a new `Enforcer` process and supervises it
+ Starts a new `Enforcer` process and supervises it.
+
+ A crashed enforcer is restarted with the state it had before the crash.
+ An enforcer stopped with `stop_enforcer/1` is not restarted.
"""
def start_enforcer(ename, cfile) do
child_spec = %{
id: Casbin.EnforcerServer,
start: {Casbin.EnforcerServer, :start_link, [ename, cfile]},
- restart: :permanent
+ restart: :transient
}
DynamicSupervisor.start_child(__MODULE__, child_spec)
end
+
+ @doc """
+ Stops the enforcer registered under `ename` and drops its state.
+
+ Starting another enforcer under the same name afterwards builds a fresh
+ one from its configuration file, which makes it safe to give every test
+ (or tenant, or request) its own enforcer name and tear it down when done.
+
+ Always returns `:ok`, whether or not an enforcer was running, so it can
+ be called from an `on_exit/1` callback without a guard.
+ """
+ def stop_enforcer(ename) do
+ case Casbin.EnforcerServer.whereis(ename) do
+ nil -> :ok
+ pid -> DynamicSupervisor.terminate_child(__MODULE__, pid)
+ end
+
+ # `terminate/2` already drops the entry on a clean shutdown; doing it here
+ # too covers the enforcer that never got there (killed on a shutdown
+ # timeout) and the one that was gone before this call.
+ :ets.delete(:enforcers_table, ename)
+ :ok
+ end
+
+ @doc """
+ Returns the names of all running enforcers.
+ """
+ def list_enforcers do
+ Casbin.EnforcerRegistry
+ |> Registry.select([{{:"$1", :"$2", :_}, [], [{{:"$1", :"$2"}}]}])
+ |> Enum.filter(fn {_ename, pid} -> Process.alive?(pid) end)
+ |> Enum.map(fn {ename, _pid} -> ename end)
+ end
end
diff --git a/test/enforcer_server_test.exs b/test/enforcer_server_test.exs
new file mode 100644
index 0000000..284702c
--- /dev/null
+++ b/test/enforcer_server_test.exs
@@ -0,0 +1,100 @@
+defmodule Casbin.EnforcerServerTest do
+ use ExUnit.Case, async: true
+
+ alias Casbin.{EnforcerServer, EnforcerSupervisor}
+
+ @cfile "data/acl.conf" |> Path.expand(__DIR__)
+ @pfile "data/acl.csv" |> Path.expand(__DIR__)
+
+ # Every test gets its own enforcer name, so tests never share state.
+ setup do
+ ename = "enforcer_server_test_#{:erlang.unique_integer([:positive])}"
+ on_exit(fn -> EnforcerSupervisor.stop_enforcer(ename) end)
+ {:ok, ename: ename}
+ end
+
+ describe "stop_enforcer/1" do
+ test "stops the running enforcer", %{ename: ename} do
+ {:ok, pid} = EnforcerSupervisor.start_enforcer(ename, @cfile)
+ ref = Process.monitor(pid)
+
+ assert EnforcerServer.running?(ename)
+ assert :ok === EnforcerSupervisor.stop_enforcer(ename)
+
+ assert_receive {:DOWN, ^ref, :process, ^pid, _reason}
+ refute EnforcerServer.running?(ename)
+ end
+
+ test "is not restarted by the supervisor", %{ename: ename} do
+ {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile)
+ :ok = EnforcerSupervisor.stop_enforcer(ename)
+
+ refute ename in EnforcerSupervisor.list_enforcers()
+ end
+
+ test "drops the stored state", %{ename: ename} do
+ {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile)
+ :ok = EnforcerServer.load_policies(ename, @pfile)
+ assert EnforcerServer.allow?(ename, ["alice", "blog_post", "create"]) === true
+
+ :ok = EnforcerSupervisor.stop_enforcer(ename)
+
+ # An enforcer started under the same name is built from `@cfile`
+ # again instead of inheriting the policies of the previous one.
+ {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile)
+ assert EnforcerServer.list_policies(ename, %{}) === []
+ assert EnforcerServer.allow?(ename, ["alice", "blog_post", "create"]) === false
+ end
+
+ test "is a no-op when no enforcer is running", %{ename: ename} do
+ assert :ok === EnforcerSupervisor.stop_enforcer(ename)
+ assert :ok === EnforcerSupervisor.stop_enforcer(ename)
+ end
+ end
+
+ describe "crash recovery" do
+ test "a restarted enforcer keeps the policies it had", %{ename: ename} do
+ {:ok, pid} = EnforcerSupervisor.start_enforcer(ename, @cfile)
+ :ok = EnforcerServer.add_policy(ename, {:p, ["alice", "blog_post", "read"]})
+
+ ref = Process.monitor(pid)
+ Process.exit(pid, :kill)
+ assert_receive {:DOWN, ^ref, :process, ^pid, :killed}
+
+ wait_until_restarted(ename, pid)
+ assert EnforcerServer.allow?(ename, ["alice", "blog_post", "read"]) === true
+ end
+ end
+
+ describe "isolation between enforcers" do
+ test "policies added to one enforcer are invisible to another", %{ename: ename} do
+ other = ename <> "_other"
+ on_exit(fn -> EnforcerSupervisor.stop_enforcer(other) end)
+
+ {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile)
+ {:ok, _pid} = EnforcerSupervisor.start_enforcer(other, @cfile)
+
+ :ok = EnforcerServer.add_policy(ename, {:p, ["alice", "blog_post", "read"]})
+
+ assert EnforcerServer.allow?(ename, ["alice", "blog_post", "read"]) === true
+ assert EnforcerServer.allow?(other, ["alice", "blog_post", "read"]) === false
+ end
+ end
+
+ # The registry drops a dead enforcer asynchronously, so waiting for *a*
+ # process under `ename` can still hand back the pid that was just killed.
+ # Wait for the replacement instead.
+ defp wait_until_restarted(ename, old_pid, attempts \\ 50) do
+ case EnforcerServer.whereis(ename) do
+ pid when is_pid(pid) and pid !== old_pid ->
+ :ok
+
+ _ when attempts === 0 ->
+ flunk("enforcer '#{ename}' was not restarted")
+
+ _ ->
+ Process.sleep(10)
+ wait_until_restarted(ename, old_pid, attempts - 1)
+ end
+ end
+end
diff --git a/test/persist/ecto_server_load_test.exs b/test/persist/ecto_server_load_test.exs
index 22c3cb4..48c15b1 100644
--- a/test/persist/ecto_server_load_test.exs
+++ b/test/persist/ecto_server_load_test.exs
@@ -17,14 +17,15 @@ defmodule Casbin.Persist.EctoServerLoadTest do
@repo_rbac MockRbacRepo
setup do
- # Ensure clean state
- :ets.delete_all_objects(:enforcers_table)
+ # Every test below starts enforcers under unique names, so there is no
+ # shared state to clear here. Wiping `:enforcers_table` would take out
+ # the enforcers of the test modules running alongside this one.
:ok
end
describe "load_policies/1 with ACL model" do
test "loads policies from EctoAdapter on startup" do
- ename = "test_acl_load_#{:erlang.unique_integer([:positive])}"
+ ename = unique_ename("test_acl_load")
# Start enforcer
{:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile_acl)
@@ -46,7 +47,7 @@ defmodule Casbin.Persist.EctoServerLoadTest do
end
test "returns error when no adapter is set" do
- ename = "test_acl_no_adapter_#{:erlang.unique_integer([:positive])}"
+ ename = unique_ename("test_acl_no_adapter")
# Start enforcer without setting adapter
{:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile_acl)
@@ -58,7 +59,7 @@ defmodule Casbin.Persist.EctoServerLoadTest do
describe "load_policies/1 and load_mapping_policies/1 with RBAC model" do
test "loads both policies and mapping policies from EctoAdapter" do
- ename = "test_rbac_load_#{:erlang.unique_integer([:positive])}"
+ ename = unique_ename("test_rbac_load")
# Start enforcer
{:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile_rbac)
@@ -92,7 +93,7 @@ defmodule Casbin.Persist.EctoServerLoadTest do
describe "backward compatibility" do
test "load_policies/2 with file path still works" do
- ename = "test_file_load_#{:erlang.unique_integer([:positive])}"
+ ename = unique_ename("test_file_load")
pfile = "../data/acl.csv" |> Path.expand(__DIR__)
# Start enforcer
@@ -107,7 +108,7 @@ defmodule Casbin.Persist.EctoServerLoadTest do
end
test "load_mapping_policies/2 with file path still works" do
- ename = "test_file_mapping_load_#{:erlang.unique_integer([:positive])}"
+ ename = unique_ename("test_file_mapping_load")
pfile = "../data/rbac.csv" |> Path.expand(__DIR__)
# Start enforcer
@@ -124,7 +125,7 @@ defmodule Casbin.Persist.EctoServerLoadTest do
describe "persistence workflow" do
test "complete workflow: set adapter, load, modify, verify" do
- ename = "test_workflow_#{:erlang.unique_integer([:positive])}"
+ ename = unique_ename("test_workflow")
# Start enforcer
{:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile_acl)
@@ -150,4 +151,13 @@ defmodule Casbin.Persist.EctoServerLoadTest do
assert length(policies) === 1
end
end
+
+ # Returns an enforcer name unique to the running test and stops that
+ # enforcer once the test is over, so tests stay isolated from each other
+ # and enforcers do not pile up while the suite runs.
+ defp unique_ename(prefix) do
+ ename = "#{prefix}_#{:erlang.unique_integer([:positive])}"
+ on_exit(fn -> EnforcerSupervisor.stop_enforcer(ename) end)
+ ename
+ end
end
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@casbin.apache.org
For additional commands, e-mail: commits-help@casbin.apache.org