Skip to content

Preserve expiry dates from nested cache collectors - #19507

Closed
timkelty wants to merge 1 commit into
5.xfrom
t3code/fix-nested-cache-expiry
Closed

Preserve expiry dates from nested cache collectors#19507
timkelty wants to merge 1 commit into
5.xfrom
t3code/fix-nested-cache-expiry

Conversation

@timkelty

Copy link
Copy Markdown
Contributor

Nested element-cache collectors could discard their expiry when the parent collector did not have one yet. Preserve the nested duration so outer caches cannot outlive expirable content.

Discovered while reviewing #19505.

Copilot AI lite review requested due to automatic review settings August 28, 2026 16:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes an edge case in craft\services\Elements cache-info collection where a nested cache collector’s computed expiry duration could be lost when unwinding back to an outer collector that hadn’t set a duration yet—allowing outer caches to potentially outlive expirable nested content.

Changes:

  • Preserve a nested collector’s $duration when the parent collector’s duration is currently null.
  • Add a unit test covering nested cache-info collection where only the inner collector sets an expiry date.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/services/Elements.php Ensures nested cache duration is propagated to the parent collector when the parent has no duration yet.
tests/unit/services/ElementsTest.php Adds regression coverage asserting nested cache expiry duration survives unwinding to the outer collector.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@timkelty timkelty closed this Aug 28, 2026
@timkelty
timkelty deleted the t3code/fix-nested-cache-expiry branch August 28, 2026 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants