refactor: unify book cache clearing for epub, txt, and xtc files by WuTofu · Pull Request #1875 · crosspoint-reader/crosspoint-reader

WuTofu · 2026-05-08T13:52:29Z

Summary

What is the goal of this PR?
Implements unified book cache invalidation method for text, epub, and xtc files content paths to ensure cache is cleared whenever files are uploaded, renamed, moved, or deleted.
What changes are included?
- Added Txt::clearCache() support in Txt.cpp / Txt.h
- Added clearBookCache() helper in BookCacheUtils.cpp / BookCacheUtils.h
- Replaced EPUB-specific cache clearing calls with the generic helper in:
  - OpdsBookBrowserActivity.cpp
  - CrossPointWebServer.cpp
  - WebDAVHandler.cpp
- Extended settings cache cleanup in ClearCacheActivity.cpp to handle txt_ cache directories as well

Additional Context

I will work on FileBrowserActivity.cpp as part of #1803's follow-up after this pr.

AI Usage

While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it
helps set the right context for reviewers.

Did you use AI tools to help write this code? PARTIALLY

coderabbitai · 2026-05-08T13:52:44Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 951b6b21-937a-41de-acc4-e847e1c2ba51

📥 Commits

Reviewing files that changed from the base of the PR and between d7ebb9a and 4c9cd3d.

📒 Files selected for processing (3)

src/activities/settings/ClearCacheActivity.cpp
src/util/BookCacheUtils.cpp
src/util/BookCacheUtils.h

🚧 Files skipped from review as they are similar to previous changes (2)

src/util/BookCacheUtils.cpp
src/util/BookCacheUtils.h

📜 Recent 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: cppcheck
GitHub Check: build
GitHub Check: clang-format

🧰 Additional context used

🧠 Learnings (5)

📚 Learning: 2026-02-23T06:18:08.408Z

Learnt from: raygan
Repo: crosspoint-reader/crosspoint-reader PR: 1116
File: src/activities/settings/WallabagSettingsActivity.cpp:57-75
Timestamp: 2026-02-23T06:18:08.408Z
Learning: When navigating from a settings activity to KeyboardEntryActivity (e.g., KOReaderSettingsActivity, WallabagSettingsActivity), call exitActivity() on the parent before calling enterNewActivity(new KeyboardEntryActivity(...)). The KeyboardEntryActivity should then call exitActivity() via its callbacks to return to the parent. Note: ConfirmationActivity does not require exitActivity() before entering KeyboardEntryActivity. Apply this pattern to similar settings activities in this module.

Applied to files:

src/activities/settings/ClearCacheActivity.cpp

📚 Learning: 2026-02-27T22:49:59.600Z

Learnt from: ngxson
Repo: crosspoint-reader/crosspoint-reader PR: 1218
File: src/activities/ActivityManager.cpp:254-265
Timestamp: 2026-02-27T22:49:59.600Z
Learning: In this codebase, assertions are always enabled (no NDEBUG). Use assert() to crash on programmer errors and surface logic bugs during development and in production builds. Do not rely on asserts for runtime error handling; they should enforce invariants that must always hold. Keep asserts side-effect free and inexpensive, and avoid relying on them for user-visible failures. Include <cassert> where appropriate and document the invariant being tested.

Applied to files:

src/activities/settings/ClearCacheActivity.cpp

📚 Learning: 2026-03-02T10:14:16.036Z

Learnt from: Uri-Tauber
Repo: crosspoint-reader/crosspoint-reader PR: 1245
File: lib/Epub/Epub/Section.cpp:277-308
Timestamp: 2026-03-02T10:14:16.036Z
Learning: Guideline: Strengthen serialization::readString to defend against unbounded growth when reading from disk data. Implement and enforce a maximum allowed length (e.g., a configured or reasonable constant) and validate the incoming length before resizing or allocating. Audit all call sites (e.g., BookMetadataCache, TextBlock, KOReaderCredentialStore, Section cache readers) to ensure they do not rely on unbounded len-based resizing. If the readString API must remain, add internal safeguards (bounds checks, length validation, and error handling) so per-call-site validations are not required. Ensure Section cache files remain versioned (SECTION_FILE_VERSION) and parameter mismatches invalidate caches, but do not rely on unsafe allocations; prefer safe, bounded reads with explicit errors on invalid data.

Applied to files:

src/activities/settings/ClearCacheActivity.cpp

📚 Learning: 2026-03-28T11:06:29.611Z

Learnt from: pablohc
Repo: crosspoint-reader/crosspoint-reader PR: 1488
File: src/activities/home/HomeActivity.cpp:92-95
Timestamp: 2026-03-28T11:06:29.611Z
Learning: When reviewing crosspoint-reader code, avoid flagging a missing `renderer.displayBuffer()` call immediately after `GUI.drawPopup()` / `BaseTheme::drawPopup()`: `BaseTheme::drawPopup()` already calls `renderer.displayBuffer()` before returning, so the popup is guaranteed to be flushed to the e-ink panel before subsequent blocking work begins. Conversely, do not require a `renderer.displayBuffer()` call after `fillPopupProgress()`; it intentionally does not flush, so intermediate progress-bar updates may not appear unless the update granularity warrants an explicit flush elsewhere.

Applied to files:

src/activities/settings/ClearCacheActivity.cpp

📚 Learning: 2026-04-12T12:28:33.205Z

Learnt from: Uri-Tauber
Repo: crosspoint-reader/crosspoint-reader PR: 1629
File: src/activities/home/HomeActivity.cpp:119-120
Timestamp: 2026-04-12T12:28:33.205Z
Learning: When reviewing code in this repository (C/C++ sources), set review comment severity according to this policy:
- Use **Major** (🟠) only for defects with realistic risk of crash, out-of-memory (OOM), invalid pointer dereference, data corruption, or other severe issues that are unlikely to be caught in casual/manual device testing.
- Use **Minor** or informational for UX gaps, logic edge-cases, style issues, or missing feature completeness that the author can verify (or has verified) through normal device use.
- Do **not** escalate severity to Major based on behavioral/UX observations alone; assume the author has already tested the feature on their own device and only treat issues as Major if they match the high-risk defect categories above.

Applied to files:

src/activities/settings/ClearCacheActivity.cpp

🔇 Additional comments (1)

src/activities/settings/ClearCacheActivity.cpp (1)

11-11: Nice refactor to centralized cache-name matching.

Switching to isBookCacheDirectoryName(...) keeps this activity aligned with the shared cache policy and cleanly enables TXT cache cleanup without duplicating prefix logic.

Also applies to: 98-99

📝 Walkthrough

Walkthrough

Adds Txt::clearCache(), introduces a unified clearBookCache(path) dispatcher and isBookCacheDirectoryName(), and updates browser, settings, CrossPointWebServer, and WebDAV handlers to call the new API instead of EPUB-only cache-clearing helpers.

Changes

Unified Book Cache Clearing Refactor

Layer / File(s)	Summary
Public API Contract `src/util/BookCacheUtils.h`	New header declares `clearBookCache(path)` and `isBookCacheDirectoryName()` for EPUB/XTC/TXT cache handling.
Backend Implementations `lib/Txt/Txt.h`, `lib/Txt/Txt.cpp`, `src/util/BookCacheUtils.cpp`	TXT backend adds `clearCache()`; BookCacheUtils routes by extension to Epub/Xtc/Txt and calls their `clearCache()`.
OpdsBookBrowserActivity Integration `src/activities/browser/OpdsBookBrowserActivity.cpp`	Replaces `Epub(...).clearCache()` with `clearBookCache(filename)` and updates includes.
ClearCacheActivity Expansion `src/activities/settings/ClearCacheActivity.cpp`	Uses `isBookCacheDirectoryName()` to include `txt_` directories when purging `/.crosspoint` subdirectories.
CrossPointWebServer Integration `src/network/CrossPointWebServer.cpp`	Removes EPUB-only helper; replaces upload/rename/move/delete/WebSocket cache invalidation calls with `clearBookCache()`.
WebDAVHandler Integration `src/network/WebDAVHandler.cpp`, `src/network/WebDAVHandler.h`	PUT, DELETE, MOVE handlers now call `clearBookCache()`; the old `clearEpubCacheIfNeeded` declaration and implementation were removed.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Server
  participant BookCacheUtils
  participant Backend
  Client->>Server: upload/rename/move/delete request
  Server->>BookCacheUtils: clearBookCache(path)
  BookCacheUtils->>Backend: select backend by extension and call clearCache()
  Backend-->>BookCacheUtils: result
  BookCacheUtils-->>Server: done

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

ngxson
jdk2pq

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title accurately describes the main change: unifying book cache clearing logic for epub, txt, and xtc files across multiple files.
Description check	✅ Passed	The PR description is directly related to the changeset, explaining the goal, listing specific changes across multiple files, and providing helpful context about follow-up work.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

Generate code and open pull requests
Plan features and break down work
Investigate incidents and troubleshoot customer tickets together
Automate recurring tasks and respond to alerts with triggers
Summarize progress and report instantly

Built for teams:

Shared memory across your entire org—no repeating context
Per-thread sandboxes to safely plan and execute work
Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/util/BookCacheUtils.cpp`:
- Around line 9-20: The unconditional success log in clearBookCache is
misleading because Epub::clearCache, Xtc::clearCache and Txt::clearCache return
status that is ignored; update clearBookCache to capture each call's
boolean/return value (e.g. auto ok = Epub(path, "/.crosspoint").clearCache()),
then only call LOG_DBG("BookCache", "Cleared metadata cache for: %s", ...) when
ok is true and call LOG_ERR("BookCache", "Failed to clear cache for: %s", ...)
(or return early) when ok is false; apply the same pattern for Epub, Xtc and Txt
so success/failure logs reflect the actual result.

🪄 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: 82b473a9-cbec-4490-ab19-f1cdd8fb1cac

📥 Commits

Reviewing files that changed from the base of the PR and between e3fb3bb and 4220d23.

📒 Files selected for processing (9)

lib/Txt/Txt.cpp
lib/Txt/Txt.h
src/activities/browser/OpdsBookBrowserActivity.cpp
src/activities/settings/ClearCacheActivity.cpp
src/network/CrossPointWebServer.cpp
src/network/WebDAVHandler.cpp
src/network/WebDAVHandler.h
src/util/BookCacheUtils.cpp
src/util/BookCacheUtils.h

💤 Files with no reviewable changes (1)

src/network/WebDAVHandler.h

📜 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). (2)

GitHub Check: build
GitHub Check: cppcheck

🧰 Additional context used

🧠 Learnings (5)

📚 Learning: 2026-02-23T06:18:08.408Z

Learnt from: raygan
Repo: crosspoint-reader/crosspoint-reader PR: 1116
File: src/activities/settings/WallabagSettingsActivity.cpp:57-75
Timestamp: 2026-02-23T06:18:08.408Z
Learning: When navigating from a settings activity to KeyboardEntryActivity (e.g., KOReaderSettingsActivity, WallabagSettingsActivity), call exitActivity() on the parent before calling enterNewActivity(new KeyboardEntryActivity(...)). The KeyboardEntryActivity should then call exitActivity() via its callbacks to return to the parent. Note: ConfirmationActivity does not require exitActivity() before entering KeyboardEntryActivity. Apply this pattern to similar settings activities in this module.

Applied to files:

src/activities/settings/ClearCacheActivity.cpp

📚 Learning: 2026-02-27T22:49:59.600Z

Learnt from: ngxson
Repo: crosspoint-reader/crosspoint-reader PR: 1218
File: src/activities/ActivityManager.cpp:254-265
Timestamp: 2026-02-27T22:49:59.600Z
Learning: In this codebase, assertions are always enabled (no NDEBUG). Use assert() to crash on programmer errors and surface logic bugs during development and in production builds. Do not rely on asserts for runtime error handling; they should enforce invariants that must always hold. Keep asserts side-effect free and inexpensive, and avoid relying on them for user-visible failures. Include <cassert> where appropriate and document the invariant being tested.

Applied to files:

src/activities/settings/ClearCacheActivity.cpp
lib/Txt/Txt.cpp
src/util/BookCacheUtils.cpp
src/network/CrossPointWebServer.cpp
src/network/WebDAVHandler.cpp
src/activities/browser/OpdsBookBrowserActivity.cpp

📚 Learning: 2026-03-02T10:14:16.036Z

Learnt from: Uri-Tauber
Repo: crosspoint-reader/crosspoint-reader PR: 1245
File: lib/Epub/Epub/Section.cpp:277-308
Timestamp: 2026-03-02T10:14:16.036Z
Learning: Guideline: Strengthen serialization::readString to defend against unbounded growth when reading from disk data. Implement and enforce a maximum allowed length (e.g., a configured or reasonable constant) and validate the incoming length before resizing or allocating. Audit all call sites (e.g., BookMetadataCache, TextBlock, KOReaderCredentialStore, Section cache readers) to ensure they do not rely on unbounded len-based resizing. If the readString API must remain, add internal safeguards (bounds checks, length validation, and error handling) so per-call-site validations are not required. Ensure Section cache files remain versioned (SECTION_FILE_VERSION) and parameter mismatches invalidate caches, but do not rely on unsafe allocations; prefer safe, bounded reads with explicit errors on invalid data.

Applied to files:

src/activities/settings/ClearCacheActivity.cpp
lib/Txt/Txt.cpp
src/util/BookCacheUtils.cpp
src/network/CrossPointWebServer.cpp
src/network/WebDAVHandler.cpp
src/activities/browser/OpdsBookBrowserActivity.cpp

📚 Learning: 2026-03-28T11:06:29.611Z

Learnt from: pablohc
Repo: crosspoint-reader/crosspoint-reader PR: 1488
File: src/activities/home/HomeActivity.cpp:92-95
Timestamp: 2026-03-28T11:06:29.611Z
Learning: When reviewing crosspoint-reader code, avoid flagging a missing `renderer.displayBuffer()` call immediately after `GUI.drawPopup()` / `BaseTheme::drawPopup()`: `BaseTheme::drawPopup()` already calls `renderer.displayBuffer()` before returning, so the popup is guaranteed to be flushed to the e-ink panel before subsequent blocking work begins. Conversely, do not require a `renderer.displayBuffer()` call after `fillPopupProgress()`; it intentionally does not flush, so intermediate progress-bar updates may not appear unless the update granularity warrants an explicit flush elsewhere.

Applied to files:

src/activities/settings/ClearCacheActivity.cpp
lib/Txt/Txt.cpp
src/util/BookCacheUtils.cpp
src/network/CrossPointWebServer.cpp
src/network/WebDAVHandler.cpp
src/activities/browser/OpdsBookBrowserActivity.cpp

📚 Learning: 2026-04-12T12:28:33.205Z

Learnt from: Uri-Tauber
Repo: crosspoint-reader/crosspoint-reader PR: 1629
File: src/activities/home/HomeActivity.cpp:119-120
Timestamp: 2026-04-12T12:28:33.205Z
Learning: When reviewing code in this repository (C/C++ sources), set review comment severity according to this policy:
- Use **Major** (🟠) only for defects with realistic risk of crash, out-of-memory (OOM), invalid pointer dereference, data corruption, or other severe issues that are unlikely to be caught in casual/manual device testing.
- Use **Minor** or informational for UX gaps, logic edge-cases, style issues, or missing feature completeness that the author can verify (or has verified) through normal device use.
- Do **not** escalate severity to Major based on behavioral/UX observations alone; assume the author has already tested the feature on their own device and only treat issues as Major if they match the high-risk defect categories above.

Applied to files:

src/activities/settings/ClearCacheActivity.cpp
lib/Txt/Txt.cpp
src/util/BookCacheUtils.cpp
src/util/BookCacheUtils.h
lib/Txt/Txt.h
src/network/CrossPointWebServer.cpp
src/network/WebDAVHandler.cpp
src/activities/browser/OpdsBookBrowserActivity.cpp

🔇 Additional comments (7)

lib/Txt/Txt.h (1)

25-25: LGTM!

Clean declaration, consistent with the setupCacheDir() / generateCoverBmp() sibling pattern.

lib/Txt/Txt.cpp (1)

158-171: LGTM!

Implementation is clean and idempotent: returns success immediately when the cache directory doesn't exist, delegates removal to Storage.removeDir (consistent with how EPUB/XTC caches are cleared), and propagates failure through the return value.

src/activities/settings/ClearCacheActivity.cpp (1)

97-99: LGTM!

The txt_ prefix correctly matches the cache directory naming scheme (txt_{hash}) produced by the Txt constructor, keeping this scan in sync with the new Txt::clearCache() introduction.

src/util/BookCacheUtils.h (1)

1-7: LGTM!

Minimal, well-documented header. The comment accurately reflects the extension-dispatch behaviour implemented in the .cpp.

src/activities/browser/OpdsBookBrowserActivity.cpp (1)

279-280: LGTM!

Switching to clearBookCache(filename) is correct here; filename is always *.epub (hardcoded extension at line 267), so the dispatch in BookCacheUtils.cpp will always route to Epub::clearCache() — semantically identical to the old call, now unified.

src/network/WebDAVHandler.cpp (1)

386-391: LGTM!

The clearBookCache(path.c_str()) placement (after the _putOk guard, before the HTTP response) correctly invalidates stale cache for all recognised overwrite scenarios.

src/network/CrossPointWebServer.cpp (1)

713-716: LGTM!

clearBookCache is called after confirming state.error.isEmpty() (upload succeeded), and the path construction mirrors the original file path. Correct placement across all five upload/rename/move/delete paths in this file.

WuTofu · 2026-05-09T03:30:40Z

@znelson I am new here. I'm not quite sure about the timing between RC and the final release

Before you release v1.3.0, I hope I can fix the issue where the book cache didn't get deleted when deleting a folder introduced by #1803.
This pr isn't that fix, but it adds a helper function, clearBookCache, which I want to use in that fix.

Would you mind reviewing this pr if the final release is coming soon?

WuTofu · 2026-05-11T07:58:17Z

@Uri-Tauber, @pablohc. If you are not too busy, could you review this pr?

This pr not only added BookCacheUtils, but also fixed the issues where we may not clear the txt or xtc cache by reusing the same helper function everywhere.

Uri-Tauber · 2026-05-11T10:30:51Z

@WuTofu Nice work — though I think this should probably wait until after release 1.3.

coderabbitai Bot reviewed May 8, 2026

View reviewed changes

Comment thread src/util/BookCacheUtils.cpp

WuTofu added 5 commits May 9, 2026 18:01

feat: implement cache clearing in Txt class

2ddbca6

feat: add clearBookCache function

58cd561

fix: update cache clearing in the settings to include txt_ directories

48a9b00

refactor: replace Epub cache clearing with clearBookCache function

737de45

Rephrase logging

d7ebb9a

WuTofu force-pushed the refactor/rm-book-cache branch from e1ecfee to d7ebb9a Compare May 9, 2026 10:02

WuTofu mentioned this pull request May 9, 2026

fix: clear cache when deleting folders in FileBrowserActivity #1892

Draft

refactor: add isBookCacheDirectoryName function

4c9cd3d

pablohc requested review from jpirnay, ngxson and pablohc May 11, 2026 08:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: unify book cache clearing for epub, txt, and xtc files#1875

refactor: unify book cache clearing for epub, txt, and xtc files#1875
WuTofu wants to merge 6 commits into
crosspoint-reader:masterfrom
WuTofu:refactor/rm-book-cache

WuTofu commented May 8, 2026

Uh oh!

coderabbitai Bot commented May 8, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Suggested reviewers

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

WuTofu commented May 9, 2026 •

edited

Loading

Uh oh!

WuTofu commented May 11, 2026

Uh oh!

Uri-Tauber commented May 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

WuTofu commented May 8, 2026

Summary

Additional Context

AI Usage

Uh oh!

coderabbitai Bot commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Suggested reviewers

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

WuTofu commented May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WuTofu commented May 11, 2026

Uh oh!

Uri-Tauber commented May 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented May 8, 2026 •

edited

Loading

WuTofu commented May 9, 2026 •

edited

Loading