Parquery / swagger-to

Generate server and client code from Swagger (OpenAPI 2.0) specification.
MIT License
57 stars 14 forks source link

Incorrectly requiring "type" field for type definitions #105

Closed abingham closed 3 years ago

abingham commented 3 years ago

It seems that swagger-to requires the type field for type definitions. From what I can tell (mostly by running the YAML through other validators) is that type is optional, though I haven't done a close reading of the spec.

With this YAML:

basePath: /
consumes:
- application/json
info:
  description: An API
  title: An API
  version: '1.0'
definitions:
  MyType:
    properties:
      object_id:
        type: integer
        format: int32
paths:
  /:
    get:
      operationId: get_foo
      responses:
        '200':
          description: Success
produces:
- application/json
swagger: '2.0'

I get this output:

% python `which swagger_to_elm_client.py` --swagger_path broken.yaml --outdir gen
Traceback (most recent call last):
  File "/Users/abingham/.virtualenvs/acquisition-system/bin/swagger_to_elm_client.py", line 11, in <module>
    load_entry_point('swagger-to', 'console_scripts', 'swagger_to_elm_client.py')()
  File "/Users/abingham/repos/swagger-to/swagger_to/bin/swagger_to_elm_client.py", line 48, in main
    intermediate_typedefs = swagger_to.intermediate.to_typedefs(swagger=swagger)
  File "/Users/abingham/repos/swagger-to/swagger_to/intermediate.py", line 302, in to_typedefs
    _preallocate_named_typedefs(definition=defi, typedefs=typedefs)
  File "/Users/abingham/repos/swagger-to/swagger_to/intermediate.py", line 171, in _preallocate_named_typedefs
    raise ValueError("Unexpected type of a typedef in the definition {!r}: {!r}".format(
ValueError: Unexpected type of a typedef in the definition 'MyType': ''

If I modify MyType by adding a type field:

MyType:
    properties:
      object_id:
        type: integer
        format: int32
    type: object

then everything seems to work:

% python `which swagger_to_elm_client.py` --swagger_path broken.yaml --outdir gen
Generated Elm client code in: gen
mristin commented 3 years ago

Am I correct: if the field properties is present, it should be assumed that the type is object?

abingham commented 3 years ago

That seems to be the case, but honestly that's just an educated guess. I started looking at the spec briefly, but it wasn't immediately clear what was expected/allowed. In any event, several linters and validators accept type-less properties.

mristin commented 3 years ago

Hi @abingham , Could you please test if #112 solves the issue with your schema and re-open the issue if not?

abingham commented 3 years ago

This seems to work for simple cases. It fails when allOf is used, e.g.:

basePath: /
consumes:
- application/json
info:
  description: An API
  title: An API
  version: '1.0'
definitions:
  Base:
    properties:
      base_prop:
        type: integer
  Sub:
    allOf:
      - $ref: "#/definitions/Base"
      - properties:
          sub_prop:
            type: integer
paths:
  /:
    get:
      operationId: get.foo.bar
      responses:
        '200':
          description: Success
produces:
- application/json
swagger: '2.0'
tags:
  - description: Test API
    name: API 

Here the type Sub type is confusing your logic by defining a type with no properties and no type:

% python `which swagger_to_elm_client.py` --swagger_path broken.yaml --outdir gen
Traceback (most recent call last):
  File "/Users/abingham/.virtualenvs/acquisition-system/bin/swagger_to_elm_client.py", line 11, in <module>
    load_entry_point('swagger-to', 'console_scripts', 'swagger_to_elm_client.py')()
  File "/Users/abingham/repos/swagger-to/swagger_to/bin/swagger_to_elm_client.py", line 48, in main
    intermediate_typedefs = swagger_to.intermediate.to_typedefs(swagger=swagger)
  File "/Users/abingham/repos/swagger-to/swagger_to/intermediate.py", line 303, in to_typedefs
    _preallocate_named_typedefs(definition=defi, typedefs=typedefs)
  File "/Users/abingham/repos/swagger-to/swagger_to/intermediate.py", line 171, in _preallocate_named_typedefs
    raise ValueError(("Could not determine the intermediate type "
ValueError: Could not determine the intermediate type for a Swagger type definition 'Sub' (here given as JSON):
{
  "allOf": [
    {
      "$ref": "#/definitions/Base"
    },
    {
      "properties": {
        "sub_prop": {
          "type": "integer"
        }
      }
    }
  ]
}

I can fix this by adding type: object to the definition of Sub, but other validators and linters agree that this shouldn't be necessary.

abingham commented 3 years ago

I don't seem to be able to re-open this issue, but I think it should be.

mristin commented 3 years ago

Hi @abingham ,t Thanks for the feedback. Handling allOf was not part of the PR -- let me tackle that issue in a separate PR. (See also the issue #106)