Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use FrozenDictionaries in ShapeTables #15651

Merged
merged 8 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
MikeAlhayek marked this conversation as resolved.
Show resolved Hide resolved
IServiceProvider serviceProvider,
ILogger<DefaultShapeTableManager> logger)
{
Expand All @@ -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)
MikeAlhayek marked this conversation as resolved.
Show resolved Hide resolved
{
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
Expand Down Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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];
});
}

Expand All @@ -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];
});
}

Expand All @@ -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];
});
}

Expand All @@ -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];
});
}

Expand All @@ -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];
});
}

Expand All @@ -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];
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Collections.Concurrent;
using System.Collections.Generic;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.ApplicationParts;
Expand Down Expand Up @@ -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>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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)
{
Expand Down Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -15,12 +14,6 @@ public class ShapeMetadata

public ShapeMetadata()
{
Wrappers = [];
Alternates = [];
BindingSources = [];
Displaying = [];
Displayed = [];
ProcessingAsync = [];
}

public string Type { get; set; }
Expand All @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
Loading
Loading