3. Make the registrar accessor agree with all(), and pin which container answers

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

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,14 +44,41 @@ 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.
 	 *
 	 * @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;
 	}
 
 	/**

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;
 		}

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 {
+	}
 }

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.