Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] [Python-experimental] Certain types in nested component schemas are interpreted as DynamicSchema #3

Closed
4 of 6 tasks
mikeknep opened this issue Jun 29, 2022 · 11 comments · Fixed by OpenAPITools/openapi-generator#12758
Labels
invalid This doesn't seem right

Comments

@mikeknep
Copy link

mikeknep commented Jun 29, 2022

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

I am unable to construct a python-experimental-generated object that includes both a nested component schema property and allOf. Certain types (integer, number, boolean) become impossible to construct, being interpreted as a DynamicSchema instead of the actual type.

openapi-generator version

6.0.0

OpenAPI declaration file content or url
openapi: 3.0.0

info:
  version: 1.0.0
  title: BugReproduction

paths: {}

components:
  schemas:
    Dog:
      type: object
      additionalProperties: false
      allOf:
        - $ref: "#/components/schemas/Animal"
      properties:
        species:
          enum: ["canine"]
        age:
          $ref: "#/components/schemas/Age"

    Age:
      type: object
      additionalProperties: false
      properties:
        unit:
          type: string
        value:
          type: integer

    Cat:
      type: object
      additionalProperties: false
      allOf:
        - $ref: "#/components/schemas/Animal"
      properties:
        species:
          enum: ["feline"]
        favorite_milk:
          type: string

    Animal:
      type: object
      properties:
        species:
          type: string
      discriminator:
        propertyName: species
        mapping:
          canine: "#/components/schemas/Dog"
          feline: "#/components/schemas/Cat"
Generation Details

I've generated the schema above with both the standard python generator and python-experimental for side-by-side comparisons:

# Using the "standard" python generator, for comparison
openapi-generator generate \
  -i openapi_specs/main.yml \
  -g python \
  --additional-properties generateSourceCodeOnly=true \
  --additional-properties packageName=pystandard

# Using the python-experimental generator
_JAVA_OPTIONS="--add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED" \
  openapi-generator generate \
  -i openapi_specs/main.yml \
  -g python-experimental \
  --additional-properties generateSourceCodeOnly=true \
  --additional-properties packageName=pyexperimental

(Note: the _JAVA_OPTIONS are passed to avoid this error:)

: java.lang.reflect.InaccessibleObjectException: Unable to make field transient java.util.HashMap$Node[] java.util.HashMap.table accessible: module java.base does not "opens java.util" to unnamed module @2c1a7075
Steps to reproduce
  • Run the generator steps above with the schema above
  • Attempt to create an object (comment in/out the different imports to compare standard and experimental)
# from pystandard.model.age import Age
# from pystandard.model.dog import Dog
from pyexperimental.model.age import Age
from pyexperimental.model.dog import Dog

def test_dog():
    Dog(species="canine", age=Age(unit="years", value=2))

The standard Dog can be created just fine, but the experimental Dog cannot be constructed:

pyexperimental.exceptions.ApiTypeError: Invalid type. Required value type is one of [Decimal, FileIO, NoneType, bool, bytes, frozendict, str, tuple] and passed type was DynamicSchema at ['args[0]']['age']['value']

I've changed the type of value to a few different simple types—string works fine, but integer/number/boolean all fail.

Related issues/PRs

None to my knowledge

Suggest a fix

Per @spacether:

Hmm this may be because Animal sees Age as an AnyTypeSchema but a Dog instance is a DynamicSchema instance but it should be let through because Age is a DictSchema.

For wider context of my real-world use case: I have an API endpoint with a polymorphic request body—it can accept oneOf several animals (to run with the reproduction example). I have several different animals (not just two), and while most have pretty distinct properties (as above—Dog and Cat are quite distinct from one another), a few of them happen to have the exact same set of fields (say... name and number_of_legs). I need to be able to disambiguate between two animals with identical properties, so I'm adding the species field as a discriminator. Each specific animal defines species as an enum-of-one field to ensure a client doesn't create something like Giraffe(species="canine").

In my real-world example there are additional, common, sometimes-optional properties that all Animals define, which is why I'm using allOf. Maybe the species property and discriminator mapping is not actually necessary in this case (since its hard-coded on each concrete animal schema), and I should just perform server-side switching using that field myself? I had assumed providing the discriminator block would yield some benefit in generated code. (Still pretty new to this tool in general so I don't know for sure, but all the polymorphism examples in the docs talk about using it.)

@spacether
Copy link
Contributor

spacether commented Jun 29, 2022

This is an important use case. Thank you for reporting this bug.
I know how to fix it.
Any instantiated model is a DynamicSchema because it is a class that has been composited using multiple other classes.
This error is happening because the age values is kept as an Age instance, but age is an additonalProperty in Animal, so it expects the value to be a (dict/list/str/int/bool/None) and it is an instance of DynamicSchema instead.

The solution is to convert all ingested instances into their primitive types (dict/list/str/int/bool/None) and store the erlier completed validations from the DynamicSchema class into the ValidationMetadata. Then those schema classes will not have to rerun validations that were already run in Age when a Dog instance is instantiated using an age=Age(...) instance
I will work on this after I finish OpenAPITools/openapi-generator#12619

@spacether
Copy link
Contributor

@mikeknep a short term work around for this is to use Dog._from_openapi_data(dict(species="canine", age=dict(unit="years", value=2)))

@spacether
Copy link
Contributor

All fixed, please pull the latest master branch (commit d6b360d or later) and regenerate your client using it.

@wing328 wing328 transferred this issue from OpenAPITools/openapi-generator Sep 25, 2022
@caniko
Copy link

caniko commented Sep 25, 2022

@mikeknep what is this HashMap error you are referring to? I am also getting it! Using your workaround for now!

Note that python-experimental -> python now

My spec yaml, it has been validated.

Run:

openapi-generator generate --input-spec /home/can/Projects/SynDB/interface/syndb-pi/syndb_openapi.yaml --output /home/can/Projects/SynDB/interface/syndb-pi/syndb_openapi_gen_python --generator-name python --additional-properties=packageName=syndb_client,packageVersion=0.4.1,projectName=syndb_client

Error:

ndb_client,packageVersion=0.4.1,projectName=syndb_client
[main] INFO  o.o.codegen.DefaultGenerator - Generating with dryRun=false
[main] INFO  o.o.c.ignore.CodegenIgnoreProcessor - Output directory (/home/can/Projects/SynDB/interface/syndb-pi/syndb_openapi_gen_python) does not exist, or is inaccessible. No file (.openapi-generator-ignore) will be evaluated.
[main] INFO  o.o.codegen.DefaultGenerator - OpenAPI Generator: python (client)
[main] INFO  o.o.codegen.DefaultGenerator - Generator 'python' is considered stable.
[main] INFO  o.o.c.l.AbstractPythonCodegen - Environment variable PYTHON_POST_PROCESS_FILE not defined so the Python code may not be properly formatted. To define it, try 'export PYTHON_POST_PROCESS_FILE="/usr/local/bin/yapf -i"' (Linux/Mac)
[main] INFO  o.o.c.l.AbstractPythonCodegen - NOTE: To enable file post-processing, 'enablePostProcessFile' must be set to `true` (--enable-post-process-file for CLI).
[main] INFO  o.o.c.ignore.CodegenIgnoreProcessor - Output directory (/home/can/Projects/SynDB/interface/syndb-pi/syndb_openapi_gen_python) does not exist, or is inaccessible. No file (.openapi-generator-ignore) will be evaluated.
[main] INFO  o.o.c.languages.PythonClientCodegen - Environment variable PYTHON_POST_PROCESS_FILE not defined so the Python code may not be properly formatted. To define it, try 'export PYTHON_POST_PROCESS_FILE="/usr/local/bin/yapf -i"' (Linux/Mac)
[main] INFO  o.o.c.languages.PythonClientCodegen - NOTE: To enable file post-processing, 'enablePostProcessFile' must be set to `true` (--enable-post-process-file for CLI).
[main] INFO  o.o.c.languages.PythonClientCodegen - generateAliasAsModel is hard coded to true in this generator. Alias models will only be generated if they contain validations or enums
[main] INFO  o.o.codegen.utils.URLPathUtils - 'host' (OAS 2.0) or 'servers' (OAS 3.0) not defined in the spec. Default to [http://localhost] for server URL [http://localhost/]
[main] INFO  o.o.codegen.utils.URLPathUtils - 'host' (OAS 2.0) or 'servers' (OAS 3.0) not defined in the spec. Default to [http://localhost] for server URL [http://localhost/]
[main] INFO  o.o.codegen.DefaultGenerator - Model Body_auth_database_login_user_auth_database_login_post not generated since it's marked as unused (due to form parameters) and `skipFormModel` (global property) set to true (default)
[main] INFO  o.o.codegen.DefaultGenerator - Model Body_auth_jwt_login_user_auth_jwt_login_post not generated since it's marked as unused (due to form parameters) and `skipFormModel` (global property) set to true (default)
Exception in thread "main" java.lang.RuntimeException: Could not generate model 'BearerResponse'
        at org.openapitools.codegen.DefaultGenerator.generateModels(DefaultGenerator.java:563)
        at org.openapitools.codegen.DefaultGenerator.generate(DefaultGenerator.java:912)
        at org.openapitools.codegen.cmd.Generate.execute(Generate.java:465)
        at org.openapitools.codegen.cmd.OpenApiGeneratorCommand.run(OpenApiGeneratorCommand.java:32)
        at org.openapitools.codegen.OpenAPIGenerator.main(OpenAPIGenerator.java:66)
Caused by: com.github.jknack.handlebars.HandlebarsException: model.handlebars:3:3: java.lang.reflect.InaccessibleObjectException: Unable to make field transient java.util.HashMap$Node[] java.util.HashMap.table accessible: module java.base does not "opens java.util" to unnamed module @42e26948
    model.handlebars:3:3
        at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354)
        at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
        at java.base/java.lang.reflect.Field.checkCanSetAccessible(Field.java:180)
        at java.base/java.lang.reflect.Field.setAccessible(Field.java:174)
        at com.github.jknack.handlebars.context.FieldValueResolver$FieldMember.setAccessible(FieldValueResolver.java:150)
        at com.github.jknack.handlebars.context.MemberValueResolver.cache(MemberValueResolver.java:82)
        at com.github.jknack.handlebars.context.MemberValueResolver.resolve(MemberValueResolver.java:54)
        at com.github.jknack.handlebars.Context$CompositeValueResolver.resolve(Context.java:199)
        at com.github.jknack.handlebars.internal.path.PropertyPath.eval(PropertyPath.java:52)
        at com.github.jknack.handlebars.Context$PathExpressionChain.next(Context.java:361)
        at com.github.jknack.handlebars.Context$PathExpressionChain.eval(Context.java:381)
        at com.github.jknack.handlebars.Context.get(Context.java:618)
        at com.github.jknack.handlebars.internal.Partial.override(Partial.java:253)
        at com.github.jknack.handlebars.internal.Partial.merge(Partial.java:226)
        at com.github.jknack.handlebars.internal.BaseTemplate.apply(BaseTemplate.java:125)
        at com.github.jknack.handlebars.internal.TemplateList.merge(TemplateList.java:95)
        at com.github.jknack.handlebars.internal.BaseTemplate.apply(BaseTemplate.java:125)
        at com.github.jknack.handlebars.internal.BaseTemplate.apply(BaseTemplate.java:113)
        at org.openapitools.codegen.templating.HandlebarsEngineAdapter.compileTemplate(HandlebarsEngineAdapter.java:92)
        at org.openapitools.codegen.TemplateManager.write(TemplateManager.java:163)
        at org.openapitools.codegen.DefaultGenerator.processTemplateToFile(DefaultGenerator.java:1058)
        at org.openapitools.codegen.DefaultGenerator.processTemplateToFile(DefaultGenerator.java:1045)
        at org.openapitools.codegen.DefaultGenerator.generateModel(DefaultGenerator.java:402)
        at org.openapitools.codegen.DefaultGenerator.generateModels(DefaultGenerator.java:554)
        ... 4 more
Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make field transient java.util.HashMap$Node[] java.util.HashMap.table accessible: module java.base does not "opens java.util" to unnamed module @42e26948
        ... 28 more

@spacether spacether reopened this Sep 25, 2022
@spacether
Copy link
Contributor

@caniko are you able to instantiate models using SomeModel__new__? for the issue use case?
If so how about I edit this issue description to identify this issue as python, unable to generate code due to java layer bug, java.lang.RuntimeException?

@caniko
Copy link

caniko commented Sep 25, 2022

@spacether I am not sure I follow. Could you expand a bit?

@spacether
Copy link
Contributor

spacether commented Sep 25, 2022

The original issue has 2 pieces

  • piece 1: java layer throws an exception, fixable with _JAVA_OPTIONS
  • piece 2: generated python code was unable to ingest model payloads passed in as arguments in SomeModel.__new__ for some use case

Piece 2 has been fixed in an earlier PR, that' why the issue was closed.
Are you seeing errors for pieces 1 + 2?
If you are only seeing the piece 1 issue then the issue description should be updated to be more specific.

@caniko
Copy link

caniko commented Sep 25, 2022

Actually, the generated code are missing some models.

I used these options: _JAVA_OPTIONS="--add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED"

Note that the error on my first post was resolved with these options. So, piece 1 is not an issue anymore.
So it is not the models can't be ingested, they don't even exist, note that this is just a subset of the models and not all of them.

I am working with application/x-www-form-urlencoded atm, and all of the models related to this one are missing (2 in total).

@caniko
Copy link

caniko commented Sep 25, 2022

My models were fine on python-prior, but it had its own set of problems with validation*.. I was hoping this new approach would fix my issues, it just introduced new ones. I am willing to go back and forth on chat for a few hours: Are you on discord or element/matrix?

The code gen is just fine with dart-dio, the issue must be on the python generators' side of things.

* = mixup between operators ge and le, not sure how this is possible.

@spacether
Copy link
Contributor

Missing models are a very fixable bug. Which models are missing?
Sure, you can msg me at https://join.slack.com/t/openapi-generator/shared_invite/zt-12jxxd7p2-XUeQM~4pzsU9x~eGLQqX2g and I am Justin Black there

@spacether
Copy link
Contributor

spacether commented Sep 25, 2022

Closing this issue Certain types in nested component schemas are interpreted as DynamicSchema because it is fixed
If users have issues where:

  • models are not being generated and they should be
  • they see a java exception java.lang.reflect.InaccessibleObjectException
    Please open new issues so they can be fixed

@caniko 's issue happened because of these logged messages:

[main] INFO  o.o.codegen.DefaultGenerator - Model Body_auth_database_login_user_auth_database_login_post not generated since it's marked as unused (due to form parameters) and `skipFormModel` (global property) set to true (default)
[main] INFO  o.o.codegen.DefaultGenerator - Model Body_auth_jwt_login_user_auth_jwt_login_post not generated since it's marked as unused (due to form parameters) and `skipFormModel` (global property) set to true (default)

@spacether spacether added the invalid This doesn't seem right label Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
3 participants