-
Notifications
You must be signed in to change notification settings - Fork 0
Add support for genres and tags with related database schema, services, and AniList integration #1260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…s, and AniList integration
Summary of ChangesHello @Ziedelth, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the application's data model by adding robust support for genres and tags for anime. It establishes the necessary database structures, data transfer objects, and business logic to manage these classifications. A key aspect of this change is the integration with AniList, enabling automatic fetching and maintenance of genre and tag data, thereby enriching the anime information available within the system. The administrative interface has also been updated to present this new data, enhancing content management capabilities. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request is a great addition, introducing comprehensive support for genres and tags. The changes span the entire stack, from database schema and entities to services, AniList integration, and UI updates. The code is well-structured and the new features are implemented thoughtfully. I have identified a few areas for improvement, including a potential bug, a typo, and opportunities to make the caching logic more robust. Overall, excellent work.
| val genres = genreCacheService.findAllByAnime(entity.uuid) | ||
| val tags = animeTagCacheService.findAllByAnime(entity.uuid) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The entity.uuid property is nullable (UUID?), but it's being passed to findAllByAnime in GenreCacheService and AnimeTagCacheService, both of which expect a non-nullable UUID. This could lead to a NullPointerException if an anime entity ever has a null UUID at this stage.
To ensure type safety and prevent potential runtime errors, you should use the non-null assertion operator (!!), similar to how it's used for animeCacheService.getAudioLocales(entity.uuid!!) on line 29.
| val genres = genreCacheService.findAllByAnime(entity.uuid) | |
| val tags = animeTagCacheService.findAllByAnime(entity.uuid) | |
| val genres = genreCacheService.findAllByAnime(entity.uuid!!) | |
| val tags = animeTagCacheService.findAllByAnime(entity.uuid!!) |
|
|
||
| fun findAllByAnime(animeUuid: UUID) = MapCache.getOrCompute( | ||
| "AnimeTagCacheService.findAllByAnime", | ||
| classes = listOf(AnimeTag::class.java, Tag::class.java), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The findAllByAnime method in AnimeTagRepository queries based on the Anime entity. Therefore, the cache for AnimeTagCacheService.findAllByAnime should be invalidated not only when AnimeTag or Tag entities change, but also when Anime entities change. Adding Anime::class.java to the list of classes for cache invalidation will make the caching mechanism more robust and prevent stale data issues.
| classes = listOf(AnimeTag::class.java, Tag::class.java), | |
| classes = listOf(Anime::class.java, AnimeTag::class.java, Tag::class.java), |
|
|
||
| fun findAllByAnime(animeUuid: UUID) = MapCache.getOrCompute( | ||
| "GenreCacheService.findAllByAnime", | ||
| classes = listOf(Genre::class.java), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The findAllByAnime method in GenreRepository queries through the Anime entity to find associated genres. This means the cache for GenreCacheService.findAllByAnime is dependent on changes in Anime entities as well. To ensure the cache is properly invalidated and avoid serving stale data, you should add Anime::class.java to the list of dependency classes.
| classes = listOf(Genre::class.java), | |
| classes = listOf(Anime::class.java, Genre::class.java), |
| import java.util.* | ||
|
|
||
| class AnimeTagService : AbstractService<AnimeTag, AnimeTagRepository>() { | ||
| @Inject private lateinit var genranimeTagRepositoryRepository: AnimeTagRepository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a typo in the variable name genranimeTagRepositoryRepository. It should be animeTagRepository for consistency and better readability.
| @Inject private lateinit var genranimeTagRepositoryRepository: AnimeTagRepository | |
| @Inject private lateinit var animeTagRepository: AnimeTagRepository |
No description provided.