Automattic / Automattic/wp-super-cache
Architecture: deepening opportunities to improve testability and navigability
- Dominant language
- PHP
- Stars
- 436
- Forks
- 130
- Avg merge
- 15h 11m
- Merged PRs (30d)
- 10
Description
This is a structured architectural audit of the plugin, surfacing **deepening opportunities** — refactors that turn shallow modules (interface nearly as complex as implementation) into deeper ones with smaller interfaces. The aim is testability and easier navigation. Filing as an umbrella issue; individual items can be split out if any are picked up.
Vocabulary used below:
- **Module** — anything with an interface + implementation.
- **Deep / shallow** — small interface hiding lots of behaviour (deep) vs interface ≈ implementation cost (shallow).
- **Deletion test** — would removing this module concentrate complexity (it earns its keep) or just move it sideways (pass-through)?
- **Locality** — change/bugs/knowledge concentrated in one place.
---
## 1. A `Config` module that owns the on-disk config file
**Files:** `wp-cache-config-sample.php`, `wp-cache-phase2.php` (`wp_cache_replace_line()` ~L1410), `rest/class.wp-super-cache-rest-get-settings.php`, `rest/class.wp-super-cache-rest-update-settings.php`, plus ~100 read sites across `wp-cache.php`, `wp-cache-phase1.php`, `wp-cache-phase2.php`.
**Problem:** Configuration is ~100 loose PHP globals declared in `wp-cache-config.php`. Reads happen by `global \$foo;` in dozens of functions. Writes happen by `wp_cache_replace_line()` — a ~100-line regex that rewrites the file in place, with no locking and no atomicity. REST endpoints re-`include()` the file per request to refresh globals. The effective config interface is the union of every regex pattern and every global name.
**Solution:** A single `Config` module that loads the file once, exposes typed `get(\$key)` / `set(\$key, \$value)` / `save()`, and is the only thing that knows the file's format. Callers stop declaring globals. `wp_cache_replace_line()` becomes private or disappears.
**Benefits:** Locality — one place to look for any persistence question. Leverage — new settings cost a schema entry, not a new setter + new regex + new global. Tests — file-rewriting logic gets a small enough surface to fuzz; REST endpoints can be tested against an in-memory config double.
---
## 2. A `CacheKey` value object
**Files:** `wp-cache-phase2.php` — `get_wp_cache_key()` (~L34), `wp_cache_check_mobile()` (~L648), `supercache_filename()` (~L1076), plus the MD5 hashing in `wp_super_cache_init()` (~L107).
**Problem:** The cache key is assembled across three functions and two `do_cacheaction()` hook points, reading `\$wp_cache_request_uri`, `\$WPSC_HTTP_HOST`, `\$wp_cache_gzip_encoding`, cookie state, and the mobile suffix. There is no single answer to \"what is the cache key for this request?\"
**Solution:** A `CacheKey` module that takes the request inputs as an explicit struct and returns both the key string and the on-disk filename. Mobile suffix, gzip flag, and plugin overrides live behind that one interface.
**Benefits:** Locality — any \"wrong page served from cache\" bug has one place to look first. Leverage — new keying axes (logged-in users, currency) become one parameter, not a new global + new function. Tests — key generation becomes pure; table-driven tests over (URL, headers, cookies) → key.
**Deletion test:** Strong pass — removing the three-function split concentrates complexity rather than scattering it.
---
## 3. A documented `Cacheaction` registry
**Files:** `wp-cache-phase2.php` (`add_cacheaction` / `do_cacheaction` ~L612–628), ~22 call sites across phase2 and `wp-cache.php`, consumers in `plugins/` and the external `wp-super-cache-plugins/`.
**Problem:** WP Super Cache has its own plugin hook system parallel to WP's. The implementation is ten lines. The *interface* — which hooks exist, what arguments they get, what the contract on the return value is — is undocumented and lives only by grepping. Textbook shallow module.
**Solution:** A registry that names each cacheaction, documents its signature/semantics in one place, and optionally validates arguments/return shape. The mechanism stays trivial; the contract gets a home.
**Benefits:** Locality — \"what plugin hooks does WPSC expose?\" answered by reading one file. Leverage — third-party plugin authors get a stable, discoverable surface; contract-breaking refactors fail loudly. Tests — hook contracts can have characterisation tests.
---
## 4. Split `wp-cache.php` along its three real responsibilities
**File:** `wp-cache.php` (4,511 LOC).
**Problem:** Three concerns that change at different rates are interleaved: (a) lifecycle (activation/deactivation, drop-in install, error notices), (b) admin settings UI (rendering + form handling), (c) the preload orchestrator (~600 LOC of cron, AJAX, state files). Each is a coherent module pretending to be a pile of free functions.
**Solution:** Three modules — `PluginLifecycle`, `AdminUI`, `Preload` — each owning its own state and hooks. The file becomes a thin wiring layer.
**Benefits:** Locality — preload bugs stop touching activation code. Leverage — each module's interface (a handful of WP hook callbacks) is dramatically smaller than the union of 180 free functions. Tests — preload state and UI rendering become testable in isolation.
Highest-impact entry on the list, also highest-risk. Best done after #1 so you're not chasing globals at the same time.
---
## 5. A `Preload` state machine
**Files:** `wp-cache.php` ~L3357–4151, `wp-cache-preload-status.txt`, preload transients.
**Problem:** Preload state lives in three places simultaneously — a WP transient, a status text file, and implicit cron schedule state — and is updated from multiple call sites (`wp_cron_preload_cache`, `wpsc_update_active_preload`, AJAX handlers). No single function answers \"is preload running and where is it up to?\" without consulting all three. Race conditions are possible by construction.
**Solution:** A `Preload` module owning the state representation, exposing `start()`, `stop()`, `status()`, `tick()`, and as the only thing that touches the transient/file. Cron and AJAX handlers become callers.
**Benefits:** Locality — all state transitions in one module. Leverage — storage can be swapped (e.g., custom table) without touching callers. Tests — state machine testable with a fake clock and fake fetcher.
Can ship standalone, or as the first slice of #4.
---
## 6. Boost migration as a small explicit state machine — or delete it
**Files:** `inc/boost.php` (234 LOC), Boost notice/dismiss/AJAX in `wp-cache.php` (~L366–482).
**Problem:** Boost detection, \"should we nag?\", dismiss handling, and the activate-Boost AJAX are spread across two files and use ad-hoc options + transients. A feature with a lifecycle, pretending to be a pile of utility functions.
**Solution A:** Pull it into a `BoostMigration` module with explicit states (not-installed / installed-not-active / active-incompatible / active-compatible / dismissed) and one place that decides what banner to show.
**Solution B:** If Boost migration is no longer a strategic priority, the deeper win is deletion. Worth asking before refactoring.
---
## 7. A `Request` value at the phase1 boundary
**Files:** `wp-cache-phase1.php`, top of `wp-cache-phase2.php`, the cache-serve path.
**Problem:** Phase1 runs before WP loads and reconstructs request facts from `\$_SERVER` + cookies + headers into a half-dozen globals (`\$wp_cache_request_uri`, `\$WPSC_HTTP_HOST`, gzip negotiation state) that phase2 then re-reads. The \"request\" is implicit and globally mutable.
**Solution:** A `Request` value built once at the phase1 entry point and threaded through (or stored on a single context object).
**Benefits:** Locality — request parsing in one place; request mutation impossible by construction. Tests — cache-serve logic can be exercised with a fabricated `Request` instead of mutating `\$_SERVER`.
Prerequisite for clean tests of #2.
---
## How these relate
\`\`\`
┌──────────────────┐
│ 1. Config module │◄──────── unblocks ─────┐
└────────┬─────────┘ │
▼ │
┌──────────────────┐ ┌─────────┴─────────┐
│ 7. Request value │──── feeds ──►│ 2. CacheKey value │
└──────────────────┘ └───────────────────┘
┌──────────────────┐
│ 3. Cacheaction │◄── documents the hooks both touch
│ registry │
└──────────────────┘
┌──────────────────────────────────────────┐
│ 4. Split wp-cache.php (Lifecycle/UI/...)│ ── easier after (1)
└──────────────────────────────────────────┘
├──► 5. Preload state machine (can ship standalone)
└──► 6. Boost migration (or delete)
\`\`\`
---
## Suggested sequencing
Highest-leverage path for the least invasive change: **1 → 7 → 2**. #4 is the biggest prize but worth doing later, after #1 has tamed the globals problem.
## Notes / caveats
- The test suite at `tests/php/` currently contains only a bootstrap. None of the modules above have characterisation tests today, which makes any refactor riskier — adding tests at the seams *before* moving code is recommended for each candidate.
- This issue is intentionally an umbrella. If maintainers want to pursue any item, splitting it into its own issue with a concrete interface proposal is the next step.
_Audit produced via \`/improve-codebase-architecture\`._
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by choosing one of the seven audit items rather than treating this umbrella issue as a single task. Read the named files and entry points for that item, then inspect tests/php/, which currently contains only a bootstrap. Done means the chosen refactor has a concrete interface proposal, focused scope, and characterization tests or another stated verification plan before implementation.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100