From 0ed15db4089935582f7f1555d01cd896f9d5c308 Mon Sep 17 00:00:00 2001 From: Shannon Deminick Date: Thu, 13 Sep 2012 11:45:06 +0700 Subject: [PATCH] Fixed the NiceUrlProvider as it some refactor on a previous commit broke some stuff. Fixed up some unit tests with NiceUrlProvider to ensure that the second node under the root get's the correct URL assigned. Added unit test to ensure that route caches are persisted correctly. --- .../Routing/NiceUrlProviderTests.cs | 39 ++++++++++++++- src/Umbraco.Tests/TestHelpers/BaseWebTest.cs | 8 ++- src/Umbraco.Web/Routing/DefaultRoutesCache.cs | 22 ++++++-- src/Umbraco.Web/Routing/NiceUrlProvider.cs | 50 +++++++++++++------ src/Umbraco.Web/UmbracoModule.cs | 4 +- 5 files changed, 103 insertions(+), 20 deletions(-) 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;