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

Commit

Permalink
Merge branch 'rel/1.1.2' into dev
Browse files Browse the repository at this point in the history
  • Loading branch information
@dougbu
dougbu committed Feb 10, 2017
2 parents 1652bf5 + 8ee3d45 commit fc40985
Show file tree
Hide file tree
Showing 10 changed files with 235 additions and 161 deletions.
23 changes: 23 additions & 0 deletions src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/ComplexTypeModelBinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Linq.Expressions;
using System.Reflection;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc.Core;
using Microsoft.AspNetCore.Mvc.Internal;

namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
Expand Down Expand Up @@ -47,6 +48,28 @@ public Task BindModelAsync(ModelBindingContext bindingContext)
return TaskCache.CompletedTask;
}

// The following check causes the ComplexTypeModelBinder to NOT participate in binding structs as
// reflection does not provide information about the implicit parameterless constructor for a struct.
// This binder would eventually fail to construct an instance of the struct as the Linq's NewExpression
// compile fails to construct it.
var modelTypeInfo = bindingContext.ModelType.GetTypeInfo();
if (bindingContext.Model == null &&
(modelTypeInfo.IsAbstract ||
modelTypeInfo.GetConstructor(Type.EmptyTypes) == null))
{
if (bindingContext.IsTopLevelObject)
{
throw new InvalidOperationException(
Resources.FormatComplexTypeModelBinder_NoParameterlessConstructor_TopLevelObject(modelTypeInfo.FullName));
}

throw new InvalidOperationException(
Resources.FormatComplexTypeModelBinder_NoParameterlessConstructor_ForProperty(
modelTypeInfo.FullName,
bindingContext.ModelName,
bindingContext.ModelMetadata.ContainerType.FullName));
}

// Perf: separated to avoid allocating a state machine when we don't
// need to go async.
return BindModelCoreAsync(bindingContext);
Expand Down
13 changes: 1 addition & 12 deletions src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/ComplexTypeModelBinderProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ public IModelBinder GetBinder(ModelBinderProviderContext context)
throw new ArgumentNullException(nameof(context));
}

if (context.Metadata.IsComplexType &&
!context.Metadata.IsCollectionType &&
HasDefaultConstructor(context.Metadata.ModelType.GetTypeInfo()))
if (context.Metadata.IsComplexType && !context.Metadata.IsCollectionType)
{
var propertyBinders = new Dictionary<ModelMetadata, IModelBinder>();
for (var i = 0; i < context.Metadata.Properties.Count; i++)
Expand All @@ -36,14 +34,5 @@ public IModelBinder GetBinder(ModelBinderProviderContext context)

return null;
}

private bool HasDefaultConstructor(TypeInfo modelTypeInfo)
{
// The following check causes the ComplexTypeModelBinder to NOT participate in binding structs.
// - Reflection does not provide information about the implicit parameterless constructor for a struct.
// - Also this binder would eventually fail to construct an instance of the struct as the Linq's
// NewExpression compile fails to construct it.
return !modelTypeInfo.IsAbstract && modelTypeInfo.GetConstructor(Type.EmptyTypes) != null;
}
}
}
32 changes: 32 additions & 0 deletions src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/Microsoft.AspNetCore.Mvc.Core/Resources.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,4 +391,10 @@
<value>'{0}' and '{1}' are out of bounds for the string.</value>
<comment>'{0}' and '{1}' are the parameters which combine to be out of bounds.</comment>
</data>
<data name="ComplexTypeModelBinder_NoParameterlessConstructor_TopLevelObject" xml:space="preserve">
<value>Could not create an instance of type '{0}'. Model bound complex types must not be abstract or value types and must have a parameterless constructor.</value>
</data>
<data name="ComplexTypeModelBinder_NoParameterlessConstructor_ForProperty" xml:space="preserve">
<value>Could not create an instance of type '{0}'. Model bound complex types must not be abstract or value types and must have a parameterless constructor. Alternatively, set the '{1}' property to a non-null value in the '{2}' constructor.</value>
</data>
</root>
7 changes: 5 additions & 2 deletions src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,14 @@ public static string GetExpressionText(LambdaExpression expression, ExpressionTe
switch (part.NodeType)
{
case ExpressionType.Call:
// Will exit loop if at Method().Property or [i,j].Property. In that case (like [i].Property),
// don't cache and don't remove ".Model" (if that's .Property).
containsIndexers = true;
lastIsModel = false;

var methodExpression = (MethodCallExpression)part;
if (IsSingleArgumentIndexer(methodExpression))
{
containsIndexers = true;
lastIsModel = false;
length += "[99]".Length;
part = methodExpression.Object;
segmentCount++;
Expand Down
55 changes: 35 additions & 20 deletions src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionTextCache.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ public bool Equals(LambdaExpression lambdaExpression1, LambdaExpression lambdaEx

while (true)
{
if (expression1 == null && expression2 == null)
{
return true;
}

if (expression1 == null || expression2 == null)
{
return false;
Expand All @@ -47,31 +52,41 @@ public bool Equals(LambdaExpression lambdaExpression1, LambdaExpression lambdaEx
return false;
}

if (expression1.NodeType == ExpressionType.MemberAccess)
switch (expression1.NodeType)
{
var memberExpression1 = (MemberExpression)expression1;
var memberName1 = memberExpression1.Member.Name;
expression1 = memberExpression1.Expression;
case ExpressionType.MemberAccess:
var memberExpression1 = (MemberExpression)expression1;
var memberName1 = memberExpression1.Member.Name;
expression1 = memberExpression1.Expression;

var memberExpression2 = (MemberExpression)expression2;
var memberName2 = memberExpression2.Member.Name;
expression2 = memberExpression2.Expression;
var memberExpression2 = (MemberExpression)expression2;
var memberName2 = memberExpression2.Member.Name;
expression2 = memberExpression2.Expression;

// If identifier contains "__", it is "reserved for use by the implementation" and likely compiler-
// or Razor-generated e.g. the name of a field in a delegate's generated class.
if (memberName1.Contains("__") && memberName2.Contains("__"))
{
return true;
}
// If identifier contains "__", it is "reserved for use by the implementation" and likely
// compiler- or Razor-generated e.g. the name of a field in a delegate's generated class.
if (memberName1.Contains("__") && memberName2.Contains("__"))
{
return true;
}

if (!string.Equals(memberName1, memberName2, StringComparison.OrdinalIgnoreCase))
{
if (!string.Equals(memberName1, memberName2, StringComparison.OrdinalIgnoreCase))
{
return false;
}
break;

case ExpressionType.ArrayIndex:
// Shouldn't be cached. Just in case, ensure indexers are all different.
return false;
}
}
else
{
return true;

case ExpressionType.Call:
// Shouldn't be cached. Just in case, ensure indexers and other calls are all different.
return false;

default:
// Everything else terminates name generation. Haven't found a difference so far...
return true;
}
}
}
Expand Down
96 changes: 0 additions & 96 deletions ...osoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/ComplexTypeModelBinderProviderTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,107 +55,11 @@ public void Create_ForSupportedTypes_ReturnsBinder()
Assert.IsType<ComplexTypeModelBinder>(result);
}

[Theory]
[InlineData(typeof(PointStructWithExplicitConstructor))]
[InlineData(typeof(PointStructWithNoExplicitConstructor))]
public void Create_ForStructModel_ReturnsNull(Type modelType)
{
// Arrange
var provider = new ComplexTypeModelBinderProvider();
var context = new TestModelBinderProviderContext(modelType);

// Act
var result = provider.GetBinder(context);

// Assert
Assert.Null(result);
}

[Theory]
[InlineData(typeof(ClassWithNoDefaultConstructor))]
[InlineData(typeof(ClassWithStaticConstructor))]
[InlineData(typeof(ClassWithInternalDefaultConstructor))]
public void Create_ForModelTypeWithNoDefaultPublicConstructor_ReturnsNull(Type modelType)
{
// Arrange
var provider = new ComplexTypeModelBinderProvider();
var context = new TestModelBinderProviderContext(modelType);

// Act
var result = provider.GetBinder(context);

// Assert
Assert.Null(result);
}

[Fact]
public void Create_ForAbstractModelTypeWithDefaultPublicConstructor_ReturnsNull()
{
// Arrange
var provider = new ComplexTypeModelBinderProvider();
var context = new TestModelBinderProviderContext(typeof(AbstractClassWithDefaultConstructor));

// Act
var result = provider.GetBinder(context);

// Assert
Assert.Null(result);
}

private struct PointStructWithNoExplicitConstructor
{
public double X { get; set; }
public double Y { get; set; }
}

private struct PointStructWithExplicitConstructor
{
public PointStructWithExplicitConstructor(double x, double y)
{
X = x;
Y = y;
}
public double X { get; }
public double Y { get; }
}

private class Person
{
public string Name { get; set; }

public int Age { get; set; }
}

private class ClassWithNoDefaultConstructor
{
public ClassWithNoDefaultConstructor(int id) { }
}

private class ClassWithInternalDefaultConstructor
{
internal ClassWithInternalDefaultConstructor() { }
}

private class ClassWithStaticConstructor
{
static ClassWithStaticConstructor() { }

public ClassWithStaticConstructor(int id) { }
}

private abstract class AbstractClassWithDefaultConstructor
{
private readonly string _name;

public AbstractClassWithDefaultConstructor()
: this("James")
{
}

public AbstractClassWithDefaultConstructor(string name)
{
_name = name;
}
}
}
}
Loading

0 comments on commit fc40985

Please sign in to comment.