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

MissingMethodException with OData v8.1.0 #980

Closed
1 task done
AndreaCuneo opened this issue Mar 28, 2023 · 7 comments
Closed
1 task done

MissingMethodException with OData v8.1.0 #980

AndreaCuneo opened this issue Mar 28, 2023 · 7 comments

Comments

@AndreaCuneo
Copy link

AndreaCuneo commented Mar 28, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

After update Microsoft.AspNetCore.OData from v8.0.12 to v8.1.0, the following exception is thrown

'Microsoft.OData.ModelBuilder.Config.DefaultQuerySettings Microsoft.AspNetCore.OData.ODataOptions.get_QuerySettings()'

during Swagger generation, tied to ApiDescription.

System.MissingMethodException: Method not found: 'Microsoft.OData.ModelBuilder.Config.DefaultQuerySettings Microsoft.AspNetCore.OData.ODataOptions.get_QuerySettings()'.
at Asp.Versioning.ApiExplorer.ODataApiDescriptionProvider.ExploreQueryOptions(IEnumerable`1 apiDescriptions)
at Asp.Versioning.ApiExplorer.ODataApiDescriptionProvider.OnProvidersExecuted(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)

This happens on Asp.Versioning.OData.ApiExplorer v6.4.1.

    <PackageReference Include="Asp.Versioning.Http" Version="[6.4.0,7)" />
    <PackageReference Include="Asp.Versioning.Mvc.ApiExplorer" Version="[6.4.0,7)" />
    <PackageReference Include="Asp.Versioning.OData.ApiExplorer" Version="[6.4.1,7)" />

Expected Behavior

Work as per v8.0.12

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

6.0

Anything else?

ASP.NET Core v6

@commonsensesoftware
Copy link
Collaborator

Unfortunately, OData strikes again. 😞 I've lost track of the number of times they have broken things downstream. It's been multiple times since 5.x. There is little-to-no management of breaking changes. Honestly, I have no idea how they track it.

PR 835 introduces a breaking change to address OData/AspNetCoreOData#834. Specifically, the ODataOptions.QuerySettings has been removed (e.g. refactored). API Versioning was using that, which is why you get MissingMethodException. A minor version bump is supposed to be backward compatible and can only include publicly visible additions. The OData team rarely follows these rules. I've even heard feedback that some of the patch versions have resulted in breaking changes for people. API Versioning always clips off OData at the next major version for this very reason.

The immediate workaround is to use OData <= 8.0.12, which should continue work without issue. I need to investigate this more to figure out what the changes are and the appropriate course of action. Since OData breaks at 8.1.0, I will have to force API Versioning consumers to take this version too. That hasn't always worked for people in the past, which is why API Versioning only depends on 8.0.2. That was the minimal viable version and allowed people to decide if a patch version would break them. I have to think carefully about how to apply my next version bump. I'll probably have to take a minor version bump to ensure I can still patch 6.4.x with 8.0.x.

@AndreaCuneo
Copy link
Author

Thanks Chris. Indeed I could have noticed that given I've tracked it to the bump of OData library, a minor ...

I'm going to report this issue to OData project, maybe they are willing to release a Patch to restore the original method and flag 8.1.0 as deprecated on Nuget.

p.s. "OData strikes again" is a nice name metal band, or subtitle of "Scary Movie 6" :-D

@commonsensesoftware
Copy link
Collaborator

@AndreaCuneo I just wanted to check in. Is this actually blocking you? I'm pretty sure I know how to fix it, but it's unclear if the OData team is going to change anything. I followed the issue you posted and added some additional thoughts. I'm inclined to hold until we have a sense of whether the OData team will make a change or not. If they don't, then I think I need to bump the minor version so I can make sure I can service the two variations (e.g. 6.4.x vs 6.5.x).

@AndreaCuneo
Copy link
Author

@commonsensesoftware thanks for the concern.
I'm fine with this kept on hold waiting for some act on OData.

Atm there is no rush as we don't need any OData v8.1 fixes nor improvements in the Applications.
Application are in active development so issues may arise later on.

I see a potential risk on OData/AspNetCoreOData#824 and OData/AspNetCoreOData#859 but no issues yet.

In any case looks like they are going to keep the change in and at most add [Obsolete] for retrocompatibility so your update will be needed in any case.

@Jamess-Lucass
Copy link

Just leaving an update here, there is currently a PR open in AspNetCoreOData which adds this back in to avoid a breaking change. Stumbled into this issue myself and it's annoying semver was not properly followed. I sympathize.

@commonsensesoftware
Copy link
Collaborator

The changes looked good and it also seems a few other bugs were fixed. The PR is approved so I don't really know what the hold up is. Best as I can tell, the OData team doesn't use feature branches to control their releases (which makes sense for a larger team). They also don't seem to ever put out hot patches. The changes always go to main and press forward. That may be why there are delays in releases.

This change/fix should been published already IMHO. The current state of affairs is a landmine waiting to be stepped on. The longer the bad versions stay out there, the more people are likely to lose a limb. 😝 At least it's being fixed and is approved.

@commonsensesoftware
Copy link
Collaborator

This was ultimately external and I believe has been fixed. Thanks for reporting it and helping drive a comprehensive E2E solution.

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