Problem/Motivation
The max_age_zero pattern of PerformanceAnalyzer::CACHE_PROBLEM_PATTERNS requires 'max-age' => 0 to come right after the opening bracket of the '#cache' array. Reproduced on current 1.x with drush audit:run performance:
$a = ['#cache' => ['max-age' => 0]]; => CACHE_MAX_AGE_ZERO
$b = ['#cache' => ['contexts' => ['user'], 'max-age' => 0]]; => missed
$c = [
'#cache' => [
'tags' => ['node_list'],
'max-age' => 0, => missed
],
];
Declaring contexts or tags before max-age is common, so the rule misses a large share of the cases it is meant to find.
Proposed resolution
Match 'max-age' => 0 anywhere inside the '#cache' array, also after keys whose values are arrays one level deep (such as 'contexts' => ['user']) and across several lines, without leaving that array: a 'max-age' => 0 in a sibling render array key is not attributed to '#cache'. The alternatives of the new pattern cannot overlap, so it does not backtrack excessively. A test covers the three cases above, a non-zero max-age and a max-age 0 outside '#cache'; it fails without the fix.
Sites will see more CACHE_MAX_AGE_ZERO findings after this change; they are real cases that were previously missed.
The original report also proposed downgrading CACHE_MAX_AGE_ZERO to a notice when every route of a controller is an admin route. That is not needed any more: the admin report case is now covered by the inline audit-ignore marker (#3626576), e.g. "// audit-ignore: CACHE_MAX_AGE_ZERO -- admin report backed by a log table", and by the rule overrides setting (#3626580), e.g. "CACHE_MAX_AGE_ZERO */src/Controller/Admin* notice". Both are explicit and reviewable, whereas guessing from routing.yml files would add a heuristic that can be wrong both ways.
Remaining tasks
- Review the merge request.
Comments
Comment #3
trebormc