From 5e876dc8b1f9806d909102a36773828776d8b2c9 Mon Sep 17 00:00:00 2001 From: Tony Winn Date: Fri, 10 Apr 2026 15:25:21 -0400 Subject: [PATCH] Rewrite documentation for professional tone, enhance CI matrix --- .github/workflows/ci.yml | 73 +++++++++++++++++++++++------- README.md | 94 ++++++++++++++++++++------------------- lib/enclave.ex | 65 ++++++++++++++------------- lib/enclave/dispatcher.ex | 29 ++++++------ lib/enclave/owners.ex | 8 ++-- 5 files changed, 158 insertions(+), 111 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 24f4f0b..e3807a7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,12 +6,46 @@ on: pull_request: branches: [main] +permissions: + contents: read + jobs: - test: - name: Elixir ${{ matrix.elixir }} / OTP ${{ matrix.otp }} + format: + name: Format + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: erlef/setup-beam@v1 + with: + elixir-version: "1.17" + otp-version: "27" + - run: mix deps.get + - run: mix format --check-formatted + + credo: + name: Credo runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: erlef/setup-beam@v1 + with: + elixir-version: "1.17" + otp-version: "27" + - uses: actions/cache@v4 + with: + path: | + deps + _build + key: ${{ runner.os }}-mix-credo-${{ hashFiles('mix.lock') }} + restore-keys: ${{ runner.os }}-mix-credo- + - run: mix deps.get + - run: mix credo --strict + test: + name: Test (Elixir ${{ matrix.elixir }} / OTP ${{ matrix.otp }}) + runs-on: ubuntu-latest strategy: + fail-fast: false matrix: include: - elixir: "1.15" @@ -20,34 +54,39 @@ jobs: otp: "27" - elixir: "1.19" otp: "28" - steps: - uses: actions/checkout@v4 - - uses: erlef/setup-beam@v1 with: elixir-version: ${{ matrix.elixir }} otp-version: ${{ matrix.otp }} - - uses: actions/cache@v4 with: path: | deps _build - key: ${{ runner.os }}-mix-${{ matrix.elixir }}-${{ matrix.otp }}-${{ hashFiles('mix.lock') }} - restore-keys: ${{ runner.os }}-mix-${{ matrix.elixir }}-${{ matrix.otp }}- + key: ${{ runner.os }}-mix-test-${{ matrix.elixir }}-${{ matrix.otp }}-${{ hashFiles('mix.lock') }} + restore-keys: ${{ runner.os }}-mix-test-${{ matrix.elixir }}-${{ matrix.otp }}- + - run: mix deps.get + - run: mix test - # Dialyzer PLT cache — keyed on the Elixir/OTP pair and mix.lock - # so it rebuilds only when deps change. Saves ~3 minutes per run. + dialyzer: + name: Dialyzer + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: erlef/setup-beam@v1 + with: + elixir-version: "1.17" + otp-version: "27" - uses: actions/cache@v4 with: - path: priv/plts - key: ${{ runner.os }}-plt-${{ matrix.elixir }}-${{ matrix.otp }}-${{ hashFiles('mix.lock') }} - restore-keys: ${{ runner.os }}-plt-${{ matrix.elixir }}-${{ matrix.otp }}- - + path: | + deps + _build + priv/plts + key: ${{ runner.os }}-mix-dialyzer-${{ hashFiles('mix.lock') }} + restore-keys: ${{ runner.os }}-mix-dialyzer- - run: mix deps.get - - run: mix compile --warnings-as-errors - - run: mix format --check-formatted - - run: mix credo --strict + - run: mkdir -p priv/plts - run: mix dialyzer --format github - - run: mix test diff --git a/README.md b/README.md index 9ffbef2..2e38b02 100644 --- a/README.md +++ b/README.md @@ -1,31 +1,35 @@ # Enclave [![CI](https://github.com/twinn/enclave/actions/workflows/ci.yml/badge.svg)](https://github.com/twinn/enclave/actions/workflows/ci.yml) +[![Hex.pm](https://img.shields.io/hexpm/v/enclave.svg)](https://hex.pm/packages/enclave) +[![Docs](https://img.shields.io/badge/hex-docs-blue.svg)](https://hexdocs.pm/enclave) Process-ancestry-based sandboxing for `Phoenix.PubSub`. -Enclave lets async tests share a single `Phoenix.PubSub` instance without -leaking broadcasts between tests. It extends the Ecto SQL sandbox's -ownership model — walking `$callers` and `$ancestors` to find the "owning" -test process — to PubSub delivery. A broadcast is only delivered to -subscribers whose enclave matches the publisher's. +Enclave allows concurrent (async) tests to share a single `Phoenix.PubSub` +instance without leaking broadcasts between test processes. It extends the +Ecto SQL sandbox ownership model -- walking `$callers` and `$ancestors` to +find the owning test process -- to PubSub delivery. A broadcast is delivered +only to subscribers whose enclave matches the publisher's. -## Why +## Motivation -When you test a LiveView (or anything subscribing to `Phoenix.PubSub`) with +When testing a LiveView (or any process subscribing to `Phoenix.PubSub`) with `async: true`: 1. Test A mounts a LiveView that subscribes to `"user:#{id}"`. 2. Test B, running concurrently, updates a user and broadcasts. -3. Test A's LiveView receives Test B's message — a flaky test, or worse, a - test that exits while downstream DB work is still in flight. +3. Test A's LiveView receives Test B's message, producing a flaky test or + a test that exits while downstream database work is still in flight. -The Ecto SQL sandbox solves the *database* half of this by scoping -connections to an owning test process. Enclave does the same thing for +The Ecto SQL sandbox solves the database half of this problem by scoping +connections to an owning test process. Enclave applies the same approach to PubSub delivery. ## Installation +Add `:enclave` to the list of dependencies in `mix.exs`: + ```elixir def deps do [ @@ -36,9 +40,10 @@ end ## Usage -### 1. Wrap your PubSub +### 1. Wrap the PubSub module -Most Phoenix apps start `Phoenix.PubSub` directly in their supervision tree: +Most Phoenix applications start `Phoenix.PubSub` directly in their +supervision tree: ```elixir # Before @@ -48,8 +53,8 @@ children = [ ] ``` -Replace it with a thin wrapper module that injects `Enclave.Dispatcher` in -test env only: +Replace it with a wrapper module that injects `Enclave.Dispatcher` in the +test environment: ```elixir # lib/my_app/pub_sub.ex @@ -73,7 +78,7 @@ defmodule MyApp.PubSub do end ``` -Then in your application and endpoint config: +Then update the application supervision tree and endpoint configuration: ```elixir # lib/my_app/application.ex @@ -87,10 +92,10 @@ config :my_app, MyAppWeb.Endpoint, pubsub_server: MyApp.PubSub ``` -In production `@dispatcher` is `Phoenix.PubSub` (the default), so the -wrapper compiles away to a plain pass-through — zero runtime overhead. +In production, `@dispatcher` is `Phoenix.PubSub` (the default), so the +wrapper compiles to a plain pass-through with no runtime overhead. -### 2. Mark your test as an enclave owner +### 2. Register the test process as an enclave owner ```elixir defmodule MyAppWeb.UserLiveTest do @@ -109,51 +114,48 @@ defmodule MyAppWeb.UserLiveTest do end ``` -That's it. A concurrently running test broadcasting to `"user:1"` in its -own enclave will not be seen by this test's LiveView. +A concurrently running test broadcasting to `"user:1"` from its own enclave +will not be delivered to this test's LiveView. ### 3. (Optional) Allow background processes -If your test hands work to a worker that wasn't spawned from the test -process (e.g. a globally-registered GenServer), explicitly allow it: +If a test delegates work to a process that was not spawned from the test +process (for example, a globally registered GenServer), explicitly allow it: ```elixir :ok = Enclave.start_owner() :ok = Enclave.allow(self(), Process.whereis(MyApp.Worker)) ``` -## How ownership is resolved +## Ownership resolution -Given any pid, Enclave finds its owner by checking in order: +Given any pid, Enclave resolves its owner by checking in order: -1. **Direct registration** — the pid called `start_owner/0`, or was named in +1. **Direct registration** -- the pid called `start_owner/0`, or was named in an `allow/2` call. -2. **`$callers`** — the process dictionary key that `Task`, `GenServer`, +2. **`$callers`** -- the process dictionary key that `Task`, `GenServer`, and Phoenix propagate to track caller chains. -3. **`$ancestors`** — the OTP-managed chain set by `:proc_lib` spawning. +3. **`$ancestors`** -- the OTP-managed chain set by `:proc_lib` spawning. -If nothing matches, the pid is `:no_owner`. Two pids are deliverable if -they resolve to the same owner — *including* both resolving to `:no_owner`, -which is what makes the wrapper a no-op in production. +If no match is found, the pid resolves to `:no_owner`. Two pids are +deliverable to each other if they resolve to the same owner, including both +resolving to `:no_owner`. This property is what makes the wrapper a no-op in +production. ## Limitations -- **Phoenix internals bypass the filter.** LiveView's channel push - fan-out, `Endpoint.broadcast/3`, and other framework-level broadcasts - call `Phoenix.PubSub.broadcast/4` with their own dispatcher. Only - broadcasts that go through `Enclave.Dispatcher` (i.e. your app's own - code going through the wrapper) get filtered. For most test suites this - is exactly the broadcast path that causes flake, but it's worth - knowing. +- **Phoenix internals bypass the filter.** LiveView channel push fan-out, + `Endpoint.broadcast/3`, and other framework-level broadcasts call + `Phoenix.PubSub.broadcast/4` with their own dispatcher. Only broadcasts + routed through `Enclave.Dispatcher` (via the application wrapper) are + filtered. For most test suites this covers the broadcast path that + causes flaky tests. - **Single-node only.** Cross-node broadcasts delegate to the configured - PubSub adapter, which is out of Enclave's reach. Don't use this in - clustered tests. -- **Requires a wrapper.** `Phoenix.PubSub` doesn't support a configurable - default dispatcher, so there's no way to enable filtering without - routing broadcasts through a module you control. An upstream PR - adding `config :phoenix_pubsub, default_dispatcher:` would let this - library drop the wrapper entirely. + PubSub adapter, which is outside of Enclave's scope. +- **Requires a wrapper module.** `Phoenix.PubSub` does not support a + configurable default dispatcher, so there is no way to enable filtering + without routing broadcasts through a module the application controls. ## License -MIT — see [LICENSE](LICENSE). +MIT -- see [LICENSE](LICENSE). diff --git a/lib/enclave.ex b/lib/enclave.ex index 829a0c3..7c830e5 100644 --- a/lib/enclave.ex +++ b/lib/enclave.ex @@ -1,20 +1,19 @@ defmodule Enclave do @moduledoc """ - Process-ancestry-based sandboxing for Phoenix.PubSub (and any pid-keyed - fan-out mechanism). + Provides process-ancestry-based sandboxing for `Phoenix.PubSub`. - Enclave lets async tests share a single Phoenix.PubSub instance without - leaking broadcasts between tests. Each test process registers itself as an - *owner*; processes spawned from (or explicitly allowed by) an owner are - members of that owner's *enclave*. A broadcast is only delivered to - subscribers whose enclave matches the publisher's. + `Enclave` allows concurrent (async) tests to share a single `Phoenix.PubSub` + instance without leaking broadcasts between test processes. Each test process + registers itself as an *owner* via `start_owner/0`. Processes spawned from, or + explicitly allowed by, an owner belong to that owner's *enclave*. A broadcast + is delivered only to subscribers whose enclave matches the publisher's. - ## Basic usage + ## Usage # In test setup: :ok = Enclave.start_owner() - # In your app's PubSub wrapper (test env only), pass Enclave.Dispatcher: + # In the application's PubSub wrapper (test env only), pass Enclave.Dispatcher: Phoenix.PubSub.broadcast(MyApp.PubSub, topic, msg, Enclave.Dispatcher) See `Enclave.Dispatcher` for integration details. @@ -25,21 +24,27 @@ defmodule Enclave do @type owner_result :: {:ok, pid} | :no_owner @doc """ - Register `self()` as an enclave owner. + Registers the calling process as an enclave owner. + + Returns `:ok` if registration succeeds, or `{:error, :already_registered}` + if the process is already registered. """ @spec start_owner() :: :ok | {:error, :already_registered} def start_owner, do: Owners.register_owner(self()) @doc """ - Unregister `self()` as an enclave owner. Removes all allowances pointing to - this process. + Unregisters the calling process as an enclave owner. + + Removes all allowances associated with this process. """ @spec stop_owner() :: :ok def stop_owner, do: Owners.unregister_owner(self()) @doc """ - Explicitly associate `allowed` with `owner`'s enclave. `owner` must already - be a registered owner. + Associates `allowed` with `owner`'s enclave. + + The `owner` must already be a registered owner. Returns `:ok` on success, + or `{:error, :not_an_owner}` if `owner` is not registered. """ @spec allow(pid, pid) :: :ok | {:error, :not_an_owner} def allow(owner, allowed) when is_pid(owner) and is_pid(allowed) do @@ -47,20 +52,19 @@ defmodule Enclave do end @doc """ - Resolve a pid to its enclave owner. + Resolves a pid to its enclave owner. - Resolution order: + Checks the following sources in order: - 1. Direct registration (explicit `start_owner`/`allow`). - 2. `$callers` chain from the process dictionary. - 3. `$ancestors` chain from the process dictionary. + 1. Direct registration via `start_owner/0` or `allow/2`. + 2. The `$callers` chain in the process dictionary. + 3. The `$ancestors` chain in the process dictionary. - Returns `{:ok, owner_pid}` or `:no_owner`. + Returns `{:ok, owner_pid}` if an owner is found, or `:no_owner` otherwise. ## Examples - A bare `spawn/1` process has no OTP ancestry and no registration, so it - is always `:no_owner`: + A process created with `spawn/1` has no OTP ancestry and no registration: iex> pid = spawn(fn -> Process.sleep(:infinity) end) iex> Enclave.owner(pid) @@ -75,20 +79,19 @@ defmodule Enclave do end @doc """ - Should a message from `from_pid` be delivered to `to_pid`? + Determines whether a message from `from_pid` should be delivered to `to_pid`. - The rule is simple: both pids must resolve to the same owner, OR both must - resolve to `:no_owner`. Everything else is dropped. This means: + Returns `true` if both pids resolve to the same owner, including both + resolving to `:no_owner`. Returns `false` otherwise. This means: - * same-test deliveries pass - * pure-production (unowned) deliveries pass - * cross-test deliveries are dropped - * test → production and production → test are dropped + * Same-enclave deliveries are permitted. + * Unowned-to-unowned deliveries are permitted (the production path). + * Cross-enclave deliveries are filtered. + * Owned-to-unowned and unowned-to-owned deliveries are filtered. ## Examples - Two processes with no enclave resolve to `:no_owner`, so they are - deliverable (this is what makes the wrapper a no-op in production): + Two unowned processes are deliverable to each other: iex> a = spawn(fn -> Process.sleep(:infinity) end) iex> b = spawn(fn -> Process.sleep(:infinity) end) diff --git a/lib/enclave/dispatcher.ex b/lib/enclave/dispatcher.ex index e089a72..4afcdea 100644 --- a/lib/enclave/dispatcher.ex +++ b/lib/enclave/dispatcher.ex @@ -1,16 +1,17 @@ defmodule Enclave.Dispatcher do @moduledoc """ A `Phoenix.PubSub` dispatcher that filters subscribers through - `Enclave.deliverable?/2` before sending. + `Enclave.deliverable?/2` before delivery. - Phoenix.PubSub accepts a dispatcher module as the last argument to - `broadcast/4`, `broadcast_from/5`, `local_broadcast/4`, etc. Passing - `Enclave.Dispatcher` causes only subscribers in the same enclave as the - publishing process to receive the message. + `Phoenix.PubSub` accepts a dispatcher module as the last argument to + `broadcast/4`, `broadcast_from/5`, and `local_broadcast/4`. Passing + `Enclave.Dispatcher` restricts delivery to subscribers that belong to + the same enclave as the publishing process. ## Usage - Typically you wire this in once via a thin wrapper module in your host app: + A thin wrapper module in the host application conditionally injects + the dispatcher in test: defmodule MyApp.PubSub do @dispatcher (if Mix.env() == :test, do: Enclave.Dispatcher, else: Phoenix.PubSub) @@ -24,18 +25,20 @@ defmodule Enclave.Dispatcher do do: Phoenix.PubSub.broadcast(__MODULE__, topic, msg, @dispatcher) end - In production `@dispatcher` is `Phoenix.PubSub` (the default), so this - indirection compiles away to the same code you would have written by hand. + In production, `@dispatcher` is `Phoenix.PubSub` (the default), so the + wrapper compiles to a plain pass-through with no runtime overhead. """ @doc """ - Called by `Phoenix.PubSub` via `Registry.dispatch/3`. Filters `entries` by - `Enclave.deliverable?/2` and sends the message to survivors. + Dispatches `message` to the given `entries`, filtering by enclave ownership. - Matches Phoenix.PubSub's own default dispatch semantics: + Called by `Phoenix.PubSub` via `Registry.dispatch/3`. Each entry is checked + with `Enclave.deliverable?/2`; only subscribers in the same enclave as the + source process receive the message. - * `from == :none` → deliver to every (allowed) subscriber - * `from == pid` → deliver to every (allowed) subscriber except `from` + When `from` is `:none`, all enclave-matching subscribers receive the message. + When `from` is a pid, that pid is additionally excluded from delivery + (matching the default `Phoenix.PubSub` broadcast-from semantics). """ @spec dispatch([{pid, term}], pid | :none, term) :: :ok def dispatch(entries, :none, message) do diff --git a/lib/enclave/owners.ex b/lib/enclave/owners.ex index f717ace..881d039 100644 --- a/lib/enclave/owners.ex +++ b/lib/enclave/owners.ex @@ -21,25 +21,25 @@ defmodule Enclave.Owners do GenServer.start_link(__MODULE__, nil, name: __MODULE__) end - @doc "Register `pid` as an owner. Idempotent failure: returns `{:error, :already_registered}`." + @doc "Registers `pid` as an owner. Returns `{:error, :already_registered}` if already registered." def register_owner(pid) when is_pid(pid) do GenServer.call(__MODULE__, {:register_owner, pid}) end - @doc "Unregister `pid` as an owner and remove all entries pointing to it." + @doc "Unregisters `pid` as an owner and removes all entries pointing to it." def unregister_owner(pid) when is_pid(pid) do GenServer.call(__MODULE__, {:unregister_owner, pid}) end @doc """ - Explicitly associate `allowed` with `owner`. `owner` must already be registered. + Associates `allowed` with `owner`'s enclave. `owner` must already be registered. """ def allow(owner, allowed) when is_pid(owner) and is_pid(allowed) do GenServer.call(__MODULE__, {:allow, owner, allowed}) end @doc """ - Direct ETS lookup — no ancestry walking. Returns `{:ok, owner_pid}` or `:error`. + Performs a direct ETS lookup without ancestry walking. Returns `{:ok, owner_pid}` or `:error`. """ def lookup(pid) when is_pid(pid) do case :ets.lookup(@table, pid) do