fix(streaming): rewrite file streaming with NIO and HTTP caching#512
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces RandomAccessFile streaming with FileChannel transfer; adds ETag and Last-Modified headers, If-None-Match (304) and If-Range validation; changes HEAD semantics; improves error/disconnect handling; updates numeric filename comparison for very long digit sequences; refactors audiobook-player teardown, stale-request guards, and chapter caching. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Service as FileStreamingService
participant FS as FileSystem (FileChannel)
participant Response as HttpResponse
rect rgba(135,206,235,0.5)
Note over Client,Response: Conditional GET with If-None-Match
Client->>Service: GET (If-None-Match)
Service->>FS: read file metadata (size, lastModified)
Service->>Service: generate ETag
Service->>Service: compare If-None-Match
alt ETag matches
Service->>Response: 304 Not Modified
Response->>Client: 304 (no body)
else ETag does not match
Service->>FS: transfer file via FileChannel
Service->>Response: 200 OK + body
Response->>Client: full file
end
end
sequenceDiagram
participant Client
participant Service as FileStreamingService
participant FS as FileSystem (FileChannel)
participant Response as HttpResponse
rect rgba(144,238,144,0.5)
Note over Client,Response: Range Request with If-Range
Client->>Service: GET (Range) + If-Range
Service->>FS: read file metadata (size, lastModified)
Service->>Service: validate If-Range (strong ETag or HTTP-date)
alt If-Range matches
Service->>FS: transfer byte range via FileChannel
Service->>Response: 206 Partial Content
Response->>Client: requested range
else If-Range stale
Service->>FS: transfer full file via FileChannel
Service->>Response: 200 OK + full content
Response->>Client: full file
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
booklore-api/src/test/java/org/booklore/service/FileStreamingServiceTest.java (1)
493-497: Keep the expected ETag calculation independent from production code.This helper calls
fileStreamingService.generateETag(...), so a regression in the generator will keep the new ETag assertions green. Compute the expected value locally in the test or assert against a fixed fixture value instead.♻️ Proposed change
private String computeExpectedETag(Path file) throws IOException { var attrs = Files.readAttributes(file, BasicFileAttributes.class); - return fileStreamingService.generateETag( - attrs.size(), attrs.lastModifiedTime().toInstant()); + return "\"" + Long.toHexString(attrs.size()) + "-" + + Long.toHexString(attrs.lastModifiedTime().toInstant().toEpochMilli()) + "\""; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@booklore-api/src/test/java/org/booklore/service/FileStreamingServiceTest.java` around lines 493 - 497, The test helper computeExpectedETag currently calls production code fileStreamingService.generateETag(...) which masks regressions; change it so the test computes the expected ETag independently (do not call fileStreamingService.generateETag). Either hard-code the expected ETag for the fixture file or inline the same deterministic algorithm used by the production method (replicate the algorithm logic inside computeExpectedETag using attrs.size() and attrs.lastModifiedTime().toInstant() to build the ETag string/bytes and apply any hash/formatting the production method uses), and then assert against that local value instead of delegating to fileStreamingService.generateETag.booklore-api/src/main/java/org/booklore/service/FileStreamingService.java (1)
55-57: Please verify the strong-validator assumption behind this ETag.
validateIfRange()is usinggenerateETag()as a strong validator, but size + last-modified millis is only safe if this storage layer guarantees those values change on every byte change. If same-size rewrites can preserve that tuple, resumable downloads can splice bytes from a different revision. If the files are not immutable after ingest, hash content or emit a weak ETag and stop using it forIf-Range.Also applies to: 136-145, 182-188
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@booklore-api/src/main/java/org/booklore/service/FileStreamingService.java` around lines 55 - 57, validateIfRange() treats generateETag() as a strong validator, but the current ETag (size + lastModified) is only strong if the storage guarantees any byte change updates size or lastModified; verify that guarantee for FileStreamingService and if it does not hold, change generateETag() to compute a content hash (e.g., SHA-256 of file bytes) to produce a true strong ETag and keep validateIfRange() behavior, otherwise emit a weak ETag (prefix "W/") and update validateIfRange() logic to stop using it as a strong validator for If-Range/resumable downloads (or skip range validation when only weak ETags are available).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@booklore-api/src/main/java/org/booklore/service/FileStreamingService.java`:
- Around line 71-75: The current equality check etag.equals(ifNoneMatch) must be
replaced with proper parsing of the If-None-Match header: read
request.getHeader("If-None-Match"), if it equals "*" immediately set response to
SC_NOT_MODIFIED and return; otherwise split the header on commas, trim each
validator token, and for each token compare according to RFC rules—treat strong
comparison for methods other than GET/HEAD, and allow weak comparison for
GET/HEAD by normalizing tokens (strip an optional "W/" prefix) before comparing
to the existing etag variable; keep using
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED) and return when a match
is found.
- Around line 153-160: The JavaDoc for transferFile is misleading about
zero-copy/sendfile; update the comment for the method transferFile(FileChannel
source, long position, long count, OutputStream out) to remove or clearly
qualify claims that "Zero-copy transfer" or "Delegates to sendfile(2)" occur
when using Channels.newChannel(out); explain that wrapping the servlet
OutputStream with Channels.newChannel produces a WritableByteChannel which
usually prevents FileChannel.transferTo from using kernel sendfile/zero-copy (it
falls back to user-space ByteBuffer copies), and if you want zero-copy you must
target a SocketChannel directly; reference transferFile, FileChannel.transferTo,
Channels.newChannel, WritableByteChannel, and OutputStream in the updated
wording.
---
Nitpick comments:
In `@booklore-api/src/main/java/org/booklore/service/FileStreamingService.java`:
- Around line 55-57: validateIfRange() treats generateETag() as a strong
validator, but the current ETag (size + lastModified) is only strong if the
storage guarantees any byte change updates size or lastModified; verify that
guarantee for FileStreamingService and if it does not hold, change
generateETag() to compute a content hash (e.g., SHA-256 of file bytes) to
produce a true strong ETag and keep validateIfRange() behavior, otherwise emit a
weak ETag (prefix "W/") and update validateIfRange() logic to stop using it as a
strong validator for If-Range/resumable downloads (or skip range validation when
only weak ETags are available).
In
`@booklore-api/src/test/java/org/booklore/service/FileStreamingServiceTest.java`:
- Around line 493-497: The test helper computeExpectedETag currently calls
production code fileStreamingService.generateETag(...) which masks regressions;
change it so the test computes the expected ETag independently (do not call
fileStreamingService.generateETag). Either hard-code the expected ETag for the
fixture file or inline the same deterministic algorithm used by the production
method (replicate the algorithm logic inside computeExpectedETag using
attrs.size() and attrs.lastModifiedTime().toInstant() to build the ETag
string/bytes and apply any hash/formatting the production method uses), and then
assert against that local value instead of delegating to
fileStreamingService.generateETag.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2b9baef8-9f41-429d-b5b6-b247857f43e8
📒 Files selected for processing (2)
booklore-api/src/main/java/org/booklore/service/FileStreamingService.javabooklore-api/src/test/java/org/booklore/service/FileStreamingServiceTest.java
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test Suite / Backend Tests
- GitHub Check: Test Suite / Frontend Tests
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (2)
booklore-api/src/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
booklore-api/src/**/*.java: Use 4-space indentation in Java files and match surrounding Java style
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce@Autowiredfield injection
Use MapStruct for entity/DTO mapping
Files:
booklore-api/src/test/java/org/booklore/service/FileStreamingServiceTest.javabooklore-api/src/main/java/org/booklore/service/FileStreamingService.java
booklore-api/src/test/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Prefer focused unit tests; use
@SpringBootTestonly when the Spring context is required
Files:
booklore-api/src/test/java/org/booklore/service/FileStreamingServiceTest.java
🧠 Learnings (5)
📚 Learning: 2026-03-26T01:46:48.863Z
Learnt from: CR
Repo: grimmory-tools/grimmory PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T01:46:48.863Z
Learning: Applies to booklore-api/src/test/**/*.java : Prefer focused unit tests; use `SpringBootTest` only when the Spring context is required
Applied to files:
booklore-api/src/test/java/org/booklore/service/FileStreamingServiceTest.java
📚 Learning: 2026-04-10T08:15:37.436Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 449
File: booklore-api/src/main/java/org/booklore/service/book/BookDownloadService.java:139-145
Timestamp: 2026-04-10T08:15:37.436Z
Learning: When using Spring `ContentDisposition.builder(...).filename(name, StandardCharsets.UTF_8).build()` (i.e., explicitly providing UTF-8), the resulting header value should include both the quoted `filename="=?UTF-8?..."` and the RFC 5987 `filename*=` parameters. In this case, any extra ASCII fallback computation (e.g., deriving an ASCII `fallbackFilename` via `NON_ASCII_PATTERN` and calling `.filename(fallbackFilename)`) is likely redundant—prefer calling only `.filename(fallbackName?, StandardCharsets.UTF_8)` as appropriate and let Spring handle the UTF-8 header parameters. Verify by comparing the emitted header for `filename` and `filename*` before deciding to keep an ASCII fallback.
Applied to files:
booklore-api/src/test/java/org/booklore/service/FileStreamingServiceTest.javabooklore-api/src/main/java/org/booklore/service/FileStreamingService.java
📚 Learning: 2026-04-14T12:43:08.698Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 502
File: booklore-api/src/main/java/org/booklore/service/reader/ChapterCacheService.java:0-0
Timestamp: 2026-04-14T12:43:08.698Z
Learning: For this codebase (booklore-api), target Java 25 with `--enable-preview`, so `_` is intentionally used as an unnamed/ignored variable (e.g., lambda parameter or pattern variable) per Java’s preview feature JEP 456. Do not flag `_` in those contexts as an invalid/reserved identifier; only flag it if it’s used in a non-supported position (e.g., where an unnamed variable is not applicable for the Java preview rules).
Applied to files:
booklore-api/src/test/java/org/booklore/service/FileStreamingServiceTest.javabooklore-api/src/main/java/org/booklore/service/FileStreamingService.java
📚 Learning: 2026-04-13T05:02:01.463Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 486
File: booklore-api/src/main/java/org/booklore/service/oidc/OidcGroupMappingService.java:130-159
Timestamp: 2026-04-13T05:02:01.463Z
Learning: In grimmory-tools/grimmory, treat `permissionDemoUser`/`permission_demo_user` as a restricting “limited demo mode” flag: it must never be set to `true` as part of any “grant all permissions” or admin-grant flow. Specifically, in code paths that grant admin permissions (for example the OIDC `isAdmin` branch in `OidcGroupMappingService.applyPermissions`), ensure `permissionDemoUser` is intentionally excluded from the admin-permission block and cannot be turned on; enabling it for admin users is counterproductive.
Applied to files:
booklore-api/src/main/java/org/booklore/service/FileStreamingService.java
📚 Learning: 2026-04-14T12:46:56.673Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 502
File: booklore-api/src/main/java/org/booklore/service/reader/ChapterCacheService.java:0-0
Timestamp: 2026-04-14T12:46:56.673Z
Learning: For this project (grimmory-tools/grimmory / booklore-api), prefer modern Java 25 idioms and language/library features when reviewing or suggesting changes. Favor current constructs such as unnamed variables (e.g., `_`) where applicable, records, pattern matching for `switch`/`instanceof`, sealed classes, sequenced collections, `Stream.toList()`, and text blocks. Avoid proposing regressions to older/legacy equivalents unless there’s a concrete compatibility constraint (e.g., a required older runtime/library behavior).
Applied to files:
booklore-api/src/main/java/org/booklore/service/FileStreamingService.java
🔀 Multi-repo context grimmory-tools/grimmory-docs
Linked repositories findings
grimmory-tools/grimmory-docs
-
docs/integration/komga-api.md: documents download endpoints such as "GET /komga/api/v1/books/{bookId}/file" and discusses streaming/download behavior (partial-content use for Komga clients). This indicates consumer-facing documentation that assumes file-download/streaming semantics and may need alignment with changes to ETag/Range/If-Range behavior. [::grimmory-tools/grimmory-docs::docs/integration/komga-api.md]
-
docs/integration/opds.md: describes OPDS downloads and notes Komga-compatible API for streaming pages and full file downloads; references "GET /komga/api/v1/books/{id}/file". These docs surface expectations around downloading full files vs. streaming per-page (Komga) which relate to Range/206 behavior introduced in the PR. [::grimmory-tools/grimmory-docs::docs/integration/opds.md]
-
docs/metadata/metadata-center.md and docs/metadata/metadata-settings.md: mention "EPUB / PDF: Download the book file directly" and file handling/download features in the UI; user-facing docs may need small updates if header behavior (ETag/Cache-Control/HEAD responses) or error semantics (ApiError vs. sendError) should be reflected. [::grimmory-tools/grimmory-docs::docs/metadata/metadata-center.md] [::grimmory-tools/grimmory-docs::docs/metadata/metadata-settings.md]
-
docs/readers/* (epub-reader.md, pdf-reader.md, cbx-reader.md) and docs/integration/komga-api.md: multiple reader/integration docs describe streaming and downloading UX for various formats (EPUB, PDF, CBX). These are consumers of the file-serving behavior and could be affected by changes to partial content handling, HEAD responses, or conditional caching semantics. Examples: [::grimmory-tools/grimmory-docs::docs/readers/epub-reader.md] [::grimmory-tools/grimmory-docs::docs/readers/pdf-reader.md] [::grimmory-tools/grimmory-docs::docs/readers/cbx-reader.md]
Assessment
- The docs repo contains several references to file download/streaming endpoints and expected behaviors (Komga/OPDS). These are documentation consumers (not code) but indicate public-facing expectations that should be verified against the new FileStreamingService behavior (ETag generation, If-Range/If-None-Match handling, HEAD behavior, and error propagation). No other code repositories were available to search for direct code consumers.
🔇 Additional comments (1)
booklore-api/src/test/java/org/booklore/service/FileStreamingServiceTest.java (1)
177-294: Nice coverage of the new conditional-request branches.These cases exercise 304, HEAD,
If-Range, andLast-Modifiedbehavior without pulling up the Spring context.Based on learnings: Prefer focused unit tests; use
@SpringBootTestonly when the Spring context is required.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
booklore-api/src/main/java/org/booklore/service/reader/AudioFileUtilityService.java (1)
113-125:⚠️ Potential issue | 🟠 MajorComparator violates contract by returning 0 for different filenames, causing nondeterministic ordering.
After numeric chunks are consumed, the fallback at line 125 compares total original lengths rather than the remaining unconsumed portions. This allows distinct names to be marked equal; for example,
"01a"vs"1ab"both evaluate to length 3 and return 0 despite being different strings.Suggested fix
if (s1_norm.length() != s2_norm.length()) { - return s1_norm.length() - s2_norm.length(); + return Integer.compare(s1_norm.length(), s2_norm.length()); } int cmp = s1_norm.compareTo(s2_norm); if (cmp != 0) return cmp; @@ - return s1.length() - s2.length(); + int remainingCmp = Integer.compare(s1.length() - i1, s2.length() - i2); + if (remainingCmp != 0) return remainingCmp; + return s1.compareToIgnoreCase(s2);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@booklore-api/src/main/java/org/booklore/service/reader/AudioFileUtilityService.java` around lines 113 - 125, The comparator in AudioFileUtilityService currently returns s1.length() - s2.length() at the end, which compares entire original lengths and can return 0 for different filenames; change the final comparison to use the remaining unconsumed portions (e.g., compare s1.substring(i1).length() vs s2.substring(i2).length() or directly compare s1.substring(i1) vs s2.substring(i2) lexicographically) so that after the loop the comparator distinguishes strings based on their remaining characters; update the return in the comparator method to use these remaining indices (i1 and i2) instead of s1.length() and s2.length().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts`:
- Around line 91-92: getCurrentChapter()/getCurrentChapterIndex() are cached and
stay stale after programmatic seeks, so ensure you explicitly refresh the cached
chapter state by calling updateCurrentChapter() whenever you set currentTime
programmatically; specifically add updateCurrentChapter() after assigning
this.audiobookInfo in loadAudiobook(), after restoring savedPosition in
applyBookDetails(), after setting audio.currentTime / this.currentTime in
onAudioLoaded(), and in the Media Session seekto handler (and any other places
you set currentTime programmatically) so currentChapter/currentChapterIdx
reflect the new position immediately.
- Around line 133-138: When paramMap emits a new bookId, finish and cleanly tear
down the current book before overwriting this.bookId or loading the new one:
call the existing saveProgress() / end session logic for the active book, pause
and reset the component's audio element (e.g. this.audioEl.pause();
this.audioEl.currentTime = 0), remove any listeners tied to the old session,
await completion of those operations, then set this.bookId and call
loadAudiobook(bookId); also ensure resumeSession() invoked for the new book
cannot run against the old session (cancel/ignore prior promises). Implement
this teardown in the observable pipeline that currently maps/switchMaps to
loadAudiobook(bookId) (or inside loadAudiobook) so transitions from book A to B
always persist/end A before B is initialized.
- Around line 1004-1013: The createBookmark() and deleteBookmark() flows can
mutate bookmarks after switching books; update those methods (createBookmark,
deleteBookmark) to pipe their HTTP observables through
takeUntilDestroyed(this.destroyRef) and, in their subscribe handlers, first
verify the local bookId argument equals this.bookId before mutating
this.bookmarks or showing toasts—if it doesn't match, return early; this mirrors
the guard used in loadBookmarks(bookId) and prevents stale responses from
affecting the newly selected book view.
---
Outside diff comments:
In
`@booklore-api/src/main/java/org/booklore/service/reader/AudioFileUtilityService.java`:
- Around line 113-125: The comparator in AudioFileUtilityService currently
returns s1.length() - s2.length() at the end, which compares entire original
lengths and can return 0 for different filenames; change the final comparison to
use the remaining unconsumed portions (e.g., compare s1.substring(i1).length()
vs s2.substring(i2).length() or directly compare s1.substring(i1) vs
s2.substring(i2) lexicographically) so that after the loop the comparator
distinguishes strings based on their remaining characters; update the return in
the comparator method to use these remaining indices (i1 and i2) instead of
s1.length() and s2.length().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 47bb59f7-161f-454b-9a06-2aa48a7444cf
📒 Files selected for processing (3)
booklore-api/src/main/java/org/booklore/service/reader/AudioFileUtilityService.javabooklore-api/src/test/java/org/booklore/service/reader/AudioFileUtilityServiceTest.javafrontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
✅ Files skipped from review due to trivial changes (1)
- booklore-api/src/test/java/org/booklore/service/reader/AudioFileUtilityServiceTest.java
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test Suite / Backend Tests
- GitHub Check: Test Suite / Frontend Tests
- GitHub Check: Analyze (java-kotlin)
🧰 Additional context used
📓 Path-based instructions (4)
booklore-api/src/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
booklore-api/src/**/*.java: Use 4-space indentation in Java files and match surrounding Java style
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce@Autowiredfield injection
Use MapStruct for entity/DTO mapping
Files:
booklore-api/src/main/java/org/booklore/service/reader/AudioFileUtilityService.java
frontend/src/**/*.{ts,tsx,html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation in TypeScript, HTML, and SCSS files
Files:
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
frontend/src/app/**/*.component.ts
📄 CodeRabbit inference engine (AGENTS.md)
Keep Angular code on standalone components. Do not add NgModules
Files:
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
frontend/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/src/app/**/*.{ts,tsx}: Preferinject()over constructor injection
Followfrontend/eslint.config.js: component selectors useapp-*, directive selectors useapp*, andanyis disallowed
Files:
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
🧠 Learnings (10)
📚 Learning: 2026-04-10T08:15:37.436Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 449
File: booklore-api/src/main/java/org/booklore/service/book/BookDownloadService.java:139-145
Timestamp: 2026-04-10T08:15:37.436Z
Learning: When using Spring `ContentDisposition.builder(...).filename(name, StandardCharsets.UTF_8).build()` (i.e., explicitly providing UTF-8), the resulting header value should include both the quoted `filename="=?UTF-8?..."` and the RFC 5987 `filename*=` parameters. In this case, any extra ASCII fallback computation (e.g., deriving an ASCII `fallbackFilename` via `NON_ASCII_PATTERN` and calling `.filename(fallbackFilename)`) is likely redundant—prefer calling only `.filename(fallbackName?, StandardCharsets.UTF_8)` as appropriate and let Spring handle the UTF-8 header parameters. Verify by comparing the emitted header for `filename` and `filename*` before deciding to keep an ASCII fallback.
Applied to files:
booklore-api/src/main/java/org/booklore/service/reader/AudioFileUtilityService.java
📚 Learning: 2026-04-14T12:43:08.698Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 502
File: booklore-api/src/main/java/org/booklore/service/reader/ChapterCacheService.java:0-0
Timestamp: 2026-04-14T12:43:08.698Z
Learning: For this codebase (booklore-api), target Java 25 with `--enable-preview`, so `_` is intentionally used as an unnamed/ignored variable (e.g., lambda parameter or pattern variable) per Java’s preview feature JEP 456. Do not flag `_` in those contexts as an invalid/reserved identifier; only flag it if it’s used in a non-supported position (e.g., where an unnamed variable is not applicable for the Java preview rules).
Applied to files:
booklore-api/src/main/java/org/booklore/service/reader/AudioFileUtilityService.java
📚 Learning: 2026-04-13T05:02:01.463Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 486
File: booklore-api/src/main/java/org/booklore/service/oidc/OidcGroupMappingService.java:130-159
Timestamp: 2026-04-13T05:02:01.463Z
Learning: In grimmory-tools/grimmory, treat `permissionDemoUser`/`permission_demo_user` as a restricting “limited demo mode” flag: it must never be set to `true` as part of any “grant all permissions” or admin-grant flow. Specifically, in code paths that grant admin permissions (for example the OIDC `isAdmin` branch in `OidcGroupMappingService.applyPermissions`), ensure `permissionDemoUser` is intentionally excluded from the admin-permission block and cannot be turned on; enabling it for admin users is counterproductive.
Applied to files:
booklore-api/src/main/java/org/booklore/service/reader/AudioFileUtilityService.java
📚 Learning: 2026-04-14T12:46:56.673Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 502
File: booklore-api/src/main/java/org/booklore/service/reader/ChapterCacheService.java:0-0
Timestamp: 2026-04-14T12:46:56.673Z
Learning: For this project (grimmory-tools/grimmory / booklore-api), prefer modern Java 25 idioms and language/library features when reviewing or suggesting changes. Favor current constructs such as unnamed variables (e.g., `_`) where applicable, records, pattern matching for `switch`/`instanceof`, sealed classes, sequenced collections, `Stream.toList()`, and text blocks. Avoid proposing regressions to older/legacy equivalents unless there’s a concrete compatibility constraint (e.g., a required older runtime/library behavior).
Applied to files:
booklore-api/src/main/java/org/booklore/service/reader/AudioFileUtilityService.java
📚 Learning: 2026-04-14T22:10:58.270Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 513
File: booklore-api/src/main/java/org/booklore/service/reader/AudiobookReaderService.java:38-39
Timestamp: 2026-04-14T22:10:58.270Z
Learning: In grimmory-tools/grimmory, when refactoring `AudiobookReaderService`-like flows that combine a DB lookup with subsequent filesystem I/O, prefer a two-layer approach: extract the `BookFileEntity` (or equivalent entity) lookup into a dedicated Spring `Service` (e.g., `AudiobookEntityLoader`) and annotate that loader method with `Transactional(readOnly = true)`. This keeps the transaction proxy limited to the database access only, ensuring the filesystem I/O runs outside the persistence context. Do not use `TransactionTemplate` (Option A) for this pattern in this codebase.
Applied to files:
booklore-api/src/main/java/org/booklore/service/reader/AudioFileUtilityService.java
📚 Learning: 2026-04-07T09:28:09.587Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 393
File: frontend/src/app/features/readers/pdf-reader/pdf-reader.component.ts:255-263
Timestamp: 2026-04-07T09:28:09.587Z
Learning: In this Angular frontend (under frontend/src/app/), flag manual resource management/cleanup patterns when there is an Angular v16+ automatic alternative. Examples to prefer: (1) Instead of manually pairing document/window event listeners with stored cleanup functions (e.g., add/removeEventListener with mouseMoveCleanup/documentClickCleanup/keydownCleanup/touchCleanup fields), register teardown via DestroyRef.onDestroy(cleanupFn) (or equivalent Angular v16+ teardown mechanism). (2) Instead of storing Subscriptions in fields and explicitly unsubscribing in ngOnDestroy (e.g., annotationSaveSubscription/annotationCacheSubscription), use takeUntilDestroyed(destroyRef) (piped into the observable) or other Angular v16+ primitives. (3) If teardown is lifecycle-coupled and can be automated via DestroyRef/takeUntilDestroyed/signals (or other Angular v16+ mechanisms), prefer the automated approach over manual ngOnDestroy cleanup. Raise a review finding for the manual pattern and recommend the aut...
Applied to files:
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
📚 Learning: 2026-04-05T21:16:01.715Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 385
File: frontend/src/app/app.component.ts:55-56
Timestamp: 2026-04-05T21:16:01.715Z
Learning: When reviewing code in the Grimmory frontend (Angular), prefer modern Angular patterns. Specifically: (1) Prefer `DestroyRef` with `takeUntilDestroyed(destroyRef)` for teardown in Angular v16+ instead of manually tracking `Subscription` arrays and calling `unsubscribe()` in `ngOnDestroy()`. (2) Prefer `inject()` for dependency injection over constructor injection where appropriate. (3) Prefer Angular signals (e.g., `signal`, `computed`) over `BehaviorSubject`/`Observable` for state where signals/computed values fit the use case. Flag older patterns when they can be replaced with these modern equivalents without changing behavior.
Applied to files:
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
📚 Learning: 2026-04-04T14:03:28.295Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 372
File: frontend/src/app/features/book/service/app-books-api.service.ts:150-178
Timestamp: 2026-04-04T14:03:28.295Z
Learning: In `frontend/src/app/features/book/service/app-books-api.service.ts`, the `summaryToBook` function intentionally uses `as unknown as Book` to cast a partial object to the `Book` type. The returned object omits some required `Book` fields (e.g. `metadata.bookId`, `pdfProgress.page`) and includes an extra `bookFiles: []` property not in the `Book` interface. This is by design — consumers of `AppBooksApiService.books()` (e.g. `BookCardComponent`) only access the subset of properties that are populated from `AppBookSummary`, so no runtime issues occur. Do not flag this as a type safety error.
Applied to files:
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
📚 Learning: 2026-03-26T01:46:48.863Z
Learnt from: CR
Repo: grimmory-tools/grimmory PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T01:46:48.863Z
Learning: Applies to frontend/src/app/**/*.component.ts : Keep Angular code on standalone components. Do not add NgModules
Applied to files:
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
📚 Learning: 2026-04-11T03:55:57.229Z
Learnt from: zachyale
Repo: grimmory-tools/grimmory PR: 439
File: frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts:178-196
Timestamp: 2026-04-11T03:55:57.229Z
Learning: In this Angular frontend (frontend/src/app/), prefer the team’s reactive i18n “signal/computed” pattern: (1) For individual reactive translated strings, use `translateSignal()` from `jsverse/transloco`. (2) For option/label arrays that must update on language switch, create a single `activeLang` signal with `toSignal(t.langChanges$, { initialValue: t.getActiveLang() })`, then derive the arrays as `computed()` signals that read `activeLang()`. This should avoid manual `langChanges$` subscriptions and any `ngOnDestroy` subscription cleanup; prefer this over subscribing in `ngOnInit` when implementing reactive localization.
Applied to files:
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
🔀 Multi-repo context grimmory-tools/grimmory-docs
Linked repositories findings
[::grimmory-tools/grimmory-docs::docs/integration/komga-api.md]
- Docs advertise the Komga-compatible API and explicitly list GET /komga/api/v1/books/{bookId}/file as the endpoint used to "Download the full book file" and to "stream individual pages" for CBX files. This indicates Komga clients are documented/expected consumers of Grimmory's file-serving behavior.
[::grimmory-tools/grimmory-docs::docs/integration/opds.md]
- OPDS docs note the Komga API at /komga/api and again list GET /komga/api/v1/books/{id}/file. They explicitly call out that per-page streaming for CBX files works through the Komga API (clients rely on Range/partial requests).
- The docs present Komga/Tachiyomi/Mihon clients as consumers that expect streaming semantics.
[::grimmory-tools/grimmory-docs::docs/metadata/metadata-center.md]
- UI docs describe user actions "Stream" and "Download" in the Metadata Center's Files tab (EPUB/PDF: Stream opens streaming reader). Changes to HEAD/Range/ETag behavior could change what the UI or readers expect when "Stream" is used.
[::grimmory-tools/grimmory-docs::sidebars.ts]
- The Komga API and OPDS docs are surfaced in the documentation sidebar (integration category), confirming the endpoints are public-facing and likely relied upon by external clients.
Implication summary
- The PR's changes to ETag/If-None-Match/If-Range handling, HEAD semantics (now always 200 with Content-Length), and switch to throwing ApiException instead of sendError may affect Komga-compatible clients (Tachiyomi, Komelia, etc.) and documented behaviors (per-page streaming, Range/206 expectations) referenced in these docs. Verify Komga clients remain compatible with the new conditional-request and range handling and update docs/UI notes if behavior differs.
🔇 Additional comments (1)
booklore-api/src/main/java/org/booklore/service/reader/AudioFileUtilityService.java (1)
101-117: Nice improvement for very large numeric chunks.The normalized length + lexicographic compare removes parsing limits and correctly handles arbitrarily long digit sequences.
…obook player (again)
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts (2)
1171-1187: Consider adding error handling for bookmark deletion.The
deleteBookmarksubscription has no error handler. If the API call fails, the user won't receive any feedback. While not critical, adding error handling would improve UX consistency withaddBookmark.Suggested improvement
this.bookMarkService.deleteBookmark(bookmarkId) .pipe(takeUntilDestroyed(this.destroyRef)) - .subscribe(() => { + .subscribe({ + next: () => { if (requestId !== this.loadRequestId || requestBookId !== this.bookId) { return; } this.bookmarks = this.bookmarks.filter(b => b.id !== bookmarkId); this.messageService.add({ severity: 'info', summary: this.t.translate('readerAudiobook.toast.bookmarkDeleted') }); - }); + }, + error: () => { + if (requestId !== this.loadRequestId || requestBookId !== this.bookId) { + return; + } + this.messageService.add({ + severity: 'error', + summary: this.t.translate('common.error'), + detail: this.t.translate('readerAudiobook.toast.bookmarkDeleteFailed') + }); + } + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts` around lines 1171 - 1187, The deleteBookmark method lacks an error handler on the bookMarkService.deleteBookmark(...) subscription; update deleteBookmark to add an error callback in the subscribe so failures do not silently fail — on error do not remove from this.bookmarks and call this.messageService.add with a severity like 'error' and a translated summary (similar to addBookmark's UX) so users see failure feedback; ensure you still check requestId/requestBookId before mutating state in the success path.
1066-1091: Stale-request guards correctly implemented.The dual-guard pattern (
requestId !== this.loadRequestId || requestBookId !== this.bookId) properly prevents late responses from affecting state after a book switch.Minor inconsistency:
loadBookmarkscallscdr.markForCheck()after updating bookmarks, butaddBookmarkanddeleteBookmarkdo not. This works since the component uses default change detection, but consider addingmarkForCheck()for consistency if OnPush is ever adopted.,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts` around lines 1066 - 1091, Add change-detection consistency by calling this.cdr.markForCheck() after you mutate bookmarks in the audiobook-player component's addBookmark and deleteBookmark subscribe handlers: when next adds a bookmark (the block that does this.bookmarks = [...this.bookmarks, bookmark]) and when delete removes a bookmark (the block that splices/filters this.bookmarks), call this.cdr.markForCheck() immediately after updating the array so the component becomes consistent with loadBookmarks which already calls markForCheck().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts`:
- Around line 1171-1187: The deleteBookmark method lacks an error handler on the
bookMarkService.deleteBookmark(...) subscription; update deleteBookmark to add
an error callback in the subscribe so failures do not silently fail — on error
do not remove from this.bookmarks and call this.messageService.add with a
severity like 'error' and a translated summary (similar to addBookmark's UX) so
users see failure feedback; ensure you still check requestId/requestBookId
before mutating state in the success path.
- Around line 1066-1091: Add change-detection consistency by calling
this.cdr.markForCheck() after you mutate bookmarks in the audiobook-player
component's addBookmark and deleteBookmark subscribe handlers: when next adds a
bookmark (the block that does this.bookmarks = [...this.bookmarks, bookmark])
and when delete removes a bookmark (the block that splices/filters
this.bookmarks), call this.cdr.markForCheck() immediately after updating the
array so the component becomes consistent with loadBookmarks which already calls
markForCheck().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5f21707e-a569-4c43-9e79-4809c24b12e4
📒 Files selected for processing (1)
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test Suite / Backend Tests
🧰 Additional context used
📓 Path-based instructions (3)
frontend/src/**/*.{ts,tsx,html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation in TypeScript, HTML, and SCSS files
Files:
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
frontend/src/app/**/*.component.ts
📄 CodeRabbit inference engine (AGENTS.md)
Keep Angular code on standalone components. Do not add NgModules
Files:
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
frontend/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/src/app/**/*.{ts,tsx}: Preferinject()over constructor injection
Followfrontend/eslint.config.js: component selectors useapp-*, directive selectors useapp*, andanyis disallowed
Files:
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
🧠 Learnings (7)
📚 Learning: 2026-04-07T09:28:09.587Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 393
File: frontend/src/app/features/readers/pdf-reader/pdf-reader.component.ts:255-263
Timestamp: 2026-04-07T09:28:09.587Z
Learning: In this Angular frontend (under frontend/src/app/), flag manual resource management/cleanup patterns when there is an Angular v16+ automatic alternative. Examples to prefer: (1) Instead of manually pairing document/window event listeners with stored cleanup functions (e.g., add/removeEventListener with mouseMoveCleanup/documentClickCleanup/keydownCleanup/touchCleanup fields), register teardown via DestroyRef.onDestroy(cleanupFn) (or equivalent Angular v16+ teardown mechanism). (2) Instead of storing Subscriptions in fields and explicitly unsubscribing in ngOnDestroy (e.g., annotationSaveSubscription/annotationCacheSubscription), use takeUntilDestroyed(destroyRef) (piped into the observable) or other Angular v16+ primitives. (3) If teardown is lifecycle-coupled and can be automated via DestroyRef/takeUntilDestroyed/signals (or other Angular v16+ mechanisms), prefer the automated approach over manual ngOnDestroy cleanup. Raise a review finding for the manual pattern and recommend the aut...
Applied to files:
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
📚 Learning: 2026-04-05T21:16:01.715Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 385
File: frontend/src/app/app.component.ts:55-56
Timestamp: 2026-04-05T21:16:01.715Z
Learning: When reviewing code in the Grimmory frontend (Angular), prefer modern Angular patterns. Specifically: (1) Prefer `DestroyRef` with `takeUntilDestroyed(destroyRef)` for teardown in Angular v16+ instead of manually tracking `Subscription` arrays and calling `unsubscribe()` in `ngOnDestroy()`. (2) Prefer `inject()` for dependency injection over constructor injection where appropriate. (3) Prefer Angular signals (e.g., `signal`, `computed`) over `BehaviorSubject`/`Observable` for state where signals/computed values fit the use case. Flag older patterns when they can be replaced with these modern equivalents without changing behavior.
Applied to files:
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
📚 Learning: 2026-04-04T14:03:28.295Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 372
File: frontend/src/app/features/book/service/app-books-api.service.ts:150-178
Timestamp: 2026-04-04T14:03:28.295Z
Learning: In `frontend/src/app/features/book/service/app-books-api.service.ts`, the `summaryToBook` function intentionally uses `as unknown as Book` to cast a partial object to the `Book` type. The returned object omits some required `Book` fields (e.g. `metadata.bookId`, `pdfProgress.page`) and includes an extra `bookFiles: []` property not in the `Book` interface. This is by design — consumers of `AppBooksApiService.books()` (e.g. `BookCardComponent`) only access the subset of properties that are populated from `AppBookSummary`, so no runtime issues occur. Do not flag this as a type safety error.
Applied to files:
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
📚 Learning: 2026-03-24T18:46:47.249Z
Learnt from: pedronave
Repo: grimmory-tools/grimmory PR: 173
File: booklore-api/src/main/java/org/booklore/service/hardcover/HardcoverSyncService.java:188-199
Timestamp: 2026-03-24T18:46:47.249Z
Learning: In grimmory-tools/grimmory (HardcoverSyncService.java), when a `hardcoverBookId` is stored in book metadata but is malformed (non-numeric), the preferred behavior is to return null and skip the sync rather than falling back to ISBN-based lookup. The reasoning is that an explicitly set bookId represents deliberate user intent, and silently resolving via ISBN could sync progress to a different book than intended.
Applied to files:
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
📚 Learning: 2026-04-04T15:36:56.558Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 372
File: booklore-api/src/main/java/org/booklore/app/service/AppBookService.java:357-359
Timestamp: 2026-04-04T15:36:56.558Z
Learning: In `booklore-api/src/main/java/org/booklore/app/service/AppBookService.java`, `shelfId` and `magicShelfId` are mutually exclusive navigation contexts in `getFilterOptions`. A request will never supply both at the same time, so the `else if (shelfId != null)` branch after the `magicBookIds != null` check is intentional and correct — there is no missing shelf-∩-magicShelf intersection case. Do not flag this pattern as a bug.
Applied to files:
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
📚 Learning: 2026-03-26T01:46:48.863Z
Learnt from: CR
Repo: grimmory-tools/grimmory PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T01:46:48.863Z
Learning: Applies to frontend/src/app/**/*.component.ts : Keep Angular code on standalone components. Do not add NgModules
Applied to files:
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
📚 Learning: 2026-04-11T03:55:57.229Z
Learnt from: zachyale
Repo: grimmory-tools/grimmory PR: 439
File: frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts:178-196
Timestamp: 2026-04-11T03:55:57.229Z
Learning: In this Angular frontend (frontend/src/app/), prefer the team’s reactive i18n “signal/computed” pattern: (1) For individual reactive translated strings, use `translateSignal()` from `jsverse/transloco`. (2) For option/label arrays that must update on language switch, create a single `activeLang` signal with `toSignal(t.langChanges$, { initialValue: t.getActiveLang() })`, then derive the arrays as `computed()` signals that read `activeLang()`. This should avoid manual `langChanges$` subscriptions and any `ngOnDestroy` subscription cleanup; prefer this over subscribing in `ngOnInit` when implementing reactive localization.
Applied to files:
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
🔀 Multi-repo context grimmory-tools/grimmory-docs
grimmory-tools/grimmory-docs
-
Komga API and per-page streaming are explicitly documented; clients are noted to rely on per-page streaming and Range semantics. Relevant doc: docs/integration/komga-api.md (Komga overview, endpoints, "Stream individual pages" / "Stream pages without downloading entire files"). [::grimmory-tools/grimmory-docs::docs/integration/komga-api.md:lines ~1-120, endpoints section]
-
Komga-specific endpoint for downloading/streaming full book files is documented: GET /komga/api/v1/books/{id}/file — consumers may expect existing Range/partial-content behavior. [::grimmory-tools/grimmory-docs::docs/integration/komga-api.md:endpoints section]
-
OPDS docs call out that per-page streaming for CBX files only works through the Komga API (/komga/api/) and that Komga clients (Tachiyomi/Mihon/Komelia) are consumers — these clients depend on Range and partial-response semantics. Changes to Range/HEAD/ETag behavior could affect compatibility. [::grimmory-tools/grimmory-docs::docs/integration/opds.md:paragraphs describing Komga compatibility and CBX streaming]
-
UI/docs describe "Stream" vs "Download" actions in the Metadata Center Files tab (Stream opens streaming reader; Download downloads file). Changes to HEAD behavior, Content-Length, ETag, or conditional responses may alter the streaming UX or how clients choose between streaming vs download. Relevant doc: docs/metadata/metadata-center.md (Files section, actions per file). [::grimmory-tools/grimmory-docs::docs/metadata/metadata-center.md:Files section]
Conclusion: documentation shows public/third‑party consumers (Komga-compatible clients) expect Range/per-page streaming via /komga/api/* and may be sensitive to changes in Range/HEAD/ETag semantics. Recommend verifying the rewritten FileStreamingService preserves per-page Range behavior and the documented streaming endpoints' contract; update docs if HEAD/response semantics changed.
🔇 Additional comments (5)
frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts (5)
1-16: Good adoption of modern Angular patterns.The migration to
DestroyRefwithtakeUntilDestroyed()and consistent use ofinject()for all dependencies aligns well with Angular v16+ best practices. TheloadRequestIdcounter for staleness tracking is a solid pattern for handling route-reuse scenarios.Also applies to: 56-69
161-175: Clean teardown before book switch.The previous-book teardown logic properly saves progress, pauses/resets the audio element, and ends the session before loading the new book. This prevents leftover state from the previous book affecting the new one.
360-383: Chapter caching logic is sound.The centralized
updateCurrentChapter()method handles edge cases well: no chapters, position before first chapter, and position past the last chapter's boundary. The fallback search from end (lines 372-379) correctly handles positions beyond a chapter'sendTimeMs.
1025-1035: Bookmark load with proper staleness guard.The guard at line 1029 correctly prevents stale bookmark responses from updating the UI after a book switch. The
cdr.markForCheck()ensures the view updates correctly.
292-310: State reset is comprehensive.The
resetState()method correctly invalidates all cached state including the chapter cache (currentChapterIdx = -1). This ensures no stale state persists between book loads.
# Conflicts: # booklore-api/src/main/java/org/booklore/service/reader/AudioFileUtilityService.java # frontend/src/app/features/readers/audiobook-player/audiobook-player.component.ts
…mmory-tools#512) * fix(streaming): rewrite file streaming with NIO and HTTP caching * fix(streaming): improve file transfer handling and enhance client disconnect detection * fix(streaming): improve If-None-Match header evaluation for improved caching * fix(streaming): improve chapter tracking and loading in audiobook player * fix(streaming): remove unused imports in audiobook player component * fix(streaming): improve chapter tracking and state management in audiobook player (again) * fix: fix merge conflict errors and update code
…mmory-tools#512) * fix(streaming): rewrite file streaming with NIO and HTTP caching * fix(streaming): improve file transfer handling and enhance client disconnect detection * fix(streaming): improve If-None-Match header evaluation for improved caching * fix(streaming): improve chapter tracking and loading in audiobook player * fix(streaming): remove unused imports in audiobook player component * fix(streaming): improve chapter tracking and state management in audiobook player (again) * fix: fix merge conflict errors and update code
* fix(streaming): rewrite file streaming with NIO and HTTP caching * fix(streaming): improve file transfer handling and enhance client disconnect detection * fix(streaming): improve If-None-Match header evaluation for improved caching * fix(streaming): improve chapter tracking and loading in audiobook player * fix(streaming): remove unused imports in audiobook player component * fix(streaming): improve chapter tracking and state management in audiobook player (again) * fix: fix merge conflict errors and update code
* fix(streaming): rewrite file streaming with NIO and HTTP caching * fix(streaming): improve file transfer handling and enhance client disconnect detection * fix(streaming): improve If-None-Match header evaluation for improved caching * fix(streaming): improve chapter tracking and loading in audiobook player * fix(streaming): remove unused imports in audiobook player component * fix(streaming): improve chapter tracking and state management in audiobook player (again) * fix: fix merge conflict errors and update code
…mmory-tools#512) * fix(streaming): rewrite file streaming with NIO and HTTP caching * fix(streaming): improve file transfer handling and enhance client disconnect detection * fix(streaming): improve If-None-Match header evaluation for improved caching * fix(streaming): improve chapter tracking and loading in audiobook player * fix(streaming): remove unused imports in audiobook player component * fix(streaming): improve chapter tracking and state management in audiobook player (again) * fix: fix merge conflict errors and update code
Description
Linked Issue: Fixes #
Changes
This pull request refactors and improves the
FileStreamingServicefor robust, efficient, and standards-compliant HTTP file streaming with range and conditional request support. It introduces zero-copy file transfer using Java NIO, adds strong ETag and last-modified handling, improves error handling, and updates tests to match the new exception-based error reporting.File streaming enhancements:
FileChannelandWritableByteChannel, enabling more efficient file serving and leveraging OS-level optimizations. (FileStreamingService.javaIf-None-Match/If-Rangeheaders, allowing for conditional requests, proper caching, and partial content delivery per RFC 7233. (FileStreamingService.javaFileStreamingService.javaError handling and API consistency:
APIExceptionusage, ensuring consistent error handling and propagation throughout the service. (FileStreamingService.javaSummary by CodeRabbit
New Features
Bug Fixes
Tests