Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warning with some object that generate bad code #185

Closed
JFCote opened this issue May 30, 2018 · 6 comments
Closed

Warning with some object that generate bad code #185

JFCote opened this issue May 30, 2018 · 6 comments

Comments

@JFCote
Copy link
Member

JFCote commented May 30, 2018

Description

When I try to run the below spec, I receive warnings that creates bad codes. I don't understand what is the problem since my spec is valid in every validator I tried. I'm pretty sure it's related to the fact that the code is now generated with real inheritance, which was not the case in swagger-codegen (I think it was using composition only).

In the code, it will generate something like this for example:
DefaultMetaOnlyResponse logout(Object UNKNOWN_BASE_TYPE) throws Exception;

Here is the complete warning:

[main] WARN org.openapitools.codegen.DefaultCodegen - The folowing schema has undefined (null) baseType. It could be due to form parameter defined in OpenAPI v2 spec with incorrect consumes. A correct 'consumes' for form parameters should be 'application/x-www-form-urlencoded' or 'multipart/form-data'
[main] WARN org.openapitools.codegen.DefaultCodegen - schema: class ObjectSchema {
    class Schema {
        title: null
        multipleOf: null
        maximum: null
        exclusiveMaximum: null
        minimum: null
        exclusiveMinimum: null
        maxLength: null
        minLength: null
        pattern: null
        maxItems: null
        minItems: null
        uniqueItems: null
        maxProperties: null
        minProperties: null
        required: null
        type: null
        not: null
        properties: null
        additionalProperties: null
        description: Empty object used for POST or PUT that doesn't shouldn't have body but need one to pass play framework validation
        format: null
        $ref: null
        nullable: null
        readOnly: null
        writeOnly: null
        example: null
        externalDocs: null
        deprecated: null
        discriminator: null
        xml: null
    }
    type: object
    defaultObject: null
}
[main] WARN org.openapitools.codegen.DefaultCodegen - Defaulting baseType to UNKNOWN_BASE_TYPE
openapi-generator version

Master (3.0.0) updated to the latest commit

OpenAPI declaration file content or url

This is a 2 files spec:

swagger.yaml

swagger: '2.0'

info:
  version: "1.0.1"
  title: Control Site REST API
  description: blablabla
  termsOfService: www
basePath: /api
host: "http://localhost:9000"
consumes:
  - application/json
produces:
  - application/json
################################################################################
#                                   Parameters                                 #
################################################################################
parameters:
  empty-body:
    in: body
    name: empty-body
    required: true
    schema:
      $ref: "./definitions.yaml#/definitions/EmptyObject"
################################################################################
#                                 EndPoints                                    #
################################################################################
paths:
  /something:
    put:
      tags:
        - Foo
      summary: Update something
      operationId: update something
      parameters:
        - $ref: "#/parameters/empty-body"
      responses:
        200:
          description: OK
          schema:
            $ref: "./definitions.yaml#/definitions/DefaultMetaOnlyResponse"

definitions.yaml

definitions:
  EmptyObject:
    type: object
    description: "Empty object used for POST or PUT that doesn't shouldn't have body but need one to pass play framework validation"
  ResponseMeta:
    type: object
    description: "Mandatory part of each response given by our API"
    required:
      - code
    properties:
      code:
        type: string
        description: "Code returned by the function"
        default: Ok
        enum:
        - Ok
        example: Ok
  ################ Responses ##################
  DefaultMetaOnlyResponse:
    type: object
    required:
      - meta
    properties:
      meta:
        $ref: "#/definitions/ResponseMeta"
Command line used for generation
java -jar openapi-generator-cli.jar generate -i ../public/swagger.yaml --generator-name
java-play-framework -o generatedServer -DhideGenerationTimestamp=true
Steps to reproduce

Run the above command line with the spec and you will see the warning.

Related issues/PRs

Nothing that I know of

Suggest a fix/enhancement

I think it's related to the object EmptyObject which is used by the parameter empty-body. The warning is talking about "form" but as you can see, it is used with a body, not a form.

@cbornet cbornet added this to the 3.0.0 milestone May 30, 2018
@jmini
Copy link
Member

jmini commented May 30, 2018

For me this is not related to the Swagger-Parser. This is the output when you load the spec as OpenAPI-Generator loads it (and then serialized to Yaml):

openapi: 3.0.1
info:
  title: Control Site REST API
  description: blablabla
  termsOfService: www
  version: 1.0.1
servers:
- url: http://localhost:9000/api
paths:
  /something:
    put:
      tags:
      - Foo
      summary: Update something
      operationId: update something
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/EmptyObject'
        required: true
      responses:
        200:
          description: OK
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/DefaultMetaOnlyResponse'
components:
  schemas:
    EmptyObject:
      type: object
      description: Empty object used for POST or PUT that doesn't shouldn't have body
        but need one to pass play framework validation
    ResponseMeta:
      required:
      - code
      type: object
      properties:
        code:
          type: string
          description: Code returned by the function
          example: Ok
          default: Ok
          enum:
          - Ok
      description: Mandatory part of each response given by our API
    DefaultMetaOnlyResponse:
      required:
      - meta
      type: object
      properties:
        meta:
          $ref: '#/components/schemas/ResponseMeta'
  requestBodies:
    empty-body:
      content:
        application/json:
          schema:
            $ref: '#/components/schemas/EmptyObject'
      required: true

@jmini
Copy link
Member

jmini commented May 30, 2018

If your EmptyObject has some properties, then everything works fine.

components:
  schemas:
    EmptyObject:
      type: object
      description: Empty object used for POST or PUT that doesn't shouldn't have body
        but need one to pass play framework validation
      properties:
        someProp:
          type: string

The problem is that the Log WARNING and the default chosen name here:

LOGGER.warn("The folowing schema has undefined (null) baseType. " +
"It could be due to form parameter defined in OpenAPI v2 spec with incorrect consumes. " +
"A correct 'consumes' for form parameters should be " +
"'application/x-www-form-urlencoded' or 'multipart/form-data'");
LOGGER.warn("schema: " + schema);
LOGGER.warn("Defaulting baseType to UNKNOWN_BASE_TYPE");
codegenProperty.baseType = "UNKNOWN_BASE_TYPE";

might be not applying in your case. I imagine you would prefer to have Object object instead of Object UNKNOWN_BASE_TYPE in your code.


Correct me if I am wrong, but the generated code can be compiled and run.

@JFCote
Copy link
Member Author

JFCote commented May 30, 2018

Hi @jmini , thanks for the quick reply:

In swagger-codegen, it was generating code like this:

DefaultMetaOnlyResponse logout(EmptyObject emptyBody) throws Exception;

instead of

DefaultMetaOnlyResponse logout(Object UNKNOWN_BASE_TYPE) throws Exception;

As for the generated code, I simply created a very very simple way to reproduce the problem. It does indeed compile. But in my real production code, when generating the "interface", my code doesn't compile anymore since the interface is talking about Object and the implementation is using EmptyObject

I think we should not change the type and the name of the object.

@jmini
Copy link
Member

jmini commented May 30, 2018

Well these changes are side effects of decision that were taken during the migration, the analysis of other cases and some unfortunate refactoring.

Your point and your use case is really valid and this might induce that we need to rethink some points.


One example I can mention (without being able to reference a commit or a statement):

It was decided to not create model object if they extends primitive types (in Java classes of the java.lang package). If I remember well it was the cases with the OuterBoolean and other Outer classes created by the fake-petstore spec in the generated samples.

We might need to adjust this decision. Maybe it does not work well with the current java play-framework templates. Maybe we should create those model classes.

It is great that you bring use-cases (and regressions compared to swagger-codegen v2) in the issue tracker.

@JFCote
Copy link
Member Author

JFCote commented May 31, 2018

@jmini

In fact, the only reason I have an "EmptyObject" is because, in Play Framework, if you have an endpoint that uses a body (PUT and POST for example), it is mandatory. I don't know why and went in several discussion with the Play Framework team thinking it was a bug but it appears that this follow some RFC rules. Here is the discussion: playframework/playframework#7092 (The thread is quite misleading since I didn't know exactly where was the problem at the time)

To summarize, a POST or a PUT must have at least the basic json object in it {} which I provide by having an empty-body parameter that is implemented by a EmptyObject model. This model doesn't have any property and is rendered as an empty json object.

As for why do I have a POST or a PUT endpoint without body is simple. Some function do "reset" in the system or "start a task". Both of them don't need any parameters in the body.

Maybe it's simply a bad REST design but I'm pretty sure other people might have the same use case :)

As for the fake-petstore spec, I was supporting it 100% with swagger-codegen with my Java Play Framework generator, so I'm not sure why we needed to change anything in this matter.

For the moment, to be able to use openapi-generator, I will try to add a random property in my EmptyObject model and see if the rest of the generator is ok. I will follow further development here about this.

Thanks

@wing328 wing328 modified the milestones: 3.0.0, 3.0.1 Jun 1, 2018
@wing328 wing328 modified the milestones: 3.0.1, 3.0.2 Jun 11, 2018
@jmini jmini modified the milestones: 3.0.2, 3.0.3 Jun 18, 2018
@ackintosh ackintosh modified the milestones: 3.0.3, 3.1.0 Jun 27, 2018
@wing328 wing328 modified the milestones: 3.1.0, 3.1.1 Jul 6, 2018
@jmini jmini modified the milestones: 3.1.1, 3.1.2 Jul 18, 2018
@wing328 wing328 modified the milestones: 3.1.2, 3.2.0 Jul 25, 2018
@wing328 wing328 modified the milestones: 3.2.0, 3.2.1 Aug 6, 2018
@wing328 wing328 modified the milestones: 3.2.1, 3.2.2 Aug 14, 2018
@wing328 wing328 modified the milestones: 3.2.2, 3.2.3 Aug 22, 2018
@wing328 wing328 removed this from the 3.2.3 milestone Aug 30, 2018
@wing328 wing328 modified the milestones: 3.3.0, 3.3.1 Oct 1, 2018
@wing328 wing328 modified the milestones: 3.3.1, 3.3.2 Oct 15, 2018
@wing328 wing328 modified the milestones: 3.3.2, 3.3.3 Oct 31, 2018
@wing328 wing328 modified the milestones: 3.3.3, 3.3.4 Nov 14, 2018
@wing328 wing328 modified the milestones: 3.3.4, 4.0.0 Nov 30, 2018
@wing328 wing328 modified the milestones: 4.0.0, 4.0.1 May 13, 2019
@wing328 wing328 modified the milestones: 4.0.1, 4.0.2 May 31, 2019
@wing328 wing328 modified the milestones: 4.0.2, 4.0.3 Jun 20, 2019
@wing328 wing328 modified the milestones: 4.0.3, 4.1.0 Jul 9, 2019
@wing328 wing328 modified the milestones: 4.1.0, 4.1.1 Aug 9, 2019
@wing328 wing328 modified the milestones: 4.1.1, 4.1.2 Aug 26, 2019
@wing328 wing328 modified the milestones: 4.1.2, 4.1.3 Sep 11, 2019
@wing328 wing328 modified the milestones: 4.1.3, 4.2.0 Oct 4, 2019
@wing328 wing328 removed this from the 4.2.0 milestone Oct 30, 2019
@wing328
Copy link
Member

wing328 commented Jun 9, 2022

Should be fixed in v6.0.0. Please give it another try when you've time.

@wing328 wing328 closed this as completed Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants