fix: eliminate ByteBuddy UnsupportedFeatureError in GraalVM native image via proxy-free entity contracts and specialized eager queries#108
Conversation
…ed eager-load query for GraalVM native image compatibility - MediaModel: replace HibernateProxy-based equals/hashCode with standard ID-based implementation (removes ByteBuddy dependency at runtime) - ExternalReferenceModel: same fix - MediaRepository: add findByIdWithExternalReferences() — JOIN FETCH query that eagerly loads only the externalReference collection; prevents lazy-load after session close in the update/detail flow without Cartesian products - MediaServiceImpl.findByIdEager: use findByIdWithExternalReferences() instead of plain findById() so external references are pre-loaded before updateAllInformation is called; MediaLazyLoader still handles alternativeTitles/genre/season separately Agent-Logs-Url: https://github.com/EspacoGeek-Teams/SpringAPI_EspacoGeek/sessions/cb902dbd-4ccb-454f-9fe5-3b5d40352431 Co-authored-by: vitorhugo-java <65777252+vitorhugo-java@users.noreply.github.com>
Qodana for JVM5 new problems were found
View the detailed Qodana reportTo be able to view the detailed Qodana report, you can either:
To get - name: 'Qodana Scan'
uses: JetBrains/qodana-action@v2025.3.2
with:
upload-result: trueContact Qodana teamContact us at qodana-support@jetbrains.com
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * {@code externalReference} via a single JOIN FETCH avoids loading all six lazy collections | ||
| * and prevents accidental Cartesian products. The remaining collections | ||
| * ({@code alternativeTitles}, {@code genre}, {@code season}) are loaded on demand by | ||
| * {@link com.espacogeek.geek.utils.MediaLazyLoader} when needed. |
There was a problem hiding this comment.
The new Javadoc is a bit misleading: it says fetching only externalReference avoids loading “all six lazy collections”, but then implies the “remaining collections” are only alternativeTitles, genre, season loaded by MediaLazyLoader. MediaModel also has lazy company and people collections, and MediaLazyLoader does not initialize those. Please adjust the wording to clearly state which collections are initialized by MediaLazyLoader vs. left for other resolvers (e.g., GraphQL @BatchMapping) so future readers don’t assume everything else is covered.
| * {@code externalReference} via a single JOIN FETCH avoids loading all six lazy collections | |
| * and prevents accidental Cartesian products. The remaining collections | |
| * ({@code alternativeTitles}, {@code genre}, {@code season}) are loaded on demand by | |
| * {@link com.espacogeek.geek.utils.MediaLazyLoader} when needed. | |
| * {@code externalReference} via a single JOIN FETCH avoids eagerly loading the other lazy | |
| * collections and helps prevent accidental Cartesian products. This query initializes only | |
| * {@code externalReference}; {@code alternativeTitles}, {@code genre}, and {@code season} | |
| * may still be initialized on demand by | |
| * {@link com.espacogeek.geek.utils.MediaLazyLoader}, while {@code company} and | |
| * {@code people} are left to other resolvers such as GraphQL {@code @BatchMapping}. |
| public Optional<MediaModel> findByIdEager(Integer id) { | ||
| MediaModel media = (MediaModel) mediaRepository.findById(id).orElse(null); | ||
| // Use the specialized query that eagerly fetches externalReference in a single JOIN FETCH. | ||
| // This avoids a lazy-load of externalReference later in the update flow, where the | ||
| // Hibernate session may no longer be active (e.g. virtual-thread context switches). | ||
| MediaModel media = (MediaModel) mediaRepository.findByIdWithExternalReferences(id).orElse(null); |
There was a problem hiding this comment.
findByIdWithExternalReferences already returns Optional<MediaModel>, so the explicit cast to MediaModel and the orElse(null) + null check are redundant. Consider using the Optional directly (e.g., early-return on empty, then operate on the value) and dropping method-level warning suppressions that are no longer needed, to avoid masking real warnings later.
| // Use the specialized query that eagerly fetches externalReference in a single JOIN FETCH. | ||
| // This avoids a lazy-load of externalReference later in the update flow, where the | ||
| // Hibernate session may no longer be active (e.g. virtual-thread context switches). | ||
| MediaModel media = (MediaModel) mediaRepository.findByIdWithExternalReferences(id).orElse(null); |
There was a problem hiding this comment.
findByIdEager now relies on the new MediaRepository#findByIdWithExternalReferences query and the expectation that MediaLazyLoader.initializeCollections is invoked afterward. There are existing unit/integration tests for MediaServiceImpl, but none currently cover findByIdEager; adding a test that verifies the repository method used and that the lazy-loader is called would help prevent regressions in this eager-loading behavior.
|
@copilot apply changes based on the comments in this thread and fix the issues of Qodana for JVM |
…iew feedback - MediaRepository: corrected Javadoc for findByIdWithExternalReferences; now accurately names company/people as @BatchMapping-resolved (not via MediaLazyLoader) per reviewer suggestion - MediaServiceImpl.findByIdEager: removed redundant (MediaModel) cast and orElse(null)+null-check anti-pattern; use Optional.isEmpty()/get() directly; dropped stale method-level @SuppressWarnings that masked nothing meaningful - MediaServiceImplTest: add findByIdEager_WhenMediaExists — verifies the specialized JOIN FETCH query is used (not findById) and that MediaLazyLoader is called; add findByIdEager_WhenMediaNotFound — verifies empty Optional is returned and neither findById nor initializeCollections are called Agent-Logs-Url: https://github.com/EspacoGeek-Teams/SpringAPI_EspacoGeek/sessions/ff0f150c-3e9c-4ad7-a8f2-19b955101e4f Co-authored-by: vitorhugo-java <65777252+vitorhugo-java@users.noreply.github.com>
Applied in commit b2e3fea:
|
MediaModel.equals/hashCode— removeHibernateProxy(root cause of ByteBuddy UnsupportedFeatureError)ExternalReferenceModel.equals/hashCode— removeHibernateProxyfindByIdWithExternalReferencesspecialized query toMediaRepositoryfor the update/detail flowMediaServiceImpl.findByIdEagerto use the new eager query (loads externalReference at query time, not lazily)MediaRepositoryJavadoc — accurately state which collections are handled by MediaLazyLoader vs @BatchMappingMediaServiceImpl.findByIdEager— remove redundant cast, use Optional directly, drop stale @SuppressWarnings (Qodana/review)findByIdEager— verify specialized query is used and LazyLoader is called