A little more Routable path & slug bug fixing and cleanup

--HG--
branch : dev

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<RoutePart>());
+            _routableService.FillSlugFromTitle(thing.As<RoutePart>());
 
             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"));
         }

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);

src/Orchard.Web/Core/Routable/Handlers/RoutePartHandler.cs

@@ -25,16 +25,13 @@ namespace Orchard.Core.Routable.Handlers {
             Action<RoutePart> 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<RoutePart>((context, routable) => {

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)) {

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>(TModel model) where TModel : RoutePart;
+        void FillSlugFromTitle<TModel>(TModel model) where TModel : RoutePart;
-        void FillSlug<TModel>(TModel model, Func<string, string> generateSlug) where TModel : RoutePart;
         string GenerateUniqueSlug(RoutePart part, IEnumerable<string> existingPaths);
 
         /// <summary>

src/Orchard.Web/Core/Routable/Services/RoutableService.cs

@@ -13,7 +13,7 @@ namespace Orchard.Core.Routable.Services {
             _contentManager = contentManager;
         }
 
-        public void FillSlug<TModel>(TModel model) where TModel : RoutePart {
+        public void FillSlugFromTitle<TModel>(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>(TModel model, Func<string, string> 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<string> 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)
+        public bool ProcessSlug(RoutePart part) {
-        {
+            FillSlugFromTitle(part);
-            FillSlug(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));

src/Orchard.Web/Modules/Orchard.Blogs/Commands/BlogCommands.cs

@@ -100,7 +100,7 @@ namespace Orchard.Blogs.Commands {
                 post.As<ICommonPart>().Container = blog;
                 var slug = Slugify(postName);
                 post.As<RoutePart>().Slug = slug;
-                post.As<RoutePart>().Path = post.As<RoutePart>().GetPathFromSlug(slug);
+                post.As<RoutePart>().Path = post.As<RoutePart>().GetPathWithSlug(slug);
                 post.As<RoutePart>().Title = postName;
                 post.As<BodyPart>().Text = item.Element("description").Value;
                 _contentManager.Create(post);

src/Orchard.Web/Modules/Orchard.Blogs/Services/XmlRpcHandler.cs

@@ -182,8 +182,8 @@ namespace Orchard.Blogs.Services {
             if (blogPost.Is<RoutePart>()) {
                 blogPost.As<RoutePart>().Title = title;
                 blogPost.As<RoutePart>().Slug = slug;
-                _routableService.FillSlug(blogPost.As<RoutePart>());
+                _routableService.FillSlugFromTitle(blogPost.As<RoutePart>());
-                blogPost.As<RoutePart>().Path = blogPost.As<RoutePart>().GetPathFromSlug(blogPost.As<RoutePart>().Slug);
+                blogPost.As<RoutePart>().Path = blogPost.As<RoutePart>().GetPathWithSlug(blogPost.As<RoutePart>().Slug);
             }
 
             _contentManager.Create(blogPost.ContentItem, VersionOptions.Draft);