Skip to content

EDM Validation Not Triggered on ODataActionParameters #1471

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fabiocbr75 opened this issue Apr 26, 2025 · 3 comments
Open

EDM Validation Not Triggered on ODataActionParameters #1471

fabiocbr75 opened this issue Apr 26, 2025 · 3 comments
Assignees

Comments

@fabiocbr75
Copy link

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!

@xuzhg
Copy link
Member

xuzhg commented Apr 29, 2025

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 xuzhg self-assigned this Apr 29, 2025
@mikepizzo
Copy link
Member

mikepizzo commented Apr 29, 2025

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.

@fabiocbr75
Copy link
Author

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();
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants