Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -230,12 +230,14 @@ whenever the host's bootstrap happens to run it. This is also why `Absorber::reg
resolves nothing — registration at plugin-file scope is a shape a host is entitled to use, and it
would otherwise register into the throwaway.

**A duplicate slug is `Registry\Registrar::register()`'s exception, not `Absorber::register()`'s.** What
**A duplicate slug is `Registry\Registrar::register()`'s refusal, not `Absorber::register()`'s.** What
`Absorber::register()` throws is config validation, from the `Sub_Plugin` constructor, in the call
the host can see in its own stack trace. The buffer reaches the registrar at the first read —
`plugins_loaded` priority 5 on a request that passes the gatekeeper, priority 6 otherwise — so the
collision surfaces from inside a core action. Both are `Config_Exception`; only one of them can name
the line the host wrote.
the host can see in its own stack trace. A collision cannot be found there: the buffer reaches the
registrar at the first read — `plugins_loaded` priority 5 on a request that passes the gatekeeper,
priority 6 otherwise — long after both `register()` calls returned. So the registrar throws, and
`Registry\Reader::flush()` catches it per entry and reports it through `_doing_it_wrong()` naming the
registration that was discarded. The first registration under the slug stands, the second is dropped,
and everything registered behind it still reaches the registrar.

**The too-late barrier measures against the first step in the sequence, not the last.**
`Boot\Scheduler` compares the priority `plugins_loaded` is already dispatching against the lowest
Expand Down Expand Up @@ -284,12 +286,15 @@ constructed with rather than through the registrar they could resolve for themse
drains the pending registrations before it reads and a registrar asked directly would miss anything
registered since the last flush.

**Both passes also catch `Config_Exception` around that read.** A duplicate slug is only found when
the buffer reaches the registrar, which is a read — long after both `register()` calls returned — and
it arrives inside `plugins_loaded`, the hook that exists to prevent a fatal, so this is the last place
allowed to cause one. The conflict pass needs the guard more than the load pass, not less: its request
gate means the only requests reaching it are admin page views, so an escaping throw lands on exactly
the screens the mistaken registration would have to be corrected from.
**Neither pass guards that read, because the read no longer raises.** The one exception it used to
carry was the duplicate slug, and that is now refused and reported inside `Registry\Reader::flush()`,
where it is found. A guard at the read was the wrong altitude for it: the first pass to read caught
it and stood down whole — the load pass loading nothing at all on the front end, the conflict pass
resolving nothing in wp-admin — over a registry that was intact and readable the entire time. One
mistaken registration is one sub-plugin's problem and the sub-plugins around it still have to load.
What remains are the backstops that were always the right altitude for an unexpected throw: the
`Throwable` catch on each `plugins_loaded` step in `Boot\Scheduler`, and the per-sub-plugin catch
inside `Loader::load_all()` and `Conflict\Resolver::resolve_all()`.

The container is no longer the other half of that. A pass is handed a reader that already holds its
registrar, so a container that cannot supply one fails while the *pass* is being built — where an
Expand Down
6 changes: 4 additions & 2 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,10 @@ container that was never taught about this library. Set the container once, befo
Sub-plugins load in **registration order**, so register a dependency before anything that
extends it at include time, and register each slug exactly once. A config array the library
cannot use throws `Config_Exception` on the spot, in the call you can see in your own stack
trace; a duplicate slug is the exception that surfaces later, on `plugins_loaded`, since
registrations are buffered until the first read.
trace. A duplicate slug is found later, at the first read of the registry — normally on
`plugins_loaded` — since registrations are buffered until then: it is refused there and reported
through `_doing_it_wrong()`, the first registration under the slug stands, and the second is
discarded.

Register unconditionally and put anything you cannot decide up front — a licence, a setting the
site owner can change — in `enabled`, which is re-evaluated on every load. See
Expand Down
8 changes: 5 additions & 3 deletions docs/recipes.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,11 @@ foreach ( $sub_plugins as $slug => $constant ) {
```

An entry the library cannot use throws `Config_Exception` out of the `Absorber::register()` call it
is in, so a typo names itself in a stack trace pointing at your loop. A duplicate `slug` surfaces
later: registrations are buffered, and the collision is raised at the first read on
`plugins_loaded`.
is in, so a typo names itself in a stack trace pointing at your loop. A duplicate `slug` is found
later, at the first read of the registry — normally on `plugins_loaded` — because registrations are
buffered until then. It is refused there and reported through `_doing_it_wrong()`: the first
registration under the slug stands, the second is discarded, and every other sub-plugin loads as
normal.

## Choose a policy, and know what the site owner sees

Expand Down
3 changes: 1 addition & 2 deletions src/Absorber.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ public static function register( array $config ): void {
*
* @since 1.0.0
*
* @throws Config_Exception When no container has been set, or two sub-plugins were registered
* under one slug.
* @throws Config_Exception When no container has been set, or its binding is unusable.
*
* @return array<string,Sub_Plugin>
*/
Expand Down
14 changes: 0 additions & 14 deletions src/Boot/Scheduler.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
use Nexcess\PluginAbsorber\Conflict\Contracts\Resolver_Interface;
use Nexcess\PluginAbsorber\Conflict\Detector;
use Nexcess\PluginAbsorber\Conflict\Gatekeeper;
use Nexcess\PluginAbsorber\Exceptions\Config_Exception;
use Nexcess\PluginAbsorber\Loader;
use StellarWP\ContainerContract\ContainerInterface;
use Throwable;
Expand Down Expand Up @@ -219,19 +218,6 @@ private static function resolve_conflicts( ContainerInterface $container ): void
// its own cannot drop one by omission -- and asking them first means a resolver is built
// only on the request that goes on to use it.
$container->get( Resolver_Interface::class )->resolve_all();
} catch ( Config_Exception $exception ) {
// Reading the registry is where a duplicate slug surfaces, and this step reads it a
// priority ahead of the load pass that has always guarded the same read. Named separately
// from the catch below because it is the one failure here a developer can act on directly,
// and the message says which.
_doing_it_wrong(
self::class,
sprintf(
'The registered sub-plugins could not be read, so no conflict was resolved: %s',
$exception->getMessage()
),
'1.0.0'
);
} catch ( Throwable $thrown ) {
// The backstop, and the promise the whole library rests on: plugins_loaded fires on every
// request a site serves, so a throw out of a step is a white screen on all of them. What
Expand Down
5 changes: 1 addition & 4 deletions src/Conflict/Rewriter.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ public function __construct( Reader $registry ) {
*
* @param string $markup Notice markup WordPress is about to print.
*
* @throws Config_Exception When no hook prefix has been set, or two sub-plugins were registered
* under one slug.
* @throws Config_Exception When no hook prefix has been set.
*
* @return string
*/
Expand Down Expand Up @@ -156,8 +155,6 @@ public function rewrite( string $markup ): string {
*
* @param string $basename Standalone plugin basename named by the request.
*
* @throws Config_Exception When two sub-plugins were registered under one slug.
*
* @return Sub_Plugin|null
*/
private function find_by_standalone_basename( string $basename ): ?Sub_Plugin {
Expand Down
32 changes: 5 additions & 27 deletions src/Loader.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@ public function __construct(
/**
* @since 1.0.0
*
* @throws Config_Exception From loading a sub-plugin, which reads the hook prefix the guard
* above has already established is set.
*
* @return void
*/
public function load_all(): void {
Expand All @@ -82,30 +79,11 @@ public function load_all(): void {

// The reader rather than the registrar directly: it drains the registrations still buffered
// on the facade before it reads, and a registrar asked on its own would miss anything
// registered since the last read.
try {
$sub_plugins = $this->registry->all();
} catch ( Config_Exception $exception ) {
// The flush is where a duplicate slug is caught, and reading the registrar is where a
// missing container or an unusable binding is. All three are bootstrap mistakes, and
// all three arrive inside plugins_loaded: letting one out would fatal every request,
// front end and admin alike, and lock the developer out of the screen where the
// registration could be corrected. The hook this runs on exists to prevent a fatal, so
// it is the last place that may cause one -- the mistake is reported to the developer
// and the load is abandoned instead.
_doing_it_wrong(
self::class,
sprintf(
'The registered sub-plugins could not be read, so none were loaded: %s',
$exception->getMessage()
),
'1.0.0'
);

return;
}

foreach ( $sub_plugins as $sub_plugin ) {
// registered since the last read. Unguarded, because the read answers with whatever the
// registrar legitimately holds: a duplicate slug is refused and reported where it is found,
// so the sub-plugins around it still reach this loop rather than a host's one mistaken
// registration costing the site every bundled plugin it has.
foreach ( $this->registry->all() as $sub_plugin ) {
// Everything past this line is somebody else's code: the enabled and dependency_check
// callables, the host's should_load filter, and the bundled file itself, which a require
// runs from top to bottom. Any of it may throw, and this loop runs inside plugins_loaded
Expand Down
61 changes: 37 additions & 24 deletions src/Registry/Reader.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,12 @@ public static function buffer( Sub_Plugin $sub_plugin ): void {
* registered since the last read, a host registering from its own `plugins_loaded` callback
* included.
*
* @since 1.0.0
* A read always answers with what the registrar legitimately holds. A duplicate slug is refused
* and reported as it drains, never raised out of here: every caller is inside `plugins_loaded`,
* and one host bootstrap mistake about one sub-plugin must not stand down a pass that had every
* other sub-plugin to get on with.
*
* @throws Config_Exception When two sub-plugins were registered under one slug.
* @since 1.0.0
*
* @return array<string,Sub_Plugin>
*/
Expand Down Expand Up @@ -118,16 +121,25 @@ static function ( $sub_plugin ): bool {
* while this object is being built, with the registrations still buffered for the read that comes
* after the host has fixed its bindings.
*
* That same emptying is why a duplicate slug is caught per entry rather than allowed to end the
* loop. The registrar refuses the collision, and letting the throw out of the loop would leave
* every sub-plugin registered *behind* the colliding one in no registrar and in no buffer — the
* host would get a report naming the two that collided and silently lose the rest, on both
* passes, for the rest of the process. Registering the whole batch and throwing afterwards costs
* the collision nothing: it still surfaces from the read, where both passes catch it.
* A collision the registrar refuses is reported here, with the discarded registration named, and
* goes no further. Throwing it on made one mistaken registration decide what a whole pass did: the
* first pass to read caught it and stood down — the load pass loading nothing at all on the front
* end, the conflict pass resolving nothing in wp-admin — while the registry it was standing down
* over was intact and readable the entire time. A slug registered twice is one sub-plugin's
* problem, and the sub-plugins around it still have to load.
*
* @since 1.0.0
* Reported as it is discovered, which is once per process and therefore once per request, since
* registration runs at plugin-file scope on every one: the host sees it in the log for as long as
* the duplicate exists, and the load pass does not repeat a sentence the conflict pass has
* already printed a priority earlier in the same request. A registration that arrives after a
* read — a host module registering from its own `plugins_loaded` callback — is checked when it
* drains, so a later collision still reports.
*
* Every collision is reported, not just the first. They are separate mistakes naming separate
* slugs, and hiding the second behind the first only means the host fixes one and gets the next
* on the following request.
*
* @throws Config_Exception When two sub-plugins were registered under one slug.
* @since 1.0.0
*
* @return void
*/
Expand All @@ -140,25 +152,26 @@ protected function flush(): void {

self::$pending = [];

// The first collision, not the last, so that a buffer containing two of them reports the one
// the host wrote first and keeps reporting the same one until it is fixed. The exception is
// rethrown as the registrar raised it: it names the slug and both bundled files, which is the
// mistake the host has to go and correct, and what this method did with the rest of the batch
// is nothing they can act on.
$collision = null;

foreach ( $pending as $sub_plugin ) {
try {
$this->registrar->register( $sub_plugin );
} catch ( Config_Exception $exception ) {
if ( $collision === null ) {
$collision = $exception;
}
// The registrar's own sentence, unwrapped: it names the slug and both bundled files,
// which is the whole of what the host has to go and correct. One clause is added,
// because the registrar refuses a registration without saying what became of it, and
// what became of it is now the consequence -- the site runs one of those two files
// and silently does not run the other. Every other report in this library says what
// the outcome was; this one has to as well.
_doing_it_wrong(
self::class,
sprintf(
'%1$s The registration already held was kept; %2$s was discarded.',
$exception->getMessage(),
$sub_plugin->get_bundled_plugin_file()
),
'1.0.0'
);
}
}

if ( $collision !== null ) {
throw $collision;
}
}
}
23 changes: 12 additions & 11 deletions tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -584,13 +584,14 @@ sequenceDiagram
```

**A duplicate slug is reported, and what was registered behind it still loads.**
The collision is the registrar's exception and it is raised long after both
The collision is the registrar's refusal and it is found long after both
`Absorber::register()` calls returned, from inside `plugins_loaded` — the hook
this library exists to keep a site off the floor on. Both passes guard the read,
so the conflict pass reports it and the load pass, finding the buffer already
drained, gets on with the load. The whole batch is registered before the
collision is rethrown, which is what keeps a host from silently losing every
sub-plugin it registered after the mistake.
this library exists to keep a site off the floor on. So it is reported where it
is found and nothing is raised out of the read: the conflict pass reports it and
resolves what it has, and the load pass behind it loads the registry that read
left standing. Only the colliding entry is refused, which is what keeps a host
from silently losing every sub-plugin it registered after the mistake — or,
back when the read still threw, every sub-plugin it registered at all.

```mermaid
sequenceDiagram
Expand All @@ -604,8 +605,8 @@ sequenceDiagram
Note over R: first read — the conflict pass, priority 5
R->>Reg: A, then A again, then B
Reg-->>R: the second A collides
R-->>R: whole batch registered, the first collision rethrown after it
Note over R: the pass reports it and abandons its own step
R-->>R: the second A refused and reported; A and B kept
Note over R: the pass carries on with the registry it has
Note over R: second read — priority 6, buffer already drained
R-->>L: A and B
L->>L: both load
Expand Down Expand Up @@ -707,9 +708,9 @@ sequenceDiagram
**The request after a deactivation does not loop.** The failure mode a merge
notice queued on every request would produce: a redirect loop, or a screen
reporting the same deactivation for ever. Nothing is re-registered between the
two requests — a duplicate slug throws — because this is the next page view, not
a second bootstrap. The second request must *not* halt, and the helper fails the
test if it does.
two requests — a duplicate slug is refused — because this is the next page view,
not a second bootstrap. The second request must *not* halt, and the helper fails
the test if it does.

```mermaid
sequenceDiagram
Expand Down
28 changes: 20 additions & 8 deletions tests/unit/AbsorberTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -373,10 +373,13 @@ public function test_reading_twice_does_not_register_twice(): void {

/**
* Deferring registration moves the duplicate-slug report from the second register() call to the
* first read. It still names both bundled files, which is what the host needs to find them.
* first read, where it is reported rather than raised: what a host asks for here is the list of
* sub-plugins, and the second registration under a slug is no reason to hand back none of them.
* The report still names both bundled files, which is what the host needs to find them.
*/
public function test_a_duplicate_slug_is_refused_at_the_first_read(): void {
$this->set_up_container();
$this->expect_incorrect_usage();

Absorber::register( $this->sub_plugin_config( 'give-recurring' ) );
Absorber::register(
Expand All @@ -387,13 +390,22 @@ public function test_a_duplicate_slug_is_refused_at_the_first_read(): void {
]
);

try {
Absorber::all();
$this->fail( 'Expected a Config_Exception.' );
} catch ( Config_Exception $exception ) {
$this->assertStringContainsString( 'give-recurring', $exception->getMessage() );
$this->assertStringContainsString( '/tmp/other/other.php', $exception->getMessage() );
}
$all = Absorber::all();

$this->assertSame( [ 'give-recurring' ], array_keys( $all ) );
$this->assertSame(
'/tmp/give-recurring/give-recurring.php',
$all['give-recurring']->get_bundled_plugin_file(),
'The registration that arrived first under a slug is the one that stands.'
);
$this->assert_the_library_reported_incorrect_usage_saying(
'Two sub-plugins are registered under the slug "give-recurring"',
'The collision is what failed, and the report has to say so rather than name some other gate.'
);
$this->assert_the_library_reported_incorrect_usage_saying(
'/tmp/other/other.php',
'The report has to name the registration that lost, or the host cannot find it.'
);
}

public function test_all_is_empty_before_anything_is_registered(): void {
Expand Down
Loading
Loading