Skip to content

[Blazor] Dispatch inbound activity handlers in Blazor Web apps #53185

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jan 11, 2024
Merged

[Blazor] Dispatch inbound activity handlers in Blazor Web apps #53185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7f2273c
Dispatch inbound activity handlers
MackinnonBuck Jan 6, 2024
6afda21
Don't await quiescence in activity handler
MackinnonBuck Jan 8, 2024
cc58f43
Remove using
MackinnonBuck Jan 8, 2024
91222af
Minor restructuring
MackinnonBuck Jan 10, 2024
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
96 changes: 57 additions & 39 deletions src/Components/Server/src/Circuits/CircuitHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ internal partial class CircuitHost : IAsyncDisposable
private readonly CircuitOptions _options;
private readonly RemoteNavigationManager _navigationManager;
private readonly ILogger _logger;
private readonly Func<Func<Task>, Task> _dispatchInboundActivity;
private Func<Func<Task>, Task> _dispatchInboundActivity;
private CircuitHandler[] _circuitHandlers;
private bool _initialized;
private bool _isFirstUpdate = true;
Expand Down Expand Up @@ -734,11 +734,10 @@ internal Task UpdateRootComponents(

return Renderer.Dispatcher.InvokeAsync(async () =>
{
var webRootComponentManager = Renderer.GetOrCreateWebRootComponentManager();
var shouldClearStore = false;
var shouldWaitForQuiescence = false;
var operations = operationBatch.Operations;
var batchId = operationBatch.BatchId;
Task[]? pendingTasks = null;
try
{
if (Descriptors.Count > 0)
Expand All @@ -751,6 +750,7 @@ internal Task UpdateRootComponents(
if (_isFirstUpdate)
{
_isFirstUpdate = false;
shouldWaitForQuiescence = true;
if (store != null)
{
shouldClearStore = true;
Expand All @@ -762,6 +762,7 @@ internal Task UpdateRootComponents(

// Retrieve the circuit handlers at this point.
_circuitHandlers = [.. _scope.ServiceProvider.GetServices<CircuitHandler>().OrderBy(h => h.Order)];
_dispatchInboundActivity = BuildInboundActivityDispatcher(_circuitHandlers, Circuit);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty odd - it's hard to see why we have to do this here in particular. Is it just a matter of lazy initialization? If so would it be clearer still if the field was a Lazy<T> that was always initialized during the constructor, and then we wouldn't need to initialize it explicitly in other places.

Copy link
Member Author

@MackinnonBuck MackinnonBuck Jan 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it's a bit confusing. I believe the reason we collect the circuit handlers here is that calling _scope.GetServices<CircuitHandler>() may construct new instances of services registered as circuit handlers, and those services may access persistent component state in their constructors. So we need to be sure that persistent component state is available by the time these services get constructed. We might be able to use a Lazy<T>, but my concern is that making circuit handlers lazily and implicitly initialized might cause us to accidentally change when circuit handlers are constructed without us knowing it.

Edit: Found the existing comment explaining it:

aspnetcore/src/Components/Server/src/Circuits/CircuitFactory.cs

Lines 85 to 91 in e175853

// In Blazor Server we have already restored the app state, so we can get the handlers from DI.
// In Blazor Web the state is provided in the first call to UpdateRootComponents, so we need to
// delay creating the handlers until then. Otherwise, a handler would be able to access the state
// in the constructor for Blazor Server, but not in Blazor Web.
var circuitHandlers = components.Count == 0 ? [] : scope.ServiceProvider.GetServices<CircuitHandler>()
.OrderBy(h => h.Order)
.ToArray();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this code is correct. The Blazor Server version also does the same thing.

await OnCircuitOpenedAsync(cancellation);
await OnConnectionUpAsync(cancellation);

Expand All @@ -773,44 +774,9 @@ internal Task UpdateRootComponents(
throw new InvalidOperationException($"The first set of update operations must always be of type {nameof(RootComponentOperationType.Add)}");
}
}

pendingTasks = new Task[operations.Length];
}

for (var i = 0; i < operations.Length; i++)
{
var operation = operations[i];
switch (operation.Type)
{
case RootComponentOperationType.Add:
var task = webRootComponentManager.AddRootComponentAsync(
operation.SsrComponentId,
operation.Descriptor.ComponentType,
operation.Marker.Value.Key,
operation.Descriptor.Parameters);
if (pendingTasks != null)
{
pendingTasks[i] = task;
}
break;
case RootComponentOperationType.Update:
// We don't need to await component updates as any unhandled exception will be reported and terminate the circuit.
_ = webRootComponentManager.UpdateRootComponentAsync(
operation.SsrComponentId,
operation.Descriptor.ComponentType,
operation.Marker.Value.Key,
operation.Descriptor.Parameters);
break;
case RootComponentOperationType.Remove:
webRootComponentManager.RemoveRootComponent(operation.SsrComponentId);
break;
}
}

if (pendingTasks != null)
{
await Task.WhenAll(pendingTasks);
}
await PerformRootComponentOperations(operations, shouldWaitForQuiescence);

await Client.SendAsync("JS.EndUpdateRootComponents", batchId);

Expand All @@ -836,6 +802,58 @@ internal Task UpdateRootComponents(
});
}

private async ValueTask PerformRootComponentOperations(
RootComponentOperation[] operations,
bool shouldWaitForQuiescence)
{
var webRootComponentManager = Renderer.GetOrCreateWebRootComponentManager();
var pendingTasks = shouldWaitForQuiescence
? new Task[operations.Length]
: null;

// The inbound activity pipeline needs to be awaited because it populates
// the pending tasks used to wait for quiescence.
await HandleInboundActivityAsync(() =>
{
for (var i = 0; i < operations.Length; i++)
{
var operation = operations[i];
switch (operation.Type)
{
case RootComponentOperationType.Add:
var task = webRootComponentManager.AddRootComponentAsync(
operation.SsrComponentId,
operation.Descriptor.ComponentType,
operation.Marker.Value.Key,
operation.Descriptor.Parameters);
if (pendingTasks != null)
{
pendingTasks[i] = task;
}
break;
case RootComponentOperationType.Update:
// We don't need to await component updates as any unhandled exception will be reported and terminate the circuit.
_ = webRootComponentManager.UpdateRootComponentAsync(
operation.SsrComponentId,
operation.Descriptor.ComponentType,
operation.Marker.Value.Key,
operation.Descriptor.Parameters);
break;
case RootComponentOperationType.Remove:
webRootComponentManager.RemoveRootComponent(operation.SsrComponentId);
break;
}
}

return Task.CompletedTask;
});

if (pendingTasks != null)
{
await Task.WhenAll(pendingTasks);
}
}

private static partial class Log
{
// 100s used for lifecycle stuff
Expand Down
8 changes: 8 additions & 0 deletions src/Components/test/E2ETest/ServerRenderingTests/InteractivityTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1133,6 +1133,14 @@ public void NavigationManagerCanRefreshSSRPageWhenServerInteractivityEnabled()
Browser.Equal("GET", () => Browser.Exists(By.Id("method")).Text);
}

[Fact]
public void InteractiveServerRootComponent_CanAccessCircuitContext()
{
Navigate($"{ServerPathBase}/interactivity/circuit-context");

Browser.Equal("True", () => Browser.FindElement(By.Id("has-circuit-context")).Text);
}

private void BlockWebAssemblyResourceLoad()
{
((IJavaScriptExecutor)Browser).ExecuteScript("sessionStorage.setItem('block-load-boot-resource', 'true')");
Expand Down
5 changes: 5 additions & 0 deletions src/Components/test/testassets/Components.TestServer/RazorComponentEndpointsStartup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Components.TestServer.RazorComponents;
using Components.TestServer.RazorComponents.Pages.Forms;
using Components.TestServer.Services;
using Microsoft.AspNetCore.Components.Server.Circuits;
using Microsoft.AspNetCore.Mvc;

namespace TestServer;
Expand Down Expand Up @@ -35,6 +36,10 @@ public void ConfigureServices(IServiceCollection services)
services.AddHttpContextAccessor();
services.AddSingleton<AsyncOperationService>();
services.AddCascadingAuthenticationState();

var circuitContextAccessor = new TestCircuitContextAccessor();
services.AddSingleton<CircuitHandler>(circuitContextAccessor);
services.AddSingleton(circuitContextAccessor);
}

// This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
Expand Down
25 changes: 25 additions & 0 deletions ...assets/Components.TestServer/RazorComponents/Pages/Interactivity/CircuitContextPage.razor
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
@page "/interactivity/circuit-context"
@rendermode RenderMode.InteractiveServer
@inject TestCircuitContextAccessor CircuitContextAccessor

<h1>Circuit context</h1>

<p>
Has circuit context: <span id="has-circuit-context">@_hasCircuitContext</span>
</p>

@code {
private bool _hasCircuitContext;

protected override async Task OnAfterRenderAsync(bool firstRender)
{
if (firstRender)
{
await Task.Yield();

_hasCircuitContext = CircuitContextAccessor.HasCircuitContext;

StateHasChanged();
}
}
}