swagger-api / swagger-codegen

swagger-codegen contains a template-driven engine to generate documentation, API clients and server stubs in different languages by parsing your OpenAPI / Swagger definition.
http://swagger.io
Apache License 2.0
16.77k stars 6.02k forks source link

[swift] query params on put get ignored if URL is a type #2978

Open romk1n opened 8 years ago

romk1n commented 8 years ago
Description

If the endpoint takes a query param in PUT request the alamofire ignores the parameters

Swagger-codegen version

2.1.6

Swagger declaration file content or url

https://github.com/swagger-api/swagger-codegen/blob/master/samples/client/petstore/swift/default/PetstoreClient/Classes/Swaggers/AlamofireImplementations.swift#L31

Command line used for generation
Steps to reproduce
Related issues

I reported a bug in Alamofire as well https://github.com/Alamofire/Alamofire/issues/1275

Suggest a Fix

use Alamofire.ParameterEncoding.URLEncodedInURL ?

wing328 commented 8 years ago

cc @Edubits @jaz-ah

jaz-ah commented 8 years ago

@romk1n not sure - that suggestion may work but looks like the alamofire behavior is as designed - looks like the swagger implementation has a bug - do you have cycles to try the suggestion and test?

romk1n commented 8 years ago

@jaz-ah alamofire confirmed that we must use URLEncodedInURL. locally i changed that and it is working for us.

romk1n commented 8 years ago

it's looks like there was an issue about it already https://github.com/swagger-api/swagger-codegen/issues/1564

wing328 commented 8 years ago

@romk1n thanks for the update. May I know if you've cycle to submit a PR for the fix?

romk1n commented 8 years ago

@wing328 are there anywhere instruction on what tests i should run and how to do it? i can try, although when i modified the file on my local copy it got overridden although i have modified in /src/main/...

wing328 commented 8 years ago

https://github.com/swagger-api/swagger-codegen/blob/master/CONTRIBUTING.md is a good starting point for making contribution.

For swift/objc API client, it's not covered by the travis-ci at the moment. You can follow the steps below to run the test locally:

(after you update the files locally)
mvn clean package
./bin/swift-petstore-all.sh
cd samples/client/petstore/swift/default
mvn integration-test

If you need further assistance, please let me know.

wing328 commented 8 years ago

@romk1n do you need help testing the change in the Swift API client locally in your machine?

romk1n commented 8 years ago

@wing328 yup, i am getting this error when running integration-test from /Users/romk1n/devel/github/swagger-codegen-new/samples/client/petstore/swift/default

[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 0.080 s
[INFO] Finished at: 2016-07-07T13:14:35-07:00
[INFO] Final Memory: 5M/245M
[INFO] ------------------------------------------------------------------------
[ERROR] The goal you specified requires a project to execute but there is no POM in this directory (/Users/roman/devel/github/swagger-codegen-new/samples/client/petstore/swift/default). Please verify you invoked Maven from the correct directory. -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MissingProjectException

here is my branch https://github.com/romk1n/swagger-codegen/tree/fix/2978

wing328 commented 7 years ago

@romk1n please file a PR so that we can review the fix.