From a7301120b1971d6d047c63db5e6911aa134f99a2 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Mon, 20 Aug 2018 18:51:16 -0700 Subject: [PATCH] Unwrap filter factories in TypeFilterAttribute & ServiceFilterAttribute Fixes #7855 --- .../ServiceFilterAttribute.cs | 12 +- .../TypeFilterAttribute.cs | 10 +- .../ServiceFilterAttributeTest.cs | 62 ++++++++++ .../TypeFilterAttributeTest.cs | 116 ++++++++++++++++++ 4 files changed, 190 insertions(+), 10 deletions(-) create mode 100644 test/Microsoft.AspNetCore.Mvc.Core.Test/ServiceFilterAttributeTest.cs create mode 100644 test/Microsoft.AspNetCore.Mvc.Core.Test/TypeFilterAttributeTest.cs diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ServiceFilterAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/ServiceFilterAttribute.cs index 525d1a82bb..9e16cb4715 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ServiceFilterAttribute.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ServiceFilterAttribute.cs @@ -3,7 +3,6 @@ using System; using System.Diagnostics; -using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.Extensions.DependencyInjection; @@ -58,14 +57,11 @@ namespace Microsoft.AspNetCore.Mvc throw new ArgumentNullException(nameof(serviceProvider)); } - var service = serviceProvider.GetRequiredService(ServiceType); - - var filter = service as IFilterMetadata; - if (filter == null) + var filter = (IFilterMetadata)serviceProvider.GetRequiredService(ServiceType); + if (filter is IFilterFactory filterFactory) { - throw new InvalidOperationException(Resources.FormatFilterFactoryAttribute_TypeMustImplementIFilter( - typeof(ServiceFilterAttribute).Name, - typeof(IFilterMetadata).Name)); + // Unwrap filter factories + filter = filterFactory.CreateInstance(serviceProvider); } return filter; diff --git a/src/Microsoft.AspNetCore.Mvc.Core/TypeFilterAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/TypeFilterAttribute.cs index bc5c19802f..a2048512a6 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/TypeFilterAttribute.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/TypeFilterAttribute.cs @@ -73,11 +73,17 @@ namespace Microsoft.AspNetCore.Mvc if (_factory == null) { var argumentTypes = Arguments?.Select(a => a.GetType())?.ToArray(); - _factory = ActivatorUtilities.CreateFactory(ImplementationType, argumentTypes ?? Type.EmptyTypes); } - return (IFilterMetadata)_factory(serviceProvider, Arguments); + var filter = (IFilterMetadata)_factory(serviceProvider, Arguments); + if (filter is IFilterFactory filterFactory) + { + // Unwrap filter factories + filter = filterFactory.CreateInstance(serviceProvider); + } + + return filter; } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ServiceFilterAttributeTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ServiceFilterAttributeTest.cs new file mode 100644 index 0000000000..e21e55018c --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ServiceFilterAttributeTest.cs @@ -0,0 +1,62 @@ +// 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 Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc +{ + public class ServiceFilterAttributeTest + { + [Fact] + public void CreateService_GetsFilterFromServiceProvider() + { + // Arrange + var expected = new TestFilter(); + var serviceProvider = new ServiceCollection() + .AddSingleton(expected) + .BuildServiceProvider(); + + var serviceFilter = new ServiceFilterAttribute(typeof(TestFilter)); + + // Act + var filter = serviceFilter.CreateInstance(serviceProvider); + + // Assert + Assert.Same(expected, filter); + } + + [Fact] + public void CreateService_UnwrapsFilterFactory() + { + // Arrange + var serviceProvider = new ServiceCollection() + .AddSingleton(new TestFilterFactory()) + .BuildServiceProvider(); + + var serviceFilter = new ServiceFilterAttribute(typeof(TestFilterFactory)); + + // Act + var filter = serviceFilter.CreateInstance(serviceProvider); + + // Assert + Assert.IsType(filter); + } + + public class TestFilter : IFilterMetadata + { + } + + public class TestFilterFactory : IFilterFactory + { + public bool IsReusable => throw new NotImplementedException(); + + public IFilterMetadata CreateInstance(IServiceProvider serviceProvider) + { + return new TestFilter(); + } + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/TypeFilterAttributeTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/TypeFilterAttributeTest.cs new file mode 100644 index 0000000000..27d407b837 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/TypeFilterAttributeTest.cs @@ -0,0 +1,116 @@ +// 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 Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc +{ + public class TypeFilterAttributeTest + { + [Fact] + public void CreateService_TypeActivatesImplementationType() + { + // Arrange + var value = "Some value"; + var uri = new Uri("http://www.asp.net"); + var serviceProvider = new ServiceCollection() + .AddSingleton(value) + .AddSingleton(uri) + .BuildServiceProvider(); + + var typeFilter = new TypeFilterAttribute(typeof(TestFilter)); + + // Act + var filter = typeFilter.CreateInstance(serviceProvider); + + // Assert + var testFilter = Assert.IsType(filter); + Assert.Same(value, testFilter.Value); + Assert.Same(uri, testFilter.Uri); + } + + [Fact] + public void CreateService_UsesArguments() + { + // Arrange + var value = "Some value"; + var uri = new Uri("http://www.asp.net"); + var serviceProvider = new ServiceCollection() + .AddSingleton("Value in DI") + .AddSingleton(uri) + .BuildServiceProvider(); + + var typeFilter = new TypeFilterAttribute(typeof(TestFilter)) + { + Arguments = new[] { value, } + }; + + // Act + var filter = typeFilter.CreateInstance(serviceProvider); + + // Assert + var testFilter = Assert.IsType(filter); + Assert.Same(value, testFilter.Value); + Assert.Same(uri, testFilter.Uri); + } + + [Fact] + public void CreateService_UnwrapsFilterFactory() + { + // Arrange + var value = "Some value"; + var uri = new Uri("http://www.asp.net"); + var serviceProvider = new ServiceCollection() + .AddSingleton("Value in DI") + .AddSingleton(uri) + .BuildServiceProvider(); + + var typeFilter = new TypeFilterAttribute(typeof(TestFilterFactory)) + { + Arguments = new[] { value, } + }; + + // Act + var filter = typeFilter.CreateInstance(serviceProvider); + + // Assert + var testFilter = Assert.IsType(filter); + Assert.Same(value, testFilter.Value); + Assert.Same(uri, testFilter.Uri); + } + + public class TestFilter : IFilterMetadata + { + public TestFilter(string value, Uri uri) + { + Value = value; + Uri = uri; + } + + public string Value { get; } + public Uri Uri { get; } + } + + public class TestFilterFactory : IFilterFactory + { + private readonly string _value; + private readonly Uri _uri; + + public TestFilterFactory(string value, Uri uri) + { + _value = value; + _uri = uri; + } + + public bool IsReusable => throw new NotImplementedException(); + + public IFilterMetadata CreateInstance(IServiceProvider serviceProvider) + { + return new TestFilter(_value, _uri); + } + } + } +}