From ffd3ffe01c6b6dda88832f8a96b3605c092107a7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 13:52:48 +0000 Subject: [PATCH 1/5] Initial plan From 460cec04de2c4df53376cf26d6b59998f7d002b9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 13:59:00 +0000 Subject: [PATCH 2/5] Add async testing support with TestHelper and AsyncCase modules Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com> --- README.md | 29 +++ guides/async_testing.md | 377 +++++++++++++++++++++++++++++++ lib/casbin/async_case.ex | 208 +++++++++++++++++ lib/casbin/test_helper.ex | 178 +++++++++++++++ test/casbin/async_case_test.exs | 179 +++++++++++++++ test/casbin/test_helper_test.exs | 187 +++++++++++++++ 6 files changed, 1158 insertions(+) create mode 100644 guides/async_testing.md create mode 100644 lib/casbin/async_case.ex create mode 100644 lib/casbin/test_helper.ex create mode 100644 test/casbin/async_case_test.exs create mode 100644 test/casbin/test_helper_test.exs diff --git a/README.md b/README.md index 1622a47..f79afdd 100644 --- a/README.md +++ b/README.md @@ -458,6 +458,35 @@ Key topics covered: ## Testing +### Async Testing with Isolated Enforcers + +When writing tests with `async: true`, you need to ensure each test has its own isolated enforcer to prevent race conditions. Casbin-Ex provides `Casbin.AsyncCase` for this purpose: + +```elixir +defmodule MyApp.PermissionsTest do + use Casbin.AsyncCase, async: true + + @config_file "path/to/model.conf" + + setup do + # Creates a unique enforcer for each test + {:ok, enforcer_name: start_test_enforcer(@config_file)} + end + + test "admin permissions", %{enforcer_name: ename} do + EnforcerServer.add_policy(ename, {:p, ["admin", "data", "read"]}) + assert EnforcerServer.allow?(ename, ["admin", "data", "read"]) + end +end +``` + +**Why this matters:** Using a fixed enforcer name (e.g., `"my_enforcer"`) causes all tests to share the same global state, leading to race conditions in async tests. See our [Async Testing Guide](guides/async_testing.md) for: + +- Complete examples and best practices +- Using `Casbin.TestHelper` for more control +- Troubleshooting common async test issues +- Migration guide from sync to async tests + ### 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/async_testing.md b/guides/async_testing.md new file mode 100644 index 0000000..d87a9fe --- /dev/null +++ b/guides/async_testing.md @@ -0,0 +1,377 @@ +# Testing with Async Mode + +This guide explains how to write Casbin tests that can safely run with `async: true`, preventing race conditions and test interference. + +## The Problem + +When using EnforcerServer with a fixed enforcer name, all tests share the same global state, causing race conditions in async tests: + +```elixir +# ❌ This fails with async: true +defmodule MyApp.AclTest do + use ExUnit.Case, async: true # Tests interfere with each other + + @enforcer_name "my_enforcer" + + setup do + # All tests use the same enforcer! + EnforcerServer.add_policy(@enforcer_name, {:p, ["admin", "data", "read"]}) + + on_exit(fn -> + # This cleanup affects ALL running tests + EnforcerServer.remove_policy(@enforcer_name, {:p, ["admin", "data", "read"]}) + end) + end + + test "admin has permissions" do + # Another test's cleanup might delete this policy mid-test! + assert EnforcerServer.allow?(@enforcer_name, ["admin", "data", "read"]) + end +end +``` + +**Symptoms:** +- `Policies.list()` returns `[]` even after adding policies +- `add_policy` returns `{:error, :already_existed}` but policies aren't in memory +- Tests pass individually but fail when run together +- Tests fail randomly depending on execution order + +## Solution 1: Use Casbin.AsyncCase (Recommended) + +The easiest solution is to use `Casbin.AsyncCase`, which automatically handles enforcer isolation: + +```elixir +# ✅ This works with async: true +defmodule MyApp.AclTest do + use Casbin.AsyncCase, async: true + + alias Casbin.EnforcerServer + + @config_file "path/to/model.conf" + + setup do + # Creates a unique enforcer for THIS test + enforcer_name = start_test_enforcer(@config_file) + + {:ok, enforcer_name: enforcer_name} + end + + test "admin has permissions", %{enforcer_name: ename} do + EnforcerServer.add_policy(ename, {:p, ["admin", "data", "read"]}) + assert EnforcerServer.allow?(ename, ["admin", "data", "read"]) + end + + test "user has limited permissions", %{enforcer_name: ename} do + # This test has its own isolated enforcer + EnforcerServer.add_policy(ename, {:p, ["user", "data", "read"]}) + assert EnforcerServer.allow?(ename, ["user", "data", "read"]) + assert EnforcerServer.allow?(ename, ["user", "data", "write"]) == false + end +end +``` + +### With Pre-loaded Policies + +```elixir +defmodule MyApp.PermissionsTest do + use Casbin.AsyncCase, async: true + + @config_file Application.app_dir(:my_app, "priv/casbin/model.conf") + @policy_file Application.app_dir(:my_app, "priv/casbin/policy.csv") + + setup do + ename = start_test_enforcer(@config_file) + + # Load pre-defined policies + :ok = EnforcerServer.load_policies(ename, @policy_file) + + {:ok, enforcer_name: ename} + end + + test "existing policies work", %{enforcer_name: ename} do + assert EnforcerServer.allow?(ename, ["alice", "blog_post", "read"]) + end + + test "can add new policies", %{enforcer_name: ename} do + :ok = EnforcerServer.add_policy(ename, {:p, ["bob", "data", "write"]}) + assert EnforcerServer.allow?(ename, ["bob", "data", "write"]) + end +end +``` + +## Solution 2: Use Casbin.TestHelper (More Control) + +For more control, use `Casbin.TestHelper` functions directly: + +```elixir +defmodule MyApp.CustomAclTest do + use ExUnit.Case, async: true + + import Casbin.TestHelper + alias Casbin.{EnforcerSupervisor, EnforcerServer} + + @config_file "path/to/model.conf" + + setup do + # Generate a unique enforcer name + ename = unique_enforcer_name("custom_acl") + + # Start the enforcer + {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @config_file) + + # Register cleanup + on_exit(fn -> cleanup_enforcer(ename) end) + + {:ok, enforcer_name: ename} + end + + test "can use custom enforcer", %{enforcer_name: ename} do + EnforcerServer.add_policy(ename, {:p, ["alice", "data", "read"]}) + assert EnforcerServer.allow?(ename, ["alice", "data", "read"]) + end +end +``` + +### Using create_test_enforcer + +For even less boilerplate: + +```elixir +defmodule MyApp.MinimalTest do + use ExUnit.Case, async: true + + import Casbin.TestHelper + alias Casbin.EnforcerServer + + test "quick test with enforcer" do + {:ok, ename} = create_test_enforcer("minimal", "path/to/model.conf") + + EnforcerServer.add_policy(ename, {:p, ["user", "resource", "action"]}) + assert EnforcerServer.allow?(ename, ["user", "resource", "action"]) + end +end +``` + +## Solution 3: Use async: false (Simplest, but Slower) + +If you don't need parallel execution, you can disable async mode: + +```elixir +# ✅ This works but runs slower +defmodule MyApp.AclTest do + use ExUnit.Case, async: false # Tests run sequentially + + @enforcer_name "my_enforcer" + + # Now safe to use a fixed enforcer name +end +``` + +**Trade-offs:** +- ✅ Simple - no code changes needed +- ❌ Slower - tests run sequentially instead of in parallel +- ❌ Doesn't scale - more tests = longer test suite runtime + +## Using with EctoAdapter and SQL.Sandbox + +When using `EctoAdapter` with `Ecto.Adapters.SQL.Sandbox`, you need to allow the enforcer process to access the database connection: + +```elixir +defmodule MyApp.DatabaseAclTest do + use MyApp.DataCase, async: true + use Casbin.AsyncCase + + alias Casbin.{EnforcerServer, Persist.EctoAdapter} + + @config_file Application.app_dir(:my_app, "priv/casbin/model.conf") + + setup do + # Check out a database connection + :ok = Ecto.Adapters.SQL.Sandbox.checkout(MyApp.Repo) + + # Start isolated enforcer + ename = start_test_enforcer(@config_file) + + # Allow enforcer process to access the database + case Registry.lookup(Casbin.EnforcerRegistry, ename) do + [{pid, _}] -> + Ecto.Adapters.SQL.Sandbox.allow(MyApp.Repo, self(), pid) + [] -> + :ok + end + + # Set up the adapter + adapter = EctoAdapter.new(MyApp.Repo) + :ok = EnforcerServer.set_persist_adapter(ename, adapter) + + {:ok, enforcer_name: ename} + end + + test "policies persist to database", %{enforcer_name: ename} do + :ok = EnforcerServer.add_policy(ename, {:p, ["user", "data", "read"]}) + + # Policies are automatically saved with EctoAdapter + assert EnforcerServer.allow?(ename, ["user", "data", "read"]) + + # Verify it was persisted + policies = EnforcerServer.list_policies(ename, %{sub: "user"}) + assert length(policies) == 1 + end +end +``` + +For transactional tests with Ecto, see the [Sandbox Testing Guide](sandbox_testing.md). + +## Best Practices + +### 1. Always Use Unique Enforcer Names + +```elixir +# ❌ Bad - shared state +@enforcer_name "my_enforcer" + +# ✅ Good - unique per test +ename = unique_enforcer_name("my_test") +``` + +### 2. Clean Up Enforcers + +```elixir +# Always register cleanup +on_exit(fn -> cleanup_enforcer(ename) end) + +# Or use AsyncCase/create_test_enforcer which handles this automatically +``` + +### 3. Use Module Attributes for Config Files + +```elixir +defmodule MyApp.AclTest do + use Casbin.AsyncCase, async: true + + # ✅ Good - easy to update + @config_file Application.app_dir(:my_app, "priv/casbin/model.conf") + @policy_file Application.app_dir(:my_app, "priv/casbin/policy.csv") + + setup do + ename = start_test_enforcer(@config_file) + :ok = EnforcerServer.load_policies(ename, @policy_file) + {:ok, enforcer_name: ename} + end +end +``` + +### 4. Organize Tests by Resource + +```elixir +# Organize by what you're testing +defmodule MyApp.BlogPermissionsTest do + use Casbin.AsyncCase, async: true + # Tests for blog post permissions +end + +defmodule MyApp.DataPermissionsTest do + use Casbin.AsyncCase, async: true + # Tests for data permissions +end +``` + +## Troubleshooting + +### "Policy not found" errors in async tests + +**Problem:** Policies disappear mid-test or `list_policies` returns `[]` + +**Solution:** Make sure each test uses a unique enforcer name: + +```elixir +# Use AsyncCase or unique_enforcer_name() +ename = unique_enforcer_name("my_test") +``` + +### "{:error, :already_existed}" but policy isn't there + +**Problem:** `add_policy` fails with `:already_existed` but the policy doesn't show in `list_policies` + +**Solution:** Another test is using the same enforcer. Use unique names: + +```elixir +# In setup +ename = start_test_enforcer(@config_file) # Unique per test +``` + +### Tests pass individually but fail together + +**Problem:** `mix test path/to/test.exs` passes but `mix test` fails + +**Solution:** Tests are sharing state. Use `async: true` with unique enforcers: + +```elixir +use Casbin.AsyncCase, async: true +``` + +### Database connection errors with EctoAdapter + +**Problem:** `DBConnection.ConnectionError` when using `EctoAdapter` + +**Solution:** Allow the enforcer process to access the connection: + +```elixir +setup do + :ok = Ecto.Adapters.SQL.Sandbox.checkout(MyApp.Repo) + ename = start_test_enforcer(@config_file) + + [{pid, _}] = Registry.lookup(Casbin.EnforcerRegistry, ename) + Ecto.Adapters.SQL.Sandbox.allow(MyApp.Repo, self(), pid) + + {:ok, enforcer_name: ename} +end +``` + +## Migration Guide + +If you have existing tests using a fixed enforcer name: + +### Before + +```elixir +defmodule MyApp.AclTest do + use ExUnit.Case # async: false by default + + @enforcer_name "my_enforcer" + + test "admin permissions" do + EnforcerServer.add_policy(@enforcer_name, {:p, ["admin", "data", "read"]}) + assert EnforcerServer.allow?(@enforcer_name, ["admin", "data", "read"]) + end +end +``` + +### After + +```elixir +defmodule MyApp.AclTest do + use Casbin.AsyncCase, async: true + + @config_file "path/to/model.conf" + + setup do + {:ok, enforcer_name: start_test_enforcer(@config_file)} + end + + test "admin permissions", %{enforcer_name: ename} do + EnforcerServer.add_policy(ename, {:p, ["admin", "data", "read"]}) + assert EnforcerServer.allow?(ename, ["admin", "data", "read"]) + end +end +``` + +## Summary + +- Use `Casbin.AsyncCase` for automatic enforcer isolation in async tests +- Use `Casbin.TestHelper` for more control over enforcer lifecycle +- Always use unique enforcer names to prevent race conditions +- Clean up enforcers in `on_exit` callbacks (or use AsyncCase/create_test_enforcer) +- Allow enforcer processes to access database connections when using EctoAdapter + +With these patterns, you can safely run Casbin tests in parallel with `async: true`, making your test suite faster and more reliable. diff --git a/lib/casbin/async_case.ex b/lib/casbin/async_case.ex new file mode 100644 index 0000000..3fd9d7a --- /dev/null +++ b/lib/casbin/async_case.ex @@ -0,0 +1,208 @@ +defmodule Casbin.AsyncCase do + @moduledoc """ + A test case template for async-safe Casbin tests. + + This module provides a convenient way to write tests that use Casbin enforcers + with `async: true`. It automatically handles enforcer isolation, preventing + race conditions that occur when multiple tests share the same enforcer instance. + + ## Problem + + When using EnforcerServer with a fixed name, all tests share the same global + state, making `async: true` tests fail with race conditions: + + ```elixir + # ❌ This fails with async: true + defmodule MyApp.PermissionsTest do + use ExUnit.Case, async: true + + test "test 1" do + EnforcerServer.add_policy("my_enforcer", {:p, ["user", "data", "read"]}) + assert EnforcerServer.allow?("my_enforcer", ["user", "data", "read"]) + end + + test "test 2" do + # Test 1's cleanup might delete policies while this test is running! + EnforcerServer.add_policy("my_enforcer", {:p, ["admin", "data", "write"]}) + assert EnforcerServer.allow?("my_enforcer", ["admin", "data", "write"]) + end + end + ``` + + ## Solution + + Use `Casbin.AsyncCase` to automatically create isolated enforcers per test: + + ```elixir + # ✅ This works with async: true + defmodule MyApp.PermissionsTest do + use Casbin.AsyncCase, async: true + + @config_file "path/to/casbin.conf" + + setup do + {:ok, enforcer_name: start_test_enforcer(@config_file)} + end + + test "test 1", %{enforcer_name: ename} do + EnforcerServer.add_policy(ename, {:p, ["user", "data", "read"]}) + assert EnforcerServer.allow?(ename, ["user", "data", "read"]) + end + + test "test 2", %{enforcer_name: ename} do + # This test has its own isolated enforcer! + EnforcerServer.add_policy(ename, {:p, ["admin", "data", "write"]}) + assert EnforcerServer.allow?(ename, ["admin", "data", "write"]) + end + end + ``` + + ## Using with EctoAdapter + + When using `EctoAdapter` with Ecto.SQL.Sandbox, you may need to allow the + enforcer process to access the database connection: + + ```elixir + defmodule MyApp.PermissionsTest do + use MyApp.DataCase, async: true + use Casbin.AsyncCase + + @config_file Application.app_dir(:my_app, "priv/casbin/model.conf") + + setup do + # Check out and allow enforcer to use the connection + :ok = Ecto.Adapters.SQL.Sandbox.checkout(MyApp.Repo) + + enforcer_name = start_test_enforcer(@config_file) + + # Allow enforcer process to access the database + case Registry.lookup(Casbin.EnforcerRegistry, enforcer_name) do + [{pid, _}] -> Ecto.Adapters.SQL.Sandbox.allow(MyApp.Repo, self(), pid) + [] -> :ok + end + + # Set up the adapter + adapter = Casbin.Persist.EctoAdapter.new(MyApp.Repo) + :ok = Casbin.EnforcerServer.set_persist_adapter(enforcer_name, adapter) + + {:ok, enforcer_name: enforcer_name} + end + + test "policies persist to database", %{enforcer_name: ename} do + :ok = EnforcerServer.add_policy(ename, {:p, ["user", "data", "read"]}) + :ok = EnforcerServer.save_policies(ename) + + # Verify it persisted + assert EnforcerServer.allow?(ename, ["user", "data", "read"]) + end + end + ``` + + ## Available Functions + + When you `use Casbin.AsyncCase`, the following functions become available: + + - `start_test_enforcer/1` - Starts an isolated enforcer for the current test + - `start_test_enforcer/2` - Starts an isolated enforcer with a custom prefix + - `unique_enforcer_name/1` - Generates a unique enforcer name + + These functions automatically handle cleanup via `on_exit/1`. + + ## Module Options + + You can pass options when using this module: + + ```elixir + use Casbin.AsyncCase, async: true, prefix: "my_test" + ``` + + Options: + - `async` - Passed to ExUnit.Case (default: true) + - `prefix` - Default prefix for enforcer names (default: module name) + + ## Implementation Details + + This module: + 1. Uses `ExUnit.Case` under the hood + 2. Imports `Casbin.TestHelper` for convenience functions + 3. Generates unique enforcer names using `:erlang.unique_integer/1` + 4. Automatically cleans up enforcers via `on_exit/1` callbacks + 5. Clears the ETS table state between tests + """ + + defmacro __using__(opts) do + # Extract async option (default to true for safety) + async = Keyword.get(opts, :async, true) + + # Extract prefix option (default to module name) + # Note: __CALLER__.module will be the test module using this macro + prefix = Keyword.get(opts, :prefix, nil) + + quote do + use ExUnit.Case, async: unquote(async) + import Casbin.TestHelper + + # Store the default prefix for this test module + @test_prefix unquote(prefix) || __MODULE__ |> to_string() |> String.replace("Elixir.", "") + + @doc """ + Starts a test enforcer with automatic isolation and cleanup. + + Creates a unique enforcer for the current test using the module's default + prefix and the provided config file. + + ## Parameters + + - `config_file` - Path to the Casbin configuration file + + ## Returns + + The unique enforcer name (string) that can be used with EnforcerServer functions. + + ## Examples + + setup do + {:ok, enforcer_name: start_test_enforcer("config/model.conf")} + end + + The enforcer is automatically cleaned up when the test exits. + """ + def start_test_enforcer(config_file) do + start_test_enforcer(@test_prefix, config_file) + end + + @doc """ + Starts a test enforcer with a custom prefix. + + Similar to `start_test_enforcer/1`, but allows specifying a custom prefix + for the enforcer name. + + ## Parameters + + - `prefix` - Custom prefix for the enforcer name + - `config_file` - Path to the Casbin configuration file + + ## Returns + + The unique enforcer name (string). + + ## Examples + + setup do + ename = start_test_enforcer("custom_prefix", "config/model.conf") + {:ok, enforcer_name: ename} + end + """ + def start_test_enforcer(prefix, config_file) do + ename = unique_enforcer_name(prefix) + + {:ok, _pid} = Casbin.EnforcerSupervisor.start_enforcer(ename, config_file) + + # Automatically clean up when test exits + on_exit(fn -> cleanup_enforcer(ename) end) + + ename + end + end + end +end diff --git a/lib/casbin/test_helper.ex b/lib/casbin/test_helper.ex new file mode 100644 index 0000000..af3db14 --- /dev/null +++ b/lib/casbin/test_helper.ex @@ -0,0 +1,178 @@ +defmodule Casbin.TestHelper do + @moduledoc """ + Helper utilities for testing with Casbin enforcers in async mode. + + This module provides functions to create isolated enforcers for each test, + preventing race conditions when running tests with `async: true`. + + ## Problem + + When using a fixed enforcer name (e.g., `"my_enforcer"`), all tests share + the same global state, causing race conditions in async tests: + + ```elixir + # This fails with async: true + defmodule MyApp.AclTest do + use ExUnit.Case, async: true # ❌ Tests interfere with each other + + test "admin has permissions" do + EnforcerServer.add_policy("my_enforcer", {:p, ["admin", "data", "read"]}) + # Another test's cleanup may delete this policy mid-test! + assert EnforcerServer.allow?("my_enforcer", ["admin", "data", "read"]) + end + end + ``` + + ## Solution + + Use `unique_enforcer_name/1` to generate a unique name for each test: + + ```elixir + defmodule MyApp.AclTest do + use ExUnit.Case, async: true # ✅ Tests are isolated + import Casbin.TestHelper + + setup do + ename = unique_enforcer_name("acl_test") + {:ok, _pid} = Casbin.EnforcerSupervisor.start_enforcer(ename, config_path) + + on_exit(fn -> cleanup_enforcer(ename) end) + + {:ok, enforcer_name: ename} + end + + test "admin has permissions", %{enforcer_name: ename} do + EnforcerServer.add_policy(ename, {:p, ["admin", "data", "read"]}) + assert EnforcerServer.allow?(ename, ["admin", "data", "read"]) + end + end + ``` + + ## Functions + + - `unique_enforcer_name/1` - Generates a unique enforcer name with optional prefix + - `cleanup_enforcer/1` - Cleans up an enforcer and its state + """ + + @doc """ + Generates a unique enforcer name for test isolation. + + The generated name includes a prefix (default: "test") and a unique integer, + ensuring no naming conflicts between parallel tests. + + ## Parameters + + - `prefix` - Optional string prefix for the enforcer name (default: "test") + + ## Examples + + iex> name1 = Casbin.TestHelper.unique_enforcer_name("acl") + iex> name2 = Casbin.TestHelper.unique_enforcer_name("acl") + iex> name1 != name2 + true + + iex> name = Casbin.TestHelper.unique_enforcer_name("my_module") + iex> String.contains?(name, "my_module") + true + + iex> name = Casbin.TestHelper.unique_enforcer_name() + iex> String.contains?(name, "test") + true + + ## Usage in Tests + + setup do + ename = unique_enforcer_name("user_permissions") + {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, config_file) + on_exit(fn -> cleanup_enforcer(ename) end) + {:ok, enforcer_name: ename} + end + """ + def unique_enforcer_name(prefix \\ "test") do + "#{prefix}_#{:erlang.unique_integer([:positive, :monotonic])}" + end + + @doc """ + Cleans up an enforcer and removes it from the ETS table. + + This function should be called in the test's `on_exit/1` callback to ensure + proper cleanup after each test. + + ## Parameters + + - `ename` - The enforcer name to clean up + + ## Examples + + on_exit(fn -> + Casbin.TestHelper.cleanup_enforcer(enforcer_name) + end) + + ## Implementation Notes + + This function: + 1. Removes the enforcer from the ETS table (`:enforcers_table`) + 2. Stops the enforcer process if it's still running + + Note: If the enforcer is supervised by `EnforcerSupervisor`, the supervisor + will automatically restart it. For tests, you typically don't need to worry + about this as the test process exits. + """ + def cleanup_enforcer(ename) do + # Remove from ETS table + :ets.delete(:enforcers_table, ename) + + # Try to stop the process if it's registered + case Registry.lookup(Casbin.EnforcerRegistry, ename) do + [{pid, _}] when is_pid(pid) -> + # Stop the process. We use :shutdown as the reason for a clean exit + # The supervisor won't restart it because it's a test scenario + if Process.alive?(pid) do + DynamicSupervisor.terminate_child(Casbin.EnforcerSupervisor, pid) + end + + [] -> + :ok + end + end + + @doc """ + Creates a test enforcer with a unique name and automatic cleanup. + + This is a convenience function that combines `unique_enforcer_name/1`, + enforcer startup, and automatic cleanup. + + ## Parameters + + - `prefix` - Optional string prefix for the enforcer name (default: "test") + - `config_file` - Path to the Casbin configuration file + + ## Returns + + `{:ok, enforcer_name}` on success, or `{:error, reason}` on failure. + + ## Examples + + test "admin permissions" do + {:ok, ename} = Casbin.TestHelper.create_test_enforcer("acl", config_path) + EnforcerServer.add_policy(ename, {:p, ["admin", "data", "read"]}) + assert EnforcerServer.allow?(ename, ["admin", "data", "read"]) + end + + Note: This function automatically registers cleanup in `on_exit/1`, so you + don't need to manually clean up the enforcer. + """ + def create_test_enforcer(prefix \\ "test", config_file) do + ename = unique_enforcer_name(prefix) + + case Casbin.EnforcerSupervisor.start_enforcer(ename, config_file) do + {:ok, _pid} -> + # Register cleanup + ExUnit.Callbacks.on_exit(fn -> cleanup_enforcer(ename) end) + {:ok, ename} + + {:error, reason} -> + {:error, reason} + end + end +end diff --git a/test/casbin/async_case_test.exs b/test/casbin/async_case_test.exs new file mode 100644 index 0000000..0a15561 --- /dev/null +++ b/test/casbin/async_case_test.exs @@ -0,0 +1,179 @@ +defmodule Casbin.AsyncCaseTest do + use Casbin.AsyncCase, async: true + + alias Casbin.EnforcerServer + + @cfile "../../data/acl.conf" |> Path.expand(__DIR__) + @pfile "../../data/acl.csv" |> Path.expand(__DIR__) + + setup do + # Clean ETS state + :ets.delete_all_objects(:enforcers_table) + + # Start isolated enforcer for this test + enforcer_name = start_test_enforcer(@cfile) + + {:ok, enforcer_name: enforcer_name} + end + + describe "AsyncCase basic functionality" do + test "creates unique enforcer per test", %{enforcer_name: ename} do + # Verify enforcer name is unique and properly formatted + assert is_binary(ename) + assert String.contains?(ename, "Casbin.AsyncCaseTest") + + # Verify enforcer is functional + :ok = EnforcerServer.add_policy(ename, {:p, ["alice", "data", "read"]}) + assert EnforcerServer.allow?(ename, ["alice", "data", "read"]) == true + end + + test "test 1 has isolated state", %{enforcer_name: ename} do + # Add policy specific to this test + :ok = EnforcerServer.add_policy(ename, {:p, ["user1", "resource1", "read"]}) + + # Verify it exists + assert EnforcerServer.allow?(ename, ["user1", "resource1", "read"]) == true + + # Verify only this test's policies exist + policies = EnforcerServer.list_policies(ename, %{}) + assert length(policies) == 1 + end + + test "test 2 has different isolated state", %{enforcer_name: ename} do + # Add different policy in this test + :ok = EnforcerServer.add_policy(ename, {:p, ["user2", "resource2", "write"]}) + + # Verify it exists + assert EnforcerServer.allow?(ename, ["user2", "resource2", "write"]) == true + + # Should not see test 1's policies + policies = EnforcerServer.list_policies(ename, %{sub: "user1"}) + assert length(policies) == 0 + end + + test "test 3 with loaded policies", %{enforcer_name: ename} do + # Load pre-defined policies + :ok = EnforcerServer.load_policies(ename, @pfile) + + # Verify policies are loaded + assert EnforcerServer.allow?(ename, ["alice", "blog_post", "create"]) == true + assert EnforcerServer.allow?(ename, ["bob", "blog_post", "read"]) == true + + # Add a new policy + :ok = EnforcerServer.add_policy(ename, {:p, ["charlie", "blog_post", "delete"]}) + + # Verify new policy works + assert EnforcerServer.allow?(ename, ["charlie", "blog_post", "delete"]) == true + end + end + + describe "AsyncCase concurrent tests" do + test "concurrent test A", %{enforcer_name: ename} do + # Simulate real work + :ok = EnforcerServer.add_policy(ename, {:p, ["concurrent_a", "data", "read"]}) + Process.sleep(5) + assert EnforcerServer.allow?(ename, ["concurrent_a", "data", "read"]) == true + end + + test "concurrent test B", %{enforcer_name: ename} do + # Simulate real work + :ok = EnforcerServer.add_policy(ename, {:p, ["concurrent_b", "data", "write"]}) + Process.sleep(5) + assert EnforcerServer.allow?(ename, ["concurrent_b", "data", "write"]) == true + end + + test "concurrent test C", %{enforcer_name: ename} do + # Simulate real work + :ok = EnforcerServer.add_policy(ename, {:p, ["concurrent_c", "data", "delete"]}) + Process.sleep(5) + assert EnforcerServer.allow?(ename, ["concurrent_c", "data", "delete"]) == true + end + end + + describe "AsyncCase with RBAC" do + @cfile_rbac "../../data/rbac.conf" |> Path.expand(__DIR__) + @pfile_rbac "../../data/rbac.csv" |> Path.expand(__DIR__) + + test "works with role-based access control" do + ename = start_test_enforcer("rbac_test", @cfile_rbac) + + # Load policies and mappings + :ok = EnforcerServer.load_policies(ename, @pfile_rbac) + :ok = EnforcerServer.load_mapping_policies(ename, @pfile_rbac) + + # Verify RBAC works + # bob has role reader, reader can read + assert EnforcerServer.allow?(ename, ["bob", "blog_post", "read"]) == true + assert EnforcerServer.allow?(ename, ["bob", "blog_post", "create"]) == false + + # alice has role admin, admin can do everything + assert EnforcerServer.allow?(ename, ["alice", "blog_post", "read"]) == true + assert EnforcerServer.allow?(ename, ["alice", "blog_post", "create"]) == true + assert EnforcerServer.allow?(ename, ["alice", "blog_post", "modify"]) == true + assert EnforcerServer.allow?(ename, ["alice", "blog_post", "delete"]) == true + end + + test "can add and remove mapping policies" do + ename = start_test_enforcer("mapping_test", @cfile_rbac) + + # Load initial state + :ok = EnforcerServer.load_policies(ename, @pfile_rbac) + :ok = EnforcerServer.load_mapping_policies(ename, @pfile_rbac) + + # Add a new user with a role + :ok = EnforcerServer.add_mapping_policy(ename, {:g, "charlie", "author"}) + + # Verify charlie has author permissions + assert EnforcerServer.allow?(ename, ["charlie", "blog_post", "create"]) == true + assert EnforcerServer.allow?(ename, ["charlie", "blog_post", "delete"]) == false + + # Remove the mapping + :ok = EnforcerServer.remove_mapping_policy(ename, {:g, "charlie", "author"}) + + # Verify charlie no longer has author permissions + assert EnforcerServer.allow?(ename, ["charlie", "blog_post", "create"]) == false + end + end + + describe "AsyncCase with policy modifications" do + test "can add policies", %{enforcer_name: ename} do + :ok = EnforcerServer.add_policy(ename, {:p, ["alice", "data1", "read"]}) + :ok = EnforcerServer.add_policy(ename, {:p, ["alice", "data1", "write"]}) + :ok = EnforcerServer.add_policy(ename, {:p, ["bob", "data2", "read"]}) + + assert EnforcerServer.allow?(ename, ["alice", "data1", "read"]) == true + assert EnforcerServer.allow?(ename, ["alice", "data1", "write"]) == true + assert EnforcerServer.allow?(ename, ["bob", "data2", "read"]) == true + + # List policies to verify + alice_policies = EnforcerServer.list_policies(ename, %{sub: "alice"}) + assert length(alice_policies) == 2 + end + + test "can remove policies", %{enforcer_name: ename} do + # Add policies + :ok = EnforcerServer.add_policy(ename, {:p, ["alice", "data1", "read"]}) + :ok = EnforcerServer.add_policy(ename, {:p, ["alice", "data1", "write"]}) + + # Verify they exist + assert EnforcerServer.allow?(ename, ["alice", "data1", "read"]) == true + assert EnforcerServer.allow?(ename, ["alice", "data1", "write"]) == true + + # Remove one policy + :ok = EnforcerServer.remove_policy(ename, {:p, ["alice", "data1", "read"]}) + + # Verify it's removed but other remains + assert EnforcerServer.allow?(ename, ["alice", "data1", "read"]) == false + assert EnforcerServer.allow?(ename, ["alice", "data1", "write"]) == true + end + + test "returns error for duplicate policy", %{enforcer_name: ename} do + # Add policy + :ok = EnforcerServer.add_policy(ename, {:p, ["alice", "data", "read"]}) + + # Try to add again + result = EnforcerServer.add_policy(ename, {:p, ["alice", "data", "read"]}) + assert {:error, :already_existed} = result + end + end +end diff --git a/test/casbin/test_helper_test.exs b/test/casbin/test_helper_test.exs new file mode 100644 index 0000000..0df1ffc --- /dev/null +++ b/test/casbin/test_helper_test.exs @@ -0,0 +1,187 @@ +defmodule Casbin.TestHelperTest do + use ExUnit.Case, async: true + + alias Casbin.{EnforcerSupervisor, EnforcerServer, TestHelper} + + @cfile "../../data/acl.conf" |> Path.expand(__DIR__) + @pfile "../../data/acl.csv" |> Path.expand(__DIR__) + + setup do + # Clean ETS state before each test + :ets.delete_all_objects(:enforcers_table) + :ok + end + + describe "unique_enforcer_name/1" do + test "generates unique names" do + name1 = TestHelper.unique_enforcer_name("test") + name2 = TestHelper.unique_enforcer_name("test") + + assert name1 != name2 + assert String.contains?(name1, "test") + assert String.contains?(name2, "test") + end + + test "uses default prefix when not provided" do + name = TestHelper.unique_enforcer_name() + assert String.contains?(name, "test") + end + + test "includes custom prefix" do + name = TestHelper.unique_enforcer_name("my_module") + assert String.contains?(name, "my_module") + end + + test "generates names suitable for concurrent tests" do + # Simulate multiple tests running in parallel + names = + 1..100 + |> Enum.map(fn _ -> + Task.async(fn -> TestHelper.unique_enforcer_name("concurrent") end) + end) + |> Enum.map(&Task.await/1) + + # All names should be unique + assert length(Enum.uniq(names)) == 100 + end + end + + describe "cleanup_enforcer/1" do + test "removes enforcer from ETS table" do + ename = TestHelper.unique_enforcer_name("cleanup_test") + {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile) + + # Verify it's in the ETS table + assert :ets.lookup(:enforcers_table, ename) != [] + + # Clean up + TestHelper.cleanup_enforcer(ename) + + # Verify it's removed + assert :ets.lookup(:enforcers_table, ename) == [] + end + + test "stops the enforcer process" do + ename = TestHelper.unique_enforcer_name("cleanup_process_test") + {:ok, pid} = EnforcerSupervisor.start_enforcer(ename, @cfile) + + assert Process.alive?(pid) + + # Clean up + TestHelper.cleanup_enforcer(ename) + + # Give it a moment to stop + Process.sleep(10) + + # Verify process is no longer registered + assert Registry.lookup(Casbin.EnforcerRegistry, ename) == [] + end + + test "handles non-existent enforcer gracefully" do + # Should not raise an error + assert :ok = TestHelper.cleanup_enforcer("non_existent_enforcer") + end + end + + describe "create_test_enforcer/2" do + test "creates enforcer and returns unique name" do + assert {:ok, ename} = TestHelper.create_test_enforcer("create_test", @cfile) + + assert String.contains?(ename, "create_test") + + # Verify enforcer works + :ok = EnforcerServer.load_policies(ename, @pfile) + assert EnforcerServer.allow?(ename, ["alice", "blog_post", "read"]) == true + end + + test "uses default prefix when not provided" do + assert {:ok, ename} = TestHelper.create_test_enforcer(@cfile) + assert String.contains?(ename, "test") + end + + test "returns error for invalid config file" do + assert {:error, _reason} = TestHelper.create_test_enforcer("test", "/invalid/path.conf") + end + end + + describe "async test isolation" do + # These tests run in parallel to verify isolation + test "test 1 - isolated enforcer" do + ename = TestHelper.unique_enforcer_name("async_1") + {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile) + on_exit(fn -> TestHelper.cleanup_enforcer(ename) end) + + :ok = EnforcerServer.add_policy(ename, {:p, ["user1", "data1", "read"]}) + + # Verify policy exists in this enforcer + assert EnforcerServer.allow?(ename, ["user1", "data1", "read"]) == true + + # Should not affect other enforcers + policies = EnforcerServer.list_policies(ename, %{sub: "user1"}) + assert length(policies) == 1 + end + + test "test 2 - isolated enforcer" do + ename = TestHelper.unique_enforcer_name("async_2") + {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile) + on_exit(fn -> TestHelper.cleanup_enforcer(ename) end) + + :ok = EnforcerServer.add_policy(ename, {:p, ["user2", "data2", "write"]}) + + # Verify policy exists in this enforcer + assert EnforcerServer.allow?(ename, ["user2", "data2", "write"]) == true + + # Should not see user1's policies + policies = EnforcerServer.list_policies(ename, %{sub: "user1"}) + assert length(policies) == 0 + end + + test "test 3 - isolated enforcer" do + ename = TestHelper.unique_enforcer_name("async_3") + {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile) + on_exit(fn -> TestHelper.cleanup_enforcer(ename) end) + + :ok = EnforcerServer.add_policy(ename, {:p, ["user3", "data3", "delete"]}) + + # Verify policy exists in this enforcer + assert EnforcerServer.allow?(ename, ["user3", "data3", "delete"]) == true + + # Should not see other users' policies + assert EnforcerServer.list_policies(ename, %{sub: "user1"}) == [] + assert EnforcerServer.list_policies(ename, %{sub: "user2"}) == [] + end + end + + describe "stress test with many concurrent enforcers" do + test "handles many parallel enforcers" do + # Create and use multiple enforcers concurrently + tasks = + 1..50 + |> Enum.map(fn i -> + Task.async(fn -> + ename = TestHelper.unique_enforcer_name("stress_#{i}") + {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile) + + # Add a unique policy + :ok = EnforcerServer.add_policy(ename, {:p, ["user#{i}", "resource#{i}", "read"]}) + + # Verify it works + result = EnforcerServer.allow?(ename, ["user#{i}", "resource#{i}", "read"]) + + # Clean up + TestHelper.cleanup_enforcer(ename) + + {ename, result} + end) + end) + |> Enum.map(&Task.await(&1, 10_000)) + + # All should succeed + assert Enum.all?(tasks, fn {_name, result} -> result == true end) + + # All names should be unique + names = Enum.map(tasks, fn {name, _result} -> name end) + assert length(Enum.uniq(names)) == 50 + end + end +end From de0789c26d8ea5e6a89b877ce6a4b8f4a867e790 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 14:03:38 +0000 Subject: [PATCH 3/5] Address code review feedback: fix parameter order and remove ETS cleanup from async tests Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com> --- guides/async_testing.md | 2 +- lib/casbin/test_helper.ex | 11 ++- test/casbin/async_case_test.exs | 3 - test/casbin/async_example.exs | 139 +++++++++++++++++++++++++++++++ test/casbin/test_helper_test.exs | 10 +-- 5 files changed, 150 insertions(+), 15 deletions(-) create mode 100644 test/casbin/async_example.exs diff --git a/guides/async_testing.md b/guides/async_testing.md index d87a9fe..9d5f523 100644 --- a/guides/async_testing.md +++ b/guides/async_testing.md @@ -144,7 +144,7 @@ defmodule MyApp.MinimalTest do alias Casbin.EnforcerServer test "quick test with enforcer" do - {:ok, ename} = create_test_enforcer("minimal", "path/to/model.conf") + {:ok, ename} = create_test_enforcer("path/to/model.conf", "minimal") EnforcerServer.add_policy(ename, {:p, ["user", "resource", "action"]}) assert EnforcerServer.allow?(ename, ["user", "resource", "action"]) diff --git a/lib/casbin/test_helper.ex b/lib/casbin/test_helper.ex index af3db14..57cf888 100644 --- a/lib/casbin/test_helper.ex +++ b/lib/casbin/test_helper.ex @@ -144,8 +144,8 @@ defmodule Casbin.TestHelper do ## Parameters - - `prefix` - Optional string prefix for the enforcer name (default: "test") - `config_file` - Path to the Casbin configuration file + - `prefix` - Optional string prefix for the enforcer name (default: "test") ## Returns @@ -154,15 +154,20 @@ defmodule Casbin.TestHelper do ## Examples test "admin permissions" do - {:ok, ename} = Casbin.TestHelper.create_test_enforcer("acl", config_path) + {:ok, ename} = Casbin.TestHelper.create_test_enforcer(config_path) EnforcerServer.add_policy(ename, {:p, ["admin", "data", "read"]}) assert EnforcerServer.allow?(ename, ["admin", "data", "read"]) end + test "with custom prefix" do + {:ok, ename} = Casbin.TestHelper.create_test_enforcer(config_path, "my_test") + # ... + end + Note: This function automatically registers cleanup in `on_exit/1`, so you don't need to manually clean up the enforcer. """ - def create_test_enforcer(prefix \\ "test", config_file) do + def create_test_enforcer(config_file, prefix \\ "test") do ename = unique_enforcer_name(prefix) case Casbin.EnforcerSupervisor.start_enforcer(ename, config_file) do diff --git a/test/casbin/async_case_test.exs b/test/casbin/async_case_test.exs index 0a15561..7106747 100644 --- a/test/casbin/async_case_test.exs +++ b/test/casbin/async_case_test.exs @@ -7,9 +7,6 @@ defmodule Casbin.AsyncCaseTest do @pfile "../../data/acl.csv" |> Path.expand(__DIR__) setup do - # Clean ETS state - :ets.delete_all_objects(:enforcers_table) - # Start isolated enforcer for this test enforcer_name = start_test_enforcer(@cfile) diff --git a/test/casbin/async_example.exs b/test/casbin/async_example.exs new file mode 100644 index 0000000..eaa15d5 --- /dev/null +++ b/test/casbin/async_example.exs @@ -0,0 +1,139 @@ +defmodule Casbin.AsyncExample do + @moduledoc """ + Example demonstrating the fix for shared global enforcer state in async tests. + + This module shows both the BEFORE (problematic) and AFTER (fixed) patterns. + """ + + defmodule BeforeAsyncFix do + @moduledoc """ + ❌ PROBLEMATIC: Using a fixed enforcer name causes race conditions in async tests. + + This is what users were experiencing - tests would fail randomly when run together. + """ + use ExUnit.Case, async: false # Had to use async: false + + alias Casbin.{EnforcerSupervisor, EnforcerServer} + + @enforcer_name "my_enforcer" # ❌ Shared global state! + @cfile "../../data/acl.conf" |> Path.expand(__DIR__) + + setup do + # All tests share the same enforcer + {:ok, _pid} = EnforcerSupervisor.start_enforcer(@enforcer_name, @cfile) + + on_exit(fn -> + # This cleanup affects ALL tests using this enforcer + :ets.delete(:enforcers_table, @enforcer_name) + end) + + :ok + end + + test "admin permissions" do + EnforcerServer.add_policy(@enforcer_name, {:p, ["admin", "data", "read"]}) + # Another test's cleanup might delete this policy mid-test! + assert EnforcerServer.allow?(@enforcer_name, ["admin", "data", "read"]) + end + + test "user permissions" do + EnforcerServer.add_policy(@enforcer_name, {:p, ["user", "data", "read"]}) + # Race condition: might see policies from other tests + assert EnforcerServer.allow?(@enforcer_name, ["user", "data", "read"]) + end + end + + defmodule AfterAsyncFix do + @moduledoc """ + ✅ FIXED: Using Casbin.AsyncCase for isolated enforcers per test. + + Each test gets its own enforcer instance, preventing race conditions. + """ + use Casbin.AsyncCase, async: true # ✅ Now safe to use async: true! + + alias Casbin.EnforcerServer + + @cfile "../../data/acl.conf" |> Path.expand(__DIR__) + + setup do + # Each test gets its own unique enforcer + {:ok, enforcer_name: start_test_enforcer(@cfile)} + end + + test "admin permissions", %{enforcer_name: ename} do + EnforcerServer.add_policy(ename, {:p, ["admin", "data", "read"]}) + # This test's policies are isolated from other tests + assert EnforcerServer.allow?(ename, ["admin", "data", "read"]) + end + + test "user permissions", %{enforcer_name: ename} do + EnforcerServer.add_policy(ename, {:p, ["user", "data", "read"]}) + # No interference from other tests! + assert EnforcerServer.allow?(ename, ["user", "data", "read"]) + end + + test "concurrent test 1", %{enforcer_name: ename} do + # Runs in parallel with other tests + EnforcerServer.add_policy(ename, {:p, ["user1", "resource1", "action"]}) + assert EnforcerServer.allow?(ename, ["user1", "resource1", "action"]) + end + + test "concurrent test 2", %{enforcer_name: ename} do + # Runs in parallel with other tests + EnforcerServer.add_policy(ename, {:p, ["user2", "resource2", "action"]}) + assert EnforcerServer.allow?(ename, ["user2", "resource2", "action"]) + end + end + + defmodule UsingTestHelper do + @moduledoc """ + ✅ Alternative: Using Casbin.TestHelper for more control. + + This approach gives you more flexibility while still ensuring isolation. + """ + use ExUnit.Case, async: true + + import Casbin.TestHelper + alias Casbin.{EnforcerSupervisor, EnforcerServer} + + @cfile "../../data/acl.conf" |> Path.expand(__DIR__) + + setup do + # Generate unique name and start enforcer manually + ename = unique_enforcer_name("test_helper_example") + {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile) + + # Register cleanup + on_exit(fn -> cleanup_enforcer(ename) end) + + {:ok, enforcer_name: ename} + end + + test "using test helper", %{enforcer_name: ename} do + EnforcerServer.add_policy(ename, {:p, ["alice", "data", "read"]}) + assert EnforcerServer.allow?(ename, ["alice", "data", "read"]) + end + end + + defmodule QuickExample do + @moduledoc """ + ✅ Convenience: Using create_test_enforcer for minimal boilerplate. + """ + use ExUnit.Case, async: true + + import Casbin.TestHelper + alias Casbin.EnforcerServer + + @cfile "../../data/acl.conf" |> Path.expand(__DIR__) + + test "quick test with minimal setup" do + # create_test_enforcer handles everything including automatic cleanup + {:ok, ename} = create_test_enforcer(@cfile, "quick") + + EnforcerServer.add_policy(ename, {:p, ["user", "resource", "action"]}) + assert EnforcerServer.allow?(ename, ["user", "resource", "action"]) + + # Cleanup happens automatically via on_exit registered by create_test_enforcer + end + end +end diff --git a/test/casbin/test_helper_test.exs b/test/casbin/test_helper_test.exs index 0df1ffc..1c41725 100644 --- a/test/casbin/test_helper_test.exs +++ b/test/casbin/test_helper_test.exs @@ -6,12 +6,6 @@ defmodule Casbin.TestHelperTest do @cfile "../../data/acl.conf" |> Path.expand(__DIR__) @pfile "../../data/acl.csv" |> Path.expand(__DIR__) - setup do - # Clean ETS state before each test - :ets.delete_all_objects(:enforcers_table) - :ok - end - describe "unique_enforcer_name/1" do test "generates unique names" do name1 = TestHelper.unique_enforcer_name("test") @@ -85,7 +79,7 @@ defmodule Casbin.TestHelperTest do describe "create_test_enforcer/2" do test "creates enforcer and returns unique name" do - assert {:ok, ename} = TestHelper.create_test_enforcer("create_test", @cfile) + assert {:ok, ename} = TestHelper.create_test_enforcer(@cfile, "create_test") assert String.contains?(ename, "create_test") @@ -100,7 +94,7 @@ defmodule Casbin.TestHelperTest do end test "returns error for invalid config file" do - assert {:error, _reason} = TestHelper.create_test_enforcer("test", "/invalid/path.conf") + assert {:error, _reason} = TestHelper.create_test_enforcer("/invalid/path.conf", "test") end end From d02c47ff0e1b658da72a66541c4598a84f507544 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 14:10:30 +0000 Subject: [PATCH 4/5] Remove test files, markdown docs, and README changes per feedback Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com> --- README.md | 29 --- guides/async_testing.md | 377 ------------------------------- lib/casbin/async_case.ex | 6 +- test/casbin/async_case_test.exs | 176 --------------- test/casbin/async_example.exs | 139 ------------ test/casbin/test_helper_test.exs | 181 --------------- 6 files changed, 3 insertions(+), 905 deletions(-) delete mode 100644 guides/async_testing.md delete mode 100644 test/casbin/async_case_test.exs delete mode 100644 test/casbin/async_example.exs delete mode 100644 test/casbin/test_helper_test.exs diff --git a/README.md b/README.md index f79afdd..1622a47 100644 --- a/README.md +++ b/README.md @@ -458,35 +458,6 @@ Key topics covered: ## Testing -### Async Testing with Isolated Enforcers - -When writing tests with `async: true`, you need to ensure each test has its own isolated enforcer to prevent race conditions. Casbin-Ex provides `Casbin.AsyncCase` for this purpose: - -```elixir -defmodule MyApp.PermissionsTest do - use Casbin.AsyncCase, async: true - - @config_file "path/to/model.conf" - - setup do - # Creates a unique enforcer for each test - {:ok, enforcer_name: start_test_enforcer(@config_file)} - end - - test "admin permissions", %{enforcer_name: ename} do - EnforcerServer.add_policy(ename, {:p, ["admin", "data", "read"]}) - assert EnforcerServer.allow?(ename, ["admin", "data", "read"]) - end -end -``` - -**Why this matters:** Using a fixed enforcer name (e.g., `"my_enforcer"`) causes all tests to share the same global state, leading to race conditions in async tests. See our [Async Testing Guide](guides/async_testing.md) for: - -- Complete examples and best practices -- Using `Casbin.TestHelper` for more control -- Troubleshooting common async test issues -- Migration guide from sync to async tests - ### 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/async_testing.md b/guides/async_testing.md deleted file mode 100644 index 9d5f523..0000000 --- a/guides/async_testing.md +++ /dev/null @@ -1,377 +0,0 @@ -# Testing with Async Mode - -This guide explains how to write Casbin tests that can safely run with `async: true`, preventing race conditions and test interference. - -## The Problem - -When using EnforcerServer with a fixed enforcer name, all tests share the same global state, causing race conditions in async tests: - -```elixir -# ❌ This fails with async: true -defmodule MyApp.AclTest do - use ExUnit.Case, async: true # Tests interfere with each other - - @enforcer_name "my_enforcer" - - setup do - # All tests use the same enforcer! - EnforcerServer.add_policy(@enforcer_name, {:p, ["admin", "data", "read"]}) - - on_exit(fn -> - # This cleanup affects ALL running tests - EnforcerServer.remove_policy(@enforcer_name, {:p, ["admin", "data", "read"]}) - end) - end - - test "admin has permissions" do - # Another test's cleanup might delete this policy mid-test! - assert EnforcerServer.allow?(@enforcer_name, ["admin", "data", "read"]) - end -end -``` - -**Symptoms:** -- `Policies.list()` returns `[]` even after adding policies -- `add_policy` returns `{:error, :already_existed}` but policies aren't in memory -- Tests pass individually but fail when run together -- Tests fail randomly depending on execution order - -## Solution 1: Use Casbin.AsyncCase (Recommended) - -The easiest solution is to use `Casbin.AsyncCase`, which automatically handles enforcer isolation: - -```elixir -# ✅ This works with async: true -defmodule MyApp.AclTest do - use Casbin.AsyncCase, async: true - - alias Casbin.EnforcerServer - - @config_file "path/to/model.conf" - - setup do - # Creates a unique enforcer for THIS test - enforcer_name = start_test_enforcer(@config_file) - - {:ok, enforcer_name: enforcer_name} - end - - test "admin has permissions", %{enforcer_name: ename} do - EnforcerServer.add_policy(ename, {:p, ["admin", "data", "read"]}) - assert EnforcerServer.allow?(ename, ["admin", "data", "read"]) - end - - test "user has limited permissions", %{enforcer_name: ename} do - # This test has its own isolated enforcer - EnforcerServer.add_policy(ename, {:p, ["user", "data", "read"]}) - assert EnforcerServer.allow?(ename, ["user", "data", "read"]) - assert EnforcerServer.allow?(ename, ["user", "data", "write"]) == false - end -end -``` - -### With Pre-loaded Policies - -```elixir -defmodule MyApp.PermissionsTest do - use Casbin.AsyncCase, async: true - - @config_file Application.app_dir(:my_app, "priv/casbin/model.conf") - @policy_file Application.app_dir(:my_app, "priv/casbin/policy.csv") - - setup do - ename = start_test_enforcer(@config_file) - - # Load pre-defined policies - :ok = EnforcerServer.load_policies(ename, @policy_file) - - {:ok, enforcer_name: ename} - end - - test "existing policies work", %{enforcer_name: ename} do - assert EnforcerServer.allow?(ename, ["alice", "blog_post", "read"]) - end - - test "can add new policies", %{enforcer_name: ename} do - :ok = EnforcerServer.add_policy(ename, {:p, ["bob", "data", "write"]}) - assert EnforcerServer.allow?(ename, ["bob", "data", "write"]) - end -end -``` - -## Solution 2: Use Casbin.TestHelper (More Control) - -For more control, use `Casbin.TestHelper` functions directly: - -```elixir -defmodule MyApp.CustomAclTest do - use ExUnit.Case, async: true - - import Casbin.TestHelper - alias Casbin.{EnforcerSupervisor, EnforcerServer} - - @config_file "path/to/model.conf" - - setup do - # Generate a unique enforcer name - ename = unique_enforcer_name("custom_acl") - - # Start the enforcer - {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @config_file) - - # Register cleanup - on_exit(fn -> cleanup_enforcer(ename) end) - - {:ok, enforcer_name: ename} - end - - test "can use custom enforcer", %{enforcer_name: ename} do - EnforcerServer.add_policy(ename, {:p, ["alice", "data", "read"]}) - assert EnforcerServer.allow?(ename, ["alice", "data", "read"]) - end -end -``` - -### Using create_test_enforcer - -For even less boilerplate: - -```elixir -defmodule MyApp.MinimalTest do - use ExUnit.Case, async: true - - import Casbin.TestHelper - alias Casbin.EnforcerServer - - test "quick test with enforcer" do - {:ok, ename} = create_test_enforcer("path/to/model.conf", "minimal") - - EnforcerServer.add_policy(ename, {:p, ["user", "resource", "action"]}) - assert EnforcerServer.allow?(ename, ["user", "resource", "action"]) - end -end -``` - -## Solution 3: Use async: false (Simplest, but Slower) - -If you don't need parallel execution, you can disable async mode: - -```elixir -# ✅ This works but runs slower -defmodule MyApp.AclTest do - use ExUnit.Case, async: false # Tests run sequentially - - @enforcer_name "my_enforcer" - - # Now safe to use a fixed enforcer name -end -``` - -**Trade-offs:** -- ✅ Simple - no code changes needed -- ❌ Slower - tests run sequentially instead of in parallel -- ❌ Doesn't scale - more tests = longer test suite runtime - -## Using with EctoAdapter and SQL.Sandbox - -When using `EctoAdapter` with `Ecto.Adapters.SQL.Sandbox`, you need to allow the enforcer process to access the database connection: - -```elixir -defmodule MyApp.DatabaseAclTest do - use MyApp.DataCase, async: true - use Casbin.AsyncCase - - alias Casbin.{EnforcerServer, Persist.EctoAdapter} - - @config_file Application.app_dir(:my_app, "priv/casbin/model.conf") - - setup do - # Check out a database connection - :ok = Ecto.Adapters.SQL.Sandbox.checkout(MyApp.Repo) - - # Start isolated enforcer - ename = start_test_enforcer(@config_file) - - # Allow enforcer process to access the database - case Registry.lookup(Casbin.EnforcerRegistry, ename) do - [{pid, _}] -> - Ecto.Adapters.SQL.Sandbox.allow(MyApp.Repo, self(), pid) - [] -> - :ok - end - - # Set up the adapter - adapter = EctoAdapter.new(MyApp.Repo) - :ok = EnforcerServer.set_persist_adapter(ename, adapter) - - {:ok, enforcer_name: ename} - end - - test "policies persist to database", %{enforcer_name: ename} do - :ok = EnforcerServer.add_policy(ename, {:p, ["user", "data", "read"]}) - - # Policies are automatically saved with EctoAdapter - assert EnforcerServer.allow?(ename, ["user", "data", "read"]) - - # Verify it was persisted - policies = EnforcerServer.list_policies(ename, %{sub: "user"}) - assert length(policies) == 1 - end -end -``` - -For transactional tests with Ecto, see the [Sandbox Testing Guide](sandbox_testing.md). - -## Best Practices - -### 1. Always Use Unique Enforcer Names - -```elixir -# ❌ Bad - shared state -@enforcer_name "my_enforcer" - -# ✅ Good - unique per test -ename = unique_enforcer_name("my_test") -``` - -### 2. Clean Up Enforcers - -```elixir -# Always register cleanup -on_exit(fn -> cleanup_enforcer(ename) end) - -# Or use AsyncCase/create_test_enforcer which handles this automatically -``` - -### 3. Use Module Attributes for Config Files - -```elixir -defmodule MyApp.AclTest do - use Casbin.AsyncCase, async: true - - # ✅ Good - easy to update - @config_file Application.app_dir(:my_app, "priv/casbin/model.conf") - @policy_file Application.app_dir(:my_app, "priv/casbin/policy.csv") - - setup do - ename = start_test_enforcer(@config_file) - :ok = EnforcerServer.load_policies(ename, @policy_file) - {:ok, enforcer_name: ename} - end -end -``` - -### 4. Organize Tests by Resource - -```elixir -# Organize by what you're testing -defmodule MyApp.BlogPermissionsTest do - use Casbin.AsyncCase, async: true - # Tests for blog post permissions -end - -defmodule MyApp.DataPermissionsTest do - use Casbin.AsyncCase, async: true - # Tests for data permissions -end -``` - -## Troubleshooting - -### "Policy not found" errors in async tests - -**Problem:** Policies disappear mid-test or `list_policies` returns `[]` - -**Solution:** Make sure each test uses a unique enforcer name: - -```elixir -# Use AsyncCase or unique_enforcer_name() -ename = unique_enforcer_name("my_test") -``` - -### "{:error, :already_existed}" but policy isn't there - -**Problem:** `add_policy` fails with `:already_existed` but the policy doesn't show in `list_policies` - -**Solution:** Another test is using the same enforcer. Use unique names: - -```elixir -# In setup -ename = start_test_enforcer(@config_file) # Unique per test -``` - -### Tests pass individually but fail together - -**Problem:** `mix test path/to/test.exs` passes but `mix test` fails - -**Solution:** Tests are sharing state. Use `async: true` with unique enforcers: - -```elixir -use Casbin.AsyncCase, async: true -``` - -### Database connection errors with EctoAdapter - -**Problem:** `DBConnection.ConnectionError` when using `EctoAdapter` - -**Solution:** Allow the enforcer process to access the connection: - -```elixir -setup do - :ok = Ecto.Adapters.SQL.Sandbox.checkout(MyApp.Repo) - ename = start_test_enforcer(@config_file) - - [{pid, _}] = Registry.lookup(Casbin.EnforcerRegistry, ename) - Ecto.Adapters.SQL.Sandbox.allow(MyApp.Repo, self(), pid) - - {:ok, enforcer_name: ename} -end -``` - -## Migration Guide - -If you have existing tests using a fixed enforcer name: - -### Before - -```elixir -defmodule MyApp.AclTest do - use ExUnit.Case # async: false by default - - @enforcer_name "my_enforcer" - - test "admin permissions" do - EnforcerServer.add_policy(@enforcer_name, {:p, ["admin", "data", "read"]}) - assert EnforcerServer.allow?(@enforcer_name, ["admin", "data", "read"]) - end -end -``` - -### After - -```elixir -defmodule MyApp.AclTest do - use Casbin.AsyncCase, async: true - - @config_file "path/to/model.conf" - - setup do - {:ok, enforcer_name: start_test_enforcer(@config_file)} - end - - test "admin permissions", %{enforcer_name: ename} do - EnforcerServer.add_policy(ename, {:p, ["admin", "data", "read"]}) - assert EnforcerServer.allow?(ename, ["admin", "data", "read"]) - end -end -``` - -## Summary - -- Use `Casbin.AsyncCase` for automatic enforcer isolation in async tests -- Use `Casbin.TestHelper` for more control over enforcer lifecycle -- Always use unique enforcer names to prevent race conditions -- Clean up enforcers in `on_exit` callbacks (or use AsyncCase/create_test_enforcer) -- Allow enforcer processes to access database connections when using EctoAdapter - -With these patterns, you can safely run Casbin tests in parallel with `async: true`, making your test suite faster and more reliable. diff --git a/lib/casbin/async_case.ex b/lib/casbin/async_case.ex index 3fd9d7a..9df1c0e 100644 --- a/lib/casbin/async_case.ex +++ b/lib/casbin/async_case.ex @@ -133,7 +133,7 @@ defmodule Casbin.AsyncCase do defmacro __using__(opts) do # Extract async option (default to true for safety) async = Keyword.get(opts, :async, true) - + # Extract prefix option (default to module name) # Note: __CALLER__.module will be the test module using this macro prefix = Keyword.get(opts, :prefix, nil) @@ -152,7 +152,7 @@ defmodule Casbin.AsyncCase do prefix and the provided config file. ## Parameters - + - `config_file` - Path to the Casbin configuration file ## Returns @@ -178,7 +178,7 @@ defmodule Casbin.AsyncCase do for the enforcer name. ## Parameters - + - `prefix` - Custom prefix for the enforcer name - `config_file` - Path to the Casbin configuration file diff --git a/test/casbin/async_case_test.exs b/test/casbin/async_case_test.exs deleted file mode 100644 index 7106747..0000000 --- a/test/casbin/async_case_test.exs +++ /dev/null @@ -1,176 +0,0 @@ -defmodule Casbin.AsyncCaseTest do - use Casbin.AsyncCase, async: true - - alias Casbin.EnforcerServer - - @cfile "../../data/acl.conf" |> Path.expand(__DIR__) - @pfile "../../data/acl.csv" |> Path.expand(__DIR__) - - setup do - # Start isolated enforcer for this test - enforcer_name = start_test_enforcer(@cfile) - - {:ok, enforcer_name: enforcer_name} - end - - describe "AsyncCase basic functionality" do - test "creates unique enforcer per test", %{enforcer_name: ename} do - # Verify enforcer name is unique and properly formatted - assert is_binary(ename) - assert String.contains?(ename, "Casbin.AsyncCaseTest") - - # Verify enforcer is functional - :ok = EnforcerServer.add_policy(ename, {:p, ["alice", "data", "read"]}) - assert EnforcerServer.allow?(ename, ["alice", "data", "read"]) == true - end - - test "test 1 has isolated state", %{enforcer_name: ename} do - # Add policy specific to this test - :ok = EnforcerServer.add_policy(ename, {:p, ["user1", "resource1", "read"]}) - - # Verify it exists - assert EnforcerServer.allow?(ename, ["user1", "resource1", "read"]) == true - - # Verify only this test's policies exist - policies = EnforcerServer.list_policies(ename, %{}) - assert length(policies) == 1 - end - - test "test 2 has different isolated state", %{enforcer_name: ename} do - # Add different policy in this test - :ok = EnforcerServer.add_policy(ename, {:p, ["user2", "resource2", "write"]}) - - # Verify it exists - assert EnforcerServer.allow?(ename, ["user2", "resource2", "write"]) == true - - # Should not see test 1's policies - policies = EnforcerServer.list_policies(ename, %{sub: "user1"}) - assert length(policies) == 0 - end - - test "test 3 with loaded policies", %{enforcer_name: ename} do - # Load pre-defined policies - :ok = EnforcerServer.load_policies(ename, @pfile) - - # Verify policies are loaded - assert EnforcerServer.allow?(ename, ["alice", "blog_post", "create"]) == true - assert EnforcerServer.allow?(ename, ["bob", "blog_post", "read"]) == true - - # Add a new policy - :ok = EnforcerServer.add_policy(ename, {:p, ["charlie", "blog_post", "delete"]}) - - # Verify new policy works - assert EnforcerServer.allow?(ename, ["charlie", "blog_post", "delete"]) == true - end - end - - describe "AsyncCase concurrent tests" do - test "concurrent test A", %{enforcer_name: ename} do - # Simulate real work - :ok = EnforcerServer.add_policy(ename, {:p, ["concurrent_a", "data", "read"]}) - Process.sleep(5) - assert EnforcerServer.allow?(ename, ["concurrent_a", "data", "read"]) == true - end - - test "concurrent test B", %{enforcer_name: ename} do - # Simulate real work - :ok = EnforcerServer.add_policy(ename, {:p, ["concurrent_b", "data", "write"]}) - Process.sleep(5) - assert EnforcerServer.allow?(ename, ["concurrent_b", "data", "write"]) == true - end - - test "concurrent test C", %{enforcer_name: ename} do - # Simulate real work - :ok = EnforcerServer.add_policy(ename, {:p, ["concurrent_c", "data", "delete"]}) - Process.sleep(5) - assert EnforcerServer.allow?(ename, ["concurrent_c", "data", "delete"]) == true - end - end - - describe "AsyncCase with RBAC" do - @cfile_rbac "../../data/rbac.conf" |> Path.expand(__DIR__) - @pfile_rbac "../../data/rbac.csv" |> Path.expand(__DIR__) - - test "works with role-based access control" do - ename = start_test_enforcer("rbac_test", @cfile_rbac) - - # Load policies and mappings - :ok = EnforcerServer.load_policies(ename, @pfile_rbac) - :ok = EnforcerServer.load_mapping_policies(ename, @pfile_rbac) - - # Verify RBAC works - # bob has role reader, reader can read - assert EnforcerServer.allow?(ename, ["bob", "blog_post", "read"]) == true - assert EnforcerServer.allow?(ename, ["bob", "blog_post", "create"]) == false - - # alice has role admin, admin can do everything - assert EnforcerServer.allow?(ename, ["alice", "blog_post", "read"]) == true - assert EnforcerServer.allow?(ename, ["alice", "blog_post", "create"]) == true - assert EnforcerServer.allow?(ename, ["alice", "blog_post", "modify"]) == true - assert EnforcerServer.allow?(ename, ["alice", "blog_post", "delete"]) == true - end - - test "can add and remove mapping policies" do - ename = start_test_enforcer("mapping_test", @cfile_rbac) - - # Load initial state - :ok = EnforcerServer.load_policies(ename, @pfile_rbac) - :ok = EnforcerServer.load_mapping_policies(ename, @pfile_rbac) - - # Add a new user with a role - :ok = EnforcerServer.add_mapping_policy(ename, {:g, "charlie", "author"}) - - # Verify charlie has author permissions - assert EnforcerServer.allow?(ename, ["charlie", "blog_post", "create"]) == true - assert EnforcerServer.allow?(ename, ["charlie", "blog_post", "delete"]) == false - - # Remove the mapping - :ok = EnforcerServer.remove_mapping_policy(ename, {:g, "charlie", "author"}) - - # Verify charlie no longer has author permissions - assert EnforcerServer.allow?(ename, ["charlie", "blog_post", "create"]) == false - end - end - - describe "AsyncCase with policy modifications" do - test "can add policies", %{enforcer_name: ename} do - :ok = EnforcerServer.add_policy(ename, {:p, ["alice", "data1", "read"]}) - :ok = EnforcerServer.add_policy(ename, {:p, ["alice", "data1", "write"]}) - :ok = EnforcerServer.add_policy(ename, {:p, ["bob", "data2", "read"]}) - - assert EnforcerServer.allow?(ename, ["alice", "data1", "read"]) == true - assert EnforcerServer.allow?(ename, ["alice", "data1", "write"]) == true - assert EnforcerServer.allow?(ename, ["bob", "data2", "read"]) == true - - # List policies to verify - alice_policies = EnforcerServer.list_policies(ename, %{sub: "alice"}) - assert length(alice_policies) == 2 - end - - test "can remove policies", %{enforcer_name: ename} do - # Add policies - :ok = EnforcerServer.add_policy(ename, {:p, ["alice", "data1", "read"]}) - :ok = EnforcerServer.add_policy(ename, {:p, ["alice", "data1", "write"]}) - - # Verify they exist - assert EnforcerServer.allow?(ename, ["alice", "data1", "read"]) == true - assert EnforcerServer.allow?(ename, ["alice", "data1", "write"]) == true - - # Remove one policy - :ok = EnforcerServer.remove_policy(ename, {:p, ["alice", "data1", "read"]}) - - # Verify it's removed but other remains - assert EnforcerServer.allow?(ename, ["alice", "data1", "read"]) == false - assert EnforcerServer.allow?(ename, ["alice", "data1", "write"]) == true - end - - test "returns error for duplicate policy", %{enforcer_name: ename} do - # Add policy - :ok = EnforcerServer.add_policy(ename, {:p, ["alice", "data", "read"]}) - - # Try to add again - result = EnforcerServer.add_policy(ename, {:p, ["alice", "data", "read"]}) - assert {:error, :already_existed} = result - end - end -end diff --git a/test/casbin/async_example.exs b/test/casbin/async_example.exs deleted file mode 100644 index eaa15d5..0000000 --- a/test/casbin/async_example.exs +++ /dev/null @@ -1,139 +0,0 @@ -defmodule Casbin.AsyncExample do - @moduledoc """ - Example demonstrating the fix for shared global enforcer state in async tests. - - This module shows both the BEFORE (problematic) and AFTER (fixed) patterns. - """ - - defmodule BeforeAsyncFix do - @moduledoc """ - ❌ PROBLEMATIC: Using a fixed enforcer name causes race conditions in async tests. - - This is what users were experiencing - tests would fail randomly when run together. - """ - use ExUnit.Case, async: false # Had to use async: false - - alias Casbin.{EnforcerSupervisor, EnforcerServer} - - @enforcer_name "my_enforcer" # ❌ Shared global state! - @cfile "../../data/acl.conf" |> Path.expand(__DIR__) - - setup do - # All tests share the same enforcer - {:ok, _pid} = EnforcerSupervisor.start_enforcer(@enforcer_name, @cfile) - - on_exit(fn -> - # This cleanup affects ALL tests using this enforcer - :ets.delete(:enforcers_table, @enforcer_name) - end) - - :ok - end - - test "admin permissions" do - EnforcerServer.add_policy(@enforcer_name, {:p, ["admin", "data", "read"]}) - # Another test's cleanup might delete this policy mid-test! - assert EnforcerServer.allow?(@enforcer_name, ["admin", "data", "read"]) - end - - test "user permissions" do - EnforcerServer.add_policy(@enforcer_name, {:p, ["user", "data", "read"]}) - # Race condition: might see policies from other tests - assert EnforcerServer.allow?(@enforcer_name, ["user", "data", "read"]) - end - end - - defmodule AfterAsyncFix do - @moduledoc """ - ✅ FIXED: Using Casbin.AsyncCase for isolated enforcers per test. - - Each test gets its own enforcer instance, preventing race conditions. - """ - use Casbin.AsyncCase, async: true # ✅ Now safe to use async: true! - - alias Casbin.EnforcerServer - - @cfile "../../data/acl.conf" |> Path.expand(__DIR__) - - setup do - # Each test gets its own unique enforcer - {:ok, enforcer_name: start_test_enforcer(@cfile)} - end - - test "admin permissions", %{enforcer_name: ename} do - EnforcerServer.add_policy(ename, {:p, ["admin", "data", "read"]}) - # This test's policies are isolated from other tests - assert EnforcerServer.allow?(ename, ["admin", "data", "read"]) - end - - test "user permissions", %{enforcer_name: ename} do - EnforcerServer.add_policy(ename, {:p, ["user", "data", "read"]}) - # No interference from other tests! - assert EnforcerServer.allow?(ename, ["user", "data", "read"]) - end - - test "concurrent test 1", %{enforcer_name: ename} do - # Runs in parallel with other tests - EnforcerServer.add_policy(ename, {:p, ["user1", "resource1", "action"]}) - assert EnforcerServer.allow?(ename, ["user1", "resource1", "action"]) - end - - test "concurrent test 2", %{enforcer_name: ename} do - # Runs in parallel with other tests - EnforcerServer.add_policy(ename, {:p, ["user2", "resource2", "action"]}) - assert EnforcerServer.allow?(ename, ["user2", "resource2", "action"]) - end - end - - defmodule UsingTestHelper do - @moduledoc """ - ✅ Alternative: Using Casbin.TestHelper for more control. - - This approach gives you more flexibility while still ensuring isolation. - """ - use ExUnit.Case, async: true - - import Casbin.TestHelper - alias Casbin.{EnforcerSupervisor, EnforcerServer} - - @cfile "../../data/acl.conf" |> Path.expand(__DIR__) - - setup do - # Generate unique name and start enforcer manually - ename = unique_enforcer_name("test_helper_example") - {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile) - - # Register cleanup - on_exit(fn -> cleanup_enforcer(ename) end) - - {:ok, enforcer_name: ename} - end - - test "using test helper", %{enforcer_name: ename} do - EnforcerServer.add_policy(ename, {:p, ["alice", "data", "read"]}) - assert EnforcerServer.allow?(ename, ["alice", "data", "read"]) - end - end - - defmodule QuickExample do - @moduledoc """ - ✅ Convenience: Using create_test_enforcer for minimal boilerplate. - """ - use ExUnit.Case, async: true - - import Casbin.TestHelper - alias Casbin.EnforcerServer - - @cfile "../../data/acl.conf" |> Path.expand(__DIR__) - - test "quick test with minimal setup" do - # create_test_enforcer handles everything including automatic cleanup - {:ok, ename} = create_test_enforcer(@cfile, "quick") - - EnforcerServer.add_policy(ename, {:p, ["user", "resource", "action"]}) - assert EnforcerServer.allow?(ename, ["user", "resource", "action"]) - - # Cleanup happens automatically via on_exit registered by create_test_enforcer - end - end -end diff --git a/test/casbin/test_helper_test.exs b/test/casbin/test_helper_test.exs deleted file mode 100644 index 1c41725..0000000 --- a/test/casbin/test_helper_test.exs +++ /dev/null @@ -1,181 +0,0 @@ -defmodule Casbin.TestHelperTest do - use ExUnit.Case, async: true - - alias Casbin.{EnforcerSupervisor, EnforcerServer, TestHelper} - - @cfile "../../data/acl.conf" |> Path.expand(__DIR__) - @pfile "../../data/acl.csv" |> Path.expand(__DIR__) - - describe "unique_enforcer_name/1" do - test "generates unique names" do - name1 = TestHelper.unique_enforcer_name("test") - name2 = TestHelper.unique_enforcer_name("test") - - assert name1 != name2 - assert String.contains?(name1, "test") - assert String.contains?(name2, "test") - end - - test "uses default prefix when not provided" do - name = TestHelper.unique_enforcer_name() - assert String.contains?(name, "test") - end - - test "includes custom prefix" do - name = TestHelper.unique_enforcer_name("my_module") - assert String.contains?(name, "my_module") - end - - test "generates names suitable for concurrent tests" do - # Simulate multiple tests running in parallel - names = - 1..100 - |> Enum.map(fn _ -> - Task.async(fn -> TestHelper.unique_enforcer_name("concurrent") end) - end) - |> Enum.map(&Task.await/1) - - # All names should be unique - assert length(Enum.uniq(names)) == 100 - end - end - - describe "cleanup_enforcer/1" do - test "removes enforcer from ETS table" do - ename = TestHelper.unique_enforcer_name("cleanup_test") - {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile) - - # Verify it's in the ETS table - assert :ets.lookup(:enforcers_table, ename) != [] - - # Clean up - TestHelper.cleanup_enforcer(ename) - - # Verify it's removed - assert :ets.lookup(:enforcers_table, ename) == [] - end - - test "stops the enforcer process" do - ename = TestHelper.unique_enforcer_name("cleanup_process_test") - {:ok, pid} = EnforcerSupervisor.start_enforcer(ename, @cfile) - - assert Process.alive?(pid) - - # Clean up - TestHelper.cleanup_enforcer(ename) - - # Give it a moment to stop - Process.sleep(10) - - # Verify process is no longer registered - assert Registry.lookup(Casbin.EnforcerRegistry, ename) == [] - end - - test "handles non-existent enforcer gracefully" do - # Should not raise an error - assert :ok = TestHelper.cleanup_enforcer("non_existent_enforcer") - end - end - - describe "create_test_enforcer/2" do - test "creates enforcer and returns unique name" do - assert {:ok, ename} = TestHelper.create_test_enforcer(@cfile, "create_test") - - assert String.contains?(ename, "create_test") - - # Verify enforcer works - :ok = EnforcerServer.load_policies(ename, @pfile) - assert EnforcerServer.allow?(ename, ["alice", "blog_post", "read"]) == true - end - - test "uses default prefix when not provided" do - assert {:ok, ename} = TestHelper.create_test_enforcer(@cfile) - assert String.contains?(ename, "test") - end - - test "returns error for invalid config file" do - assert {:error, _reason} = TestHelper.create_test_enforcer("/invalid/path.conf", "test") - end - end - - describe "async test isolation" do - # These tests run in parallel to verify isolation - test "test 1 - isolated enforcer" do - ename = TestHelper.unique_enforcer_name("async_1") - {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile) - on_exit(fn -> TestHelper.cleanup_enforcer(ename) end) - - :ok = EnforcerServer.add_policy(ename, {:p, ["user1", "data1", "read"]}) - - # Verify policy exists in this enforcer - assert EnforcerServer.allow?(ename, ["user1", "data1", "read"]) == true - - # Should not affect other enforcers - policies = EnforcerServer.list_policies(ename, %{sub: "user1"}) - assert length(policies) == 1 - end - - test "test 2 - isolated enforcer" do - ename = TestHelper.unique_enforcer_name("async_2") - {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile) - on_exit(fn -> TestHelper.cleanup_enforcer(ename) end) - - :ok = EnforcerServer.add_policy(ename, {:p, ["user2", "data2", "write"]}) - - # Verify policy exists in this enforcer - assert EnforcerServer.allow?(ename, ["user2", "data2", "write"]) == true - - # Should not see user1's policies - policies = EnforcerServer.list_policies(ename, %{sub: "user1"}) - assert length(policies) == 0 - end - - test "test 3 - isolated enforcer" do - ename = TestHelper.unique_enforcer_name("async_3") - {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile) - on_exit(fn -> TestHelper.cleanup_enforcer(ename) end) - - :ok = EnforcerServer.add_policy(ename, {:p, ["user3", "data3", "delete"]}) - - # Verify policy exists in this enforcer - assert EnforcerServer.allow?(ename, ["user3", "data3", "delete"]) == true - - # Should not see other users' policies - assert EnforcerServer.list_policies(ename, %{sub: "user1"}) == [] - assert EnforcerServer.list_policies(ename, %{sub: "user2"}) == [] - end - end - - describe "stress test with many concurrent enforcers" do - test "handles many parallel enforcers" do - # Create and use multiple enforcers concurrently - tasks = - 1..50 - |> Enum.map(fn i -> - Task.async(fn -> - ename = TestHelper.unique_enforcer_name("stress_#{i}") - {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile) - - # Add a unique policy - :ok = EnforcerServer.add_policy(ename, {:p, ["user#{i}", "resource#{i}", "read"]}) - - # Verify it works - result = EnforcerServer.allow?(ename, ["user#{i}", "resource#{i}", "read"]) - - # Clean up - TestHelper.cleanup_enforcer(ename) - - {ename, result} - end) - end) - |> Enum.map(&Task.await(&1, 10_000)) - - # All should succeed - assert Enum.all?(tasks, fn {_name, result} -> result == true end) - - # All names should be unique - names = Enum.map(tasks, fn {name, _result} -> name end) - assert length(Enum.uniq(names)) == 50 - end - end -end From 0e12cac71dc7578f34cbc7ad45786d1a262c2a08 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 14:22:17 +0000 Subject: [PATCH 5/5] Remove test templates, keep only dynamic enforcer name utilities Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com> --- lib/casbin/async_case.ex | 208 -------------------------------------- lib/casbin/test_helper.ex | 45 --------- 2 files changed, 253 deletions(-) delete mode 100644 lib/casbin/async_case.ex diff --git a/lib/casbin/async_case.ex b/lib/casbin/async_case.ex deleted file mode 100644 index 9df1c0e..0000000 --- a/lib/casbin/async_case.ex +++ /dev/null @@ -1,208 +0,0 @@ -defmodule Casbin.AsyncCase do - @moduledoc """ - A test case template for async-safe Casbin tests. - - This module provides a convenient way to write tests that use Casbin enforcers - with `async: true`. It automatically handles enforcer isolation, preventing - race conditions that occur when multiple tests share the same enforcer instance. - - ## Problem - - When using EnforcerServer with a fixed name, all tests share the same global - state, making `async: true` tests fail with race conditions: - - ```elixir - # ❌ This fails with async: true - defmodule MyApp.PermissionsTest do - use ExUnit.Case, async: true - - test "test 1" do - EnforcerServer.add_policy("my_enforcer", {:p, ["user", "data", "read"]}) - assert EnforcerServer.allow?("my_enforcer", ["user", "data", "read"]) - end - - test "test 2" do - # Test 1's cleanup might delete policies while this test is running! - EnforcerServer.add_policy("my_enforcer", {:p, ["admin", "data", "write"]}) - assert EnforcerServer.allow?("my_enforcer", ["admin", "data", "write"]) - end - end - ``` - - ## Solution - - Use `Casbin.AsyncCase` to automatically create isolated enforcers per test: - - ```elixir - # ✅ This works with async: true - defmodule MyApp.PermissionsTest do - use Casbin.AsyncCase, async: true - - @config_file "path/to/casbin.conf" - - setup do - {:ok, enforcer_name: start_test_enforcer(@config_file)} - end - - test "test 1", %{enforcer_name: ename} do - EnforcerServer.add_policy(ename, {:p, ["user", "data", "read"]}) - assert EnforcerServer.allow?(ename, ["user", "data", "read"]) - end - - test "test 2", %{enforcer_name: ename} do - # This test has its own isolated enforcer! - EnforcerServer.add_policy(ename, {:p, ["admin", "data", "write"]}) - assert EnforcerServer.allow?(ename, ["admin", "data", "write"]) - end - end - ``` - - ## Using with EctoAdapter - - When using `EctoAdapter` with Ecto.SQL.Sandbox, you may need to allow the - enforcer process to access the database connection: - - ```elixir - defmodule MyApp.PermissionsTest do - use MyApp.DataCase, async: true - use Casbin.AsyncCase - - @config_file Application.app_dir(:my_app, "priv/casbin/model.conf") - - setup do - # Check out and allow enforcer to use the connection - :ok = Ecto.Adapters.SQL.Sandbox.checkout(MyApp.Repo) - - enforcer_name = start_test_enforcer(@config_file) - - # Allow enforcer process to access the database - case Registry.lookup(Casbin.EnforcerRegistry, enforcer_name) do - [{pid, _}] -> Ecto.Adapters.SQL.Sandbox.allow(MyApp.Repo, self(), pid) - [] -> :ok - end - - # Set up the adapter - adapter = Casbin.Persist.EctoAdapter.new(MyApp.Repo) - :ok = Casbin.EnforcerServer.set_persist_adapter(enforcer_name, adapter) - - {:ok, enforcer_name: enforcer_name} - end - - test "policies persist to database", %{enforcer_name: ename} do - :ok = EnforcerServer.add_policy(ename, {:p, ["user", "data", "read"]}) - :ok = EnforcerServer.save_policies(ename) - - # Verify it persisted - assert EnforcerServer.allow?(ename, ["user", "data", "read"]) - end - end - ``` - - ## Available Functions - - When you `use Casbin.AsyncCase`, the following functions become available: - - - `start_test_enforcer/1` - Starts an isolated enforcer for the current test - - `start_test_enforcer/2` - Starts an isolated enforcer with a custom prefix - - `unique_enforcer_name/1` - Generates a unique enforcer name - - These functions automatically handle cleanup via `on_exit/1`. - - ## Module Options - - You can pass options when using this module: - - ```elixir - use Casbin.AsyncCase, async: true, prefix: "my_test" - ``` - - Options: - - `async` - Passed to ExUnit.Case (default: true) - - `prefix` - Default prefix for enforcer names (default: module name) - - ## Implementation Details - - This module: - 1. Uses `ExUnit.Case` under the hood - 2. Imports `Casbin.TestHelper` for convenience functions - 3. Generates unique enforcer names using `:erlang.unique_integer/1` - 4. Automatically cleans up enforcers via `on_exit/1` callbacks - 5. Clears the ETS table state between tests - """ - - defmacro __using__(opts) do - # Extract async option (default to true for safety) - async = Keyword.get(opts, :async, true) - - # Extract prefix option (default to module name) - # Note: __CALLER__.module will be the test module using this macro - prefix = Keyword.get(opts, :prefix, nil) - - quote do - use ExUnit.Case, async: unquote(async) - import Casbin.TestHelper - - # Store the default prefix for this test module - @test_prefix unquote(prefix) || __MODULE__ |> to_string() |> String.replace("Elixir.", "") - - @doc """ - Starts a test enforcer with automatic isolation and cleanup. - - Creates a unique enforcer for the current test using the module's default - prefix and the provided config file. - - ## Parameters - - - `config_file` - Path to the Casbin configuration file - - ## Returns - - The unique enforcer name (string) that can be used with EnforcerServer functions. - - ## Examples - - setup do - {:ok, enforcer_name: start_test_enforcer("config/model.conf")} - end - - The enforcer is automatically cleaned up when the test exits. - """ - def start_test_enforcer(config_file) do - start_test_enforcer(@test_prefix, config_file) - end - - @doc """ - Starts a test enforcer with a custom prefix. - - Similar to `start_test_enforcer/1`, but allows specifying a custom prefix - for the enforcer name. - - ## Parameters - - - `prefix` - Custom prefix for the enforcer name - - `config_file` - Path to the Casbin configuration file - - ## Returns - - The unique enforcer name (string). - - ## Examples - - setup do - ename = start_test_enforcer("custom_prefix", "config/model.conf") - {:ok, enforcer_name: ename} - end - """ - def start_test_enforcer(prefix, config_file) do - ename = unique_enforcer_name(prefix) - - {:ok, _pid} = Casbin.EnforcerSupervisor.start_enforcer(ename, config_file) - - # Automatically clean up when test exits - on_exit(fn -> cleanup_enforcer(ename) end) - - ename - end - end - end -end diff --git a/lib/casbin/test_helper.ex b/lib/casbin/test_helper.ex index 57cf888..733b1f3 100644 --- a/lib/casbin/test_helper.ex +++ b/lib/casbin/test_helper.ex @@ -135,49 +135,4 @@ defmodule Casbin.TestHelper do :ok end end - - @doc """ - Creates a test enforcer with a unique name and automatic cleanup. - - This is a convenience function that combines `unique_enforcer_name/1`, - enforcer startup, and automatic cleanup. - - ## Parameters - - - `config_file` - Path to the Casbin configuration file - - `prefix` - Optional string prefix for the enforcer name (default: "test") - - ## Returns - - `{:ok, enforcer_name}` on success, or `{:error, reason}` on failure. - - ## Examples - - test "admin permissions" do - {:ok, ename} = Casbin.TestHelper.create_test_enforcer(config_path) - EnforcerServer.add_policy(ename, {:p, ["admin", "data", "read"]}) - assert EnforcerServer.allow?(ename, ["admin", "data", "read"]) - end - - test "with custom prefix" do - {:ok, ename} = Casbin.TestHelper.create_test_enforcer(config_path, "my_test") - # ... - end - - Note: This function automatically registers cleanup in `on_exit/1`, so you - don't need to manually clean up the enforcer. - """ - def create_test_enforcer(config_file, prefix \\ "test") do - ename = unique_enforcer_name(prefix) - - case Casbin.EnforcerSupervisor.start_enforcer(ename, config_file) do - {:ok, _pid} -> - # Register cleanup - ExUnit.Callbacks.on_exit(fn -> cleanup_enforcer(ename) end) - {:ok, ename} - - {:error, reason} -> - {:error, reason} - end - end end