diff --git a/src/Umbraco.Tests/Routing/NiceUrlProviderTests.cs b/src/Umbraco.Tests/Routing/NiceUrlProviderTests.cs
index c6970cbeb3..a8e5b1e64d 100644
--- a/src/Umbraco.Tests/Routing/NiceUrlProviderTests.cs
+++ b/src/Umbraco.Tests/Routing/NiceUrlProviderTests.cs
@@ -32,7 +32,7 @@ namespace Umbraco.Tests.Routing
}
///
- /// This checks that when we retrieve a NiceUrl for multiple items that there are no issues with cache overlap
+ /// This checks that when we retrieve a NiceUrl for multiple items that there are no issues with cache overlap
/// and that they are all cached correctly.
///
[Test]
@@ -71,11 +71,19 @@ namespace Umbraco.Tests.Routing
var cache = routingContext.UmbracoContext.ContentCache.InnerCache as PublishedContentCache;
if (cache == null) throw new Exception("Unsupported IPublishedContentCache, only the Xml one is supported.");
- var cachedRoutes = cache.RoutesCache.GetCachedRoutes();
- // GetUrl does not write to cache
- Assert.AreEqual(0, cachedRoutes.Count);
- }
+ var cachedRoutes = cache.RoutesCache.GetCachedRoutes();
+ Assert.AreEqual(8, cachedRoutes.Count);
+
+ foreach (var sample in samples)
+ {
+ Assert.IsTrue(cachedRoutes.ContainsKey(sample.Key));
+ Assert.AreEqual(sample.Value, cachedRoutes[sample.Key]);
+ }
+
+ var cachedIds = cache.RoutesCache.GetCachedIds();
+ Assert.AreEqual(0, cachedIds.Count);
+ }
// test hideTopLevelNodeFromPath false
[TestCase(1046, "/home/")]
@@ -133,10 +141,10 @@ namespace Umbraco.Tests.Routing
var routingContext = GetRoutingContext("http://example.com/test", 1111, umbracoSettings: _umbracoSettings);
-
+
Assert.AreEqual("/home/sub1/custom-sub-1/", routingContext.UrlProvider.GetUrl(1177));
-
+
requestMock.Setup(x => x.UseDomainPrefixes).Returns(true);
Assert.AreEqual("http://example.com/home/sub1/custom-sub-1/", routingContext.UrlProvider.GetUrl(1177));
@@ -159,14 +167,14 @@ namespace Umbraco.Tests.Routing
Assert.AreEqual("#", routingContext.UrlProvider.GetUrl(999999));
- requestMock.Setup(x => x.UseDomainPrefixes).Returns(true);
-
+ requestMock.Setup(x => x.UseDomainPrefixes).Returns(true);
+
Assert.AreEqual("#", routingContext.UrlProvider.GetUrl(999999));
requestMock.Setup(x => x.UseDomainPrefixes).Returns(false);
routingContext.UrlProvider.Mode = UrlProviderMode.Absolute;
-
+
Assert.AreEqual("#", routingContext.UrlProvider.GetUrl(999999));
}
}
diff --git a/src/Umbraco.Tests/Routing/NiceUrlRoutesTests.cs b/src/Umbraco.Tests/Routing/NiceUrlRoutesTests.cs
index 79f2b2b64d..131107e8e6 100644
--- a/src/Umbraco.Tests/Routing/NiceUrlRoutesTests.cs
+++ b/src/Umbraco.Tests/Routing/NiceUrlRoutesTests.cs
@@ -7,6 +7,10 @@ using Umbraco.Web.Routing;
namespace Umbraco.Tests.Routing
{
+ // purpose: test the values returned by PublishedContentCache.GetRouteById
+ // and .GetByRoute (no caching at all, just routing nice urls) including all
+ // the quirks due to hideTopLevelFromPath and backward compatibility.
+
[DatabaseTestBehavior(DatabaseBehavior.NewDbFileAndSchemaPerFixture)]
[TestFixture]
public class NiceUrlRoutesTests : BaseRoutingTest
@@ -85,7 +89,7 @@ GetByRoute(route, hide = null):
# there are not other reason not to cache it
if content and no domain between root and content:
- cache route
+ cache route (as trusted)
return content
@@ -136,7 +140,11 @@ GetRouteById(id):
# never cache the route, it may be colliding
- return DetermineRouteById(id)
+ route = DetermineRouteById(id)
+ if route:
+ cache route (as not trusted)
+
+ return route
@@ -180,8 +188,6 @@ DetermineRouteById(id):
* C 2005
* E 2006
*
- * And the tests should verify all the quirks that are due to
- * hideTopLevelFromPath
*/
[TestCase(1000, false, "/a")]
@@ -214,10 +220,40 @@ DetermineRouteById(id):
SettingsForTests.HideTopLevelNodeFromPath = hide;
- var route = cache.GetRouteById(umbracoContext, false, id);
+ const bool preview = true; // make sure we don't cache
+ var route = cache.GetRouteById(umbracoContext, preview, id);
Assert.AreEqual(expected, route);
}
+ [Test]
+ public void GetRouteByIdCache()
+ {
+ var umbracoContext = GetUmbracoContext("/test", 0);
+ var cache = umbracoContext.ContentCache.InnerCache as PublishedContentCache;
+ if (cache == null) throw new Exception("Unsupported IPublishedContentCache, only the Xml one is supported.");
+
+ SettingsForTests.HideTopLevelNodeFromPath = false;
+
+ // make sure we cache
+ PublishedContentCache.UnitTesting = false;
+ const bool preview = false;
+
+ var route = cache.GetRouteById(umbracoContext, preview, 1000);
+ Assert.AreEqual("/a", route);
+
+ // GetRouteById registers a non-trusted route, which is cached for
+ // id -> route queries (fast GetUrl) but *not* for route -> id
+ // queries (safe inbound routing)
+
+ var cachedRoutes = cache.RoutesCache.GetCachedRoutes();
+ Assert.AreEqual(1, cachedRoutes.Count);
+ Assert.IsTrue(cachedRoutes.ContainsKey(1000));
+ Assert.AreEqual("/a", cachedRoutes[1000]);
+
+ var cachedIds = cache.RoutesCache.GetCachedIds();
+ Assert.AreEqual(0, cachedIds.Count);
+ }
+
[TestCase("/", false, 1000)]
[TestCase("/a", false, 1000)] // yes!
[TestCase("/a/b", false, 1001)]
@@ -241,7 +277,8 @@ DetermineRouteById(id):
SettingsForTests.HideTopLevelNodeFromPath = hide;
- var content = cache.GetByRoute(umbracoContext, false, route);
+ const bool preview = true; // make sure we don't cache
+ var content = cache.GetByRoute(umbracoContext, preview, route);
if (expected < 0)
{
Assert.IsNull(content);
@@ -252,5 +289,37 @@ DetermineRouteById(id):
Assert.AreEqual(expected, content.Id);
}
}
+
+ [Test]
+ public void GetByRouteCache()
+ {
+ var umbracoContext = GetUmbracoContext("/test", 0);
+ var cache = umbracoContext.ContentCache.InnerCache as PublishedContentCache;
+ if (cache == null) throw new Exception("Unsupported IPublishedContentCache, only the Xml one is supported.");
+
+ SettingsForTests.HideTopLevelNodeFromPath = false;
+
+ // make sure we cache
+ PublishedContentCache.UnitTesting = false;
+ const bool preview = false;
+
+ var content = cache.GetByRoute(umbracoContext, preview, "/a/b/c");
+ Assert.IsNotNull(content);
+ Assert.AreEqual(1002, content.Id);
+
+ // GetByRoute registers a trusted route, which is cached both for
+ // id -> route queries (fast GetUrl) and for route -> id queries
+ // (fast inbound routing)
+
+ var cachedRoutes = cache.RoutesCache.GetCachedRoutes();
+ Assert.AreEqual(1, cachedRoutes.Count);
+ Assert.IsTrue(cachedRoutes.ContainsKey(1002));
+ Assert.AreEqual("/a/b/c", cachedRoutes[1002]);
+
+ var cachedIds = cache.RoutesCache.GetCachedIds();
+ Assert.AreEqual(1, cachedIds.Count);
+ Assert.IsTrue(cachedIds.ContainsKey("/a/b/c"));
+ Assert.AreEqual(1002, cachedIds["/a/b/c"]);
+ }
}
}
diff --git a/src/Umbraco.Tests/Routing/NiceUrlsProviderWithDomainsTests.cs b/src/Umbraco.Tests/Routing/NiceUrlsProviderWithDomainsTests.cs
index 6e71efcbf3..ab25e0b43c 100644
--- a/src/Umbraco.Tests/Routing/NiceUrlsProviderWithDomainsTests.cs
+++ b/src/Umbraco.Tests/Routing/NiceUrlsProviderWithDomainsTests.cs
@@ -79,7 +79,7 @@ namespace Umbraco.Tests.Routing
protected override string GetXmlContent(int templateId)
{
return @"
-
]>
@@ -302,10 +302,20 @@ namespace Umbraco.Tests.Routing
var cache = routingContext.UmbracoContext.ContentCache.InnerCache as PublishedContentCache;
if (cache == null) throw new Exception("Unsupported IPublishedContentCache, only the Xml one is supported.");
- var cachedRoutes = cache.RoutesCache.GetCachedRoutes();
- // GetUrl does not write to cache
- Assert.AreEqual(0, cachedRoutes.Count);
+ var cachedRoutes = cache.RoutesCache.GetCachedRoutes();
+ Assert.AreEqual(7, cachedRoutes.Count);
+
+ var cachedIds = cache.RoutesCache.GetCachedIds();
+ Assert.AreEqual(0, cachedIds.Count);
+
+ CheckRoute(cachedRoutes, cachedIds, 1001, "1001/");
+ CheckRoute(cachedRoutes, cachedIds, 10011, "10011/");
+ CheckRoute(cachedRoutes, cachedIds, 100111, "10011/1001-1-1");
+ CheckRoute(cachedRoutes, cachedIds, 10012, "10012/");
+ CheckRoute(cachedRoutes, cachedIds, 100121, "10012/1001-2-1");
+ CheckRoute(cachedRoutes, cachedIds, 10013, "1001/1001-3");
+ CheckRoute(cachedRoutes, cachedIds, 1002, "/1002");
// use the cache
Assert.AreEqual("/", routingContext.UrlProvider.GetUrl(1001, new Uri("http://domain1.com"), false));
@@ -319,12 +329,11 @@ namespace Umbraco.Tests.Routing
Assert.AreEqual("http://domain1.com/fr/1001-2-1/", routingContext.UrlProvider.GetUrl(100121, new Uri("http://domain2.com"), false));
}
- void CheckRoute(IDictionary routes, IDictionary ids, int id, string route)
+ private static void CheckRoute(IDictionary routes, IDictionary ids, int id, string route)
{
Assert.IsTrue(routes.ContainsKey(id));
Assert.AreEqual(route, routes[id]);
- Assert.IsTrue(ids.ContainsKey(route));
- Assert.AreEqual(id, ids[route]);
+ Assert.IsFalse(ids.ContainsKey(route));
}
[Test]
diff --git a/src/Umbraco.Tests/Routing/UrlsWithNestedDomains.cs b/src/Umbraco.Tests/Routing/UrlsWithNestedDomains.cs
index 29a60232fa..19420e3120 100644
--- a/src/Umbraco.Tests/Routing/UrlsWithNestedDomains.cs
+++ b/src/Umbraco.Tests/Routing/UrlsWithNestedDomains.cs
@@ -26,7 +26,7 @@ namespace Umbraco.Tests.Routing
public void DoNotPolluteCache()
{
SettingsForTests.UseDirectoryUrls = true;
- SettingsForTests.HideTopLevelNodeFromPath = false; // ignored w/domains
+ SettingsForTests.HideTopLevelNodeFromPath = false; // ignored w/domains
var settings = SettingsForTests.GenerateMockSettings();
var request = Mock.Get(settings.RequestHandler);
@@ -36,7 +36,7 @@ namespace Umbraco.Tests.Routing
RoutingContext routingContext;
string url = "http://domain1.com/1001-1/1001-1-1";
-
+
// get the nice url for 100111
routingContext = GetRoutingContext(url, 9999, umbracoSettings: settings);
Assert.AreEqual("http://domain2.com/1001-1-1/", routingContext.UrlProvider.GetUrl(100111, true));
@@ -44,8 +44,8 @@ namespace Umbraco.Tests.Routing
// check that the proper route has been cached
var cache = routingContext.UmbracoContext.ContentCache.InnerCache as PublishedContentCache;
if (cache == null) throw new Exception("Unsupported IPublishedContentCache, only the Xml one is supported.");
- //var cachedRoutes = cache.RoutesCache.GetCachedRoutes();
- //Assert.AreEqual("10011/1001-1-1", cachedRoutes[100111]);
+ var cachedRoutes = cache.RoutesCache.GetCachedRoutes();
+ Assert.AreEqual("10011/1001-1-1", cachedRoutes[100111]);
// route a rogue url
url = "http://domain1.com/1001-1/1001-1-1";
@@ -61,9 +61,9 @@ namespace Umbraco.Tests.Routing
Assert.AreEqual(100111, pcr.PublishedContent.Id);
// has the cache been polluted?
- //cachedRoutes = cache.RoutesCache.GetCachedRoutes();
- //Assert.AreEqual("10011/1001-1-1", cachedRoutes[100111]); // no
- ////Assert.AreEqual("1001/1001-1/1001-1-1", cachedRoutes[100111]); // yes
+ cachedRoutes = cache.RoutesCache.GetCachedRoutes();
+ Assert.AreEqual("10011/1001-1-1", cachedRoutes[100111]); // no
+ //Assert.AreEqual("1001/1001-1/1001-1-1", cachedRoutes[100111]); // yes
// what's the nice url now?
Assert.AreEqual("http://domain2.com/1001-1-1/", routingContext.UrlProvider.GetUrl(100111)); // good
@@ -89,7 +89,7 @@ namespace Umbraco.Tests.Routing
protected override string GetXmlContent(int templateId)
{
return @"
-
]>
diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedContentCache.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedContentCache.cs
index d9b0363408..c8ddd77afd 100644
--- a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedContentCache.cs
+++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedContentCache.cs
@@ -2,11 +2,9 @@ using System;
using System.Collections.Generic;
using System.Globalization;
using System.Runtime.CompilerServices;
-using System.Text;
using System.Xml;
using System.Xml.XPath;
using Umbraco.Core.Configuration;
-using Umbraco.Core.Logging;
using Umbraco.Core;
using Umbraco.Core.Models;
using Umbraco.Core.Models.PublishedContent;
@@ -18,7 +16,6 @@ using umbraco.BusinessLogic;
using umbraco.presentation.preview;
using Umbraco.Core.Services;
using GlobalSettings = umbraco.GlobalSettings;
-using Task = System.Threading.Tasks.Task;
namespace Umbraco.Web.PublishedCache.XmlPublishedCache
{
@@ -83,7 +80,7 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache
&& DomainHelper.ExistsDomainInPath(umbracoContext.Application.Services.DomainService.GetAll(false), content.Path, domainRootNodeId) == false;
if (deepest)
- _routesCache.Store(content.Id, route);
+ _routesCache.Store(content.Id, route, true); // trusted route
}
public virtual string GetRouteById(UmbracoContext umbracoContext, bool preview, int contentId)
@@ -98,9 +95,16 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache
// else actually determine the route
route = DetermineRouteById(umbracoContext, preview, contentId);
- // may be null if node not found
- // do NOT cache the route: it may be colliding - and since checking for a collision implies
- // doing one DetermineIdByRoute anyways we are not causing any perf penalty by not caching
+ // node not found
+ if (route == null)
+ return null;
+
+ // cache the route BUT do NOT trust it as it can be a colliding route
+ // meaning if we GetRouteById again, we'll get it from cache, but it
+ // won't be used for inbound routing
+ if (preview == false)
+ _routesCache.Store(contentId, route, false);
+
return route;
}
diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/RoutesCache.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/RoutesCache.cs
index 9c8eed322b..4e775b3253 100644
--- a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/RoutesCache.cs
+++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/RoutesCache.cs
@@ -83,10 +83,12 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache
///
/// The node identified.
/// The route.
- public void Store(int nodeId, string route)
+ /// A value indicating whether the value can be trusted for inbound routing.
+ public void Store(int nodeId, string route, bool trust)
{
_routes.AddOrUpdate(nodeId, i => route, (i, s) => route);
- _nodeIds.AddOrUpdate(route, i => nodeId, (i, s) => nodeId);
+ if (trust)
+ _nodeIds.AddOrUpdate(route, i => nodeId, (i, s) => nodeId);
}
///
@@ -119,15 +121,12 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache
/// The node identifier.
public void ClearNode(int nodeId)
{
- if (!_routes.ContainsKey(nodeId)) return;
-
- string key;
- if (!_routes.TryGetValue(nodeId, out key)) return;
-
- int val;
- _nodeIds.TryRemove(key, out val);
- string val2;
- _routes.TryRemove(nodeId, out val2);
+ string route;
+ if (_routes.TryRemove(nodeId, out route))
+ {
+ int id;
+ _nodeIds.TryRemove(route, out id);
+ }
}
///