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] Using allOf and $ref generates an object in the python model when it should be a string #8457

Closed
5 tasks done
cgfarmer4 opened this issue Jan 16, 2021 · 21 comments
Closed
5 tasks done
Labels
Client: Python Inline Schema Handling Schema contains a complex schema in items/additionalProperties/allOf/oneOf/anyOf Issue: Bug

Comments

@cgfarmer4
Copy link
Contributor

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
openapi-generator version

Docker CLI latest

OpenAPI declaration file content or url
BankTransferCreateRequest:
  title: BankTransferCreateRequest
  type: object
  description: BankTransferCreateRequest defines the request schema for `/bank_transfer/create`
  properties:
    access_token:
      description: Some Description
      allOf:
        - $ref: '#/components/schemas/AccessToken'
 
AccessToken:
  title: AccessToken
  type: string
  description: The access token associated with the Item data is being requested for.

/bank_transfer/create:
  post:
    operationId: bankTransferCreate
    requestBody:
      required: true
      content:
        application/json:
          schema:
            $ref: '#/components/schemas/BankTransferCreateRequest'
Generation Details

$(OPENAPI_GENERATOR_LATEST) -g python -i local/$(OPENAPI_FILE) -o local/$(OUTPUT_FOLDER)/generated-python

Steps to reproduce

Run the generator, try accessing the BankTransferCreateRequest model. See that access_token is object instead of string.

return {
    'access_token': (object,),  # noqa: E501
}
Related issues/PRs
Suggest a fix
@spacether
Copy link
Contributor

Why not change your spec to have access_token point directly to the referenced #/components/schemas/AccessToken without using allOf? That should solve your problem.

@cgfarmer4
Copy link
Contributor Author

cgfarmer4 commented Jan 19, 2021

@spacether reading here:

https://swagger.io/docs/specification/data-models/oneof-anyof-allof-not/

allOf should reference the subschema and its not. Its transforming it to object when the $ref schema is a string.

@spacether
Copy link
Contributor

Yes, I agree that there is a bug here.
Inline composed schemas are not yet very well supported in openapi-generator.

My suggestion is a work around which may work for you.
Another work-around would be to place your composed schema in a component and ref it.

@cgfarmer4
Copy link
Contributor Author

Oh I see. Yep that does fix my problem :D!

@cgfarmer4
Copy link
Contributor Author

Hmm spoke too soon. Any timeline on better $ref support? Can workaround for now, just curious

@spacether
Copy link
Contributor

spacether commented Jan 20, 2021

No firm timeline here. I am slowly working on a number of improvementss to the Python generator in a new generator python-experimental. This feature will be included in that.

The issue is not with ref support. It is with inline composed schema support.

@spacether
Copy link
Contributor

spacether commented Jan 21, 2021

For this use case you can delete your AccessToken schema and just define BankTransferCreateRequest.access_token as type: string. Python does not produce a model for AccessToken because it is an alias to a primitive type and does not include any validations or enums. If there is a validation or enum in AccessToken it will be made as a model.

@cgfarmer4
Copy link
Contributor Author

@spacether I see. May have to make a workaround for just python. We'd prefer not to have to hack for one language as we are generating a bunch of things with this file.

@spacether
Copy link
Contributor

How about:
minLength: 0?
What other languages are you generating clients in and what do they do with AccessToken?

@cgfarmer4
Copy link
Contributor Author

cgfarmer4 commented Jan 21, 2021

Ruby, Node, Python, Go, Java and js client side SwaggerParser for our docs.

Ive run into another issue with using allOf again today and have confirmed its only python and doesn't effect ruby/node. Dont have the other ones tested yet.

@cgfarmer4
Copy link
Contributor Author

Going off the Swagger docs this format should be valid.

@spacether
Copy link
Contributor

It is a valid spec, our tooling just isn't there yet.
The main composed schema use case that the generator supports is a component object schema that contains allOf or oneOf or anyOf.
Additionally, python supports the use cases where:

  • oneOf can have mixed types
  • allOf is combined with oneOf or anyOf

@cgfarmer4
Copy link
Contributor Author

I think we have a workaround for now. Thanks for talking through it @spacether!! Hoping to get a sponsorship going here soon for you all.

@alfechner
Copy link

alfechner commented Jul 6, 2021

I've got the same issue but slightly different. I've got a $ref to a schema of type object.

PatientScanDto:
  type: object
  patient:
    allOf:
      - $ref: '#/components/schemas/PatientInfoDto'
  nullable: true
  readOnly: true
PatientInfoDto:
  type: object
  properties:
    patientId:
      type: string

Python classes PatientScanDto and PatientInfoDto are generated appropriately.

However the property patient in class PatientScanDto has the wrong type.

In generator version 5.1.0 the type is object which causes an ApiValueError: Unsupported type: <class 'object'>.

Generator version 5.1.1 considers patient to be of type dict. While that doesn't raise an exception it is the wrong type and accessing properties from a dict works probably around type checks.

@spacether
Copy link
Contributor

spacether commented Jul 8, 2021

If you define patient as its own component and then $ref to patient in PatientScanDto does this work?
I suspect that this is an openapi inline schema issue and is not specific to the python generator.
Does you allOf define the value in patient? If so should allOf be indented?

@spacether
Copy link
Contributor

spacether commented Jan 11, 2022

Just for clarity here, I want to describe the openapi definition of your specs
@cgfarmer4

BankTransferCreateRequest:
  type: object
  properties:
    access_token:
      description: Some Description
      allOf:
        - $ref: '#/components/schemas/AccessToken'
 
AccessToken:
  type: string

BankTransferCreateRequest.access_token does not define a type so it allows in any type in the
BankTransferCreateRequest.access_token schema. The allOf schema which must also be validated against further constrains the property to be type string only.

@alfechner

PatientScanDto:
  type: object
  patient:
    allOf:
      - $ref: '#/components/schemas/PatientInfoDto'
  nullable: true
  readOnly: true

PatientInfoDto:
  type: object
  properties:
    patientId:
      type: string

PatientScanDto.patient allows in any type of payload because type is unset. However because PatientInfoDto further constrains the payload to only be type object, only object type payloads will pass validation.

@spacether
Copy link
Contributor

spacether commented Jan 11, 2022

For components with allOf this is working in the python-experimental generator.
In the Cat schema Cat can be any type because type is unset and the allOf schemas in it will only allow in type: object payloads.
One can see this in the Cat model which inherits from ComposedSchema which allows in any type. And the Animal and CatAllOf classes which are also validated against are constrained to type object, shown by their DictSchema base classes.

@alfechner
Copy link

@spacether thanks for your reply, you are right, allOf was supposed to be indented. I fixed that.

I guess you're also right about your assumption that it's an inline issue. You mentioned that in an older answer already. In my use case the spec is generated by another framework with inline objects and then serves to generated kind of a proxy server. Currently I need to modify the spec manually before generating the server.

@spacether
Copy link
Contributor

spacether commented Jan 11, 2022

Can you try your spec with python-experimental and see if it works?

In the future I will add more inline examples to the python-experimental generator to see how well it is handling them in all locations. Endpoints are definitely sometimes generating inline schemas for parameters in the endpoint files. If any locations are not working I will have to turn the inline model resolver off for python experimental so the generator will handle the spec exactly as it was defined by the user.

@spacether
Copy link
Contributor

spacether commented Jan 11, 2022

Re-opening this until examples have been added in python-experimental verifying that this is working at levels deeper than a root component with composition. Tests are needed of:

  1. component property with composition
  2. parameter with composition
  3. request body with composition
  4. response body with composition
  5. parameter property with composition
  6. request body property with composition
  7. response body property with composition

@spacether
Copy link
Contributor

spacether commented Feb 3, 2022

The merged PR #11420
adds verification schemas and test of

  • component property with composition
  • parameter with composition
  • request body with composition
  • response body with composition
  • parameter property with composition
  • request body property with composition
  • response body property with composition

So this feature works in python-experimental
Closing this issue because we have a python client generator which supports this inline composed schema use case

Please switch to the python-experimental generator if you need complex inline schema support.

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

4 participants
@cgfarmer4 @spacether @alfechner and others