Allow cache misses to include partial data

sublime/adapters/adapter_base.py

@@ -21,10 +21,25 @@ from .api_objects import (
 
 class CacheMissError(Exception):
     """
-    This exception should be thrown by caching adapters when the request
-    data is not available or is invalid.
+    This exception should be thrown by caching adapters when the request data
+    is not available or is invalid. If some of the data is available, but not
+    all of it, the ``partial_data`` parameter should be set with the partial
+    data. If the ground truth adapter can't service the request, or errors for
+    some reason, the UI will try to populate itself with the partial data
+    returned in this exception (with the necessary error text to inform the
+    user that retrieval from the ground truth adapter failed).
     """
-    pass
+    def __init__(self, *args, partial_data: Any = None):
+        """
+        Create a :class:`CacheMissError` exception.
+
+        :param args: arguments to pass to the :class:`BaseException` base
+            class.
+        :param partial_data: the actual partial data for the UI to use in case
+            of ground truth adapter failure.
+        """
+        self.partial_data = partial_data
+        super().__init__(*args)
 
 
 @dataclass

sublime/adapters/filesystem/adapter.py

@@ -62,8 +62,10 @@ class FilesystemAdapter(CachingAdapter):
     def get_playlists(self) -> Sequence[Playlist]:
         playlists = list(models.Playlist.select())
         if self.is_cache and len(playlists) == 0:
-            # Determine if the adapter has ingested data for get_playlists
-            # before. If not, cache miss.
+            # This does not necessary mean that we have a cache miss. It could
+            # just mean that the list of playlists is actually empty. Determine
+            # if the adapter has ingested data for get_playlists before, and if
+            # not, cache miss.
             function_name = CachingAdapter.FunctionNames.GET_PLAYLISTS
             if not models.CacheInfo.get_or_none(
                     models.CacheInfo.query_name == function_name):
@@ -78,11 +80,17 @@ class FilesystemAdapter(CachingAdapter):
     ) -> PlaylistDetails:
         playlist = models.Playlist.get_or_none(
             models.Playlist.id == playlist_id)
-        if not playlist:
-            if self.is_cache:
-                raise CacheMissError()
-            else:
-                raise Exception(f'Playlist {playlist_id} does not exist.')
+        if not playlist and not self.is_cache:
+            raise Exception(f'Playlist {playlist_id} does not exist.')
+
+        # If we haven't ingested data for this playlist before, raise a
+        # CacheMissError with the partial playlist data.
+        function_name = CachingAdapter.FunctionNames.GET_PLAYLIST_DETAILS
+        cache_info = models.CacheInfo.get_or_none(
+            models.CacheInfo.query_name == function_name,
+            params_hash=hash((playlist_id, ), ))
+        if not cache_info:
+            raise CacheMissError(partial_data=playlist)
 
         return playlist
 
@@ -100,6 +108,7 @@ class FilesystemAdapter(CachingAdapter):
 
         models.CacheInfo.insert(
             query_name=function,
+            params_hash=hash(params),
             last_ingestion_time=datetime.now(),
         ).on_conflict_replace().execute()
 

sublime/adapters/filesystem/models.py

@@ -209,6 +209,7 @@ class Song(BaseModel):
 
 class CacheInfo(BaseModel):
     query_name = CacheConstantsField(unique=True, primary_key=True)
+    params_hash = IntegerField(null=False)
     last_ingestion_time = TzDateTimeField(null=False)
 
 

tests/adapter_tests/filesystem_adapter_tests.py

@@ -47,10 +47,7 @@ def mock_data_files(
                 yield file, f.read()
 
 
-def test_caching_get_playlists(
-    cache_adapter: FilesystemAdapter,
-    tmp_path: Path,
-):
+def test_caching_get_playlists(cache_adapter: FilesystemAdapter):
     with pytest.raises(CacheMissError):
         cache_adapter.get_playlists()
 
@@ -79,7 +76,7 @@ def test_caching_get_playlists(
     assert (playlists[1].id, playlists[1].name) == ('2', 'test2')
 
 
-def test_no_caching_get_playlists(adapter: FilesystemAdapter, tmp_path: Path):
+def test_no_caching_get_playlists(adapter: FilesystemAdapter):
     adapter.get_playlists()
 
     # TODO: Create a playlist (that should be allowed only if this is acting as
@@ -90,10 +87,7 @@ def test_no_caching_get_playlists(adapter: FilesystemAdapter, tmp_path: Path):
     # TODO: verify playlist
 
 
-def test_caching_get_playlist_details(
-    cache_adapter: FilesystemAdapter,
-    tmp_path: Path,
-):
+def test_caching_get_playlist_details(cache_adapter: FilesystemAdapter):
     with pytest.raises(CacheMissError):
         cache_adapter.get_playlist_details('1')
 
@@ -135,24 +129,24 @@ def test_caching_get_playlist_details(
 
     # "Force refresh" the playlist
     songs = [
-        SubsonicAPI.Song(
-            '1',
-            'Song 1',
-            parent='foo',
-            album='foo',
-            artist='foo',
-            duration=timedelta(seconds=10.2),
-            path='/foo/song1.mp3',
-        ),
         SubsonicAPI.Song(
             '3',
             'Song 3',
             parent='foo',
             album='foo',
             artist='foo',
-            duration=timedelta(seconds=21.8),
+            duration=timedelta(seconds=10.2),
             path='/foo/song3.mp3',
         ),
+        SubsonicAPI.Song(
+            '1',
+            'Song 1',
+            parent='foo',
+            album='foo',
+            artist='foo',
+            duration=timedelta(seconds=21.8),
+            path='/foo/song1.mp3',
+        ),
     ]
     cache_adapter.ingest_new_data(
         FilesystemAdapter.FunctionNames.GET_PLAYLIST_DETAILS,
@@ -173,10 +167,7 @@ def test_caching_get_playlist_details(
         cache_adapter.get_playlist_details('2')
 
 
-def test_no_caching_get_playlist_details(
-    adapter: FilesystemAdapter,
-    tmp_path: Path,
-):
+def test_no_caching_get_playlist_details(adapter: FilesystemAdapter):
     with pytest.raises(Exception):
         adapter.get_playlist_details('1')
 
@@ -186,3 +177,25 @@ def test_no_caching_get_playlist_details(
 
     # adapter.get_playlist_details('1')
     # TODO: verify playlist details
+
+
+def test_caching_get_playlist_then_details(cache_adapter: FilesystemAdapter):
+    # Ingest a list of playlists (like the sidebar, without songs)
+    cache_adapter.ingest_new_data(
+        FilesystemAdapter.FunctionNames.GET_PLAYLISTS,
+        (),
+        [
+            SubsonicAPI.Playlist('1', 'test1', comment='comment'),
+            SubsonicAPI.Playlist('2', 'test2'),
+        ],
+    )
+
+    # Trying to get playlist details should generate a cache miss, but should
+    # include the data that we know about.
+    try:
+        cache_adapter.get_playlist_details('1')
+        assert False, 'DID NOT raise CacheMissError'
+    except CacheMissError as e:
+        assert e.partial_data
+        assert e.partial_data.id == '1'
+        assert e.partial_data.name == 'test1'