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

Helidon 4 support mp #1

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Helidon 4 support mp #1

wants to merge 19 commits into from

Conversation

tjquinno
Copy link
Owner

@tjquinno tjquinno commented Mar 31, 2024

Resolves OpenAPITools#18261 to support generation of Helidon 4 MP clients and servers. (A later PR will add support for Helidon 4 SE clients and servers.)

Key changes:

  1. Update the pom.xml templates for the Helidon client and server generators:

    • The Helidon MP test component has changed its GAV in 4.x. Conditionalize the pom.xml template accordingly and set a mustache attribute from the Java code to indicate which one to use.
    • The Jandex plug-in has changed its GAV. Conditionalize the pom.xml template and set a mustache attribute from the Java code to indicate which to use.
  2. Add support for retrieving released Helidon versions and allowing users to specify a prefix for the desired version.

    The generators now find the highest Helidon release that matches the user-supplied prefix as helidonVersion. The helidon.io website serves a metadata file listing released versions. The Helidon generators attempt to retrieve that list from the website and, if successful, they save the list locally using the Java preferences API. If the internet is not available then previously-stored preferences are used if available and, otherwise, a version list hardcoded in the code is used.

    The PR also contains a new test to exercise this feature.

  3. Adjust workflows.

    • Add samples-jdk21.yaml. Helidon 4 requires Java 21, so the samples generated for Helidon 4 do as well.
    • Rename samples-java-helidon.yaml (which deals with samples for Helidon 3) to samples-java-helidon-v3.yaml and change the samples paths to include /v3.
    • Add samples-java-helidon-v4.yaml (which deals with samples for Helidon 4) and use subdirectories /v4.
    • Revise samples-jdk17.yaml to reflect the new locations for the generated Helidon sample projects.

Why so many changed files?

Because the build process for all generators creates sample apps using an OpenAPI document and the various generators, and because this PR puts Helidon 3 samples in a new subdirectory, this PR also moves the sample files that were generated by the previous Helidon 3 generators into the new v3 subdirectory.

The number of changed files in the PR seems very large as a result. The generated Helidon 4 MP apps go into the new v4 subdirectory (to be joined eventually by generated SE samples in a later PR). Doing this allows us to generate samples for multiple Helidon target releases (3 and 4, for now) and avoid collisions between the generated projects while making sure our changes to the generator to support 4.x do not break the generated code for Helidon 3 as we still have active Helidon 3 users.

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/configs/*.yaml
    ./bin/utils/export_docs_generators.sh
    
    (For Windows users, please run the script in Git BASH)
    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*.
    IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • File the PR against the correct branch: master (upcoming 7.1.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@tjquinno tjquinno self-assigned this Mar 31, 2024
@@ -18,10 +18,10 @@ jobs:
matrix:
sample:
- samples/client/petstore/java-helidon-client/mp
- samples/client/petstore/java-helidon-client/se
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is temporary until a follow-on PR adds Helidon 4 SE support.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm actually in the process of reworking the samples stuff a bit. We want both Helidon 3 and Helidon 4 samples which require different configs to drive the generator to produce the right output. Same for testing the generators.

Stay tuned.

@@ -39,7 +39,7 @@ These options may be applied as additional-properties (cli) or configOptions (pl
|enumUnknownDefaultCase|If the server adds new enum cases, that are unknown by an old spec/client, the client will fail to parse the network response.With this option enabled, each enum will have a new case, 'unknown_default_open_api', so that when the server sends an enum case that is not known by the client/spec, they can safely fallback to this case.|<dl><dt>**false**</dt><dd>No changes to the enum's are made, this is the default option.</dd><dt>**true**</dt><dd>With this option enabled, each enum will have a new case, 'unknown_default_open_api', so that when the enum case sent by the server is not known by the client/spec, can safely be decoded to this case.</dd></dl>|false|
|fullProject|If set to true, it will generate all files; if set to false, it will only generate API files. If unspecified, the behavior depends on whether a project exists or not: if it does not, same as true; if it does, same as false. Note that test files are never overwritten.| ||
|groupId|groupId in generated pom.xml| |org.openapitools|
|helidonVersion|Helidon version for generated code| |3.0.1|
|helidonVersion|Helidon version for generated code| |4.0.6|
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the default be "4"? Just an idea. Not sure what I think about that.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already changed (locally) one place so the default version is described as the highest released version that matches any supplied prefix.

I am trying very hard to avoid needing to update the generators every time we release Helidon, and we won't need to do that if we can instead rely on the enhanced version handling in the PR.

This is another spot that I had not gotten to. Good to have it flagged.


HttpResponse<String> response = httpClient.send(req, HttpResponse.BodyHandlers.ofString());

Preferences versionPrefs = Preferences.userRoot().node(PREFERENCES_KEY);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Helidon CLI caches versions.xml in ~/.helidon/cache/versions.xml. We should at least discuss if using that mechanism here makes sense -- I see pros and cons.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I knew that the CLI caches the data.

My thinking about using Preferences was mostly that as a Java feature, not a Helidon-specific one, using it might cause less friction in getting the PR approved. Eventually the generators will likely use the yet-to-be-designed Helidon-to-MP-release mapping file that would be different from versions.xml in case that makes any difference.

tjquinno pushed a commit that referenced this pull request May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants