huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Remove FormatFilter dependency on IActionContextAccessor 

This change simplifies IFormatFilter's API and removes the dependency on
IActionContext accessor.

The old API for IFormatFilter required computing state based on the
current request as part of the constructor, which in turn implied the use
of a context accessor. This isn't really needed. I didn't preserve caching of
the 'format' as that seems like an early optimization.
This commit is contained in:
Ryan Nowak 2015-11-18 18:31:50 -08:00
parent d8cc2b85d5
commit a14fdd8637
6 changed files with 69 additions and 98 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/Microsoft.AspNet.Mvc.Core/DependencyInjection/MvcCoreMvcCoreBuilderExtensions.cs
View File

@ -62,7 +62,7 @@ namespace Microsoft.Extensions.DependencyInjection
// Internal for testing.
internal static void AddFormatterMappingsServices(IServiceCollection services)
{
services.TryAddTransient<FormatFilter, FormatFilter>();
services.TryAddSingleton<FormatFilter, FormatFilter>();
}
public static IMvcCoreBuilder AddAuthorization(this IMvcCoreBuilder builder)

84
src/Microsoft.AspNet.Mvc.Core/Formatters/FormatFilter.cs
View File

@ -6,7 +6,6 @@ using System.Collections.Generic;
using System.Linq;
using Microsoft.AspNet.Mvc.ApiExplorer;
using Microsoft.AspNet.Mvc.Filters;
using Microsoft.AspNet.Mvc.Infrastructure;
using Microsoft.Extensions.OptionsModel;
using Microsoft.Net.Http.Headers;
@ -18,39 +17,36 @@ namespace Microsoft.AspNet.Mvc.Formatters
/// </summary>
public class FormatFilter : IFormatFilter, IResourceFilter, IResultFilter
{
private readonly MvcOptions _options;
/// <summary>
/// Initializes an instance of <see cref="FormatFilter"/>.
/// </summary>
/// <param name="options">The <see cref="IOptions{MvcOptions}"/></param>
/// <param name="actionContextAccessor">The <see cref="IActionContextAccessor"/></param>
public FormatFilter(IOptions<MvcOptions> options, IActionContextAccessor actionContextAccessor)
public FormatFilter(IOptions<MvcOptions> options)
{
IsActive = true;
Format = GetFormat(actionContextAccessor.ActionContext);
if (string.IsNullOrEmpty(Format))
{
IsActive = false;
return;
}
ContentType = options.Value.FormatterMappings.GetMediaTypeMappingForFormat(Format);
_options = options.Value;
}
/// <summary>
/// Format value in the current request. <c>null</c> if format not present in the current request.
/// </summary>
public string Format { get; }
/// <inheritdoc />
public string GetFormat(ActionContext context)
{
object obj;
if (context.RouteData.Values.TryGetValue("format", out obj))
{
// null and string.Empty are equivalent for route values.
var routeValue = obj?.ToString();
return string.IsNullOrEmpty(routeValue) ? null : routeValue;
}
/// <summary>
/// <see cref="MediaTypeHeaderValue"/> for the format value in the current request.
/// </summary>
public MediaTypeHeaderValue ContentType { get; }
var query = context.HttpContext.Request.Query["format"];
if (query.Count > 0)
{
return query.ToString();
}
/// <summary>
/// <c>true</c> if the current <see cref="FormatFilter"/> is active and will execute.
/// </summary>
public bool IsActive { get; }
return null;
}
/// <summary>
/// As a <see cref="IResourceFilter"/>, this filter looks at the request and rejects it before going ahead if
@ -65,13 +61,15 @@ namespace Microsoft.AspNet.Mvc.Formatters
throw new ArgumentNullException(nameof(context));
}
if (!IsActive)
var format = GetFormat(context);
if (format == null)
{
// no format specified by user, so the filter is muted
return;
}
if (ContentType == null)
var contentType = _options.FormatterMappings.GetMediaTypeMappingForFormat(format);
if (contentType == null)
{
// no contentType exists for the format, return 404
context.Result = new HttpNotFoundResult();
@ -93,7 +91,7 @@ namespace Microsoft.AspNet.Mvc.Formatters
// request's format and IApiResponseMetadataProvider-provided content types similarly to an Accept
// header and an output formatter's SupportedMediaTypes: Confirm action supports a more specific media
// type than requested e.g. OK if "text/*" requested and action supports "text/plain".
if (!supportedMediaTypes.Any(contentType => contentType.IsSubsetOf(ContentType)))
if (!supportedMediaTypes.Any(c => c.IsSubsetOf(contentType)))
{
context.Result = new HttpNotFoundResult();
}
@ -116,43 +114,25 @@ namespace Microsoft.AspNet.Mvc.Formatters
throw new ArgumentNullException(nameof(context));
}
if (!IsActive)
var format = GetFormat(context);
if (format == null)
{
return; // no format specified by user, so the filter is muted
// no format specified by user, so the filter is muted
return;
}
var objectResult = context.Result as ObjectResult;
if (objectResult != null)
{
var contentType = _options.FormatterMappings.GetMediaTypeMappingForFormat(format);
objectResult.ContentTypes.Clear();
objectResult.ContentTypes.Add(ContentType);
objectResult.ContentTypes.Add(contentType);
}
}
/// <inheritdoc />
public void OnResultExecuted(ResultExecutedContext context)
{
if (context == null)
{
throw new ArgumentNullException(nameof(context));
}
}
private string GetFormat(ActionContext context)
{
object format = null;
if (!context.RouteData.Values.TryGetValue("format", out format))
{
format = context.HttpContext.Request.Query["format"];
}
if (format != null)
{
return format.ToString();
}
return null;
}
}
}

17
src/Microsoft.AspNet.Mvc.Core/Formatters/IFormatFilter.cs
View File

@ -2,7 +2,6 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using Microsoft.AspNet.Mvc.Filters;
using Microsoft.Net.Http.Headers;
namespace Microsoft.AspNet.Mvc.Formatters
{
@ -12,18 +11,10 @@ namespace Microsoft.AspNet.Mvc.Formatters
public interface IFormatFilter : IFilterMetadata
{
/// <summary>
/// format value in the current request. <c>null</c> if format not present in the current request.
/// Gets the format value for the request associated with the provided <see cref="ActionContext"/>.
/// </summary>
string Format { get; }
/// <summary>
/// <see cref="MediaTypeHeaderValue"/> for the format value in the current request.
/// </summary>
MediaTypeHeaderValue ContentType { get; }
/// <summary>
/// <c>true</c> if the current <see cref="FormatFilter"/> is active and will execute.
/// </summary>
bool IsActive { get; }
/// <param name="context">The <see cref="ActionContext"/> associated with the current request.</param>
/// <returns>A format value, or <c>null</c> if a format cannot be determined for the request.</returns>
string GetFormat(ActionContext context);
}
}

2
src/Microsoft.AspNet.Mvc.Core/ProducesAttribute.cs
View File

@ -67,7 +67,7 @@ namespace Microsoft.AspNet.Mvc
{
// Check if there are any IFormatFilter in the pipeline, and if any of them is active. If there is one,
// do not override the content type value.
if (context.Filters.OfType<IFormatFilter>().All(f => !f.IsActive))
if (context.Filters.OfType<IFormatFilter>().All(f => f.GetFormat(context) == null))
{
SetContentTypes(objectResult.ContentTypes);
}

52
test/Microsoft.AspNet.Mvc.Core.Test/Formatters/FormatFilterTest.cs
View File

@ -38,7 +38,7 @@ namespace Microsoft.AspNet.Mvc.Formatters
var resultExecutingContext = mockObjects.CreateResultExecutingContext();
var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilterMetadata[] { });
var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor);
var filter = new FormatFilter(mockObjects.OptionsManager);
// Act
filter.OnResourceExecuting(resourceExecutingContext);
@ -85,7 +85,7 @@ namespace Microsoft.AspNet.Mvc.Formatters
ac,
new IFilterMetadata[] { });
var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor);
var filter = new FormatFilter(mockObjects.OptionsManager);
// Act
filter.OnResourceExecuting(resourceExecutingContext);
@ -117,7 +117,7 @@ namespace Microsoft.AspNet.Mvc.Formatters
format,
MediaTypeHeaderValue.Parse(contentType));
var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor);
var filter = new FormatFilter(mockObjects.OptionsManager);
// Act
filter.OnResourceExecuting(resourceExecutingContext);
@ -140,7 +140,7 @@ namespace Microsoft.AspNet.Mvc.Formatters
var mockObjects = new MockObjects(format, place);
var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilterMetadata[] { });
var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor);
var filter = new FormatFilter(mockObjects.OptionsManager);
// Act
filter.OnResourceExecuting(resourceExecutingContext);
@ -157,7 +157,7 @@ namespace Microsoft.AspNet.Mvc.Formatters
var mockObjects = new MockObjects();
var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilterMetadata[] { });
var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor);
var filter = new FormatFilter(mockObjects.OptionsManager);
// Act
filter.OnResourceExecuting(resourceExecutingContext);
@ -179,7 +179,7 @@ namespace Microsoft.AspNet.Mvc.Formatters
var mockObjects = new MockObjects(format, place);
var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilterMetadata[] { produces });
var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor);
var filter = new FormatFilter(mockObjects.OptionsManager);
// Act
filter.OnResourceExecuting(resourceExecutingContext);
@ -200,7 +200,7 @@ namespace Microsoft.AspNet.Mvc.Formatters
"xml",
MediaTypeHeaderValue.Parse("application/xml"));
var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor);
var filter = new FormatFilter(mockObjects.OptionsManager);
// Act
filter.OnResourceExecuting(resourceExecutingContext);
@ -221,7 +221,7 @@ namespace Microsoft.AspNet.Mvc.Formatters
"xml",
MediaTypeHeaderValue.Parse("application/xml;version=1"));
var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor);
var filter = new FormatFilter(mockObjects.OptionsManager);
// Act
filter.OnResourceExecuting(resourceExecutingContext);
@ -247,7 +247,7 @@ namespace Microsoft.AspNet.Mvc.Formatters
"xml",
MediaTypeHeaderValue.Parse("application/xml"));
var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor);
var filter = new FormatFilter(mockObjects.OptionsManager);
// Act
filter.OnResourceExecuting(resourceExecutingContext);
@ -266,7 +266,7 @@ namespace Microsoft.AspNet.Mvc.Formatters
// Arrange
var mockObjects = new MockObjects(format, place);
var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilterMetadata[] { });
var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor);
var filter = new FormatFilter(mockObjects.OptionsManager);
// Act
filter.OnResourceExecuting(resourceExecutingContext);
@ -276,23 +276,26 @@ namespace Microsoft.AspNet.Mvc.Formatters
}
[Theory]
[InlineData("json", FormatSource.RouteData, true)]
[InlineData("json", FormatSource.QueryData, true)]
[InlineData("", FormatSource.RouteAndQueryData, false)]
[InlineData(null, FormatSource.RouteAndQueryData, false)]
public void FormatFilter_IsActive(
string format,
[InlineData("json", FormatSource.RouteData, "json")]
[InlineData("json", FormatSource.QueryData, "json")]
[InlineData("", FormatSource.RouteAndQueryData, null)]
[InlineData(null, FormatSource.RouteAndQueryData, null)]
public void FormatFilter_GetFormat(
string input,
FormatSource place,
bool expected)
string expected)
{
// Arrange
var mockObjects = new MockObjects(format, place);
var resultExecutingContext = mockObjects.CreateResultExecutingContext();
var mockObjects = new MockObjects(input, place);
var context = mockObjects.CreateResultExecutingContext();
var filterAttribute = new FormatFilterAttribute();
var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor);
var filter = new FormatFilter(mockObjects.OptionsManager);
// Act and Assert
Assert.Equal(expected, filter.IsActive);
// Act
var format = filter.GetFormat(context);
// Assert
Assert.Equal(expected, filter.GetFormat(context));
}
private static void AssertMediaTypesEqual(
@ -317,7 +320,6 @@ namespace Microsoft.AspNet.Mvc.Formatters
public HttpContext MockHttpContext { get; private set; }
public ActionContext MockActionContext { get; private set; }
public IActionContextAccessor ActionContextAccessor { get; private set; }
public IOptions<MvcOptions> OptionsManager { get; private set; }
public MockObjects(string format = null, FormatSource? place = null)
@ -394,10 +396,6 @@ namespace Microsoft.AspNet.Mvc.Formatters
// Setup MVC services on mock service provider
MockActionContext = CreateMockActionContext(httpContext, format, place);
ActionContextAccessor = new ActionContextAccessor()
{
ActionContext = MockActionContext,
};
}
}
}

10
test/Microsoft.AspNet.Mvc.Core.Test/ProducesAttributeTests.cs
View File

@ -45,8 +45,9 @@ namespace Microsoft.AspNet.Mvc.Test
var producesContentAttribute = new ProducesAttribute("application/xml");
var formatFilter = new Mock<IFormatFilter>();
formatFilter.Setup(f => f.IsActive)
.Returns(false);
formatFilter
.Setup(f => f.GetFormat(It.IsAny<ActionContext>()))
.Returns((string)null);
var filters = new IFilterMetadata[] { producesContentAttribute, formatFilter.Object };
var resultExecutingContext = CreateResultExecutingContext(filters);
@ -69,8 +70,9 @@ namespace Microsoft.AspNet.Mvc.Test
var producesContentAttribute = new ProducesAttribute("application/xml");
var formatFilter = new Mock<IFormatFilter>();
formatFilter.Setup(f => f.IsActive)
.Returns(true);
formatFilter
.Setup(f => f.GetFormat(It.IsAny<ActionContext>()))
.Returns("xml");
var filters = new IFilterMetadata[] { producesContentAttribute, formatFilter.Object };
var resultExecutingContext = CreateResultExecutingContext(filters);