swagger-codegen icon indicating copy to clipboard operation
swagger-codegen copied to clipboard

[JAVA][jaxrs-spec] Add support for enum as QueryParam argument type (#5075)

Open foxx1337 opened this issue 8 years ago • 6 comments

[JAVA][jaxrs-spec] Add support for generating type-safe query parameters (#5075)

All the logic is already in place so I just make sure parameters in:query also insert an enum in the generated API and that the type for the REST operation parameter is that enum (and not just String as it was before).

The code is somewhat ugly and its functionality is still limited: for example Container<Container<String>> param; (or member) won't work. I think it's enough for now to use this for the regular cases when only List<Integer> and List<String> are being passed around.

The state of the if-variable-then-variable if-notVariable-then-other is quite ludicrous now in enumClass.mustache and queryParams.mustache. It's probably a good idea to refactor that.

PR checklist

  • [x] Read the contribution guidelines.
  • [x] Ran the shell/batch 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)
  • [x] Filed the PR against the correct branch: master for non-breaking changes and 2.3.0 branch for breaking (non-backward compatible) changes.

Description of the PR

I've just looked at the generated files under samples/server/petstore/jaxrs-spec and it looks fine to me.

foxx1337 avatar Mar 16 '17 01:03 foxx1337

@jfiala - this is somehow out of the blue, but could you please have a look at what I did here and let me know your thoughts? I'm asking you because you're probably the best for it at this time :)

foxx1337 avatar Mar 23 '17 11:03 foxx1337

Once this goes in and I get to it, I'll create another pull request to annotate the code thus generated with @ApiParam(allowableValues="list,of,enum,constants"), just to ensure bijection is maintained between OpenAPI <-> Java generators.

foxx1337 avatar Mar 27 '17 22:03 foxx1337

I like the idea, but generally dislike inlined enums, please make sure to verify it with outer enums as they make much more sense regarding the generated code (see OuterEnum in petstore-with-fake-endpoints-models-for-testing.json). Additionally, the chinese characters in the generated swagger.json file are broken (???).

jfiala avatar Mar 28 '17 06:03 jfiala

I sent a new commit. Basically I want this:

paths:
  /health:
    put:
      operationId: putHealth
      tags:
        - Health
      produces:
        - text/plain
      parameters:
        - name: phoey
          in: query
          type: string
          default: foobar
          enum:
            - foobar
            - foobat
            - barbaz
          required: false

to turn into this

    @PUT
    @Produces({ "text/plain" })
    @ApiOperation(value = "", notes = "", response = void.class, tags={ "Health" })
    @ApiResponses(value = { 
        @ApiResponse(code = 201, message = "Null response", response = void.class),
        @ApiResponse(code = 200, message = "unexpected error", response = void.class) })
    public Response putHealth(@ApiParam(value = "", allowableValues="foobar, foobat, barbaz", defaultValue="foobar") @QueryParam("phoey")  @DefaultValue("foobar") PhoeyEnum phoey) {
        return Response.ok().entity("magic!").build();
    }

Now, regarding your comment with moving the enum outside of the the *Api class, I'm not sure it would work due to name clashes should different REST endpoints have identically named params. Even as it is right now, as internal class, there could be name clashes if different methods on the same endpoint (put, get, post, ...) end up defining params with the same names.

foxx1337 avatar Mar 28 '17 12:03 foxx1337

How would you suggest, @jfiala, I improve this?

Please also see my above comment - I believe that it makes little sense in this context moving the enums outside of the generated *Api classes, but I do want to see this change hitting master.

foxx1337 avatar Apr 25 '17 16:04 foxx1337

Any news about this?

iliassk avatar May 07 '23 17:05 iliassk