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..2108af1 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 */ @@ -43,6 +44,27 @@ final class Absorber { private static $booted = false; /** + * The registrar, holding every registration made so far. + * + * 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. + * + * 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 + * 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 +72,13 @@ final class Absorber { * @return Registrar_Interface */ public static function registrar(): Registrar_Interface { - return self::collaborator( Registrar_Interface::class ); + // 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 ); + + 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 { + } } diff --git a/tests/unit/AbsorberTest.php b/tests/unit/AbsorberTest.php index f54f68f..6f50776 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(); @@ -553,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(); @@ -814,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.