[COLDBOX-1331] Add ability to ignore or include RC keys when caching events

[homestar9]

@lmajano
This pull request is in response to this discussion in the Coldbox community and is a revival of the original PR here: #503

This new feature for Coldbox gives developers more flexibility with event caching and different RC scopes. The end result should be more efficient caching and improved page load speed for many users.

The Problem:
Currently, event caching considers the entire RC scope when generating a cache key. This means that changing any URL or FORM variable on a request will force Coldbox to generate a new cache entry. This behavior is problematic for a few reasons:

A malicious attacker could take advantage and create an unlimited number of cache entries for the same page simply by adding a random URL variable.
Marketing efforts often require appending a unique tracking variable to a URL (e.g. utm_source). In this type of scenario, there is no need to create a new cached event for every unique URL.

The Solution
I was inspired by Wirebox's populator include/exclude arguments which lets developers specify a list of keys to consider when populating an entity. My idea was to create a new metadata property called cacheIncludeRcKeys which would allow specifying a list of RC keys to include when generating a cache key. All keys not in the list are ignored. To maintain backward compatibility, the default value for cacheIncludeRcKeys is * which considers all keys.

How Does it Work?
Simply include a new metadata attribute cacheIncludeRcKeys to a handler and specify a list of RC keys you want to be included in the cache key generation process. Here are some examples:

function show( event, rc, prc ) cache="true" cacheTimeout="10" cacheIncludeRcKeys="slug" {
  ...
}

The above code will ignore all RC keys except for slug when generating a cache key. So, if you had a route pages/:slug resolve to pages.show, the following URLs would all utilize the same cache key:
https://mysite.com/pages/foo
https://mysite.com/pages/foo?utm_source=google
https://mysite.com/pages/foo?utm_source=bing

You can also include multiple RC keys by specifying a list in the metadata attribute like this:

function show( event, rc, prc ) cache="true" cacheTimeout="10" cacheIncludeRcKeys="slug,id" {
  ...
}

[homestar9]

@lmajano I see some tests failed, but I do not think they are related to my PR.

[homestar9]

Hi @lmajano, I just wanted to ensure you saw this PR you requested me to create again. This would be a really powerful addition to Coldbox and help many users who use event caching, but also rely on marketing parameters like UTM variables, which can pollute the RC scope and bypass event caching.

[jclausen]

I think this is a really good addition. The name cacheIncludeRcKeys is a little awkward, but until we nest the event config, a more descriptive name is probably better. I might suggest eventCacheInclude and then support a syntax like. eventCacheInclude = "rc.*,prc.authUserId" ( defaulted to rc.* ). You wouldn't have to use isDefined. You could just check known contexts, split off the prefix and then filter on those keys. A bit more code, but more powerful.

The reason for the abstraction is that, authentication info might be retained in the prc rather than exposed in the rc, and it would be handy to be able to handle granular caching needs that are behind authentication.

In the function arguments, though, I think you can use shorter argument names, because the context is already clear.

I would also suggest adding handling for this at the function annotation level:

function myAction( event, rc, prc ) eventCacheInclude="rc.*,prc.authUserId"{
...
}

[homestar9]

@jclausen Great idea! I like the idea of 1) simplifying the meta annotation and 2) being able to use the prc scope when generating cache keys.

As for naming, I think we could call the annotation cacheInclude for maximum simplicity. The end result might look like this:

function show( event, rc, prc ) cache="true" cacheTimeout="10" cacheInclude="rc.slug,rc.id,prc.userId" {

Is that in line with what you were thinking? If my assumptions are correct, it sounds like we need to make some changes to the getUniqueHash() in the EventUrlFacade.cfc file. I'm just mocking changes here, so let me know if you have any feedback:

string function getUniqueHash( required event, string cacheInclude="rc.*" ){

	// Build two arrays of keys to include: rcKeys and prcKeys
	var rcKeys  = [];
	var prcKeys = [];
	for ( var a=1; a <= listLen(cacheInclude); a++ ) {
		var part = trim( listGetAt( cacheInclude, a ) );
		var dots = listToArray( part,"." );
		if ( dots.len() == 2 ) {
			var scope = dots[ 1 ];
			var key   = dots[ 2 ];
			if ( scope == "rc" ) {
				arrayAppend( rcKeys, key );
			}
			else if ( scope == "prc" ) {
				arrayAppend( prcKeys, key );
			}
		}
	}
	
	// Isolate keys in the rc collection that match our expected keys
	var rcTarget = arguments.event.getCollection().filter( function( key, value ){
		return (
			key != "event" && // Remove event, not needed for hashing purposes
			(
				cacheIncludeRcKeys == "*" || // include all keys if *
				arrayFindNoCase( rcKeys, key ) // or if the key is specified
			)
		);
	} );
	
	// now do the same thing for the prc
	var prcTarget = arguments.event.getPrivateCollection.filter( function( key, value ){
		return (
			key != "event" && // Remove event, not needed for hashing purposes
			(
				cacheIncludeRcKeys == "*" || // include all keys if *
				arrayFindNoCase( prcKeys, key ) // or if the key is specified
			)
		);
	} );

	// systemOutput( "=====> uniquehash-rcTarget: #variables.jTreeMap.init( rcTarget ).toString()#", true );
	// systemOutput( "=====> uniquehash-rcTargetHash: #hash( variables.jTreeMap.init( rcTarget ).toString() )#", true );

	var targetMixer = {
		// Get the original incoming context hash
		"incomingHash" : hash( variables.jTreeMap.init( rcTarget ).toString() ) & 
			hash( variables.jTreeMap.init( prcTarget ).toString(),
		// Multi-Host support
		"cgihost"      : buildAppLink()
	};	
	// ...

Edit: I might want to check rcKeys and prcKeys for a length before filtering for performance reasons.

[homestar9]

I would be curious to hear @lmajano's thoughts on this item as well. I'm happy to make the edits to the PR once I get feedback.

[homestar9]

@jclausen can you think of a faster way to isolate the rc and prc keys? Performance is paramount, as all this will occur on every request.

// Build two arrays of keys to include: rcKeys and prcKeys
var rcKeys  = [];
var prcKeys = [];
for ( var a=1; a <= listLen(cacheInclude); a++ ) {
	var part = trim( listGetAt( cacheInclude, a ) );
	var dots = listToArray( part,"." );
	if ( dots.len() == 2 ) {
		var scope = dots[ 1 ];
		var key   = dots[ 2 ];
		if ( scope == "rc" ) {
			arrayAppend( rcKeys, key );
		}
		else if ( scope == "prc" ) {
			arrayAppend( prcKeys, key );
		}
	}
}

[lmajano]

Ok, I have reviewed it all. Here are my comments and suggestions so we can incorporate.

1. I do prefer cacheInclude as we have the cacheXXXX precedent for the annotations. I do like that the default is *.
2. I prefer if these are rc only, because that's what affects the caching hash. If you want to use prc which the developer controls, then you can use the https://coldbox.ortusbooks.com/the-basics/event-handlers/event-caching#onrequestcapture-influence-cache-keys influencer to accomplish this, which has been the standard way of incorporating custom data into the hash.
3. Since we do include, we need the opposite. So add cacheExclude as well.
4. If we want deeper control, we should incorporate a cacheFilter which maps to a UDF on the handler that must return the closure/lambda to process the keys. This closure will be stored in the md entry for the handler, so we can retrieve and execute in isolation.

Examples

// Default, it includes *
function show( event, rc, prc ) cache="true" cacheTimeout="10"{
}

// Simple include - only use these RC keys for caching
function show( event, rc, prc ) cache="true" cacheTimeout="10" cacheInclude="id,category,status" {
}

// Simple exclude - use all RC keys except these
function list( event, rc, prc ) cache="true" cacheTimeout="10" cacheExclude="timestamp,csrf,debug" {
}

// Custom filter function for complex logic
function search( event, rc, prc ) cache="true" cacheTimeout="10" cacheFilter="getSearchCacheKeys" {
}

private function getProductCacheKeys() {
    // Can capture handler state or configuration in the closure
    var validKeys = this.getCacheableKeys(); // some handler method
    
    return ( rc ) => {
        return rc.filter( ( key, value ) => arrayFindNoCase( validKeys, key ) > 0 );
    };
}

Implementation Details

I think at this time, it's easier to pass in the eventDictionaryEntry because it has so much good data that event caching requires. So I would say, that the following method buildEventKey() which is called from the request service, needs to be updated to take in another argument: eventDictionary which will be processed internally by the EventURLFacade

/**
 * Build an event key according to passed in params
 *
 * @targetEvent     The targeted ColdBox event executed
 * @targetContext   The targeted request context object
 * @eventDictionary The event metadata containing cache annotations
 */
string function buildEventKey(
    required targetEvent,
    required targetContext,
    required struct eventDictionary
){
    return buildBasicCacheKey( 
        keySuffix   = arguments.eventDictionary.suffix,
        targetEvent = arguments.targetEvent 
    ) & getUniqueHash( arguments.targetContext, arguments.eventDictionary );
}

This way, the dictionary has all the metadata needed for the cache request.

// Build the event cache key according to incoming request
eventCache[ "cacheKey" ] = oEventURLFacade.buildEventKey(
    targetEvent   = currentEvent,
    targetContext = arguments.context,
    eventDictionary = eventDictionary
);

The suffix is inside the dictionary so we can remove that as well and just encapsulate it into the dictionary.

So we can have ultimately something like this

/**
 * Build a unique hash from an incoming request context
 *
 * @event           A request context object
 * @eventDictionary The event metadata containing cache annotations
 */
string function getUniqueHash( required event, required struct eventDictionary ){
    var rcTarget = arguments.event.getCollection();

    // Remove "event" key if it exists
    rcTarget.delete( "event", false );
    
    // Apply cache key filtering based on annotations
    
    // Use custom filter closure stored in dictionary
    if ( isClosure( arguments.eventDictionary?.cacheFilter ) ) {
        rcTarget = arguments.eventDictionary.cacheFilter( rcTarget );
    } 
    // Cache Includes
    else if ( len( arguments.eventDictionary?.cacheInclude ) ) {
        // Whitelist specific keys
        var includeKeys = arguments.eventDictionary.cacheInclude.listToArray();
        rcTarget = rcTarget.filter( ( key, value ) => {
            return includeKeys.findNoCase( key ) > 0;
        });
    } 
    // Cache Excludes
    else if ( len( arguments.eventDictionary?.cacheExclude ) ) {
        // Blacklist specific keys
        var excludeKeys = arguments.eventDictionary.cacheExclude.listToArray();
        rcTarget = rcTarget.filter( ( key, value ) => {
            return excludeKeys.findNoCase( key ) == 0;
        });
    }

    var targetMixer = {
        // Get the original incoming context hash
        "incomingHash" : hash( variables.jTreeMap.init( rcTarget ).toString() ),
        // Multi-Host support
        "cgihost"      : buildAppLink()
    };

    // Incorporate Routed Structs
    targetMixer.append( arguments.event.getRoutedStruct(), true );

    // Return unique identifier
    return hash( targetmixer.toString() );
}

[homestar9]

@lmajano This update adds support for more granular control over cache key generation via event metadata. Based on your feedback, the following enhancements are now supported in the EventURLFacade:

cacheFilter: When specified, the framework will look for a private method in the handler matching the given name. This method receives a struct, rcTarget, containing request collection data, which can be modified as needed before the cache key is generated.

cacheInclude (defaults to *): Limits the cache key to only the specified keys in the request collection.

cacheExclude: Excludes the specified keys from being used in the cache key.

These filters are not mutually exclusive, giving developers flexibility to use one or more depending on their use case. However, the order is important. We execute them in this order: cacheFilter --> cacheInclude --> cacheExclude

Test coverage is included to validate different combinations and behaviors. Please let me know if you have any feedback or run into any issues.

[homestar9]

Woot! Looks like most of the tests are passing. The ACF 2023 fail has something to do with PDFs, so I am unsure if it is related.

[lmajano]

Sorry but this is not what was discussed for a cache filter. This goes through tons of machinery to make it work. The idea for the cacheFilter is that it's a function that returns a closure, which is stored in the metadata.

This way, it's just executed and used. This is going to create more issues than assist. See the code samples sent

[homestar9]

Hi @lmajano
I'm not sure I understand your original intent. I don't think you can include a closure in the metadata, can you?

This is the code sample you sent:

// Custom filter function for complex logic
function search( event, rc, prc ) cache="true" cacheTimeout="10" cacheFilter="getSearchCacheKeys" {
}

private function getProductCacheKeys() {
    // Can capture handler state or configuration in the closure
    var validKeys = this.getCacheableKeys(); // some handler method
    
    return ( rc ) => {
        return rc.filter( ( key, value ) => arrayFindNoCase( validKeys, key ) > 0 );
    };
}

My interpretation was that cacheFilter points to a private method in the handler. I assumed you made a typo with the getProductCacheKeys() method and meant getSearchCacheKeys().

When I tried to insert a closure into the cacheFilter metadata directly, it triggers an exception "Value of attribute [cachefilter] must have a literal/constant value"

// non-working example:
function withFilterClosure( event, rc, prc ) 
        cache        ="true"
        cacheTimeout ="10"
        cacheFilter  = function( rcTarget ) { 
            // Example filter that removes UTM parameters
            return filterUtmParams( rcTarget ); 
        }

If you're not looking to execute a handler method here, can you please clarify?

[homestar9]

I had an idea to accomplish what I think you're looking for... what do you think of this theoretical solution?

// HandlerService.cfc
private struct function getEventCachingMetadata( required ehBean, required oEventHandler ){
    // ...
    
    mdEntry.cacheFilter = arguments.ehBean.getActionMetadata( "cacheFilter", "" );

    // if the cacheFilter is a closure, then we store it in the mdEntry
    if ( 
        len( mdEntry.cacheFilter ) && 
        (
            isClosure( arguments.oEventHandler[ mdEntry.cacheFilter ] ) || 
            isCustomFunction( arguments.oEventHandler[ mdEntry.cacheFilter ] )
		)
    ) {
        mdEntry.cacheFilter = arguments.oEventHandler[ mdEntry.cacheFilter ];
    }
    
    // ...

Then...

// EventUrlFacade.cfc
string function getUniqueHash( 
	required event,
	required struct eventDictionary
){

	// ...
	
	// Use custom filter closure stored in dictionary
	if ( isClosure( arguments.eventDictionary.cacheFilter ) ) {
		rcTarget = arguments.eventDictionary.cacheFilter( rcTarget );
	} 
	
	// ...

The only catch is that the custom function/closure can't be private. Otherwise, it's not accessible in oEventHandler

[homestar9]

@lmajano, please take a look at my changes and let me know if they better match what you originally intended.

[lmajano]

You are almost there. What you are missing is the following.

1. The cacheFilter annotation points to a private method
2. The private method MUST return a closure/lambda

The handler service does the following:

1. Detect if the cacheFilter annotation exists and is not empty
2. Verify if the method exists on the event handler by using the eventhandler. _actionExists() method all handlers have
3. If it doesn't throw an exception and warn the user about it
4. If it does, then call the private method using the special function eventHandler. _privateInvoker( annotation ), this will return the CLOSURE/LAMBDA we need, store that in the metadata

EventURLFacade

1. Detects if the cacheFilter is not a simple value, meaning it is set
2. Call it!

[homestar9]

@lmajano, when you have a minute, could you please check again to see if it meets your expectations? I am currently working on updating the documentation.

[Copilot]

Pull Request Overview

This PR extends ColdBox event caching to allow developers to include, exclude, or filter specific request collection (RC) keys when generating cache keys. It introduces three new metadata options (cacheInclude, cacheExclude, and cacheFilter), updates the core services and URL facade to support these options, and adds integration and unit tests covering the new behavior.

• Adds cacheInclude, cacheExclude, and cacheFilter metadata to handler definitions and propagates them through HandlerService and RequestService.
• Revamps EventURLFacade to apply custom filter closures, key whitelists, and key blacklists when building cache hashes.
• Introduces new tests and example handlers in the test harness to validate include/exclude/filter behaviors.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/specs/integration/EventCachingSpec.cfc	New integration specs for cacheInclude, cacheExclude, cacheFilter cases
tests/specs/cache/util/EventsURLFacadeTest.cfc	Updated unit tests to simulate event metadata in URL facade methods
test-harness/handlers/eventcaching.cfc	Added handler actions covering all combinations of include/exclude/filter options
system/web/services/RequestService.cfc	Passes new eventDictionary into URL facade for request-level caching
system/web/services/HandlerService.cfc	Reads and validates cacheInclude, cacheExclude, cacheFilter metadata and invokes closures
system/cache/util/EventURLFacade.cfc	Implements include/exclude/filter logic in getUniqueHash and updates buildEventKey signature

Comments suppressed due to low confidence (4)

system/web/services/RequestService.cfc:158

• The new buildEventKey signature requires both a targetEvent (used by buildBasicCacheKey) and eventDictionary, but this call only passes targetContext and eventDictionary. Add targetEvent (or keySuffix) to the argument list so the cache key is constructed correctly.

			eventCache[ "cacheKey" ] = oEventURLFacade.buildEventKey(

system/web/services/HandlerService.cfc:211

• Similar to RequestService, this call to buildEventKey omits the targetEvent parameter. The method relies on arguments.targetEvent for the suffix, so include the event name or adjust the signature accordingly.

                eventCachingData.cacheKey = oEventURLFacade.buildEventKey(

system/cache/util/EventURLFacade.cfc:92

• CFML structs do not have an append method; the original structAppend built-in must be used to merge routedStruct into targetMixer. Replace this call with structAppend(targetMixer, arguments.event.getRoutedStruct(), true).

        targetMixer.append( arguments.event.getRoutedStruct(), true );

tests/specs/cache/util/EventsURLFacadeTest.cfc:53

• The variable is declared as testHash but referenced as testhash. Fix the casing to match (expect(testHash).notToBeEmpty()).

				expect( testhash ).notToBeEmpty();

[lmajano]

Get rid of the ?. -> . it will always exist.

[lmajano]

Fantastic job. Few moer changes and this is solid gold!