From a0e79c903462a75c6a71a3f14061d0b612d3ab3e Mon Sep 17 00:00:00 2001 From: Aleksandr Vishniakov Date: Fri, 28 Feb 2020 11:44:41 +0200 Subject: [PATCH 1/4] VP-1558: Merge settings between base and current themes Fully backward-compatible: see changes in settings Support merge between themes in different stores and themes in the same store --- .../LiquidThemeEngineOptions.cs | 22 ++++- .../ShopifyLiquidThemeEngine.cs | 86 +++++++++++++------ VirtoCommerce.Storefront/appsettings.json | 11 ++- 3 files changed, 86 insertions(+), 33 deletions(-) diff --git a/VirtoCommerce.LiquidThemeEngine/LiquidThemeEngineOptions.cs b/VirtoCommerce.LiquidThemeEngine/LiquidThemeEngineOptions.cs index ec57876d3..4cf2632c8 100644 --- a/VirtoCommerce.LiquidThemeEngine/LiquidThemeEngineOptions.cs +++ b/VirtoCommerce.LiquidThemeEngine/LiquidThemeEngineOptions.cs @@ -10,11 +10,31 @@ public class LiquidThemeEngineOptions public IList TemplatesDiscoveryFolders { get; set; } = new List() { "templates", "snippets", "layout", "assets" }; public string ThemesAssetsRelativeUrl { get; set; } = "~/themes/assets"; public bool RethrowLiquidRenderErrors { get; set; } = false; + + /// + /// The path to the base theme that will be used to discover the theme resources not found by the path of theme for current store. + /// This parameter can be used for theme inheritance logic. + /// Example values: + /// Electronics/default -> wwwroot/cms-content/Themes/Electronics/default + /// default-> wwwroot/cms-content/Themes/default + /// + public string BaseThemePath { get; set; } + /// + /// Original description: /// The name of the base theme that will be used to discover the theme resources not found by the path of theme for current store. /// This parameter can be used for theme inheritance logic. - /// Example values: default_theme -> wwwroot/cms-content/default_theme + /// Example values: default_theme -> wwwroot/cms-content/Themes/default_theme/default + /// + /// How it actually worked: + /// Storefront used this parameter as a store name, i.e. Electronics -> wwwroot/cms-content/Themes/Electronics/default /// + [Obsolete("Obsolete. Use BaseThemePath instead.")] public string BaseThemeName { get; set; } + + /// + /// Set to true if you want to merge current theme settings with base theme settings instead of placement + /// + public bool MergeBaseSettings { get; set; } } } diff --git a/VirtoCommerce.LiquidThemeEngine/ShopifyLiquidThemeEngine.cs b/VirtoCommerce.LiquidThemeEngine/ShopifyLiquidThemeEngine.cs index 3d0468e7a..b1b0b63f0 100644 --- a/VirtoCommerce.LiquidThemeEngine/ShopifyLiquidThemeEngine.cs +++ b/VirtoCommerce.LiquidThemeEngine/ShopifyLiquidThemeEngine.cs @@ -94,7 +94,12 @@ public ShopifyLiquidThemeEngine(IStorefrontMemoryCache memoryCache, IWorkContext private string CurrentThemePath => Path.Combine("Themes", WorkContext.CurrentStore.Id, CurrentThemeName); //Relative path to the discovery of theme resources that weren't found by the current path. - private string BaseThemePath => !string.IsNullOrEmpty(_options.BaseThemeName) ? Path.Combine("Themes", _options.BaseThemeName, "default") : null; + private string BaseThemePath => + !string.IsNullOrEmpty(_options.BaseThemePath) ? Path.Combine("Themes", _options.BaseThemePath) : +#pragma warning disable 618 + // We need to use obsolete value here for backward compatibility. + !string.IsNullOrEmpty(_options.BaseThemeName) ? Path.Combine("Themes", _options.BaseThemeName, "default") : null; +#pragma warning restore 618 private string BaseThemeSettingPath => BaseThemePath != null ? Path.Combine(BaseThemePath, "config", "settings_data.json") : null; public string BaseThemeLocalePath => BaseThemePath != null ? Path.Combine(BaseThemePath, "locales") : null; @@ -347,44 +352,39 @@ public ValueTask RenderTemplateAsync(string templateContent, string temp public IDictionary GetSettings(string defaultValue = null) { var cacheKey = CacheKey.With(GetType(), "GetSettings", CurrentThemeSettingPath, defaultValue); - return _memoryCache.GetOrCreateExclusive(cacheKey, (cacheItem) => + return _memoryCache.GetOrCreateExclusive(cacheKey, cacheItem => { cacheItem.AddExpirationToken(new CompositeChangeToken(new[] { ThemeEngineCacheRegion.CreateChangeToken(), _themeBlobProvider.Watch(CurrentThemeSettingPath) })); - var retVal = new Dictionary().WithDefaultValue(defaultValue); - //Load all data from current theme config - var resultSettings = InnerGetAllSettings(_themeBlobProvider, CurrentThemeSettingPath); - if (resultSettings == null && BaseThemeSettingPath != null) + var result = new JObject(); + if (_options.MergeBaseSettings) { - resultSettings = InnerGetAllSettings(_themeBlobProvider, BaseThemeSettingPath); + //Try to load settings from base theme path and merge them with resources for local theme + if (BaseThemeSettingPath != null) + { + cacheItem.AddExpirationToken(new CompositeChangeToken(new[] { ThemeEngineCacheRegion.CreateChangeToken(), _themeBlobProvider.Watch(BaseThemeSettingPath) })); + result = GetCurrentPreset(InnerGetAllSettings(_themeBlobProvider, BaseThemeSettingPath)); + } + + var currentSettings = GetCurrentPreset(InnerGetAllSettings(_themeBlobProvider, CurrentThemeSettingPath)); + result.Merge(currentSettings ?? new JObject(), new JsonMergeSettings { MergeArrayHandling = MergeArrayHandling.Merge }); } - if (resultSettings != null) + else { - //Get actual preset from merged config - var currentPreset = resultSettings.GetValue("current"); - if (currentPreset is JValue) + //Load all data from current theme config + var currentSettings = InnerGetAllSettings(_themeBlobProvider, CurrentThemeSettingPath); + if (currentSettings == null && BaseThemeSettingPath != null) { - var currentPresetName = ((JValue)currentPreset).Value.ToString(); - if (!(resultSettings.GetValue("presets") is JObject presets) || !presets.Children().Any()) - { - throw new StorefrontException("Setting presets not defined"); - } - - IList allPresets = presets.Children().Cast().ToList(); - resultSettings = allPresets.FirstOrDefault(p => p.Name == currentPresetName).Value as JObject; - if (resultSettings == null) - { - throw new StorefrontException($"Setting preset with name '{currentPresetName}' not found"); - } + //Fallback load data from base theme config + cacheItem.AddExpirationToken(new CompositeChangeToken(new[] { ThemeEngineCacheRegion.CreateChangeToken(), _themeBlobProvider.Watch(BaseThemeSettingPath) })); + currentSettings = InnerGetAllSettings(_themeBlobProvider, BaseThemeSettingPath); } - if (currentPreset is JObject) + if (currentSettings != null) { - resultSettings = (JObject)currentPreset; + result = GetCurrentPreset(currentSettings); } - - retVal = resultSettings.ToObject>().ToDictionary(x => x.Key, x => x.Value).WithDefaultValue(defaultValue); } - return retVal; + return result.ToObject>().ToDictionary(x => x.Key, x => x.Value).WithDefaultValue(defaultValue); }); } @@ -487,6 +487,36 @@ private static JObject InnerGetAllSettings(IContentBlobProvider themeBlobProvide return retVal; } + /// + /// Get actual preset from config + /// + /// + private static JObject GetCurrentPreset(JObject allSettings) + { + var currentPreset = allSettings.GetValue("current"); + if (currentPreset is JValue currentPresetValue) + { + var currentPresetName = currentPresetValue.Value.ToString(); + if (!(allSettings.GetValue("presets") is JObject presets) || !presets.Children().Any()) + { + throw new StorefrontException("Setting presets not defined"); + } + + IList allPresets = presets.Children().Cast().ToList(); + allSettings = allPresets.FirstOrDefault(p => p.Name == currentPresetName)?.Value as JObject; + if (allSettings == null) + { + throw new StorefrontException($"Setting preset with name '{currentPresetName}' not found"); + } + } + if (currentPreset is JObject preset) + { + allSettings = preset; + } + + return allSettings; + } + private string ReadTemplateByPath(string templatePath) { if (string.IsNullOrEmpty(templatePath)) diff --git a/VirtoCommerce.Storefront/appsettings.json b/VirtoCommerce.Storefront/appsettings.json index faba7a60d..346101f5c 100644 --- a/VirtoCommerce.Storefront/appsettings.json +++ b/VirtoCommerce.Storefront/appsettings.json @@ -38,10 +38,13 @@ }, "LiquidThemeEngine": { "RethrowLiquidRenderErrors": false, - //The name of the base theme that will be used to discover the theme resources not found by the path of theme for current store. - //This parameter can be used for theme inheritance logic. - // Example values: 'default_theme' will map to this path 'wwwroot/cms-content/default_theme' - "BaseThemeName": "" + // The path to the base theme that will be used to discover the theme resources not found by the path of theme for current store. + // This parameter can be used for theme inheritance logic. + // Example values: Electronics/default_theme -> wwwroot/cms-content/Themes/Electronics/default_theme + "BaseThemePath": "", + "BaseThemeName": "", + // Set to true if you want to merge current theme settings with base theme settings instead of placement + "MergeBaseSettings": false }, "RequireHttps": { "Enabled": false, From 84bc2fbf1fa205795b0c37a46e87e8a6acca57db Mon Sep 17 00:00:00 2001 From: Aleksandr Vishniakov Date: Fri, 28 Feb 2020 12:20:12 +0200 Subject: [PATCH 2/4] Refactoring --- .../LiquidThemeEngineOptions.cs | 2 +- .../ShopifyLiquidThemeEngine.cs | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/VirtoCommerce.LiquidThemeEngine/LiquidThemeEngineOptions.cs b/VirtoCommerce.LiquidThemeEngine/LiquidThemeEngineOptions.cs index 4cf2632c8..894628357 100644 --- a/VirtoCommerce.LiquidThemeEngine/LiquidThemeEngineOptions.cs +++ b/VirtoCommerce.LiquidThemeEngine/LiquidThemeEngineOptions.cs @@ -24,7 +24,7 @@ public class LiquidThemeEngineOptions /// Original description: /// The name of the base theme that will be used to discover the theme resources not found by the path of theme for current store. /// This parameter can be used for theme inheritance logic. - /// Example values: default_theme -> wwwroot/cms-content/Themes/default_theme/default + /// Example values: default_theme -> wwwroot/cms-content/default_theme /// /// How it actually worked: /// Storefront used this parameter as a store name, i.e. Electronics -> wwwroot/cms-content/Themes/Electronics/default diff --git a/VirtoCommerce.LiquidThemeEngine/ShopifyLiquidThemeEngine.cs b/VirtoCommerce.LiquidThemeEngine/ShopifyLiquidThemeEngine.cs index b1b0b63f0..e079ddd77 100644 --- a/VirtoCommerce.LiquidThemeEngine/ShopifyLiquidThemeEngine.cs +++ b/VirtoCommerce.LiquidThemeEngine/ShopifyLiquidThemeEngine.cs @@ -493,6 +493,7 @@ private static JObject InnerGetAllSettings(IContentBlobProvider themeBlobProvide /// private static JObject GetCurrentPreset(JObject allSettings) { + var result = allSettings; var currentPreset = allSettings.GetValue("current"); if (currentPreset is JValue currentPresetValue) { @@ -503,18 +504,18 @@ private static JObject GetCurrentPreset(JObject allSettings) } IList allPresets = presets.Children().Cast().ToList(); - allSettings = allPresets.FirstOrDefault(p => p.Name == currentPresetName)?.Value as JObject; - if (allSettings == null) + result = allPresets.FirstOrDefault(p => p.Name == currentPresetName)?.Value as JObject; + if (result == null) { throw new StorefrontException($"Setting preset with name '{currentPresetName}' not found"); } } if (currentPreset is JObject preset) { - allSettings = preset; + result = preset; } - return allSettings; + return result; } private string ReadTemplateByPath(string templatePath) From 7183d32fe095a2a10aa4de8cfb85fb263b0408d7 Mon Sep 17 00:00:00 2001 From: Eugeny Tatarincev Date: Tue, 3 Mar 2020 13:33:41 +0200 Subject: [PATCH 3/4] Code polishing --- .../ShopifyLiquidThemeEngine.cs | 43 +++++++------------ VirtoCommerce.Storefront/appsettings.json | 1 - 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/VirtoCommerce.LiquidThemeEngine/ShopifyLiquidThemeEngine.cs b/VirtoCommerce.LiquidThemeEngine/ShopifyLiquidThemeEngine.cs index e079ddd77..a4f3511aa 100644 --- a/VirtoCommerce.LiquidThemeEngine/ShopifyLiquidThemeEngine.cs +++ b/VirtoCommerce.LiquidThemeEngine/ShopifyLiquidThemeEngine.cs @@ -355,33 +355,20 @@ public IDictionary GetSettings(string defaultValue = null) return _memoryCache.GetOrCreateExclusive(cacheKey, cacheItem => { cacheItem.AddExpirationToken(new CompositeChangeToken(new[] { ThemeEngineCacheRegion.CreateChangeToken(), _themeBlobProvider.Watch(CurrentThemeSettingPath) })); - var result = new JObject(); - if (_options.MergeBaseSettings) - { - //Try to load settings from base theme path and merge them with resources for local theme - if (BaseThemeSettingPath != null) - { - cacheItem.AddExpirationToken(new CompositeChangeToken(new[] { ThemeEngineCacheRegion.CreateChangeToken(), _themeBlobProvider.Watch(BaseThemeSettingPath) })); - result = GetCurrentPreset(InnerGetAllSettings(_themeBlobProvider, BaseThemeSettingPath)); - } - var currentSettings = GetCurrentPreset(InnerGetAllSettings(_themeBlobProvider, CurrentThemeSettingPath)); - result.Merge(currentSettings ?? new JObject(), new JsonMergeSettings { MergeArrayHandling = MergeArrayHandling.Merge }); + var baseThemeSettings = new JObject(); + JObject result, currentThemeSettings; + currentThemeSettings = result = GetCurrentSettingsPreset(InnerGetAllSettings(_themeBlobProvider, CurrentThemeSettingPath)); + //Try to load localization resources from base theme path and merge them with resources for local theme + if (!string.IsNullOrEmpty(BaseThemeLocalePath)) + { + cacheItem.AddExpirationToken(new CompositeChangeToken(new[] { ThemeEngineCacheRegion.CreateChangeToken(), _themeBlobProvider.Watch(BaseThemeSettingPath) })); + result = baseThemeSettings = GetCurrentSettingsPreset(InnerGetAllSettings(_themeBlobProvider, BaseThemeSettingPath)); } - else + if (_options.MergeBaseSettings) { - //Load all data from current theme config - var currentSettings = InnerGetAllSettings(_themeBlobProvider, CurrentThemeSettingPath); - if (currentSettings == null && BaseThemeSettingPath != null) - { - //Fallback load data from base theme config - cacheItem.AddExpirationToken(new CompositeChangeToken(new[] { ThemeEngineCacheRegion.CreateChangeToken(), _themeBlobProvider.Watch(BaseThemeSettingPath) })); - currentSettings = InnerGetAllSettings(_themeBlobProvider, BaseThemeSettingPath); - } - if (currentSettings != null) - { - result = GetCurrentPreset(currentSettings); - } + result = currentThemeSettings; + result.Merge(baseThemeSettings ?? new JObject(), new JsonMergeSettings { MergeArrayHandling = MergeArrayHandling.Merge }); } return result.ToObject>().ToDictionary(x => x.Key, x => x.Value).WithDefaultValue(defaultValue); @@ -475,23 +462,23 @@ private static JObject InnerGetAllSettings(IContentBlobProvider themeBlobProvide throw new ArgumentNullException(nameof(settingsPath)); } - JObject retVal = null; + var result = new JObject(); if (themeBlobProvider.PathExists(settingsPath)) { using (var stream = themeBlobProvider.OpenRead(settingsPath)) { - retVal = JsonConvert.DeserializeObject(stream.ReadToString()); + result = JsonConvert.DeserializeObject(stream.ReadToString()); } } - return retVal; + return result; } /// /// Get actual preset from config /// /// - private static JObject GetCurrentPreset(JObject allSettings) + private static JObject GetCurrentSettingsPreset(JObject allSettings) { var result = allSettings; var currentPreset = allSettings.GetValue("current"); diff --git a/VirtoCommerce.Storefront/appsettings.json b/VirtoCommerce.Storefront/appsettings.json index 346101f5c..37d329c9a 100644 --- a/VirtoCommerce.Storefront/appsettings.json +++ b/VirtoCommerce.Storefront/appsettings.json @@ -42,7 +42,6 @@ // This parameter can be used for theme inheritance logic. // Example values: Electronics/default_theme -> wwwroot/cms-content/Themes/Electronics/default_theme "BaseThemePath": "", - "BaseThemeName": "", // Set to true if you want to merge current theme settings with base theme settings instead of placement "MergeBaseSettings": false }, From d373d1800a71dd346a55a9625745d29873e9f11d Mon Sep 17 00:00:00 2001 From: Aleksandr Vishniakov Date: Tue, 3 Mar 2020 14:08:09 +0200 Subject: [PATCH 4/4] Fixed bugs in code polishing --- .../ShopifyLiquidThemeEngine.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/VirtoCommerce.LiquidThemeEngine/ShopifyLiquidThemeEngine.cs b/VirtoCommerce.LiquidThemeEngine/ShopifyLiquidThemeEngine.cs index a4f3511aa..cc2bfb116 100644 --- a/VirtoCommerce.LiquidThemeEngine/ShopifyLiquidThemeEngine.cs +++ b/VirtoCommerce.LiquidThemeEngine/ShopifyLiquidThemeEngine.cs @@ -356,19 +356,21 @@ public IDictionary GetSettings(string defaultValue = null) { cacheItem.AddExpirationToken(new CompositeChangeToken(new[] { ThemeEngineCacheRegion.CreateChangeToken(), _themeBlobProvider.Watch(CurrentThemeSettingPath) })); + JObject result; var baseThemeSettings = new JObject(); - JObject result, currentThemeSettings; - currentThemeSettings = result = GetCurrentSettingsPreset(InnerGetAllSettings(_themeBlobProvider, CurrentThemeSettingPath)); - //Try to load localization resources from base theme path and merge them with resources for local theme - if (!string.IsNullOrEmpty(BaseThemeLocalePath)) + var currentThemeSettings = result = GetCurrentSettingsPreset(InnerGetAllSettings(_themeBlobProvider, CurrentThemeSettingPath)); + + //Try to load settings from base theme path and merge them with resources for local theme + if ((_options.MergeBaseSettings || currentThemeSettings == null) && !string.IsNullOrEmpty(BaseThemeSettingPath)) { cacheItem.AddExpirationToken(new CompositeChangeToken(new[] { ThemeEngineCacheRegion.CreateChangeToken(), _themeBlobProvider.Watch(BaseThemeSettingPath) })); result = baseThemeSettings = GetCurrentSettingsPreset(InnerGetAllSettings(_themeBlobProvider, BaseThemeSettingPath)); } + if (_options.MergeBaseSettings) { - result = currentThemeSettings; - result.Merge(baseThemeSettings ?? new JObject(), new JsonMergeSettings { MergeArrayHandling = MergeArrayHandling.Merge }); + result = baseThemeSettings; + result.Merge(currentThemeSettings ?? new JObject(), new JsonMergeSettings { MergeArrayHandling = MergeArrayHandling.Merge }); } return result.ToObject>().ToDictionary(x => x.Key, x => x.Value).WithDefaultValue(defaultValue);