Problem/Motivation
From
// Force a direct download if a "dl" or "download" query string is present.
if ($this->request->query->has('dl') || $this->request->query->has('download')) {
$response->setContentDisposition(ResponseHeaderBag::DISPOSITION_ATTACHMENT, $file->getFilename());
}
$response->getCacheableMetadata()->addCacheContexts([
'url.query_args:edit-media',
'user.permissions',
'url.query_args:dl',
'url.query_args:download',
]);
Not sure what url.query_args:edit-media is doing there.
But that shouldn't be the full problem. The full problem is that you have a cache ID with values for the following contexts:
user.permissions, url.query_args:dl, url.query_args:download, route, request_format
At said cache ID, DisplayController is trying to either store a value or store a redirect telling VariationCache that a few more cache contexts need to be checked. It turns out to be the latter in your case. Now it seems we got to that same CID multiple times and the first time we said: "Oh hey, you need to check for the following cache contexts too: user.roles:authenticated, languages:language_interface, theme,, url.site, timezone, url.query_args:_wrapper_format"
But the second time we got there, it says we only need to check for url.query_args:edit-media. That part is coming from the code above.
So the question is: Where in the flow of DisplayController (or the code that runs before that!) does it get to the point where it returns the first set of contexts and why aren't those contexts carried over to the point where we return the second set of contexts?
In the test I see the following code, where the last drupalGet triggers the error.
// Test when the kill switch is enabled.
\Drupal::configFactory()
->getEditable('media_alias_display.settings')
->set('kill_switch', TRUE)
->save(TRUE);
$this->drupalGet('media/' . $media->id());
$assert_session->statusCodeEquals(200);
$assert_session->responseHeaderContains('Content-Type', 'text/html');
// Test when no bundle is selected, should default to all.
// Turn off kill switch.
\Drupal::configFactory()
->getEditable('media_alias_display.settings')
->set('kill_switch', FALSE)
->save(TRUE);
// There should be no need to explicitly flush the Drupal cache assuming
// we pass in all the relevant cache tags to the original responses.
$this->resetAll();
$this->drupalGet('media/' . $media->id());
So probably that kill_switch affects the code flow but the code doesn't actually have a cache context to represent it. If you are supposed to clear the cache after you toggle the switch, then your test also needs to flush the cache, unlike what that comment claims in the test.
Yup, at the top of DisplayController::check you have
if (!empty($config->get('kill_switch')) && $config->get('kill_switch') === TRUE) {
return $this->updateRenderCache(parent::__invoke($media, $view_mode), $config);
}
But there is no cache context representing that variation in any way. Toggling the kill switch in the test without clearing caches rightfully confuses the hell out of VariationCache and leads to this exception.
The right fix is to add a cache context representing said toggle, Group Sites has a similar one, see: GroupSitesAdminModeCacheContext
The easy way out is to document that people need to clear their cache when they change the toggle status and then reqrite the test to do the same.
Steps to reproduce
Proposed resolution
Fix cache tags
Remaining tasks
User interface changes
API changes
Data model changes