Skip to content

Update SDK to bring in RSG one assembly changes #30827

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

Closed
wants to merge 8 commits into from
Closed

Update SDK to bring in RSG one assembly changes #30827

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
755513d
Update SDK to bring in RSG one assembly changes
captainsafia Mar 11, 2021
8c2efb6
Update version to pick up bug fix
captainsafia Mar 15, 2021
e84fe3b
Update WASM authentication tests
captainsafia Mar 15, 2021
0e4c0f0
Remove unneeded assembly attributes file
captainsafia Mar 16, 2021
63e71c1
Fix up Startup for test site
captainsafia Mar 16, 2021
c19003c
Bump to new SDK
captainsafia Mar 17, 2021
4986a10
Clean up consolidated views APIs
captainsafia Mar 17, 2021
79e023c
Update RazorExtensions.cs
captainsafia Mar 17, 2021
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
4 changes: 2 additions & 2 deletions global.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{
"sdk": {
"version": "6.0.100-preview.3.21160.5"
"version": "6.0.100-preview.3.21167.3"
},
"tools": {
"dotnet": "6.0.100-preview.3.21160.5",
"dotnet": "6.0.100-preview.3.21167.3",
"runtimes": {
"dotnet/x64": [
"2.1.25",
Expand Down
2 changes: 1 addition & 1 deletion ...ureAD/Authentication.AzureAD.UI/src/Microsoft.AspNetCore.Authentication.AzureAD.UI.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
<PackageTags>aspnetcore;authentication;AzureAD</PackageTags>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<ProvideApplicationPartFactoryAttributeTypeName>Microsoft.AspNetCore.Mvc.ApplicationParts.NullApplicationPartFactory, Microsoft.AspNetCore.Mvc.Core</ProvideApplicationPartFactoryAttributeTypeName>
<AddRazorSupportForMvc>true</AddRazorSupportForMvc>
<NoWarn>RS0016</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion ...Authentication.AzureADB2C.UI/src/Microsoft.AspNetCore.Authentication.AzureADB2C.UI.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
<PackageTags>aspnetcore;authentication;AzureADB2C</PackageTags>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<ProvideApplicationPartFactoryAttributeTypeName>Microsoft.AspNetCore.Mvc.ApplicationParts.NullApplicationPartFactory, Microsoft.AspNetCore.Mvc.Core</ProvideApplicationPartFactoryAttributeTypeName>
<AddRazorSupportForMvc>true</AddRazorSupportForMvc>
<NoWarn>RS0016</NoWarn>
Copy link
Member Author

Choose a reason for hiding this comment

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

The build for this codepath relies on the RazorPageDocumentClassifier which we didn't update to support consolidated views. As a result, the types generated in the output assembly are still public and picked up by the public API analyzer during the build.

I fixed this in this PR but we'll need to wait for the aspnetcore -> SDK -> installer flow with this change to finish before we can remove this check.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough. I would block on that, but it's good that we picked it up quickly

</PropertyGroup>

<ItemGroup>
Expand Down
6 changes: 0 additions & 6 deletions src/Azure/AzureAD/test/FunctionalTests/WebAuthenticationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,8 @@ public WebAuthenticationTests(WebApplicationFactory<Startup> fixture)
public static TheoryData<string> NotAddedEndpoints =>
new TheoryData<string>()
{
"/AzureAD/Account/AccessDenied",
Copy link
Contributor

Choose a reason for hiding this comment

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

What happened here?

Copy link
Member Author

Choose a reason for hiding this comment

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

So, previously, the Azure AD UI projects used a NullApplicationFactory and instead relied on their own logic to discover the application parts from the related assembly (see https://github.com/dotnet/aspnetcore/pull/31039/files#diff-754ffcfbe4655cc4cca0a446a4f1b0eb3552a8795c136385b6ffad03259e0a7dL199). This API gets invoked from the AddAzureAD extension method.

Since the views are compiled into the app assembly and we are no longer using the NullApplicationFactory, we don't need to rely on the discovery logic (I removed it in the new PR above).

The controllers don't change though because those are discovered with a feature provider that exists outside of the views story.

"/AzureAD/Account/Error",
"/AzureAD/Account/SignedOut",
"/AzureAD/Account/SignIn",
"/AzureAD/Account/SignOut",
"/AzureADB2C/Account/AccessDenied",
"/AzureADB2C/Account/Error",
"/AzureADB2C/Account/SignedOut",
"/AzureADB2C/Account/SignIn",
"/AzureADB2C/Account/ResetPassword",
"/AzureADB2C/Account/EditProfile",
Expand Down
2 changes: 1 addition & 1 deletion src/Identity/UI/src/IdentityBuilderUIExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public static IdentityBuilder AddDefaultUI(this IdentityBuilder builder)
private static readonly IDictionary<UIFramework, string> _assemblyMap =
new Dictionary<UIFramework, string>()
{
[UIFramework.Bootstrap4] = "Microsoft.AspNetCore.Identity.UI.Views.V4",
[UIFramework.Bootstrap4] = "Microsoft.AspNetCore.Identity.UI.",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this goes away. This used to switch the related part but we're not generating any now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep. You're right. I should've looked way closer at this. I made the appropriate changes in #31039.

};

private static void AddRelatedParts(IdentityBuilder builder)
Expand Down
3 changes: 1 addition & 2 deletions src/Identity/UI/src/Microsoft.AspNetCore.Identity.UI.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<PackageTags>aspnetcore;identity;membership;razorpages</PackageTags>
<ProvideApplicationPartFactoryAttributeTypeName>Microsoft.AspNetCore.Mvc.ApplicationParts.NullApplicationPartFactory, Microsoft.AspNetCore.Mvc.Core</ProvideApplicationPartFactoryAttributeTypeName>
<EnableDefaultRazorGenerateItems>false</EnableDefaultRazorGenerateItems>
<AddRazorSupportForMvc>true</AddRazorSupportForMvc>
<RazorTargetName>Microsoft.AspNetCore.Identity.UI.Views.V4</RazorTargetName>
<NoWarn>RS0016</NoWarn>

<DisableStaticWebAssetsBuildPropsFileGeneration>true</DisableStaticWebAssetsBuildPropsFileGeneration>
<StaticWebAssetsDisableProjectBuildPropsFileGeneration>true</StaticWebAssetsDisableProjectBuildPropsFileGeneration>
Expand Down
6 changes: 0 additions & 6 deletions src/Identity/UI/src/Properties/AssemblyInfo.ApplicationParts.cs
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion src/Identity/test/Identity.FunctionalTests/Infrastructure/ServerFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ private static void AddRelatedParts(IServiceCollection services, string framewor
var _assemblyMap =
new Dictionary<UIFramework, string>()
{
[UIFramework.Bootstrap4] = "Microsoft.AspNetCore.Identity.UI.Views.V4",
[UIFramework.Bootstrap4] = "Microsoft.AspNetCore.Identity.UI",
};

var mvcBuilder = services
Expand Down
5 changes: 1 addition & 4 deletions src/Mvc/test/WebSites/ControllersFromServicesWebSite/Startup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,7 @@ public void ConfigureServices(IServiceCollection services)
typeof(ComponentFromServicesViewComponent),
typeof(InServicesTagHelper)));

var relatedAssenbly = RelatedAssemblyAttribute
.GetRelatedAssemblies(GetType().Assembly, throwOnError: true)
.SingleOrDefault();
foreach (var part in CompiledRazorAssemblyApplicationPartFactory.GetDefaultApplicationParts(relatedAssenbly))
foreach (var part in CompiledRazorAssemblyApplicationPartFactory.GetDefaultApplicationParts(Assembly.GetExecutingAssembly()))
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't the consolidated part factory already take care of this?

Copy link
Member Author

Choose a reason for hiding this comment

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

It does. However, this particularly startup logic removes all the discovered application parts and adds its own.

aspnetcore/src/Mvc/test/WebSites/ControllersFromServicesWebSite/Startup.cs

Line 26 in 35bb1b8

.ConfigureApplicationPartManager(manager => manager.ApplicationParts.Clear())

I think it does this to validate the behavior of the specific extension methods used here.

{
manager.ApplicationParts.Add(part);
}
Expand Down
90 changes: 0 additions & 90 deletions ...icrosoft.AspNetCore.Mvc.Razor.Extensions/src/ConsolidatedMvcViewDocumentClassifierPass.cs
View file
Open in desktop

This file was deleted.

19 changes: 17 additions & 2 deletions src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/src/MvcViewDocumentClassifierPass.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,19 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions
{
public class MvcViewDocumentClassifierPass : DocumentClassifierPassBase
{
private bool _useConsolidatedMvcViews = false;

public static readonly string MvcViewDocumentKind = "mvc.1.0.view";

protected override string DocumentKind => MvcViewDocumentKind;

protected override bool IsMatch(RazorCodeDocument codeDocument, DocumentIntermediateNode documentNode) => true;

public MvcViewDocumentClassifierPass(bool useConsolidatedMvcViews = false)
{
_useConsolidatedMvcViews = useConsolidatedMvcViews;
}

protected override void OnDocumentStructureCreated(
RazorCodeDocument codeDocument,
NamespaceDeclarationIntermediateNode @namespace,
Expand All @@ -25,7 +32,7 @@ protected override void OnDocumentStructureCreated(

if (!codeDocument.TryComputeNamespace(fallbackToRootNamespace: false, out var namespaceName))
{
@namespace.Content = "AspNetCore";
@namespace.Content = _useConsolidatedMvcViews ? "AspNetCoreGeneratedDocument" : "AspNetCore";
}
else
{
Expand All @@ -47,7 +54,15 @@ protected override void OnDocumentStructureCreated(

@class.BaseType = "global::Microsoft.AspNetCore.Mvc.Razor.RazorPage<TModel>";
@class.Modifiers.Clear();
@class.Modifiers.Add("public");
if (_useConsolidatedMvcViews)
{
@class.Modifiers.Add("internal");
@class.Modifiers.Add("sealed");
}
else
{
@class.Modifiers.Add("public");
}

method.MethodName = "ExecuteAsync";
method.Modifiers.Clear();
Expand Down
7 changes: 4 additions & 3 deletions src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/src/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#nullable enable
Microsoft.AspNetCore.Mvc.Razor.Extensions.ConsolidatedMvcViewDocumentClassifierPass
Microsoft.AspNetCore.Mvc.Razor.Extensions.ConsolidatedMvcViewDocumentClassifierPass.ConsolidatedMvcViewDocumentClassifierPass() -> void
~static readonly Microsoft.AspNetCore.Mvc.Razor.Extensions.ConsolidatedMvcViewDocumentClassifierPass.MvcViewDocumentKind -> string
*REMOVED*Microsoft.AspNetCore.Mvc.Razor.Extensions.MvcViewDocumentClassifierPass.MvcViewDocumentClassifierPass() -> void
Copy link
Member Author

Choose a reason for hiding this comment

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

Eh, since we're doing this in multiple classes now I think using a constructor that takes a flag will be better than duplicating the code to avoid ragrets later.

*REMOVED*Microsoft.AspNetCore.Mvc.Razor.Extensions.RazorPageDocumentClassifierPass.RazorPageDocumentClassifierPass() -> void
Microsoft.AspNetCore.Mvc.Razor.Extensions.MvcViewDocumentClassifierPass.MvcViewDocumentClassifierPass(bool useConsolidatedMvcViews = false) -> void
Microsoft.AspNetCore.Mvc.Razor.Extensions.RazorPageDocumentClassifierPass.RazorPageDocumentClassifierPass(bool useConsolidatedMvcViews = false) -> void
5 changes: 3 additions & 2 deletions src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/src/RazorExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,15 @@ public static void Register(RazorProjectEngineBuilder builder)
builder.Features.Add(new ModelExpressionPass());
builder.Features.Add(new PagesPropertyInjectionPass());
builder.Features.Add(new ViewComponentTagHelperPass());
builder.Features.Add(new RazorPageDocumentClassifierPass());

if (builder.Configuration.UseConsolidatedMvcViews)
{
builder.Features.Add(new ConsolidatedMvcViewDocumentClassifierPass());
builder.Features.Add(new RazorPageDocumentClassifierPass(useConsolidatedMvcViews: true));
builder.Features.Add(new MvcViewDocumentClassifierPass(useConsolidatedMvcViews: true));
Comment on lines +42 to +43
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
builder.Features.Add(new RazorPageDocumentClassifierPass(useConsolidatedMvcViews: true));
builder.Features.Add(new MvcViewDocumentClassifierPass(useConsolidatedMvcViews: true));
builder.Features.Add(new RazorPageDocumentClassifierPass(builder.Configuration.UseConsolidatedMvcViews));
builder.Features.Add(new MvcViewDocumentClassifierPass(builder.Configuration.UseConsolidatedMvcViews));

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't need the if

}
else
{
builder.Features.Add(new RazorPageDocumentClassifierPass());
builder.Features.Add(new MvcViewDocumentClassifierPass());
}

Expand Down
19 changes: 17 additions & 2 deletions src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/src/RazorPageDocumentClassifierPass.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,16 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions
{
public class RazorPageDocumentClassifierPass : DocumentClassifierPassBase
{
private bool _useConsolidatedMvcViews = false;

public static readonly string RazorPageDocumentKind = "mvc.1.0.razor-page";
public static readonly string RouteTemplateKey = "RouteTemplate";

public RazorPageDocumentClassifierPass(bool useConsolidatedMvcViews = false)
{
_useConsolidatedMvcViews = useConsolidatedMvcViews;
}

private static readonly RazorProjectEngine LeadingDirectiveParsingEngine = RazorProjectEngine.Create(
RazorConfiguration.Create(RazorLanguageVersion.Version_3_0, "leading-directive-parser", Array.Empty<RazorExtension>()),
RazorProjectFileSystem.Create("/"),
Expand Down Expand Up @@ -50,7 +57,7 @@ protected override void OnDocumentStructureCreated(

if (!codeDocument.TryComputeNamespace(fallbackToRootNamespace: false, out var namespaceName))
{
@namespace.Content = "AspNetCore";
@namespace.Content = _useConsolidatedMvcViews ? "AspNetCoreGeneratedDocument" : "AspNetCore";
}
else
{
Expand All @@ -71,7 +78,15 @@ protected override void OnDocumentStructureCreated(

@class.BaseType = "global::Microsoft.AspNetCore.Mvc.RazorPages.Page";
@class.Modifiers.Clear();
@class.Modifiers.Add("public");
if (_useConsolidatedMvcViews)
{
@class.Modifiers.Add("internal");
@class.Modifiers.Add("sealed");
}
else
{
@class.Modifiers.Add("public");
}

method.MethodName = "ExecuteAsync";
method.Modifiers.Clear();
Expand Down
12 changes: 6 additions & 6 deletions ...oft.AspNetCore.Mvc.Razor.Extensions/test/ConsolidatedMvcViewDocumentClassifierPassTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public void ConsolidatedMvcViewDocumentClassifierPass_SetsDifferentNamespace()

var projectEngine = CreateProjectEngine();
var irDocument = CreateIRDocument(projectEngine, codeDocument);
var pass = new ConsolidatedMvcViewDocumentClassifierPass
var pass = new MvcViewDocumentClassifierPass(useConsolidatedMvcViews: true)
{
Engine = projectEngine.Engine
};
Expand All @@ -42,7 +42,7 @@ public void ConsolidatedMvcViewDocumentClassifierPass_SetsClass()

var projectEngine = CreateProjectEngine();
var irDocument = CreateIRDocument(projectEngine, codeDocument);
var pass = new ConsolidatedMvcViewDocumentClassifierPass
var pass = new MvcViewDocumentClassifierPass(useConsolidatedMvcViews: true)
{
Engine = projectEngine.Engine
};
Expand All @@ -67,7 +67,7 @@ public void MvcViewDocumentClassifierPass_NullFilePath_SetsClass()

var projectEngine = CreateProjectEngine();
var irDocument = CreateIRDocument(projectEngine, codeDocument);
var pass = new ConsolidatedMvcViewDocumentClassifierPass
var pass = new MvcViewDocumentClassifierPass(useConsolidatedMvcViews: true)
{
Engine = projectEngine.Engine
};
Expand All @@ -86,15 +86,15 @@ public void MvcViewDocumentClassifierPass_NullFilePath_SetsClass()
[Theory]
[InlineData("/Views/Home/Index.cshtml", "_Views_Home_Index")]
[InlineData("/Areas/MyArea/Views/Home/About.cshtml", "_Areas_MyArea_Views_Home_About")]
public void MvcViewDocumentClassifierPass_UsesRelativePathToGenerateTypeName(string relativePath, string expected)
public void ConsolidatedMvcViewDocumentClassifierPass_UsesRelativePathToGenerateTypeName(string relativePath, string expected)
{
// Arrange
var properties = new RazorSourceDocumentProperties(filePath: "ignored", relativePath: relativePath);
var codeDocument = RazorCodeDocument.Create(RazorSourceDocument.Create("some-content", properties));

var projectEngine = CreateProjectEngine();
var irDocument = CreateIRDocument(projectEngine, codeDocument);
var pass = new ConsolidatedMvcViewDocumentClassifierPass
var pass = new MvcViewDocumentClassifierPass(useConsolidatedMvcViews: true)
{
Engine = projectEngine.Engine
};
Expand All @@ -117,7 +117,7 @@ public void ConsolidatedMvcViewDocumentClassifierPass_SetsUpExecuteAsyncMethod()

var projectEngine = CreateProjectEngine();
var irDocument = CreateIRDocument(projectEngine, codeDocument);
var pass = new ConsolidatedMvcViewDocumentClassifierPass
var pass = new MvcViewDocumentClassifierPass(useConsolidatedMvcViews: true)
{
Engine = projectEngine.Engine
};
Expand Down
4 changes: 4 additions & 0 deletions src/SignalR/clients/ts/yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
Copy link
Member Author

Choose a reason for hiding this comment

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

yarn expects there to be a lock file even it if is empty. Commiting this to avoid the annoyance of always having this as an untracked file.

Copy link
Contributor

Choose a reason for hiding this comment

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

@BrennanConroy why do you do this to us?

# yarn lockfile v1