diff --git a/src/Umbraco.Tests/Composing/TypeLoaderTests.cs b/src/Umbraco.Tests/Composing/TypeLoaderTests.cs index 7b2f9766f9..75edcd6404 100644 --- a/src/Umbraco.Tests/Composing/TypeLoaderTests.cs +++ b/src/Umbraco.Tests/Composing/TypeLoaderTests.cs @@ -58,18 +58,17 @@ namespace Umbraco.Tests.Composing // cleanup var assDir = new FileInfo(Assembly.GetExecutingAssembly().Location).Directory; - foreach (var d in Directory.GetDirectories(Path.Combine(assDir.FullName, "TypeLoader"))) - Directory.Delete(d, true); + var tlDir = Path.Combine(assDir.FullName, "TypeLoader"); + if (!Directory.Exists(tlDir)) + return; + Directory.Delete(tlDir, true); } private DirectoryInfo PrepareFolder() { var assDir = new FileInfo(Assembly.GetExecutingAssembly().Location).Directory; - var dir = Directory.CreateDirectory(Path.Combine(assDir.FullName, "TypeLoader", Guid.NewGuid().ToString("N"))); - foreach (var f in dir.GetFiles()) - { - f.Delete(); - } + var tlDir = Path.Combine(assDir.FullName, "TypeLoader"); + var dir = Directory.CreateDirectory(Path.Combine(tlDir, Guid.NewGuid().ToString("N"))); return dir; } diff --git a/src/Umbraco.Tests/Routing/GetContentUrlsTests.cs b/src/Umbraco.Tests/Routing/GetContentUrlsTests.cs new file mode 100644 index 0000000000..a2a627227e --- /dev/null +++ b/src/Umbraco.Tests/Routing/GetContentUrlsTests.cs @@ -0,0 +1,136 @@ +using System.Collections.Generic; +using System.Globalization; +using System.Linq; +using Moq; +using NUnit.Framework; +using Umbraco.Core.Configuration.UmbracoSettings; +using Umbraco.Core.Models; +using Umbraco.Core.Services; +using Umbraco.Tests.TestHelpers; +using Umbraco.Tests.TestHelpers.Entities; +using Umbraco.Web.Routing; + +namespace Umbraco.Tests.Routing +{ + [TestFixture] + public class GetContentUrlsTests : UrlRoutingTestBase + { + private IUmbracoSettingsSection _umbracoSettings; + + public override void SetUp() + { + base.SetUp(); + + //generate new mock settings and assign so we can configure in individual tests + _umbracoSettings = SettingsForTests.GenerateMockUmbracoSettings(); + SettingsForTests.ConfigureSettings(_umbracoSettings); + } + + private ILocalizedTextService GetTextService() + { + var textService = Mock.Of( + x => x.Localize("content/itemNotPublished", + It.IsAny(), + It.IsAny>()) == "content/itemNotPublished"); + return textService; + } + + private ILocalizationService GetLangService(params string[] isoCodes) + { + var allLangs = isoCodes + .Select(CultureInfo.GetCultureInfo) + .Select(culture => new Language(culture.Name) + { + CultureName = culture.DisplayName, + IsDefault = true, + IsMandatory = true + }).ToArray(); + + var langService = Mock.Of(x => x.GetAllLanguages() == allLangs); + return langService; + } + + [Test] + public void Content_Not_Published() + { + var contentType = MockedContentTypes.CreateBasicContentType(); + var content = MockedContent.CreateBasicContent(contentType); + content.Id = 1046; //fixme: we are using this ID only because it's built into the test XML published cache + content.Path = "-1,1046"; + + var umbContext = GetUmbracoContext("http://localhost:8000"); + var publishedRouter = CreatePublishedRouter(Container, + contentFinders: new ContentFinderCollection(new[] { new ContentFinderByUrl(Logger) })); + var urls = content.GetContentUrls(publishedRouter, + umbContext, + GetLangService("en-US", "fr-FR"), GetTextService(), ServiceContext.ContentService, + Logger).ToList(); + + Assert.AreEqual(1, urls.Count); + Assert.AreEqual("content/itemNotPublished", urls[0].Text); + Assert.IsFalse(urls[0].IsUrl); + } + + [Test] + public void Invariant_Root_Content_Published_No_Domains() + { + var contentType = MockedContentTypes.CreateBasicContentType(); + var content = MockedContent.CreateBasicContent(contentType); + content.Id = 1046; //fixme: we are using this ID only because it's built into the test XML published cache + content.Path = "-1,1046"; + content.Published = true; + + var umbContext = GetUmbracoContext("http://localhost:8000", + urlProviders: new []{ new DefaultUrlProvider(_umbracoSettings.RequestHandler, Logger, TestObjects.GetGlobalSettings(), new SiteDomainHelper()) }); + var publishedRouter = CreatePublishedRouter(Container, + contentFinders:new ContentFinderCollection(new[]{new ContentFinderByUrl(Logger) })); + var urls = content.GetContentUrls(publishedRouter, + umbContext, + GetLangService("en-US", "fr-FR"), GetTextService(), ServiceContext.ContentService, + Logger).ToList(); + + Assert.AreEqual(1, urls.Count); + Assert.AreEqual("/home/", urls[0].Text); + Assert.AreEqual("en-US", urls[0].Culture); + Assert.IsTrue(urls[0].IsUrl); + } + + [Test] + public void Invariant_Child_Content_Published_No_Domains() + { + var contentType = MockedContentTypes.CreateBasicContentType(); + var parent = MockedContent.CreateBasicContent(contentType); + parent.Id = 1046; //fixme: we are using this ID only because it's built into the test XML published cache + parent.Name = "home"; + parent.Path = "-1,1046"; + parent.Published = true; + var child = MockedContent.CreateBasicContent(contentType); + child.Name = "sub1"; + child.Id = 1173; //fixme: we are using this ID only because it's built into the test XML published cache + child.Path = "-1,1046,1173"; + child.Published = true; + + var umbContext = GetUmbracoContext("http://localhost:8000", + urlProviders: new[] { new DefaultUrlProvider(_umbracoSettings.RequestHandler, Logger, TestObjects.GetGlobalSettings(), new SiteDomainHelper()) }); + var publishedRouter = CreatePublishedRouter(Container, + contentFinders: new ContentFinderCollection(new[] { new ContentFinderByUrl(Logger) })); + var urls = child.GetContentUrls(publishedRouter, + umbContext, + GetLangService("en-US", "fr-FR"), GetTextService(), ServiceContext.ContentService, + Logger).ToList(); + + Assert.AreEqual(1, urls.Count); + Assert.AreEqual("/home/sub1/", urls[0].Text); + Assert.AreEqual("en-US", urls[0].Culture); + Assert.IsTrue(urls[0].IsUrl); + } + + //TODO: We need a lot of tests here, the above was just to get started with being able to unit test this method + // * variant URLs without domains assigned, what happens? + // * variant URLs with domains assigned, but also having more languages installed than there are domains/cultures assigned + // * variant URLs with an ancestor culture unpublished + // * invariant URLs with ancestors as variants + // * ... probably a lot more + + } +} diff --git a/src/Umbraco.Tests/Routing/UrlsProviderWithDomainsTests.cs b/src/Umbraco.Tests/Routing/UrlsProviderWithDomainsTests.cs index c6bd0fdb88..c7ce42e4bc 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://domain1b.com/en/1001-1-1/"); + Assert.AreEqual(result[1].Text, "http://domain1a.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/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index baeb667fcc..ed2324e02c 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -132,6 +132,7 @@ + diff --git a/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs b/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs index e28d11dc4e..9bd00f3d96 100644 --- a/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs @@ -79,7 +79,7 @@ namespace Umbraco.Tests.Web.Controllers }); var textService = new Mock(); - textService.Setup(x => x.Localize(It.IsAny(), It.IsAny(), It.IsAny>())).Returns(""); + textService.Setup(x => x.Localize(It.IsAny(), It.IsAny(), It.IsAny>())).Returns("text"); Container.RegisterSingleton(f => Mock.Of()); Container.RegisterSingleton(f => userServiceMock.Object); 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.UI.Client/src/common/directives/components/content/umbcontentnodeinfo.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbcontentnodeinfo.directive.js index 5da37ff64e..f2e8b261c3 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbcontentnodeinfo.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbcontentnodeinfo.directive.js @@ -10,6 +10,8 @@ var auditTrailLoaded = false; var labels = {}; scope.publishStatus = []; + scope.currentVariant = null; + scope.currentUrls = []; scope.disableTemplates = Umbraco.Sys.ServerVariables.features.disabledFeatures.disableTemplates; scope.allowChangeDocumentType = false; @@ -17,6 +19,18 @@ function onInit() { + // set currentVariant + scope.currentVariant = _.find(scope.node.variants, (v) => v.active); + + // find the urls for the currently selected language + if(scope.node.variants.length > 1) { + // nodes with variants + scope.currentUrls = _.filter(scope.node.urls, (url) => scope.currentVariant.language.culture === url.culture); + } else { + // invariant nodes + scope.currentUrls = scope.node.urls; + } + // if there are any infinite editors open we are in infinite editing scope.isInfiniteMode = editorService.getNumberOfEditors() > 0 ? true : false; @@ -34,7 +48,10 @@ "content_publishedPendingChanges", "content_notCreated", "prompt_unsavedChanges", - "prompt_doctypeChangeWarning" + "prompt_doctypeChangeWarning", + "general_history", + "auditTrails_historyIncludingVariants", + "content_itemNotPublished" ]; localizationService.localizeMany(keys) @@ -46,8 +63,22 @@ labels.notCreated = data[4]; labels.unsavedChanges = data[5]; labels.doctypeChangeWarning = data[6]; + labels.notPublished = data[9]; + + scope.historyLabel = scope.node.variants && scope.node.variants.length === 1 ? data[7] : data[8]; + + setNodePublishStatus(); - setNodePublishStatus(scope.node); + if (scope.currentUrls.length === 0) { + if (scope.node.id > 0) { + //it's created but not published + scope.currentUrls.push({ text: labels.notPublished, isUrl: false }); + } + else { + //it's new + scope.currentUrls.push({ text: labels.notCreated, isUrl: false }) + } + } }); @@ -55,6 +86,9 @@ "id": scope.node.id }; + // make sure dates are formatted to the user's locale + formatDatesToLocal(); + // get available templates scope.availableTemplates = scope.node.allowedTemplates; @@ -218,12 +252,13 @@ function setAuditTrailLogTypeColor(auditTrail) { angular.forEach(auditTrail, function (item) { - switch (item.logType) { case "Publish": + case "PublishVariant": item.logTypeColor = "success"; break; case "Unpublish": + case "UnpublishVariant": case "Delete": item.logTypeColor = "danger"; break; @@ -233,51 +268,40 @@ }); } - function setNodePublishStatus(node) { + function setNodePublishStatus() { - scope.publishStatus = []; + scope.status = {}; // deleted node - if (node.trashed === true) { - scope.publishStatus.push({ - label: labels.deleted, - color: "danger" - }); + if (scope.node.trashed === true) { + scope.status.color = "danger"; return; } - if (node.variants) { - for (var i = 0; i < node.variants.length; i++) { - - var variant = node.variants[i]; - - var status = { - culture: variant.language ? variant.language.culture : null - }; - - if (variant.state === "NotCreated") { - status.label = labels.notCreated; - status.color = "gray"; - } - else if (variant.state === "Draft") { - // draft node - status.label = labels.unpublished; - status.color = "gray"; - } - else if (variant.state === "Published") { - // published node - status.label = labels.published; - status.color = "success"; - } - else if (variant.state === "PublishedPendingChanges") { - // published node with pending changes - status.label = labels.publishedPendingChanges; - status.color = "success"; - } - - scope.publishStatus.push(status); - } + // variant status + if (scope.currentVariant.state === "NotCreated") { + // not created + scope.status.color = "gray"; } + else if (scope.currentVariant.state === "Draft") { + // draft node + scope.status.color = "gray"; + } + else if (scope.currentVariant.state === "Published") { + // published node + scope.status.color = "success"; + } + else if (scope.currentVariant.state === "PublishedPendingChanges") { + // published node with pending changes + scope.status.color = "success"; + } + } + + function formatDatesToLocal() { + // get current backoffice user and format dates + userService.getCurrentUser().then(function (currentUser) { + scope.currentVariant.createDateFormatted = dateHelper.getLocalDate(scope.currentVariant.createDate, currentUser.locale, 'LLL'); + }); } // load audit trail and redirects when on the info tab @@ -303,7 +327,7 @@ auditTrailLoaded = false; loadAuditTrail(); loadRedirectUrls(); - setNodePublishStatus(scope.node); + setNodePublishStatus(); } }); diff --git a/src/Umbraco.Web.UI.Client/src/views/components/content/umb-content-node-info.html b/src/Umbraco.Web.UI.Client/src/views/components/content/umb-content-node-info.html index 429cceb4d9..b1757d927f 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/content/umb-content-node-info.html +++ b/src/Umbraco.Web.UI.Client/src/views/components/content/umb-content-node-info.html @@ -2,21 +2,25 @@
- +
    -
  • +
  • + + {{url.culture}} {{url.text}} - {{url.culture}} +
    - {{url.text}} - {{url.culture}} + + {{url.culture}} + {{url.text}} +
@@ -43,7 +47,8 @@ + title="{{historyLabel}}"> +
- +
@@ -96,7 +102,6 @@ class="history-item__badge" size="xs" color="{{item.logTypeColor}}"> - {{ item.logType }} @@ -130,16 +135,13 @@ -
- {{status.culture}}: - - {{status.label}} - -
+ + + - {{node.createDateFormatted}} by {{ node.owner.name }} + {{currentVariant.createDateFormatted}} diff --git a/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml b/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml index a3b5185613..a7b632fc2d 100644 --- a/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml +++ b/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml @@ -147,8 +147,9 @@ Viewing for Content deleted Content unpublished - Content saved and Published - Content saved and published for languages: %0% + Content unpublished for languages: %0% + Content published + Content published for languages: %0% Content saved Content saved for languages: %0% Content moved @@ -165,10 +166,12 @@ Save Delete Unpublish + Unpublish Rollback Send To Publish Send To Publish Sort + History (all variants) To change the document type for the selected content, first select from the list of valid types for this location. diff --git a/src/Umbraco.Web/Editors/KeepAliveController.cs b/src/Umbraco.Web/Editors/KeepAliveController.cs index 78b1289e38..b067a5b67e 100644 --- a/src/Umbraco.Web/Editors/KeepAliveController.cs +++ b/src/Umbraco.Web/Editors/KeepAliveController.cs @@ -4,6 +4,11 @@ using Umbraco.Web.WebApi; namespace Umbraco.Web.Editors { + // fixme + // this is not authenticated, and therefore public, and therefore reveals we + // are running Umbraco - but, all requests should come from localhost really, + // so there should be a way to 404 when the request comes from the outside. + public class KeepAliveController : UmbracoApiController { [HttpGet] @@ -14,7 +19,7 @@ namespace Umbraco.Web.Editors Success = true, Message = "I'm alive!" }; - } + } } public class KeepAlivePingResult 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/DomainHelper.cs b/src/Umbraco.Web/Routing/DomainHelper.cs index b6d79e788a..edfcb33aa5 100644 --- a/src/Umbraco.Web/Routing/DomainHelper.cs +++ b/src/Umbraco.Web/Routing/DomainHelper.cs @@ -30,10 +30,10 @@ namespace Umbraco.Web.Routing /// The culture, or null. /// The domain and its uri, if any, that best matches the specified uri and culture, else null. /// - /// f at least a domain is set on the node then the method returns the domain that + /// If at least a domain is set on the node then the method returns the domain that /// best matches the specified uri and culture, else it returns null. - /// If culture is null, uses the default culture for the installation instead. - /// fixme not exactly - if culture is !null, we MUST have a value for THAT culture, else we use default as hint + /// If culture is null, uses the default culture for the installation instead. Otherwise, + /// will try with the specified culture, else return null. /// internal DomainAndUri DomainForNode(int nodeId, Uri current, string culture = null) { @@ -49,13 +49,9 @@ namespace Umbraco.Web.Routing return null; // else filter - var domainAndUri = SelectDomain(domains, current, culture, _domainCache.DefaultCulture, + // it could be that none apply (due to culture) + return SelectDomain(domains, current, culture, _domainCache.DefaultCulture, (cdomainAndUris, ccurrent, cculture, cdefaultCulture) => _siteDomainHelper.MapDomain(cdomainAndUris, ccurrent, cculture, cdefaultCulture)); - - if (domainAndUri == null) - throw new Exception("DomainForUri returned null."); - - return domainAndUri; } /// @@ -202,13 +198,12 @@ namespace Umbraco.Web.Routing private static IReadOnlyCollection SelectByCulture(IReadOnlyCollection domainsAndUris, string culture, string defaultCulture) { + // we try our best to match cultures, but may end with a bogus domain + if (culture != null) // try the supplied culture { var cultureDomains = domainsAndUris.Where(x => x.Culture.Name.InvariantEquals(culture)).ToList(); if (cultureDomains.Count > 0) return cultureDomains; - - // if a culture is supplied, we *want* a url for that culture, else fail - throw new InvalidOperationException($"Could not find a domain for culture \"{culture}\"."); } if (defaultCulture != null) // try the defaultCulture culture @@ -224,13 +219,12 @@ namespace Umbraco.Web.Routing { DomainAndUri domainAndUri; + // we try our best to match cultures, but may end with a bogus domain + if (culture != null) // try the supplied culture { domainAndUri = domainsAndUris.FirstOrDefault(x => x.Culture.Name.InvariantEquals(culture)); if (domainAndUri != null) return domainAndUri; - - // if a culture is supplied, we *want* a url for that culture, else fail - throw new InvalidOperationException($"Could not find a domain for culture \"{culture}\"."); } if (defaultCulture != null) // try the defaultCulture culture 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..ae5c4b412c 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) { + 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..7ed530093c 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,12 +243,9 @@ 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()); - - return urls; + return _urlProviders.SelectMany(provider => provider.GetOtherUrls(_umbracoContext, id, current)); } #endregion diff --git a/src/Umbraco.Web/Routing/UrlProviderExtensions.cs b/src/Umbraco.Web/Routing/UrlProviderExtensions.cs index 14b9c0a465..8db657ed30 100644 --- a/src/Umbraco.Web/Routing/UrlProviderExtensions.cs +++ b/src/Umbraco.Web/Routing/UrlProviderExtensions.cs @@ -5,7 +5,6 @@ using Umbraco.Core.Models; using Umbraco.Core.Services; using Umbraco.Core; using Umbraco.Core.Logging; -using LightInject; namespace Umbraco.Web.Routing { @@ -18,7 +17,7 @@ namespace Umbraco.Web.Routing /// Use when displaying Urls. If errors occur when generating the Urls, they will show in the list. /// Contains all the Urls that we can figure out (based upon domains, etc). /// - public static IEnumerable GetContentUrls(this IContent content, + public static IEnumerable GetContentUrls(this IContent content, PublishedRouter publishedRouter, UmbracoContext umbracoContext, ILocalizationService localizationService, @@ -34,25 +33,71 @@ 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")); + yield break; } - + // build a list of urls, for the back-office // which will contain // - the 'main' urls, which is what .Url would return, for each culture // - the 'other' urls we know (based upon domains, etc) // - // need to work on each culture. - // on invariant trees, each culture return the same thing - // but, we don't know if the tree to this content is invariant + // need to work through each installed culture: + // on invariant nodes, each culture returns the same url segment but, + // we don't know if the branch to this content is invariant, so we need to ask + // for URLs for all cultures. + // and, not only for those assigned to domains in the branch, because we want + // to show what GetUrl() would return, for every culture. + var urls = new HashSet(); var cultures = localizationService.GetAllLanguages().Select(x => x.IsoCode).ToList(); + //get all URLs for all cultures + //in a HashSet, so de-duplicates too + foreach (var cultureUrl in GetContentUrlsByCulture(content, cultures, publishedRouter, umbracoContext, contentService, textService, logger)) + { + urls.Add(cultureUrl); + } + + //return the real urls first, then the messages + foreach (var urlGroup in urls.GroupBy(x => x.IsUrl).OrderByDescending(x => x.Key)) + { + //in some cases there will be the same URL for multiple cultures: + // * The entire branch is invariant + // * If there are less domain/cultures assigned to the branch than the number of cultures/languages installed + + foreach (var dUrl in urlGroup.DistinctBy(x => x.Text.ToUpperInvariant()).OrderBy(x => x.Text).ThenBy(x => x.Culture)) + yield return dUrl; + } + + // 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. + foreach (var otherUrl in umbracoContext.UrlProvider.GetOtherUrls(content.Id).OrderBy(x => x.Text).ThenBy(x => x.Culture)) + if (urls.Add(otherUrl)) //avoid duplicates + yield return otherUrl; + } + + /// + /// Tries to return a for each culture for the content while detecting collisions/errors + /// + /// + /// + /// + /// + /// + /// + /// + /// + private static IEnumerable GetContentUrlsByCulture(IContent content, + IEnumerable cultures, + PublishedRouter publishedRouter, + UmbracoContext umbracoContext, + IContentService contentService, + ILocalizedTextService textService, + ILogger logger) + { foreach (var culture in cultures) { // if content is variant, and culture is not published, skip @@ -76,53 +121,26 @@ namespace Umbraco.Web.Routing { // deal with 'could not get the url' case "#": - HandleCouldNotGetUrl(content, culture, urls, contentService, textService); + yield return HandleCouldNotGetUrl(content, culture, contentService, textService); break; // deal with exceptions case "#ex": - urls.Add(UrlInfo.Message(textService.Localize("content/getUrlException"), culture)); + yield return UrlInfo.Message(textService.Localize("content/getUrlException"), culture); break; // got a url, deal with collisions, add url default: - if (!DetectCollision(content, url, urls, culture, umbracoContext, publishedRouter, textService)) // detect collisions, etc - urls.Add(UrlInfo.Url(url, culture)); + if (DetectCollision(content, url, culture, umbracoContext, publishedRouter, textService, out var urlInfo)) // detect collisions, etc + yield return urlInfo; + else + yield return UrlInfo.Url(url, culture); break; } } - - // 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)))); - - // 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; } - private static void HandleCouldNotGetUrl(IContent content, string culture, List urls, IContentService contentService, ILocalizedTextService textService) + private static UrlInfo HandleCouldNotGetUrl(IContent content, string culture, 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. @@ -134,16 +152,16 @@ namespace Umbraco.Web.Routing while (parent != null && parent.Published && (!parent.ContentType.VariesByCulture() || parent.IsCulturePublished(culture))); if (parent == null) // oops, internal error - urls.Add(UrlInfo.Message(textService.Localize("content/parentNotPublishedAnomaly"), culture)); + return UrlInfo.Message(textService.Localize("content/parentNotPublishedAnomaly"), culture); else if (!parent.Published) // totally not published - urls.Add(UrlInfo.Message(textService.Localize("content/parentNotPublished", new[] { parent.Name }), culture)); + return UrlInfo.Message(textService.Localize("content/parentNotPublished", new[] {parent.Name}), culture); else // culture not published - urls.Add(UrlInfo.Message(textService.Localize("content/parentCultureNotPublished", new[] { parent.Name }), culture)); + return 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, string culture, UmbracoContext umbracoContext, PublishedRouter publishedRouter, ILocalizedTextService textService, out UrlInfo urlInfo) { // test for collisions on the 'main' url var uri = new Uri(url.TrimEnd('/'), UriKind.RelativeOrAbsolute); @@ -152,9 +170,11 @@ namespace Umbraco.Web.Routing var pcr = publishedRouter.CreateRequest(umbracoContext, uri); publishedRouter.TryRouteRequest(pcr); + urlInfo = null; + if (pcr.HasPublishedContent == false) { - urls.Add(UrlInfo.Message(textService.Localize("content/routeErrorCannotRoute"), culture)); + urlInfo = UrlInfo.Message(textService.Localize("content/routeErrorCannotRoute"), culture); return true; } @@ -173,7 +193,7 @@ namespace Umbraco.Web.Routing l.Reverse(); var s = "/" + string.Join("/", l) + " (id=" + pcr.PublishedContent.Id + ")"; - urls.Add(UrlInfo.Message(textService.Localize("content/routeError", new[] { s }), culture)); + urlInfo = UrlInfo.Message(textService.Localize("content/routeError", new[] { s }), culture); return true; } diff --git a/src/Umbraco.Web/Scheduling/KeepAlive.cs b/src/Umbraco.Web/Scheduling/KeepAlive.cs index bf8610e005..8068e387f8 100644 --- a/src/Umbraco.Web/Scheduling/KeepAlive.cs +++ b/src/Umbraco.Web/Scheduling/KeepAlive.cs @@ -59,7 +59,7 @@ namespace Umbraco.Web.Scheduling return true; // repeat } - var url = umbracoAppUrl + "api/keepalive/ping"; + var url = umbracoAppUrl.TrimEnd('/') + "/api/keepalive/ping"; var request = new HttpRequestMessage(HttpMethod.Get, url); var result = await _httpClient.SendAsync(request, token);