Avoid logging unsupported alternative for complex type binding #39353
Conversation
ghost
added
the
area-mvc
captainsafia
force-pushed
the
safia/complex-type-model
branch
from
4df285f to
e7dc09e
Compare
|
/azp run |
|
Needs a full re-run given #39361. |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| @@ -588,11 +588,8 @@ public void CreateModel_ForStructModelType_AsProperty_ThrowsException() | |||
| string.Format( | |||
| CultureInfo.CurrentCulture, | |||
| "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" + | |||
There was a problem hiding this comment.
Isn't this message still valid / correct, isn't it? Should we be changing it for this case?
There was a problem hiding this comment.
Hmmmm. My understanding was we wanted to stop logging this recommendation for any complex type where we couldn't reliably examine the constructor to verify that they were initializing the property inside it?
There was a problem hiding this comment.
The way I had thought this would work was that we would not show this message for top-level objects. We would still show this for arbitrary properties that we couldn't initialize because it is valid for users to initialize it and for us to bind properties on it. That said, this one is a struct so it might have unique characteristics.
There was a problem hiding this comment.
I think this exception still makes sense. For a set of types like:
private class Location
{
public PointStruct Point { get; set; }
}
private readonly struct PointStruct
{
public PointStruct(double x, double y)
{
X = x;
Y = y;
}
public double X { get; }
public double Y { get; }
}We can't examine the Location constructor to check if Point is initialized so it doesn't make sense to log that behavior?
There was a problem hiding this comment.
Ok, I re-read the message and it works out alrigh - it says you cannot model bind a value type. The part about the ctor is entirely irrelevant here. This seems alright enough, let's get this in.
| @@ -588,11 +588,8 @@ public void CreateModel_ForStructModelType_AsProperty_ThrowsException() | |||
| string.Format( | |||
| CultureInfo.CurrentCulture, | |||
| "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" + | |||
There was a problem hiding this comment.
Ok, I re-read the message and it works out alrigh - it says you cannot model bind a value type. The part about the ctor is entirely irrelevant here. This seems alright enough, let's get this in.
Closes #21916.