Added unit tests on slugs generation unicity among the sae content types

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

@@ -10,6 +10,10 @@ using Orchard.ContentManagement.Records;
 using Orchard.Core.Common.Models;
 using Orchard.Core.Common.Services;
 using Orchard.Tests.Modules;
+using Orchard.Core.Common.Handlers;
+using System.Web.Mvc;
+using System.Web.Routing;
+using Orchard.Tests.Stubs;
 
 namespace Orchard.Core.Tests.Common.Services {
     [TestFixture]
@@ -23,7 +27,13 @@ namespace Orchard.Core.Tests.Common.Services {
         public override void Register(ContainerBuilder builder) {
             builder.RegisterType<DefaultContentManager>().As<IContentManager>();
             builder.RegisterType<ThingHandler>().As<IContentHandler>();
+            builder.RegisterType<StuffHandler>().As<IContentHandler>();
             builder.RegisterType<RoutableService>().As<IRoutableService>();
+
+            builder.RegisterType<DefaultContentQuery>().As<IContentQuery>();
+            builder.RegisterInstance(new UrlHelper(new RequestContext(new StubHttpContext("~/"), new RouteData()))).As<UrlHelper>();
+            builder.RegisterType<RoutableAspectHandler>().As<IContentHandler>();
+
         }
 
         private IRoutableService _routableService;
@@ -111,6 +121,47 @@ namespace Orchard.Core.Tests.Common.Services {
             Assert.That(thing.Slug, Is.EqualTo("this-is-some-interesting-title"));
         }
 
+        [Test]
+        public void GeneratedSlugsShouldBeUniqueAmongContentType()
+        {
+            var contentManager = _container.Resolve<IContentManager>();
+
+            var thing1 = contentManager.Create<Thing>(ThingDriver.ContentType.Name, t =>
+            {
+                t.As<RoutableAspect>().Record = new RoutableRecord();
+                t.Title = "This Is Some Interesting Title";
+            });
+            
+            var thing2 = contentManager.Create<Thing>(ThingDriver.ContentType.Name , t =>
+            {
+                t.As<RoutableAspect>().Record = new RoutableRecord();
+                t.Title = "This Is Some Interesting Title";
+            });
+
+            Assert.AreNotEqual(thing1.Slug, thing2.Slug);
+        }
+
+        [Test]
+        public void SlugsCanBeDuplicatedAccrossContentTypes()
+        {
+            var contentManager = _container.Resolve<IContentManager>();
+
+            var thing = contentManager.Create<Thing>(ThingDriver.ContentType.Name, t =>
+            {
+                t.As<RoutableAspect>().Record = new RoutableRecord();
+                t.Title = "This Is Some Interesting Title";
+            });
+
+            var stuff = contentManager.Create<Stuff>(StuffDriver.ContentType.Name, s =>
+            {
+                s.As<RoutableAspect>().Record = new RoutableRecord();
+                s.Title = "This Is Some Interesting Title";
+            });
+
+            Assert.AreEqual(thing.Slug, stuff.Slug);
+        }
+
+
         protected override IEnumerable<Type> DatabaseTypes {
             get {
                 return new[] {
@@ -155,5 +206,43 @@ namespace Orchard.Core.Tests.Common.Services {
                 DisplayName = "Thing"
             };
         }
+
+        [UsedImplicitly]
+        public class StuffHandler : ContentHandler
+        {
+            public StuffHandler()
+            {
+                Filters.Add(new ActivatingFilter<Stuff>(StuffDriver.ContentType.Name));
+                Filters.Add(new ActivatingFilter<ContentPart<CommonVersionRecord>>(StuffDriver.ContentType.Name));
+                Filters.Add(new ActivatingFilter<CommonAspect>(StuffDriver.ContentType.Name));
+                Filters.Add(new ActivatingFilter<RoutableAspect>(StuffDriver.ContentType.Name));
+            }
+        }
+
+        public class Stuff : ContentPart
+        {
+            public int Id { get { return ContentItem.Id; } }
+
+            public string Title
+            {
+                get { return this.As<RoutableAspect>().Title; }
+                set { this.As<RoutableAspect>().Title = value; }
+            }
+
+            public string Slug
+            {
+                get { return this.As<RoutableAspect>().Slug; }
+                set { this.As<RoutableAspect>().Slug = value; }
+            }
+        }
+
+        public class StuffDriver : ContentItemDriver<Stuff>
+        {
+            public readonly static ContentType ContentType = new ContentType
+            {
+                Name = "stuff",
+                DisplayName = "Stuff"
+            };
+        }
     }
 }

src/Orchard.Core.Tests/Orchard.Core.Tests.csproj

@@ -60,6 +60,9 @@
       <HintPath>..\..\lib\sqlite\System.Data.SQLite.DLL</HintPath>
       <Private>True</Private>
     </Reference>
+    <Reference Include="System.Web.Abstractions">
+      <RequiredTargetFramework>3.5</RequiredTargetFramework>
+    </Reference>
     <Reference Include="System.Web.Mvc, Version=2.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
       <SpecificVersion>False</SpecificVersion>
       <HintPath>..\..\lib\aspnetmvc\System.Web.Mvc.dll</HintPath>

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

@@ -5,6 +5,7 @@ using Orchard.Core.Common.Models;
 using Orchard.Core.Common.ViewModels;
 using Orchard.Core.Common.Services;
 using Orchard.Localization;
+using Orchard.UI.Notify;
 
 namespace Orchard.Core.Common.Drivers {
     [UsedImplicitly]
@@ -40,7 +41,12 @@ namespace Orchard.Core.Common.Drivers {
                 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);
+            string originalSlug = part.Slug;
+            if(!_routableService.ProcessSlug(part)) {
+                _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));
+            }
+                
             return ContentPartTemplate(model, TemplateName, Prefix).Location("primary", "before.5");
         }
 

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

@@ -34,7 +34,7 @@ namespace Orchard.Core.Common.Handlers {
                 routable.ContentItemBasePath = url;
             });
 
-            OnPublished<RoutableAspect>((context, bp) => routableService.ProcessSlug(bp));
+            OnCreated<RoutableAspect>((context, ra) => routableService.ProcessSlug(ra));
 
         }
 

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

@@ -21,7 +21,8 @@ namespace Orchard.Core.Common.Services {
         /// <summary>
         /// Defines the slug of a RoutableAspect and validate its unicity
         /// </summary>
-        void ProcessSlug(RoutableAspect part);
+        /// <returns>True if the slug has been created, False if a conflict occured</returns>
+        bool ProcessSlug(RoutableAspect part);
 
     }
 }

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

@@ -11,14 +11,10 @@ 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) {
+        public RoutableService(IContentManager contentManager) {
-            _services = services;
             _contentManager = contentManager;
-            T = NullLocalizer.Instance;
         }
 
         public void FillSlug<TModel>(TModel model) where TModel : RoutableAspect {
@@ -71,10 +67,9 @@ namespace Orchard.Core.Common.Services {
         {
             return
                 _contentManager.Query(contentType).Join<RoutableRecord>()
-                    .Where(rr => rr.Slug.StartsWith(slug, StringComparison.OrdinalIgnoreCase))
                     .List()
-                    .Cast<RoutableRecord>()
+                    .Select(i => i.As<RoutableAspect>().Slug)
-                    .Select(i => i.Slug)
+                    .Where(rr => rr.StartsWith(slug, StringComparison.OrdinalIgnoreCase)) // todo: for some reason the filter doesn't work within the query, even without StringComparison or StartsWith
                     .ToArray();
         }
 
@@ -82,13 +77,13 @@ namespace Orchard.Core.Common.Services {
             return slug == null || String.IsNullOrEmpty(slug.Trim()) || !Regex.IsMatch(slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$");
         }
 
-        public void ProcessSlug(RoutableAspect part)
+        public bool ProcessSlug(RoutableAspect part)
         {
             FillSlug(part);
 
             if (string.IsNullOrEmpty(part.Slug))
             {
-                return;
+                return true;
             }
 
             var slugsLikeThis = GetSimilarSlugs(part.ContentItem.ContentType, part.Slug);
@@ -101,10 +96,11 @@ namespace Orchard.Core.Common.Services {
                 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}\"",
+                    return false;
-                            originalSlug, part.Slug, part.ContentItem.ContentType));
                 }
             }
+
+            return true;
         }
     }
 }