diff --git a/CLAUDE.md b/CLAUDE.md index 1143f48..c76cb86 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -118,8 +118,8 @@ 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. -`Loader` 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 +`Loader` keeps the public surface. `registrar()`, `notices()` and `resolver()` 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. `Sub_Plugin` is a value object answering the per-sub-plugin questions it can answer **without a @@ -133,13 +133,13 @@ the plugin to ask about, and the collaborator does the asking. ### What exists today -`src/Conflict/` — `Resolver`, `Gatekeeper`, `Redirector` — and `Activator` are not built yet. -Currently: +Every behaviour described above is built; nothing in `src/` is still owed. What is left in the plan +is the end-to-end suite and the release pass. | Path | What | |---|---| | `src/Config.php` | Static facade: hook prefix + container. | -| `src/Loader.php` | Static facade: the registration buffer, `boot()`, and the accessors. | +| `src/Loader.php` | Static facade: the registration buffer, `boot()`, the accessors, and the two notice trampolines. | | `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/Load/Runner.php` | The load pass: the gate chain, the `require_once`, the activation callback. | @@ -147,9 +147,11 @@ 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/Activator.php` | Runs a sub-plugin's activation callback once ever, recorded in one option. | +| `src/Conflict/` | `Resolver` (which policy branch to take), `Gatekeeper` (which requests may take one), `Redirector` (where the user lands afterwards), `Contracts\Resolver_Interface`. | | `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`. | +| `src/Notices/` | `Queue` (what a notice says, who may consume it, and the activation-error rewrite), `Store` (keeps it), `Renderer` (draws it), `Contracts\Queue_Interface`. | +| `src/Contracts/`, `src/Exceptions/` | `Provider_Interface`, `Registrar_Interface`, `Plugin_Deactivator_Interface`, `Plugin_Checker_Interface`, `Activator_Interface`, `Config_Exception`. | ### Boot lifecycle @@ -161,7 +163,7 @@ Loader::boot(); // idempotent → Provider::register() // every binding → Boot\Scheduler // every hook, as a closure over the container -plugins_loaded @1 → Conflict\Resolver::resolve_all() [gated by Conflict\Gatekeeper] +plugins_loaded @1 → Conflict\Gatekeeper, then Conflict\Resolver::resolve_all() plugins_loaded @2 → Load\Runner::load_all() all_admin_notices → Loader::render_notices() [is_admin() only] wp_admin_notice_markup → Loader::filter_activation_error_markup() [is_admin() only] @@ -177,10 +179,28 @@ earlier holds an orphan whose bindings are discarded. This is also why `Loader:: and resolves nothing — registration at plugin-file scope, which the spec sanctions, would otherwise register into the throwaway. +**The too-late barrier measures against the first step in the sequence, not the last.** +`Boot\Scheduler` compares the priority `plugins_loaded` is already dispatching against the lowest +priority it has to wire — conflict resolution at 1, not the load at 2 — and over that line it runs +the whole sequence inline in hook order rather than wiring any of it. Measuring against the load +would let a host booting at priority 1 wire the load and silently lose the conflict pass, which is +the half of the sequence a fatal depends on. The comparison is inclusive, because a callback added +at the priority currently being dispatched is accepted and never reached. + `load_all()` gates each sub-plugin in order, skipping on the first failure: enabled → not already loaded → dependencies met → file exists → `should_load` filter → `require_once` → activation callback (only after a *successful* require). +The activation callback is the last of those and runs through `Activator`, which `Load\Runner` takes +as a constructor argument like the notice queue. Last, because a bundled plugin is included rather +than activated: `register_activation_hook()` never fires for it, so the callback stands in for +whatever that hook would have done, and it has to run with the plugin's own code already in memory. +Only after a require that happened, because creating tables and seeding options for a sub-plugin +whose code is *not* loaded is worse than not creating them — and the once-ever record would then +stand the callback down for good, the first time the sub-plugin really did load. The record is +written after the callback returns, never before, so a callback that throws is retried next request +rather than marked done. + The guard constant is checked **before** the dependency check, not after. It is one `defined()`, it carries the whole re-declaration guarantee, and it is the only gate meaning "this plugin is already running" — warning that requirements are unmet for a plugin the admin can watch working would send @@ -188,28 +208,66 @@ them after the wrong problem. `docs/filters.md` and the spec agree. `Loader::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 interface signature — so it is filtered once where the untrusted value enters. All three readers +— the load pass, the conflict pass and the activation-error rewrite — go through `Loader::all()` +rather than through the registrar they could resolve for themselves, because it flushes the pending +registrations before it reads and a registrar asked directly would miss anything registered since +the last flush. `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. -`Conflict\Redirector` decides where to; it returns `false` when the referrer is already `plugins.php`, -so an inline update is never interrupted. It decides and never navigates — `wp_safe_redirect()` and -`exit` stay in the resolver, so the policy action and the admin-URL knowledge change for separate -reasons. +`DEACTIVATE` (the default) deactivates network-aware, queues a merge notice, and redirects. It is +the worked example of required injection — `Plugin_Checker_Interface` to detect the standalone, +`Plugin_Deactivator_Interface` to turn it off, `Queue_Interface` for the notice and +`Conflict\Redirector` for the destination, all four constructor arguments with no default — so the +object a test builds is the object the provider builds, and a host's rebinding of either plugin seam +reaches it without the resolver knowing a container exists. + +`Conflict\Redirector::after_deactivation( $referrer )` decides where the user lands and never goes +there: `wp_safe_redirect()` and the `exit` after it stay in the resolver, so the policy action and +the admin-URL knowledge change for separate reasons, and every destination is assertable without a +test standing in for the end of a request. It returns `false` — stay put — when the referrer is +already `plugins.php`, since that list is about to render the deactivation anyway; it sends +`update.php` and `update-core.php` to `plugins.php`, since reloading either re-runs an update; with +no usable referrer, `plugins.php`. It matches on the screen basename, not on a substring of an +absolute URL: `wp_get_referer()` prefers the bare `_wp_http_referer` path that every nonce-bearing +admin form carries, so comparing against `admin_url()` would miss the network admin and every site +behind a TLS-terminating proxy. **Who may have a conflict resolved is `Conflict\Gatekeeper`'s business, not the resolver's.** It -gates on an interactive admin `GET` (`plugins_loaded` fires on every request) *and* on -`current_user_can( 'activate_plugins' )` (`plugins_loaded` runs before `auth_redirect()`, so an -unauthenticated GET of an admin URL gets that far). The hook resolves the gatekeeper rather than the -resolver, so a host binding its own `Resolver_Interface` cannot drop either gate by omission. The -capability gate covers every policy, not just the destructive one, and that is free: the other -branches only queue a notice, and `Notices\Queue::render()` refuses to render *or clear* for a user -without the same capability, so queuing earlier would only park it until a capable admin arrives. +gates on an interactive admin `GET` (`plugins_loaded` fires on every request, including cron, CLI +and a visitor's POST) *and* on `current_user_can( 'activate_plugins' )` (`plugins_loaded` runs +before `auth_redirect()`, so an unauthenticated GET of an admin URL gets that far). The +`plugins_loaded` step asks the gatekeeper *before* it resolves `Resolver_Interface` at all, so a +host binding its own resolver cannot drop either gate by omission — and a request that fails one +never builds a resolver. The capability gate covers every policy, not just the destructive one, and +that is free: the other branches only queue a notice, and `Notices\Queue::render()` refuses to +render *or clear* for a user without the same capability, so queuing earlier would only park it +until a capable admin arrives. An unknown policy must be handled as its own case via `Conflict_Policy::is_valid()`, never left to a `default:` fallthrough — a typo like `'defered'` would otherwise deactivate a plugin the site owner deliberately turned on. +**The activation-error rewrite lives on `Notices\Queue`, not on a class of its own.** It is the one +conflict the load guard cannot prevent — core includes the plugin being activated *after* the +bundled copy is in memory, so the re-declaration really does fatal — and all this library gets to do +about it is reword the sentence core's sandbox prints. That sentence is +`conflict_notice_message`, the same message the merge notice carries, so wording it belongs where +every other notice is worded; a host that binds its own `Queue_Interface` owns the error screen +along with the rest. `Loader::filter_activation_error_markup()` is the trampoline, and it takes an +**untyped** argument: a filter receives whatever the filter before it returned, and a `string` +declaration would turn another plugin's sloppy return into a TypeError raised from here, on the +screen least able to afford a second one. The rewrite refuses unless the screen is `plugins`, the +`plugin` query arg names a registered standalone and `_error_nonce` verifies — and it sanitises with +`wp_kses_post()` *before* testing for emptiness, since a message that filters down to nothing must +leave core's wording standing rather than blank the notice box. + +**`Boot\Scheduler` wires `wp_admin_notice_markup` as a named static callback, not a closure.** Both +admin-only hooks are `[ Loader::class, … ]` pairs that resolve the queue when they fire, so neither +builds anything at boot; what the name buys on this one is `remove_filter()`, which a host wanting +core's wording back has no other way to reach. The `plugins_loaded` steps are closures for a reason +these two do not share — that sequence has to be runnable inline as well as wirable. + ### Keys - Filters: `{$hook_prefix}/plugin_absorber/should_load`, `{$hook_prefix}/plugin_absorber/conflict_policy` diff --git a/README.md b/README.md index 7a9ac63..310b576 100644 --- a/README.md +++ b/README.md @@ -39,11 +39,11 @@ add_action( 'plugins_loaded', function () { The container is required — any StellarWP `ContainerInterface` implementation, the one you already hand to Telemetry or Uplink. Every collaborator comes from it. -Keep the `, 0`. `boot()` wires the load at `plugins_loaded` priority 2, and WordPress silently -ignores a callback added at or past the priority it is already dispatching — so configuring the -library from a provider that itself runs at priority 2 or later races the library it is configuring. -Booting later is reported through `_doing_it_wrong()` and loaded inline, but the ordering guarantees -are weaker. +Keep the `, 0`. `boot()` wires conflict resolution at `plugins_loaded` priority 1 and the load at +priority 2, and WordPress silently ignores a callback added at or past the priority it is already +dispatching — so configuring the library from a provider that itself runs at priority 1, which is +where several hosts wire their container today, races the library it is configuring. Booting later is +reported through `_doing_it_wrong()` and loaded inline, but the ordering guarantees are weaker. Put this in the block that owns your container, not in a service provider, and pass the container you intend to keep: a host that builds one lazily and replaces it later leaves us holding an orphan whose @@ -53,7 +53,7 @@ bindings were discarded. - [Installing](docs/installing.md) — Composer, Strauss, and the constants Strauss must leave alone. - [Configuration](docs/configuration.md) — the hook prefix, the container, every sub-plugin key. -- [Conflict handling](docs/conflict-handling.md) — the policies, the load guard, and its limits. +- [Conflict handling](docs/conflict-handling.md) — the policies, when they run, and the guard's limits. - [Filters](docs/filters.md) — the runtime overrides for policies and notice text. - [Notices](docs/notices.md) — where the queue lives, who may see it, and how to render it yourself. diff --git a/docs/configuration.md b/docs/configuration.md index c5ab7f1..ac2fc41 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -47,11 +47,20 @@ $container->singleton( Registrar_Interface::class, My_Registrar::class ); | `Notices\Contracts\Queue_Interface` | `Notices\Queue` | Queues and renders the admin notices. | | `Contracts\Plugin_Deactivator_Interface` | `Plugin_Deactivator` | Deactivates the standalone. | | `Contracts\Plugin_Checker_Interface` | `Plugin_Checker` | Answers whether a plugin is active. | +| `Conflict\Contracts\Resolver_Interface` | `Conflict\Resolver` | Detects the active standalone and applies the policy. | +| `Contracts\Activator_Interface` | `Activator` | Runs a sub-plugin's activation callback once, ever. | `Plugin_Checker_Interface` is the seam to rebind when your plugin filters `option_active_plugins` or `site_option_active_sitewide_plugins` — LearnDash injects and then strips a synthetic path — because `is_plugin_active()` then does not report what is in the database. +Rebinding `Resolver_Interface` does not put you in charge of *when* resolution may run. Both gates — +[an interactive admin `GET`, and the `activate_plugins` +capability](conflict-handling.md#when-resolution-runs) — live in `Conflict\Gatekeeper`, which the +hook consults before it resolves the resolver at all, so an implementation that never thought about +either is still safe. Everything the resolver *does* — which policy branch, what the notice says, +where the user lands — is yours. + `set_container()` is a configuration call like `set_hook_prefix()`, and order does not matter among the configuration calls: it may come before or after your `Loader::register()` calls, so long as it comes before boot. Registering buffers the sub-plugin and resolves nothing, so nothing is decided @@ -95,6 +104,29 @@ at the second `register()` call; a config array the library cannot use is still Register unconditionally and put anything you cannot decide up front — a licence that may not be active, a setting the site owner can change — in `enabled`, which is re-evaluated on every load. +## Activation + +A bundled plugin is `require_once`d, not activated, so `register_activation_hook()` never fires for +it — whatever that hook would have done, creating a table or seeding options, would otherwise never +happen at all. [`activation_callback`](#sub-plugin-keys) fills that gap: + +```php +'activation_callback' => static function ( Sub_Plugin $sub_plugin ) { + \Give\Recurring\Install::create_tables(); +}, +``` + +It runs exactly once ever per slug, is passed the `Sub_Plugin`, and runs only after a require that +actually happened — never for a sub-plugin whose load was skipped, because a schema appearing for a +plugin that is not loaded is worse than no schema at all. + +The record lives in the `{option_prefix}_plugin_absorber_activations` option, a network option on +multisite for the same reason the [notice queue](notices.md) is one: `deactivate_plugins()` is +network-wide, so a merge that happened network-wide must not re-run the callback on every site. The +slug is recorded *after* the callback returns, so a callback that fails is retried on the next +request rather than marked done and silently skipped forever. Bind `Activator_Interface` to record +"once, ever" somewhere else — your own migration table, say. + ## The bundled file is included from a function, not from global scope WordPress includes plugins from `wp-settings.php` at global scope; this library includes them from diff --git a/docs/conflict-handling.md b/docs/conflict-handling.md index 43cd75b..6c23467 100644 --- a/docs/conflict-handling.md +++ b/docs/conflict-handling.md @@ -10,8 +10,61 @@ When a sub-plugin's standalone counterpart is still active: | `Conflict_Policy::DEFER` | Leave the standalone active; the load guard stands the bundled copy down. | | `Conflict_Policy::NOTICE_ONLY` | Leave it active and ask the user to deactivate it. | -Set one per sub-plugin with the `conflict_policy` key, or decide it at runtime with the -`conflict_policy` [filter](filters.md), which has the final say. +Set one per sub-plugin with the `conflict_policy` key — a constant, or a `callable( Sub_Plugin ): +string`. The `conflict_policy` [filter](filters.md) runs after that and has the final say: + +```php +// In the config: stand down when a newer standalone supersedes the bundled copy. +'conflict_policy' => static fn( Sub_Plugin $sub ) => give_standalone_is_newer( $sub ) + ? Conflict_Policy::DEFER + : Conflict_Policy::DEACTIVATE, + +// Anywhere, and last: +add_filter( 'give/plugin_absorber/conflict_policy', static function ( $policy, $sub ) { + return $sub->get_slug() === 'give-recurring' ? Conflict_Policy::NOTICE_ONLY : $policy; +}, 10, 2 ); +``` + +**An unrecognised policy is treated as `NOTICE_ONLY`**, never as consent to deactivate. +`Conflict_Policy::is_valid()` decides, so a typo like `'defered'` — in a policy a host persisted in +an option, or in whatever that filter returned — only produces a notice. A value nobody chose must +not turn off a plugin somebody chose. + +A policy is only reached for a sub-plugin that is enabled, names a `standalone_plugin_basename`, and +whose standalone is active right now; everything else is skipped before any policy is read. + +## When resolution runs + +At `plugins_loaded` priority 1, one ahead of the load pass at 2: a standalone that survives the +conflict defines the guard constant as it loads, and the load pass has to see that. + +It runs **only on an interactive admin `GET`** — not WP-CLI, not cron, not ajax, not a form POST — +because resolving can deactivate a plugin and end the request with a redirect. Ungated, a visitor's +checkout POST would come back as a 302 that discards what was submitted and drops the order, and a +WP-CLI command would exit having printed nothing, because `header()` is a no-op under the CLI SAPI. +Waiting costs nothing: the standalone is still there to detect on the next page view. + +It also requires `current_user_can( 'activate_plugins' )`: `plugins_loaded` fires well before +`auth_redirect()`, so an unauthenticated GET of an admin URL reaches this code on its way to the +login screen, and whoever cannot activate a plugin must not be able to deactivate one. This applies +to every policy rather than only to `deactivate`, which costs nothing — the other policies just +queue a notice, and a notice is neither shown nor cleared for a user without that same capability, +so nothing is consumed by waiting for one who has it. + +Both gates live in `Conflict\Gatekeeper`, and the hook asks it *before* it resolves +`Conflict\Contracts\Resolver_Interface` at all. So binding your own resolver cannot drop them by +omission: on a request that fails either gate your implementation is never built, let alone called. + +## The redirect + +After deactivating, the user goes back to whatever they were looking at, so it re-renders without the +standalone. Two referrers differ: `plugins.php` stays put, since the list is about to show the change +anyway, and the update screens (`update.php`, `update-core.php`) go to `plugins.php` instead, because +reloading one of those would re-run an update. With no referrer at all, `plugins.php`. + +`Conflict\Redirector` makes that decision and returns it; the redirect itself is the resolver's. The +merge notice is queued before either, so the explanation survives whether or not the request ends in +a redirect. ## The load guard @@ -43,7 +96,26 @@ jobs. The cost is a missed detection; the alternative costs the guarantee the gu ## What the guard cannot do -The guard cannot help on the request that *activates* the standalone: WordPress includes it after -the bundled copy has already loaded, so that re-declaration is a real fatal. WordPress catches it -in its activation sandbox, and this library rewrites the resulting error screen into an -explanation. +The guard cannot help on the request that *activates* the standalone: WordPress includes the plugin +being activated **after** the bundled copy has already loaded, so that re-declaration is a real +fatal. Core catches it in its activation sandbox and prints *"Plugin could not be activated because +it triggered a fatal error."* — true, and useless to whoever pressed the button. + +So the library filters `wp_admin_notice_markup` and swaps that sentence for the sub-plugin's +`conflict_notice_message`, falling back to a generic one naming the slug. This is what puts the +WordPress floor at 6.4: the filter does not exist before it. + +It touches nothing else. The markup comes back unchanged unless all three hold — the screen is +`plugins`, the `plugin` query arg names a standalone this library has registered, and `_error_nonce` +verifies against `plugin-activation-error_{basename}`. Another plugin's fatal is another plugin's +business. + +The replacement runs through `wp_kses_post()`, so a knowledge-base link survives, and it is +sanitised *before* it is checked for emptiness: a message that filters down to nothing leaves core's +wording in place rather than blanking the notice. + +The filter is wired by `Boot\Scheduler` under `is_admin()`, as +`[ Loader::class, 'filter_activation_error_markup' ]` — a named callback, so a host that would +rather keep core's wording can `remove_filter()` it. The rewriting itself is +`Notices\Queue::filter_activation_error_markup()`, so a host that binds its own `Queue_Interface` +owns this screen along with the rest of the notices. diff --git a/docs/filters.md b/docs/filters.md index 4063f16..14e8da2 100644 --- a/docs/filters.md +++ b/docs/filters.md @@ -12,7 +12,9 @@ Each runs last, after the configured value and any fallback. Because they fire w asked for rather than when the sub-plugin is registered, they are also the place to call `__()` — by then the textdomain is loaded. -A filter returning a non-scalar yields an empty string rather than a fatal cast. +A filter returning a non-scalar yields an empty string rather than a fatal cast. A `conflict_policy` +return that is not one of the three constants is treated as [`NOTICE_ONLY`, never as consent to +deactivate](conflict-handling.md#policies). ## The load gate diff --git a/docs/notices.md b/docs/notices.md index a1e73f0..99e7a19 100644 --- a/docs/notices.md +++ b/docs/notices.md @@ -21,6 +21,13 @@ only warning an administrator was ever going to get. On multisite `activate_plugins` maps through `manage_network_plugins`, so it is a network administrator, not the site administrator who installed the plugin, who sees these. +## `conflict_notice_message` is used twice + +The same message backs the queued notice raised when the standalone is deactivated *and* the +rewritten activation-error screen a user meets if they try to re-activate it — see +[conflict handling](conflict-handling.md). Write one sentence that reads sensibly both as a report of +something already done and as the explanation standing in for a fatal-error warning. + ## Rendering them yourself `Notices\Queue::option_name()` is public, so you can render the queue yourself without replacing diff --git a/docs/superpowers/plans/2026-08-12-container-required-rework.md b/docs/superpowers/plans/2026-08-12-container-required-rework.md index ae092fa..e27f91b 100644 --- a/docs/superpowers/plans/2026-08-12-container-required-rework.md +++ b/docs/superpowers/plans/2026-08-12-container-required-rework.md @@ -287,15 +287,17 @@ level 9, so most breakage surfaces from `composer test:analysis` without standin ### Task 13 — `13-activation` -- [ ] `Activation` → `Activator`, `Activation_Interface` → `Activator_Interface`; constructor injection; bind in `Provider`. +- [x] `Activation` → `Activator`, `Activation_Interface` → `Activator_Interface`; constructor injection; bind in `Provider`. ### Task 14 — `14-activation-error-rewrite` -- [ ] Rebase; de-null any collaborator constructor the branch adds; keep `wp_admin_notice_markup` wiring in `Boot\Scheduler`. +- [x] Rebase; de-null any collaborator constructor the branch adds; keep `wp_admin_notice_markup` wiring in `Boot\Scheduler`. ### Task 15 — `15-e2e-suite` -- [ ] The end-to-end bootstrap now builds a container and calls `Provider::register()` before `Loader::boot()`. +- [x] The end-to-end bootstrap now hands `Config::set_container()` a bare container and lets `Loader::boot()` + run `Provider::register()` over it, which is the sequence a host runs. A test about a rebinding host binds + its own implementations into that container first. ### Task 16 — `16-readme-release` diff --git a/src/Activator.php b/src/Activator.php new file mode 100644 index 0000000..68b94c7 --- /dev/null +++ b/src/Activator.php @@ -0,0 +1,83 @@ + true map in one option. + * + * @since 1.0.0 + */ +class Activator implements Activator_Interface { + /** + * @since 1.0.0 + * + * @param Sub_Plugin $sub_plugin Sub-plugin that has just been loaded. + * + * @throws Config_Exception When no hook prefix has been set. + * + * @return void + */ + public function maybe_run( Sub_Plugin $sub_plugin ): void { + $callback = $sub_plugin->get_activation_callback(); + + if ( $callback === null ) { + return; + } + + $slug = $sub_plugin->get_slug(); + $done = $this->completed(); + + if ( ! empty( $done[ $slug ] ) ) { + return; + } + + // Recorded after the callback rather than before it. A callback that fatals halfway leaves + // the site half-migrated either way, but recording first would also mean the next request + // skips it, so the half-finished state becomes permanent and invisible. + $callback( $sub_plugin ); + + $done[ $slug ] = true; + + update_site_option( self::option_name(), $done ); + } + + /** + * The option every slug's activation record lives in. + * + * @since 1.0.0 + * + * @throws Config_Exception When no hook prefix has been set. + * + * @return string + */ + public static function option_name(): string { + return Config::get_option_name( 'activations' ); + } + + /** + * Slugs whose activation callback has already run, keyed by slug. + * + * A site option, not a plain one: `deactivate_plugins()` is network-wide on multisite, so an + * activation that follows a network-wide merge has to be recorded network-wide too, or every + * site in the network runs the callback again. + * + * @since 1.0.0 + * + * @throws Config_Exception When no hook prefix has been set. + * + * @return array + */ + private function completed(): array { + $done = get_site_option( self::option_name(), [] ); + + // Anything that is not an array is replaced rather than trusted. A corrupted option would + // otherwise fatal on the array read, inside plugins_loaded, on every request. + return is_array( $done ) ? $done : []; + } +} diff --git a/src/Boot/Scheduler.php b/src/Boot/Scheduler.php index 3c69b4c..8d2347f 100644 --- a/src/Boot/Scheduler.php +++ b/src/Boot/Scheduler.php @@ -5,6 +5,8 @@ namespace Nexcess\PluginAbsorber\Boot; +use Nexcess\PluginAbsorber\Conflict\Contracts\Resolver_Interface; +use Nexcess\PluginAbsorber\Conflict\Gatekeeper; use Nexcess\PluginAbsorber\Load\Runner; use Nexcess\PluginAbsorber\Loader; use StellarWP\ContainerContract\ContainerInterface; @@ -34,6 +36,18 @@ class Scheduler { */ private const LOAD_PRIORITY = 2; + /** + * plugins_loaded priority conflict resolution runs at, ahead of the load pass. + * + * A standalone that survives the conflict defines the guard constant as it loads, and the load + * pass has to see that, so resolution cannot share a priority with it. + * + * @since 1.0.0 + * + * @var int + */ + private const RESOLVE_PRIORITY = 1; + /** * @since 1.0.0 * @@ -57,6 +71,10 @@ public function __construct( ContainerInterface $container ) { * collaborator when the hook fires, so a host may still rebind one after boot() and up until * plugins_loaded, and a binding nothing reaches is never built at all. * + * Called too late, the steps run inline instead of being wired — and conflict resolution can + * end the request, so on an admin page load this call may not return. Boot at plugins_loaded + * priority 0, as documented, and it always does. + * * @since 1.0.0 * * @return void @@ -68,6 +86,14 @@ public function wire(): void { // superadmin working in the network admin -- exactly where a network-wide // deactivation gets noticed -- would never see the queue rendered. add_action( 'all_admin_notices', [ Loader::class, 'render_notices' ] ); + + // A named static trampoline like the notice step above, not a closure. Both resolve + // the queue when they fire, so neither builds anything at boot, but a named callback + // can also be taken back with remove_filter() -- which matters most here, on the one + // hook that rewrites a screen WordPress drew rather than adding one of our own. The + // closures below are shaped that way for a reason these two do not share: the + // plugins_loaded sequence has to be runnable inline as well as wirable. + add_filter( 'wp_admin_notice_markup', [ Loader::class, 'filter_activation_error_markup' ] ); } // Adding an action at a priority the current dispatch has already passed is accepted and @@ -77,7 +103,7 @@ public function wire(): void { if ( $this->wiring_window_has_closed() ) { _doing_it_wrong( Loader::class . '::boot', - 'Loader::boot() must run before plugins_loaded priority 2. Loading inline instead.', + 'Loader::boot() must run before plugins_loaded priority 1. Resolving and loading inline instead.', '1.0.0' ); @@ -109,12 +135,25 @@ public function wire(): void { * * @since 1.0.0 * - * @return array + * @return non-empty-array */ private function sequence(): array { $container = $this->container; return [ + [ + 'priority' => self::RESOLVE_PRIORITY, + 'run' => static function () use ( $container ): void { + // The gatekeeper first, and the resolver only once it has said yes. Who may have + // a conflict resolved is the library's invariant rather than the host's policy, + // so it is settled before anything a host may have rebound is even built. + if ( ! $container->get( Gatekeeper::class )->may_resolve() ) { + return; + } + + $container->get( Resolver_Interface::class )->resolve_all(); + }, + ], [ 'priority' => self::LOAD_PRIORITY, 'run' => static function () use ( $container ): void { @@ -125,13 +164,17 @@ private function sequence(): array { } /** - * Whether it is already too late to wire the load hook. + * Whether it is already too late to wire the first step of the sequence. + * + * Measured against the earliest priority in sequence(), read rather than restated, because a + * boot that can still wire a later step but has missed an earlier one has missed something — + * and with resolution at 1 and the load at 2, booting between the two is a real window. * * The comparison is inclusive. A callback added to the priority currently being dispatched is * accepted and never reached either: WP_Hook::apply_filters() walks `$this->callbacks[$priority]` * with a by-value foreach, so the append lands on an array the running loop has already copied. - * Booting from plugins_loaded at priority 2 is the case a host is likeliest to hit by accident, - * and an exclusive comparison would let exactly that one through unreported. + * Booting from plugins_loaded at that priority is the case a host is likeliest to hit by + * accident, and an exclusive comparison would let exactly that one through unreported. * * @since 1.0.0 * @@ -148,6 +191,7 @@ private function wiring_window_has_closed(): bool { $hook = $GLOBALS['wp_filter']['plugins_loaded'] ?? null; - return $hook instanceof WP_Hook && $hook->current_priority() >= self::LOAD_PRIORITY; + return $hook instanceof WP_Hook + && $hook->current_priority() >= min( array_column( $this->sequence(), 'priority' ) ); } } diff --git a/src/Conflict/Contracts/Resolver_Interface.php b/src/Conflict/Contracts/Resolver_Interface.php new file mode 100644 index 0000000..1819b5e --- /dev/null +++ b/src/Conflict/Contracts/Resolver_Interface.php @@ -0,0 +1,32 @@ +is_interactive_admin_request() + && $this->can_resolve_conflicts() + && self::has_hook_prefix(); + } + + /** + * Whether this request is one a person is watching in wp-admin. + * + * Conflict resolution deactivates a plugin and ends the request, so it must only run where + * someone is there to see the result. Unguarded it fires at plugins_loaded on every request: + * a visitor's checkout POST becomes a 302 that drops the order, a login POST bounces back to + * a blank form, wp-cron never reaches its event loop, and a WP-CLI command exits 0 having + * printed nothing, because header() is a no-op under the CLI SAPI. + * + * is_admin() alone is not enough: admin-ajax.php and admin-post.php both define WP_ADMIN. + * + * @since 1.0.0 + * + * @return bool + */ + private function is_interactive_admin_request(): bool { + if ( defined( 'WP_CLI' ) && WP_CLI ) { + return false; + } + + if ( wp_doing_cron() || wp_doing_ajax() ) { + return false; + } + + // Only a GET. A redirect discards the request, and the browser follows it with a GET, so + // anything submitted is gone -- which is exactly what would happen to a form posted to + // admin-post.php or options.php, both of which define WP_ADMIN and neither of which + // wp_doing_ajax() catches. Core draws the same line in wp_cron(). Deferring resolution to + // the next page view costs nothing: the standalone is still there to detect. + if ( ( $_SERVER['REQUEST_METHOD'] ?? 'GET' ) !== 'GET' ) { + return false; + } + + return is_admin(); + } + + /** + * Whether the current user may have a conflict resolved on their request. + * + * Reaching conflict resolution does not mean anyone is signed in. wp-admin/admin.php loads + * wp-load.php -- which dispatches plugins_loaded -- well before it calls auth_redirect(), so an + * unauthenticated GET of any admin URL gets this far. Without this check a stranger could turn + * the standalone off site-wide by requesting a page they are about to be bounced off. + * + * Here rather than inside the default resolver, because it is the one thing about conflict + * resolution that must survive a host binding its own: whoever cannot activate a plugin must not + * be able to deactivate one, and a replacement that forgot to re-check would reopen exactly that. + * + * It gates every policy, not only the destructive one, and that costs nothing. The other + * policies queue a notice, and Notices\Queue::render() will not render -- or clear -- for a user + * without this same capability. Queuing on a request that cannot act only parks the notice until + * a capable administrator arrives, which is the request this gate lets resolution run on anyway. + * Nothing is consumed or suppressed by waiting: the standalone is still there to detect. + * + * @since 1.0.0 + * + * @return bool + */ + private function can_resolve_conflicts(): bool { + return current_user_can( 'activate_plugins' ); + } +} diff --git a/src/Conflict/Redirector.php b/src/Conflict/Redirector.php new file mode 100644 index 0000000..08e8f16 --- /dev/null +++ b/src/Conflict/Redirector.php @@ -0,0 +1,56 @@ +plugin_checker = $plugin_checker; + $this->plugin_deactivator = $plugin_deactivator; + $this->notices = $notices; + $this->redirector = $redirector; + } + + /** + * @since 1.0.0 + * + * @throws Config_Exception When no hook prefix has been set, or a container binding is unusable. + * + * @return void + */ + public function resolve_all(): void { + // Loader::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 ( Loader::all() as $sub_plugin ) { + if ( ! $sub_plugin->is_enabled() || ! $sub_plugin->has_standalone_plugin() ) { + continue; + } + + if ( ! $this->plugin_checker->is_active( $sub_plugin->get_standalone_plugin_basename() ) ) { + continue; + } + + $this->resolve( $sub_plugin ); + } + } + + /** + * @since 1.0.0 + * + * @param Sub_Plugin $sub_plugin Sub-plugin whose standalone is active. + * + * @throws Config_Exception When no hook prefix has been set, or a container binding is unusable. + * + * @return void + */ + protected function resolve( Sub_Plugin $sub_plugin ): void { + $policy = $sub_plugin->get_conflict_policy(); + + // A host may persist a policy in an option and a filter may return anything. Falling + // through to deactivate() would turn off a plugin the site owner deliberately activated + // on the strength of a typo, so an unrecognised policy takes the conservative branch. + if ( ! Conflict_Policy::is_valid( $policy ) ) { + $policy = Conflict_Policy::NOTICE_ONLY; + } + + switch ( $policy ) { + case Conflict_Policy::DEFER: + // The standalone wins. Its own constant makes the load path skip the bundled copy. + return; + + case Conflict_Policy::DEACTIVATE: + $this->deactivate( $sub_plugin ); + + return; + + // NOTICE_ONLY, and anything is_valid() would accept that this switch has grown no + // branch for. The default sits on the branch that only talks, never on the one that + // deactivates: a policy nobody wrote must not be read as consent to turn a plugin off. + default: + $this->notices->queue_conflict_notice( $sub_plugin ); + } + } + + /** + * @since 1.0.0 + * + * @param Sub_Plugin $sub_plugin Sub-plugin whose standalone is active. + * + * @throws Config_Exception When no hook prefix has been set, or a container binding is unusable. + * + * @return void + */ + protected function deactivate( Sub_Plugin $sub_plugin ): void { + $this->plugin_deactivator->deactivate( $sub_plugin->get_standalone_plugin_basename() ); + + // Queued after the deactivation but before the redirect, so the explanation is durable + // whether or not the request goes on to end here. + $this->notices->queue_merge_notice( $sub_plugin ); + + $destination = $this->redirector->after_deactivation( wp_get_referer() ); + + if ( $destination !== false ) { + wp_safe_redirect( $destination ); + + exit; + } + } +} diff --git a/src/Contracts/Activator_Interface.php b/src/Contracts/Activator_Interface.php new file mode 100644 index 0000000..9bc16f1 --- /dev/null +++ b/src/Contracts/Activator_Interface.php @@ -0,0 +1,33 @@ +notices = $notices; + private $activator; + + /** + * @since 1.0.0 + * + * @param Queue_Interface $notices Where a sub-plugin that could not load says so. + * @param Activator_Interface $activator Runs the activation callback of one that did. + */ + public function __construct( Queue_Interface $notices, Activator_Interface $activator ) { + $this->notices = $notices; + $this->activator = $activator; } /** @@ -129,5 +139,12 @@ private function load( Sub_Plugin $sub_plugin ): void { // file are function-local as a result -- documented for hosts, because no amount of // wrapping here can hand a required file the global scope it would have had. require_once $file; + + // Only after a require that actually happened. A bundled plugin is included rather than + // activated, so register_activation_hook() never fires for it and whatever that hook would + // have done -- creating a table, seeding options -- would never happen at all. Running it + // for a sub-plugin that was skipped would be worse: the schema would appear for a plugin + // that is not loaded. + $this->activator->maybe_run( $sub_plugin ); } } diff --git a/src/Loader.php b/src/Loader.php index d2216bf..4abb6d7 100644 --- a/src/Loader.php +++ b/src/Loader.php @@ -6,6 +6,7 @@ namespace Nexcess\PluginAbsorber; use Nexcess\PluginAbsorber\Boot\Scheduler; +use Nexcess\PluginAbsorber\Conflict\Contracts\Resolver_Interface; use Nexcess\PluginAbsorber\Contracts\Provider_Interface; use Nexcess\PluginAbsorber\Contracts\Registrar_Interface; use Nexcess\PluginAbsorber\Exceptions\Config_Exception; @@ -64,6 +65,17 @@ public static function notices(): Queue_Interface { return self::collaborator( Queue_Interface::class ); } + /** + * @since 1.0.0 + * + * @throws Config_Exception When no container has been set, or its binding is unusable. + * + * @return Resolver_Interface + */ + public static function resolver(): Resolver_Interface { + return self::collaborator( Resolver_Interface::class ); + } + /** * Register one bundled sub-plugin. Call once per sub-plugin, before boot(). * @@ -167,6 +179,33 @@ public static function render_notices(): void { self::notices()->render(); } + /** + * Rewrite the activation-error notice for a standalone this library has absorbed. + * + * The parameter is untyped because a filter receives whatever the filter before it returned, + * and a `string` declaration would turn another plugin's sloppy return into a TypeError raised + * from here. + * + * @since 1.0.0 + * + * @param mixed $markup Notice markup WordPress is about to print. + * + * @throws Config_Exception When no container has been set, or a container binding is unusable. + * + * @return string + */ + public static function filter_activation_error_markup( $markup ): string { + $markup = is_string( $markup ) ? $markup : ''; + + // Returned unchanged rather than thrown out of: this runs while WordPress is drawing an + // error screen, and a second fatal there would replace the one the user came to read. + if ( ! self::has_hook_prefix() ) { + return $markup; + } + + return self::notices()->filter_activation_error_markup( $markup ); + } + /** * The object bound to a collaborator interface, checked before it is handed on. * diff --git a/src/Notices/Contracts/Queue_Interface.php b/src/Notices/Contracts/Queue_Interface.php index e3cf04a..daade27 100644 --- a/src/Notices/Contracts/Queue_Interface.php +++ b/src/Notices/Contracts/Queue_Interface.php @@ -85,4 +85,26 @@ public function queue_dependency_notice( Sub_Plugin $sub_plugin ): void; * @return void */ public function render(): void; + + /** + * Replace WordPress's generic fatal-activation text with the sub-plugin's own explanation. + * + * Filters `wp_admin_notice_markup`. This is the one conflict the load guard cannot prevent: + * WordPress includes a plugin being activated after the bundled copy has already loaded, so + * the re-declaration is a real fatal, caught in core's activation sandbox and reported as + * "the plugin triggered a fatal error" — true, and useless to whoever pressed the button. + * + * An implementation must return the markup untouched unless the request really is a + * nonce-verified activation error, on the plugins screen, for a standalone this library + * knows about. + * + * @since 1.0.0 + * + * @param string $markup Notice markup WordPress is about to print. + * + * @throws Config_Exception When no hook prefix has been set, or a container binding is unusable. + * + * @return string + */ + public function filter_activation_error_markup( string $markup ): string; } diff --git a/src/Notices/Queue.php b/src/Notices/Queue.php index dd34180..0e1f1fb 100644 --- a/src/Notices/Queue.php +++ b/src/Notices/Queue.php @@ -6,6 +6,7 @@ namespace Nexcess\PluginAbsorber\Notices; use Nexcess\PluginAbsorber\Exceptions\Config_Exception; +use Nexcess\PluginAbsorber\Loader; use Nexcess\PluginAbsorber\Notices\Contracts\Queue_Interface; use Nexcess\PluginAbsorber\Sub_Plugin; @@ -177,6 +178,84 @@ public function render(): void { $this->store->clear(); } + /** + * @since 1.0.0 + * + * @param string $markup Notice markup WordPress is about to print. + * + * @throws Config_Exception When no hook prefix has been set, or a container binding is unusable. + * + * @return string + */ + public function filter_activation_error_markup( string $markup ): string { + if ( ! is_admin() || ! function_exists( 'get_current_screen' ) ) { + return $markup; + } + + $screen = get_current_screen(); + + // Both plugin lists, because wp-admin/network/plugins.php is a one-line require of + // wp-admin/plugins.php and draws the identical activation error -- but WP_Screen appends + // `-network` to the id there. On a default multisite the subsite has no plugins UI at all, + // so the network screen is the *only* place a super admin can reactivate the standalone, + // and matching 'plugins' alone would decline on the one screen that matters most. + if ( $screen === null || ! in_array( $screen->id, [ 'plugins', 'plugins-network' ], true ) ) { + return $markup; + } + + // phpcs:disable WordPress.Security.NonceVerification.Recommended -- verified below, once + // the plugin named turns out to be one this library owns. Nothing is acted on until then. + $basename = isset( $_GET['plugin'] ) + ? sanitize_text_field( wp_unslash( $_GET['plugin'] ) ) + : ''; + + if ( $basename === '' ) { + return $markup; + } + + // Looked up before the nonce is checked, deliberately: there is no nonce work to do for a + // plugin this library does not own, and the nonce is still verified before any markup is + // touched. + $sub_plugin = $this->find_by_standalone_basename( $basename ); + + if ( $sub_plugin === null ) { + return $markup; + } + + $nonce = isset( $_GET['_error_nonce'] ) + ? sanitize_text_field( wp_unslash( $_GET['_error_nonce'] ) ) + : ''; + // phpcs:enable WordPress.Security.NonceVerification.Recommended + + if ( ! wp_verify_nonce( $nonce, 'plugin-activation-error_' . $basename ) ) { + return $markup; + } + + $message = $sub_plugin->get_conflict_notice_message( + sprintf( + '%s is bundled with this plugin and loads automatically. The standalone copy cannot' + . ' be activated alongside it.', + $sub_plugin->get_slug() + ) + ); + + // Sanitised before the emptiness check rather than after. wp_kses_post( '' ) + // is the empty string, and swapping WordPress's wording for nothing leaves a blank notice + // box where the explanation should be. wp_kses_post() and not esc_html(), for the reason + // the renderer uses it: these messages come from the host's own config, and a link to a + // knowledge-base article has to survive. + $message = trim( wp_kses_post( $message ) ); + + if ( $message === '' ) { + return $markup; + } + + // phpcs:ignore WordPress.WP.I18n.TextDomainMismatch -- core's own string, matched on purpose. + $core_text = __( 'Plugin could not be activated because it triggered a fatal error.', 'default' ); + + return str_replace( $core_text, $message, $markup ); + } + /** * The option name backing the queue. Read it directly to render these notices yourself. * @@ -210,4 +289,31 @@ public static function option_name(): string { private function queue( Sub_Plugin $sub_plugin, string $type, string $message ): void { $this->store->put( $sub_plugin->get_slug() . ':' . $type, $message ); } + + /** + * The registered sub-plugin a standalone basename belongs to, if any. + * + * Read through `Loader::all()` rather than a registrar of this queue's own, for the reason the + * resolver and the load pass do: it flushes the registrations still buffered on the facade + * before it reads, and a registrar asked directly would miss anything registered since the + * last read. + * + * @since 1.0.0 + * + * @param string $basename Standalone plugin basename named by the request. + * + * @throws Config_Exception When the container cannot produce a usable registrar, or two + * sub-plugins were registered under one slug. + * + * @return Sub_Plugin|null + */ + private function find_by_standalone_basename( string $basename ): ?Sub_Plugin { + foreach ( Loader::all() as $sub_plugin ) { + if ( $sub_plugin->get_standalone_plugin_basename() === $basename ) { + return $sub_plugin; + } + } + + return null; + } } diff --git a/src/Provider.php b/src/Provider.php index 31a200d..e2a96b0 100644 --- a/src/Provider.php +++ b/src/Provider.php @@ -6,6 +6,11 @@ namespace Nexcess\PluginAbsorber; use Nexcess\PluginAbsorber\Boot\Scheduler; +use Nexcess\PluginAbsorber\Conflict\Contracts\Resolver_Interface; +use Nexcess\PluginAbsorber\Conflict\Gatekeeper; +use Nexcess\PluginAbsorber\Conflict\Redirector; +use Nexcess\PluginAbsorber\Conflict\Resolver; +use Nexcess\PluginAbsorber\Contracts\Activator_Interface; use Nexcess\PluginAbsorber\Contracts\Plugin_Checker_Interface; use Nexcess\PluginAbsorber\Contracts\Plugin_Deactivator_Interface; use Nexcess\PluginAbsorber\Contracts\Provider_Interface; @@ -62,8 +67,11 @@ public function register(): void { $this->bind_once( Registrar_Interface::class, Registrar::class ); $this->bind_once( Plugin_Checker_Interface::class, Plugin_Checker::class ); $this->bind_once( Plugin_Deactivator_Interface::class, Plugin_Deactivator::class ); + $this->bind_once( Activator_Interface::class, Activator::class ); $this->bind_once( Store::class ); $this->bind_once( Renderer::class ); + $this->bind_once( Redirector::class ); + $this->bind_once( Gatekeeper::class ); // Explicit factories rather than a class name for everything with a constructor argument: // container-contract promises `bind`, `get`, `has` and `singleton` and nothing about @@ -75,10 +83,25 @@ static function () use ( $container ): Queue { } ); + $this->bind_once( + Resolver_Interface::class, + static function () use ( $container ): Resolver { + return new Resolver( + $container->get( Plugin_Checker_Interface::class ), + $container->get( Plugin_Deactivator_Interface::class ), + $container->get( Queue_Interface::class ), + $container->get( Redirector::class ) + ); + } + ); + $this->bind_once( Runner::class, static function () use ( $container ): Runner { - return new Runner( $container->get( Queue_Interface::class ) ); + return new Runner( + $container->get( Queue_Interface::class ), + $container->get( Activator_Interface::class ) + ); } ); diff --git a/tests/README.md b/tests/README.md index adc7d3a..6c7c165 100644 --- a/tests/README.md +++ b/tests/README.md @@ -116,15 +116,42 @@ $this->assertSame( 1, $this->bundled_plugin_loads() ); Every call writes a *new* file under a unique name, and every guard constant is unique too. Neither is tidiness: `require_once` dedupes by resolved path for the lifetime of the PHP process, so a shared fixture lets a later test pass -without loading anything, and the fixture defines its constant for real, so a -reused name makes a later sub-plugin read as already loaded. Call -`remove_bundled_plugin_files()` from tearDown. +without loading anything — including if the load logic were deleted outright — +and the fixture defines its constant for real, so a reused name makes a later +sub-plugin read as already loaded. + +The generated file does two things and nothing else: it increments a load +counter in `$GLOBALS`, and it defines the constant inside a `defined()` check, +the shape a real bundled sub-plugin has. The counter is what separates "loaded +twice" and "never loaded" from "loaded once"; the constant alone cannot tell +those three apart. + +`remove_bundled_plugin_files()` cleans up on the trait's own `@after` hook, and +the tests that clear other state alongside it call it from tearDown as well. +Never leave it to the end of a test body: a failed assertion aborts the test +where it stands, so that is exactly the line that does not run on the day it +matters. A fixture helper cannot be called `make()`, `makeEmpty()`, `construct()`, or `constructEmpty()`: those are public methods on `Codeception\Test\Unit`, which `WPTestCase` extends, and redeclaring one with narrower visibility is a fatal at class-compile time. The suite does not fail, it fails to start. +## Users and capabilities + +Two of the library's gates turn on `activate_plugins`, so a test that reaches +either needs a user who has it. `WithUsers` owns both halves: + +```php +$this->become_plugin_administrator(); // someone who may resolve a conflict +$this->create_user( 'subscriber' ); // someone who may not +``` + +`become_plugin_administrator()` is not just `create_user( 'administrator' )`. On +multisite `activate_plugins` maps through `manage_network_plugins`, which a site +administrator does not have — so it grants super admin there and sets the current +user either way. A test about *that* difference creates the administrator itself. + ## Stubbing functions Use `UopzFunctions` from wp-browser. Do not add a local `WithUopz` trait — this @@ -274,3 +301,52 @@ $this->assert_the_library_reported_incorrect_usage(); An unexpected report still fails the test, because everything the listener sees is recorded and asserted to belong to this library. Call `stop_expecting_incorrect_usage()` from tearDown. + +## The end-to-end suite + +`tests/unit/EndToEndTest.php` drives the library the way a host plugin does, +against real WordPress state: the real `active_plugins` option, a real +`deactivate_plugins()` that really writes it, and real site options behind the +notice queue and the activation record. Nothing about the library is doubled, +except in the two tests that are *about* a host binding its own collaborators. + +It reaches for no entry point a host does not have. The bootstrap is +`Config::set_hook_prefix()`, `Config::set_container()`, `Loader::register()` and +`Loader::boot()`, and everything after that arrives through the hooks boot() +wired — a request is `do_action( 'plugins_loaded' )`, an admin page load is +`do_action( 'all_admin_notices' )`. The container is handed over bare rather +than through `WithContainer`, because `boot()` running the provider over it is +one of the steps under test. + +Only two functions are stubbed: `wp_safe_redirect`, which throws so the request +halts where production calls `exit`, and `wp_get_referer`, which is a request +header no test can send and which decides where that redirect would have gone. + +Four preconditions have to hold before any of it means anything, and setUp +establishes all four: + +- **An interactive admin GET** — `set_current_screen( 'plugins' )` plus + `$_SERVER['REQUEST_METHOD'] = 'GET'`. `Conflict\Gatekeeper` turns away + anything else, so without both of these every policy test would pass while + resolving nothing at all. +- **A user who can `activate_plugins`** — `WithUsers::become_plugin_administrator()`. + The gatekeeper checks the capability before anything is resolved, and the + queue checks the same one before it renders, so as nobody the suite would be + asserting that a no-op is a no-op. +- **The hook prefix** — `Config::set_hook_prefix()`. Both plugins_loaded steps + report and return without one, and the queue and activation option names are + derived from it. +- **A rewound `plugins_loaded` counter** — the harness dispatched the hook + before any test ran, so `boot()` would rightly report that it is too late to + wire and run everything inline. tearDown puts the count back. + +The screen, the request method and that counter are all process-global, so all +three are restored in teardown; leaving any of them set turns an unrelated later +test into an admin request. + +Run both legs — `slic run unit` and `slic run unit --env multisite`. Multisite +is not a formality here: `deactivate_plugins()` is network-aware, +`activate_plugins` maps through `manage_network_plugins` so the administrator +who passes on singlesite is not the one who passes on multisite, and the queue +and activation record are `get_site_option()` values, which are network options +there. Every precondition above resolves differently on the second leg. diff --git a/tests/_support/Spy_Activator.php b/tests/_support/Spy_Activator.php new file mode 100644 index 0000000..daf42f3 --- /dev/null +++ b/tests/_support/Spy_Activator.php @@ -0,0 +1,39 @@ +slugs` off a value typed as + * `Activator_Interface` is reading a property the interface does not declare, and static analysis + * rightly rejects it. Named, the spy's own type carries the recorder. + * + * It records rather than runs, which is the point — a test that binds this one proves the load path + * reached the activator it was handed without writing the default's option along the way. + * + * @since 1.0.0 + */ +class Spy_Activator implements Activator_Interface { + /** + * Slugs maybe_run() was called for, in call order. + * + * @var string[] + */ + public $slugs = []; + + /** + * @param Sub_Plugin $sub_plugin Sub-plugin that has just been loaded. + * + * @return void + */ + public function maybe_run( Sub_Plugin $sub_plugin ): void { + $this->slugs[] = $sub_plugin->get_slug(); + } +} diff --git a/tests/_support/Spy_Gatekeeper.php b/tests/_support/Spy_Gatekeeper.php new file mode 100644 index 0000000..7248df8 --- /dev/null +++ b/tests/_support/Spy_Gatekeeper.php @@ -0,0 +1,50 @@ +may_resolve_calls` off a + * value the container handed back. + * + * @since 1.0.0 + */ +class Spy_Gatekeeper extends Gatekeeper { + /** + * How many times may_resolve() was called. + * + * @var int + */ + public $may_resolve_calls = 0; + + /** + * @var bool + */ + private $answer; + + /** + * @param bool $answer The answer this gatekeeper always gives. + */ + public function __construct( bool $answer ) { + $this->answer = $answer; + } + + /** + * @return bool + */ + public function may_resolve(): bool { + ++$this->may_resolve_calls; + + return $this->answer; + } +} diff --git a/tests/_support/Spy_Queue.php b/tests/_support/Spy_Queue.php index 2b42400..d3563ec 100644 --- a/tests/_support/Spy_Queue.php +++ b/tests/_support/Spy_Queue.php @@ -49,6 +49,23 @@ class Spy_Queue implements Queue_Interface { */ public $render_calls = 0; + /** + * Markup handed to filter_activation_error_markup(), in order. + * + * @var string[] + */ + public $filtered = []; + + /** + * What filter_activation_error_markup() hands back. + * + * Deliberately not the argument: a trampoline that returned its own input instead of the + * queue's answer would be indistinguishable from one that delegated properly. + * + * @var string + */ + public $filtered_markup = '

Rewritten by the queue.

'; + /** * @param Sub_Plugin $sub_plugin Sub-plugin concerned. * @@ -82,4 +99,15 @@ public function queue_dependency_notice( Sub_Plugin $sub_plugin ): void { public function render(): void { ++$this->render_calls; } + + /** + * @param string $markup Notice markup WordPress is about to print. + * + * @return string + */ + public function filter_activation_error_markup( string $markup ): string { + $this->filtered[] = $markup; + + return $this->filtered_markup; + } } diff --git a/tests/_support/Spy_Resolver.php b/tests/_support/Spy_Resolver.php new file mode 100644 index 0000000..8d0085f --- /dev/null +++ b/tests/_support/Spy_Resolver.php @@ -0,0 +1,36 @@ +resolve_calls` off a value typed + * as `Resolver_Interface` is reading a property the interface does not declare, and static analysis + * rightly rejects it. Named, the spy's own type carries the counter. + * + * It resolves nothing, which is the point — a test that binds this one proves the conflict step + * reached a resolver at all without deactivating anything or ending the request. + * + * @since 1.0.0 + */ +class Spy_Resolver implements Resolver_Interface { + /** + * How many times resolve_all() was called. + * + * @var int + */ + public $resolve_calls = 0; + + /** + * @return void + */ + public function resolve_all(): void { + ++$this->resolve_calls; + } +} diff --git a/tests/_support/Traits/WithBundledPlugins.php b/tests/_support/Traits/WithBundledPlugins.php index 6d96ef2..96f0bd9 100644 --- a/tests/_support/Traits/WithBundledPlugins.php +++ b/tests/_support/Traits/WithBundledPlugins.php @@ -30,6 +30,14 @@ trait WithBundledPlugins { /** * Write a bundled plugin that counts its own loads and defines its guard constant. * + * The counter is what separates "loaded twice" and "never loaded" from "loaded once". The + * constant alone cannot tell those three apart, because it ends up defined exactly once either + * way. + * + * The constant is defined inside a `defined()` check, because the file stands in for a real + * plugin: a bundled copy that redeclared a constant the standalone had already defined would + * raise a notice, and the guard is what a plugin actually ships. + * * @since 1.0.0 * * @param string $constant Guard constant the file defines, as a bundled plugin's own header would. @@ -100,7 +108,14 @@ protected function reset_bundled_plugin_loads(): void { } /** - * Remove every fixture this test wrote. Call from tearDown. + * Remove every fixture this test wrote. + * + * Runs itself, as PHPUnit's own `@after` hook, and is safe to call from tearDown as well — which + * is where the tests that clear other state alongside it do call it. A test body must never be + * the only thing that removes these: a failed assertion aborts the test where it stands, so a + * cleanup line at the end of the body is exactly the one that does not run on the day it matters. + * + * @after * * @since 1.0.0 * diff --git a/tests/_support/Traits/WithUsers.php b/tests/_support/Traits/WithUsers.php new file mode 100644 index 0000000..bf6dd2d --- /dev/null +++ b/tests/_support/Traits/WithUsers.php @@ -0,0 +1,63 @@ + uniqid( 'absorber-' ), + 'user_pass' => wp_generate_password(), + 'role' => $role, + ] + ); + + if ( $user_id instanceof WP_Error ) { + throw new RuntimeException( 'Could not create a ' . $role . ': ' . $user_id->get_error_message() ); + } + + return $user_id; + } + + /** + * Become someone who can activate plugins. + * + * On multisite activate_plugins maps through manage_network_plugins, so an administrator of a + * single site does not have it and the network administrator is the one who does. + * + * @since 1.0.0 + * + * @return int + */ + protected function become_plugin_administrator(): int { + $user_id = $this->create_user( 'administrator' ); + + if ( is_multisite() ) { + grant_super_admin( $user_id ); + } + + wp_set_current_user( $user_id ); + + return $user_id; + } +} diff --git a/tests/unit/ActivatorTest.php b/tests/unit/ActivatorTest.php new file mode 100644 index 0000000..ea5878f --- /dev/null +++ b/tests/unit/ActivatorTest.php @@ -0,0 +1,256 @@ +clear_activations(); + } + + public function tearDown(): void { + $this->clear_activations(); + Config_State::reset(); + parent::tearDown(); + } + + public function test_it_runs_the_activation_callback(): void { + $calls = []; + + ( new Activator() )->maybe_run( $this->recording_sub_plugin( $calls ) ); + + $this->assertCount( 1, $calls ); + } + + /** + * The sub-plugin is the callback's one argument, so a host's migration can read the slug and the + * bundled file it is running for rather than being closed over one of them at config time. + */ + public function test_the_callback_receives_the_sub_plugin(): void { + $calls = []; + $sub_plugin = $this->recording_sub_plugin( $calls ); + + ( new Activator() )->maybe_run( $sub_plugin ); + + $this->assertSame( [ $sub_plugin ], $calls ); + } + + public function test_it_runs_the_callback_only_once(): void { + $calls = []; + $sub_plugin = $this->recording_sub_plugin( $calls ); + $activator = new Activator(); + + $activator->maybe_run( $sub_plugin ); + $activator->maybe_run( $sub_plugin ); + + $this->assertCount( 1, $calls ); + } + + /** + * The record is the option, not the object holding it. Every request builds a fresh collaborator, + * so an in-memory guard would run the callback again on the next page load — forever. + */ + public function test_a_fresh_instance_does_not_run_the_callback_again(): void { + $calls = []; + $sub_plugin = $this->recording_sub_plugin( $calls ); + + ( new Activator() )->maybe_run( $sub_plugin ); + ( new Activator() )->maybe_run( $sub_plugin ); + + $this->assertCount( 1, $calls ); + } + + public function test_it_records_the_slug_in_the_option(): void { + $calls = []; + + ( new Activator() )->maybe_run( $this->recording_sub_plugin( $calls ) ); + + $this->assertSame( [ 'give-recurring' => true ], $this->recorded() ); + } + + /** + * Most sub-plugins configure no callback at all. Writing a row for them would put an option on + * every site that has nothing to record, and one that can never be read for anything. + */ + public function test_it_does_nothing_without_an_activation_callback(): void { + ( new Activator() )->maybe_run( $this->make_sub_plugin() ); + + $this->assertFalse( get_site_option( self::OPTION, false ), 'No callback means no option.' ); + } + + public function test_two_slugs_are_tracked_independently(): void { + $recurring_calls = []; + $recovery_calls = []; + $recurring = $this->recording_sub_plugin( $recurring_calls, [ 'slug' => 'give-recurring' ] ); + $recovery = $this->recording_sub_plugin( $recovery_calls, [ 'slug' => 'give-fee-recovery' ] ); + $activator = new Activator(); + + $activator->maybe_run( $recurring ); + $activator->maybe_run( $recovery ); + $activator->maybe_run( $recurring ); + + $this->assertCount( 1, $recurring_calls ); + $this->assertCount( 1, $recovery_calls, 'One slug being recorded must not stand the other down.' ); + $this->assertSame( + [ 'give-recurring' => true, 'give-fee-recovery' => true ], + $this->recorded() + ); + } + + /** + * @dataProvider corrupted_options + * + * @param mixed $stored Raw option value to seed. + */ + public function test_it_recovers_from_a_corrupted_option( $stored ): void { + update_site_option( self::OPTION, $stored ); + + $calls = []; + + ( new Activator() )->maybe_run( $this->recording_sub_plugin( $calls ) ); + + $this->assertCount( 1, $calls ); + $this->assertSame( [ 'give-recurring' => true ], $this->recorded() ); + } + + /** + * Anything that is not an array reaches an array read inside plugins_loaded, on every request — + * so it is replaced rather than trusted, and the site heals on the next write. + * + * @return Generator + */ + public static function corrupted_options(): Generator { + yield 'a string instead of an array' => [ 'not-a-record' ]; + + yield 'an object instead of an array' => [ (object) [ 'give-recurring' => true ] ]; + + yield 'a number instead of an array' => [ 42 ]; + } + + /** + * Two hosts on one site each get their own record, and a prefix that names filters is folded + * before it becomes a storage key. + */ + public function test_the_option_name_follows_the_hook_prefix(): void { + Config_State::reset(); + Config::set_hook_prefix( 'woo' ); + + $this->assertSame( Config::get_option_name( 'activations' ), Activator::option_name() ); + $this->assertSame( self::OPTION_WOO, Activator::option_name() ); + + $calls = []; + + ( new Activator() )->maybe_run( $this->recording_sub_plugin( $calls ) ); + + $this->assertSame( [ 'give-recurring' => true ], get_site_option( self::OPTION_WOO ) ); + $this->assertFalse( get_site_option( self::OPTION, false ), 'Another host must not be written to.' ); + } + + /** + * The slug is recorded after the callback returns, never before. A callback that fatals halfway + * leaves the site half-migrated either way, but recording first would make the half-finished + * state permanent: the next request would see the record and skip the retry. + */ + public function test_a_throwing_callback_leaves_the_slug_unrecorded(): void { + $failing = $this->make_sub_plugin( + [ + 'activation_callback' => static function (): void { + throw new RuntimeException( 'the migration could not reach the database' ); + }, + ] + ); + + $threw = false; + + try { + ( new Activator() )->maybe_run( $failing ); + } catch ( RuntimeException $exception ) { + $threw = true; + + $this->assertSame( 'the migration could not reach the database', $exception->getMessage() ); + } + + $this->assertTrue( $threw, 'The callback has to have run for this to be about the ordering.' ); + $this->assertFalse( get_site_option( self::OPTION, false ), 'A failed run must not be recorded.' ); + + $calls = []; + + ( new Activator() )->maybe_run( $this->recording_sub_plugin( $calls ) ); + + $this->assertCount( 1, $calls, 'The slug has to be retried, not skipped forever.' ); + } + + /** + * A sub-plugin whose activation callback appends itself to the given array. + * + * The whole `Sub_Plugin` is recorded rather than its slug, so the argument the callback is handed + * can be asserted on as well as counted. + * + * @param array $calls Recorder to append to. + * @param array $overrides Config overrides. + * + * @return Sub_Plugin + */ + private function recording_sub_plugin( array &$calls, array $overrides = [] ): Sub_Plugin { + return $this->make_sub_plugin( + array_merge( + [ + 'activation_callback' => static function ( Sub_Plugin $sub_plugin ) use ( &$calls ): void { + $calls[] = $sub_plugin; + }, + ], + $overrides + ) + ); + } + + /** + * The record as it stands, for whichever prefix the test set. + * + * @return array + */ + private function recorded(): array { + $done = get_site_option( Activator::option_name(), [] ); + + return is_array( $done ) ? $done : []; + } + + /** + * @return void + */ + private function clear_activations(): void { + delete_site_option( self::OPTION ); + delete_site_option( self::OPTION_WOO ); + } +} diff --git a/tests/unit/Boot/SchedulerTest.php b/tests/unit/Boot/SchedulerTest.php index 993f54c..8f7c71d 100644 --- a/tests/unit/Boot/SchedulerTest.php +++ b/tests/unit/Boot/SchedulerTest.php @@ -10,15 +10,22 @@ use LogicException; use Nexcess\PluginAbsorber\Boot\Scheduler; use Nexcess\PluginAbsorber\Config; +use Nexcess\PluginAbsorber\Conflict\Contracts\Resolver_Interface; +use Nexcess\PluginAbsorber\Conflict\Gatekeeper; +use Nexcess\PluginAbsorber\Conflict_Policy; +use Nexcess\PluginAbsorber\Contracts\Plugin_Checker_Interface; use Nexcess\PluginAbsorber\Loader; use Nexcess\PluginAbsorber\Notices\Contracts\Queue_Interface; use Nexcess\PluginAbsorber\Tests\Support\Config_State; use Nexcess\PluginAbsorber\Tests\Support\Loader_State; +use Nexcess\PluginAbsorber\Tests\Support\Spy_Gatekeeper; use Nexcess\PluginAbsorber\Tests\Support\Spy_Queue; +use Nexcess\PluginAbsorber\Tests\Support\Spy_Resolver; use Nexcess\PluginAbsorber\Tests\Support\Test_Container; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithBundledPlugins; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithContainer; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithIncorrectUsage; +use Nexcess\PluginAbsorber\Tests\Support\Traits\WithUsers; use ReflectionClass; use WP_Hook; @@ -40,12 +47,18 @@ class SchedulerTest extends WPTestCase { use WithBundledPlugins; use WithContainer; use WithIncorrectUsage; + use WithUsers; /** * @var int */ private $plugins_loaded_count = 0; + /** + * @var string|null + */ + private $request_method; + /** * Hook callbacks these tests added, as [ hook, callback, priority ] triples. * @@ -65,6 +78,12 @@ public function setUp(): void { Config::set_hook_prefix( 'give' ); $this->set_up_container(); $this->reset_bundled_plugin_loads(); + $this->clear_notices(); + + // The conflict step reads the request method, so the one test that lets the real gatekeeper + // answer depends on it rather than on whatever the harness happened to leave behind. + $this->request_method = $_SERVER['REQUEST_METHOD'] ?? null; + $_SERVER['REQUEST_METHOD'] = 'GET'; // The harness has to boot WordPress before it can run anything, so plugins_loaded has already // fired by the time any test starts — and boot() would rightly report that it is too late to @@ -77,6 +96,12 @@ public function setUp(): void { public function tearDown(): void { $GLOBALS['wp_actions']['plugins_loaded'] = $this->plugins_loaded_count; + if ( $this->request_method === null ) { + unset( $_SERVER['REQUEST_METHOD'] ); + } else { + $_SERVER['REQUEST_METHOD'] = $this->request_method; + } + // In tearDown rather than at the end of the test body: a failing assertion would otherwise // leak an admin screen into every test that runs after it, since is_admin() checks the // current screen before WP_ADMIN. @@ -90,6 +115,7 @@ public function tearDown(): void { $this->stop_expecting_incorrect_usage(); $this->remove_bundled_plugin_files(); + $this->clear_notices(); Loader_State::reset(); Config_State::reset(); $this->tear_down_container(); @@ -105,6 +131,93 @@ public function test_the_load_step_runs_early_in_plugins_loaded(): void { $this->assertSame( 2, $this->load_priority() ); } + /** + * A standalone that survives the conflict defines its guard constant as it loads, and the load + * pass has to see that — so resolution runs first and cannot share a priority with it. + */ + public function test_the_conflict_step_runs_before_the_load_step(): void { + $this->assertSame( 1, $this->resolve_priority() ); + $this->assertLessThan( $this->load_priority(), $this->resolve_priority() ); + } + + public function test_it_wires_the_conflict_step_at_the_resolve_priority(): void { + [ 'resolver' => $resolver ] = $this->bind_conflict_doubles( true ); + + $before = $this->callbacks_at( 'plugins_loaded', $this->resolve_priority() ); + + Loader::boot(); + + $this->assertSame( + $before + 1, + $this->callbacks_at( 'plugins_loaded', $this->resolve_priority() ), + 'boot() must wire the conflict step rather than run it.' + ); + $this->assertSame( 0, $resolver->resolve_calls, 'Wiring must not resolve anything yet.' ); + + do_action( 'plugins_loaded' ); + + $this->assertSame( 1, $resolver->resolve_calls ); + } + + /** + * The gate is asked first and the resolver is built only once it has said yes. That order is the + * whole guarantee: a host binding its own `Resolver_Interface` decides what a conflict means, not + * who is allowed to have one resolved, and a replacement that forgot a gate cannot reopen it + * because it is never reached. + */ + public function test_the_conflict_step_asks_the_gatekeeper_before_resolving(): void { + [ 'gatekeeper' => $gatekeeper, 'resolver' => $resolver ] = $this->bind_conflict_doubles( false ); + + Loader::boot(); + + do_action( 'plugins_loaded' ); + + $this->assertSame( 1, $gatekeeper->may_resolve_calls, 'The step has to ask the gate.' ); + $this->assertSame( 0, $resolver->resolve_calls, 'A refused request must not reach a resolver at all.' ); + } + + public function test_the_conflict_step_resolves_once_the_gatekeeper_admits_the_request(): void { + [ 'gatekeeper' => $gatekeeper, 'resolver' => $resolver ] = $this->bind_conflict_doubles( true ); + + Loader::boot(); + + do_action( 'plugins_loaded' ); + + $this->assertSame( 1, $gatekeeper->may_resolve_calls ); + $this->assertSame( 1, $resolver->resolve_calls ); + } + + /** + * The real gate, on the request it exists to turn away. The capability check covers the policies + * that only queue a notice as well as the destructive one, and nothing is lost by that — the + * standalone is still there to detect once someone who can act on it arrives, which is what the + * second half asserts. + */ + public function test_a_user_who_cannot_activate_plugins_has_nothing_resolved_or_queued(): void { + set_current_screen( 'dashboard' ); + + $this->bind_active_standalone(); + $this->register_conflicted_sub_plugin(); + + wp_set_current_user( $this->create_user( 'subscriber' ) ); + + Loader::boot(); + + do_action( 'plugins_loaded' ); + + $this->assertSame( [], $this->notice_queue(), 'A user who could never read the notice must not consume it.' ); + + $this->become_plugin_administrator(); + + do_action( 'plugins_loaded' ); + + $this->assertArrayHasKey( + 'give-recurring:conflict', + $this->notice_queue(), + 'The conflict has to still be detectable once someone who can act on it arrives.' + ); + } + public function test_it_wires_the_load_step_at_the_load_priority(): void { $this->register_sub_plugin(); @@ -204,18 +317,68 @@ static function () use ( &$fired ): void { $this->assertSame( 0, $notices->render_calls ); } + /** + * The activation-error rewrite is wired next to the notice step and under the same guard: both + * only ever have work to do on an admin screen. It is a named static callback rather than a + * closure, so unlike the plugins_loaded steps it can be asserted on by identity — and, for the + * same reason, taken back off by a host that wants core's wording. + */ + public function test_it_wires_the_activation_error_filter_in_the_admin(): void { + set_current_screen( 'plugins' ); + + $notices = $this->bind_spy_queue(); + + Loader::boot(); + + $this->assertNotFalse( + has_filter( 'wp_admin_notice_markup', [ Loader::class, 'filter_activation_error_markup' ] ) + ); + $this->assertSame( + $notices->filtered_markup, + apply_filters( 'wp_admin_notice_markup', '

Core.

', '', [] ), + 'The wired filter has to reach the bound queue.' + ); + } + + /** + * `wp_admin_notice_markup` is a general-purpose hook another plugin is free to apply anywhere, + * and the rewrite only ever has work to do on an activation-error request in wp-admin. + */ + public function test_it_does_not_wire_the_activation_error_filter_on_the_front_end(): void { + set_current_screen( 'front' ); + + Loader::boot(); + + $observed = [ has_filter( 'wp_admin_notice_markup', [ Loader::class, 'filter_activation_error_markup' ] ) ]; + + // Wired by hand and read a second time. Without that reading, a probe that could never + // report this callback at all — a renamed method, a mistyped hook name — would satisfy the + // assertion below however boot() had behaved. + $this->add_tracked_filter( 'wp_admin_notice_markup', [ Loader::class, 'filter_activation_error_markup' ] ); + + $observed[] = has_filter( 'wp_admin_notice_markup', [ Loader::class, 'filter_activation_error_markup' ] ); + + $this->assertSame( + [ false, 10 ], + $observed, + 'The filter must be admin-only, and the probe must be able to see a registration.' + ); + } + /** * Adding an action at a priority the running dispatch has already passed is accepted and then * never fires. Booting from plugins_loaded at the default priority instead of 0 would otherwise * load nothing at all, on a site that looks completely healthy. * - * The load priority itself is the boundary case: a callback added to the priority currently being + * The window is measured from the earliest step in the sequence, so the resolve priority is the + * boundary case rather than the load priority: a callback added to the priority currently being * dispatched is never reached either, because the dispatch loop walks a by-value copy of that - * priority's callback array. + * priority's callback array. Booting between the two steps still reports, and still loads. * * @dataProvider late_boot_priorities * - * @param int $offset How far past the load priority the host boots from. + * @param int $offset How far past the load priority the host boots from; negative for the window + * between conflict resolution and the load pass. */ public function test_booting_too_late_in_plugins_loaded_loads_inline_instead( int $offset ): void { $this->expect_incorrect_usage(); @@ -249,11 +412,42 @@ static function () use ( $path, $constant ): void { * @return Generator */ public static function late_boot_priorities(): Generator { + yield 'at the resolve priority' => [ -1 ]; yield 'at the load priority' => [ 0 ]; yield 'one past it' => [ 1 ]; yield 'the default a host omits' => [ 8 ]; } + /** + * The other side of the same boundary. The window closes at the first step rather than the last, + * so a host booting at priority 0 — which is what the README tells it to do — must still wire both + * steps and be reported on for nothing. + */ + public function test_booting_at_the_start_of_plugins_loaded_still_wires(): void { + $constant = $this->make_guard_constant(); + $path = $this->make_bundled_plugin_file( $constant ); + + $this->add_tracked_action( + 'plugins_loaded', + static function () use ( $path, $constant ): void { + Loader::register( + [ + 'slug' => 'give-recurring', + 'bundled_plugin_file' => $path, + 'plugin_loaded_constant' => $constant, + ] + ); + + Loader::boot(); + }, + 0 + ); + + do_action( 'plugins_loaded' ); + + $this->assertSame( 1, $this->bundled_plugin_loads() ); + } + public function test_booting_after_plugins_loaded_has_finished_loads_inline(): void { $this->expect_incorrect_usage(); @@ -275,14 +469,16 @@ public function test_booting_after_plugins_loaded_has_finished_loads_inline(): v public function test_the_state_helper_unwires_the_hooks_boot_added(): void { set_current_screen( 'dashboard' ); - $load_step = $this->callbacks_at( 'plugins_loaded', $this->load_priority() ); - $notice_step = $this->callbacks_at( 'all_admin_notices' ); + $load_step = $this->callbacks_at( 'plugins_loaded', $this->load_priority() ); + $notice_step = $this->callbacks_at( 'all_admin_notices' ); + $activation_error = $this->callbacks_at( 'wp_admin_notice_markup' ); Loader::boot(); Loader_State::reset(); $this->assertSame( $load_step, $this->callbacks_at( 'plugins_loaded', $this->load_priority() ) ); $this->assertSame( $notice_step, $this->callbacks_at( 'all_admin_notices' ) ); + $this->assertSame( $activation_error, $this->callbacks_at( 'wp_admin_notice_markup' ) ); } /** @@ -335,6 +531,24 @@ private function load_priority(): int { return $priority; } + /** + * The priority the conflict step is wired at, read from the scheduler rather than restated. + * + * @throws LogicException When the constant is missing or not an int, rather than counting + * callbacks at priority zero and passing for the wrong reason. + * + * @return int + */ + private function resolve_priority(): int { + $priority = ( new ReflectionClass( Scheduler::class ) )->getConstant( 'RESOLVE_PRIORITY' ); + + if ( ! is_int( $priority ) ) { + throw new LogicException( 'Boot\Scheduler::RESOLVE_PRIORITY must be an int.' ); + } + + return $priority; + } + /** * How many callbacks are on a hook, at one priority or in total. * @@ -387,6 +601,104 @@ static function () use ( $notices ): Spy_Queue { return $notices; } + /** + * Bind a gatekeeper with a fixed answer and a resolver that records being reached. + * + * Both bound before the provider runs, which is the only order that leaves them bound. The pair is + * returned rather than resolved back out of the container, so the assertions read a spy's own type + * — `Resolver_Interface` declares no counter, and nothing here narrows a container's return. + * + * @param bool $may_resolve The answer the gate always gives. + * + * @return array{gatekeeper:Spy_Gatekeeper,resolver:Spy_Resolver} + */ + private function bind_conflict_doubles( bool $may_resolve ): array { + $gatekeeper = new Spy_Gatekeeper( $may_resolve ); + $resolver = new Spy_Resolver(); + + $container = new Test_Container(); + $container->singleton( + Gatekeeper::class, + static function () use ( $gatekeeper ): Gatekeeper { + return $gatekeeper; + } + ); + $container->singleton( + Resolver_Interface::class, + static function () use ( $resolver ): Resolver_Interface { + return $resolver; + } + ); + + $this->set_up_container( $container ); + + return [ + 'gatekeeper' => $gatekeeper, + 'resolver' => $resolver, + ]; + } + + /** + * Report every standalone as active, without reaching WordPress for the answer. + * + * @return void + */ + private function bind_active_standalone(): void { + $container = new Test_Container(); + $container->singleton( + Plugin_Checker_Interface::class, + static function (): Plugin_Checker_Interface { + return new class() implements Plugin_Checker_Interface { + /** + * @param string $basename Plugin basename. + * + * @return bool + */ + public function is_active( string $basename ): bool { + return true; + } + }; + } + ); + + $this->set_up_container( $container ); + } + + /** + * Register a sub-plugin whose standalone is in conflict, under the policy that only talks. + * + * @return void + */ + private function register_conflicted_sub_plugin(): void { + $constant = $this->make_guard_constant(); + + Loader::register( + [ + 'slug' => 'give-recurring', + 'bundled_plugin_file' => $this->make_bundled_plugin_file( $constant ), + 'plugin_loaded_constant' => $constant, + 'standalone_plugin_basename' => 'give-recurring/give-recurring.php', + 'conflict_policy' => Conflict_Policy::NOTICE_ONLY, + ] + ); + } + + /** + * The queue is stored as a site option on every install — on single site that call falls through + * to the plain option table — so there is one place to read it from. + * + * @return array + */ + private function notice_queue(): array { + $queue = get_site_option( 'give_plugin_absorber_notices', [] ); + + return is_array( $queue ) ? $queue : []; + } + + private function clear_notices(): void { + delete_site_option( 'give_plugin_absorber_notices' ); + } + /** * Register a sub-plugin whose bundled file records that it was loaded. * @@ -418,4 +730,21 @@ private function add_tracked_action( string $hook, callable $callback, int $prio add_action( $hook, $callback, $priority ); } + + /** + * The same, for a filter. Spelled separately from add_tracked_action() even though WordPress + * keeps actions and filters in one registry, so a reader is never left wondering whether a + * filter was wired by an add_action() on purpose. + * + * @param string $hook Hook to add to. + * @param callable $callback Callback to add. + * @param int $priority Priority to add it at. + * + * @return void + */ + private function add_tracked_filter( string $hook, callable $callback, int $priority = 10 ): void { + $this->added_actions[] = [ $hook, $callback, $priority ]; + + add_filter( $hook, $callback, $priority ); + } } diff --git a/tests/unit/Conflict/GatekeeperTest.php b/tests/unit/Conflict/GatekeeperTest.php new file mode 100644 index 0000000..3af805c --- /dev/null +++ b/tests/unit/Conflict/GatekeeperTest.php @@ -0,0 +1,228 @@ +set_up_container(); + + // plugins_loaded fires on every request, so the request method is part of what the gate reads + // rather than something the harness happens to leave lying around. + $this->request_method = $_SERVER['REQUEST_METHOD'] ?? null; + $_SERVER['REQUEST_METHOD'] = 'GET'; + + set_current_screen( 'dashboard' ); + $this->become_plugin_administrator(); + } + + public function tearDown(): void { + if ( $this->request_method === null ) { + unset( $_SERVER['REQUEST_METHOD'] ); + } else { + $_SERVER['REQUEST_METHOD'] = $this->request_method; + } + + // In tearDown rather than at the end of the test body: a failing assertion would otherwise + // leak an admin screen into every test that runs after it, since is_admin() checks the + // current screen before WP_ADMIN. + set_current_screen( 'front' ); + + $this->stop_expecting_incorrect_usage(); + Config_State::reset(); + $this->tear_down_container(); + parent::tearDown(); + } + + public function test_it_admits_an_interactive_admin_get(): void { + $this->assertTrue( $this->gatekeeper()->may_resolve() ); + } + + /** + * Unguarded, resolution fires at plugins_loaded on every request: a visitor's checkout POST + * becomes a 302 that drops the order, and a front-end page load deactivates a plugin with nobody + * there to read the notice about it. + */ + public function test_it_refuses_a_front_end_request(): void { + set_current_screen( 'front' ); + + $this->assertFalse( $this->gatekeeper()->may_resolve() ); + + set_current_screen( 'dashboard' ); + + $this->assert_the_gate_opens_again(); + } + + /** + * admin-post.php and options.php define WP_ADMIN and never define DOING_AJAX, so is_admin() is + * true and wp_doing_ajax() is false. Deactivating and redirecting there turns a submitted form + * into a 302 the browser follows with a GET, and the submission is gone — the same data loss the + * gate exists to prevent, one layer in. Nothing is lost by waiting for the next page view. + */ + public function test_it_refuses_an_admin_form_submission(): void { + $_SERVER['REQUEST_METHOD'] = 'POST'; + + $this->assertFalse( $this->gatekeeper()->may_resolve() ); + + $_SERVER['REQUEST_METHOD'] = 'GET'; + + $this->assert_the_gate_opens_again(); + } + + /** + * wp-cron never reaches its event loop if the request it rides on is redirected away. + */ + public function test_it_refuses_a_cron_request(): void { + $this->setConstant( 'DOING_CRON', true ); + + $this->assertFalse( $this->gatekeeper()->may_resolve() ); + + // unsetConstant() rather than a second setConstant(): it restores whatever the constant was + // before this test touched it, where setting it twice would record the test's own value as + // the one to put back. + $this->unsetConstant( 'DOING_CRON' ); + + $this->assert_the_gate_opens_again(); + } + + public function test_it_refuses_an_ajax_request(): void { + $this->setConstant( 'DOING_AJAX', true ); + + $this->assertFalse( $this->gatekeeper()->may_resolve() ); + + $this->unsetConstant( 'DOING_AJAX' ); + + $this->assert_the_gate_opens_again(); + } + + /** + * header() is a no-op under the CLI SAPI, so a WP-CLI command would exit 0 having printed + * nothing and having deactivated a plugin the operator never asked about. + */ + public function test_it_refuses_a_wp_cli_request(): void { + $this->setConstant( 'WP_CLI', true ); + + $this->assertFalse( $this->gatekeeper()->may_resolve() ); + + $this->unsetConstant( 'WP_CLI' ); + + $this->assert_the_gate_opens_again(); + } + + /** + * plugins_loaded is dispatched by wp-load.php, which wp-admin/admin.php requires long before it + * calls auth_redirect(). An unauthenticated GET of an admin URL therefore reaches the conflict + * step, and without the capability check a stranger could deactivate the standalone site-wide. + * + * @dataProvider users_who_cannot_activate_plugins + * + * @param string|null $role Role to ask as, or null for a logged-out visitor. + */ + public function test_it_refuses_a_user_who_cannot_activate_plugins( ?string $role ): void { + wp_set_current_user( $role === null ? 0 : $this->create_user( $role ) ); + + $this->assertFalse( + $this->gatekeeper()->may_resolve(), + 'Only someone who can activate a plugin may deactivate one.' + ); + + $this->become_plugin_administrator(); + + $this->assert_the_gate_opens_again(); + } + + /** + * @return Generator + */ + public static function users_who_cannot_activate_plugins(): Generator { + yield 'a subscriber' => [ 'subscriber' ]; + yield 'a logged-out visitor' => [ null ]; + } + + /** + * The prefix names the conflict_policy filter and the option the notice queue lives in, so + * resolution without one has nowhere to put what it would do. It is the only gate that reports the + * mistake, and it is checked last so that report lands on the admin request that was about to + * resolve something rather than on every front-end request the site serves. + */ + public function test_it_refuses_a_request_with_no_hook_prefix(): void { + $container = $this->container(); + + // The prefix goes, the container stays: a gatekeeper that could not be built at all would + // fail this test for the other reason. + Config_State::reset(); + Config::set_container( $container ); + $this->expect_incorrect_usage(); + + $this->assertFalse( $this->gatekeeper()->may_resolve() ); + $this->assert_the_library_reported_incorrect_usage(); + + Config::set_hook_prefix( 'give' ); + + $this->assert_the_gate_opens_again(); + } + + /** + * The gatekeeper as the container builds it, which is how the conflict step reaches it. + * + * @return Gatekeeper + */ + private function gatekeeper(): Gatekeeper { + return $this->resolve( Gatekeeper::class ); + } + + /** + * With the one condition under test put back, the gate has to open. + * + * Without this every test here would pass on a gatekeeper that refused everything, including the + * request it is supposed to admit. + * + * @return void + */ + private function assert_the_gate_opens_again(): void { + $this->assertTrue( + $this->gatekeeper()->may_resolve(), + 'The condition under test has to be the one deciding the answer.' + ); + } +} diff --git a/tests/unit/Conflict/RedirectorTest.php b/tests/unit/Conflict/RedirectorTest.php new file mode 100644 index 0000000..1bc4d8d --- /dev/null +++ b/tests/unit/Conflict/RedirectorTest.php @@ -0,0 +1,57 @@ +assertSame( $expected, ( new Redirector() )->after_deactivation( $referrer ) ); + } + + /** + * Absolute admin URLs and bare paths both appear, because wp_get_referer() returns whichever + * the request carried: the _wp_http_referer field of an admin form holds a path with no scheme + * or host, while the Referer header holds a full URL. + * + * @return Generator + */ + public static function referrers(): Generator { + yield 'no referrer at all' => [ false, admin_url( 'plugins.php' ) ]; + yield 'an empty referrer' => [ '', admin_url( 'plugins.php' ) ]; + + yield 'a plugin update screen' => [ admin_url( 'update.php?action=upgrade-plugin' ), admin_url( 'plugins.php' ) ]; + yield 'the core update screen' => [ admin_url( 'update-core.php' ), admin_url( 'plugins.php' ) ]; + + yield 'the plugins list' => [ admin_url( 'plugins.php' ), false ]; + + // On multisite this is /wp-admin/network/plugins.php, which no comparison against + // admin_url() would recognise. + yield 'the network plugins list' => [ network_admin_url( 'plugins.php' ), false ]; + + yield 'another admin screen' => [ + admin_url( 'options-general.php?settings-updated=true' ), + admin_url( 'options-general.php?settings-updated=true' ), + ]; + yield 'another network admin screen' => [ network_admin_url( 'sites.php' ), network_admin_url( 'sites.php' ) ]; + + // What an admin form POST actually carries. Matching on the screen is what makes these two + // behave the same as their absolute equivalents. + yield 'a bare referrer path to the plugins list' => [ '/wp-admin/plugins.php?plugin_status=all', false ]; + yield 'a bare referrer path to another screen' => [ '/wp-admin/options-general.php', '/wp-admin/options-general.php' ]; + } +} diff --git a/tests/unit/Conflict/ResolverTest.php b/tests/unit/Conflict/ResolverTest.php new file mode 100644 index 0000000..fcc4405 --- /dev/null +++ b/tests/unit/Conflict/ResolverTest.php @@ -0,0 +1,617 @@ +> + */ + private $deactivations = []; + + /** + * @var string|null + */ + private $request_method; + + public function setUp(): void { + parent::setUp(); + + Loader_State::reset(); + Config_State::reset(); + Config::set_hook_prefix( 'give' ); + $this->set_up_container(); + $this->clear_notices(); + + // uopz cannot stub a function that does not exist yet. + require_once ABSPATH . 'wp-admin/includes/plugin.php'; + + $this->deactivations = []; + + // Set explicitly rather than inherited from the harness. Nothing here is gated on the request + // method any more, but wp_get_referer() reads $_REQUEST and $_SERVER, so the tests that assert + // a destination depend on the request looking like the one they describe. + $this->request_method = $_SERVER['REQUEST_METHOD'] ?? null; + $_SERVER['REQUEST_METHOD'] = 'GET'; + + // uopz runs a replacement with no class scope, so $this and self:: are both fatal inside this + // closure. Bind a reference to the property instead. See tests/README.md. + $deactivations = &$this->deactivations; + + $this->setFunctionReturn( + 'deactivate_plugins', + static function ( $plugins, $silent = false, $network_wide = null ) use ( &$deactivations ) { + $deactivations[] = [ + 'plugins' => $plugins, + 'silent' => $silent, + 'network_wide' => $network_wide, + ]; + }, + true + ); + } + + public function tearDown(): void { + if ( $this->request_method === null ) { + unset( $_SERVER['REQUEST_METHOD'] ); + } else { + $_SERVER['REQUEST_METHOD'] = $this->request_method; + } + + $this->clear_notices(); + Loader_State::reset(); + Config_State::reset(); + $this->tear_down_container(); + parent::tearDown(); + } + + public function test_the_loader_resolves_the_default_resolver(): void { + $this->assertInstanceOf( Resolver::class, Loader::resolver() ); + } + + public function test_the_default_resolver_satisfies_the_contract(): void { + $this->assertInstanceOf( Resolver_Interface::class, $this->resolver() ); + } + + /** + * The point of the required constructor arguments. Every peer arrives from the container, so a host + * that rebinds one has it reached — and nothing in the run touches the option the default notice + * queue is backed by, which is what makes the resolver testable without standing up global state. + */ + public function test_the_collaborators_come_from_the_container(): void { + $checker = new class() implements Plugin_Checker_Interface { + /** + * @var string[] + */ + public $asked = []; + + /** + * @param string $basename Plugin basename. + * + * @return bool + */ + public function is_active( string $basename ): bool { + $this->asked[] = $basename; + + return true; + } + }; + + $deactivator = new class() implements Plugin_Deactivator_Interface { + /** + * @var string[] + */ + public $deactivated = []; + + /** + * @param string $basename Plugin basename. + * + * @return void + */ + public function deactivate( string $basename ): void { + $this->deactivated[] = $basename; + } + }; + + $notices = new Spy_Queue(); + + $container = new Test_Container(); + $container->singleton( + Plugin_Checker_Interface::class, + static function () use ( $checker ): Plugin_Checker_Interface { + return $checker; + } + ); + $container->singleton( + Plugin_Deactivator_Interface::class, + static function () use ( $deactivator ): Plugin_Deactivator_Interface { + return $deactivator; + } + ); + $container->singleton( + Queue_Interface::class, + static function () use ( $notices ): Queue_Interface { + return $notices; + } + ); + $this->set_up_container( $container ); + + $this->register(); + $this->setFunctionReturn( 'wp_get_referer', admin_url( 'plugins.php' ) ); + + $this->resolve_all(); + + $this->assertSame( [ 'give-recurring/give-recurring.php' ], $checker->asked ); + $this->assertSame( [ 'give-recurring/give-recurring.php' ], $deactivator->deactivated ); + $this->assertSame( [ 'give-recurring' ], $notices->merge_notices ); + $this->assertSame( + [], + $this->queued_notices(), + 'The bound queue stands in for the option-backed one, which must be left untouched.' + ); + $this->assertSame( + [], + $this->deactivations, + 'A bound deactivator is what deactivates; WordPress must not have been called as well.' + ); + } + + public function test_it_asks_the_redirector_it_was_given(): void { + $redirector = new class() extends Redirector { + /** + * @var array + */ + public $asked = []; + + /** + * @param string|false $referrer Referrer under test. + * + * @return string|false + */ + public function after_deactivation( $referrer ) { + $this->asked[] = $referrer; + + return admin_url( 'tools.php' ); + } + }; + + $container = new Test_Container(); + $container->singleton( + Redirector::class, + static function () use ( $redirector ): Redirector { + return $redirector; + } + ); + $this->set_up_container( $container ); + + $this->standalone_is( true ); + $this->register(); + $this->setFunctionReturn( 'wp_get_referer', admin_url( 'options-general.php' ) ); + + $location = $this->capture_resolution(); + + $this->assertSame( [ admin_url( 'options-general.php' ) ], $redirector->asked ); + $this->assertSame( admin_url( 'tools.php' ), $location ); + } + + public function test_deactivate_deactivates_notifies_and_redirects(): void { + $this->standalone_is( true ); + $this->register( [ 'conflict_policy' => Conflict_Policy::DEACTIVATE ] ); + $this->setFunctionReturn( 'wp_get_referer', false ); + + $location = $this->capture_resolution(); + + $this->assertCount( 1, $this->deactivations ); + $this->assertSame( 'give-recurring/give-recurring.php', $this->deactivations[0]['plugins'] ); + $this->assertArrayHasKey( 'give-recurring:merge', $this->queued_notices() ); + + // The destination, not merely that a redirect happened: a resolver that redirected somewhere + // else entirely would satisfy the count without sending anyone anywhere useful. + $this->assertSame( admin_url( 'plugins.php' ), $location ); + } + + public function test_deactivate_is_the_default_policy(): void { + $this->standalone_is( true ); + $this->register(); + $this->setFunctionReturn( 'wp_get_referer', false ); + + $this->capture_resolution(); + + $this->assertCount( 1, $this->deactivations ); + } + + /** + * Silent, and with no $network_wide argument — core's default of null is what handles both + * scopes, and the standalone's deactivation hook must not run at plugins_loaded. + * + * Asserted here as well as in PluginDeactivatorTest, because this is the path that actually + * deactivates a site's plugin: a resolver that reached WordPress by some other route would leave + * that unit test green and the site 404ing. + */ + public function test_it_deactivates_silently_and_lets_core_decide_the_scope(): void { + $this->standalone_is( true ); + $this->register(); + $this->setFunctionReturn( 'wp_get_referer', false ); + + $this->capture_resolution(); + + $this->assertTrue( $this->deactivations[0]['silent'], 'An unattended deactivation must be silent.' ); + $this->assertNull( + $this->deactivations[0]['network_wide'], + 'Core enters the network branch on false !== $network_wide and the blog branch on true !== $network_wide, so null takes both.' + ); + } + + /** + * Against real core rather than a stub, because the whole reason the scope argument was + * dropped is a claim about what core does with the default. + */ + public function test_it_really_deactivates_a_site_active_standalone(): void { + $this->unsetFunctionReturn( 'deactivate_plugins' ); + + $basename = 'absorber-fixture/absorber-fixture.php'; + update_option( 'active_plugins', [ $basename ] ); + + $this->register( [ 'standalone_plugin_basename' => $basename ] ); + $this->setFunctionReturn( 'wp_get_referer', false ); + + $this->capture_resolution(); + + $this->assertNotContains( $basename, (array) get_option( 'active_plugins', [] ) ); + + delete_option( 'active_plugins' ); + } + + public function test_it_really_deactivates_a_network_active_standalone(): void { + if ( ! is_multisite() ) { + $this->markTestSkipped( 'Network activation only exists on multisite.' ); + } + + $this->unsetFunctionReturn( 'deactivate_plugins' ); + + $basename = 'absorber-fixture/absorber-fixture.php'; + update_site_option( 'active_sitewide_plugins', [ $basename => time() ] ); + + $this->register( [ 'standalone_plugin_basename' => $basename ] ); + $this->setFunctionReturn( 'wp_get_referer', false ); + + $this->capture_resolution(); + + $this->assertArrayNotHasKey( + $basename, + (array) get_site_option( 'active_sitewide_plugins', [] ), + 'Omitting $network_wide must still clear a network activation.' + ); + + delete_site_option( 'active_sitewide_plugins' ); + } + + /** + * The notice is queued after the deactivation, so it must not depend on the plugin still + * being active — and it is the only record the site owner gets. + */ + public function test_the_merge_notice_is_queued_before_the_redirect_halts_the_request(): void { + $this->standalone_is( true ); + $this->register(); + $this->setFunctionReturn( 'wp_get_referer', false ); + + $this->capture_resolution(); + + $this->assertArrayHasKey( 'give-recurring:merge', $this->queued_notices() ); + } + + public function test_defer_does_nothing_at_all(): void { + $this->standalone_is( true ); + $this->register( [ 'conflict_policy' => Conflict_Policy::DEFER ] ); + + $this->resolve_all(); + + $this->assertSame( [], $this->deactivations ); + $this->assertSame( [], $this->queued_notices() ); + } + + public function test_notice_only_notifies_without_deactivating(): void { + $this->standalone_is( true ); + $this->register( [ 'conflict_policy' => Conflict_Policy::NOTICE_ONLY ] ); + + $this->resolve_all(); + + $this->assertSame( [], $this->deactivations ); + $this->assertArrayHasKey( 'give-recurring:conflict', $this->queued_notices() ); + } + + /** + * A policy read from an option, or returned by someone else's filter, can be anything. + * Falling through to the destructive branch on a typo would turn off a plugin the site owner + * deliberately activated. + * + * @dataProvider unknown_policies + * + * @param string $policy Policy under test. + */ + public function test_an_unknown_policy_takes_the_conservative_branch( string $policy ): void { + $this->standalone_is( true ); + $this->register( [ 'conflict_policy' => $policy ] ); + + $this->resolve_all(); + + $this->assertSame( [], $this->deactivations, 'An unrecognised policy must never deactivate.' ); + $this->assertArrayHasKey( 'give-recurring:conflict', $this->queued_notices() ); + } + + /** + * @return Generator + */ + public static function unknown_policies(): Generator { + yield 'typo' => [ 'defered' ]; + yield 'empty' => [ '' ]; + yield 'wrong case' => [ 'DEACTIVATE' ]; + } + + public function test_a_callable_policy_selects_the_branch(): void { + $this->standalone_is( true ); + $this->register( + [ + 'conflict_policy' => static function ( Sub_Plugin $sub_plugin ) { + return $sub_plugin->get_slug() === 'give-recurring' + ? Conflict_Policy::DEFER + : Conflict_Policy::DEACTIVATE; + }, + ] + ); + + $this->resolve_all(); + + $this->assertSame( [], $this->deactivations, 'The callable chose DEFER for this slug.' ); + } + + public function test_the_filter_can_override_the_policy(): void { + $this->standalone_is( true ); + $this->register( [ 'conflict_policy' => Conflict_Policy::DEACTIVATE ] ); + + add_filter( + 'give/plugin_absorber/conflict_policy', + static function () { + return Conflict_Policy::DEFER; + } + ); + + $this->resolve_all(); + + $this->assertSame( [], $this->deactivations ); + } + + public function test_it_skips_a_disabled_sub_plugin(): void { + $this->standalone_is( true ); + $this->register( [ 'enabled' => false ] ); + + $this->resolve_all(); + + $this->assertSame( [], $this->deactivations ); + $this->assertSame( [], $this->queued_notices(), 'A skipped sub-plugin has nothing to say to the site owner.' ); + } + + public function test_it_skips_when_the_standalone_is_not_active(): void { + $this->standalone_is( false ); + $this->register(); + + $this->resolve_all(); + + $this->assertSame( [], $this->deactivations ); + $this->assertSame( [], $this->queued_notices(), 'There is no conflict to report when the standalone is gone.' ); + } + + public function test_it_skips_a_sub_plugin_with_no_standalone(): void { + $this->standalone_is( true ); + Loader::register( + [ + 'slug' => 'give-fee-recovery', + 'bundled_plugin_file' => '/tmp/give-fee-recovery.php', + 'plugin_loaded_constant' => 'GIVE_FEE_RECOVERY_VERSION_FIXTURE', + ] + ); + + $this->resolve_all(); + + $this->assertSame( [], $this->deactivations ); + $this->assertSame( + [], + $this->queued_notices(), + 'A sub-plugin that names no standalone can never be in conflict with one.' + ); + } + + /** + * Where a referrer sends the user is the redirector's own decision and is covered case by case in + * RedirectorTest. What belongs here is that the resolver asks it and honours a false. + */ + public function test_it_redirects_to_where_the_redirector_says(): void { + $this->standalone_is( true ); + $this->register(); + $this->setFunctionReturn( 'wp_get_referer', admin_url( 'options-general.php' ) ); + + $this->assertSame( admin_url( 'options-general.php' ), $this->capture_resolution() ); + } + + /** + * A false from the redirector means stay put, and staying put has to include not ending the + * request. `resolve_all()` fails on a redirect, which is what makes the absence of one an + * assertion rather than an assumption — coming from the plugins list there is nothing to send the + * user back to. + */ + public function test_it_deactivates_without_redirecting_from_the_plugins_page(): void { + $this->standalone_is( true ); + $this->register(); + $this->setFunctionReturn( 'wp_get_referer', admin_url( 'plugins.php' ) ); + + $this->resolve_all(); + + $this->assertCount( 1, $this->deactivations ); + } + + public function test_resolve_all_needs_a_hook_prefix(): void { + $this->standalone_is( true ); + $this->register(); + + $resolver = $this->resolver(); + $container = $this->container(); + + // The prefix goes, the container stays: this is about the missing prefix, and a resolver that + // could not reach its registrar would throw the same exception for the other reason. + Config_State::reset(); + Config::set_container( $container ); + + $this->expectException( Config_Exception::class ); + + $resolver->resolve_all(); + } + + /** + * The resolver the container builds, which is the one the conflict step reaches. + * + * @return Resolver_Interface + */ + private function resolver(): Resolver_Interface { + return $this->resolve( Resolver_Interface::class ); + } + + /** + * Run resolution on a path that must not end the request. + * + * The redirect is stubbed to throw rather than left alone: unstubbed, a resolver that redirected + * anyway would reach the real `wp_safe_redirect()` and the `exit` behind it, taking the whole test + * process with it. Throwing instead turns that into one failed test naming the path it happened on. + * + * @return void + */ + private function resolve_all(): void { + $message = 'The resolver must not end the request on this path.'; + + $this->setFunctionReturn( + 'wp_safe_redirect', + static function () use ( $message ) { + throw new TestException( $message ); + }, + true + ); + + try { + $this->resolver()->resolve_all(); + } catch ( TestException $exception ) { + $this->fail( $exception->getMessage() ); + } finally { + // In a finally block so a failed assertion cannot strand the stub for the rest of the + // process, where a later test's redirect would throw for no reason it can see. + $this->unsetFunctionReturn( 'wp_safe_redirect' ); + } + } + + /** + * Run resolution on a path that must redirect and terminate, and return where it sent the user. + * + * @return string + */ + private function capture_resolution(): string { + $resolver = $this->resolver(); + + return $this->capture_redirect( + static function () use ( $resolver ): void { + $resolver->resolve_all(); + } + ); + } + + /** + * @param array $overrides Config overrides. + * + * @return void + */ + private function register( array $overrides = [] ): void { + Loader::register( + array_merge( + [ + 'slug' => 'give-recurring', + 'bundled_plugin_file' => '/tmp/give-recurring.php', + 'plugin_loaded_constant' => 'GIVE_RECURRING_VERSION_FIXTURE', + 'standalone_plugin_basename' => 'give-recurring/give-recurring.php', + ], + $overrides + ) + ); + } + + /** + * Only is_plugin_active(), which is the one function Plugin_Checker::is_active() calls — and it + * ORs the network check in itself, so stubbing is_plugin_active_for_network() alongside it + * would be inert and would read as though a network path were being exercised. + * + * @param bool $active Whether the standalone is active. + * + * @return void + */ + private function standalone_is( bool $active ): void { + $this->setFunctionReturn( 'is_plugin_active', $active ); + } + + private function clear_notices(): void { + delete_site_option( 'give_plugin_absorber_notices' ); + } + + /** + * The queue is stored as a site option on every install — on single site that call falls through + * to the plain option table — so there is one place to read it from. + * + * @return array + */ + private function queued_notices(): array { + $queue = get_site_option( 'give_plugin_absorber_notices', [] ); + + return is_array( $queue ) ? $queue : []; + } +} diff --git a/tests/unit/EndToEndTest.php b/tests/unit/EndToEndTest.php new file mode 100644 index 0000000..8b8daf1 --- /dev/null +++ b/tests/unit/EndToEndTest.php @@ -0,0 +1,955 @@ +fatal error.'; + + /** + * The notice core is about to print, as `wp_admin_notice_markup` hands it over. + * + * @var string + */ + private const MARKUP = '

' . self::CORE_TEXT . '

'; + + /** + * Guard constants a test defined through uopz, undone in tearDown. + * + * @var string[] + */ + private $constants = []; + + /** + * Hook callbacks these tests added, as [ hook, callback, priority ] triples. + * + * Tracked so tearDown can take back exactly what a test put there. `remove_all_filters()` would + * strip the hook bare instead, discarding every callback WordPress and the rest of the suite have + * on it for the remainder of the process. + * + * @var array + */ + private $added_hooks = []; + + /** + * @var string|null + */ + private $request_method = null; + + /** + * The plugins_loaded count as the harness left it. + * + * @var int + */ + private $plugins_loaded_count = 0; + + public function setUp(): void { + parent::setUp(); + + Loader_State::reset(); + Config_State::reset(); + + // The first half of the bootstrap. The container is the second, and each test builds its own + // so that a host binding its implementations first has somewhere to bind them. + Config::set_hook_prefix( self::HOOK_PREFIX ); + + // Conflict resolution runs only on an interactive admin GET, since plugins_loaded fires on + // every request. Without both of these every policy test below would pass while resolving + // nothing at all. + set_current_screen( 'plugins' ); + $this->request_method = $_SERVER['REQUEST_METHOD'] ?? null; + $_SERVER['REQUEST_METHOD'] = 'GET'; + + // Deactivating a standalone and consuming the notice queue are both gated on + // activate_plugins, which on multisite maps through manage_network_plugins. + $this->become_plugin_administrator(); + + // A referrer is a header no test can send. False is the ordinary case — a link followed from + // somewhere outside the admin — and it sends the user to the plugins list. + $this->setFunctionReturn( 'wp_get_referer', false ); + + $this->clear_state(); + $this->reset_bundled_plugin_loads(); + + // The harness has to boot WordPress before it can run anything, so plugins_loaded has already + // fired by the time any test starts — and boot() would rightly report that it is too late to + // wire. Rewind the counter so a test sees the timing a host bootstrap sees; the late-boot test + // dispatches the hook itself to close the window again. + $this->plugins_loaded_count = did_action( 'plugins_loaded' ); + unset( $GLOBALS['wp_actions']['plugins_loaded'] ); + } + + public function tearDown(): void { + // In tearDown rather than at the end of each test body: a failed assertion would otherwise + // leave an admin screen, a pinned request method, a half-built activation-error request and a + // rewound hook counter standing for every test that runs afterwards in this process. + $GLOBALS['wp_actions']['plugins_loaded'] = $this->plugins_loaded_count; + + if ( $this->request_method === null ) { + unset( $_SERVER['REQUEST_METHOD'] ); + } else { + $_SERVER['REQUEST_METHOD'] = $this->request_method; + } + + unset( $_GET['plugin'], $_GET['_error_nonce'] ); + set_current_screen( 'front' ); + + foreach ( $this->constants as $constant ) { + $this->unsetConstant( $constant ); + } + $this->constants = []; + + // Only what these tests added by hand. What boot() wired comes off in Loader_State::reset(). + foreach ( $this->added_hooks as [ $hook, $callback, $priority ] ) { + remove_filter( $hook, $callback, $priority ); + } + $this->added_hooks = []; + + $this->stop_expecting_incorrect_usage(); + $this->remove_bundled_plugin_files(); + $this->clear_state(); + Loader_State::reset(); + Config_State::reset(); + parent::tearDown(); + } + + /** + * The happy path, and the one every other scenario is a deviation from: nothing else claims the + * plugin, so the bundled copy loads, defines the guard the standalone would have defined, and + * gets the one-time setup that `register_activation_hook()` never gives it. + */ + public function test_a_fresh_load_defines_the_guard_and_activates_exactly_once(): void { + $activated = []; + + $constant = $this->register( + [ + 'activation_callback' => static function ( Sub_Plugin $sub_plugin ) use ( &$activated ): void { + $activated[] = $sub_plugin->get_slug(); + }, + ] + ); + + $this->boot(); + $this->run_request(); + + $this->assertSame( 1, $this->bundled_plugin_loads() ); + $this->assertTrue( defined( $constant ), 'The bundled copy defines the guard the standalone would have.' ); + $this->assertSame( [ self::SLUG ], $activated ); + $this->assertSame( [ self::SLUG => true ], $this->activation_record() ); + + // The next page view, with nothing re-registered and nothing re-booted. The constant the file + // really defined stands the load down, and the record really written stands the callback down. + $this->run_request(); + + $this->assertSame( 1, $this->bundled_plugin_loads() ); + $this->assertSame( [ self::SLUG ], $activated, 'Activation runs once for the life of the site.' ); + } + + /** + * The default policy, against core's own `deactivate_plugins()` and the real `active_plugins` + * option rather than a stub of either. + */ + public function test_deactivate_deactivates_notifies_and_redirects(): void { + update_option( 'active_plugins', [ self::STANDALONE ] ); + + $this->register( + [ + 'standalone_plugin_basename' => self::STANDALONE, + 'conflict_policy' => Conflict_Policy::DEACTIVATE, + ] + ); + + $this->boot(); + + $location = $this->run_halted_request(); + + $this->assertNotContains( self::STANDALONE, $this->active_plugins() ); + $this->assertArrayHasKey( self::SLUG . ':merge', $this->notice_queue() ); + + // The destination, not merely that one was asked for: a redirect somewhere else entirely + // would satisfy "the request ended in a redirect" without sending anyone anywhere useful. + $this->assertSame( admin_url( 'plugins.php' ), $location ); + + // The request really ended in the resolver. The bundled copy loads on the next one, which is + // what the standalone's own guard constant forces in production. + $this->assertSame( 0, $this->bundled_plugin_loads() ); + } + + /** + * All the way to the screen. The merge notice is the one this library raises exactly once and + * never re-queues, so the admin page load after the deactivation has to draw it — and consume it, + * or the owner reads the same deactivation report for ever. + */ + public function test_the_merge_notice_renders_on_the_next_admin_screen_and_clears(): void { + update_option( 'active_plugins', [ self::STANDALONE ] ); + + $this->register( [ 'standalone_plugin_basename' => self::STANDALONE ] ); + + $this->boot(); + $this->run_halted_request(); + + $rendered = $this->render_admin_notices(); + + $this->assertStringContainsString( self::SLUG, $rendered ); + $this->assertStringContainsString( 'has been deactivated', $rendered ); + $this->assertStringContainsString( + 'notice-warning', + $rendered, + 'A conflict the library has already handled is a warning, not an error.' + ); + $this->assertSame( [], $this->notice_queue(), 'Rendering consumes the queue.' ); + } + + /** + * The failure mode a merge notice queued on every request would produce: a redirect loop, or an + * admin screen that reports the same deactivation for ever. Nothing is re-registered between the + * two requests — a duplicate slug throws — because this is the next page view, not a second + * bootstrap. + */ + public function test_the_request_after_a_deactivation_does_not_loop(): void { + update_option( 'active_plugins', [ self::STANDALONE ] ); + + $constant = $this->register( [ 'standalone_plugin_basename' => self::STANDALONE ] ); + + $this->boot(); + $this->run_halted_request(); + + $this->assertArrayHasKey( self::SLUG . ':merge', $this->notice_queue() ); + + // The owner has been told. Emptying the queue is what makes a second notice visible at all: + // re-queuing writes the same `slug:merge` key, so a queue left as it is would look identical + // whether or not the resolver ran again. + delete_site_option( Queue::option_name() ); + + // This one must not halt, and run_request() fails the test if it does — which is the + // redirect loop, asserted rather than described. + $this->run_request(); + + $this->assertSame( [], $this->notice_queue(), 'Nothing is left to resolve, so nothing is left to say.' ); + $this->assertSame( 1, $this->bundled_plugin_loads(), 'With the standalone gone the bundled copy takes over.' ); + $this->assertTrue( defined( $constant ) ); + } + + /** + * DEFER hands the request to the standalone, and WordPress includes an active plugin from + * wp-settings.php long before plugins_loaded — so by the time the resolver runs, the standalone + * has already defined the guard constant. Defining it up front is what makes this the scenario + * the policy actually describes rather than a resolver that merely declined to act. + */ + public function test_defer_leaves_the_standalone_active_and_loads_nothing(): void { + update_option( 'active_plugins', [ self::STANDALONE ] ); + + $constant = $this->define_guard( 'ABSORBER_E2E_DEFERRED_GUARD' ); + + $this->register( + [ + 'standalone_plugin_basename' => self::STANDALONE, + 'conflict_policy' => Conflict_Policy::DEFER, + ], + $constant + ); + + $this->boot(); + $this->run_request(); + + $this->assertContains( self::STANDALONE, $this->active_plugins() ); + $this->assertSame( 0, $this->bundled_plugin_loads(), 'The standalone won; the guard stands the bundled copy down.' ); + $this->assertSame( [], $this->notice_queue() ); + } + + public function test_notice_only_notifies_without_deactivating(): void { + update_option( 'active_plugins', [ self::STANDALONE ] ); + + $this->register( + [ + 'standalone_plugin_basename' => self::STANDALONE, + 'conflict_policy' => Conflict_Policy::NOTICE_ONLY, + 'conflict_notice_message' => static fn() => 'Deactivate the standalone when you get a chance.', + ] + ); + + $this->boot(); + + // A policy that only talks must not end the request, which is what run_request() asserts. + $this->run_request(); + + $this->assertContains( self::STANDALONE, $this->active_plugins() ); + $this->assertSame( + [ self::SLUG . ':conflict' => 'Deactivate the standalone when you get a chance.' ], + $this->notice_queue() + ); + } + + /** + * The gate that survives every policy and every rebinding: whoever cannot activate a plugin must + * not be able to deactivate one by loading an admin page. Nothing is consumed by refusing — the + * standalone is still there to detect on the next request, from someone who can act on it, which + * is what the second half asserts. + */ + public function test_a_user_who_cannot_activate_plugins_resolves_nothing(): void { + update_option( 'active_plugins', [ self::STANDALONE ] ); + + $this->register( [ 'standalone_plugin_basename' => self::STANDALONE ] ); + + wp_set_current_user( $this->create_user( 'subscriber' ) ); + + $this->boot(); + $this->run_request(); + + $this->assertContains( self::STANDALONE, $this->active_plugins(), 'A subscriber must not deactivate anything.' ); + $this->assertSame( [], $this->notice_queue(), 'A user who could never read the notice must not consume it.' ); + + $this->become_plugin_administrator(); + + $this->run_halted_request(); + + $this->assertNotContains( self::STANDALONE, $this->active_plugins() ); + $this->assertArrayHasKey( self::SLUG . ':merge', $this->notice_queue() ); + } + + /** + * The conflict the load guard cannot prevent: the owner reinstalls the standalone and presses + * Activate, WordPress includes it on top of the bundled copy, and the re-declaration is a real + * fatal that core's sandbox reports as "the plugin triggered a fatal error" — true, and useless. + * + * Driven through `Loader::boot()` and core's own filter dispatch rather than by calling the queue + * directly, because the wiring is half of what has to work: an admin-only `add_filter()` that + * never ran leaves the useless sentence on the screen. + */ + public function test_a_reactivation_attempt_yields_the_friendly_message(): void { + $this->register( + [ + 'standalone_plugin_basename' => self::STANDALONE, + 'conflict_notice_message' => static fn() => 'Recurring is already bundled with the host plugin.', + ] + ); + + $this->boot(); + + // The request core redirects to once the sandboxed activation has fataled. + $_GET['plugin'] = self::STANDALONE; + $_GET['_error_nonce'] = wp_create_nonce( 'plugin-activation-error_' . self::STANDALONE ); + + $rewritten = apply_filters( 'wp_admin_notice_markup', self::MARKUP, self::CORE_TEXT, [] ); + + $this->assertIsString( $rewritten, 'The filter must hand back markup, whatever it did with it.' ); + + $filtered = is_string( $rewritten ) ? $rewritten : ''; + + $this->assertStringContainsString( 'Recurring is already bundled with the host plugin.', $filtered ); + $this->assertStringNotContainsString( self::CORE_TEXT, $filtered ); + + // The notice box stays core's to draw — its classes, its dismiss button, its wrapper. Only + // the sentence inside belongs to this library. + $this->assertStringStartsWith( '

', $filtered ); + } + + /** + * The guard is not only about the standalone. A must-use copy, a second host plugin bundling the + * same code, or the site owner's own snippet all define the same constant, and any of them means + * the code is already in memory. + */ + public function test_the_bundled_copy_stands_down_when_the_guard_is_already_defined(): void { + $constant = $this->define_guard( 'ABSORBER_E2E_ALREADY_LOADED_GUARD' ); + + $this->register( [], $constant ); + + $this->boot(); + $this->run_request(); + + $this->assertSame( 0, $this->bundled_plugin_loads() ); + $this->assertSame( [], $this->notice_queue(), 'A plugin the admin can see running has nothing to explain.' ); + } + + /** + * The toggle is read on every request rather than resolved at registration, so flipping it and + * running the next request is what proves the first load was skipped for the toggle and not for + * something else entirely — a missing file, say, which would leave the same empty counter. + */ + public function test_a_sub_plugin_toggled_off_loads_nothing(): void { + $enabled = false; + + $constant = $this->register( + [ + 'enabled' => static function () use ( &$enabled ) { + return $enabled; + }, + ] + ); + + $this->boot(); + $this->run_request(); + + $this->assertSame( 0, $this->bundled_plugin_loads() ); + $this->assertFalse( defined( $constant ) ); + $this->assertSame( [], $this->notice_queue(), 'A sub-plugin nobody asked for has nothing to report.' ); + + $enabled = true; + + $this->run_request(); + + $this->assertSame( 1, $this->bundled_plugin_loads(), 'The toggle is the only thing that was stopping it.' ); + } + + /** + * The host's last word before the require, on the hook name its own prefix builds. + */ + public function test_the_should_load_filter_can_veto_a_load(): void { + $constant = $this->register(); + + $this->add_tracked_filter( + Config::get_hook_name( 'should_load' ), + static function ( $should_load, $sub_plugin ) { + return $sub_plugin instanceof Sub_Plugin && $sub_plugin->get_slug() === self::SLUG + ? false + : $should_load; + }, + 10, + 2 + ); + + $this->boot(); + $this->run_request(); + + $this->assertSame( 0, $this->bundled_plugin_loads() ); + $this->assertFalse( defined( $constant ) ); + $this->assertSame( [], $this->notice_queue(), 'A host that vetoed the load does not need telling about it.' ); + } + + /** + * Registration order, not slug order and not filesystem order: a host bundles plugins that + * depend on one another, and the order it registers them in is the only say it gets. + */ + public function test_two_sub_plugins_load_in_one_request_in_registration_order(): void { + $loaded = []; + + // Recorded from the activation callback, which runs immediately after each require — so this + // is the order the files were really required in, not the order they were registered in. + $record = static function ( Sub_Plugin $sub_plugin ) use ( &$loaded ): void { + $loaded[] = $sub_plugin->get_slug(); + }; + + $first = $this->register( [ 'activation_callback' => $record ] ); + $second = $this->register( + [ + 'slug' => 'absorber-fee-recovery', + 'activation_callback' => $record, + ] + ); + + $this->boot(); + $this->run_request(); + + $this->assertSame( 2, $this->bundled_plugin_loads() ); + $this->assertTrue( defined( $first ) ); + $this->assertTrue( defined( $second ) ); + $this->assertSame( [ self::SLUG, 'absorber-fee-recovery' ], $loaded ); + } + + /** + * All the way to the screen again, from the other end: the load is skipped, the host's own + * explanation is queued, the render draws it as an error, and the render consumes the queue so + * the owner is told once rather than on every admin page load for ever. + */ + public function test_an_unmet_dependency_blocks_the_load_and_queues_the_explanation(): void { + $this->register( + [ + 'dependency_check' => static fn() => false, + 'dependency_notice_message' => static fn() => 'GiveWP 3.0 or later is required.', + ] + ); + + $this->boot(); + $this->run_request(); + + $this->assertSame( 0, $this->bundled_plugin_loads() ); + $this->assertSame( + [ self::SLUG . ':dependency' => 'GiveWP 3.0 or later is required.' ], + $this->notice_queue() + ); + + $rendered = $this->render_admin_notices(); + + $this->assertStringContainsString( 'GiveWP 3.0 or later is required.', $rendered ); + $this->assertStringContainsString( 'notice-error', $rendered, 'A plugin that did not load at all is an error.' ); + $this->assertSame( [], $this->notice_queue(), 'Rendering consumes the queue.' ); + } + + /** + * Booting from plugins_loaded at the default priority is the commonest hook mistake there is, and + * an add_action() at a priority the running dispatch has already passed is accepted and then never + * fires. The library reports the mistake and runs the sequence inline, so the site the host + * shipped still gets its bundled plugins. + */ + public function test_a_host_that_boots_too_late_still_gets_its_sub_plugins(): void { + $this->expect_incorrect_usage(); + + $constant = $this->register(); + + $this->add_tracked_action( + 'plugins_loaded', + function (): void { + $this->boot(); + } + ); + + $this->run_request(); + + $this->assertSame( 1, $this->bundled_plugin_loads(), 'A late boot must still load.' ); + $this->assertTrue( defined( $constant ) ); + $this->assert_the_library_reported_incorrect_usage(); + } + + /** + * The whole point of a required container: a host binds its own implementation of an interface + * before boot, and that is the object the library uses for the rest of the request. + * + * One request covers the conflict step and the load pass because the referrer is the plugins + * list, where the redirector says to stay put. The defaults are asserted *not* to have run + * alongside the doubles — a library that resolved a second copy of the queue or the deactivator + * behind the host's back would satisfy every positive assertion here. + */ + public function test_a_host_binding_reaches_every_step_of_the_request(): void { + $registrar = new Spy_Registrar(); + $notices = new Spy_Queue(); + $activator = new Spy_Activator(); + + $checker = new class() implements Plugin_Checker_Interface { + /** + * @var string[] + */ + public $basenames = []; + + /** + * @param string $basename Plugin basename. + * + * @return bool + */ + public function is_active( string $basename ): bool { + $this->basenames[] = $basename; + + return true; + } + }; + + $deactivator = new class() implements Plugin_Deactivator_Interface { + /** + * @var string[] + */ + public $basenames = []; + + /** + * @param string $basename Plugin basename. + * + * @return void + */ + public function deactivate( string $basename ): void { + $this->basenames[] = $basename; + } + }; + + // Really active, so that the default deactivator would have emptied this option had it been + // the one reached. Nothing else in this test would notice the difference. + update_option( 'active_plugins', [ self::STANDALONE ] ); + + $container = new Test_Container(); + $container->singleton( + Registrar_Interface::class, + static function () use ( $registrar ): Registrar_Interface { + return $registrar; + } + ); + $container->singleton( + Plugin_Checker_Interface::class, + static function () use ( $checker ): Plugin_Checker_Interface { + return $checker; + } + ); + $container->singleton( + Plugin_Deactivator_Interface::class, + static function () use ( $deactivator ): Plugin_Deactivator_Interface { + return $deactivator; + } + ); + $container->singleton( + Queue_Interface::class, + static function () use ( $notices ): Queue_Interface { + return $notices; + } + ); + $container->singleton( + Activator_Interface::class, + static function () use ( $activator ): Activator_Interface { + return $activator; + } + ); + + // From the plugins list the redirector says to stay put, so the request runs on into the load + // pass instead of ending in the resolver. + $this->setFunctionReturn( 'wp_get_referer', admin_url( 'plugins.php' ) ); + + $this->register( + [ + 'standalone_plugin_basename' => self::STANDALONE, + 'conflict_policy' => Conflict_Policy::DEACTIVATE, + 'activation_callback' => static fn() => null, + ] + ); + + $this->boot( $container ); + $this->run_request(); + + $this->assertSame( [ self::SLUG ], array_keys( $registrar->sub_plugins ), 'The host registrar holds the registration.' ); + $this->assertSame( [ self::STANDALONE ], $checker->basenames, 'The host checker answers whether the standalone is active.' ); + $this->assertSame( [ self::STANDALONE ], $deactivator->basenames, 'The host deactivator is the one asked to turn it off.' ); + $this->assertSame( [ self::SLUG ], $notices->merge_notices, 'The host queue is told what happened.' ); + $this->assertSame( [ self::SLUG ], $activator->slugs, 'The host activator runs the one-time setup.' ); + $this->assertSame( 1, $this->bundled_plugin_loads() ); + + $this->assertContains( self::STANDALONE, $this->active_plugins(), 'The default deactivator must not have run too.' ); + $this->assertSame( [], $this->notice_queue(), 'The default queue must not have been resolved alongside it.' ); + $this->assertSame( [], $this->activation_record(), 'The default activator must not have recorded anything.' ); + } + + /** + * The same guarantee for the two the conflict step resolves itself. A host owns what a conflict + * means — but not who may have one resolved, which is why the gate is asked first and separately, + * and is asserted here to have been asked at all. + */ + public function test_a_host_binding_replaces_the_gatekeeper_and_the_resolver(): void { + $gatekeeper = new Spy_Gatekeeper( true ); + $resolver = new Spy_Resolver(); + + update_option( 'active_plugins', [ self::STANDALONE ] ); + + $container = new Test_Container(); + $container->singleton( + Gatekeeper::class, + static function () use ( $gatekeeper ): Gatekeeper { + return $gatekeeper; + } + ); + $container->singleton( + Resolver_Interface::class, + static function () use ( $resolver ): Resolver_Interface { + return $resolver; + } + ); + + $this->register( + [ + 'standalone_plugin_basename' => self::STANDALONE, + 'conflict_policy' => Conflict_Policy::DEACTIVATE, + ] + ); + + $this->boot( $container ); + $this->run_request(); + + $this->assertSame( 1, $gatekeeper->may_resolve_calls, 'The conflict step has to ask the gate.' ); + $this->assertSame( 1, $resolver->resolve_calls ); + $this->assertContains( + self::STANDALONE, + $this->active_plugins(), + 'A host resolver that does nothing means nothing is deactivated.' + ); + $this->assertSame( [], $this->notice_queue() ); + $this->assertSame( 1, $this->bundled_plugin_loads(), 'The load pass still runs after it.' ); + } + + /** + * The second half of the bootstrap, and the point every test above starts from. + * + * The container is handed over bare: `Loader::boot()` is what runs the provider over it, so a + * test that pre-registered the bindings would be asserting against a container the library never + * had to teach. + * + * @param Test_Container|null $container Container to bootstrap with, when a test has bound its + * own implementations into one. + * + * @return void + */ + private function boot( ?Test_Container $container = null ): void { + Config::set_container( $container ?? new Test_Container() ); + + Loader::boot(); + } + + /** + * One page view, which must not end in a redirect. + * + * `wp_safe_redirect()` is stubbed even here, where nothing should reach it. The real one is + * followed by `exit`, which would take the whole test process down rather than fail one test — + * so the stub throws, and a request that redirected when it should not have fails right here + * instead of silently passing somewhere else. + * + * @return void + */ + private function run_request(): void { + $message = self::halted_at_exit_message(); + + $this->setFunctionReturn( + 'wp_safe_redirect', + static function () use ( $message ) { + throw new TestException( $message ); + }, + true + ); + + try { + do_action( 'plugins_loaded' ); + } catch ( TestException $exception ) { + $this->fail( 'The request must not redirect and end here. ' . $exception->getMessage() ); + } finally { + // In a finally block so a failed assertion cannot strand the stub for the rest of the + // process, where a later test's redirect would throw for no reason it can see. + $this->unsetFunctionReturn( 'wp_safe_redirect' ); + } + } + + /** + * One page view that must end where production calls exit(), and where it sent the user. + * + * @return string + */ + private function run_halted_request(): string { + return $this->capture_redirect( + static function (): void { + do_action( 'plugins_loaded' ); + } + ); + } + + /** + * An admin page load, as far as this library is concerned: the hook it renders the queue on. + * + * Dispatched rather than calling `Loader::render_notices()`, because the admin-only `add_action()` + * is half of what has to work — a queue nothing renders is a queue nothing clears either. + * + * @return string + */ + private function render_admin_notices(): string { + ob_start(); + + do_action( 'all_admin_notices' ); + + return (string) ob_get_clean(); + } + + /** + * Register one sub-plugin, backed by a bundled file that exists. + * + * Called before the container is set, which is legal and deliberate: registration is buffered, so + * a host that builds its config array before it builds its container still works. The guard + * constant is unique per call unless the test names one, because loading the file defines it with + * a real `define()` that lasts for the whole PHP process. + * + * @param array $overrides Config values to override. + * @param string|null $constant Guard constant to use, when the test needs to define it. + * + * @return string + */ + private function register( array $overrides = [], ?string $constant = null ): string { + $constant = $constant ?? $this->make_guard_constant(); + $slug = isset( $overrides['slug'] ) && is_string( $overrides['slug'] ) && $overrides['slug'] !== '' + ? $overrides['slug'] + : self::SLUG; + + Loader::register( + array_merge( + [ + 'slug' => $slug, + 'bundled_plugin_file' => $this->make_bundled_plugin_file( $constant ), + 'plugin_loaded_constant' => $constant, + ], + $overrides + ) + ); + + return $constant; + } + + /** + * Define a guard constant for the duration of one test, undone in tearDown. + * + * uopz is what makes this reversible: a plain `define()` lasts for the whole PHP process, and a + * guard left standing makes every later test read its sub-plugin as already loaded. + * + * @param string $constant Constant to define. + * + * @return string + */ + private function define_guard( string $constant ): string { + $this->constants[] = $constant; + + $this->setConstant( $constant, '1.0.0' ); + + return $constant; + } + + /** + * Add an action tearDown can take back by identity rather than by clearing the whole hook. + * + * @param string $hook Hook to add to. + * @param callable $callback Callback to add. + * @param int $priority Priority to add it at. + * + * @return void + */ + private function add_tracked_action( string $hook, callable $callback, int $priority = 10 ): void { + $this->added_hooks[] = [ $hook, $callback, $priority ]; + + add_action( $hook, $callback, $priority ); + } + + /** + * The same, for a filter. Spelled separately even though WordPress keeps actions and filters in + * one registry, so a reader is never left wondering whether a filter was wired by an add_action() + * on purpose. + * + * @param string $hook Hook to add to. + * @param callable $callback Callback to add. + * @param int $priority Priority to add it at. + * @param int $accepted_args How many arguments the callback takes. + * + * @return void + */ + private function add_tracked_filter( + string $hook, + callable $callback, + int $priority = 10, + int $accepted_args = 1 + ): void { + $this->added_hooks[] = [ $hook, $callback, $priority ]; + + add_filter( $hook, $callback, $priority, $accepted_args ); + } + + /** + * Everything this suite writes outside its own fixtures, cleared before and after each test. + * + * @return void + */ + private function clear_state(): void { + delete_site_option( Queue::option_name() ); + delete_site_option( Activator::option_name() ); + delete_option( 'active_plugins' ); + delete_site_option( 'active_sitewide_plugins' ); + } + + /** + * @return array + */ + private function active_plugins(): array { + return (array) get_option( 'active_plugins', [] ); + } + + /** + * The queue is an option and not a transient: with a persistent object cache a transient never + * reaches the database, and a `wp_cache_flush()` would destroy a merge notice raised exactly once. + * `get_site_option()` is `get_option()` outside multisite, so one read covers both install types. + * + * @return array + */ + private function notice_queue(): array { + $queue = get_site_option( Queue::option_name(), [] ); + + return is_array( $queue ) ? $queue : []; + } + + /** + * @return array + */ + private function activation_record(): array { + $done = get_site_option( Activator::option_name(), [] ); + + return is_array( $done ) ? $done : []; + } +} diff --git a/tests/unit/Load/RunnerTest.php b/tests/unit/Load/RunnerTest.php index 1f89644..df5128f 100644 --- a/tests/unit/Load/RunnerTest.php +++ b/tests/unit/Load/RunnerTest.php @@ -8,6 +8,7 @@ use Codeception\TestCase\WPTestCase; use lucatume\WPBrowser\Traits\UopzFunctions; use Nexcess\PluginAbsorber\Config; +use Nexcess\PluginAbsorber\Contracts\Activator_Interface; use Nexcess\PluginAbsorber\Contracts\Registrar_Interface; use Nexcess\PluginAbsorber\Load\Runner; use Nexcess\PluginAbsorber\Loader; @@ -15,12 +16,14 @@ use Nexcess\PluginAbsorber\Sub_Plugin; use Nexcess\PluginAbsorber\Tests\Support\Config_State; use Nexcess\PluginAbsorber\Tests\Support\Loader_State; +use Nexcess\PluginAbsorber\Tests\Support\Spy_Activator; use Nexcess\PluginAbsorber\Tests\Support\Spy_Queue; use Nexcess\PluginAbsorber\Tests\Support\Spy_Registrar; use Nexcess\PluginAbsorber\Tests\Support\Test_Container; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithBundledPlugins; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithContainer; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithIncorrectUsage; +use Nexcess\PluginAbsorber\Tests\Support\Traits\WithSubPlugins; /** * The load loop and its gate chain. @@ -36,6 +39,7 @@ class RunnerTest extends WPTestCase { use WithBundledPlugins; use WithContainer; use WithIncorrectUsage; + use WithSubPlugins; /** * Guard constants a test defined through uopz. @@ -62,6 +66,7 @@ public function setUp(): void { Config::set_hook_prefix( 'give' ); $this->set_up_container(); $this->clear_notices(); + $this->clear_activations(); $this->reset_bundled_plugin_loads(); $this->should_load_calls = []; } @@ -79,6 +84,7 @@ public function tearDown(): void { $this->stop_expecting_incorrect_usage(); $this->clear_notices(); + $this->clear_activations(); Loader_State::reset(); Config_State::reset(); $this->tear_down_container(); @@ -316,6 +322,81 @@ public function test_the_should_load_filter_is_not_consulted_when_dependencies_a $this->assert_the_should_load_recorder_works(); } + /** + * The activation callback stands in for the register_activation_hook() a bundled plugin never + * gets, so it has to run with the plugin's own code already in memory: a migration that calls a + * function the bundled file declares would otherwise fatal. + */ + public function test_it_runs_the_activation_callback_after_requiring_the_file(): void { + $loads_at_activation = null; + + $this->register( + [ + 'activation_callback' => function () use ( &$loads_at_activation ): void { + $loads_at_activation = $this->bundled_plugin_loads(); + }, + ] + ); + + $this->runner()->load_all(); + + $this->assertSame( 1, $this->bundled_plugin_loads() ); + $this->assertSame( 1, $loads_at_activation, 'The bundled file has to be in memory first.' ); + $this->assertSame( [ 'give-recurring' => true ], $this->activation_record() ); + } + + /** + * Activation is tied to a require that actually happened. Running it for a skipped sub-plugin + * would create the tables and seed the options of a plugin whose code is not loaded, and the + * record would then stand the callback down for good once the sub-plugin really did load. + */ + public function test_a_skipped_load_runs_no_activation_callback(): void { + $calls = []; + + $record = static function ( Sub_Plugin $sub_plugin ) use ( &$calls ): void { + $calls[] = $sub_plugin->get_slug(); + }; + + $this->register( [ 'enabled' => false, 'activation_callback' => $record ] ); + + $this->runner()->load_all(); + + $this->assertSame( 0, $this->bundled_plugin_loads() ); + $this->assertSame( [], $calls ); + $this->assertSame( [], $this->activation_record() ); + + // The recorder has to be shown to work. A closure that never reached the config array — a + // mistyped key, a value the constructor refused — leaves this empty for a reason that has + // nothing to do with the load being skipped, and the assertion above proves nothing. + $record( $this->make_sub_plugin() ); + + $this->assertSame( [ 'give-recurring' ], $calls, 'The recorder must catch a call that really happened.' ); + } + + /** + * The activator is injected, not resolved inside the load path, so a host that records "once, + * ever" in its own migration table binds one implementation and the load pass uses it — and the + * default's option is never written behind its back. + */ + public function test_a_bound_activator_reaches_the_load_path(): void { + $activator = new Spy_Activator(); + $container = new Test_Container(); + $container->singleton( + Activator_Interface::class, + static function () use ( $activator ): Activator_Interface { + return $activator; + } + ); + $this->set_up_container( $container ); + + $this->register( [ 'activation_callback' => static fn() => null ] ); + + $this->runner()->load_all(); + + $this->assertSame( [ 'give-recurring' ], $activator->slugs ); + $this->assertSame( [], $this->activation_record(), 'The default activator must not have run too.' ); + } + public function test_it_loads_every_registered_sub_plugin(): void { $this->register( [ 'slug' => 'give-recurring' ] ); $this->register( [ 'slug' => 'give-fee-recovery' ] ); @@ -401,6 +482,8 @@ static function () use ( $registrar ): Registrar_Interface { * when the second one's guard constant never gets defined. */ public function test_one_bundled_file_behind_two_registrations_loads_once(): void { + // The file's own guard constant is one neither registration names, so both of them still + // reach require_once and only the path dedupe can stop the second load. $path = $this->make_bundled_plugin_file( $this->make_guard_constant() ); foreach ( [ 'give-recurring', 'give-fee-recovery' ] as $slug ) { @@ -538,6 +621,21 @@ private function clear_notices(): void { delete_site_option( 'give_plugin_absorber_notices' ); } + private function clear_activations(): void { + delete_site_option( 'give_plugin_absorber_activations' ); + } + + /** + * Slugs whose activation callback has run, as the option holds them. + * + * @return array + */ + private function activation_record(): array { + $done = get_site_option( 'give_plugin_absorber_activations', [] ); + + return is_array( $done ) ? $done : []; + } + /** * The queue is stored as a site option on every install — on single site that call falls through * to the plain option table — so there is one place to read it from. diff --git a/tests/unit/LoaderTest.php b/tests/unit/LoaderTest.php index ebe2fc6..7388c3f 100644 --- a/tests/unit/LoaderTest.php +++ b/tests/unit/LoaderTest.php @@ -8,6 +8,7 @@ use Codeception\TestCase\WPTestCase; use Generator; use Nexcess\PluginAbsorber\Config; +use Nexcess\PluginAbsorber\Conflict\Resolver; use Nexcess\PluginAbsorber\Contracts\Registrar_Interface; use Nexcess\PluginAbsorber\Exceptions\Config_Exception; use Nexcess\PluginAbsorber\Loader; @@ -23,10 +24,11 @@ use Nexcess\PluginAbsorber\Tests\Support\Traits\WithContainer; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithIncorrectUsage; use RuntimeException; +use stdClass; use Throwable; /** - * The public surface: the accessors, registration, and the notice trampoline. + * The public surface: the accessors, registration, and the two notice trampolines. * * Boot timing lives in `Boot\SchedulerTest` and the load loop in `Load\RunnerTest`, which is where * those behaviours moved. What is left here is what a host actually calls. @@ -95,8 +97,9 @@ public function test_a_binding_that_does_not_implement_its_interface_is_reported * @return Generator */ public static function collaborator_accessors(): Generator { - yield 'the registrar' => [ 'registrar', Registrar::class ]; - yield 'the notice queue' => [ 'notices', Queue::class ]; + yield 'the registrar' => [ 'registrar', Registrar::class ]; + yield 'the notice queue' => [ 'notices', Queue::class ]; + yield 'the conflict resolver' => [ 'resolver', Resolver::class ]; } /** @@ -118,8 +121,9 @@ public function test_an_accessor_without_a_container_is_a_configuration_error( s * @return Generator */ public static function accessor_names(): Generator { - yield 'the registrar' => [ 'registrar' ]; - yield 'the notice queue' => [ 'notices' ]; + yield 'the registrar' => [ 'registrar' ]; + yield 'the notice queue' => [ 'notices' ]; + yield 'the conflict resolver' => [ 'resolver' ]; } /** @@ -323,15 +327,8 @@ public function test_the_state_helper_clears_buffered_registrations(): void { } public function test_render_notices_delegates_to_the_bound_queue(): void { - $notices = new Spy_Queue(); - $container = new Test_Container(); - $container->singleton( - Queue_Interface::class, - static function () use ( $notices ): Queue_Interface { - return $notices; - } - ); - $this->set_up_container( $container ); + $notices = new Spy_Queue(); + $this->bind_queue( $notices ); Loader::render_notices(); @@ -359,6 +356,97 @@ public function test_render_notices_does_nothing_without_a_hook_prefix(): void { $this->assert_the_library_reported_incorrect_usage(); } + /** + * The trampoline is a named static method rather than a closure over the container, so that a + * host can take the filter back off — but it still has to reach whatever the host bound, or a + * replacement queue would be handed every other notice and never the activation error. + */ + public function test_the_activation_error_trampoline_delegates_to_the_bound_queue(): void { + $notices = new Spy_Queue(); + $this->bind_queue( $notices ); + + $this->assertSame( + $notices->filtered_markup, + Loader::filter_activation_error_markup( '

Core.

' ) + ); + $this->assertSame( [ '

Core.

' ], $notices->filtered ); + } + + /** + * Reported and handed back untouched rather than thrown out of: this runs while WordPress is + * drawing an error screen, and a second fatal there would replace the one the user came to read. + */ + public function test_the_activation_error_trampoline_does_nothing_without_a_hook_prefix(): void { + $notices = new Spy_Queue(); + $this->bind_queue( $notices ); + + // The prefix goes, the container stays: this is about the missing prefix, and a trampoline + // that reached the container first would pass this test for the other reason. + $container = $this->container(); + Config_State::reset(); + Config::set_container( $container ); + $this->expect_incorrect_usage(); + + $this->assertSame( '

Core.

', Loader::filter_activation_error_markup( '

Core.

' ) ); + $this->assertSame( [], $notices->filtered, 'The queue must not be reached at all.' ); + $this->assert_the_library_reported_incorrect_usage(); + } + + /** + * A filter receives whatever the filter before it returned, so the trampoline takes an untyped + * argument and coerces it. Declaring `string` there would turn another plugin's sloppy return + * into a TypeError raised from this library, on the screen least able to afford one. + * + * The queue still sees the coerced value: the guard is about what crosses the boundary, not + * about standing the rewrite down, and a trampoline that returned early would leave the + * interface's `string` promise resting on an untested path. + * + * @dataProvider non_string_markup + * + * @param mixed $markup Whatever the previous filter returned. + */ + public function test_the_activation_error_trampoline_coerces_a_non_string( $markup ): void { + $notices = new Spy_Queue(); + $this->bind_queue( $notices ); + + $this->assertSame( $notices->filtered_markup, Loader::filter_activation_error_markup( $markup ) ); + $this->assertSame( [ '' ], $notices->filtered ); + } + + /** + * An integer earns its place alongside the two that would fatal: `42` casts cleanly to `"42"`, + * so it is the case a missing guard would pass rather than crash on. + * + * @return Generator + */ + public static function non_string_markup(): Generator { + yield 'null' => [ null ]; + yield 'an array' => [ [ '

Core.

' ] ]; + yield 'an object' => [ new stdClass() ]; + yield 'an integer' => [ 42 ]; + yield 'false' => [ false ]; + } + + /** + * Bind a recording queue, in the order a host binds one: before the provider fills in what is + * missing. + * + * @param Spy_Queue $notices Queue to bind. + * + * @return void + */ + private function bind_queue( Spy_Queue $notices ): void { + $container = new Test_Container(); + $container->singleton( + Queue_Interface::class, + static function () use ( $notices ): Queue_Interface { + return $notices; + } + ); + + $this->set_up_container( $container ); + } + /** * Bind a recording registrar, in the order a host binds one: before the provider fills in what is * missing. diff --git a/tests/unit/Notices/QueueActivationErrorTest.php b/tests/unit/Notices/QueueActivationErrorTest.php new file mode 100644 index 0000000..3f960d3 --- /dev/null +++ b/tests/unit/Notices/QueueActivationErrorTest.php @@ -0,0 +1,345 @@ +fatal error.'; + + /** + * The notice core is about to print, as `wp_admin_notice_markup` hands it over. + * + * @var string + */ + private const MARKUP = '

' . self::CORE_TEXT . '

'; + + public function setUp(): void { + parent::setUp(); + + Loader_State::reset(); + Config_State::reset(); + Config::set_hook_prefix( 'give' ); + + // The rewrite reads the registry through Loader::all(), which resolves the registrar, so + // these tests need a container like every other test that reaches a collaborator. + $this->set_up_container(); + + // is_admin() reads the current screen before WP_ADMIN, so this is what puts the request in + // the admin as well as on the right screen. + set_current_screen( 'plugins' ); + + // Every test starts from the request core actually redirects to after a sandboxed fatal, and + // states only the part it is about. + $_GET['plugin'] = self::STANDALONE; + $_GET['_error_nonce'] = wp_create_nonce( 'plugin-activation-error_' . self::STANDALONE ); + } + + public function tearDown(): void { + // In tearDown rather than at the end of each test body: a failed assertion would otherwise + // leave an admin screen and a half-built activation-error request standing for every test + // that runs afterwards in this process. + unset( $_GET['plugin'], $_GET['_error_nonce'] ); + set_current_screen( 'front' ); + + Loader_State::reset(); + Config_State::reset(); + $this->tear_down_container(); + parent::tearDown(); + } + + public function test_it_replaces_the_fatal_error_text_with_the_configured_message(): void { + $this->register( [ 'conflict_notice_message' => static fn() => 'Give Recurring is already bundled with Give.' ] ); + + $filtered = $this->make_queue()->filter_activation_error_markup( self::MARKUP ); + + $this->assertStringContainsString( 'Give Recurring is already bundled with Give.', $filtered ); + $this->assertStringNotContainsString( self::CORE_TEXT, $filtered ); + + // The notice box stays core's to draw — its classes, its dismiss button, its wrapper. Only + // the sentence inside belongs to this library. + $this->assertStringStartsWith( '

', $filtered ); + $this->assertStringEndsWith( '

', $filtered ); + } + + public function test_the_default_names_the_sub_plugin(): void { + $this->register(); + + $filtered = $this->make_queue()->filter_activation_error_markup( self::MARKUP ); + + // The fallback is not pinned word for word — it is allowed to be reworded, as long as it + // still names the sub-plugin and still displaces core's sentence. + $this->assertStringContainsString( 'give-recurring', $filtered ); + $this->assertStringNotContainsString( self::CORE_TEXT, $filtered ); + } + + /** + * The request names one plugin, and only the sub-plugin that claims that basename may speak for + * it. Reading the first registration instead would put one bundled plugin's explanation on + * another's activation error. + */ + public function test_it_uses_the_sub_plugin_whose_standalone_the_request_names(): void { + $this->register( [ 'conflict_notice_message' => static fn() => 'The wrong one.' ] ); + $this->register( + [ + 'slug' => 'give-fee-recovery', + 'standalone_plugin_basename' => 'give-fee-recovery/give-fee-recovery.php', + 'conflict_notice_message' => static fn() => 'The right one.', + ] + ); + + $_GET['plugin'] = 'give-fee-recovery/give-fee-recovery.php'; + $_GET['_error_nonce'] = wp_create_nonce( 'plugin-activation-error_give-fee-recovery/give-fee-recovery.php' ); + + $filtered = $this->make_queue()->filter_activation_error_markup( self::MARKUP ); + + $this->assertStringContainsString( 'The right one.', $filtered ); + $this->assertStringNotContainsString( 'The wrong one.', $filtered ); + } + + /** + * Nothing draws an admin notice on the front end, and `get_current_screen()` does not exist + * there — the guard is what keeps this filter from fataling if another plugin ever applies + * `wp_admin_notice_markup` outside wp-admin. + */ + public function test_it_leaves_the_markup_alone_outside_the_admin(): void { + $this->register( [ 'conflict_notice_message' => static fn() => 'Ours.' ] ); + + set_current_screen( 'front' ); + + $this->assertSame( self::MARKUP, $this->make_queue()->filter_activation_error_markup( self::MARKUP ) ); + } + + /** + * The activation error is only ever reported on the plugins screen. Every other admin screen + * that prints a notice carrying core's sentence — a bulk action reported elsewhere, a plugin + * quoting it — is somebody else's. + */ + public function test_it_leaves_the_markup_alone_off_the_plugins_screen(): void { + $this->register( [ 'conflict_notice_message' => static fn() => 'Ours.' ] ); + + set_current_screen( 'dashboard' ); + + $this->assertSame( self::MARKUP, $this->make_queue()->filter_activation_error_markup( self::MARKUP ) ); + } + + /** + * wp-admin/network/plugins.php is a one-line require of wp-admin/plugins.php, so it draws the + * same activation error — but WP_Screen appends `-network` to the id. On a default multisite + * the subsite has no plugins UI, so this is the only screen a super admin can reactivate an + * absorbed standalone from, and matching 'plugins' alone would decline exactly there. + */ + public function test_it_rewrites_the_markup_on_the_network_plugins_screen(): void { + $this->register( [ 'conflict_notice_message' => static fn() => 'Ours.' ] ); + + set_current_screen( 'plugins-network' ); + + $this->assertStringContainsString( + 'Ours.', + $this->make_queue()->filter_activation_error_markup( self::MARKUP ) + ); + } + + /** + * The nonce is valid here, so the ownership lookup is the only thing that can stop the rewrite: + * an activation error for a plugin this library knows nothing about keeps core's wording, which + * for that plugin is the accurate one. + */ + public function test_it_leaves_the_markup_alone_for_a_plugin_no_sub_plugin_claims(): void { + $this->register( [ 'conflict_notice_message' => static fn() => 'Ours.' ] ); + + $_GET['plugin'] = 'akismet/akismet.php'; + $_GET['_error_nonce'] = wp_create_nonce( 'plugin-activation-error_akismet/akismet.php' ); + + $this->assertSame( self::MARKUP, $this->make_queue()->filter_activation_error_markup( self::MARKUP ) ); + } + + /** + * @dataProvider requests_that_are_not_an_activation_error + * + * @param callable $arrange Turns the request in setUp() into the one this case is about. + */ + public function test_it_leaves_the_markup_alone( callable $arrange ): void { + $this->register( [ 'conflict_notice_message' => static fn() => 'Ours.' ] ); + + $arrange(); + + $this->assertSame( self::MARKUP, $this->make_queue()->filter_activation_error_markup( self::MARKUP ) ); + } + + /** + * The nonce cases are the ones that matter most: `plugin` is attacker-controlled, and without + * verification any link could make an arbitrary admin page quote a sub-plugin's message back at + * whoever followed it. The nonce is built inside the closure rather than in the provider, + * because a provider runs before setUp() and a nonce is bound to the user current at the time. + * + * @return Generator + */ + public static function requests_that_are_not_an_activation_error(): Generator { + yield 'a nonce for another plugin' => [ + static function (): void { + $_GET['_error_nonce'] = wp_create_nonce( 'plugin-activation-error_akismet/akismet.php' ); + }, + ]; + + yield 'a nonce that is not one' => [ + static function (): void { + $_GET['_error_nonce'] = 'not-a-nonce'; + }, + ]; + + yield 'an empty nonce' => [ + static function (): void { + $_GET['_error_nonce'] = ''; + }, + ]; + + yield 'no nonce at all' => [ + static function (): void { + unset( $_GET['_error_nonce'] ); + }, + ]; + + yield 'no plugin named' => [ + static function (): void { + unset( $_GET['plugin'] ); + }, + ]; + + yield 'an empty plugin name' => [ + static function (): void { + $_GET['plugin'] = ''; + }, + ]; + + // Not a string at all: `plugin[]=x` in the query string arrives as an array, which + // sanitize_text_field() would fatal on if it were not unslashed and sanitised as one. + yield 'a plugin name that is an array' => [ + static function (): void { + $_GET['plugin'] = [ self::STANDALONE ]; + }, + ]; + } + + /** + * The message is sanitised before it is checked for emptiness, and this is the case that pins + * the order: `wp_kses_post( '' )` is the empty string, so swapping core's + * wording for it would leave an empty notice box where the explanation should be. Leaving + * core's sentence in place is the better of the two bad outcomes. + */ + public function test_a_message_that_sanitises_away_leaves_the_markup_alone(): void { + $this->register( [ 'conflict_notice_message' => static fn() => '' ] ); + + $this->assertSame( self::MARKUP, $this->make_queue()->filter_activation_error_markup( self::MARKUP ) ); + } + + /** + * A message that is only whitespace is the same failure with a friendlier shape. + */ + public function test_a_whitespace_only_message_leaves_the_markup_alone(): void { + $this->register( [ 'conflict_notice_message' => static fn() => " \n\t" ] ); + + $this->assertSame( self::MARKUP, $this->make_queue()->filter_activation_error_markup( self::MARKUP ) ); + } + + /** + * `wp_kses_post()` and not `esc_html()`, for the reason the renderer uses it: these messages + * come from the host's own configuration and filters rather than from user input, so the link to + * the knowledge-base article explaining the merge has to reach the screen intact — while the + * event handler and the script a message must never be able to ship do not. + */ + public function test_it_strips_unsafe_markup_from_the_replacement_but_keeps_a_link(): void { + $this->register( + [ + 'conflict_notice_message' => static fn() => 'See the docs.' + . '', + ] + ); + + $filtered = $this->make_queue()->filter_activation_error_markup( self::MARKUP ); + + $this->assertStringContainsString( 'the docs', $filtered ); + $this->assertStringNotContainsString( 'onclick', $filtered ); + $this->assertStringNotContainsString( 'assertStringContainsString( 'alert(2)', $filtered ); + } + + /** + * Register a sub-plugin that claims the standalone this suite's request names. + * + * @param array $overrides Config values to override. + * + * @return void + */ + private function register( array $overrides = [] ): void { + $slug = isset( $overrides['slug'] ) && is_string( $overrides['slug'] ) && $overrides['slug'] !== '' + ? $overrides['slug'] + : 'give-recurring'; + + Loader::register( + array_merge( + [ + 'slug' => $slug, + 'bundled_plugin_file' => "/tmp/{$slug}/{$slug}.php", + 'plugin_loaded_constant' => strtoupper( str_replace( '-', '_', $slug ) ) . '_VERSION_FIXTURE', + 'standalone_plugin_basename' => self::STANDALONE, + ], + $overrides + ) + ); + } + + /** + * The queue under test, built directly rather than resolved. + * + * Its two collaborators are required arguments and neither is reached by the rewrite — nothing + * here is stored and nothing here is drawn — so handing over the real pair states plainly that + * this file is about one method. + * + * @return Queue + */ + private function make_queue(): Queue { + return new Queue( new Store(), new Renderer() ); + } +} diff --git a/tests/unit/Notices/QueueTest.php b/tests/unit/Notices/QueueTest.php index 8551bd4..896b31b 100644 --- a/tests/unit/Notices/QueueTest.php +++ b/tests/unit/Notices/QueueTest.php @@ -15,8 +15,7 @@ use Nexcess\PluginAbsorber\Notices\Store; use Nexcess\PluginAbsorber\Tests\Support\Config_State; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithSubPlugins; -use RuntimeException; -use WP_Error; +use Nexcess\PluginAbsorber\Tests\Support\Traits\WithUsers; use wpdb; /** @@ -24,6 +23,7 @@ */ class QueueTest extends WPTestCase { use WithSubPlugins; + use WithUsers; private const OPTION = 'give_plugin_absorber_notices'; @@ -39,16 +39,9 @@ public function setUp(): void { $this->clear_queue(); // render() consumes the queue, so it is gated on a capability. Most tests care about the - // queue rather than the gate, so they run as someone who has it. - $user_id = $this->create_user( 'administrator' ); - - // On multisite activate_plugins is a network capability, so an administrator of a site is - // not enough — see test_a_site_administrator_on_multisite_cannot_consume_the_queue(). - if ( is_multisite() ) { - grant_super_admin( $user_id ); - } - - wp_set_current_user( $user_id ); + // queue rather than the gate, so they run as someone who has it — which on multisite is a + // network administrator, see test_a_site_administrator_on_multisite_cannot_consume_the_queue(). + $this->become_plugin_administrator(); } public function tearDown(): void { @@ -651,30 +644,6 @@ private function clear_queue(): void { delete_site_option( self::OPTION ); } - /** - * @param string $role Role to give the new user. - * - * @throws RuntimeException When the user cannot be created, rather than letting a later - * capability assertion fail for an unrelated reason. - * - * @return int - */ - private function create_user( string $role ): int { - $user_id = wp_insert_user( - [ - 'user_login' => uniqid( 'absorber-' ), - 'user_pass' => wp_generate_password(), - 'role' => $role, - ] - ); - - if ( $user_id instanceof WP_Error ) { - throw new RuntimeException( 'Could not create a ' . $role . ': ' . $user_id->get_error_message() ); - } - - return $user_id; - } - /** * The queue as the container builds it, or with one half replaced. * diff --git a/tests/unit/ProviderTest.php b/tests/unit/ProviderTest.php index c898287..e63e6b5 100644 --- a/tests/unit/ProviderTest.php +++ b/tests/unit/ProviderTest.php @@ -8,6 +8,12 @@ use Codeception\TestCase\WPTestCase; use Generator; use Nexcess\PluginAbsorber\Boot\Scheduler; +use Nexcess\PluginAbsorber\Conflict\Contracts\Resolver_Interface; +use Nexcess\PluginAbsorber\Conflict\Gatekeeper; +use Nexcess\PluginAbsorber\Conflict\Redirector; +use Nexcess\PluginAbsorber\Conflict\Resolver; +use Nexcess\PluginAbsorber\Activator; +use Nexcess\PluginAbsorber\Contracts\Activator_Interface; use Nexcess\PluginAbsorber\Contracts\Plugin_Checker_Interface; use Nexcess\PluginAbsorber\Contracts\Plugin_Deactivator_Interface; use Nexcess\PluginAbsorber\Contracts\Provider_Interface; @@ -22,8 +28,10 @@ use Nexcess\PluginAbsorber\Provider; use Nexcess\PluginAbsorber\Registrar; use Nexcess\PluginAbsorber\Tests\Support\Config_State; +use Nexcess\PluginAbsorber\Tests\Support\Spy_Activator; use Nexcess\PluginAbsorber\Tests\Support\Spy_Queue; use Nexcess\PluginAbsorber\Tests\Support\Spy_Registrar; +use Nexcess\PluginAbsorber\Tests\Support\Spy_Resolver; use Nexcess\PluginAbsorber\Tests\Support\Test_Container; use StellarWP\ContainerContract\ContainerInterface; @@ -77,6 +85,10 @@ public static function default_bindings(): Generator { yield 'the notice renderer' => [ Renderer::class, Renderer::class ]; yield 'the plugin checker' => [ Plugin_Checker_Interface::class, Plugin_Checker::class ]; yield 'the deactivator' => [ Plugin_Deactivator_Interface::class, Plugin_Deactivator::class ]; + yield 'the activator' => [ Activator_Interface::class, Activator::class ]; + yield 'the conflict resolver' => [ Resolver_Interface::class, Resolver::class ]; + yield 'the redirector' => [ Redirector::class, Redirector::class ]; + yield 'the conflict gate' => [ Gatekeeper::class, Gatekeeper::class ]; yield 'the load runner' => [ Runner::class, Runner::class ]; yield 'the boot scheduler' => [ Scheduler::class, Scheduler::class ]; } @@ -133,8 +145,13 @@ static function () use ( $bound ): object { * @return Generator */ public static function host_bindings(): Generator { - yield 'the registrar' => [ Registrar_Interface::class, new Spy_Registrar() ]; - yield 'the notice queue' => [ Queue_Interface::class, new Spy_Queue() ]; + yield 'the registrar' => [ Registrar_Interface::class, new Spy_Registrar() ]; + yield 'the notice queue' => [ Queue_Interface::class, new Spy_Queue() ]; + yield 'the conflict resolver' => [ Resolver_Interface::class, new Spy_Resolver() ]; + + // "Once, ever" is recorded in an option here, which is one opinion among several — a host + // that tracks it in its own migration table has to be able to say so. + yield 'the activator' => [ Activator_Interface::class, new Spy_Activator() ]; } /**