Hotfix/cache issue fix - #675
Open
homestar9 wants to merge 2 commits into
Open
Conversation
… first request The docs say EVENT_CACHE_SUFFIX can be a closure evaluated at runtime, but HandlerService.getEventCachingMetadata() evaluates it once and memoizes the result for the life of the app. Two of these specs fail on current code: - 'evaluates a closure suffix on every request producing distinct cache keys' The needle [-beta-] was not found in [cbox_event-eventcachingsuffix.index-alpha-FBEE7A3D7291A76924037D5B7F2AE52E] - 'keeps the closure in the memoized dictionary entry after requests' Expected [false] to be true
…the first value
The memoized eventCacheDictionary entry now stores the closure as declared.
Both read points resolve it for the current request on a shallow copy via the
new private resolveCacheSuffix():
- Store side: getEventCachingMetadata() resolves with the in-flight handler bean.
- Serve side: getEventMetadataEntry() resolves via getHandlerBean(), with an
isSimpleValue() fast path so static suffixes skip the work.
The closure now also receives the request context as a second argument:
this.EVENT_CACHE_SUFFIX = function( eventHandlerBean, event ){ ... }
Existing single-argument closures keep working.
The closure runs twice per request (serve-side lookup + store-side key build),
so it must read request-stable inputs (rc, session, locale) and never time or
randomness - the same contract the hashed rc already has.
All EventCachingSpec specs pass, including the two that failed before this
change; full integration suite green (86/86 on BoxLang).
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a bug in ColdBox event caching where a dynamic EVENT_CACHE_SUFFIX closure could be evaluated once and then effectively “frozen” into the memoized metadata entry, causing different requests (e.g., varying slug/locale) to share the same cache key and potentially serve incorrect cached content.
Changes:
- Update
HandlerServiceto storeEVENT_CACHE_SUFFIX“as declared” (closure or static value) and resolve closure-based suffixes per request via a newresolveCacheSuffix()path. - Add integration specs that reproduce the original issue and verify per-request suffix evaluation and key consistency between serve-side lookup and store-side key building.
- Introduce a dedicated test-harness handler fixture (
eventcachingSuffix) to isolate suffix behavior from other event caching specs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/specs/integration/EventCachingSpec.cfc | Adds integration tests covering per-request evaluation and key consistency for EVENT_CACHE_SUFFIX. |
| test-harness/handlers/eventcachingSuffix.cfc | Adds a dedicated handler fixture whose cache key varies only by suffix derived from request context. |
| system/web/services/HandlerService.cfc | Changes event cache metadata handling to avoid freezing request-time suffix values; introduces per-request suffix resolution helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes a bug where a dynamic EVENT_CACHE_SUFFIX closure was only evaluated once and then reused for every request. This could cause different requests, such as pages with different slugs or locales, to share the same cache key and return the wrong cached content.
The closure is now evaluated for each request (on cache misses) and receives the current request context, while existing single-argument closures and static suffixes continue to work as before. New tests reproduce the original bug and confirm that each request now uses the correct cache entry.
Jira Issues
https://ortussolutions.atlassian.net/browse/COLDBOX-1411
Type of change
Checklist