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

Commit

Permalink
Fix #2330 - Reimagine *FormatterContext
Browse files Browse the repository at this point in the history
This change simplifies InputFormatterContext/OutputFormatterContext by
swapping ActionContext for HttpContext.

This change is important especially for InputFormatterContext as it
decouples ModelState from ActionContext - allowing us to fix a
related bug where the _wrong_ ModelState can be passed in for a
TryUpdateModel operation.
  • Loading branch information
rynowak committed May 12, 2015
1 parent fcf7b15 commit 39fe063
Show file tree
Hide file tree
Showing 36 changed files with 402 additions and 370 deletions.
34 changes: 26 additions & 8 deletions src/Microsoft.AspNet.Mvc.Abstractions/InputFormatterContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,33 +2,51 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using Microsoft.AspNet.Http;
using Microsoft.AspNet.Mvc.ModelBinding;
using Microsoft.Framework.Internal;

namespace Microsoft.AspNet.Mvc
{
/// <summary>
/// Represents information used by an input formatter for
/// deserializing the request body into an object.
/// A context object used by an input formatter for deserializing the request body into an object.
/// </summary>
public class InputFormatterContext
{
/// <summary>
/// Creates a new instance of <see cref="InputFormatterContext"/>.
/// </summary>
public InputFormatterContext([NotNull] ActionContext actionContext,
[NotNull] Type modelType)
/// <param name="httpContext">
/// The <see cref="Http.HttpContext"/> for the current operation.
/// </param>
/// <param name="modelState">
/// The <see cref="ModelStateDictionary"/> for recording errors.
/// </param>
/// <param name="modelType">
/// The <see cref="Type"/> of the model to deserialize.
/// </param>
public InputFormatterContext(
[NotNull] HttpContext httpContext,
[NotNull] ModelStateDictionary modelState,
[NotNull] Type modelType)
{
ActionContext = actionContext;
HttpContext = httpContext;
ModelState = modelState;
ModelType = modelType;
}

/// <summary>
/// Action context associated with the current call.
/// Gets the <see cref="Http.HttpContext"/> associated with the current operation.
/// </summary>
public ActionContext ActionContext { get; private set; }
public HttpContext HttpContext { get; private set; }

/// <summary>
/// Represents the expected type of the model represented by the request body.
/// Gets the <see cref="ModelStateDictionary"/> associated with the current operation.
/// </summary>
public ModelStateDictionary ModelState { get; }

/// <summary>
/// Gets the expected <see cref="Type"/> of the model represented by the request body.
/// </summary>
public Type ModelType { get; private set; }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Text;
using Microsoft.AspNet.Http;
using Microsoft.Net.Http.Headers;

namespace Microsoft.AspNet.Mvc
Expand All @@ -24,9 +25,9 @@ public class OutputFormatterContext
public Type DeclaredType { get; set; }

/// <summary>
/// Action context associated with the current call.
/// Gets or sets the <see cref="HttpContext"/> context associated with the current operation.
/// </summary>
public ActionContext ActionContext { get; set; }
public HttpContext HttpContext { get; set; }

/// <summary>
/// The encoding which is chosen by the selected formatter.
Expand Down
4 changes: 1 addition & 3 deletions src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public override async Task ExecuteResultAsync([NotNull] ActionContext context)

var formatterContext = new OutputFormatterContext()
{
ActionContext = context,
HttpContext = context.HttpContext,
DeclaredType = objectResult.DeclaredType,
Object = Value,
};
Expand All @@ -125,7 +125,6 @@ private IOutputFormatter SelectFormatter(ObjectResult objectResult, OutputFormat
{
// If no formatter was provided, then run Conneg with the formatters configured in options.
var formatters = formatterContext
.ActionContext
.HttpContext
.RequestServices
.GetRequiredService<IOptions<MvcOptions>>()
Expand All @@ -140,7 +139,6 @@ private IOutputFormatter SelectFormatter(ObjectResult objectResult, OutputFormat
// If the available user-configured formatters can't write this type, then fall back to the
// 'global' one.
formatter = formatterContext
.ActionContext
.HttpContext
.RequestServices
.GetRequiredService<JsonOutputFormatter>();
Expand Down
18 changes: 9 additions & 9 deletions src/Microsoft.AspNet.Mvc.Core/ActionResults/ObjectResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public override async Task ExecuteResultAsync(ActionContext context)
var formatterContext = new OutputFormatterContext()
{
DeclaredType = DeclaredType,
ActionContext = context,
HttpContext = context.HttpContext,
Object = Value,
StatusCode = StatusCode
};
Expand Down Expand Up @@ -83,8 +83,7 @@ public virtual IOutputFormatter SelectFormatter(
OutputFormatterContext formatterContext,
IEnumerable<IOutputFormatter> formatters)
{
var logger = formatterContext.ActionContext.HttpContext.RequestServices
.GetRequiredService<ILogger<ObjectResult>>();
var logger = formatterContext.HttpContext.RequestServices.GetRequiredService<ILogger<ObjectResult>>();

// Check if any content-type was explicitly set (for example, via ProducesAttribute
// or Url path extension mapping). If yes, then ignore content-negotiation and use this content-type.
Expand All @@ -108,7 +107,7 @@ public virtual IOutputFormatter SelectFormatter(
// which can write the type.
MediaTypeHeaderValue requestContentType = null;
MediaTypeHeaderValue.TryParse(
formatterContext.ActionContext.HttpContext.Request.ContentType,
formatterContext.HttpContext.Request.ContentType,
out requestContentType);
if (!sortedAcceptHeaderMediaTypes.Any() && requestContentType == null)
{
Expand Down Expand Up @@ -240,17 +239,18 @@ public virtual IOutputFormatter SelectFormatterUsingAnyAcceptableContentType(
private IEnumerable<MediaTypeHeaderValue> GetSortedAcceptHeaderMediaTypes(
OutputFormatterContext formatterContext)
{
var request = formatterContext.ActionContext.HttpContext.Request;
var request = formatterContext.HttpContext.Request;
var incomingAcceptHeaderMediaTypes = request.GetTypedHeaders().Accept ?? new MediaTypeHeaderValue[] { };

// By default we want to ignore considering accept headers for content negotiation when
// they have a media type like */* in them. Browsers typically have these media types.
// In these cases we would want the first formatter in the list of output formatters to
// write the response. This default behavior can be changed through options, so checking here.
var options = formatterContext.ActionContext.HttpContext
.RequestServices
.GetRequiredService<IOptions<MvcOptions>>()
.Options;
var options = formatterContext
.HttpContext
.RequestServices
.GetRequiredService<IOptions<MvcOptions>>()
.Options;

var respectAcceptHeader = true;
if (options.RespectBrowserAcceptHeader == false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue

public Task WriteAsync(OutputFormatterContext context)
{
var response = context.ActionContext.HttpContext.Response;
var response = context.HttpContext.Response;
response.ContentLength = 0;
response.StatusCode = context.StatusCode ?? StatusCodes.Status204NoContent;
return Task.FromResult(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue
/// <inheritdoc />
public Task WriteAsync(OutputFormatterContext context)
{
var response = context.ActionContext.HttpContext.Response;
var response = context.HttpContext.Response;
response.StatusCode = StatusCodes.Status406NotAcceptable;
return Task.FromResult(true);
}
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.AspNet.Mvc.Core/Formatters/InputFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public virtual bool CanRead(InputFormatterContext context)
return false;
}

var contentType = context.ActionContext.HttpContext.Request.ContentType;
var contentType = context.HttpContext.Request.ContentType;
MediaTypeHeaderValue requestContentType;
if (!MediaTypeHeaderValue.TryParse(contentType, out requestContentType))
{
Expand All @@ -72,7 +72,7 @@ protected virtual bool CanReadType(Type type)
/// <inheritdoc />
public virtual async Task<object> ReadAsync(InputFormatterContext context)
{
var request = context.ActionContext.HttpContext.Request;
var request = context.HttpContext.Request;
if (request.ContentLength == 0)
{
return GetDefaultValueForType(context.ModelType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public JsonSerializerSettings SerializerSettings
public override Task<object> ReadRequestBodyAsync([NotNull] InputFormatterContext context)
{
var type = context.ModelType;
var request = context.ActionContext.HttpContext.Request;
var request = context.HttpContext.Request;
MediaTypeHeaderValue requestContentType = null;
MediaTypeHeaderValue.TryParse(request.ContentType, out requestContentType);

Expand All @@ -70,7 +70,7 @@ public override Task<object> ReadRequestBodyAsync([NotNull] InputFormatterContex
errorHandler = (sender, e) =>
{
var exception = e.ErrorContext.Error;
context.ActionContext.ModelState.TryAddModelError(e.ErrorContext.Path, e.ErrorContext.Error);
context.ModelState.TryAddModelError(e.ErrorContext.Path, e.ErrorContext.Error);

// Error must always be marked as handled
// Failure to do so can cause the exception to be rethrown at every recursive level and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ private JsonSerializer CreateJsonSerializer()

public override Task WriteResponseBodyAsync(OutputFormatterContext context)
{
var response = context.ActionContext.HttpContext.Response;
var response = context.HttpContext.Response;
var selectedEncoding = context.SelectedEncoding;

using (var writer = new HttpResponseStreamWriter(response.Body, selectedEncoding))
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.AspNet.Mvc.Core/Formatters/OutputFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public virtual IReadOnlyList<MediaTypeHeaderValue> GetSupportedContentTypes(
/// <returns>The <see cref="Encoding"/> to use when reading the request or writing the response.</returns>
public virtual Encoding SelectCharacterEncoding([NotNull] OutputFormatterContext context)
{
var request = context.ActionContext.HttpContext.Request;
var request = context.HttpContext.Request;
var encoding = MatchAcceptCharacterEncoding(request.GetTypedHeaders().AcceptCharset);
if (encoding == null)
{
Expand Down Expand Up @@ -195,7 +195,7 @@ public virtual void WriteResponseHeaders([NotNull] OutputFormatterContext contex
selectedMediaType.Charset = selectedEncoding.WebName;

context.SelectedContentType = context.SelectedContentType ?? selectedMediaType;
var response = context.ActionContext.HttpContext.Response;
var response = context.HttpContext.Response;
response.ContentType = selectedMediaType.ToString();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public async Task WriteAsync([NotNull] OutputFormatterContext context)
{
using (var valueAsStream = ((Stream)context.Object))
{
var response = context.ActionContext.HttpContext.Response;
var response = context.HttpContext.Response;

if (context.SelectedContentType != null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public override async Task WriteResponseBodyAsync(OutputFormatterContext context
return;
}

var response = context.ActionContext.HttpContext.Response;
var response = context.HttpContext.Response;

await response.WriteAsync(valueAsString, context.SelectedEncoding);
}
Expand Down
14 changes: 9 additions & 5 deletions src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.Linq;
using System.Reflection;
using System.Threading.Tasks;
using Microsoft.AspNet.Mvc.Core;
using Microsoft.Framework.DependencyInjection;
Expand All @@ -26,14 +25,19 @@ public BodyModelBinder()
}

/// <inheritdoc />
protected async override Task<ModelBindingResult> BindModelCoreAsync([NotNull] ModelBindingContext bindingContext)
protected async override Task<ModelBindingResult> BindModelCoreAsync(
[NotNull] ModelBindingContext bindingContext)
{
var requestServices = bindingContext.OperationBindingContext.HttpContext.RequestServices;

var actionContext = requestServices.GetRequiredService<IScopedInstance<ActionContext>>().Value;
var formatters = requestServices.GetRequiredService<IScopedInstance<ActionBindingContext>>().Value.InputFormatters;
var httpContext = bindingContext.OperationBindingContext.HttpContext;
var formatters = requestServices
.GetRequiredService<IScopedInstance<ActionBindingContext>>().Value.InputFormatters;

var formatterContext = new InputFormatterContext(actionContext, bindingContext.ModelType);
var formatterContext = new InputFormatterContext(
httpContext,
bindingContext.ModelState,
bindingContext.ModelType);
var formatter = formatters.FirstOrDefault(f => f.CanRead(formatterContext));

if (formatter == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue

public async Task WriteAsync(OutputFormatterContext context)
{
var response = context.ActionContext.HttpContext.Response;
var response = context.HttpContext.Response;

var responseMessage = context.Object as HttpResponseMessage;
if (responseMessage == null)
Expand All @@ -35,7 +35,7 @@ public async Task WriteAsync(OutputFormatterContext context)
{
response.StatusCode = (int)responseMessage.StatusCode;

var responseFeature = context.ActionContext.HttpContext.GetFeature<IHttpResponseFeature>();
var responseFeature = context.HttpContext.GetFeature<IHttpResponseFeature>();
if (responseFeature != null)
{
responseFeature.ReasonPhrase = responseMessage.ReasonPhrase;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public DataContractSerializerSettings SerializerSettings
/// <returns>Task which reads the input.</returns>
public override Task<object> ReadRequestBodyAsync(InputFormatterContext context)
{
var request = context.ActionContext.HttpContext.Request;
var request = context.HttpContext.Request;

MediaTypeHeaderValue requestContentType;
MediaTypeHeaderValue.TryParse(request.ContentType , out requestContentType);
Expand All @@ -106,7 +106,7 @@ public override Task<object> ReadRequestBodyAsync(InputFormatterContext context)

_dataAnnotationRequiredAttributeValidation.Validate(
type,
context.ActionContext.ModelState);
context.ModelState);

var serializer = GetCachedSerializer(type);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ public override Task WriteResponseBodyAsync([NotNull] OutputFormatterContext con
var tempWriterSettings = WriterSettings.Clone();
tempWriterSettings.Encoding = context.SelectedEncoding;

var innerStream = context.ActionContext.HttpContext.Response.Body;
var innerStream = context.HttpContext.Response.Body;

using (var outputStream = new NonDisposableStream(innerStream))
using (var xmlWriter = CreateXmlWriter(outputStream, tempWriterSettings))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public XmlDictionaryReaderQuotas XmlDictionaryReaderQuotas
/// <returns>Task which reads the input.</returns>
public override Task<object> ReadRequestBodyAsync(InputFormatterContext context)
{
var request = context.ActionContext.HttpContext.Request;
var request = context.HttpContext.Request;

MediaTypeHeaderValue requestContentType;
MediaTypeHeaderValue.TryParse(request.ContentType, out requestContentType);
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.AspNet.Mvc.Xml/XmlSerializerOutputFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,12 @@ public virtual XmlWriter CreateXmlWriter([NotNull] Stream writeStream,
/// <inheritdoc />
public override Task WriteResponseBodyAsync([NotNull] OutputFormatterContext context)
{
var response = context.ActionContext.HttpContext.Response;
var response = context.HttpContext.Response;

var tempWriterSettings = WriterSettings.Clone();
tempWriterSettings.Encoding = context.SelectedEncoding;

var innerStream = context.ActionContext.HttpContext.Response.Body;
var innerStream = context.HttpContext.Response.Body;

using (var outputStream = new NonDisposableStream(innerStream))
using (var xmlWriter = CreateXmlWriter(outputStream, tempWriterSettings))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ public void SelectFormatter_WithNoMatchingAcceptHeadersAndRequestContentType_Pic

var context = new OutputFormatterContext()
{
ActionContext = actionContext,
HttpContext = actionContext.HttpContext,
Object = input,
DeclaredType = typeof(string)
};
Expand Down Expand Up @@ -530,7 +530,7 @@ public async Task ObjectResult_Execute_CallsJsonResult_SetsContent()
new ActionDescriptor());
var formatterContext = new OutputFormatterContext()
{
ActionContext = tempActionContext,
HttpContext = tempActionContext.HttpContext,
Object = nonStringValue,
DeclaredType = nonStringValue.GetType()
};
Expand Down
Loading

0 comments on commit 39fe063

Please sign in to comment.