From 9f82b7be7523e4057cc610bc9aaadf28887952c3 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Tue, 16 Jul 2019 13:38:08 -0700 Subject: [PATCH] Cache the result of ComponentResolver (#11810) * Add caching to ComponentFactory and ComponentResolver --- .../test/DirectiveRazorIntegrationTest.cs | 4 +- .../Components/src/ComponentFactory.cs | 62 +++---- .../Components/src/ComponentResolver.cs | 73 -------- .../Components/src/Rendering/Renderer.cs | 7 +- .../Components/src/Routing/RouteEntry.cs | 19 +- .../Components/src/Routing/RouteTable.cs | 116 +----------- .../src/Routing/RouteTableFactory.cs | 137 ++++++++++++++ .../Components/src/Routing/RouteTemplate.cs | 8 +- .../Components/src/Routing/Router.cs | 3 +- .../Components/test/ComponentFactoryTest.cs | 168 ++++++++++++++++++ ...ableTests.cs => RouteTableFactoryTests.cs} | 8 +- .../test/Routing/TemplateParserTests.cs | 12 +- .../test/E2ETest/Tests/RoutingTest.cs | 22 ++- .../BasicTestApp/RouterTest/Default.razor | 1 + .../BasicTestApp/RouterTest/Links.razor | 9 +- .../RouteableComponentFromPackage.razor | 16 ++ .../test/testassets/TestServer/Startup.cs | 1 + 17 files changed, 405 insertions(+), 261 deletions(-) delete mode 100644 src/Components/Components/src/ComponentResolver.cs create mode 100644 src/Components/Components/src/Routing/RouteTableFactory.cs create mode 100644 src/Components/Components/test/ComponentFactoryTest.cs rename src/Components/Components/test/Routing/{RouteTableTests.cs => RouteTableFactoryTests.cs} (97%) create mode 100644 src/Components/test/testassets/TestContentPackage/RouteableComponentFromPackage.razor diff --git a/src/Components/Blazor/Build/test/DirectiveRazorIntegrationTest.cs b/src/Components/Blazor/Build/test/DirectiveRazorIntegrationTest.cs index 5d5fccef0d..2c610bcc95 100644 --- a/src/Components/Blazor/Build/test/DirectiveRazorIntegrationTest.cs +++ b/src/Components/Blazor/Build/test/DirectiveRazorIntegrationTest.cs @@ -132,8 +132,8 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test var serviceProvider = new TestServiceProvider(); serviceProvider.AddService(new MyService1Impl()); serviceProvider.AddService(new MyService2Impl()); - var componentFactory = new ComponentFactory(serviceProvider); - var component = componentFactory.InstantiateComponent(componentType); + var componentFactory = new ComponentFactory(); + var component = componentFactory.InstantiateComponent(serviceProvider, componentType); var frames = GetRenderTree(component); // Assert 2: Rendered component behaves correctly diff --git a/src/Components/Components/src/ComponentFactory.cs b/src/Components/Components/src/ComponentFactory.cs index 78f3b7dbfd..aebf96bc06 100644 --- a/src/Components/Components/src/ComponentFactory.cs +++ b/src/Components/Components/src/ComponentFactory.cs @@ -1,44 +1,41 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using Microsoft.AspNetCore.Components.Reflection; using System; using System.Collections.Concurrent; -using System.Collections.Generic; using System.Linq; using System.Reflection; +using Microsoft.AspNetCore.Components.Reflection; namespace Microsoft.AspNetCore.Components { + /// + /// The property on this type is used as a static global cache. Ensure any changes to this type + /// are thread safe and can be safely cached statically. + /// internal class ComponentFactory { - private readonly static BindingFlags _injectablePropertyBindingFlags + private static readonly BindingFlags _injectablePropertyBindingFlags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic; - private readonly IServiceProvider _serviceProvider; - private readonly IDictionary> _cachedInitializers - = new ConcurrentDictionary>(); + private readonly ConcurrentDictionary> _cachedInitializers + = new ConcurrentDictionary>(); - public ComponentFactory(IServiceProvider serviceProvider) - { - _serviceProvider = serviceProvider - ?? throw new ArgumentNullException(nameof(serviceProvider)); - } + public static readonly ComponentFactory Instance = new ComponentFactory(); - public IComponent InstantiateComponent(Type componentType) + public IComponent InstantiateComponent(IServiceProvider serviceProvider, Type componentType) { - if (!typeof(IComponent).IsAssignableFrom(componentType)) + var instance = Activator.CreateInstance(componentType); + if (!(instance is IComponent component)) { - throw new ArgumentException($"The type {componentType.FullName} does not " + - $"implement {nameof(IComponent)}.", nameof(componentType)); + throw new ArgumentException($"The type {componentType.FullName} does not implement {nameof(IComponent)}.", nameof(componentType)); } - var instance = (IComponent)Activator.CreateInstance(componentType); - PerformPropertyInjection(instance); - return instance; + PerformPropertyInjection(serviceProvider, component); + return component; } - private void PerformPropertyInjection(IComponent instance) + private void PerformPropertyInjection(IServiceProvider serviceProvider, IComponent instance) { // This is thread-safe because _cachedInitializers is a ConcurrentDictionary. // We might generate the initializer more than once for a given type, but would @@ -47,18 +44,19 @@ namespace Microsoft.AspNetCore.Components if (!_cachedInitializers.TryGetValue(instanceType, out var initializer)) { initializer = CreateInitializer(instanceType); - _cachedInitializers[instanceType] = initializer; + _cachedInitializers.TryAdd(instanceType, initializer); } - initializer(instance); + initializer(serviceProvider, instance); } - private Action CreateInitializer(Type type) + private Action CreateInitializer(Type type) { // Do all the reflection up front var injectableProperties = MemberAssignment.GetPropertiesIncludingInherited(type, _injectablePropertyBindingFlags) - .Where(p => p.GetCustomAttribute() != null); + .Where(p => p.IsDefined(typeof(InjectAttribute))); + var injectables = injectableProperties.Select(property => ( propertyName: property.Name, @@ -66,23 +64,25 @@ namespace Microsoft.AspNetCore.Components setter: MemberAssignment.CreatePropertySetter(type, property) )).ToArray(); + return Initialize; + // Return an action whose closure can write all the injected properties // without any further reflection calls (just typecasts) - return instance => + void Initialize(IServiceProvider serviceProvider, IComponent component) { - foreach (var injectable in injectables) + foreach (var (propertyName, propertyType, setter) in injectables) { - var serviceInstance = _serviceProvider.GetService(injectable.propertyType); + var serviceInstance = serviceProvider.GetService(propertyType); if (serviceInstance == null) { throw new InvalidOperationException($"Cannot provide a value for property " + - $"'{injectable.propertyName}' on type '{type.FullName}'. There is no " + - $"registered service of type '{injectable.propertyType}'."); + $"'{propertyName}' on type '{type.FullName}'. There is no " + + $"registered service of type '{propertyType}'."); } - injectable.setter.SetValue(instance, serviceInstance); + setter.SetValue(component, serviceInstance); } - }; + } } } } diff --git a/src/Components/Components/src/ComponentResolver.cs b/src/Components/Components/src/ComponentResolver.cs deleted file mode 100644 index 4e107da3db..0000000000 --- a/src/Components/Components/src/ComponentResolver.cs +++ /dev/null @@ -1,73 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Collections.Generic; -using System.Linq; -using System.Reflection; - -namespace Microsoft.AspNetCore.Components -{ - /// - /// Resolves components for an application. - /// - internal static class ComponentResolver - { - /// - /// Lists all the types - /// - /// - /// - public static IEnumerable ResolveComponents(Assembly appAssembly) - { - var componentsAssembly = typeof(IComponent).Assembly; - - return EnumerateAssemblies(appAssembly.GetName(), componentsAssembly, new HashSet(new AssemblyComparer())) - .SelectMany(a => a.ExportedTypes) - .Where(t => typeof(IComponent).IsAssignableFrom(t)); - } - - private static IEnumerable EnumerateAssemblies( - AssemblyName assemblyName, - Assembly componentAssembly, - HashSet visited) - { - var assembly = Assembly.Load(assemblyName); - if (visited.Contains(assembly)) - { - // Avoid traversing visited assemblies. - yield break; - } - visited.Add(assembly); - var references = assembly.GetReferencedAssemblies(); - if (!references.Any(r => string.Equals(r.FullName, componentAssembly.FullName, StringComparison.Ordinal))) - { - // Avoid traversing references that don't point to Components (like netstandard2.0) - yield break; - } - else - { - yield return assembly; - - // Look at the list of transitive dependencies for more components. - foreach (var reference in references.SelectMany(r => EnumerateAssemblies(r, componentAssembly, visited))) - { - yield return reference; - } - } - } - - private class AssemblyComparer : IEqualityComparer - { - public bool Equals(Assembly x, Assembly y) - { - return string.Equals(x?.FullName, y?.FullName, StringComparison.Ordinal); - } - - public int GetHashCode(Assembly obj) - { - return obj.FullName.GetHashCode(); - } - } - } -} diff --git a/src/Components/Components/src/Rendering/Renderer.cs b/src/Components/Components/src/Rendering/Renderer.cs index cce99aca88..f45e109a14 100644 --- a/src/Components/Components/src/Rendering/Renderer.cs +++ b/src/Components/Components/src/Rendering/Renderer.cs @@ -17,7 +17,7 @@ namespace Microsoft.AspNetCore.Components.Rendering /// public abstract partial class Renderer : IDisposable { - private readonly ComponentFactory _componentFactory; + private readonly IServiceProvider _serviceProvider; private readonly Dictionary _componentStateById = new Dictionary(); private readonly RenderBatchBuilder _batchBuilder = new RenderBatchBuilder(); private readonly Dictionary _eventBindings = new Dictionary(); @@ -61,9 +61,8 @@ namespace Microsoft.AspNetCore.Components.Rendering throw new ArgumentNullException(nameof(loggerFactory)); } - _componentFactory = new ComponentFactory(serviceProvider); + _serviceProvider = serviceProvider; _logger = loggerFactory.CreateLogger(); - _componentFactory = new ComponentFactory(serviceProvider); } /// @@ -77,7 +76,7 @@ namespace Microsoft.AspNetCore.Components.Rendering /// The type of the component to instantiate. /// The component instance. protected IComponent InstantiateComponent(Type componentType) - => _componentFactory.InstantiateComponent(componentType); + => ComponentFactory.Instance.InstantiateComponent(_serviceProvider, componentType); /// /// Associates the with the , assigning diff --git a/src/Components/Components/src/Routing/RouteEntry.cs b/src/Components/Components/src/Routing/RouteEntry.cs index d96317151b..cff9420bd9 100644 --- a/src/Components/Components/src/Routing/RouteEntry.cs +++ b/src/Components/Components/src/Routing/RouteEntry.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -26,8 +26,8 @@ namespace Microsoft.AspNetCore.Components.Routing } // Parameters will be lazily initialized. - IDictionary parameters = null; - for (int i = 0; i < Template.Segments.Length; i++) + Dictionary parameters = null; + for (var i = 0; i < Template.Segments.Length; i++) { var segment = Template.Segments[i]; var pathSegment = context.Segments[i]; @@ -39,23 +39,14 @@ namespace Microsoft.AspNetCore.Components.Routing { if (segment.IsParameter) { - GetParameters()[segment.Value] = matchedParameterValue; + parameters ??= new Dictionary(StringComparer.Ordinal); + parameters[segment.Value] = matchedParameterValue; } } } context.Parameters = parameters; context.Handler = Handler; - - IDictionary GetParameters() - { - if (parameters == null) - { - parameters = new Dictionary(); - } - - return parameters; - } } } } diff --git a/src/Components/Components/src/Routing/RouteTable.cs b/src/Components/Components/src/Routing/RouteTable.cs index 4449c00f3c..029bc47657 100644 --- a/src/Components/Components/src/Routing/RouteTable.cs +++ b/src/Components/Components/src/Routing/RouteTable.cs @@ -1,12 +1,6 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; -using System.Collections.Generic; -using System.Linq; -using System.Reflection; -using Microsoft.AspNetCore.Components; - namespace Microsoft.AspNetCore.Components.Routing { internal class RouteTable @@ -16,117 +10,13 @@ namespace Microsoft.AspNetCore.Components.Routing Routes = routes; } - public RouteEntry[] Routes { get; set; } - - public static RouteTable Create(IEnumerable types) - { - var routes = new List(); - foreach (var type in types) - { - // We're deliberately using inherit = false here. - // - // RouteAttribute is defined as non-inherited, because inheriting a route attribute always causes an - // ambiguity. You end up with two components (base class and derived class) with the same route. - var routeAttributes = type.GetCustomAttributes(inherit: false); - - foreach (var routeAttribute in routeAttributes) - { - var template = TemplateParser.ParseTemplate(routeAttribute.Template); - var entry = new RouteEntry(template, type); - routes.Add(entry); - } - } - - return new RouteTable(routes.OrderBy(id => id, RoutePrecedence).ToArray()); - } - - public static IComparer RoutePrecedence { get; } = Comparer.Create(RouteComparison); - - /// - /// Route precedence algorithm. - /// We collect all the routes and sort them from most specific to - /// less specific. The specificity of a route is given by the specificity - /// of its segments and the position of those segments in the route. - /// * A literal segment is more specific than a parameter segment. - /// * A parameter segment with more constraints is more specific than one with fewer constraints - /// * Segment earlier in the route are evaluated before segments later in the route. - /// For example: - /// /Literal is more specific than /Parameter - /// /Route/With/{parameter} is more specific than /{multiple}/With/{parameters} - /// /Product/{id:int} is more specific than /Product/{id} - /// - /// Routes can be ambiguous if: - /// They are composed of literals and those literals have the same values (case insensitive) - /// They are composed of a mix of literals and parameters, in the same relative order and the - /// literals have the same values. - /// For example: - /// * /literal and /Literal - /// /{parameter}/literal and /{something}/literal - /// /{parameter:constraint}/literal and /{something:constraint}/literal - /// - /// To calculate the precedence we sort the list of routes as follows: - /// * Shorter routes go first. - /// * A literal wins over a parameter in precedence. - /// * For literals with different values (case insensitive) we choose the lexical order - /// * For parameters with different numbers of constraints, the one with more wins - /// If we get to the end of the comparison routing we've detected an ambiguous pair of routes. - /// - internal static int RouteComparison(RouteEntry x, RouteEntry y) - { - var xTemplate = x.Template; - var yTemplate = y.Template; - if (xTemplate.Segments.Length != y.Template.Segments.Length) - { - return xTemplate.Segments.Length < y.Template.Segments.Length ? -1 : 1; - } - else - { - for (int i = 0; i < xTemplate.Segments.Length; i++) - { - var xSegment = xTemplate.Segments[i]; - var ySegment = yTemplate.Segments[i]; - if (!xSegment.IsParameter && ySegment.IsParameter) - { - return -1; - } - if (xSegment.IsParameter && !ySegment.IsParameter) - { - return 1; - } - - if (xSegment.IsParameter) - { - if (xSegment.Constraints.Length > ySegment.Constraints.Length) - { - return -1; - } - else if (xSegment.Constraints.Length < ySegment.Constraints.Length) - { - return 1; - } - } - else - { - var comparison = string.Compare(xSegment.Value, ySegment.Value, StringComparison.OrdinalIgnoreCase); - if (comparison != 0) - { - return comparison; - } - } - } - - throw new InvalidOperationException($@"The following routes are ambiguous: -'{x.Template.TemplateText}' in '{x.Handler.FullName}' -'{y.Template.TemplateText}' in '{y.Handler.FullName}' -"); - } - } + public RouteEntry[] Routes { get; } internal void Route(RouteContext routeContext) { - foreach (var route in Routes) + for (var i = 0; i < Routes.Length; i++) { - route.Match(routeContext); + Routes[i].Match(routeContext); if (routeContext.Handler != null) { return; diff --git a/src/Components/Components/src/Routing/RouteTableFactory.cs b/src/Components/Components/src/Routing/RouteTableFactory.cs new file mode 100644 index 0000000000..eae29a7530 --- /dev/null +++ b/src/Components/Components/src/Routing/RouteTableFactory.cs @@ -0,0 +1,137 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using Microsoft.AspNetCore.Components.Routing; + +namespace Microsoft.AspNetCore.Components +{ + /// + /// Resolves components for an application. + /// + internal static class RouteTableFactory + { + private static readonly ConcurrentDictionary Cache = + new ConcurrentDictionary(); + public static readonly IComparer RoutePrecedence = Comparer.Create(RouteComparison); + + public static RouteTable Create(Assembly appAssembly) + { + if (Cache.TryGetValue(appAssembly, out var resolvedComponents)) + { + return resolvedComponents; + } + + var componentTypes = appAssembly.ExportedTypes.Where(t => typeof(IComponent).IsAssignableFrom(t)); + var routeTable = Create(componentTypes); + Cache.TryAdd(appAssembly, routeTable); + return routeTable; + } + + internal static RouteTable Create(IEnumerable componentTypes) + { + var routes = new List(); + foreach (var type in componentTypes) + { + // We're deliberately using inherit = false here. + // + // RouteAttribute is defined as non-inherited, because inheriting a route attribute always causes an + // ambiguity. You end up with two components (base class and derived class) with the same route. + var routeAttributes = type.GetCustomAttributes(inherit: false); + + foreach (var routeAttribute in routeAttributes) + { + var template = TemplateParser.ParseTemplate(routeAttribute.Template); + var entry = new RouteEntry(template, type); + routes.Add(entry); + } + } + + return new RouteTable(routes.OrderBy(id => id, RoutePrecedence).ToArray()); + } + + /// + /// Route precedence algorithm. + /// We collect all the routes and sort them from most specific to + /// less specific. The specificity of a route is given by the specificity + /// of its segments and the position of those segments in the route. + /// * A literal segment is more specific than a parameter segment. + /// * A parameter segment with more constraints is more specific than one with fewer constraints + /// * Segment earlier in the route are evaluated before segments later in the route. + /// For example: + /// /Literal is more specific than /Parameter + /// /Route/With/{parameter} is more specific than /{multiple}/With/{parameters} + /// /Product/{id:int} is more specific than /Product/{id} + /// + /// Routes can be ambiguous if: + /// They are composed of literals and those literals have the same values (case insensitive) + /// They are composed of a mix of literals and parameters, in the same relative order and the + /// literals have the same values. + /// For example: + /// * /literal and /Literal + /// /{parameter}/literal and /{something}/literal + /// /{parameter:constraint}/literal and /{something:constraint}/literal + /// + /// To calculate the precedence we sort the list of routes as follows: + /// * Shorter routes go first. + /// * A literal wins over a parameter in precedence. + /// * For literals with different values (case insensitive) we choose the lexical order + /// * For parameters with different numbers of constraints, the one with more wins + /// If we get to the end of the comparison routing we've detected an ambiguous pair of routes. + /// + internal static int RouteComparison(RouteEntry x, RouteEntry y) + { + var xTemplate = x.Template; + var yTemplate = y.Template; + if (xTemplate.Segments.Length != y.Template.Segments.Length) + { + return xTemplate.Segments.Length < y.Template.Segments.Length ? -1 : 1; + } + else + { + for (var i = 0; i < xTemplate.Segments.Length; i++) + { + var xSegment = xTemplate.Segments[i]; + var ySegment = yTemplate.Segments[i]; + if (!xSegment.IsParameter && ySegment.IsParameter) + { + return -1; + } + if (xSegment.IsParameter && !ySegment.IsParameter) + { + return 1; + } + + if (xSegment.IsParameter) + { + if (xSegment.Constraints.Length > ySegment.Constraints.Length) + { + return -1; + } + else if (xSegment.Constraints.Length < ySegment.Constraints.Length) + { + return 1; + } + } + else + { + var comparison = string.Compare(xSegment.Value, ySegment.Value, StringComparison.OrdinalIgnoreCase); + if (comparison != 0) + { + return comparison; + } + } + } + + throw new InvalidOperationException($@"The following routes are ambiguous: +'{x.Template.TemplateText}' in '{x.Handler.FullName}' +'{y.Template.TemplateText}' in '{y.Handler.FullName}' +"); + } + } + } +} diff --git a/src/Components/Components/src/Routing/RouteTemplate.cs b/src/Components/Components/src/Routing/RouteTemplate.cs index 1856ef20d2..bb59be07ee 100644 --- a/src/Components/Components/src/Routing/RouteTemplate.cs +++ b/src/Components/Components/src/Routing/RouteTemplate.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. @@ -6,11 +6,9 @@ namespace Microsoft.AspNetCore.Components.Routing { internal class RouteTemplate { - public static readonly char[] Separators = new[] { '/' }; - - public RouteTemplate(string TemplateText, TemplateSegment[] segments) + public RouteTemplate(string templateText, TemplateSegment[] segments) { - this.TemplateText = TemplateText; + TemplateText = templateText; Segments = segments; } diff --git a/src/Components/Components/src/Routing/Router.cs b/src/Components/Components/src/Routing/Router.cs index 87a66a692a..5f26e5dc7e 100644 --- a/src/Components/Components/src/Routing/Router.cs +++ b/src/Components/Components/src/Routing/Router.cs @@ -70,8 +70,7 @@ namespace Microsoft.AspNetCore.Components.Routing public Task SetParametersAsync(ParameterCollection parameters) { parameters.SetParameterProperties(this); - var types = ComponentResolver.ResolveComponents(AppAssembly); - Routes = RouteTable.Create(types); + Routes = RouteTableFactory.Create(AppAssembly); Refresh(isNavigationIntercepted: false); return Task.CompletedTask; } diff --git a/src/Components/Components/test/ComponentFactoryTest.cs b/src/Components/Components/test/ComponentFactoryTest.cs new file mode 100644 index 0000000000..d0ec7a44d9 --- /dev/null +++ b/src/Components/Components/test/ComponentFactoryTest.cs @@ -0,0 +1,168 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Text; +using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace Microsoft.AspNetCore.Components +{ + public class ComponentFactoryTest + { + [Fact] + public void InstantiateComponent_CreatesInstance() + { + // Arrange + var componentType = typeof(EmptyComponent); + var factory = new ComponentFactory(); + + // Act + var instance = factory.InstantiateComponent(GetServiceProvider(), componentType); + + // Assert + Assert.NotNull(instance); + Assert.IsType(instance); + } + + [Fact] + public void InstantiateComponent_AssignsPropertiesWithInjectAttribute() + { + // Arrange + var componentType = typeof(ComponentWithInjectProperties); + var factory = new ComponentFactory(); + + // Act + var instance = factory.InstantiateComponent(GetServiceProvider(), componentType); + + // Assert + Assert.NotNull(instance); + var component = Assert.IsType(instance); + // Public, and non-public properties, and properties with non-public setters should get assigned + Assert.NotNull(component.Property1); + Assert.NotNull(component.GetProperty2()); + Assert.NotNull(component.Property3); + Assert.NotNull(component.Property4); + } + + [Fact] + public void InstantiateComponent_AssignsPropertiesWithInjectAttributeOnBaseType() + { + // Arrange + var componentType = typeof(DerivedComponent); + var factory = new ComponentFactory(); + + // Act + var instance = factory.InstantiateComponent(GetServiceProvider(), componentType); + + // Assert + Assert.NotNull(instance); + var component = Assert.IsType(instance); + Assert.NotNull(component.Property1); + Assert.NotNull(component.GetProperty2()); + Assert.NotNull(component.Property3); + + // Property on derived type without [Inject] should not be assigned + Assert.Null(component.Property4); + // Property on the base type with the [Inject] attribute should + Assert.NotNull(((ComponentWithInjectProperties)component).Property4); + } + + [Fact] + public void InstantiateComponent_IgnoresPropertiesWithoutInjectAttribute() + { + // Arrange + var componentType = typeof(ComponentWithNonInjectableProperties); + var factory = new ComponentFactory(); + + // Act + var instance = factory.InstantiateComponent(GetServiceProvider(), componentType); + + // Assert + Assert.NotNull(instance); + var component = Assert.IsType(instance); + // Public, and non-public properties, and properties with non-public setters should get assigned + Assert.NotNull(component.Property1); + Assert.Null(component.Property2); + } + + private static IServiceProvider GetServiceProvider() + { + return new ServiceCollection() + .AddTransient() + .AddTransient() + .BuildServiceProvider(); + } + + private class EmptyComponent : IComponent + { + public void Configure(RenderHandle renderHandle) + { + throw new NotImplementedException(); + } + + public Task SetParametersAsync(ParameterCollection parameters) + { + throw new NotImplementedException(); + } + } + + private class ComponentWithInjectProperties : IComponent + { + [Inject] + public TestService1 Property1 { get; set; } + + [Inject] + private TestService2 Property2 { get; set; } + + [Inject] + public TestService1 Property3 { get; private set; } + + [Inject] + public TestService1 Property4 { get; set; } + + public TestService2 GetProperty2() => Property2; + + public void Configure(RenderHandle renderHandle) + { + throw new NotImplementedException(); + } + + public Task SetParametersAsync(ParameterCollection parameters) + { + throw new NotImplementedException(); + } + } + + private class ComponentWithNonInjectableProperties : IComponent + { + [Inject] + public TestService1 Property1 { get; set; } + + public TestService1 Property2 { get; set; } + + public void Configure(RenderHandle renderHandle) + { + throw new NotImplementedException(); + } + + public Task SetParametersAsync(ParameterCollection parameters) + { + throw new NotImplementedException(); + } + } + + private class DerivedComponent : ComponentWithInjectProperties + { + public new TestService2 Property4 { get; set; } + + [Inject] + public TestService2 Property5 { get; set; } + } + + public class TestService1 { } + public class TestService2 { } + } +} diff --git a/src/Components/Components/test/Routing/RouteTableTests.cs b/src/Components/Components/test/Routing/RouteTableFactoryTests.cs similarity index 97% rename from src/Components/Components/test/Routing/RouteTableTests.cs rename to src/Components/Components/test/Routing/RouteTableFactoryTests.cs index c2e34d4298..f6733d8ceb 100644 --- a/src/Components/Components/test/Routing/RouteTableTests.cs +++ b/src/Components/Components/test/Routing/RouteTableFactoryTests.cs @@ -9,13 +9,13 @@ using Xunit; namespace Microsoft.AspNetCore.Components.Test.Routing { - public class RouteTableTests + public class RouteTableFactoryTests { [Fact] public void CanDiscoverRoute() { // Arrange & Act - var routes = RouteTable.Create(new[] { typeof(MyComponent), }); + var routes = RouteTableFactory.Create(new[] { typeof(MyComponent), }); // Assert Assert.Equal("Test1", Assert.Single(routes.Routes).Template.TemplateText); @@ -30,7 +30,7 @@ namespace Microsoft.AspNetCore.Components.Test.Routing public void CanDiscoverRoutes_WithInheritance() { // Arrange & Act - var routes = RouteTable.Create(new[] { typeof(MyComponent), typeof(MyInheritedComponent), }); + var routes = RouteTableFactory.Create(new[] { typeof(MyComponent), typeof(MyInheritedComponent), }); // Assert Assert.Collection( @@ -363,7 +363,7 @@ namespace Microsoft.AspNetCore.Components.Test.Routing { return new RouteTable(_routeTemplates .Select(rt => new RouteEntry(TemplateParser.ParseTemplate(rt.Item1), rt.Item2)) - .OrderBy(id => id, RouteTable.RoutePrecedence) + .OrderBy(id => id, RouteTableFactory.RoutePrecedence) .ToArray()); } catch (InvalidOperationException ex) when (ex.InnerException is InvalidOperationException) diff --git a/src/Components/Components/test/Routing/TemplateParserTests.cs b/src/Components/Components/test/Routing/TemplateParserTests.cs index cd5951bb44..752963f52d 100644 --- a/src/Components/Components/test/Routing/TemplateParserTests.cs +++ b/src/Components/Components/test/Routing/TemplateParserTests.cs @@ -159,22 +159,12 @@ namespace Microsoft.AspNetCore.Components.Routing public bool Equals(RouteTemplate x, RouteTemplate y) { - if (x == null && y == null) - { - return true; - } - - if ((x == null) != (y == null)) - { - return false; - } - if (x.Segments.Length != y.Segments.Length) { return false; } - for (int i = 0; i < x.Segments.Length; i++) + for (var i = 0; i < x.Segments.Length; i++) { var xSegment = x.Segments[i]; var ySegment = y.Segments[i]; diff --git a/src/Components/test/E2ETest/Tests/RoutingTest.cs b/src/Components/test/E2ETest/Tests/RoutingTest.cs index 09e1816205..ce0dc725e6 100644 --- a/src/Components/test/E2ETest/Tests/RoutingTest.cs +++ b/src/Components/test/E2ETest/Tests/RoutingTest.cs @@ -4,7 +4,6 @@ using System; using System.Linq; using System.Runtime.InteropServices; -using System.Threading.Tasks; using BasicTestApp; using BasicTestApp.RouterTest; using Microsoft.AspNetCore.Components.E2ETest.Infrastructure; @@ -389,6 +388,27 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Tests Browser.Equal(initialUrl, () => app.FindElement(By.Id("test-info")).Text); } + [Fact] + public void CanArriveAtRouteWithExtension() + { + // This is an odd test, but it's primarily here to verify routing for routeablecomponentfrompackage isn't available due to + // some unknown reason + SetUrlViaPushState("/Default.html"); + + var app = MountTestComponent(); + Assert.Equal("This is the default page.", app.FindElement(By.Id("test-info")).Text); + AssertHighlightedLinks("With extension"); + } + + [Fact] + public void RoutingToComponentOutsideMainAppDoesNotWork() + { + SetUrlViaPushState("/routeablecomponentfrompackage.html"); + + var app = MountTestComponent(); + Assert.Equal("Oops, that component wasn't found!", app.FindElement(By.Id("test-info")).Text); + } + private string SetUrlViaPushState(string relativeUri) { var pathBaseWithoutHash = ServerPathBase.Split('#')[0]; diff --git a/src/Components/test/testassets/BasicTestApp/RouterTest/Default.razor b/src/Components/test/testassets/BasicTestApp/RouterTest/Default.razor index 1417993522..09c74d19ec 100644 --- a/src/Components/test/testassets/BasicTestApp/RouterTest/Default.razor +++ b/src/Components/test/testassets/BasicTestApp/RouterTest/Default.razor @@ -1,3 +1,4 @@ @page "/" +@page "/Default.html"
This is the default page.
diff --git a/src/Components/test/testassets/BasicTestApp/RouterTest/Links.razor b/src/Components/test/testassets/BasicTestApp/RouterTest/Links.razor index fdedf1852f..64811b666b 100644 --- a/src/Components/test/testassets/BasicTestApp/RouterTest/Links.razor +++ b/src/Components/test/testassets/BasicTestApp/RouterTest/Links.razor @@ -1,11 +1,17 @@ @using Microsoft.AspNetCore.Components.Routing @inject Microsoft.AspNetCore.Components.IUriHelper uriHelper - +
  • Default (matches all)
  • Default with base-relative URL (matches all)
  • Default with query
  • Default with hash
  • +
  • With extension
  • Other
  • Other with base-relative URL (matches all)
  • Other with query
  • @@ -29,4 +35,5 @@ Target (_blank) Not a component +Cannot route to me diff --git a/src/Components/test/testassets/TestContentPackage/RouteableComponentFromPackage.razor b/src/Components/test/testassets/TestContentPackage/RouteableComponentFromPackage.razor new file mode 100644 index 0000000000..1c5dde9e16 --- /dev/null +++ b/src/Components/test/testassets/TestContentPackage/RouteableComponentFromPackage.razor @@ -0,0 +1,16 @@ +@page "/routeablecomponentfrompackage.html" + +
    + This component, including the CSS and image required to produce its + elegant styling, is in an external NuGet package. + +
    + +@code { + string buttonLabel = "Click me"; + + void ChangeLabel() + { + buttonLabel = "It works"; + } +} diff --git a/src/Components/test/testassets/TestServer/Startup.cs b/src/Components/test/testassets/TestServer/Startup.cs index cecc31e7f5..8df9d439d7 100644 --- a/src/Components/test/testassets/TestServer/Startup.cs +++ b/src/Components/test/testassets/TestServer/Startup.cs @@ -3,6 +3,7 @@ using BasicTestApp; using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting;