diff --git a/system/web/Renderer.cfc b/system/web/Renderer.cfc index 72d910b29..497ff87c0 100755 --- a/system/web/Renderer.cfc +++ b/system/web/Renderer.cfc @@ -61,6 +61,10 @@ component // Per-name locate caches, gated by viewDiscoveryCaching variables.locateViewCache = {}; variables.locateLayoutCache = {}; + // Normalized folders implicit views may resolve from, per configured folder path + variables.implicitViewRoots = {} + // Implicit view lookups, gated by viewDiscoveryCaching + variables.implicitViewCache = {} super.init(); return this; } @@ -963,6 +967,67 @@ component return locateView( arguments.view ); } + /** + * Locate a view chosen from the request's event name (implicit views). Unlike locateView(), only views inside the + * declared view folders are returned: the views convention folder, the external views location and, for a module + * event, the module's views folder. Templates anywhere else under the application root are ignored. + * + * @view The view to locate + * @module The module of the event, if any + * + * @return The view path, or an empty string when the view doesn't exist in a declared view folder + */ + function locateImplicitView( required view, module = "" ){ + // This runs on every render that uses an implicit view, so the result is remembered like locateView()'s. + // A module lookup depends on the module's current views folder, so that folder is part of the key: + // a module reloaded with another views folder gets a fresh lookup. + var cacheKey = "|" & arguments.view + if ( len( arguments.module ) ) { + var moduleConfig = variables.modulesConfig[ arguments.module ] + cacheKey = arguments.module & "|" & moduleConfig.mapping & "/" & moduleConfig.conventions.viewsLocation & "|" & arguments.view + } + if ( variables.isDiscoveryCaching && variables.implicitViewCache.keyExists( cacheKey ) ) { + return variables.implicitViewCache[ cacheKey ] + } + + var viewPath = findImplicitView( arguments.view, arguments.module ) + if ( variables.isDiscoveryCaching ) { + variables.implicitViewCache[ cacheKey ] = viewPath + } + return viewPath + } + + /** + * Find a view chosen from the request's event name in the declared view folders, without the lookup cache + * + * @view The view to locate + * @module The module of the event, if any + * + * @return The view path, or an empty string when the view doesn't exist in a declared view folder + */ + private string function findImplicitView( required view, module = "" ){ + // Remove a lowercase extension like locateView() does, which returns the view as is when no template exists + var extension = right( arguments.view, 4 ) + if ( !compare( extension, ".cfm" ) || !compare( extension, ".bxm" ) ) { + arguments.view = left( arguments.view, len( arguments.view ) - 4 ) + } + var viewPath = len( arguments.module ) ? locateModuleView( arguments.view, arguments.module ) : locateView( + arguments.view + ) + if ( viewPath == arguments.view ) { + return "" + } + + // Compare case-sensitively: on a case-sensitive file system, folders that differ only by case are different + var templatePath = normalizeFilePath( cachedExpandPath( viewPath ) ) + for ( var viewRoot in getImplicitViewRoots( arguments.module ) ) { + if ( !compare( left( templatePath, len( viewRoot ) ), viewRoot ) ) { + return viewPath + } + } + return "" + } + /** * Discover view+helper path locations. The returned helper and view locations will have the appropriate extension. * @@ -1063,15 +1128,74 @@ component var cEvent = reReplaceNoCase( event.getCurrentEvent(), "^([^:.]*):", "" ); // Implicit views - if ( variables.controller.getSetting( name = "caseSensitiveImplicitViews", defaultValue = true ) ) { - event.setView( replace( cEvent, ".", "/", "all" ) ); - } else { - event.setView( lCase( replace( cEvent, ".", "/", "all" ) ) ); + var implicitView = replace( cEvent, ".", "/", "all" ) + if ( !variables.controller.getSetting( name = "caseSensitiveImplicitViews", defaultValue = true ) ) { + implicitView = lCase( implicitView ) } + // The view must exist in a declared view folder, never elsewhere under the app root + if ( !len( locateImplicitView( implicitView, event.getCurrentModule() ) ) ) { + throw( + message = "The implicit view [#implicitView#] for the event [#event.getCurrentEvent()#] was not found in the views folders.", + detail = "Implicit views resolve only from the views folder, the external views location or a module's views folder. Use event.setView() to render a template from another location.", + type = "Renderer.ImplicitViewNotFoundException" + ) + } + + event.setView( implicitView ) + return this; } + /** + * The folders implicit views may resolve from, as normalized absolute paths ending in / + * + * @module The module of the event, if any + */ + private array function getImplicitViewRoots( module = "" ){ + var roots = [ "/#variables.appMapping#/#variables.viewsConvention#" ] + if ( len( variables.viewsExternalLocation ) ) { + roots.append( variables.viewsExternalLocation ) + } + // Read the module's folder on every lookup, a module reload can change it + if ( len( arguments.module ) ) { + roots.append( + "#variables.modulesConfig[ arguments.module ].mapping#/#variables.modulesConfig[ arguments.module ].conventions.viewsLocation#" + ) + } + + var viewRoots = [] + for ( var root in roots ) { + if ( !variables.implicitViewRoots.keyExists( root ) ) { + variables.implicitViewRoots[ root ] = normalizeFilePath( + cachedExpandPath( reReplace( root, "//", "/", "all" ) ) + ) & "/" + } + viewRoots.append( variables.implicitViewRoots[ root ] ) + } + return viewRoots + } + + /** + * Normalize an absolute file path: use / separators and remove . and .. segments, without resolving symlinks + * + * @path The absolute path + */ + private string function normalizeFilePath( required string path ){ + var segments = [] + for ( var segment in listToArray( replace( arguments.path, "\", "/", "all" ), "/" ) ) { + if ( segment == ".." ) { + if ( segments.len() ) { + segments.deleteAt( segments.len() ) + } + } else if ( segment != "." ) { + segments.append( segment ) + } + } + // Keep the leading slash of a *nix path, a Windows path starts with its drive + return ( left( arguments.path, 1 ) == "/" ? "/" : "" ) & segments.toList( "/" ) + } + /** * Incorporate a rendering region into the arguments struct * diff --git a/system/web/services/HandlerService.cfc b/system/web/services/HandlerService.cfc index 1e1d53cc1..1da7c4315 100644 --- a/system/web/services/HandlerService.cfc +++ b/system/web/services/HandlerService.cfc @@ -397,16 +397,16 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" { if ( find( ":", arguments.event ) ) { // Validate that it is a valid module, else it is an invalid view. if ( structKeyExists( variables.modules, targetModule ) ) { - targetView = renderer.locateModuleView( cEvent, targetModule ); + targetView = renderer.locateImplicitView( cEvent, targetModule ); } else { return false; } } else { - targetView = renderer.locateView( cEvent ); + targetView = renderer.locateImplicitView( cEvent ); } - // 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. + // locateImplicitView returns a path with .cfm/.bxm extension only when the file was verified + // to exist in a declared view folder — 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/test-harness/handlers/implicitViews.cfc b/test-harness/handlers/implicitViews.cfc new file mode 100644 index 000000000..95dcf1858 --- /dev/null +++ b/test-harness/handlers/implicitViews.cfc @@ -0,0 +1,19 @@ +/** + * Actions for the implicit view specs in tests/specs/integration/ImplicitViewsSpec.cfc + */ +component { + + // Sets no view: views/implicitViews/withView.cfm renders implicitly + function withView( event, rc, prc ){ + } + + // Sets no view: the only matching template is implicitViews/noView.cfm in the app root + function noView( event, rc, prc ){ + } + + // Selects the app root template explicitly, which is still allowed + function explicitRoot( event, rc, prc ){ + event.setView( "implicitViews/noView" ) + } + +} diff --git a/test-harness/implicitViews/noView.cfm b/test-harness/implicitViews/noView.cfm new file mode 100644 index 000000000..21bc63af2 --- /dev/null +++ b/test-harness/implicitViews/noView.cfm @@ -0,0 +1,2 @@ + +ROOT-ONLY-NO-VIEW diff --git a/test-harness/main/rootonlyaction.cfm b/test-harness/main/rootonlyaction.cfm new file mode 100644 index 000000000..938c736a4 --- /dev/null +++ b/test-harness/main/rootonlyaction.cfm @@ -0,0 +1,2 @@ + +ROOT-ONLY-ACTION diff --git a/test-harness/rootonlyview.cfm b/test-harness/rootonlyview.cfm new file mode 100644 index 000000000..0282218eb --- /dev/null +++ b/test-harness/rootonlyview.cfm @@ -0,0 +1,2 @@ + +ROOT-ONLY-VIEW diff --git a/test-harness/views/implicitViews/withView.cfm b/test-harness/views/implicitViews/withView.cfm new file mode 100644 index 000000000..230ed33c1 --- /dev/null +++ b/test-harness/views/implicitViews/withView.cfm @@ -0,0 +1 @@ +IMPLICIT-WITH-VIEW-OK diff --git a/tests/specs/integration/EventCachingSpec.cfc b/tests/specs/integration/EventCachingSpec.cfc index 6e77de033..1ce51facc 100755 --- a/tests/specs/integration/EventCachingSpec.cfc +++ b/tests/specs/integration/EventCachingSpec.cfc @@ -576,7 +576,8 @@ } ); it( "keeps the serve-side and store-side keys in sync even with handlerCaching off", function(){ - var handlerService = controller.getHandlerService() + var handlerService = controller.getHandlerService() + var originalCaching = handlerService.getHandlerCaching() handlerService.setHandlerCaching( false ) try { @@ -593,7 +594,9 @@ expect( storeKey ).toInclude( "delta-present" ) expect( serveKey ).toBe( storeKey ) } finally { - handlerService.setHandlerCaching( true ) + // Put back the setting the spec found. The test app runs with handler caching off, so + // turning it on here would leave every later spec with cached handler lookups. + handlerService.setHandlerCaching( originalCaching ) } } ); diff --git a/tests/specs/integration/ImplicitViewsSpec.cfc b/tests/specs/integration/ImplicitViewsSpec.cfc new file mode 100644 index 000000000..937fe8a70 --- /dev/null +++ b/tests/specs/integration/ImplicitViewsSpec.cfc @@ -0,0 +1,86 @@ +/** + * Implicit views resolve only from the declared view folders: the views convention, the external views location + * and module views. Templates elsewhere under the app root are never chosen from the event name. + * Fixtures: test-harness/handlers/implicitViews.cfc, test-harness/rootonlyview.cfm, + * test-harness/main/rootonlyaction.cfm, test-harness/implicitViews/noView.cfm, test-harness/views/implicitViews/ + * The root-only fixtures are lowercase because view dispatch lowercases the event, so on a case-sensitive file + * system the old lookup finds them too. + */ +component extends="tests.resources.BaseIntegrationTest" { + + function run(){ + describe( "Implicit views", function(){ + beforeEach( function( currentSpec ){ + setup() + // The harness leaves implicit views off. The renderer reads the setting, the handler service caches it. + variables.handlerService = getController().getHandlerService() + variables.originalSetting = getController().getSetting( "ImplicitViews" ) + getController().setSetting( "ImplicitViews", true ) + prepareMock( variables.handlerService ).$property( "implicitViews", "variables", true ) + // Don't cache handler lookups made with implicit views on: a later spec with them off would get them + variables.originalHandlerCaching = variables.handlerService.getHandlerCaching() + variables.handlerService.setHandlerCaching( false ) + } ) + + afterEach( function( currentSpec ){ + getController().setSetting( "ImplicitViews", variables.originalSetting ) + variables.handlerService.$property( + "implicitViews", + "variables", + variables.originalSetting + ) + variables.handlerService.setHandlerCaching( variables.originalHandlerCaching ) + } ) + + describe( "dispatch from a declared view folder", function(){ + it( "renders a view in the views folder for a missing handler", function(){ + var e = execute( event = "simpleview", renderResults = true ) + expect( e.getRenderedContent() ).toInclude( "I am a simple view" ) + } ) + + it( "renders a view in the external views location for a missing handler", function(){ + var e = execute( event = "externalview", renderResults = true ) + expect( e.getRenderedContent() ).toInclude( "I am an external view" ) + } ) + + it( "renders a module view for a missing module handler", function(){ + var e = execute( event = "moduleLookup:module-level", renderResults = true ) + expect( e.getRenderedContent() ).toInclude( "This is a module level view" ) + } ) + + it( "renders the views folder view of an action that sets no view", function(){ + var e = execute( event = "implicitViews.withView", renderResults = true ) + expect( e.getRenderedContent() ).toInclude( "IMPLICIT-WITH-VIEW-OK" ) + } ) + } ) + + describe( "templates outside the declared view folders", function(){ + it( "are not dispatched for a missing handler", function(){ + var e = execute( event = "rootonlyview", renderResults = true ) + expect( e.getPrivateValue( "invalidevent", "" ) ).toBe( "rootonlyview" ) + expect( e.getStatusCode() ).toBe( 404 ) + expect( e.getRenderedContent() ).notToInclude( "ROOT-ONLY-VIEW" ) + } ) + + it( "are not dispatched for a missing action", function(){ + var e = execute( event = "main.rootonlyaction", renderResults = true ) + expect( e.getPrivateValue( "invalidevent", "" ) ).toBe( "main.rootonlyaction" ) + expect( e.getStatusCode() ).toBe( 404 ) + expect( e.getRenderedContent() ).notToInclude( "ROOT-ONLY-ACTION" ) + } ) + + it( "are not rendered for an action that sets no view", function(){ + expect( function(){ + execute( event = "implicitViews.noView", renderResults = true ) + } ).toThrow( type = "Renderer.ImplicitViewNotFoundException" ) + } ) + + it( "still render when handler code selects them explicitly", function(){ + var e = execute( event = "implicitViews.explicitRoot", renderResults = true ) + expect( e.getRenderedContent() ).toInclude( "ROOT-ONLY-NO-VIEW" ) + } ) + } ) + } ) + } + +} diff --git a/tests/specs/web/RendererTest.cfc b/tests/specs/web/RendererTest.cfc index 86d9f297a..ff178b905 100755 --- a/tests/specs/web/RendererTest.cfc +++ b/tests/specs/web/RendererTest.cfc @@ -211,6 +211,52 @@ component extends="tests.resources.BaseIntegrationTest" { } ); } ); + describe( "Implicit view lookup", () => { + it( "finds a view in the views folder", () => { + expect( renderer.locateImplicitView( "simpleview" ) ).toInclude( "simpleview.cfm" ) + } ) + + it( "finds a view in the external views location", () => { + expect( renderer.locateImplicitView( "externalview" ) ).toInclude( "testViews/externalview.cfm" ) + } ) + + it( "finds a module view", () => { + expect( renderer.locateImplicitView( "module-level", "moduleLookup" ) ).toInclude( + "moduleLookup/views/module-level.cfm" + ) + } ) + + it( "ignores an app root template that locateView() finds", () => { + expect( renderer.locateView( "rootonlyview" ) ).toInclude( "rootonlyview.cfm" ) + expect( renderer.locateImplicitView( "rootonlyview" ) ).toBe( "" ) + } ) + + it( "returns an empty string for a missing view", () => { + expect( renderer.locateImplicitView( "no/such/view" ) ).toBe( "" ) + } ) + + it( "returns an empty string for a missing view with an extension", () => { + expect( renderer.locateImplicitView( "/cbTestHarness/views/missing.CFM" ) ).toBe( "" ) + } ) + + it( "uses a module's current views folder", () => { + var conventions = renderer.$getProperty( "modulesConfig", "variables" ).moduleLookup.conventions + var viewsLocation = conventions.viewsLocation + expect( renderer.locateImplicitView( "module-level", "moduleLookup" ) ).toInclude( + "moduleLookup/views/module-level.cfm" + ) + try { + // As after a module reload with another views folder + conventions.viewsLocation = "layouts" + expect( renderer.locateImplicitView( "module-level", "moduleLookup" ) ).toInclude( + "moduleLookup/layouts/module-level.cfm" + ) + } finally { + conventions.viewsLocation = viewsLocation + } + } ) + } ) + feature( "ColdBox can render view collections", () => { beforeEach( ( currentSpec ) => { aUsers = [