Moved slugs management from Blogs and Pages to Routable

Corrected slug generation on client side

src/Orchard.Web/Core/Common/Drivers/RoutableDriver.cs

@@ -3,16 +3,30 @@ using Orchard.ContentManagement;
 using Orchard.ContentManagement.Drivers;
 using Orchard.Core.Common.Models;
 using Orchard.Core.Common.ViewModels;
+using Orchard.Core.Common.Services;
+using Orchard.Localization;
 
 namespace Orchard.Core.Common.Drivers {
     [UsedImplicitly]
     public class Routable : ContentPartDriver<RoutableAspect> {
         private const string TemplateName = "Parts/Common.Routable";
 
+        private readonly IOrchardServices _services;
+        private readonly IRoutableService _routableService;
+        private Localizer T { get; set; }
+
         protected override string Prefix {
             get { return "Routable"; }
         }
 
+        public Routable(IOrchardServices services, IRoutableService routableService)
+        {
+            _services = services;
+            _routableService = routableService;
+
+            T = NullLocalizer.Instance;
+        }
+
         protected override DriverResult Editor(RoutableAspect part) {
             var model = new RoutableEditorViewModel { Prefix = Prefix, RoutableAspect = part };
             return ContentPartTemplate(model, TemplateName, Prefix).Location("primary", "before.5");
@@ -21,7 +35,14 @@ namespace Orchard.Core.Common.Drivers {
         protected override DriverResult Editor(RoutableAspect part, IUpdateModel updater) {
             var model = new RoutableEditorViewModel { Prefix = Prefix, RoutableAspect = part };
             updater.TryUpdateModel(model, Prefix, null, null);
+
+            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).").ToString());
+            }
+
+            _routableService.ProcessSlug(part);
             return ContentPartTemplate(model, TemplateName, Prefix).Location("primary", "before.5");
         }
+
     }
 }

src/Orchard.Web/Core/Common/Handlers/RoutableAspectHandler.cs

@@ -7,6 +7,7 @@ using Orchard.ContentManagement;
 using Orchard.ContentManagement.Drivers;
 using Orchard.ContentManagement.Handlers;
 using Orchard.Core.Common.Models;
+using Orchard.Core.Common.Services;
 using Orchard.Data;
 
 namespace Orchard.Core.Common.Handlers {
@@ -14,7 +15,7 @@ namespace Orchard.Core.Common.Handlers {
     public class RoutableAspectHandler : ContentHandler {
         private readonly IEnumerable<IContentItemDriver> _contentItemDrivers;
 
-        public RoutableAspectHandler(IRepository<RoutableRecord> repository, IEnumerable<IContentItemDriver> contentItemDrivers, UrlHelper urlHelper) {
+        public RoutableAspectHandler(IRepository<RoutableRecord> repository, IEnumerable<IContentItemDriver> contentItemDrivers, IRoutableService routableService, UrlHelper urlHelper) {
             _contentItemDrivers = contentItemDrivers;
 
             Filters.Add(StorageFilter.For(repository));
@@ -32,6 +33,9 @@ namespace Orchard.Core.Common.Handlers {
 
                 routable.ContentItemBasePath = url;
             });
+
+            OnPublished<RoutableAspect>((context, bp) => routableService.ProcessSlug(bp));
+
         }
 
         private static RouteValueDictionary GetRouteValues(IContentItemDriver driver, ContentItem contentItem) {

src/Orchard.Web/Core/Common/Services/IRoutableService.cs

@@ -7,5 +7,21 @@ namespace Orchard.Core.Common.Services {
         void FillSlug<TModel>(TModel model) where TModel : RoutableAspect;
         void FillSlug<TModel>(TModel model, Func<string, string> generateSlug) where TModel : RoutableAspect;
         string GenerateUniqueSlug(string slugCandidate, IEnumerable<string> existingSlugs);
+
+        /// <summary>
+        /// Returns any content item of the specified content type with similar slugs
+        /// </summary>
+        string[] GetSimilarSlugs(string contentType, string slug);
+
+        /// <summary>
+        /// Validates the given slug
+        /// </summary>
+        bool IsSlugValid(string slug);
+
+        /// <summary>
+        /// Defines the slug of a RoutableAspect and validate its unicity
+        /// </summary>
+        void ProcessSlug(RoutableAspect part);
+
     }
 }

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

@@ -4,10 +4,23 @@ using System.Linq;
 using System.Text.RegularExpressions;
 using JetBrains.Annotations;
 using Orchard.Core.Common.Models;
+using Orchard.ContentManagement;
+using Orchard.Localization;
+using Orchard.UI.Notify;
 
 namespace Orchard.Core.Common.Services {
     [UsedImplicitly]
     public class RoutableService : IRoutableService {
+        private readonly IOrchardServices _services;
+        private readonly IContentManager _contentManager;
+        private Localizer T { get; set; }
+
+        public RoutableService(IOrchardServices services, IContentManager contentManager) {
+            _services = services;
+            _contentManager = contentManager;
+            T = NullLocalizer.Instance;
+        }
+
         public void FillSlug<TModel>(TModel model) where TModel : RoutableAspect {
             if (!string.IsNullOrEmpty(model.Slug) || string.IsNullOrEmpty(model.Title))
                 return;
@@ -31,8 +44,6 @@ namespace Orchard.Core.Common.Services {
             model.Slug = generateSlug(model.Title).ToLowerInvariant();
         }
 
-
-
         public string GenerateUniqueSlug(string slugCandidate, IEnumerable<string> existingSlugs) {
             if (existingSlugs == null || !existingSlugs.Contains(slugCandidate))
                 return slugCandidate;
@@ -55,5 +66,45 @@ namespace Orchard.Core.Common.Services {
                        ? (int?)++v
                        : null;
         }
+
+        public string[] GetSimilarSlugs(string contentType, string slug)
+        {
+            return
+                _contentManager.Query(contentType).Join<RoutableRecord>()
+                    .Where(rr => rr.Slug.StartsWith(slug, StringComparison.OrdinalIgnoreCase))
+                    .List()
+                    .Cast<RoutableRecord>()
+                    .Select(i => i.Slug)
+                    .ToArray();
+        }
+
+        public bool IsSlugValid(string slug) {
+            return slug == null || String.IsNullOrEmpty(slug.Trim()) || !Regex.IsMatch(slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$");
+        }
+
+        public void ProcessSlug(RoutableAspect part)
+        {
+            FillSlug(part);
+
+            if (string.IsNullOrEmpty(part.Slug))
+            {
+                return;
+            }
+
+            var slugsLikeThis = GetSimilarSlugs(part.ContentItem.ContentType, part.Slug);
+
+            //todo: (heskew) need better messages
+            if (slugsLikeThis.Length > 0)
+            {
+                var originalSlug = part.Slug;
+                //todo: (heskew) make auto-uniqueness optional
+                part.Slug = GenerateUniqueSlug(part.Slug, slugsLikeThis);
+
+                if (originalSlug != part.Slug) {
+                    _services.Notifier.Warning(T("Slugs in conflict. \"{0}\" is already set for a previously created {2} so now it has the slug \"{1}\"",
+                            originalSlug, part.Slug, part.ContentItem.ContentType));
+                }
+            }
+        }
     }
 }

src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogDriver.cs

@@ -27,14 +27,12 @@ namespace Orchard.Blogs.Drivers {
         private readonly IContentManager _contentManager;
         private readonly IBlogService _blogService;
         private readonly IBlogPostService _blogPostService;
-        private readonly IRoutableService _routableService;
 
-        public BlogDriver(IOrchardServices services, IContentManager contentManager, IBlogService blogService, IBlogPostService blogPostService, IRoutableService routableService) {
+        public BlogDriver(IOrchardServices services, IContentManager contentManager, IBlogService blogService, IBlogPostService blogPostService) {
             Services = services;
             _contentManager = contentManager;
             _blogService = blogService;
             _blogPostService = blogPostService;
-            _routableService = routableService;
             T = NullLocalizer.Instance;
         }
 
@@ -97,47 +95,9 @@ namespace Orchard.Blogs.Drivers {
         protected override DriverResult Editor(Blog blog, IUpdateModel updater) {
             updater.TryUpdateModel(blog, Prefix, null, null);
 
-            //todo: (heskew) something better needs to be done with this...still feels shoehorned in here
-            ProcessSlug(blog, updater);
-
             return Combined(
                 ContentItemTemplate("Items/Blogs.Blog"),
                 ContentPartTemplate(blog, "Parts/Blogs.Blog.Fields").Location("primary", "1"));
         }
-
-        private void ProcessSlug(Blog blog, IUpdateModel updater) {
-            _routableService.FillSlug(blog.As<RoutableAspect>());
-            
-
-            if (string.IsNullOrEmpty(blog.Slug)) {
-                return;
-
-                // OR
-
-                //    updater.AddModelError("Routable.Slug", T("The slug is required.").ToString());
-                //    return;
-            }
-
-
-            if (!Regex.IsMatch(blog.Slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$")) {
-                //todo: (heskew) get rid of the hard-coded prefix
-                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).").ToString());
-            }
-
-            var slugsLikeThis = _blogService.Get().Where(
-                b => b.Slug.StartsWith(blog.Slug, StringComparison.OrdinalIgnoreCase) &&
-                     b.Id != blog.Id).Select(b => b.Slug);
-
-            //todo: (heskew) need better messages
-            if (slugsLikeThis.Count() > 0) {
-                var originalSlug = blog.Slug;
-                //todo: (heskew) make auto-uniqueness optional
-                blog.Slug = _routableService.GenerateUniqueSlug(blog.Slug, slugsLikeThis);
-
-                if (originalSlug != blog.Slug)
-                    Services.Notifier.Warning(T("Slugs in conflict. \"{0}\" is already set for a previously created blog so this blog now has the slug \"{1}\"",
-                                                originalSlug, blog.Slug));
-            }
-        }
     }
 }

src/Orchard.Web/Modules/Orchard.Blogs/Handlers/BlogPostHandler.cs

@@ -16,12 +16,10 @@ namespace Orchard.Blogs.Handlers {
     [UsedImplicitly]
     public class BlogPostHandler : ContentHandler {
         private readonly IBlogPostService _blogPostService;
-        private readonly IRoutableService _routableService;
         private readonly IOrchardServices _orchardServices;
 
-        public BlogPostHandler(IBlogService blogService, IBlogPostService blogPostService, IRoutableService routableService, IOrchardServices orchardServices, RequestContext requestContext) {
+        public BlogPostHandler(IBlogService blogService, IBlogPostService blogPostService, IOrchardServices orchardServices, RequestContext requestContext) {
             _blogPostService = blogPostService;
-            _routableService = routableService;
             _orchardServices = orchardServices;
             T = NullLocalizer.Instance;
 
@@ -63,8 +61,6 @@ namespace Orchard.Blogs.Handlers {
             OnVersioned<BlogPost>((context, bp1, bp2) => updateBlogPostCount(bp2.Blog));
             OnRemoved<BlogPost>((context, bp) => updateBlogPostCount(bp.Blog));
 
-            OnPublished<BlogPost>((context, bp) => ProcessSlug(bp));
-
             OnRemoved<Blog>(
                 (context, b) =>
                 blogPostService.Get(context.ContentItem.As<Blog>()).ToList().ForEach(
@@ -72,23 +68,5 @@ namespace Orchard.Blogs.Handlers {
         }
 
         Localizer T { get; set; }
-
-        private void ProcessSlug(BlogPost post) {
-            _routableService.FillSlug(post.As<RoutableAspect>());
-
-            var slugsLikeThis = _blogPostService.Get(post.Blog, VersionOptions.Published).Where(
-                p => p.Slug.StartsWith(post.Slug, StringComparison.OrdinalIgnoreCase) &&
-                     p.Id != post.Id).Select(p => p.Slug);
-
-            //todo: (heskew) need better messages
-            if (slugsLikeThis.Count() > 0) {
-                //todo: (heskew) need better messages
-                var originalSlug = post.Slug;
-                post.Slug = _routableService.GenerateUniqueSlug(post.Slug, slugsLikeThis);
-
-                if (originalSlug != post.Slug)
-                    _orchardServices.Notifier.Warning(T("A different blog post is already published with this same slug ({0}) so a unique slug ({1}) was generated for this post.", originalSlug, post.Slug));
-            }
-        }
     }
 }

src/Orchard.Web/Modules/Orchard.Pages/Drivers/PageDriver.cs

@@ -76,74 +76,11 @@ namespace Orchard.Pages.Drivers {
         protected override DriverResult Editor(Page page, IUpdateModel updater) {
             updater.TryUpdateModel(page, Prefix, null, null);
 
-            //todo: (heskew) something better needs to be done with this...still feels shoehorned in here
-            ProcessSlug(page, updater);
-
             DateTime scheduled;
             if (DateTime.TryParse(string.Format("{0} {1}", page.ScheduledPublishUtcDate, page.ScheduledPublishUtcTime), out scheduled))
                 page.ScheduledPublishUtc = scheduled;
 
             return Editor(page);
         }
-
-        private void ProcessSlug(Page page, IUpdateModel updater) {
-            _routableService.FillSlug(page.As<RoutableAspect>(), Slugify);
-
-            if (string.IsNullOrEmpty(page.Slug)) {
-                return;
-
-                // OR
-
-                //updater.AddModelError("Routable.Slug", T("The slug is required.").ToString());
-                //return;
-            }
-
-            if (!Regex.IsMatch(page.Slug, @"^[^/:?#\[\]@!$&'()*+,;=\s](?(?=/)/[^/:?#\[\]@!$&'()*+,;=\s]|[^:?#\[\]@!$&'()*+,;=\s])*$")) {
-                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).").ToString());
-                return;
-            }
-
-            var slugsLikeThis = _pageService.Get(PageStatus.Published).Where(
-                p => p.Slug.StartsWith(page.Slug, StringComparison.OrdinalIgnoreCase) &&
-                     p.Id != page.Id).Select(p => p.Slug);
-
-            if (slugsLikeThis.Count() > 0) {
-                //todo: (heskew) need better messages
-                Services.Notifier.Warning(T("A different page is already published with this same slug."));
-
-                if (page.ContentItem.VersionRecord == null || page.ContentItem.VersionRecord.Published) {
-                    var originalSlug = page.Slug;
-                    //todo: (heskew) make auto-uniqueness optional
-                    page.Slug = _routableService.GenerateUniqueSlug(page.Slug, slugsLikeThis);
-
-                    //todo: (heskew) need better messages
-                    if (originalSlug != page.Slug)
-                        Services.Notifier.Warning(T("Slugs in conflict. \"{0}\" is already set for a previously published page so this page now has the slug \"{1}\"",
-                                                    originalSlug, page.Slug));
-                }
-            }
-        }
-
-        private static string Slugify(string value)
-        {
-            if (!string.IsNullOrEmpty(value))
-            {
-                //todo: (heskew) improve - just doing multi-pass regex replaces for now with the simple rules of
-                // (1) can't begin with a '/', (2) can't have adjacent '/'s and (3) can't have these characters
-                var startsoffbad = new Regex(@"^[\s/]+");
-                var slashhappy = new Regex("/{2,}");
-                var dissallowed = new Regex(@"[:?#\[\]@!$&'()*+,;=\s]+");
-
-                value = startsoffbad.Replace(value, "-");
-                value = slashhappy.Replace(value, "/");
-                value = dissallowed.Replace(value, "-");
-                value = value.Trim('-');
-
-                if (value.Length > 1000)
-                    value = value.Substring(0, 1000);
-            }
-
-            return value;
-        }
     }
 }

src/Orchard.Web/Modules/Orchard.Pages/Handlers/PageHandler.cs

@@ -15,12 +15,10 @@ namespace Orchard.Pages.Handlers {
     [UsedImplicitly]
     public class PageHandler : ContentHandler {
         private readonly IPageService _pageService;
-        private readonly IRoutableService _routableService;
         private readonly IOrchardServices _orchardServices;
 
-        public PageHandler(IPageService pageService, IRoutableService routableService, IOrchardServices orchardServices) {
+        public PageHandler(IPageService pageService, IOrchardServices orchardServices) {
             _pageService = pageService;
-            _routableService = routableService;
             _orchardServices = orchardServices;
             T = NullLocalizer.Instance;
 
@@ -31,27 +29,8 @@ namespace Orchard.Pages.Handlers {
             Filters.Add(new ActivatingFilter<BodyAspect>(PageDriver.ContentType.Name));
 
             OnLoaded<Page>((context, p) => p.ScheduledPublishUtc =  _pageService.GetScheduledPublishUtc(p));
-            OnPublished<Page>((context, p) => ProcessSlug(p));
         }
 
         Localizer T { get; set; }
-
-        private void ProcessSlug(Page page) {
-            _routableService.FillSlug(page.As<RoutableAspect>());
-
-            var slugsLikeThis = _pageService.Get(PageStatus.Published).Where(
-                p => p.Slug.StartsWith(page.Slug, StringComparison.OrdinalIgnoreCase) &&
-                     p.Id != page.Id).Select(p => p.Slug);
-
-            //todo: (heskew) need better messages
-            if (slugsLikeThis.Count() > 0) {
-                //todo: (heskew) need better messages
-                var originalSlug = page.Slug;
-                page.Slug = _routableService.GenerateUniqueSlug(page.Slug, slugsLikeThis);
-
-                if (originalSlug != page.Slug)
-                    _orchardServices.Notifier.Warning(T("A different page is already published with this same slug ({0}) so a unique slug ({1}) was generated for this page.", originalSlug, page.Slug));
-            }
-        }
     }
 }