Accounting for themes with base themes with base themes with...

--HG--
branch : dev

src/Orchard.Tests.Modules/Themes/Services/ThemeServiceTests.cs

@@ -123,6 +123,18 @@ namespace Orchard.Tests.Modules.Themes.Services {
             Assert.That(siteTheme.ThemeName, Is.EqualTo("ThemeOne"));
         }
 
+        [Test]
+        public void ThemeWithCircularBaseDepTrowsExceptionOnActivation() {
+            _themeService.SetSiteTheme("ThemeOne");
+            try {
+                _themeService.SetSiteTheme("ThemeFourBasedOnFive");
+            } catch (InvalidOperationException ex) {
+                Assert.That(ex.Message, Is.StringMatching("ThemeFiveBasedOnFour"));
+            }
+            var siteTheme = _themeService.GetSiteTheme();
+            Assert.That(siteTheme.ThemeName, Is.EqualTo("ThemeOne"));
+        }
+
         #region Stubs
 
         public class TestSessionLocator : ISessionLocator {
@@ -155,6 +167,8 @@ namespace Orchard.Tests.Modules.Themes.Services {
                     new ExtensionDescriptor {Name = "ThemeOne", ExtensionType = "Theme"},
                     new ExtensionDescriptor {Name = "ThemeTwo", BaseTheme = "ThemeOne", ExtensionType = "Theme"},
                     new ExtensionDescriptor {Name = "ThemeThree", BaseTheme = "TheThemeThatIsntThere", ExtensionType = "Theme"},
+                    new ExtensionDescriptor {Name = "ThemeFourBasedOnFive", BaseTheme = "ThemeFiveBasedOnFour", ExtensionType = "Theme"},
+                    new ExtensionDescriptor {Name = "ThemeFiveBasedOnFour", BaseTheme = "ThemeFourBasedOnFive", ExtensionType = "Theme"},
                 };
 
                 foreach (var extension in extensions) {

src/Orchard.Web/Modules/Orchard.Themes/Services/ThemeService.cs

@@ -6,6 +6,7 @@ using System.Web.Routing;
 using JetBrains.Annotations;
 using Orchard.Environment.Extensions;
 using Orchard.Environment.Extensions.Models;
+using Orchard.Localization;
 using Orchard.Logging;
 using Orchard.ContentManagement;
 using Orchard.Modules;
@@ -27,8 +28,10 @@ namespace Orchard.Themes.Services {
             _themeSelectors = themeSelectors;
             _moduleService = moduleService;
             Logger = NullLogger.Instance;
+            T = NullLocalizer.Instance;
         }
 
+        public Localizer T { get; set; }
         public ILogger Logger { get; set; }
         protected virtual ISite CurrentSite { get; [UsedImplicitly] private set; }
 
@@ -51,26 +54,71 @@ namespace Orchard.Themes.Services {
             if (themeToSet == null)
                 return;
 
-            //if there's a base theme, it needs to be present
+            // ensure all base themes down the line are present and accounted for
-            ITheme baseTheme = null;
+            //todo: (heskew) dito on the need of a meaningful message
-            if (!string.IsNullOrWhiteSpace(themeToSet.BaseTheme)) {
+            if (!AllBaseThemesAreInstalled(themeToSet.BaseTheme))
-                baseTheme = GetThemeByName(themeToSet.BaseTheme);
-                if (baseTheme == null)
                 return;
-            }
 
-            var currentTheme = CurrentSite.As<ThemeSiteSettingsPart>().CurrentThemeName;
+            // disable all theme features
-            if ( !string.IsNullOrEmpty(currentTheme) )
+            DisableThemeFeatures(CurrentSite.As<ThemeSiteSettingsPart>().CurrentThemeName);
-                _moduleService.DisableFeatures(new[] {currentTheme}, true);
 
-            if (baseTheme != null)
+            // enable all theme features
-                _moduleService.EnableFeatures(new[] {themeToSet.BaseTheme});
+            EnableThemeFeatures(themeToSet.ThemeName);
-
-            _moduleService.EnableFeatures(new[] {themeToSet.ThemeName}, true);
 
             CurrentSite.As<ThemeSiteSettingsPart>().Record.CurrentThemeName = themeToSet.ThemeName;
         }
 
+        private bool AllBaseThemesAreInstalled(string baseThemeName) {
+            var themesSeen = new List<string>();
+            while (!string.IsNullOrWhiteSpace(baseThemeName)) {
+                //todo: (heskew) need a better way to protect from recursive references
+                if (themesSeen.Contains(baseThemeName))
+                    throw new InvalidOperationException(T("The theme \"{0}\" was already seen - looks like we're going aruond in circles.", baseThemeName).Text);
+                themesSeen.Add(baseThemeName);
+
+                var baseTheme = GetThemeByName(baseThemeName);
+                if (baseTheme == null)
+                    return false;
+                baseThemeName = baseTheme.BaseTheme;
+            }
+
+            return true;
+        }
+
+        private void DisableThemeFeatures(string themeName) {
+            var themes = new Queue<string>();
+            while (themeName != null) {
+                if (themes.Contains(themeName))
+                    throw new InvalidOperationException(T("The theme \"{0}\" is already in the stack of themes that need features disabled.", themeName).Text);
+                themes.Enqueue(themeName);
+
+                var theme = GetThemeByName(themeName);
+                themeName = !string.IsNullOrWhiteSpace(theme.BaseTheme)
+                    ? theme.BaseTheme
+                    : null;
+            }
+
+            while (themes.Count > 0)
+                _moduleService.DisableFeatures(new[] { themes.Dequeue() });
+        }
+
+        private void EnableThemeFeatures(string themeName) {
+            var themes = new Stack<string>();
+            while(themeName != null) {
+                if (themes.Contains(themeName))
+                    throw new InvalidOperationException(T("The theme \"{0}\" is already in the stack of themes that need features enabled.", themeName).Text);
+                themes.Push(themeName);
+
+                var theme = GetThemeByName(themeName);
+                themeName = !string.IsNullOrWhiteSpace(theme.BaseTheme)
+                    ? theme.BaseTheme
+                    : null;
+            }
+
+            while (themes.Count > 0)
+                _moduleService.DisableFeatures(new[] {themes.Pop()});
+        }
+
         public ITheme GetRequestTheme(RequestContext requestContext) {
             var requestTheme = _themeSelectors
                 .Select(x => x.GetTheme(requestContext))