Dynamic cache does not respect query parameters

A response MUST vary based on route (because a different controller is called for each route) and request format (again, different controller, or at least different post-processing for the same controller's return value).

Any variations beyond that are handled automatically by Dynamic Page Cache. On one condition: your controller specifies the correct cacheability metadata.

Everything in this issue suggests that Drupal Commerce is missing some cacheability metadata: it should specify

$build['#cache']['contexts'] = 'url.query_args:refid';

in the code building a render array which varies depending on the specified ?refid in the URL.

Sorry, I missed #3.

    $url = Url::fromRoute('commerce_checkout.form', ['commerce_order' => $order->id(), 'step' => $step_id])->toString(TRUE);
    $response = new TrustedRedirectResponse($url->getGeneratedUrl());
    // Added this when dependencies didn't do anything.
    $response->setMaxAge(0);
    $response->addCacheableDependency($url);
    $response->addCacheableDependency($order);
    return $response;

So this adds the cache tag for the Order entity. It also sets the cache tags + cache contexts that are bubbled during URL generation. Likely you're getting the url.site cache context. But how would $url know about this refid URL query argument from the incoming request, which is causing this redirect?

From what you described, you need to add

$response->getCacheableMetadata()->addCacheContexts(['url.query_args:refid'])

Ideally, I would be able to see the entire controller logic, then I can answer this properly.

So that means you were able to get it working as expected, by adding the missing cacheability metadata? :)

Hm, but wait:

    $response->setMaxAge(0);

That should have ensured the response wasn't cached by Dynamic Page Cache. We even have test coverage for that in \Drupal\Tests\dynamic_page_cache\Functional\DynamicPageCacheIntegrationTest:

    // Max-age = 0 responses are ignored by Dynamic Page Cache.
    $this->drupalGet('dynamic-page-cache-test/html/uncacheable/max-age');
    $this->assertEqual('UNCACHEABLE', $this->drupalGetHeader(DynamicPageCacheSubscriber::HEADER), 'Render array returned, rendered as HTML response, but uncacheable: Dynamic Page Cache is running, but not caching.');

If that's not working, then that would be a bug. Test to prove that this indeed should work (existing test coverage should already be sufficient, but since this issue is suggesting it doesn't actually work, adding more explicit test coverage).

Correcting the cacheability metadata has fixed your problems?

Well, existing test coverage should be sufficient, but I'd love to hear it if that weren't the case.

Marking as postponed on the feedback #16 indicates will be provided in the future :)

@Wim I am trying this within a block that I would think would bubble up to the page? I am using both the getCacheMaxAge() method (setting it to zero) and getCacheContexts() using 'url.query_args' but the page cache is still ignoring the query string. I am using page_manager which should also respect cache contexts but maybe the bug is in page manager (in which case I can log an issue there)?

Note page_cache !== dynamic_page_cache. I think it's best if you open a new issue and the actual code (sanitized). And page_manager has been known to have many cacheability bugs, so yes, that's quite likely.

Just a feedback, I have a pretty similar case described in this issue and adding $response->getCacheableMetadata()->addCacheContexts(['url.query_args:query']) solved the issue for me, dynamic cache is caching both urls, with and without the query string.

Pinged @mglaman yesterday to get feedback per #17.

Alright, thanks for confirming! 👍

I had this problem with my controller's module, which took the cache value instead of the url one (i'm using Drupal 10)

I solved the problem with #cache and max-age to 0:

        $address = $request->query->get('adresse');

        return [
            '#address' => $address,
            '#cache' => [
                'max-age' => 0,
            ],
        ];