[BUG][Python] Reference to schema property produces undefined types

cmin764 commented 10 months ago

Bug Report Checklist

[x] Have you provided a full/minimal spec to reproduce the issue?
[x] Have you validated the input using an OpenAPI validator (example)?
[x] Have you tested with the latest master to confirm the issue still exists?
[x] Have you searched for related issues/PRs?
[x] What's the actual output vs expected output?
[ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

Given Robocorp API the generated response models contain undefined types coming from pagination_details.PaginationDetails like HasMore and Next. E.g. list_processes200_response.py:

class ListProcesses200Response(BaseModel):
    """
    ListProcesses200Response
    """
    next: Next = Field(...)
    has_more: HasMore = Field(...)
    data: conlist(ListProcesses200ResponseDataInner) = Field(...)
    __properties = ["next", "has_more", "data"]

where there's no resource created for these Next and HasMore (non existing model classes), and the only reference of them I found in the pagination_details.py model as attributes.

openapi-generator version

7.0.1, 8.0.0-SNAPSHOT, 7.1.0-SNAPSHOT

OpenAPI declaration file content or url

--> https://robocorp.com/api/openapi.json

Generation Details

% openapi-generator-cli generate -g python -i https://robocorp.com/api/openapi.json -c openapiconf.yaml --skip-validate-spec -o src -t templates

openapiconf.yaml:

generateSourceCodeOnly: true
packageName: robocorp.workspace
packageUrl: https://github.com/robocorp/robo/
packageVersion: 0.1.0
projectName: robocorp-workspace

Generated source.

Steps to reproduce

Just run the command above and look into the models using these pagination properties: HasMore, Next.

Related issues/PRs

Couldn't find one.

Suggest a fix

The problem can be solved in multiple ways:

A. ~The codegen detects that these properties are coming from a shared resource and the models including them (has_more, next) should include and use the parent resource actually, through pagination_details attribute taking pagination_details.PaginationDetails as type. (maybe it works out of the box if the OpenAPI schema will return such a resource instead of the props alone)~
B. ~The codegen generates such individual models for properties from PaginationDetails like HasMore and Next so these can be imported and used by the models as they are.~
C. The codegen will simply detect references to schema properties and will just use primitive types as it should. (see structure below)

Property references from a common shared schema:

"properties": {
                                        "next": {
                                            "$ref": "#/components/schemas/paginationDetails/properties/next"
                                        },
                                        "has_more": {
                                            "$ref": "#/components/schemas/paginationDetails/properties/has_more"
                                        },
                                        "data": {
                                            "type": "array",
                                            "items": {
                                                "$ref": "#/components/schemas/WorkerGroupResource"
                                            }
                                        }
                                    }

The schema:

"paginationDetails": {
                "type": "object",
                "required": [
                    "next",
                    "has_more"
                ],
                "properties": {
                    "next": {
                        "type": "string",
                        "description": "The full URL to access the next set of results. Null if there are no next set of results.",
                        "nullable": true,
                        "format": "url"
                    },
                    "has_more": {
                        "type": "boolean",
                        "description": "Whether or not there are more elements available after this set. If false, this set comprises the end of the list."
                    }
                }
            }

Expecting to get something like this in every model referencing them:

class PaginationDetails(BaseModel):
    """
    PaginationDetails
    """
    next: Optional[StrictStr] = Field(description="The full URL to access the next set of results. Null if there are no next set of results.")
    has_more: StrictBool = Field(description="Whether or not there are more elements available after this set. If false, this set comprises the end of the list.")
    __properties: ClassVar[List[str]] = ["next", "has_more"]

wing328 commented 10 months ago

Thanks for reporting the issue.

Can you please try the latest master as we've updated the python client generator to generate code with pydantic v2 instead of v1?

SNAPSHOT JAR can be found in the project's README.

cmin764 commented 10 months ago

That sounds like a great idea, will do!

cmin764 commented 10 months ago

@wing328 I get an error during the project build:

[INFO] Reactor Summary for openapi-generator-project 7.1.0-SNAPSHOT:
[INFO]
[INFO] openapi-generator-project .......................... SUCCESS [01:10 min]
[INFO] openapi-generator-core ............................. SUCCESS [01:32 min]
[INFO] openapi-generator (core library) ................... FAILURE [01:31 min]
[INFO] openapi-generator (executable) ..................... SKIPPED
[INFO] openapi-generator (maven-plugin) ................... SKIPPED
[INFO] openapi-generator-gradle-plugin (maven wrapper) .... SKIPPED
[INFO] openapi-generator-online ........................... SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  04:29 min
[INFO] Finished at: 2023-11-07T12:02:33+02:00
[INFO] ------------------------------------------------------------------------
[INFO] 38 goals, 36 executed, 2 from cache, saving at least 2s
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.11.0:testCompile (default-testCompile) on project openapi-generator: Fatal error compiling: java.lang.NoSuchFieldError: Class com.sun.tools.javac.tree.JCTree$JCImport does not have member field 'com.sun.tools.javac.tree.JCTree qualid' -> [Help 1]

Currently reading this Stackoverflow, any ideas for a quicker solution?

My JDK/RE:

% java -version
java version "21" 2023-09-19 LTS
Java(TM) SE Runtime Environment (build 21+35-LTS-2513)
Java HotSpot(TM) 64-Bit Server VM (build 21+35-LTS-2513, mixed mode, sharing)

cmin764 commented 10 months ago

I just downloaded the 8.0.0 snapshot and I end up with this kind of model now:

class ListProcesses200Response(BaseModel):
    """
    ListProcesses200Response
    """
    next: Next
    has_more: HasMore
    data: List[ListProcesses200ResponseDataInner]
    __properties: ClassVar[List[str]] = ["next", "has_more", "data"]

⚠️ So now beside the fact that I still don't get models generated for Next and HasMore, I'm ending up with a non-existing reference to ClassVar (from typing I'd assume).

wing328 commented 10 months ago

instead of building it locally, what about using https://oss.sonatype.org/content/repositories/snapshots/org/openapitools/openapi-generator-cli/7.1.0-SNAPSHOT/ instead?

or can you try skipping the tests: mvn clean package -DskipTests -Dmaven.javadoc.skip=true ?

I don't think we've tested this project with JDK 21 yet. Will do it later and fix whatever issues that come up.

cmin764 commented 10 months ago

instead of building it locally, what about using https://oss.sonatype.org/content/repositories/snapshots/org/openapitools/openapi-generator-cli/7.1.0-SNAPSHOT/ instead?

or can you try skipping the tests: mvn clean package -DskipTests -Dmaven.javadoc.skip=true ?

I don't think we've tested this project with JDK 21 yet. Will do it later and fix whatever issues that come up.

Thank you for the latest snapshots URL, tried with openapi-generator-cli-7.1.0-20231108.064355-129.jar and it solves the ClassVar import, but still fails on classifying references to basic data types from a shared schema as primitives in the generated code, thus ending up with non-existing referenced models:

class ListProcesses200Response(BaseModel):
    """
    ListProcesses200Response
    """ # noqa: E501
    next: Next
    has_more: StrictBool
    data: List[ListProcesses200ResponseDataInner]
    __properties: ClassVar[List[str]] = ["next", "has_more", "data"]

By looking through the logs, I see these warnings:

[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined

so is there a problem with the codegen finding these properties defined in the schema?

Tried with replacing with my own:

--type-mappings HasMore=StrictBool,Next=StrictStr --import-mappings HasMore=pydantic.StrictBool,Next=pydantic.StrictStr

but they are still considered models (not primitives) for some reason, and in the end I have to solve issues like: "next": Next.from_dict(obj.get("next")) if obj.get("next") is not None else None, which obviously doesn't work as there's no from_dict() in such primitives treated as models.

Then tried running the codegen with --language-specific-primitives HasMore,Next, and it throws a Java error:

Exception in thread "main" java.lang.RuntimeException: Error! Codegen Property not yet supported in getPydanticType:

And passing --language-specific-primitives StrictBool,StrictStr works, but has no effect on making the template understand it's dealing with primitive types and not wrapping classes.

wing328 commented 10 months ago

got the same with the java client generator so looks like it cannot handle reference property at the moment:

[main] INFO  o.o.codegen.DefaultGenerator - Model ProcessWebhookEnabledEvents not generated since it's an alias to array (without property) and `generateAliasAsModel` is set to false (default)
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/next is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/components/schemas/paginationDetails/properties/has_more is not defined

wing328 commented 10 months ago

can you please PM me via Slack when you've time?

https://join.slack.com/t/openapi-generator/shared_invite/zt-12jxxd7p2-XUeQM~4pzsU9x~eGLQqX2g

cmin764 commented 10 months ago

And then the problem is with the isEnumOrRef attribute, as this is checked in the _mogelgeneric.mustache template as:

{{#items.isEnumOrRef}}
            "{{{baseName}}}": obj.get("{{{baseName}}}"){{^-last}},{{/-last}}
            {{/items.isEnumOrRef}}
            {{^items.isEnumOrRef}}
            "{{{baseName}}}": [{{{items.dataType}}}.from_dict(_item) for _item in obj.get("{{{baseName}}}")] if obj.get("{{{baseName}}}") is not None else None{{^-last}},{{/-last}}
            {{/items.isEnumOrRef}}

And it should generate code with "{{{baseName}}}": obj.get("{{{baseName}}}"){{^-last}},{{/-last}} (which would be correct) if this attribute is set accordingly.

OpenAPITools / openapi-generator