[BUG] [Python] __setitem__ throwing an error for composed instance

[the-akhil-nair]

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

When a schema references another schema under allOf and when setattr operation is performed on an schema instance, the latest code is trying to set attribute on self and composed instances.

If the composed schema does not contain that property, the openapi-generated code is trying to validate and convert the attribute into primitive data types.

If the data type for the attribute is already a primitive data type then no error will be thrown.
But if the attribute is expecting a relationship schema then conversion of such attribute will throw an error.

Snippet of spec file

schemas:
    Dog:
      allOf:
        - $ref: '#/components/schemas/Animal'
        - type: object
          properties:
            breed:
              type: string
            legs:
               $ref: '#/components/schemas/Legs'
    Legs:
        type: object
        required:
        - legs
        properties:
            legs:
              enum:
              - '2'
              - '4'
              default: '4'
              x-enum-as-string: true
            name:
               type: string
    Animal:
      type: object
      discriminator:
        propertyName: className
      required:
        - className
      properties:
        className:
          type: string
        color:
          type: string
          default: red

As we can see in the above case Dog class contain Animal class as part of allOf composed schema.
The Dog class contain an attribute legs which references to Legs schema.

The Animal class does not contain this attribute(legs).

When an operation :

dog_instance.legs = legs

is performed(Where legs is an instance of Legs class) the latest code will try to set the attribute for self and composed instance, then for composed instance i.e. Animal class as this attribute is not present in openapi_types, the code will try to convert legs into a valid_type which in our case is into primitive data type.

Hence the code will throw an error because the expected type for that attribute will be primitive data type but what the code will recieve is of type of an instance of Legs class.

Code used to test:

from openapi_client.model.dog import Dog
from openapi_client.model.legs import Legs


def define_legs():
    return Legs(legs="4")


def dog():
    legs = define_legs()

    dog_instance = Dog(class_name="Dog",
                       color="Black")

    dog_instance.breed = "Bulldog"

    dog_instance.legs = legs

dog()

Error captured:

Traceback (most recent call last):
  File "E:\Dev\testing\petstore\dog.py", line 19, in <module>
    dog()
  File "E:\Dev\testing\petstore\dog.py", line 17, in dog
    dog_instance.legs = legs
  File "E:\Dev\testing\petstore\openapi_client\model_utils.py", line 183, in __setattr__
    self[attr] = value
  File "E:\Dev\testing\petstore\openapi_client\model_utils.py", line 600, in __setitem__
    setattr(model_instance, name, value)
  File "E:\Dev\testing\petstore\openapi_client\model_utils.py", line 183, in __setattr__
    self[attr] = value
  File "E:\Dev\testing\petstore\openapi_client\model_utils.py", line 483, in __setitem__
    self.set_attribute(name, value)
  File "E:\Dev\testing\petstore\openapi_client\model_utils.py", line 155, in set_attribute
    value = validate_and_convert_types(
  File "E:\Dev\testing\petstore\openapi_client\model_utils.py", line 1552, in validate_and_convert_types
    raise get_type_error(input_value, path_to_item, valid_classes,
openapi_client.exceptions.ApiTypeError: Invalid type for variable 'legs'. Required value type is one of [NoneType, bool, date, datetime, dict, float, int, list, str] and passed type was Legs at ['legs']

Process finished with exit code 1

Expected output:

No Error should be thrown.

Actual output:

openapi_client.exceptions.ApiTypeError: Invalid type for variable 'legs'. Required value type is one of [NoneType, bool, date, datetime, dict, float, int, list, str] and passed type was Legs at ['legs']

openapi-generator version

5.2.1

OpenAPI declaration file content or url

https://gist.github.com/AKKI191289/fef7a46578c4070ce86ed9f45b9f46b0

Generation Details

java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -i animal.yaml -g python -o animal

Steps to reproduce

1. Pull the master branch and compile the openapi-generator code using mvn.
2. Put spec in a file
3. Run generation cli commands as mentioned in the above steps.

Related issues/PRs

#8802

Suggest a fix

Suggestion:

Earlier the code was finding out the valid class and then only setting the attributes.
There is a variable name _var_name_to_model_instances which was having a mapping of attribute and list of valid classes.
So for above example the mapping for legs will be Dog and DogAllOf class.
So then when attribute was getting initialized for both Dog and DogAllOf no error is thrown as both the class contains
legs as an attribute.

But now the code is initializing legs attribute for Dog, DogAllOf and Animal and since Animal does not contain the legs
attribute it is throwing an error.

The older logic for setting the attribute for proper class should be used.

[vvb]

/cc @spacether @wing328

[spacether]

All variables should be set in all composed model instances.
This is a requirement from OpenApi as it requires that all matching schemas validate the same payload (properties).
It looks like our code is not seeing that the Legs instance is an allowed type. Do you want to write a PR to fix this issue?

[vvb]

@spacether some queries,

All variables should be set in all composed model instances.

By design, variables might not be present in all of the composed model instances. For this reason, the older code(51b6b9d#diff-2996debf5a47e29df72469d8fede8b41a687d367de6a46d2e2ddf21e69d4abe3) was setting them only on matching composed model instances. OpenApi specification says that all "matching" schemas need to validate the properties. And so the older code was calculating the "matching" schemas first. The new code is invoking on all composed model schemas. This does not seem right.

This is a requirement from OpenApi as it requires that all matching schemas validate the same payload (properties).

My understanding is that older code was doing exactly that.

It looks like our code is not seeing that the Legs instance is an allowed type. Do you want to write a PR to fix this issue?

I can submit a patch, but help us understand what might be the right way to fix this, if not the older code. Do you prefer a slight re-organization, where properties are set on every composed model instance but the model instance internally figures out that the property is not present in that schema and returns?

[spacether]

Variables must be validatable on all required schemas composed or otherwise. If a propertyis not defined then it must be acceptable as an additional property through additionalProperties accepting anything. Let's have a meeting to clear this up. Full payloads must be used. If a payload fails to validate then the schema is not a match and an exception should be thrown. If that schema is required then the exception will not be caught and will be raised.