diff --git a/src/Umbraco.Tests/Mvc/RenderIndexActionSelectorAttributeTests.cs b/src/Umbraco.Tests/Mvc/RenderIndexActionSelectorAttributeTests.cs new file mode 100644 index 0000000000..3f327d3155 --- /dev/null +++ b/src/Umbraco.Tests/Mvc/RenderIndexActionSelectorAttributeTests.cs @@ -0,0 +1,173 @@ +using System; +using System.Linq; +using System.Reflection; +using System.Threading.Tasks; +using System.Web; +using System.Web.Mvc; +using System.Web.Routing; +using Moq; +using NUnit.Framework; +using Umbraco.Core; +using Umbraco.Core.Configuration.UmbracoSettings; +using Umbraco.Core.Logging; +using Umbraco.Core.Profiling; +using Umbraco.Web; +using Umbraco.Web.Models; +using Umbraco.Web.Mvc; +using Umbraco.Web.Routing; +using Umbraco.Web.Security; + +namespace Umbraco.Tests.Mvc +{ + [TestFixture] + public class RenderIndexActionSelectorAttributeTests + { + private MethodInfo GetRenderMvcControllerIndexMethodFromCurrentType(Type currType) + { + return currType.GetMethods().Single(x => + { + if (x.Name != "Index") return false; + if (x.ReturnParameter == null || x.ReturnParameter.ParameterType != typeof (ActionResult)) return false; + var p = x.GetParameters(); + if (p.Length != 1) return false; + if (p[0].ParameterType != typeof (RenderModel)) return false; + return true; + }); + } + + [Test] + public void Matches_Default_Index() + { + var attr = new RenderIndexActionSelectorAttribute(); + var req = new RequestContext(); + var appCtx = new ApplicationContext( + CacheHelper.CreateDisabledCacheHelper(), + new ProfilingLogger(Mock.Of(), Mock.Of())); + var umbCtx = UmbracoContext.EnsureContext( + Mock.Of(), + appCtx, + new Mock(null, null).Object, + Mock.Of(), + Enumerable.Empty(), + true); + var ctrl = new MatchesDefaultIndexController(umbCtx); + var controllerCtx = new ControllerContext(req, ctrl); + var result = attr.IsValidForRequest(controllerCtx, + GetRenderMvcControllerIndexMethodFromCurrentType(ctrl.GetType())); + + Assert.IsTrue(result); + } + + [Test] + public void Matches_Overriden_Index() + { + var attr = new RenderIndexActionSelectorAttribute(); + var req = new RequestContext(); + var appCtx = new ApplicationContext( + CacheHelper.CreateDisabledCacheHelper(), + new ProfilingLogger(Mock.Of(), Mock.Of())); + var umbCtx = UmbracoContext.EnsureContext( + Mock.Of(), + appCtx, + new Mock(null, null).Object, + Mock.Of(), + Enumerable.Empty(), + true); + var ctrl = new MatchesOverriddenIndexController(umbCtx); + var controllerCtx = new ControllerContext(req, ctrl); + var result = attr.IsValidForRequest(controllerCtx, + GetRenderMvcControllerIndexMethodFromCurrentType(ctrl.GetType())); + + Assert.IsTrue(result); + } + + [Test] + public void Matches_Custom_Index() + { + var attr = new RenderIndexActionSelectorAttribute(); + var req = new RequestContext(); + var appCtx = new ApplicationContext( + CacheHelper.CreateDisabledCacheHelper(), + new ProfilingLogger(Mock.Of(), Mock.Of())); + var umbCtx = UmbracoContext.EnsureContext( + Mock.Of(), + appCtx, + new Mock(null, null).Object, + Mock.Of(), + Enumerable.Empty(), + true); + var ctrl = new MatchesCustomIndexController(umbCtx); + var controllerCtx = new ControllerContext(req, ctrl); + var result = attr.IsValidForRequest(controllerCtx, + GetRenderMvcControllerIndexMethodFromCurrentType(ctrl.GetType())); + + Assert.IsFalse(result); + } + + [Test] + public void Matches_Async_Index_Same_Signature() + { + var attr = new RenderIndexActionSelectorAttribute(); + var req = new RequestContext(); + var appCtx = new ApplicationContext( + CacheHelper.CreateDisabledCacheHelper(), + new ProfilingLogger(Mock.Of(), Mock.Of())); + var umbCtx = UmbracoContext.EnsureContext( + Mock.Of(), + appCtx, + new Mock(null, null).Object, + Mock.Of(), + Enumerable.Empty(), + true); + var ctrl = new MatchesAsyncIndexController(umbCtx); + var controllerCtx = new ControllerContext(req, ctrl); + var result = attr.IsValidForRequest(controllerCtx, + GetRenderMvcControllerIndexMethodFromCurrentType(ctrl.GetType())); + + Assert.IsFalse(result); + } + + public class MatchesDefaultIndexController : RenderMvcController + { + public MatchesDefaultIndexController(UmbracoContext umbracoContext) : base(umbracoContext) + { + } + } + + public class MatchesOverriddenIndexController : RenderMvcController + { + public MatchesOverriddenIndexController(UmbracoContext umbracoContext) : base(umbracoContext) + { + } + + public override ActionResult Index(RenderModel model) + { + return base.Index(model); + } + } + + public class MatchesCustomIndexController : RenderMvcController + { + public MatchesCustomIndexController(UmbracoContext umbracoContext) : base(umbracoContext) + { + } + + public ActionResult Index(RenderModel model, int page) + { + return base.Index(model); + } + } + + public class MatchesAsyncIndexController : RenderMvcController + { + public MatchesAsyncIndexController(UmbracoContext umbracoContext) : base(umbracoContext) + { + } + + public new async Task Index(RenderModel model) + { + return await Task.FromResult(base.Index(model)); + } + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Tests/Mvc/ViewDataDictionaryExtensionTests.cs b/src/Umbraco.Tests/Mvc/ViewDataDictionaryExtensionTests.cs index c13818a03c..4c06d30796 100644 --- a/src/Umbraco.Tests/Mvc/ViewDataDictionaryExtensionTests.cs +++ b/src/Umbraco.Tests/Mvc/ViewDataDictionaryExtensionTests.cs @@ -1,6 +1,4 @@ -using System; -using System.Collections.Generic; -using System.Linq; +using System.Collections.Generic; using System.Text; using System.Web.Mvc; using NUnit.Framework; diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 0e7dc711b8..59a9166e5a 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -180,6 +180,7 @@ + diff --git a/src/Umbraco.Web/Mvc/DefaultRenderMvcControllerResolver.cs b/src/Umbraco.Web/Mvc/DefaultRenderMvcControllerResolver.cs index dffd80c19a..641491c5ba 100644 --- a/src/Umbraco.Web/Mvc/DefaultRenderMvcControllerResolver.cs +++ b/src/Umbraco.Web/Mvc/DefaultRenderMvcControllerResolver.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.ComponentModel; using System.Linq; using System.Text; using System.Web.Mvc; @@ -37,6 +38,7 @@ namespace Umbraco.Web.Mvc /// Returns an instance of the default controller instance. /// [Obsolete("This method will be removed in future versions and should not be used to resolve a controller instance, the IControllerFactory is used for that purpose")] + [EditorBrowsable(EditorBrowsableState.Never)] public IRenderMvcController GetControllerInstance() { //try the dependency resolver, then the activator @@ -65,10 +67,9 @@ namespace Umbraco.Web.Mvc /// private void ValidateType(Type type) { - if (TypeHelper.IsTypeAssignableFrom(type) == false - && TypeHelper.IsTypeAssignableFrom(type) == false) + if (TypeHelper.IsTypeAssignableFrom(type) == false) { - throw new InvalidOperationException("The Type specified (" + type + ") is not of type " + typeof(IRenderMvcController) + " or of type " + typeof(IAsyncRenderMvcController)); + throw new InvalidOperationException("The Type specified (" + type + ") is not of type " + typeof (IRenderController)); } } diff --git a/src/Umbraco.Web/Mvc/IAsyncRenderMvcController.cs b/src/Umbraco.Web/Mvc/IAsyncRenderMvcController.cs deleted file mode 100644 index 95983a625f..0000000000 --- a/src/Umbraco.Web/Mvc/IAsyncRenderMvcController.cs +++ /dev/null @@ -1,24 +0,0 @@ -using System.Threading.Tasks; -using System.Web.Mvc; -using Umbraco.Web.Models; - -namespace Umbraco.Web.Mvc -{ - /// - /// The interface that can be implemented for an async controller to be designated to execute for route hijacking - /// - public interface IAsyncRenderMvcController : IController - { - /// - /// The default action to render the front-end view - /// - /// - /// - /// - /// Ideally we could have made a marker interface as a base interface for both the IRenderMvcController and IAsyncRenderMvcController - /// however that would require breaking changes in other classes like DefaultRenderMvcControllerResolver since the result would have - /// to be changed. Instead we are hiding the underlying interface method. - /// - Task Index(RenderModel model); - } -} \ No newline at end of file diff --git a/src/Umbraco.Web/Mvc/IRenderController.cs b/src/Umbraco.Web/Mvc/IRenderController.cs new file mode 100644 index 0000000000..e4303e06e5 --- /dev/null +++ b/src/Umbraco.Web/Mvc/IRenderController.cs @@ -0,0 +1,12 @@ +using System.Web.Mvc; + +namespace Umbraco.Web.Mvc +{ + /// + /// A marker interface to designate that a controller will be used for Umbraco front-end requests and/or route hijacking + /// + public interface IRenderController : IController + { + + } +} \ No newline at end of file diff --git a/src/Umbraco.Web/Mvc/IRenderMvcController.cs b/src/Umbraco.Web/Mvc/IRenderMvcController.cs index 88890bf410..53865cd4ee 100644 --- a/src/Umbraco.Web/Mvc/IRenderMvcController.cs +++ b/src/Umbraco.Web/Mvc/IRenderMvcController.cs @@ -1,3 +1,4 @@ +using System; using System.Web.Http.Filters; using System.Web.Mvc; using System.Web.Routing; @@ -9,7 +10,7 @@ namespace Umbraco.Web.Mvc /// /// The interface that must be implemented for a controller to be designated to execute for route hijacking /// - public interface IRenderMvcController : IController + public interface IRenderMvcController : IRenderController { /// /// The default action to render the front-end view diff --git a/src/Umbraco.Web/Mvc/RenderActionInvoker.cs b/src/Umbraco.Web/Mvc/RenderActionInvoker.cs index de08210df6..55be6591ad 100644 --- a/src/Umbraco.Web/Mvc/RenderActionInvoker.cs +++ b/src/Umbraco.Web/Mvc/RenderActionInvoker.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Concurrent; +using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using System.Web.Mvc; @@ -12,6 +13,8 @@ namespace Umbraco.Web.Mvc /// public class RenderActionInvoker : AsyncControllerActionInvoker { + + /// /// Ensures that if an action for the Template name is not explicitly defined by a user, that the 'Index' action will execute /// @@ -26,15 +29,14 @@ namespace Umbraco.Web.Mvc //now we need to check if it exists, if not we need to return the Index by default if (ad == null) { - //check if the controller is an instance of IRenderMvcController or IAsyncRenderMvcController and find the index - if (controllerContext.Controller is IRenderMvcController - || controllerContext.Controller is IAsyncRenderMvcController) + //check if the controller is an instance of IRenderController and find the index + if (controllerContext.Controller is IRenderController) { return controllerDescriptor.FindAction(controllerContext, "Index"); } } return ad; } - + } } \ No newline at end of file diff --git a/src/Umbraco.Web/Mvc/RenderIndexActionSelectorAttribute.cs b/src/Umbraco.Web/Mvc/RenderIndexActionSelectorAttribute.cs new file mode 100644 index 0000000000..1d1c56db11 --- /dev/null +++ b/src/Umbraco.Web/Mvc/RenderIndexActionSelectorAttribute.cs @@ -0,0 +1,42 @@ +using System; +using System.Collections.Concurrent; +using System.Linq; +using System.Reflection; +using System.Web.Mvc; + +namespace Umbraco.Web.Mvc +{ + /// + /// A custom ActionMethodSelector which will ensure that the RenderMvcController.Index(RenderModel model) action will be executed + /// if the + /// + internal class RenderIndexActionSelectorAttribute : ActionMethodSelectorAttribute + { + private static readonly ConcurrentDictionary ControllerDescCache = new ConcurrentDictionary(); + + /// + /// Determines whether the action method selection is valid for the specified controller context. + /// + /// + /// true if the action method selection is valid for the specified controller context; otherwise, false. + /// + /// The controller context.Information about the action method. + public override bool IsValidForRequest(ControllerContext controllerContext, MethodInfo methodInfo) + { + var currType = methodInfo.ReflectedType; + var baseType = methodInfo.DeclaringType; + + //It's the same type, so this must be the Index action to use + if (currType == baseType) return true; + + if (currType == null) return false; + + var controllerDesc = ControllerDescCache.GetOrAdd(currType, type => new ReflectedControllerDescriptor(currType)); + var actions = controllerDesc.GetCanonicalActions(); + + //If there are more than one Index action for this controller, then + // this base class one should not be matched + return actions.Count(x => x.ActionName == "Index") <= 1; + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Web/Mvc/RenderMvcController.cs b/src/Umbraco.Web/Mvc/RenderMvcController.cs index 03f93d89e3..dd06b3ced1 100644 --- a/src/Umbraco.Web/Mvc/RenderMvcController.cs +++ b/src/Umbraco.Web/Mvc/RenderMvcController.cs @@ -1,22 +1,17 @@ using System; -using System.IO; using System.Web.Mvc; -using Umbraco.Core; using Umbraco.Core.Logging; using Umbraco.Core.Models; -using Umbraco.Core.Services; -using Umbraco.Core.Models; using Umbraco.Web.Models; using Umbraco.Web.Routing; -using Umbraco.Web.Security; namespace Umbraco.Web.Mvc { /// - /// A controller to render front-end requests + /// The default controller to render front-end requests /// - [PreRenderViewActionFilter] + [PreRenderViewActionFilter] public class RenderMvcController : UmbracoController, IRenderMvcController { @@ -64,7 +59,7 @@ namespace Umbraco.Web.Mvc { if (_publishedContentRequest != null) return _publishedContentRequest; - if (!RouteData.DataTokens.ContainsKey("umbraco-doc-request")) + if (RouteData.DataTokens.ContainsKey("umbraco-doc-request") == false) { throw new InvalidOperationException("DataTokens must contain an 'umbraco-doc-request' key with a PublishedContentRequest object"); } @@ -111,10 +106,10 @@ namespace Umbraco.Web.Mvc /// /// /// + [RenderIndexActionSelector] public virtual ActionResult Index(RenderModel model) { return CurrentTemplate(model); } - } } \ No newline at end of file diff --git a/src/Umbraco.Web/Mvc/RenderRouteHandler.cs b/src/Umbraco.Web/Mvc/RenderRouteHandler.cs index 5d230d7177..bce3515c1b 100644 --- a/src/Umbraco.Web/Mvc/RenderRouteHandler.cs +++ b/src/Umbraco.Web/Mvc/RenderRouteHandler.cs @@ -310,8 +310,8 @@ namespace Umbraco.Web.Mvc //check if that controller exists if (controllerType != null) { - //ensure the controller is of type IRenderMvcController/IAsyncRenderMvcController and ControllerBase - if ((TypeHelper.IsTypeAssignableFrom(controllerType) || TypeHelper.IsTypeAssignableFrom(controllerType)) + //ensure the controller is of type IRenderMvcController and ControllerBase + if (TypeHelper.IsTypeAssignableFrom(controllerType) && TypeHelper.IsTypeAssignableFrom(controllerType)) { //set the controller and name to the custom one @@ -325,11 +325,10 @@ namespace Umbraco.Web.Mvc else { LogHelper.Warn( - "The current Document Type {0} matches a locally declared controller of type {1}. Custom Controllers for Umbraco routing must implement either '{2}' or '{3}' and inherit from '{4}'.", + "The current Document Type {0} matches a locally declared controller of type {1}. Custom Controllers for Umbraco routing must implement '{2}' and inherit from '{3}'.", () => publishedContentRequest.PublishedContent.DocumentTypeAlias, () => controllerType.FullName, - () => typeof(IRenderMvcController).FullName, - () => typeof(IAsyncRenderMvcController).FullName, + () => typeof(IRenderController).FullName, () => typeof(ControllerBase).FullName); //we cannot route to this custom controller since it is not of the correct type so we'll continue with the defaults diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index ee2f8906e3..834964c8ca 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -304,7 +304,8 @@ - + +