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

Commit

Permalink
Splitting ModelBinding Context into operation specific and model spec…
Browse files Browse the repository at this point in the history
…ific context.
  • Loading branch information
harshgMSFT committed Nov 14, 2014
1 parent 77814f7 commit 6ba2199
Show file tree
Hide file tree
Showing 24 changed files with 274 additions and 141 deletions.
14 changes: 9 additions & 5 deletions src/Microsoft.AspNet.Mvc.Core/ControllerActionArgumentBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,21 +89,25 @@ internal static ModelBindingContext GetModelBindingContext(ModelMetadata modelMe
propertyName => BindAttribute.IsPropertyAllowed(propertyName,
modelMetadata.IncludedProperties,
modelMetadata.ExcludedProperties);
var operationBindingContext = new OperationBindingContext
{
ModelBinder = actionBindingContext.ModelBinder,
ValidatorProvider = actionBindingContext.ValidatorProvider,
MetadataProvider = actionBindingContext.MetadataProvider,
HttpContext = actionBindingContext.ActionContext.HttpContext,
OriginalValueProvider = actionBindingContext.ValueProvider,

This comment has been minimized.

Copy link
@rynowak

rynowak Nov 18, 2014

Member

Can this just be ValueProvider?

This comment has been minimized.

Copy link
@harshgMSFT

harshgMSFT Nov 18, 2014

Author Contributor

Yes.

};

var modelBindingContext = new ModelBindingContext
{
ModelName = modelMetadata.ModelName ?? modelMetadata.PropertyName,
ModelMetadata = modelMetadata,
ModelState = actionBindingContext.ActionContext.ModelState,
ModelBinder = actionBindingContext.ModelBinder,
ValidatorProvider = actionBindingContext.ValidatorProvider,
MetadataProvider = actionBindingContext.MetadataProvider,
HttpContext = actionBindingContext.ActionContext.HttpContext,
PropertyFilter = propertyFilter,
// Fallback only if there is no explicit model name set.
FallbackToEmptyPrefix = modelMetadata.ModelName == null,
ValueProvider = actionBindingContext.ValueProvider,
OriginalValueProvider = actionBindingContext.ValueProvider,
OperationBindingContext = operationBindingContext,
};

return modelBindingContext;
Expand Down
6 changes: 3 additions & 3 deletions src/Microsoft.AspNet.Mvc.Core/ModelBinders/BodyModelBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ protected override async Task<bool> BindAsync(
if (formatter == null)
{
var unsupportedContentType = Resources.FormatUnsupportedContentType(
bindingContext.HttpContext.Request.ContentType);
bindingContext.OperationBindingContext.HttpContext.Request.ContentType);
bindingContext.ModelState.AddModelError(bindingContext.ModelName, unsupportedContentType);

// Should always return true so that the model binding process ends here.
Expand All @@ -51,8 +51,8 @@ protected override async Task<bool> BindAsync(

// Validate the deserialized object
var validationContext = new ModelValidationContext(
bindingContext.MetadataProvider,
bindingContext.ValidatorProvider,
bindingContext.OperationBindingContext.MetadataProvider,
bindingContext.OperationBindingContext.ValidatorProvider,
bindingContext.ModelState,
bindingContext.ModelMetadata,
containerMetadata: null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,23 @@ public static async Task<bool> TryUpdateModelAsync<TModel>(
var modelMetadata = metadataProvider.GetMetadataForType(
modelAccessor: null,
modelType: typeof(TModel));
var operationBindingContext = new OperationBindingContext
{
ModelBinder = modelBinder,
ValidatorProvider = validatorProvider,
MetadataProvider = metadataProvider,
HttpContext = httpContext
};

var modelBindingContext = new ModelBindingContext
{
ModelMetadata = modelMetadata,
ModelName = prefix,
Model = model,
ModelState = modelState,
ModelBinder = modelBinder,
ValueProvider = valueProvider,
ValidatorProvider = validatorProvider,
MetadataProvider = metadataProvider,
FallbackToEmptyPrefix = true,
HttpContext = httpContext
OperationBindingContext = operationBindingContext,
};

if (await modelBinder.BindModelAsync(modelBindingContext))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public Task<bool> BindModelAsync(ModelBindingContext bindingContext)
{
if (bindingContext.ModelType == typeof(CancellationToken))
{
bindingContext.Model = bindingContext.HttpContext.RequestAborted;
bindingContext.Model =bindingContext.OperationBindingContext.HttpContext.RequestAborted;

This comment has been minimized.

Copy link
@rynowak

rynowak Nov 18, 2014

Member

spacing

return Task.FromResult(true);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ internal async Task<List<TElement>> BindSimpleCollection(ModelBindingContext bin
{
var innerBindingContext = new ModelBindingContext(bindingContext)
{
ModelMetadata = bindingContext.MetadataProvider.GetMetadataForType(null, typeof(TElement)),
ModelMetadata = bindingContext.OperationBindingContext.MetadataProvider.GetMetadataForType(null, typeof(TElement)),
ModelName = bindingContext.ModelName,
ValueProvider = new CompositeValueProvider
{
Expand All @@ -60,7 +60,7 @@ internal async Task<List<TElement>> BindSimpleCollection(ModelBindingContext bin
};

object boundValue = null;
if (await bindingContext.ModelBinder.BindModelAsync(innerBindingContext))
if (await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(innerBindingContext))
{
boundValue = innerBindingContext.Model;
bindingContext.ValidationNode.ChildNodes.Add(innerBindingContext.ValidationNode);
Expand Down Expand Up @@ -101,7 +101,7 @@ internal async Task<List<TElement>> BindComplexCollectionFromIndexes(ModelBindin
var fullChildName = ModelBindingHelper.CreateIndexModelName(bindingContext.ModelName, indexName);
var childBindingContext = new ModelBindingContext(bindingContext)

This comment has been minimized.

Copy link
@rynowak

rynowak Nov 18, 2014

Member

is there a case where you'd call the copy constructor and not set modelName and modelmetadata? Consider just making these required params, or a static factory method

This comment has been minimized.

Copy link
@harshgMSFT

harshgMSFT Nov 19, 2014

Author Contributor

Not related to this change hence do not want to touch it.
The most probable reason for not setting modelName and modelMetadata would be for instance CompositeModelBinder creates a new copy of model metadata first time so that it does not update the passed in context. ( Questionable, but still that is the behavior today). I do not want to do unrelated cleanups here.

This comment has been minimized.

Copy link
@rynowak

rynowak Nov 19, 2014

Member

Not related to this change hence do not want to touch it.

Isn't the purpose of this refactor to cleanup usage of ModelBindingContext? Your call about whether or not it's an improvement, but if you're refactoring MBC, I don't think it's out of bounds to tailor the constructors to the expected usage.

{
ModelMetadata = bindingContext.MetadataProvider.GetMetadataForType(null, typeof(TElement)),
ModelMetadata = bindingContext.OperationBindingContext.MetadataProvider.GetMetadataForType(null, typeof(TElement)),
ModelName = fullChildName
};

Expand All @@ -110,7 +110,7 @@ internal async Task<List<TElement>> BindComplexCollectionFromIndexes(ModelBindin

var modelType = bindingContext.ModelType;

if (await bindingContext.ModelBinder.BindModelAsync(childBindingContext))
if (await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(childBindingContext))
{
didBind = true;
boundValue = childBindingContext.Model;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public async Task<bool> BindModelAsync(ModelBindingContext bindingContext)

// bind and propagate the values
// If we can't bind, then leave the result missing (don't add a null).
if (await bindingContext.ModelBinder.BindModelAsync(propertyBindingContext))
if (await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(propertyBindingContext))
{
var result = new ComplexModelDtoResult(propertyBindingContext.Model,
propertyBindingContext.ValidationNode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ public virtual async Task<bool> BindModelAsync([NotNull] ModelBindingContext bin
bindingContext.ModelName);
}

var validationContext = new ModelValidationContext(bindingContext.MetadataProvider,
bindingContext.ValidatorProvider,
var validationContext = new ModelValidationContext( bindingContext.OperationBindingContext.MetadataProvider,
bindingContext.OperationBindingContext.ValidatorProvider,
bindingContext.ModelState,
bindingContext.ModelMetadata,
containerMetadata: null);
Expand Down Expand Up @@ -131,11 +131,7 @@ private static ModelBindingContext CreateNewBindingContext(ModelBindingContext o
ModelName = modelName,
ModelState = oldBindingContext.ModelState,
ValueProvider = oldBindingContext.ValueProvider,
OriginalValueProvider = oldBindingContext.OriginalValueProvider,
ValidatorProvider = oldBindingContext.ValidatorProvider,
MetadataProvider = oldBindingContext.MetadataProvider,
ModelBinder = oldBindingContext.ModelBinder,
HttpContext = oldBindingContext.HttpContext,
OperationBindingContext = oldBindingContext.OperationBindingContext,
PropertyFilter = oldBindingContext.PropertyFilter,

This comment has been minimized.

Copy link
@rynowak

rynowak Nov 18, 2014

Member

should we be using the copy constructor here?

This comment has been minimized.

Copy link
@harshgMSFT

harshgMSFT Nov 19, 2014

Author Contributor

even if we do there would be 3 properties that would need to be set.
I am not pro or anti... to me it sounds unnecessary change, let me know if you feel strongly about it.

};

Expand All @@ -155,7 +151,7 @@ private static ModelBindingContext CreateNewBindingContext(ModelBindingContext o
// of all value providers. This is so that every artifact that is explicitly marked using an
// IValueProviderMetadata can restrict model binding to only use value providers which support this
// IValueProviderMetadata.
var valueProvider = oldBindingContext.OriginalValueProvider as IMetadataAwareValueProvider;
var valueProvider = oldBindingContext.OperationBindingContext.OriginalValueProvider as IMetadataAwareValueProvider;
if (valueProvider != null)
{
newBindingContext.ValueProvider = valueProvider.Filter(metadata);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ internal async Task<BindResult<TModel>> TryBindStrongModel<TModel>(ModelBindingC
{
var propertyBindingContext = new ModelBindingContext(parentBindingContext)
{
ModelMetadata = parentBindingContext.MetadataProvider.GetMetadataForType(modelAccessor: null,
ModelMetadata = parentBindingContext.OperationBindingContext.MetadataProvider.GetMetadataForType(modelAccessor: null,
modelType: typeof(TModel)),
ModelName = ModelBindingHelper.CreatePropertyModelName(parentBindingContext.ModelName, propertyName)
};

if (await propertyBindingContext.ModelBinder.BindModelAsync(propertyBindingContext))
if (await propertyBindingContext.OperationBindingContext.ModelBinder.BindModelAsync(propertyBindingContext))
{
var untypedModel = propertyBindingContext.Model;
var model = ModelBindingHelper.CastOrDefault<TModel>(untypedModel);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ private async Task<bool> CanBindValue(ModelBindingContext bindingContext, ModelM
if (valueProviderMetadata != null)
{
// if there is a binder metadata and since the property can be bound using a value provider.
var metadataAwareValueProvider = bindingContext.OriginalValueProvider as IMetadataAwareValueProvider;
var metadataAwareValueProvider = bindingContext.OperationBindingContext.OriginalValueProvider as IMetadataAwareValueProvider;
if (metadataAwareValueProvider != null)
{
valueProvider = metadataAwareValueProvider.Filter(valueProviderMetadata);
Expand Down Expand Up @@ -212,12 +212,12 @@ private ComplexModelDto CreateAndPopulateDto(ModelBindingContext bindingContext,
var originalDto = new ComplexModelDto(bindingContext.ModelMetadata, propertyMetadatas);
var dtoBindingContext = new ModelBindingContext(bindingContext)
{
ModelMetadata = bindingContext.MetadataProvider.GetMetadataForType(() => originalDto,
ModelMetadata = bindingContext.OperationBindingContext.MetadataProvider.GetMetadataForType(() => originalDto,
typeof(ComplexModelDto)),
ModelName = bindingContext.ModelName
};

bindingContext.ModelBinder.BindModelAsync(dtoBindingContext);
bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(dtoBindingContext);
return (ComplexModelDto)dtoBindingContext.Model;
}

Expand Down Expand Up @@ -265,8 +265,9 @@ protected virtual void EnsureModel(ModelBindingContext bindingContext)
protected virtual IEnumerable<ModelMetadata> GetMetadataForProperties(ModelBindingContext bindingContext)
{
var validationInfo = GetPropertyValidationInfo(bindingContext);
var propertyTypeMetadata = bindingContext.MetadataProvider
.GetMetadataForType(null, bindingContext.ModelType);
var propertyTypeMetadata = bindingContext.OperationBindingContext
.MetadataProvider
.GetMetadataForType(null, bindingContext.ModelType);
Predicate<string> newPropertyFilter =
propertyName => bindingContext.PropertyFilter(propertyName) &&
BindAttribute.IsPropertyAllowed(
Expand Down Expand Up @@ -298,7 +299,8 @@ internal static PropertyValidationInfo GetPropertyValidationInfo(ModelBindingCon
{
var propertyName = property.Name;
var propertyMetadata = bindingContext.PropertyMetadata[propertyName];
var requiredValidator = bindingContext.ValidatorProvider
var requiredValidator = bindingContext.OperationBindingContext
.ValidatorProvider
.GetValidators(propertyMetadata)
.FirstOrDefault(v => v != null && v.IsRequired);
if (requiredValidator != null)
Expand Down
35 changes: 3 additions & 32 deletions src/Microsoft.AspNet.Mvc.ModelBinding/ModelBindingContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,12 @@ public ModelBindingContext(ModelBindingContext bindingContext)
{
ModelState = bindingContext.ModelState;
ValueProvider = bindingContext.ValueProvider;
OriginalValueProvider = bindingContext.OriginalValueProvider;
MetadataProvider = bindingContext.MetadataProvider;
ModelBinder = bindingContext.ModelBinder;
ValidatorProvider = bindingContext.ValidatorProvider;
HttpContext = bindingContext.HttpContext;
OperationBindingContext = bindingContext.OperationBindingContext;
}
}

public OperationBindingContext OperationBindingContext { get; set; }

/// <summary>
/// Gets or sets the model associated with this context.
/// </summary>
Expand Down Expand Up @@ -130,38 +128,11 @@ public Type ModelType
/// </summary>
public bool FallbackToEmptyPrefix { get; set; }

/// <summary>
/// Gets or sets the <see cref="HttpContext"/> for the current request.
/// </summary>
public HttpContext HttpContext { get; set; }

/// <summary>
/// Gets unaltered value provider collection.
/// Value providers can be filtered by specific model binders.
/// </summary>
public IValueProvider OriginalValueProvider { get; set; }

/// <summary>
/// Gets or sets the <see cref="IValueProvider"/> associated with this context.
/// </summary>
public IValueProvider ValueProvider { get; set; }

/// <summary>
/// Gets or sets the <see cref="IModelBinder"/> associated with this context.
/// </summary>
public IModelBinder ModelBinder { get; set; }

/// <summary>
/// Gets or sets the <see cref="IModelMetadataProvider"/> associated with this context.
/// </summary>
public IModelMetadataProvider MetadataProvider { get; set; }

/// <summary>
/// Gets or sets the <see cref="IModelValidatorProvider"/> instance used for model validation with this
/// context.
/// </summary>
public IModelValidatorProvider ValidatorProvider { get; set; }

/// <summary>
/// Gets a dictionary of property name to <see cref="ModelMetadata"/> instances for
/// <see cref="ModelMetadata.Properties"/>
Expand Down
67 changes: 67 additions & 0 deletions src/Microsoft.AspNet.Mvc.ModelBinding/OperationBindingContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using Microsoft.AspNet.Http;

namespace Microsoft.AspNet.Mvc.ModelBinding
{
/// <summary>
/// A context that contains operating information for model binding and validation.
/// </summary>
public class OperationBindingContext
{
/// <summary>
/// Initializes a new instance of the <see cref="OperationBindingContext"/> class.
/// </summary>
public OperationBindingContext()
{
}

/// <summary>
/// Initializes a new instance of the <see cref="OperationBindingContext"/> class using the
/// <param name="bindingContext" />.
// </summary>
/// <remarks>
/// This constructor copies certain values that won't change between parent and child objects,
/// e.g. ValueProvider, ModelState
/// </remarks>
public OperationBindingContext(OperationBindingContext bindingContext)
{
if (bindingContext != null)

This comment has been minimized.

Copy link
@rynowak

rynowak Nov 18, 2014

Member

why is this needed?

This comment has been minimized.

Copy link
@harshgMSFT

harshgMSFT Nov 18, 2014

Author Contributor

you mean the copy constructor or the check?
No specific reason for the copy constructor, can be removed.

{
OriginalValueProvider = bindingContext.OriginalValueProvider;
MetadataProvider = bindingContext.MetadataProvider;
ModelBinder = bindingContext.ModelBinder;
ValidatorProvider = bindingContext.ValidatorProvider;
HttpContext = bindingContext.HttpContext;
}
}

/// <summary>
/// Gets or sets the <see cref="HttpContext"/> for the current request.
/// </summary>
public HttpContext HttpContext { get; set; }

/// <summary>
/// Gets unaltered value provider collection.
/// Value providers can be filtered by specific model binders.
/// </summary>
public IValueProvider OriginalValueProvider { get; set; }

/// <summary>
/// Gets or sets the <see cref="IModelBinder"/> associated with this context.
/// </summary>
public IModelBinder ModelBinder { get; set; }

/// <summary>
/// Gets or sets the <see cref="IModelMetadataProvider"/> associated with this context.
/// </summary>
public IModelMetadataProvider MetadataProvider { get; set; }

/// <summary>
/// Gets or sets the <see cref="IModelValidatorProvider"/> instance used for model validation with this
/// context.
/// </summary>
public IModelValidatorProvider ValidatorProvider { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ public class ModelValidationContext
{
public ModelValidationContext([NotNull] ModelBindingContext bindingContext,
[NotNull] ModelMetadata metadata)
: this(bindingContext.MetadataProvider,
bindingContext.ValidatorProvider,
: this(bindingContext.OperationBindingContext.MetadataProvider,
bindingContext.OperationBindingContext.ValidatorProvider,
bindingContext.ModelState,
metadata,
bindingContext.ModelMetadata)
Expand Down
Loading

0 comments on commit 6ba2199

Please sign in to comment.