The code that adds media types in MvcCoreBuilderExtensions.cs mean the response for JSON will ALWAYS will return application/vnd.restful+json

[VictorioBerra]

Which project template is the issue about?
API
...

Which version of the project template is the issue about?

...

Which version of Visual Studio or the dotnet SDK were you using?
2.2.203
...

If you call the API with a client and you set Accept to application/json it is completely ignored and you will ALWAYS get application/vnd.restful+json back.

Is this desired? This does not seem like it should be that way.

https://github.com/Dotnet-Boxed/Templates/blob/master/Source/Content/ApiTemplate/Source/ApiTemplate/MvcCoreBuilderExtensions.cs#L94

[VictorioBerra]

The reason I think it should not be this way is that if you removed the second output formatter and just left jsonOutputFormatterMediaTypes.Insert(0, ContentType.ProblemJson); you ALWAYS get "application/problem+json" back. Even with a 200OK.

I also tried setting Content-Type to no avail.

[RehanSaeed]

I'm not actually certain what the correct behaviour should be. If you request json and we send a specific subset of json, I'm not certain that is incorrect. Do you have some references detailing the correct behaviour? Kind Regards, Muhammad Rehan Saeed RehanSaeed.com From: Victorio Berra <[email protected]> Sent: Wednesday, June 12, 2019 10:51:55 PM To: Dotnet-Boxed/Templates Cc: Subscribed Subject: [Dotnet-Boxed/Templates] The code that addfs media types mean they ALWAYS will return `application/vnd.restful+json` (#333) Which project template is the issue about? API ... Which version of the project template is the issue about? ... Which version of Visual Studio or the dotnet SDK were you using? 2.2.203 ... If you call the API with a client and you set Accept to application/json it is completely ignored and you will ALWAYS get application/vnd.restful+json back. Is this desired? This does not seem like it should be that way. https://github.com/Dotnet-Boxed/Templates/blob/master/Source/Content/ApiTemplate/Source/ApiTemplate/MvcCoreBuilderExtensions.cs#L94 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<https://github.com/Dotnet-Boxed/Templates/issues/333?email_source=notifications&email_token=AARKJHAEAVVDH6JAIEMCMYTP2FVXXA5CNFSM4HXS3DB2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GZFPD6Q>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AARKJHF4FKHC5B6V6D37SL3P2FVXXANCNFSM4HXS3DBQ>.

[VictorioBerra]

@RehanSaeed I am not sure exactly I need to study "content negotiation" a little better. I assumed you should always get back what you said you accept. Getting problem json for a 200ok seems wrong.

[VictorioBerra]

Also, I think that since those are content subtypes (+json) you do not need to actually add them. khellang/Middleware#54 (comment)

[RehanSaeed]

I read @khellang's comment in khellang/Middleware#54 (comment). If I've understood correctly, there isn't much else we can do. If so, can we close?

[VictorioBerra]

I vote that we remove all of this

https://github.com/Dotnet-Boxed/Templates/blob/master/Source/ApiTemplate/Source/ApiTemplate/MvcCoreBuilderExtensions.cs#L87-L94

[RehanSaeed]

Yeah, maybe. I feel like I should write a few unit tests for each scenario. Here is what I think currently happens or should happen. Not sure if any of this is correct. I'd love to find some time to find documentation or maybe we should ask a StackOverflow question and farm out that work.

• No Accept - Success returns: application/vnd.restful+json

• Accept: application/json - Success returns: application/vnd.restful+json

• Accept: application/vnd.restful+json - Success returns: application/vnd.restful+json

• No Accept - 400 returns: application/problem+json

• Accept: application/json - 400 returns: application/problem+json

• Accept: application/vnd.restful+json - 400 returns: application/problem+json

[VictorioBerra]

Yes that is what I would expect, maybe:

Accept: application/json - Success returns: application/json

I say that because i think we should be writing the least amount of extra boilerplate as possible. I would like to use whatever MVC Core uses out of the box.

[RehanSaeed]

That is the one I'm just not sure about and would need to see some evidence to back up.

Agree on writing the minimum boilerplate.

[RehanSaeed]

I messed with input and output formatters and wrote more integration tests for the ASP.NET Core 3.0 update. Please check out the netcore3 branch and comment.

[VictorioBerra]

The input/output formatters still look roughly the same right? They will still always return whatever you insert at the 0 index. I do like the test assertions for the problem details, I think that is helpful.

[RehanSaeed]

@VictorioBerra Did you manage to take a look at the tests?

One solution to the Swagger UI selecting an incorrect MIME type in the drop down is to add the following to the controllers. However, it's annoying to have to add this to all controllers. You could add it globally but sometimes you might want to use other MIME types, so I don't think thats a good idea.

[Produces("application/application/vnd.restful+json")]
[Produces("application/problem+json")]

This has been on my mind for a while. I need to experiment with it to see what to do. Feel free to do so yourself.

[VictorioBerra]

@RehanSaeed You just mean these content type checks you made on the tests right? https://github.com/Dotnet-Boxed/Templates/blob/master/Source/ApiTemplate/Tests/ApiTemplate.IntegrationTest/Controllers/CarsControllerTest.cs#L107

I think the way this works is fine for now. Id leave the Input/OutputFormatterMediaTypes stuff in the MvcBuilderExtensions as they are.

For the Swagger UI thing, that is definitely annoying.

I would also agree not to add those to every single controller, if you add globally is there a way to make that configurable in appsettings maybe? It seems like Swagger with force us to manually put it somewhere.

[RehanSaeed]

I wouldn't want to put it in globally as an MVC filter. That will cause problems when someone wants to respond with some other MIME type. Only place is on the Controller. I also forgot to mention that we'd need to also add [Consumes()] in a similar way.

[VictorioBerra]

I suppose its necessary then. Its too bad we cant add them to something like the new API conventions feature.

[RehanSaeed]

Yes, that might be one way to do it.

[RehanSaeed]

The MIME types in Swashbuckle are all over the place.

Adding [Produces(...)] and [Consumes(...)] all over the place or in the new API conventions is the only fix I can see currently. However, that is still not going to show the correct MIME type for each response example.

I think the ideal scenario is if SwaggerResponseAttribute could also take a MIME type:

[SwaggerResponse(StatusCodes.Status201Created, "The car was created.", typeof(Car), "application/vnd.restful+json")]
[SwaggerResponse(StatusCodes.Status400BadRequest, "The car is invalid.", typeof(ProblemDetails), "application/problem+json")]

Raised domaindrivendev/Swashbuckle.AspNetCore#1691 to propose this.

[RehanSaeed]

Also wondering if I should stop using comments on controllers and switch completely to attributes.

[VictorioBerra]

I think the comments can provide more info than the attributes. Also, read this domaindrivendev/Swashbuckle.AspNetCore#816 (comment)

[RehanSaeed]

Nice. You're one step ahead of me.

[RehanSaeed]

Still no answer in domaindrivendev/Swashbuckle.AspNetCore#1691

[VictorioBerra]

I bet the change is a ton of work for them, especially since the UI stuff is by a different team I think.

…

On Wed, Nov 4, 2020, 10:51 AM Muhammad Rehan Saeed ***@***.***> wrote: Still no answer in domaindrivendev/Swashbuckle.AspNetCore#1691 <domaindrivendev/Swashbuckle.AspNetCore#1691> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/Dotnet-Boxed/Templates/issues/333#issuecomment-721813077>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAWMN26YRJ7QY4EGD55JGT3SOF2ARANCNFSM4HXS3DBQ> .

[RehanSaeed]

I'm not sure we're going to get an answer any time soon. On the other hand, I don't want to add [Produces(...)] and [Consumes(...)] to every controller action as that is kind of overkill and easy to get wrong when you consider exceptions.

[VictorioBerra]

Yeah, you could leverage the new API conventions but your point still stands. There would be exceptions. https://docs.microsoft.com/en-us/aspnet/core/web-api/advanced/conventions?view=aspnetcore-3.1