Use DTOs to parse tracking API responses #1103
Merged
Conversation
This file contains 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
|
Checked MAL, AL, Kitsu, MU, and Shikimori, they all seem to work just fine. I don't have a Bangumi account nor do I have Komga/Kavita/Suwayomi set up (although I didn't touch those last three, they should be the same as before). Bangumi needs checking whether it works correctly (searching, adding/deleting, updating progress, status, scores, dates). Should also check if Mihon can correctly fetch data from Bangumi when the user opens the tracking sheet by manually changing data on the website first and seeing if Mihon correctly updates when the tracking sheet is opened. Once Bangumi is checked, this can be ready for review. |
This comment was marked as resolved.
This comment was marked as resolved.
AntsyLich
requested changes
Aug 11, 2024
app/src/main/java/eu/kanade/tachiyomi/data/track/anilist/AnilistUtils.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/eu/kanade/tachiyomi/data/track/anilist/AnilistUtils.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/eu/kanade/tachiyomi/data/track/bangumi/BangumiUtils.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/eu/kanade/tachiyomi/data/track/kitsu/KitsuUtils.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/eu/kanade/tachiyomi/data/track/kitsu/KitsuUtils.kt
Outdated
Show resolved
Hide resolved
MajorTanya
force-pushed
the
migrate-trackers-to-dtos
branch
2 times, most recently
from
d318dc0
to
8e45d07
Compare
MajorTanya
force-pushed
the
migrate-trackers-to-dtos
branch
2 times, most recently
from
d11c4ec
to
d8b9e82
Compare
MajorTanya
force-pushed
the
migrate-trackers-to-dtos
branch
from
d8b9e82
to
e9a1b7a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
MajorTanya
force-pushed
the
migrate-trackers-to-dtos
branch
2 times, most recently
from
e3b014a
to
4d24dee
Compare
This comment has been minimized.
This comment has been minimized.
MajorTanya
force-pushed
the
migrate-trackers-to-dtos
branch
from
4d24dee
to
89b77b8
Compare
|
Shikimori still crashes the app if a user tries to delete an entry from it when the remote entry doesn't exist anymore (user deleted it through other means like the website) due to an uncaught HttpException (404). This is unchanged to previous behaviour but should be addressed somehow. Edit: It also appears like none of the trackers remove a track in Mihon if the remote track has been externally removed like described. Behaviour is different between trackers though: |
|
Aside from the above mentioned and (somewhat) unrelated issues, I have checked all trackers and they appear to be behaving as expected. Checklist:
|
MajorTanya
force-pushed
the
migrate-trackers-to-dtos
branch
from
89b77b8
to
6df7caf
Compare
Changes the handling of tracker API responses to be parsed to DTOs instead of doing so "manually" by use of `jsonPrimitive`s and/or `Json.decodeFromString` invocations. This greatly simplifies the API response handling. Renamed constants to SCREAMING_SNAKE_CASE. Largely tried to name the DTOs in a uniform pattern, with the tracker's (short) name at the beginning of file and data class names (ALOAuth instead of OAuth, etc). With these changes, no area of the code base should be using `jsonPrimitive` and/or `Json.decodeFromString` anymore.
This API returns start and end dates as Long and the score as Double. Kitsu's docs claim they're strings (and they are, when requesting manga details from Kitsu directly) but the Algolia search results return Longs and Double, respectively.
- Renamed `BangumiX` classes to `BGMX` classes. - Renamed `toXStatus` and `toXScore` to `toApiStatus` and `toApiScore`
Removed Suppressions added for detekt. Specifically removed: - `SwallowedException` where an exception ends as a default value - `MagicNumber` - `CyclomaticComplexMethod` - `TooGenericExceptionThrown` Also ran spotlessApply which changed SMAddMangaResponse
The `included` attribute seems to only appear when the user already has the entry in their Kitsu list. Since both `data` and `included` are required for `firstToTrack`, a guard clause has been added before all its calls.
Previously, the non-null assertion (!!) would cause a NullPointerException and a Toast with "Bangumi error: " (no message) when the user had removed their list entry from Bangumi through other means like the website. Now it will show "Bangumi error: Could not find manga". This is analogous to the error shown by Kitsu under these circumstances.
The user would see no indication that Shikimori could not properly refresh the track from the remote. This change causes the error Toast notification to pop up with the following message "Shikimori error: Could not find manga". This is analogous to Kitsu and Bangumi.
MajorTanya
force-pushed
the
migrate-trackers-to-dtos
branch
from
5169ba9
to
07e9b0e
Compare
These particular occurrences weren't needed because properties are directly accessible to further act upon. This neatly simplifies these clauses.
AntsyLich
requested changes
Sep 2, 2024
app/src/main/java/eu/kanade/tachiyomi/data/track/kitsu/KitsuApi.kt
Outdated
Show resolved
Hide resolved
AntsyLich
approved these changes
Sep 2, 2024
AntsyLich
changed the title
cuong-tran
pushed a commit
to komikku-app/komikku
that referenced
this pull request
Sep 4, 2024
* Migrate tracking APIs to DTOs Changes the handling of tracker API responses to be parsed to DTOs instead of doing so "manually" by use of `jsonPrimitive`s and/or `Json.decodeFromString` invocations. This greatly simplifies the API response handling. Renamed constants to SCREAMING_SNAKE_CASE. Largely tried to name the DTOs in a uniform pattern, with the tracker's (short) name at the beginning of file and data class names (ALOAuth instead of OAuth, etc). With these changes, no area of the code base should be using `jsonPrimitive` and/or `Json.decodeFromString` anymore. * Fix wrong types in KitsuAlgoliaSearchItem This API returns start and end dates as Long and the score as Double. Kitsu's docs claim they're strings (and they are, when requesting manga details from Kitsu directly) but the Algolia search results return Longs and Double, respectively. * Apply review changes - Renamed `BangumiX` classes to `BGMX` classes. - Renamed `toXStatus` and `toXScore` to `toApiStatus` and `toApiScore` * Handle migration from detekt to spotless Removed Suppressions added for detekt. Specifically removed: - `SwallowedException` where an exception ends as a default value - `MagicNumber` - `CyclomaticComplexMethod` - `TooGenericExceptionThrown` Also ran spotlessApply which changed SMAddMangaResponse * Fix Kitsu failing to add series The `included` attribute seems to only appear when the user already has the entry in their Kitsu list. Since both `data` and `included` are required for `firstToTrack`, a guard clause has been added before all its calls. * Fix empty Bangumi error when entry doesn't exist Previously, the non-null assertion (!!) would cause a NullPointerException and a Toast with "Bangumi error: " (no message) when the user had removed their list entry from Bangumi through other means like the website. Now it will show "Bangumi error: Could not find manga". This is analogous to the error shown by Kitsu under these circumstances. * Fix Shikimori ignoring missing remote entry The user would see no indication that Shikimori could not properly refresh the track from the remote. This change causes the error Toast notification to pop up with the following message "Shikimori error: Could not find manga". This is analogous to Kitsu and Bangumi. * Remove usage of let where not needed These particular occurrences weren't needed because properties are directly accessible to further act upon. This neatly simplifies these clauses. * Remove missed let (cherry picked from commit 9f99f03)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Changes the handling of tracker API responses to be parsed to DTOs instead of doing so "manually" by use of
jsonPrimitives and/orJson.decodeFromStringinvocations.This greatly simplifies the API response handling.
Renamed constants to SCREAMING_SNAKE_CASE.
Largely tried to name the DTOs in a uniform pattern, with the tracker's (short) name at the beginning of file and data class names (ALOAuth instead of OAuth, etc).
With these changes, no area of the code base should be using
jsonPrimitiveand/orJson.decodeFromStringanymore.