From 9c70163f672c535dac4062f2852efa0d633c2668 Mon Sep 17 00:00:00 2001 From: Nathan Heskew Date: Mon, 23 Aug 2010 16:14:10 -0700 Subject: [PATCH 1/4] Fixing up the (Orchard.Core's) Routable module so items can't be published with conflicting paths --HG-- branch : dev --- .../Common/Services/RoutableServiceTests.cs | 88 ++++++++----------- .../Core/Routable/Drivers/RoutePartDriver.cs | 6 +- .../Routable/Handlers/RoutePartHandler.cs | 33 ++++++- .../Core/Routable/Models/RoutePart.cs | 10 +-- .../Routable/Services/IRoutableService.cs | 6 +- .../Core/Routable/Services/RoutableService.cs | 26 +++--- 6 files changed, 92 insertions(+), 77 deletions(-) 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; From 6c86c89414bf05704b38e0608130ae5f04d5643c Mon Sep 17 00:00:00 2001 From: Nathan Heskew Date: Tue, 24 Aug 2010 11:41:41 -0700 Subject: [PATCH 2/4] A little more Routable path & slug bug fixing and cleanup --HG-- branch : dev --- .../Common/Services/RoutableServiceTests.cs | 6 ++--- .../Core/Routable/Drivers/RoutePartDriver.cs | 3 +-- .../Routable/Handlers/RoutePartHandler.cs | 5 +---- .../Core/Routable/Models/RoutePart.cs | 2 +- .../Routable/Services/IRoutableService.cs | 3 +-- .../Core/Routable/Services/RoutableService.cs | 22 +++++-------------- .../Orchard.Blogs/Commands/BlogCommands.cs | 2 +- .../Orchard.Blogs/Services/XmlRpcHandler.cs | 4 ++-- 8 files changed, 16 insertions(+), 31 deletions(-) diff --git a/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs b/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs index 77372a047..a08d40c8c 100644 --- a/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs +++ b/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs @@ -63,7 +63,7 @@ namespace Orchard.Core.Tests.Common.Services { t.Title = "Please do not use any of the following characters in your slugs: \":\", \"/\", \"?\", \"#\", \"[\", \"]\", \"@\", \"!\", \"$\", \"&\", \"'\", \"(\", \")\", \"*\", \"+\", \",\", \";\", \"=\""; }); - _routableService.FillSlug(thing.As()); + _routableService.FillSlugFromTitle(thing.As()); Assert.That(thing.Slug, Is.EqualTo("please-do-not-use-any-of-the-following-characters-in-your-slugs-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"")); } @@ -93,7 +93,7 @@ namespace Orchard.Core.Tests.Common.Services { veryVeryLongTitle += "aaaaaaaaaa"; var thing = CreateRoutePart(veryVeryLongTitle); - _routableService.FillSlug(thing); + _routableService.FillSlugFromTitle(thing); Assert.That(veryVeryLongTitle.Length, Is.AtLeast(1001)); Assert.That(thing.Slug.Length, Is.EqualTo(1000)); @@ -139,7 +139,7 @@ namespace Orchard.Core.Tests.Common.Services { public void GeneratedSlugIsLowerCased() { var thing = CreateRoutePart("This Is Some Interesting Title"); - _routableService.FillSlug(thing); + _routableService.FillSlugFromTitle(thing); Assert.That(thing.Slug, Is.EqualTo("this-is-some-interesting-title")); } diff --git a/src/Orchard.Web/Core/Routable/Drivers/RoutePartDriver.cs b/src/Orchard.Web/Core/Routable/Drivers/RoutePartDriver.cs index d806abeef..451b97c97 100644 --- a/src/Orchard.Web/Core/Routable/Drivers/RoutePartDriver.cs +++ b/src/Orchard.Web/Core/Routable/Drivers/RoutePartDriver.cs @@ -76,7 +76,6 @@ namespace Orchard.Core.Routable.Drivers { updater.TryUpdateModel(model, Prefix, null, null); part.Title = model.Title; part.Slug = model.Slug; - part.Path = part.GetPathFromSlug(model.Slug); if (!_routableService.IsSlugValid(part.Slug)) { updater.AddModelError("Routable.Slug", T("Please do not use any of the following characters in your slugs: \"/\", \":\", \"?\", \"#\", \"[\", \"]\", \"@\", \"!\", \"$\", \"&\", \"'\", \"(\", \")\", \"*\", \"+\", \",\", \";\", \"=\". No spaces are allowed (please use dashes or underscores instead).")); @@ -89,7 +88,7 @@ namespace Orchard.Core.Routable.Drivers { } // TEMP: path format patterns replaces this logic - part.Path = part.GetPathFromSlug(part.Slug); + part.Path = part.GetPathWithSlug(part.Slug); if (part.ContentItem.Id != 0 && model.PromoteToHomePage && _routableHomePageProvider != null) { CurrentSite.HomePage = _routableHomePageProvider.GetSettingValue(part.ContentItem.Id); diff --git a/src/Orchard.Web/Core/Routable/Handlers/RoutePartHandler.cs b/src/Orchard.Web/Core/Routable/Handlers/RoutePartHandler.cs index b0284e3f6..3fd4e05af 100644 --- a/src/Orchard.Web/Core/Routable/Handlers/RoutePartHandler.cs +++ b/src/Orchard.Web/Core/Routable/Handlers/RoutePartHandler.cs @@ -25,16 +25,13 @@ namespace Orchard.Core.Routable.Handlers { 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); + routable.Path = routable.GetPathWithSlug(routable.Slug); }); OnPublished((context, routable) => { diff --git a/src/Orchard.Web/Core/Routable/Models/RoutePart.cs b/src/Orchard.Web/Core/Routable/Models/RoutePart.cs index 3f1915ff4..2bc95485d 100644 --- a/src/Orchard.Web/Core/Routable/Models/RoutePart.cs +++ b/src/Orchard.Web/Core/Routable/Models/RoutePart.cs @@ -29,7 +29,7 @@ namespace Orchard.Core.Routable.Models { return null; } - public string GetPathFromSlug(string slug) { + public string GetPathWithSlug(string slug) { // TEMP: path format patterns replaces this logic var containerPath = GetContainerPath(); if (string.IsNullOrEmpty(containerPath)) { diff --git a/src/Orchard.Web/Core/Routable/Services/IRoutableService.cs b/src/Orchard.Web/Core/Routable/Services/IRoutableService.cs index 8156ce4d6..37ecc3772 100644 --- a/src/Orchard.Web/Core/Routable/Services/IRoutableService.cs +++ b/src/Orchard.Web/Core/Routable/Services/IRoutableService.cs @@ -4,8 +4,7 @@ using Orchard.Core.Routable.Models; 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; + void FillSlugFromTitle(TModel model) where TModel : RoutePart; string GenerateUniqueSlug(RoutePart part, IEnumerable existingPaths); /// diff --git a/src/Orchard.Web/Core/Routable/Services/RoutableService.cs b/src/Orchard.Web/Core/Routable/Services/RoutableService.cs index 5a79c0648..a35caf878 100644 --- a/src/Orchard.Web/Core/Routable/Services/RoutableService.cs +++ b/src/Orchard.Web/Core/Routable/Services/RoutableService.cs @@ -13,7 +13,7 @@ namespace Orchard.Core.Routable.Services { _contentManager = contentManager; } - public void FillSlug(TModel model) where TModel : RoutePart { + public void FillSlugFromTitle(TModel model) where TModel : RoutePart { if (!string.IsNullOrEmpty(model.Slug) || string.IsNullOrEmpty(model.Title)) return; @@ -29,13 +29,6 @@ namespace Orchard.Core.Routable.Services { model.Slug = slug.ToLowerInvariant(); } - public void FillSlug(TModel model, Func generateSlug) where TModel : RoutePart { - if (!string.IsNullOrEmpty(model.Slug) || string.IsNullOrEmpty(model.Title)) - return; - - model.Slug = generateSlug(model.Title).ToLowerInvariant(); - } - public string GenerateUniqueSlug(RoutePart part, IEnumerable existingPaths) { var slugCandidate = part.Slug; if (existingPaths == null || !existingPaths.Contains(part.Path)) @@ -74,24 +67,21 @@ namespace Orchard.Core.Routable.Services { return slug == null || String.IsNullOrEmpty(slug.Trim()) || Regex.IsMatch(slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$"); } - public bool ProcessSlug(RoutePart part) - { - FillSlug(part); + public bool ProcessSlug(RoutePart part) { + FillSlugFromTitle(part); if (string.IsNullOrEmpty(part.Slug)) - { return true; - } + part.Path = part.GetPathWithSlug(part.Slug); var pathsLikeThis = GetSimilarPaths(part.Path); - // Don't include this part in the list + // 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 (pathsLikeThis.Count() > 0) - { + if (pathsLikeThis.Count() > 0) { var originalSlug = part.Slug; //todo: (heskew) make auto-uniqueness optional part.Slug = GenerateUniqueSlug(part, pathsLikeThis.Select(p => p.Path)); diff --git a/src/Orchard.Web/Modules/Orchard.Blogs/Commands/BlogCommands.cs b/src/Orchard.Web/Modules/Orchard.Blogs/Commands/BlogCommands.cs index bb500fe05..6fd5f3349 100644 --- a/src/Orchard.Web/Modules/Orchard.Blogs/Commands/BlogCommands.cs +++ b/src/Orchard.Web/Modules/Orchard.Blogs/Commands/BlogCommands.cs @@ -100,7 +100,7 @@ namespace Orchard.Blogs.Commands { post.As().Container = blog; var slug = Slugify(postName); post.As().Slug = slug; - post.As().Path = post.As().GetPathFromSlug(slug); + post.As().Path = post.As().GetPathWithSlug(slug); post.As().Title = postName; post.As().Text = item.Element("description").Value; _contentManager.Create(post); diff --git a/src/Orchard.Web/Modules/Orchard.Blogs/Services/XmlRpcHandler.cs b/src/Orchard.Web/Modules/Orchard.Blogs/Services/XmlRpcHandler.cs index 820a45236..f56ab1803 100644 --- a/src/Orchard.Web/Modules/Orchard.Blogs/Services/XmlRpcHandler.cs +++ b/src/Orchard.Web/Modules/Orchard.Blogs/Services/XmlRpcHandler.cs @@ -182,8 +182,8 @@ namespace Orchard.Blogs.Services { if (blogPost.Is()) { blogPost.As().Title = title; blogPost.As().Slug = slug; - _routableService.FillSlug(blogPost.As()); - blogPost.As().Path = blogPost.As().GetPathFromSlug(blogPost.As().Slug); + _routableService.FillSlugFromTitle(blogPost.As()); + blogPost.As().Path = blogPost.As().GetPathWithSlug(blogPost.As().Slug); } _contentManager.Create(blogPost.ContentItem, VersionOptions.Draft); From cdbdf0db7ff7d3f1edb9f26baa4308fd6e1b6544 Mon Sep 17 00:00:00 2001 From: Nathan Heskew Date: Tue, 24 Aug 2010 12:45:23 -0700 Subject: [PATCH 3/4] Removing an extra little bit of CSS from the TheAdmin theme that set all text input to basically the same width as .textMedium If any text input elements on that page need to be wider then the appropriate class(es) should be used insted. --HG-- branch : dev --- src/Orchard.Web/Themes/TheAdmin/Styles/site.css | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Orchard.Web/Themes/TheAdmin/Styles/site.css b/src/Orchard.Web/Themes/TheAdmin/Styles/site.css index 61ae5be10..4c67cfb98 100644 --- a/src/Orchard.Web/Themes/TheAdmin/Styles/site.css +++ b/src/Orchard.Web/Themes/TheAdmin/Styles/site.css @@ -825,11 +825,6 @@ table .button { .settings legend { margin:0 0 -.4em 0; } -/* todo: This style is the same as textMedium. Consolidate if possible */ -.settings input.text, .settings input.text-box { - width:26em; -} - /* Fields ----------------------------------------------------------*/ From f3f04d6ea045ab04fbb82c13d26d207573a7f5c0 Mon Sep 17 00:00:00 2001 From: Nathan Heskew Date: Thu, 26 Aug 2010 07:32:45 -0700 Subject: [PATCH 4/4] Allowing the '/' in slugs. - Also moved the RoutableServiceTests to a more appropriate location --HG-- branch : dev --- src/Orchard.Core.Tests/Orchard.Core.Tests.csproj | 2 +- .../Services/RoutableServiceTests.cs | 13 +++++++++---- .../Core/Routable/Drivers/RoutePartDriver.cs | 2 +- .../Core/Routable/Services/RoutableService.cs | 2 +- 4 files changed, 12 insertions(+), 7 deletions(-) rename src/Orchard.Core.Tests/{Common => Routable}/Services/RoutableServiceTests.cs (94%) diff --git a/src/Orchard.Core.Tests/Orchard.Core.Tests.csproj b/src/Orchard.Core.Tests/Orchard.Core.Tests.csproj index 7eab2d21c..1c720d5d5 100644 --- a/src/Orchard.Core.Tests/Orchard.Core.Tests.csproj +++ b/src/Orchard.Core.Tests/Orchard.Core.Tests.csproj @@ -101,7 +101,7 @@ - + diff --git a/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs b/src/Orchard.Core.Tests/Routable/Services/RoutableServiceTests.cs similarity index 94% rename from src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs rename to src/Orchard.Core.Tests/Routable/Services/RoutableServiceTests.cs index a08d40c8c..62b493e66 100644 --- a/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs +++ b/src/Orchard.Core.Tests/Routable/Services/RoutableServiceTests.cs @@ -24,7 +24,7 @@ using System.Web.Routing; using Orchard.Tests.Stubs; using Orchard.UI.Notify; -namespace Orchard.Core.Tests.Common.Services { +namespace Orchard.Core.Tests.Routable.Services { [TestFixture] public class RoutableServiceTests : DatabaseEnabledTestsBase { [SetUp] @@ -60,12 +60,17 @@ namespace Orchard.Core.Tests.Common.Services { var thing = contentManager.Create(ThingDriver.ContentType.Name, t => { t.As().Record = new RoutePartRecord(); - t.Title = "Please do not use any of the following characters in your slugs: \":\", \"/\", \"?\", \"#\", \"[\", \"]\", \"@\", \"!\", \"$\", \"&\", \"'\", \"(\", \")\", \"*\", \"+\", \",\", \";\", \"=\""; + t.Title = "Please do not use any of the following characters in your slugs: \":\", \"?\", \"#\", \"[\", \"]\", \"@\", \"!\", \"$\", \"&\", \"'\", \"(\", \")\", \"*\", \"+\", \",\", \";\", \"=\""; }); _routableService.FillSlugFromTitle(thing.As()); - Assert.That(thing.Slug, Is.EqualTo("please-do-not-use-any-of-the-following-characters-in-your-slugs-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"")); + Assert.That(thing.Slug, Is.EqualTo("please-do-not-use-any-of-the-following-characters-in-your-slugs-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"")); + } + + [Test] + public void SlashInSlugIsAllowed() { + Assert.That(_routableService.IsSlugValid("some/page"), Is.True); } [Test] @@ -80,7 +85,7 @@ namespace Orchard.Core.Tests.Common.Services { public void InvalidCharacterShouldBeRefusedInSlugs() { Assert.That(_routableService.IsSlugValid("aaaa-_aaaa"), Is.True); - foreach (var c in @"/:?#[]@!$&'()*+,;= ") { + foreach (var c in @":?#[]@!$&'()*+,;= ") { Assert.That(_routableService.IsSlugValid("a" + c + "b"), Is.False); } } diff --git a/src/Orchard.Web/Core/Routable/Drivers/RoutePartDriver.cs b/src/Orchard.Web/Core/Routable/Drivers/RoutePartDriver.cs index 451b97c97..727184b00 100644 --- a/src/Orchard.Web/Core/Routable/Drivers/RoutePartDriver.cs +++ b/src/Orchard.Web/Core/Routable/Drivers/RoutePartDriver.cs @@ -78,7 +78,7 @@ namespace Orchard.Core.Routable.Drivers { part.Slug = model.Slug; if (!_routableService.IsSlugValid(part.Slug)) { - updater.AddModelError("Routable.Slug", T("Please do not use any of the following characters in your slugs: \"/\", \":\", \"?\", \"#\", \"[\", \"]\", \"@\", \"!\", \"$\", \"&\", \"'\", \"(\", \")\", \"*\", \"+\", \",\", \";\", \"=\". No spaces are allowed (please use dashes or underscores instead).")); + updater.AddModelError("Routable.Slug", T("Please do not use any of the following characters in your slugs: \":\", \"?\", \"#\", \"[\", \"]\", \"@\", \"!\", \"$\", \"&\", \"'\", \"(\", \")\", \"*\", \"+\", \",\", \";\", \"=\". No spaces are allowed (please use dashes or underscores instead).")); } string originalSlug = part.Slug; diff --git a/src/Orchard.Web/Core/Routable/Services/RoutableService.cs b/src/Orchard.Web/Core/Routable/Services/RoutableService.cs index a35caf878..7d29d7397 100644 --- a/src/Orchard.Web/Core/Routable/Services/RoutableService.cs +++ b/src/Orchard.Web/Core/Routable/Services/RoutableService.cs @@ -64,7 +64,7 @@ namespace Orchard.Core.Routable.Services { public bool IsSlugValid(string slug) { // see http://tools.ietf.org/html/rfc3987 for prohibited chars - return slug == null || String.IsNullOrEmpty(slug.Trim()) || Regex.IsMatch(slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$"); + return slug == null || String.IsNullOrEmpty(slug.Trim()) || Regex.IsMatch(slug, @"^[^:?#\[\]@!$&'()*+,;=\s]+$"); } public bool ProcessSlug(RoutePart part) {