Add deprecation warnings to direct users towards using cache_handler (#630)

* Refactor functions into static methods of AuthBase

Functions is_token_expired was a loose function that was added to
SpotifyClientCredentials, SpotifyPKCE, and SpotifyImplicitGrant classes
through a method on each class that passed the call to the loose
is_token_expired function. Function _is_scope_subset was duplicated on
SpotifyClientCredentials, SpotifyPKCE, and SpotifyImplicitGrant classes.

Refactoring is_token_expired and _is_scope_subset to be static methods
on SpotifyAuthBase means both are available for all derived classes and
require less boilerplate.

* Create CacheHandler to abstract caching tokens

Previous code only supported caching to and from json files in a given
directory. In addition, the get_cached_token method mixed getting and
getting the token in the same method.

This change creates a CacheHandler class to abstract out the caching
implementation and allow the user to cache tokens in any way they
see fit. For example, the user could create a MongoCache class to store
and retrieve tokens from a Mongo database and specify that
cache_handler=MongoCache in creating an auth_manager object.

To implement the CacheHandler abstraction, the following changes are
implemented:

The validation code in each get_cached_token method in SpotifyOAuth,
SpotifyPKCE, and SpotifyImplicitGrant is moved into a validate_token
method in each class.

The CacheHandler class is created with get_cached_token and
save_token_to_cache methods.

Previous instances of self.get_cached_token() are now replaced with
self.validate_token(self.cache_handler.get_cached_token()) to preserve
the getting and validation behaviour.

cache_handler is added as an argument to SpotifyOAuth, SpotifyPKCE, and
SpotifyImplicitGrant. Specifying a cache_handler now overrides any
specification of cache_path and/or username.

To preserve backwards compatibility in handling cache files, a
CacheFileHandler class extending CacheHandler is created. If no
cache_handler is specified, the cache_path and username arguments are
used to create an instance of CacheFileHandler. It may be worth
deprecating the cache_path and username fields in favour of using
CacheFileHandler.

Tests are also modified and extended to cover the new functionality. A
sample MemoryCache CacheHandler is created to test getting and saving to
a custom CacheHandler.

* Fix cache_handler subclass check for Python 2

* Split assert message to fix line over max length

* Split cache handlers into cache_handler.py

* flake8 and autopep fixes

* Fix init to allow importing CacheHandler

When spotipy is installed as a package, CacheHandler is not accessible
from a `from spotipy import CacheHandler` statement because the import
is not specified in the __init__.py file. This commit adds CacheHandler
and CacheFileHandler to the init file so the user can import them.

* flake8 fix

* Add cache_path & username deprecation warning

When cache_path or username are specified in the constructors of
SpotifyOAuth, SpotifyPKCE, or SpotifyImplicitGrant, the constructor
creates a CacheFileHandler instance under the hood. The user is
currently able to create a CacheFileHandler instance in two ways:

1.  By creating it outside the SpotifyOAuth (or etc.) constructor and
    passing it as the cache_handler
2.  By passing cache_path and username to the constructor

Ideally, there would be one and only one obvious way to specify a
CacheFileHandler instance and the cache_handler approach allows any
CacheHandler to be used, so passing the cache_path or username to the
constructor should be deprecated.

* Update flask example to use CacheFileHandler

* Update changelog with deprecation warning info

* Restore token caching methods on auth_manager

Change 9550c8fd86 in
https://github.com/plamere/spotipy accidentally broke the caching
functionality in SpotifyOAUth, SpotifyPKCE, and SpotifyImplicitGrant by
removing the get_cached_token and _save_token_info methods from the
auth_manager object. Users with existing codebases that use the
get_cached_token and _save_token_info methods directly will experience
errors if they upgrade spotipy.

This commit restores the get_cached_token and _save_token_info methods
on the three auth_manager classes as aliases for the corresponding
methods in the cache_handler. Deprecation warnings are also added to
the get_cached_token and _save_token_info methods to direct users to
switch to using the new cache_handler approach.

* Add deprecation warning to docstrings

* Rearrange depr. warn for cache_path & username

Rearrange logic so that deprecation warning always triggers if
cache_path or username are specified. Previously, if cache_handler was
specified, no deprecation warning would be raised if cache_path or
username were specified.

In addition, if both cache_handler and cache_path or username are
specified, a new warning will be raised alongside the deprecation
warning to let the user know that the cache_path and username fields
will be ignored in favour of the cache_handler.
This commit is contained in:
Tony Jackson authored and GitHub committed 2021-01-13 11:34:53 +01:00
1 parent 44970c3348
commit a7b25d0f6e
4 files changed
+185 -17

No files matched your search

+51
View File
@@ -79,9 +79,11 @@ class OAuthCacheTest(unittest.TestCase):
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)
self.assertIsNotNone(cached_tok)
self.assertIsNotNone(cached_tok_legacy)
self.assertEqual(refresh_access_token.call_count, 0)
@patch.multiple(SpotifyOAuth,
@@ -140,6 +142,21 @@ class OAuthCacheTest(unittest.TestCase):
opener.assert_called_with(path, 'w')
self.assertTrue(fi.write.called)
@patch('spotipy.cache_handler.open', create=True)
def test_saves_to_cache_path_legacy(self, opener):
scope = "playlist-modify-private"
path = ".cache-username"
tok = _make_fake_token(1, 1, scope)
fi = _fake_file()
opener.return_value = fi
spot = SpotifyOAuth("CLID", "CLISEC", "REDIR", "STATE", scope, path)
spot._save_token_info(tok)
opener.assert_called_with(path, 'w')
self.assertTrue(fi.write.called)
def test_cache_handler(self):
scope = "playlist-modify-private"
tok = _make_fake_token(1, 1, scope)
@@ -258,9 +275,11 @@ class ImplicitGrantCacheTest(unittest.TestCase):
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)
self.assertIsNotNone(cached_tok)
self.assertIsNotNone(cached_tok_legacy)
@patch.object(SpotifyImplicitGrant, "is_token_expired", DEFAULT)
@patch('spotipy.cache_handler.open', create=True)
@@ -311,6 +330,21 @@ class ImplicitGrantCacheTest(unittest.TestCase):
opener.assert_called_with(path, 'w')
self.assertTrue(fi.write.called)
@patch('spotipy.cache_handler.open', create=True)
def test_saves_to_cache_path_legacy(self, opener):
scope = "playlist-modify-private"
path = ".cache-username"
tok = _make_fake_token(1, 1, scope)
fi = _fake_file()
opener.return_value = fi
spot = SpotifyImplicitGrant("CLID", "REDIR", "STATE", scope, path)
spot._save_token_info(tok)
opener.assert_called_with(path, 'w')
self.assertTrue(fi.write.called)
class TestSpotifyImplicitGrant(unittest.TestCase):
@@ -378,9 +412,11 @@ class SpotifyPKCECacheTest(unittest.TestCase):
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)
self.assertIsNotNone(cached_tok)
self.assertIsNotNone(cached_tok_legacy)
self.assertEqual(refresh_access_token.call_count, 0)
@patch.multiple(SpotifyPKCE,
@@ -439,6 +475,21 @@ class SpotifyPKCECacheTest(unittest.TestCase):
opener.assert_called_with(path, 'w')
self.assertTrue(fi.write.called)
@patch('spotipy.cache_handler.open', create=True)
def test_saves_to_cache_path_legacy(self, opener):
scope = "playlist-modify-private"
path = ".cache-username"
tok = _make_fake_token(1, 1, scope)
fi = _fake_file()
opener.return_value = fi
spot = SpotifyPKCE("CLID", "REDIR", "STATE", scope, path)
spot._save_token_info(tok)
opener.assert_called_with(path, 'w')
self.assertTrue(fi.write.called)
class TestSpotifyPKCE(unittest.TestCase):