Skip to content

Require symfony/cache, wired in services.yaml - #90

Merged
sylfabre merged 3 commits into
mainfrom
sylvain_require-symfony-cache
Aug 21, 2026
Merged

Require symfony/cache, wired in services.yaml#90
sylfabre merged 3 commits into
mainfrom
sylvain_require-symfony-cache

Conversation

@sylfabre

@sylfabre sylfabre commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Adds symfony/cache: ^7.0 to require.

config/services.yaml wires Symfony\Component\Cache\Psr16Cache and Symfony\Component\Cache\Adapter\FilesystemAdapter for the php-domain-parser public-suffix-list cache, but the package providing them was never required — it only worked because consuming apps happen to ship symfony/cache themselves.

psr/simple-cache stays: it looks redundant but is load-bearing — both symfony/cache and php-domain-parser list it only in their require-dev, so without our direct require the Psr\SimpleCache\CacheInterface implemented by Psr16Cache would not be installed at runtime (verified: dropping it removes vendor/psr/simple-cache entirely, and the test suite does not catch it because the pdp cache services are not instantiated there).

Validated in a PHP 8.4 container: composer validate --strict, composer update, PHPUnit green (290 tests).

🤖 Generated with Claude Code

Test coverage

The suite previously never exercised the DI wiring: every test builds validators by hand, so neither a missing symfony/cache nor a missing psr/simple-cache could be detected (both were verified to slip through). This PR adds PublicSuffixListCacheChainTest, a functional smoke test that boots the kernel, compiles the real services.yaml, and runs PublicSuffixListCacheWarmer twice against a stubbed, network-free PSR-18 client — asserting the second warm-up is served from the PSR-16 cache. Verified: the test errors when either package is removed from vendor/.

Enabling this surfaced three latent issues, fixed here:

  • the Pdp\Storage\ glob resource ('../../../jeremykendall/php-domain-parser/src/Storage/*') only resolves when the bundle sits under a consumer's vendor/; container compilation inside this repo failed with FileLocatorFileNotFoundException. Replaced by the single explicit service that relied on it (PublicSuffixListPsr18Client). Note for consumers: the unused pdp TopLevelDomain* classes are no longer auto-registered.
  • the shipped functional test app used framework.validation.enable_annotations, removed in Symfony 7 — the fixture kernel was un-bootable (enable_attributes now).
  • symfony/var-exporter is pinned ^7.0 in require-dev: as an unconstrained transitive dep it resolved to 8.x, where ProxyHelper::generateLazyGhost() no longer exists, making doctrine/orm 3.6 refuse to boot without native lazy objects.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

sylfabre and others added 2 commits August 21, 2026 15:29
The DI wiring in services.yaml was never exercised by the test suite:
all tests build validators by hand, so a missing symfony/cache or
psr/simple-cache package could not be detected. This boots the real
container, runs PublicSuffixListCacheWarmer against a stubbed PSR-18
client, and asserts the second warm-up is served from the PSR-16 cache.

The Pdp\Storage glob resource only resolved when the bundle was
installed under a consumer vendor/ directory; it is replaced by the
single explicit service that relied on it so the container can compile
inside this repository too.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Symfony 7.0 runs non-optional warmers during kernel boot while 7.4 does
not, so the stub request count is asserted relative to a post-boot
baseline instead of an absolute value.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

1 similar comment
@sonarqubecloud

Copy link
Copy Markdown

@sylfabre
sylfabre merged commit 78da8c0 into main Aug 21, 2026
9 checks passed
@sylfabre
sylfabre deleted the sylvain_require-symfony-cache branch August 21, 2026 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant