lsm/Orchard
1
0
Fork 0
mirror of https://github.com/OrchardCMS/Orchard.git synced 2025-10-15 19:54:57 +08:00
Code Issues Actions 4 Packages Projects Releases Wiki Activity

Fixing up the (Orchard.Core's) Routable module so items can't be published with conflicting paths

Browse Source 
--HG--
branch : dev
This commit is contained in:
Nathan Heskew
2010-08-23 16:14:10 -07:00
parent b36ad88a2f
commit 9c70163f67
6 changed files with 92 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files

84
src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs
View File

@@ -5,6 +5,7 @@ using JetBrains.Annotations;
using Moq; using Moq;
using NUnit.Framework; using NUnit.Framework;
using Orchard.ContentManagement; using Orchard.ContentManagement;
using Orchard.ContentManagement.Aspects;
using Orchard.ContentManagement.Drivers; using Orchard.ContentManagement.Drivers;
using Orchard.ContentManagement.Handlers; using Orchard.ContentManagement.Handlers;
using Orchard.ContentManagement.MetaData; using Orchard.ContentManagement.MetaData;
@@ -14,10 +15,14 @@ using Orchard.Core.Routable;
using Orchard.Core.Routable.Handlers; using Orchard.Core.Routable.Handlers;
using Orchard.Core.Routable.Models; using Orchard.Core.Routable.Models;
using Orchard.Core.Routable.Services; using Orchard.Core.Routable.Services;
using Orchard.Data;
using Orchard.Environment;
using Orchard.Security;
using Orchard.Tests.Modules; using Orchard.Tests.Modules;
using System.Web.Mvc; using System.Web.Mvc;
using System.Web.Routing; using System.Web.Routing;
using Orchard.Tests.Stubs; using Orchard.Tests.Stubs;
using Orchard.UI.Notify;
namespace Orchard.Core.Tests.Common.Services { namespace Orchard.Core.Tests.Common.Services {
[TestFixture] [TestFixture]
@@ -32,6 +37,10 @@ namespace Orchard.Core.Tests.Common.Services {
builder.RegisterType<DefaultContentManager>().As<IContentManager>(); builder.RegisterType<DefaultContentManager>().As<IContentManager>();
builder.RegisterType<DefaultContentManagerSession>().As<IContentManagerSession>(); builder.RegisterType<DefaultContentManagerSession>().As<IContentManagerSession>();
builder.RegisterInstance(new Mock<IContentDefinitionManager>().Object); builder.RegisterInstance(new Mock<IContentDefinitionManager>().Object);
builder.RegisterInstance(new Mock<ITransactionManager>().Object);
builder.RegisterInstance(new Mock<IAuthorizer>().Object);
builder.RegisterInstance(new Mock<INotifier>().Object);
builder.RegisterType<OrchardServices>().As<IOrchardServices>();
builder.RegisterType<ThingHandler>().As<IContentHandler>(); builder.RegisterType<ThingHandler>().As<IContentHandler>();
builder.RegisterType<StuffHandler>().As<IContentHandler>(); builder.RegisterType<StuffHandler>().As<IContentHandler>();
@@ -41,7 +50,6 @@ namespace Orchard.Core.Tests.Common.Services {
builder.RegisterType<DefaultContentQuery>().As<IContentQuery>(); builder.RegisterType<DefaultContentQuery>().As<IContentQuery>();
builder.RegisterInstance(new UrlHelper(new RequestContext(new StubHttpContext("~/"), new RouteData()))).As<UrlHelper>(); builder.RegisterInstance(new UrlHelper(new RequestContext(new StubHttpContext("~/"), new RouteData()))).As<UrlHelper>();
builder.RegisterType<RoutePartHandler>().As<IContentHandler>(); builder.RegisterType<RoutePartHandler>().As<IContentHandler>();
} }
private IRoutableService _routableService; private IRoutableService _routableService;
@@ -80,18 +88,12 @@ namespace Orchard.Core.Tests.Common.Services {
[Test] [Test]
public void VeryLongStringTruncatedTo1000Chars() { public void VeryLongStringTruncatedTo1000Chars() {
var contentManager = _container.Resolve<IContentManager>();
var veryVeryLongTitle = "this is a very long title..."; var veryVeryLongTitle = "this is a very long title...";
for (var i = 0; i < 100; i++) for (var i = 0; i < 100; i++)
veryVeryLongTitle += "aaaaaaaaaa"; veryVeryLongTitle += "aaaaaaaaaa";
var thing = contentManager.Create<Thing>(ThingDriver.ContentType.Name, t => { var thing = CreateRoutePart(veryVeryLongTitle);
t.As<RoutePart>().Record = new RoutePartRecord(); _routableService.FillSlug(thing);
t.Title = veryVeryLongTitle;
});
_routableService.FillSlug(thing.As<RoutePart>());
Assert.That(veryVeryLongTitle.Length, Is.AtLeast(1001)); Assert.That(veryVeryLongTitle.Length, Is.AtLeast(1001));
Assert.That(thing.Slug.Length, Is.EqualTo(1000)); Assert.That(thing.Slug.Length, Is.EqualTo(1000));
@@ -99,86 +101,72 @@ namespace Orchard.Core.Tests.Common.Services {
[Test] [Test]
public void NoExistingLikeSlugsGeneratesSameSlug() { public void NoExistingLikeSlugsGeneratesSameSlug() {
string slug = _routableService.GenerateUniqueSlug("woohoo", null); string slug = _routableService.GenerateUniqueSlug(CreateRoutePart("woohoo"), null);
Assert.That(slug, Is.EqualTo("woohoo")); Assert.That(slug, Is.EqualTo("woohoo"));
} }
[Test] [Test]
public void ExistingSingleLikeSlugThatsAConflictGeneratesADash2() { public void ExistingSingleLikeSlugThatsAConflictGeneratesADash2() {
string slug = _routableService.GenerateUniqueSlug("woohoo", new List<string> { "woohoo" }); string slug = _routableService.GenerateUniqueSlug(CreateRoutePart("woohoo"), new List<string> { "woohoo" });
Assert.That(slug, Is.EqualTo("woohoo-2")); Assert.That(slug, Is.EqualTo("woohoo-2"));
} }
[Test] [Test]
public void ExistingSingleLikeSlugThatsNotAConflictGeneratesSameSlug() { public void ExistingSingleLikeSlugThatsNotAConflictGeneratesSameSlug() {
string slug = _routableService.GenerateUniqueSlug("woohoo", new List<string> { "woohoo-2" }); string slug = _routableService.GenerateUniqueSlug(CreateRoutePart("woohoo"), new List<string> { "woohoo-2" });
Assert.That(slug, Is.EqualTo("woohoo")); Assert.That(slug, Is.EqualTo("woohoo"));
} }
[Test] [Test]
public void ExistingLikeSlugsWithAConflictGeneratesADashVNext() { public void ExistingLikeSlugsWithAConflictGeneratesADashVNext() {
string slug = _routableService.GenerateUniqueSlug("woohoo", new List<string> { "woohoo", "woohoo-2" }); string slug = _routableService.GenerateUniqueSlug(CreateRoutePart("woohoo"), new List<string> { "woohoo", "woohoo-2" });
Assert.That(slug, Is.EqualTo("woohoo-3")); Assert.That(slug, Is.EqualTo("woohoo-3"));
} }
[Test] [Test]
public void ExistingSlugsWithVersionGapsAndNoMatchGeneratesSameSlug() { public void ExistingSlugsWithVersionGapsAndNoMatchGeneratesSameSlug() {
string slug = _routableService.GenerateUniqueSlug("woohoo", new List<string> { "woohoo-2", "woohoo-4", "woohoo-5" }); string slug = _routableService.GenerateUniqueSlug(CreateRoutePart("woohoo"), new List<string> { "woohoo-2", "woohoo-4", "woohoo-5" });
Assert.That(slug, Is.EqualTo("woohoo")); Assert.That(slug, Is.EqualTo("woohoo"));
} }
[Test] [Test]
public void ExistingSlugsWithVersionGapsAndAMatchGeneratesADash2() { public void ExistingSlugsWithVersionGapsAndAMatchGeneratesADash2() {
string slug = _routableService.GenerateUniqueSlug("woohoo-2", new List<string> { "woohoo-2", "woohoo-4", "woohoo-5" }); string slug = _routableService.GenerateUniqueSlug(CreateRoutePart("woohoo-2"), new List<string> { "woohoo-2", "woohoo-4", "woohoo-5" });
Assert.That(slug, Is.EqualTo("woohoo-2-2")); Assert.That(slug, Is.EqualTo("woohoo-2-2"));
} }
[Test] [Test]
public void GeneratedSlugIsLowerCased() { public void GeneratedSlugIsLowerCased() {
var contentManager = _container.Resolve<IContentManager>(); var thing = CreateRoutePart("This Is Some Interesting Title");
var thing = contentManager.Create<Thing>(ThingDriver.ContentType.Name, t => { _routableService.FillSlug(thing);
t.As<RoutePart>().Record = new RoutePartRecord();
t.Title = "This Is Some Interesting Title";
});
_routableService.FillSlug(thing.As<RoutePart>());
Assert.That(thing.Slug, Is.EqualTo("this-is-some-interesting-title")); Assert.That(thing.Slug, Is.EqualTo("this-is-some-interesting-title"));
} }
[Test, Ignore("Fix pending")] [Test]
public void GeneratedSlugsShouldBeUniqueAmongContentType() { public void SlugInConflictWithAnExistingItemsPathIsVersioned() {
var contentManager = _container.Resolve<IContentManager>(); var thing1 = CreateRoutePart("bar", "bar", "foo");
var thing2 = CreateRoutePart("fooslashbar", "foo/bar");
var thing1 = contentManager.Create<Thing>(ThingDriver.ContentType.Name, t => { Assert.That(thing2.Slug, Is.EqualTo("foo/bar-2"));
t.As<RoutePart>().Record = new RoutePartRecord();
t.Title = "This Is Some Interesting Title";
});
var thing2 = contentManager.Create<Thing>(ThingDriver.ContentType.Name, t => {
t.As<RoutePart>().Record = new RoutePartRecord();
t.Title = "This Is Some Interesting Title";
});
Assert.AreNotEqual(thing1.Slug, thing2.Slug);
} }
[Test] private RoutePart CreateRoutePart(string title, string slug = "", string containerPath = "") {
public void SlugsCanBeDuplicatedAccrossContentTypes() {
var contentManager = _container.Resolve<IContentManager>(); var contentManager = _container.Resolve<IContentManager>();
return contentManager.Create<Thing>(ThingDriver.ContentType.Name, t => {
var thing = contentManager.Create<Thing>(ThingDriver.ContentType.Name, t => {
t.As<RoutePart>().Record = new RoutePartRecord(); t.As<RoutePart>().Record = new RoutePartRecord();
t.Title = "This Is Some Interesting Title"; if (!string.IsNullOrWhiteSpace(slug))
t.As<RoutePart>().Slug = slug;
t.Title = title;
if (!string.IsNullOrWhiteSpace(containerPath)) {
t.As<ICommonPart>().Container = contentManager.Create<Thing>(ThingDriver.ContentType.Name, tt => {
tt.As<RoutePart>().Path = containerPath;
tt.As<RoutePart>().Slug = containerPath;
}); });
}
var stuff = contentManager.Create<Stuff>(StuffDriver.ContentType.Name, s => { })
s.As<RoutePart>().Record = new RoutePartRecord(); .As<RoutePart>();
s.Title = "This Is Some Interesting Title";
});
Assert.AreEqual(thing.Slug, stuff.Slug);
} }

6
src/Orchard.Web/Core/Routable/Drivers/RoutePartDriver.cs
View File

@@ -59,9 +59,9 @@ namespace Orchard.Core.Routable.Drivers {
model.DisplayLeadingPath = path.Substring(0, path.Length - slug.Length); model.DisplayLeadingPath = path.Substring(0, path.Length - slug.Length);
} }
else { else {
var containerSlug = part.GetContainerSlug(); var containerPath = part.GetContainerPath();
model.DisplayLeadingPath = !string.IsNullOrWhiteSpace(containerSlug) model.DisplayLeadingPath = !string.IsNullOrWhiteSpace(containerPath)
? string.Format("{0}/", containerSlug) ? string.Format("{0}/", containerPath)
: ""; : "";
} }

33
src/Orchard.Web/Core/Routable/Handlers/RoutePartHandler.cs
View File

@@ -1,24 +1,53 @@
using System.Web.Routing; using System;
using System.Web.Routing;
using Orchard.ContentManagement; using Orchard.ContentManagement;
using Orchard.ContentManagement.Handlers; using Orchard.ContentManagement.Handlers;
using Orchard.Core.Routable.Models; using Orchard.Core.Routable.Models;
using Orchard.Core.Routable.Services;
using Orchard.Data; using Orchard.Data;
using Orchard.Localization;
using Orchard.UI.Notify;
namespace Orchard.Core.Routable.Handlers { namespace Orchard.Core.Routable.Handlers {
public class RoutePartHandler : ContentHandler { public class RoutePartHandler : ContentHandler {
private readonly IOrchardServices _services;
private readonly IRoutablePathConstraint _routablePathConstraint; private readonly IRoutablePathConstraint _routablePathConstraint;
private readonly IRoutableService _routableService;
public RoutePartHandler(IRepository<RoutePartRecord> repository, IRoutablePathConstraint routablePathConstraint) { public RoutePartHandler(IOrchardServices services, IRepository<RoutePartRecord> repository, IRoutablePathConstraint routablePathConstraint, IRoutableService routableService) {
_services = services;
_routablePathConstraint = routablePathConstraint; _routablePathConstraint = routablePathConstraint;
_routableService = routableService;
T = NullLocalizer.Instance;
Filters.Add(StorageFilter.For(repository)); Filters.Add(StorageFilter.For(repository));
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);
});
OnPublished<RoutePart>((context, routable) => { OnPublished<RoutePart>((context, routable) => {
if (context.PublishingItemVersionRecord != null)
processSlug(routable);
if (!string.IsNullOrEmpty(routable.Path)) if (!string.IsNullOrEmpty(routable.Path))
_routablePathConstraint.AddPath(routable.Path); _routablePathConstraint.AddPath(routable.Path);
}); });
OnIndexing<RoutePart>((context, part) => context.DocumentIndex.Add("title", part.Record.Title).RemoveTags().Analyze()); OnIndexing<RoutePart>((context, part) => context.DocumentIndex.Add("title", part.Record.Title).RemoveTags().Analyze());
} }
public Localizer T { get; set; }
} }
public class RoutePartHandlerBase : ContentHandlerBase { public class RoutePartHandlerBase : ContentHandlerBase {

10
src/Orchard.Web/Core/Routable/Models/RoutePart.cs
View File

@@ -18,12 +18,12 @@ namespace Orchard.Core.Routable.Models {
set { Record.Path = value; } set { Record.Path = value; }
} }
public string GetContainerSlug() { public string GetContainerPath() {
var commonAspect = this.As<ICommonPart>(); var commonAspect = this.As<ICommonPart>();
if (commonAspect != null && commonAspect.Container != null) { if (commonAspect != null && commonAspect.Container != null) {
var routable = commonAspect.Container.As<IRoutableAspect>(); var routable = commonAspect.Container.As<IRoutableAspect>();
if (routable != null) { if (routable != null) {
return routable.Slug; return routable.Path;
} }
} }
return null; return null;
@@ -31,11 +31,11 @@ namespace Orchard.Core.Routable.Models {
public string GetPathFromSlug(string slug) { public string GetPathFromSlug(string slug) {
// TEMP: path format patterns replaces this logic // TEMP: path format patterns replaces this logic
var containerSlug = GetContainerSlug(); var containerPath = GetContainerPath();
if (string.IsNullOrEmpty(containerSlug)) { if (string.IsNullOrEmpty(containerPath)) {
return slug; return slug;
} }
return containerSlug + "/" + slug; return containerPath + "/" + slug;
} }
} }
} }

6
src/Orchard.Web/Core/Routable/Services/IRoutableService.cs
View File

@@ -6,12 +6,12 @@ namespace Orchard.Core.Routable.Services {
public interface IRoutableService : IDependency { public interface IRoutableService : IDependency {
void FillSlug<TModel>(TModel model) where TModel : RoutePart; void FillSlug<TModel>(TModel model) where TModel : RoutePart;
void FillSlug<TModel>(TModel model, Func<string, string> generateSlug) where TModel : RoutePart; void FillSlug<TModel>(TModel model, Func<string, string> generateSlug) where TModel : RoutePart;
string GenerateUniqueSlug(string slugCandidate, IEnumerable<string> existingSlugs); string GenerateUniqueSlug(RoutePart part, IEnumerable<string> existingPaths);
/// <summary> /// <summary>
/// Returns any content item of the specified content type with similar slugs /// Returns any content item with similar path
/// </summary> /// </summary>
IEnumerable<RoutePart> GetSimilarSlugs(string contentType, string slug); IEnumerable<RoutePart> GetSimilarPaths(string path);
/// <summary> /// <summary>
/// Validates the given slug /// Validates the given slug

26
src/Orchard.Web/Core/Routable/Services/RoutableService.cs
View File

@@ -3,7 +3,6 @@ using System.Collections.Generic;
using System.Linq; using System.Linq;
using System.Text.RegularExpressions; using System.Text.RegularExpressions;
using Orchard.ContentManagement; using Orchard.ContentManagement;
using Orchard.Core.Common.Models;
using Orchard.Core.Routable.Models; using Orchard.Core.Routable.Models;
namespace Orchard.Core.Routable.Services { namespace Orchard.Core.Routable.Services {
@@ -37,11 +36,12 @@ namespace Orchard.Core.Routable.Services {
model.Slug = generateSlug(model.Title).ToLowerInvariant(); model.Slug = generateSlug(model.Title).ToLowerInvariant();
} }
public string GenerateUniqueSlug(string slugCandidate, IEnumerable<string> existingSlugs) { public string GenerateUniqueSlug(RoutePart part, IEnumerable<string> existingPaths) {
if (existingSlugs == null || !existingSlugs.Contains(slugCandidate)) var slugCandidate = part.Slug;
if (existingPaths == null || !existingPaths.Contains(part.Path))
return slugCandidate; 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 return version != null
? string.Format("{0}-{1}", slugCandidate, version) ? string.Format("{0}-{1}", slugCandidate, version)
@@ -60,13 +60,12 @@ namespace Orchard.Core.Routable.Services {
: null; : null;
} }
public IEnumerable<RoutePart> GetSimilarSlugs(string contentType, string slug) public IEnumerable<RoutePart> GetSimilarPaths(string path) {
{
return return
_contentManager.Query().Join<RoutePartRecord>() _contentManager.Query().Join<RoutePartRecord>()
.List() .List()
.Select(i => i.As<RoutePart>()) .Select(i => i.As<RoutePart>())
.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(); .ToArray();
} }
@@ -84,19 +83,18 @@ namespace Orchard.Core.Routable.Services {
return true; 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 // Don't include this part in the list
// of slug to consider for conflict detection // of slugs to consider for conflict detection
if (part.ContentItem.Id != 0) pathsLikeThis = pathsLikeThis.Where(p => p.ContentItem.Id != part.ContentItem.Id);
slugsLikeThis = slugsLikeThis.Where(p => p.ContentItem.Id != part.ContentItem.Id);
//todo: (heskew) need better messages //todo: (heskew) need better messages
if (slugsLikeThis.Count() > 0) if (pathsLikeThis.Count() > 0)
{ {
var originalSlug = part.Slug; var originalSlug = part.Slug;
//todo: (heskew) make auto-uniqueness optional //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) { if (originalSlug != part.Slug) {
return false; return false;