diff --git a/CLAUDE.md b/CLAUDE.md index 2f367fd..717cf17 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -74,10 +74,10 @@ seams a host may rebind: | `Contracts\Plugin_Checker_Interface` | `Plugin_Checker` | answers whether a plugin is active | | `Contracts\Activator_Interface` | `Activator` | run-once activation-callback tracking | -The rest — `Boot\Scheduler`, `Loader`, `Conflict\Gatekeeper`, `Conflict\Redirector`, -`Notices\Store`, `Notices\Renderer` — are bound as concrete classes. A host that wants one of them -different rebinds the class name; there is no interface because nothing in the library dispatches on -one. +The rest — `Boot\Scheduler`, `Loader`, `Registry_Reader`, `Conflict\Detector`, `Conflict\Gatekeeper`, +`Conflict\Redirector`, `Notices\Store`, `Notices\Renderer` — are bound as concrete classes. A host +that wants one of them different rebinds the class name; there is no interface because nothing in the +library dispatches on one. An interface belonging to a folder-scoped concern lives in that folder's `Contracts\`, not beside its implementation and not in the top-level `src/Contracts/`. `src/Contracts/` is for the interfaces whose @@ -118,10 +118,20 @@ it is di52-only: `stellarwp/container-contract` declares `bind`, `get`, `has` an nothing else. `[ $resolved_object, 'method' ]` is the other wrong answer — it forces every collaborator to be built at boot. -`Absorber` keeps the public surface. `registrar()` and `notices()` are one-line delegations to -`$container->get()`, so what a host calls is unchanged; what changed is that a *collaborator* now +`Absorber` keeps the public surface. `registrar()`, `notices()` and `all()` are one-line delegations +to `$container->get()`, so what a host calls is unchanged; what changed is that a *collaborator* now depends on the peer it was handed rather than on the facade. +**Nothing but `Absorber` names `Absorber`.** The registration buffer belongs to `Registry_Reader`, +which is also what reads it back out: `Absorber::register()` pushes a `Sub_Plugin` into it and +`Absorber::all()` delegates to it, while `Conflict\Detector` and `Loader` are each handed one. The +buffer is static because it must be — `register()` is a static call a host makes at plugin-file +scope, before there is a container to resolve a registrar from — and what is decided +is only which class pays for that. Leaving it on the facade left an edge pointing back up: the passes +the facade boots read the registry by calling the facade, so `Absorber` sat both above and below its +own collaborators, and a pass could not be handed a registry to work on. The arrows run one way now, +and a pass is complete the moment it is built. + `Sub_Plugin` is a value object answering the per-sub-plugin questions it can answer **without a container-bound collaborator** (`is_enabled()`, `is_already_loaded()`, `has_standalone_plugin()`, `get_conflict_policy()`, …). Note that this is not the same as "config alone": `is_already_loaded()` @@ -139,7 +149,7 @@ Currently: | Path | What | |---|---| | `src/Config.php` | Static facade: hook prefix + container. | -| `src/Absorber.php` | Static facade: the registration buffer, `boot()`, and the accessors. | +| `src/Absorber.php` | Static facade: registration, `boot()`, and the accessors. Holds no collaborator's state. | | `src/Provider.php` | Binds every collaborator; the only file that names a default implementation. | | `src/Boot/Scheduler.php` | Hook wiring and boot timing: the sequence, the priorities, and the fallback for a host that boots too late. | | `src/Loader.php` | The load pass: the gate chain, the `require_once`, the activation callback. | @@ -147,6 +157,8 @@ Currently: | `src/Conflict_Policy.php` | The three policy constants, `default()`, `is_valid()`. | | `src/Plugin_Deactivator.php`, `src/Plugin_Checker.php` | The only files that touch WordPress plugin functions, through `Traits\Loads_Plugin_Functions`. | | `src/Registrar.php` | Holds registered `Sub_Plugin` objects. | +| `src/Registry_Reader.php` | The registration buffer, drained into the registrar on the way past; the object every pass reads the registry through. | +| `src/Conflict/` | `Detector` (whether a standalone is in the way), `Gatekeeper` (which requests, and which users, may have one resolved), `Redirector` (where the user lands afterwards). | | `src/Traits/` | `Loads_Plugin_Functions` (pulls in `wp-admin/includes/plugin.php`), `Guards_Hook_Prefix` (a missing prefix warns and stands down rather than throwing). | | `src/Notices/` | `Queue` (what a notice says, who may consume it), `Store` (keeps it), `Renderer` (draws it), `Contracts\Queue_Interface`. | | `src/Contracts/`, `src/Exceptions/` | `Provider_Interface`, `Registrar_Interface`, `Plugin_Deactivator_Interface`, `Plugin_Checker_Interface`, `Config_Exception`. | @@ -186,9 +198,19 @@ carries the whole re-declaration guarantee, and it is the only gate meaning "thi running" — warning that requirements are unmet for a plugin the admin can watch working would send them after the wrong problem. `docs/filters.md` and the spec agree. -`Absorber::all()` narrows to `Sub_Plugin` instances itself, so no caller repeats that guard. A host -may bind a registrar returning anything, and PHP 7.4 cannot express `array` in -the interface signature — so it is filtered once where the untrusted value enters. +`Registry_Reader::all()` narrows to `Sub_Plugin` instances itself, so no caller repeats that guard. A +host may bind a registrar returning anything, and PHP 7.4 cannot express `array` in +the interface signature — so it is filtered once where the untrusted value enters. The load pass and +`Conflict\Detector` read through the reader they were constructed with rather than through the +registrar they could resolve for themselves, because it drains the pending registrations before it +reads and a registrar asked directly would miss anything registered since the last flush. + +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 +unbuildable `Queue_Interface` or `Plugin_Checker_Interface` has always failed. Read-time and +build-time failures stopped being the same event when the registry became an argument, and the +registrar now fails like every other binding rather than being the one collaborator whose broken +binding surfaced late and politely. `Conflict\Resolver` switches on the policy: `DEFER` no-ops, `NOTICE_ONLY` queues a notice, and `DEACTIVATE` (the default) deactivates network-aware, queues a merge notice, and redirects. diff --git a/src/Absorber.php b/src/Absorber.php index caad33f..5870100 100644 --- a/src/Absorber.php +++ b/src/Absorber.php @@ -29,13 +29,6 @@ final class Absorber { use Guards_Hook_Prefix; - /** - * Sub-plugins registered but not yet handed to the registrar. - * - * @var Sub_Plugin[] - */ - private static $pending = []; - /** * Whether the hooks have been wired. * @@ -69,10 +62,10 @@ public static function notices(): Queue_Interface { * Register one bundled sub-plugin. Call once per sub-plugin, before boot(). * * The sub-plugin is buffered rather than handed straight to the registrar, so that registering - * resolves nothing. Reaching the registrar needs the container, and a host that registers before - * it calls Config::set_container() would otherwise fail on a call that has nothing to do with - * the container. Buffering is what lets the container arrive at any point before boot, like - * every other configuration call. + * resolves nothing — not even the container. A host that registers before it calls + * Config::set_container() would otherwise fail on a call that has nothing to do with the + * container. The buffer belongs to `Registry_Reader`, which is where it is read back out: this + * class hands its collaborators no work and holds none of their state. * * The configuration is still validated here: building the Sub_Plugin is what rejects it, and * that happens at the call the host can see in its own stack trace. It is built rather than @@ -88,12 +81,17 @@ public static function notices(): Queue_Interface { * @return void */ public static function register( array $config ): void { - self::$pending[] = new Sub_Plugin( $config ); + Registry_Reader::buffer( new Sub_Plugin( $config ) ); } /** * Every registered sub-plugin, keyed by slug, in registration order. * + * A delegation like the accessors above it, and for the same reason: what a host calls is here, + * what it does is the collaborator's. The passes that read the registry are handed that + * collaborator directly rather than calling back through this method — a facade sits in front of + * its collaborators, never underneath them. + * * @since 1.0.0 * * @throws Config_Exception When no container has been set, or two sub-plugins were registered @@ -102,20 +100,7 @@ public static function register( array $config ): void { * @return array */ public static function all(): array { - self::flush(); - - // Registrar_Interface::all() can only declare `array` — PHP 7.4 has no way to say - // array in a signature — so a host binding its own registrar may return - // anything at all. Narrowed once here, where the untrusted value crosses into the library, - // rather than at each call site: a consumer that forgot the check would fatal inside - // plugins_loaded on its first predicate call, which is the exact failure this library - // exists to prevent, and every future consumer would have to remember it too. - return array_filter( - self::registrar()->all(), - static function ( $sub_plugin ): bool { - return $sub_plugin instanceof Sub_Plugin; - } - ); + return self::collaborator( Registry_Reader::class )->all(); } /** @@ -228,38 +213,4 @@ private static function collaborator( string $interface ): object { return $collaborator; } - - /** - * Hand every buffered registration to the registrar. - * - * 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. - * - * The buffer is emptied before the loop, so a second read cannot re-register what the - * registrar already holds and trip its duplicate-slug guard. It is emptied *after* the - * registrar resolves, so a container binding that throws leaves the registrations buffered - * for the next read rather than dropping them on the floor. - * - * @since 1.0.0 - * - * @throws Config_Exception When no container has been set, or two sub-plugins were registered - * under one slug. - * - * @return void - */ - private static function flush(): void { - if ( self::$pending === [] ) { - return; - } - - $registrar = self::registrar(); - $pending = self::$pending; - - self::$pending = []; - - foreach ( $pending as $sub_plugin ) { - $registrar->register( $sub_plugin ); - } - } } diff --git a/src/Conflict/Detector.php b/src/Conflict/Detector.php index 99d7a63..775a285 100644 --- a/src/Conflict/Detector.php +++ b/src/Conflict/Detector.php @@ -7,9 +7,9 @@ namespace Nexcess\PluginAbsorber\Conflict; -use Nexcess\PluginAbsorber\Absorber; use Nexcess\PluginAbsorber\Contracts\Plugin_Checker_Interface; use Nexcess\PluginAbsorber\Exceptions\Config_Exception; +use Nexcess\PluginAbsorber\Registry_Reader; use Nexcess\PluginAbsorber\Sub_Plugin; /** @@ -31,6 +31,13 @@ * @since 1.0.0 */ class Detector { + /** + * @since 1.0.0 + * + * @var Registry_Reader + */ + private $registry; + /** * @since 1.0.0 * @@ -41,9 +48,11 @@ class Detector { /** * @since 1.0.0 * + * @param Registry_Reader $registry Which sub-plugins are registered. * @param Plugin_Checker_Interface $plugin_checker Whether the standalone is active. */ - public function __construct( Plugin_Checker_Interface $plugin_checker ) { + public function __construct( Registry_Reader $registry, Plugin_Checker_Interface $plugin_checker ) { + $this->registry = $registry; $this->plugin_checker = $plugin_checker; } @@ -60,10 +69,10 @@ public function __construct( Plugin_Checker_Interface $plugin_checker ) { * @return bool */ public function has_conflict(): bool { - // Absorber::all() rather than a registrar of our own: it flushes the pending registrations - // before it reads, and a registrar asked directly would not see anything registered since - // the last read. - foreach ( Absorber::all() as $sub_plugin ) { + // The reader rather than a registrar of our own: it drains the registrations still buffered + // on the facade before it reads, and a registrar asked directly would miss anything + // registered since the last read. + foreach ( $this->registry->all() as $sub_plugin ) { if ( $this->is_in_conflict( $sub_plugin ) ) { return true; } diff --git a/src/Loader.php b/src/Loader.php index 8534848..5d05101 100644 --- a/src/Loader.php +++ b/src/Loader.php @@ -21,6 +21,13 @@ class Loader { use Guards_Hook_Prefix; + /** + * @since 1.0.0 + * + * @var Registry_Reader + */ + private $registry; + /** * @since 1.0.0 * @@ -31,10 +38,12 @@ class Loader { /** * @since 1.0.0 * - * @param Queue_Interface $notices Where a sub-plugin that could not load says so. + * @param Registry_Reader $registry Which sub-plugins are registered. + * @param Queue_Interface $notices Where a sub-plugin that could not load says so. */ - public function __construct( Queue_Interface $notices ) { - $this->notices = $notices; + public function __construct( Registry_Reader $registry, Queue_Interface $notices ) { + $this->registry = $registry; + $this->notices = $notices; } /** @@ -53,11 +62,11 @@ public function load_all(): void { return; } - // Absorber::all() rather than the registrar directly: it flushes 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. + // 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 = Absorber::all(); + $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 diff --git a/src/Provider.php b/src/Provider.php index 5a1dcb9..003a3e2 100644 --- a/src/Provider.php +++ b/src/Provider.php @@ -80,17 +80,30 @@ static function () use ( $container ): Queue { } ); + $this->bind_once( + Registry_Reader::class, + static function () use ( $container ): Registry_Reader { + return new Registry_Reader( $container->get( Registrar_Interface::class ) ); + } + ); + $this->bind_once( Detector::class, static function () use ( $container ): Detector { - return new Detector( $container->get( Plugin_Checker_Interface::class ) ); + return new Detector( + $container->get( Registry_Reader::class ), + $container->get( Plugin_Checker_Interface::class ) + ); } ); $this->bind_once( Loader::class, static function () use ( $container ): Loader { - return new Loader( $container->get( Queue_Interface::class ) ); + return new Loader( + $container->get( Registry_Reader::class ), + $container->get( Queue_Interface::class ) + ); } ); diff --git a/src/Registry_Reader.php b/src/Registry_Reader.php new file mode 100644 index 0000000..9f36b41 --- /dev/null +++ b/src/Registry_Reader.php @@ -0,0 +1,138 @@ +registrar = $registrar; + } + + /** + * Hold a registration until something reads. + * + * Static, and it stores rather than registers, because `Absorber::register()` must resolve + * nothing: the host container LearnDash hands us is *replaced* at `plugins_loaded` priority 0, so + * a registration that reached a registrar before that point would go into the container being + * thrown away. Buffering is what lets the container arrive at any point before boot. + * + * @since 1.0.0 + * + * @param Sub_Plugin $sub_plugin Sub-plugin to hold. + * + * @return void + */ + public static function buffer( Sub_Plugin $sub_plugin ): void { + self::$pending[] = $sub_plugin; + } + + /** + * Every registered sub-plugin, keyed by slug, in registration order. + * + * The buffer is drained on the way past, which is why a pass is handed this rather than the + * registrar it could resolve for itself: a registrar asked directly would miss everything + * registered since the last read, a host registering from its own `plugins_loaded` callback + * included. + * + * @since 1.0.0 + * + * @throws Config_Exception When two sub-plugins were registered under one slug. + * + * @return array + */ + public function all(): array { + $this->flush(); + + // Registrar_Interface::all() can only declare `array` -- PHP 7.4 has no way to say + // array in a signature -- so a host binding its own registrar may return + // anything at all. Narrowed once here, where the untrusted value crosses into the library, + // rather than at each call site: a consumer that forgot the check would fatal inside + // plugins_loaded on its first predicate call, which is the exact failure this library + // exists to prevent, and every future consumer would have to remember it too. + return array_filter( + $this->registrar->all(), + static function ( $sub_plugin ): bool { + return $sub_plugin instanceof Sub_Plugin; + } + ); + } + + /** + * Hand every buffered registration to the registrar. + * + * 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. + * + * The buffer is emptied before the loop, so a second read cannot re-register what the registrar + * already holds and trip its duplicate-slug guard. Nothing has to empty it *after* a failure + * either: the registrar is a constructor argument, so a container that cannot build one fails + * while this object is being built, with the registrations still buffered for the read that comes + * after the host has fixed its bindings. + * + * @since 1.0.0 + * + * @throws Config_Exception When two sub-plugins were registered under one slug. + * + * @return void + */ + protected function flush(): void { + if ( self::$pending === [] ) { + return; + } + + $pending = self::$pending; + + self::$pending = []; + + foreach ( $pending as $sub_plugin ) { + $this->registrar->register( $sub_plugin ); + } + } +} diff --git a/tests/_support/Absorber_State.php b/tests/_support/Absorber_State.php index b120a7b..e0596ad 100644 --- a/tests/_support/Absorber_State.php +++ b/tests/_support/Absorber_State.php @@ -8,13 +8,14 @@ use Closure; use LogicException; use Nexcess\PluginAbsorber\Absorber; +use Nexcess\PluginAbsorber\Registry_Reader; use ReflectionClass; use ReflectionFunction; use ReflectionProperty; use WP_Hook; /** - * Restores `Absorber`'s static state between tests, and unwires the hooks boot() added. + * Restores the boot path's static state between tests, and unwires the hooks boot() added. * * `Absorber` has no public way to clear itself, and deliberately so: a reset method would be API the * library then has to support forever for the sake of its own test suite, and a host that reached for @@ -26,17 +27,23 @@ */ class Absorber_State { /** - * The value each of `Absorber`'s static properties starts life with. + * The value each static property of the boot path starts life with, by the class holding it. + * + * Two classes, because the registration buffer belongs to `Registry_Reader` — a host registers + * before there is a container to reach a registrar through, so the pre-store is static, and it + * sits with the object that reads it rather than with the facade that writes to it. A test that + * cleared only the facade would leave one test's registrations to drain into the next test's + * registrar. * * Spelled out rather than read from `ReflectionClass::getDefaultProperties()`, which reports a * static property's *current* value on PHP below 8.3 — a reset built on it is a silent no-op on * the 7.4 leg. * - * @var array + * @var array> */ protected const DEFAULTS = [ - 'pending' => [], - 'booted' => false, + Absorber::class => [ 'booted' => false ], + Registry_Reader::class => [ 'pending' => [] ], ]; /** @@ -72,19 +79,19 @@ class Absorber_State { public static function reset(): void { self::unwire(); - $reflection = new ReflectionClass( Absorber::class ); + foreach ( self::DEFAULTS as $class => $defaults ) { + foreach ( ( new ReflectionClass( $class ) )->getProperties( ReflectionProperty::IS_STATIC ) as $property ) { + $name = $property->getName(); - foreach ( $reflection->getProperties( ReflectionProperty::IS_STATIC ) as $property ) { - $name = $property->getName(); + if ( ! array_key_exists( $name, $defaults ) ) { + throw new LogicException( + sprintf( '%s::$%s has no default in %s. Add one.', $class, $name, self::class ) + ); + } - if ( ! array_key_exists( $name, self::DEFAULTS ) ) { - throw new LogicException( - sprintf( 'Absorber::$%s has no default in %s. Add one.', $name, self::class ) - ); + $property->setAccessible( true ); + $property->setValue( null, $defaults[ $name ] ); } - - $property->setAccessible( true ); - $property->setValue( null, self::DEFAULTS[ $name ] ); } } diff --git a/tests/_support/Stub_Registry_Reader.php b/tests/_support/Stub_Registry_Reader.php new file mode 100644 index 0000000..9cc56ae --- /dev/null +++ b/tests/_support/Stub_Registry_Reader.php @@ -0,0 +1,50 @@ + + */ + public $sub_plugins = []; + + /** + * @param Sub_Plugin[] $sub_plugins Sub-plugins this reader knows about, in registration order. + */ + public function __construct( array $sub_plugins = [] ) { + foreach ( $sub_plugins as $sub_plugin ) { + $this->sub_plugins[ $sub_plugin->get_slug() ] = $sub_plugin; + } + } + + /** + * @return array + */ + public function all(): array { + return $this->sub_plugins; + } +} diff --git a/tests/unit/Conflict/DetectorTest.php b/tests/unit/Conflict/DetectorTest.php index 715be3a..fbb335c 100644 --- a/tests/unit/Conflict/DetectorTest.php +++ b/tests/unit/Conflict/DetectorTest.php @@ -19,6 +19,7 @@ use Nexcess\PluginAbsorber\Sub_Plugin; use Nexcess\PluginAbsorber\Tests\Support\Absorber_State; use Nexcess\PluginAbsorber\Tests\Support\Config_State; +use Nexcess\PluginAbsorber\Tests\Support\Stub_Registry_Reader; use Nexcess\PluginAbsorber\Tests\Support\Test_Container; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithContainer; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithNoticeQueue; @@ -35,10 +36,11 @@ * thing it must not read — a host's policy callable and the filter behind it can do anything at all, * and neither says whether a standalone is running. * - * `has_conflict()` goes through `Absorber::all()`, so it needs the container; `is_in_conflict()` is - * handed the sub-plugin and needs nothing but its checker, which is why the tests for it build a - * detector directly. Both readings matter: the conflict step reaches the first, and the resolver - * reaches the second once for every sub-plugin it walks. + * `has_conflict()` reads the registry through the reader it was built with, so most of the tests for + * it register through the facade the default reader is behind; `is_in_conflict()` is handed the + * sub-plugin and needs nothing but its checker, which is why the tests for it build a detector + * directly. Both readings matter: the conflict step reaches the first, and the resolver reaches the + * second once for every sub-plugin it walks. * * @since 1.0.0 */ @@ -123,6 +125,24 @@ public function test_the_checker_comes_from_the_container(): void { $this->assertSame( [ 'give-recurring/give-recurring.php' ], $this->asked ); } + /** + * The registry arrives as an argument like everything else, so nothing is registered here at all: + * the sub-plugin this probe finds is one the facade has never been told about. + */ + public function test_it_reads_the_registry_it_was_handed(): void { + $detector = new Detector( + new Stub_Registry_Reader( [ $this->make_sub_plugin( [ 'standalone_plugin_basename' => 'give-recurring/give-recurring.php' ] ) ] ), + $this->recording_checker( true ) + ); + + $this->assertTrue( $detector->has_conflict() ); + $this->assertSame( + [ 'give-recurring/give-recurring.php' ], + $this->asked, + 'The sub-plugin asked about has to be the one the reader handed over.' + ); + } + public function test_it_finds_an_active_standalone(): void { $this->standalone_is( true ); $this->register(); @@ -269,23 +289,19 @@ public function test_it_stops_at_the_first_conflict_it_finds(): void { } /** - * A registry read is a container read. The probe reads the registry and nothing else, which is - * what keeps it cheap enough to ask of every admin GET. + * The probe reads the registry and nothing else, which is what keeps it cheap enough to ask of + * every admin GET — and a duplicate slug is the one bootstrap mistake that read can still raise, + * because it is only found when the buffer reaches the registrar. The conflict step catches this + * exception type around the probe for exactly this case, so it has to arrive as this type. */ - public function test_it_needs_a_container(): void { + public function test_a_duplicate_slug_surfaces_from_the_probe(): void { $this->standalone_is( true ); $this->register(); - - $detector = $this->detector(); - - // The prefix stays, the container goes: a probe that could not reach the registrar would - // throw the same exception for the other reason. - Config_State::reset(); - Config::set_hook_prefix( 'give' ); + $this->register(); $this->expectException( Config_Exception::class ); - $detector->has_conflict(); + $this->detector()->has_conflict(); } /** @@ -293,7 +309,7 @@ public function test_it_needs_a_container(): void { * through the registry — no container, no registration, just the object and its checker. */ public function test_is_in_conflict_is_true_for_an_active_standalone(): void { - $detector = new Detector( $this->recording_checker( true ) ); + $detector = new Detector( new Stub_Registry_Reader(), $this->recording_checker( true ) ); $this->assertTrue( $detector->is_in_conflict( @@ -303,7 +319,7 @@ public function test_is_in_conflict_is_true_for_an_active_standalone(): void { } public function test_is_in_conflict_is_false_when_the_standalone_is_not_active(): void { - $detector = new Detector( $this->recording_checker( false ) ); + $detector = new Detector( new Stub_Registry_Reader(), $this->recording_checker( false ) ); $this->assertFalse( $detector->is_in_conflict( @@ -321,7 +337,7 @@ public function test_is_in_conflict_is_false_when_the_standalone_is_not_active() * The two cheap keys are read first, so a site with a disabled sub-plugin pays nothing for it. */ public function test_is_in_conflict_asks_nothing_about_a_disabled_sub_plugin(): void { - $detector = new Detector( $this->recording_checker( true ) ); + $detector = new Detector( new Stub_Registry_Reader(), $this->recording_checker( true ) ); $this->assertFalse( $detector->is_in_conflict( @@ -339,7 +355,7 @@ public function test_is_in_conflict_asks_nothing_about_a_disabled_sub_plugin(): } public function test_is_in_conflict_asks_nothing_about_a_sub_plugin_with_no_standalone(): void { - $detector = new Detector( $this->recording_checker( true ) ); + $detector = new Detector( new Stub_Registry_Reader(), $this->recording_checker( true ) ); $this->assertFalse( $detector->is_in_conflict( $this->make_sub_plugin() ) ); $this->assertSame( [], $this->asked, 'There is no basename to ask about.' ); @@ -353,7 +369,7 @@ public function test_is_in_conflict_asks_nothing_about_a_sub_plugin_with_no_stan * from either copy, not whether the standalone is switched on. */ public function test_is_in_conflict_asks_about_the_configured_basename(): void { - $detector = new Detector( $this->recording_checker( true ) ); + $detector = new Detector( new Stub_Registry_Reader(), $this->recording_checker( true ) ); $detector->is_in_conflict( $this->make_sub_plugin( diff --git a/tests/unit/LoaderTest.php b/tests/unit/LoaderTest.php index 13bae99..fc2d112 100644 --- a/tests/unit/LoaderTest.php +++ b/tests/unit/LoaderTest.php @@ -17,6 +17,7 @@ use Nexcess\PluginAbsorber\Tests\Support\Config_State; use Nexcess\PluginAbsorber\Tests\Support\Spy_Queue; use Nexcess\PluginAbsorber\Tests\Support\Spy_Registrar; +use Nexcess\PluginAbsorber\Tests\Support\Stub_Registry_Reader; use Nexcess\PluginAbsorber\Tests\Support\Test_Container; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithBundledPlugins; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithContainer; @@ -107,6 +108,36 @@ public function test_it_requires_the_bundled_file(): void { $this->assertTrue( defined( $constant ) ); } + /** + * The registry arrives as a constructor argument, so a load pass can be pointed at a sub-plugin + * nothing ever registered — and the file it requires proves the loop read the object it was + * handed rather than the facade. + */ + public function test_it_loads_the_registry_it_was_handed(): void { + $constant = $this->make_guard_constant(); + $path = $this->make_bundled_plugin_file( $constant ); + + $loader = new Loader( + new Stub_Registry_Reader( + [ + new Sub_Plugin( + [ + 'slug' => 'give-recurring', + 'bundled_plugin_file' => $path, + 'plugin_loaded_constant' => $constant, + ] + ), + ] + ), + new Spy_Queue() + ); + + $loader->load_all(); + + $this->assertSame( 1, $this->bundled_plugin_loads() ); + $this->assertTrue( defined( $constant ) ); + } + public function test_it_requires_the_bundled_file_exactly_once(): void { $this->register(); diff --git a/tests/unit/ProviderTest.php b/tests/unit/ProviderTest.php index f7e4f65..342dc7b 100644 --- a/tests/unit/ProviderTest.php +++ b/tests/unit/ProviderTest.php @@ -24,6 +24,7 @@ use Nexcess\PluginAbsorber\Plugin_Deactivator; use Nexcess\PluginAbsorber\Provider; use Nexcess\PluginAbsorber\Registrar; +use Nexcess\PluginAbsorber\Registry_Reader; use Nexcess\PluginAbsorber\Tests\Support\Config_State; use Nexcess\PluginAbsorber\Tests\Support\Spy_Queue; use Nexcess\PluginAbsorber\Tests\Support\Spy_Registrar; @@ -75,6 +76,7 @@ public function test_it_binds_every_default( string $id, string $expected ): voi */ public static function default_bindings(): Generator { yield 'the registrar' => [ Registrar_Interface::class, Registrar::class ]; + yield 'the registry reader' => [ Registry_Reader::class, Registry_Reader::class ]; yield 'the notice queue' => [ Queue_Interface::class, Queue::class ]; yield 'the notice store' => [ Store::class, Store::class ]; yield 'the notice renderer' => [ Renderer::class, Renderer::class ]; @@ -108,6 +110,7 @@ public function test_a_binding_resolves_to_one_instance( string $id ): void { */ public static function single_instance_bindings(): Generator { yield 'the registrar' => [ Registrar_Interface::class ]; + yield 'the registry reader' => [ Registry_Reader::class ]; yield 'the notice queue' => [ Queue_Interface::class ]; yield 'the notice store' => [ Store::class ]; yield 'the notice renderer' => [ Renderer::class ]; @@ -144,6 +147,7 @@ public function test_it_binds_a_class_id_the_container_reports_it_already_has( s * @return Generator */ public static function class_id_bindings(): Generator { + yield 'the registry reader' => [ Registry_Reader::class ]; yield 'the notice store' => [ Store::class ]; yield 'the notice renderer' => [ Renderer::class ]; yield 'the conflict detector' => [ Detector::class ]; diff --git a/tests/unit/Registry_ReaderTest.php b/tests/unit/Registry_ReaderTest.php new file mode 100644 index 0000000..60bf962 --- /dev/null +++ b/tests/unit/Registry_ReaderTest.php @@ -0,0 +1,194 @@ +tear_down_container(); + parent::tearDown(); + } + + public function test_it_reads_every_registered_sub_plugin(): void { + $this->set_up_container(); + $this->register( 'give-recurring' ); + $this->register( 'give-fee-recovery' ); + + $all = $this->reader()->all(); + + $this->assertSame( [ 'give-recurring', 'give-fee-recovery' ], array_keys( $all ) ); + $this->assertInstanceOf( Sub_Plugin::class, $all['give-recurring'] ); + } + + public function test_it_is_empty_before_anything_is_registered(): void { + $this->set_up_container(); + + $this->assertSame( [], $this->reader()->all() ); + } + + /** + * The whole reason a pass is handed this rather than a registrar. Registration is buffered on the + * facade until something reads, so a collaborator holding the registrar itself would miss every + * sub-plugin registered since the last read — a host registering from its own `plugins_loaded` + * callback included. + */ + public function test_it_flushes_what_was_registered_since_the_last_read(): void { + $this->set_up_container(); + $this->register( 'give-recurring' ); + + $reader = $this->reader(); + + $this->assertSame( [ 'give-recurring' ], array_keys( $reader->all() ) ); + + $this->register( 'give-fee-recovery' ); + + $this->assertSame( + [ 'give-recurring', 'give-fee-recovery' ], + array_keys( $reader->all() ), + 'A read has to drain the buffer as it finds it, not as it found it the first time.' + ); + } + + /** + * The registrar stays the single source of truth: a host that bound its own is read by this, and + * does not have to know the passes exist. + */ + public function test_it_reads_through_the_bound_registrar(): void { + $registrar = new Spy_Registrar(); + $container = new Test_Container(); + $container->singleton( + Registrar_Interface::class, + static function () use ( $registrar ): Registrar_Interface { + return $registrar; + } + ); + $this->set_up_container( $container ); + + $this->register( 'give-recurring' ); + + $this->assertSame( [ 'give-recurring' ], array_keys( $this->reader()->all() ) ); + $this->assertArrayHasKey( 'give-recurring', $registrar->sub_plugins ); + } + + /** + * The one bootstrap mistake that can still arrive at read time, and the reason both passes catch + * `Config_Exception` around their read: a slug is only found to be a duplicate when the buffer + * reaches the registrar, which is a read after both `register()` calls have returned. + * + * The container is not the other half of that any more. It is needed to *build* a reader, not to + * read from one — the registrar arrives as a constructor argument, so a reader that exists has + * one, and a container that could not supply it failed while this object was being built. + */ + public function test_a_duplicate_slug_surfaces_from_the_read(): void { + $this->set_up_container(); + $this->register( 'give-recurring' ); + $this->register( 'give-recurring' ); + + $reader = $this->reader(); + + $this->expectException( Config_Exception::class ); + + $reader->all(); + } + + /** + * A registrar the container cannot build leaves the registrations where they are: the reader is + * what drains the buffer, and it is never built at all if its own argument cannot be. + */ + public function test_registrations_survive_a_registrar_the_container_cannot_build(): void { + $container = new Test_Container(); + $container->singleton( + Registrar_Interface::class, + static function (): Registrar_Interface { + throw new RuntimeException( 'the host factory needed a database connection' ); + } + ); + $this->set_up_container( $container ); + + $this->register( 'give-recurring' ); + + $failed = false; + + try { + $this->reader(); + } catch ( Throwable $exception ) { + $failed = true; + } + + $this->assertTrue( $failed, 'A registrar that cannot be built has to surface, not be swallowed.' ); + + $this->set_up_container(); + + $this->assertSame( + [ 'give-recurring' ], + array_keys( $this->reader()->all() ), + 'The buffered registration must still be there once the container is usable.' + ); + } + + /** + * The reader the container builds, which is the one every pass is handed. + * + * @return Registry_Reader + */ + private function reader(): Registry_Reader { + return $this->resolve( Registry_Reader::class ); + } + + /** + * @param string $slug Slug to register under. + * + * @return void + */ + private function register( string $slug ): void { + Absorber::register( + [ + 'slug' => $slug, + 'bundled_plugin_file' => sprintf( '/tmp/%s.php', $slug ), + 'plugin_loaded_constant' => strtoupper( str_replace( '-', '_', $slug ) ) . '_VERSION_FIXTURE', + ] + ); + } +}