Skip to content
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

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

Open
1 task done
mathew-tolj opened this issue Oct 15, 2024 · 3 comments
Open
1 task done

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

mathew-tolj opened this issue Oct 15, 2024 · 3 comments
Assignees

Comments

@mathew-tolj
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

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
Copy link
Collaborator

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.

@mathew-tolj
Copy link
Author

Thanks to your guidance I have been able to make some progress on this. I have run into a related issue and I am unsure if it is my lack of EDM model knowledge, if I am implementing the versioning incorrectly, or if it is actually a bug.

To configure the model to solve the original issue, I have added a model configuration:

public class CustomerModelConfiguration : IModelConfiguration
{
    public void Apply(ODataModelBuilder builder, ApiVersion apiVersion, string? routePrefix)
    {
        _ = builder.EntitySet<Customer>("Customers");
        _ = builder.ComplexType<Address>();
    }
}

This works fine but in my production code the entity causing the issue is only in version 2 of the API so I added a check against the API version to only configure the model for version 2:

public class CustomerModelConfiguration : IModelConfiguration
{
    private static readonly ApiVersion s_version1 = new(1, 0);

    public void Apply(ODataModelBuilder builder, ApiVersion apiVersion, string? routePrefix)
    {
        if (apiVersion > s_version1)
        {
            _ = builder.EntitySet<Customer>("Customers");
            _ = builder.ComplexType<Address>();
        }
    }
}

With this check in place the exception will still occur when processing version 1. This error appears to short circuit the configuration process and version 2 is never configured. I have tried casting the builder to ODataConventionModelBuilder and calling Ignore for the types when configuring version 1 but it does not seem to have helped. Is there a better way for me to handle this without configuring the types for versions where they are not used?

@commonsensesoftware
Copy link
Collaborator

Hmm... This might actually be a bug. The generated EDM is only created and used by API Explorer. It's actually ephemeral. The workaround should be to use the same model configuration for every API version. I'll have to dig deeper as to why it still causes a problem. A distinct EDM is created for each API version. I wouldn't expect an explicit Ignore to have any effect because it's not in the EDM unless you put it there first. Nothing is automatic.

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

2 participants