dotnet / aspnet-api-versioning

Provides a set of libraries which add service API versioning to ASP.NET Web API, OData with ASP.NET Web API, and ASP.NET Core.
MIT License
3.04k stars 703 forks source link

Error in EDM model when using ODataApiExplorer with JSON columns #1113

Open mathew-tolj opened 5 hours ago

mathew-tolj commented 5 hours ago

Is there an existing issue for this?

Describe the bug

I recently configured JSON columns in a service using Microsoft.EntityFrameworkCore.SqlServer 8.0.10 and this caused Swashbuckle.AspNetCore 6.8.1 to fail. Investigating further it looks like this might be caused by the EDM model created by Asp.Versioning.OData.ApiExplorer 8.1.0 failing validation. It seems like the EDM model is expecting the owned entities from the JSON to have a key but in my case they are more like value objects. The exception only causes Swashbuckle to fail. I can still call the endpoint and get a response using methods other than the Swagger endpoint.

Expected Behavior

No response

Steps To Reproduce

Create an entity that owns a list of other entities:

public class Customer
{
    public Guid Id { get; set; } = Guid.NewGuid();

    public string Name { get; set; } = string.Empty;

    public ICollection<Address> Addresses { get; } = [];
}
public class Address
{
    public string Street { get; set; } = string.Empty;

    public string City { get; set; } = string.Empty;
}

Configure a DbContext that persists the list in database as JSON:

public class ReproContext : DbContext
{
    public DbSet<Customer> Customers { get; set; }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder
            .Entity<Customer>()
            .OwnsMany(
                customer => customer.Addresses,
                addressesBuilder =>
                {
                    addressesBuilder.ToJson();
                });
    }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder
            .UseSqlServer(@"Data Source=(local);Encrypt=False;Initial Catalog=Repro;Integrated Security=True;Persist Security Info=False")
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();
    }
}

Create a controller for the entity:

[ApiController]
[ApiVersion("1.0")]
[Route("[controller]")]
public class CustomerController(ReproContext context) : ControllerBase
{
    [HttpGet]
    [Produces("application/json")]
    [ProducesResponseType(typeof(IEnumerable<Customer>), StatusCodes.Status200OK)]
    public IActionResult Get(ODataQueryOptions<Customer> options) => Ok(options.ApplyTo(context.Customers.AsQueryable()));
}

Wire it all up in Program.cs:

WebApplicationBuilder builder = WebApplication.CreateBuilder(args);

// Add services to the container.
builder.Services.AddDbContext<ReproContext>();

builder.Services.AddControllers().AddOData();

// Learn more about configuring Swagger/OpenAPI at https://aka.ms/aspnetcore/swashbuckle.
builder.Services.AddApiVersioning().AddODataApiExplorer();
builder.Services.AddTransient<IConfigureOptions<SwaggerGenOptions>, ConfigureSwaggerGenOptions>();
builder.Services.AddSwaggerGen();

WebApplication app = builder.Build();

// Configure the HTTP request pipeline.
if (app.Environment.IsDevelopment())
{
    app.UseSwagger();
    app.UseSwaggerUI(
        options =>
        {
            // Build a swagger endpoint for each discovered API version.
            foreach (ApiVersionDescription apiVersionDescription in app.DescribeApiVersions())
            {
                options.SwaggerEndpoint($"/swagger/{apiVersionDescription.GroupName}/swagger.json", apiVersionDescription.GroupName.ToUpperInvariant());
            }
        });
}

app.UseHttpsRedirection();
app.UseAuthorization();
app.MapControllers();

app.Run();

Exceptions (if any)

System.InvalidOperationException: The entity type 'ODataApiExplorerException.Address' of navigation property 'Addresses' on structural type 'ODataApiExplorerException.Customer' does not have a key defined.
    at Microsoft.OData.ModelBuilder.ODataModelBuilder.ValidateModel(IEdmModel model)
    at Microsoft.OData.ModelBuilder.ODataConventionModelBuilder.ValidateModel(IEdmModel model)
    at Microsoft.OData.ModelBuilder.ODataModelBuilder.GetEdmModel()
    at Microsoft.OData.ModelBuilder.ODataConventionModelBuilder.GetEdmModel()
    at Asp.Versioning.OData.VersionedODataModelBuilder.BuildModelPerApiVersion(IReadOnlyList`1 apiVersions, IReadOnlyList`1 configurations, List`1 models, String routePrefix)
    at Asp.Versioning.OData.VersionedODataModelBuilder.GetEdmModels(String routePrefix)
    at Asp.Versioning.ApiExplorer.PartialODataDescriptionProvider.OnProvidersExecuting(ApiDescriptionProviderContext context)
    at Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollectionProvider.GetCollection(ActionDescriptorCollection actionDescriptors)
    at Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollectionProvider.get_ApiDescriptionGroups()
    at Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GetSwaggerDocumentWithoutFilters(String documentName, String host, String basePath)
    at Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GetSwaggerAsync(String documentName, String host, String basePath)
    at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider)
    at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
    at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

.NET Version

8.0.403

Anything else?

No response

commonsensesoftware commented 4 hours ago

This might be a gap in the implementation. I can tell by what you've shared that you are in the Partial OData camp. You're not really using OData the protocol. You're just taking advantage of the query options. There's nothing wrong with that per se and why reinvent a HTTP-based query language.

This path, however, does come with some quirks and limitations. First and foremost, the API Explorer extensions for OData are based on an EDM. EF and OData both use EDM, but not the same EDM model or instance. The EDM for EF is about mapping your model to a database. The OData EDM is for mapping your model over the wire (e.g. HTTP). You don't have an OData EDM (that I see). That is to be expected in this scenario. Since the API Explorer extensions only know how to operate on an OData EDM, it generates one on the fly.

It appears you've encountered a sharp edge where:

  1. The implementation, or default implementation, doesn't do the right thing
  2. Additional information is required to generate the proper EDM (for OData)

I'm almost certain it's #2. You need to tell the OData side that the Addresses is either contained or a complex type. This configuration can be achieved by using IModelConfiguration, which will be picked up by DI or you can explicitly configure it through ODataApiExplorerOptions.AdHocModelBuilder. This is the ODataModelBuilder that is used to build the ad hoc EDM used for this scenario.

The reason you haven't seen this before is that you aren't actually using OData in your controller. That requires applying [ODataRouting], which is typically applied by inheriting from ODataController.

I think with a little bit of configuration help on your side, you'll achieve the result you're looking for.