馃悰 Fix stale thumbnail caching - #1380
ajselzilic wants to merge 1 commit into
Conversation
e632317 to
545437f
Compare
|
Hey, just wanted to say this has not slipped my radar. I'm hoping to have time closer to or over the weekend to give this a proper look |
aaronleopold
left a comment
aaronleopold
left a comment
There was a problem hiding this comment.
I had a few comments, but otherwise I think this is on the right track. Thanks for working on this! Let me know if you have any questions
| pub is_custom: bool, | ||
| pub target: ThumbnailTarget, | ||
| } | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| pub enum ThumbnailTarget { | ||
| Media, | ||
| Series(String), | ||
| Library(String), | ||
| } |
There was a problem hiding this comment.
I'm not sure I understand the move away from filename and towards an enum, rather than just making the callers set filename accordingly (e.g., mediaIdaryid.generated for a generated one or mediaId for custom).
I guess coming back to this after a little more review, it's intention is for the update clause. I think we can get away without the is_custom flag, though. I don't see why a custom thumbnail would even pass through the generate flow.
Separately, I think my decision to include database updates in the generate_book_thumbnail is one I am currently questioning. If we had e.g. generate_library_thumbnail that calls (renamed) generate_thumbnail_from_book then that library-specific one can handle the update call and be a bit cleaner to follow. Then the same would follow for generate_series_thumbnail/generate_book_thumbnail calls generate_thumbnail_from_book and handles their own update.
| if is_custom { | ||
| match &target { | ||
| ThumbnailTarget::Media => { | ||
| bump_media_thumbnail_fallbacks(conn, Some(&book.series_id)).await? | ||
| }, | ||
| ThumbnailTarget::Series(id) => { | ||
| bump_series_thumbnail_fallbacks(conn, std::slice::from_ref(id)).await? | ||
| }, | ||
| ThumbnailTarget::Library(_) => {}, | ||
| } |
There was a problem hiding this comment.
Same as above re: why would a custom thumb flow through to the generate flow?
| .is_some_and(|stem| stem.ends_with(".generated")) | ||
| } | ||
|
|
||
| pub async fn bump_media_thumbnail_fallbacks<C>( |
There was a problem hiding this comment.
I understand why these bumps were added, but I don't love how it works around an awkward point (image fallbacks and cache-friendly URLs). I think it's fine for now, just being a bit pedantic
| .then((data) => { | ||
| if (active) { | ||
| setLoadedImage({ src, token, url: URL.createObjectURL(data) }) | ||
| } | ||
| }) | ||
| .catch((error) => { | ||
| if (active) { | ||
| setLoadedImage({ src, token, url: src }) | ||
| console.error('Failed to load authenticated image:', error) | ||
| } | ||
| }) |
There was a problem hiding this comment.
I generally prefer async/await, which is why originally I handled the local state in the fetchImage callback. If it was refactored for the active I think react-query has utilities to cancel in-flight queries instead
2 participants
Closes #913
Related to #546 and #619
Regenerates Stump-generated thumbnails during forced library rebuilds and adds modification timestamps to media, series, and library thumbnail URLs. When a thumbnail changes, the related timestamp changes with it, giving the browser a new URL and bypassing the existing long-lived cache without disabling caching.
Relevant browser queries are refreshed after thumbnail uploads, selections, deletions, scans, and generation jobs. Thumbnail consumers, including tables and the File Explorer, now use the versioned URLs returned by GraphQL instead of rebuilding unversioned URLs locally.
New generated thumbnails include a .generated filename marker so Stump can distinguish them from custom uploads. This allows forced rebuilds to replace generated thumbnails without overwriting user-provided covers. A filename marker was chosen instead of a database migration because the distinction is only needed while managing the files, and existing thumbnails cannot be reliably classified after the fact. Existing unmarked thumbnails are therefore treated as custom or legacy files, while newly generated thumbnails can be safely replaced going forward.
While tracing the full thumbnail flow, this also fixes:
Library thumbnail cleanup now clears its database state transactionally. It also uses relational queries instead of passing every series and media ID as a separate SQL parameter, avoiding SQLite bind limits on large libraries.