diff --git a/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs b/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs index d613c7411..77372a047 100644 --- a/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs +++ b/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs @@ -5,6 +5,7 @@ using JetBrains.Annotations; using Moq; using NUnit.Framework; using Orchard.ContentManagement; +using Orchard.ContentManagement.Aspects; using Orchard.ContentManagement.Drivers; using Orchard.ContentManagement.Handlers; using Orchard.ContentManagement.MetaData; @@ -14,10 +15,14 @@ using Orchard.Core.Routable; using Orchard.Core.Routable.Handlers; using Orchard.Core.Routable.Models; using Orchard.Core.Routable.Services; +using Orchard.Data; +using Orchard.Environment; +using Orchard.Security; using Orchard.Tests.Modules; using System.Web.Mvc; using System.Web.Routing; using Orchard.Tests.Stubs; +using Orchard.UI.Notify; namespace Orchard.Core.Tests.Common.Services { [TestFixture] @@ -32,6 +37,10 @@ namespace Orchard.Core.Tests.Common.Services { builder.RegisterType().As(); builder.RegisterType().As(); builder.RegisterInstance(new Mock().Object); + builder.RegisterInstance(new Mock().Object); + builder.RegisterInstance(new Mock().Object); + builder.RegisterInstance(new Mock().Object); + builder.RegisterType().As(); builder.RegisterType().As(); builder.RegisterType().As(); @@ -41,7 +50,6 @@ namespace Orchard.Core.Tests.Common.Services { builder.RegisterType().As(); builder.RegisterInstance(new UrlHelper(new RequestContext(new StubHttpContext("~/"), new RouteData()))).As(); builder.RegisterType().As(); - } private IRoutableService _routableService; @@ -80,18 +88,12 @@ namespace Orchard.Core.Tests.Common.Services { [Test] public void VeryLongStringTruncatedTo1000Chars() { - var contentManager = _container.Resolve(); - var veryVeryLongTitle = "this is a very long title..."; for (var i = 0; i < 100; i++) veryVeryLongTitle += "aaaaaaaaaa"; - var thing = contentManager.Create(ThingDriver.ContentType.Name, t => { - t.As().Record = new RoutePartRecord(); - t.Title = veryVeryLongTitle; - }); - - _routableService.FillSlug(thing.As()); + var thing = CreateRoutePart(veryVeryLongTitle); + _routableService.FillSlug(thing); Assert.That(veryVeryLongTitle.Length, Is.AtLeast(1001)); Assert.That(thing.Slug.Length, Is.EqualTo(1000)); @@ -99,86 +101,72 @@ namespace Orchard.Core.Tests.Common.Services { [Test] public void NoExistingLikeSlugsGeneratesSameSlug() { - string slug = _routableService.GenerateUniqueSlug("woohoo", null); + string slug = _routableService.GenerateUniqueSlug(CreateRoutePart("woohoo"), null); Assert.That(slug, Is.EqualTo("woohoo")); } [Test] public void ExistingSingleLikeSlugThatsAConflictGeneratesADash2() { - string slug = _routableService.GenerateUniqueSlug("woohoo", new List { "woohoo" }); + string slug = _routableService.GenerateUniqueSlug(CreateRoutePart("woohoo"), new List { "woohoo" }); Assert.That(slug, Is.EqualTo("woohoo-2")); } [Test] public void ExistingSingleLikeSlugThatsNotAConflictGeneratesSameSlug() { - string slug = _routableService.GenerateUniqueSlug("woohoo", new List { "woohoo-2" }); + string slug = _routableService.GenerateUniqueSlug(CreateRoutePart("woohoo"), new List { "woohoo-2" }); Assert.That(slug, Is.EqualTo("woohoo")); } [Test] public void ExistingLikeSlugsWithAConflictGeneratesADashVNext() { - string slug = _routableService.GenerateUniqueSlug("woohoo", new List { "woohoo", "woohoo-2" }); + string slug = _routableService.GenerateUniqueSlug(CreateRoutePart("woohoo"), new List { "woohoo", "woohoo-2" }); Assert.That(slug, Is.EqualTo("woohoo-3")); } [Test] public void ExistingSlugsWithVersionGapsAndNoMatchGeneratesSameSlug() { - string slug = _routableService.GenerateUniqueSlug("woohoo", new List { "woohoo-2", "woohoo-4", "woohoo-5" }); + string slug = _routableService.GenerateUniqueSlug(CreateRoutePart("woohoo"), new List { "woohoo-2", "woohoo-4", "woohoo-5" }); Assert.That(slug, Is.EqualTo("woohoo")); } [Test] public void ExistingSlugsWithVersionGapsAndAMatchGeneratesADash2() { - string slug = _routableService.GenerateUniqueSlug("woohoo-2", new List { "woohoo-2", "woohoo-4", "woohoo-5" }); + string slug = _routableService.GenerateUniqueSlug(CreateRoutePart("woohoo-2"), new List { "woohoo-2", "woohoo-4", "woohoo-5" }); Assert.That(slug, Is.EqualTo("woohoo-2-2")); } [Test] public void GeneratedSlugIsLowerCased() { - var contentManager = _container.Resolve(); + var thing = CreateRoutePart("This Is Some Interesting Title"); - var thing = contentManager.Create(ThingDriver.ContentType.Name, t => { - t.As().Record = new RoutePartRecord(); - t.Title = "This Is Some Interesting Title"; - }); - - _routableService.FillSlug(thing.As()); + _routableService.FillSlug(thing); Assert.That(thing.Slug, Is.EqualTo("this-is-some-interesting-title")); } - [Test, Ignore("Fix pending")] - public void GeneratedSlugsShouldBeUniqueAmongContentType() { - var contentManager = _container.Resolve(); + [Test] + public void SlugInConflictWithAnExistingItemsPathIsVersioned() { + var thing1 = CreateRoutePart("bar", "bar", "foo"); + var thing2 = CreateRoutePart("fooslashbar", "foo/bar"); - var thing1 = contentManager.Create(ThingDriver.ContentType.Name, t => { - t.As().Record = new RoutePartRecord(); - t.Title = "This Is Some Interesting Title"; - }); - - var thing2 = contentManager.Create(ThingDriver.ContentType.Name, t => { - t.As().Record = new RoutePartRecord(); - t.Title = "This Is Some Interesting Title"; - }); - - Assert.AreNotEqual(thing1.Slug, thing2.Slug); + Assert.That(thing2.Slug, Is.EqualTo("foo/bar-2")); } - [Test] - public void SlugsCanBeDuplicatedAccrossContentTypes() { + private RoutePart CreateRoutePart(string title, string slug = "", string containerPath = "") { var contentManager = _container.Resolve(); - - var thing = contentManager.Create(ThingDriver.ContentType.Name, t => { - t.As().Record = new RoutePartRecord(); - t.Title = "This Is Some Interesting Title"; - }); - - var stuff = contentManager.Create(StuffDriver.ContentType.Name, s => { - s.As().Record = new RoutePartRecord(); - s.Title = "This Is Some Interesting Title"; - }); - - Assert.AreEqual(thing.Slug, stuff.Slug); + return contentManager.Create(ThingDriver.ContentType.Name, t => { + t.As().Record = new RoutePartRecord(); + if (!string.IsNullOrWhiteSpace(slug)) + t.As().Slug = slug; + t.Title = title; + if (!string.IsNullOrWhiteSpace(containerPath)) { + t.As().Container = contentManager.Create(ThingDriver.ContentType.Name, tt => { + tt.As().Path = containerPath; + tt.As().Slug = containerPath; + }); + } + }) + .As(); } diff --git a/src/Orchard.Web/Core/Routable/Drivers/RoutePartDriver.cs b/src/Orchard.Web/Core/Routable/Drivers/RoutePartDriver.cs index ec618c5c6..d806abeef 100644 --- a/src/Orchard.Web/Core/Routable/Drivers/RoutePartDriver.cs +++ b/src/Orchard.Web/Core/Routable/Drivers/RoutePartDriver.cs @@ -59,9 +59,9 @@ namespace Orchard.Core.Routable.Drivers { model.DisplayLeadingPath = path.Substring(0, path.Length - slug.Length); } else { - var containerSlug = part.GetContainerSlug(); - model.DisplayLeadingPath = !string.IsNullOrWhiteSpace(containerSlug) - ? string.Format("{0}/", containerSlug) + var containerPath = part.GetContainerPath(); + model.DisplayLeadingPath = !string.IsNullOrWhiteSpace(containerPath) + ? string.Format("{0}/", containerPath) : ""; } diff --git a/src/Orchard.Web/Core/Routable/Handlers/RoutePartHandler.cs b/src/Orchard.Web/Core/Routable/Handlers/RoutePartHandler.cs index 3246f6fb2..b0284e3f6 100644 --- a/src/Orchard.Web/Core/Routable/Handlers/RoutePartHandler.cs +++ b/src/Orchard.Web/Core/Routable/Handlers/RoutePartHandler.cs @@ -1,24 +1,53 @@ -using System.Web.Routing; +using System; +using System.Web.Routing; using Orchard.ContentManagement; using Orchard.ContentManagement.Handlers; using Orchard.Core.Routable.Models; +using Orchard.Core.Routable.Services; using Orchard.Data; +using Orchard.Localization; +using Orchard.UI.Notify; namespace Orchard.Core.Routable.Handlers { public class RoutePartHandler : ContentHandler { + private readonly IOrchardServices _services; private readonly IRoutablePathConstraint _routablePathConstraint; + private readonly IRoutableService _routableService; - public RoutePartHandler(IRepository repository, IRoutablePathConstraint routablePathConstraint) { + public RoutePartHandler(IOrchardServices services, IRepository repository, IRoutablePathConstraint routablePathConstraint, IRoutableService routableService) { + _services = services; _routablePathConstraint = routablePathConstraint; + _routableService = routableService; + T = NullLocalizer.Instance; + Filters.Add(StorageFilter.For(repository)); + Action processSlug = ( + routable => { + var originalSlug = routable.Slug; + if (string.IsNullOrWhiteSpace(routable.Path)) + routable.Path = routable.GetPathFromSlug(routable.Slug); + + if (!_routableService.ProcessSlug(routable)) { + _services.Notifier.Warning(T("Slugs in conflict. \"{0}\" is already set for a previously created {2} so now it has the slug \"{1}\"", + originalSlug, routable.Slug, routable.ContentItem.ContentType)); + } + + // TEMP: path format patterns replaces this logic + routable.Path = routable.GetPathFromSlug(routable.Slug); + }); + OnPublished((context, routable) => { + if (context.PublishingItemVersionRecord != null) + processSlug(routable); if (!string.IsNullOrEmpty(routable.Path)) _routablePathConstraint.AddPath(routable.Path); }); OnIndexing((context, part) => context.DocumentIndex.Add("title", part.Record.Title).RemoveTags().Analyze()); } + + public Localizer T { get; set; } } public class RoutePartHandlerBase : ContentHandlerBase { diff --git a/src/Orchard.Web/Core/Routable/Models/RoutePart.cs b/src/Orchard.Web/Core/Routable/Models/RoutePart.cs index dfda3e744..3f1915ff4 100644 --- a/src/Orchard.Web/Core/Routable/Models/RoutePart.cs +++ b/src/Orchard.Web/Core/Routable/Models/RoutePart.cs @@ -18,12 +18,12 @@ namespace Orchard.Core.Routable.Models { set { Record.Path = value; } } - public string GetContainerSlug() { + public string GetContainerPath() { var commonAspect = this.As(); if (commonAspect != null && commonAspect.Container != null) { var routable = commonAspect.Container.As(); if (routable != null) { - return routable.Slug; + return routable.Path; } } return null; @@ -31,11 +31,11 @@ namespace Orchard.Core.Routable.Models { public string GetPathFromSlug(string slug) { // TEMP: path format patterns replaces this logic - var containerSlug = GetContainerSlug(); - if (string.IsNullOrEmpty(containerSlug)) { + var containerPath = GetContainerPath(); + if (string.IsNullOrEmpty(containerPath)) { return slug; } - return containerSlug + "/" + slug; + return containerPath + "/" + slug; } } } \ No newline at end of file diff --git a/src/Orchard.Web/Core/Routable/Services/IRoutableService.cs b/src/Orchard.Web/Core/Routable/Services/IRoutableService.cs index 101fce3c4..8156ce4d6 100644 --- a/src/Orchard.Web/Core/Routable/Services/IRoutableService.cs +++ b/src/Orchard.Web/Core/Routable/Services/IRoutableService.cs @@ -6,12 +6,12 @@ namespace Orchard.Core.Routable.Services { public interface IRoutableService : IDependency { void FillSlug(TModel model) where TModel : RoutePart; void FillSlug(TModel model, Func generateSlug) where TModel : RoutePart; - string GenerateUniqueSlug(string slugCandidate, IEnumerable existingSlugs); + string GenerateUniqueSlug(RoutePart part, IEnumerable existingPaths); /// - /// Returns any content item of the specified content type with similar slugs + /// Returns any content item with similar path /// - IEnumerable GetSimilarSlugs(string contentType, string slug); + IEnumerable GetSimilarPaths(string path); /// /// Validates the given slug diff --git a/src/Orchard.Web/Core/Routable/Services/RoutableService.cs b/src/Orchard.Web/Core/Routable/Services/RoutableService.cs index 20bd4e8c6..5a79c0648 100644 --- a/src/Orchard.Web/Core/Routable/Services/RoutableService.cs +++ b/src/Orchard.Web/Core/Routable/Services/RoutableService.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using System.Linq; using System.Text.RegularExpressions; using Orchard.ContentManagement; -using Orchard.Core.Common.Models; using Orchard.Core.Routable.Models; namespace Orchard.Core.Routable.Services { @@ -37,11 +36,12 @@ namespace Orchard.Core.Routable.Services { model.Slug = generateSlug(model.Title).ToLowerInvariant(); } - public string GenerateUniqueSlug(string slugCandidate, IEnumerable existingSlugs) { - if (existingSlugs == null || !existingSlugs.Contains(slugCandidate)) + public string GenerateUniqueSlug(RoutePart part, IEnumerable existingPaths) { + var slugCandidate = part.Slug; + if (existingPaths == null || !existingPaths.Contains(part.Path)) return slugCandidate; - int? version = existingSlugs.Select(s => GetSlugVersion(slugCandidate, s)).OrderBy(i => i).LastOrDefault(); + int? version = existingPaths.Select(s => GetSlugVersion(slugCandidate, s)).OrderBy(i => i).LastOrDefault(); return version != null ? string.Format("{0}-{1}", slugCandidate, version) @@ -60,13 +60,12 @@ namespace Orchard.Core.Routable.Services { : null; } - public IEnumerable GetSimilarSlugs(string contentType, string slug) - { + public IEnumerable GetSimilarPaths(string path) { return _contentManager.Query().Join() .List() .Select(i => i.As()) - .Where(routable => routable.Path != null && routable.Path.Equals(slug, StringComparison.OrdinalIgnoreCase)) // todo: for some reason the filter doesn't work within the query, even without StringComparison or StartsWith + .Where(routable => routable.Path != null && routable.Path.StartsWith(path, StringComparison.OrdinalIgnoreCase)) // todo: for some reason the filter doesn't work within the query, even without StringComparison or StartsWith .ToArray(); } @@ -84,19 +83,18 @@ namespace Orchard.Core.Routable.Services { return true; } - var slugsLikeThis = GetSimilarSlugs(part.ContentItem.ContentType, part.Path); + var pathsLikeThis = GetSimilarPaths(part.Path); - // If the part is already a valid content item, don't include it in the list - // of slug to consider for conflict detection - if (part.ContentItem.Id != 0) - slugsLikeThis = slugsLikeThis.Where(p => p.ContentItem.Id != part.ContentItem.Id); + // Don't include this part in the list + // of slugs to consider for conflict detection + pathsLikeThis = pathsLikeThis.Where(p => p.ContentItem.Id != part.ContentItem.Id); //todo: (heskew) need better messages - if (slugsLikeThis.Count() > 0) + if (pathsLikeThis.Count() > 0) { var originalSlug = part.Slug; //todo: (heskew) make auto-uniqueness optional - part.Slug = GenerateUniqueSlug(part.Slug, slugsLikeThis.Select(p => p.Slug)); + part.Slug = GenerateUniqueSlug(part, pathsLikeThis.Select(p => p.Path)); if (originalSlug != part.Slug) { return false;