Automattic / Automattic/wp-super-cache

Architecture: deepening opportunities to improve testability and navigability

Open
#1,047 0 comments 0 reactions 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.