Revert driver initialization logic and adapt Notifications cache (#16677

)

src/OrchardCore.Modules/OrchardCore.Notifications/Drivers/NotificationNavbarDisplayDriver.cs

@@ -38,20 +38,22 @@ public override async Task<IDisplayResult> DisplayAsync(Navbar model, BuildDispl
             return null;
         }
 
-        var result = Initialize<UserNotificationNavbarViewModel>("UserNotificationNavbar", async model =>
-        {
-            var userId = _httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier);
-            var notifications = (await _session.Query<Notification, NotificationIndex>(x => x.UserId == userId && !x.IsRead, collection: NotificationConstants.NotificationCollection)
-                .OrderByDescending(x => x.CreatedAtUtc)
-                .Take(_notificationOptions.TotalUnreadNotifications + 1)
-                .ListAsync()).ToList();
+        var result = Initialize<UserNotificationNavbarViewModel>("UserNotificationNavbar")
+            .Processing<UserNotificationNavbarViewModel>(async model =>
+            {
+                var userId = _httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier);
+                var notifications = (await _session.Query<Notification, NotificationIndex>(x => x.UserId == userId && !x.IsRead, collection: NotificationConstants.NotificationCollection)
+                    .OrderByDescending(x => x.CreatedAtUtc)
+                    .Take(_notificationOptions.TotalUnreadNotifications + 1)
+                    .ListAsync()).ToList();
 
-            model.Notifications = notifications;
-            model.MaxVisibleNotifications = _notificationOptions.TotalUnreadNotifications;
-            model.TotalUnread = notifications.Count;
+                model.Notifications = notifications;
+                model.MaxVisibleNotifications = _notificationOptions.TotalUnreadNotifications;
+                model.TotalUnread = notifications.Count;
 
-        }).Location("Detail", "Content:9")
-        .Location("DetailAdmin", "Content:9");
+            })
+            .Location("Detail", "Content:9")
+            .Location("DetailAdmin", "Content:9");
 
         if (_notificationOptions.AbsoluteCacheExpirationSeconds > 0 || _notificationOptions.SlidingCacheExpirationSeconds > 0)
         {

src/OrchardCore/OrchardCore.DisplayManagement/Handlers/DisplayDriverBase.cs

@@ -7,15 +7,31 @@ public class DisplayDriverBase
     protected string Prefix { get; set; } = string.Empty;
 
     /// <summary>
-    /// Creates a new strongly typed shape and initializes it if it needs to be rendered.
+    /// Creates a new strongly typed shape.
+    /// </summary>
+    public ShapeResult Initialize<TModel>() where TModel : class
+    {
+        return Initialize<TModel>(shape => { });
+    }
+
+    /// <summary>
+    /// Creates a new strongly typed shape.
+    /// </summary>
+    public ShapeResult Initialize<TModel>(string shapeType) where TModel : class
+    {
+        return Initialize<TModel>(shapeType, shape => { });
+    }
+
+    /// <summary>
+    /// Creates a new strongly typed shape and initializes it before it is displayed.
     /// </summary>
     public ShapeResult Initialize<TModel>(Action<TModel> initialize) where TModel : class
     {
         return Initialize<TModel>(typeof(TModel).Name, initialize);
     }
 
     /// <summary>
-    /// Creates a new strongly typed shape and initializes it if it needs to be rendered.
+    /// Creates a new strongly typed shape and initializes it before it is displayed.
     /// </summary>
     public ShapeResult Initialize<TModel>(string shapeType, Action<TModel> initialize) where TModel : class
     {
@@ -28,7 +44,7 @@ public ShapeResult Initialize<TModel>(string shapeType, Action<TModel> initializ
     }
 
     /// <summary>
-    /// Creates a new strongly typed shape and initializes it if it needs to be rendered.
+    /// Creates a new strongly typed shape and initializes it before it is displayed.
     /// </summary>
     public ShapeResult Initialize<TModel>(Func<TModel, ValueTask> initializeAsync) where TModel : class
     {
@@ -39,7 +55,7 @@ public ShapeResult Initialize<TModel>(Func<TModel, ValueTask> initializeAsync) w
     }
 
     /// <summary>
-    /// Creates a new strongly typed shape and initializes it if it needs to be rendered.
+    /// Creates a new strongly typed shape and initializes it before it is displayed.
     /// </summary>
     public ShapeResult Initialize<TModel>(string shapeType, Func<TModel, ValueTask> initializeAsync) where TModel : class
     {
@@ -59,7 +75,7 @@ public ShapeResult Copy<TModel>(string shapeType, TModel model) where TModel : c
     }
 
     /// <summary>
-    /// Creates a new loosely typed shape and initializes it if it needs to be rendered.
+    /// Creates a new loosely typed shape and initializes it before it is displayed.
     /// </summary>
     public ShapeResult Dynamic(string shapeType, Func<dynamic, Task> initializeAsync)
     {
@@ -71,7 +87,7 @@ public ShapeResult Dynamic(string shapeType, Func<dynamic, Task> initializeAsync
     }
 
     /// <summary>
-    /// Creates a new loosely typed shape and initializes it if it needs to be rendered.
+    /// Creates a new loosely typed shape and initializes it before it is displayed.
     /// </summary>
     public ShapeResult Dynamic(string shapeType, Action<dynamic> initialize)
     {
@@ -86,7 +102,7 @@ public ShapeResult Dynamic(string shapeType, Action<dynamic> initialize)
     }
 
     /// <summary>
-    /// If the shape needs to be rendered, it is created automatically from its type name.
+    /// When the shape is displayed, it is created automatically from its type name.
     /// </summary>
     public ShapeResult Dynamic(string shapeType)
     {
@@ -126,7 +142,7 @@ public ShapeResult Factory(string shapeType, Func<IBuildShapeContext, IShape> sh
     }
 
     /// <summary>
-    /// If the shape needs to be rendered, it is created by the delegate.
+    /// If the shape needs to be displayed, it is created by the delegate.
     /// </summary>
     /// <remarks>
     /// This method is ultimately called by all drivers to create a shape. It's made virtual

src/OrchardCore/OrchardCore.DisplayManagement/Implementation/DefaultHtmlDisplay.cs

@@ -123,8 +123,12 @@ public async Task<IHtmlContent> ExecuteAsync(DisplayContext context)
                 }
 
                 // Now find the actual binding to render, taking alternates into account.
-                var actualBinding = await GetShapeBindingAsync(shapeMetadata.Type, shapeMetadata.Alternates, shapeTable)
-                    ?? throw new Exception($"The shape type '{shapeMetadata.Type}' is not found");
+                var actualBinding = await GetShapeBindingAsync(shapeMetadata.Type, shapeMetadata.Alternates, shapeTable);
+
+                if (actualBinding == null)
+                {
+                    throw new InvalidOperationException($"The shape type '{shapeMetadata.Type}' is not found for the theme '{theme?.Id}'");
+                }
 
                 await shapeMetadata.ProcessingAsync.InvokeAsync((action, displayContext) => action(displayContext.Shape), displayContext, _logger);
 

src/OrchardCore/OrchardCore.DisplayManagement/Shapes/ShapeMetadata.cs

@@ -13,6 +13,10 @@ public ShapeMetadata()
     {
     }
 
+    private List<Action<ShapeDisplayContext>> _displaying;
+    private List<Func<IShape, Task>> _processing;
+    private List<Action<ShapeDisplayContext>> _displayed;
+
     public string Type { get; set; }
     public string DisplayType { get; set; }
     public string Position { get; set; }
@@ -28,40 +32,46 @@ public ShapeMetadata()
     public bool IsCached => _cacheContext != null;
     public IHtmlContent ChildContent { get; set; }
 
+    // The casts in (IReadOnlyList<T>)_displaying ?? [] are important as they convert [] to Array.Empty.
+    // It would use List<T> otherwise which is not what we want here, we don't want to allocate.
+
     /// <summary>
     /// Event use for a specific shape instance.
     /// </summary>
     [JsonIgnore]
-    public IReadOnlyList<Action<ShapeDisplayContext>> Displaying { get; private set; } = [];
+    public IReadOnlyList<Action<ShapeDisplayContext>> Displaying => (IReadOnlyList<Action<ShapeDisplayContext>>)_displaying ?? [];
 
     /// <summary>
     /// Event use for a specific shape instance.
     /// </summary>
     [JsonIgnore]
-    public IReadOnlyList<Func<IShape, Task>> ProcessingAsync { get; private set; } = [];
+    public IReadOnlyList<Func<IShape, Task>> ProcessingAsync => (IReadOnlyList<Func<IShape, Task>>)_processing ?? [];
 
     /// <summary>
     /// Event use for a specific shape instance.
     /// </summary>
     [JsonIgnore]
-    public IReadOnlyList<Action<ShapeDisplayContext>> Displayed { get; private set; } = [];
+    public IReadOnlyList<Action<ShapeDisplayContext>> Displayed => (IReadOnlyList<Action<ShapeDisplayContext>>)_displayed ?? [];
 
     [JsonIgnore]
     public IReadOnlyList<string> BindingSources { get; set; } = [];
 
     public void OnDisplaying(Action<ShapeDisplayContext> context)
     {
-        Displaying = [.. Displaying, context];
+        _displaying ??= new List<Action<ShapeDisplayContext>>();
+        _displaying.Add(context);
     }
 
     public void OnProcessing(Func<IShape, Task> context)
     {
-        ProcessingAsync = [.. ProcessingAsync, context];
+        _processing ??= new List<Func<IShape, Task>>();
+        _processing.Add(context);
     }
 
     public void OnDisplayed(Action<ShapeDisplayContext> context)
     {
-        Displayed = [.. Displayed, context];
+        _displayed ??= new List<Action<ShapeDisplayContext>>();
+        _displayed.Add(context);
     }
 
     /// <summary>

src/OrchardCore/OrchardCore.DisplayManagement/Views/ShapeResult.cs

@@ -17,25 +17,37 @@ public class ShapeResult : IDisplayResult
     private string _cacheId;
     private readonly string _shapeType;
     private readonly Func<IBuildShapeContext, ValueTask<IShape>> _shapeBuilder;
-    private readonly Func<IShape, Task> _processing;
+    private readonly Func<IShape, Task> _initializing;
     private Action<CacheContext> _cache;
     private string _groupId;
     private Action<ShapeDisplayContext> _displaying;
+    private Func<IShape, Task> _processing;
     private Func<Task<bool>> _renderPredicateAsync;
 
+    /// <summary>
+    /// Creates a new instance of <see cref="ShapeResult"/>.
+    /// </summary>
+    /// <param name="shapeType">The Shape type used for the created Shape.</param>
+    /// <param name="shapeBuilder">A delegate that creates the shape instance.</param>
     public ShapeResult(string shapeType, Func<IBuildShapeContext, ValueTask<IShape>> shapeBuilder)
         : this(shapeType, shapeBuilder, null)
     {
     }
 
-    public ShapeResult(string shapeType, Func<IBuildShapeContext, ValueTask<IShape>> shapeBuilder, Func<IShape, Task> processing)
+    /// <summary>
+    /// Creates a new instance of <see cref="ShapeResult"/>.
+    /// </summary>
+    /// <param name="shapeType">The Shape type used for the created Shape.</param>
+    /// <param name="shapeBuilder">A delegate that creates the shape instance.</param>
+    /// <param name="initializing">A delegate that is executed after the shape is created.</param>
+    public ShapeResult(string shapeType, Func<IBuildShapeContext, ValueTask<IShape>> shapeBuilder, Func<IShape, Task> initializing)
     {
         // The shape type is necessary before the shape is created as it will drive the placement
         // resolution which itself can prevent the shape from being created.
 
         _shapeType = shapeType;
         _shapeBuilder = shapeBuilder;
-        _processing = processing;
+        _initializing = initializing;
     }
 
     public Task ApplyAsync(BuildDisplayContext context)
@@ -118,13 +130,17 @@ private async Task ApplyImplementationAsync(BuildShapeContext context, string di
         newShapeMetadata.Column = placement.GetColumn();
         newShapeMetadata.Type = _shapeType;
 
+        // Invoke the initialization code first when all Displaying events are invoked.
+        // These Displaying methods are used to create alternates for instance, so the
+        // Shape needs to have required properties available first.
+
+        _initializing?.Invoke(Shape);
+
         if (_displaying != null)
         {
             newShapeMetadata.OnDisplaying(_displaying);
         }
 
-        // The _processing callback is used to delay execution of costly initialization
-        // that can be prevented by caching.
         if (_processing != null)
         {
             newShapeMetadata.OnProcessing(_processing);
@@ -226,7 +242,7 @@ public ShapeResult Location(string displayType, string location)
     }
 
     /// <summary>
-    /// Sets the location to use for a matching display type.
+    /// Sets the delegate to be executed when the shape is being displayed.
     /// </summary>
     public ShapeResult Displaying(Action<ShapeDisplayContext> displaying)
     {
@@ -235,6 +251,26 @@ public ShapeResult Displaying(Action<ShapeDisplayContext> displaying)
         return this;
     }
 
+    /// <summary>
+    /// Sets the delegate to be executed when the shape is rendered (not cached).
+    /// </summary>
+    public ShapeResult Processing(Func<IShape, Task> processing)
+    {
+        _processing = processing;
+
+        return this;
+    }
+
+    /// <summary>
+    /// Sets the delegate to be executed when the shape is rendered (not cached).
+    /// </summary>
+    public ShapeResult Processing<T>(Func<T, Task> processing)
+    {
+        _processing = shape => processing?.Invoke((T)shape);
+
+        return this;
+    }
+
     /// <summary>
     /// Sets the shape name regardless its 'Differentiator'.
     /// </summary>

test/OrchardCore.Tests/DisplayManagement/DynamicCacheTests.cs

@@ -141,6 +141,7 @@ public async Task ShapeResultsAreRenderedOnceWhenCached()
         var cacheTag = "mytag";
 
         var initializedCalled = 0;
+        var processedCalled = 0;
         var bindCalled = 0;
 
         var displayManager = _serviceProvider.GetService<IHtmlDisplay>();
@@ -165,11 +166,16 @@ public async Task ShapeResultsAreRenderedOnceWhenCached()
         ShapeResult CreateShapeResult() => new ShapeResult(
             shapeType,
             shapeBuilder: ctx => factory.CreateAsync<MyModel>(shapeType, model => model.MyProperty = 7),
-            processing: shape =>
+            initializing: shape =>
             {
                 initializedCalled++;
                 return Task.CompletedTask;
-            }).Location("Content").Cache("mycontent", ctx => ctx.WithExpiryAfter(TimeSpan.FromSeconds(1)).AddTag(cacheTag));
+            }).Location("Content").Cache("mycontent", ctx => ctx.WithExpiryAfter(TimeSpan.FromSeconds(1)).AddTag(cacheTag))
+            .Processing(shape =>
+            {
+                processedCalled++;
+                return Task.CompletedTask;
+            });
 
         var shapeResult = CreateShapeResult();
         var contentShape = await factory.CreateAsync("Content");
@@ -186,7 +192,7 @@ public async Task ShapeResultsAreRenderedOnceWhenCached()
         Assert.Equal(1, bindCalled);
         Assert.Equal(1, initializedCalled);
 
-        for (var i = 0; i < 10; i++)
+        for (var i = 1; i <= 10; i++)
         {
             // Create new ShapeResult.
             shapeResult = CreateShapeResult();
@@ -197,7 +203,8 @@ public async Task ShapeResultsAreRenderedOnceWhenCached()
 
             // Shape is not rendered twice.
             Assert.Equal(1, bindCalled);
-            Assert.Equal(1, initializedCalled);
+            Assert.Equal(1, processedCalled);
+            Assert.Equal(i + 1, initializedCalled);
             Assert.Equal("Hi there!", result.ToString());
         }
 
@@ -214,7 +221,8 @@ public async Task ShapeResultsAreRenderedOnceWhenCached()
 
         // Shape is processed and rendered again.
         Assert.Equal(2, bindCalled);
-        Assert.Equal(2, initializedCalled);
+        Assert.Equal(2, processedCalled);
+        Assert.Equal(12, initializedCalled);
         Assert.Equal("Hi there!", result.ToString());
     }