Model Validation Not Automatically Applied in OData Controller

[Supernectar]

Assemblies affected ASP.NET Core OData 8.2.3

Describe the bug The OData controller, specifically the CustomersController, is not automatically applying model validation to the key parameter in the Delete endpoint, despite having a model (Customer) with a MinLength validation attribute set for the Id property.

Reproduce steps

1. Clone this repo https://github.com/Supernectar/ODataValidationAttributeBug and run the project
2. Send a DELETE request to the /Customers endpoint with a key parameter that violates the MinLength validation constraint.
3. Observe that the controller does not perform automatic model validation, and the request is processed without considering the validation attribute.

Data Model

public class Customer
{
    [MinLength(5)]
    public string? Id { get; set; }
    public string? Name { get; set; }
}

EDM (CSDL) Model

<?xml version="1.0" encoding="utf-8"?>
<edmx:Edmx Version="4.0" xmlns:edmx="http://docs.oasis-open.org/odata/ns/edmx">
    <edmx:DataServices>
        <Schema Namespace="WebApplication1" xmlns="http://docs.oasis-open.org/odata/ns/edm">
            <EntityType Name="Customer">
                <Key>
                    <PropertyRef Name="Id" />
                </Key>
                <Property Name="Id" Type="Edm.String" Nullable="false" />
                <Property Name="Name" Type="Edm.String" />
            </EntityType>
        </Schema>
        <Schema Namespace="Default" xmlns="http://docs.oasis-open.org/odata/ns/edm">
            <EntityContainer Name="Container">
                <EntitySet Name="Customers" EntityType="WebApplication1.Customer" />
            </EntityContainer>
        </Schema>
    </edmx:DataServices>
</edmx:Edmx>

Request/Response Request Uri https://localhost:7234/Customers/123 Response Body 200 OK

Expected behavior The OData controller should automatically apply model validation to the key parameter based on the validation attributes specified in the corresponding model (Customer). Specifically, it should enforce the MinLength constraint for the Id property.

[julealgon]

Do you have the [ApiController] attribute applied to your controller class? Automatic validation is enabled as part of the capabilities of that attribute.

[Supernectar]

@julealgon yes, in the example repo I referenced I am making use of the ApiController attribute.

As an additional note, adding this ApiController Attribute breaks the path parameters conventions coming with the OdataController, meaning that I am required to specify a [FromRoute] attribute in the key parameter in the delete method.. If not it is assumed to be a query parameter.

[julealgon]

@Supernectar I guess I'm confused as to what you expected, or how you expected it to even work...

You pass a raw string value into the controller action, and you expected the framework to magically trace it back to the Id property in the Customer model and then take the validations from that?

I mean... that would be cool, but fairly unrealistic IMHO. That's not how validation works usually (in standard ASPNET MVC). If you want validation to happen, the validation needs to be in the parameter that you are passing, not in some other type...

If you want the [MinLength] validation to apply to the parameter in the Delete controller action, just specify it there:

public ActionResult Delete([FromRoute] [MinLength(5)] string key)

What you are trying to achieve, with the "link" between the string key parameter and the underlying model certainly will not work: there is no support for such thing. Your post implies it should "just work", which is surprising to me. Have you seen evidence of such cross validation happening anywhere else, or mentioned in OData documentation?

[Supernectar]

@julealgon Let's take a look at this example (notice how there is no ApiController attribute and the datatype of the Id property is now a Guid)

public class CustomersController : ODataController
{
    public ActionResult Delete(string key)
    {
        return Ok();
    }
}

public class Customer
{
    public Guid? Id { get; set; }
    public string? Name { get; set; }
}

In this case, if you try to make the following request DELETE /Customers/1234 You get a 500 Bad Request with the following exception is thrown

Microsoft.OData.ODataException: The key value (123) from request is not valid. The key value should be format of type 'Edm.Guid'.
 ---> Microsoft.OData.ODataException: Type verification failed. Expected type 'Edm.Guid' but received the value '123'.
   at Microsoft.OData.ODataUriConversionUtils.VerifyAndCoerceUriPrimitiveLiteral(Object primitiveValue, String literalValue, IEdmModel model, IEdmTypeReference expectedTypeReference)
   at Microsoft.OData.ODataUriUtils.ConvertFromUriLiteral(String value, ODataVersion version, IEdmModel model, IEdmTypeReference typeReference)
   at Microsoft.AspNetCore.OData.Routing.Template.KeySegmentTemplate.TryTranslate(ODataTemplateTranslateContext context)
   --- End of inner exception stack trace ---
   at Microsoft.AspNetCore.OData.Routing.Template.KeySegmentTemplate.TryTranslate(ODataTemplateTranslateContext context)
   at Microsoft.AspNetCore.OData.Routing.Template.DefaultODataTemplateTranslator.Translate(ODataPathTemplate path, ODataTemplateTranslateContext context)
   at Microsoft.AspNetCore.OData.Routing.ODataRoutingMatcherPolicy.ApplyAsync(HttpContext httpContext, CandidateSet candidates)
   at Microsoft.AspNetCore.Routing.Matching.DfaMatcher.SelectEndpointWithPoliciesAsync(HttpContext httpContext, IEndpointSelectorPolicy[] policies, CandidateSet candidateSet)
   at Microsoft.AspNetCore.Routing.EndpointRoutingMiddleware.<Invoke>g__AwaitMatch|8_1(EndpointRoutingMiddleware middleware, HttpContext httpContext, Task matchTask)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)

This example demonstrates that the AspNetCoreOData framework seamlessly connects the key route parameter to the corresponding Id property in the Customer model. What I expect is to take advantage of this link to not only match the datatype but also to interpret any validation attribute applied to the Id property. I get that this might not be achievable at the moment but I'm hopefully trying to convince you that this is a bug hehe

[aboryczko]

I think that this is very different. Here there is a type mismatch and since the action's key parameter type is Guid it cannot convert 1234 into one. Also there is no MinLength attribute in the EDM so I don't see anyway that it could work based on the model (although it also doesn't work for MaxLength which is in the mode) When looking at the code there is nothing that would indicate that any type of model validation happens for key segments.

[julealgon]

This example demonstrates that the AspNetCoreOData framework seamlessly connects the key route parameter to the corresponding Id property in the Customer model. What I expect is to take advantage of this link to not only match the datatype but also to interpret any validation attribute applied to the Id property.

Fair enough. An interesting ask, but I'm not sure it will be implemented. Wait for what the OData team will say on this.

I get that this might not be achievable at the moment but I'm hopefully trying to convince you that this is a bug hehe

It is most definitely not a bug. Validation was never supposed to work like this by design. The type of "validation" you are seeing is completely different and much lower level. They are not comparable.

[mikepizzo]

As per the discussion, there are really two feature asks here.

The first feature would be to support mapping the language attribute to the MaxLength facet in the model. This seems a reasonable feature request, and we would be open to contributions in this area.

The second feature request would be to validate constraints defined in the model and, in this case, handle certain constraints without calling the controller. As @julealgon says, that type of validation is not something that we've contemplated for the framework, but always considered something that the controllers would handle naturally.