Skip to content

Commit

Permalink
More feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@javiercn
javiercn committed May 20, 2023
1 parent 86e0298 commit 09eb756
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 63 deletions.
36 changes: 21 additions & 15 deletions src/Components/Endpoints/src/Discovery/ComponentApplicationBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,33 @@ public class ComponentApplicationBuilder
/// <exception cref="InvalidOperationException">When the assembly has already been added
/// to this component application builder.
/// </exception>
public void AddLibrary(AssemblyComponentLibraryDescriptor libraryBuilder)
public void AddComponentAssembly(AssemblyComponentLibraryDescriptor libraryBuilder)
{
if (_assemblies.Contains(libraryBuilder.AssemblyName))
{
throw new InvalidOperationException("Assembly already defined.");
}
_assemblies.Add(libraryBuilder.AssemblyName);
Pages.AddFromLibraryInfo(libraryBuilder.AssemblyName, libraryBuilder.Pages);
Components.AddFromLibraryInfo(libraryBuilder.AssemblyName, libraryBuilder.Components);
Pages.Add(libraryBuilder.AssemblyName, libraryBuilder.Pages);
Components.Add(libraryBuilder.AssemblyName, libraryBuilder.Components);
}

/// <summary>
/// Removes the given <paramref name="assembly"/> and the associated definitions from
/// the current <see cref="ComponentApplicationBuilder"/>.
/// </summary>
/// <param name="assembly">The assembly name.</param>
public void RemoveComponentAssembly(Assembly assembly)
{
var assemblyName = assembly.GetName().FullName;
RemoveComponentAssembly(assemblyName);
}

internal void RemoveComponentAssembly(string assemblyName)
{
_assemblies.Remove(assemblyName);
Pages.Remove(assemblyName);
Components.Remove(assemblyName);
}

/// <summary>
Expand Down Expand Up @@ -80,18 +98,6 @@ public void Exclude(ComponentApplicationBuilder builder)
Components.Exclude(builder.Components);
}

/// <summary>
/// Removes the given <paramref name="assembly"/> and the associated definitions from
/// the current <see cref="ComponentApplicationBuilder"/>.
/// </summary>
/// <param name="assembly">The assembly name.</param>
public void RemoveLibrary(string assembly)
{
_assemblies.Remove(assembly);
Pages.RemoveFromAssembly(assembly);
Components.Remove(assembly);
}

/// <summary>
/// Gets the <see cref="ComponentApplicationBuilder"/> for the given <typeparamref name="TComponent"/>.
/// </summary>
Expand Down
24 changes: 12 additions & 12 deletions src/Components/Endpoints/src/Discovery/ComponentCollectionBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,44 +9,44 @@ namespace Microsoft.AspNetCore.Components.Discovery;
/// </summary>
internal class ComponentCollectionBuilder
{
private readonly Dictionary<string, IReadOnlyList<ComponentBuilder>> _components = new();
private readonly Dictionary<string, IReadOnlyList<ComponentBuilder>> _componentsByAssemblyName = new();

internal void Combine(ComponentCollectionBuilder components)
{
foreach (var (assembly, pageCollection) in components._components)
foreach (var (assembly, pageCollection) in components._componentsByAssemblyName)
{
if (!_components.ContainsKey(assembly))
if (!_componentsByAssemblyName.ContainsKey(assembly))
{
_components.Add(assembly, pageCollection);
_componentsByAssemblyName.Add(assembly, pageCollection);
}
}
}

internal void Exclude(ComponentCollectionBuilder components)
{
foreach (var (assembly, _) in components._components)
foreach (var (assembly, _) in components._componentsByAssemblyName)
{
if (_components.ContainsKey(assembly))
if (_componentsByAssemblyName.ContainsKey(assembly))
{
_components.Remove(assembly);
_componentsByAssemblyName.Remove(assembly);
}
}
}

internal void Remove(string name)
{
_components.Remove(name);
_componentsByAssemblyName.Remove(name);
}

internal void AddFromLibraryInfo(string assemblyName, IReadOnlyList<ComponentBuilder> components)
internal void Add(string assemblyName, IReadOnlyList<ComponentBuilder> components)
{
_components.Add(assemblyName, components);
_componentsByAssemblyName.Add(assemblyName, components);
}

internal ComponentInfo[] ToComponentCollection()
{
var totalCount = 0;
foreach (var value in _components.Values)
foreach (var value in _componentsByAssemblyName.Values)
{
totalCount += value.Count;
}
Expand All @@ -58,7 +58,7 @@ internal ComponentInfo[] ToComponentCollection()

var current = 0;
var result = new ComponentInfo[totalCount];
foreach (var (_, components) in _components)
foreach (var (_, components) in _componentsByAssemblyName)
{
for (var i = 0; i < components.Count; i++, current++)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Components/Endpoints/src/Discovery/DefaultRazorComponentApplication.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public ComponentApplicationBuilder GetBuilder()
// so that users can decide the list of things that get considered as endpoints.
var libraryName = typeof(TComponent).Assembly.FullName!;
var (pages, components) = CreatePageRouteCollection(libraryName);
builder.AddLibrary(new AssemblyComponentLibraryDescriptor(libraryName, pages, components));
builder.AddComponentAssembly(new AssemblyComponentLibraryDescriptor(libraryName, pages, components));
return builder;

static (IReadOnlyList<PageComponentBuilder>, IReadOnlyList<ComponentBuilder>) CreatePageRouteCollection(string name)
Expand Down
32 changes: 18 additions & 14 deletions src/Components/Endpoints/src/Discovery/PageCollectionBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,44 +8,44 @@ namespace Microsoft.AspNetCore.Components.Discovery;
/// </summary>
internal class PageCollectionBuilder
{
private readonly Dictionary<string, IReadOnlyList<PageComponentBuilder>> _pages = new();
private readonly Dictionary<string, IReadOnlyList<PageComponentBuilder>> _pagesByAssemblyName = new();

internal void Combine(PageCollectionBuilder pages)
{
foreach (var (assembly, pageCollection) in pages._pages)
foreach (var (assembly, pageCollection) in pages._pagesByAssemblyName)
{
if (!_pages.ContainsKey(assembly))
if (!_pagesByAssemblyName.ContainsKey(assembly))
{
_pages.Add(assembly, pageCollection);
_pagesByAssemblyName.Add(assembly, pageCollection);
}
}
}

internal void Exclude(PageCollectionBuilder pages)
{
foreach (var (assembly, _) in pages._pages)
foreach (var (assembly, _) in pages._pagesByAssemblyName)
{
if (_pages.ContainsKey(assembly))
if (_pagesByAssemblyName.ContainsKey(assembly))
{
_pages.Remove(assembly);
_pagesByAssemblyName.Remove(assembly);
}
}
}

internal void RemoveFromAssembly(string name)
internal void Remove(string name)
{
_pages.Remove(name);
_pagesByAssemblyName.Remove(name);
}

internal void AddFromLibraryInfo(string assemblyName, IReadOnlyList<PageComponentBuilder> pages)
internal void Add(string assemblyName, IReadOnlyList<PageComponentBuilder> pages)
{
_pages.Add(assemblyName, pages);
_pagesByAssemblyName.Add(assemblyName, pages);
}

internal PageComponentInfo[] ToPageCollection()
{
var totalCount = 0;
foreach (var value in _pages.Values)
foreach (var value in _pagesByAssemblyName.Values)
{
totalCount += value.Count;
}
Expand All @@ -58,13 +58,13 @@ internal PageComponentInfo[] ToPageCollection()
var list = new List<PageComponentInfo>(totalCount);
// Reuse a buffer for computing the metadata
var metadata = new List<object>();
foreach (var (assembly, pages) in _pages)
foreach (var (assembly, pages) in _pagesByAssemblyName)
{
for (var i = 0; i < pages.Count; i++)
{
var page = pages[i];
ResolveMetadata(page.PageType!, metadata);
var pageMetadata = metadata.ToArray();
var pageMetadata = metadata.Count == 0 ? Array.Empty<object>() : metadata.ToArray();
foreach (var route in page.RouteTemplates!)
{
list.Add(page.Build(route, pageMetadata));
Expand All @@ -79,6 +79,9 @@ private static void ResolveMetadata(Type componentType, List<object> result)
{
// We remove the route attribute since it is captured on the endpoint.
// This is similar to how MVC behaves.
// We don't want to have multiple routes on the same endpoint, as only
// one of them is associated with it.
// It's confusing for any consumer of the endpoint metadata to require
var attributes = componentType.GetCustomAttributes(inherit: true);
result.Clear();
foreach (var attribute in attributes)
Expand All @@ -87,6 +90,7 @@ private static void ResolveMetadata(Type componentType, List<object> result)
{
continue;
}

result.Add(attribute);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/Components/Endpoints/src/PublicAPI.Unshipped.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ Microsoft.AspNetCore.Components.Discovery.AssemblyComponentLibraryDescriptor.Ass
Microsoft.AspNetCore.Components.Discovery.AssemblyComponentLibraryDescriptor.Components.get -> System.Collections.Generic.IReadOnlyList<Microsoft.AspNetCore.Components.Discovery.ComponentBuilder!>!
Microsoft.AspNetCore.Components.Discovery.AssemblyComponentLibraryDescriptor.Pages.get -> System.Collections.Generic.IReadOnlyList<Microsoft.AspNetCore.Components.Discovery.PageComponentBuilder!>!
Microsoft.AspNetCore.Components.Discovery.ComponentApplicationBuilder
Microsoft.AspNetCore.Components.Discovery.ComponentApplicationBuilder.AddLibrary(Microsoft.AspNetCore.Components.Discovery.AssemblyComponentLibraryDescriptor! libraryBuilder) -> void
Microsoft.AspNetCore.Components.Discovery.ComponentApplicationBuilder.AddComponentAssembly(Microsoft.AspNetCore.Components.Discovery.AssemblyComponentLibraryDescriptor! libraryBuilder) -> void
Microsoft.AspNetCore.Components.Discovery.ComponentApplicationBuilder.Combine(Microsoft.AspNetCore.Components.Discovery.ComponentApplicationBuilder! other) -> void
Microsoft.AspNetCore.Components.Discovery.ComponentApplicationBuilder.ComponentApplicationBuilder() -> void
Microsoft.AspNetCore.Components.Discovery.ComponentApplicationBuilder.Exclude(Microsoft.AspNetCore.Components.Discovery.ComponentApplicationBuilder! builder) -> void
Microsoft.AspNetCore.Components.Discovery.ComponentApplicationBuilder.HasLibrary(string! assemblyName) -> bool
Microsoft.AspNetCore.Components.Discovery.ComponentApplicationBuilder.RemoveLibrary(string! assembly) -> void
Microsoft.AspNetCore.Components.Discovery.ComponentApplicationBuilder.RemoveComponentAssembly(System.Reflection.Assembly! assembly) -> void
Microsoft.AspNetCore.Components.Discovery.ComponentBuilder
Microsoft.AspNetCore.Components.Discovery.ComponentBuilder.AssemblyName.get -> string!
Microsoft.AspNetCore.Components.Discovery.ComponentBuilder.AssemblyName.set -> void
Expand Down
34 changes: 17 additions & 17 deletions src/Components/Endpoints/test/Discovery/ComponentApplicationBuilderTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public void ComponentApplicationBuilder_CanAddLibrary()
{
// Arrange
var builder = new ComponentApplicationBuilder();
builder.AddLibrary(new AssemblyComponentLibraryDescriptor(
builder.AddComponentAssembly(new AssemblyComponentLibraryDescriptor(
"App1",
CreateApp1Pages("App1"),
CreateApp1Components("App1")));
Expand Down Expand Up @@ -41,12 +41,12 @@ public void ComponentApplicationBuilder_CanAddMultipleLibraries()
{
// Arrange
var builder = new ComponentApplicationBuilder();
builder.AddLibrary(new AssemblyComponentLibraryDescriptor(
builder.AddComponentAssembly(new AssemblyComponentLibraryDescriptor(
"App1",
CreateApp1Pages("App1"),
CreateApp1Components("App1")));

builder.AddLibrary(new AssemblyComponentLibraryDescriptor(
builder.AddComponentAssembly(new AssemblyComponentLibraryDescriptor(
"App2",
CreateApp2Pages("App2"),
CreateApp2Components("App2")));
Expand Down Expand Up @@ -80,17 +80,17 @@ public void ComponentApplicationBuilder_CanRemoveLibrary()
{
// Arrange
var builder = new ComponentApplicationBuilder();
builder.AddLibrary(new AssemblyComponentLibraryDescriptor(
builder.AddComponentAssembly(new AssemblyComponentLibraryDescriptor(
"App1",
CreateApp1Pages("App1"),
CreateApp1Components("App1")));

builder.AddLibrary(new AssemblyComponentLibraryDescriptor(
builder.AddComponentAssembly(new AssemblyComponentLibraryDescriptor(
"App2",
CreateApp2Pages("App2"),
CreateApp2Components("App2")));

builder.RemoveLibrary("App1");
builder.RemoveComponentAssembly("App1");

var app = builder.Build();

Expand All @@ -115,13 +115,13 @@ public void ComponentApplicationBuilder_CanCombineBuilders()
{
// Arrange
var builder = new ComponentApplicationBuilder();
builder.AddLibrary(new AssemblyComponentLibraryDescriptor(
builder.AddComponentAssembly(new AssemblyComponentLibraryDescriptor(
"App1",
CreateApp1Pages("App1"),
CreateApp1Components("App1")));

var builder2 = new ComponentApplicationBuilder();
builder2.AddLibrary(new AssemblyComponentLibraryDescriptor(
builder2.AddComponentAssembly(new AssemblyComponentLibraryDescriptor(
"App2",
CreateApp2Pages("App2"),
CreateApp2Components("App2")));
Expand Down Expand Up @@ -156,23 +156,23 @@ public void ComponentApplicationBuilder_CombiningDoesNotDuplicateSharedDependenc
{
// Arrange
var builder = new ComponentApplicationBuilder();
builder.AddLibrary(new AssemblyComponentLibraryDescriptor(
builder.AddComponentAssembly(new AssemblyComponentLibraryDescriptor(
"App1",
CreateApp1Pages("App1"),
CreateApp1Components("App1")));

builder.AddLibrary(new AssemblyComponentLibraryDescriptor(
builder.AddComponentAssembly(new AssemblyComponentLibraryDescriptor(
"Shared",
CreateSharedPages("Shared"),
CreateSharedComponents("Shared")));

var builder2 = new ComponentApplicationBuilder();
builder2.AddLibrary(new AssemblyComponentLibraryDescriptor(
builder2.AddComponentAssembly(new AssemblyComponentLibraryDescriptor(
"App2",
CreateApp2Pages("App2"),
CreateApp2Components("App2")));

builder2.AddLibrary(new AssemblyComponentLibraryDescriptor(
builder2.AddComponentAssembly(new AssemblyComponentLibraryDescriptor(
"Shared",
CreateSharedPages("Shared"),
CreateSharedComponents("Shared")));
Expand Down Expand Up @@ -215,28 +215,28 @@ public void ComponentApplicationBuilder_CanExcludeOtherBuilders()
{
// Arrange
var builder = new ComponentApplicationBuilder();
builder.AddLibrary(new AssemblyComponentLibraryDescriptor(
builder.AddComponentAssembly(new AssemblyComponentLibraryDescriptor(
"App1",
CreateApp1Pages("App1"),
CreateApp1Components("App1")));

builder.AddLibrary(new AssemblyComponentLibraryDescriptor(
builder.AddComponentAssembly(new AssemblyComponentLibraryDescriptor(
"App2",
CreateApp2Pages("App2"),
CreateApp2Components("App2")));

builder.AddLibrary(new AssemblyComponentLibraryDescriptor(
builder.AddComponentAssembly(new AssemblyComponentLibraryDescriptor(
"Shared",
CreateSharedPages("Shared"),
CreateSharedComponents("Shared")));

var builder2 = new ComponentApplicationBuilder();
builder2.AddLibrary(new AssemblyComponentLibraryDescriptor(
builder2.AddComponentAssembly(new AssemblyComponentLibraryDescriptor(
"App2",
CreateApp2Pages("App2"),
CreateApp2Components("App2")));

builder2.AddLibrary(new AssemblyComponentLibraryDescriptor(
builder2.AddComponentAssembly(new AssemblyComponentLibraryDescriptor(
"Shared",
CreateSharedPages("Shared"),
CreateSharedComponents("Shared")));
Expand Down
3 changes: 1 addition & 2 deletions src/Components/Endpoints/test/RazorComponentEndpointDataSourceTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,11 +231,10 @@ public void ExplicitlySetRenderModesAreRespected(
}
};


private ComponentApplicationBuilder CreateBuilder(params Type[] types)
{
var builder = new ComponentApplicationBuilder();
builder.AddLibrary(new AssemblyComponentLibraryDescriptor(
builder.AddComponentAssembly(new AssemblyComponentLibraryDescriptor(
"TestAssembly",
Array.Empty<PageComponentBuilder>(),
types.Select(t => new ComponentBuilder
Expand Down

0 comments on commit 09eb756

Please sign in to comment.