OData / AspNetCoreOData

ASP.NET Core OData: A server library built upon ODataLib and ASP.NET Core
Other
453 stars 160 forks source link

Uninformative error messages from the model state because of the ODataInputFormatter #962

Closed rayandaher811 closed 1 year ago

rayandaher811 commented 1 year ago

Assemblies affected Which assemblies and versions are known to be affected e.g. ASP.NET Core OData 8.x .Net 6 AspNet Core 6 Microsoft.AspNetCore.OData Version="[8.0.6]" Microsoft.OData.Core Version="[7.13.0]" Microsoft.Data.OData Version="[5.8.5]"

Describe the bug We are using OData EDM model with our ASPNet service and getting non-informative error messages from the aspnet model state.

  1. For wrong Enum values: Let's say I'm sending object from type A to our service and A have an enum type property, if the property has wrong Enum value the error message I will get from the model state is "Field A is required" without giving more information like "You have inserted wrong Enum type for type A" for example.
  2. For wrong built in abstract types: Lets say the same A from the previous example has another property with abstract class type, I will send to my service a type A without specifying its type like ""@odata.type": "#mynamespace.myabstractClass", The whole A object is built ok but my abstract type property will be without type specification. For that case I will get "Field A is required" from the model state error message.

For both cases it doesn't matter in what nested type I will be in A, I could have three nesting types A->B->C and then C contains my Enum/abstract type; the error message will stay "Field A is required".

Expected behavior Instead of this error message in the model state. image I would expect a relevant error message that relates to specific wrong built like, your property "X" has wrong Enum type in order to understand what is wrong in my json body.

Additional context I can't share my code/EDMs due confidential issues, please feel free to contact me: rayandaher@microsoft.com

julealgon commented 1 year ago

@rayandaher811 how do these messages look like if you are using raw MVC without OData?

gathogojr commented 1 year ago

Thank you @rayandaher811 for reaching out to us. Unfortunately, I couldn't successfully repro the issue you reported. Below are the characteristics of a simple OData service I created in my attempt to reproduce the issue:

Data Model

namespace NS.Models
{
    public enum OrderType
    {
        Market,
        Limit,
        Stop
    }

    public abstract class BaseEntity
    {
        public int Id { get; set; }
    }

    public class Customer : BaseEntity
    {
        public string? Name { get; set; }
    }

    public class Order
    {
        public int Id { get; set; }
        public OrderType Type { get; set; }
        public BaseEntity? Customer { get; set; }
    }
}

Controller:

namespace NS.Controllers
{
    public class OrdersController : ODataController
    {
        public ActionResult Post([FromBody] Order order)
        {
            return Accepted();
        }
    }
}

Edm model and service configuration:

var builder = WebApplication.CreateBuilder(args);
var modelBuilder = new ODataConventionModelBuilder();
modelBuilder.EntitySet<Order>("Orders");

builder.Services.AddControllers().AddOData(
    options => options.EnableQueryFeatures().AddRouteComponents(
        model: modelBuilder.GetEdmModel()));

var app = builder.Build();
app.UseRouting();
app.UseEndpoints(endpoints => endpoints.MapControllers());
app.Run();

For the first scenario, if I send a payload to the /Orders endpoint with an invalid value for OrderType, e.g.,

GET http://localhost:5259/Orders

Request body:

{
    "Id":3,
    "Type":"Undefined"
}

I get a relevant error message in the ModelState object: image

For the second scenario, if I fail to include @odata.type annotation, e.g.,

{
    "Id":3,
    "Type":"Limit",
    "Customer":{"Id":7,"Name":"Sue"}
}

I still get a relevant error message: image

I understand you can't share your code and model due to confidentiality issues. However, it should be easy to create a minimal repro for the scenarios you have described.

Find below the code for the repro project I created. ReproAspNetCoreODataIssue962.zip

If you share a repro, it'll be easier to identify what the issue might be. cc. @xuzhg

gathogojr commented 1 year ago

@rayandaher811 Applying the [ApiController] attribute combined with attribute routing is responsible for the behaviour. Based on ASP.NET Core documentation, the [ApiController] attribute can be applied to a controller class to enable the following opinionated, API-specific behaviors:

Specifically because of the third point is why we're observing the error message. What we have advised customers is not to mix OData routing with ASP.NET Core routing. If you'd wish to use attribute routing, OData supports that too and you don't need to apply the [ApiController] attribute.

Find the project after I applied the [ApiController] attribute: ReproAspNetCoreODataIssue962.zip

If I send a payload with an invalid value for the enum, I get a similar error message to the one you're observing.

POST http://localhost:5259/Orders

Request:

{
    "Id": 3,
    "Type": "Unknown"
}

Response:

{
    "type": "https://tools.ietf.org/html/rfc7231#section-6.5.1",
    "title": "One or more validation errors occurred.",
    "status": 400,
    "traceId": "00-9a04f0bba6f776e6ff3a46e319332d1d-b7014ac08a4e0941-00",
    "errors": {
        "": [
            "The input was not valid."
        ],
        "order": [
            "The order field is required."
        ]
    }
}

cc. @xuzhg

gathogojr commented 1 year ago

Issue attributed to a mitigable clash between ASP.NET Core routing and OData routing.