Parquery / swagger-to

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

Incorrectly requiring 'format' field for integer- and number-typed properties #104

Closed abingham closed 3 years ago

abingham commented 3 years ago

It seems that swagger-to is incorrectly expecting a format field (or is providing an invalid default value) for number an integer properties. For example, with this yaml:

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

I get this result:

% 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 8, in <module>
    sys.exit(main())
  File "/Users/abingham/.virtualenvs/acquisition-system/lib/python3.8/site-packages/swagger_to/bin/swagger_to_elm_client.py", line 46, in main
    raise ValueError("Failed to parse Swagger file {}:\n{}".format(swagger_path, "\n".join(errs)))
ValueError: Failed to parse Swagger file broken.yaml:
missing tag "name" in the swagger specification
in definition 'MyType': in property 'prop': Unexpected format for type 'integer': ''

That is, swagger-to is applying a default value of '' to the MyProp integer property. The same happens if I change the type to number.

If I explicitly provide a format (e.g. format: int32) then everything works.

mristin commented 3 years ago

@abingham let me paraphrase to see if I get the problem right. The swagger-to expects a format property to be set when given an integer, but the error message is misleading and confusing since it refers to an empty default value?

(Swagger-to indeed needs to know the format of the integer in order to generate Go code and possibly code in other languages which need a specific integer length.)

abingham commented 3 years ago

It's a bit of both, I guess. I don't think swagger-to should be expecting a 'format', so that's issue 1. The error message is a bit misleading since it reports an implementation detail (i.e. swagger-to selected the invalid empty string as the default value for format) rather than the actual problem. Of course, since there really is no problem with the user input (as far as I can tell), the incorrect error message is moot...we shouldn't see it at all.

On Fri, Nov 27, 2020 at 10:06 AM Marko Ristin notifications@github.com wrote:

@abingham https://github.com/abingham let me paraphrase to see if I get the problem right. The swagger-to expects a format property to be set when given an integer, but the error message is misleading and confusing since it refers to an empty default value?

(Swagger-to indeed needs to know the format of the integer in order to generate Go code and possibly code in other languages which need a specific integer length.)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Parquery/swagger-to/issues/104#issuecomment-734727071, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAATK6H2VN7TAZC5BEEQJP3SR5TYBANCNFSM4UEUJEVA .

mristin commented 3 years ago

Thanks for clarification, @abingham. I'll make it optional and raise an error in related generators if they indeed require the specific format.

(I also noticed you collected many other issues with Elm -- which is much appreciated!)

mristin commented 3 years ago

Hi @abingham , Could you please check if the PR #115 solves the issue on your schema?

abingham commented 3 years ago

Seems to work well. Thanks!