diff --git a/src/Umbraco.Tests/Routing/UrlsProviderWithDomainsTests.cs b/src/Umbraco.Tests/Routing/UrlsProviderWithDomainsTests.cs index c6bd0fdb88..2f1bef3b9a 100644 --- a/src/Umbraco.Tests/Routing/UrlsProviderWithDomainsTests.cs +++ b/src/Umbraco.Tests/Routing/UrlsProviderWithDomainsTests.cs @@ -418,8 +418,8 @@ namespace Umbraco.Tests.Routing foreach (var x in result) Console.WriteLine(x); Assert.AreEqual(2, result.Length); - Assert.IsTrue(result.Contains("http://domain1a.com/en/1001-1-1/")); - Assert.IsTrue(result.Contains("http://domain1b.com/en/1001-1-1/")); + Assert.AreEqual(result[0].Text, "http://domain1a.com/en/1001-1-1/"); + Assert.AreEqual(result[1].Text, "http://domain1b.com/en/1001-1-1/"); } } } diff --git a/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestControllerActivatorBase.cs b/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestControllerActivatorBase.cs index 2cf64f04d1..7152622b07 100644 --- a/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestControllerActivatorBase.cs +++ b/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestControllerActivatorBase.cs @@ -159,7 +159,7 @@ namespace Umbraco.Tests.TestHelpers.ControllerTesting var urlHelper = new Mock(); urlHelper.Setup(provider => provider.GetUrl(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) - .Returns("/hello/world/1234"); + .Returns(UrlInfo.Url("/hello/world/1234")); var membershipHelper = new MembershipHelper(umbCtx, Mock.Of(), Mock.Of()); diff --git a/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs b/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs index 51855f7e19..5b93de0c09 100644 --- a/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs +++ b/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs @@ -75,7 +75,7 @@ namespace Umbraco.Tests.Testing.TestingTests var urlProviderMock = new Mock(); urlProviderMock.Setup(provider => provider.GetUrl(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) - .Returns("/hello/world/1234"); + .Returns(UrlInfo.Url("/hello/world/1234")); var urlProvider = urlProviderMock.Object; var theUrlProvider = new UrlProvider(umbracoContext, new [] { urlProvider }, umbracoContext.VariationContextAccessor); diff --git a/src/Umbraco.Tests/Web/TemplateUtilitiesTests.cs b/src/Umbraco.Tests/Web/TemplateUtilitiesTests.cs index 1e6229fa4c..d98afa6bcc 100644 --- a/src/Umbraco.Tests/Web/TemplateUtilitiesTests.cs +++ b/src/Umbraco.Tests/Web/TemplateUtilitiesTests.cs @@ -79,7 +79,7 @@ namespace Umbraco.Tests.Web var testUrlProvider = new Mock(); testUrlProvider .Setup(x => x.GetUrl(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) - .Returns((UmbracoContext umbCtx, IPublishedContent content, UrlProviderMode mode, string culture, Uri url) => "/my-test-url"); + .Returns((UmbracoContext umbCtx, IPublishedContent content, UrlProviderMode mode, string culture, Uri url) => UrlInfo.Url("/my-test-url")); var globalSettings = SettingsForTests.GenerateMockGlobalSettings(); diff --git a/src/Umbraco.Web/Routing/AliasUrlProvider.cs b/src/Umbraco.Web/Routing/AliasUrlProvider.cs index 8c851d139f..4c879e931f 100644 --- a/src/Umbraco.Web/Routing/AliasUrlProvider.cs +++ b/src/Umbraco.Web/Routing/AliasUrlProvider.cs @@ -31,7 +31,7 @@ namespace Umbraco.Web.Routing #region GetUrl /// - public string GetUrl(UmbracoContext umbracoContext, IPublishedContent content, UrlProviderMode mode, string culture, Uri current) + public UrlInfo GetUrl(UmbracoContext umbracoContext, IPublishedContent content, UrlProviderMode mode, string culture, Uri current) { return null; // we have nothing to say } @@ -51,13 +51,14 @@ namespace Umbraco.Web.Routing /// Other urls are those that GetUrl would not return in the current context, but would be valid /// urls for the node in other contexts (different domain for current request, umbracoUrlAlias...). /// - public IEnumerable GetOtherUrls(UmbracoContext umbracoContext, int id, Uri current) + public IEnumerable GetOtherUrls(UmbracoContext umbracoContext, int id, Uri current) { var node = umbracoContext.ContentCache.GetById(id); - if (node == null) return Enumerable.Empty(); + if (node == null) + yield break; if (!node.HasProperty(Constants.Conventions.Content.UrlAlias)) - return Enumerable.Empty(); + yield break; var domainHelper = umbracoContext.GetDomainHelper(_siteDomainHelper); @@ -82,20 +83,19 @@ namespace Umbraco.Web.Routing // the content finder may work, depending on the 'current' culture, // but there's no way we can return something meaningful here if (varies) - return Enumerable.Empty(); + yield break; var umbracoUrlName = node.Value(Constants.Conventions.Content.UrlAlias); if (string.IsNullOrWhiteSpace(umbracoUrlName)) - return Enumerable.Empty(); + yield break; var path = "/" + umbracoUrlName; var uri = new Uri(path, UriKind.Relative); - return new[] { UriUtility.UriFromUmbraco(uri, _globalSettings, _requestConfig).ToString() }; + yield return UrlInfo.Url(UriUtility.UriFromUmbraco(uri, _globalSettings, _requestConfig).ToString()); } else { // some domains: one url per domain, which is "/" - var result = new List(); foreach(var domainUri in domainUris) { // if the property is invariant, get the invariant value, url is "/" @@ -108,14 +108,13 @@ namespace Umbraco.Web.Routing ? node.Value(Constants.Conventions.Content.UrlAlias, culture: domainUri.Culture.Name) : node.Value(Constants.Conventions.Content.UrlAlias); - if (!string.IsNullOrWhiteSpace(umbracoUrlName)) - { - var path = "/" + umbracoUrlName; - var uri = new Uri(CombinePaths(domainUri.Uri.GetLeftPart(UriPartial.Path), path)); - result.Add(UriUtility.UriFromUmbraco(uri, _globalSettings, _requestConfig).ToString()); - } + if (string.IsNullOrWhiteSpace(umbracoUrlName)) + continue; + + var path = "/" + umbracoUrlName; + var uri = new Uri(CombinePaths(domainUri.Uri.GetLeftPart(UriPartial.Path), path)); + yield return UrlInfo.Url(UriUtility.UriFromUmbraco(uri, _globalSettings, _requestConfig).ToString(), domainUri.Culture.Name); } - return result; } } diff --git a/src/Umbraco.Web/Routing/CustomRouteUrlProvider.cs b/src/Umbraco.Web/Routing/CustomRouteUrlProvider.cs index ce1fc1ffab..ae17ff6258 100644 --- a/src/Umbraco.Web/Routing/CustomRouteUrlProvider.cs +++ b/src/Umbraco.Web/Routing/CustomRouteUrlProvider.cs @@ -15,7 +15,7 @@ namespace Umbraco.Web.Routing /// /// This will return the URL that is returned by the assigned custom if this is a custom route /// - public string GetUrl(UmbracoContext umbracoContext, IPublishedContent content, UrlProviderMode mode, string culture, Uri current) + public UrlInfo GetUrl(UmbracoContext umbracoContext, IPublishedContent content, UrlProviderMode mode, string culture, Uri current) { if (umbracoContext?.PublishedRequest?.PublishedContent == null) return null; if (umbracoContext.HttpContext?.Request?.RequestContext?.RouteData?.DataTokens == null) return null; @@ -26,13 +26,15 @@ namespace Umbraco.Web.Routing //NOTE: This looks like it might cause an infinite loop because PublishedContentBase.Url calls into UmbracoContext.Current.UrlProvider.GetUrl which calls back into the IUrlProvider pipeline // but the specific purpose of this is that a developer is using their own IPublishedContent that returns a specific Url and doesn't go back into the UrlProvider pipeline. //TODO: We could put a try/catch here just in case, else we could do some reflection checking to see if the implementation is PublishedContentBase and the Url property is not overridden. - return content.Id == umbracoContext.PublishedRequest.PublishedContent.Id - ? umbracoContext.PublishedRequest.PublishedContent.GetUrl(culture) - : null; + return UrlInfo.Url( + content.Id == umbracoContext.PublishedRequest.PublishedContent.Id + ? umbracoContext.PublishedRequest.PublishedContent.GetUrl(culture) + : null, + culture); } /// - /// This always returns null because this url provider is used purely to deal with Umbraco custom routes with + /// This always returns an empty result because this url provider is used purely to deal with Umbraco custom routes with /// UmbracoVirtualNodeRouteHandler, we really only care about the normal URL so that RedirectToCurrentUmbracoPage() works /// with SurfaceControllers /// @@ -40,9 +42,9 @@ namespace Umbraco.Web.Routing /// /// /// - public IEnumerable GetOtherUrls(UmbracoContext umbracoContext, int id, Uri current) + public IEnumerable GetOtherUrls(UmbracoContext umbracoContext, int id, Uri current) { - return null; + yield break; } } } diff --git a/src/Umbraco.Web/Routing/DefaultUrlProvider.cs b/src/Umbraco.Web/Routing/DefaultUrlProvider.cs index 61437b6640..5045b1af96 100644 --- a/src/Umbraco.Web/Routing/DefaultUrlProvider.cs +++ b/src/Umbraco.Web/Routing/DefaultUrlProvider.cs @@ -29,7 +29,7 @@ namespace Umbraco.Web.Routing #region GetUrl /// - public virtual string GetUrl(UmbracoContext umbracoContext, IPublishedContent content, UrlProviderMode mode, string culture, Uri current) + public virtual UrlInfo GetUrl(UmbracoContext umbracoContext, IPublishedContent content, UrlProviderMode mode, string culture, Uri current) { if (!current.IsAbsoluteUri) throw new ArgumentException("Current url must be absolute.", nameof(current)); @@ -39,7 +39,7 @@ namespace Umbraco.Web.Routing return GetUrlFromRoute(route, umbracoContext, content.Id, current, mode, culture); } - internal string GetUrlFromRoute(string route, UmbracoContext umbracoContext, int id, Uri current, UrlProviderMode mode, string culture) + internal UrlInfo GetUrlFromRoute(string route, UmbracoContext umbracoContext, int id, Uri current, UrlProviderMode mode, string culture) { if (string.IsNullOrWhiteSpace(route)) { @@ -58,7 +58,9 @@ namespace Umbraco.Web.Routing : domainHelper.DomainForNode(int.Parse(route.Substring(0, pos)), current, culture); // assemble the url from domainUri (maybe null) and path - return AssembleUrl(domainUri, path, current, mode).ToString(); + var url = AssembleUrl(domainUri, path, current, mode).ToString(); + + return UrlInfo.Url(url, culture); } #endregion @@ -76,10 +78,11 @@ namespace Umbraco.Web.Routing /// Other urls are those that GetUrl would not return in the current context, but would be valid /// urls for the node in other contexts (different domain for current request, umbracoUrlAlias...). /// - public virtual IEnumerable GetOtherUrls(UmbracoContext umbracoContext, int id, Uri current) + public virtual IEnumerable GetOtherUrls(UmbracoContext umbracoContext, int id, Uri current) { var node = umbracoContext.ContentCache.GetById(id); - if (node == null) return Enumerable.Empty(); + if (node == null) + yield break; var domainHelper = umbracoContext.GetDomainHelper(_siteDomainHelper); @@ -94,13 +97,14 @@ namespace Umbraco.Web.Routing // no domains = exit if (domainUris ==null) - return Enumerable.Empty(); + yield break; - var result = new List(); foreach (var d in domainUris) { + var culture = d?.Culture?.Name; + //although we are passing in culture here, if any node in this path is invariant, it ignores the culture anyways so this is ok - var route = umbracoContext.ContentCache.GetRouteById(id, d?.Culture?.Name); + var route = umbracoContext.ContentCache.GetRouteById(id, culture); if (route == null) continue; //need to strip off the leading ID for the route if it exists (occurs if the route is for a node with a domain assigned) @@ -109,9 +113,8 @@ namespace Umbraco.Web.Routing var uri = new Uri(CombinePaths(d.Uri.GetLeftPart(UriPartial.Path), path)); uri = UriUtility.UriFromUmbraco(uri, _globalSettings, _requestSettings); - result.Add(uri.ToString()); + yield return UrlInfo.Url(uri.ToString(), culture); } - return result; } #endregion @@ -146,7 +149,7 @@ namespace Umbraco.Web.Routing uri = new Uri(path, UriKind.Relative); break; default: - throw new ArgumentOutOfRangeException("mode"); + throw new ArgumentOutOfRangeException(nameof(mode)); } } else // a domain was found @@ -169,7 +172,7 @@ namespace Umbraco.Web.Routing uri = new Uri(CombinePaths(domainUri.Uri.AbsolutePath, path), UriKind.Relative); break; default: - throw new ArgumentOutOfRangeException("mode"); + throw new ArgumentOutOfRangeException(nameof(mode)); } } diff --git a/src/Umbraco.Web/Routing/IUrlProvider.cs b/src/Umbraco.Web/Routing/IUrlProvider.cs index 0f102dceb8..55d39880d6 100644 --- a/src/Umbraco.Web/Routing/IUrlProvider.cs +++ b/src/Umbraco.Web/Routing/IUrlProvider.cs @@ -24,7 +24,7 @@ namespace Umbraco.Web.Routing /// when no culture is specified, the current culture. /// If the provider is unable to provide a url, it should return null. /// - string GetUrl(UmbracoContext umbracoContext, IPublishedContent content, UrlProviderMode mode, string culture, Uri current); + UrlInfo GetUrl(UmbracoContext umbracoContext, IPublishedContent content, UrlProviderMode mode, string culture, Uri current); /// /// Gets the other urls of a published content. @@ -37,6 +37,6 @@ namespace Umbraco.Web.Routing /// Other urls are those that GetUrl would not return in the current context, but would be valid /// urls for the node in other contexts (different domain for current request, umbracoUrlAlias...). /// - IEnumerable GetOtherUrls(UmbracoContext umbracoContext, int id, Uri current); + IEnumerable GetOtherUrls(UmbracoContext umbracoContext, int id, Uri current); } } diff --git a/src/Umbraco.Web/Routing/UrlInfo.cs b/src/Umbraco.Web/Routing/UrlInfo.cs index 01dbe4a0e1..a795f1577b 100644 --- a/src/Umbraco.Web/Routing/UrlInfo.cs +++ b/src/Umbraco.Web/Routing/UrlInfo.cs @@ -1,4 +1,5 @@ -using System.Runtime.Serialization; +using System; +using System.Runtime.Serialization; namespace Umbraco.Web.Routing { @@ -6,13 +7,25 @@ namespace Umbraco.Web.Routing /// Represents infos for a url. /// [DataContract(Name = "urlInfo", Namespace = "")] - public class UrlInfo + public class UrlInfo : IEquatable { + + /// + /// Creates a instance representing a true url. + /// + public static UrlInfo Url(string text, string culture = null) => new UrlInfo(text, true, culture); + + /// + /// Creates a instance representing a message. + /// + public static UrlInfo Message(string text, string culture = null) => new UrlInfo(text, false, culture); + /// /// Initializes a new instance of the class. /// - public UrlInfo(string text, bool isUrl, string culture) + private UrlInfo(string text, bool isUrl, string culture) { + if (string.IsNullOrWhiteSpace(text)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(text)); IsUrl = isUrl; Text = text; Culture = culture; @@ -38,13 +51,47 @@ namespace Umbraco.Web.Routing public string Text { get; } /// - /// Creates a instance representing a true url. + /// Checks equality /// - public static UrlInfo Url(string text, string culture = null) => new UrlInfo(text, true, culture); + /// + /// + /// + /// Compare both culture and Text as invariant strings since URLs are not case sensitive, nor are culture names within Umbraco + /// + public bool Equals(UrlInfo other) + { + if (ReferenceEquals(null, other)) return false; + if (ReferenceEquals(this, other)) return true; + return string.Equals(Culture, other.Culture, StringComparison.InvariantCultureIgnoreCase) && IsUrl == other.IsUrl && string.Equals(Text, other.Text, StringComparison.InvariantCultureIgnoreCase); + } - /// - /// Creates a instance representing a message. - /// - public static UrlInfo Message(string text, string culture = null) => new UrlInfo(text, false, culture); + public override bool Equals(object obj) + { + if (ReferenceEquals(null, obj)) return false; + if (ReferenceEquals(this, obj)) return true; + if (obj.GetType() != this.GetType()) return false; + return Equals((UrlInfo) obj); + } + + public override int GetHashCode() + { + unchecked + { + var hashCode = (Culture != null ? StringComparer.InvariantCultureIgnoreCase.GetHashCode(Culture) : 0); + hashCode = (hashCode * 397) ^ IsUrl.GetHashCode(); + hashCode = (hashCode * 397) ^ (Text != null ? StringComparer.InvariantCultureIgnoreCase.GetHashCode(Text) : 0); + return hashCode; + } + } + + public static bool operator ==(UrlInfo left, UrlInfo right) + { + return Equals(left, right); + } + + public static bool operator !=(UrlInfo left, UrlInfo right) + { + return !Equals(left, right); + } } } diff --git a/src/Umbraco.Web/Routing/UrlProvider.cs b/src/Umbraco.Web/Routing/UrlProvider.cs index b265d48923..491deee9e3 100644 --- a/src/Umbraco.Web/Routing/UrlProvider.cs +++ b/src/Umbraco.Web/Routing/UrlProvider.cs @@ -202,7 +202,7 @@ namespace Umbraco.Web.Routing var url = _urlProviders.Select(provider => provider.GetUrl(_umbracoContext, content, mode, culture, current)) .FirstOrDefault(u => u != null); - return url ?? "#"; // legacy wants this + return url?.Text ?? "#"; // legacy wants this } internal string GetUrlFromRoute(int id, string route, string culture) @@ -210,7 +210,7 @@ namespace Umbraco.Web.Routing var provider = _urlProviders.OfType().FirstOrDefault(); var url = provider == null ? route // what else? - : provider.GetUrlFromRoute(route, UmbracoContext.Current, id, _umbracoContext.CleanedUmbracoUrl, Mode, culture); + : provider.GetUrlFromRoute(route, UmbracoContext.Current, id, _umbracoContext.CleanedUmbracoUrl, Mode, culture)?.Text; return url ?? "#"; } @@ -228,7 +228,7 @@ namespace Umbraco.Web.Routing /// urls for the node in other contexts (different domain for current request, umbracoUrlAlias...). /// The results depend on the current url. /// - public IEnumerable GetOtherUrls(int id) + public IEnumerable GetOtherUrls(int id) { return GetOtherUrls(id, _umbracoContext.CleanedUmbracoUrl); } @@ -243,10 +243,10 @@ namespace Umbraco.Web.Routing /// Other urls are those that GetUrl would not return in the current context, but would be valid /// urls for the node in other contexts (different domain for current request, umbracoUrlAlias...). /// - public IEnumerable GetOtherUrls(int id, Uri current) + public IEnumerable GetOtherUrls(int id, Uri current) { // providers can return null or an empty list or a non-empty list, be prepared - var urls = _urlProviders.SelectMany(provider => provider.GetOtherUrls(_umbracoContext, id, current) ?? Enumerable.Empty()); + var urls = _urlProviders.SelectMany(provider => provider.GetOtherUrls(_umbracoContext, id, current)); return urls; } diff --git a/src/Umbraco.Web/Routing/UrlProviderExtensions.cs b/src/Umbraco.Web/Routing/UrlProviderExtensions.cs index 14b9c0a465..3a0caeec78 100644 --- a/src/Umbraco.Web/Routing/UrlProviderExtensions.cs +++ b/src/Umbraco.Web/Routing/UrlProviderExtensions.cs @@ -34,14 +34,11 @@ namespace Umbraco.Web.Routing if (contentService == null) throw new ArgumentNullException(nameof(contentService)); if (logger == null) throw new ArgumentNullException(nameof(logger)); - var urls = new List(); - if (content.Published == false) - { - urls.Add(UrlInfo.Message(textService.Localize("content/itemNotPublished"))); - return urls; - } - + yield return UrlInfo.Message(textService.Localize("content/itemNotPublished")); + + var urls = new HashSet(); + // build a list of urls, for the back-office // which will contain // - the 'main' urls, which is what .Url would return, for each culture @@ -92,37 +89,18 @@ namespace Umbraco.Web.Routing } } - // prepare for de-duplication - var durl = new Dictionary>(); - var dmsg = new Dictionary>(); - foreach (var url in urls) - { - var d = url.IsUrl ? durl : dmsg; - if (!d.TryGetValue(url.Text, out var l)) - d[url.Text] = l = new List(); - l.Add(url); - } - - // deduplicate, order urls first then messages, concatenate cultures (hide if 'all') - var ret = new List(); - foreach (var (text, infos) in durl) - ret.Add(UrlInfo.Url(text, infos.Count == cultures.Count ? null : string.Join(", ", infos.Select(x => x.Culture)))); - foreach (var (text, infos) in dmsg) - ret.Add(UrlInfo.Message(text, infos.Count == cultures.Count ? null : string.Join(", ", infos.Select(x => x.Culture)))); + //return the real urls first, then the messages + foreach (var urlGroup in urls.GroupBy(x => x.IsUrl).OrderByDescending(x => x.Key)) + foreach (var url in urlGroup) + yield return url; // get the 'other' urls - ie not what you'd get with GetUrl() but urls that would route to the document, nevertheless. // for these 'other' urls, we don't check whether they are routable, collide, anything - we just report them. - // also, we are not dealing with cultures at all - that will have to wait - foreach(var otherUrl in umbracoContext.UrlProvider.GetOtherUrls(content.Id)) - { - if (urls.Any(x => x.IsUrl && x.Text == otherUrl)) continue; - ret.Add(UrlInfo.Url(otherUrl)); - } - - return ret; + foreach (var otherUrl in umbracoContext.UrlProvider.GetOtherUrls(content.Id)) + yield return otherUrl; } - private static void HandleCouldNotGetUrl(IContent content, string culture, List urls, IContentService contentService, ILocalizedTextService textService) + private static void HandleCouldNotGetUrl(IContent content, string culture, ICollection urls, IContentService contentService, ILocalizedTextService textService) { // document has a published version yet its url is "#" => a parent must be // unpublished, walk up the tree until we find it, and report. @@ -143,7 +121,7 @@ namespace Umbraco.Web.Routing urls.Add(UrlInfo.Message(textService.Localize("content/parentCultureNotPublished", new[] { parent.Name }), culture)); } - private static bool DetectCollision(IContent content, string url, List urls, string culture, UmbracoContext umbracoContext, PublishedRouter publishedRouter, ILocalizedTextService textService) + private static bool DetectCollision(IContent content, string url, ICollection urls, string culture, UmbracoContext umbracoContext, PublishedRouter publishedRouter, ILocalizedTextService textService) { // test for collisions on the 'main' url var uri = new Uri(url.TrimEnd('/'), UriKind.RelativeOrAbsolute);