diff --git a/fix-python-fastapi-additionalproperties-true-parent.md b/fix-python-fastapi-additionalproperties-true-parent.md new file mode 100644 index 000000000000..01c9d3a80f08 --- /dev/null +++ b/fix-python-fastapi-additionalproperties-true-parent.md @@ -0,0 +1,97 @@ +# Fix Plan: python-fastapi additionalProperties:true generates wrong parent class + +## Issue +GitHub issue #20153: `python-fastapi` generator produces invalid imports and wrong base class for +models that have `additionalProperties: true` (a boolean) at the schema level. + +**Broken output:** +```python +from openapi_server.models.object import object +class HTTPRequest(object): + ... +``` + +**Expected output:** +```python +from pydantic import BaseModel +class HTTPRequest(BaseModel): + ... +``` + +## Root Cause + +`ModelUtils.isMapSchema()` returns `true` for any schema with `additionalProperties: true` because +the boolean value counts as "map-like" per the OAS spec. This triggers +`updateModelForObject()` → `addAdditionPropertiesToCodeGenModel()` → `addParentContainer()` → +`addParentFromContainer()`, which sets `model.parent = toInstantiationType(schema)`. + +For `additionalProperties: true`, `ModelUtils.getAdditionalProperties()` returns `new Schema()` +(an empty schema), and `getSchemaType(new Schema())` returns `"object"`. + +So `model.parent` becomes `"object"` — Python's built-in type. Then in +`AbstractPythonCodegen.postProcessModels()`: + +```java +if (!StringUtils.isEmpty(model.parent)) { + modelImports.add(model.parent); // → "from openapi_server.models.object import object" +} +``` + +And the mustache template: +```mustache +class {{classname}}({{#parent}}{{{.}}}{{/parent}}{{^parent}}BaseModel{{/parent}}): +``` + +generates `class HTTPRequest(object):` instead of `class HTTPRequest(BaseModel):`. + +`PythonClientCodegen` already overrides `addAdditionPropertiesToCodeGenModel()` to avoid this, +but `PythonFastAPIServerCodegen` and `AbstractPythonConnexionServerCodegen` do not — they fall +through to `DefaultCodegen` which calls `addParentContainer()`. + +## Fix + +Move the correct `addAdditionPropertiesToCodeGenModel()` override from `PythonClientCodegen` up +to `AbstractPythonCodegen`. This ensures ALL Python generators (FastAPI, Connexion, client) share +the same correct behaviour without duplicating the fix. + +```java +// AbstractPythonCodegen.java +@Override +protected void addAdditionPropertiesToCodeGenModel(CodegenModel codegenModel, Schema schema) { + final Schema additionalProperties = ModelUtils.getAdditionalProperties(schema); + if (additionalProperties != null) { + codegenModel.additionalPropertiesType = getSchemaType(additionalProperties); + } + // Do NOT call addParentContainer() — Python uses the `additional_properties` field + // pattern in templates rather than class inheritance to represent additionalProperties. + // Calling the default would set model.parent = "object" which generates an invalid + // "from openapi_server.models.object import object" import. +} +``` + +Remove the duplicate override from `PythonClientCodegen`. + +## Test + +Add a regression test in `PythonFastAPIServerCodegenTest` using a new fixture spec +`src/test/resources/bugs/issue_20153.yaml` that includes a schema with +`additionalProperties: true` and named properties (modelling the TAMS `http-request.json`). + +The test verifies: +- `from openapi_server.models.object import object` is NOT present in the generated model +- `class HttpRequest(BaseModel):` IS present (correct base class) +- `additional_properties: Dict[str, Any] = {}` IS present (the correct pattern) + +## Files Changed + +1. `modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractPythonCodegen.java` + — add `addAdditionPropertiesToCodeGenModel()` override + +2. `modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java` + — remove now-duplicate `addAdditionPropertiesToCodeGenModel()` override + +3. `modules/openapi-generator/src/test/resources/bugs/issue_20153.yaml` + — new minimal OAS 3.1 fixture + +4. `modules/openapi-generator/src/test/java/org/openapitools/codegen/python/PythonFastAPIServerCodegenTest.java` + — new regression test `testAdditionalPropertiesTrueUsesBaseModel` diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractPythonCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractPythonCodegen.java index ee16c3805081..294486c781c3 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractPythonCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractPythonCodegen.java @@ -1529,6 +1529,25 @@ public boolean isDataTypeString(String dataType) { return "str".equals(dataType); } + /** + * Python generators represent additionalProperties via a dedicated `additional_properties` + * field in the generated model class rather than through class inheritance. Setting + * model.parent to "object" (the return value of toInstantiationType when additionalProperties + * is the boolean true) causes the template to emit an invalid import: + * from openapi_server.models.object import object + * and an incorrect base class declaration: + * class Foo(object): + * Override here to only set additionalPropertiesType (used for typing) without calling + * addParentContainer(), keeping model.parent null so the template falls back to BaseModel. + */ + @Override + protected void addAdditionPropertiesToCodeGenModel(CodegenModel codegenModel, Schema schema) { + final Schema additionalProperties = ModelUtils.getAdditionalProperties(schema); + if (additionalProperties != null) { + codegenModel.additionalPropertiesType = getSchemaType(additionalProperties); + } + } + /* The definition for a Python type. * * This encapsulate all the type definition: the actual type, and potentially: diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java index fdf19feff107..99fbdd6044a3 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java @@ -17,7 +17,6 @@ package org.openapitools.codegen.languages; -import io.swagger.v3.oas.models.media.Schema; import io.swagger.v3.oas.models.security.SecurityScheme; import lombok.Setter; import org.apache.commons.lang3.Strings; @@ -25,7 +24,6 @@ import org.openapitools.codegen.meta.GeneratorMetadata; import org.openapitools.codegen.meta.Stability; import org.openapitools.codegen.meta.features.*; -import org.openapitools.codegen.utils.ModelUtils; import org.openapitools.codegen.utils.ProcessUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -457,15 +455,6 @@ public String generatorLanguageVersion() { return "3.10+"; } - @Override - protected void addAdditionPropertiesToCodeGenModel(CodegenModel codegenModel, Schema schema) { - final Schema additionalProperties = ModelUtils.getAdditionalProperties(schema); - - if (additionalProperties != null) { - codegenModel.additionalPropertiesType = getSchemaType(additionalProperties); - } - } - @Override public String escapeReservedWord(String name) { if (this.reservedWordsMappings().containsKey(name)) { diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/python/PythonFastAPIServerCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/python/PythonFastAPIServerCodegenTest.java index ff381bacecbd..bdeadadf0a9b 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/python/PythonFastAPIServerCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/python/PythonFastAPIServerCodegenTest.java @@ -106,6 +106,21 @@ public void testToPythonExamplePrefersExampleOverExamples() { Assert.assertEquals(codegen.exposeToPythonExample(cp), "\"doggie\""); } + @Test(description = "schema with additionalProperties:true inherits BaseModel, not object (issue #20153)") + public void testAdditionalPropertiesTrueUsesBaseModel() throws IOException { + final DefaultCodegen codegen = new PythonFastAPIServerCodegen(); + final String outputPath = generateFiles(codegen, "src/test/resources/bugs/issue_20153.yaml"); + final Path model = Paths.get(outputPath + "src/openapi_server/models/http_request.py"); + + assertFileExists(model); + // Must use BaseModel as the base class, not Python's built-in object type + assertFileContains(model, "class HttpRequest(BaseModel):"); + // Must carry the additional_properties field for the additionalProperties:true schema + assertFileContains(model, "additional_properties: Dict[str, Any] = {}"); + // Must NOT generate a bad import that tries to import object as a module + assertFileNotContains(model, "from openapi_server.models.object import object"); + } + @Test(description = "binary multipart form fields are typed as FastAPI UploadFile") public void testBinaryMultipartFieldUsesUploadFile() throws IOException { final DefaultCodegen codegen = new PythonFastAPIServerCodegen(); diff --git a/modules/openapi-generator/src/test/resources/bugs/issue_20153.yaml b/modules/openapi-generator/src/test/resources/bugs/issue_20153.yaml new file mode 100644 index 000000000000..90ec3d9d0e04 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/bugs/issue_20153.yaml @@ -0,0 +1,45 @@ +openapi: "3.1.0" +info: + title: Issue 20153 - additionalProperties true generates wrong parent class + version: "1.0" +paths: + /requests: + post: + operationId: createRequest + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/HttpRequest' + responses: + "200": + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/HttpRequest' +components: + schemas: + HttpRequest: + title: HTTP Request + description: Gives information on a particular http request a client should perform + type: object + required: + - url + properties: + url: + description: The URL to make the request to + type: string + body: + description: The body of the request + type: string + content-type: + description: The content type to use + type: string + headers: + description: Additional headers to include + type: object + additionalProperties: + type: string + additionalProperties: true