diff --git a/src/Umbraco.Tests/Routing/NiceUrlProviderTests.cs b/src/Umbraco.Tests/Routing/NiceUrlProviderTests.cs index 098b491fc9..102bf8d744 100644 --- a/src/Umbraco.Tests/Routing/NiceUrlProviderTests.cs +++ b/src/Umbraco.Tests/Routing/NiceUrlProviderTests.cs @@ -1,5 +1,7 @@ +using System; using System.Configuration; using NUnit.Framework; +using Umbraco.Web.Routing; namespace Umbraco.Tests.Routing { @@ -15,6 +17,41 @@ namespace Umbraco.Tests.Routing ConfigurationManager.AppSettings.Set("umbracoHideTopLevelNodeFromPath", ""); } + internal override IRoutesCache GetRoutesCache() + { + return new DefaultRoutesCache(false); + } + + /// + /// This checks that when we retreive a NiceUrl for multiple items that there are no issues with cache overlap + /// and that they are all cached correctly. + /// + [Test] + public void Ensure_Cache_Is_Correct() + { + + var routingContext = GetRoutingContext("/test", 1111); + ConfigurationManager.AppSettings.Set("umbracoUseDirectoryUrls", "true"); + var ids = new[] + { + new Tuple(1046, "/home"), + new Tuple(1173, "/home/sub1"), + new Tuple(1174, "/home/sub1/sub2"), + new Tuple(1176, "/home/sub1/sub-3"), + new Tuple(1177, "/home/sub1/custom-sub-1"), + new Tuple(1178, "/home/sub1/custom-sub-2"), + new Tuple(1175, "/home/sub-2"), + new Tuple(1172, "/test-page") + }; + foreach(var i in ids) + { + var result = routingContext.NiceUrlProvider.GetNiceUrl(i.Item1); + Assert.AreEqual(i.Item2, result); + } + Assert.AreEqual(8, ((DefaultRoutesCache)routingContext.UmbracoContext.RoutesCache).GetCachedRoutes().Count); + Assert.AreEqual(8, ((DefaultRoutesCache)routingContext.UmbracoContext.RoutesCache).GetCachedIds().Count); + } + [TestCase(1046, "/home.aspx")] [TestCase(1173, "/home/sub1.aspx")] [TestCase(1174, "/home/sub1/sub2.aspx")] @@ -65,7 +102,7 @@ namespace Umbraco.Tests.Routing ConfigurationManager.AppSettings.Set("umbracoHideTopLevelNodeFromPath", "true"); - var result = routingContext.NiceUrlProvider.GetNiceUrl(nodeId); + var result = routingContext.NiceUrlProvider.GetNiceUrl(nodeId); Assert.AreEqual(niceUrlMatch, result); } diff --git a/src/Umbraco.Tests/TestHelpers/BaseWebTest.cs b/src/Umbraco.Tests/TestHelpers/BaseWebTest.cs index d8e4c15b90..06addb5738 100644 --- a/src/Umbraco.Tests/TestHelpers/BaseWebTest.cs +++ b/src/Umbraco.Tests/TestHelpers/BaseWebTest.cs @@ -5,6 +5,7 @@ using Umbraco.Core; using Umbraco.Core.ObjectResolution; using Umbraco.Tests.Stubs; using Umbraco.Web; +using Umbraco.Web.Routing; using umbraco.BusinessLogic; using umbraco.cms.businesslogic.cache; using umbraco.cms.businesslogic.template; @@ -59,12 +60,17 @@ namespace Umbraco.Tests.TestHelpers protected ApplicationContext ApplicationContext { get; private set; } + internal virtual IRoutesCache GetRoutesCache() + { + return new FakeRoutesCache(); + } + protected UmbracoContext GetUmbracoContext(string url, int templateId, RouteData routeData = null) { var ctx = new UmbracoContext( GetHttpContextFactory(url, routeData).HttpContext, ApplicationContext, - new FakeRoutesCache()); + GetRoutesCache()); SetupUmbracoContextForTest(ctx, templateId); return ctx; } diff --git a/src/Umbraco.Web/Routing/DefaultRoutesCache.cs b/src/Umbraco.Web/Routing/DefaultRoutesCache.cs index a5a7bf8d02..4f6082e448 100644 --- a/src/Umbraco.Web/Routing/DefaultRoutesCache.cs +++ b/src/Umbraco.Web/Routing/DefaultRoutesCache.cs @@ -39,11 +39,27 @@ namespace Umbraco.Web.Routing global::umbraco.content.AfterRefreshContent += (sender, e) => Clear(); global::umbraco.content.AfterUpdateDocumentCache += (sender, e) => Clear(); - } - - + } } + /// + /// Used ONLY for unit tests + /// + /// + internal IDictionary GetCachedRoutes() + { + return _routes; + } + + /// + /// Used ONLY for unit tests + /// + /// + internal IDictionary GetCachedIds() + { + return _nodeIds; + } + /// /// Stores a route for a node. /// diff --git a/src/Umbraco.Web/Routing/NiceUrlProvider.cs b/src/Umbraco.Web/Routing/NiceUrlProvider.cs index a11a89fa57..41addb9c5c 100644 --- a/src/Umbraco.Web/Routing/NiceUrlProvider.cs +++ b/src/Umbraco.Web/Routing/NiceUrlProvider.cs @@ -4,6 +4,7 @@ using System.Linq; using System.Web; using Umbraco.Core; +using Umbraco.Core.Logging; using Umbraco.Web.Routing; using umbraco; @@ -30,10 +31,7 @@ namespace Umbraco.Web.Routing private readonly UmbracoContext _umbracoContext; private readonly IPublishedContentStore _publishedContentStore; - - // note: this could be a parameter... - const string UrlNameProperty = "@urlName"; - + #region GetNiceUrl /// @@ -70,37 +68,61 @@ namespace Umbraco.Web.Routing } else { - var node = _publishedContentStore.GetDocumentById(_umbracoContext, nodeId); - if (node == null) - return "#"; // legacy wrote to the log here... + var originalNode = _publishedContentStore.GetDocumentById(_umbracoContext, nodeId); + if (originalNode == null) + { + LogHelper.Warn( + "Couldn't find any page with the nodeId = {0}. This is most likely caused by the page isn't published!", + nodeId); + + return "#"; + } + var pathParts = new List(); int id = nodeId; domainUri = DomainUriAtNode(id, current); + var recursiveNode = originalNode; while (domainUri == null && id > 0) { - var urlName = node.GetPropertyValue(UrlNameProperty); + var urlName = recursiveNode.UrlName; pathParts.Add(urlName); - node = node.Parent; // set to parent node - if (node == null) + recursiveNode = recursiveNode.Parent; // set to parent node + if (recursiveNode == null) { id = -1; } else { - id = node.Id; + id = recursiveNode.Id; } domainUri = id > 0 ? DomainUriAtNode(id, current) : null; } // no domain, respect HideTopLevelNodeFromPath for legacy purposes if (domainUri == null && global::umbraco.GlobalSettings.HideTopLevelNodeFromPath) - pathParts.RemoveAt(pathParts.Count - 1); + { + //here we need to determine if this node is another root node (next sibling(s) to the first) because + // in that case, we do not remove the path part. In theory, like in v5, this node would require + // a domain assigned but to maintain compatibility we'll add this check + if (originalNode.Parent == null) + { + var rootNode = _publishedContentStore.GetDocumentByRoute(_umbracoContext, "/", true); + if (rootNode.Id == originalNode.Id) + { + pathParts.RemoveAt(pathParts.Count - 1); + } + } + else + { + pathParts.RemoveAt(pathParts.Count - 1); + } + } + pathParts.Reverse(); route = "/" + string.Join("/", pathParts); - //FIX THIS, it stores over the top of a real route! if (!_umbracoContext.InPreviewMode) _umbracoContext.RoutesCache.Store(nodeId, route); } @@ -147,7 +169,7 @@ namespace Umbraco.Web.Routing domainUris = DomainUrisAtNode(id, current); while (!domainUris.Any() && id > 0) { - var urlName = node.GetPropertyValue(UrlNameProperty); + var urlName = node.UrlName; pathParts.Add(urlName); node = node.Parent; //set to parent node id = node.Id; diff --git a/src/Umbraco.Web/UmbracoModule.cs b/src/Umbraco.Web/UmbracoModule.cs index 5b4768439d..44482f3d60 100644 --- a/src/Umbraco.Web/UmbracoModule.cs +++ b/src/Umbraco.Web/UmbracoModule.cs @@ -77,7 +77,9 @@ namespace Umbraco.Web //Create a document request since we are rendering a document on the front-end // create the new document request - var docreq = new DocumentRequest(uri, routingContext); + var docreq = new DocumentRequest( + umbracoContext.UmbracoUrl, //very important to use this url! it is the path only lowercased version of the current URL. + routingContext); //assign the document request to the umbraco context now that we know its a front end request umbracoContext.DocumentRequest = docreq;