refactor(frontend): custom font files caching

[NamLe029]

Description

Utilize persistent CacheStorage to cache font files on client-side, which may slightly improve load time.

Changes

• Use CacheStorage to cache font files.
• Include Last-Modified header in server response.
• Pass authentication token by Authorizatrion header (used to be passed as a URL parameter).

Notes

• Must be merged after refactor(SecurityFilter): Stop filters from being registered on all routes #361, as Authorizatrion header authentication for custom font is only supported until then

Summary by CodeRabbit

• New Features

  • Optional custom-font caching: when enabled, fonts load from local cache for faster startup; when disabled, fonts are fetched on demand.

• Improvements

  • Font downloads now set clearer HTTP cache and content headers (including Last-Modified) to improve client handling.

• Frontend

  • Font loading uses blob-derived object URLs for consistent rendering across cache/no-cache modes.

• Tests

  • Updated tests to cover the new cache-dependent font-loading behavior.

[coderabbitai]

♻️ Duplicate comments (1)

frontend/src/app/shared/service/custom-font.service.ts (1)

110-118: ⚠️ Potential issue | 🟡 Minor

Revoke temporary blob URLs to avoid leaking browser memory.

URL.createObjectURL(...) is used, but the created URL is never released. Please revoke it in a finally block after FontFace.load()/add flow.

Proposed fix

   async loadFontFace(font: CustomFont): Promise<void> {
     if (this.loadedFonts.has(font.fontName)) {
       return;
     }

+    let objectUrl: string | null = null;
     try {
-      let fontUrl = this.getFontUrl(font.id);
+      const fontUrl = this.getFontUrl(font.id);

       if (this.localSettingsService.get().cacheStorageEnabled) {
         const cachedBlob = await (await this.cacheStorageService.getCache(fontUrl)).blob();
-        fontUrl = URL.createObjectURL(cachedBlob);
+        objectUrl = URL.createObjectURL(cachedBlob);
       } else {
         const freshBlob = await firstValueFrom(this.http.get(fontUrl, {responseType: 'blob'}));
-        fontUrl = URL.createObjectURL(freshBlob);
+        objectUrl = URL.createObjectURL(freshBlob);
       }

       const fontFace = new FontFace(
         font.fontName,
-        `url(${fontUrl})`,
+        `url(${objectUrl})`,
         {
           weight: 'normal',
           style: 'normal'
         }
       );

       await fontFace.load();
       document.fonts.add(fontFace);
       this.loadedFonts.add(font.fontName);
     } catch (error) {
       console.error(`Failed to load font ${font.fontName}:`, error);
       throw error;
+    } finally {
+      if (objectUrl) {
+        URL.revokeObjectURL(objectUrl);
+      }
     }
   }

#!/bin/bash
# Verify object URL creation/revocation pairing in this service.
rg -n -C2 'createObjectURL|revokeObjectURL' frontend/src/app/shared/service/custom-font.service.ts

Also applies to: 122-131

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/app/shared/service/custom-font.service.ts` around lines 110 -
118, The code creates temporary blob URLs via URL.createObjectURL in
custom-font.service.ts (see getFontUrl usage and branches that call
cacheStorageService.getCache(...) and http.get(...)), but never revokes them;
wrap the FontFace.load()/font addition flow in a try/finally and store the
created object URL in a local variable (e.g., tempFontUrl) so that in finally
you call URL.revokeObjectURL(tempFontUrl) if it was created; ensure you only
revoke URLs you created (skip revoking the original remote URL string) and
handle both branches (cachedBlob and freshBlob) so every createObjectURL call
has a matching revoke.

🧹 Nitpick comments (1)

booklore-api/src/main/java/org/booklore/controller/CustomFontController.java (1)

78-80: Avoid double lookup for the same font in one request.

Line 78 and Line 79 currently trigger separate service/repository reads for identical (fontId, userId). Consolidating into one service call reduces DB load and removes a small inconsistency window.

♻️ Suggested direction

- File fontFile = customFontService.getFontFile(fontId, user.getId());
- FontFormat format = customFontService.getFontFormat(fontId, user.getId());
+ var fontResource = customFontService.getFontResource(fontId, user.getId());
+ File fontFile = fontResource.file();
+ FontFormat format = fontResource.format();

🤖 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/controller/CustomFontController.java`
around lines 78 - 80, CustomFontController currently calls
customFontService.getFontFile(fontId, user.getId()) and
customFontService.getFontFormat(fontId, user.getId()) separately causing
duplicate lookups; change the service to return both file and format in a single
call (e.g., add a DTO or pair) and replace the two calls with one call like
customFontService.getFontResource(fontId, user.getId()) that returns {file,
format}, then use the returned file for
FileUtils.getFileLastModified(fontFile.toPath()) and the returned format
(FontFormat) where needed to eliminate the double lookup and any inconsistency
window.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@frontend/src/app/shared/service/custom-font.service.ts`:
- Around line 110-118: The code creates temporary blob URLs via
URL.createObjectURL in custom-font.service.ts (see getFontUrl usage and branches
that call cacheStorageService.getCache(...) and http.get(...)), but never
revokes them; wrap the FontFace.load()/font addition flow in a try/finally and
store the created object URL in a local variable (e.g., tempFontUrl) so that in
finally you call URL.revokeObjectURL(tempFontUrl) if it was created; ensure you
only revoke URLs you created (skip revoking the original remote URL string) and
handle both branches (cachedBlob and freshBlob) so every createObjectURL call
has a matching revoke.

---

Nitpick comments:
In
`@booklore-api/src/main/java/org/booklore/controller/CustomFontController.java`:
- Around line 78-80: CustomFontController currently calls
customFontService.getFontFile(fontId, user.getId()) and
customFontService.getFontFormat(fontId, user.getId()) separately causing
duplicate lookups; change the service to return both file and format in a single
call (e.g., add a DTO or pair) and replace the two calls with one call like
customFontService.getFontResource(fontId, user.getId()) that returns {file,
format}, then use the returned file for
FileUtils.getFileLastModified(fontFile.toPath()) and the returned format
(FontFormat) where needed to eliminate the double lookup and any inconsistency
window.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 94e48418-e4dc-4981-a483-7c494bb88d5e

📥 Commits

Reviewing files that changed from the base of the PR and between 6246cca and 0d28ee0.

📒 Files selected for processing (5)

• booklore-api/src/main/java/org/booklore/controller/CustomFontController.java
• booklore-api/src/main/java/org/booklore/service/customfont/CustomFontService.java
• booklore-api/src/test/java/org/booklore/service/customfont/CustomFontServiceTest.java
• frontend/src/app/features/readers/ebook-reader/features/fonts/custom-font.service.ts
• frontend/src/app/shared/service/custom-font.service.ts

✅ Files skipped from review due to trivial changes (1)

• frontend/src/app/features/readers/ebook-reader/features/fonts/custom-font.service.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 (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 @Autowired field injection
Use MapStruct for entity/DTO mapping

Files:

• booklore-api/src/main/java/org/booklore/controller/CustomFontController.java
• booklore-api/src/main/java/org/booklore/service/customfont/CustomFontService.java
• booklore-api/src/test/java/org/booklore/service/customfont/CustomFontServiceTest.java

booklore-api/src/test/**/*.java

📄 CodeRabbit inference engine (AGENTS.md)

Prefer focused unit tests; use @SpringBootTest only when the Spring context is required

Files:

• booklore-api/src/test/java/org/booklore/service/customfont/CustomFontServiceTest.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/shared/service/custom-font.service.ts

frontend/src/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

frontend/src/app/**/*.{ts,tsx}: Prefer inject() over constructor injection
Follow frontend/eslint.config.js: component selectors use app-*, directive selectors use app*, and any is disallowed

Files:

• frontend/src/app/shared/service/custom-font.service.ts

🧠 Learnings (2)

📚 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/shared/service/custom-font.service.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/shared/service/custom-font.service.ts

🔇 Additional comments (3)

frontend/src/app/shared/service/custom-font.service.ts (1)

3-3: Nice split between cache-backed and network-backed blob loading.

The cacheStorageEnabled branch and inject()-based dependency wiring are clean and consistent with the existing service design.

Also applies to: 8-9, 22-23, 112-118

booklore-api/src/main/java/org/booklore/service/customfont/CustomFontService.java (1)

157-177: Return-type refactor is consistent and safe here.

Line 157 and Line 177 correctly switch this method to return File while preserving authorization, path validation, and on-disk existence checks.

booklore-api/src/test/java/org/booklore/service/customfont/CustomFontServiceTest.java (1)

305-310: Test update correctly follows the new service contract.

The assertion now validates the returned File path directly, which aligns with the refactored getFontFile return type.

[coderabbitai]

Actionable comments posted: 1

🤖 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/ebook-reader/features/fonts/custom-font.service.spec.ts`:
- Around line 47-50: The tests currently stub localSettingsService.get() to
always return {cacheStorageEnabled: false}, so add at least one test case that
stubs localSettingsService.get() to return {cacheStorageEnabled: true} and
asserts the cache-enabled behavior of CustomFontService (or the test subject in
custom-font.service.spec.ts); specifically, update the localSettingsService mock
used by the suite to return true within this new test, invoke the relevant
method(s) that should take the cache path (e.g., the method that reads/writes
font data), and assert that the caching functions are called/used (or that the
cache-specific side effects occur) rather than the non-cache path.

🪄 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: 35f8ee99-8b86-4602-a2ce-40514bd8fae1

📥 Commits

Reviewing files that changed from the base of the PR and between 0d28ee0 and baf9bfd.

📒 Files selected for processing (1)

• frontend/src/app/features/readers/ebook-reader/features/fonts/custom-font.service.spec.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). (4)

• GitHub Check: Test Suite / Backend Tests
• GitHub Check: Test Suite / Frontend Tests
• GitHub Check: Analyze (javascript-typescript)
• GitHub Check: Analyze (java-kotlin)

🧰 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/ebook-reader/features/fonts/custom-font.service.spec.ts

frontend/src/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

frontend/src/app/**/*.{ts,tsx}: Prefer inject() over constructor injection
Follow frontend/eslint.config.js: component selectors use app-*, directive selectors use app*, and any is disallowed

Files:

• frontend/src/app/features/readers/ebook-reader/features/fonts/custom-font.service.spec.ts

frontend/src/**/*.{test,spec}.ts

📄 CodeRabbit inference engine (AGENTS.md)

Use Vitest for tests in the frontend

Files:

• frontend/src/app/features/readers/ebook-reader/features/fonts/custom-font.service.spec.ts

🧠 Learnings (2)

📚 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/ebook-reader/features/fonts/custom-font.service.spec.ts

📚 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/ebook-reader/features/fonts/custom-font.service.spec.ts

🔇 Additional comments (1)

frontend/src/app/features/readers/ebook-reader/features/fonts/custom-font.service.spec.ts (1)

9-9: Good dependency alignment in TestBed setup.

Adding LocalSettingsService to the test module keeps this spec aligned with the new service dependency.

Also applies to: 61-61

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Add coverage for cacheStorageEnabled: true path.

get() is hardcoded to { cacheStorageEnabled: false }, so this suite only validates the non-cache flow. The new cache-enabled branch should have at least one dedicated test that sets cacheStorageEnabled: true and asserts the cache path behavior.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/features/readers/ebook-reader/features/fonts/custom-font.service.spec.ts`
around lines 47 - 50, The tests currently stub localSettingsService.get() to
always return {cacheStorageEnabled: false}, so add at least one test case that
stubs localSettingsService.get() to return {cacheStorageEnabled: true} and
asserts the cache-enabled behavior of CustomFontService (or the test subject in
custom-font.service.spec.ts); specifically, update the localSettingsService mock
used by the suite to return true within this new test, invoke the relevant
method(s) that should take the cache path (e.g., the method that reads/writes
font data), and assert that the caching functions are called/used (or that the
cache-specific side effects occur) rather than the non-cache path.

[zachyale]

@NamLe029 The functional changes in the PR look good, but specs are failing and need to be updated for closer compliance with your new implementation of EpubCustomFontService and the HttpClient + header/cache approach.

Let me know when specs have been addressed and we can get this in 👍