diff --git a/system/web/services/HandlerService.cfc b/system/web/services/HandlerService.cfc index f4ba338eb..2660a235a 100644 --- a/system/web/services/HandlerService.cfc +++ b/system/web/services/HandlerService.cfc @@ -167,12 +167,7 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" { // method check finalized. // Store metadata in execution bean - if ( !variables.handlerCaching || !arguments.ehBean.isMetadataLoaded() ) { - var md = getMetadata( oEventHandler ) - arguments.ehBean - .setActionMetadata( oEventHandler._actionMetadata( arguments.ehBean.getMethod() ) ) - .setHandlerMetadata( md.keyExists( "annotations" ) ? md.annotations : md ) - } + ensureHandlerMetadata( arguments.ehBean, oEventHandler ) // Are they trying to execute an internal ColdBox method? if ( arguments.ehBean.actionMetadataExists( "cbMethod" ) ) { @@ -189,7 +184,11 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" { arguments.ehBean.getFullEvent() EQ oRequestContext.getCurrentEvent() ) { // Get event action caching metadata - var eventDictionaryEntry = getEventCachingMetadata( arguments.ehBean, oEventHandler ); + var eventDictionaryEntry = getEventCachingMetadata( + arguments.ehBean, + oEventHandler, + oRequestContext + ); // Do we need to cache this event's output after it executes?? if ( eventDictionaryEntry.cacheable ) { @@ -570,14 +569,29 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" { /** * Get an event string's metadata entry. If not found, then you will get a new metadata entry using the `getNewMDEntry()` method. * - * @targetEvent The event to match for metadata. + * @targetEvent The event to match for metadata. + * @requestContext The request context for the current request, passed through to a closure suffix untouched. */ - struct function getEventMetadataEntry( required targetEvent ){ + struct function getEventMetadataEntry( required targetEvent, required requestContext ){ if ( NOT structKeyExists( variables.eventCacheDictionary, arguments.targetEvent ) ) { return getNewMDEntry() } - return variables.eventCacheDictionary[ arguments.targetEvent ] + var mdEntry = variables.eventCacheDictionary[ arguments.targetEvent ] + + // Fast path: a static suffix needs no per-request work, hand back the memoized entry + if ( isSimpleValue( mdEntry.suffix ) ) { + return mdEntry + } + + // Closure suffix: resolve it for THIS request. getHandlerBean() gives back a bean whose + // action/handler metadata is only guaranteed loaded when handlerCaching lets it reuse a + // previously-executed instance - ensureHandlerMetadata() closes that gap so a closure + // reading eventHandlerBean.getActionMetadata(...) sees the same data the store-side + // getEventCachingMetadata() call already guarantees, regardless of that setting. + var bean = getHandlerBean( arguments.targetEvent ) + ensureHandlerMetadata( bean ) + return resolveCacheSuffix( mdEntry, bean, arguments.requestContext ) } /** @@ -784,12 +798,17 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" { /** * Return the event caching metadata for an action execution context. * - * @ehBean The event handler bean - * @oEventHandler The event handler to execute + * @ehBean The event handler bean + * @oEventHandler The event handler to execute + * @requestContext The request context for the current request, passed through to a closure suffix untouched. * * @return strc */ - private struct function getEventCachingMetadata( required ehBean, required oEventHandler ){ + private struct function getEventCachingMetadata( + required ehBean, + required oEventHandler, + required requestContext + ){ var cacheKey = arguments.ehBean.getFullEvent(); // Double lock for race conditions @@ -828,15 +847,12 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" { mdEntry.lastModified = arguments.ehBean.getActionMetadata( "lastModified", false ); mdEntry.cacheControl = arguments.ehBean.getActionMetadata( "cacheControl", "" ); - // Handler Event Cache Key Suffix, this is global to the event - if ( - isClosure( arguments.oEventHandler.EVENT_CACHE_SUFFIX ) || - isCustomFunction( arguments.oEventHandler.EVENT_CACHE_SUFFIX ) - ) { - mdEntry.suffix = oEventHandler.EVENT_CACHE_SUFFIX( arguments.ehBean ); - } else { - mdEntry.suffix = arguments.oEventHandler.EVENT_CACHE_SUFFIX; - } + // Handler Event Cache Key Suffix, this is global to the event. + // Stored AS DECLARED: a closure must NOT be evaluated here because this + // entry is memoized for the life of the app, and a request-time value + // (locale, session, slug) would freeze into every later request's cache + // key. resolveCacheSuffix() evaluates it on every read instead. + mdEntry.suffix = arguments.oEventHandler.EVENT_CACHE_SUFFIX; // if the cacheFilter has a length and is a method, then we need to verify and store the resulting closure if ( len( mdEntry.cacheFilter ) ) { @@ -879,7 +895,75 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" { } // end if - return variables.eventCacheDictionary[ cacheKey ]; + return resolveCacheSuffix( + variables.eventCacheDictionary[ cacheKey ], + arguments.ehBean, + arguments.requestContext + ); + } + + /** + * Ensure an event handler bean has its action/handler metadata loaded, building and + * reflecting a handler instance only if it doesn't already have one to reuse. + * + * Factored out of getHandler() so the event-caching suffix lookup path + * (getEventMetadataEntry()) can guarantee the same metadata is present on the bean it hands + * to an EVENT_CACHE_SUFFIX closure, whether or not handlerCaching lets getHandlerBean() reuse + * a bean instance that a prior getHandler() call already populated. + * + * @ehBean The event handler bean to load metadata onto + * @oEventHandler An already-built handler instance to reflect, if the caller has one; built via newHandler() otherwise + */ + private function ensureHandlerMetadata( required ehBean, oEventHandler ){ + if ( arguments.ehBean.isMetadataLoaded() ) { + return arguments.ehBean; + } + + var handler = isNull( arguments.oEventHandler ) ? newHandler( arguments.ehBean ) : arguments.oEventHandler; + var md = getMetadata( handler ); + + arguments.ehBean + .setActionMetadata( handler._actionMetadata( arguments.ehBean.getMethod() ) ) + .setHandlerMetadata( md.keyExists( "annotations" ) ? md.annotations : md ); + + return arguments.ehBean; + } + + /** + * Resolve a metadata entry's cache-key suffix for the CURRENT request. + * + * A static string suffix passes the entry through untouched. A closure suffix is + * evaluated now, on a shallow COPY of the entry - the memoized entry keeps the + * closure so every request re-evaluates it (locale, session, slug use cases). + * + * The closure receives ( eventHandlerBean, event ) and runs twice per request + * (serve-side key lookup + store-side key build), so it must be deterministic + * within a request: read request-stable inputs only, never time or randomness, + * and mutate nothing - the same contract the hashed rc and the stored + * cacheFilter closure already have. + * + * @mdEntry The memoized event caching metadata entry + * @ehBean The event handler bean, passed to the closure as its first argument + * @requestContext The request context for the current request, passed to the closure as its second argument + */ + private struct function resolveCacheSuffix( + required struct mdEntry, + required ehBean, + required requestContext + ){ + // We check for isClosure and isCustomFunction for ACF/Lucee/BoxLang compatibility + if ( + !isClosure( arguments.mdEntry.suffix ) && + !isCustomFunction( arguments.mdEntry.suffix ) + ) { + return arguments.mdEntry; + } + + var resolved = structCopy( arguments.mdEntry ); + var suffixUDF = arguments.mdEntry.suffix; + resolved.suffix = suffixUDF( arguments.ehBean, arguments.requestContext ); + + return resolved; } } diff --git a/system/web/services/RequestService.cfc b/system/web/services/RequestService.cfc index d8037a0dc..ae8284278 100644 --- a/system/web/services/RequestService.cfc +++ b/system/web/services/RequestService.cfc @@ -156,7 +156,7 @@ component extends="coldbox.system.web.services.BaseService" { arguments.context.removeEventCacheableEntry() // Get metadata entry for event that's fired. - var eventDictionary = variables.handlerService.getEventMetaDataEntry( currentEvent ) + var eventDictionary = variables.handlerService.getEventMetaDataEntry( currentEvent, arguments.context ) // Verify that it is cacheable, else quit, no need for testing anymore. if ( NOT eventDictionary.cacheable ) { diff --git a/test-harness/handlers/eventcachingSuffix.cfc b/test-harness/handlers/eventcachingSuffix.cfc new file mode 100644 index 000000000..0efcaf842 --- /dev/null +++ b/test-harness/handlers/eventcachingSuffix.cfc @@ -0,0 +1,30 @@ +/** + * Fixture for EVENT_CACHE_SUFFIX closure specs. It lives apart from `eventcaching.cfc` + * because the suffix is handler-global and would change the cache keys of every spec + * that uses that handler. + */ +component output="false" { + + // Evaluated per request: the suffix carries the incoming slug (proves per-request + // re-evaluation) and a custom action annotation read off the bean (proves the bean it + // receives has its action metadata loaded - reading it falls back to "missing" instead + // of "present" if metadata isn't loaded, which is silent otherwise). + this.EVENT_CACHE_SUFFIX = function( eventHandlerBean, event ){ + return arguments.event.getValue( "slug", "none" ) & "-" & arguments.eventHandlerBean.getActionMetadata( + "suffixTag", + "missing" + ) + } + + // cacheInclude="" keeps the rc hash constant, so only the suffix varies the cache key + function index( event, rc, prc ) + cache ="true" + cacheTimeout="10" + cacheInclude="" + suffixTag ="present" + { + prc.data = { when : now() } + return prc.data + } + +} diff --git a/tests/specs/integration/EventCachingSpec.cfc b/tests/specs/integration/EventCachingSpec.cfc index 50a80d2d6..057ec0b6b 100755 --- a/tests/specs/integration/EventCachingSpec.cfc +++ b/tests/specs/integration/EventCachingSpec.cfc @@ -495,6 +495,90 @@ expect( data2 ).notToBe( data ); } ); } ); + + describe( "EVENT_CACHE_SUFFIX", function(){ + it( "evaluates a closure suffix on every request producing distinct cache keys", function(){ + getRequestContext().setValue( "slug", "alpha" ) + var event1 = execute( event = "eventcachingSuffix.index", renderResults = true ) + var key1 = event1.getPrivateCollection().cbox_eventCacheableEntry.cacheKey + + expect( key1 ).toInclude( "alpha-present" ) + + // reset to simulate another request with a different slug + setup() + getRequestContext().setValue( "slug", "beta" ) + var event2 = execute( event = "eventcachingSuffix.index", renderResults = true ) + var key2 = event2.getPrivateCollection().cbox_eventCacheableEntry.cacheKey + + // the closure must re-evaluate per request, not freeze on the first request's value + expect( key2 ).toInclude( "beta-present" ) + expect( key2 ).notToBe( key1 ) + } ); + + it( "produces the same key on the serve-side lookup and the store-side build", function(){ + getRequestContext().setValue( "slug", "gamma" ) + var event = execute( event = "eventcachingSuffix.index", renderResults = true ) + var storeKey = event.getPrivateCollection().cbox_eventCacheableEntry.cacheKey + + // re-run the real serve-side path: getEventMetadataEntry() -> buildEventKey() + controller.getRequestService().eventCachingTest( event ) + var serveKey = event.getPrivateCollection().cbox_eventCacheableEntry.cacheKey + + // if lookup and storage keys disagree, cached responses are never served + expect( serveKey ).toBe( storeKey ) + } ); + + it( "keeps the serve-side and store-side keys in sync even with handlerCaching off", function(){ + var handlerService = controller.getHandlerService() + handlerService.setHandlerCaching( false ) + + try { + getRequestContext().setValue( "slug", "delta" ) + var event = execute( event = "eventcachingSuffix.index", renderResults = true ) + var storeKey = event.getPrivateCollection().cbox_eventCacheableEntry.cacheKey + + controller.getRequestService().eventCachingTest( event ) + var serveKey = event.getPrivateCollection().cbox_eventCacheableEntry.cacheKey + + // Both keys must resolve the "present" tag, not just agree with each other - + // otherwise a bean with unloaded action metadata on BOTH sides would still + // produce two equal-but-wrong ("delta-missing") keys and this test would miss it. + expect( storeKey ).toInclude( "delta-present" ) + expect( serveKey ).toBe( storeKey ) + } finally { + handlerService.setHandlerCaching( true ) + } + } ); + + it( "leaves static string suffixes untouched when resolving", function(){ + var handlerService = controller.getHandlerService() + makePublic( handlerService, "resolveCacheSuffix" ) + + var mdEntry = { "cacheable" : true, "suffix" : "static" } + var resolved = handlerService.resolveCacheSuffix( + mdEntry, + handlerService.getHandlerBean( "eventcachingSuffix.index" ), + getRequestContext() + ) + + expect( isSimpleValue( resolved.suffix ) ).toBeTrue() + expect( resolved.suffix ).toBe( "static" ) + } ); + + it( "keeps the closure in the memoized dictionary entry after requests", function(){ + getRequestContext().setValue( "slug", "epsilon" ) + execute( event = "eventcachingSuffix.index", renderResults = true ) + + var dictionary = prepareMock( controller.getHandlerService() ).$getProperty( + "eventCacheDictionary", + "variables" + ) + var suffix = dictionary[ "eventcachingSuffix.index" ].suffix + + // the dictionary must keep the closure so later requests can re-evaluate it + expect( isClosure( suffix ) || isCustomFunction( suffix ) ).toBeTrue() + } ); + } ); } ); }