Use FrozenDictionaries in ShapeTables (#15651)

OrchardCMS · Apr 4, 2024 · a248c92 · a248c92
1 parent 74d923c
commit a248c92
Show file tree

Hide file tree

Showing 13 changed files with 89 additions and 96 deletions.
diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Descriptors/DefaultShapeTableManager.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Descriptors/DefaultShapeTableManager.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Concurrent;
+using System.Collections.Frozen;
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
@@ -28,13 +29,13 @@ public class DefaultShapeTableManager : IShapeTableManager
         private static readonly object _syncLock = new();
 
         // Singleton cache to hold a tenant's theme ShapeTable 
-        private readonly Dictionary<string, ShapeTable> _shapeTableCache;
+        private readonly IDictionary<string, ShapeTable> _shapeTableCache;
 
         private readonly IServiceProvider _serviceProvider;
         private readonly ILogger _logger;
 
         public DefaultShapeTableManager(
-            [FromKeyedServices(nameof(DefaultShapeTableManager))] Dictionary<string, ShapeTable> shapeTableCache,
+            [FromKeyedServices(nameof(DefaultShapeTableManager))] IDictionary<string, ShapeTable> shapeTableCache,
             IServiceProvider serviceProvider,
             ILogger<DefaultShapeTableManager> logger)
         {
@@ -45,20 +46,28 @@ public DefaultShapeTableManager(
 
         public Task<ShapeTable> GetShapeTableAsync(string themeId)
         {
-            // This method is intentionally kept non-async since most calls
+            // This method is intentionally not awaited since most calls
             // are from cache.
 
             if (_shapeTableCache.TryGetValue(themeId ?? DefaultThemeIdKey, out var shapeTable))
             {
                 return Task.FromResult(shapeTable);
             }
 
-            return BuildShapeTableAsync(themeId);
+            lock (_shapeTableCache)
+            {
+                if (_shapeTableCache.TryGetValue(themeId ?? DefaultThemeIdKey, out shapeTable))
+                {
+                    return Task.FromResult(shapeTable);
+                }
+
+                return BuildShapeTableAsync(themeId);
+            }
         }
 
         private async Task<ShapeTable> BuildShapeTableAsync(string themeId)
         {
-            _logger.LogInformation("Start building shape table");
+            _logger.LogInformation("Start building shape table for {Theme}", themeId);
 
             // These services are resolved lazily since they are only required when initializing the shape tables
             // during the first request. And binding strategies would be expensive to build since this service is called many times
@@ -125,11 +134,11 @@ private async Task<ShapeTable> BuildShapeTableAsync(string themeId)
 
             var shapeTable = new ShapeTable
             (
-                descriptors: descriptors.ToDictionary(sd => sd.ShapeType, x => (ShapeDescriptor)x, StringComparer.OrdinalIgnoreCase),
-                bindings: descriptors.SelectMany(sd => sd.Bindings).ToDictionary(kv => kv.Key, kv => kv.Value, StringComparer.OrdinalIgnoreCase)
+                descriptors: descriptors.ToFrozenDictionary(sd => sd.ShapeType, x => (ShapeDescriptor)x, StringComparer.OrdinalIgnoreCase),
+                bindings: descriptors.SelectMany(sd => sd.Bindings).ToFrozenDictionary(kv => kv.Key, kv => kv.Value, StringComparer.OrdinalIgnoreCase)
             );
 
-            _logger.LogInformation("Done building shape table");
+            _logger.LogInformation("Done building shape table for {Theme}", themeId);
 
             _shapeTableCache[themeId ?? DefaultThemeIdKey] = shapeTable;
 

diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Descriptors/FeatureShapeDescriptor.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Descriptors/FeatureShapeDescriptor.cs
@@ -76,15 +76,15 @@ public ShapeDescriptorIndex(
 
         public override IDictionary<string, ShapeBinding> Bindings => _bindings;
 
-        public override IEnumerable<Func<ShapeCreatingContext, Task>> CreatingAsync => _creatingAsync;
+        public override IReadOnlyList<Func<ShapeCreatingContext, Task>> CreatingAsync => _creatingAsync;
 
-        public override IEnumerable<Func<ShapeCreatedContext, Task>> CreatedAsync => _createdAsync;
+        public override IReadOnlyList<Func<ShapeCreatedContext, Task>> CreatedAsync => _createdAsync;
 
-        public override IEnumerable<Func<ShapeDisplayContext, Task>> DisplayingAsync => _displayingAsync;
+        public override IReadOnlyList<Func<ShapeDisplayContext, Task>> DisplayingAsync => _displayingAsync;
 
-        public override IEnumerable<Func<ShapeDisplayContext, Task>> ProcessingAsync => _processingAsync;
+        public override IReadOnlyList<Func<ShapeDisplayContext, Task>> ProcessingAsync => _processingAsync;
 
-        public override IEnumerable<Func<ShapeDisplayContext, Task>> DisplayedAsync => _displayedAsync;
+        public override IReadOnlyList<Func<ShapeDisplayContext, Task>> DisplayedAsync => _displayedAsync;
 
         public override Func<ShapePlacementContext, PlacementInfo> Placement => CalculatePlacement;
 
@@ -104,27 +104,15 @@ private PlacementInfo CalculatePlacement(ShapePlacementContext ctx)
             return info ?? DefaultPlacementAction(ctx);
         }
 
-        public override IList<string> Wrappers => _wrappers;
+        public override IReadOnlyList<string> Wrappers => _wrappers;
 
-        public override IList<string> BindingSources => _bindingSources;
+        public override IReadOnlyList<string> BindingSources => _bindingSources;
     }
 
     public class ShapeDescriptor
     {
         public ShapeDescriptor()
         {
-            if (this is not ShapeDescriptorIndex)
-            {
-                CreatingAsync = [];
-                CreatedAsync = [];
-                DisplayingAsync = [];
-                ProcessingAsync = [];
-                DisplayedAsync = [];
-                Wrappers = [];
-                BindingSources = [];
-                Bindings = new Dictionary<string, ShapeBinding>(StringComparer.OrdinalIgnoreCase);
-            }
-
             Placement = DefaultPlacementAction;
         }
 
@@ -154,19 +142,19 @@ protected PlacementInfo DefaultPlacementAction(ShapePlacementContext context)
         public virtual Func<DisplayContext, Task<IHtmlContent>> Binding =>
             Bindings[ShapeType].BindingAsync;
 
-        public virtual IDictionary<string, ShapeBinding> Bindings { get; set; }
+        public virtual IDictionary<string, ShapeBinding> Bindings { get; } = new Dictionary<string, ShapeBinding>(StringComparer.OrdinalIgnoreCase);
 
-        public virtual IEnumerable<Func<ShapeCreatingContext, Task>> CreatingAsync { get; set; }
-        public virtual IEnumerable<Func<ShapeCreatedContext, Task>> CreatedAsync { get; set; }
-        public virtual IEnumerable<Func<ShapeDisplayContext, Task>> DisplayingAsync { get; set; }
-        public virtual IEnumerable<Func<ShapeDisplayContext, Task>> ProcessingAsync { get; set; }
-        public virtual IEnumerable<Func<ShapeDisplayContext, Task>> DisplayedAsync { get; set; }
+        public virtual IReadOnlyList<Func<ShapeCreatingContext, Task>> CreatingAsync { get; set; } = [];
+        public virtual IReadOnlyList<Func<ShapeCreatedContext, Task>> CreatedAsync { get; set; } = [];
+        public virtual IReadOnlyList<Func<ShapeDisplayContext, Task>> DisplayingAsync { get; set; } = [];
+        public virtual IReadOnlyList<Func<ShapeDisplayContext, Task>> ProcessingAsync { get; set; } = [];
+        public virtual IReadOnlyList<Func<ShapeDisplayContext, Task>> DisplayedAsync { get; set; } = [];
 
         public virtual Func<ShapePlacementContext, PlacementInfo> Placement { get; set; }
         public string DefaultPlacement { get; set; }
 
-        public virtual IList<string> Wrappers { get; set; }
-        public virtual IList<string> BindingSources { get; set; }
+        public virtual IReadOnlyList<string> Wrappers { get; set; } = [];
+        public virtual IReadOnlyList<string> BindingSources { get; set; } = [];
     }
 
     public class ShapeBinding

diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Descriptors/ShapeAlterationBuilder.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Descriptors/ShapeAlterationBuilder.cs
@@ -1,6 +1,5 @@
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Html;
 using OrchardCore.DisplayManagement.Implementation;
@@ -57,7 +56,7 @@ public ShapeAlterationBuilder BoundAs(string bindingSource, Func<DisplayContext,
 
                 // ShapeDescriptor.Bindings is a case insensitive dictionary.
                 descriptor.Bindings[_bindingName] = binding;
-                descriptor.BindingSources.Add(bindingSource);
+                descriptor.BindingSources = [..descriptor.BindingSources, bindingSource];
             });
         }
 
@@ -80,8 +79,7 @@ public ShapeAlterationBuilder OnCreating(Func<ShapeCreatingContext, Task> action
         {
             return Configure(descriptor =>
             {
-                var existing = descriptor.CreatingAsync ?? [];
-                descriptor.CreatingAsync = existing.Concat(new[] { actionAsync });
+                descriptor.CreatingAsync = [.. descriptor.CreatingAsync ?? [], actionAsync];
             });
         }
 
@@ -104,8 +102,7 @@ public ShapeAlterationBuilder OnCreated(Func<ShapeCreatedContext, Task> actionAs
         {
             return Configure(descriptor =>
             {
-                var existing = descriptor.CreatedAsync ?? [];
-                descriptor.CreatedAsync = existing.Concat(new[] { actionAsync });
+                descriptor.CreatedAsync = [.. descriptor.CreatedAsync ?? [], actionAsync];
             });
         }
 
@@ -128,8 +125,7 @@ public ShapeAlterationBuilder OnDisplaying(Func<ShapeDisplayContext, Task> actio
         {
             return Configure(descriptor =>
             {
-                var existing = descriptor.DisplayingAsync ?? [];
-                descriptor.DisplayingAsync = existing.Concat(new[] { actionAsync });
+                descriptor.DisplayingAsync = [.. descriptor.DisplayingAsync ?? [], actionAsync];
             });
         }
 
@@ -152,8 +148,7 @@ public ShapeAlterationBuilder OnProcessing(Func<ShapeDisplayContext, Task> actio
         {
             return Configure(descriptor =>
             {
-                var existing = descriptor.ProcessingAsync ?? [];
-                descriptor.ProcessingAsync = existing.Concat(new[] { actionAsync });
+                descriptor.ProcessingAsync = [.. descriptor.ProcessingAsync ?? [], actionAsync];
             });
         }
 
@@ -176,8 +171,7 @@ public ShapeAlterationBuilder OnDisplayed(Func<ShapeDisplayContext, Task> action
         {
             return Configure(descriptor =>
             {
-                var existing = descriptor.DisplayedAsync ?? [];
-                descriptor.DisplayedAsync = existing.Concat(new[] { actionAsync });
+                descriptor.DisplayedAsync = [.. descriptor.DisplayedAsync ?? [], actionAsync];
             });
         }
 

diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Descriptors/ShapeTable.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Descriptors/ShapeTable.cs
@@ -4,13 +4,13 @@ namespace OrchardCore.DisplayManagement.Descriptors
 {
     public class ShapeTable
     {
-        public ShapeTable(Dictionary<string, ShapeDescriptor> descriptors, Dictionary<string, ShapeBinding> bindings)
+        public ShapeTable(IDictionary<string, ShapeDescriptor> descriptors, IDictionary<string, ShapeBinding> bindings)
         {
             Descriptors = descriptors;
             Bindings = bindings;
         }
 
-        public Dictionary<string, ShapeDescriptor> Descriptors { get; }
-        public Dictionary<string, ShapeBinding> Bindings { get; }
+        public IDictionary<string, ShapeDescriptor> Descriptors { get; }
+        public IDictionary<string, ShapeBinding> Bindings { get; }
     }
 }
diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Implementation/DefaultHtmlDisplay.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Implementation/DefaultHtmlDisplay.cs
@@ -97,15 +97,16 @@ public async Task<IHtmlContent> ExecuteAsync(DisplayContext context)
                     await shapeDescriptor.DisplayingAsync.InvokeAsync((action, displayContext) => action(displayContext), displayContext, _logger);
 
                     // Copy all binding sources (all templates for this shape) in order to use them as Localization scopes.
-                    shapeMetadata.BindingSources = shapeDescriptor.BindingSources.Where(x => x != null).ToList();
-                    if (!shapeMetadata.BindingSources.Any())
+                    shapeMetadata.BindingSources = shapeDescriptor.BindingSources;
+
+                    if (shapeMetadata.BindingSources.Count == 0)
                     {
-                        shapeMetadata.BindingSources.Add(shapeDescriptor.BindingSource);
+                        shapeMetadata.BindingSources = [shapeDescriptor.BindingSource];
                     }
                 }
 
                 // Invoking ShapeMetadata displaying events.
-                shapeMetadata.Displaying.Invoke(action => action(displayContext), _logger);
+                shapeMetadata.Displaying.Invoke((action, displayContext) => action(displayContext), displayContext, _logger);
 
                 // Use pre-fetched content if available (e.g. coming from specific cache implementation).
                 if (displayContext.ChildContent != null)

diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/OrchardCoreBuilderExtensions.cs b/src/OrchardCore/OrchardCore.DisplayManagement/OrchardCoreBuilderExtensions.cs
@@ -1,3 +1,4 @@
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Mvc.ApplicationParts;
@@ -58,7 +59,7 @@ public static OrchardCoreBuilder AddTheming(this OrchardCoreBuilder builder)
                     services.AddScoped<IViewLocationExpanderProvider, ThemeViewLocationExpanderProvider>();
 
                     services.AddScoped<IShapeTemplateHarvester, BasicShapeTemplateHarvester>();
-                    services.AddKeyedSingleton<Dictionary<string, ShapeTable>>(nameof(DefaultShapeTableManager));
+                    services.AddKeyedSingleton<IDictionary<string, ShapeTable>>(nameof(DefaultShapeTableManager), new ConcurrentDictionary<string, ShapeTable>());
                     services.AddScoped<IShapeTableManager, DefaultShapeTableManager>();
 
                     services.AddScoped<IShapeTableProvider, ShapeAttributeBindingStrategy>();

diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Shapes/AlternatesCollection.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Shapes/AlternatesCollection.cs
@@ -11,7 +11,7 @@ namespace OrchardCore.DisplayManagement.Shapes
     /// </summary>
     public class AlternatesCollection : IEnumerable<string>
     {
-        public static readonly AlternatesCollection Empty = [];
+        public static AlternatesCollection Empty = [];
 
         private KeyedAlternateCollection _collection;
 
@@ -25,9 +25,9 @@ public AlternatesCollection(params string[] alternates)
             }
         }
 
-        public string this[int index] => _collection[index];
+        public string this[int index] => _collection?[index] ?? "";
 
-        public string Last => _collection.LastOrDefault() ?? "";
+        public string Last => _collection?.LastOrDefault() ?? "";
 
         public void Add(string alternate)
         {
@@ -99,6 +99,11 @@ public void AddRange(IEnumerable<string> alternates)
 
         private void EnsureCollection()
         {
+            if (this == Empty)
+            {
+                throw new NotSupportedException("AlternateCollection can't be changed.");
+            }
+
             _collection ??= new KeyedAlternateCollection();
         }
 

diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Shapes/ShapeMetadata.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Shapes/ShapeMetadata.cs
@@ -1,6 +1,5 @@
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using System.Text.Json.Serialization;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Html;
@@ -15,12 +14,6 @@ public class ShapeMetadata
 
         public ShapeMetadata()
         {
-            Wrappers = [];
-            Alternates = [];
-            BindingSources = [];
-            Displaying = [];
-            Displayed = [];
-            ProcessingAsync = [];
         }
 
         public string Type { get; set; }
@@ -33,45 +26,45 @@ public ShapeMetadata()
         public string Prefix { get; set; }
         public string Name { get; set; }
         public string Differentiator { get; set; }
-        public AlternatesCollection Wrappers { get; set; }
-        public AlternatesCollection Alternates { get; set; }
+        public AlternatesCollection Wrappers { get; set; } = [];
+        public AlternatesCollection Alternates { get; set; } = [];
         public bool IsCached => _cacheContext != null;
         public IHtmlContent ChildContent { get; set; }
 
         /// <summary>
         /// Event use for a specific shape instance.
         /// </summary>
         [JsonIgnore]
-        public IEnumerable<Action<ShapeDisplayContext>> Displaying { get; private set; }
+        public IReadOnlyList<Action<ShapeDisplayContext>> Displaying { get; private set; } = [];
 
         /// <summary>
         /// Event use for a specific shape instance.
         /// </summary>
         [JsonIgnore]
-        public IEnumerable<Func<IShape, Task>> ProcessingAsync { get; private set; }
+        public IReadOnlyList<Func<IShape, Task>> ProcessingAsync { get; private set; } = [];
 
         /// <summary>
         /// Event use for a specific shape instance.
         /// </summary>
         [JsonIgnore]
-        public IEnumerable<Action<ShapeDisplayContext>> Displayed { get; private set; }
+        public IReadOnlyList<Action<ShapeDisplayContext>> Displayed { get; private set; } = [];
 
         [JsonIgnore]
-        public IList<string> BindingSources { get; set; }
+        public IReadOnlyList<string> BindingSources { get; set; } = [];
 
         public void OnDisplaying(Action<ShapeDisplayContext> context)
         {
-            Displaying = Displaying.Concat(new[] { context });
+            Displaying = [..Displaying, context];
         }
 
         public void OnProcessing(Func<IShape, Task> context)
         {
-            ProcessingAsync = ProcessingAsync.Concat(new[] { context });
+            ProcessingAsync = [.. ProcessingAsync, context];
         }
 
         public void OnDisplayed(Action<ShapeDisplayContext> context)
         {
-            Displayed = Displayed.Concat(new[] { context });
+            Displayed = [.. Displayed, context];
         }
 
         /// <summary>

diff --git a/src/OrchardCore/OrchardCore.DisplayManagement/Theming/ThemeManager.cs b/src/OrchardCore/OrchardCore.DisplayManagement/Theming/ThemeManager.cs
@@ -36,13 +36,13 @@ public async Task<IExtensionInfo> GetThemeAsync()
                     }
                 }
 
-                themeResults.Sort((x, y) => y.Priority.CompareTo(x.Priority));
-
                 if (themeResults.Count == 0)
                 {
                     return null;
                 }
 
+                themeResults.Sort((x, y) => y.Priority.CompareTo(x.Priority));
+
                 // Try to load the theme to ensure it's present
                 foreach (var theme in themeResults)
                 {