From 9852c56cb261a63d0387b13785927c03388de263 Mon Sep 17 00:00:00 2001 From: loudej Date: Sun, 29 Nov 2009 09:08:25 +0000 Subject: [PATCH] Slugs in pages module no longer case sensitive. Introduces concept of transient and singleton dependencies. Singleton dependencies really mean single instance per shell. --HG-- extra : convert_revision : svn%3A5ff7c347-ad56-4c35-b696-ccb81de16e03/trunk%4042638 --- .../Orchard.Tests.Packages.csproj | 1 + .../Controllers/TemplatesControllerTests.cs | 11 ++ .../Pages/Services/SlugConstraintTests.cs | 49 ++++++++ .../Environment/DefaultOrchardHostTests.cs | 109 +++++++++++++++++- .../TestDependencies/TestDependency.cs | 29 +++++ src/Orchard.Tests/Orchard.Tests.csproj | 1 + .../Controllers/TemplatesController.cs | 8 +- .../Orchard.CmsPages/Orchard.CmsPages.csproj | 1 + .../Packages/Orchard.CmsPages/Routes.cs | 27 ++--- .../Services/SlugConstraint.cs | 53 +++++++++ src/Orchard/Environment/DefaultOrchardHost.cs | 22 +++- .../Environment/DefaultOrchardShell.cs | 2 - src/Orchard/IDependency.cs | 7 ++ 13 files changed, 290 insertions(+), 30 deletions(-) create mode 100644 src/Orchard.Tests.Packages/Pages/Services/SlugConstraintTests.cs create mode 100644 src/Orchard.Tests/Environment/TestDependencies/TestDependency.cs create mode 100644 src/Orchard.Web/Packages/Orchard.CmsPages/Services/SlugConstraint.cs diff --git a/src/Orchard.Tests.Packages/Orchard.Tests.Packages.csproj b/src/Orchard.Tests.Packages/Orchard.Tests.Packages.csproj index 8b1333f40..aa7b6a996 100644 --- a/src/Orchard.Tests.Packages/Orchard.Tests.Packages.csproj +++ b/src/Orchard.Tests.Packages/Orchard.Tests.Packages.csproj @@ -90,6 +90,7 @@ + diff --git a/src/Orchard.Tests.Packages/Pages/Controllers/TemplatesControllerTests.cs b/src/Orchard.Tests.Packages/Pages/Controllers/TemplatesControllerTests.cs index f8cee5990..0468db964 100644 --- a/src/Orchard.Tests.Packages/Pages/Controllers/TemplatesControllerTests.cs +++ b/src/Orchard.Tests.Packages/Pages/Controllers/TemplatesControllerTests.cs @@ -27,6 +27,8 @@ namespace Orchard.Tests.Packages.Pages.Controllers { var revision = _pageManager.CreatePage(new CreatePageParams(null, "slug", null)); _pageManager.Publish(revision, new PublishOptions()); + + _container.Resolve().SetCurrentlyPublishedSlugs(_pageManager.GetCurrentlyPublishedSlugs()); } public override void Register(ContainerBuilder builder) { @@ -35,6 +37,7 @@ namespace Orchard.Tests.Packages.Pages.Controllers { builder.Register().As(); builder.Register().As(); builder.Register(new StubTemplateEntryProvider()).As(); + builder.Register().As(); } protected override IEnumerable DatabaseTypes { @@ -69,5 +72,13 @@ namespace Orchard.Tests.Packages.Pages.Controllers { var page = (PageRevision) (((ViewResult) result).ViewData.Model); Assert.That(page.Slug, Is.EqualTo("slug")); } + + + [Test] + public void TheWrongCaseShouldStillWork() { + var result = _controller.Show("sLUg"); + var page = (PageRevision)(((ViewResult)result).ViewData.Model); + Assert.That(page.Slug, Is.EqualTo("slug")); + } } } \ No newline at end of file diff --git a/src/Orchard.Tests.Packages/Pages/Services/SlugConstraintTests.cs b/src/Orchard.Tests.Packages/Pages/Services/SlugConstraintTests.cs new file mode 100644 index 000000000..f787ce014 --- /dev/null +++ b/src/Orchard.Tests.Packages/Pages/Services/SlugConstraintTests.cs @@ -0,0 +1,49 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Web.Routing; +using NUnit.Framework; +using Orchard.CmsPages.Services; + +namespace Orchard.Tests.Packages.Pages.Services { + [TestFixture] + public class SlugConstraintTests { + [Test] + public void MatchShouldBeTrueWhenSlugIsInSet() { + var slugConstraint = new SlugConstraint(); + + var before = slugConstraint.Match(null, null, "slug", new RouteValueDictionary {{"slug", "foo"}}, + RouteDirection.IncomingRequest); + Assert.That(before, Is.False); + + slugConstraint.SetCurrentlyPublishedSlugs(new[]{"foo"}); + var after = slugConstraint.Match(null, null, "slug", new RouteValueDictionary { { "slug", "foo" } }, + RouteDirection.IncomingRequest); + Assert.That(after, Is.True); + } + + [Test] + public void MatchShouldIgnoreCase() { + var slugConstraint = new SlugConstraint(); + slugConstraint.SetCurrentlyPublishedSlugs(new[]{"foo", "bAr"}); + + var foo = slugConstraint.Match(null, null, "slug", new RouteValueDictionary { { "slug", "FOO" } }, + RouteDirection.IncomingRequest); + Assert.That(foo, Is.True); + + var bar = slugConstraint.Match(null, null, "slug", new RouteValueDictionary { { "slug", "bar" } }, + RouteDirection.IncomingRequest); + Assert.That(bar, Is.True); + + } + + [Test] + public void CollisionsShouldNotThrowExceptions() { + var slugConstraint = new SlugConstraint(); + slugConstraint.SetCurrentlyPublishedSlugs(new[] { "foo", "FOO" }); + + + } + } +} diff --git a/src/Orchard.Tests/Environment/DefaultOrchardHostTests.cs b/src/Orchard.Tests/Environment/DefaultOrchardHostTests.cs index 9c245d47e..2feb8ba14 100644 --- a/src/Orchard.Tests/Environment/DefaultOrchardHostTests.cs +++ b/src/Orchard.Tests/Environment/DefaultOrchardHostTests.cs @@ -16,6 +16,7 @@ using Orchard.Mvc; using Orchard.Mvc.ModelBinders; using Orchard.Mvc.Routes; using Orchard.Packages; +using Orchard.Tests.Environment.TestDependencies; using Orchard.Tests.Mvc.Routes; using Orchard.Tests.Stubs; @@ -44,7 +45,7 @@ namespace Orchard.Tests.Environment { builder.Register(_controllerBuilder); builder.Register(_routeCollection); builder.Register(_modelBinderDictionary); - builder.Register(new ViewEngineCollection{new WebFormViewEngine()}); + builder.Register(new ViewEngineCollection { new WebFormViewEngine() }); builder.Register(new StuPackageManager()).As(); }); } @@ -78,7 +79,11 @@ namespace Orchard.Tests.Environment { } public IEnumerable GetDependencyTypes() { - return Enumerable.Empty(); + return new[] { + typeof (TestDependency), + typeof (TestSingletonDependency), + typeof(TestTransientDependency) + }; } public IEnumerable GetRecordTypes() { @@ -93,5 +98,105 @@ namespace Orchard.Tests.Environment { var runtime2 = host.CreateShell(); Assert.That(runtime1, Is.Not.SameAs(runtime2)); } + + + [Test] + public void NormalDependenciesShouldBeUniquePerRequestContainer() { + var host = (DefaultOrchardHost)_container.Resolve(); + var container1 = host.CreateShellContainer(); + var container2 = host.CreateShellContainer(); + var requestContainer1a = container1.CreateInnerContainer(); + var requestContainer1b = container1.CreateInnerContainer(); + var requestContainer2a = container2.CreateInnerContainer(); + var requestContainer2b = container2.CreateInnerContainer(); + + var dep1 = container1.Resolve(); + var dep1a = requestContainer1a.Resolve(); + var dep1b = requestContainer1b.Resolve(); + var dep2 = container2.Resolve(); + var dep2a = requestContainer2a.Resolve(); + var dep2b = requestContainer2b.Resolve(); + + Assert.That(dep1, Is.Not.SameAs(dep2)); + Assert.That(dep1, Is.Not.SameAs(dep1a)); + Assert.That(dep1, Is.Not.SameAs(dep1b)); + Assert.That(dep2, Is.Not.SameAs(dep2a)); + Assert.That(dep2, Is.Not.SameAs(dep2b)); + + var again1 = container1.Resolve(); + var again1a = requestContainer1a.Resolve(); + var again1b = requestContainer1b.Resolve(); + var again2 = container2.Resolve(); + var again2a = requestContainer2a.Resolve(); + var again2b = requestContainer2b.Resolve(); + + Assert.That(again1, Is.SameAs(dep1)); + Assert.That(again1a, Is.SameAs(dep1a)); + Assert.That(again1b, Is.SameAs(dep1b)); + Assert.That(again2, Is.SameAs(dep2)); + Assert.That(again2a, Is.SameAs(dep2a)); + Assert.That(again2b, Is.SameAs(dep2b)); + } + [Test] + public void SingletonDependenciesShouldBeUniquePerShell() { + var host = (DefaultOrchardHost)_container.Resolve(); + var container1 = host.CreateShellContainer(); + var container2 = host.CreateShellContainer(); + var requestContainer1a = container1.CreateInnerContainer(); + var requestContainer1b = container1.CreateInnerContainer(); + var requestContainer2a = container2.CreateInnerContainer(); + var requestContainer2b = container2.CreateInnerContainer(); + + var dep1 = container1.Resolve(); + var dep1a = requestContainer1a.Resolve(); + var dep1b = requestContainer1b.Resolve(); + var dep2 = container2.Resolve(); + var dep2a = requestContainer2a.Resolve(); + var dep2b = requestContainer2b.Resolve(); + + Assert.That(dep1, Is.Not.SameAs(dep2)); + Assert.That(dep1, Is.SameAs(dep1a)); + Assert.That(dep1, Is.SameAs(dep1b)); + Assert.That(dep2, Is.SameAs(dep2a)); + Assert.That(dep2, Is.SameAs(dep2b)); + } + [Test] + public void TransientDependenciesShouldBeUniquePerResolve() { + var host = (DefaultOrchardHost)_container.Resolve(); + var container1 = host.CreateShellContainer(); + var container2 = host.CreateShellContainer(); + var requestContainer1a = container1.CreateInnerContainer(); + var requestContainer1b = container1.CreateInnerContainer(); + var requestContainer2a = container2.CreateInnerContainer(); + var requestContainer2b = container2.CreateInnerContainer(); + + var dep1 = container1.Resolve(); + var dep1a = requestContainer1a.Resolve(); + var dep1b = requestContainer1b.Resolve(); + var dep2 = container2.Resolve(); + var dep2a = requestContainer2a.Resolve(); + var dep2b = requestContainer2b.Resolve(); + + Assert.That(dep1, Is.Not.SameAs(dep2)); + Assert.That(dep1, Is.Not.SameAs(dep1a)); + Assert.That(dep1, Is.Not.SameAs(dep1b)); + Assert.That(dep2, Is.Not.SameAs(dep2a)); + Assert.That(dep2, Is.Not.SameAs(dep2b)); + + var again1 = container1.Resolve(); + var again1a = requestContainer1a.Resolve(); + var again1b = requestContainer1b.Resolve(); + var again2 = container2.Resolve(); + var again2a = requestContainer2a.Resolve(); + var again2b = requestContainer2b.Resolve(); + + Assert.That(again1, Is.Not.SameAs(dep1)); + Assert.That(again1a, Is.Not.SameAs(dep1a)); + Assert.That(again1b, Is.Not.SameAs(dep1b)); + Assert.That(again2, Is.Not.SameAs(dep2)); + Assert.That(again2a, Is.Not.SameAs(dep2a)); + Assert.That(again2b, Is.Not.SameAs(dep2b)); + + } } } \ No newline at end of file diff --git a/src/Orchard.Tests/Environment/TestDependencies/TestDependency.cs b/src/Orchard.Tests/Environment/TestDependencies/TestDependency.cs new file mode 100644 index 000000000..59322ddcc --- /dev/null +++ b/src/Orchard.Tests/Environment/TestDependencies/TestDependency.cs @@ -0,0 +1,29 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; + +namespace Orchard.Tests.Environment.TestDependencies { + + public interface ITestDependency : IDependency { + + } + + public class TestDependency : ITestDependency{ + } + + public interface ITestSingletonDependency : ISingletonDependency { + + } + + public class TestSingletonDependency : ITestSingletonDependency { + } + + + public interface ITestTransientDependency : ITransientDependency { + + } + + public class TestTransientDependency : ITestTransientDependency { + } +} diff --git a/src/Orchard.Tests/Orchard.Tests.csproj b/src/Orchard.Tests/Orchard.Tests.csproj index f169410ea..5b9b4c0ea 100644 --- a/src/Orchard.Tests/Orchard.Tests.csproj +++ b/src/Orchard.Tests/Orchard.Tests.csproj @@ -104,6 +104,7 @@ + diff --git a/src/Orchard.Web/Packages/Orchard.CmsPages/Controllers/TemplatesController.cs b/src/Orchard.Web/Packages/Orchard.CmsPages/Controllers/TemplatesController.cs index cc49b4ebc..62868f72d 100644 --- a/src/Orchard.Web/Packages/Orchard.CmsPages/Controllers/TemplatesController.cs +++ b/src/Orchard.Web/Packages/Orchard.CmsPages/Controllers/TemplatesController.cs @@ -6,9 +6,11 @@ using Orchard.CmsPages.Services; namespace Orchard.CmsPages.Controllers { public class TemplatesController : Controller { private readonly IPageManager _pageManager; + private readonly ISlugConstraint _slugConstraint; - public TemplatesController(IPageManager pageManager) { + public TemplatesController(IPageManager pageManager, ISlugConstraint slugConstraint) { _pageManager = pageManager; + _slugConstraint = slugConstraint; } public ActionResult Show(string slug) { @@ -17,7 +19,9 @@ namespace Orchard.CmsPages.Controllers { throw new ArgumentNullException("slug"); } - var revision = _pageManager.GetPublishedBySlug(slug); + var correctedSlug = _slugConstraint.LookupPublishedSlug(slug); + + var revision = _pageManager.GetPublishedBySlug(correctedSlug); if (revision == null) { //TODO: Error message throw new HttpException(404, "slug " + slug + " was not found"); diff --git a/src/Orchard.Web/Packages/Orchard.CmsPages/Orchard.CmsPages.csproj b/src/Orchard.Web/Packages/Orchard.CmsPages/Orchard.CmsPages.csproj index a891228f4..2ba6d2c58 100644 --- a/src/Orchard.Web/Packages/Orchard.CmsPages/Orchard.CmsPages.csproj +++ b/src/Orchard.Web/Packages/Orchard.CmsPages/Orchard.CmsPages.csproj @@ -96,6 +96,7 @@ + diff --git a/src/Orchard.Web/Packages/Orchard.CmsPages/Routes.cs b/src/Orchard.Web/Packages/Orchard.CmsPages/Routes.cs index 96e552c3c..7c6fcdf8e 100644 --- a/src/Orchard.Web/Packages/Orchard.CmsPages/Routes.cs +++ b/src/Orchard.Web/Packages/Orchard.CmsPages/Routes.cs @@ -1,18 +1,16 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Web; +using System.Collections.Generic; using System.Web.Mvc; using System.Web.Routing; using Orchard.CmsPages.Services; using Orchard.Mvc.Routes; namespace Orchard.CmsPages { - public class Routes : IRouteProvider, IRouteConstraint { - private readonly IPageManager _pageManager; - public Routes(IPageManager pageManager) { - _pageManager = pageManager; + public class Routes : IRouteProvider { + private readonly ISlugConstraint _slugConstraint; + + public Routes(ISlugConstraint slugConstraint) { + _slugConstraint = slugConstraint; } public void GetRoutes(ICollection routes) { @@ -21,7 +19,6 @@ namespace Orchard.CmsPages { } public IEnumerable GetRoutes() { - IRouteConstraint slugConstraint = this; return new[] { new RouteDescriptor { Priority = 10, @@ -33,7 +30,7 @@ namespace Orchard.CmsPages { {"action", "show"} }, new RouteValueDictionary { - {"slug", slugConstraint} + {"slug", _slugConstraint} }, new RouteValueDictionary { {"area", "Orchard.CmsPages"} @@ -43,15 +40,5 @@ namespace Orchard.CmsPages { }; } - public bool Match(HttpContextBase httpContext, Route route, string parameterName, RouteValueDictionary values, RouteDirection routeDirection) { - //TEMP: direct db call... - object value; - if (values.TryGetValue(parameterName, out value)) { - var parameterValue = Convert.ToString(value); - bool result = _pageManager.GetCurrentlyPublishedSlugs().Count(slug => slug == parameterValue) != 0; - return result; - } - return false; - } } } diff --git a/src/Orchard.Web/Packages/Orchard.CmsPages/Services/SlugConstraint.cs b/src/Orchard.Web/Packages/Orchard.CmsPages/Services/SlugConstraint.cs new file mode 100644 index 000000000..b941e580f --- /dev/null +++ b/src/Orchard.Web/Packages/Orchard.CmsPages/Services/SlugConstraint.cs @@ -0,0 +1,53 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Web; +using System.Web.Routing; +using Orchard.Tasks; + +namespace Orchard.CmsPages.Services { + public interface ISlugConstraint : IRouteConstraint, ISingletonDependency { + void SetCurrentlyPublishedSlugs(IEnumerable slugs); + string LookupPublishedSlug(string slug); + } + + public class SlugConstraint : ISlugConstraint { + private IDictionary _currentlyPublishedSlugs = new Dictionary(); + + public void SetCurrentlyPublishedSlugs(IEnumerable values) { + _currentlyPublishedSlugs = values + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToDictionary(value => value, StringComparer.OrdinalIgnoreCase); + } + + public string LookupPublishedSlug(string slug) { + string actual; + if (_currentlyPublishedSlugs.TryGetValue(slug, out actual)) + return actual; + return slug; + } + + public bool Match(HttpContextBase httpContext, Route route, string parameterName, RouteValueDictionary values, RouteDirection routeDirection) { + object value; + if (values.TryGetValue(parameterName, out value)) { + var parameterValue = Convert.ToString(value); + return _currentlyPublishedSlugs.ContainsKey(parameterValue); + } + return false; + } + } + + public class SlugConstraintUpdater : IBackgroundTask{ + private readonly IPageManager _pageManager; + private readonly ISlugConstraint _slugConstraint; + + public SlugConstraintUpdater(IPageManager pageManager, ISlugConstraint slugConstraint) { + _pageManager = pageManager; + _slugConstraint = slugConstraint; + } + + public void Sweep() { + _slugConstraint.SetCurrentlyPublishedSlugs(_pageManager.GetCurrentlyPublishedSlugs()); + } + } +} \ No newline at end of file diff --git a/src/Orchard/Environment/DefaultOrchardHost.cs b/src/Orchard/Environment/DefaultOrchardHost.cs index 79630d3e3..9ac01dc83 100644 --- a/src/Orchard/Environment/DefaultOrchardHost.cs +++ b/src/Orchard/Environment/DefaultOrchardHost.cs @@ -45,7 +45,12 @@ namespace Orchard.Environment { protected virtual IOrchardShell CreateShell() { + var shellContainer = CreateShellContainer(); + return shellContainer.Resolve(); + } + + public virtual IContainer CreateShellContainer() { // add module types to container being built var addingModulesAndServices = new ContainerBuilder(); foreach (var moduleType in _compositionStrategy.GetModuleTypes()) { @@ -55,8 +60,18 @@ namespace Orchard.Environment { // add components by the IDependency interfaces they expose foreach (var serviceType in _compositionStrategy.GetDependencyTypes()) { foreach (var interfaceType in serviceType.GetInterfaces()) - if (typeof(IDependency).IsAssignableFrom(interfaceType)) - addingModulesAndServices.Register(serviceType).As(interfaceType).ContainerScoped(); + if (typeof(IDependency).IsAssignableFrom(interfaceType)) { + var registrar = addingModulesAndServices.Register(serviceType).As(interfaceType); + if (typeof(ISingletonDependency).IsAssignableFrom(interfaceType)){ + registrar.SingletonScoped(); + } + else if (typeof(ITransientDependency).IsAssignableFrom(interfaceType)){ + registrar.FactoryScoped(); + } + else { + registrar.ContainerScoped(); + } + } } var shellContainer = _container.CreateInnerContainer(); @@ -69,8 +84,7 @@ namespace Orchard.Environment { addingModules.RegisterModule(module); } addingModules.Build(shellContainer); - - return shellContainer.Resolve(); + return shellContainer; } #region IOrchardHost Members diff --git a/src/Orchard/Environment/DefaultOrchardShell.cs b/src/Orchard/Environment/DefaultOrchardShell.cs index a76988627..27d6a3401 100644 --- a/src/Orchard/Environment/DefaultOrchardShell.cs +++ b/src/Orchard/Environment/DefaultOrchardShell.cs @@ -1,4 +1,3 @@ -using System; using System.Collections.Generic; using System.IO; using System.Linq; @@ -7,7 +6,6 @@ using Orchard.Logging; using Orchard.Mvc.ModelBinders; using Orchard.Mvc.Routes; using Orchard.Packages; -using Orchard.Tasks; namespace Orchard.Environment { public class DefaultOrchardShell : IOrchardShell { diff --git a/src/Orchard/IDependency.cs b/src/Orchard/IDependency.cs index e3e83ad12..eb81e0a5c 100644 --- a/src/Orchard/IDependency.cs +++ b/src/Orchard/IDependency.cs @@ -1,4 +1,11 @@ namespace Orchard { public interface IDependency { } + + public interface ISingletonDependency : IDependency { + + } + public interface ITransientDependency : IDependency { + + } }