helidon-io / helidon

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

Builder enhanced to support integration with OpenAPI #6407

Open trentjeff opened 1 year ago

trentjeff commented 1 year ago

See the OCI's Example as background.

What is happening here behind the scenes is that there is an OpenAPI spec yaml where GetBucketResponse and GetBucketRequest schemas trigger generation of pojo's in a builder style. And then the generated pojo instances are built (builder().namespaceName("myNamespace").bucketName("myBucket").build()) and then passed to the service interface.

This functionality would be a nice enhancement to integrate into our Helidon Builder, where we would delegate directly to BuilderCreator to generate the backing pojo types for this use case.

Details to be worked out through a POC if we think this is worthwhile to pursue.

tjquinno commented 1 year ago

The OpenAPITools generators in general--and the ones our team provided to support Helidon in particular--generate POJO classes for each of the schemas declared in the OpenAPI document. These POJOs include getters and setters for the attributes as illustrated in this generated POJO from our example generated project https://github.com/helidon-io/helidon/blob/helidon-3.x/examples/openapi-tools/quickstart-mp/mp-server/src/main/java/org/openapitools/server/model/Message.java.

One possibility could be to enhance the Helidon OpenAPITools generators with an optional flag to detect a new, optional flag (supported only if the Helidon version is 4.x?) and then:

We should consider how valuable we think users would find such a feature vs. simply using the already-generated setters and getters on the POJO classes. We also need to weigh our own priorities for various possible enhancements to our generators.

trentjeff commented 1 year ago

Interestingly our generated pojo does not follow Helidon's record-style coding style guidelines.

/**
 * An message for the user
 **/
public class Message {

    private String message;

    private String greeting;

    /**
     * Get message
     *
     * @return message
     **/
    public String getMessage() {
        return message;
    }

Was that intentional?

Also, what about the no-arg constructor - is that required for serialization purposes?

Everything else here seems like it would be low-hanging fruit to convert to use Builder, and it would make the development experience standard across the product for our users.

tjquinno commented 1 year ago

Several factors:

  1. The generated POJO classes are not part of Helidon but become part of the user's project. With that in mind, the generated code encourages best practices as appropriate but should not necessarily impose any of our own stylistic preferences.
  2. The Helidon generators support both 2.x (requires Java 11+) and 3.x (requires Java 17+). Records do not appear in Java until Java 14. Generating POJO classes when the user targets Helidon 2.x but records when the user targets 3.x or 4.x would add complexity to the generator and templates.
  3. Changing how the generators create POJOs would be backward-incompatible and would need to await a major release of the generator.
  4. Some users might not want to use builders with the generated POJOs (whether classes or records). They might not use Helidon-based builders at all in their apps, particularly for MP apps, so there would be no "standard" experience to preserve for such users.

Any or all of what you describe could be done, technically, but, again, we have to prioritize carefully among the possible enhancements to the generators (and our other work).