-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Properly generate client code for string and array of string responses #3467
Conversation
See Github issues RicoSuter#2384 and RicoSuter#2596. Attempt to resolve both issues handling plain text and JSON lists by checking for the response's schema type. Treat as plaintext if schema type is string or marked as produces text/plain content.
!_response.Content.ContainsKey("application/json") && | ||
(_response.Content.ContainsKey("text/plain") || _operationModel.Produces == "text/plain"); | ||
(_response.Content.ContainsKey("application/json") && | ||
_response.Schema != null && _response.Schema?.Type == JsonObjectType.String) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that this is actually a hack for ASP.NET Core but from the perspective of OAI not correct... i.e. a response of type JSON and string should return "foo bar"
and not foo bar
- the problem is that ASP.NET Core (at least in older versions) returned a string and not a JSON string changing the type of the response from JSON to text/plain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ref: aspnet/Mvc#4945
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not aware of that specific hack for ASP.NET Core and I see how this change would not be a sufficient fix. It seems that in some locations throughout the codebase preference is given to "application/json" over "text/plain" for response types. Would it then be sufficient to select a response type of "application/json" when there are both "application/json" and "text/plain" are available? This would be a special case for strings.
JsonObjectType primitive = JsonObjectType.String | JsonObjectType.Integer | JsonObjectType.Number | JsonObjectType.Boolean; | ||
return ActualResponseSchema != null && | ||
(ActualResponseSchema.Type & primitive) == ActualResponseSchema.Type && | ||
(ActualResponseSchema.Type | primitive) == primitive; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the response schema only contains primitive data types, then the response can be considered plain text.
Superseded by #3535 |
See Github issues #2384 and #2596.
Attempt to resolve both issues handling plain text and JSON lists by
checking for the response's schema type. Treat as plaintext if schema
type is string or marked as produces text/plain content.