Skip to content

[Java-server] Fix improper quoting of default values for query parameters#708

Merged
jmini merged 3 commits intoOpenAPITools:masterfrom
OmriHarary:fix_issue540
Aug 21, 2018
Merged

[Java-server] Fix improper quoting of default values for query parameters#708
jmini merged 3 commits intoOpenAPITools:masterfrom
OmriHarary:fix_issue540

Conversation

@OmriHarary
Copy link
Contributor

@OmriHarary OmriHarary commented Aug 1, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: master, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.
    cc: @bbdouglas @JFCote @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger

Description of the PR

Fixes issue described in #540 where non-enum string default values for query parameters were being surrounded with two sets of quotes so generated code would not compile.

Modified templates for spring, java-pkmst, java-msf4j, jaxrs-cxf, jaxrs-spec, jaxrs-cxf-cdi, jaxrs-cxf-client, jaxrs-jersey, and jaxrs-resteasy generators to only add double quotes around non-string and string enum default values, since AbstractJavaCodegen.toDefaultValue() already adds the quotes to non-enum string values.

The issue is also present in code generated by java-play-framework, but attempted fixes caused problems with non-query params and likely require more substantial changes.

Also fixes a related issue with java-vertx generation where the quotes on string default values were being HTML escaped, and string enum default values were not being quoted at all.

Copy link

@jeff9finger jeff9finger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK to me.

Conflicts:
	modules/openapi-generator/src/main/resources/JavaJaxRS/spec/queryParams.mustache
@jmini
Copy link
Member

jmini commented Aug 21, 2018

Thank you a lot, I have merged master into this PR and I have solved the conflict introduced by merge of #813.

If the CI is green I will merge this PR.

Tested locally with jaxrs-jersey and the spec derived from what is indicated in #540:

openapi: 3.0.0
info:
  title: Issue 540 example
  version: 1.0.0
paths:
  /foo:
    get:
      parameters:
        - name: bar
          in: query
          required: false
          schema:
            type: string
            default: foobar
      responses:
        default:
          description: successful operation
components:
  schemas:
    SomeObj:
      properties:
        property:
          type: integer
          format: int64

@jmini jmini merged commit ca0c35a into OpenAPITools:master Aug 21, 2018
@jmini jmini changed the title Fix improper quoting of default values for query parameters (#540) [java-server] Fix improper quoting of default values for query parameters (#540) Aug 21, 2018
@jmini jmini added this to the 3.2.2 milestone Aug 21, 2018
@wing328 wing328 changed the title [java-server] Fix improper quoting of default values for query parameters (#540) [Java-server] Fix improper quoting of default values for query parameters Aug 22, 2018
@wing328
Copy link
Member

wing328 commented Aug 22, 2018

@OmriHarary thanks for the fix, which is included in the v3.2.2 stable release: https://twitter.com/oas_generator/status/1032252335131512832

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.

4 participants

Comments