[JAVA] Add generic okhttp generator - #24355
Draft
rar91279 wants to merge 11 commits into
Draft
Conversation
…s serialisation library as well as JSpecify Nullability assertions.
add configs for the new generic okhttp generator Execute ./mvnw clean package || exit ./bin/generate-samples.sh ./bin/configs/*.yaml || exit ./bin/utils/export_docs_generators.sh || exit
…d missed Samples files
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
23 issues found across 2935 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="samples/client/others/java/okhttp-oneOf-array/.travis.yml">
<violation number="1" location="samples/client/others/java/okhttp-oneOf-array/.travis.yml:20">
P2: `before_install` makes `./gradlew` executable but `script` calls `gradle test` (system Gradle) instead of `./gradlew test` (the wrapper). This bypasses the pinned Gradle version, risking build inconsistency or failure if the system Gradle is a different version.</violation>
</file>
<file name="samples/client/petstore/java/okhttp-3.1-duplicated-operationid/git_push.sh">
<violation number="1" location="samples/client/petstore/java/okhttp-3.1-duplicated-operationid/git_push.sh:57">
P2: The script hardcodes `master` as the branch name for both `git pull` and `git push`. Many GitHub repos now use `main` as their default branch (or a custom name via `init.defaultBranch`). When the local branch or remote default doesn't match `master`, these commands will fail with "fatal: couldn't find remote ref master" or "fatal: 'master' does not appear to be a git repository". Consider accepting the branch name as a parameter (e.g., `$5`) or detecting it from the remote to make the script portable.</violation>
</file>
<file name="samples/client/petstore/java/okhttp-3.1-duplicated-operationid/.travis.yml">
<violation number="1" location="samples/client/petstore/java/okhttp-3.1-duplicated-operationid/.travis.yml:20">
P2: The `before_install` step makes `gradlew` executable (`chmod a+x ./gradlew`), but the `script` step uses `gradle test` (system-installed Gradle) instead of `./gradlew test` (the wrapper). This is inconsistent — the wrapper setup is wasted effort, and the build may use a different Gradle version than expected. Use `./gradlew test` to match the wrapper setup.</violation>
</file>
<file name="samples/client/echo_api/java/okhttp-gson/src/main/java/org/openapitools/client/model/Query.java">
<violation number="1" location="samples/client/echo_api/java/okhttp-gson/src/main/java/org/openapitools/client/model/Query.java:172">
P1: Additional properties are not represented as OAS additional properties: Gson serializes this field as a nested `additionalProperties` member and discards unknown top-level response fields. Exclude the holder from default Gson reflection and add the generated TypeAdapter logic that flattens/captures undeclared keys.</violation>
</file>
<file name="samples/client/others/java/okhttp-oneOf/build.sbt">
<violation number="1" location="samples/client/others/java/okhttp-oneOf/build.sbt:1">
P3: `build.sbt` contains only `# TODO` — this is a placeholder with no actual build configuration. The sibling build files (`build.gradle`, `pom.xml`) are fully populated. Also, `#` is not valid SBT comment syntax; SBT uses `//` for single-line comments, so this would cause a parse error if someone tries to use this project with SBT.</violation>
</file>
<file name="samples/client/petstore/java/okhttp-3.1/build.sbt">
<violation number="1" location="samples/client/petstore/java/okhttp-3.1/build.sbt:1">
P1: This file uses `# TODO` but `#` is not valid SBT/Scala comment syntax — SBT will fail to parse the file. Use `// TODO` instead (or remove the file if SBT is not needed for this sample).</violation>
</file>
<file name="samples/client/others/java/okhttp-streaming/.github/workflows/maven.yml">
<violation number="1" location="samples/client/others/java/okhttp-streaming/.github/workflows/maven.yml:16">
P3: Job name `Build ping some object` looks like a leftover template placeholder —er — "ping some object" doesn't describe the actual work (building and testing this Java client under Java 17/21).</violation>
</file>
<file name="samples/client/others/java/okhttp-oneOf-array/build.sbt">
<violation number="1" location="samples/client/others/java/okhttp-oneOf-array/build.sbt:1">
P3: `# TODO` uses invalid SBT comment syntax — `#` is not recognized as a comment marker in `.sbt` files (SBT uses `//` for line comments like Scala). If this file is meant to be parsed by SBT, `#` will cause a parse error. Replace with `// TODO` or a proper SBT project definition.</violation>
</file>
<file name="samples/client/others/java/okhttp-oneOf-array/src/main/java/org/openapitools/client/ProgressRequestBody.java">
<violation number="1" location="samples/client/others/java/okhttp-oneOf-array/src/main/java/org/openapitools/client/ProgressRequestBody.java:33">
P1: `callback.onUploadProgress()` at line 69 will throw NPE if `callback` is null. While the generated `ApiClient` guards against null before constructing this class, `ProgressRequestBody` is a public API that could be used directly. Add `Objects.requireNonNull(callback)` in the constructor to fail fast and make the contract explicit.</violation>
<violation number="2" location="samples/client/others/java/okhttp-oneOf-array/src/main/java/org/openapitools/client/ProgressRequestBody.java:55">
P2: Method `sink` shares the same name as its parameter (`Sink sink`), creating confusing name shadowing. In `writeTo`, the call `sink(sink)` is ambiguous — the outer `sink` is the method and the inner `sink` is the `BufferedSink` parameter from `writeTo`. Rename the method (e.g., `countingSink`) or the parameter to avoid this collision.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/Java/libraries/okhttp/modelInnerEnum.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/Java/libraries/okhttp/modelInnerEnum.mustache:5">
P1: Gson models with an inline enum fail compilation: `@JsonAdapter` is emitted but its annotation is not imported by `pojo.mustache`. Add the conditional `JsonAdapter` import alongside the existing Gson imports.</violation>
<violation number="2" location="modules/openapi-generator/src/main/resources/Java/libraries/okhttp/modelInnerEnum.mustache:37">
P1: JSON-B models with an inline enum fail compilation because generated `@JsonbValue` has no import. Include `jakarta.json.bind.annotation.JsonbValue` in the JSON-B imports in `pojo.mustache`.</violation>
</file>
<file name="samples/client/others/java/okhttp-streaming/src/main/java/org/openapitools/client/ServerConfiguration.java">
<violation number="1" location="samples/client/others/java/okhttp-streaming/src/main/java/org/openapitools/client/ServerConfiguration.java:55">
P1: Custom values for an unrestricted server variable crash URL construction with `NullPointerException`, because `enumValues` is nullable. Guard it for null (and update `Java/ServerConfiguration.mustache`) so non-enumerated variables can be substituted.</violation>
</file>
<file name="samples/client/petstore/java/okhttp-3.1-duplicated-operationid/src/main/java/org/openapitools/client/Pair.java">
<violation number="1" location="samples/client/petstore/java/okhttp-3.1-duplicated-operationid/src/main/java/org/openapitools/client/Pair.java:34">
P3: Observation: The method isValidString(String) currently only checks for null (return arg != null), so empty strings ("") incorrectly appear “valid” despite the method name implying broader validation. This mismatch can cause incorrect assumptions at call sites and maintenance surprises. Recommendation: Either rename the method to reflect its actual behavior (e.g., isNotNull) or implement the intended validation (e.g., check for non-empty with !arg.isEmpty() or trim before checking). Renaming is the least invasive fix if the intent is only a null-check; expand the check if callers rely on a notion of a “valid” string.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/Java/libraries/okhttp/auth/AWS4Auth.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/Java/libraries/okhttp/auth/AWS4Auth.mustache:64">
P1: Combining AWS4 with a query-based auth scheme signs the URL before that scheme adds its query parameter, then sends a different URL; AWS rejects the request. Sign after all query authentication parameters have been applied, using the final request URI.</violation>
</file>
<file name="samples/client/petstore/java/okhttp-3.1/.travis.yml">
<violation number="1" location="samples/client/petstore/java/okhttp-3.1/.travis.yml:20">
P2: `before_install` makes `./gradlew` executable but `script` runs `gradle test` (system-installed Gradle) instead of `./gradlew test` (the project wrapper). This bypasses the wrapper's version pinning — the system Gradle version may differ from the one in `gradle/wrapper/gradle-wrapper.properties`, causing inconsistent or broken builds.</violation>
</file>
<file name="samples/client/others/java/okhttp-streaming/build.sbt">
<violation number="1" location="samples/client/others/java/okhttp-streaming/build.sbt:1">
P2: This `build.sbt` file contains only a `# TODO` placeholder with no actual build configuration. Every other Java sample `build.sbt` in the repository has a complete SBT build definition (organization, name, version, dependencies, etc.). SBT users cloning this sample won't be able to build the project with this file.
Either populate the file with a proper SBT build definition (following the pattern in sibling files like `samples/client/others/java/okhttp-gson-streaming/build.sbt`) or remove it if SBT is not intended to be supported.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/Java/libraries/okhttp/AbstractOpenApiSchema.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/Java/libraries/okhttp/AbstractOpenApiSchema.mustache:49">
P1: JSON-B requests/responses for oneOf/anyOf models serialize wrapper properties (`actualInstance`, `schemas`, `schemaType`, `nullable`) instead of the wrapped value. Add JSON-B composed-schema serialization/deserialization support (or exclude wrapper getters and adapt the instance) before advertising JSON-B support.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/Java/libraries/okhttp/GzipRequestInterceptor.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/Java/libraries/okhttp/GzipRequestInterceptor.mustache:18">
P3: This adds a second identical interceptor template, creating two maintenance points for one behavior. Consider rendering a shared Mustache partial/template for both OkHttp libraries.</violation>
</file>
<file name="samples/client/others/java/okhttp-streaming/src/main/java/org/openapitools/client/ServerVariable.java">
<violation number="1" location="samples/client/others/java/okhttp-streaming/src/main/java/org/openapitools/client/ServerVariable.java:32">
P2: Constructor parameter uses implementation type `HashSet<String>` instead of interface `Set<String>`, forcing callers to pass a `HashSet` specifically. Using `Set<String>` would accept any `Set` implementation (e.g., `TreeSet`, `LinkedHashSet`, unmodifiable wrappers) and follow Java's "program to an interface" convention.</violation>
</file>
<file name="samples/client/others/java/okhttp-streaming/src/main/java/org/openapitools/client/StringUtil.java">
<violation number="1" location="samples/client/others/java/okhttp-streaming/src/main/java/org/openapitools/client/StringUtil.java:51">
P2: The custom `join(String[], String)` and `join(Collection<String>, String)` methods fully duplicate `String.join()` which the JDK has provided since Java 8. Since this project requires Java 11 (`java@11`), these 23 lines of custom implementation carry unnecessary maintenance burden — any future edits risk diverging from the well-tested JDK behaviour. Replace both bodies with direct delegations: `return String.join(separator, array)` and `return String.join(separator, list)` respectively.</violation>
</file>
<file name="samples/client/others/java/okhttp-streaming/git_push.sh">
<violation number="1" location="samples/client/others/java/okhttp-streaming/git_push.sh:53">
P2: `git pull origin master` always runs after the remote setup block, but hardcodes "master". If the remote uses "main" or another default branch (modern GitHub default), this pull will fail. Suggest using the remote HEAD ref or making the branch configurable.</violation>
<violation number="2" location="samples/client/others/java/okhttp-streaming/git_push.sh:57">
P2: `git push origin master 2>&1 | grep -v 'To https'` pipes through `grep -v`, making the pipeline's exit code grep's, not git push's. If git push fails (e.g., rejected, auth failure), the pipeline still exits 0 (grep found nothing to filter), so the script silently reports success.</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 200 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
Re-trigger cubic
| @@ -0,0 +1 @@ | |||
| # TODO | |||
Contributor
There was a problem hiding this comment.
P1: This file uses # TODO but # is not valid SBT/Scala comment syntax — SBT will fail to parse the file. Use // TODO instead (or remove the file if SBT is not needed for this sample).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/java/okhttp-3.1/build.sbt, line 1:
<comment>This file uses `# TODO` but `#` is not valid SBT/Scala comment syntax — SBT will fail to parse the file. Use `// TODO` instead (or remove the file if SBT is not needed for this sample).</comment>
<file context>
@@ -0,0 +1 @@
+# TODO
</file context>
Suggested change
| # TODO | |
| // TODO |
| @JsonValue | ||
| {{/isJackson}} | ||
| {{#isJsonb}} | ||
| @JsonbValue |
Contributor
There was a problem hiding this comment.
P1: JSON-B models with an inline enum fail compilation because generated @JsonbValue has no import. Include jakarta.json.bind.annotation.JsonbValue in the JSON-B imports in pojo.mustache.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/Java/libraries/okhttp/modelInnerEnum.mustache, line 37:
<comment>JSON-B models with an inline enum fail compilation because generated `@JsonbValue` has no import. Include `jakarta.json.bind.annotation.JsonbValue` in the JSON-B imports in `pojo.mustache`.</comment>
<file context>
@@ -0,0 +1,82 @@
+ @JsonValue
+ {{/isJackson}}
+ {{#isJsonb}}
+ @JsonbValue
+ {{/isJsonb}}
+ public {{{dataType}}} getValue() {
</file context>
| * {{description}}{{^description}}Gets or Sets {{{name}}}{{/description}} | ||
| */ | ||
| {{#isGson}} | ||
| @JsonAdapter({{{datatypeWithEnum}}}{{^datatypeWithEnum}}{{classname}}{{/datatypeWithEnum}}.Adapter.class) |
Contributor
There was a problem hiding this comment.
P1: Gson models with an inline enum fail compilation: @JsonAdapter is emitted but its annotation is not imported by pojo.mustache. Add the conditional JsonAdapter import alongside the existing Gson imports.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/Java/libraries/okhttp/modelInnerEnum.mustache, line 5:
<comment>Gson models with an inline enum fail compilation: `@JsonAdapter` is emitted but its annotation is not imported by `pojo.mustache`. Add the conditional `JsonAdapter` import alongside the existing Gson imports.</comment>
<file context>
@@ -0,0 +1,82 @@
+ * {{description}}{{^description}}Gets or Sets {{{name}}}{{/description}}
+ */
+ {{#isGson}}
+ @JsonAdapter({{{datatypeWithEnum}}}{{^datatypeWithEnum}}{{classname}}{{/datatypeWithEnum}}.Adapter.class)
+ {{/isGson}}
+{{#withXml}}
</file context>
| throws ApiException { | ||
|
|
||
| SdkHttpFullRequest.Builder requestBuilder = | ||
| SdkHttpFullRequest.builder().uri(uri).method(SdkHttpMethod.fromValue(method)); |
Contributor
There was a problem hiding this comment.
P1: Combining AWS4 with a query-based auth scheme signs the URL before that scheme adds its query parameter, then sends a different URL; AWS rejects the request. Sign after all query authentication parameters have been applied, using the final request URI.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/Java/libraries/okhttp/auth/AWS4Auth.mustache, line 64:
<comment>Combining AWS4 with a query-based auth scheme signs the URL before that scheme adds its query parameter, then sends a different URL; AWS rejects the request. Sign after all query authentication parameters have been applied, using the final request URI.</comment>
<file context>
@@ -0,0 +1,105 @@
+ throws ApiException {
+
+ SdkHttpFullRequest.Builder requestBuilder =
+ SdkHttpFullRequest.builder().uri(uri).method(SdkHttpMethod.fromValue(method));
+
+ ContentStreamProvider provider = new ContentStreamProvider() {
</file context>
| * | ||
| * Taken from https://github.com/square/okhttp/issues/350 | ||
| */ | ||
| class GzipRequestInterceptor implements Interceptor { |
Contributor
There was a problem hiding this comment.
P3: This adds a second identical interceptor template, creating two maintenance points for one behavior. Consider rendering a shared Mustache partial/template for both OkHttp libraries.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/Java/libraries/okhttp/GzipRequestInterceptor.mustache, line 18:
<comment>This adds a second identical interceptor template, creating two maintenance points for one behavior. Consider rendering a shared Mustache partial/template for both OkHttp libraries.</comment>
<file context>
@@ -0,0 +1,74 @@
+ *
+ * Taken from https://github.com/square/okhttp/issues/350
+ */
+class GzipRequestInterceptor implements Interceptor {
+ @Override
+ public Response intercept(Chain chain) throws IOException {
</file context>
|
|
||
| jobs: | ||
| build: | ||
| name: Build ping some object |
Contributor
There was a problem hiding this comment.
P3: Job name Build ping some object looks like a leftover template placeholder —er — "ping some object" doesn't describe the actual work (building and testing this Java client under Java 17/21).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/java/okhttp-streaming/.github/workflows/maven.yml, line 16:
<comment>Job name `Build ping some object` looks like a leftover template placeholder —er — "ping some object" doesn't describe the actual work (building and testing this Java client under Java 17/21).</comment>
<file context>
@@ -0,0 +1,30 @@
+
+jobs:
+ build:
+ name: Build ping some object
+ runs-on: ubuntu-latest
+ strategy:
</file context>
Suggested change
| name: Build ping some object | |
| name: Build and test |
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
…rove auth parameter handling. Add missing Gson and JSON-B annotations (from AI review).
rar91279
changed the title
Contributor
|
Sorry, but the scope of this PR is too large. I would suggest creating a PR per feature. So that it can be reviewed, tested and documented properly. |
Author
|
Hello @Chrimle, thank you for your review. Sorry for that. it's a bit challenging for me to split it in single feature sets. It's a completely new set of mustache templates for generating OkHttp clients supporting all three already available serialization libraries. Default okhttp-gson shouldn't be touched. Do you have an idea for me for a common way to organize such big scopes into single PRs ? Or was it just a mistake to commit all files generated by pre PR scripts? |
Contributor
|
So, I would rather propose to the Java tech committee to merge most of these generators. As you've noticed, there is a lot of duplication and redundancy. Whenever new features are added, or bug fixes, there's too many places to update... so things are missed, or mistakes happen and go unnoticed. But, I won't hijack this PR with that discussion, and I won't block this PR, but I will not be able to review this PR. There are plenty of other maintainers that could chip in here though, perhaps with other opinions and approaches. So, let's see 🙏 |
Author
|
:) so I just should try to fix all concerns from cubic and failing checks with further commits? |
rar91279
marked this pull request as draft
July 26, 2026 20:31
- Remove unused `forceContentLength` method in GzipRequestInterceptor. - Enhance `oneOf`/`composedSchemas` templates for better type handling. - Add optional AWS V4 signature support. - Migrate auth `applyToParams` payload from String to byte array. - Add SBT files and update Gradle templates for OkHttp. - Update Java codegen logic to support composed schemas in additional properties. - Improve test coverage and fix minor issues in generated samples.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OkHttp generator supporting Jackson 2/3, Gson and JSONB as well as JSpecify nullability annotations.
Adressing issue [REQ] [Java] [OkHttp-Gson] Add serialization support for Jackson #20655
PR checklist
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.