Skip to content

Conversation

@ePaul
Copy link
Contributor

@ePaul ePaul commented Mar 12, 2017

PR checklist

  • Read the contribution guidelines.
  • Ran the shell/batch script under ./bin/ to update Petstore sample so that CIs can verify the change.
    → bin/groovy-petstore.sh
  • Filed the PR against the correct branch: master for non-breaking changes

Description of the PR

This fixes (or hopes to fix) part of #4898 for the groovy generator.

The problem likely occurred when a query or header parameter name (as declared in the OpenAPI definition) is of a form which doesn't match the form used in Groovy, or is a reserved word there. In this case the parameter gets renamed for use in the code, and the parameter will be sent with the sanitized name instead of the original one.

This commit changes the replacement to using {{baseName}} (which is the original name of the parameter definition as parsed from the API definition) instead of {{paramName}} (which is the sanitized version of the parameter name).

Note that it looks like the Groovy generator has no support at all for path, form or body parameters – those are accepted by the generated code, but then ignored, and not passed to the server. I didn't try to fix this, as my Groovy knowledge is too rusty for this, but added some TODOs in the template. Related issue: #4289

Our Petstore sample API definition doesn't seem to contain any query parameter which would get modified by the sanitation, so the regenerated samples just have a single difference in a header parameter (which looks better now to me).

I don't know anything about Groovy, so this needs review by Groovy experts.

Paŭlo Ebermann added 2 commits March 12, 2017 20:39
This also adds some TODOs for the missing path, form and body parameters.
@ePaul ePaul force-pushed the fix-partially-4898-for-groovy branch from f59262d to bda0eff Compare March 12, 2017 19:39
@wing328 wing328 added this to the v2.2.3 milestone Mar 13, 2017
@wing328
Copy link
Contributor

wing328 commented Mar 13, 2017

@ePaul the fix looks good to me. I should be able to add the path parameter support.

@wing328 wing328 merged commit a2d1edc into swagger-api:master Mar 13, 2017
@wing328 wing328 changed the title Fix (partially) #4898 for groovy [Groovy] Fix parameter names with {{baseName}} Mar 13, 2017
@ePaul ePaul deleted the fix-partially-4898-for-groovy branch March 13, 2017 14:03
spr3nk3ls pushed a commit to spr3nk3ls/swagger-codegen that referenced this pull request Mar 28, 2017
* Fix (partially) swagger-api#4898 for groovy.

This also adds some TODOs for the missing path, form and body parameters.

* Update Groovy samples (after partial fix for swagger-api#4898)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants