Batch download songs; add tests for song invalidation and deletion

sublime/adapters/adapter_manager.py

@@ -1,8 +1,10 @@
 import logging
 import tempfile
+import threading
 from concurrent.futures import Future, ThreadPoolExecutor
 from dataclasses import dataclass
 from pathlib import Path
+from time import sleep
 from typing import (
     Any,
     Callable,
@@ -11,6 +13,7 @@ from typing import (
     Optional,
     Sequence,
     Set,
+    Tuple,
     Type,
     TypeVar,
     Union,
@@ -18,6 +21,7 @@ from typing import (
 
 import requests
 
+from sublime import util
 from sublime.config import AppConfiguration
 
 from .adapter_base import Adapter, CacheMissError, CachingAdapter, SongCacheStatus
@@ -95,6 +99,8 @@ class Result(Generic[T]):
 
 class AdapterManager:
     available_adapters: Set[Any] = {FilesystemAdapter, SubsonicAdapter}
+    current_download_hashes: Set[str] = set()
+    download_set_lock = threading.Lock()
     executor: ThreadPoolExecutor = ThreadPoolExecutor()
     is_shutting_down: bool = False
 
@@ -102,10 +108,14 @@ class AdapterManager:
     class _AdapterManagerInternal:
         ground_truth_adapter: Adapter
         caching_adapter: Optional[CachingAdapter] = None
+        concurrent_download_limit: int = 5
 
         def __post_init__(self):
             self._download_dir = tempfile.TemporaryDirectory()
             self.download_path = Path(self._download_dir.name)
+            self.download_limiter_semaphore = threading.Semaphore(
+                self.concurrent_download_limit
+            )
 
         def shutdown(self):
             self.ground_truth_adapter.shutdown()
@@ -176,7 +186,9 @@ class AdapterManager:
             )
 
         AdapterManager._instance = AdapterManager._AdapterManagerInternal(
-            ground_truth_adapter, caching_adapter=caching_adapter,
+            ground_truth_adapter,
+            caching_adapter=caching_adapter,
+            concurrent_download_limit=config.concurrent_download_limit,
         )
 
     # Data Helper Methods
@@ -218,6 +230,86 @@ class AdapterManager:
             action_name
         ) or AdapterManager._cache_can_do(action_name)
 
+    @staticmethod
+    def _create_future_fn(
+        function_name: str,
+        before_download: Callable[[], None] = lambda: None,
+        *args,
+        **kwargs,
+    ) -> Result:
+        def future_fn() -> Any:
+            assert AdapterManager._instance
+            if before_download:
+                before_download()
+            return getattr(
+                AdapterManager._instance.ground_truth_adapter, function_name
+            )(*args, **kwargs)
+
+        return Result(future_fn)
+
+    @staticmethod
+    def _create_download_fn(
+        uri: str, params_hash: str, before_download: Callable[[], None] = lambda: None,
+    ) -> Callable:
+        def download_fn() -> str:
+            assert AdapterManager._instance
+            download_tmp_filename = AdapterManager._instance.download_path.joinpath(
+                params_hash
+            )
+
+            resource_downloading = False
+            with AdapterManager.download_set_lock:
+                if params_hash in AdapterManager.current_download_hashes:
+                    resource_downloading = True
+                AdapterManager.current_download_hashes.add(params_hash)
+
+            # TODO figure out how to retry
+            if resource_downloading:
+                logging.info(f"{uri} already being downloaded.")
+
+                # The resource is already being downloaded. Busy loop until
+                # it has completed. Then, just return the path to the
+                # resource.
+                t = 0.0
+                while params_hash in AdapterManager.current_download_hashes and t < 20:
+                    sleep(0.2)
+                    t += 0.2
+                    # TODO handle the timeout
+            else:
+                logging.info(f"{uri} not found. Downloading...")
+                if before_download:
+                    before_download()
+
+                try:
+                    data = requests.get(uri)
+
+                    # TODO (#122): make better
+                    if not data:
+                        raise Exception("Download failed!")
+                    if "json" in data.headers.get("Content-Type", ""):
+                        raise Exception("Didn't expect JSON!")
+
+                    with open(download_tmp_filename, "wb+") as f:
+                        f.write(data.content)
+                finally:
+                    # Always release the download set lock, even if there's an error.
+                    with AdapterManager.download_set_lock:
+                        AdapterManager.current_download_hashes.discard(params_hash)
+
+            logging.info(f"{uri} downloaded. Returning.")
+            return str(download_tmp_filename)
+
+        return download_fn
+
+    @staticmethod
+    def _get_scheme():
+        scheme_priority = ("https", "http")
+        schemes = sorted(
+            AdapterManager._instance.ground_truth_adapter.supported_schemes,
+            key=scheme_priority.index,
+        )
+        return list(schemes)[0]
+
     # TODO abstract more stuff
 
     # Usage and Availability Properties
@@ -247,9 +339,14 @@ class AdapterManager:
         return AdapterManager._any_adapter_can_do("get_cover_art_uri")
 
     @staticmethod
-    def can_get_song_uri() -> bool:
+    def can_get_song_filename_or_stream() -> bool:
         return AdapterManager._any_adapter_can_do("get_song_uri")
 
+    @staticmethod
+    def can_batch_download_songs() -> bool:
+        # We can only download from the ground truth adapter.
+        return AdapterManager._ground_truth_can_do("get_song_uri")
+
     # Data Retrieval Methods
     # ==================================================================================
     @staticmethod
@@ -279,13 +376,9 @@ class AdapterManager:
                 return partial_playlists_data
             raise Exception(f'No adapters can service {"get_playlists"} at the moment.')
 
-        def future_fn() -> Sequence[Playlist]:
+        future: Result[Sequence[Playlist]] = AdapterManager._create_future_fn(
-            assert AdapterManager._instance
+            "get_playlists", before_download
-            if before_download:
+        )
-                before_download()
-            return AdapterManager._instance.ground_truth_adapter.get_playlists()
-
-        future: Result[Sequence[Playlist]] = Result(future_fn)
 
         if AdapterManager._instance.caching_adapter:
 
@@ -336,15 +429,9 @@ class AdapterManager:
                 f'No adapters can service {"get_playlist_details"} at the moment.'
             )
 
-        def future_fn() -> PlaylistDetails:
+        future: Result[PlaylistDetails] = AdapterManager._create_future_fn(
-            assert AdapterManager._instance
+            "get_playlist_details", before_download, playlist_id
-            if before_download:
+        )
-                before_download()
-            return AdapterManager._instance.ground_truth_adapter.get_playlist_details(
-                playlist_id
-            )
-
-        future: Result[PlaylistDetails] = Result(future_fn)
 
         if AdapterManager._instance.caching_adapter:
 
@@ -370,15 +457,9 @@ class AdapterManager:
     ) -> Result[None]:
         assert AdapterManager._instance
 
-        def future_fn():
+        future: Result[None] = AdapterManager._create_future_fn(
-            assert AdapterManager._instance
+            "get_playlist_details", before_download, name, songs=songs
-            if before_download:
+        )
-                before_download()
-            AdapterManager._instance.ground_truth_adapter.create_playlist(
-                name, songs=songs
-            )
-
-        future: Result[None] = Result(future_fn)
 
         if AdapterManager._instance.caching_adapter:
 
@@ -412,20 +493,15 @@ class AdapterManager:
         force: bool = False,  # TODO: rename to use_ground_truth_adapter?
     ) -> Result[PlaylistDetails]:
         assert AdapterManager._instance
-
+        future: Result[PlaylistDetails] = AdapterManager._create_future_fn(
-        def future_fn() -> PlaylistDetails:
+            "update_playlist",
-            assert AdapterManager._instance
+            before_download,
-            if before_download:
+            playlist_id,
-                before_download()
+            name=name,
-            return AdapterManager._instance.ground_truth_adapter.update_playlist(
+            comment=comment,
-                playlist_id,
+            public=public,
-                name=name,
+            song_ids=song_ids,
-                comment=comment,
+        )
-                public=public,
-                song_ids=song_ids,
-            )
-
-        future: Result[PlaylistDetails] = Result(future_fn)
 
         if AdapterManager._instance.caching_adapter:
 
@@ -493,37 +569,15 @@ class AdapterManager:
                 f'No adapters can service {"get_cover_art_uri"} at the moment.'
             )
 
-        def future_fn() -> str:
+        future: Result[str] = Result(
-            assert AdapterManager._instance
+            AdapterManager._create_download_fn(
-            if before_download:
-                before_download()
-
-            scheme_priority = ("https", "http")
-            schemes = sorted(
-                AdapterManager._instance.ground_truth_adapter.supported_schemes,
-                key=scheme_priority.index,
-            )
-
-            # TODO guard for already being downloaded
-            data = requests.get(
                 AdapterManager._instance.ground_truth_adapter.get_cover_art_uri(
-                    cover_art_id, list(schemes)[0]
+                    cover_art_id, AdapterManager._get_scheme()
-                )
+                ),
+                util.params_hash("cover_art", cover_art_id),
+                before_download=before_download,
             )
-
+        )
-            # TODO (#122): make better
-            if "json" in data.headers.get("Content-Type", ""):
-                raise Exception("Didn't expect JSON.")
-
-            download_dir = AdapterManager._instance.download_path.joinpath("cover_art")
-            download_dir.mkdir(parents=True, exist_ok=True)
-            cover_art_filename = download_dir.joinpath(cover_art_id)
-            with open(cover_art_filename, "wb+") as f:
-                f.write(data.content)
-
-            return str(cover_art_filename)
-
-        future: Result[str] = Result(future_fn)
 
         if AdapterManager._instance.caching_adapter:
 
@@ -540,6 +594,65 @@ class AdapterManager:
 
         return future
 
+    @staticmethod
+    def get_song_filename_or_stream() -> Tuple[str, bool]:
+        raise NotImplementedError()
+
+    @staticmethod
+    def batch_download_songs(
+        song_ids: List[str],
+        before_download: Callable[[], None],
+        on_song_download_complete: Callable[[], None],
+    ):
+        assert AdapterManager._instance
+
+        # This only really makes sense if we have a caching_adapter.
+        if not AdapterManager._instance.caching_adapter:
+            return
+
+        def do_download_song(song_id: str):
+            assert AdapterManager._instance
+            assert AdapterManager._instance.caching_adapter
+
+            try:
+                if AdapterManager.is_shutting_down:
+                    return
+
+                song_tmp_filename = AdapterManager._create_download_fn(
+                    AdapterManager._instance.ground_truth_adapter.get_song_uri(
+                        song_id, AdapterManager._get_scheme()
+                    ),
+                    util.params_hash("song", song_id),
+                    before_download=before_download,
+                )()
+
+                AdapterManager._instance.caching_adapter.ingest_new_data(
+                    CachingAdapter.CachedDataKey.SONG_FILE,
+                    (song_id,),
+                    song_tmp_filename,
+                )
+
+                on_song_download_complete()
+            finally:
+                # Release the semaphore lock. This will allow the next song in the queue
+                # to be downloaded. I'm doing this in the finally block so that it
+                # always runs, regardless of whether an exception is thrown or the
+                # function returns.
+                AdapterManager._instance.download_limiter_semaphore.release()
+
+        def do_batch_download_songs():
+            for song_id in song_ids:
+                # Only allow a certain number of songs to be downloaded
+                # simultaneously.
+                AdapterManager._instance.download_limiter_semaphore.acquire()
+
+                # Prevents further songs from being downloaded.
+                if AdapterManager.is_shutting_down:
+                    break
+                Result(lambda: do_download_song(song_id))
+
+        Result(do_batch_download_songs)
+
     @staticmethod
     def get_song_uri(
         song_id: str,
@@ -558,4 +671,7 @@ class AdapterManager:
         if not AdapterManager._instance.caching_adapter:
             return SongCacheStatus.NOT_CACHED
 
+        if util.params_hash("song", song.id) in AdapterManager.current_download_hashes:
+            return SongCacheStatus.DOWNLOADING
+
         return AdapterManager._instance.caching_adapter.get_cached_status(song)

sublime/adapters/filesystem/adapter.py

@@ -1,5 +1,3 @@
-import hashlib
-import json
 import logging
 import shutil
 import threading
@@ -8,6 +6,7 @@ from datetime import datetime
 from pathlib import Path
 from typing import Any, Dict, Optional, Sequence, Tuple
 
+from sublime import util
 from sublime.adapters import api_objects as API
 
 from . import models
@@ -36,7 +35,10 @@ class FilesystemAdapter(CachingAdapter):
     ):
         self.data_directory = data_directory
         self.cover_art_dir = self.data_directory.joinpath("cover_art")
+        self.music_dir = self.data_directory.joinpath("music")
+
         self.cover_art_dir.mkdir(parents=True, exist_ok=True)
+        self.music_dir.mkdir(parents=True, exist_ok=True)
 
         self.is_cache = is_cache
 
@@ -64,17 +66,13 @@ class FilesystemAdapter(CachingAdapter):
 
     # Data Helper Methods
     # ==================================================================================
-    def _params_hash(self, *params: Any) -> str:
-        return hashlib.sha1(bytes(json.dumps(params), "utf8")).hexdigest()
 
     # Data Retrieval Methods
     # ==================================================================================
     def get_cached_status(self, song: API.Song) -> SongCacheStatus:
-        # TODO: check if being downloaded
-
         # TODO: change this path to be the correct dir
         relative_path = models.Song.get_by_id(song.id).path
-        cache_path = self.data_directory.parent.joinpath(relative_path)
+        cache_path = self.music_dir.joinpath(relative_path)
         if cache_path.exists():
             # TODO check if path is permanently cached
             return SongCacheStatus.CACHED
@@ -107,7 +105,7 @@ class FilesystemAdapter(CachingAdapter):
         cache_key = CachingAdapter.CachedDataKey.PLAYLIST_DETAILS
         cache_info = models.CacheInfo.get_or_none(
             models.CacheInfo.cache_key == cache_key,
-            models.CacheInfo.params_hash == self._params_hash(playlist_id),
+            models.CacheInfo.params_hash == util.params_hash(playlist_id),
         )
         if not cache_info:
             raise CacheMissError(partial_data=playlist)
@@ -115,13 +113,13 @@ class FilesystemAdapter(CachingAdapter):
         return playlist
 
     def get_cover_art_uri(self, cover_art_id: str, scheme: str) -> str:
-        params_hash = self._params_hash(cover_art_id)
+        params_hash = util.params_hash(cover_art_id)
         cover_art_filename = self.cover_art_dir.joinpath(params_hash)
 
         # Handle the case that this is the ground truth adapter.
         if not self.is_cache:
             if not cover_art_filename.exists:
-                raise Exception(f"Cover Art {cover_art_id} does not exist.")
+                raise Exception(f"Cover Art for {cover_art_id} does not exist.")
             return str(cover_art_filename)
 
         if not cover_art_filename.exists():
@@ -129,7 +127,8 @@ class FilesystemAdapter(CachingAdapter):
 
         cache_key = CachingAdapter.CachedDataKey.COVER_ART_FILE
         cache_info = models.CacheInfo.get_or_none(
-            models.CacheInfo.cache_key == cache_key, params_hash == params_hash
+            models.CacheInfo.cache_key == cache_key,
+            models.CacheInfo.params_hash == params_hash,
         )
         if not cache_info:
             raise CacheMissError(partial_data=str(cover_art_filename))
@@ -137,7 +136,33 @@ class FilesystemAdapter(CachingAdapter):
         return str(cover_art_filename)
 
     def get_song_uri(self, song_id: str, scheme: str, stream=False) -> str:
-        raise CacheMissError()
+        song = models.Song.get_or_none(song_id)
+        if not song:
+            if self.is_cache:
+                raise CacheMissError()
+            else:
+                raise Exception(f"Song {song_id} does not exist.")
+
+        music_filename = self.music_dir.joinpath(song.path)
+
+        # Handle the case that this is the ground truth adapter.
+        if not self.is_cache:
+            if not music_filename.exists:
+                raise Exception(f"Music File for song {song_id} does not exist.")
+            return str(music_filename)
+
+        if not music_filename.exists():
+            raise CacheMissError()
+
+        cache_key = CachingAdapter.CachedDataKey.SONG_FILE
+        cache_info = models.CacheInfo.get_or_none(
+            models.CacheInfo.cache_key == cache_key,
+            models.CacheInfo.params_hash == util.params_hash(song_id),
+        )
+        if not cache_info:
+            raise CacheMissError(partial_data=str(music_filename))
+
+        return str(music_filename)
 
     # Data Ingestion Methods
     # ==================================================================================
@@ -180,7 +205,7 @@ class FilesystemAdapter(CachingAdapter):
         params: Tuple[Any, ...],
         data: Any,
     ):
-        params_hash = self._params_hash(*params)
+        params_hash = util.params_hash(*params)
         models.CacheInfo.insert(
             cache_key=data_key,
             params_hash=params_hash,
@@ -227,13 +252,24 @@ class FilesystemAdapter(CachingAdapter):
         elif data_key == CachingAdapter.CachedDataKey.COVER_ART_FILE:
             # ``data`` is the filename of the tempfile in this case
             shutil.copy(str(data), str(self.cover_art_dir.joinpath(params_hash)))
+        elif data_key == CachingAdapter.CachedDataKey.SONG_FILE:
+            relative_path = models.Song.get_by_id(params[0]).path
+            absolute_path = self.music_dir.joinpath(relative_path)
+            absolute_path.parent.mkdir(parents=True, exist_ok=True)
+            shutil.copy(str(data), str(absolute_path))
+
+    def _invalidate_cover_art(self, cover_art_id: str):
+        models.CacheInfo.delete().where(
+            models.CacheInfo.cache_key == CachingAdapter.CachedDataKey.COVER_ART_FILE,
+            models.CacheInfo.params_hash == util.params_hash(cover_art_id),
+        ).execute()
 
     def _do_invalidate_data(
         self, data_key: "CachingAdapter.CachedDataKey", params: Tuple[Any, ...],
     ):
         models.CacheInfo.delete().where(
             models.CacheInfo.cache_key == data_key,
-            models.CacheInfo.params_hash == self._params_hash(*params),
+            models.CacheInfo.params_hash == util.params_hash(*params),
         ).execute()
 
         if data_key == CachingAdapter.CachedDataKey.PLAYLIST_DETAILS:
@@ -243,12 +279,16 @@ class FilesystemAdapter(CachingAdapter):
                 return
 
             if playlist.cover_art:
-                cover_art_key = CachingAdapter.CachedDataKey.COVER_ART_FILE
+                self._invalidate_cover_art(playlist.cover_art)
-                cover_art_params_hash = self._params_hash(playlist.cover_art)
+
-                models.CacheInfo.delete().where(
+        elif data_key == CachingAdapter.CachedDataKey.SONG_FILE:
-                    models.CacheInfo.cache_key == cover_art_key,
+            # Invalidate the corresponding cover art.
-                    models.CacheInfo.params_hash == cover_art_params_hash,
+            song = models.Song.get_or_none(models.Song.id == params[0])
-                ).execute()
+            if not song:
+                return
+
+            if song.cover_art:
+                self._invalidate_cover_art(song.cover_art)
 
     def _do_delete_data(
         self, data_key: "CachingAdapter.CachedDataKey", params: Tuple[Any, ...],
@@ -256,9 +296,15 @@ class FilesystemAdapter(CachingAdapter):
         # Delete it from the cache info.
         models.CacheInfo.delete().where(
             models.CacheInfo.cache_key == data_key,
-            models.CacheInfo.params_hash == self._params_hash(*params),
+            models.CacheInfo.params_hash == util.params_hash(*params),
         ).execute()
 
+        def delete_cover_art(cover_art_filename):
+            cover_art_params_hash = util.params_hash(playlist.cover_art)
+            if cover_art_file := self.cover_art_dir.joinpath(cover_art_params_hash):
+                cover_art_file.unlink(missing_ok=True)
+            self._invalidate_cover_art(playlist.cover_art)
+
         if data_key == CachingAdapter.CachedDataKey.PLAYLIST_DETAILS:
             # Delete the playlist and corresponding cover art.
             playlist = models.Playlist.get_or_none(models.Playlist.id == params[0])
@@ -266,13 +312,17 @@ class FilesystemAdapter(CachingAdapter):
                 return
 
             if playlist.cover_art:
-                cover_art_params_hash = self._params_hash(playlist.cover_art)
+                delete_cover_art(playlist.cover_art)
-                if cover_art_file := self.cover_art_dir.joinpath(cover_art_params_hash):
-                    cover_art_file.unlink(missing_ok=True)
-                cover_art_key = CachingAdapter.CachedDataKey.COVER_ART_FILE
-                models.CacheInfo.delete().where(
-                    models.CacheInfo.cache_key == cover_art_key,
-                    models.CacheInfo.params_hash == cover_art_params_hash,
-                ).execute()
 
             playlist.delete_instance()
+
+        elif data_key == CachingAdapter.CachedDataKey.SONG_FILE:
+            # Delete the song and corresponding cover art.
+            song = models.Song.get_or_none(models.Song.id == params[0])
+            if not song:
+                return
+
+            if song.cover_art:
+                delete_cover_art(song.cover_art)
+
+            song.delete_instance()

sublime/adapters/filesystem/models.py

@@ -1,7 +1,7 @@
 from peewee import (
     BooleanField,
     CompositeKey,
-    ForeignKeyField,
+    # ForeignKeyField,
     IntegerField,
     Model,
     SqliteDatabase,

sublime/ui/playlists.py

@@ -10,7 +10,6 @@ from gi.repository import Gdk, Gio, GLib, GObject, Gtk, Pango
 
 from sublime.adapters import AdapterManager
 from sublime.adapters.api_objects import Playlist, PlaylistDetails
-from sublime.cache_manager import CacheManager
 from sublime.config import AppConfiguration
 from sublime.ui import util
 from sublime.ui.common import (
@@ -595,7 +594,7 @@ class PlaylistDetailPanel(Gtk.Overlay):
             )
 
         song_ids = [s[-1] for s in self.playlist_song_store]
-        CacheManager.batch_download_songs(
+        AdapterManager.batch_download_songs(
             song_ids,
             before_download=download_state_change,
             on_song_download_complete=download_state_change,

sublime/util.py

@@ -0,0 +1,7 @@
+import hashlib
+import json
+from typing import Any
+
+
+def params_hash(*params: Any) -> str:
+    return hashlib.sha1(bytes(json.dumps(params), "utf8")).hexdigest()

tests/adapter_tests/filesystem_adapter_tests.py

@@ -6,12 +6,14 @@ from typing import Any, Generator, Tuple
 
 import pytest
 
+from sublime import util
 from sublime.adapters import CacheMissError
 from sublime.adapters.filesystem import FilesystemAdapter
 from sublime.adapters.subsonic import api_objects as SubsonicAPI
 
 MOCK_DATA_FILES = Path(__file__).parent.joinpath("mock_data")
 MOCK_ALBUM_ART = MOCK_DATA_FILES.joinpath("album-art.png")
+MOCK_SONG_FILE = MOCK_DATA_FILES.joinpath("test-song.mp3")
 
 
 @pytest.fixture
@@ -115,7 +117,7 @@ def test_caching_get_playlist_details(cache_adapter: FilesystemAdapter):
             album="foo",
             artist="foo",
             duration=timedelta(seconds=20.8),
-            path="/foo/song2.mp3",
+            path="foo/song2.mp3",
         ),
         SubsonicAPI.Song(
             "1",
@@ -124,7 +126,7 @@ def test_caching_get_playlist_details(cache_adapter: FilesystemAdapter):
             album="foo",
             artist="foo",
             duration=timedelta(seconds=10.2),
-            path="/foo/song1.mp3",
+            path="foo/song1.mp3",
         ),
     ]
     cache_adapter.ingest_new_data(
@@ -151,7 +153,7 @@ def test_caching_get_playlist_details(cache_adapter: FilesystemAdapter):
             album="foo",
             artist="foo",
             duration=timedelta(seconds=10.2),
-            path="/foo/song3.mp3",
+            path="foo/song3.mp3",
         ),
         SubsonicAPI.Song(
             "1",
@@ -160,7 +162,7 @@ def test_caching_get_playlist_details(cache_adapter: FilesystemAdapter):
             album="foo",
             artist="foo",
             duration=timedelta(seconds=21.8),
-            path="/foo/song1.mp3",
+            path="foo/song1.mp3",
         ),
         SubsonicAPI.Song(
             "1",
@@ -169,7 +171,7 @@ def test_caching_get_playlist_details(cache_adapter: FilesystemAdapter):
             album="foo",
             artist="foo",
             duration=timedelta(seconds=21.8),
-            path="/foo/song1.mp3",
+            path="foo/song1.mp3",
         ),
     ]
     cache_adapter.ingest_new_data(
@@ -230,7 +232,7 @@ def test_caching_get_playlist_then_details(cache_adapter: FilesystemAdapter):
             album="foo",
             artist="foo",
             duration=timedelta(seconds=10.2),
-            path="/foo/song3.mp3",
+            path="foo/song3.mp3",
         ),
     ]
     cache_adapter.ingest_new_data(
@@ -256,14 +258,12 @@ def test_cache_cover_art(cache_adapter: FilesystemAdapter):
     with pytest.raises(CacheMissError):
         cache_adapter.get_cover_art_uri("pl_test1", "file")
 
-    sample_file_path = MOCK_DATA_FILES.joinpath(MOCK_ALBUM_ART)
-
     # After ingesting the data, reading from the cache should give the exact same file.
     cache_adapter.ingest_new_data(
-        FilesystemAdapter.CachedDataKey.COVER_ART_FILE, ("pl_test1",), sample_file_path,
+        FilesystemAdapter.CachedDataKey.COVER_ART_FILE, ("pl_test1",), MOCK_ALBUM_ART,
     )
     with open(cache_adapter.get_cover_art_uri("pl_test1", "file"), "wb+") as cached:
-        with open(sample_file_path, "wb+") as expected:
+        with open(MOCK_ALBUM_ART, "wb+") as expected:
             assert cached.read() == expected.read()
 
 
@@ -274,9 +274,7 @@ def test_invalidate_data(cache_adapter: FilesystemAdapter):
         [SubsonicAPI.Playlist("1", "test1"), SubsonicAPI.Playlist("2", "test2")],
     )
     cache_adapter.ingest_new_data(
-        FilesystemAdapter.CachedDataKey.COVER_ART_FILE,
+        FilesystemAdapter.CachedDataKey.COVER_ART_FILE, ("pl_test1",), MOCK_ALBUM_ART,
-        ("pl_test1",),
-        MOCK_DATA_FILES.joinpath(MOCK_ALBUM_ART),
     )
     cache_adapter.ingest_new_data(
         FilesystemAdapter.CachedDataKey.PLAYLIST_DETAILS,
@@ -284,9 +282,7 @@ def test_invalidate_data(cache_adapter: FilesystemAdapter):
         SubsonicAPI.PlaylistWithSongs("2", "test2", cover_art="pl_2", songs=[]),
     )
     cache_adapter.ingest_new_data(
-        FilesystemAdapter.CachedDataKey.COVER_ART_FILE,
+        FilesystemAdapter.CachedDataKey.COVER_ART_FILE, ("pl_2",), MOCK_ALBUM_ART,
-        ("pl_2",),
-        MOCK_DATA_FILES.joinpath(MOCK_ALBUM_ART),
     )
     stale_uri_1 = cache_adapter.get_cover_art_uri("pl_test1", "file")
     stale_uri_2 = cache_adapter.get_cover_art_uri("pl_2", "file")
@@ -331,6 +327,39 @@ def test_invalidate_data(cache_adapter: FilesystemAdapter):
         assert e.partial_data == stale_uri_2
 
 
+def test_invalidate_song_data(cache_adapter: FilesystemAdapter):
+    # TODO change to ingest song details?
+    songs = [
+        SubsonicAPI.Song(
+            "1",
+            "Song 1",
+            parent="foo",
+            album="foo",
+            artist="foo",
+            duration=timedelta(seconds=10.2),
+            path="foo/song1.mp3",
+        ),
+    ]
+    cache_adapter.ingest_new_data(
+        FilesystemAdapter.CachedDataKey.PLAYLIST_DETAILS,
+        ("1",),
+        SubsonicAPI.PlaylistWithSongs("1", "test1", songs=songs),
+    )
+
+    cache_adapter.ingest_new_data(
+        FilesystemAdapter.CachedDataKey.SONG_FILE, ("1",), MOCK_SONG_FILE
+    )
+    stale_song_file = cache_adapter.get_song_uri("1", "file")
+    cache_adapter.invalidate_data(FilesystemAdapter.CachedDataKey.SONG_FILE, ("1",))
+
+    try:
+        cache_adapter.get_song_uri("1", "file")
+        assert 0, "DID NOT raise CacheMissError"
+    except CacheMissError as e:
+        assert e.partial_data
+        assert e.partial_data == stale_song_file
+
+
 def test_delete_data(cache_adapter: FilesystemAdapter):
     cache_adapter.ingest_new_data(
         FilesystemAdapter.CachedDataKey.PLAYLIST_DETAILS,
@@ -343,9 +372,7 @@ def test_delete_data(cache_adapter: FilesystemAdapter):
         SubsonicAPI.PlaylistWithSongs("2", "test1", cover_art="pl_2", songs=[]),
     )
     cache_adapter.ingest_new_data(
-        FilesystemAdapter.CachedDataKey.COVER_ART_FILE,
+        FilesystemAdapter.CachedDataKey.COVER_ART_FILE, ("pl_1",), MOCK_ALBUM_ART,
-        ("pl_1",),
-        MOCK_DATA_FILES.joinpath(MOCK_ALBUM_ART),
     )
 
     # Deleting a playlist should get rid of it entirely.
@@ -366,8 +393,39 @@ def test_delete_data(cache_adapter: FilesystemAdapter):
 
     # Even if the cover art failed to be deleted, it should cache miss.
     shutil.copy(
-        MOCK_DATA_FILES.joinpath(MOCK_ALBUM_ART),
+        MOCK_ALBUM_ART,
-        str(cache_adapter.cover_art_dir.joinpath(cache_adapter._params_hash("pl_1"))),
+        str(cache_adapter.cover_art_dir.joinpath(util.params_hash("pl_1"))),
     )
     with pytest.raises(CacheMissError):
         cache_adapter.get_cover_art_uri("pl_1", "file")
+
+
+def test_delete_song_data(cache_adapter: FilesystemAdapter):
+    # TODO change to ingest song details?
+    songs = [
+        SubsonicAPI.Song(
+            "1",
+            "Song 1",
+            parent="foo",
+            album="foo",
+            artist="foo",
+            duration=timedelta(seconds=10.2),
+            path="foo/song1.mp3",
+        ),
+    ]
+    cache_adapter.ingest_new_data(
+        FilesystemAdapter.CachedDataKey.PLAYLIST_DETAILS,
+        ("1",),
+        SubsonicAPI.PlaylistWithSongs("1", "test1", songs=songs),
+    )
+
+    cache_adapter.ingest_new_data(
+        FilesystemAdapter.CachedDataKey.SONG_FILE, ("1",), MOCK_SONG_FILE
+    )
+    cache_adapter.delete_data(FilesystemAdapter.CachedDataKey.SONG_FILE, ("1",))
+
+    try:
+        cache_adapter.get_song_uri("1", "file")
+        assert 0, "DID NOT raise CacheMissError"
+    except CacheMissError as e:
+        assert e.partial_data is None

tests/adapter_tests/mock_data/README

@@ -0,0 +1,2 @@
+test-song.mp3 was originally named Happy_Music-2018-09-18_-_Beautiful_Memories_-_David_Fesliyan.mp3
+which is royalty free music from https://www.fesliyanstudios.com