Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Removing IInputFormatterSelector #2353

Merged
merged 1 commit into from
Apr 8, 2015
+119 −146
Merged

Removing IInputFormatterSelector #2353

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 0 additions & 26 deletions src/Microsoft.AspNet.Mvc.Core/Formatters/DefaultInputFormatterSelector.cs
View file
Open in desktop

This file was deleted.

15 changes: 0 additions & 15 deletions src/Microsoft.AspNet.Mvc.Core/Formatters/IInputFormatterSelector.cs
View file
Open in desktop

This file was deleted.

24 changes: 5 additions & 19 deletions src/Microsoft.AspNet.Mvc.Core/ModelBinders/BodyModelBinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,11 @@ protected async override Task<ModelBindingResult> BindModelCoreAsync([NotNull] M
{
var requestServices = bindingContext.OperationBindingContext.HttpContext.RequestServices;

var formatterSelector = requestServices.GetRequiredService<IInputFormatterSelector>();
var actionContext = requestServices.GetRequiredService<IScopedInstance<ActionContext>>().Value;
var formatters = requestServices.GetRequiredService<IScopedInstance<ActionBindingContext>>().Value.InputFormatters;

var formatterContext = new InputFormatterContext(actionContext, bindingContext.ModelType);
var formatter = formatterSelector.SelectFormatter(formatters.ToList(), formatterContext);
var formatter = formatters.FirstOrDefault(f => f.CanRead(formatterContext));

if (formatter == null)
{
Expand All @@ -48,34 +47,21 @@ protected async override Task<ModelBindingResult> BindModelCoreAsync([NotNull] M
return new ModelBindingResult(model: null, key: bindingContext.ModelName, isModelSet: false);
}

object model = null;
try
{
model = await formatter.ReadAsync(formatterContext);
var model = await formatter.ReadAsync(formatterContext);

// key is empty to ensure that the model name is not used as a prefix for validation.
return new ModelBindingResult(model, key: string.Empty, isModelSet: true);
}
catch (Exception ex)
{
model = GetDefaultValueForType(bindingContext.ModelType);
bindingContext.ModelState.AddModelError(bindingContext.ModelName, ex);

// This model binder is the only handler for the Body binding source.
// Always tell the model binding system to skip other model binders i.e. return non-null.
return new ModelBindingResult(model: null, key: bindingContext.ModelName, isModelSet: false);
Copy link
Member

Choose a reason for hiding this comment

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

This was my bad. I think the first parameter here was supposed to be assigned the default value for type model calculated above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That said, it seems like fixing it specifically for BodyModelBinder is incorrect and we should have a single top level piece of code that addresses this, not individual binders.

Copy link
Member

Choose a reason for hiding this comment

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

👍

}

// Success
// key is empty to ensure that the model name is not used as a prefix for validation.
return new ModelBindingResult(model, key: string.Empty, isModelSet: true);
}

private object GetDefaultValueForType(Type modelType)
{
if (modelType.GetTypeInfo().IsValueType)
{
return Activator.CreateInstance(modelType);
}

return null;
}
}
}
1 change: 0 additions & 1 deletion src/Microsoft.AspNet.Mvc/MvcServices.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ public static IServiceCollection GetDefaultServices()
return new DefaultCompositeMetadataDetailsProvider(options.ModelMetadataDetailsProviders);
});

services.AddTransient<IInputFormatterSelector, DefaultInputFormatterSelector>();
services.AddInstance(new JsonOutputFormatter());

// Razor, Views and runtime compilation
Expand Down
67 changes: 0 additions & 67 deletions test/Microsoft.AspNet.Mvc.Core.Test/DefaultInputFormatterSelectorTests.cs
View file
Open in desktop

This file was deleted.

83 changes: 65 additions & 18 deletions ...Net.Mvc.Core.Test/BodyModelBinderTests.cs → ...Test/ModelBinding/BodyModelBinderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,18 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Microsoft.AspNet.Http;
using Microsoft.AspNet.Http.Core;
using Microsoft.AspNet.Mvc.ModelBinding;
using Microsoft.AspNet.Routing;
using Microsoft.Framework.DependencyInjection;
using Microsoft.Net.Http.Headers;
using Moq;
using Xunit;

namespace Microsoft.AspNet.Mvc
namespace Microsoft.AspNet.Mvc.ModelBinding
{
public class BodyModelBinderTests
{
Expand All @@ -24,6 +24,9 @@ public async Task BindModel_CallsSelectedInputFormatterOnce()
{
// Arrange
var mockInputFormatter = new Mock<IInputFormatter>();
mockInputFormatter.Setup(f => f.CanRead(It.IsAny<InputFormatterContext>()))
.Returns(true)
.Verifiable();
mockInputFormatter.Setup(o => o.ReadAsync(It.IsAny<InputFormatterContext>()))
.Returns(Task.FromResult<object>(new Person()))
.Verifiable();
Expand All @@ -32,14 +35,18 @@ public async Task BindModel_CallsSelectedInputFormatterOnce()
var provider = new TestModelMetadataProvider();
provider.ForType<Person>().BindingDetails(d => d.BindingSource = BindingSource.Body);

var bindingContext = GetBindingContext(typeof(Person), inputFormatter, metadataProvider: provider);
var bindingContext = GetBindingContext(
typeof(Person),
new[] { inputFormatter },
metadataProvider: provider);

var binder = new BodyModelBinder();

// Act
var binderResult = await binder.BindModelAsync(bindingContext);

// Assert
mockInputFormatter.Verify(v => v.CanRead(It.IsAny<InputFormatterContext>()), Times.Once);
mockInputFormatter.Verify(v => v.ReadAsync(It.IsAny<InputFormatterContext>()), Times.Once);
Assert.NotNull(binderResult);
Assert.True(binderResult.IsModelSet);
Expand Down Expand Up @@ -138,7 +145,7 @@ public async Task CustomFormatterDeserializationException_AddedToModelState()

var bindingContext = GetBindingContext(
typeof(Person),
inputFormatter: new XyzFormatter(),
inputFormatters: new[] { new XyzFormatter() },
httpContext: httpContext,
metadataProvider: provider);

Expand Down Expand Up @@ -170,7 +177,7 @@ public async Task NullFormatterError_AddedToModelState()

var bindingContext = GetBindingContext(
typeof(Person),
inputFormatter: null,
inputFormatters: null,
httpContext: httpContext,
metadataProvider: provider);

Expand All @@ -190,17 +197,44 @@ public async Task NullFormatterError_AddedToModelState()
Assert.Equal("Unsupported content type 'text/xyz'.", errorMessage);
}

[Fact]
public async Task BindModelCoreAsync_UsesFirstFormatterWhichCanRead()
{
// Arrange
var canReadFormatter1 = new TestInputFormatter(canRead: true);
var canReadFormatter2 = new TestInputFormatter(canRead: true);
var inputFormatters = new List<IInputFormatter>()
{
new TestInputFormatter(canRead: false),
new TestInputFormatter(canRead: false),
canReadFormatter1,
canReadFormatter2
};
var provider = new TestModelMetadataProvider();
provider.ForType<Person>().BindingDetails(d => d.BindingSource = BindingSource.Body);
var bindingContext = GetBindingContext(typeof(Person), inputFormatters, metadataProvider: provider);
var binder = bindingContext.OperationBindingContext.ModelBinder;

// Act
var binderResult = await binder.BindModelAsync(bindingContext);

// Assert
Assert.True(binderResult.IsModelSet);
Assert.Same(canReadFormatter1, binderResult.Model);
}

private static ModelBindingContext GetBindingContext(
Type modelType,
IInputFormatter inputFormatter = null,
IEnumerable<IInputFormatter> inputFormatters = null,
HttpContext httpContext = null,
IModelMetadataProvider metadataProvider = null)
{
if (httpContext == null)
{
httpContext = new DefaultHttpContext();
}
UpdateServiceProvider(httpContext, inputFormatter);

UpdateServiceProvider(httpContext, inputFormatters ?? Enumerable.Empty<IInputFormatter>());

if (metadataProvider == null)
{
Expand All @@ -227,21 +261,14 @@ private static ModelBindingContext GetBindingContext(
return bindingContext;
}

private static void UpdateServiceProvider(HttpContext httpContext, IInputFormatter inputFormatter)
private static void UpdateServiceProvider(
HttpContext httpContext,
IEnumerable<IInputFormatter> inputFormatters)
{
var serviceProvider = new ServiceCollection();
var inputFormatterSelector = new Mock<IInputFormatterSelector>();
inputFormatterSelector
.Setup(o => o.SelectFormatter(
It.IsAny<IReadOnlyList<IInputFormatter>>(),
It.IsAny<InputFormatterContext>()))
.Returns(inputFormatter);

serviceProvider.AddInstance(inputFormatterSelector.Object);

var bindingContext = new ActionBindingContext()
{
InputFormatters = new List<IInputFormatter>(),
InputFormatters = inputFormatters.ToArray(),
};

var bindingContextAccessor = new MockScopedInstance<ActionBindingContext>()
Expand Down Expand Up @@ -296,5 +323,25 @@ public override Task<object> ReadRequestBodyAsync(InputFormatterContext context)
throw new InvalidOperationException("Your input is bad!");
}
}

private class TestInputFormatter : IInputFormatter
{
private readonly bool _canRead;

public TestInputFormatter(bool canRead)
{
_canRead = canRead;
}

public bool CanRead(InputFormatterContext context)
{
return _canRead;
}

public Task<object> ReadAsync(InputFormatterContext context)
{
return Task.FromResult<object>(this);
}
}
}
}
38 changes: 38 additions & 0 deletions test/Microsoft.AspNet.Mvc.FunctionalTests/InputFormatterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,44 @@ public async Task JsonInputFormatter_IsModelStateValid_ForValidContentType(strin
Assert.Equal(expectedSampleIntValue.ToString(), responseBody);
}

[Theory]
[InlineData("\"I'm a JSON string!\"")]
[InlineData("true")]
[InlineData("\"\"")] // Empty string
public async Task JsonInputFormatter_ReturnsDefaultValue_ForValueTypes(string input)
Copy link
Contributor

Choose a reason for hiding this comment

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

How about an empty string content? that should be treated as a null value and a model state error should be added by JsonInputformatter

{
// Arrange
var server = TestHelper.CreateServer(_app, SiteName, _configureServices);
var client = server.CreateClient();
var content = new StringContent(input, Encoding.UTF8, "application/json");

// Act
var response = await client.PostAsync("http://localhost/JsonFormatter/ValueTypeAsBody/", content);
var responseBody = await response.Content.ReadAsStringAsync();

//Assert
Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
Assert.Equal("0", responseBody);
}

[Fact]
public async Task JsonInputFormatter_ReadsPrimitiveTypes()
{
// Arrange
var expected = "1773";
var server = TestHelper.CreateServer(_app, SiteName, _configureServices);
var client = server.CreateClient();
var content = new StringContent(expected, Encoding.UTF8, "application/json");

// Act
var response = await client.PostAsync("http://localhost/JsonFormatter/ValueTypeAsBody/", content);
var responseBody = await response.Content.ReadAsStringAsync();

//Assert
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.Equal(expected, responseBody);
}

[Theory]
[InlineData("{\"SampleInt\":10}")]
[InlineData("{}")]
Expand Down
Loading