-
Notifications
You must be signed in to change notification settings - Fork 6k
[Swift] responses: schema: type: object --> RequestBuilder<Inline_response_200> ? #2093
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
Comments
You define the response as "object" without specifying the definition of the "Object" (e.g. properties). Here is an example: (there are a few PRs for Swift merged into master so would also recommend you to pull the latest) |
@wing328 thanks for getting back to me this quickly! Yes, for those which has |
Can you elaborate a bit on the plain JS object by providing an example of the HTTP body? Is it like |
Sure, it is like |
Would |
No, I'm afraid not. The value part could be any type of JS value (not limited to "a simple string to string mapping"). |
Given that the HTTP response body can be any JSON object, what about modelling your response as |
The thing is the server side specification cannot be changed that easily since it is not just a server but a framework (https://github.com/strongloop/loopback) which already has users. Some of the methods it provides return an |
From reading Swagger spec and JSON Schema Validations, I believe it is allowed to specify I think our intent "an object with unspecified properties" would be best described using the following Swagger: schema:
type: object
additionalProperties: true
Now I am not familiar with swagger-codegen code base at all, but perhaps |
I would eventually like to improve our swagger spec generator to correctly describe the response format (instead of using a generic "object" type), see strongloop/loopback-swagger#27. That would solve this particular issue for us. However, I thought it would be good to report the issue to this project and get it fixed here too, as other users may run into the same problem. |
@hideya for your question below
I think the answer is yes. Given that no $ref, swagger codegen will automatically create a model (e.g. Inline_response_200) |
@wing328 thanks for getting bak to me! Would you have any comments on what @bajtos was describing above? I also checked out Swagger Spec. My take is: type: object
required:
- name
properties:
name:
type: string
address:
$ref: '#/definitions/Address'
age:
type: integer
format: int32
minimum: 0 |
Yes, ideally one should define the properties for a model as the properties should be known to the spec owner. |
@wing328 thanks for the confirmation. |
As far as I understand the spec, property definition is optional, as can be seen in Model with Map/Dictionary Properties. That leads me to conclusion that the following definition is a valid swagger: schema:
type: object
additionalProperties: true To be clear, I am not saying that swagger-codegen has to support such schemas (maps/dictionaries). As far as LoopBack is concerned, we really should fix our swagger generator to describe response properties, since we have that metadata available (strongloop/loopback-swagger#27). |
I think I understood the situation, and am closing this as not an issue. Swagger spec should have been like the following for such a case where server responds an object like responses:
'200':
description: Request was successful
schema:
type: object
required:
- count
properties:
count:
type: number
[coffeeShopApi coffeeShopCountWithWhere:nil completionHandler:^(XXInlineResponse2001 *output, NSError *error) {
NSLog(@"count: %@", output.count);
}]; If we don't list any |
@hideya @bajtos My apologies for late reply on this. For the following:
Yes, property is optional (and we've seen a lot of cases in which there's no property defined for For |
@wing328 yes, that is something I initially wanted! Sounds really good to me! |
@wing328 I think I can check that out this coming weekend. Is it too late? |
Take you time. We'll of course perform testing in our side as well. |
@wing328 I quickly tested the latest master, and found an issue with Swift. If an argument's schema is /Users/login:
post:
tags:
- User
summary: Login a user with username/email and password.
description: !<tag:yaml.org,2002:js/undefined> ''
operationId: User.login
parameters:
- name: credentials
in: body
description: !<tag:yaml.org,2002:js/undefined> ''
required: true
schema:
type: object
- name: include
in: query
description: Related objects to include in the response. See the description of return value for more details.
required: false
type: string
format: JSON
responses:
... The following generated method causes a syntax error public class func userLoginWithRequestBuilder(credentials credentials: AnyObject, include: String?) -> RequestBuilder<AnyObject> {
let path = "/Users/login"
let URLString = CoffeeShopClientAPI.basePath + path
let parameters = credentials.encodeToJSON() as? [String:AnyObject]
let requestBuilder: RequestBuilder<AnyObject>.Type = CoffeeShopClientAPI.requestBuilderFactory.getBuilder()
return requestBuilder.init(method: "POST", URLString: URLString, parameters: parameters, isBody: false)
} It used to generate public class Credentials: JSONEncodable {
public init() {}
// MARK: JSONEncodable
func encodeToJSON() -> AnyObject {
var nillableDictionary = [String:AnyObject?]()
let dictionary: [String:AnyObject] = APIHelper.rejectNil(nillableDictionary) ?? [:]
return dictionary
}
} What needs to be done I think is that, instead of simply saying Does the above explanation make sense? |
For PHP, I need to set the type of the data to I think Swift may need to do something similar. I'll test C# API client later to see if similar changes need to be done. Java and Ruby have been updated to handled |
@hideya I've submitted #2317 for C#, which returns For Swift, I think it comes down to how Alamofire handles the deserialization if I'm not mistaken. There's a discussion related to serialization/deserialization of Object in Swift here Please let me know if you've any questions. |
@hideya do you need any help with the change for Swift? |
@wing328 yeah, I decided to change my job and having a bit of difficulty to spare time on this... sorry for not being able to be helpful... |
@hideya no problem. The community will help work on this. All the best with your new role. |
I came across InlineResponse200 when I decided to take a "shortcut" and define response right in the operation definition. I was thinking about giving more proper name for generated object, for example, based on "title" property (if provided). Do you think it may make sense? |
@tomekc I would suggest you to make your suggestion via https://github.com/OAI/OpenAPI-Specification/issues as the OpenAPI core team is working on the next version of OpenAPI/Swagger spec. For the time being, please define the model properly to set a proper name for the model. |
@tomekc Regarding "title". I have opened issue to use "title", same way as Swagger UI using it. Use "Title" as model name if provided in swagger definition. Please support Implementation code is ready too, I'll send pull request for approval |
When I feed an API with the following responses definition:
the codegen generates something like the following client-side code:
where I don't understand how to handle the returned
object
value...?I would expect an plain object is mapped to a dictionary so that access key-value pairs can be read...
Would you kindly tell me if I'm mistaking something?
When I tried Obj-C and it also behaves in a similar manner:
So, there might be some design decision already discussed and it has been decided to generate
Inline_response_200
thing forobject
response type. But I still don't see how to handle the returned object. Any help would be much appreciated! (@bajtos wanted to keep informed about this too)The text was updated successfully, but these errors were encountered: