fix-tags-cache - #314
Conversation
Resolves the issue were tags are not updated from radarr/sonarr after initial DB startup
There was a problem hiding this comment.
Pull request overview
Fixes #222 by restoring effectiveness of Sonarr/Radarr exclude tags after items have already been queued for deletion, and corrects an inverted cache check in the tags cache that caused every run to bypass the cache and emit a misleading log.
Changes:
- Adds
removeItemsWithExcludedTagsin the engine to cross-reference already-queued items against fresh Arr tag state and remove them from the queue when an exclude tag (orjellysweep-ignore) is now present. - Introduces
DBDeleteReasonExcludeTagto label such removals. - Rewrites
getTagsin both the Sonarr and Radarr clients so the cache is consulted only whenforceRefresh=false, eliminating the always-failing cache check and its spurious debug log.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| internal/engine/engine.go | Adds removeItemsWithExcludedTags and invokes it after gathering media items in the cleanup job. |
| internal/engine/arr/sonarr/sonarr.go | Simplifies getTags so caching is actually used when forceRefresh is false. |
| internal/engine/arr/radarr/radarr.go | Same caching fix as Sonarr for the Radarr client. |
| internal/database/interface.go | Adds new DBDeleteReasonExcludeTag constant for the new removal path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if shouldRemove { | ||
| log.Info("Removing item from deletion queue due to exclude tag", "title", dbItem.Title, "tag", tagName) | ||
| dbItem.DBDeleteReason = database.DBDeleteReasonExcludeTag | ||
| if err := e.db.DeleteMediaItem(ctx, &dbItem); err != nil { | ||
| log.Error("Failed to remove item from database", "title", dbItem.Title, "error", err) | ||
| } | ||
| break | ||
| } |
There was a problem hiding this comment.
The clanker is right. There should be an event for the history db that an item is now ignored.
Please see https://github.com/jon4hz/jellysweep/blob/main/internal/engine/events.go as a starting point.
|
@spyhunter493 can you take a look at the review comments? Especially #314 (comment) should be addressed. If you don't have the time, please let me know, and I'll do it. |
Fixes #222
Problem
Once a media item was marked for deletion and stored in the database, adding an exclude tag to it in Sonarr or Radarr had no effect. The database filter runs first in the pipeline and excludes already-queued items from all subsequent filters, meaning the tags filter never had a chance to re-evaluate them. The only workaround was deleting the entire database.
Additionally, getTags() in both the Sonarr and Radarr clients contained dead code: forceRefresh=true was always passed from GetItems(), which cleared the cache immediately before checking it — making the cache check condition (len(cachedTags) != 0 && !forceRefresh) impossible to satisfy. This caused a misleading "Failed to get Sonarr/Radarr tags from cache" debug log on every single run.
Changes
• Added removeItemsWithExcludedTags() to the engine, called each cleanup cycle after gatherMediaItems. It cross-references items currently in the database against their freshly-fetched tag state from Sonarr/Radarr. Any item that now carries an exclude tag (or jellysweep-ignore) is removed from the deletion queue automatically.
• Added DBDeleteReasonExcludeTag so history events correctly reflect why an item was removed from the queue.
• Simplified getTags() in both Sonarr and Radarr clients to fix the inverted cache logic — cache is now read when forceRefresh=false and bypassed when forceRefresh=true, as intended. Eliminates the spurious cache-miss debug log.