Problem/Motivation

AmpEventSubscriber::onView() sets _wrapper_format=html on the request query bag for non-AMP routes. This happens on KernelEvents::VIEW, which fires after the Dynamic Page Cache (DPC) subscriber has already performed its cache lookup on KernelEvents::REQUEST at priority 27.

Because of this timing mismatch, the two cache IDs used by DPC are different:

- Lookup CID (built at REQUEST p27): uses _wrapper_format='' (not yet set)
- Store CID (built at RESPONSE p7, after VIEW): uses _wrapper_format=html (set by AmpEventSubscriber)

Since the two CIDs never match, every request results in a permanent DPC MISS for any site with the amp module enabled.

This is detectable via the X-Drupal-Dynamic-Cache: MISS response header on every page load, even for anonymous users on cacheable routes.

Steps to reproduce

1. Install the amp module on a Drupal 10/11 site.
2. Enable Dynamic Page Cache (enabled by default).
3. Visit any non-AMP cacheable page (e.g. a node) twice as an anonymous user.
4. Inspect the X-Drupal-Dynamic-Cache response header — it reads MISS on every request instead of HIT on the second.
5. Optionally, enable debug logging and compare the cache context key for url.query_args:_wrapper_format at request time vs. response time — the values differ ('' vs 'html').

Proposed resolution

Add a KernelEvents::REQUEST listener at priority 29 (after RouterListener at 32, before DPC at 27) that sets _wrapper_format=html for non-AMP requests that do not already carry a wrapper format.

This mirrors exactly what onView() does for the non-AMP case, but at the right moment so both the DPC lookup and the DPC store use the same _wrapper_format value.

The onView() method is then simplified to only handle the AMP route case (_wrapper_format=amp), since the non-AMP case is now covered earlier. As a secondary improvement, onView() is updated to read _wrapper_format from $request->query->get() instead of $_GET directly, which is the correct Symfony/Drupal approach.

Proposed patch attached. The changes are limited to src/EventSubscriber/AmpEventSubscriber.php:

- Add use Symfony\Component\HttpKernel\Event\RequestEvent;
- Add onRequest(RequestEvent $event) method
- Simplify onView() to AMP-route handling only, replacing $_GET access with $request->query->get()
- Register the new listener in getSubscribedEvents()

Remaining tasks

- [ ] Review and test the patch
- [ ] Confirm behavior is correct for AMP routes (?amp parameter) — these are intentionally left to onView() because AmpContext::isAmpRoute() requires the router to be resolved first
- [ ] Add test coverage if the module has a test suite for the event subscriber

User interface changes

None.

API changes

AmpEventSubscriber gains a new public method onRequest() and its getSubscribedEvents() now registers an additional KernelEvents::REQUEST listener. No changes to public APIs or service definitions.

Data model changes

None.

Comments

jansete created an issue. See original summary.

jansete’s picture

Status: Active » Needs review
StatusFileSize
new3.62 KB

Attach the patch.