diff --git a/docs/conflict-handling.md b/docs/conflict-handling.md index 90f1ed3..d53f908 100644 --- a/docs/conflict-handling.md +++ b/docs/conflict-handling.md @@ -109,6 +109,11 @@ guard](#the-load-guard) stands the bundled copy down network-wide as under `DEFE notice](notices.md) tells a network administrator why and how to finish, by network-activating the host or removing the standalone from the Network Admin. It recurs until one of those is done. +Pass `plugin_basename( __FILE__ )`, not `__FILE__`. A basename no installed plugin answers to reads +as a host that is never network-active, so the guard would decline for ever and the notice would +recur with nothing to act on — the library reports that one to the developer instead, through +`_doing_it_wrong()`, and leaves the standalone where it is. + The guard is **opt-in and single-site-safe**: with no host basename set it never fires, and off a network it never fires, so in every other topology — both network-active, both per-site, or a per-site standalone — deactivation behaves exactly as it always has. diff --git a/src/Conflict/Detector.php b/src/Conflict/Detector.php index ff3edb9..7a74840 100644 --- a/src/Conflict/Detector.php +++ b/src/Conflict/Detector.php @@ -10,6 +10,7 @@ use Nexcess\PluginAbsorber\Config; use Nexcess\PluginAbsorber\Exceptions\Config_Exception; use Nexcess\PluginAbsorber\Plugin\Contracts\Checker_Interface; +use Nexcess\PluginAbsorber\Plugin\Loads_Plugin_Functions; use Nexcess\PluginAbsorber\Registry\Reader; use Nexcess\PluginAbsorber\Sub_Plugin; @@ -25,13 +26,16 @@ * * Nothing here leaves a mark on the request. Nothing resolves a user, * deactivates a plugin or queues a notice — an answer is all a caller gets, and the acting is the - * resolver's. + * resolver's. The one thing either of them writes is a report to the developer about a bootstrap + * that cannot be right, which changes nothing on the site and is addressed to nobody on it. * * Not `final`: it is bound by class name, which is the seam a host rebinds and a test subclasses. * * @since 1.0.0 */ class Detector { + use Loads_Plugin_Functions; + /** * @since 1.0.0 * @@ -46,6 +50,15 @@ class Detector { */ private $plugin_checker; + /** + * Whether the configured host basename has been looked up in this request. + * + * @since 1.0.0 + * + * @var bool + */ + private $host_plugin_looked_up = false; + /** * @since 1.0.0 * @@ -141,6 +154,20 @@ public function is_in_conflict( Sub_Plugin $sub_plugin ): bool { * `Checker_Interface::is_network_active()` is `false` off a network, so the whole predicate is * `false` on a single site. * + * The basename itself is checked against the installed plugins on the way past, because a name no + * plugin answers to answers "not network-active" for ever, and that is indistinguishable from the + * guard working: the standalone is left active on a network where nothing would have been + * stranded, and the notice comes back on every admin page load telling the owner to + * network-activate a plugin that already runs everywhere. A typo does it, so does an mu-plugin or + * a plugin behind a symlink that `plugin_basename()` cannot round-trip, and so does passing + * `__FILE__` where `plugin_basename( __FILE__ )` was meant. The answer is not changed by the + * report -- a name this library cannot resolve is not consent to take a plugin off every site on + * a network -- and the developer is told which one, which is the only thing that fixes it. + * + * The lookup sits behind both cheap guards. `get_plugins()` parses the header of every plugin + * installed, so it is paid for only where the answer is about to matter: a configured host, and a + * standalone that really is network-active. + * * @since 1.0.0 * * @param Sub_Plugin $sub_plugin Sub-plugin whose standalone is active. @@ -158,6 +185,55 @@ public function deactivation_would_strand_sites( Sub_Plugin $sub_plugin ): bool return false; } + $this->report_a_host_basename_no_plugin_answers_to( $host_basename ); + return ! $this->plugin_checker->is_network_active( $host_basename ); } + + /** + * Tell the developer when the configured host basename names nothing that is installed. + * + * Here rather than in `Config::set_host_plugin_basename()`, which is where a reader looks first: + * that is a static setter a host calls at plugin-file scope, and `get_plugins()` lives in + * `wp-admin/includes/plugin.php`, which is not loaded then and which nothing should be loading + * that early to validate an argument. The honest place is the one point of use, on the request + * where the value is about to decide whether a standalone is deactivated. + * + * Once per request, and per instance rather than through a static: the detector is a container + * singleton, so one instance is one request, and the host hears about the mistake again on the + * next one until it is fixed. A static flag would report from the first request a PHP worker + * served and then stay quiet for every request that worker went on to serve -- and it would need + * a reset for the suite's benefit, which is API this library would then support for ever. + * + * @since 1.0.0 + * + * @param string $host_basename Host plugin basename, as the host configured it. + * + * @return void + */ + private function report_a_host_basename_no_plugin_answers_to( string $host_basename ): void { + if ( $this->host_plugin_looked_up ) { + return; + } + + $this->host_plugin_looked_up = true; + + $this->load_plugin_functions(); + + if ( array_key_exists( $host_basename, get_plugins() ) ) { + return; + } + + _doing_it_wrong( + self::class . '::report_a_host_basename_no_plugin_answers_to', + sprintf( + 'The host plugin basename "%s" names no installed plugin, so the multisite stranding ' + . 'guard reads the host as never network-active: a standalone that would strand no ' + . 'site is left active and its notice recurs on every admin page load. ' + . 'Config::set_host_plugin_basename() takes plugin_basename( __FILE__ ), not __FILE__.', + $host_basename + ), + '1.0.0' + ); + } } diff --git a/tests/unit/Conflict/DetectorTest.php b/tests/unit/Conflict/DetectorTest.php index 18ec927..3213acd 100644 --- a/tests/unit/Conflict/DetectorTest.php +++ b/tests/unit/Conflict/DetectorTest.php @@ -573,6 +573,11 @@ public function test_it_reports_whether_deactivation_would_strand_sites( bool $expected ): void { Config::set_host_plugin_basename( $host_basename ); + + // The host names a plugin that really is installed in every one of these, so which sites it + // is switched on for is the only thing varying. A basename nothing answers to is its own + // case, below. + $this->install_plugins( array_filter( [ $host_basename, 'give-recurring/give-recurring.php' ] ) ); $this->install_checker( $this->checker_network_active_for( $network_active ) ); $sub_plugin = $this->make_sub_plugin( @@ -599,6 +604,144 @@ public static function stranding_topologies(): Generator { yield 'no host basename configured' => [ '', [ $standalone ], false ]; } + /** + * `Config::set_host_plugin_basename()` takes the string and stores it, because it cannot do + * anything else: a host calls it at plugin-file scope, where `get_plugins()` does not exist yet. + * So the name is checked here, where it is about to decide something — and three ordinary shapes + * make it wrong for ever: a typo, `__FILE__` where `plugin_basename( __FILE__ )` was meant, and a + * host that `plugin_basename()` does not round-trip, an mu-plugin or one behind a symlink. Each + * one reads as a host that is not network-active, so the guard declines a deactivation that would + * strand nobody and the stranding notice comes back on every admin page load, telling the owner to + * network-activate a plugin that is already running everywhere. + */ + public function test_it_reports_a_host_basename_no_installed_plugin_answers_to(): void { + Config::set_host_plugin_basename( 'give/give.php' ); + + // Only the standalone is installed. The host names a plugin nothing on the site answers to. + $this->install_plugins( [ 'give-recurring/give-recurring.php' ] ); + $this->install_checker( $this->checker_network_active_for( [ 'give-recurring/give-recurring.php' ] ) ); + $this->expect_incorrect_usage(); + + $this->assertTrue( + $this->detector()->deactivation_would_strand_sites( + $this->make_sub_plugin( [ 'standalone_plugin_basename' => 'give-recurring/give-recurring.php' ] ) + ), + 'The answer is unchanged: a name nothing answers to is not consent to deactivate network-wide.' + ); + + $this->assert_the_library_reported_incorrect_usage_saying( + 'give/give.php', + 'The report has to name the basename that was configured, or there is nothing to correct.' + ); + $this->assertCount( + 1, + $this->incorrect_usage_reports, + 'Said once, and to the developer: the lookup is per request, not per call.' + ); + } + + /** + * One bootstrap mistake, reported once — not once for every sub-plugin the resolver walks past it + * with. `get_plugins()` parses the header of every plugin on the site, and a report repeated per + * sub-plugin buries the one sentence the host has to act on under copies of itself. + */ + public function test_it_reports_an_unknown_host_basename_once_however_many_sub_plugins_it_is_asked_about(): void { + Config::set_host_plugin_basename( 'give/give.php' ); + $this->install_plugins( [] ); + $this->install_checker( + $this->checker_network_active_for( + [ 'give-recurring/give-recurring.php', 'give-fee-recovery/give-fee-recovery.php' ] + ) + ); + $this->expect_incorrect_usage(); + + $detector = $this->detector(); + + $detector->deactivation_would_strand_sites( + $this->make_sub_plugin( [ 'standalone_plugin_basename' => 'give-recurring/give-recurring.php' ] ) + ); + $detector->deactivation_would_strand_sites( + $this->make_sub_plugin( + [ + 'slug' => 'give-fee-recovery', + 'standalone_plugin_basename' => 'give-fee-recovery/give-fee-recovery.php', + ] + ) + ); + + $this->assertCount( + 1, + $this->incorrect_usage_reports, + 'The mistake is the host\'s bootstrap, not either sub-plugin.' + ); + } + + /** + * The ordinary case, which has to stay silent: a host that passed + * `plugin_basename( __FILE__ )` names a plugin the site really has. + */ + public function test_it_says_nothing_about_a_host_basename_an_installed_plugin_answers_to(): void { + Config::set_host_plugin_basename( 'give/give.php' ); + $this->install_plugins( [ 'give/give.php', 'give-recurring/give-recurring.php' ] ); + $this->install_checker( $this->checker_network_active_for( [ 'give-recurring/give-recurring.php' ] ) ); + + $sub_plugin = $this->make_sub_plugin( + [ 'standalone_plugin_basename' => 'give-recurring/give-recurring.php' ] + ); + + // Asserted by its absence, and by WPTestCase rather than by anything here: a test that + // receives a _doing_it_wrong() it never expected fails on it, and this one expects none. + $this->assertTrue( $this->detector()->deactivation_would_strand_sites( $sub_plugin ) ); + + // An expectation that was never armed would keep an empty log just as convincingly, so the + // same recorder is shown catching a real report before the emptiness above is believed. + Config::set_host_plugin_basename( 'give/nothing-answers-to-this.php' ); + $this->expect_incorrect_usage(); + + $fresh = new Detector( + new Stub_Registry_Reader(), + $this->checker_network_active_for( [ 'give-recurring/give-recurring.php' ] ) + ); + + $fresh->deactivation_would_strand_sites( $sub_plugin ); + + $this->assert_the_library_reported_incorrect_usage_saying( + 'give/nothing-answers-to-this.php', + 'The recorder really is on the hook, and it is this basename it caught.' + ); + } + + /** + * The opt-in stays exactly as documented: with no host basename set the guard stands down before + * anything is read, the plugin list included. + */ + public function test_it_looks_up_nothing_when_no_host_basename_is_configured(): void { + $looked = 0; + + $this->setFunctionReturn( + 'get_plugins', + static function () use ( &$looked ): array { + ++$looked; + + return []; + }, + true + ); + $this->install_checker( $this->checker_network_active_for( [ 'give-recurring/give-recurring.php' ] ) ); + + $this->assertFalse( + $this->detector()->deactivation_would_strand_sites( + $this->make_sub_plugin( [ 'standalone_plugin_basename' => 'give-recurring/give-recurring.php' ] ) + ) + ); + $this->assertSame( 0, $looked, 'A host that never opted in pays nothing for the guard.' ); + + // The counter has to be shown to work, or a stub that failed to install passes this. + get_plugins(); + + $this->assertSame( 1, $looked ); + } + /** * The detector the container builds, which is the one the conflict step reaches. * @@ -896,4 +1039,19 @@ private function remove_the_load_gate(): void { $this->load_gate = null; } + + /** + * Say which plugins the site has installed, as `get_plugins()` answers it. + * + * Stubbed rather than written to disk: the host basename check asks WordPress what is installed, + * and a test that put a directory under wp-content/plugins would be asserting about the machine + * the suite runs on. + * + * @param string[] $basenames Plugin basenames the site has installed. + * + * @return void + */ + private function install_plugins( array $basenames ): void { + $this->setFunctionReturn( 'get_plugins', array_fill_keys( $basenames, [ 'Name' => 'Fixture' ] ) ); + } } diff --git a/tests/unit/Scenario/ConflictTest.php b/tests/unit/Scenario/ConflictTest.php index 4415688..db87a27 100644 --- a/tests/unit/Scenario/ConflictTest.php +++ b/tests/unit/Scenario/ConflictTest.php @@ -48,9 +48,10 @@ class ConflictTest extends Bootstrap_Test_Case { private const SECOND_SLUG = 'absorber-fee-recovery'; /** - * A host plugin basename, for the multisite stranding scenarios. Only ever an entry in - * `active_sitewide_plugins` and the value handed to `Config::set_host_plugin_basename()` — no - * fixture file stands behind it, because the guard reads its activation state and nothing more. + * A host plugin basename, for the multisite stranding scenarios. An entry in + * `active_sitewide_plugins`, the value handed to `Config::set_host_plugin_basename()`, and — since + * the guard also refuses to believe a basename nothing answers to — something `get_plugins()` + * reports as installed. No fixture file stands behind it: nothing ever loads the host itself. * * @var string */ @@ -114,7 +115,10 @@ public function test_a_network_active_standalone_is_left_when_the_host_is_not_ne update_site_option( 'active_sitewide_plugins', [ self::STANDALONE => time() ] ); // A host basename that is not itself network-active: the bundled copy would not load on the - // sites the standalone is being removed from, which is the whole reason to leave it. + // sites the standalone is being removed from, which is the whole reason to leave it. Installed + // but switched on nowhere, which is the topology under test — a basename naming no plugin at + // all is a bootstrap mistake and is reported as one. + $this->install_the_host_plugin(); Config::set_host_plugin_basename( self::HOST ); $constant = $this->define_guard( 'ABSORBER_E2E_STRANDING_GUARD' ); @@ -151,6 +155,56 @@ public function test_a_network_active_standalone_is_left_when_the_host_is_not_ne ); } + /** + * The same topology, with the one difference a host cannot see: the basename it named itself with + * is not a plugin this site has. `plugin_basename( __FILE__ )` misspelled, `__FILE__` passed + * instead of it, or a host WordPress reaches by a path that does not round-trip — all of them + * answer "not network-active" for ever, which is exactly what a correctly configured + * per-site host answers, so the guard declines and the notice recurs with nothing to fix it. + * + * Nothing about the site changes: a name the library cannot resolve is not consent to take a + * plugin off every site on a network. What changes is that the developer is told which basename + * nothing answers to, which is the only thing that ends the loop. + */ + public function test_a_host_basename_no_installed_plugin_answers_to_is_reported(): void { + if ( ! is_multisite() ) { + $this->markTestSkipped( 'Network activation only exists on multisite.' ); + } + + update_site_option( 'active_sitewide_plugins', [ self::STANDALONE => time() ] ); + + // Deliberately not installed: no `install_the_host_plugin()` here, so `get_plugins()` answers + // for the real site and nothing in it answers to this name. + Config::set_host_plugin_basename( self::HOST ); + + $constant = $this->define_guard( 'ABSORBER_E2E_UNKNOWN_HOST_GUARD' ); + + $this->register( + [ + 'standalone_plugin_basename' => self::STANDALONE, + 'conflict_policy' => Conflict_Policy::DEACTIVATE, + ], + $constant + ); + + $this->expect_incorrect_usage(); + + $this->boot(); + $this->run_request(); + + $this->assert_the_library_reported_incorrect_usage_saying( + self::HOST, + 'The developer has to be told which basename nothing answers to.' + ); + + $this->assertArrayHasKey( + self::STANDALONE, + (array) get_site_option( 'active_sitewide_plugins', [] ), + 'The report is a diagnosis: it does not turn a name we cannot resolve into a network-wide deactivation.' + ); + $this->assertArrayHasKey( self::SLUG . ':stranding', $this->queued_notices() ); + } + /** * The other half: when the host plugin is itself network-active, its bundled copy loads on every * site the standalone is removed from, so the network-wide deactivation strands nothing and the @@ -169,6 +223,7 @@ public function test_a_network_active_standalone_is_deactivated_when_the_host_is ] ); + $this->install_the_host_plugin(); Config::set_host_plugin_basename( self::HOST ); $this->register( @@ -654,4 +709,28 @@ public function test_a_reactivation_attempt_yields_the_friendly_message(): void // the sentence inside belongs to this library. $this->assertStringStartsWith( '

', $filtered ); } + + /** + * Say the host plugin is one the site has installed, as `get_plugins()` answers it. + * + * The stranding guard checks the basename it was configured with against the installed plugins, so + * a scenario about *activation scope* has to name a plugin that exists — otherwise it would be + * running the misconfiguration case above under another name. Stubbed rather than written into + * wp-content/plugins, which would leave a directory behind for every test after it. + * + * @return void + */ + private function install_the_host_plugin(): void { + // uopz cannot stub a function that does not exist yet, and get_plugins() lives in an admin + // file WordPress loads on demand. + require_once ABSPATH . 'wp-admin/includes/plugin.php'; + + $this->setFunctionReturn( + 'get_plugins', + [ + self::HOST => [ 'Name' => 'Absorber Host' ], + self::STANDALONE => [ 'Name' => 'Absorber Standalone' ], + ] + ); + } }