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

DefaultCodegen: fix UNKNOWN_BASE_TYPE and "Object" type problem #383

Merged
merged 2 commits into from
Jul 13, 2018

Conversation

JFCote
Copy link
Member

@JFCote JFCote commented Jun 22, 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, 3.1.x, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

This is to fix problem identified here: #185
I didn't run the shell scripts since it touch DefaultCodegen.java and I want the advice of the core team before I generate everything. The other reason is that I'm not sure how to re-generate ALL the samples. Will need your help with that :)

Feedback is welcome!

@wing328 (2015/07)
@jimschubert (2016/05)
@cbornet (2016/05)
@jaz-ah (2016/05)
@ackintosh (2018/02)
@JFCote (2018/03)
@jmini (2018/04)

@JFCote
Copy link
Member Author

JFCote commented Jun 22, 2018

I don't understand why the test are not passing since I've done the build with test on my computer without any problem.

[INFO] Reactor Summary:
[INFO] 
[INFO] openapi-generator-project .......................... SUCCESS [  1.946 s]
[INFO] openapi-generator (core library) ................... SUCCESS [ 41.471 s]
[INFO] openapi-generator (executable) ..................... SUCCESS [  6.532 s]
[INFO] openapi-generator (maven-plugin) ................... SUCCESS [  3.929 s]
[INFO] openapi-generator-gradle-plugin (maven wrapper) .... SUCCESS [  0.314 s]
[INFO] openapi-generator-online ........................... SUCCESS [  2.232 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 57.068 s
[INFO] Finished at: 2018-06-22T13:34:53-04:00
[INFO] Final Memory: 59M/620M
[INFO] ------------------------------------------------------------------------

@jmini jmini changed the title Fix for issue #185 DefaultCodegen: fix UNKNOWN_BASE_TYPE and "Object" type problem Jun 24, 2018
@jmini
Copy link
Member

jmini commented Jun 24, 2018

I am not sure why @wing328 added following code:

LOGGER.warn("The following 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";
codegenParameter.baseName = codegenProperty.baseType;
codegenParameter.baseType = codegenProperty.baseType;
codegenParameter.dataType = codegenProperty.dataType;
codegenParameter.description = codegenProperty.description;
codegenParameter.paramName = toParamName(codegenProperty.baseType);

See commit 23fc7a8: "rearrange how map, array, model are checked in body request"


Your change seems to have no impact on the samples code, because "Shippable" CI generate the most important samples and fail if there are differences with the code that is in the git repository.

This is highly possible, because the Petstore or the Fake-Petstore might not have the construct of an EmptyObject


I think it would be important to add a unit test to cover your use case.

We can use this small spec #185 (comment) (derived from your multiple files) as input.

@wing328
Copy link
Member

wing328 commented Jun 25, 2018

Previously, we discovered the issue that a 2.0 spec has the consumes set to application/json while form parameter(s) is used in that operation (however body parameter should be used instead to correctly describe a JSON payload). I added the warning above so that the user can fix the issue in the spec (which is still valid but not correctly describing the underlying API)

Does your spec have similar issue?

@jmini
Copy link
Member

jmini commented Jun 25, 2018

@wing328 : the case here is definitively an other one. If you take this valid OAS3:

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

Then you also get the warnings in the log and the generated code is:

DefaultMetaOnlyResponse logout(Object UNKNOWN_BASE_TYPE) throws Exception;

Instead of:

DefaultMetaOnlyResponse logout(EmptyObject emptyBody) throws Exception;

It works fine if the EmptyObject definition contains more properties:

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

This PR try to solve the case where EmptyObject contains no properties:

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

Maybe there is an other solution... I did not check in detail for now.

@wing328
Copy link
Member

wing328 commented Jul 9, 2018

I'll test it again tomorrow and merge into master if the test result is good.

@JFCote
Copy link
Member Author

JFCote commented Jul 9, 2018

Thanks @wing328

@wing328 wing328 merged commit 791f836 into master Jul 13, 2018
@wing328 wing328 deleted the fix-unknown-base-type branch July 14, 2018 08:42
@prathik457
Copy link

I am still getting this error after generating a python client. I am using the version 3.2.0

@ackintosh
Copy link
Contributor

@prathik457 Thanks for your reporting. Could you show me your OpenAPI spec file which reproduce the issue?

@wing328
Copy link
Member

wing328 commented Aug 9, 2018

Agreed with @ackintosh that we need a spec to reproduce the issue.

Your issue may be fixed by #736 (which will be merged later today)

@prathik457
Copy link

Oh, I realized that this PR is not released in the version 3.2.0.

@jmini
Copy link
Member

jmini commented Aug 13, 2018

Do you mean you have something fixed with 3.1.1 and not with 3.2.0? If so please open an issue with all the details.

@sturlath
Copy link

Is this supposed to be fixed?

I'm not sure if my issue is related, if not I´m sorry but I´m fairly new to this all (OpenApi/Yaml) but I get UNKNOWN_BASE_TYPE (see bottom for result) when trying to generate code from my mustach file with the following yaml file.

YAML

swagger: '2.0'
info:
  title: Swagger
  version: '1.0.0'
basePath: /api
schemes:
  - https
paths:
  /offers/add:
    post:
      tags:
        - offers
      operationId: addOffer
      consumes:
        - application/json
      produces:
        - application/json
        - application/xml
        - text/html
      parameters:
        - in: body
          name: body
          required: true
          schema:
            type: object
            items:
              $ref: '#/definitions/offerDTO'
      responses:
        '200':
          description: OK
          schema:
            type: array
            items:
              $ref: '#/definitions/offerDTO'
definitions:
  offerDTO:
    required:
      - id
    type: object
    properties:
      comment:
        type: string

mustach

[{{httpMethod}}]
[Route("{{{basePathWithoutHost}}}{{{path}}}")]
public IActionResult {{operationId}}({{#allParams}}{{>pathParam}}{{>queryParam}}{{>bodyParam}}{{>formParam}}{{>headerParam}}{{#hasMore}}, {{/hasMore}}{{/allParams}})
{
}

With the resulting UNKNOWN_BASE_TYPE

[HttpPost]
[Route("/api/offers/add")]
public IActionResult AddOffer([FromBody]UNKNOWN_BASE_TYPE UNKNOWN_BASE_TYPE)
{
}

Hope somebody can point me in the right direction for this holiday hack of mine :-)

@ackintosh
Copy link
Contributor

Hi @sturlath ,

The issue may has been fixed by #1751 , which is merged into master branch today. Could you please try with latest master? 💡

@sturlath
Copy link

If I use openapi-generator-cli-4.0.0-20181226.105224-120.jar from Snapshot (not the latest?) on the the yaml here above I get the following result with Object

    [HttpPost]
    [Route("/api/offers/add")]
    public IActionResult AddOffer([FromBody]Object body)
    {
    }

@sturlath
Copy link

Ok heads up, for some reason when I ran it again I get the right result.

[HttpPost]
    [Route("/api/offers/add")]
    public IActionResult AddOffer([FromBody]List<OfferDTO>offerDTO)
    {
    }

I did not change anything.. just ran it again. Probably something to do with my setup. I have a psi script that is run on post build in this project of mine.

p.s
Now I just need to find out why I get a list but that's probably easy.

@jimschubert
Copy link
Member

@sturlath your body parameter has the $ref defined as items, which is only used for arrays. This should just be:

      parameters:
        - in: body
          name: body
          required: true
          schema:
            $ref: '#/definitions/offerDTO'

A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
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.

None yet

7 participants