[Swift] two fixes to latest param mapping code #4587
Conversation
…d out w/ rawValue to get the proper string name
jaz-ah
changed the title
| @@ -130,7 +130,7 @@ open class {{classname}}: APIBase { | |||
| {{#hasQueryParams}} | |||
| url?.queryItems = APIHelper.mapValuesToQueryItems(values:[ | |||
| {{#queryParams}} | |||
| "{{paramName}}": {{paramName}}{{#hasMore}}, {{/hasMore}} | |||
| "{{paramName}}": {{paramName}}{{#isEnum}}{{#isContainer}}{{{dataType}}}{{/isContainer}}{{^isContainer}}{{^required}}?{{/required}}.rawValue{{/isContainer}}{{/isEnum}}{{#hasMore}}, {{/hasMore}} | |||
There was a problem hiding this comment.
I believe it is now correct, but I'd address the readability of this fragment in the future.
What isContainer means by the way?
There was a problem hiding this comment.
y - I actually have to make one more fix here - this isn't all the way correct yet. let me see if I cleanup the readability as well.
| "enumQueryInteger": enumQueryInteger | ||
| "enum_query_string_array": enumQueryStringArray, | ||
| "enum_query_string": enumQueryString?.rawValue, | ||
| "enum_query_integer": enumQueryInteger?.encodeToJSON() |
There was a problem hiding this comment.
This works for me locally - looks like the correct change.
|
@jgavris did the new code work out for you? |
|
@jaz-ah We're actually running the 2.3.0 branch and there are conflicts with master at the moment. I might have to defer to someone else. |
C#, PHP, Python, Ruby and more are using fake petstore for testing. https://github.com/swagger-api/swagger-codegen/blob/master/bin/csharp-petstore.sh Is that what you're looking for? Totally agree with you about adding an additional test cases, if not too difficult, to cover this issue. |
|
@wing328 might be a little more difficult - I'd need to run testEnumParametersWithRequestBuilder and I need a different cased enum, and the the server to fail if the case of the enum isn't correct... unless you have an idea of a different way to test this one - I agree we should have something to make sure this doesn't break in the future. (also not sure why the build broke on this) |
|
@jaz-ah right, definitely not easy. I'll create a task to track various test cases we want to include. Petstore (fake or original) definitely is not the best spec for testing various test cases. Moving forward, we want to leverage one of the server stub generators to generate server code covering more scenarios. |
|
@wing328 not sure why this build isn't building though... (this succeeds for me) |
|
@jaz-ah I've restarted the jobs and the results now look good. PR merged into master. |
* two fixes: 1) extra ?'s at end of some url's 2) enums not being called out w/ rawValue to get the proper string name * update samples * one step closer * closer implementation
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates)2.3.0branch for breaking (non-backward compatible) changes.Description of the PR
extra ?'s at end of some url's
-if you had nil parameters passed into a function, then you would get an api call like:
http://foo.com?
enums not being called out w/ rawValue to get the proper string name
-if you defined an enum:
enum {
foo = "Foo"
}
the new mapping wouldn't call rawValue so you would get the lowercase foo instead of Foo