Skip to content

SK-2814: Java SDK v2.1 — Public interface nomenclature cleanup, backward compat deprecations, and docs#308

Merged
Devesh-Skyflow merged 55 commits into
release/26.5.2from
devesh/SK-2814-java--public-interface-cleanup
May 20, 2026
Merged

SK-2814: Java SDK v2.1 — Public interface nomenclature cleanup, backward compat deprecations, and docs#308
Devesh-Skyflow merged 55 commits into
release/26.5.2from
devesh/SK-2814-java--public-interface-cleanup

Conversation

@Devesh-Skyflow
Copy link
Copy Markdown
Collaborator

@Devesh-Skyflow Devesh-Skyflow commented May 18, 2026

Summary

This PR delivers the public interface cleanup for the Java SDK v2.1.x release,
restoring full backward compatibility for v2 customers while introducing
deprecation signals for renamed APIs.


What Changed

Nomenclature fixes (HIGH — breaking in prior commit, now restored)

  • skyflow_idskyflowId in Get and Query response maps. Both keys now
    coexist simultaneously. skyflow_id emits a [DEPRECATED] WARN log on access.
  • clientId / keyId / tokenUri accepted alongside legacy clientID /
    keyID / tokenURI in credentials JSON. Both forms permanently supported —
    no migration required, no deprecation log.

Deprecations (compile-time + runtime signals)

  • GetRequest.downloadURL()downloadUrl() — old methods kept as
    @Deprecated(since="2.1", forRemoval=true) delegates with runtime WARN log.
  • DetokenizeRequest.downloadURL()downloadUrl() — same pattern.

Additive changes (non-breaking)

  • QueryResponse.getErrors() added — was the only response class missing it.
  • SDK-level null/empty field validation removed from Insert and Update —
    backend is authoritative per API spec (additionalProperties: Any type).

Documentation

  • docs/migrate_to_v2.md — extracted from README (260 lines), with new v2.1
    sections for credential field renames and skyflow_id deprecation.
  • README.md — v2.1.x banner added, migration section replaced with link.
  • CHANGELOG.md — simplified to point to GitHub releases (Node SDK pattern).
  • docs/v2-public-interface-changes.md — PM-facing change summary.

Claude Code setup

  • CLAUDE.md, .claude/commands/, .claude/skills/ — project-level AI tooling.

Test coverage

All changes follow TDD. Every deprecated method is covered by both:

  • An old-form test (deprecated method still works)
  • A new-form test (new method works, cross-asserts same value)

skyflow_id backward compat is covered by 3 tests asserting both keys present.


Test plan

  • mvn test — 374 tests,
  • Verify [DEPRECATED] WARN log appears in logs when skyflow_id key is accessed
  • Verify @Deprecated strikethrough appears in IDE for downloadURL() methods
  • Live sample runs confirm backward compat (credentials with old keys work)

Devesh-Skyflow and others added 30 commits May 13, 2026 22:57
Captures the design for public interface renames per the server-side SDK
nomenclature changes spec: credential field fallbacks (clientId/keyId/tokenUri),
skyflow_id→skyflowId in Get/Query responses, and QueryResponse errors/tokenizedData
field additions.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Adds explanation for why tokenizedData change is valid despite the Query
API currently not returning tokens — based on V1FieldRecords schema
support and cross-SDK consistency requirement.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Adds detailed rationale to each design section: why the naming convention
matters, why the fallback strategy was chosen over a hard cut, why
skyflow_id normalization is inconsistent today, the tokenizedData API
schema vs docs discrepancy, and why getErrors() is missing only from
QueryResponse.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Clarifies that ignoring record.getTokens() in getFormattedQueryRecord
is intentional (Query API cannot return tokens), and that the fix is
to promote the toString() hack into a real always-empty field rather
than reading from the API response.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Query API cannot return tokens; the toString() inconsistency is not
worth fixing since callers have no reason to access tokenizedData
programmatically on query results.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
5-task TDD plan covering credential field renames with fallback,
skyflow_id normalisation in Get/Query responses, and QueryResponse
getErrors() accessor.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…old form

Add fallback lookup logic so getBearerTokenFromCredentials tries new camelCase
keys (clientId, keyId, tokenUri) first and falls back to the legacy all-caps
forms (clientID, keyID, tokenURI) for backward compatibility during migration.
Add testBearerTokenWithNewFormCredentialKeys to verify the new key form is
recognized end-to-end.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…form

Add fallback logic to GenerateSignedTokensFromCredentials so both new-form
keys (clientId/keyId) and legacy all-caps keys (clientID/keyID) are accepted
during migration. Mirrors the pattern already applied to BearerToken.java.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Insert and Update responses already used camelCase skyflowId; Get and
Query were passing through the raw wire-format snake_case key. Add the
rename in getFormattedGetRecord and getFormattedQueryRecord, and add
reflection-based unit tests to cover both formatters.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Adds a private final errors field (always null) and its public accessor
to QueryResponse, matching the pattern in GetResponse and InsertResponse.
Removes the hardcoded responseObject.add("errors", null) from toString()
since serializeNulls on the declared field handles it automatically.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Adapted from skyflow-node PR #305. Includes:
- CLAUDE.md with project overview, structure, naming conventions, build commands
- .claude/settings.json with PostToolUse compile+checkstyle hooks, PreToolUse
  generated-code guard, Stop notification; paths are relative (no hardcoded user dirs)
- .claude/commands/: code-review, code-security, sdk-sample, test slash commands

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- CLAUDE.md: add vault/bin/ package, all 5 controllers, pre-existing
  test failure baseline
- settings.json: fix checkstyle hook to print violations (was silently
  swallowing output with capture_output=True)
- sdk-sample.md: fix InsertOptions (doesn't exist), correct sample
  package structure, correct credential type per feature
- code-review.md: fix validation location (controller not build()),
  fix HashMap rule (SDK pattern is raw HashMaps)
- test.md: document pre-existing failures, note checkstyle failsOnError

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
6-task TDD plan: restore skyflow_id key alongside skyflowId in Get/Query
responses, add WARN deprecation logs for old credential fields, Javadoc
on affected response methods.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…y standard

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…ecation

Non-technical overview of credential field renames and skyflow_id response
key deprecation — covers customer impact, deprecation warnings, migration
guide, and what is NOT changing.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Deprecation plan: add Task 6 for GetRequest + DetokenizeRequest with
  @deprecated annotation approach (compile-time signal vs runtime log)
- PM doc: add section 3 for downloadURL rename with migration example

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…nd Update

The Skyflow API accepts additionalProperties of Any type including null and
empty strings. SDK should not add validation on top of BE — pass through
and let BE decide.

Removed:
- value == null/isEmpty check in validateInsertRequest
- value == null/isEmpty check in validateUpdateRequest
- value == null/isEmpty check in validateTokensMapWithTokenStrict
- values.isEmpty() check in validateInsertRequest (no minItems in API spec)

Kept:
- values == null check (NPE guard — cannot iterate null array)
- key == null/isEmpty check (null keys cannot be JSON-serialized)

Deleted 6 tests that asserted on the removed behaviour.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- PM doc: new section explaining how @deprecated(forRemoval) shows in
  IntelliJ/VS Code autocomplete (strikethrough, orange underline, tooltip
  with clickable link to new method) vs runtime WARN log for map keys
- Deprecation plan: update downloadURL tasks to use
  @deprecated(since="2.1", forRemoval=true) + {link} for stronger IDE signal

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…mmand

Splits old Section 6 into:
- Section 6: Code quality (actionable correctness checks)
- Section 7: Code smells (structural signals, flagged at Smell severity)

Code smell catalogue covers: long methods/classes, business logic in
data classes, toString() with logic, deep nesting, magic numbers,
raw HashMap chains, dead code, stale comments, temporary fields.

Severity table clarified: Critical/Bug/Edge Case/Quality = fix before
merge; Smell = flag and track.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…mpat

Both skyflow_id (deprecated) and skyflowId (new form) are now present
in response maps simultaneously. WARN log emitted per record.
…d DetokenizeRequest

Old downloadURL() methods kept as @deprecated(forRemoval=true) delegates.
Runtime WARN log emitted on old form usage. 100% test coverage:
new form, deprecated form, default value for both classes.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…pported

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
… tests

Both old (downloadURL) and new (downloadUrl) builder methods tested:
- GetTests: 2 new tests for downloadUrl() with cross-assertion on
  deprecated getDownloadURL() returning same value
- DetokenizeTests: 1 new test same pattern
- VaultClientTests: 1 new integration test for DetokenizeRequest.downloadUrl()

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Extract 260-line migration section from README.md to docs/migrate_to_v2.md
  following the pattern established in skyflow-node PR #258
- README now links to docs/migrate_to_v2.md instead of inline content
- docs/migrate_to_v2.md adds v2.1+ sections for credential field renames
  and skyflow_id deprecation (new content)
- CHANGELOG.md: add v2.0.4 release notes covering nomenclature changes,
  backward compat deprecations, and validation removal

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging.

@skyflowapi skyflowapi deleted a comment from github-actions Bot May 18, 2026
@skyflowapi skyflowapi deleted a comment from github-actions Bot May 18, 2026
@skyflowapi skyflowapi deleted a comment from github-actions Bot May 18, 2026
Calling .downloadUrl(null) previously stored null in the field, creating
an NPE risk for callers who read getDownloadUrl() back without a null
check. Now null -> false (matching the default), consistent with the
continueOnError(null) guard in the same builder.

Added test: testDetokenizeRequestDownloadUrlNullTreatedAsFalse

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging.

Devesh-Skyflow and others added 3 commits May 18, 2026 17:41
EmptyValues, EmptyValueInValues, EmptyValueInTokens (ErrorMessage) and
EMPTY_VALUES, EMPTY_OR_NULL_VALUE_IN_VALUES, EMPTY_OR_NULL_VALUE_IN_TOKENS
(ErrorLogs) are unreachable since the SDK-level null/empty field validation
was removed. Deleted to prevent accidental re-wiring.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Three new tests assert that previously-blocked inputs now pass SDK
validation (SDK defers to BE per API spec additionalProperties: Any type):
- Empty values array [] passes
- Null field value passes
- Empty string field value passes

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging.

@Devesh-Skyflow Devesh-Skyflow changed the base branch from main to release/26.5.2 May 19, 2026 04:01
Devesh-Skyflow and others added 5 commits May 19, 2026 11:59
…e request ID

- Add EMPTY_STRING, QUOTE, HTTPS_PROTOCOL, CURLY_PLACEHOLDER and
  HttpUtilityExtra (RAW_BODY_KEY, SDK_GENERATED_PREFIX) to Constants
- HttpUtility: conditional content-type header, __raw_body__ passthrough,
  UUID fallback when server omits x-request-id, URL-encoded form params
- Utils: URL-encode path and query params with graceful fallback
- Validations: accept String request bodies for non-JSON content types
- ConnectionController: wrap String bodies in __raw_body__ for non-JSON
  content types; fall back to raw string when response is not JSON
- InfoLogs: "Bearer token is expired" → "Bearer token is invalid or expired"
- HttpUtilityTests: add raw body, no content-type, null request ID and
  special-character form-encoding tests

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…etectController

- Remove dead statement `response.getEntities().get(0).getFile()` that discarded
  its result and threw IndexOutOfBoundsException/NPE when entities was null or empty
  (C1); the guarded block below already handles entity processing correctly
- Replace unguarded `response.getOutput().get()` in getFirstOutput() with
  `.orElse(null)` so an absent output Optional returns null instead of throwing
  NoSuchElementException (C2)
- Replace unguarded `firstOutput.getProcessedFileExtension().get().toString()` with
  `.map(Object::toString).orElse(UNKNOWN)`, reusing the already-computed Optional
  and matching the safe pattern used for processedFileType one line above (C3)
- Add 4 unit tests covering all three fixes via reflection

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rm-encode

- VaultController.delete: replace unguarded getRecordIdResponse().get() with
  .orElse(Collections.emptyList()) so an absent field in the API response
  returns an empty list instead of throwing NoSuchElementException (M2)
- BearerToken, SignedDataTokens: wrap FileReader in try/finally to guarantee
  close() is called even when JsonParser throws JsonSyntaxException; close
  IOException is intentionally swallowed since the parse already completed (M3)
- HttpUtility.formatJsonToFormEncodedString: guard against empty entry set so
  substring(0, -1) is not called on an empty StringBuilder (M4)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace deprecated credential key names in examples: clientID→clientId,
  keyID→keyId, TokenURI→tokenUri
- Update Insert, Get, and Query response examples to use skyflowId (the SDK
  has always returned skyflowId for Insert; Get/Query now return skyflowId
  as the primary key)
- Add deprecation note after each affected response block: skyflow_id is
  deprecated and will be removed in an upcoming release

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@skyflowapi skyflowapi deleted a comment from github-actions Bot May 20, 2026
@skyflowapi skyflowapi deleted a comment from github-actions Bot May 20, 2026
@skyflowapi skyflowapi deleted a comment from github-actions Bot May 20, 2026
@skyflowapi skyflowapi deleted a comment from github-actions Bot May 20, 2026
Comment thread src/main/java/com/skyflow/vault/controller/VaultController.java Outdated
Comment thread src/main/java/com/skyflow/vault/controller/VaultController.java
Devesh-Skyflow and others added 4 commits May 20, 2026 12:10
…d header

When the response carries no x-request-id header, set requestID to null
instead of fabricating a UUID with an SDK-Generated- prefix. A null value
is the honest signal to callers that no server request ID is available.
Also removes the now-unused UUID import.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace per-record printWarningLog calls for DEPRECATED_SKYFLOW_ID_KEY
with printWarningLogOnce so the deprecation notice fires at most once
per JVM session regardless of result set size.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The SDK no longer generates a fallback UUID when the server omits
x-request-id; getRequestID() now returns null. Update the assertion
to match the new behaviour.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The SDK puts camelCase requestIndex in response maps; the README
examples incorrectly showed snake_case request_index.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Devesh-Skyflow Devesh-Skyflow merged commit de8273b into release/26.5.2 May 20, 2026
4 of 5 checks passed
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.

2 participants