Additional properties support

[antssilva96]

Hey guys, I just stumbled upon this extension and it is exactly what I need!
Thank you for your effort in creating this!

However due to the issue reported in OpenAPITools/openapi-generator#2901 I have to provide an additional property to be able to generate the sources. The property is the outcome of the issue linked above: openApiNullable=false.

I took the liberty of forking the project and implementing the missing functionality in https://github.com/antssilva96/quarkus-openapi-generator/tree/add-additional-properties-support (also fixed what seems like a typo sort of bug in the template for the Authentication Provider).

I wanted to know if not allowing for additional properties was a design decision and if there is some other way of passing the mentioned property. If it was just a missing feature I'll be happy to create a pull request

[ricardozanini]

Hey, @antssilva96, please open a PR with your changes. They are more than welcome! :)

Additional properties were suppressed because, in the Quarkus context, they might not make sense. But in some cases, I understand that some of these properties should be supported via our prefix. We can add them on demand based on the use case.

If the property is applied in codegen phase, use the prefix quarkus.openapi-generator.codegen.spec.<filename>..

[ricardozanini]

Hmm reading again the issue you linked, maybe we can fix this the Quarkus way? What's happening in your case? The import is being generated every time? I think you can remove this import for good in the QuarkusJavaClientCodegen

If that doesn't make sense, you can add as a property like I mentioned above: quarkus.openapi-generator.codegen.spec.<filename>.open-api-nullable=false.

[antssilva96]

Hi @ricardozanini thanks for your time!

Can you elaborate a bit on

Additional properties were suppressed because, in the Quarkus context, they might not make sense

I'm fairly new to Quarkus and might be missing the bigger picture 😛

Regarding my issue, I'm getting package org.openapitools.jackson.nullable does not exist errors trying to use the generator on Airflow's openapi spec available in https://airflow.apache.org/docs/apache-airflow/stable/stable-rest-api-ref.html.
The import is import org.openapitools.jackson.nullable.JsonNullable;

The import is being generated every time?

It is generated every time for the same Model classes. It seems to be present only if the spec of that model had a field with nullable: true. However, the JsonNullable is never used in these model classes..

Both quarkus.openapi-generator.codegen.spec.<filename>.open-api-nullable=false and quarkus.openapi-generator.codegen.spec.<filename>.openApiNullable=false do not work for me.

I tried checking the code and I do not find where the parsing of .spec.<filename>.<property> would be done, I just find it for .model. but this also does not work since these properties are inserted in an additional property of their own model-codegen.

[antssilva96]

Meanwhile I opened a PR with the other fix, that is independent from this discussion:
#67

[ricardozanini]

Meanwhile I opened a PR with the other fix, that is independent from this discussion: #67

Thank you! We've merged, and I'll release a patch version right now.

Can you elaborate a bit on

Well, some of the properties of the internal generator might not make sense in the context of the extension just because the original generator runs as a maven plugin and this extension runs in Quarkus Codegen build. It's similar but different :)

Additionally, we have our own template engine. Some properties there might not make sense here.

However, the JsonNullable is never used in these model classes

Hmm, I see. In this case, I think our templates are not registering these annotations. Well, we have some work to do then:

1. Add the property openApiNullable=false here. This will stop the internal generator from adding this import.
2. Add @JsonInclude(Include.NON_NULL) to the properties in the model template if they are nullable: true in the OpenAPI spec.

Do you think you can handle this PR?

Many thanks! :)

[antssilva96]

Do you think you can handle this PR?

I'll take a shot !

One question,

1. Add the property openApiNullable=false here.

I'm still unsure if this is something we want hardcoded for all use cases. For my openapi spec the import was added but the JsonNullable class was unused. How can we be sure this is the case for every spec? If it was I think the property added in the PR I originally linked was not needed in the first place, it could just be forced in the generator instead of an opt in functionality. What do you think ?

Currently it feels like fields with nullable: true are treated like any other which is weird to me.

[antssilva96]

Hi @ricardozanini !

Regarding the "unused" import...

After some digging we figured that the problem is that the quarkus generator is using its own template (model.qute and pojo.qute) where imports are retrieved from JavaClientCodegen which gets them from AbstractJavaCodegen.

So, without the new property set to false, we are including the import but our (quarkus openapi generator) templates do not use it.

The solution is indeed adding the property where you linked above.

Regarding the second part, of adding the @JsonInclude(Include.NON_NULL) annotation doesn't make sense for us. The nullable openapi spec definition is that the value may be null. Since we're generating everything in wrapper class return types (no primitives) callers already "know" these may be null.

I'll open the PR to add the property. The other alternative would be to modify the mode.qute to exclude that import if it is present, but I think the property is more elegant.

[ricardozanini]

Regarding the second part, of adding the @JsonInclude(Include.NON_NULL) annotation doesn't make sense for us. The nullable openapi spec definition is that the value may be null. Since we're generating everything in wrapper class return types (no primitives) callers already "know" these may be null.

Yes, indeed. You're right. I thought from the spec side, whereas aligning the Jackson annotation would make sense. And yes, since we are wrapping the types, wouldn't make sense to add it.

[antssilva96]

Thanks for the help, I think we can close this issue now!