Remove all marketing get parameters to minimize the cache using built-in FPC

[rogerdz]

Description (*)

Same logic when use varnish (https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/PageCache/etc/varnish7.vcl#L84) but for build-in fpc

Related Pull Requests

Fixed Issues (if relevant)

1. Fixes magento/magento2#<issue_number>

Manual testing scenarios (*)

1. Clear cache: bin/magento cache:clean
2. Run command: curl -I --location-trusted 'https://magento2.test/gear/bags.html?test=111&utm_source=123&gclid=abc' | grep -i X-Magento-Cache-Debug, it return x-magento-cache-debug: MISS => correct
3. Run command: curl -I --location-trusted 'https://magento2.test/gear/bags.html?test=111&utm_source=123&gclid=abc' | grep -i X-Magento-Cache-Debug, it return x-magento-cache-debug: HIT => correct
4. Run command: curl -I --location-trusted 'https://magento2.test/gear/bags.html?test=111' | grep -i X-Magento-Cache-Debug => it should return x-magento-cache-debug: HIT

Questions or comments

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
• All automated tests passed successfully (all builds are green)

Resolved issues:

1. resolves [Issue] Remove all marketing get parameters to minimize the cache #39266: Remove all marketing get parameters to minimize the cache

[m2-assistant]

Hi @rogerdz. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.

Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests
13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

---

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

[rogerdz]

@magento run all tests

[rogerdz]

@magento run all tests

[sprankhub]

Nice. Maybe you want to update the parameters based on https://github.com/magento/magento2/pull/39188/files#diff-40e00b332f8dd95fbf48a312b9b8cfeee0d4e70bd7528c0e794c84c9e00c113fR85.

[engcom-Hotel]

@magento create issue

[engcom-Hotel]

@magento run all tests

[engcom-Hotel]

@magento run Database Compare, Functional Tests B2B, Functional Tests CE

[sprankhub]

Please mind that #39188 is also community-picked and is more complete. Specifically, it includes the changes from this PR. Hence, I think the solution from #39188 should be merged.

[hostep]

@sprankhub, I think you got confused and meant to comment on #35228
This PR here adds the extra parameters to the non-Varnish based FPC solution from Magento. But I believe the list should be updated with the list from #39188 so they match 100%.

Ideally we should have one place where the entire list is defined and can be used both by the Varnish VCL file generator and the non-Varnish based FPC solution. Maybe using a backoffice configuration field? We have a new request for that since today BTW: #39268 (even though the example given there is a very bad idea)

[ihor-sviziev]

If I got right, you’re removing cache parameters on Magento (php) side.
for that purpose, Varnish does way better job, since it works way better and uses resources more efficiently.
I don’t see how someone would use built in full page cache in production, so I don’t think we should update it

Example use case:
you added a new service, let’s say Klaviyo, that adds a dynamic value for each client.
Marketing team has sent an email to all subscribers and they starting clicking on link that goes to a single product page.
What we have right now is - the dynamic get parameter will be removed by Varnish, and after the first request - it will put the result in cache. all next requests will not get to Magento/php - so they will be delivered to clients waaaay faster and won’t create load to web servers, database and redis.
if we accept your changes - this would mean that this dynamic get parameter will be removed on php side, which will result in additional slower responses, additional load to web servers, database and redis.

Having all that in mind, I don’t think we can accept this pull request, but instead, we should actualize the list in Varnish config.

Please correct me if I’m wrong

[ihor-sviziev]

Sorry, looks like I got confused a bit.
But still, it’s not clear if it worth fixing built in full page cache. Does anyone using it in production? If no - why would we support the list in 2 places?

[Copilot]

Copilot wasn't able to review any files in this pull request.

Files not reviewed (4)

• app/code/Magento/PageCache/Model/App/Request/Http/IdentifierForSave.php: Language not supported
• app/code/Magento/PageCache/Test/Unit/Model/App/Request/Http/IdentifierForSaveTest.php: Language not supported
• lib/internal/Magento/Framework/App/PageCache/Identifier.php: Language not supported
• lib/internal/Magento/Framework/App/Test/Unit/PageCache/IdentifierTest.php: Language not supported

[engcom-Hotel]

@magento run Functional Tests CE, Unit Tests

[engcom-Hotel]

Hello @rogerdz @engcom-Dash,

The changes looks good to me. But please address the failed automated tests.

Thanks

[engcom-Dash]

@magento run all tests

[engcom-Bravo]

Hi @rogerdz,

Thanks for your Contribution!!.

We have verified the issue in Latest 2.4-develop Magento Instance and the issue is not reproducible.Kindly refer the screenshots.

The last request also showing Hit.Could you please let us know if we are missing anything.

Thanks.

[ihor-sviziev]

@engcom-Bravo, are you testing on built-in FPC without Varnish?

[engcom-Bravo]

Hi @ihor-sviziev,

Thanks for your update!!.

We have tested with Full Page Cache(built in) without varnish.

Thanks.

[rogerdz]

@engcom-Bravo last request must call url without marketing parameter

[engcom-Bravo]

@magento run all tests

[engcom-Bravo]

Hi @rogerdz,

Thanks for the collaboration & contribution!

✔️ QA Passed

Preconditions:

• Install fresh Magento 2.4-develop

Steps to reproduce

• Clear cache: bin/magento cache:clean
• Run command: curl -I --location-trusted 'https://magento2.test/gear/bags.html?test=111&utm_source=123&gclid=abc' | grep -i X-Magento-Cache-Debug, it return x-magento-cache-debug: MISS => correct
• Run command: curl -I --location-trusted 'https://magento2.test/gear/bags.html?test=111&utm_source=123&gclid=abc' | grep -i X-Magento-Cache-Debug, it return x-magento-cache-debug: HIT => correct
• Run command: curl -I --location-trusted 'https://magento2.test/gear/bags.html?test=111' | grep -i X-Magento-Cache-Debug => it should return x-magento-cache-debug: HIT

Before: ✖️ 

After: ✔️

Builds are failed. Hence, moving this PR to Extended Testing.

Thanks.

[engcom-Dash]

@magento run Integration Tests, Functional Tests CE, Functional Tests B2B

[engcom-Dash]

@magento run Integration Tests, Functional Tests CE, Functional Tests B2B

[engcom-Dash]

The Functional B2B and CE test failures seem flaky and some of these failures are known issues. They are neither part of PR nor failing because of PR changes. Hence moving it to Merge in Progress.

Functional Tests B2B
Run 1: https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/39099/1875e843e30894817894cbc522dd0261/Functional/allure-report-b2b/index.html#categories/3a3a8306986cbb01e5245b23e6e5b238/6e05a95598af89e8/

Run 2: https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/39099/66f491bf0b44bceef57107d51d8cdf55/Functional/allure-report-b2b/index.html#categories/3a3a8306986cbb01e5245b23e6e5b238/5d0fa8a0b7cc1e95/

Known issues:
MultiShippingWithCreationNewCustomerAndAddressesDuringCheckoutTest: ACQE-7229
StorefrontReuseCouponCodeAfterOrderCanceledTest: ACQE-7230

Functional Tests CE
Run 1: https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/39099/977f45eeeb8f1049aa14572e5d9f74fc/Functional/allure-report-ce/index.html#categories/6d7e37752c3cc83cb3d2063131f608bb/30a31853636a9131/

Run 2: https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/39099/5eff451448e049fcb61454043d729cc1/Functional/allure-report-ce/index.html#categories/6d7e37752c3cc83cb3d2063131f608bb/476c47e6a72b24e0/

Known issue:
VerifyDefaultWYSIWYGToolbarOnProductTestCest: ACQE-7227