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

Fix #1579 - Bind top-level collections as an empty collection #2583

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ public override Task<ModelBindingResult> BindModelAsync([NotNull] ModelBindingCo
return base.BindModelAsync(bindingContext);
}

protected override object CreateEmptyCollection()
{
return new TElement[0];
}

/// <inheritdoc />
protected override object GetModel(IEnumerable<TElement> newCollection)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,31 @@ public virtual async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBind
{
ModelBindingHelper.ValidateBindingContext(bindingContext);

object model;

if (!await bindingContext.ValueProvider.ContainsPrefixAsync(bindingContext.ModelName))
{
// If this is the fallback case, and we failed to find data as a top-level model, then generate a
// default 'empty' model and return it.
var isTopLevelObject = bindingContext.ModelMetadata.ContainerType == null;
var hasExplicitAlias = bindingContext.BinderModelName != null;

if (isTopLevelObject && (hasExplicitAlias || bindingContext.ModelName == string.Empty))
{
model = CreateEmptyCollection();

var validationNode = new ModelValidationNode(
bindingContext.ModelName,
bindingContext.ModelMetadata,
model);

return new ModelBindingResult(
model,
bindingContext.ModelName,
isModelSet: true,
validationNode: validationNode);
}

return null;
}

Expand All @@ -46,7 +69,7 @@ public virtual async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBind
boundCollection = result.Model;
}

var model = bindingContext.Model;
model = bindingContext.Model;
if (model == null)
{
model = GetModel(boundCollection);
Expand All @@ -64,6 +87,12 @@ public virtual async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBind
validationNode: result?.ValidationNode);
}

// Called when we're creating a default 'empty' model for a top level bind.
protected virtual object CreateEmptyCollection()
{
return new List<TElement>();
}

// Used when the ValueProvider contains the collection to be bound as a single element, e.g. the raw value
// is [ "1", "2" ] and needs to be converted to an int[].
internal async Task<CollectionResult> BindSimpleCollection(
Expand Down Expand Up @@ -165,8 +194,6 @@ internal async Task<CollectionResult> BindComplexCollectionFromIndexes(
var didBind = false;
object boundValue = null;

var modelType = bindingContext.ModelType;

var result =
await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(childBindingContext);
if (result != null && result.IsModelSet)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,10 @@ protected override object GetModel(IEnumerable<KeyValuePair<TKey, TValue>> newCo
{
return newCollection?.ToDictionary(kvp => kvp.Key, kvp => kvp.Value);
}

protected override object CreateEmptyCollection()
{
return new Dictionary<TKey, TValue>();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,27 @@ public async Task<ModelBindingResult> BindModelAsync(ModelBindingContext binding
}
else
{
// Caller (GenericModelBinder) was able to resolve a binder type and will create a ModelBindingResult
// that exits current ModelBinding loop.
// If this is the fallback case, and we failed to find data as a top-level model, then generate a
// default 'empty' model and return it.
Copy link
Member

Choose a reason for hiding this comment

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

Odd case. Curious whether Web API 2 or MVC 5 pass default(KeyValuePair<TKey, TValue>) into an action. Of course it's fine to support here.

var isTopLevelObject = bindingContext.ModelMetadata.ContainerType == null;
var hasExplicitAlias = bindingContext.BinderModelName != null;

if (isTopLevelObject && (hasExplicitAlias || bindingContext.ModelName == string.Empty))
{
var model = new KeyValuePair<TKey, TValue>();

var validationNode = new ModelValidationNode(
bindingContext.ModelName,
bindingContext.ModelMetadata,
model);

return new ModelBindingResult(
model,
bindingContext.ModelName,
isModelSet: true,
validationNode: validationNode);
}

return null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

#if DNX451
using System.Collections.Generic;
using System.Threading.Tasks;
using Microsoft.AspNet.Http.Internal;
using Moq;
using Xunit;

Expand Down Expand Up @@ -35,14 +37,105 @@ public async Task BindModelAsync_ValueProviderContainPrefix_Succeeds()
}

[Fact]
public async Task BindModelAsync_ValueProviderDoesNotContainPrefix_ReturnsNull()
public async Task ArrayModelBinder_DoesNotCreateCollection_ForTopLevelModel_OnFirstPass()
{
// Arrange
var bindingContext = GetBindingContext(new SimpleHttpValueProvider());
var binder = new ArrayModelBinder<int>();
var binder = new ArrayModelBinder<string>();

var context = CreateContext();
context.ModelName = "param";

var metadataProvider = context.OperationBindingContext.MetadataProvider;
context.ModelMetadata = metadataProvider.GetMetadataForType(typeof(string[]));

context.ValueProvider = new TestValueProvider(new Dictionary<string, object>());

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

// Assert
Assert.Null(result);
}

[Fact]
public async Task ArrayModelBinder_CreatesEmptyCollection_ForTopLevelModel_OnFallback()
{
// Arrange
var binder = new ArrayModelBinder<string>();

var context = CreateContext();
context.ModelName = string.Empty;

var metadataProvider = context.OperationBindingContext.MetadataProvider;
context.ModelMetadata = metadataProvider.GetMetadataForType(typeof(string[]));

context.ValueProvider = new TestValueProvider(new Dictionary<string, object>());

// Act
var result = await binder.BindModelAsync(context);

// Assert
Assert.NotNull(result);

Assert.Empty(Assert.IsType<string[]>(result.Model));
Assert.Equal(string.Empty, result.Key);
Assert.True(result.IsModelSet);

Assert.Same(result.ValidationNode.Model, result.Model);
Assert.Same(result.ValidationNode.Key, result.Key);
Assert.Same(result.ValidationNode.ModelMetadata, context.ModelMetadata);
}

[Fact]
public async Task ArrayModelBinder_CreatesEmptyCollection_ForTopLevelModel_WithExplicitPrefix()
{
// Arrange
var binder = new ArrayModelBinder<string>();

var context = CreateContext();
context.ModelName = "prefix";
context.BinderModelName = "prefix";

var metadataProvider = context.OperationBindingContext.MetadataProvider;
context.ModelMetadata = metadataProvider.GetMetadataForType(typeof(string[]));

context.ValueProvider = new TestValueProvider(new Dictionary<string, object>());

// Act
var result = await binder.BindModelAsync(context);

// Assert
Assert.NotNull(result);

Assert.Empty(Assert.IsType<string[]>(result.Model));
Assert.Equal("prefix", result.Key);
Assert.True(result.IsModelSet);

Assert.Same(result.ValidationNode.Model, result.Model);
Assert.Same(result.ValidationNode.Key, result.Key);
Assert.Same(result.ValidationNode.ModelMetadata, context.ModelMetadata);
}

[Theory]
[InlineData("")]
[InlineData("param")]
public async Task ArrayModelBinder_DoesNotCreateCollection_ForNonTopLevelModel(string prefix)
{
// Arrange
var binder = new ArrayModelBinder<string>();

var context = CreateContext();
context.ModelName = ModelNames.CreatePropertyModelName(prefix, "ArrayProperty");

var metadataProvider = context.OperationBindingContext.MetadataProvider;
context.ModelMetadata = metadataProvider.GetMetadataForProperty(
typeof(ModelWithArrayProperty),
nameof(ModelWithArrayProperty.ArrayProperty));

context.ValueProvider = new TestValueProvider(new Dictionary<string, object>());

// Act
var result = await binder.BindModelAsync(context);

// Assert
Assert.Null(result);
Expand Down Expand Up @@ -155,6 +248,25 @@ private static ModelBindingContext GetBindingContext(
};
return bindingContext;
}

private static ModelBindingContext CreateContext()
{
var modelBindingContext = new ModelBindingContext()
{
OperationBindingContext = new OperationBindingContext()
{
HttpContext = new DefaultHttpContext(),
MetadataProvider = new TestModelMetadataProvider(),
}
};

return modelBindingContext;
}

private class ModelWithArrayProperty
{
public string[] ArrayProperty { get; set; }
}
}
}
#endif
Loading