diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d8765e..7acbdc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +# [Unreleased] + +### Features + +* add `Acx.TestHelper` module for async test isolation - enables `async: true` tests by providing unique enforcer names per test +* add comprehensive async testing guide at `guides/async_testing.md` with examples and migration instructions +* update README with testing section and link to async testing guide + # [1.5.0](https://github.com/casbin/casbin-ex/compare/v1.4.0...v1.5.0) (2025-11-01) diff --git a/README.md b/README.md index a5ca447..8bcfb4b 100644 --- a/README.md +++ b/README.md @@ -305,6 +305,28 @@ case EnforcerServer.allow?(ename, new_req) do end ``` +## Testing + +When writing tests with `async: true`, each test needs its own isolated enforcer instance to avoid race conditions. See the [Async Testing Guide](guides/async_testing.md) for detailed instructions on how to write async-safe tests. + +Quick example: + +```elixir +defmodule MyApp.PolicyTest do + use ExUnit.Case, async: true + import Acx.TestHelper + + setup do + setup_enforcer("path/to/config.conf") + end + + test "some test", %{enforcer_name: ename} do + Acx.EnforcerServer.add_policy(ename, {:p, ["alice", "data", "read"]}) + assert Acx.EnforcerServer.allow?(ename, ["alice", "data", "read"]) + end +end +``` + ## Supported Models Casbin-Ex supports the following access control models: diff --git a/examples/async_test_example.exs b/examples/async_test_example.exs new file mode 100644 index 0000000..1ad0c95 --- /dev/null +++ b/examples/async_test_example.exs @@ -0,0 +1,114 @@ +# Example: Writing async-safe tests with Acx.TestHelper +# +# This example demonstrates how to write tests that can run in parallel +# without interfering with each other. + +defmodule MyApp.PolicyTest do + use ExUnit.Case, async: true + import Acx.TestHelper + + alias Acx.EnforcerServer + + @cfile "../test/data/acl.conf" |> Path.expand(__DIR__) + + # Setup: Each test gets its own unique enforcer + setup do + # This creates a unique enforcer for this test + # and automatically cleans it up when the test completes + setup_enforcer(@cfile) + end + + test "admin can create blog posts", %{enforcer_name: ename} do + # Add a policy specific to this test + :ok = EnforcerServer.add_policy(ename, {:p, ["admin", "blog_post", "create"]}) + + # Verify the policy + assert EnforcerServer.allow?(ename, ["admin", "blog_post", "create"]) + refute EnforcerServer.allow?(ename, ["admin", "blog_post", "delete"]) + end + + test "user can read blog posts", %{enforcer_name: ename} do + # This test is completely isolated from the "admin" test above + # It has its own enforcer with no policies yet + :ok = EnforcerServer.add_policy(ename, {:p, ["user", "blog_post", "read"]}) + + assert EnforcerServer.allow?(ename, ["user", "blog_post", "read"]) + refute EnforcerServer.allow?(ename, ["user", "blog_post", "write"]) + end + + test "policies are isolated between tests", %{enforcer_name: ename} do + # This test starts with a clean slate + # It doesn't see policies from other tests + policies = EnforcerServer.list_policies(ename, %{}) + assert policies == [] + + # Add some policies + :ok = EnforcerServer.add_policy(ename, {:p, ["alice", "data1", "read"]}) + :ok = EnforcerServer.add_policy(ename, {:p, ["bob", "data2", "write"]}) + + # Verify they exist + policies = EnforcerServer.list_policies(ename, %{}) + assert length(policies) == 2 + end +end + +# Example with custom prefix for better debugging +defmodule MyApp.RbacTest do + use ExUnit.Case, async: true + import Acx.TestHelper + + alias Acx.EnforcerServer + + @cfile "../test/data/rbac.conf" |> Path.expand(__DIR__) + + setup do + # Use a custom prefix to identify these tests in logs + setup_enforcer("rbac_test", @cfile) + end + + test "role inheritance works", %{enforcer_name: ename} do + # The enforcer name will be something like "rbac_test_12345" + # which makes it easy to identify in logs + + # Add role mapping + :ok = EnforcerServer.add_mapping_policy(ename, {:g, "alice", "admin"}) + + # Add policy for admin role + :ok = EnforcerServer.add_policy(ename, {:p, ["admin", "data", "write"]}) + + # Alice should inherit admin permissions + assert EnforcerServer.allow?(ename, ["alice", "data", "write"]) + end +end + +# Example with manual setup for more control +defmodule MyApp.CustomSetupTest do + use ExUnit.Case, async: true + import Acx.TestHelper + + alias Acx.{EnforcerServer, EnforcerSupervisor} + + @cfile "../test/data/acl.conf" |> Path.expand(__DIR__) + @pfile "../test/data/acl.csv" |> Path.expand(__DIR__) + + setup do + # Manual setup gives you more control + ename = unique_enforcer_name("custom") + + {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile) + + # Load initial policies + EnforcerServer.load_policies(ename, @pfile) + + # Register cleanup + on_exit(fn -> cleanup_enforcer(ename) end) + + {:ok, enforcer_name: ename} + end + + test "works with pre-loaded policies", %{enforcer_name: ename} do + # The enforcer already has policies from acl.csv + policies = EnforcerServer.list_policies(ename, %{}) + assert length(policies) > 0 + end +end diff --git a/guides/async_testing.md b/guides/async_testing.md new file mode 100644 index 0000000..4b74e43 --- /dev/null +++ b/guides/async_testing.md @@ -0,0 +1,220 @@ +# Testing with Async Tests + +This guide explains how to write async-safe tests when using Casbin-Ex enforcers. + +## The Problem with Shared State + +When using `EnforcerServer` with a fixed enforcer name (e.g., `"my_enforcer"`), all tests that reference that name will share the same enforcer instance. This creates race conditions when using `async: true`: + +```elixir +defmodule MyApp.PolicyTest do + use ExUnit.Case, async: true # ❌ This will cause issues! + + @enforcer_name "my_enforcer" # Shared across all tests + + setup do + # This enforcer is shared by ALL tests + Acx.EnforcerSupervisor.start_enforcer(@enforcer_name, "config.conf") + :ok + end + + test "admin permissions" do + Acx.EnforcerServer.add_policy(@enforcer_name, {:p, ["admin", "data", "write"]}) + # Another test's cleanup might delete this policy mid-test! + assert Acx.EnforcerServer.allow?(@enforcer_name, ["admin", "data", "write"]) + end +end +``` + +**Symptoms of this problem:** +- `list_policies/2` returns `[]` even after adding policies +- `add_policy/2` returns `{:error, :already_existed}` but policies aren't visible +- Tests pass individually but fail when run together +- Tests fail non-deterministically + +## Solution 1: Use Acx.TestHelper (Recommended) + +The `Acx.TestHelper` module provides utilities to create isolated enforcer instances for each test: + +```elixir +defmodule MyApp.PolicyTest do + use ExUnit.Case, async: true # ✅ Safe with isolated enforcers + import Acx.TestHelper + + setup do + # Each test gets its own unique enforcer + setup_enforcer("path/to/config.conf") + end + + test "admin permissions", %{enforcer_name: ename} do + Acx.EnforcerServer.add_policy(ename, {:p, ["admin", "data", "write"]}) + assert Acx.EnforcerServer.allow?(ename, ["admin", "data", "write"]) + end + + test "user permissions", %{enforcer_name: ename} do + # Completely isolated from the "admin permissions" test + Acx.EnforcerServer.add_policy(ename, {:p, ["user", "data", "read"]}) + assert Acx.EnforcerServer.allow?(ename, ["user", "data", "read"]) + end +end +``` + +### Manual Setup with TestHelper + +If you need more control over the setup process: + +```elixir +defmodule MyApp.PolicyTest do + use ExUnit.Case, async: true + import Acx.TestHelper + + setup do + # Generate a unique name for this test's enforcer + ename = unique_enforcer_name() + + # Start the enforcer with the unique name + {:ok, _pid} = Acx.EnforcerSupervisor.start_enforcer(ename, "config.conf") + + # Load initial policies + Acx.EnforcerServer.load_policies(ename, "policies.csv") + + # Register cleanup to run after the test + on_exit(fn -> cleanup_enforcer(ename) end) + + {:ok, enforcer_name: ename} + end + + test "some test", %{enforcer_name: ename} do + # Use ename in your test + Acx.EnforcerServer.add_policy(ename, {:p, ["alice", "data", "read"]}) + assert Acx.EnforcerServer.allow?(ename, ["alice", "data", "read"]) + end +end +``` + +### Using Custom Name Prefixes + +For better test output and debugging, you can use custom prefixes: + +```elixir +setup do + # Name will be like "acl_test_12345" + setup_enforcer("acl_test", "path/to/config.conf") +end +``` + +Or manually: + +```elixir +setup do + ename = unique_enforcer_name("my_feature_test") + # ename will be something like "my_feature_test_12345" + ... +end +``` + +## Solution 2: Disable Async Tests + +If you have existing tests that are difficult to refactor, you can disable async testing: + +```elixir +defmodule MyApp.PolicyTest do + use ExUnit.Case, async: false # ✅ Tests run sequentially + + @enforcer_name "my_enforcer" + + # Rest of your existing test code... +end +``` + +**Trade-offs:** +- ✅ No code changes needed +- ✅ Tests still share state but run sequentially +- ❌ Tests run slower +- ❌ Doesn't scale well with many tests + +## Solution 3: Use Enforcer Directly (No Server) + +For pure unit tests that don't need the server functionality, use the `Acx.Enforcer` module directly: + +```elixir +defmodule MyApp.EnforcerLogicTest do + use ExUnit.Case, async: true # ✅ Safe, no shared state + + alias Acx.Enforcer + + setup do + {:ok, enforcer} = Enforcer.init("config.conf") + enforcer = Enforcer.load_policies!(enforcer, "policies.csv") + {:ok, enforcer: enforcer} + end + + test "policy evaluation", %{enforcer: e} do + # Each test gets its own enforcer struct + {:ok, e} = Enforcer.add_policy(e, {:p, ["alice", "data", "read"]}) + assert Enforcer.allow?(e, ["alice", "data", "read"]) + end +end +``` + +**Benefits:** +- ✅ No server overhead +- ✅ Fully isolated, immutable state +- ✅ Perfect for testing policy logic +- ❌ Can't test server-specific features +- ❌ No persistence layer interaction + +## Best Practices + +1. **Always use unique enforcer names** when testing with `EnforcerServer` +2. **Clean up after tests** using `on_exit` callbacks +3. **Use `Acx.TestHelper.setup_enforcer/1`** for simple cases +4. **Consider `Enforcer` directly** for pure unit tests +5. **Document your test setup** so other developers understand the pattern + +## Migration Guide + +If you have existing tests with shared state, here's how to migrate: + +### Before (Problematic) + +```elixir +defmodule MyApp.AclTest do + use ExUnit.Case, async: true + + @enforcer "my_app_enforcer" + + setup do + Acx.EnforcerSupervisor.start_enforcer(@enforcer, "acl.conf") + on_exit(fn -> + # This cleanup affects other running tests! + Acx.EnforcerServer.remove_policy(@enforcer, {:p, ["admin", "data", "write"]}) + end) + :ok + end +end +``` + +### After (Fixed) + +```elixir +defmodule MyApp.AclTest do + use ExUnit.Case, async: true + import Acx.TestHelper + + setup do + setup_enforcer("acl.conf") + end + + test "admin can write", %{enforcer_name: ename} do + Acx.EnforcerServer.add_policy(ename, {:p, ["admin", "data", "write"]}) + assert Acx.EnforcerServer.allow?(ename, ["admin", "data", "write"]) + end +end +``` + +## Additional Resources + +- See `test/test_helper_test.exs` for complete examples +- Review the `Acx.TestHelper` module documentation for all available functions +- Check the main README for general Casbin-Ex usage diff --git a/lib/acx/test_helper.ex b/lib/acx/test_helper.ex new file mode 100644 index 0000000..4993861 --- /dev/null +++ b/lib/acx/test_helper.ex @@ -0,0 +1,169 @@ +defmodule Acx.TestHelper do + @moduledoc """ + Test utilities for working with Casbin enforcers in async tests. + + This module provides helpers to create isolated enforcer instances for each test, + enabling `async: true` tests that don't interfere with each other. + + ## Usage + + In your test file: + + defmodule MyApp.CasbinTest do + use ExUnit.Case, async: true + import Acx.TestHelper + + setup do + # Creates a unique enforcer for this test + ename = unique_enforcer_name() + cfile = "path/to/config.conf" + + Acx.EnforcerSupervisor.start_enforcer(ename, cfile) + + # Automatically cleanup on test exit + on_exit(fn -> cleanup_enforcer(ename) end) + + {:ok, enforcer_name: ename} + end + + test "some test", %{enforcer_name: ename} do + Acx.EnforcerServer.add_policy(ename, {:p, ["alice", "data", "read"]}) + assert Acx.EnforcerServer.allow?(ename, ["alice", "data", "read"]) + end + end + + ## Alternative: Using setup_enforcer/1 + + For simpler setup, you can use the `setup_enforcer/1` function: + + defmodule MyApp.CasbinTest do + use ExUnit.Case, async: true + import Acx.TestHelper + + setup do + setup_enforcer("path/to/config.conf") + end + + test "some test", %{enforcer_name: ename} do + Acx.EnforcerServer.add_policy(ename, {:p, ["alice", "data", "read"]}) + assert Acx.EnforcerServer.allow?(ename, ["alice", "data", "read"]) + end + end + """ + + @doc """ + Generates a unique enforcer name for the current test. + + The name is based on the test's process ID and a timestamp, ensuring + that each test gets its own isolated enforcer instance. + + ## Examples + + iex> ename = Acx.TestHelper.unique_enforcer_name() + iex> is_binary(ename) + true + """ + @spec unique_enforcer_name() :: String.t() + def unique_enforcer_name do + "test_enforcer_#{:erlang.unique_integer([:positive])}" + end + + @doc """ + Generates a unique enforcer name with a custom prefix. + + ## Examples + + iex> ename = Acx.TestHelper.unique_enforcer_name("my_test") + iex> String.starts_with?(ename, "my_test_") + true + """ + @spec unique_enforcer_name(String.t()) :: String.t() + def unique_enforcer_name(prefix) when is_binary(prefix) do + "#{prefix}_#{:erlang.unique_integer([:positive])}" + end + + @doc """ + Cleans up an enforcer instance and removes it from the registry. + + This function should be called in an `on_exit` callback to ensure + proper cleanup after each test. + + ## Examples + + on_exit(fn -> cleanup_enforcer(ename) end) + """ + @spec cleanup_enforcer(String.t()) :: :ok + def cleanup_enforcer(ename) when is_binary(ename) do + # Try to stop the enforcer process if it exists + case Registry.lookup(Acx.EnforcerRegistry, ename) do + [{pid, _}] -> + # Stop the GenServer gracefully + if Process.alive?(pid) do + GenServer.stop(pid, :normal) + end + + [] -> + :ok + end + + # Clean up the ETS table entry + :ets.delete(:enforcers_table, ename) + + :ok + end + + @doc """ + Sets up a unique enforcer for the current test with automatic cleanup. + + This is a convenience function that combines `unique_enforcer_name/0`, + `Acx.EnforcerSupervisor.start_enforcer/2`, and automatic cleanup. + + Returns `{:ok, enforcer_name: ename}` which can be used directly in test setup. + + ## Examples + + setup do + setup_enforcer("path/to/config.conf") + end + + test "some test", %{enforcer_name: ename} do + # Use ename here + end + """ + @spec setup_enforcer(String.t()) :: {:ok, keyword()} + def setup_enforcer(cfile) when is_binary(cfile) do + ename = unique_enforcer_name() + + case Acx.EnforcerSupervisor.start_enforcer(ename, cfile) do + {:ok, _pid} -> + ExUnit.Callbacks.on_exit(fn -> cleanup_enforcer(ename) end) + {:ok, enforcer_name: ename} + + {:error, reason} -> + raise "Failed to start enforcer with config '#{cfile}': #{inspect(reason)}" + end + end + + @doc """ + Sets up a unique enforcer with a custom name prefix. + + ## Examples + + setup do + setup_enforcer("my_test", "path/to/config.conf") + end + """ + @spec setup_enforcer(String.t(), String.t()) :: {:ok, keyword()} + def setup_enforcer(prefix, cfile) when is_binary(prefix) and is_binary(cfile) do + ename = unique_enforcer_name(prefix) + + case Acx.EnforcerSupervisor.start_enforcer(ename, cfile) do + {:ok, _pid} -> + ExUnit.Callbacks.on_exit(fn -> cleanup_enforcer(ename) end) + {:ok, enforcer_name: ename} + + {:error, reason} -> + raise "Failed to start enforcer with config '#{cfile}': #{inspect(reason)}" + end + end +end diff --git a/test/test_helper_test.exs b/test/test_helper_test.exs new file mode 100644 index 0000000..832d43b --- /dev/null +++ b/test/test_helper_test.exs @@ -0,0 +1,107 @@ +defmodule Acx.TestHelperTest do + use ExUnit.Case, async: true + + import Acx.TestHelper + + alias Acx.{EnforcerServer, EnforcerSupervisor} + + @cfile "../data/acl.conf" |> Path.expand(__DIR__) + + describe "unique_enforcer_name/0" do + test "generates unique names" do + name1 = unique_enforcer_name() + name2 = unique_enforcer_name() + + assert is_binary(name1) + assert is_binary(name2) + assert name1 != name2 + assert String.starts_with?(name1, "test_enforcer_") + assert String.starts_with?(name2, "test_enforcer_") + end + end + + describe "unique_enforcer_name/1" do + test "generates unique names with custom prefix" do + name1 = unique_enforcer_name("my_test") + name2 = unique_enforcer_name("my_test") + + assert is_binary(name1) + assert is_binary(name2) + assert name1 != name2 + assert String.starts_with?(name1, "my_test_") + assert String.starts_with?(name2, "my_test_") + end + end + + describe "cleanup_enforcer/1" do + test "removes enforcer from registry and ETS" do + ename = unique_enforcer_name() + {:ok, _pid} = EnforcerSupervisor.start_enforcer(ename, @cfile) + + # Verify enforcer exists + assert :ets.lookup(:enforcers_table, ename) != [] + assert Registry.lookup(Acx.EnforcerRegistry, ename) != [] + + # Cleanup + cleanup_enforcer(ename) + + # Verify cleanup + assert :ets.lookup(:enforcers_table, ename) == [] + assert Registry.lookup(Acx.EnforcerRegistry, ename) == [] + end + + test "handles non-existent enforcer gracefully" do + assert :ok = cleanup_enforcer("non_existent_enforcer") + end + end + + describe "setup_enforcer/1" do + test "creates unique enforcer and returns it in context" do + {:ok, context} = setup_enforcer(@cfile) + + assert Keyword.has_key?(context, :enforcer_name) + ename = context[:enforcer_name] + + assert is_binary(ename) + assert Registry.lookup(Acx.EnforcerRegistry, ename) != [] + + # Cleanup + cleanup_enforcer(ename) + end + end + + describe "async test isolation" do + setup do + setup_enforcer(@cfile) + end + + test "test 1 can add and query policies independently", %{enforcer_name: ename} do + :ok = EnforcerServer.add_policy(ename, {:p, ["test1_user", "test1_data", "read"]}) + + policies = EnforcerServer.list_policies(ename, %{sub: "test1_user"}) + assert length(policies) == 1 + + assert EnforcerServer.allow?(ename, ["test1_user", "test1_data", "read"]) + refute EnforcerServer.allow?(ename, ["test2_user", "test2_data", "read"]) + end + + test "test 2 can add and query policies independently", %{enforcer_name: ename} do + :ok = EnforcerServer.add_policy(ename, {:p, ["test2_user", "test2_data", "write"]}) + + policies = EnforcerServer.list_policies(ename, %{sub: "test2_user"}) + assert length(policies) == 1 + + assert EnforcerServer.allow?(ename, ["test2_user", "test2_data", "write"]) + refute EnforcerServer.allow?(ename, ["test1_user", "test1_data", "read"]) + end + + test "test 3 starts with clean state", %{enforcer_name: ename} do + # Should not see policies from test 1 or test 2 + policies = EnforcerServer.list_policies(ename, %{}) + assert policies == [] + + refute EnforcerServer.allow?(ename, ["test1_user", "test1_data", "read"]) + refute EnforcerServer.allow?(ename, ["test2_user", "test2_data", "write"]) + end + end +end