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][Python] Wrong generated Type tree for Inline response schema #8743

Closed
5 of 6 tasks
uncleflo opened this issue Feb 18, 2021 · 10 comments
Closed
5 of 6 tasks

[BUG][Python] Wrong generated Type tree for Inline response schema #8743

uncleflo opened this issue Feb 18, 2021 · 10 comments
Labels
Client: Python Inline Schema Handling Schema contains a complex schema in items/additionalProperties/allOf/oneOf/anyOf Issue: Bug

Comments

@uncleflo
Copy link

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?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

I am trying to retrieve the data from a path using the Python-generated code from an openapi spec file. The python code generated returns a ApiTypeError. When investigating deeper, it seems the openapi spec file is not read correctly, or is missing some types in the inline type checking. Separately, the _check_return_type=False does not seem to work to skip this step.

openapi-generator version

Both 5.0.0 and 5.0.1 have been tried

OpenAPI declaration file content or url
{
  "swagger": "2.0",
  "info": {
    "title": "Quick Base API",
    "version": "1.0.0"
  },
  "host": "api.quickbase.com/v1",
  "paths": {
    "/fields/{fieldId}": {
      "parameters": [ ... ],
      "get": {
        "description": "Gets the properties of an individual field, based on field id.",
        "parameters": [ ... ],
        "responses": {
          "200": {
            "description": "Success",
            "schema": {
              "type": "object",
              "properties": {
                "id": { ... },
                "fieldType": { ... },
                "label": { ... },
                "properties": {
                  "description": "Additional properties for the field. ",
                  "type": "object",
                  "additionalProperties": true,
                  "properties": {
                    "comments": { ... },
                    "format": { ... },
                    "maxLength": { ... },
                    "allowNewChoices": { ... },
                    "displayAsLink": { ... },
                    "allowHTML": { ... },
                  }
                },
              }
            }
          }
        },
      },
    },
  },
}
Generation Details

java -jar openapi-generator-cli.jar generate
-i oas.json
-o quickbase-openapi-client
-g python

Steps to reproduce

As above

Related issues/PRs

Checked, not found

Suggest a fix

In file:
https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java

In function:
private String getTypeString(Schema p, String prefix, String suffix, List referencedModelNames)

This seems to result with the above oas and generation into: fields_api.py

...
        self.get_field = Endpoint(
            settings={
                "response_type": ( {
                        str: ( {
                                str: (bool,date, datetime, dict, float, int, list, str, none_type, )
                            }, )
                    }, ),
                "auth": [],
                "endpoint_path": "/fields/{fieldId}",
                "operation_id": "get_field",
                "http_method": "GET",
                "servers": None,
            },
            params_map={
...

...But this returns a ApiTypeError: Invalid type for variable 'id'. Required value type is dict and passed type was int at ['received_data']['id']

However, when the above is changed to:

        self.get_field = Endpoint(
            settings={
                "response_type": ( {
                        str: ( 
                            bool,date, datetime, dict, float, int, list, str, none_type,
                            {
                                str: (bool,date, datetime, dict, float, int, list, str, none_type, )
                            }, )
                    }, ),
                "auth": [],
                "endpoint_path": "/fields/{fieldId}",
                "operation_id": "get_field",
                "http_method": "GET",
                "servers": None,
            },
            params_map={
...

Then the code runs fine.

It seems as if the Response Types Tree does not resemble the json response schema very well.

Further (though this may be a seperate issue), passing _check_return_type=False, into kwargs, does not prevent Type checking to be conducted, this would otherwise have (to some degree) resolved this issue.

@auto-labeler
Copy link

auto-labeler bot commented Feb 18, 2021

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

@spacether
Copy link
Contributor

spacether commented Feb 19, 2021

Hi there. Thank you for reporting this issue.
When I generated a client with your spec using the v5.0.0 generator I get:

# openapi-generator generate -i issue_8743_wrong_response_type.yaml -o issue_8743 -g python
swagger: '2.0'
info:
  title: Quick Base API
  version: 1.0.0
host: api.quickbase.com
paths:
  '/fields/{fieldId}':
    get:
      operationId: getFieldProperties
      description: 'Gets the properties of an individual field, based on field id.'
      parameters:
        - name: fieldId
          in: path
          required: true
          type: integer
      responses:
        '200':
          description: Success
          schema:
            type: object
            properties:
              id:
                type: integer
              fieldType:
                type: string
              label:
                type: string
              properties:
                description: 'Additional properties for the field. '
                type: object
                additionalProperties: true
                properties:
                  comments:
                    type: string
                  format:
                    type: string
                  maxLength:
                    type: integer
                  allowNewChoices:
                    type: boolean
                  displayAsLink:
                    type: boolean
                  allowHTML:
                    type: boolean

I see the endpoint defined as:

        self.get_field_properties = Endpoint(
            settings={
                'response_type': (InlineResponse200,),
                'auth': [],
                'endpoint_path': '/fields/{fieldId}',
                'operation_id': 'get_field_properties',
                'http_method': 'GET',
                'servers': None,
            },

In that model I see id defined as int:

    @cached_property
    def openapi_types():
        """
        This must be a method because a model may have properties that are
        of type self, this must run after the class is loaded

        Returns
            openapi_types (dict): The key is attribute name
                and the value is attribute type.
        """
        lazy_import()
        return {
            'id': (int,),  # noqa: E501
            'field_type': (str,),  # noqa: E501
            'label': (str,),  # noqa: E501
            'properties': (InlineResponse200Properties,),  # noqa: E501
        }

Can you try using my provided yaml file in your spec?
Hmm not sure why your additionalProprties are not being generated in your inner schema..

@spacether
Copy link
Contributor

So there are two issues at play here:

  • With a v3 spec, when the response root schema contains the property called properties which contains another schema, the properties schema is not extracted out to be another schema component (model). Because this is not done, the generator just sees it as type object with additionalProperties.
    • Fix: extract out the properties schema and ref it
    • Note: this is an issue that is caused by underlaying java code and likely impacts many generators
  • additional_properties_type is not set correctly in either root schema or the properties schema models

@uncleflo
Copy link
Author

The provided OAS schema is a shortened schema. The original OAS file is attached:
qb.prepared.zip

We are focussing on path fields/{field} because we are using it (only). Regreteably, it does not use refered components, that seems to have solved the issue for the most part. The OAS file is not ours, we have to make do with it.

Thank you for your time.

@spacether
Copy link
Contributor

spacether commented Feb 19, 2021

So when I generate your client with my above v2 yaml spec it generates 2 models WITHOUT using a ref. Does that work for you? Can this ticket be closed?
Hmm looks like the full spec is more complicated than your sample and you need to use a ref to get it to work.
Does this work in clients generated in other languages? Which ones?

@uncleflo
Copy link
Author

We have tried PHP, and that went without a hitch on the full OAS. We have not tried creating a working implementation with the PHP client, but surfacially it looks good (better and cleaner than python really). PHP still has the same inlineobject123 files, and it seems that the type checking is a simple 'map[string,object]' or a reference to a class '\OpenAPI\Client\Model\InlineResponse2004[]'.

In your above generated model:

self.get_field_properties = Endpoint(
            settings={
                'response_type': (InlineResponse200,),

It seems that in our generated python client code, wherever it is using classes for 'response_type', it works. But wherever it is using inline type tree definitions such as:

"response_type": ( {
                        str: ( {
                                str: (bool,date, datetime, dict, float, int, list, str, none_type, )
                            }, )
                    }, ),

It does not work, and should be fixed (apparently or in the direction of my issue description suggestions) in the java generation code.

With your 'above v2 yaml spec', what are you suggesting? That I use your process of generating yaml first, before generating the python client? This MAY work (have not tried this yet), but it occurs to me as not the ideal solution for anyone (the ideal solution being to fix the java generator for python code for inline type trees).

Adding refs to the extended original OAS file using a preprocessing script is also a potential solution we have thought of, but it requires constant monitoring that correct interpreteable names for the components are used. Again, if it's not ideal for us, it hardly could be ideal for someone else.

At the moment, we are resolving our problem by manually editing the generated python client code, with the correct inline type tree, as I described in the issue description. I prefer this ticket to remain open until the issue is resolved (the generator not outputting correct inline type trees).

@spacether spacether added the Inline Schema Handling Schema contains a complex schema in items/additionalProperties/allOf/oneOf/anyOf label Mar 30, 2021
@spacether
Copy link
Contributor

Please use the python-experimental generator. It correctly generates inline schemas of any depth and will solve this issue

@polavishnu4444
Copy link

I am facing this issue still... I get the type to be the union for generic Object kind of value, but get an error saying failed as below

ApiTypeError("Invalid type for variable 'name'. Required value type is dict and passed type was str at ['properties']['payload']['name']"), <traceback object at 0x1023d6c00>

@spacether
Copy link
Contributor

spacether commented Oct 14, 2022

@uncleflo your spec is very old as it uses swagger 2.0 which was released on 2014-09-08
openapi-generator v6.2.0 python client required openapi specs >= v3.0.0, released in 2017-07-26
When I converted it to v3.0.0 using swagger-editor I got these errors:

Semantic error at paths./apps/{appId}.delete.requestBody
DELETE operations cannot have a requestBody.
Jump to line 331
Semantic error at paths./fields.delete.requestBody
DELETE operations cannot have a requestBody.
Jump to line 2320
Semantic error at paths./records.delete.requestBody
DELETE operations cannot have a requestBody.
Jump to line 3463

When I generate client using the v6.2.0 python client generator I get this response class for the route
/fields/{fieldId}.get

@dataclass
class ApiResponseFor200(api_client.ApiResponse):
    response: urllib3.HTTPResponse
    body: typing.Union[
        SchemaFor200ResponseBody,
    ]
    headers: schemas.Unset = schemas.unset

And the body is deserialized using the schema SchemaFor200ResponseBody
which is defined as:

class SchemaFor200ResponseBody(
    schemas.DictSchema
):


    class MetaOapg:
        additional_properties = schemas.DictSchema
    
    def __getitem__(self, name: typing.Union[str, ]) -> MetaOapg.additional_properties:
        # dict_instance[name] accessor
        return super().__getitem__(name)
    
    def get_item_oapg(self, name: typing.Union[str, ]) -> MetaOapg.additional_properties:
        return super().get_item_oapg(name)

    def __new__(
        cls,
        *args: typing.Union[dict, frozendict.frozendict, ],
        _configuration: typing.Optional[schemas.Configuration] = None,
        **kwargs: typing.Union[MetaOapg.additional_properties, dict, frozendict.frozendict, ],
    ) -> 'SchemaFor200ResponseBody':
        return super().__new__(
            cls,
            *args,
            _configuration=_configuration,
            **kwargs,
        )

Which is consistent with your defined response schema:

      responses:
        '200':
          description: Success
          content:
            '*/*':
              schema:
                type: object
                additionalProperties:
                  type: object

You are requiring that the response body is a dict, which the python code enforces with the SchemaFor200ResponseBody inheriting schemas.DictSchema. Your schema has no properties defined, so every value in your key value pair that is ingested must be what is defined in additionalProperties. additionalProperties defines values as type object (which is a dict in python).
From the generated code we see that ***kwargs ingested in the new signature must be type dict or schemas.DictSchema.
So the generated code is consistent with your schema.
If you need to ingest non-dict values, then define them in your schema as properties.
If this is unclear, please read all about schema definitions in the spec here.

@spacether
Copy link
Contributor

spacether commented Oct 14, 2022

@polavishnu4444 please us the 6.2.0 openapi-generator with the python client. If you are still having this issue please post your spec here and point out which schema and payload are not working as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client: Python Inline Schema Handling Schema contains a complex schema in items/additionalProperties/allOf/oneOf/anyOf Issue: Bug
Projects
None yet
Development

No branches or pull requests

3 participants