Unable to configure `match_response` for `cache_control` match rules for Symfony 7

[alongosz]

When trying to configure match_response on a clean Symfony 7 project with FOSHttpCacheBundle (v3.1.1) enabled, using the following config:

fos_http_cache:
    cache_control:
        rules:
            - match:
                  host: ^login.example.com$
                  match_response: 'response.isFresh()'
              headers:
                  cache_control: {public: false}

We encounter the following error when clearing Symfony cache:

Service "fos_http_cache.cache_control.expression.e4ac4270c49eeac61019bc369235175a": Cannot replace arguments for class "FOS\HttpCacheBundle\Http\ResponseMatcher\ExpressionResponseMatcher" if none have been configured yet.

It appears that the Symfony DI behavior has changed since Symfony 6+. Instead of calling replaceArgument, the extension should likely call setArgument instead.

The issue can also be reproduced by defining a dummy config in e.g. ./tests/Functional/Fixtures/app/config/config.yml:

      - match:
          path: ^/foo
          match_response: 'response.isFresh()'
        headers:
          cache_control: {public: true}

This causes all tests that compile the Symfony container to fail with the same error.

I’d be happy to submit a PR to fix this if the approach sounds correct. One point I’m unsure about is the subsequent lines in FOSHttpCacheExtension, which reference match_response_expression_service and again use replaceArgument. This setting doesn’t seem to be exposed in the semantic configuration - could this have been intended to use 'expression_language' instead, based on the available configuration options?

[dbu]

hi, thanks for the report. i wonder why the phpunit tests don't show this problem - maybe we do not hit the edge case of the argument not being defined in the parent?

the symfony doc uses named arguments in setArgument, but according to the code, the int position is also supported. please use the numeric index, to avoid a regression if somebody extended ResponseMatcher and used a different name for the argument.

and i think you are correct that we messed up with the second argument, not sure what happened. glad if you can fix the name.
and if you have time to add to the extension test to cover these things, it would be highly appreciated (best create the test first to see things fail to be sure we test the actual problem, then do the fixes)

[alongosz]

hi, thanks for the report. i wonder why the phpunit tests don't show this problem - maybe we do not hit the edge case of the argument not being defined in the parent?

It's because none of these tests compile the container. The definition is correct regardless if setArgument or replaceArgument is used. Seems that Symfony 6+ started to interpret those differently.

KernelTestCase-based tests compile the container, but there's no match_response config anywhere in ./tests/Functional/Fixtures/app/config/config.yml.

Created #644 Pull Request.