[PR #2114] [MERGED] fix: add cache invalidation after library prune #2081

New issue

Closed

opened 2026-02-26 03:33:14 +03:00 by kerem · 0 comments

kerem commented

2026-02-26 03:33:14 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/koel/koel/pull/2114
Author: @Doppelkeks12
Created: 9/30/2025
Status: ✅ Merged
Merged: 10/12/2025
Merged by: @phanan

Base: master ← Head: fix/album-artist_cache-invalidation

📝 Commits (8)

3a55d0f fix: add cache invalidation after library prune
916feef Revert "fix: add cache invalidation after library prune"
5442d15 fix: use in-memory cache for ScannerCacheStrategy
4933a76 feat: make the maxCacheSize in ScannerCache configurable
44e0cf8 fix: fix Off-by-one error during cache eviction
6b3b520 refactor: use get/put for cache access
0eb6876 Update app/Services/Scanners/ScannerCacheStrategy.php
89fc631 Update app/Services/Scanners/ScannerCacheStrategy.php

📊 Changes

5 files changed (+98 additions, -17 deletions)

View changed files

📝 app/Services/Scanners/Contracts/ScannerCacheStrategy.php (+0 -3)
📝 app/Services/Scanners/ScannerCacheStrategy.php (+23 -9)
📝 app/Services/Scanners/ScannerNoCacheStrategy.php (+0 -3)
📝 app/Services/SongService.php (+0 -2)
➕ tests/Unit/Services/Scanners/ScannerCacheStrategyTest.php (+75 -0)

📄 Description

This commit clears the artist and album cache after they are deleted from the database.

It fixes a bug where, if a song is deleted along with its artist/album and immediately re-uploaded, the cache provides an invalid state.

Reproduction steps:

Upload a song with a new album and artist
Delete the song
Reupload the same song
=> You got logged out. No Log in the laravel log, but in the response is "SQLSTATE[23000]: Integrity constraint violation: 1452 Cannot add or update a child row: a foreign key constraint fails (db.songs, CONSTRAINT songs_artist_id_foreign FOREIGN KEY (artist_id) REFERENCES artists (id) ON DELETE CASCADE ON UPDATE CASCADE) " (Maybe an issue on its own. I think the user should not be logged out, but gets an default error message. The SQL Error Log should be in the laravel log)

Questions for Review:

Should I add tests? I didn’t find existing unit tests for the changed files.
Could changing the existing cache keys cause any issues?
Should I extract a dedicated CacheKeyBuilder service instead of using a static method (single responsibility principle)?
Would it make sense to use an interface for cache deletion (interface segregation principle)?

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/koel/koel/pull/2114 **Author:** [@Doppelkeks12](https://github.com/Doppelkeks12) **Created:** 9/30/2025 **Status:** ✅ Merged **Merged:** 10/12/2025 **Merged by:** [@phanan](https://github.com/phanan) **Base:** `master` ← **Head:** `fix/album-artist_cache-invalidation` --- ### 📝 Commits (8) - [`3a55d0f`](https://github.com/koel/koel/commit/3a55d0fe153142c8e0dbf05f44cf6af9c761da5b) fix: add cache invalidation after library prune - [`916feef`](https://github.com/koel/koel/commit/916feefff0976cca181da2fee8f674e5860d88c9) Revert "fix: add cache invalidation after library prune" - [`5442d15`](https://github.com/koel/koel/commit/5442d15d4ce56c3c04f431b9da7497395ca470b1) fix: use in-memory cache for ScannerCacheStrategy - [`4933a76`](https://github.com/koel/koel/commit/4933a76efb9e1016938ffe67b9b93f442a9aa5f3) feat: make the maxCacheSize in ScannerCache configurable - [`44e0cf8`](https://github.com/koel/koel/commit/44e0cf86c4b506e88522da6635b02f6882740bd1) fix: fix Off-by-one error during cache eviction - [`6b3b520`](https://github.com/koel/koel/commit/6b3b5207b9176c5ff72ffc7b4f72f0c5bcea3537) refactor: use get/put for cache access - [`0eb6876`](https://github.com/koel/koel/commit/0eb68762a1fc21c40ac1c3f1ada1f8eaac28f82b) Update app/Services/Scanners/ScannerCacheStrategy.php - [`89fc631`](https://github.com/koel/koel/commit/89fc631a4f4d0a76a1280966ec72b73704419266) Update app/Services/Scanners/ScannerCacheStrategy.php ### 📊 Changes **5 files changed** (+98 additions, -17 deletions) <details> <summary>View changed files</summary> 📝 `app/Services/Scanners/Contracts/ScannerCacheStrategy.php` (+0 -3) 📝 `app/Services/Scanners/ScannerCacheStrategy.php` (+23 -9) 📝 `app/Services/Scanners/ScannerNoCacheStrategy.php` (+0 -3) 📝 `app/Services/SongService.php` (+0 -2) ➕ `tests/Unit/Services/Scanners/ScannerCacheStrategyTest.php` (+75 -0) </details> ### 📄 Description This commit clears the artist and album cache after they are deleted from the database. It fixes a bug where, if a song is deleted along with its artist/album and immediately re-uploaded, the cache provides an invalid state. **Reproduction steps:** 1. Upload a song with a new album and artist 2. Delete the song 3. Reupload the same song => You got logged out. No Log in the laravel log, but in the response is "SQLSTATE[23000]: Integrity constraint violation: 1452 Cannot add or update a child row: a foreign key constraint fails (`db`.`songs`, CONSTRAINT `songs_artist_id_foreign` FOREIGN KEY (`artist_id`) REFERENCES `artists` (`id`) ON DELETE CASCADE ON UPDATE CASCADE) " (Maybe an issue on its own. I think the user should not be logged out, but gets an default error message. The SQL Error Log should be in the laravel log) Questions for Review: 1. Should I add tests? I didn’t find existing unit tests for the changed files. 2. Could changing the existing cache keys cause any issues? 3. Should I extract a dedicated CacheKeyBuilder service instead of using a static method (single responsibility principle)? 4. Would it make sense to use an interface for cache deletion (interface segregation principle)? --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

kerem

2026-02-26 03:33:14 +03:00

closed this issue
added the
pull-request
label

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

starred/koel-koel#2081

No description provided.

Rows
Columns