[python] Fixes additional_properties_type for models (!8802) · Merge requests · OpenAPI Tools / openapi-generator

Merged Administrator requested to merge github/fork/spacether/fixes_python_model_addprops_type into 5.2.x Feb 23, 2021

Created by: spacether

Before this PR, master branch python client code incorrectly set the additional_properties_type of models to None when it was unset, when it should be set to allow any type. Allowing any type is the default behavior when additionalProperties is unset per openapi versions >= 2. This PR fixes additionalProperties values for models #8771 (closed), correctly setting additional_properties_type.

updates sample model docs
adds test of additionalProperties in Tag
fixes Fruit and GmFruit tests
a new v2 python sample has been added for disallowAdditionalPropertiesIfNotPresent=true. So now for v2 specs we have one sample for each boolean value of disallowAdditionalPropertiesIfNotPresent. The standard python v2 sample now uses disallowAdditionalPropertiesIfNotPresent=false (the option is omitted which default to false in the python generator)
I have updated the discard_unknown_keys test @sebastien-rosset
For v3 specs, where additionalProperties: False works, we have a test of it here
For v3 specs where a composed schema contains schemas where additionalProperties is False we have tests here

requiredVars have been moved out of the composed schema init signature

requiredVars have been moved out of the composed schema init signature because the required vars were probably defined in the composed oneOf/anyOf/allOf schemas and this schema does not know about them. Every schema is validated separately. So that means that for a composed schema with no type required or properties or additionalProperties definition inside it:

all types are valid
there are no required properties
for object types, all properties are allowed because additionalProperties omitted defaults to additionalProperties: True which allows string property names and any type of value
THEN the payload is checked against any oneOf/anyOf/allOf schemas

This change is needed because in our java code, if a oneOf class include required variables, those variable are set in composed schema that includes them. This is incorrect because those variable are not required in the composed schema, they are optional there. One can see this by looking at the Fruit is a oneOf apple, banana example. This PR updates it so apple and banana include required parameters. Before making the composed schema init change, those parameters were incorrectly present as positional arguments in Fruit. That's because the java code added those apple and banana requiredVars into Fruit's requiredVars which is incorrect.

v2 Spec Implications

There is a bug in swagger-parser for v2 specs. With that bug, values of True, False, and unset are all seen as null additionalProperties which now becomes AnyType schema. Previously, the default behavior was to treat all 3 conditions as setting additional_properties_type to None, which did not allow any additional properties to be sent for a given object payload. In order to have additionalProperties True/False, and unset work correctly, users need to upgrade their specifications to v3. If users still need the old behavior where additionalProperties was None for additionalProperties=True/False/unset, then they should set disallowAdditionalPropertiesIfNotPresent to True. Doing that will break composition usages oneOf/anyOf/allOf because for that to work, in the composed schema additionalProperties must be any type to allow in properties not defined in the composed schema.properties. There's another bug where schemas with unset type (AnyType) have their type set to object. This PR works around that bug. If you need to differentiate between AnyType schemas and free form objects (type: object with no properties) then update your spec to v3 or higher.

Breaking Change with Fallback

For v2 specs that use compositiona and were setting disallowAdditionalPropertiesIfNotPresent=True this is a breaking change because composition (allOf schemas) will stop working. If you need composition to work, you must omit setting disallowAdditionalPropertiesIfNotPresent which will then default it to False. Composed schemas will then work. This conforms with the openapi tech spec.

Note: this different PR https://github.com/OpenAPITools/openapi-generator/pull/8816 will fix it so the variables defined in any schema/model in python only include variables for schema.properties and do not include composed schema properties.

PR checklist

Read the contribution guidelines.
Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
Run the following to build the project and update samples:
```
./mvnw clean package 
./bin/generate-samples.sh
./bin/utils/export_docs_generators.sh
```
Commit all changed files. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*. For Windows users, please run the script in Git BASH.
File the PR against the correct branch: master, 5.1.x, 6.0.x
If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.