Corrected slugs validation and added unit tests

src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs

@@ -52,6 +52,24 @@ namespace Orchard.Core.Tests.Common.Services {
             Assert.That(thing.Slug, Is.EqualTo("please-do-not-use-any-of-the-following-characters-in-your-slugs-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\""));
         }
 
+        [Test]
+        public void EmptySlugsShouldBeConsideredValid() {
+            // so that automatic generation on Publish occurs
+            Assert.That(_routableService.IsSlugValid(null), Is.True);
+            Assert.That(_routableService.IsSlugValid(String.Empty), Is.True);
+            Assert.That(_routableService.IsSlugValid("    "), Is.True);
+        }
+
+        [Test]
+        public void InvalidCharacterShouldBeRefusedInSlugs() {
+            Assert.That(_routableService.IsSlugValid("aaaa-_aaaa"), Is.True);
+
+            foreach (var c in @"/:?#[]@!$&'()*+,;= ") {
+                Assert.That(_routableService.IsSlugValid("a" + c + "b"), Is.False);
+            }
+        }
+    
+
         [Test]
         public void VeryLongStringTruncatedTo1000Chars() {
             var contentManager = _container.Resolve<IContentManager>();

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

@@ -74,7 +74,8 @@ namespace Orchard.Core.Common.Services {
         }
 
         public bool IsSlugValid(string slug) {
-            return slug == null || String.IsNullOrEmpty(slug.Trim()) || !Regex.IsMatch(slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$");
+            // see http://tools.ietf.org/html/rfc3987 for prohibited chars
+            return slug == null || String.IsNullOrEmpty(slug.Trim()) || Regex.IsMatch(slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$");
         }
 
         public bool ProcessSlug(RoutableAspect part)

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

@@ -1,16 +1,12 @@
 using System;
-using System.Linq;
-using System.Text.RegularExpressions;
 using System.Web.Routing;
 using JetBrains.Annotations;
 using Orchard.Blogs.Models;
 using Orchard.Blogs.Services;
 using Orchard.ContentManagement;
 using Orchard.ContentManagement.Drivers;
-using Orchard.Core.Common.Models;
 using Orchard.Core.Common.Services;
 using Orchard.Localization;
-using Orchard.UI.Notify;
 
 namespace Orchard.Blogs.Drivers {
     [UsedImplicitly]                                                                                                                                                                                        
@@ -78,54 +74,11 @@ namespace Orchard.Blogs.Drivers {
         protected override DriverResult Editor(BlogPost post, IUpdateModel updater) {
             updater.TryUpdateModel(post, Prefix, null, null);
 
-            //todo: (heskew) something better needs to be done with this...still feels shoehorned in here
-            ProcessSlug(post, updater);
-
             DateTime scheduled;
             if (DateTime.TryParse(string.Format("{0} {1}", post.ScheduledPublishUtcDate, post.ScheduledPublishUtcTime), out scheduled))
                 post.ScheduledPublishUtc = scheduled;
 
             return Editor(post);
         }
-
-        private void ProcessSlug(BlogPost post, IUpdateModel updater) {
-            _routableService.FillSlug(post.As<RoutableAspect>());
-
-            if (string.IsNullOrEmpty(post.Slug)) {
-                return;
-
-                // OR
-
-                //updater.AddModelError("Routable.Slug", T("The slug is required.").ToString());
-                //return;
-            }
-
-            if (!Regex.IsMatch(post.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());
-                return;
-            }
-
-            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
-                Services.Notifier.Warning(T("A different blog post is already published with this same slug."));
-
-                if (post.ContentItem.VersionRecord == null || post.ContentItem.VersionRecord.Published)
-                {
-                    var originalSlug = post.Slug;
-                    //todo: (heskew) make auto-uniqueness optional
-                    post.Slug = _routableService.GenerateUniqueSlug(post.Slug, slugsLikeThis);
-
-                    if (originalSlug != post.Slug)
-                        Services.Notifier.Warning(T("Slugs in conflict. \"{0}\" is already set for a previously created blog post so this post now has the slug \"{1}\"",
-                                                    originalSlug, post.Slug));
-                }
-            }
-        }
     }
 }