diff --git a/system/web/routing/Router.cfc b/system/web/routing/Router.cfc index a08aaed0f..2e5d0da79 100644 --- a/system/web/routing/Router.cfc +++ b/system/web/routing/Router.cfc @@ -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 ) ) { @@ -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 diff --git a/system/web/services/HandlerService.cfc b/system/web/services/HandlerService.cfc index a5d56d87b..9934db30e 100644 --- a/system/web/services/HandlerService.cfc +++ b/system/web/services/HandlerService.cfc @@ -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() @@ -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; @@ -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; } diff --git a/system/web/services/RoutingService.cfc b/system/web/services/RoutingService.cfc index 70ff60e4d..39b277243 100644 --- a/system/web/services/RoutingService.cfc +++ b/system/web/services/RoutingService.cfc @@ -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; @@ -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. @@ -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" ); } diff --git a/tests/specs/web/routing/RouterTest.cfc b/tests/specs/web/routing/RouterTest.cfc index b39aac485..34da95617 100644 --- a/tests/specs/web/routing/RouterTest.cfc +++ b/tests/specs/web/routing/RouterTest.cfc @@ -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 ); + } ); + } ); } } diff --git a/tests/specs/web/services/HandlerServiceTest.cfc b/tests/specs/web/services/HandlerServiceTest.cfc index dc5589eb7..86ce1c260 100755 --- a/tests/specs/web/services/HandlerServiceTest.cfc +++ b/tests/specs/web/services/HandlerServiceTest.cfc @@ -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(); + } ); + } ); } }