Ensures that the RenderActionInvoker isn't doing anything special so that MVC takes care of everything regarding async vs non-async and any controller descriptor/action descriptor lookups and cache. Creates a RenderIndexActionSelectorAttribute - this is used to decorate the underlying RenderMvcController.Index action. MVC will call into this method to check if the MethodInfo is valid, we then do a quick comparison of types, if the current type is the same as the reflected type, this means that the Index action has been overridden or there is no custom controller... lets use it. If the types don't match we'll do a simple reflected lookup to check if the reflected controller type (current controller) has more than one index action, if so, it means that a custom controller is in play and it has a custom index action... so we won't use the base class action and then it's up to MVC to find any other matching Index action based on the current request parameters. Added some tests for this too.

This commit is contained in:
Shannon
2015-11-18 14:59:29 +01:00
parent 9fd80d791a
commit 916bad82df
12 changed files with 251 additions and 50 deletions

View File

@@ -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<ILogger>(), Mock.Of<IProfiler>()));
var umbCtx = UmbracoContext.EnsureContext(
Mock.Of<HttpContextBase>(),
appCtx,
new Mock<WebSecurity>(null, null).Object,
Mock.Of<IUmbracoSettingsSection>(),
Enumerable.Empty<IUrlProvider>(),
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<ILogger>(), Mock.Of<IProfiler>()));
var umbCtx = UmbracoContext.EnsureContext(
Mock.Of<HttpContextBase>(),
appCtx,
new Mock<WebSecurity>(null, null).Object,
Mock.Of<IUmbracoSettingsSection>(),
Enumerable.Empty<IUrlProvider>(),
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<ILogger>(), Mock.Of<IProfiler>()));
var umbCtx = UmbracoContext.EnsureContext(
Mock.Of<HttpContextBase>(),
appCtx,
new Mock<WebSecurity>(null, null).Object,
Mock.Of<IUmbracoSettingsSection>(),
Enumerable.Empty<IUrlProvider>(),
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<ILogger>(), Mock.Of<IProfiler>()));
var umbCtx = UmbracoContext.EnsureContext(
Mock.Of<HttpContextBase>(),
appCtx,
new Mock<WebSecurity>(null, null).Object,
Mock.Of<IUmbracoSettingsSection>(),
Enumerable.Empty<IUrlProvider>(),
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<ActionResult> Index(RenderModel model)
{
return await Task.FromResult(base.Index(model));
}
}
}
}

View File

@@ -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;

View File

@@ -180,6 +180,7 @@
<Compile Include="Logging\AsyncRollingFileAppenderTest.cs" />
<Compile Include="Logging\DebugAppender.cs" />
<Compile Include="Logging\ParallelForwarderTest.cs" />
<Compile Include="Mvc\RenderIndexActionSelectorAttributeTests.cs" />
<Compile Include="Persistence\Repositories\AuditRepositoryTest.cs" />
<Compile Include="Persistence\Repositories\DomainRepositoryTest.cs" />
<Compile Include="Persistence\Repositories\PartialViewRepositoryTests.cs" />

View File

@@ -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.
/// </summary>
[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
/// <param name="type"></param>
private void ValidateType(Type type)
{
if (TypeHelper.IsTypeAssignableFrom<IRenderMvcController>(type) == false
&& TypeHelper.IsTypeAssignableFrom<IAsyncRenderMvcController>(type) == false)
if (TypeHelper.IsTypeAssignableFrom<IRenderController>(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));
}
}

View File

@@ -1,24 +0,0 @@
using System.Threading.Tasks;
using System.Web.Mvc;
using Umbraco.Web.Models;
namespace Umbraco.Web.Mvc
{
/// <summary>
/// The interface that can be implemented for an async controller to be designated to execute for route hijacking
/// </summary>
public interface IAsyncRenderMvcController : IController
{
/// <summary>
/// The default action to render the front-end view
/// </summary>
/// <param name="model"></param>
/// <returns></returns>
/// <remarks>
/// 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.
/// </remarks>
Task<ActionResult> Index(RenderModel model);
}
}

View File

@@ -0,0 +1,12 @@
using System.Web.Mvc;
namespace Umbraco.Web.Mvc
{
/// <summary>
/// A marker interface to designate that a controller will be used for Umbraco front-end requests and/or route hijacking
/// </summary>
public interface IRenderController : IController
{
}
}

View File

@@ -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
/// <summary>
/// The interface that must be implemented for a controller to be designated to execute for route hijacking
/// </summary>
public interface IRenderMvcController : IController
public interface IRenderMvcController : IRenderController
{
/// <summary>
/// The default action to render the front-end view

View File

@@ -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
/// </summary>
public class RenderActionInvoker : AsyncControllerActionInvoker
{
/// <summary>
/// Ensures that if an action for the Template name is not explicitly defined by a user, that the 'Index' action will execute
/// </summary>
@@ -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;
}
}
}

View File

@@ -0,0 +1,42 @@
using System;
using System.Collections.Concurrent;
using System.Linq;
using System.Reflection;
using System.Web.Mvc;
namespace Umbraco.Web.Mvc
{
/// <summary>
/// A custom ActionMethodSelector which will ensure that the RenderMvcController.Index(RenderModel model) action will be executed
/// if the
/// </summary>
internal class RenderIndexActionSelectorAttribute : ActionMethodSelectorAttribute
{
private static readonly ConcurrentDictionary<Type, ReflectedControllerDescriptor> ControllerDescCache = new ConcurrentDictionary<Type, ReflectedControllerDescriptor>();
/// <summary>
/// Determines whether the action method selection is valid for the specified controller context.
/// </summary>
/// <returns>
/// true if the action method selection is valid for the specified controller context; otherwise, false.
/// </returns>
/// <param name="controllerContext">The controller context.</param><param name="methodInfo">Information about the action method.</param>
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;
}
}
}

View File

@@ -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
{
/// <summary>
/// A controller to render front-end requests
/// The default controller to render front-end requests
/// </summary>
[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
/// </summary>
/// <param name="model"></param>
/// <returns></returns>
[RenderIndexActionSelector]
public virtual ActionResult Index(RenderModel model)
{
return CurrentTemplate(model);
}
}
}

View File

@@ -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<IRenderMvcController>(controllerType) || TypeHelper.IsTypeAssignableFrom<IAsyncRenderMvcController>(controllerType))
//ensure the controller is of type IRenderMvcController and ControllerBase
if (TypeHelper.IsTypeAssignableFrom<IRenderController>(controllerType)
&& TypeHelper.IsTypeAssignableFrom<ControllerBase>(controllerType))
{
//set the controller and name to the custom one
@@ -325,11 +325,10 @@ namespace Umbraco.Web.Mvc
else
{
LogHelper.Warn<RenderRouteHandler>(
"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

View File

@@ -304,7 +304,8 @@
<Compile Include="Media\EmbedProviders\Flickr.cs" />
<Compile Include="Models\ContentEditing\SimpleNotificationModel.cs" />
<Compile Include="Models\PublishedContentWithKeyBase.cs" />
<Compile Include="Mvc\IAsyncRenderMvcController.cs" />
<Compile Include="Mvc\IRenderController.cs" />
<Compile Include="Mvc\RenderIndexActionSelectorAttribute.cs" />
<Compile Include="PropertyEditors\DatePreValueEditor.cs" />
<Compile Include="RequestLifespanMessagesFactory.cs" />
<Compile Include="Scheduling\LatchedBackgroundTaskBase.cs" />