helidon-io / helidon

Java libraries for writing microservices
https://helidon.io
Apache License 2.0
3.52k stars 565 forks source link

Define desired characteristics of server code from OpenAPITools generator #4017

Open tjquinno opened 2 years ago

tjquinno commented 2 years ago

Environment Details


Problem Description

There are some characteristics of the server code from the OCI SDK code generator--or good characteristics in principle, apart from the OCI SDK generator--that are desirable in what we would get from the OpenAPITools generator.

We will use this issue to describe:

tjquinno commented 2 years ago

As an example. @trentjeff has mentioned that the OCI SDK code generator creates abstract classes which developers then extend.

The existing OpenAPITools JavaJaxRS/spec generator (which is the one we have customized for Helidon) can generate either classes (not abstract) or interfaces, based on the interfacesOnly property which defaults to false.

trentjeff commented 2 years ago

I do think generating the abstracts is a great option/alternative to the interfaces; it would allow default impls to be tucked into the base.

Furthermore, what I envision here is that there will be different mustache file "overlays" associated with a particular template name. Templates could be "interfaceOnly" or "abstractsOnly", but could also allow us or our consumers to create extensions to these, most notably the "astractsOnly'. An extension would mean only "overlaying" certain mustache files over the base template and giving it a new name (e.g., "helidon-mp" or "helidon-mp-oci", etc.).

A key driver for this is would be security, where we would expect certain key/principal/identity collateral is picked up on the client-side dictated by the particular named template profile, and then on the server-side integrated with the "proper' security providers. Security collateral is just one form of context propagation, but generally speaking, we should support tracing context propagation as well to further allow for a customizable/extensions mechanism for our customers to use for their cases as well.

spericas commented 2 years ago

Is there really an advantage to generate abstract classes over concrete, non-final classes with simple no-ops? I like the idea of being able to compile and run the generated code first, and then override its behavior via subclassing/overriding. Being able to generate interfaces may be useful as well in some cases.

tjquinno commented 2 years ago

I do think generating the abstracts is a great option/alternative to the interfaces; it would allow default impls to be tucked into the base.

As I mentioned above, the generator on which the Helidon support is based already generates either interfaces or classes based on the interfacesOnly option. The generated classes are concrete, not abstract, and also not final. Hence my earlier question: is having the generated classes be abstract instead of concrete important enough to add an option to the code or use a custom template?

Santiago makes a very good related point about a concrete, non-final class with a no-op or trivial implementation having the advantage of being immediately buildable and runnable.

Furthermore, what I envision here is that there will be different mustache file "overlays" associated with a particular template name. Templates could be "interfaceOnly" or "abstractsOnly", but could also allow us or our consumers to create extensions to these, most notably the "astractsOnly'. An extension would mean only "overlaying" certain mustache files over the base template and giving it a new name (e.g., "helidon-mp" or "helidon-mp-oci", etc.).

"Overlays" and "extensions" are not terms used by the OpenAPITools generator documentation.

The generator supports three ways to adjust the generation at generation-time, what their website refers to as:

IIUC, the templating approach might be sufficient for what you've described.

If there were a group of templates that needed customization in a coherent way for, say, OCI services, then customized versions of those templates could be gathered into a single JAR once and then used repeatedly and by many developers, presumably, for generating any number of services. This is the easiest approach to test and to make available to users.

Someone (presumably Helidon) could build, document, and publish one or more JAR files each containing customized templates tuned to a particular developer group or set of use cases: OCI, for example.

trentjeff commented 2 years ago

IIUC, the templating approach might be sufficient for what you've described.

+1

if there were a group of templates that needed customization in a coherent way for

I had assumed (maybe wrongly) that this will be inevitable, especially on the client-side, in order to pick up the propagated context including any security collateral.

tjquinno commented 2 years ago

I would like this issue to focus on the server-side code generation simply because the client vs. server side generator code and templates are separate when viewed from the point of view of the OpenAPITools project (although certainly related from our point of view), and so changes to the client vs. server generation would most likely be in separate PRs driven from separate issues.

I had assumed (maybe wrongly) that this will be inevitable, especially on the client-side, in order to pick up the propagated context including any security collateral.

Pretty soon we need to drill down to the next level of detail. Can you add snippets of example server code (not templates) here to show how code that works the way you have in mind would ultimately look?

spericas commented 2 years ago
  • What, if anything, could be improved about the code in the generated classes?

Indentation and general formatting of the code is poor. We can't control everything, but we should make the generated code more consistent with our rules. Current rules for JavaJaxRS seem inconsistent, for example Api code and Model code is formatted very differently. This will likely require more Helidon-specific templates.

  • One possible enhancement could be to add an option to generate an abstract class instead of a concrete one. How useful and important would that be?

The concrete classes generated by the tool seem fine to me, other than perhaps the BV annotations, which perhaps should not be generated by default.

tjquinno commented 2 years ago

RE: BV annotations. We could turn those off by default for the helidon library by adding a line to the Helidon-specific Java code: https://github.com/OpenAPITools/openapi-generator/blob/21c399f2b88e2ba824648d749025880ed5359cd3/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/JavaJAXRSSpecServerCodegen.java#L216-L223

Something like

additionalProperties.computeIfAbsent(BeanValidationFeatures.USE_BEANVALIDATION, key -> "false");
trentjeff commented 2 years ago

another request here is to provide an option to generate either sync or async clients, or both.

tjquinno commented 2 years ago

Again, let's focus this issue on desired characteristics of generated server code.

On the client side, we have two broad categories covered by other issues or by other work already completed:

  1. For clients connecting to OCI services we want to encourage developers to use the OCI CDI extension. Yes, this is MP-only but as I understand it that's the majority of use cases.
  2. For clients connecting to non-OCI services we have two open issues: a. #4012 to cover Helidon MP b. #4022 This work needs to be revived but already generates both sync and async support.