feat(java-sdk): remove omitted basic auth fields from generated SDK API#14408
feat(java-sdk): remove omitted basic auth fields from generated SDK API#14408Swimburger wants to merge 13 commits intomainfrom
Conversation
… configured in IR Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
…ted flag Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| // Build per-field checks: omittable fields are always satisfied, required fields must be present | ||
| String usernameCheck; | ||
| if (usernameOmitted) { | ||
| usernameCheck = "true"; | ||
| } else { | ||
| StringBuilder uc = new StringBuilder("(usernameSupplier != null"); | ||
| if (usernameEnvVar != null) { | ||
| uc.append(" || System.getenv(\"").append(usernameEnvVar).append("\") != null"); | ||
| } | ||
| uc.append(")"); | ||
| usernameCheck = uc.toString(); | ||
| } | ||
| condition.append(") && (passwordSupplier != null"); | ||
| if (passwordEnvVar != null) { | ||
| condition.append(" || System.getenv(\"").append(passwordEnvVar).append("\") != null"); | ||
|
|
||
| String passwordCheck; | ||
| if (passwordOmitted) { | ||
| passwordCheck = "true"; | ||
| } else { | ||
| StringBuilder pc = new StringBuilder("(passwordSupplier != null"); | ||
| if (passwordEnvVar != null) { | ||
| pc.append(" || System.getenv(\"").append(passwordEnvVar).append("\") != null"); | ||
| } | ||
| pc.append(")"); | ||
| passwordCheck = pc.toString(); | ||
| } | ||
| condition.append(")"); | ||
|
|
||
| builder.addStatement("return " + condition); | ||
| builder.addStatement("return " + usernameCheck + " && " + passwordCheck); |
There was a problem hiding this comment.
Logic inconsistency between canCreate() and getAuthHeaders(). When both username and password are omittable, canCreate() returns true even when no credentials are provided (since usernameCheck="true" and passwordCheck="true" results in true && true). However, getAuthHeaders() throws a RuntimeException if both are null (line 154). This causes canCreate() to incorrectly return true, then later fail at runtime when getAuthHeaders() is called.
When both are omittable, canCreate() should use || logic to ensure at least one credential is available:
if (usernameOmitted && passwordOmitted) {
// Both optional - need at least one
builder.addStatement("return (" + usernameCheck + ") || (" + passwordCheck + ")");
} else {
builder.addStatement("return " + usernameCheck + " && " + passwordCheck);
}Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
…er-field omit checks for build validation and setAuthentication Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…optional password Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…c-auth-optional-java-sdk
…r, use empty string internally Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
… omitted password Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…h provider setup Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…OINT_SECURITY path Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| // Only add credentials() method with non-omitted fields as parameters | ||
| MethodSpec.Builder credentialsMethodBuilder = MethodSpec.methodBuilder("credentials") | ||
| .addModifiers(Modifier.PUBLIC); | ||
| if (!usernameOmitted) { | ||
| credentialsMethodBuilder.addParameter( | ||
| ParameterSpec.builder(String.class, usernameFieldName).build()); | ||
| credentialsMethodBuilder.addStatement("this.$L = $L", usernameFieldName, usernameFieldName); | ||
| } | ||
| if (!passwordOmitted) { | ||
| credentialsMethodBuilder.addParameter( | ||
| ParameterSpec.builder(String.class, passwordFieldName).build()); | ||
| credentialsMethodBuilder.addStatement("this.$L = $L", passwordFieldName, passwordFieldName); | ||
| } | ||
| // Only add credentials() method if at least one field is present | ||
| if (!usernameOmitted || !passwordOmitted) { | ||
| credentialsMethodBuilder | ||
| .addStatement(isExtensible ? "return self()" : "return this") | ||
| .returns(isExtensible ? TypeVariableName.get("T") : builderName); | ||
| this.clientBuilder.addMethod(credentialsMethodBuilder.build()); | ||
| } |
There was a problem hiding this comment.
🔴 Generated snippets pass omitted fields to credentials(), causing compilation failure
The v1 generator in AbstractRootClientGenerator.java:1326-1344 correctly generates a credentials() method with only non-omitted parameters (e.g., only username when password has omit: true). However, the v2 dynamic snippet generator at generators/java-v2/dynamic-snippets/src/EndpointSnippetGenerator.ts:444-445 always emits both username and password arguments: java.TypeLiteral.raw('"${values.username}", "${values.password}"'). For the basic-auth-optional fixture (where password has omit: true), this produces .credentials("<username>", "<password>") in all generated snippets and the README, while the builder only accepts .credentials(String username). The generated SDK will fail to compile.
Evidence from generated seed output
The generated builder only has a single-parameter credentials method:
seed/java-sdk/basic-auth-optional/src/main/java/com/seed/basicAuthOptional/SeedBasicAuthOptionalClientBuilder.java:30:
public SeedBasicAuthOptionalClientBuilder credentials(String username)
But all snippets call it with two parameters:
seed/java-sdk/basic-auth-optional/src/main/java/com/snippets/Example0.java:8:
.credentials("<username>", "<password>")
Prompt for agents
The Java v2 dynamic snippet generator at generators/java-v2/dynamic-snippets/src/EndpointSnippetGenerator.ts needs to be updated in the getRootClientBasicAuthArgs method (around line 435-448) to account for the omit field on basic auth. Currently it always generates credentials("<username>", "<password>") with both arguments. It needs to check auth.username.omit and auth.password.omit (if those fields exist on the dynamic IR BasicAuth type), and only include non-omitted fields in the credentials() call arguments. If the dynamic IR doesn't yet expose omit information for basic auth fields, the IR definition at packages/ir-sdk/fern/apis/ir-types-latest/definition/ may also need to be updated to include omit on the dynamic BasicAuth type.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Valid finding. The dynamic snippet generator (generators/java-v2/dynamic-snippets/src/EndpointSnippetGenerator.ts) reads from the dynamic IR's BasicAuth type, which currently doesn't have omit fields — it only has username: Name and password: Name. The dynamic IR definition would need to be updated to include usernameOmit/passwordOmit fields, which is an IR-level change similar to the Ruby/PHP IR version issue that's out of scope for this PR.
The snippet files (Example0.java, snippet.json, README.md) will generate incorrect .credentials("<username>", "<password>") calls until the dynamic IR is updated. The core SDK generator (v1) correctly generates the single-parameter credentials(String username) builder method — the mismatch is only in the v2 dynamic snippet output.
This should be addressed in a follow-up when the dynamic IR is updated to carry omit information for basic auth fields.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…e omitted Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Description
Split from #14378 (one PR per generator).
When
usernameOmitorpasswordOmitflags are set in the IR'sBasicAuthScheme, the corresponding field is completely removed from the generated SDK's public API (builder fields,credentials()parameters, constructor parameters). Internally, omitted fields are treated as empty strings when encoding theAuthorization: Basicheader. Default behavior (both fields required) is preserved when no omit flags are set.Changes Made
AbstractRootClientGenerator.java:credentials()method only includes parameters for non-omitted fields (e.g.credentials(String username)when password is omitted)build()validation only checks non-omitted fieldsconfigureAuthMethodcondition only requires non-omitted fields to be presentthis.username + ":" + ""AuthProviderInfonow carriesfieldNameOmitted/secondaryFieldNameOmittedflags (new constructor overload)addRoutingAuthProviderSetup()(ENDPOINT_SECURITY path) uses omit flags to: only check non-omitted builder fields in the condition, and only pass constructor arguments for non-omitted fields to matchBasicAuthProvider's constructor signatureBasicAuthProviderGenerator.java:getAuthHeaders: uses""directly for omitted fields instead of reading from a suppliercanCreate: only checks non-omitted fields (env var or supplier presence)versions.yml: new 4.0.10 entry (feat)basic-auth-optionaltest fixture withpassword: omit: true, plus full seed output atseed/java-sdk/basic-auth-optional/Updates since last revision
AuthProviderInfonow tracks per-field omit flags (fieldNameOmitted,secondaryFieldNameOmitted)addRoutingAuthProviderSetup()only passes constructor args for non-omitted fields, matchingBasicAuthProvider's generated constructor signature (previously always passed two args, causing a compilation error when a field is omitted)BasicAuthProviderGenerator: now conditional onusernameOmitted/passwordOmittedflags so it only references fields the user can actually provideKnown limitation
Java v2 dynamic snippets (
generators/java-v2/dynamic-snippets/src/EndpointSnippetGenerator.ts) always emit bothusernameandpasswordin.credentials()calls regardless of omit flags. This produces incorrect snippet output (e.g..credentials("<username>", "<password>")when the builder only accepts.credentials(String username)). The dynamic IR'sBasicAuthtype doesn't carry omit information — fixing this requires an IR-level change and is out of scope for this PR.Testing
basic-auth-optionalfixtureSeedBasicAuthOptionalClientBuilderhas nopasswordfield andcredentials()takes onlyusernameaddRoutingAuthProviderSetup()(lines ~2404-2423), verify the dynamically-builtconstructorArgsstring correctly matchesBasicAuthProvider's constructor for all four omit combinations. This path is not covered by the seed test fixture (no multi-scheme routing inbasic-auth-optional).seed/java-sdk/basic-auth-optional/src/main/java/com/seed/basicAuthOptional/SeedBasicAuthOptionalClientBuilder.java— verifypasswordis fully absent from fields,credentials()signature, and validationseed/java-sdk/basic-auth-optional/src/main/java/com/seed/basicAuthOptional/core/BasicAuthProvider.java— verifycanCreateandgetAuthHeaderscorrectly handle the omitted password (empty string in token, no supplier reference)usernameOmitandpasswordOmitare true, thecredentials()method is not generated at all,configureAuthMethodusesif (true), and ENDPOINT_SECURITY passesnew BasicAuthProvider()with no args — verify this is acceptableusernameSupplierField/passwordSupplierFieldis set tonullwhen omitted and passed tobuildGetAuthHeaders/buildCanCreateMethod— verify these methods correctly handle the null field reference without NPEseed/java-sdk/basic-auth-optional/will have.credentials("<username>", "<password>")which doesn't compile against the generated builder — confirm this is acceptable as a known limitation until the dynamic IR is updatedLink to Devin session: https://app.devin.ai/sessions/0786b963284f4799acb409d5373cde0a
Requested by: @Swimburger