Changed cache_handler.py to utilize Python's Context Management Protocol (#991)

.github/workflows/integration_tests.yml

@@ -4,7 +4,6 @@ on: [push, pull_request]
 
 jobs:
   build:
-
     runs-on: ubuntu-latest
     env:
       SPOTIPY_CLIENT_ID: ${{ secrets.SPOTIPY_CLIENT_ID }}

.github/workflows/unit_tests.yml

@@ -4,7 +4,6 @@ on: [push, pull_request]
 
 jobs:
   build:
-
     runs-on: ubuntu-20.04
     strategy:
       matrix:

CHANGELOG.md

@@ -6,22 +6,31 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## Unreleased
+
 Add your changes below.
 
 ### Added
+
 - Added examples for audiobooks, shows and episodes methods to examples directory
 - Use newer string formatters (https://pyformat.info)
 - Marked `recommendation_genre_seeds` as deprecated
 
 ### Fixed
+
 - Fixed scripts in examples directory that didn't run correctly
 - Updated documentation for `Client.current_user_top_artists` to indicate maximum number of artists limit
 
+### Changed
+
+- Updated get_cached_token and save_token_to_cache methods to utilize Python's Context Management Protocol
+- Added except clause to get_cached_token method to handle json decode errors
+
 ### Removed
 
 ## [2.25.0] - 2025-03-01
 
 ### Added
+
 - Added unit tests for queue functions
 - Added detailed function docstrings to 'util.py', including descriptions and special sections that lists arguments, returns, and raises.
 - Updated order of instructions for Python and pip package manager installation in TUTORIAL.md
@@ -44,44 +53,57 @@ Add your changes below.
 ### Changed
 
 - Split test and lint workflows
+- Updated get_cached_token and save_token_to_cache methods to utilize Python's Context Management Protocol
+- Added except clause to get_cached_token method to handle json decode errors
 
 ### Fixed
+
 - Audiobook integration tests
 - Edited docstrings for certain functions in client.py for functions that are no longer in use and have been replaced.
 - `current_user_unfollow_playlist()` now supports playlist IDs, URLs, and URIs rather than previously where it only supported playlist IDs.
 
 ### Removed
+
 - `mock` no longer listed as a test dependency. Only built-in `unittest.mock` is actually used.
 
 ## [2.24.0] - 2024-05-30
 
 ### Added
+
 - Added `MemcacheCacheHandler`, a cache handler that stores the token info using pymemcache.
 - Added support for audiobook endpoints: `get_audiobook`, `get_audiobooks`, and `get_audiobook_chapters`.
 - Added integration tests for audiobook endpoints.
 - Added `update` field to `current_user_follow_playlist`.
 
 ### Changed
+
+- Updated get_cached_token and save_token_to_cache methods to utilize Python's Context Management Protocol
+- Added except clause to get_cached_token method to handle json decode errors
+
 - Fixed error obfuscation when Spotify class is being inherited and an error is raised in the Child's `__init__`
 - Replaced `artist_albums(album_type=...)` with `artist_albums(include_groups=...)` due to an API change.
 - Updated `_regex_spotify_url` to ignore `/intl-<countrycode>` in Spotify links
 - Improved README, docs and examples
 
 ### Fixed
+
 - Readthedocs build
 - Split `test_current_user_save_and_usave_tracks` unit test
 
 ### Removed
+
 - Drop support for EOL Python 3.7
 
 ## [2.23.0] - 2023-04-07
 
 ### Added
+
 - Added optional `encoder_cls` argument to `CacheFileHandler`, which overwrite default encoder for token before writing to disk
 - Integration tests for searching multiple types in multiple markets (non-user endpoints)
 - Publish to PyPI action
 
 ### Fixed
+
 - Fixed the regex for matching playlist URIs with the format spotify:user:USERNAME:playlist:PLAYLISTID.
 - `search_markets` now factors the counts of all types in the `total` rather than just the first type ([#534](https://github.com/spotipy-dev/spotipy/issues/534))
 
@@ -466,6 +488,7 @@ Add your changes below.
 ### Changed
 
 - Made instructions in the CONTRIBUTING.md file more clear such that it is easier to onboard and there are no conflicts with TUTORIAL.md
+
 ## [2.5.0] - 2020-01-11
 
 Added follow and player endpoints

examples

@@ -1 +0,0 @@
-Subproject commit c610a79705ef4aa55e4d61572a012f77b6f7245d

spotipy/cache_handler.py

@@ -41,7 +41,6 @@ class CacheHandler():
         Save a token_info dictionary object to the cache and return None.
         """
         raise NotImplementedError()
-        return None
 
 
 class CacheFileHandler(CacheHandler):
@@ -77,24 +76,24 @@ class CacheFileHandler(CacheHandler):
         token_info = None
 
         try:
-            f = open(self.cache_path)
+            with open(self.cache_path, encoding='utf-8') as f:
                 token_info_string = f.read()
-            f.close()
             token_info = json.loads(token_info_string)
 
         except OSError as error:
             if error.errno == errno.ENOENT:
                 logger.debug(f"cache does not exist at: {self.cache_path}")
             else:
-                logger.warning(f"Couldn't read cache at: {self.cache_path}")
+                logger.warning("Couldn't read cache at: %s", self.cache_path)
+        except json.JSONDecodeError:
+            logger.warning("Couldn't decode JSON from cache at: %s", self.cache_path)
 
         return token_info
 
     def save_token_to_cache(self, token_info):
         try:
-            f = open(self.cache_path, "w")
+            with open(self.cache_path, "w", encoding='utf-8') as f:
                 f.write(json.dumps(token_info, cls=self.encoder_cls))
-            f.close()
         except OSError:
             logger.warning(f"Couldn't write token to cache at: {self.cache_path}")
 
@@ -214,6 +213,7 @@ class RedisCacheHandler(CacheHandler):
 class MemcacheCacheHandler(CacheHandler):
     """A Cache handler that stores the token info in Memcache using the pymemcache client
     """
+
     def __init__(self, memcache, key=None) -> None:
         """
         Parameters:

spotipy/util.py

@@ -153,6 +153,7 @@ class Retry(urllib3.Retry):
     """
     Custom class for printing a warning when a rate/request limit is reached.
     """
+
     def increment(
             self,
             method: str | None = None,

tests/unit/test_oauth.py

@@ -52,18 +52,21 @@ class OAuthCacheTest(unittest.TestCase):
     @patch('spotipy.cache_handler.open', create=True)
     def test_gets_from_cache_path(self, opener,
                                   is_token_expired, refresh_access_token):
+        """Test that the token is retrieved from the cache path."""
         scope = "playlist-modify-private"
         path = ".cache-username"
         tok = _make_fake_token(1, 1, scope)
-
-        opener.return_value = _token_file(json.dumps(tok, ensure_ascii=False))
+        token_file = _token_file(json.dumps(tok, ensure_ascii=False))
+        opener.return_value = token_file
+        opener.return_value.__enter__ = mock.Mock(return_value=token_file)
+        opener.return_value.__exit__ = mock.Mock(return_value=False)
         is_token_expired.return_value = False
 
         spot = _make_oauth(scope, path)
         cached_tok = spot.validate_token(spot.cache_handler.get_cached_token())
         cached_tok_legacy = spot.get_cached_token()
 
-        opener.assert_called_with(path)
+        opener.assert_called_with(path, encoding='utf-8')
         self.assertIsNotNone(cached_tok)
         self.assertIsNotNone(cached_tok_legacy)
         self.assertEqual(refresh_access_token.call_count, 0)
@@ -73,13 +76,15 @@ class OAuthCacheTest(unittest.TestCase):
     @patch('spotipy.cache_handler.open', create=True)
     def test_expired_token_refreshes(self, opener,
                                      is_token_expired, refresh_access_token):
+        """Test that an expired token is refreshed."""
         scope = "playlist-modify-private"
         path = ".cache-username"
         expired_tok = _make_fake_token(0, None, scope)
         fresh_tok = _make_fake_token(1, 1, scope)
 
         token_file = _token_file(json.dumps(expired_tok, ensure_ascii=False))
-        opener.return_value = token_file
+        opener.return_value.__enter__ = mock.Mock(return_value=token_file)
+        opener.return_value.__exit__ = mock.Mock(return_value=False)
         refresh_access_token.return_value = fresh_tok
 
         spot = _make_oauth(scope, path)
@@ -87,7 +92,7 @@ class OAuthCacheTest(unittest.TestCase):
 
         is_token_expired.assert_called_with(expired_tok)
         refresh_access_token.assert_called_with(expired_tok['refresh_token'])
-        opener.assert_any_call(path)
+        opener.assert_any_call(path, encoding='utf-8')
 
     @patch.multiple(SpotifyOAuth,
                     is_token_expired=DEFAULT, refresh_access_token=DEFAULT)
@@ -99,29 +104,35 @@ class OAuthCacheTest(unittest.TestCase):
         path = ".cache-username"
         tok = _make_fake_token(1, 1, token_scope)
 
-        opener.return_value = _token_file(json.dumps(tok, ensure_ascii=False))
+        token_file = _token_file(json.dumps(tok, ensure_ascii=False))
+        opener.return_value = token_file
+        opener.return_value.__enter__ = mock.Mock(return_value=token_file)
+        opener.return_value.__exit__ = mock.Mock(return_value=False)
         is_token_expired.return_value = False
 
         spot = _make_oauth(requested_scope, path)
         cached_tok = spot.validate_token(spot.cache_handler.get_cached_token())
 
-        opener.assert_called_with(path)
+        opener.assert_called_with(path, encoding='utf-8')
         self.assertIsNone(cached_tok)
         self.assertEqual(refresh_access_token.call_count, 0)
 
     @patch('spotipy.cache_handler.open', create=True)
     def test_saves_to_cache_path(self, opener):
+        """Test that the token is saved to the cache path."""
         scope = "playlist-modify-private"
         path = ".cache-username"
         tok = _make_fake_token(1, 1, scope)
 
         fi = _fake_file()
         opener.return_value = fi
+        opener.return_value.__enter__ = mock.Mock(return_value=fi)
+        opener.return_value.__exit__ = mock.Mock(return_value=False)
 
         spot = SpotifyOAuth("CLID", "CLISEC", "REDIR", "STATE", scope, path)
         spot.cache_handler.save_token_to_cache(tok)
 
-        opener.assert_called_with(path, 'w')
+        opener.assert_called_with(path, 'w', encoding='utf-8')
         self.assertTrue(fi.write.called)
 
     @patch('spotipy.cache_handler.open', create=True)
@@ -132,11 +143,13 @@ class OAuthCacheTest(unittest.TestCase):
 
         fi = _fake_file()
         opener.return_value = fi
+        opener.return_value.__enter__ = mock.Mock(return_value=fi)
+        opener.return_value.__exit__ = mock.Mock(return_value=False)
 
         spot = SpotifyOAuth("CLID", "CLISEC", "REDIR", "STATE", scope, path)
         spot._save_token_info(tok)
 
-        opener.assert_called_with(path, 'w')
+        opener.assert_called_with(path, 'w', encoding='utf-8')
         self.assertTrue(fi.write.called)
 
     def test_cache_handler(self):
@@ -252,32 +265,38 @@ class ImplicitGrantCacheTest(unittest.TestCase):
         path = ".cache-username"
         tok = _make_fake_token(1, 1, scope)
 
-        opener.return_value = _token_file(json.dumps(tok, ensure_ascii=False))
+        token_file = _token_file(json.dumps(tok, ensure_ascii=False))
+        opener.return_value = token_file
+        opener.return_value.__enter__ = mock.Mock(return_value=token_file)
+        opener.return_value.__exit__ = mock.Mock(return_value=False)
         is_token_expired.return_value = False
 
         spot = _make_implicitgrantauth(scope, path)
         cached_tok = spot.cache_handler.get_cached_token()
         cached_tok_legacy = spot.get_cached_token()
 
-        opener.assert_called_with(path)
+        opener.assert_called_with(path, encoding='utf-8')
         self.assertIsNotNone(cached_tok)
         self.assertIsNotNone(cached_tok_legacy)
 
     @patch.object(SpotifyImplicitGrant, "is_token_expired", DEFAULT)
     @patch('spotipy.cache_handler.open', create=True)
     def test_expired_token_returns_none(self, opener, is_token_expired):
+        """Test that an expired token returns None."""
         scope = "playlist-modify-private"
         path = ".cache-username"
         expired_tok = _make_fake_token(0, None, scope)
 
         token_file = _token_file(json.dumps(expired_tok, ensure_ascii=False))
         opener.return_value = token_file
+        opener.return_value.__enter__ = mock.Mock(return_value=token_file)
+        opener.return_value.__exit__ = mock.Mock(return_value=False)
 
         spot = _make_implicitgrantauth(scope, path)
         cached_tok = spot.validate_token(spot.cache_handler.get_cached_token())
 
         is_token_expired.assert_called_with(expired_tok)
-        opener.assert_any_call(path)
+        opener.assert_any_call(path, encoding='utf-8')
         self.assertIsNone(cached_tok)
 
     @patch.object(SpotifyImplicitGrant, "is_token_expired", DEFAULT)
@@ -288,13 +307,16 @@ class ImplicitGrantCacheTest(unittest.TestCase):
         path = ".cache-username"
         tok = _make_fake_token(1, 1, token_scope)
 
-        opener.return_value = _token_file(json.dumps(tok, ensure_ascii=False))
+        token_file = _token_file(json.dumps(tok, ensure_ascii=False))
+        opener.return_value = token_file
+        opener.return_value.__enter__ = mock.Mock(return_value=token_file)
+        opener.return_value.__exit__ = mock.Mock(return_value=False)
         is_token_expired.return_value = False
 
         spot = _make_implicitgrantauth(requested_scope, path)
         cached_tok = spot.validate_token(spot.cache_handler.get_cached_token())
 
-        opener.assert_called_with(path)
+        opener.assert_called_with(path, encoding='utf-8')
         self.assertIsNone(cached_tok)
 
     @patch('spotipy.cache_handler.open', create=True)
@@ -306,10 +328,12 @@ class ImplicitGrantCacheTest(unittest.TestCase):
         fi = _fake_file()
         opener.return_value = fi
 
+        opener.return_value.__enter__ = mock.Mock(return_value=fi)
+        opener.return_value.__exit__ = mock.Mock(return_value=False)
         spot = SpotifyImplicitGrant("CLID", "REDIR", "STATE", scope, path)
         spot.cache_handler.save_token_to_cache(tok)
 
-        opener.assert_called_with(path, 'w')
+        opener.assert_called_with(path, 'w', encoding='utf-8')
         self.assertTrue(fi.write.called)
 
     @patch('spotipy.cache_handler.open', create=True)
@@ -320,11 +344,13 @@ class ImplicitGrantCacheTest(unittest.TestCase):
 
         fi = _fake_file()
         opener.return_value = fi
+        opener.return_value.__enter__ = mock.Mock(return_value=fi)
+        opener.return_value.__exit__ = mock.Mock(return_value=False)
 
         spot = SpotifyImplicitGrant("CLID", "REDIR", "STATE", scope, path)
         spot._save_token_info(tok)
 
-        opener.assert_called_with(path, 'w')
+        opener.assert_called_with(path, 'w', encoding='utf-8')
         self.assertTrue(fi.write.called)
 
 
@@ -389,14 +415,17 @@ class SpotifyPKCECacheTest(unittest.TestCase):
         path = ".cache-username"
         tok = _make_fake_token(1, 1, scope)
 
-        opener.return_value = _token_file(json.dumps(tok, ensure_ascii=False))
+        token_file = _token_file(json.dumps(tok, ensure_ascii=False))
+        opener.return_value = token_file
+        opener.return_value.__enter__ = mock.Mock(return_value=token_file)
+        opener.return_value.__exit__ = mock.Mock(return_value=False)
         is_token_expired.return_value = False
 
         spot = _make_pkceauth(scope, path)
         cached_tok = spot.cache_handler.get_cached_token()
         cached_tok_legacy = spot.get_cached_token()
 
-        opener.assert_called_with(path)
+        opener.assert_called_with(path, encoding='utf-8')
         self.assertIsNotNone(cached_tok)
         self.assertIsNotNone(cached_tok_legacy)
         self.assertEqual(refresh_access_token.call_count, 0)
@@ -412,7 +441,8 @@ class SpotifyPKCECacheTest(unittest.TestCase):
         fresh_tok = _make_fake_token(1, 1, scope)
 
         token_file = _token_file(json.dumps(expired_tok, ensure_ascii=False))
-        opener.return_value = token_file
+        opener.return_value.__enter__ = mock.Mock(return_value=token_file)
+        opener.return_value.__exit__ = mock.Mock(return_value=False)
         refresh_access_token.return_value = fresh_tok
 
         spot = _make_pkceauth(scope, path)
@@ -420,7 +450,7 @@ class SpotifyPKCECacheTest(unittest.TestCase):
 
         is_token_expired.assert_called_with(expired_tok)
         refresh_access_token.assert_called_with(expired_tok['refresh_token'])
-        opener.assert_any_call(path)
+        opener.assert_any_call(path, encoding='utf-8')
 
     @patch.multiple(SpotifyPKCE,
                     is_token_expired=DEFAULT, refresh_access_token=DEFAULT)
@@ -432,13 +462,16 @@ class SpotifyPKCECacheTest(unittest.TestCase):
         path = ".cache-username"
         tok = _make_fake_token(1, 1, token_scope)
 
-        opener.return_value = _token_file(json.dumps(tok, ensure_ascii=False))
+        token_file = _token_file(json.dumps(tok, ensure_ascii=False))
+        opener.return_value = token_file
+        opener.return_value.__enter__ = mock.Mock(return_value=token_file)
+        opener.return_value.__exit__ = mock.Mock(return_value=False)
         is_token_expired.return_value = False
 
         spot = _make_pkceauth(requested_scope, path)
         cached_tok = spot.validate_token(spot.cache_handler.get_cached_token())
 
-        opener.assert_called_with(path)
+        opener.assert_called_with(path, encoding='utf-8')
         self.assertIsNone(cached_tok)
         self.assertEqual(refresh_access_token.call_count, 0)
 
@@ -450,11 +483,12 @@ class SpotifyPKCECacheTest(unittest.TestCase):
 
         fi = _fake_file()
         opener.return_value = fi
-
+        opener.return_value.__enter__ = mock.Mock(return_value=fi)
+        opener.return_value.__exit__ = mock.Mock(return_value=False)
         spot = SpotifyPKCE("CLID", "REDIR", "STATE", scope, path)
         spot.cache_handler.save_token_to_cache(tok)
 
-        opener.assert_called_with(path, 'w')
+        opener.assert_called_with(path, 'w', encoding='utf-8')
         self.assertTrue(fi.write.called)
 
     @patch('spotipy.cache_handler.open', create=True)
@@ -465,11 +499,13 @@ class SpotifyPKCECacheTest(unittest.TestCase):
 
         fi = _fake_file()
         opener.return_value = fi
+        opener.return_value.__enter__ = mock.Mock(return_value=fi)
+        opener.return_value.__exit__ = mock.Mock(return_value=False)
 
         spot = SpotifyPKCE("CLID", "REDIR", "STATE", scope, path)
         spot._save_token_info(tok)
 
-        opener.assert_called_with(path, 'w')
+        opener.assert_called_with(path, 'w', encoding='utf-8')
         self.assertTrue(fi.write.called)