From 2c4b8690ae9e6972c9f9a169a2a9dee6af72e91c Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Mon, 24 Aug 2026 15:11:37 +0200 Subject: [PATCH 1/5] Hand the registrar accessor a registry that agrees with all() --- docs/extending.md | 4 ++- src/Absorber.php | 22 +++++++++++- tests/unit/AbsorberTest.php | 72 +++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/docs/extending.md b/docs/extending.md index cd2bc30..471aa6c 100644 --- a/docs/extending.md +++ b/docs/extending.md @@ -226,7 +226,9 @@ container cannot build at all is reported the same way and not raised at you raw is caught and wrapped in a `Config_Exception` that names the id, keeping the original as `getPrevious()`. `Absorber::boot()` is the one that does not wrap — a container that cannot build the provider or the scheduler throws its own exception out of your `boot()` call. Past the check, -`Absorber::all()` also drops anything a rebound registrar returns that is not a `Sub_Plugin`. +`Absorber::all()` also drops anything a rebound registrar returns that is not a `Sub_Plugin`, and +`Absorber::registrar()` hands back a registrar with every registration made so far already in it, so +the two answer alike whenever you ask. `Absorber::resolver()` hands you the resolver, not the gates in front of it: `resolve_all()` re-checks neither the request gate nor the capability gate, because the conflict step has already asked both by diff --git a/src/Absorber.php b/src/Absorber.php index 65b318f..4c2cf62 100644 --- a/src/Absorber.php +++ b/src/Absorber.php @@ -43,6 +43,19 @@ final class Absorber { private static $booted = false; /** + * The registrar, holding every registration made so far. + * + * Drained on the way past, like `all()` and for the same reason: registration is buffered until + * something reads it, so a registrar handed over as it is holds nothing at all until the first + * pass reads at plugins_loaded priority 5 — which is after every point a host bootstrap gets to + * ask. The two public reads of the registry would then disagree, one of them against the + * contract `Registrar_Interface::all()` states, and neither would say so. + * + * Drained *after* the binding has been resolved and checked, not before. `Registry\Reader` takes + * a registrar as a constructor argument, so a registrar bound to the wrong class is a reader + * that cannot be built either — and a drain in front would report the reader, a collaborator the + * host never bound, in place of the one binding it did get wrong. + * * @since 1.0.0 * * @throws Config_Exception When no container has been set, or its binding is unusable. @@ -50,7 +63,14 @@ final class Absorber { * @return Registrar_Interface */ public static function registrar(): Registrar_Interface { - return self::collaborator( Registrar_Interface::class ); + $registrar = self::collaborator( Registrar_Interface::class ); + + // Read for the drain rather than for the list: handing the pending registrations over is + // what `Registry\Reader::all()` does on its way to the registrar, and the list it comes back + // with is what `all()` exists to give a host. + self::collaborator( Reader::class )->all(); + + return $registrar; } /** diff --git a/tests/unit/AbsorberTest.php b/tests/unit/AbsorberTest.php index f54f68f..b3eec00 100644 --- a/tests/unit/AbsorberTest.php +++ b/tests/unit/AbsorberTest.php @@ -19,6 +19,7 @@ use Nexcess\PluginAbsorber\Notices\Writer; use Nexcess\PluginAbsorber\Provider; use Nexcess\PluginAbsorber\Registry\Contracts\Registrar_Interface; +use Nexcess\PluginAbsorber\Registry\Reader; use Nexcess\PluginAbsorber\Registry\Registrar; use Nexcess\PluginAbsorber\Sub_Plugin; use Nexcess\PluginAbsorber\Tests\Support\Absorber_State; @@ -288,6 +289,77 @@ public static function container_binding_methods(): Generator { yield 'bind' => [ 'bind' ]; } + /** + * The two public reads of the registry have to answer alike. Registration is buffered until + * something reads it, so a registrar handed over undrained holds nothing at all until the first + * pass reads at plugins_loaded priority 5 — while its own contract promises every registered + * sub-plugin, in registration order, and `Absorber::all()` hands back exactly that. A host + * asking either question during its bootstrap, which is every host, would get two answers. + */ + public function test_the_registrar_accessor_holds_what_all_reports(): void { + $this->set_up_container(); + + Absorber::register( $this->sub_plugin_config( 'give-recurring' ) ); + + // Asked before Absorber::all(), which is the whole of the test: a read through the reader + // first would drain the buffer and leave nothing for the two to disagree about. + $registrar = Absorber::registrar(); + + $this->assertSame( [ 'give-recurring' ], array_keys( $registrar->all() ) ); + $this->assertSame( array_keys( Absorber::all() ), array_keys( $registrar->all() ) ); + } + + /** + * Into the registrar the accessor is about to hand back, and once. The buffer is emptied as it + * drains, so asking again must not hand the registrar a slug it already holds and trip the + * duplicate guard on a registration the host only made once. + */ + public function test_the_registrar_accessor_drains_into_the_registrar_it_returns(): void { + $bound = $this->bind_registrar(); + + Absorber::register( $this->sub_plugin_config( 'give-recurring' ) ); + + $this->assertSame( $bound, Absorber::registrar() ); + $this->assertArrayHasKey( 'give-recurring', $bound->sub_plugins ); + $this->assertSame( 1, $bound->register_calls ); + + Absorber::registrar(); + Absorber::all(); + + $this->assertSame( 1, $bound->register_calls, 'Neither read may register what the registrar holds.' ); + } + + /** + * The drain happens after the binding has been resolved and checked, and this is the ordering + * that buys. `Registry\Reader` takes a registrar as a constructor argument, so a registrar bound + * to the wrong class is a reader that cannot be built either — and a drain that ran first would + * send the host after a collaborator it never bound, instead of naming the one binding it did + * get wrong. + */ + public function test_a_registrar_of_the_wrong_type_is_reported_before_the_accessor_drains(): void { + $container = new Test_Container(); + $container->singleton( + Registrar_Interface::class, + static function (): object { + return new stdClass(); + } + ); + $this->set_up_container( $container ); + + try { + Absorber::registrar(); + $this->fail( 'Expected a Config_Exception.' ); + } catch ( Config_Exception $exception ) { + $this->assertStringContainsString( Registrar_Interface::class, $exception->getMessage() ); + $this->assertStringContainsString( 'does not implement', $exception->getMessage() ); + $this->assertStringNotContainsString( + Reader::class, + $exception->getMessage(), + 'The reader is this library\'s own collaborator; naming it sends the host to the wrong file.' + ); + } + } + public function test_register_builds_a_sub_plugin_and_stores_it(): void { $this->set_up_container(); From 6680f3a0755b4eb9659f8b05e190e37434639d70 Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Mon, 24 Aug 2026 15:39:49 +0200 Subject: [PATCH 2/5] Say why the facade is final in terms of what it actually holds Eight of its members are public static -- they are the API a host calls. What no subclass can reach is the booted flag and the collaborator helper, and what makes an override pointless is that every internal call is self::. --- src/Absorber.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Absorber.php b/src/Absorber.php index 4c2cf62..05c52be 100644 --- a/src/Absorber.php +++ b/src/Absorber.php @@ -26,9 +26,10 @@ * `Provider`, when they run to `Boot\Scheduler`, and the load pass itself to `Loader` — so * the only reason to open this file is to change what a host may say to the library. * - * `final` because it cannot usefully be extended: every member is private static and every internal - * call is `self::`, so a subclass would inherit the API, be unable to override any of it, and change - * nothing — which is the silent no-op this class reports on everywhere else. + * `final` because it cannot usefully be extended: every member is static, the one property and the + * one helper behind the API are private, and every internal call is `self::`, so a subclass would + * inherit the API, be unable to change what any of it does, and change nothing — which is the silent + * no-op this class reports on everywhere else. * * @since 1.0.0 */ From 5459d2c3ff28579497ef3f9e113fadb7c1eb7abe Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Tue, 25 Aug 2026 11:34:42 +0200 Subject: [PATCH 3/5] Drain through flush(), so the accessor says what the line is for Reading `Registry\Reader::all()` for its effect and discarding the list left the reason for the line entirely in a comment. `flush()` is the drain without the read, so it goes public and the call site names it. That widens the class to two methods a rebound reader owes, and `Stub_Registry_Reader` is what pays: it answers from the sub-plugins a test named and never calls the parent constructor, so the inherited flush would reach for a registrar that was never initialised. --- src/Absorber.php | 18 +++++++++--------- src/Registry/Reader.php | 8 +++++++- tests/_support/Stub_Registry_Reader.php | 10 ++++++++++ 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/Absorber.php b/src/Absorber.php index 05c52be..ad98ce1 100644 --- a/src/Absorber.php +++ b/src/Absorber.php @@ -46,11 +46,12 @@ final class Absorber { /** * The registrar, holding every registration made so far. * - * Drained on the way past, like `all()` and for the same reason: registration is buffered until - * something reads it, so a registrar handed over as it is holds nothing at all until the first - * pass reads at plugins_loaded priority 5 — which is after every point a host bootstrap gets to - * ask. The two public reads of the registry would then disagree, one of them against the - * contract `Registrar_Interface::all()` states, and neither would say so. + * Drained before it is handed back, as `all()` is on its way to the list and for the same + * reason: registration is buffered until something reads it, so a registrar handed over as it is + * holds nothing at all until the first pass reads at plugins_loaded priority 5 — which is after + * every point a host bootstrap gets to ask. The two public reads of the registry would then + * disagree, one of them against the contract `Registrar_Interface::all()` states, and neither + * would say so. * * Drained *after* the binding has been resolved and checked, not before. `Registry\Reader` takes * a registrar as a constructor argument, so a registrar bound to the wrong class is a reader @@ -64,12 +65,11 @@ final class Absorber { * @return Registrar_Interface */ public static function registrar(): Registrar_Interface { + // Resolved before the drain, not after. The ordering is the paragraph above: the reader is + // built from this binding, so asking for it first is what names the binding at fault. $registrar = self::collaborator( Registrar_Interface::class ); - // Read for the drain rather than for the list: handing the pending registrations over is - // what `Registry\Reader::all()` does on its way to the registrar, and the list it comes back - // with is what `all()` exists to give a host. - self::collaborator( Reader::class )->all(); + self::collaborator( Reader::class )->flush(); return $registrar; } diff --git a/src/Registry/Reader.php b/src/Registry/Reader.php index 31c0e55..ad34a38 100644 --- a/src/Registry/Reader.php +++ b/src/Registry/Reader.php @@ -108,6 +108,12 @@ static function ( $sub_plugin ): bool { /** * Hand every buffered registration to the registrar. * + * Public because the drain is wanted without the read. `Absorber::registrar()` hands a host the + * registrar itself, which is empty until something drains into it, and reading `all()` there for + * its effect and discarding the list said nothing about why the line was there. The two are the + * whole of this class's surface and a rebound reader owes both: `all()` is this method plus the + * registrar's contents, narrowed. + * * The registrar stays the single source of truth: the buffer is a pre-store that needs no * container, and duplicate-slug detection and ordering remain the registrar's alone rather than * being restated here in a second dialect. @@ -131,7 +137,7 @@ static function ( $sub_plugin ): bool { * * @return void */ - protected function flush(): void { + public function flush(): void { if ( self::$pending === [] ) { return; } diff --git a/tests/_support/Stub_Registry_Reader.php b/tests/_support/Stub_Registry_Reader.php index cf16528..5dd3838 100644 --- a/tests/_support/Stub_Registry_Reader.php +++ b/tests/_support/Stub_Registry_Reader.php @@ -47,4 +47,14 @@ public function __construct( array $sub_plugins = [] ) { public function all(): array { return $this->sub_plugins; } + + /** + * Nothing to drain, and nowhere to drain it to. This reader answers from the sub-plugins a test + * named, so it holds no registrar — and the inherited flush would reach for one that was never + * constructed the moment a test that used this stub had also registered something. + * + * @return void + */ + public function flush(): void { + } } From 45e79272d80831cd9eb03b611285ef3e42996715 Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Mon, 24 Aug 2026 15:12:57 +0200 Subject: [PATCH 4/5] Pin what a container set after boot really does --- tests/unit/AbsorberTest.php | 107 ++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/tests/unit/AbsorberTest.php b/tests/unit/AbsorberTest.php index b3eec00..6f50776 100644 --- a/tests/unit/AbsorberTest.php +++ b/tests/unit/AbsorberTest.php @@ -625,6 +625,94 @@ static function () use ( $provider ): Provider_Interface { ); } + /** + * Set the container first, and do not replace it: what happens otherwise is documented on + * `boot()` and until now was only documented. `Boot\Scheduler` closes over the container boot() + * handed it, so the steps on the hooks keep resolving from that one however many containers + * arrive afterwards. + * + * Asserted by loading a sub-plugin rather than by counting resolutions: the promise is that the + * request the host is in the middle of still works, and it does — against bindings the host + * believes it has replaced. + */ + public function test_a_container_set_after_boot_does_not_reach_the_wired_steps(): void { + $wired = $this->boot_with_a_watched_registrar(); + + $this->register_bundled_sub_plugin(); + + $late = $this->bind_registrar(); + + do_action( 'plugins_loaded' ); + + $this->assertSame( 1, $this->bundled_plugin_loads(), 'The load pass runs, whichever container it runs from.' ); + $this->assertArrayHasKey( + 'give-recurring', + $wired->sub_plugins, + 'The registry the steps read is the one the container at boot time built.' + ); + $this->assertSame( 0, $late->register_calls, 'Nothing the replacement container built is on a hook.' ); + + // The recorder has to be shown to work: a spy no hook ever reached and a spy that cannot + // record look exactly alike from the assertion above. + Absorber::register( $this->sub_plugin_config( 'give-fee-recovery' ) ); + Absorber::all(); + + $this->assertSame( 1, $late->register_calls, 'The replacement registrar really does record what reaches it.' ); + } + + /** + * The other half of the same split, and the half a host sees: the accessors resolve from + * whatever `Config` holds when they are called, so after a second `set_container()` they answer + * for the replacement while the hooks answer for the original. `Absorber::all()` reports nothing + * registered, on a request that has already loaded the sub-plugin — two live registries, no + * report, and a host debugging the empty one. + */ + public function test_a_container_set_after_boot_splits_the_accessors_from_the_hooks(): void { + $this->boot_with_a_watched_registrar(); + + $this->register_bundled_sub_plugin(); + + $late = $this->bind_registrar(); + + do_action( 'plugins_loaded' ); + + $this->assertSame( $late, Absorber::registrar(), 'The accessors read whatever Config holds now.' ); + $this->assertSame( [], Absorber::all(), 'So the registry a host can reach is the empty one …' ); + $this->assertSame( 1, $this->bundled_plugin_loads(), '… while the sub-plugin really did load.' ); + } + + /** + * And booting again is no way out of it. `boot()` is idempotent by the flag it sets last, so the + * second call returns before the provider runs: the container that arrived after boot never gets + * the library's bindings, and the accessors that resolve from it fail on every one. + */ + public function test_a_second_boot_binds_nothing_into_a_container_set_afterwards(): void { + $this->rewind_plugins_loaded(); + $this->set_up_container(); + + Absorber::boot(); + + $replacement = $this->bare_container(); + + Config::set_container( $replacement ); + + Absorber::boot(); + + // An interface id, for the reason the provider test gives: a container answers for any class + // that exists whether or not anything bound it, so only an interface says what ran. + $this->assertFalse( + $replacement->has( Registrar_Interface::class ), + 'The second boot() returns early, so no provider runs over the container that replaced the first.' + ); + + try { + Absorber::registrar(); + $this->fail( 'Expected a Config_Exception.' ); + } catch ( Config_Exception $exception ) { + $this->assertStringContainsString( Registrar_Interface::class, $exception->getMessage() ); + } + } + public function test_render_notices_delegates_to_the_bound_presenter(): void { $presenter = $this->bind_presenter(); @@ -886,6 +974,25 @@ private function rewind_plugins_loaded(): void { unset( $GLOBALS['wp_actions']['plugins_loaded'] ); } + /** + * Boot against a container whose registrar this test can watch. + * + * The hook counter is rewound first, or the scheduler rightly reports that it is too late to + * wire and runs the sequence inline — which is the one shape these tests are not about, since + * nothing inline is left holding a container when the next one arrives. + * + * @return Spy_Registrar + */ + private function boot_with_a_watched_registrar(): Spy_Registrar { + $this->rewind_plugins_loaded(); + + $bound = $this->bind_registrar(); + + Absorber::boot(); + + return $bound; + } + /** * Bind a recording registrar, in the order a host binds one: before the provider fills in what is * missing. From f001e135c09a66f42cdc8de31ae6c5ee92fcd75f Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Tue, 25 Aug 2026 13:04:07 +0200 Subject: [PATCH 5/5] Say why the registrar is resolved either side of the drain A reader arriving at this method counts two container resolutions and reads a second registrar into the second one. Every binding is a singleton, so it is the same object -- and draining into the instance resolved here, which is the fix that misreading suggests, would empty the buffer into an object no pass holds. --- src/Absorber.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Absorber.php b/src/Absorber.php index ad98ce1..2108af1 100644 --- a/src/Absorber.php +++ b/src/Absorber.php @@ -53,6 +53,13 @@ final class Absorber { * disagree, one of them against the contract `Registrar_Interface::all()` states, and neither * would say so. * + * Two resolutions, and not two registrars: every binding `Provider` makes is a singleton, so the + * instance handed back here is the one `Registry\Reader` was constructed with and the one + * `flush()` drains into. A host that binds `Registrar_Interface` transiently breaks that — and + * the answer is still not to drain into the instance resolved here, which would empty the buffer + * into an object nothing else holds and leave `all()`, the load pass and the conflict pass + * reading a registrar those registrations never reached. + * * Drained *after* the binding has been resolved and checked, not before. `Registry\Reader` takes * a registrar as a constructor argument, so a registrar bound to the wrong class is a reader * that cannot be built either — and a drain in front would report the reader, a collaborator the