From 7059031b7b1868b019268b0b7a67c2460761c84e Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Wed, 12 Aug 2026 15:14:24 +0200 Subject: [PATCH] Resolve a standalone conflict through injected collaborators Conflict\Resolver takes its four collaborators as required constructor arguments -- the checker, the deactivator, the notice queue and the redirector -- because the container is now mandatory and nothing has to be constructible without one. The nullable peers and the accessors that fell back to a static are gone with the class they fell back to. Conflict\Gatekeeper owns who may resolve: an interactive admin GET, the activate_plugins capability, and a hook prefix. plugins_loaded runs on every request and fires before auth_redirect(), so an unauthenticated GET of an admin URL reaches this code, and the capability gate covers every policy rather than only the destructive one -- the other branches queue a notice the same user could not render anyway. The priority-1 step asks the gatekeeper before it resolves Resolver_Interface at all, so a host binding its own resolver cannot drop either gate by omission. Conflict\Destination becomes Conflict\Redirector. It decides where to send the user 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. A class that returns a URL from a filter still earns the agent noun. The boot barrier now measures from the lowest priority in the sequence rather than from the load priority, or a host booting between conflict resolution and the load would be told nothing while half its wiring silently failed. --- CLAUDE.md | 61 +- README.md | 12 +- docs/configuration.md | 8 + docs/conflict-handling.md | 57 +- docs/filters.md | 4 +- src/Boot/Scheduler.php | 48 +- src/Conflict/Contracts/Resolver_Interface.php | 32 + src/Conflict/Gatekeeper.php | 104 +++ src/Conflict/Redirector.php | 56 ++ src/Conflict/Resolver.php | 164 +++++ src/Loader.php | 12 + src/Provider.php | 18 + tests/README.md | 15 + tests/_support/Spy_Gatekeeper.php | 50 ++ tests/_support/Spy_Resolver.php | 36 + tests/_support/Traits/WithUsers.php | 63 ++ tests/unit/Boot/SchedulerTest.php | 268 +++++++- tests/unit/Conflict/GatekeeperTest.php | 228 +++++++ tests/unit/Conflict/RedirectorTest.php | 57 ++ tests/unit/Conflict/ResolverTest.php | 617 ++++++++++++++++++ tests/unit/LoaderTest.php | 11 +- tests/unit/Notices/QueueTest.php | 41 +- tests/unit/ProviderTest.php | 13 +- 23 files changed, 1897 insertions(+), 78 deletions(-) create mode 100644 src/Conflict/Contracts/Resolver_Interface.php create mode 100644 src/Conflict/Gatekeeper.php create mode 100644 src/Conflict/Redirector.php create mode 100644 src/Conflict/Resolver.php create mode 100644 tests/_support/Spy_Gatekeeper.php create mode 100644 tests/_support/Spy_Resolver.php create mode 100644 tests/_support/Traits/WithUsers.php create mode 100644 tests/unit/Conflict/GatekeeperTest.php create mode 100644 tests/unit/Conflict/RedirectorTest.php create mode 100644 tests/unit/Conflict/ResolverTest.php diff --git a/CLAUDE.md b/CLAUDE.md index 1143f48..7405032 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,8 +133,7 @@ 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: +`Activator` is not built yet. Currently: | Path | What | |---|---| @@ -147,6 +146,7 @@ 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/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`. | @@ -161,7 +161,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,6 +177,14 @@ 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). @@ -188,23 +196,40 @@ 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. Both passes read +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 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..3058271 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -47,11 +47,19 @@ $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. | `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 diff --git a/docs/conflict-handling.md b/docs/conflict-handling.md index 43cd75b..e323abc 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 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/src/Boot/Scheduler.php b/src/Boot/Scheduler.php index 3c69b4c..9589098 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 @@ -77,7 +95,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 +127,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 +156,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 +183,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/Loader.php b/src/Loader.php index d2216bf..5f33e77 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(). * diff --git a/src/Provider.php b/src/Provider.php index 31a200d..16307c0 100644 --- a/src/Provider.php +++ b/src/Provider.php @@ -6,6 +6,10 @@ 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\Plugin_Checker_Interface; use Nexcess\PluginAbsorber\Contracts\Plugin_Deactivator_Interface; use Nexcess\PluginAbsorber\Contracts\Provider_Interface; @@ -64,6 +68,8 @@ public function register(): void { $this->bind_once( Plugin_Deactivator_Interface::class, Plugin_Deactivator::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,6 +81,18 @@ 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 { diff --git a/tests/README.md b/tests/README.md index adc7d3a..f2246d1 100644 --- a/tests/README.md +++ b/tests/README.md @@ -125,6 +125,21 @@ A fixture helper cannot be called `make()`, `makeEmpty()`, `construct()`, or `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 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_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/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/Boot/SchedulerTest.php b/tests/unit/Boot/SchedulerTest.php index 993f54c..6100100 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(); @@ -209,13 +322,15 @@ static function () use ( &$fired ): void { * 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 +364,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(); @@ -335,6 +481,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 +551,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. * 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/LoaderTest.php b/tests/unit/LoaderTest.php index ebe2fc6..89a6c65 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; @@ -95,8 +96,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 +120,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' ]; } /** 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..37ad389 100644 --- a/tests/unit/ProviderTest.php +++ b/tests/unit/ProviderTest.php @@ -8,6 +8,10 @@ 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\Contracts\Plugin_Checker_Interface; use Nexcess\PluginAbsorber\Contracts\Plugin_Deactivator_Interface; use Nexcess\PluginAbsorber\Contracts\Provider_Interface; @@ -24,6 +28,7 @@ use Nexcess\PluginAbsorber\Tests\Support\Config_State; use Nexcess\PluginAbsorber\Tests\Support\Spy_Queue; use Nexcess\PluginAbsorber\Tests\Support\Spy_Registrar; +use Nexcess\PluginAbsorber\Tests\Support\Spy_Resolver; use Nexcess\PluginAbsorber\Tests\Support\Test_Container; use StellarWP\ContainerContract\ContainerInterface; @@ -77,6 +82,9 @@ 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 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 +141,9 @@ 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() ]; } /**