From f9ee8b9d2e56a855c519c891d2145596489748d2 Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Mon, 24 Aug 2026 15:11:37 +0200 Subject: [PATCH 1/2] 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 42a63fb..8a4b648 100644 --- a/docs/extending.md +++ b/docs/extending.md @@ -80,7 +80,9 @@ remove_action( 'all_admin_notices', [ Absorber::class, 'render_notices' ] ); a `Config_Exception` naming the interface and the class that failed it, rather than letting a `TypeError` blame this library for your typo inside `plugins_loaded`. Whatever your container raises for a binding it cannot build at all comes through unwrapped. `Absorber::all()` drops anything a -rebound registrar returns that is not a `Sub_Plugin`. +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. Nothing is built at boot beyond the two objects that do the booting: each hook resolves its collaborator when it fires, so a request that reaches none of them builds none of them, and you may diff --git a/src/Absorber.php b/src/Absorber.php index 24cedd8..e7ad15a 100644 --- a/src/Absorber.php +++ b/src/Absorber.php @@ -45,6 +45,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. @@ -52,7 +65,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 b3b8a7d..79cba5f 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 20f9d81aca6b5579c84e9c5b0354ebb5dfa1abd1 Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Mon, 24 Aug 2026 15:39:49 +0200 Subject: [PATCH 2/2] 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 e7ad15a..5e6feb7 100644 --- a/src/Absorber.php +++ b/src/Absorber.php @@ -27,9 +27,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 */