[python] Fixes additional_properties_type for models (#8802)

* Fixes additionalProperties values for models, updates docs, adds tag test of it, fixes frit and gmfruit tests

* Moves this.setDisallowAdditionalPropertiesIfNotPresent higher

* Makes setting additional_properties_model_instances contingent on the presence of addprosp in schema, updates sample spec composed schemas to remove addprops False form two

* Fixes oneOf anyOf allOf instantiation logic

* Removes Address from Cat definition

* Adds required vars for apple and banana, removes required vars from composed schema init sig

* Updates composed schema vars to be set on self and all composed instances

* Removes get_unused_args, get_var_name_to_model_instances, and get_additional_properties_model_instances

* Fixes fruit + deserilization tests, creates ComposedSchemaWithPropsAndNoAddProps

* Fixes FruitReq tests

* Fixes GmFruit tests

* Fixes discard_unknown_keys tests

* Samples updated

* Removes additionalproperties False in Child

* Samples updated

* Improves handling of v2 and v3 specs for isFreeFormObject, v2 sample spec updated with link to bug

* Adds cli option disallowAdditionalPropertiesIfNotPresent to python

* Adds getAdditionalProperties method so the value for addProps will be correct

* Reverts file

* Reverts file

* Updates python doc

* Reverted anytype_3 definition

* Updates test_deserialize_lizard

* Updates test_deserialize_dict_str_dog

* Updates testDog

* Updates testChild

* Adds v2 python_composition sample

* Adds needed files for python testing

* Adds existing tests into the new python sample

* Fixes test_dog

* Removes addProps false form Dog

* Fixes testChild

* Updates how additionalProperties are set

* Fixes empty_map type

* Type generation fixed for v2 and v3 specs

* Refactors getTypeString, updates artifactids in pom.xml files

* Adds new python sample to CI testing I think

* Fixes artifactId collision, regenrates docs

bin/configs/python-oas2.yaml

@@ -1,7 +1,10 @@
+# this file exists because in this file we omit setting disallowAdditionalPropertiesIfNotPresent
+# which makes it default to false
+# that false setting is needed for composed schemas to work
+# Composed schemas are schemas that contain the allOf/oneOf/anyOf keywords. v2 specs only support the allOf keyword.
 generatorName: python
 outputDir: samples/client/petstore/python
 inputSpec: modules/openapi-generator/src/test/resources/2_0/python-client-experimental/petstore-with-fake-endpoints-models-for-testing.yaml
 templateDir: modules/openapi-generator/src/main/resources/python
 additionalProperties:
-  disallowAdditionalPropertiesIfNotPresent: "true"
   packageName: petstore_api

bin/configs/python-oas2_disallowAdditionalPropertiesIfNotPresent.yaml

@@ -0,0 +1,7 @@
+generatorName: python
+outputDir: samples/client/petstore/python_disallowAdditionalPropertiesIfNotPresent
+inputSpec: modules/openapi-generator/src/test/resources/2_0/python-client-experimental/petstore-with-fake-endpoints-models-for-testing.yaml
+templateDir: modules/openapi-generator/src/main/resources/python
+additionalProperties:
+  disallowAdditionalPropertiesIfNotPresent: "true"
+  packageName: petstore_api

docs/generators/python.md

@@ -7,6 +7,7 @@ These options may be applied as additional-properties (cli) or configOptions (pl
 
 | Option | Description | Values | Default |
 | ------ | ----------- | ------ | ------- |
+|disallowAdditionalPropertiesIfNotPresent|If false, the 'additionalProperties' implementation (set to true by default) is compliant with the OAS and JSON schema specifications. If true (default), keep the old (incorrect) behaviour that 'additionalProperties' is set to false by default.|<dl><dt>**false**</dt><dd>The 'additionalProperties' implementation is compliant with the OAS and JSON schema specifications.</dd><dt>**true**</dt><dd>Keep the old (incorrect) behaviour that 'additionalProperties' is set to false by default. NOTE: this option breaks composition and will be removed in 6.0.0</dd></dl>|false|
 |generateSourceCodeOnly|Specifies that only a library source code is to be generated.| |false|
 |hideGenerationTimestamp|Hides the generation timestamp when files are generated.| |true|
 |library|library template (sub-template) to use: asyncio, tornado, urllib3| |urllib3|
@@ -28,7 +29,6 @@ These options may be applied as additional-properties (cli) or configOptions (pl
 
 | Type/Alias | Instantiated By |
 | ---------- | --------------- |
-|map|dict|
 
 
 ## LANGUAGE PRIMITIVES

modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java

@@ -65,10 +65,6 @@ public PythonClientCodegen() {
         // in other code generators, support needs to be enabled on a case-by-case basis.
         supportsAdditionalPropertiesWithComposedSchema = true;
 
-        // When the 'additionalProperties' keyword is not present in a OAS schema, allow
-        // undeclared properties. This is compliant with the JSON schema specification.
-        this.setDisallowAdditionalPropertiesIfNotPresent(false);
-
         modifyFeatureSet(features -> features
                 .includeDocumentationFeatures(DocumentationFeature.Readme)
                 .wireFormatFeatures(EnumSet.of(WireFormatFeature.JSON, WireFormatFeature.XML, WireFormatFeature.Custom))
@@ -94,9 +90,8 @@ public PythonClientCodegen() {
                         ParameterFeature.Cookie
                 )
         );
-
-        // this may set datatype right for additional properties
-        instantiationTypes.put("map", "dict");
+        // needed for type object with additionalProperties: false
+        typeMapping.put("object", "dict");
 
         languageSpecificPrimitives.add("file_type");
         languageSpecificPrimitives.add("none_type");
@@ -111,6 +106,20 @@ public PythonClientCodegen() {
         cliOptions.add(new CliOption(CodegenConstants.PYTHON_ATTR_NONE_IF_UNSET, CodegenConstants.PYTHON_ATTR_NONE_IF_UNSET_DESC)
                 .defaultValue(Boolean.FALSE.toString()));
 
+        // option to change how we process + set the data in the 'additionalProperties' keyword.
+        CliOption disallowAdditionalPropertiesIfNotPresentOpt = CliOption.newBoolean(
+                CodegenConstants.DISALLOW_ADDITIONAL_PROPERTIES_IF_NOT_PRESENT,
+                CodegenConstants.DISALLOW_ADDITIONAL_PROPERTIES_IF_NOT_PRESENT_DESC).defaultValue(Boolean.FALSE.toString());
+        Map<String, String> disallowAdditionalPropertiesIfNotPresentOpts = new HashMap<>();
+        disallowAdditionalPropertiesIfNotPresentOpts.put("false",
+                "The 'additionalProperties' implementation is compliant with the OAS and JSON schema specifications.");
+        disallowAdditionalPropertiesIfNotPresentOpts.put("true",
+                "Keep the old (incorrect) behaviour that 'additionalProperties' is set to false by default. NOTE: "+
+                "this option breaks composition and will be removed in 6.0.0"
+        );
+        disallowAdditionalPropertiesIfNotPresentOpt.setEnum(disallowAdditionalPropertiesIfNotPresentOpts);
+        cliOptions.add(disallowAdditionalPropertiesIfNotPresentOpt);
+
         generatorMetadata = GeneratorMetadata.newBuilder(generatorMetadata)
                 .stability(Stability.EXPERIMENTAL)
                 .build();
@@ -160,6 +169,18 @@ public void processOpts() {
         }
         additionalProperties.put("attrNoneIfUnset", attrNoneIfUnset);
 
+        // When the 'additionalProperties' keyword is not present in a OAS schema, allow
+        // undeclared properties. This is compliant with the JSON schema specification.
+        // setting this to false is required to have composed schemas work because:
+        // anyOf SchemaA + SchemaB, requires that props present only in A are accepted in B because in B
+        // they are additional properties
+        Boolean disallowAddProps = false;
+        if (additionalProperties.containsKey(CodegenConstants.DISALLOW_ADDITIONAL_PROPERTIES_IF_NOT_PRESENT)) {
+            disallowAddProps = Boolean.valueOf(additionalProperties.get(CodegenConstants.DISALLOW_ADDITIONAL_PROPERTIES_IF_NOT_PRESENT).toString());
+        }
+        this.setDisallowAdditionalPropertiesIfNotPresent(disallowAddProps);
+
+
         // check library option to ensure only urllib3 is supported
         if (!DEFAULT_LIBRARY.equals(getLibrary())) {
             throw new RuntimeException("Only the `urllib3` library is supported in the refactored `python` client generator at the moment. Please fall back to `python-legacy` client generator for the time being. We welcome contributions to add back `asyncio`, `tornado` support to the `python` client generator.");
@@ -728,6 +749,62 @@ public String getModelName(Schema sc) {
         return null;
     }
 
+    @Override
+    protected Schema getAdditionalProperties(Schema schema) {
+        /*
+        Use cases:
+        1. addProps set to schema in spec: return that schema
+        2. addProps unset w/ getDisallowAdditionalPropertiesIfNotPresent -> null
+        3. addProps unset w/ getDisallowAdditionalPropertiesIfNotPresent=False -> new Schema()
+        4. addProps true -> new Schema() NOTE: v3 only
+        5. addprops false -> null NOTE: v3 only
+         */
+        Object addProps = schema.getAdditionalProperties();
+        if (addProps instanceof Schema) {
+            return (Schema) addProps;
+        }
+        if (addProps == null) {
+            // When reaching this code path, this should indicate the 'additionalProperties' keyword is
+            // not present in the OAS schema. This is true for OAS 3.0 documents.
+            // However, the parsing logic is broken for OAS 2.0 documents because of the
+            // https://github.com/swagger-api/swagger-parser/issues/1369 issue.
+            // When OAS 2.0 documents are parsed, the swagger-v2-converter ignores the 'additionalProperties'
+            // keyword if the value is boolean. That means codegen is unable to determine whether
+            // additional properties are allowed or not.
+            //
+            // The original behavior was to assume additionalProperties had been set to false.
+            if (getDisallowAdditionalPropertiesIfNotPresent()) {
+                // If the 'additionalProperties' keyword is not present in a OAS schema,
+                // interpret as if the 'additionalProperties' keyword had been set to false.
+                // This is NOT compliant with the JSON schema specification. It is the original
+                // 'openapi-generator' behavior.
+                return null;
+            }
+            /*
+            // The disallowAdditionalPropertiesIfNotPresent CLI option has been set to true,
+            // but for now that only works with OAS 3.0 documents.
+            // The new behavior does not work with OAS 2.0 documents.
+            if (extensions == null || !extensions.containsKey(EXTENSION_OPENAPI_DOC_VERSION)) {
+                // Fallback to the legacy behavior.
+                return null;
+            }
+            // Get original swagger version from OAS extension.
+            // Note openAPI.getOpenapi() is always set to 3.x even when the document
+            // is converted from a OAS/Swagger 2.0 document.
+            // https://github.com/swagger-api/swagger-parser/pull/1374
+            SemVer version = new SemVer((String)extensions.get(EXTENSION_OPENAPI_DOC_VERSION));
+            if (version.major != 3) {
+                return null;
+            }
+            */
+        }
+        if (addProps == null || (addProps instanceof Boolean && (Boolean) addProps)) {
+            // Return empty schema to allow any type
+            return new Schema();
+        }
+        return null;
+    }
+
     /**
      * Return a string representation of the Python types for the specified OAS schema.
      * Primitive types in the OAS specification are implemented in Python using the corresponding
@@ -767,16 +844,39 @@ private String getTypeString(Schema p, String prefix, String suffix, List<String
             }
         }
         if (isAnyTypeSchema(p)) {
+            // for v2 specs only, swagger-parser never generates an AnyType schemas even though it should generate them
+            // https://github.com/swagger-api/swagger-parser/issues/1378
+            // switch to v3 if you need AnyType to work
             return prefix + "bool, date, datetime, dict, float, int, list, str, none_type" + suffix;
         }
+        String originalSpecVersion = "X";
+        if (this.openAPI.getExtensions() != null && this.openAPI.getExtensions().containsKey("x-original-swagger-version")) {
+            originalSpecVersion = (String) this.openAPI.getExtensions().get("x-original-swagger-version");
+            originalSpecVersion = originalSpecVersion.substring(0, 1);
+
+        }
+        Boolean v2DisallowAdditionalPropertiesIfNotPresentAddPropsNullCase = (getAdditionalProperties(p) == null && this.getDisallowAdditionalPropertiesIfNotPresent() && originalSpecVersion.equals("2"));
+        Schema emptySchema = new Schema();
+        Boolean v2WithCompositionAddPropsAnyTypeSchemaCase = (getAdditionalProperties(p) != null && emptySchema.equals(getAdditionalProperties(p)) && originalSpecVersion.equals("2"));
+        if (isFreeFormObject(p) && (v2DisallowAdditionalPropertiesIfNotPresentAddPropsNullCase || v2WithCompositionAddPropsAnyTypeSchemaCase)) {
+            // for v2 specs only, input AnyType schemas (type unset) or schema {} results in FreeFromObject schemas
+            // per https://github.com/swagger-api/swagger-parser/issues/1378
+            // v2 spec uses cases
+            // 1. AnyType schemas
+            // 2. type object schema with no other info
+            // use case 1 + 2 -> both become use case 1
+            // switch to v3 if you need use cases 1 + 2 to work correctly
+            return prefix + "bool, date, datetime, dict, float, int, list, str, none_type" + fullSuffix;
+        }
         // Resolve $ref because ModelUtils.isXYZ methods do not automatically resolve references.
         if (ModelUtils.isNullable(ModelUtils.getReferencedSchema(this.openAPI, p))) {
             fullSuffix = ", none_type" + suffix;
         }
-        if (isFreeFormObject(p) && getAdditionalProperties(p) == null) {
-            return prefix + "bool, date, datetime, dict, float, int, list, str" + fullSuffix;
-        }
-        if ((ModelUtils.isMapSchema(p) || "object".equals(p.getType())) && getAdditionalProperties(p) != null) {
+        Boolean v3WithCompositionAddPropsAnyTypeSchemaCase = (getAdditionalProperties(p) != null && emptySchema.equals(getAdditionalProperties(p)) && originalSpecVersion.equals("3"));
+        if (isFreeFormObject(p) && v3WithCompositionAddPropsAnyTypeSchemaCase) {
+            // v3 code path, use case: type object schema with no other schema info
+            return prefix + "{str: (bool, date, datetime, dict, float, int, list, str, none_type)}" + fullSuffix;
+        } else if ((ModelUtils.isMapSchema(p) || "object".equals(p.getType())) && getAdditionalProperties(p) != null) {
             Schema inner = getAdditionalProperties(p);
             return prefix + "{str: " + getTypeString(inner, "(", ")", referencedModelNames) + "}" + fullSuffix;
         } else if (ModelUtils.isArraySchema(p)) {
@@ -835,7 +935,7 @@ protected void addAdditionPropertiesToCodeGenModel(CodegenModel codegenModel, Sc
             // The 'addProps' may be a reference, getTypeDeclaration will resolve
             // the reference.
             List<String> referencedModelNames = new ArrayList<String>();
-            codegenModel.additionalPropertiesType = getTypeString(addProps, "", "", referencedModelNames);
+            getTypeString(addProps, "", "", referencedModelNames);
             if (referencedModelNames.size() != 0) {
                 // Models that are referenced in the 'additionalPropertiesType' keyword
                 // must be added to the imports.

modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonLegacyClientCodegen.java

@@ -35,6 +35,7 @@ public class PythonLegacyClientCodegen extends AbstractPythonCodegen implements
     // nose is a python testing framework, we use pytest if USE_NOSE is unset
     public static final String USE_NOSE = "useNose";
     public static final String RECURSION_LIMIT = "recursionLimit";
+    public static final String PYTHON_ATTR_NONE_IF_UNSET = "pythonAttrNoneIfUnset";
 
     protected String packageUrl;
     protected String apiDocPath = "docs/";

modules/openapi-generator/src/main/resources/python/model_doc.mustache

@@ -28,9 +28,9 @@ Name | Type | Description | Notes
 {{#optionalVars}}
 **{{name}}** | {{^complexType}}**{{dataType}}**{{/complexType}}{{#complexType}}[**{{dataType}}**]({{complexType}}.md){{/complexType}} | {{description}} | [optional] {{#isReadOnly}}[readonly] {{/isReadOnly}}{{#defaultValue}} if omitted the server will use the default value of {{{.}}}{{/defaultValue}}
 {{/optionalVars}}
-{{#additionalPropertiesType}}
-**any string name** | **{{additionalPropertiesType}}** | any string name can be used but the value must be the correct type | [optional]
-{{/additionalPropertiesType}}
+{{#additionalProperties}}
+**any string name** | {{^complexType}}**{{dataType}}**{{/complexType}}{{#complexType}}[**{{dataType}}**]({{complexType}}.md){{/complexType}} | any string name can be used but the value must be the correct type | [optional]
+{{/additionalProperties}}
 
 [[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md)
 

modules/openapi-generator/src/main/resources/python/model_templates/classvars.mustache

@@ -60,7 +60,7 @@
 {{/optionalVars}}
     }
 
-{{#additionalPropertiesType}}
+{{#additionalProperties}}
     @cached_property
     def additional_properties_type():
         """
@@ -72,11 +72,11 @@
         lazy_import()
 {{/-first}}
 {{/imports}}
-        return ({{{additionalPropertiesType}}},)  # noqa: E501
-{{/additionalPropertiesType}}
-{{^additionalPropertiesType}}
+        return ({{{dataType}}},)  # noqa: E501
+{{/additionalProperties}}
+{{^additionalProperties}}
     additional_properties_type = None
-{{/additionalPropertiesType}}
+{{/additionalProperties}}
 
     _nullable = {{#isNullable}}True{{/isNullable}}{{^isNullable}}False{{/isNullable}}
 

modules/openapi-generator/src/main/resources/python/model_templates/method_init_composed.mustache

@@ -10,7 +10,52 @@
         '_additional_properties_model_instances',
     ])
 
-{{> model_templates/method_init_shared }}
+    @convert_js_args_to_python_args
+    def __init__(self, *args, **kwargs):  # noqa: E501
+        """{{classname}} - a model defined in OpenAPI
+
+        Keyword Args:
+{{#requiredVars}}
+{{#defaultValue}}
+            {{name}} ({{{dataType}}}):{{#description}} {{{description}}}.{{/description}} defaults to {{{defaultValue}}}{{#allowableValues}}, must be one of [{{#enumVars}}{{{value}}}, {{/enumVars}}]{{/allowableValues}}  # noqa: E501
+{{/defaultValue}}
+{{^defaultValue}}
+            {{name}} ({{{dataType}}}):{{#description}} {{{description}}}{{/description}}
+{{/defaultValue}}
+{{/requiredVars}}
+{{> model_templates/docstring_init_required_kwargs }}
+{{#optionalVars}}
+            {{name}} ({{{dataType}}}):{{#description}} {{{description}}}.{{/description}} [optional]{{#defaultValue}} if omitted the server will use the default value of {{{defaultValue}}}{{/defaultValue}}  # noqa: E501
+{{/optionalVars}}
+        """
+
+{{#requiredVars}}
+{{#defaultValue}}
+        {{name}} = kwargs.get('{{name}}', {{{defaultValue}}})
+{{/defaultValue}}
+{{/requiredVars}}
+        _check_type = kwargs.pop('_check_type', True)
+        _spec_property_naming = kwargs.pop('_spec_property_naming', False)
+        _path_to_item = kwargs.pop('_path_to_item', ())
+        _configuration = kwargs.pop('_configuration', None)
+        _visited_composed_classes = kwargs.pop('_visited_composed_classes', ())
+
+        if args:
+            raise ApiTypeError(
+                "Invalid positional arguments=%s passed to %s. Remove those invalid positional arguments." % (
+                    args,
+                    self.__class__.__name__,
+                ),
+                path_to_item=_path_to_item,
+                valid_classes=(self.__class__,),
+            )
+
+        self._data_store = {}
+        self._check_type = _check_type
+        self._spec_property_naming = _spec_property_naming
+        self._path_to_item = _path_to_item
+        self._configuration = _configuration
+        self._visited_composed_classes = _visited_composed_classes + (self.__class__,)
 
         constant_args = {
             '_check_type': _check_type,
@@ -19,28 +64,18 @@
             '_configuration': _configuration,
             '_visited_composed_classes': self._visited_composed_classes,
         }
-        required_args = {
-{{#requiredVars}}
-            '{{name}}': {{name}},
-{{/requiredVars}}
-        }
-        model_args = {}
-        model_args.update(required_args)
-        model_args.update(kwargs)
         composed_info = validate_get_composed_info(
-            constant_args, model_args, self)
+            constant_args, kwargs, self)
         self._composed_instances = composed_info[0]
         self._var_name_to_model_instances = composed_info[1]
         self._additional_properties_model_instances = composed_info[2]
-        unused_args = composed_info[3]
+        discarded_args = composed_info[3]
 
-        for var_name, var_value in required_args.items():
-            setattr(self, var_name, var_value)
         for var_name, var_value in kwargs.items():
-            if var_name in unused_args and \
+            if var_name in discarded_args and \
                         self._configuration is not None and \
                         self._configuration.discard_unknown_keys and \
-                        not self._additional_properties_model_instances:
+                        self._additional_properties_model_instances:
                 # discard variable.
                 continue
             setattr(self, var_name, var_value)