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

[BUG] DefaultCodeGen doesn't recurse into nested types when looking for imports #11220

Open
5 tasks done
eak24 opened this issue Jan 3, 2022 · 3 comments
Open
5 tasks done

Comments

@eak24
Copy link
Contributor

eak24 commented Jan 3, 2022

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
Description

When specifying schemas that have nested types (for instance, Map<String, Map<String, MyType>>), the MyType custom type doesn't get added to the list of imports and so all the generated clients fail to resolve the MyType symbol in the related API. I noticed this with Python Client, but I believe it would effect others as well.

openapi-generator version

5.3.1

OpenAPI declaration file content or url
openapi: 3.0.1
info:
  title: Test additional properties with ref
  version: '1.0'
servers:
  - url: 'http://localhost:8000/'
paths:
  /ping:
    post:
      operationId: ping
      responses:
        default:
          description: default response
          content:
            application/json:
              schema:
                type: object
                additionalProperties:
                  type: object
                  additionalProperties:
                    type: object
                    additionalProperties:
                      "$ref": "#/components/schemas/Person"
components:
  schemas:
    Person:
      type: object
      properties:
        lastName:
          type: string
        firstName:
          type: string

Also here

Generation Details

A simple generation command, like openapi-generator generate -g python will make a Python client that fails during symbol resolution because Person won't be imported in default_api.py.

Steps to reproduce

Assuming pipenv and openapi-generator-cli are both installed and on the path...

openapi-generator-cli generate -g python -i https://gist.githubusercontent.com/ethan92429/07a09ac228dcd2f9ec0b957ddb44148b/raw/074beff13727280dd8119c3547ac36d28e58559b/openApi_generator_issue_11220.yaml &&
pipenv install -r requirements.txt &&
pipenv install -r test-requirements.txt &&
pytest --cov=openapi_client
Related issues/PRs

#10279

Suggest a fix

Working on this...

@mwilby
Copy link

mwilby commented Jan 4, 2022

Its not just the import library that's missing, its also the type information (at least in pyhon-flask, although I suspect its everything.)

This does not show up until the object is loaded. It will write the object OK, but when a serialized version is loaded, it causes a runtime error on the type information.

For example

vector:               
  type: array
  items:
  type: number
  format: float
  minItems: 2
  maxItems: 2
description: image vector in the format [x,y]
polygon:
  type: array
  items:
  $ref: '#/vector'
path:
  $ref: '#/polygon'
  description: Path in some object.

path represents some property in an object, it will be defined as

        self.openapi_types = {
            ...,
            'path': List[List],
            ...
        }

when it should be

        self.openapi_types = {
            ...,
            'path': List[List[float]],
            ...
        }

The List[List] structure is in the json document sent to mustache, so its probably caused by a similar recursive failiure.

eak24 added a commit to eak24/openapi-generator that referenced this issue Jan 7, 2022
@mwilby
Copy link

mwilby commented Jan 10, 2022

I thought I it would be a good idea to create a better and extended test for the type problem, so I built the following set of test cases

$schema: http://json-schema.org/draft-07/schema#
vector: 
  type: array
  items:
    type: number
    format: float
    minItems: 2
    maxItems: 2
  description: image vector in the format [x,y]
polygon: 
  type: array
  items:
    $ref: '#/vector'
ext_polygon:
  type: array
  items:
    type: array
    items:
      $ref: '#/vector'
raw_polygon:
  type: array
  items:
    type: array
    items:
      type: number
      format: float
      minItems: 2
      maxItems: 2
ext_raw_polygon:
  type: array
  items:
    type: array
    items:
      type: array
      items:
        type: number
        format: float
        minItems: 2
        maxItems: 2
shape:      # Can be referenced via '#/shape'
  type: object
  description: path element
  properties:
    path:
      $ref: '#/polygon'
    ext_path:
      $ref: '#/ext_polygon'
    raw_path:
      $ref: '#/raw_polygon'
    ext_raw_path:
      $ref: '#/ext_raw_polygon'

I did the following tests on 5.3.0, but the problem goes back at least as far as 4.3, probably it goes back a lot earlier.

The python generator is close, but still gets the type of ext_polygon wrong. However, in this case it knows its made a mistake and warns you by generating the message

[main] ERROR o.o.codegen.DefaultCodegen - String to be sanitized is null. Default to ERROR_UNKNOWN
[main] ERROR o.o.codegen.DefaultCodegen - Error in generating `example` for the property ext_polygon. Default to ERROR_TO_EXAMPLE_VALUE. Enable debugging for more info.

The resultant type is

    @cached_property
    def openapi_types():
        lazy_import()
        return {
            'value': ([[ERRORUNKNOWN]],),
        }

This is the best attempt of the generators I have tried so far, so its probably the best place to look for a possible solution.

The python-flask generator is much worse, getting both the polygon and the ext_polygon wrong and it is silent, reporting no errors. It generates the following type information for the shape object.

        self.openapi_types = {
            'path': List[List],
            'ext_path': List[List[object]],
            'raw_path': List[List[float]],
            'ext_raw_path': List[List[List[float]]]
        }

It should read:

        self.openapi_types = {
            'path': List[List[float]],
            'ext_path': List[List[List[float]]],
            'raw_path': List[List[float]],
            'ext_raw_path': List[List[List[float]]]
        }

I actually came across this using the rust generators, where both options produce odd results. My understanding of Rust and the associated generators is limited, so my interpretation may be suspect. The rust generator produces:

pub struct Shape {
    #[serde(rename = "path", skip_serializing_if = "Option::is_none")]
    pub path: Option<Vec<crate::models::Array>>,
    #[serde(rename = "ext_path", skip_serializing_if = "Option::is_none")]
    pub ext_path: Option<Vec<Vec<serde_json::Value>>>,
    #[serde(rename = "raw_path", skip_serializing_if = "Option::is_none")]
    pub raw_path: Option<Vec<Vec<f32>>>,
    #[serde(rename = "ext_raw_path", skip_serializing_if = "Option::is_none")]
    pub ext_raw_path: Option<Vec<Vec<Vec<f32>>>>,
}

There is no models::Array and the generalized serde_json::Value is consistent with the object generated with python-flask.

The rust-server, produces

pub struct Shape {
    #[serde(rename = "path")]
    #[serde(skip_serializing_if="Option::is_none")]
    pub path: Option<Vec<models::Vector>>,

    #[serde(rename = "ext_path")]
    #[serde(skip_serializing_if="Option::is_none")]
    pub ext_path: Option<Vec<Vec<#Vector>>>,

    #[serde(rename = "raw_path")]
    #[serde(skip_serializing_if="Option::is_none")]
    pub raw_path: Option<Vec<Vec<f32>>>,

    #[serde(rename = "ext_raw_path")]
    #[serde(skip_serializing_if="Option::is_none")]
    pub ext_raw_path: Option<Vec<Vec<Vec<f32>>>>,

}

The errors are not so similar to those seen in either python case, it looks OK for the path object, but it does not produce the models::Vector struct, and honestly I don't know what its trying to do with #Vector, so both path and ext_path are evaluated incorrectly.

This confirms the problem runs across languages, but because of the differences I am not sure if there is a common cause, or if there is, there are additional errors.

It might be related to the method getPrimitiveType in DefaultCodegen, where

        ...
        } else if (ModelUtils.isMapSchema(schema)) {
            return "map";
        } else if (ModelUtils.isArraySchema(schema)) {
            return "array";
        }
        ...

simply assigns the container type and does not look any deeper, but I don't know enough to know if this is anything to do with it.

spacether pushed a commit that referenced this issue Jan 14, 2022
#11221)

* recursively search for types during import type collection in deeply nested schemas #11220

* composed schema recursive type import handling

* change Importable to ComplexType to modularize related type-wrangling.

* Problems...

* Revert "Problems..."

This reverts commit 6154f202a0f4db7a706fe3d61b45573581d164d4.

* Reverted attempts to reuse recursive type-finding code.

* add javadoc comments

* fix javadoc warning.

* fix NPE

* PR feedback incorporation

* Include additionalProperties recursion.

* More feedback

* More feedback

* Add comments from feedback
@FreekBes
Copy link

Was this fixed in #11221? Issue is still open.

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

3 participants