Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions system/web/routing/Router.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -1065,6 +1065,15 @@ component
}
}

// Pre-parse response string placeholders so renderResponse() skips regex on every request
if ( isSimpleValue( thisRoute.response ) && len( thisRoute.response ) ) {
thisRoute.responsePlaceholders = reMatchNoCase( "{[^{]+?}", thisRoute.response ).map( function( token ){
return { token : token, key : reReplaceNoCase( token, "({|})", "", "all" ) };
} );
} else {
thisRoute.responsePlaceholders = [];
}

// Add it to the corresponding routing table
// MODULES
if ( len( arguments.module ) ) {
Expand Down Expand Up @@ -1133,6 +1142,7 @@ component
"rc" : {}, // The RC params to add incorporate if matched
"redirect" : "", // The redirection location
"response" : "", // Do we have an inline response closure
"responsePlaceholders" : [], // Pre-parsed {token} list for string responses
"ssl" : false, // Are we forcing SSL
"statusCode" : 200, // The response status code
"valuePairTranslation" : true, // If we translate name-value pairs in the URL by convention
Expand Down
8 changes: 5 additions & 3 deletions system/web/services/HandlerService.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" {
variables.handlersPath = variables.controller.getSetting( "handlersPath" )
variables.interceptorService = variables.controller.getInterceptorService()
variables.invalidEventHandler = variables.controller.getSetting( "invalidEventHandler" )
variables.implicitViews = variables.controller.getSetting( "ImplicitViews" )
variables.modules = variables.controller.getSetting( "modules" )
variables.templateCache = variables.controller.getCache( "template" )
variables.wirebox = variables.controller.getWireBox()
Expand Down Expand Up @@ -155,7 +156,7 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" {

// Test for Implicit View Dispatch
if (
controller.getSetting( "ImplicitViews" ) AND
variables.implicitViews AND
isViewDispatch( arguments.ehBean.getFullEvent(), arguments.ehBean )
) {
return oEventHandler;
Expand Down Expand Up @@ -404,8 +405,9 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" {
targetView = renderer.locateView( cEvent );
}

// CFML View
if ( fileExists( expandPath( targetView ) ) ) {
// locateView/locateModuleView return a path with .cfm/.bxm extension only when
// the file was verified to exist — no need for a redundant filesystem call here.
if ( right( targetView, 4 ) == ".cfm" || right( targetView, 4 ) == ".bxm" ) {
arguments.ehBean.setViewDispatch( true );
return true;
}
Expand Down
20 changes: 9 additions & 11 deletions system/web/services/RoutingService.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" {

// Check if we found a route, else just return empty params struct
if ( results.route.isEmpty() ) {
if ( getLogger().canDebug() ) {
if ( canDebug ) {
getLogger().debug( "No URL routes matched on routed string: #requestString#" );
}
return results;
Expand Down Expand Up @@ -599,7 +599,7 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" {
// reset pattern matching, if packages found.
if ( compare( packagedRequestString, requestString ) NEQ 0 ) {
// Log package resolved
if ( getLogger().canDebug() ) {
if ( canDebug ) {
getLogger().debug( "URL Routing Package Resolved: #packagedRequestString#" );
}
// Return found Route recursively.
Expand Down Expand Up @@ -781,16 +781,14 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" {
// simple values
if ( isSimpleValue( aRoute.response ) ) {
// setup default response
theResponse = aRoute.response;
// String replacements
var replacements = reMatchNoCase( "{[^{]+?}", aRoute.response );
for ( var thisReplacement in replacements ) {
var thisKey = reReplaceNoCase( thisReplacement, "({|})", "", "all" );
if ( event.valueExists( thisKey ) ) {
theResponse = aRoute.response;
// Apply pre-parsed placeholders (tokens resolved once at route registration)
for ( var placeholder in aRoute.responsePlaceholders ) {
if ( event.valueExists( placeholder.key ) ) {
theResponse = replace(
aRoute.response,
thisReplacement,
event.getValue( thisKey ),
theResponse,
placeholder.token,
event.getValue( placeholder.key ),
"all"
);
}
Expand Down
54 changes: 54 additions & 0 deletions tests/specs/web/routing/RouterTest.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,60 @@ component extends="coldbox.system.testing.BaseModelTest" {
} );
} );
} );

describe( "Response placeholder pre-parsing", function(){
beforeEach( function( currentSpec ){
router = createMock( "coldbox.system.web.routing.Router" )
.init()
.setController( controller )
.setLogBox( controller.getLogBox() )
.setLog( controller.getLogBox().getLogger( this ) )
.setCacheBox( controller.getCacheBox() )
.setWireBox( controller.getWireBox() );
} );

it( "a route with a static string response pre-parses placeholders at registration", function(){
router.addRoute( pattern = "/hello/:name", response = "Hello {name}!" );
var routes = router.getRoutes();
expect( routes ).toHaveLength( 1 );
expect( routes[ 1 ] ).toHaveKey( "responsePlaceholders" );
expect( routes[ 1 ].responsePlaceholders ).toHaveLength( 1 );
expect( routes[ 1 ].responsePlaceholders[ 1 ].token ).toBe( "{name}" );
expect( routes[ 1 ].responsePlaceholders[ 1 ].key ).toBe( "name" );
} );

it( "a route with multiple placeholders pre-parses all of them", function(){
router.addRoute( pattern = "/greet/:name/:mod", response = "Hello {name} from {mod}" );
var routes = router.getRoutes();
expect( routes[ 1 ].responsePlaceholders ).toHaveLength( 2 );
expect( routes[ 1 ].responsePlaceholders[ 1 ].key ).toBe( "name" );
expect( routes[ 1 ].responsePlaceholders[ 2 ].key ).toBe( "mod" );
} );

it( "a route with no placeholders has an empty responsePlaceholders array", function(){
router.addRoute( pattern = "/static", response = "No placeholders here" );
var routes = router.getRoutes();
expect( routes[ 1 ].responsePlaceholders ).toBeArray();
expect( routes[ 1 ].responsePlaceholders ).toHaveLength( 0 );
} );

it( "a route with a closure response has an empty responsePlaceholders array", function(){
router.addRoute(
pattern = "/closure",
response = function( event, rc, prc ){ return "hi"; }
);
var routes = router.getRoutes();
expect( routes[ 1 ].responsePlaceholders ).toBeArray();
expect( routes[ 1 ].responsePlaceholders ).toHaveLength( 0 );
} );

it( "a route with no response has an empty responsePlaceholders array", function(){
router.addRoute( pattern = "/noop", event = "main.index" );
var routes = router.getRoutes();
expect( routes[ 1 ].responsePlaceholders ).toBeArray();
expect( routes[ 1 ].responsePlaceholders ).toHaveLength( 0 );
} );
} );
}

}
26 changes: 26 additions & 0 deletions tests/specs/web/services/HandlerServiceTest.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,32 @@ component extends="tests.resources.BaseIntegrationTest" {
} );
} );
} );

describe( "Hot-path caching optimizations", () => {
beforeEach( () => {
setup();
variables.handlerService = controller.getHandlerService();
} );

it( "caches implicitViews setting in variables scope after configuration load", () => {
// implicitViews must be a boolean cached from getSetting("ImplicitViews")
prepareMock( variables.handlerService );
expect( variables.handlerService.$getProperty( "implicitViews", "variables" ) ).toBeBoolean();
expect( variables.handlerService.$getProperty( "implicitViews", "variables" ) ).toBeTrue();
} );

it( "isViewDispatch detects a view by extension without a filesystem call", () => {
// simpleview exists — getHandlerBean triggers isViewDispatch internally
// If the extension-check logic is wrong the viewDispatch flag won't be set
var results = variables.handlerService.getHandlerBean( "simpleview" );
expect( results.getViewDispatch() ).toBeTrue();
} );

it( "isViewDispatch returns false for an event with no matching view", () => {
var results = variables.handlerService.getHandlerBean( "nonexistent_view_xyz" );
expect( results.getViewDispatch() ).toBeFalse();
} );
} );
}

}
Loading