ProducesDefaultResponseType not behaving as expected

[agilenut]

I've noticed that with Pre-release 2, the ProducesDefaultResponseType doesn't seem to behave the way it did previously. I'm now getting this in SwaggerUI:

Previously, it looked like:

[commonsensesoftware]

I'm going to call this a 🐞 simply because it didn't do what you expected. There is a significant refactor in Preview 3 that will address this. Effectively, the API Versioning-specific code for this has been removed and things should now behave the same as any other Minimal API. You didn't previous state that this was for a Minimal API, but I presume that it must be.

Preview 3 is now available and contains the fix for this issue. Review the release notes and updated examples to see how things have changed.

[agilenut]

@commonsensesoftware - Sorry, you're correct. I didn't specify. This actually isn't for minimal apis. And, unfortunately, I just tried preview 3 and I still got the same behavior.

[commonsensesoftware]

😲

Do you have a repro or more details you can share? Are you saying there is some previous version of API Versioning where this didn't happen? The response type would have been generated by the intrinsic API Explorer implementation. The one and only case I can think of where that would be potentially different is OData, but even then, the media type wouldn't be changed. ProblemDetails shouldn't be affected.

With some additional context I can investigate further. Repros are always best so I don't have to guess at recreating the conditions.

[agilenut]

@commonsensesoftware - Sure. I don't have a public repro right now but I'll create one.

[agilenut]

@commonsensesoftware - Here is the repro.

Let me know if you have any questions.

[agilenut]

@commonsensesoftware - I'd be willing to put up a PR but I haven't found exactly what is making the new version behave differently than the old version. If you had an idea where to look, I could investigate further.

[commonsensesoftware]

Thanks for the repro. This will give me something to look at and compare against. Honestly, I can't think of a place where the response types would be changed or otherwise affected, but you are currently seeing some type of change.

If you want to take a peek or crack at the code, the place to start is VersionedApiDescriptionProvider.cs. That is the entry point for the API Explorer extensions. This class does 3 things:

1. Collate ApiDescription by API version
   a. If the same endpoint maps to multiple API versions, the ApiDescription is cloned
   b. This might be the cause
2. Add one or more ApiParameterDescription instances for ApiVersion
3. Substitute ApiVersion in the ApiDescription.RelativePath when SubstituteApiVersionInUrl = true

I'll work on carving out some time this week to look at this deeper. If you find anything, please share. I'll accept a PR as well.

[commonsensesoftware]

Ok. I found the problem. It's not that the response is changed per se, but descriptors are cloned for each API version so that they don't result in side effects across versions. You can see here that IsDefaultResponse is not cloned. Now, the very strange thing that I can't wrap my head around is that this is the exact same code from the original! I vaguely recalled running into a similar issue for BindingInfo, which wasn't in earlier versions (that was my clue). As properties have been added over the versions, I didn't realize it and missed cloning some of the properties. I'll have to see about how to invest in preventing this from happening in the future without relying on Reflection.

By making this one tiny change, Api.Works and Api.DoesntWork produce the same results. The only other thing that I wanted to check with you on was the media type. You showed application/problem+json, but this is not output in either of the repros you provided. Furthermore, looking at the original descriptors, this media type is not reported. It's unclear to me if that is the default behavior (which it seems to be) or if you have some customization that does the right thing. The fact that text/plain is reported seems to confirm that the ApiResponseFormats are being purely derived from the registered OutputFormatters.

This fix will be in the next release. I'm hesitant to change any default behaviors of the response formats, even if there is a more correct configuration because that could end up conflicting with futures changes in ASP.NET Core. If API Versioning is doing something wrong, I'm happy to fix it. If the default media types are wrong for this response, then the ASP.NET Core team should fix it. I'm willing to chime in and help drive that conversation should that be the case.

[commonsensesoftware]

The closed issue dotnet/aspnetcore#9324 seems to confirm that these are the default, expected media types. 😢

[agilenut]

It's unclear to me if that is the default behavior (which it seems to be) or if you have some customization that does the right thing.

Yes. The initial github issue was from my actual application. In that application, I have a custom swagger operation filter that looks for status codes >=400 or where it has a key of default and set the appropriate application/problem+json media type. One reason I noticed the issue is that the filter was not getting fired anymore for the results that were based on ProducesDefaultResponseType. When I created the repro, I left all that out to keep it simple.

I agree with you, I would not expect you to have to fix the mistake in aspnetcore for how it is handling the media types. It is enough for me if you can ensure that they are still marked as default so that it appears correctly in swagger and my filter will fire.

But, if you'd like to help drive the discussion, that would be fantastic. Here is a link to a newer version of this issue that I'm following. Supposedly they are doing some problem details work but it is not clear to me if this is going to make the cut. But if it did, I could remove this extra filter from every API. I also know others that are using a custom produces attribute to work around the underlying aspnetcore issue. I'm sure they'd like to ditch their custom version and use the built-in attribute if it worked correctly.