Skip to content

Commit 6cc20e8

Browse files
captainsafiaShreyasJejurkar
authored andcommitted
Avoid logging unsupported alternative for complex type binding (dotnet#39353)
1 parent f4908fd commit 6cc20e8

File tree

3 files changed

+49
-10
lines changed

3 files changed

+49
-10
lines changed

src/Mvc/Mvc.Core/src/ModelBinding/Binders/ComplexTypeModelBinder.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,14 @@ protected virtual object CreateModel(ModelBindingContext bindingContext)
484484
var modelType = bindingContext.ModelType;
485485
if (modelType.IsAbstract || modelType.GetConstructor(Type.EmptyTypes) == null)
486486
{
487+
// If the model is not a top-level object, we can't examine the defined constructor
488+
// to evaluate if the non-null property has been set so we do not provide this as a valid
489+
// alternative.
490+
if (!bindingContext.IsTopLevelObject)
491+
{
492+
throw new InvalidOperationException(Resources.FormatComplexTypeModelBinder_NoParameterlessConstructor_ForType(modelType.FullName));
493+
}
494+
487495
var metadata = bindingContext.ModelMetadata;
488496
switch (metadata.MetadataKind)
489497
{

src/Mvc/Mvc.Core/test/ModelBinding/Binders/ComplexTypeModelBinderTest.cs

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -583,11 +583,8 @@ public void CreateModel_ForStructModelType_AsProperty_ThrowsException()
583583
string.Format(
584584
CultureInfo.CurrentCulture,
585585
"Could not create an instance of type '{0}'. Model bound complex types must not be abstract or " +
586-
"value types and must have a parameterless constructor. Alternatively, set the '{1}' property to" +
587-
" a non-null value in the '{2}' constructor.",
588-
typeof(PointStruct).FullName,
589-
nameof(Location.Point),
590-
typeof(Location).FullName),
586+
"value types and must have a parameterless constructor.",
587+
typeof(PointStruct).FullName),
591588
exception.Message);
592589
}
593590

@@ -997,6 +994,32 @@ public async Task BindModelAsync_Success()
997994
Assert.True(bindingContext.ModelState.IsValid);
998995
}
999996

997+
// Validates fix for https://github.com/dotnet/aspnetcore/issues/21916
998+
[Fact]
999+
public async Task BindModelAsync_PropertyInitializedInNonParameterlessConstructorConstructor()
1000+
{
1001+
// Arrange
1002+
var model = new ModelWithPropertyInitializedInConstructor("TestName");
1003+
var property = GetMetadataForProperty(model.GetType(), nameof(ModelWithPropertyInitializedInConstructor.NameContainer));
1004+
var nestedProperty = GetMetadataForProperty(typeof(ClassWithNoParameterlessConstructor), nameof(ClassWithNoParameterlessConstructor.Name));
1005+
var bindingContext = CreateContext(property);
1006+
bindingContext.IsTopLevelObject = false;
1007+
var valueProvider = new Mock<IValueProvider>(MockBehavior.Strict);
1008+
valueProvider
1009+
.Setup(provider => provider.ContainsPrefix("theModel.Name"))
1010+
.Returns(true);
1011+
bindingContext.ValueProvider = valueProvider.Object;
1012+
var binder = CreateBinder(bindingContext.ModelMetadata);
1013+
1014+
binder.Results[nestedProperty] = ModelBindingResult.Success(null);
1015+
1016+
// Act
1017+
var exception = await Assert.ThrowsAsync<InvalidOperationException>(async () => await binder.BindModelAsync(bindingContext));
1018+
// Assert
1019+
var unexpectedMessage = "Alternatively, set the 'NameContainer' property to a non-null value in the 'Microsoft.AspNetCore.Mvc.ModelBinding.Binders.ComplexTypeModelBinderTest+ModelWithPropertyInitializedInConstructor' constructor.";
1020+
Assert.DoesNotContain(exception.Message, unexpectedMessage);
1021+
}
1022+
10001023
[Fact]
10011024
public void SetProperty_PropertyHasDefaultValue_DefaultValueAttributeDoesNothing()
10021025
{
@@ -1297,6 +1320,17 @@ public ClassWithNoParameterlessConstructor(string name)
12971320
public string Name { get; set; }
12981321
}
12991322

1323+
private class ModelWithPropertyInitializedInConstructor
1324+
{
1325+
public ModelWithPropertyInitializedInConstructor(string name)
1326+
{
1327+
NameContainer = new ClassWithNoParameterlessConstructor(name);
1328+
}
1329+
1330+
[ValueBinderMetadataAttribute]
1331+
public ClassWithNoParameterlessConstructor NameContainer { get; set; }
1332+
}
1333+
13001334
private class BindingOptionalProperty
13011335
{
13021336
[BindingBehavior(BindingBehavior.Optional)]

src/Mvc/test/Mvc.IntegrationTests/ActionParametersIntegrationTest.cs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -473,11 +473,8 @@ public async Task ActionParameter_UsingComplexTypeModelBinder_ModelPropertyTypeW
473473
string.Format(
474474
CultureInfo.CurrentCulture,
475475
"Could not create an instance of type '{0}'. Model bound complex types must not be abstract or " +
476-
"value types and must have a parameterless constructor. Alternatively, set the '{1}' property to" +
477-
" a non-null value in the '{2}' constructor.",
478-
typeof(ClassWithNoDefaultConstructor).FullName,
479-
nameof(Class1.Property1),
480-
typeof(Class1).FullName),
476+
"value types and must have a parameterless constructor.",
477+
typeof(ClassWithNoDefaultConstructor).FullName),
481478
exception.Message);
482479
}
483480

0 commit comments

Comments
 (0)