From a6f9598a23e41687d2d6b0f6b0e518cf830ab825 Mon Sep 17 00:00:00 2001 From: Stephan Date: Thu, 31 Jan 2019 09:08:51 +0100 Subject: [PATCH] Introduce IPublishedRouter --- .../TestControllerActivatorBase.cs | 2 +- .../Testing/TestingTests/MockTests.cs | 2 +- src/Umbraco.Tests/Testing/UmbracoTestBase.cs | 2 +- .../Web/Mvc/SurfaceControllerTests.cs | 2 +- .../Models/Mapping/ContentUrlResolver.cs | 4 +- .../EnsurePublishedContentRequestAttribute.cs | 8 ++- src/Umbraco.Web/Mvc/RenderRouteHandler.cs | 2 +- .../Mvc/UmbracoVirtualNodeRouteHandler.cs | 2 +- src/Umbraco.Web/Routing/IPublishedRouter.cs | 54 +++++++++++++++++++ src/Umbraco.Web/Routing/PublishedRequest.cs | 8 +-- src/Umbraco.Web/Routing/PublishedRouter.cs | 43 +++++---------- .../Routing/UrlProviderExtensions.cs | 6 +-- src/Umbraco.Web/Runtime/WebRuntimeComposer.cs | 2 +- src/Umbraco.Web/Security/MembershipHelper.cs | 27 +++++++--- src/Umbraco.Web/Templates/TemplateRenderer.cs | 2 +- src/Umbraco.Web/Umbraco.Web.csproj | 1 + src/Umbraco.Web/UmbracoInjectedModule.cs | 4 +- 17 files changed, 111 insertions(+), 60 deletions(-) create mode 100644 src/Umbraco.Web/Routing/IPublishedRouter.cs diff --git a/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestControllerActivatorBase.cs b/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestControllerActivatorBase.cs index 602b5907d8..6414ea469d 100644 --- a/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestControllerActivatorBase.cs +++ b/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestControllerActivatorBase.cs @@ -150,7 +150,7 @@ namespace Umbraco.Tests.TestHelpers.ControllerTesting urlHelper.Setup(provider => provider.GetUrl(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Returns(UrlInfo.Url("/hello/world/1234")); - var membershipHelper = new MembershipHelper(new TestUmbracoContextAccessor(umbCtx), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), null, Mock.Of(), Mock.Of()); + var membershipHelper = new MembershipHelper(new TestUmbracoContextAccessor(umbCtx), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of()); var umbHelper = new UmbracoHelper(umbCtx, Mock.Of(), diff --git a/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs b/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs index 575f14e818..622186a547 100644 --- a/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs +++ b/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs @@ -65,7 +65,7 @@ namespace Umbraco.Tests.Testing.TestingTests Mock.Of(), Mock.Of(), Mock.Of(), - new MembershipHelper(new TestUmbracoContextAccessor(umbracoContext), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), null, Mock.Of(), Mock.Of()), + new MembershipHelper(new TestUmbracoContextAccessor(umbracoContext), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of()), ServiceContext.CreatePartial()); Assert.Pass(); } diff --git a/src/Umbraco.Tests/Testing/UmbracoTestBase.cs b/src/Umbraco.Tests/Testing/UmbracoTestBase.cs index 9616a26891..7c8d1100f8 100644 --- a/src/Umbraco.Tests/Testing/UmbracoTestBase.cs +++ b/src/Umbraco.Tests/Testing/UmbracoTestBase.cs @@ -213,7 +213,7 @@ namespace Umbraco.Tests.Testing // web Composition.RegisterUnique(_ => Umbraco.Web.Composing.Current.UmbracoContextAccessor); - Composition.RegisterUnique(); + Composition.RegisterUnique(); Composition.WithCollectionBuilder(); Composition.RegisterUnique(); Composition.RegisterUnique(); diff --git a/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs b/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs index 69f6c5d13e..ec37793c10 100644 --- a/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs +++ b/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs @@ -132,7 +132,7 @@ namespace Umbraco.Tests.Web.Mvc Mock.Of(), Mock.Of(), Mock.Of(), - new MembershipHelper(new TestUmbracoContextAccessor(umbracoContext), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), null, Mock.Of(), Mock.Of()), + new MembershipHelper(new TestUmbracoContextAccessor(umbracoContext), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of()), ServiceContext.CreatePartial()); var ctrl = new TestSurfaceController(umbracoContext, helper); diff --git a/src/Umbraco.Web/Models/Mapping/ContentUrlResolver.cs b/src/Umbraco.Web/Models/Mapping/ContentUrlResolver.cs index f09638330b..42c4997d86 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentUrlResolver.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentUrlResolver.cs @@ -11,7 +11,7 @@ namespace Umbraco.Web.Models.Mapping internal class ContentUrlResolver : IValueResolver { private readonly IUmbracoContextAccessor _umbracoContextAccessor; - private readonly PublishedRouter _publishedRouter; + private readonly IPublishedRouter _publishedRouter; private readonly ILocalizationService _localizationService; private readonly ILocalizedTextService _textService; private readonly IContentService _contentService; @@ -19,7 +19,7 @@ namespace Umbraco.Web.Models.Mapping public ContentUrlResolver( IUmbracoContextAccessor umbracoContextAccessor, - PublishedRouter publishedRouter, + IPublishedRouter publishedRouter, ILocalizationService localizationService, ILocalizedTextService textService, IContentService contentService, diff --git a/src/Umbraco.Web/Mvc/EnsurePublishedContentRequestAttribute.cs b/src/Umbraco.Web/Mvc/EnsurePublishedContentRequestAttribute.cs index 62a7c48d2b..471cec6db7 100644 --- a/src/Umbraco.Web/Mvc/EnsurePublishedContentRequestAttribute.cs +++ b/src/Umbraco.Web/Mvc/EnsurePublishedContentRequestAttribute.cs @@ -33,8 +33,7 @@ namespace Umbraco.Web.Mvc /// public EnsurePublishedContentRequestAttribute(UmbracoContext umbracoContext, int contentId) { - if (umbracoContext == null) throw new ArgumentNullException(nameof(umbracoContext)); - _umbracoContext = umbracoContext; + _umbracoContext = umbracoContext ?? throw new ArgumentNullException(nameof(umbracoContext)); _contentId = contentId; } @@ -63,8 +62,7 @@ namespace Umbraco.Web.Mvc /// public EnsurePublishedContentRequestAttribute(UmbracoContext umbracoContext, string dataTokenName) { - if (umbracoContext == null) throw new ArgumentNullException(nameof(umbracoContext)); - _umbracoContext = umbracoContext; + _umbracoContext = umbracoContext ?? throw new ArgumentNullException(nameof(umbracoContext)); _dataTokenName = dataTokenName; } @@ -74,7 +72,7 @@ namespace Umbraco.Web.Mvc protected UmbracoContext UmbracoContext => _umbracoContext ?? (_umbracoContext = UmbracoContext.Current); // TODO: try lazy property injection? - private PublishedRouter PublishedRouter => Core.Composing.Current.Factory.GetInstance(); + private IPublishedRouter PublishedRouter => Core.Composing.Current.Factory.GetInstance(); /// /// Exposes an UmbracoHelper diff --git a/src/Umbraco.Web/Mvc/RenderRouteHandler.cs b/src/Umbraco.Web/Mvc/RenderRouteHandler.cs index 215eb39573..2c550effc4 100644 --- a/src/Umbraco.Web/Mvc/RenderRouteHandler.cs +++ b/src/Umbraco.Web/Mvc/RenderRouteHandler.cs @@ -377,7 +377,7 @@ namespace Umbraco.Web.Mvc if ((request.HasTemplate == false && Features.Disabled.DisableTemplates == false) && routeDef.HasHijackedRoute == false) { - request.UpdateOnMissingTemplate(); // request will go 404 + request.UpdateToNotFound(); // request will go 404 // HandleHttpResponseStatus returns a value indicating that the request should // not be processed any further, eg because it has been redirect. then, exit. diff --git a/src/Umbraco.Web/Mvc/UmbracoVirtualNodeRouteHandler.cs b/src/Umbraco.Web/Mvc/UmbracoVirtualNodeRouteHandler.cs index 48e16b48d7..1af434e94e 100644 --- a/src/Umbraco.Web/Mvc/UmbracoVirtualNodeRouteHandler.cs +++ b/src/Umbraco.Web/Mvc/UmbracoVirtualNodeRouteHandler.cs @@ -12,7 +12,7 @@ namespace Umbraco.Web.Mvc public abstract class UmbracoVirtualNodeRouteHandler : IRouteHandler { // TODO: try lazy property injection? - private PublishedRouter PublishedRouter => Core.Composing.Current.Factory.GetInstance(); + private IPublishedRouter PublishedRouter => Current.Factory.GetInstance(); /// /// Returns the UmbracoContext for this route handler diff --git a/src/Umbraco.Web/Routing/IPublishedRouter.cs b/src/Umbraco.Web/Routing/IPublishedRouter.cs new file mode 100644 index 0000000000..df06e42e55 --- /dev/null +++ b/src/Umbraco.Web/Routing/IPublishedRouter.cs @@ -0,0 +1,54 @@ +using System; +using System.Collections.Generic; +using Umbraco.Core.Models; + +namespace Umbraco.Web.Routing +{ + /// + /// Routes requests. + /// + public interface IPublishedRouter + { + // TODO: consider this and RenderRouteHandler - move some code around? + + /// + /// Creates a published request. + /// + /// The current Umbraco context. + /// The (optional) request Uri. + /// A published request. + PublishedRequest CreateRequest(UmbracoContext umbracoContext, Uri uri = null); + + /// + /// Prepares a request for rendering. + /// + /// The request. + /// A value indicating whether the request was successfully prepared and can be rendered. + bool PrepareRequest(PublishedRequest request); + + /// + /// Tries to route a request. + /// + /// The request. + /// A value indicating whether the request can be routed to a document. + bool TryRouteRequest(PublishedRequest request); + + /// + /// Gets a template. + /// + /// The template alias + /// The template. + ITemplate GetTemplate(string alias); + + /// + /// Updates the request to "not found". + /// + /// The request. + /// + /// This method is invoked when the pipeline decides it cannot render + /// the request, for whatever reason, and wants to force it to be re-routed + /// and rendered as if no document were found (404). + /// + void UpdateRequestToNotFound(PublishedRequest request); + } +} diff --git a/src/Umbraco.Web/Routing/PublishedRequest.cs b/src/Umbraco.Web/Routing/PublishedRequest.cs index c5475f8b73..52f5820744 100644 --- a/src/Umbraco.Web/Routing/PublishedRequest.cs +++ b/src/Umbraco.Web/Routing/PublishedRequest.cs @@ -17,7 +17,7 @@ namespace Umbraco.Web.Routing /// public class PublishedRequest { - private readonly PublishedRouter _publishedRouter; + private readonly IPublishedRouter _publishedRouter; private bool _readonly; // after prepared private bool _readonlyUri; // after preparing @@ -35,7 +35,7 @@ namespace Umbraco.Web.Routing /// The published router. /// The Umbraco context. /// The request Uri. - internal PublishedRequest(PublishedRouter publishedRouter, UmbracoContext umbracoContext, Uri uri = null) + internal PublishedRequest(IPublishedRouter publishedRouter, UmbracoContext umbracoContext, Uri uri = null) { UmbracoContext = umbracoContext ?? throw new ArgumentNullException(nameof(umbracoContext)); _publishedRouter = publishedRouter ?? throw new ArgumentNullException(nameof(publishedRouter)); @@ -291,11 +291,11 @@ namespace Umbraco.Web.Routing /// public bool HasTemplate => TemplateModel != null; - internal void UpdateOnMissingTemplate() + internal void UpdateToNotFound() { var __readonly = _readonly; _readonly = false; - _publishedRouter.UpdateRequestOnMissingTemplate(this); + _publishedRouter.UpdateRequestToNotFound(this); _readonly = __readonly; } diff --git a/src/Umbraco.Web/Routing/PublishedRouter.cs b/src/Umbraco.Web/Routing/PublishedRouter.cs index 14c36198d8..7dd1ec9f1a 100644 --- a/src/Umbraco.Web/Routing/PublishedRouter.cs +++ b/src/Umbraco.Web/Routing/PublishedRouter.cs @@ -6,6 +6,7 @@ using System.Globalization; using System.IO; using System.Web.Security; using Umbraco.Core; +using Umbraco.Core.Cache; using Umbraco.Core.Composing; using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.IO; @@ -18,8 +19,10 @@ using Umbraco.Web.Security; namespace Umbraco.Web.Routing { - // TODO: making sense to have an interface? - public class PublishedRouter + /// + /// Provides the default implementation. + /// + public class PublishedRouter : IPublishedRouter { private readonly IWebRoutingSection _webRoutingSection; private readonly ContentFinderCollection _contentFinders; @@ -47,15 +50,9 @@ namespace Umbraco.Web.Routing _profilingLogger = proflog ?? throw new ArgumentNullException(nameof(proflog)); _variationContextAccessor = variationContextAccessor ?? throw new ArgumentNullException(nameof(variationContextAccessor)); _logger = proflog; - - GetRolesForLogin = s => Roles.Provider.GetRolesForUser(s); } - // TODO: in 7.7 this is cached in the PublishedContentRequest, which ... makes little sense - // killing it entirely, if we need cache, just implement it properly !! - // this is all so weird - public Func> GetRolesForLogin { get; } - + /// public PublishedRequest CreateRequest(UmbracoContext umbracoContext, Uri uri = null) { return new PublishedRequest(this, umbracoContext, uri ?? umbracoContext.CleanedUmbracoUrl); @@ -63,10 +60,8 @@ namespace Umbraco.Web.Routing #region Request - /// - /// Tries to route the request. - /// - internal bool TryRouteRequest(PublishedRequest request) + /// + public bool TryRouteRequest(PublishedRequest request) { // disabled - is it going to change the routing? //_pcr.OnPreparing(); @@ -96,12 +91,7 @@ namespace Umbraco.Web.Routing _variationContextAccessor.VariationContext = new VariationContext(culture); } - /// - /// Prepares the request. - /// - /// - /// Returns false if the request was not successfully prepared - /// + /// public bool PrepareRequest(PublishedRequest request) { // note - at that point the original legacy module did something do handle IIS custom 404 errors @@ -213,11 +203,8 @@ namespace Umbraco.Web.Routing return true; } - /// - /// Updates the request when there is no template to render the content. - /// - /// This is called from Mvc when there's a document to render but no template. - public void UpdateRequestOnMissingTemplate(PublishedRequest request) + /// + public void UpdateRequestToNotFound(PublishedRequest request) { // clear content var content = request.PublishedContent; @@ -386,11 +373,7 @@ namespace Umbraco.Web.Routing #region Document and template - /// - /// Gets a template. - /// - /// The template alias - /// The template. + /// public ITemplate GetTemplate(string alias) { return _services.FileService.GetTemplate(alias); @@ -607,7 +590,7 @@ namespace Umbraco.Web.Routing if (loginPageId != request.PublishedContent.Id) request.PublishedContent = request.UmbracoContext.PublishedSnapshot.Content.GetById(loginPageId); } - else if (_services.PublicAccessService.HasAccess(request.PublishedContent.Id, _services.ContentService, membershipHelper.CurrentUserName, GetRolesForLogin(membershipHelper.CurrentUserName)) == false) + else if (_services.PublicAccessService.HasAccess(request.PublishedContent.Id, _services.ContentService, membershipHelper.CurrentUserName, membershipHelper.GetCurrentUserRoles()) == false) { _logger.Debug("EnsurePublishedContentAccess: Current member has not access, redirect to error page"); var errorPageId = publicAccessAttempt.Result.NoAccessNodeId; diff --git a/src/Umbraco.Web/Routing/UrlProviderExtensions.cs b/src/Umbraco.Web/Routing/UrlProviderExtensions.cs index 8db657ed30..2a840828b6 100644 --- a/src/Umbraco.Web/Routing/UrlProviderExtensions.cs +++ b/src/Umbraco.Web/Routing/UrlProviderExtensions.cs @@ -18,7 +18,7 @@ namespace Umbraco.Web.Routing /// Contains all the Urls that we can figure out (based upon domains, etc). /// public static IEnumerable GetContentUrls(this IContent content, - PublishedRouter publishedRouter, + IPublishedRouter publishedRouter, UmbracoContext umbracoContext, ILocalizationService localizationService, ILocalizedTextService textService, @@ -92,7 +92,7 @@ namespace Umbraco.Web.Routing /// private static IEnumerable GetContentUrlsByCulture(IContent content, IEnumerable cultures, - PublishedRouter publishedRouter, + IPublishedRouter publishedRouter, UmbracoContext umbracoContext, IContentService contentService, ILocalizedTextService textService, @@ -161,7 +161,7 @@ namespace Umbraco.Web.Routing return UrlInfo.Message(textService.Localize("content/parentCultureNotPublished", new[] {parent.Name}), culture); } - private static bool DetectCollision(IContent content, string url, string culture, UmbracoContext umbracoContext, PublishedRouter publishedRouter, ILocalizedTextService textService, out UrlInfo urlInfo) + private static bool DetectCollision(IContent content, string url, string culture, UmbracoContext umbracoContext, IPublishedRouter publishedRouter, ILocalizedTextService textService, out UrlInfo urlInfo) { // test for collisions on the 'main' url var uri = new Uri(url.TrimEnd('/'), UriKind.RelativeOrAbsolute); diff --git a/src/Umbraco.Web/Runtime/WebRuntimeComposer.cs b/src/Umbraco.Web/Runtime/WebRuntimeComposer.cs index de3ca47b38..4070ee9ece 100644 --- a/src/Umbraco.Web/Runtime/WebRuntimeComposer.cs +++ b/src/Umbraco.Web/Runtime/WebRuntimeComposer.cs @@ -178,7 +178,7 @@ namespace Umbraco.Web.Runtime composition.RegisterAuto(typeof(UmbracoViewPage<>)); // register published router - composition.RegisterUnique(); + composition.RegisterUnique(); composition.Register(_ => Current.Configs.Settings().WebRouting); // register preview SignalR hub diff --git a/src/Umbraco.Web/Security/MembershipHelper.cs b/src/Umbraco.Web/Security/MembershipHelper.cs index e053617806..daec4bb1f7 100644 --- a/src/Umbraco.Web/Security/MembershipHelper.cs +++ b/src/Umbraco.Web/Security/MembershipHelper.cs @@ -31,7 +31,6 @@ namespace Umbraco.Web.Security private readonly IMemberTypeService _memberTypeService; private readonly IUserService _userService; private readonly IPublicAccessService _publicAccessService; - private readonly PublishedRouter _publishedRouter; private readonly AppCaches _appCaches; private readonly ILogger _logger; @@ -46,7 +45,6 @@ namespace Umbraco.Web.Security IMemberTypeService memberTypeService, IUserService userService, IPublicAccessService publicAccessService, - PublishedRouter publishedRouter, AppCaches appCaches, ILogger logger ) @@ -57,7 +55,6 @@ namespace Umbraco.Web.Security _memberTypeService = memberTypeService; _userService = userService; _publicAccessService = publicAccessService; - _publishedRouter = publishedRouter; _appCaches = appCaches; _logger = logger; @@ -116,7 +113,7 @@ namespace Umbraco.Web.Security { return UmbracoContext.PublishedRequest == null ? _publicAccessService.HasAccess(path, CurrentUserName, roleProvider.GetRolesForUser) - : _publicAccessService.HasAccess(path, CurrentUserName, _publishedRouter.GetRolesForLogin); + : _publicAccessService.HasAccess(path, CurrentUserName, GetUserRoles); } /// @@ -514,6 +511,24 @@ namespace Umbraco.Web.Security } #endregion + /// + /// Gets the current user's roles. + /// + /// Roles are cached per user name, at request level. + public IEnumerable GetCurrentUserRoles() + => GetUserRoles(CurrentUserName); + + /// + /// Gets a user's roles. + /// + /// Roles are cached per user name, at request level. + public IEnumerable GetUserRoles(string userName) + { + // optimize by caching per-request (v7 cached per PublishedRequest, in PublishedRouter) + var key = "Umbraco.Web.Security.MembershipHelper__Roles__" + userName; + return _appCaches.RequestCache.GetCacheItem(key, () => Roles.Provider.GetRolesForUser(userName)); + } + /// /// Returns the login status model of the currently logged in member, if no member is logged in it returns null; /// @@ -618,7 +633,7 @@ namespace Umbraco.Web.Security var provider = _membershipProvider; string username; - + if (provider.IsUmbracoMembershipProvider()) { var member = GetCurrentPersistedMember(); @@ -626,7 +641,7 @@ namespace Umbraco.Web.Security if (member == null) return false; username = member.Username; - + // If types defined, check member is of one of those types var allowTypesList = allowTypes as IList ?? allowTypes.ToList(); if (allowTypesList.Any(allowType => allowType != string.Empty)) diff --git a/src/Umbraco.Web/Templates/TemplateRenderer.cs b/src/Umbraco.Web/Templates/TemplateRenderer.cs index 9279eea124..7d678b93ca 100644 --- a/src/Umbraco.Web/Templates/TemplateRenderer.cs +++ b/src/Umbraco.Web/Templates/TemplateRenderer.cs @@ -36,7 +36,7 @@ namespace Umbraco.Web.Templates } private IFileService FileService => Current.Services.FileService; // TODO: inject - private PublishedRouter PublishedRouter => Core.Composing.Current.Factory.GetInstance(); // TODO: inject + private IPublishedRouter PublishedRouter => Core.Composing.Current.Factory.GetInstance(); // TODO: inject /// diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index f078e9d79f..709d938ba4 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -183,6 +183,7 @@ + diff --git a/src/Umbraco.Web/UmbracoInjectedModule.cs b/src/Umbraco.Web/UmbracoInjectedModule.cs index 95819e79d7..ba1a384484 100644 --- a/src/Umbraco.Web/UmbracoInjectedModule.cs +++ b/src/Umbraco.Web/UmbracoInjectedModule.cs @@ -45,7 +45,7 @@ namespace Umbraco.Web private readonly UrlProviderCollection _urlProviders; private readonly IRuntimeState _runtime; private readonly ILogger _logger; - private readonly PublishedRouter _publishedRouter; + private readonly IPublishedRouter _publishedRouter; private readonly IVariationContextAccessor _variationContextAccessor; public UmbracoInjectedModule( @@ -56,7 +56,7 @@ namespace Umbraco.Web UrlProviderCollection urlProviders, IRuntimeState runtime, ILogger logger, - PublishedRouter publishedRouter, + IPublishedRouter publishedRouter, IVariationContextAccessor variationContextAccessor) { _combinedRouteCollection = new Lazy(CreateRouteCollection);