Uh oh!
There was an error while loading. Please reload this page.
SK-3061: revert getFields() to Map<String, Object>, keep getTokens() typed - #409
Merged
Devesh-Skyflow merged 1 commit intoAug 13, 2026
Merged
Conversation
…typed getTokens() itself (item 6, commit 8148504) stays Map<String, List<Token>>. But getFields() delegating straight to it changed its return-type generics from Object to List<Token> - a needless japicmp break for a deprecated alias nobody should be adding new calls to anyway, and one this PR had flagged as avoidable but initially left in favor of just regenerating the baseline. Reverted getFields() to its original Map<String, Object> contract by rendering getTokens()'s typed data back into that raw shape via a new package-private Token.toRawTokens(Map<String, List<Token>>) - the inverse of Token.parseTokens(). Package-private keeps it outside the accessModifier=protected japicmp contract, so it doesn't itself become a compatibility commitment. The round trip is lossless for map-shaped wire input (the normal case) but lossy for the bare-value edge case parseTokens() also handles (a raw "tok-abc" string loses its way back to {"token": "tok-abc", "tokenGroupName": null} - still valid data, just not byte-identical to the original wire shape). Updated every test asserting on getFields() accordingly, and added direct toRawTokens() coverage (null input, single/multiple token groups, round-trip-with-map-input) in ResponseComponentTests. Regenerated flowvault/api-report/skyflow-flowvault-java.baseline.jar via scripts/contract-snapshot-update.sh flowvault - this supersedes the baseline regenerated in f326ec2, since that one still had getFields() returning the typed map. mvn -pl common,flowvault -am verify passes clean against it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contract baseline change detected ( |
| Status | Type | Serialization | Compatibility Changes |
|---|---|---|---|
| Source-incompatible | com.skyflow.vault.data.InsertResponseRecord |  |  |
| Modified | com.skyflow.vault.data.Token | |  |
Expand for details.
com.skyflow.vault.data.InsertResponseRecord
- Binary-compatible
- Source-compatible
- Serialization-compatible
| Status | Modifiers | Type | Name | Extends | JDK | Serialization | Compatibility Changes |
|---|---|---|---|---|---|---|---|
| Source-incompatible | public | Class | InsertResponseRecord | Object | JDK 8 | | |
Methods
| Status | Modifiers | Generics | Type | Method | Annotations | Throws | Compatibility Changes |
|---|---|---|---|---|---|---|---|
| Source-incompatible | public | Map<String, List<Token>>Map<String, Object> | getFields() | |
com.skyflow.vault.data.Token
- Binary-compatible
- Source-compatible
- Serialization-compatible
| Status | Modifiers | Type | Name | Extends | JDK | Serialization | Compatibility Changes |
|---|---|---|---|---|---|---|---|
| Modified | public | Class | Token | Object | JDK 8 | | |
Warning
All missing classes, i.e. superclasses and interfaces that could not be found on the classpath were ignored.
Hence changes caused by these superclasses and interfaces are not reflected in the output.
Generated on: 2026-08-13 15:53:26.244+0000.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## flowvault-release/26.8.13 #409 +/- ##
============================================================
Coverage ? 91.37% Complexity ? 28 ============================================================
Files ? 158 Lines ? 6448 Branches ? 861 ============================================================
Hits ? 5892 Misses ? 362 Partials ? 194
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Follow-up to #408 (already merged). That PR made
InsertResponseRecord.getTokens()typed (Map<String, List<Token>>), and hadgetFields()(the deprecatedfields→tokensalias) delegate straight to it — which meantgetFields()'s return type became typed too,Map<String, List<Token>>instead of its originalMap<String, Object>.This reverts just that:
getFields()is back topublic Map<String, Object> getFields(), its pre-#408 contract.getTokens()is untouched and stays typed.Why
getFields()is@Deprecated(forRemoval = true)— nobody should be writing new code against it, and changing its return type generics is an avoidablejapicmp-breaking change for a method that exists purely so old callers keep compiling. #408 accepted that break (and regenerated the baseline) rather than doing the extra work to avoid it; this PR does that extra work instead, since it's a small, contained fix.Change
Token.toRawTokens(Map<String, List<Token>>): Map<String, Object>— package-private, the inverse ofToken.parseTokens(Map<String, Object>). Package-private keeps it outside theaccessModifier=protectedjapicmpcontract inflowvault/pom.xml, so it isn't itself a compatibility commitment.InsertResponseRecord.getFields()now doesreturn Token.toRawTokens(getTokens());instead of delegating directly.{token, tokenGroupName}entries per column) but lossy for the bare-value edge caseparseTokens()also has to handle: a raw"tok-abc"string round-trips to{"token": "tok-abc", "tokenGroupName": null}— still valid, just not byte-identical to the original wire shape. Only affectsgetFields(), the deprecated path.getFields()'s shape (ResponseComponentTests,BulkResponseTests,UtilsTests,VaultControllerTests), and added directtoRawTokens()coverage (null input, single/multiple token groups, round-trip-with-map-input) inResponseComponentTests.flowvault/README.md's description ofgetFields()to match.flowvault/api-report/skyflow-flowvault-java.baseline.jarviascripts/contract-snapshot-update.sh flowvault—getFields()'s return type change (typed → back toMap<String, Object>) is itself ajapicmp-flagged change relative to SK-3061: revert fields→tokens rename, restore data field on bulkInsert response #408's baseline, so the baseline needs updating again here. No samples callgetFields(), so none needed changes.Testing
mvn -pl common,flowvault -am test -Dtest='!TokenTests#testExpiredTokenForIsExpiredToken' -DfailIfNoTests=false→ 696 tests, 0 failures (the excluded test is a pre-existing, unrelated failure — reads a CI-only secret from a.envfile not committed to the repo).mvn -pl common,flowvault -am verify -Dgpg.skip=true(same exclusion) → BUILD SUCCESS,japicmppasses clean against the regenerated baseline in this PR.🤖 Generated with Claude Code