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.03k stars 704 forks source link

Breaking changes when migrating to OData8 + new versioning #1051

Closed cfauchere closed 6 months ago

cfauchere commented 9 months ago

Our projects are currently on OData 7. We are also trying to move from Net7 to Net8. However when doing so we hit a stackoverflow issue that is preventing us from just bumping to Net8 (though we have a hack to workaround). It motivated me enough to try to move to the new stuff instead of keeping the legacy on life support. eg I am trying to move from mvc to endpoint routing and Odata 8 + asp.versioning packages. Doing so I ran into new issues:

  1. ODataRoutePrefix / ODataRoute attributes not being supported though I was able to refactor the controllers for that issue
  2. the casing has changed, used to be Pascal, now it's camelCase
  3. query options likes $select, $expand etc seem to have changed to select, expand etc
  4. OData functions that were admitting optional parameters now requires the parameters to be part of the path making it mandatory instead of in the query previously

packages

  <PackageReference Include="Microsoft.AspNetCore.OData" Version="8.0.11" />
  <PackageReference Include="Asp.Versioning.OData" Version="7.1.0" />
  <PackageReference Include="Asp.Versioning.OData.ApiExplorer" Version="7.1.0" />

Samples

odata/$metadata

before

<ComplexType Name="Image">
<Property Name="Id" Type="Edm.String"/>
<Property Name="Name" Type="Edm.String"/>
<Property Name="Location" Type="Edm.String"/>
<Property Name="LicenseRequirement" Type="UIPath.Hypervisor.LicenseRequirement" Nullable="false"/>
<Property Name="ImageSource" Type="UIPath.Hypervisor.ImageSource" Nullable="false"/>
<Property Name="HostedInCommunityGallery" Type="Edm.Boolean" Nullable="false"/>
</ComplexType>

now

<ComplexType Name="Image">
<Property Name="id" Type="Edm.String"/>
<Property Name="name" Type="Edm.String"/>
<Property Name="location" Type="Edm.String"/>
<Property Name="licenseRequirement" Type="UIPath.Hypervisor.LicenseRequirement" Nullable="false"/>
<Property Name="imageSource" Type="UIPath.Hypervisor.ImageSource" Nullable="false"/>
<Property Name="hostedInCommunityGallery" Type="Edm.Boolean" Nullable="false"/>
</ComplexType>

Swagger

before

image

now

image

commonsensesoftware commented 9 months ago

Nice! Glad to see you moving to the new. The migration is a little rough, even with just plain 'ol OData, but the end result is largely better.

Issue 1

I don't have any control or influence over what the OData team does with routing. I just follow the leader. I can say that OData 8.0+ is more inline with intrinsic ASP.NET Core routing, which means you have less to learn about the OData way to do things. There's still gotchas and you have to know the OData URL conventions, but it's better. For API Versioning, this meant significantly less forking, hacking, and conversion of OData routing mechanics. I can lean in a lot more on what OData provides, especially for URL generation, which is nice.

Issue 2

When you say casing, I presume you are referring to the casing of JSON returned. I had to spelunk the commits, but this was changed over 5 years ago. Comparing dates, it looks like this has been the behavior for OData on ASP.NET Core since API Versioning 3.0.0-beta1, which was the first version to support OData. If you had been using OData on legacy ASP.NET Web API, Pascal/Title case that was the default up until 2.2.0. This change was made because the majority of users expect JSON to be Camelcase in alignment with JSON standards. Furthermore, that is the default in ASP.NET Core itself now (for years), even without OData. The goal was to have congruency and one less knob to turn.

Fear not! You can easily go back to Pascalcase if you want it.

var builder = WebApplication.CreateBuilder( args );

builder.Services.AddControllers().AddOData(); // ← this adds OData services
builder.Services.AddProblemDetails();
builder.Services.AddApiVersioning()
                .AddOData( // ← this adds OData to API Versioning
                    options =>
                    {
                        // the default setup uses:
                        // new ODataConventionModelBuilder().EnableLowerCamelCase() 
                        options.ModelBuilder.ModelBuilderFactory = static () => new ODataConventionModelBuilder();

                        // route components have to be add here (sadly 😢) because there is no way to extend,
                        // customize, or otherwise get in front of the OData configuration. every effort was made
                        // to make things look and feel like the native OData API and configuration
                        options.AddRouteComponents( "api" );
                    } );

Issue 3

The OData spec itself has continued to evolve. The $ prefix is no longer required. There's a long history that some implementers didn't like the $ prefix, which hindered adoption (like Azure). The original intend was for these query parameters to be unambiguously specific to OData. The OData spec now allows all System Operators to not require a $ prefix, which makes them look more like any other query parameter or REST API.

OData 8.0 established parity with this capability back in the first 8.0 release candidate circa 2/2021. The default configuration is to not include the $, but you can add it back:

var builder = WebApplication.CreateBuilder( args );

// OData query options and route options are shared across all versioned routes so you only need
// to define them once for your application. Route components (e.g. AddRouteComponents) MUST
// be done through the API Versioning extensions not the native OData methods. such routes are
// ignored by API Versioning if you do.
//
// Re-enable the $ prefix
builder.Services.AddControllers().AddOData(options = options.EnableNoDollarQueryOptions = false);
builder.Services.AddProblemDetails();
builder.Services.AddApiVersioning().AddOData();

Issue 4

I'm not 100% sure on this one. Is this in reference to defining a parameter as optional in an EDM function or in the OpenAPI documentation? I'd have to refer back to the specs and documentation. OData has little-to-no OpenAPI support and it produces a lot less than the API Explorer extensions that API Versioning provides. My best guess is that there is either a mismatch or the route template isn't correct (which I've been snagged by many times).

I took a few of the existing functions and just added optional query parameters. It seemed to work just fine. I did not include the parameter in the EDM. I'm not sure if that is a requirement.

/// <summary>
/// Gets the sales tax for a postal code.
/// </summary>
/// <param name="postalCode">The postal code to get the sales tax for.</param>
/// <param name="extra">Something extra.</param>
/// <returns>The sales tax rate for the postal code.</returns>
[HttpGet( "api/GetSalesTaxRate(PostalCode={postalCode})" )]
[ProducesResponseType( typeof( double ), Status200OK )]
public IActionResult GetSalesTaxRate( int postalCode, string extra = null ) => Ok( 5.6 );
image
/// <summary>
/// Gets the new hires since the specified date.
/// </summary>
/// <param name="since">The date and time since people were hired.</param>
/// <param name="options">The current OData query options.</param>
/// <param name="extra">This is extra.</param>
/// <returns>The matching new hires.</returns>
/// <response code="200">The people were successfully retrieved.</response>
[HttpGet( "api/People/NewHires(Since={since})" )]
[Produces( "application/json" )]
[ProducesResponseType( typeof( ODataValue<IEnumerable<Person>> ), Status200OK )]
public IActionResult NewHires(
    DateTime since,
    ODataQueryOptions<Person> options,
    string extra = null ) => Get( options );
image
cfauchere commented 9 months ago

Thanks for the quick turn around. I believe it should be able to unblock our migration. I will reach out again for other issues/questions.

cfauchere commented 9 months ago

Most of my issues are addressed except Issue 2 I get a compile time error.

'ODataApiVersioningOptions' does not contain a definition for 'ModelBuilderFactory' and no accessible extension method 'ModelBuilderFactory' accepting a first argument of type 'ODataApiVersioningOptions' could be found (are you missing a using directive or an assembly reference?)

commonsensesoftware commented 9 months ago

Whoops! It should be:

options.ModelBuilder.ModelBuilderFactory = static () => new ODataConventionModelBuilder();

I've updated the comment. That's what happens when you try to do it all from memory. 😄

cfauchere commented 9 months ago

@commonsensesoftware been there done that :) Thanks for your help!

Preethi-Angel commented 9 months ago

Hi, I'm currently using the version 7.6.3 and want to migrate to 8+. Can anyone help me with the what are the changes need to be done in startup.cs and controllers and modelbuilders with respect to odata 8+ with Asp.versioning.Odata and Asp.Versioning.Odata.ApiExplorer please. I'm stuck and confused about where to start.

cfauchere commented 9 months ago

@Preethi-Angel Did you look at the examples?

Preethi-Angel commented 8 months ago

@cfauchere Yes I did, Thanks for sharing it. In the controllers, I get error on the lines where I have return ok or notfound or badrequest. Anyone encountered the same?

commonsensesoftware commented 8 months ago

@Preethi-Angel Can you provide some more context or examples. If you have code or a repo you can share, that would be best.

In terms of differences, moving from OData 7.x to 8.x there are significant changes there, even without API Versioning. OData has a new configuration that looks like:

services.AddOData();

The normal OData configuration would define EDMs and routes here. Unfortunately, that will not work with API Versioning. I tried coordinating with the OData team, but nothing came of it. The only thing you can define there are common query and routing options (ex: $ or not, parenthesis key /Entities(42) vs segment key /Entities/42). These options copied to all versioned routes.

API Versioning now centralizes its configuration around IApiVersioningBuilder. This makes all of the various extension points common and in one place. For example:

services.AddApiVersioning()
        .AddOData(options => options.AddRouteComponents("api"))
        .AddODataApiExplorer();

AddRouteComponents does not require a prefix. AddRouteComponents mirrors the built-in OData functionality in 8.x. ODataApiVersioningOptions.ModelBuilder is how to now access the VersionedODataModelBuilder. In earlier versions, you would have requested it directly though DI.

Aside from namespace changes, most other types, functionality, and behaviors are identical. There is no other change to IModelConfiguration. It is still discovered and configured as it was before.

Happy to provide additional assistant or explanation, but without some more specifics, I'm not sure where to steer you.

commonsensesoftware commented 6 months ago

This issue has gone idle and the OP seems to have been answered. If it hasn't, I'm happy to re-open the issue and discuss further.

@Preethi-Angel , it's not clear if you ever resolved your issue. If you haven't, consider opening a discussion or open an new issue if you think you've hit a roadblock.