AspNetCoreOData icon indicating copy to clipboard operation
AspNetCoreOData copied to clipboard

EDM Validation Not Triggered on ODataActionParameters

Open fabiocbr75 opened this issue 7 months ago • 3 comments

Hello,

I'm trying to apply EDM validation on ODataActionParameters, but it doesn't seem to work as expected. Below is a minimal reproduction:

public partial class foo
{
    [Key]
    public int id { get; set; }
    public string name { get; set; }
    public string? description { get; set; }
}

private static IEdmModel GetEdmModel()
{
    ODataConventionModelBuilder builder = new ODataConventionModelBuilder();
    builder.EntitySet<foo>("foo");

    builder.EntityType<foo>().Property(e => e.name).IsRequired();

    builder.EntityType<foo>().Collection.Action("myaction")
        .CollectionEntityParameter<foo>("foo_list");

    return builder.GetEdmModel();
}

public class fooController : ODataController
{
    private readonly ILogger<fooController> _logger;

    public fooController(ILogger<fooController> logger)
    {
        _logger = logger;
    }

    [HttpPost("foo/myaction")]
    public IActionResult myaction(ODataActionParameters parameters)
    {
        if (!ModelState.IsValid)
        {
            return BadRequest();
        }

        try
        {
            var foo_list = parameters["foo_list"] as IEnumerable<foo>;
        }
        catch (Exception ex)
        {
            return BadRequest(ex.Message);
        }

        return Ok();
    }
}

Tested with the following curl command:

curl --request POST \
  --url http://localhost:5168/foo/myaction \
  --header 'Content-Type: application/json' \
  --data '{
  "foo_list": [
    {
      "id": 1,
      "name": "Equipment Name1",
      "description": "Equipment Description1"
    },
    {
      "id": 2,
      "description": "Equipment Description2"
    }
  ]
}'

In the second object of the foo_list, the name field is missing, even though it is marked as required in the EDM model. However, no validation error is triggered, and the request is processed normally.

Question: Did I miss something in the setup to enable validation? Is there a recommended way to enforce EDM-based validation on ODataActionParameters?

Thanks in advance for your help!

fabiocbr75 avatar Apr 26 '25 18:04 fabiocbr75

It could be related to the 'ODataReader' created at 'https://github.com/OData/AspNetCoreOData/blob/main/src/Microsoft.AspNetCore.OData/Formatter/Deserialization/ODataActionPayloadDeserializer.cs#L132', There's no 'type' or 'schema' input to create the reader.

If you check it with 'ODataResourceSetDeserializer' at https://github.com/OData/AspNetCoreOData/blob/main/src/Microsoft.AspNetCore.OData/Formatter/Deserialization/ODataResourceSetDeserializer.cs#L65, it inputs a 'structuredType' to create the ODataReader.

xuzhg avatar Apr 29 '25 16:04 xuzhg

Important to note that IsRequired() just sets Nullable=false for the property. Nullable=false does NOT mean that it has to be present in every payload.

For example, if the property has a default value, or is computed or server generated, then, even though it is Nullable=false, it does not need to be specified in a create request payload. Similarly, it doesn't need to be in the payload of a PATCH request if it already has a value (or the value is computed/default/etc.).

For response payloads, a property may not be present because it was not requested.

Nullable=false just means that the value will never be null, not that it is present in every payload.

In this scenario, since the value is used within an action parameter, then as long as we know it is not computed, not server generated, and has no default value, we might be able to assume it has no value if not specified in the payload. However, we need to make sure that any validations we add are in scenarios where the property really is required to be present, not just non-nullable.

Also, as discussed, for backward compatibility, as well as potential edge cases, we would need to allow opt'ing out of any such validation. For example, if I am creating a custom action that mimics a PATCH request, I would need the ability to pass a payload that omits non-nullable properties that are not computed, not server generated, and have no default value.

mikepizzo avatar Apr 29 '25 17:04 mikepizzo

In any case, from a validation point of view, the issue is that the payload is deserialized directly into the final class Foo. As a result, it’s not possible to perform any validation inside the action—for example, value types are always populated with their default values.

In my example, I used an EntitySet as a parameter. However, if I use a POCO class instead, how can I provide my client with information about which fields are required in the $metadata?

Below is the updated example:

public partial class foo
{
    [Key]
    public int id { get; set; }
    public string name { get; set; }
    public string? description { get; set; }
}

public partial class foo_poco
{
    public int id { get; set; }
    public string name { get; set; }
    public string? description { get; set; }
}

private static IEdmModel GetEdmModel()
{
    ODataConventionModelBuilder builder = new ODataConventionModelBuilder();
    builder.EntitySet<foo>("foo");

    builder.EntityType<foo>().Property(e => e.name).IsRequired();

    builder.EntityType<foo>().Collection.Action("myaction")
        .CollectionParameter<foo_poco>("foo_list");

    return builder.GetEdmModel();
}

public class fooController : ODataController
{
    private readonly ILogger<fooController> _logger;

    public fooController(ILogger<fooController> logger)
    {
        _logger = logger;
    }

    [HttpPost("foo/myaction")]
    public IActionResult myaction(ODataActionParameters parameters)
    {
        if (!ModelState.IsValid)
        {
            return BadRequest();
        }

        try
        {
            var foo_list = parameters["foo_list"] as IEnumerable<foo_poco>;
        }
        catch (Exception ex)
        {
            return BadRequest(ex.Message);
        }

        return Ok();
    }
}

fabiocbr75 avatar Apr 29 '25 21:04 fabiocbr75