diff --git a/src/Umbraco.Core/Models/ContentModel.cs b/src/Umbraco.Core/Models/ContentModel.cs index 9e4a9a3024..5ab2c0b7b3 100644 --- a/src/Umbraco.Core/Models/ContentModel.cs +++ b/src/Umbraco.Core/Models/ContentModel.cs @@ -1,4 +1,4 @@ -using System; +using System; using Umbraco.Core.Models.PublishedContent; namespace Umbraco.Web.Models @@ -11,12 +11,7 @@ namespace Umbraco.Web.Models /// /// Initializes a new instance of the class with a content. /// - /// - public ContentModel(IPublishedContent content) - { - if (content == null) throw new ArgumentNullException(nameof(content)); - Content = content; - } + public ContentModel(IPublishedContent content) => Content = content ?? throw new ArgumentNullException(nameof(content)); /// /// Gets the content. diff --git a/src/Umbraco.Core/Models/ContentModelOfTContent.cs b/src/Umbraco.Core/Models/ContentModelOfTContent.cs index bf4d81dbf3..31c8cbb2c7 100644 --- a/src/Umbraco.Core/Models/ContentModelOfTContent.cs +++ b/src/Umbraco.Core/Models/ContentModelOfTContent.cs @@ -1,4 +1,4 @@ -using Umbraco.Core.Models.PublishedContent; +using Umbraco.Core.Models.PublishedContent; namespace Umbraco.Web.Models { @@ -8,12 +8,8 @@ namespace Umbraco.Web.Models /// /// Initializes a new instance of the class with a content. /// - /// public ContentModel(TContent content) - : base(content) - { - Content = content; - } + : base(content) => Content = content; /// /// Gets the content. diff --git a/src/Umbraco.Web.Common/Controllers/IVirtualPageController.cs b/src/Umbraco.Web.Common/Controllers/IVirtualPageController.cs index fc36207ee0..bfea5c8d87 100644 --- a/src/Umbraco.Web.Common/Controllers/IVirtualPageController.cs +++ b/src/Umbraco.Web.Common/Controllers/IVirtualPageController.cs @@ -1,4 +1,5 @@ -using Umbraco.Core.Models.PublishedContent; +using Microsoft.AspNetCore.Mvc.Filters; +using Umbraco.Core.Models.PublishedContent; namespace Umbraco.Web.Common.Controllers { @@ -10,6 +11,6 @@ namespace Umbraco.Web.Common.Controllers /// /// Returns the to use as the current page for the request /// - IPublishedContent FindContent(); + IPublishedContent FindContent(ActionExecutingContext actionExecutingContext); } } diff --git a/src/Umbraco.Web.Common/Extensions/ControllerActionEndpointConventionBuilderExtensions.cs b/src/Umbraco.Web.Common/Extensions/ControllerActionEndpointConventionBuilderExtensions.cs index becb79a70e..d30436c87b 100644 --- a/src/Umbraco.Web.Common/Extensions/ControllerActionEndpointConventionBuilderExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/ControllerActionEndpointConventionBuilderExtensions.cs @@ -20,7 +20,7 @@ namespace Umbraco.Web.Common.Extensions /// public static void ForUmbracoPage( this ControllerActionEndpointConventionBuilder builder, - Func findContent) + Func findContent) => builder.Add(convention => { // filter out matched endpoints that are suppressed @@ -35,8 +35,18 @@ namespace Umbraco.Web.Common.Extensions // to execute in order to find the IPublishedContent for the request. var filter = new UmbracoVirtualPageFilterAttribute(); - actionDescriptor.FilterDescriptors.Add(new FilterDescriptor(filter, 0)); - convention.Metadata.Add(filter); + + // Check if this already contains this filter since we don't want it applied twice. + // This could occur if the controller being routed is IVirtualPageController AND + // is being routed with ForUmbracoPage. In that case, ForUmbracoPage wins + // because the UmbracoVirtualPageFilterAttribute will check for the metadata first since + // that is more explicit and flexible in case the same controller is routed multiple times. + if (!actionDescriptor.FilterDescriptors.Any(x => x.Filter is UmbracoVirtualPageFilterAttribute)) + { + actionDescriptor.FilterDescriptors.Add(new FilterDescriptor(filter, 0)); + convention.Metadata.Add(filter); + } + convention.Metadata.Add(new CustomRouteContentFinderDelegate(findContent)); } } diff --git a/src/Umbraco.Web.Common/Filters/UmbracoVirtualPageFilterAttribute.cs b/src/Umbraco.Web.Common/Filters/UmbracoVirtualPageFilterAttribute.cs index b6b372c14a..988608c2c2 100644 --- a/src/Umbraco.Web.Common/Filters/UmbracoVirtualPageFilterAttribute.cs +++ b/src/Umbraco.Web.Common/Filters/UmbracoVirtualPageFilterAttribute.cs @@ -2,6 +2,7 @@ using System; using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.Extensions.DependencyInjection; @@ -30,18 +31,22 @@ namespace Umbraco.Web.Common.Filters if (contentFinder != null) { - await SetUmbracoRouteValues(context, contentFinder.FindContent(context.HttpContext)); + await SetUmbracoRouteValues(context, contentFinder.FindContent(context)); } else { // Check if the controller is IVirtualPageController and then use that to FindContent if (context.Controller is IVirtualPageController ctrl) { - await SetUmbracoRouteValues(context, ctrl.FindContent()); + await SetUmbracoRouteValues(context, ctrl.FindContent(context)); } } - await next(); + // if we've assigned not found, just exit + if (!(context.Result is NotFoundResult)) + { + await next(); + } } private async Task SetUmbracoRouteValues(ActionExecutingContext context, IPublishedContent content) @@ -60,6 +65,11 @@ namespace Umbraco.Web.Common.Filters context.HttpContext.Features.Set(routeValues); } + else + { + // if there is no content then it should be a not found + context.Result = new NotFoundResult(); + } } } } diff --git a/src/Umbraco.Web.Common/Routing/CustomRouteContentFinderDelegate.cs b/src/Umbraco.Web.Common/Routing/CustomRouteContentFinderDelegate.cs index e81f5f75f8..5be3b4b952 100644 --- a/src/Umbraco.Web.Common/Routing/CustomRouteContentFinderDelegate.cs +++ b/src/Umbraco.Web.Common/Routing/CustomRouteContentFinderDelegate.cs @@ -1,15 +1,16 @@ using System; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Filters; using Umbraco.Core.Models.PublishedContent; namespace Umbraco.Web.Common.Extensions { internal class CustomRouteContentFinderDelegate { - private readonly Func _findContent; + private readonly Func _findContent; - public CustomRouteContentFinderDelegate(Func findContent) => _findContent = findContent; + public CustomRouteContentFinderDelegate(Func findContent) => _findContent = findContent; - public IPublishedContent FindContent(HttpContext httpContext) => _findContent(httpContext); + public IPublishedContent FindContent(ActionExecutingContext actionExecutingContext) => _findContent(actionExecutingContext); } }