-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Add ClaimData for AuthenticationStateData and fix overtrimming #56878
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
Conversation
- Addresses API review feedback from #52769
| namespace Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures; | ||
|
|
||
| // REVIEW: Should this be merged into BasicTestAppServerSiteFixture? Is there any case where we wouldn't | ||
| // want to trim BasicTestAppServerSiteFixture tests when TestTrimmedOrMultithreadingApps is true? |
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.
@dotnet/aspnet-blazor-eng I'm curious what everyone thinks about this.
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 would say that if other tests don't break, we could enable it, but I suspect we wanted to have this isolated. I believe in the past we would test all the apps with trimming during release. I don't know if/when that changed, but I'm ok if we bring that back.
src/Components/test/testassets/Components.TestServer/RazorComponentEndpointsStartup.cs
Outdated
Show resolved
Hide resolved
amcasey
left a comment
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.
Product change LGTM. I'll let the stakeholders figure out the test infra changes.
This PR is a follow up to #55821 which added the
AuthenticationStateProviderimplementations from the .NET 8 Blazor web project templates to the runtime. You can opt into using these by adding the new services as follows.These services rely on JSON serializing and deserializing the newly added
AuthenticationStateDatatype which previously contained aClaimsproperty of typeIList<KeyValuePair<string, string>>. After API review feedback in #52769, we've decided to updateAuthenticationStateData.Claimsto be anIList<ClaimData>.This PR also fixes #56207 which was caused by the overtrimming
AuthenticationStateData.Claimsand updates theServerRenderedAuthenticationStateTestandDefaultAuthenticationStateSerializationOptionsTestto test published trimmed WebAssembly static assets.As part of this, I refactored the common logic of most tests that currently use static assets published to the "trimmed-or-threading" to use the new
TrimmingServerFixture. However, I wonder if we should go further and makeBasicTestAppServerSiteFixtureuse trimmed static assets by default whenTestTrimmedOrMultithreadingAppsis configured.Fixes #52769
Fixes #56207