wp-rocket icon indicating copy to clipboard operation
wp-rocket copied to clipboard

Fix #7029: ALR hashes are added to source when DONOTROCKETOPTIMIZE is set to true

Open Miraeld opened this issue 5 months ago • 3 comments

Description

Fixes #7029 When DONOTROCKETOPTIMIZE is set to true, WP Rocket should completely disable all optimization features. However, Performance Hints (LRC/LCP) hashes were still being added to pages and warmup processes were still running, which could impact site performance and user experience.

This fix ensures that when DONOTROCKETOPTIMIZE is enabled:

  • No Performance Hints hashes are added to pages
  • No warmup processes are triggered for LRC/LCP
  • The beacon injection remains disabled (as previously implemented in #7022)

Type of change

  • [x] Bug fix (non-breaking change which fixes an issue).

Detailed scenario

What was tested

Automated Tests:

  • Unit tests for LazyRenderContent Context is_allowed() method with DONOTROCKETOPTIMIZE scenarios
  • Integration tests for WarmUp Controller methods (warm_up() and warm_up_home()) with DONOTROCKETOPTIMIZE scenarios
  • All existing Performance Hints tests continue to pass

Manual Scenarios:

  • Before fix: With define('DONOTROCKETOPTIMIZE', true); in wp-config.php, visiting pages still showed Performance Hints hashes in the HTML
  • After fix: With define('DONOTROCKETOPTIMIZE', true); in wp-config.php, no Performance Hints hashes are added to pages
  • Warmup behavior: No warmup jobs are queued when DONOTROCKETOPTIMIZE is enabled

How to test

Test Steps:

  • Add to wp-config.php: define('DONOTROCKETOPTIMIZE', true);
  • Visit any page in incognito mode
  • Check page source - no Performance Hints hashes should be present
  • Check that no warmup jobs are triggered in the background

Technical description

Documentation

This pull request introduces a new condition to respect the DONOTROCKETOPTIMIZE constant, ensuring that certain optimizations are bypassed when this constant is set. Corresponding changes include updates to logic in the codebase and the addition of test cases to validate this behavior.

Core Logic Updates:

  • Added a check for the DONOTROCKETOPTIMIZE constant in the is_allowed method of WarmUp\Controller to prevent optimizations when the constant is set. (inc/Engine/Common/PerformanceHints/WarmUp/Controller.php, inc/Engine/Common/PerformanceHints/WarmUp/Controller.phpR70)
  • Updated the is_allowed method in LazyRenderContent\Context to include a similar check for the DONOTROCKETOPTIMIZE constant. (inc/Engine/Optimization/LazyRenderContent/Context/Context.php, inc/Engine/Optimization/LazyRenderContent/Context/Context.phpR20-R23)

Test Enhancements:

  • Added new test cases to verify the behavior when DONOTROCKETOPTIMIZE is set, ensuring optimizations are skipped in WarmUp and LazyRenderContent contexts. (tests/Fixtures/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUp.php, [1]; tests/Fixtures/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php, [2]; tests/Fixtures/inc/Engine/Optimization/LazyRenderContent/Context/Context/isAllowed.php, [3]
  • Updated integration tests to mock the DONOTROCKETOPTIMIZE constant and validate its impact on behavior. (tests/Integration/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUp.php, [1]; tests/Integration/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php, [2]
  • Adjusted unit tests to include the DONOTROCKETOPTIMIZE constant in the configuration and verify its influence on the isAllowed logic. (tests/Unit/inc/Engine/Optimization/LazyRenderContent/Context/Context/isAllowed.php, tests/Unit/inc/Engine/Optimization/LazyRenderContent/Context/Context/isAllowed.phpR27)

New dependencies

None

Risks

None

Mandatory Checklist

Code validation

  • [x] I validated all the Acceptance Criteria. If possible, provide screenshots or videos.
  • [x] I triggered all changed lines of code at least once without new errors/warnings/notices.
  • [x] I implemented built-in tests to cover the new/changed code.

Code style

  • [x] I wrote a self-explanatory code about what it does.
  • [x] I protected entry points against unexpected inputs.
  • [x] I did not introduce unnecessary complexity.
  • [x] Output messages (errors, notices, logs) are explicit enough for users to understand the issue and are actionnable.

Miraeld avatar Jul 18 '25 00:07 Miraeld

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
Report missing for 883963da4df2fe50ff1cbcf9809e47bae7650561[^1] :white_check_mark: 100.00% (target: 50.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (883963da4df2fe50ff1cbcf9809e47bae7650561) Report Missing Report Missing Report Missing
Head commit (346b61146e1922661ce72510a012c3972b8d0e57) 41884 18891 45.10%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#7515) 5 5 100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

[^1]: Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.

codacy-production[bot] avatar Jul 18 '25 02:07 codacy-production[bot]

When we visit any page that can't be cached (which is the case with sending the warmup request) we set the constant DONOTROCKETOPTIMIZE to be true here: https://github.com/wp-media/wp-rocket/blob/8b0e79150b6b32c9cd23883a6077212e9d943fba/inc/classes/Buffer/class-cache.php#L82-L86

So this condition will fail here: https://github.com/wp-media/wp-rocket/blob/ba4d3e885c4aa520752f99eb5a3e66c9e38095fb/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php#L83-L85

wordpressfan avatar Aug 05 '25 06:08 wordpressfan

Fixed Circular Logic Issue

Thanks to @wordpressfan for identifying the problem! I've updated the PR to fix the circular logic issue.

What Changed

Removed:

  • DONOTROCKETOPTIMIZE check from WarmUp Controller
  • ❌ Related test scenarios for WarmUp

Kept (these are correct):

  • DONOTROCKETOPTIMIZE check in LazyRenderContent Context
  • DONOTROCKETOPTIMIZE check in LazyRenderContent Frontend Controller
  • ✅ All LazyRenderContent tests

Why This Is Correct

The Original Issue (#7029) - STILL FIXED: When a developer adds define('DONOTROCKETOPTIMIZE', true); to wp-config.php, Performance Hints hashes are now properly prevented from being added to pages. This is handled by the LazyRenderContent checks.

The Circular Logic Problem - NOW FIXED: WP Rocket internally sets DONOTROCKETOPTIMIZE during warmup requests (in Buffer/Cache.php lines 82-86). If we checked this constant in the WarmUp Controller, it would:

  1. Try to run warmup
  2. WP Rocket sets DONOTROCKETOPTIMIZE during the warmup request
  3. WarmUp sees the constant and stops
  4. Warmup never completes ❌

The Solution:

  • WarmUp should NOT check DONOTROCKETOPTIMIZE (it's an internal process)
  • LazyRenderContent SHOULD check DONOTROCKETOPTIMIZE (it handles user-facing pages)

Files Changed in Latest Commit

inc/Engine/Common/PerformanceHints/WarmUp/Controller.php        (removed check)
tests/Fixtures/.../WarmUp/Subscriber/warmUp.php                 (removed test scenario)
tests/Fixtures/.../WarmUp/Subscriber/warmUpHome.php             (removed test scenario)
tests/Integration/.../WarmUp/Subscriber/warmUp.php              (removed mocking)
tests/Integration/.../WarmUp/Subscriber/warmUpHome.php          (removed mocking)

Ready for re-review! 🚀

Miraeld avatar Dec 11 '25 02:12 Miraeld

Thank you @Miraeld for this PR.

When testing the warmup of priority elements with DONOTROCKETOPTIMIZE set to true in wp-config.php, URLs are still being fetched on a fresh install and when permalinks are changed.

Steps to reproduce:

  1. Set DONOTROCKETOPTIMIZE in wp-config.php
  2. Fresh install and activate the PR zip
  3. Check the LCP, LRC tables

Current behavior: URLs are fetched and added to the priority elements tables.

hanna-meda avatar Dec 16 '25 07:12 hanna-meda

@Miraeld, checking after latest changes on PR, now the reverse has happened, no URLs are ever fetched/warmed-up:

  1. DONOTROCKETOPTIMIZE NOT in the wp-config.php file.
  2. Fresh install / change permalink / change theme.
  3. Check LCP, LRC tables. Optionally, if WPR debug on, also search inside wp-rocket-debug.log.html file for "Add to queue request arguments" logs.

Current behavior: No URLs are fetched. Inside the "wp-rocket-debug.log.html" file there are only 2 logs of home URL.

hanna-meda avatar Dec 18 '25 17:12 hanna-meda