Dev#31
Conversation
Replace in-memory JSONArray readers with streaming BufferedReader-based readers for movies and series to avoid loading entire payloads into memory. Each reader now consumes an InputStream from MediaApi, parses one JSON object per line with JSONParser, tracks currentLine for checkpointing in ExecutionContext, skips already-processed lines on open, and closes the reader in close(). Added @slf4j logging and improved error logs. Also changed DelayChunkListener log level from INFO to DEBUG for the post-chunk delay message.
Add stream-based TMDB fetching and update readers: MediaApi gained updateTitlesStream(), MovieAPIImpl and TvSeriesApiImpl now return a GZIP InputStream with retry semantics. DataJumpUtils was refactored to expose getDataJumpTMDBStream(...) (returning an uncompressed GZIP InputStream) and to rebuild getDataJumpTMDBArray(...) by reading/parsing the stream line-by-line into a JSONArray with improved error handling. MovieJsonReader and SerieJsonReader now consume the JSONArray in-memory with index-based checkpointing instead of line-based BufferedReader parsing; update() persists nextIndex. Also changed DelayChunkListener logging from debug to info and expanded .gitignore entries. These changes centralize HTTP/stream handling, add better retry/error handling, and simplify JSON reader logic.
Replace bitwise & with logical && in MediaController input checks to correctly evaluate null/empty name conditions. In MediaServiceImpl, avoid casting Optional to Page when an id is provided: assemble a single-element list, use PageImpl and a safe pageable fallback so single-result queries behave like paged results. Add @SuppressWarnings where needed. Change artwork refresh loop to call MediaUtils.updateMediaWhenLastTimeUpdateMoreThanOneDay(...) before switching and add break statements to prevent fall-through. Make MediaUtils.updateMediaWhenLastTimeUpdateMoreThanOneDay public so it can be reused. Minor cleanup of an extraneous comment.
Qodana for JVM42 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.1
with:
upload-result: trueContact Qodana teamContact us at qodana-support@jetbrains.com
|
There was a problem hiding this comment.
Pull request overview
This PR refactors TMDB “datajump” export fetching to support streaming, fixes several media lookup/pagination paths that previously performed invalid casts, and reduces unnecessary artwork refreshes by gating updates based on last-update age.
Changes:
- Add a streaming TMDB daily export fetch path (
getDataJumpTMDBStream) and expose it throughMediaApi.updateTitlesStream()with implementations for movies and TV series. - Fix
find*ByIdOrNamemethods to return a properPage(viaPageImpl) instead of an invalid cast fromOptional. - Only update artworks in
mountMediaPagewhen media hasn’t been updated in more than one day; also fix switch fall-through by addingbreaks.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
src/main/java/com/espacogeek/geek/utils/MediaUtils.java |
Exposes the “updated more than one day ago” predicate for reuse outside the utility class. |
src/main/java/com/espacogeek/geek/utils/DataJumpUtils.java |
Introduces TMDB export streaming + refactors array-building to read line-by-line from a stream. |
src/main/java/com/espacogeek/geek/services/impl/MediaServiceImpl.java |
Fixes ID-lookup pagination and gates artwork refreshes by last-update time. |
src/main/java/com/espacogeek/geek/data/api/impl/TvSeriesApiImpl.java |
Implements updateTitlesStream() for series exports. |
src/main/java/com/espacogeek/geek/data/api/impl/MovieAPIImpl.java |
Implements updateTitlesStream() for movie exports. |
src/main/java/com/espacogeek/geek/data/api/MediaApi.java |
Adds a default updateTitlesStream() API. |
src/main/java/com/espacogeek/geek/controllers/MediaController.java |
Replaces bitwise & with logical && in input validation checks. |
src/main/java/com/espacogeek/geek/controllers/BatchController.java |
Fixes the role name used to authorize stopping batch jobs. |
.gitignore |
Adds ignores for environment/secrets, node modules, logs, and local AIOS files. |
Comments suppressed due to low confidence (1)
src/main/java/com/espacogeek/geek/utils/DataJumpUtils.java:58
- The TMDB export URL is using plain HTTP. This allows MITM tampering of the gzipped JSON feed; switch to HTTPS for the
files.tmdb.orgexport endpoint if supported.
request = new Request.Builder()
.url(MessageFormat.format("http://files.tmdb.org/p/exports/{3}_ids_{0}_{1}_{2}.json.gz", month, day, year, type.getValue()))
.method("GET", null)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Response response = null; | ||
| try { | ||
| response = client.newCall(request).execute(); | ||
| if (!response.isSuccessful()) { | ||
| log.error("Failed to fetch TMDB daily export: {}", response.message()); | ||
| response.close(); | ||
| throw new com.espacogeek.geek.exception.RequestException(); | ||
| } | ||
| assert response.body() != null; | ||
| return new GZIPInputStream(response.body().byteStream()); | ||
| } catch (IOException e) { |
There was a problem hiding this comment.
getDataJumpTMDBStream returns a stream backed by an OkHttp Response, but the Response is never closed on the success path. This will leak connections/sockets unless the response is reliably closed when the returned stream is closed; consider wrapping the returned InputStream so close() also closes the OkHttp Response (and replace the assert response.body() != null with an explicit null check that throws a RequestException).
| } | ||
| jsonArray.add(jsonObject); | ||
| } catch (Exception e) { | ||
| log.error("Error reading TMDB array: {}", e.getMessage()); |
There was a problem hiding this comment.
getDataJumpTMDBArray is annotated with @Retryable(...RequestException.class) but the outer catch (Exception e) swallows failures and returns a partial/empty array, which prevents retries and hides errors from callers. Re-throw a RequestException (or let it propagate) after logging so retry semantics work as intended.
| log.error("Error reading TMDB array: {}", e.getMessage()); | |
| log.error("Error reading TMDB array", e); | |
| throw new com.espacogeek.geek.exception.RequestException(); |
| try (InputStream is = getDataJumpTMDBStream(type); | ||
| BufferedReader reader = new BufferedReader(new InputStreamReader(is))) { | ||
| String line; |
There was a problem hiding this comment.
new InputStreamReader(is) uses the platform default charset; JSON from TMDB exports is UTF-8. Specify StandardCharsets.UTF_8 to avoid parsing issues on non-UTF-8 default platforms.
| if (name == null && id == null || name == "" && id == null) { | ||
| return new MediaPage(); |
There was a problem hiding this comment.
This check uses name == "", which compares references rather than string content; empty (or whitespace-only) names may not be detected and will fall through to the service call unexpectedly. After trimming, prefer name.isEmpty()/name.isBlank() (and simplify to something like if (id == null && (name == null || name.isBlank()))).
| if (name == null && id == null || name == "" && id == null) { | ||
| return new MediaPage(); |
There was a problem hiding this comment.
This check uses name == "", which compares references rather than string content; empty (or whitespace-only) names may not be detected and will fall through to the service call unexpectedly. After trimming, prefer name.isEmpty()/name.isBlank() (and simplify to something like if (id == null && (name == null || name.isBlank()))).
| if (name == null && id == null || name == "" && id == null) { | ||
| return response; |
There was a problem hiding this comment.
This check uses name == "", which compares references rather than string content; empty (or whitespace-only) names may not be detected and will fall through to the service call unexpectedly. After trimming, prefer name.isEmpty()/name.isBlank() (and simplify to something like if (id == null && (name == null || name.isBlank()))).
| if (name == null && id == null || name == "" && id == null) { | ||
| return new MediaPage(); |
There was a problem hiding this comment.
This check uses name == "", which compares references rather than string content; empty (or whitespace-only) names may not be detected and will fall through to the service call unexpectedly. After trimming, prefer name.isEmpty()/name.isBlank() (and simplify to something like if (id == null && (name == null || name.isBlank()))).
| if (name == null && id == null || name == "" && id == null) { | ||
| return new MediaPage(); |
There was a problem hiding this comment.
This check uses name == "", which compares references rather than string content; empty (or whitespace-only) names may not be detected and will fall through to the service call unexpectedly. After trimming, prefer name.isEmpty()/name.isBlank() (and simplify to something like if (id == null && (name == null || name.isBlank()))).
| * or if the update date is null), <code>false</code> otherwise | ||
| */ | ||
| private static Boolean updateMediaWhenLastTimeUpdateMoreThanOneDay(MediaModel media) { | ||
| public static Boolean updateMediaWhenLastTimeUpdateMoreThanOneDay(MediaModel media) { |
There was a problem hiding this comment.
This method is now public and used as a boolean predicate, but it returns the boxed Boolean type. Prefer returning primitive boolean to avoid unnecessary boxing and the possibility of null-related issues as this becomes part of a wider API surface.
| public static Boolean updateMediaWhenLastTimeUpdateMoreThanOneDay(MediaModel media) { | |
| public static boolean updateMediaWhenLastTimeUpdateMoreThanOneDay(MediaModel media) { |
|
|
||
| import lombok.Getter; | ||
| import org.json.simple.JSONArray; | ||
| import org.json.simple.JSONObject; |
There was a problem hiding this comment.
org.json.simple.JSONObject is imported but no longer used in this file. In Java, unused imports fail compilation; remove the unused import (or reintroduce the usage if it was intended).
| import org.json.simple.JSONObject; |
No description provided.