-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Ability to monitor Blazor Server circuit activity #46968
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
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
b1bfa90
Initial prototype
MackinnonBuck 265e3e4
Update LoggingCircuitHandler.cs
MackinnonBuck ac87f05
Revert changes in preparation for revised approach
MackinnonBuck 96a1e56
Revised prototype
MackinnonBuck d1e081b
PR feedback
MackinnonBuck a273251
PR feedback, started on tests
MackinnonBuck e133aca
E2E test
MackinnonBuck 0defcd5
Cleanup
MackinnonBuck 5db6a97
PR feedback
MackinnonBuck cb6f404
Update CircuitHost.cs
MackinnonBuck e737508
Rename 'event' to 'activity'
MackinnonBuck cc7e76f
Update PublicAPI.Unshipped.txt
MackinnonBuck 2e5ac8f
Update CircuitHostTest.cs
MackinnonBuck File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
23 changes: 23 additions & 0 deletions
23
src/Components/Server/src/Circuits/CircuitInboundActivityContext.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
namespace Microsoft.AspNetCore.Components.Server.Circuits; | ||
|
||
/// <summary> | ||
/// Contains information about inbound <see cref="Circuits.Circuit"/> activity. | ||
/// </summary> | ||
public sealed class CircuitInboundActivityContext | ||
{ | ||
internal Func<Task> Handler { get; } | ||
|
||
/// <summary> | ||
/// Gets the <see cref="Circuits.Circuit"/> associated with the activity. | ||
/// </summary> | ||
public Circuit Circuit { get; } | ||
|
||
internal CircuitInboundActivityContext(Func<Task> handler, Circuit circuit) | ||
{ | ||
Handler = handler; | ||
Circuit = circuit; | ||
} | ||
} |
18 changes: 18 additions & 0 deletions
18
src/Components/Server/src/Circuits/IHandleCircuitActivity.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
namespace Microsoft.AspNetCore.Components.Server.Circuits; | ||
|
||
/// <summary> | ||
/// A handler to process inbound circuit activity. | ||
/// </summary> | ||
public interface IHandleCircuitActivity | ||
{ | ||
/// <summary> | ||
/// Invoked when inbound activity on the circuit causes an asynchronous task to be dispatched on the server. | ||
/// </summary> | ||
/// <param name="context">The <see cref="CircuitInboundActivityContext"/>.</param> | ||
/// <param name="next">The next handler to invoke.</param> | ||
/// <returns>A <see cref="Task"/> that completes when the activity has finished.</returns> | ||
Task HandleInboundActivityAsync(CircuitInboundActivityContext context, Func<CircuitInboundActivityContext, Task> next); | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,5 @@ | ||
#nullable enable | ||
Microsoft.AspNetCore.Components.Server.Circuits.CircuitInboundActivityContext | ||
Microsoft.AspNetCore.Components.Server.Circuits.CircuitInboundActivityContext.Circuit.get -> Microsoft.AspNetCore.Components.Server.Circuits.Circuit! | ||
Microsoft.AspNetCore.Components.Server.Circuits.IHandleCircuitActivity | ||
Microsoft.AspNetCore.Components.Server.Circuits.IHandleCircuitActivity.HandleInboundActivityAsync(Microsoft.AspNetCore.Components.Server.Circuits.CircuitInboundActivityContext! context, System.Func<Microsoft.AspNetCore.Components.Server.Circuits.CircuitInboundActivityContext!, System.Threading.Tasks.Task!>! next) -> System.Threading.Tasks.Task! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
51 changes: 51 additions & 0 deletions
51
src/Components/test/E2ETest/ServerExecutionTests/CircuitContextTest.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using Components.TestServer; | ||
using Microsoft.AspNetCore.Components.E2ETest; | ||
using Microsoft.AspNetCore.Components.E2ETest.Infrastructure; | ||
using Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures; | ||
using Microsoft.AspNetCore.E2ETesting; | ||
using OpenQA.Selenium; | ||
using TestServer; | ||
using Xunit.Abstractions; | ||
|
||
namespace Microsoft.AspNetCore.Components.E2ETests.ServerExecutionTests; | ||
|
||
public class CircuitContextTest : ServerTestBase<BasicTestAppServerSiteFixture<ServerStartup>> | ||
{ | ||
public CircuitContextTest( | ||
BrowserFixture browserFixture, | ||
BasicTestAppServerSiteFixture<ServerStartup> serverFixture, | ||
ITestOutputHelper output) | ||
: base(browserFixture, serverFixture, output) | ||
{ | ||
} | ||
|
||
protected override void InitializeAsyncCore() | ||
{ | ||
Navigate(ServerPathBase, noReload: false); | ||
Browser.MountTestComponent<CircuitContextComponent>(); | ||
Browser.Equal("Circuit Context", () => Browser.Exists(By.TagName("h1")).Text); | ||
} | ||
|
||
[Fact] | ||
public void ComponentMethods_HaveCircuitContext() | ||
{ | ||
Browser.Click(By.Id("trigger-click-event-button")); | ||
|
||
Browser.True(() => HasCircuitContext("SetParametersAsync")); | ||
Browser.True(() => HasCircuitContext("OnInitializedAsync")); | ||
Browser.True(() => HasCircuitContext("OnParametersSetAsync")); | ||
Browser.True(() => HasCircuitContext("OnAfterRenderAsync")); | ||
Browser.True(() => HasCircuitContext("InvokeDotNet")); | ||
Browser.True(() => HasCircuitContext("OnClickEvent")); | ||
|
||
bool HasCircuitContext(string eventName) | ||
{ | ||
var resultText = Browser.FindElement(By.Id($"circuit-context-result-{eventName}")).Text; | ||
var result = bool.Parse(resultText); | ||
return result; | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
5 changes: 5 additions & 0 deletions
5
src/Components/test/testassets/BasicTestApp/wwwroot/js/circuitContextTest.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
window.circuitContextTest = { | ||
invokeDotNetMethod: async (dotNetObject) => { | ||
await dotNetObject.invokeMethodAsync('InvokeDotNet'); | ||
}, | ||
}; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following is nitpicky so feel free to ignore it, especially if you think it makes the code unpleasant!
It may be possible to avoid the extra allocation caused by capturing these locals in the closure by redefining
HandleInboundActivityAsync
asHandleInboundActivityAsync<T>(T data, Func<T, Task> callback)
, then passing in the otherwise-captured values as theT data
parameter (for example, the caller could pass a tuple containing whatever combination of values they want). The lambda can then be defined asstatic
.Not sure if that's justified or would make the code hard to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in order to make this work, we would have to include
data
in theCircuitInboundActivityContext
class so it can be passed all the way through to the last handler (otherwise a capture would have to happen somewhere else). But then we would probably need to makeCircuitInboundActivityContext
generic in order to avoid boxing thedata
parameter. And then we would need to introduce a non-generic base type forCircuitInboundActivityContext
to avoid also makingIHandleCircuitActivity.HandleInboundActivityAsync
generic.Or maybe there's a way simpler way of doing this that I'm missing, but if it really does require that we do all this ^ then I think it might be best to leave as is. Is that okay?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am ok if we leave it as is for now, and file an issue to revisit. If in the case that nobody registers an
IHandleCircuitActivity
implementation we don't allocate, I think it is ok to leave it like this for now.