Better support for inline schemas in parameters #12369

wing328 · 2022-05-14T07:05:29Z

better support for inline schemas in parameters
add isModelWithPropertiesOnly method

FYI. @OpenAPITools/generator-core-team

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.
If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

spacether · 2022-05-15T16:19:47Z

modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java

+     * @param schema potentially containing a '$ref'
+     * @return true if it's a model with at least one properties
+     */
+    public static boolean isModelWithPropertiesOnly(Schema schema) {


Do you want this to be true for AnyType models with properties and additionalProperties set? Right now it is true for that case.

Do you want this to be true for array/string/number models with properties and additionalProperties set? Right now it is true for that case.

How about limiting this to type object and AnyType models only?
Also type object with additionalProperties unset is the same as type object with additionalProperties true so why is this code returning different results for those same use cases?

Thanks for the review.

I previously didn't check getAdditionalProperties() but got a few issues (change in samples). Adding the check seems to fix it but you're right that it needs to revised further.

I simply want to set the isModel flag correctly because now isMap is set to true for a simple model. I've not touched the code in modules/openapi-generator/src/main/java/org/openapitools/codegen/IJsonSchemaValidationProperties.java for a while so I welcome suggestions on other way to fix the problem.

Let me look into what is happening with python experimental. I think it is changing because the component schema changed

Root cause investigation is here: https://github.com/OpenAPITools/openapi-generator/pull/12369/files#r876380534

Should this be true if the schema also has anyOf or allOf definitions?

...penapi-generator/src/main/java/org/openapitools/codegen/IJsonSchemaValidationProperties.java

spacether · 2022-05-18T19:04:23Z

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

        if (parameter.getSchema() != null) {
-            parameterSchema = parameter.getSchema();
+            parameterModelName = getParameterDataType(parameter ,parameter.getSchema());
+            parameterSchema = ModelUtils.getReferencedSchema(openAPI, parameter.getSchema());


Something odd is happening with unaliasSchema
The original code extracts the schema, unaliases it, then uses that unaliased schemas to set properties.

This code instead gets the schema from the ref automatically.
So why is unaliasSchema not handling this case automatically originally?

spacether · 2022-05-18T21:26:59Z

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

-        if (parameterDataType != null) {
-            codegenParameter.dataType = parameterDataType;
+        if (parameterModelName != null) {
+            codegenParameter.dataType = parameterModelName;


Why should codegenParameters handle dataType and complexType differently than codegenProperty?

The change above simply uses a different (better) name in the code block as parameterDataType originally aims to store the model name so no change in code logic if I remember correctly.

* Uses unaliasSchema rather than ModelUtils.getReferencedSchema * Fixes python-experimental, delays param schema setting * Samples regenerated * Adds parameterModelName setting back in * Samples regenerated * removes needToSetSchema * Sets schema differently depending on if inline model resolver is used * Adds step for getting ref schema * Samples regen

…Tools/openapi-generator into inline-parameter-enhance

Dimon70007 · 2022-05-19T19:04:09Z

Hi guys. Will this PR fixes #11151 ?

wing328 · 2022-05-20T12:25:35Z

@Dimon70007 please pull the latest master to give it a try.

Dimon70007 · 2022-05-20T13:09:20Z

Thanks. I'll try it this evening.

* master: update gson to newer version in kotlin templates (OpenAPITools#12425) update gson to 2.8.9 in android httpclient (OpenAPITools#12423) update android dependencies to newer versions (OpenAPITools#12421) [Java] Update rest-assured dependencies (OpenAPITools#12420) update cwiki samples Add - Status colors in Wiki, Example per parameter (OpenAPITools#12394) Better support for inline schemas in parameters (OpenAPITools#12369) Upgrade Spring Boot to 2.5.14 / 2.7.0 (OpenAPITools#12408) update java samples Remove javadoc comment for unthrown IOException (OpenAPITools#12401) remove errorObjectSubtype from java client genreator (OpenAPITools#12405) [java][Micronaut] generator fixes (OpenAPITools#11803) Apply style and explode values from encoding for form-encoded request body parameters (OpenAPITools#12162)

Dimon70007 · 2022-05-23T17:49:47Z

Have tried to build from source with

mvn clean package

but there were failed tests

[ERROR] Failures: 
[ERROR]   DefaultGeneratorTest.supportCustomTemplateEngine:313 » Runtime Could not generate api file for 'Ping'
[ERROR]   StringHelpersTest.startsWithSectionalTest:47->evaluate:28 » Handlebars inline@486b7642:1:4: 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 @3b22cdd0
    inline@486b7642:1:4
[ERROR]   StringHelpersTest.startsWithTest:61->evaluate:28 » Handlebars inline@43bd77be:1:2: 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 @3b22cdd0
    inline@43bd77be:1:2
[ERROR]   StringHelpersTest.startsWithYesOverrideTest:81->evaluate:28 » Handlebars inline@689e2599:1:2: 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 @3b22cdd0
    inline@689e2599:1:2
[INFO] 
[ERROR] Tests run: 1634, Failures: 4, Errors: 0, Skipped: 0

Did i do something wrong?

Dimon70007 · 2022-05-25T14:15:17Z

succeed package jar with ./mvnw clean package
but have no luck when parsing schema with many $ref

spacether · 2022-10-09T20:11:57Z

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

+                                    .map(entry -> {
+                                        CodegenProperty property = fromProperty(entry.getKey(), entry.getValue());
+                                        property.baseName = codegenParameter.baseName + "[" + entry.getKey() + "]";
+                                        return property;


Why is property.baseName being modified here?
Can this be made to work without modifying the baseName?
Why not modify name instead or another string property.
baseName is historically used to store the spec case property name.

wing328 added 4 commits May 13, 2022 15:47

better support for inline schema in parameters

eedc6f5

fix parameter model type

195f1da

add new method for model

95ca392

minor update

2b10b64

wing328 marked this pull request as ready for review May 14, 2022 16:45

wing328 added Enhancement: General Inline Schema Handling Schema contains a complex schema in items/additionalProperties/allOf/oneOf/anyOf labels May 14, 2022

wing328 added this to the 6.0.0 milestone May 14, 2022

wing328 added 3 commits May 15, 2022 01:25

fix isModelWithProperties

559409d

fix is model check

33a1b6b

null check for properties

9348b15

spacether reviewed May 15, 2022

View reviewed changes

sorin-florea mentioned this pull request May 18, 2022

[Java] Codegen Bad object serialization when query parameter is Object #907

Open

spacether reviewed May 18, 2022

View reviewed changes

...penapi-generator/src/main/java/org/openapitools/codegen/IJsonSchemaValidationProperties.java Outdated Show resolved Hide resolved

spacether reviewed May 18, 2022

View reviewed changes

spacether and others added 4 commits May 19, 2022 10:50

Merge branch 'inline-parameter-enhance' of https://github.com/OpenAPI…

5ffc8c8

…Tools/openapi-generator into inline-parameter-enhance

fix test

f531f37

bug fix for rust generator

b7aa873

wing328 mentioned this pull request May 19, 2022

[ALL][CLIENT] Fix serialization for query params specified by ref #11864

Closed

5 tasks

This was referenced May 20, 2022

[BUG] [Python] UNKNOWNBASETYPE appear in api py #12404

Closed

[BUG] [OCaml] Generation of unknown_base_type:UNKNOWN_BASE_TYPE isn't valid #8398

Closed

wing328 merged commit c270640 into master May 20, 2022

wing328 deleted the inline-parameter-enhance branch May 20, 2022 12:25

wing328 added the Breaking change (without fallback) label May 22, 2022

oujesky mentioned this pull request Jun 24, 2022

[BUG][JAVA] Java imports for allOf schema in parameter broken in 6.0.0 #12690

Open

6 tasks

spacether mentioned this pull request Oct 9, 2022

[BUG][Elm] Generation crashes with Exception: Property type is missing from getVars #13571

Open

5 tasks

spacether reviewed Oct 9, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better support for inline schemas in parameters #12369

Better support for inline schemas in parameters #12369

wing328 commented May 14, 2022 •

edited

spacether May 15, 2022 •

edited

wing328 May 16, 2022

spacether May 16, 2022

spacether May 18, 2022 •

edited

spacether May 18, 2022

spacether May 18, 2022 •

edited

spacether May 18, 2022

wing328 May 19, 2022

Dimon70007 commented May 19, 2022

wing328 commented May 20, 2022

Dimon70007 commented May 20, 2022

Dimon70007 commented May 23, 2022

Dimon70007 commented May 25, 2022 •

edited

spacether Oct 9, 2022 •

edited

Better support for inline schemas in parameters #12369

Better support for inline schemas in parameters #12369

Conversation

wing328 commented May 14, 2022 • edited

PR checklist

spacether May 15, 2022 • edited

Choose a reason for hiding this comment

wing328 May 16, 2022

Choose a reason for hiding this comment

spacether May 16, 2022

Choose a reason for hiding this comment

spacether May 18, 2022 • edited

Choose a reason for hiding this comment

spacether May 18, 2022

Choose a reason for hiding this comment

spacether May 18, 2022 • edited

Choose a reason for hiding this comment

spacether May 18, 2022

Choose a reason for hiding this comment

wing328 May 19, 2022

Choose a reason for hiding this comment

Dimon70007 commented May 19, 2022

wing328 commented May 20, 2022

Dimon70007 commented May 20, 2022

Dimon70007 commented May 23, 2022

Dimon70007 commented May 25, 2022 • edited

spacether Oct 9, 2022 • edited

Choose a reason for hiding this comment

wing328 commented May 14, 2022 •

edited

spacether May 15, 2022 •

edited

spacether May 18, 2022 •

edited

spacether May 18, 2022 •

edited

Dimon70007 commented May 25, 2022 •

edited

spacether Oct 9, 2022 •

edited