Repository navigation
fix(models)!: deserialize model objects reflectively with gson - #473
Merged
Merged
Conversation
The hand-written JsonUtil parsers restated for every field what Gson derives from the field name, so 61 of them are replaced by an inherited reflective implementation and the snake_case naming policy. Only the cases reflection cannot express are kept: adapters for the enum wire formats, Spotify's date format, the polymorphic playlist item and the cursors object, plus hand-written parsers for Disallows, ExternalId, ExternalUrl and PlaybackQueue, whose JSON does not mirror their fields. Three fields were read under keys the API never sends and so were always null. They now bind, which is a visible behaviour change: AudioAnalysisTrack.windowSeconds (read as "windows_seconds"), AudioAnalysisTrack.endOfFadeIn (read as "end_of_face_in") and AudioAnalysisSection.mode (guarded on a "type" key sections do not carry). A malformed date now nulls its own field instead of the whole enclosing object. Model builders are untouched and remain public API.
Nothing ever constructed an ObjectMapper, so the 179 @JsonDeserialize annotations and the two @JsonPOJOBuilder ones only described a deserialization path this library never took. Gson does all the parsing. BREAKING CHANGE: consumers who fed these model classes into their own ObjectMapper got builder-based deserialization for free from those annotations and now have to configure it themselves. Jackson remains on the test classpath transitively via wiremock, but is no longer a compile dependency of the jar.
The "Default constructor." blocks say nothing, and a model builder's setter javadoc repeats the field comment and the getter word for word. Neither is a surface anyone browses: the builders are called by nothing since deserialization became reflective. Getter javadoc stays, since that is what a consumer reads in the published API reference, and SpotifyApi and the requests package are untouched for the same reason. Deleting a comment on a public member makes doclint report it, so its missing group is turned off. The javadoc CI step exists to catch dead references before a tagged release attaches the docs, and all,-missing still fails the build on those.
The Java version claimed 17+ while every profile compiles with --release 9, the pinned version had drifted four release candidates behind, and the request recipe still told agents to add a @JsonDeserialize annotation. Adds a note on how model objects are deserialized now, since that changes what adding a field to one costs.
ModelObjectGson delegates to the hand-written parser on Disallows, ExternalId and ExternalUrl, so removing one of those overrides makes the inherited reflective path call the adapter that calls it back, until the stack runs out. PlaybackQueue's override is what returns an empty list for a missing "queue". Disallows, ExternalUrl and PlaybackQueue were already covered by request tests. ExternalId was not covered at all, and no model object holds one, so it is read directly here. Each override also carries a note saying why it exists.
Beta added an externalIds field to Track and Album, which conflicted with the reflective deserialization on this branch. The field, getter and toString entry are kept; the hand-written parser branch beta added for it is dropped, since reflection reads external_ids on its own now. ExternalId is therefore no longer an orphan, so ModelObjectGson's adapter for it is live and ModelObjectMappingTest asserts it through Album rather than alone.
The earlier pass spared any block whose @PARAM happened to contain a {@link} or an <a href>, which split the setters on a distinction that carries no meaning: "@PARAM copyrights {@link Copyright} objects." says no more than the signature. Every link in the spared blocks is already on the matching getter, which keeps its javadoc, so the published reference loses nothing. The javadoc run still reports no warnings.
Nothing in the library, the tests or the examples referred to any of these. PlaylistTrackPosition, ProductType and AlbumGroup lost their last caller when 10.0.0 dropped the getters that used them. Paging.JsonUtil and PagingCursorbased.JsonUtil resolve their type argument to Paging<X> rather than a Class, so instantiating either has always thrown; paging objects are built through AbstractModelObject.JsonUtil.createModelObjectPaging, which takes the element type from the enclosing JsonUtil. IJsonUtil.createModelObjectArray(JsonArray, Class) had no callers left. Also drops the @SuppressWarnings on the ExternalId and ExternalUrl parsers, which no longer suppress anything. BREAKING CHANGE: these types and methods are removed. They existed in 9.x, so a consumer still referencing one will fail to compile against 10.0.0.
Adds the removed classes, the switch to reflective deserialization and the dropped Jackson dependency, and the three getters that stopped returning null. AGENTS.md now describes the example and README index step, what ci/check-docs.py enforces, and the rule that a name removed from the public API has to be recorded in a v9 column of MIGRATION.md so the guide keeps resolving.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## beta #473 +/- ##
=============================================
- Coverage 48.93% 29.66% -19.27%
+ Complexity 716 679 -37
=============================================
Files 325 323 -2
Lines 11195 8851 -2344
Branches 1319 776 -543
=============================================
- Hits 5478 2626 -2852
- Misses 4696 5669 +973
+ Partials 1021 556 -465 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merged
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
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.
The hand-written JsonUtil parsers restated for every field what Gson derives from the field name, so 61 of them are gone, replaced by an inherited reflective implementation and the snake_case naming policy. Only the cases reflection cannot express are kept: adapters for the enum wire formats, Spotify's date format, the polymorphic playlist item and the cursors object, plus hand-written parsers for Disallows, ExternalId, ExternalUrl and PlaybackQueue, whose JSON does not mirror their fields. Eleven fields carry a @SerializedName because their wire name is not the snake_case form of the field.
Three getters were reading keys the API never sends and so always returned null. AudioAnalysisTrack.getWindowSeconds() was read as "windows_seconds", getEndOfFadeIn() as "end_of_face_in", and AudioAnalysisSection.getMode() was guarded on a "type" key that sections do not carry. The vendored spec confirms the correct spellings. Code that null-checked these will now take the other branch. A malformed date also nulls its own field now instead of the whole enclosing object.
Jackson is dropped. Nothing ever constructed an ObjectMapper, so the 179 @JsonDeserialize annotations described a deserialization path the library never took.
Unreferenced public API is deleted: PlaylistTrackPosition, the ProductType and AlbumGroup enums, Paging.JsonUtil and PagingCursorbased.JsonUtil, and the IModelObject array helper taking an explicit element class. The two JsonUtil shells resolved their type argument to a parameterized type rather than a class, so instantiating either always threw.
Javadoc that only restates the signature is removed from the model builder setters and the default constructors. Getter javadoc stays, since that is what a consumer reads in the published reference, and every link the setters carried is already on the matching getter. Deleting a comment on a public member makes doclint report it, so its missing group is turned off; all,-missing still fails the build on a dead reference, which is what the javadoc CI step guards.
Model builders are otherwise untouched and remain public API. Nothing calls them any more, which makes them the natural next target: generating them and the request classes from spec/openapi.yaml is deliberately left out of this PR.
MIGRATION.md records the removals, the deserialization change and the three fixed getters. AGENTS.md gains the example and README index step, what ci/check-docs.py enforces, and the rule that a removed name has to sit in a v9 column of the guide; it also had three stale facts corrected, having claimed Java 17+ where every profile compiles with --release 9, pinned a version four release candidates behind, and told agents to add a @JsonDeserialize annotation.
401 tests pass, up from 395, and ci/check-docs.py and the javadoc build are clean.
BREAKING CHANGE: consumers who fed these model classes into their own ObjectMapper got builder-based deserialization from the Jackson annotations and now have to configure it themselves. The deleted types and methods existed in 9.x and will no longer compile.