Skip to content

Introduce a Config module to own the config-file write path (#1062)#1067

Open
donnchawp wants to merge 15 commits into
trunkfrom
config-module-write-path-1062
Open

Introduce a Config module to own the config-file write path (#1062)#1067
donnchawp wants to merge 15 commits into
trunkfrom
config-module-write-path-1062

Conversation

@donnchawp

@donnchawp donnchawp commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR was generated by AI.

Implements #1062 — a Config module that owns the on-disk wp-cache-config.php write path. Conservative, behaviour-preserving refactor: the write path is centralised and made testable while runtime behaviour, the globals read API, and the public function surface stay exactly as they were.

Closes #1062.

What changed

  • New owner. Automattic\WPSC\Config (src/config/class-config.php) is the single owner of the config-file format: set() (updates $GLOBALS[$field], formats, writes), format_value() (per-type encoding), and write_line() (the atomic tempnam→rename→chmod→opcache_invalidate rewriter). write_line() is a verbatim transplant of the old wp_cache_replace_line() body.
  • Public functions become delegating shims. wp_cache_setting() and wp_cache_replace_line() keep identical signatures and remain in wp-cache-phase2.php (third-party cache plugins call them); they lazy-load the class via a class_exists + file_exists guard so the engine-only/debug-log path keeps working.
  • Caller migration. 39 direct wp_cache_replace_line() callers across inc/ that are simple $field = value; writes were migrated to Config::set(). 50 internal wp_cache_setting() callers were converted to Config::set() for consistency (phase2's engine-path debug-log calls deliberately stay on the guarded shim).
  • Documented exceptions. Writes whose on-disk format isn't reproducible by Config::format_value() (define rewrites, line removals, var_export cache_path, imploded wp_cache_mobile_groups, numeric-but-quoted cache_time_interval/cache_max_time, cache_page_secret, $wp_cache_pages[…] sub-keys, the ___-encoded cache_rejected_user_agent, the unanchored sem_id regex) stay on the raw function — catalogued in ADR 0002.
  • Scope held tight. No read site migrated; no Config::get(); no new locking; no runtime autoloader (explicit require_once; classmap is dev/test-only).

Out of scope (recorded in ADR 0002)

Read migration (global $foo / $GLOBALS), removing/deprecating the globals (a permanent read API), moving config off the flat pre-WordPress PHP file, and file locking.

Tests

  • New: tests/php/integration/ConfigWritePathTest.php (characterizes the write path byte-for-byte), ConfigClassTest.php (the class API), plus 3 schedule-cluster cases added to SettingsFormUpdatersTest.php that pin the quoted-vs-unquoted cache_time_interval/cache_max_time distinction.
  • Docs: ADR docs/adr/0002-config-module-write-path.md + a CONTEXT.md entry.

Test plan

  • composer test-php — smoke suite (no DB): 34/34
  • make test-integration — WP + DB integration: 62/62 (incl. WpCacheSettingBackCompatTest locking the public shim contract)
  • composer test-e2e — Docker/Jest e2e: 6 suites / 28 tests
  • composer lint — changed-lines PHPCS: exit 0

donnchawp added 13 commits June 22, 2026 14:13
@donnchawp
docs: implementation plan for Config write-path module (#1062)
8ee3f5e
@donnchawp
docs: fix filtered integration-run command in plan (#1062)
8f2ab29
@donnchawp
test: characterize config-file write path before Config module (#1062)
745eced
@donnchawp
feat: add Automattic\WPSC\Config write-path module (#1062)
ac3b485
@donnchawp
refactor: delegate wp_cache_replace_line() to Config::write_line() (#…
be5cbc7 
…1062)
@donnchawp
style: align ConfigClassTest helper assignments to satisfy phpcs (#1062)
04896ae
@donnchawp
refactor: delegate wp_cache_setting() to Config::set() (#1062)
7bf7b89
@donnchawp
refactor: migrate lifecycle config writes to Config::set() (#1062)
92e3539 
Converts the one migratable wp_cache_replace_line() call in
inc/lifecycle.php (line 84) to \Automattic\WPSC\Config::set().

Seven raw-function sites left unchanged:

- Lines ~416, ~418: rewrite define('WPCACHEHOME', ...) — define rewrite,
  not a key/value set; Config::set() does not emit define statements.
- Line ~424: regex is bare unanchored 'sem_id', not '^ *\$sem_id';
  Config::set() emits an anchored regex, which would change match
  semantics.
- Lines ~467, ~608: rewrite define('WPCACHEHOME'/'WP_CACHE', ...) in
  $global_config_file (site wp-config.php) — define rewrite.
- Lines ~637, ~647: replacement string is '' (line removal, not a set).
@donnchawp
refactor: migrate settings-form config writes to Config::set() (#1062)
bcb2b1c 
Converts 14 direct wp_cache_replace_line() callers in inc/settings-forms.php
to \Automattic\WPSC\Config::set() per the #1062 migration plan.

Adds three characterization test methods to SettingsFormUpdatersTest covering
wp_cache_time_update() (interval path, initialization path, clock path). Tests
ran GREEN against the pre-migration code and remain GREEN after migration,
confirming byte-identical config output.

Six raw wp_cache_replace_line() calls left unchanged:

- L26:  define('WPLOCKDOWN', ...) — not a key/value write; different format.
- L77:  $cached_direct_pages — hand-built array( ) literal uses legacy syntax
        (no integer keys); format_value's var_export form would differ. Byte difference.
- L150: $cache_time_interval (init) — numeric value but written QUOTED by the
        original; Config::set() would emit it unquoted. Byte difference.
- L176: $cache_time_interval (interval path) — same numeric-quoted trap as L150.
- L219: $cache_rejected_user_agent — $text has ___ → space restored AFTER
        wp_cache_sanitize_value(); the array still holds ___ at write time so
        format_value(array) != $text. Leave on raw call.
- L249: $wp_cache_pages["$page"] — array sub-key write; Config::set() cannot
        express it.
@donnchawp
refactor: migrate admin-UI config writes to Config::set() (#1062)
230e8bb 
Converts 24 of 28 wp_cache_replace_line() calls in inc/admin-ui.php
(the wp_cache_manager_updates() cluster and related warning-dismiss
handlers) to \Automattic\WPSC\Config::set().

Four sites left on raw wp_cache_replace_line() — each is a byte-
divergence trap:

- L373 cache_page_secret: value is md5() (hex string) written QUOTED.
  Config::set would unquote it if the md5 were all-digits (is_numeric).
- L397 cache_time_interval: numeric value 600 written QUOTED here;
  Config::set emits unquoted.
- L398 cache_max_time: numeric value 1800 written QUOTED here;
  Config::set emits unquoted.
- L434 cache_path: uses var_export() for backslash/quote escaping;
  Config::set string branch does raw interpolation — diverges on paths
  containing backslashes.
@donnchawp
refactor: route internal config writes through Config::set() (#1062)
5ee7858
@donnchawp
docs: record Config write-path module and contract (#1062)
61f086f
@donnchawp
docs: add cache_rejected_user_agent exception + global-sync note to A…
0aad2cd 
…DR 0002 (#1062)
@donnchawp donnchawp self-assigned this Jun 22, 2026
donnchawp added 2 commits June 22, 2026 17:27
@donnchawp
test: lock wp_cache_setting()/wp_cache_replace_line() back-compat con…
e645a65 
…tract (#1062)
@donnchawp
fix: shim returns false on missing Config class file; correct ADR res…
b8bd9cc 
…t/* row (#1062)

- wp_cache_setting() / wp_cache_replace_line() shims now early-return false
  when src/config/class-config.php is absent, instead of fataling on the
  Config:: call. Restores the old inline functions' graceful degradation on
  the engine-only/partial-install path the lazy-load guard protects.
- ADR 0002: split the stale 'plugins/* and rest/* out of scope' row. The REST
  handler was migrated to Config::set(); only set_ossdlcdn stays on the raw
  call. Document that separately.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@donnchawp donnchawp

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Refactor: introduce a Config module to own the config-file write path

1 participant

@donnchawp