From 82d286e8eaacb88ed1f4c22f25acfd4b116f4cc6 Mon Sep 17 00:00:00 2001 From: Dave Reed Date: Mon, 14 Mar 2011 12:54:26 -0700 Subject: [PATCH] NavigationManager hardened against broken navigation providers. --HG-- branch : dev --- .../UI/Navigation/NavigationManagerTests.cs | 16 +++++++++ .../Services/AdminMenuNavigationProvider.cs | 22 +++++-------- .../UI/Navigation/NavigationManager.cs | 33 ++++++++++++++++--- 3 files changed, 53 insertions(+), 18 deletions(-) diff --git a/src/Orchard.Tests/UI/Navigation/NavigationManagerTests.cs b/src/Orchard.Tests/UI/Navigation/NavigationManagerTests.cs index 7476bdd73..1aa26edec 100644 --- a/src/Orchard.Tests/UI/Navigation/NavigationManagerTests.cs +++ b/src/Orchard.Tests/UI/Navigation/NavigationManagerTests.cs @@ -45,6 +45,14 @@ namespace Orchard.Tests.UI.Navigation { Assert.That(menuItems.Last().Items.Single().Text.TextHint, Is.EqualTo("Frap")); } + [Test] + public void NavigationManagerShouldCatchProviderErrors() { + var manager = new NavigationManager(new[] { new BrokenProvider() }, new StubAuth(), new UrlHelper(new RequestContext(new StubHttpContext("~/"), new RouteData())), new StubOrchardServices()); + + var menuItems = manager.BuildMenu("admin"); + Assert.That(menuItems.Count(), Is.EqualTo(0)); + } + [Test] public void NavigationManagerShouldMergeAndOrderNavigation() { var manager = new NavigationManager(new INavigationProvider[] { new StubProvider(), new Stub2Provider() }, new StubAuth(), new UrlHelper(new RequestContext(new StubHttpContext("~/"), new RouteData())), new StubOrchardServices()); @@ -83,6 +91,14 @@ namespace Orchard.Tests.UI.Navigation { } } + public class BrokenProvider : INavigationProvider { + public string MenuName { get { return "admin"; } } + + public void GetNavigation(NavigationBuilder builder) { + throw new NullReferenceException(); + } + } + public class Stub2Provider : INavigationProvider { public string MenuName { get { return "admin"; } } diff --git a/src/Orchard.Web/Core/Navigation/Services/AdminMenuNavigationProvider.cs b/src/Orchard.Web/Core/Navigation/Services/AdminMenuNavigationProvider.cs index 1c78367fc..a9f865a21 100644 --- a/src/Orchard.Web/Core/Navigation/Services/AdminMenuNavigationProvider.cs +++ b/src/Orchard.Web/Core/Navigation/Services/AdminMenuNavigationProvider.cs @@ -20,20 +20,16 @@ namespace Orchard.Core.Navigation.Services { public string MenuName { get { return "admin"; } } public void GetNavigation(NavigationBuilder builder) { - var partDefinition = _contentDefinitionManager.GetPartDefinition("AdminMenuPart"); - // if the part doesn't even exist, it hasn't been migrated yet... trying to query for AdminMenuPart items would cause an error. - if (partDefinition != null) { - var menuParts = _contentManager.Query().Where(x => x.OnAdminMenu).List(); - foreach (var menuPart in menuParts) { - if (menuPart != null) { - var part = menuPart; + var menuParts = _contentManager.Query().Where(x => x.OnAdminMenu).List(); + foreach (var menuPart in menuParts) { + if (menuPart != null) { + var part = menuPart; - builder.Add(new LocalizedString(HttpUtility.HtmlEncode(part.AdminMenuText)), - part.AdminMenuPosition, - item => item.Action(_contentManager.GetItemMetadata(part.ContentItem).AdminRouteValues)); - // todo: somehow determine if they will ultimately have rights to the destination and hide if not. possibly would need to add a Permission to metadata. - // todo: give an iconset somehow (e.g. based on convention, module/content/.adminmenu.png). - } + builder.Add(new LocalizedString(HttpUtility.HtmlEncode(part.AdminMenuText)), + part.AdminMenuPosition, + item => item.Action(_contentManager.GetItemMetadata(part.ContentItem).AdminRouteValues)); + // todo: somehow determine if they will ultimately have rights to the destination and hide if not. possibly would need to add a Permission to metadata. + // todo: give an iconset somehow (e.g. based on convention, module/content/.adminmenu.png). } } } diff --git a/src/Orchard/UI/Navigation/NavigationManager.cs b/src/Orchard/UI/Navigation/NavigationManager.cs index 11da24d2e..ba396539f 100644 --- a/src/Orchard/UI/Navigation/NavigationManager.cs +++ b/src/Orchard/UI/Navigation/NavigationManager.cs @@ -1,7 +1,9 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Linq; using System.Web.Mvc; using System.Web.Routing; +using Orchard.Logging; using Orchard.Security; using Orchard.Security.Permissions; @@ -17,8 +19,11 @@ namespace Orchard.UI.Navigation { _authorizationService = authorizationService; _urlHelper = urlHelper; _orchardServices = orchardServices; + Logger = NullLogger.Instance; } + public ILogger Logger { get; set; } + public IEnumerable BuildMenu(string menuName) { var sources = GetSources(menuName); return FinishMenu(Reduce(Merge(sources)).ToArray()); @@ -84,8 +89,17 @@ namespace Orchard.UI.Navigation { foreach (var provider in _providers) { if (provider.MenuName == menuName) { var builder = new NavigationBuilder(); - provider.GetNavigation(builder); - yield return builder.Build(); + IEnumerable items = null; + try { + provider.GetNavigation(builder); + items = builder.Build(); + } + catch (Exception ex) { + Logger.Error(ex, "Unexpected error while querying a navigation provider. It was ignored. The menu provided by the provider may not be complete."); + } + if (items != null) { + yield return items; + } } } } @@ -94,8 +108,17 @@ namespace Orchard.UI.Navigation { foreach (var provider in _providers) { if (provider.MenuName == menuName) { var builder = new NavigationBuilder(); - provider.GetNavigation(builder); - yield return builder.BuildImageSets(); + IEnumerable imageSets = null; + try { + provider.GetNavigation(builder); + imageSets = builder.BuildImageSets(); + } + catch (Exception ex) { + Logger.Error(ex, "Unexpected error while querying a navigation provider. It was ignored. The menu provided by the provider may not be complete."); + } + if (imageSets != null) { + yield return imageSets; + } } } }