feat(sdk,server): add Java SDK and Spring Boot server implementation #7
Conversation
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
- Generate complete Java client SDK - Include Spring Boot 2 server implementation with full model support - Support Maven and Gradle build systems with proper dependencies
WalkthroughAdds Java client SDK as a new module (java-client) generated from OpenAPI, including source, models, APIs, auth, docs, tests, and build tooling (Maven, Gradle, SBT, CI). Updates generate.sh to produce Java client and a Spring Boot 2 server, introduces Maven Wrapper for the server, and optional auto-start flag. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Script as generate.sh
participant OAGen as OpenAPI Generator
participant Spring as spring-boot2/
participant JavaSDK as java-client/
User->>Script: ./generate.sh [--start-spring-boot]
Script->>OAGen: Generate Python/Rust/TS (existing)
Script->>OAGen: Generate Java client -> java-client/
OAGen-->>JavaSDK: Source, models, docs, builds
Script->>OAGen: Generate Spring Boot 2 server -> spring-boot2/
Script->>Spring: mvn -N io.takari:maven:wrapper
alt --start-spring-boot provided
Script->>Spring: ./mvnw spring-boot:run (bg)
Spring-->>User: Server running at http://localhost:8080
else
Script-->>User: Print manual start instructions
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 39
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
java-client/git_push.sh (1)
31-58: Dangerous script: can push current tree to origin (hardcoded master), leaks tokens via remote URL.
- Hardcodes branch “master” in a repo using “main”.
- Adds a credentialed remote URL; tokens persist in .git/config and may leak.
- Unconditionally runs
git init,add,commit,pull,push.Either remove this file or gate it behind an opt-in env flag and make it branch-safe.
Apply this mitigation diff:
#!/bin/sh +set -euo pipefail @@ -git init +git rev-parse --is-inside-work-tree >/dev/null 2>&1 || { echo "[ERROR] Not a git repo"; exit 1; } @@ -git add . -git commit -m "$release_note" +echo "[INFO] Dry-run by default. Set ALLOW_PUSH_SCRIPT=true to enable." +if [ "${ALLOW_PUSH_SCRIPT:-}" != "true" ]; then + echo "[INFO] Skipping add/commit/push." + exit 0 +fi +git add -A +git commit -m "$release_note" || echo "[INFO] Nothing to commit" @@ -if [ "$git_remote" = "" ]; then # git remote not defined +if [ "$git_remote" = "" ]; then # git remote not defined @@ - git remote add origin https://${git_user_id}:"${GIT_TOKEN}"@${git_host}/${git_user_id}/${git_repo_id}.git + echo "[ERROR] Refusing to embed token in remote URL. Use a credential helper instead." + exit 1 fi fi - -git pull origin master +BRANCH=${BRANCH:-$(git rev-parse --abbrev-ref HEAD)} +git fetch origin "$BRANCH" || true +git pull --ff-only origin "$BRANCH" || true @@ -echo "Git pushing to https://${git_host}/${git_user_id}/${git_repo_id}.git" -git push origin master 2>&1 | grep -v 'To https' +echo "Git pushing to origin $BRANCH" +git push origin "$BRANCH"java-client/docs/OpenInstanceDTO.md (1)
19-20: Fix CI: trim extra trailing line endingsCI failed on “multiple trailing line endings” (LineEnd at line 19). Remove redundant blank lines and keep a single newline at EOF.
Apply this diff:
- - - +java-client/docs/NamespaceBranchManagementApi.md (1)
1-734: Fix mismatched/missing HTTP request lines in java-client/docs/NamespaceBranchManagementApi.mdDoc contains missing HTTP request lines and at least one path missing the "/openapi/v1/envs/" prefix — align the documented "HTTP request" lines with the generator output.
- java-client/docs/NamespaceBranchManagementApi.md — Line 417: "GET /apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/branches/{branchName}/items" is missing the "/openapi/v1/envs/{env}/" prefix; update the HTTP request line.
- Sections missing an HTTP method line before the "### Example" block: line 175 (openapiV1EnvsEnvAppsAppIdClustersClusterNameNamespacesNamespaceNameBranchesBranchNameGrayDelReleasesPost), line 490 (...MergePost), line 656 (...ReleasesPost) — add the correct method+path lines (they should start with "/openapi/v1/envs/...").
- Heading vs method inconsistency: line 412 (openapiV1EnvsEnvAppsAppIdClustersClusterNameNamespacesNamespaceNameBranchesBranchNameItemsGet) implies an env-prefixed path but its method line (417) does not — make names and documented HTTP request lines consistent.
| @@ -2,13 +2,15 @@ | |||
| set -e | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Stronger bash safety flags
Use pipefail and unset var checks.
-set -e
+set -Eeuo pipefail📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| set -e | |
| set -Eeuo pipefail |
🤖 Prompt for AI Agents
In generate.sh around line 2, replace the single "set -e" with stronger bash
safety flags to fail on unset variables and pipeline errors: add -u for unset
var checks and enable pipefail (e.g., set -euo pipefail or set -e -u -o
pipefail) so the script exits on errors, undefined variables, or failing
pipeline stages; optionally also set a safe IFS if needed for robust
word-splitting.
Comment on lines
+33
to
+39
| echo "🚀 Generating Java Client SDK..." | ||
| openapi-generator generate \ | ||
| -i "$SPEC_FILE" \ | ||
| -g java \ | ||
| -o "$JAVA_CLIENT_DIR" \ | ||
| --additional-properties=groupId=com.apollo,artifactId=apollo-openapi-client,artifactVersion=0.0.1,packageName=com.apollo.openapi.client | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Package mismatch: generated Java code uses org.openapitools.client but script sets packageName=com.apollo.openapi.client
Java generator ignores packageName; you must set invokerPackage/apiPackage/modelPackage. As‑is, regeneration will not reproduce the checked‑in packages.
Run to confirm current packages and the script setting:
Proposed fix:
- --additional-properties=groupId=com.apollo,artifactId=apollo-openapi-client,artifactVersion=0.0.1,packageName=com.apollo.openapi.client
+ --additional-properties=groupId=com.apollo,artifactId=apollo-openapi-client,artifactVersion=0.0.1,invokerPackage=org.openapitools.client,apiPackage=org.openapitools.client.api,modelPackage=org.openapitools.client.model🏁 Script executed:
#!/bin/bash
rg -nP --glob 'java-client/**/*.java' '^package\s+([a-zA-Z0-9_.]+);' | awk -F' ' '{print $2}' | sed 's/;//' | sort -u
echo
rg -n ' -g java ' generate.sh -n -C3
rg -n 'packageName=' generate.sh -n -C1Length of output: 1138
Fix Java client package settings in generate.sh
generate.sh (lines 33–39): the Java client invocation sets packageName=com.apollo.openapi.client but the checked-in java-client sources are under org.openapitools.client.*. openapi-generator for Java uses invokerPackage/apiPackage/modelPackage, so regeneration will not reproduce the checked-in packages — change packageName to the invoker/api/model package properties.
- --additional-properties=groupId=com.apollo,artifactId=apollo-openapi-client,artifactVersion=0.0.1,packageName=com.apollo.openapi.client
+ --additional-properties=groupId=com.apollo,artifactId=apollo-openapi-client,artifactVersion=0.0.1,invokerPackage=org.openapitools.client,apiPackage=org.openapitools.client.api,modelPackage=org.openapitools.client.model📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| echo "🚀 Generating Java Client SDK..." | |
| openapi-generator generate \ | |
| -i "$SPEC_FILE" \ | |
| -g java \ | |
| -o "$JAVA_CLIENT_DIR" \ | |
| --additional-properties=groupId=com.apollo,artifactId=apollo-openapi-client,artifactVersion=0.0.1,packageName=com.apollo.openapi.client | |
| echo "🚀 Generating Java Client SDK..." | |
| openapi-generator generate \ | |
| -i "$SPEC_FILE" \ | |
| -g java \ | |
| -o "$JAVA_CLIENT_DIR" \ | |
| --additional-properties=groupId=com.apollo,artifactId=apollo-openapi-client,artifactVersion=0.0.1,invokerPackage=org.openapitools.client,apiPackage=org.openapitools.client.api,modelPackage=org.openapitools.client.model |
🤖 Prompt for AI Agents
In generate.sh around lines 33 to 39, the openapi-generator invocation uses
packageName=com.apollo.openapi.client which won't reproduce the checked-in Java
sources under org.openapitools.client.*; replace the single packageName property
with the Java-specific properties invokerPackage, apiPackage, and modelPackage
and set them to match the checked-in layout (for example
invokerPackage=org.openapitools.client, apiPackage=org.openapitools.client.api,
modelPackage=org.openapitools.client.model) so regeneration produces the same
package structure.
Comment on lines
+8
to
+13
| on: | ||
| push: | ||
| branches: [ main, master ] | ||
| pull_request: | ||
| branches: [ main, master ] | ||
|
|
There was a problem hiding this comment.
Workflow location won’t run from a subdirectory
GitHub Actions only loads workflows from .github/workflows at repo root. Move this file to the repository’s root .github/workflows.
🤖 Prompt for AI Agents
In java-client/.github/workflows/maven.yml around lines 8 to 13, the workflow is
placed in a subdirectory so GitHub Actions will not load it; move this workflow
file from java-client/.github/workflows/maven.yml into the repository root at
.github/workflows/maven.yml (preserving file name and contents), and after
moving verify any repository-relative paths or referenced secrets/runners still
apply and update them if necessary.
Comment on lines
+29
to
+30
| - name: Build with Maven | ||
| run: mvn -B package --no-transfer-progress --file pom.xml |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Wrong Maven path; build likely targets non‑existent root pom
Either set working-directory to java-client or pass -f java-client/pom.xml.
- - name: Build with Maven
- run: mvn -B package --no-transfer-progress --file pom.xml
+ - name: Build with Maven
+ working-directory: ./java-client
+ run: mvn -B package --no-transfer-progress -f pom.xml🏁 Script executed:
#!/bin/bash
# Verify presence of root workflows and pom locations
fd -a '.github/workflows' -t d -d 3
fd -a 'pom.xml' -d 3Length of output: 452
Fix Maven path — workflow targets non‑existent root pom
Repository has java-client/pom.xml but no top-level pom.xml; running mvn ... -f pom.xml from the repo root will fail. Either run the step in ./java-client or point -f at java-client/pom.xml.
- - name: Build with Maven
- run: mvn -B package --no-transfer-progress --file pom.xml
+ - name: Build with Maven
+ working-directory: ./java-client
+ run: mvn -B package --no-transfer-progress -f pom.xml📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Build with Maven | |
| run: mvn -B package --no-transfer-progress --file pom.xml | |
| - name: Build with Maven | |
| working-directory: ./java-client | |
| run: mvn -B package --no-transfer-progress -f pom.xml |
🤖 Prompt for AI Agents
In java-client/.github/workflows/maven.yml around lines 29 to 30, the workflow
runs Maven against pom.xml in the repo root which does not exist; update the
step to either set the step's working-directory to java-client so mvn runs
there, or change the -f argument to point to java-client/pom.xml (e.g., mvn -B
package --no-transfer-progress -f java-client/pom.xml); pick one approach and
apply it consistently so the job finds the correct pom.
| @@ -0,0 +1,166 @@ | |||
| apply plugin: 'idea' | |||
| apply plugin: 'eclipse' | |||
| apply plugin: 'java' | |||
There was a problem hiding this comment.
Remove unconditional application of the Java plugin (conflicts with Android branch).
apply plugin: 'java' is applied globally and again in the non-Android branch. When targeting Android, this can conflict with com.android.library.
Apply this diff:
-apply plugin: 'java'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| apply plugin: 'java' |
🤖 Prompt for AI Agents
In java-client/build.gradle around line 3, the file unconditionally applies the
Java plugin which conflicts with Android projects; remove the global apply
plugin: 'java' line and instead apply the Java plugin only where appropriate
(e.g., in the non-Android branch or in the specific Java submodule), or wrap its
application in a conditional that checks for the Android plugin/Android-target
branch before applying; ensure the Android branch uses com.android.library
without the Java plugin to avoid plugin conflicts.
Comment on lines
+448
to
+466
| public static void validateJsonElement(JsonElement jsonElement) throws IOException { | ||
| if (jsonElement == null) { | ||
| if (!ItemDTO.openapiRequiredFields.isEmpty()) { // has required fields but JSON element is null | ||
| throw new IllegalArgumentException(String.format("The required field(s) %s in ItemDTO is not found in the empty JSON string", ItemDTO.openapiRequiredFields.toString())); | ||
| } | ||
| } | ||
|
|
||
| Set<Map.Entry<String, JsonElement>> entries = jsonElement.getAsJsonObject().entrySet(); | ||
| // check to see if the JSON string contains additional fields | ||
| for (Map.Entry<String, JsonElement> entry : entries) { | ||
| if (!ItemDTO.openapiFields.contains(entry.getKey())) { | ||
| throw new IllegalArgumentException(String.format("The field `%s` in the JSON string is not defined in the `ItemDTO` properties. JSON: %s", entry.getKey(), jsonElement.toString())); | ||
| } | ||
| } | ||
| JsonObject jsonObj = jsonElement.getAsJsonObject(); | ||
| if ((jsonObj.get("dataChangeCreatedBy") != null && !jsonObj.get("dataChangeCreatedBy").isJsonNull()) && !jsonObj.get("dataChangeCreatedBy").isJsonPrimitive()) { | ||
| throw new IllegalArgumentException(String.format("Expected the field `dataChangeCreatedBy` to be a primitive type in the JSON string but got `%s`", jsonObj.get("dataChangeCreatedBy").toString())); | ||
| } | ||
| if ((jsonObj.get("dataChangeLastModifiedBy") != null && !jsonObj.get("dataChangeLastModifiedBy").isJsonNull()) && !jsonObj.get("dataChangeLastModifiedBy").isJsonPrimitive()) { |
There was a problem hiding this comment.
Guard validation against JsonNull and non-object inputs.
Same failure mode as other models.
Apply this diff:
- public static void validateJsonElement(JsonElement jsonElement) throws IOException {
- if (jsonElement == null) {
+ public static void validateJsonElement(JsonElement jsonElement) throws IOException {
+ if (jsonElement == null || jsonElement.isJsonNull()) {
if (!ItemDTO.openapiRequiredFields.isEmpty()) { // has required fields but JSON element is null
throw new IllegalArgumentException(String.format("The required field(s) %s in ItemDTO is not found in the empty JSON string", ItemDTO.openapiRequiredFields.toString()));
}
- }
-
- Set<Map.Entry<String, JsonElement>> entries = jsonElement.getAsJsonObject().entrySet();
+ return;
+ }
+ if (!jsonElement.isJsonObject()) {
+ throw new IllegalArgumentException(String.format("Expected JSON object for ItemDTO but got `%s`", jsonElement.toString()));
+ }
+ JsonObject jsonObj = jsonElement.getAsJsonObject();
+ Set<Map.Entry<String, JsonElement>> entries = jsonObj.entrySet();
// check to see if the JSON string contains additional fields
for (Map.Entry<String, JsonElement> entry : entries) {
if (!ItemDTO.openapiFields.contains(entry.getKey())) {
throw new IllegalArgumentException(String.format("The field `%s` in the JSON string is not defined in the `ItemDTO` properties. JSON: %s", entry.getKey(), jsonElement.toString()));
}
}
- JsonObject jsonObj = jsonElement.getAsJsonObject();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public static void validateJsonElement(JsonElement jsonElement) throws IOException { | |
| if (jsonElement == null) { | |
| if (!ItemDTO.openapiRequiredFields.isEmpty()) { // has required fields but JSON element is null | |
| throw new IllegalArgumentException(String.format("The required field(s) %s in ItemDTO is not found in the empty JSON string", ItemDTO.openapiRequiredFields.toString())); | |
| } | |
| } | |
| Set<Map.Entry<String, JsonElement>> entries = jsonElement.getAsJsonObject().entrySet(); | |
| // check to see if the JSON string contains additional fields | |
| for (Map.Entry<String, JsonElement> entry : entries) { | |
| if (!ItemDTO.openapiFields.contains(entry.getKey())) { | |
| throw new IllegalArgumentException(String.format("The field `%s` in the JSON string is not defined in the `ItemDTO` properties. JSON: %s", entry.getKey(), jsonElement.toString())); | |
| } | |
| } | |
| JsonObject jsonObj = jsonElement.getAsJsonObject(); | |
| if ((jsonObj.get("dataChangeCreatedBy") != null && !jsonObj.get("dataChangeCreatedBy").isJsonNull()) && !jsonObj.get("dataChangeCreatedBy").isJsonPrimitive()) { | |
| throw new IllegalArgumentException(String.format("Expected the field `dataChangeCreatedBy` to be a primitive type in the JSON string but got `%s`", jsonObj.get("dataChangeCreatedBy").toString())); | |
| } | |
| if ((jsonObj.get("dataChangeLastModifiedBy") != null && !jsonObj.get("dataChangeLastModifiedBy").isJsonNull()) && !jsonObj.get("dataChangeLastModifiedBy").isJsonPrimitive()) { | |
| public static void validateJsonElement(JsonElement jsonElement) throws IOException { | |
| if (jsonElement == null || jsonElement.isJsonNull()) { | |
| if (!ItemDTO.openapiRequiredFields.isEmpty()) { // has required fields but JSON element is null | |
| throw new IllegalArgumentException(String.format("The required field(s) %s in ItemDTO is not found in the empty JSON string", ItemDTO.openapiRequiredFields.toString())); | |
| } | |
| return; | |
| } | |
| if (!jsonElement.isJsonObject()) { | |
| throw new IllegalArgumentException(String.format("Expected JSON object for ItemDTO but got `%s`", jsonElement.toString())); | |
| } | |
| JsonObject jsonObj = jsonElement.getAsJsonObject(); | |
| Set<Map.Entry<String, JsonElement>> entries = jsonObj.entrySet(); | |
| // check to see if the JSON string contains additional fields | |
| for (Map.Entry<String, JsonElement> entry : entries) { | |
| if (!ItemDTO.openapiFields.contains(entry.getKey())) { | |
| throw new IllegalArgumentException(String.format("The field `%s` in the JSON string is not defined in the `ItemDTO` properties. JSON: %s", entry.getKey(), jsonElement.toString())); | |
| } | |
| } | |
| if ((jsonObj.get("dataChangeCreatedBy") != null && !jsonObj.get("dataChangeCreatedBy").isJsonNull()) && !jsonObj.get("dataChangeCreatedBy").isJsonPrimitive()) { | |
| throw new IllegalArgumentException(String.format("Expected the field `dataChangeCreatedBy` to be a primitive type in the JSON string but got `%s`", jsonObj.get("dataChangeCreatedBy").toString())); | |
| } | |
| if ((jsonObj.get("dataChangeLastModifiedBy") != null && !jsonObj.get("dataChangeLastModifiedBy").isJsonNull()) && !jsonObj.get("dataChangeLastModifiedBy").isJsonPrimitive()) { |
🤖 Prompt for AI Agents
In java-client/src/main/java/org/openapitools/client/model/ItemDTO.java around
lines 448 to 466, validateJsonElement currently calls getAsJsonObject() without
ensuring the input is neither JsonNull nor a non-object, which can cause
exceptions; add guards: if jsonElement is JsonNull treat like null (throw the
same required-fields error if needed), and if !jsonElement.isJsonObject() throw
an IllegalArgumentException indicating the JSON must be an object for ItemDTO;
place these checks before calling getAsJsonObject() so subsequent property
checks are safe.
Comment on lines
+190
to
+206
| public static void validateJsonElement(JsonElement jsonElement) throws IOException { | ||
| if (jsonElement == null) { | ||
| if (!ListItemDiffs.openapiRequiredFields.isEmpty()) { // has required fields but JSON element is null | ||
| throw new IllegalArgumentException(String.format("The required field(s) %s in ListItemDiffs is not found in the empty JSON string", ListItemDiffs.openapiRequiredFields.toString())); | ||
| } | ||
| } | ||
|
|
||
| Set<Map.Entry<String, JsonElement>> entries = jsonElement.getAsJsonObject().entrySet(); | ||
| // check to see if the JSON string contains additional fields | ||
| for (Map.Entry<String, JsonElement> entry : entries) { | ||
| if (!ListItemDiffs.openapiFields.contains(entry.getKey())) { | ||
| throw new IllegalArgumentException(String.format("The field `%s` in the JSON string is not defined in the `ListItemDiffs` properties. JSON: %s", entry.getKey(), jsonElement.toString())); | ||
| } | ||
| } | ||
| JsonObject jsonObj = jsonElement.getAsJsonObject(); | ||
| // validate the optional field `namespace` | ||
| if (jsonObj.get("namespace") != null && !jsonObj.get("namespace").isJsonNull()) { |
There was a problem hiding this comment.
Harden JSON validator against JsonNull and non-object payloads (prevents IllegalStateException).
If the payload is JSON null or not an object, getAsJsonObject() will throw at runtime. Guard early and reuse jsonObj.
Apply this diff:
- public static void validateJsonElement(JsonElement jsonElement) throws IOException {
- if (jsonElement == null) {
+ public static void validateJsonElement(JsonElement jsonElement) throws IOException {
+ if (jsonElement == null || jsonElement.isJsonNull()) {
if (!ListItemDiffs.openapiRequiredFields.isEmpty()) { // has required fields but JSON element is null
throw new IllegalArgumentException(String.format("The required field(s) %s in ListItemDiffs is not found in the empty JSON string", ListItemDiffs.openapiRequiredFields.toString()));
}
- }
-
- Set<Map.Entry<String, JsonElement>> entries = jsonElement.getAsJsonObject().entrySet();
+ return;
+ }
+ if (!jsonElement.isJsonObject()) {
+ throw new IllegalArgumentException(String.format("Expected JSON object for ListItemDiffs but got `%s`", jsonElement.toString()));
+ }
+ JsonObject jsonObj = jsonElement.getAsJsonObject();
+ Set<Map.Entry<String, JsonElement>> entries = jsonObj.entrySet();
// check to see if the JSON string contains additional fields
for (Map.Entry<String, JsonElement> entry : entries) {
if (!ListItemDiffs.openapiFields.contains(entry.getKey())) {
throw new IllegalArgumentException(String.format("The field `%s` in the JSON string is not defined in the `ListItemDiffs` properties. JSON: %s", entry.getKey(), jsonElement.toString()));
}
}
- JsonObject jsonObj = jsonElement.getAsJsonObject();Also applies to: 197-205, 204-216
🤖 Prompt for AI Agents
In java-client/src/main/java/org/openapitools/client/model/ListItemDiffs.java
around lines 190-206 (and similarly apply to the other indicated ranges 197-205
and 204-216), the validator calls jsonElement.getAsJsonObject() without guarding
for null or non-object JSON which can throw IllegalStateException; update the
method to first check if jsonElement is null or jsonElement.isJsonNull(), and
then verify jsonElement.isJsonObject() before calling getAsJsonObject(); assign
the result once to a local JsonObject jsonObj and reuse it for subsequent
validations, and if the element is not a JSON object throw a clear
IllegalArgumentException stating the expected object type (or handle according
to existing behavior) so that JsonNull and non-object payloads are safely
rejected.
Comment on lines
+21
to
+24
| public Pair(String name, String value) { | ||
| this.name = isValidString(name) ? name : ""; | ||
| this.value = isValidString(value) ? value : ""; | ||
| } |
There was a problem hiding this comment.
Avoid coercing null to empty string (query/header semantics may break).
Empty string differs from null; this can emit unintended param= or blank headers.
Apply:
public Pair(String name, String value) {
- this.name = isValidString(name) ? name : "";
- this.value = isValidString(value) ? value : "";
+ this.name = name;
+ this.value = value;
}Optionally, fail fast if name == null depending on ApiClient behavior.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public Pair(String name, String value) { | |
| this.name = isValidString(name) ? name : ""; | |
| this.value = isValidString(value) ? value : ""; | |
| } | |
| public Pair(String name, String value) { | |
| this.name = name; | |
| this.value = value; | |
| } |
Comment on lines
+44
to
+61
| public String URL(Map<String, String> variables) { | ||
| String url = this.URL; | ||
|
|
||
| // go through variables and replace placeholders | ||
| for (Map.Entry<String, ServerVariable> variable: this.variables.entrySet()) { | ||
| String name = variable.getKey(); | ||
| ServerVariable serverVariable = variable.getValue(); | ||
| String value = serverVariable.defaultValue; | ||
|
|
||
| if (variables != null && variables.containsKey(name)) { | ||
| value = variables.get(name); | ||
| if (serverVariable.enumValues.size() > 0 && !serverVariable.enumValues.contains(value)) { | ||
| throw new IllegalArgumentException("The variable " + name + " in the server URL has invalid value " + value + "."); | ||
| } | ||
| } | ||
| url = url.replace("{" + name + "}", value); | ||
| } | ||
| return url; |
There was a problem hiding this comment.
Harden URL templating: null checks for variables, enumValues, and defaults
Possible NPEs when this.variables is null, enumValues is null, or value/defaultValue is null. Also ensure enum validation only runs when a non‑empty enum is defined.
- public String URL(Map<String, String> variables) {
- String url = this.URL;
-
- // go through variables and replace placeholders
- for (Map.Entry<String, ServerVariable> variable: this.variables.entrySet()) {
- String name = variable.getKey();
- ServerVariable serverVariable = variable.getValue();
- String value = serverVariable.defaultValue;
-
- if (variables != null && variables.containsKey(name)) {
- value = variables.get(name);
- if (serverVariable.enumValues.size() > 0 && !serverVariable.enumValues.contains(value)) {
- throw new IllegalArgumentException("The variable " + name + " in the server URL has invalid value " + value + ".");
- }
- }
- url = url.replace("{" + name + "}", value);
- }
- return url;
- }
+ public String URL(Map<String, String> variables) {
+ String url = this.URL;
+ Map<String, ServerVariable> vars = (this.variables != null) ? this.variables : java.util.Collections.emptyMap();
+ for (Map.Entry<String, ServerVariable> entry : vars.entrySet()) {
+ String name = entry.getKey();
+ ServerVariable serverVariable = entry.getValue();
+ if (serverVariable == null) continue;
+
+ String value = (variables != null && variables.containsKey(name))
+ ? variables.get(name)
+ : serverVariable.defaultValue;
+
+ if (value == null) {
+ throw new IllegalArgumentException("Missing value for server URL variable {" + name + "} and no default provided.");
+ }
+ if (serverVariable.enumValues != null && !serverVariable.enumValues.isEmpty()
+ && !serverVariable.enumValues.contains(value)) {
+ throw new IllegalArgumentException("The variable " + name + " in the server URL has invalid value " + value + ".");
+ }
+ url = url.replace("{" + name + "}", value);
+ }
+ return url;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public String URL(Map<String, String> variables) { | |
| String url = this.URL; | |
| // go through variables and replace placeholders | |
| for (Map.Entry<String, ServerVariable> variable: this.variables.entrySet()) { | |
| String name = variable.getKey(); | |
| ServerVariable serverVariable = variable.getValue(); | |
| String value = serverVariable.defaultValue; | |
| if (variables != null && variables.containsKey(name)) { | |
| value = variables.get(name); | |
| if (serverVariable.enumValues.size() > 0 && !serverVariable.enumValues.contains(value)) { | |
| throw new IllegalArgumentException("The variable " + name + " in the server URL has invalid value " + value + "."); | |
| } | |
| } | |
| url = url.replace("{" + name + "}", value); | |
| } | |
| return url; | |
| public String URL(Map<String, String> variables) { | |
| String url = this.URL; | |
| Map<String, ServerVariable> vars = (this.variables != null) ? this.variables : java.util.Collections.emptyMap(); | |
| for (Map.Entry<String, ServerVariable> entry : vars.entrySet()) { | |
| String name = entry.getKey(); | |
| ServerVariable serverVariable = entry.getValue(); | |
| if (serverVariable == null) continue; | |
| String value = (variables != null && variables.containsKey(name)) | |
| ? variables.get(name) | |
| : serverVariable.defaultValue; | |
| if (value == null) { | |
| throw new IllegalArgumentException("Missing value for server URL variable {" + name + "} and no default provided."); | |
| } | |
| if (serverVariable.enumValues != null && !serverVariable.enumValues.isEmpty() | |
| && !serverVariable.enumValues.contains(value)) { | |
| throw new IllegalArgumentException("The variable " + name + " in the server URL has invalid value " + value + "."); | |
| } | |
| url = url.replace("{" + name + "}", value); | |
| } | |
| return url; | |
| } |
🤖 Prompt for AI Agents
In java-client/src/main/java/org/openapitools/client/ServerConfiguration.java
around lines 44 to 61, the URL templating can throw NPEs when this.variables, a
ServerVariable, its enumValues, or defaultValue (or the provided variables map
entries) are null; update the method to first return the base URL if
this.variables is null, skip null ServerVariable entries, set value to a safe
default (e.g. empty string) when defaultValue is null, replace value with
variables.get(name) only if the variables map is non-null and contains a
non-null entry, and run enum validation only when serverVariable.enumValues is
non-null and not empty before checking contains(value).
Comment on lines
+23
to
+36
| public String description; | ||
| public String defaultValue; | ||
| public HashSet<String> enumValues = null; | ||
|
|
||
| /** | ||
| * @param description A description for the server variable. | ||
| * @param defaultValue The default value to use for substitution. | ||
| * @param enumValues An enumeration of string values to be used if the substitution options are from a limited set. | ||
| */ | ||
| public ServerVariable(String description, String defaultValue, HashSet<String> enumValues) { | ||
| this.description = description; | ||
| this.defaultValue = defaultValue; | ||
| this.enumValues = enumValues; | ||
| } |
There was a problem hiding this comment.
Prevent NPE: initialize enumValues and null‑guard in ctor
ServerConfiguration.URL(...) calls enumValues.size() without null checks. With enumValues defaulting to null here, this will NPE at runtime when the schema omits enum. Initialize to empty and coalesce in ctor.
- public HashSet<String> enumValues = null;
+ public HashSet<String> enumValues = new HashSet<>();
@@
- this.enumValues = enumValues;
+ this.enumValues = (enumValues != null) ? enumValues : new HashSet<>();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public String description; | |
| public String defaultValue; | |
| public HashSet<String> enumValues = null; | |
| /** | |
| * @param description A description for the server variable. | |
| * @param defaultValue The default value to use for substitution. | |
| * @param enumValues An enumeration of string values to be used if the substitution options are from a limited set. | |
| */ | |
| public ServerVariable(String description, String defaultValue, HashSet<String> enumValues) { | |
| this.description = description; | |
| this.defaultValue = defaultValue; | |
| this.enumValues = enumValues; | |
| } | |
| public String description; | |
| public String defaultValue; | |
| public HashSet<String> enumValues = new HashSet<>(); | |
| /** | |
| * @param description A description for the server variable. | |
| * @param defaultValue The default value to use for substitution. | |
| * @param enumValues An enumeration of string values to be used if the substitution options are from a limited set. | |
| */ | |
| public ServerVariable(String description, String defaultValue, HashSet<String> enumValues) { | |
| this.description = description; | |
| this.defaultValue = defaultValue; | |
| this.enumValues = (enumValues != null) ? enumValues : new HashSet<>(); | |
| } |
🤖 Prompt for AI Agents
In java-client/src/main/java/org/openapitools/client/ServerVariable.java around
lines 23-36, enumValues is initialized to null and the constructor assigns it
directly, which causes NPEs when callers call enumValues.size(); initialize the
field to an empty HashSet<> by default and in the constructor coalesce the
incoming enumValues parameter (if null) to a new empty HashSet<>() before
assigning so the field is never null.
hezhangjian
approved these changes
Sep 18, 2025
This was referenced Oct 19, 2025
Closed
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.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores