Replace class SpotifyAuthorize with @must_be_authorized (#506)

* @must_be_authorized decorator for functions in spotify_tools.py

* We don't need this

* Add tests

* Update CHANGELOG.md
This commit is contained in:
Ritiek Malhotra
2019-02-27 09:48:18 -08:00
committed by GitHub
parent 1767899a8a
commit ac7d42535f
8 changed files with 282 additions and 252 deletions

View File

@@ -13,7 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
-
### Fixed
-
- Fix writing playlist tracks to file ([@ritiek](https://github.com/ritiek)) (#506)
## [1.1.2] - 2019-02-10
### Changed

View File

@@ -202,12 +202,8 @@ class ListDownloader:
try:
track_dl = Downloader(raw_song, number=number)
track_dl.download_single()
except spotipy.client.SpotifyException:
# token expires after 1 hour
self._regenerate_token()
track_dl.download_single()
# detect network problems
except (urllib.request.URLError, TypeError, IOError) as e:
# detect network problems
self._cleanup(raw_song, e)
# TODO: remove this sleep once #397 is fixed
# wait 0.5 sec to avoid infinite looping
@@ -233,11 +229,6 @@ class ListDownloader:
with open(self.write_successful_file, "a") as f:
f.write("\n" + raw_song)
@staticmethod
def _regenerate_token():
log.debug("Token expired, generating new one and authorizing")
spotify_tools.refresh_token()
def _cleanup(self, raw_song, exception):
self.tracks.append(raw_song)
# remove the downloaded song from file

View File

@@ -51,7 +51,6 @@ def main():
internals.filter_path(const.args.folder)
youtube_tools.set_api_key()
spotify = spotify_tools.SpotifyAuthorize()
logzero.setup_default_logger(formatter=const._formatter, level=const.args.log_level)

View File

@@ -8,51 +8,53 @@ from logzero import logger as log
import pprint
import sys
import os
import functools
from spotdl import const
from spotdl import internals
# token = generate_token()
# spotify = spotipy.Spotify(auth=token)
spotify = None
class SpotifyAuthorize:
""" Class to handle all interactions with spotipy instance. """
def __init__(self):
self.client_id = const.args.spotify_client_id
self.client_secret = const.args.spotify_client_secret
token = self.generate_token()
self.spotify = spotipy.Spotify(auth=token)
def generate_token(self):
def generate_token():
""" Generate the token. """
credentials = oauth2.SpotifyClientCredentials(
client_id=self.client_id,
client_secret=self.client_secret,
client_id=const.args.spotify_client_id,
client_secret=const.args.spotify_client_secret,
)
token = credentials.get_access_token()
return token
def refresh_token(self):
""" Refresh expired token. """
new_token = self.generate_token()
self.spotify = spotipy.Spotify(auth=new_token)
def generate_metadata(self, raw_song):
def must_be_authorized(func, spotify=spotify):
def wrapper(*args, **kwargs):
global spotify
try:
assert spotify
return func(*args, **kwargs)
except (AssertionError, spotipy.client.SpotifyException):
token = generate_token()
spotify = spotipy.Spotify(auth=token)
return func(*args, **kwargs)
return wrapper
@must_be_authorized
def generate_metadata(raw_song):
""" Fetch a song's metadata from Spotify. """
if internals.is_spotify(raw_song):
# fetch track information directly if it is spotify link
log.debug("Fetching metadata for given track URL")
meta_tags = self.spotify.track(raw_song)
meta_tags = spotify.track(raw_song)
else:
# otherwise search on spotify and fetch information from first result
log.debug('Searching for "{}" on Spotify'.format(raw_song))
try:
meta_tags = self.spotify.search(raw_song, limit=1)["tracks"]["items"][0]
meta_tags = spotify.search(raw_song, limit=1)["tracks"]["items"][0]
except IndexError:
return None
artist = self.spotify.artist(meta_tags["artists"][0]["id"])
album = self.spotify.album(meta_tags["album"]["id"])
artist = spotify.artist(meta_tags["artists"][0]["id"])
album = spotify.album(meta_tags["album"]["id"])
try:
meta_tags[u"genre"] = titlecase(artist["genres"][0])
@@ -92,15 +94,19 @@ class SpotifyAuthorize:
log.debug(pprint.pformat(meta_tags))
return meta_tags
def write_user_playlist(self, username, text_file=None):
""" Write user playlists to text_file """
links = self.get_playlists(username=username)
playlist = internals.input_link(links)
return self.write_playlist(playlist, text_file)
def get_playlists(self, username):
@must_be_authorized
def write_user_playlist(username, text_file=None):
""" Write user playlists to text_file """
links = get_playlists(username=username)
playlist = internals.input_link(links)
return write_playlist(playlist, text_file)
@must_be_authorized
def get_playlists(username):
""" Fetch user playlists when using the -u option. """
playlists = self.spotify.user_playlists(username)
playlists = spotify.user_playlists(username)
links = []
check = 1
@@ -119,13 +125,15 @@ class SpotifyAuthorize:
links.append(playlist_url)
check += 1
if playlists["next"]:
playlists = self.spotify.next(playlists)
playlists = spotify.next(playlists)
else:
break
return links
def fetch_playlist(self, playlist):
@must_be_authorized
def fetch_playlist(playlist):
try:
playlist_id = internals.extract_spotify_id(playlist)
except IndexError:
@@ -133,7 +141,7 @@ class SpotifyAuthorize:
log.error("The provided playlist URL is not in a recognized format!")
sys.exit(10)
try:
results = self.spotify.user_playlist(
results = spotify.user_playlist(
user=None, playlist_id=playlist_id, fields="tracks,next,name"
)
except spotipy.client.SpotifyException:
@@ -143,19 +151,25 @@ class SpotifyAuthorize:
return results
def write_playlist(self, playlist_url, text_file=None):
playlist = self.fetch_playlist(playlist_url)
@must_be_authorized
def write_playlist(playlist_url, text_file=None):
playlist = fetch_playlist(playlist_url)
tracks = playlist["tracks"]
if not text_file:
text_file = u"{0}.txt".format(slugify(playlist["name"], ok="-_()[]{}"))
return self.write_tracks(tracks, text_file)
return write_tracks(tracks, text_file)
def fetch_album(self, album):
@must_be_authorized
def fetch_album(album):
album_id = internals.extract_spotify_id(album)
album = self.spotify.album(album_id)
album = spotify.album(album_id)
return album
def fetch_albums_from_artist(self, artist_url, album_type=None):
@must_be_authorized
def fetch_albums_from_artist(artist_url, album_type=None):
"""
This funcction returns all the albums from a give artist_url using the US
market
@@ -168,19 +182,20 @@ class SpotifyAuthorize:
# fetching artist's albums limitting the results to the US to avoid duplicate
# albums from multiple markets
artist_id = internals.extract_spotify_id(artist_url)
results = self.spotify.artist_albums(artist_id, album_type=album_type, country="US")
results = spotify.artist_albums(artist_id, album_type=album_type, country="US")
albums = results["items"]
# indexing all pages of results
while results["next"]:
results = self.spotify.next(results)
results = spotify.next(results)
albums.extend(results["items"])
return albums
def write_all_albums_from_artist(self, artist_url, text_file=None):
@must_be_authorized
def write_all_albums_from_artist(artist_url, text_file=None):
"""
This function gets all albums from an artist and writes it to a file in the
current working directory called [ARTIST].txt, where [ARTIST] is the artist
@@ -192,7 +207,7 @@ class SpotifyAuthorize:
album_base_url = "https://open.spotify.com/album/"
# fetching all default albums
albums = self.fetch_albums_from_artist(artist_url, album_type=None)
albums = fetch_albums_from_artist(artist_url, album_type=None)
# if no file if given, the default save file is in the current working
# directory with the name of the artist
@@ -202,16 +217,20 @@ class SpotifyAuthorize:
for album in albums:
# logging album name
log.info("Fetching album: " + album["name"])
self.write_album(album_base_url + album["id"], text_file=text_file)
write_album(album_base_url + album["id"], text_file=text_file)
def write_album(self, album_url, text_file=None):
album = self.fetch_album(album_url)
tracks = self.spotify.album_tracks(album["id"])
@must_be_authorized
def write_album(album_url, text_file=None):
album = fetch_album(album_url)
tracks = spotify.album_tracks(album["id"])
if not text_file:
text_file = u"{0}.txt".format(slugify(album["name"], ok="-_()[]{}"))
return self.write_tracks(tracks, text_file)
return write_tracks(tracks, text_file)
def write_tracks(self, tracks, text_file):
@must_be_authorized
def write_tracks(tracks, text_file):
log.info(u"Writing {0} tracks to {1}".format(tracks["total"], text_file))
track_urls = []
with open(text_file, "a") as file_out:
@@ -235,7 +254,7 @@ class SpotifyAuthorize:
# 1 page = 50 results
# check if there are more pages
if tracks["next"]:
tracks = self.spotify.next(tracks)
tracks = spotify.next(tracks)
else:
break
return track_urls

View File

@@ -48,7 +48,6 @@ def go_pafy(raw_song, meta_tags=None):
def match_video_and_metadata(track):
""" Get and match track data from YouTube and Spotify. """
meta_tags = None
spotify = spotify_tools.SpotifyAuthorize()
def fallback_metadata(meta_tags):
@@ -68,13 +67,13 @@ def match_video_and_metadata(track):
content = go_pafy(track, meta_tags=None)
track = slugify(content.title).replace("-", " ")
if not const.args.no_metadata:
meta_tags = spotify.generate_metadata(track)
meta_tags = spotify_tools.generate_metadata(track)
meta_tags = fallback_metadata(meta_tags)
elif internals.is_spotify(track):
log.debug("Input song is a Spotify URL")
# Let it generate metadata, YouTube doesn't know Spotify slang
meta_tags = spotify.generate_metadata(track)
meta_tags = spotify_tools.generate_metadata(track)
content = go_pafy(track, meta_tags)
if const.args.no_metadata:
meta_tags = None
@@ -84,7 +83,7 @@ def match_video_and_metadata(track):
if const.args.no_metadata:
content = go_pafy(track, meta_tags=None)
else:
meta_tags = spotify.generate_metadata(track)
meta_tags = spotify_tools.generate_metadata(track)
content = go_pafy(track, meta_tags=meta_tags)
meta_tags = fallback_metadata(meta_tags)

View File

@@ -33,7 +33,7 @@ def pytest_namespace():
@pytest.fixture(scope="module")
def metadata_fixture():
meta_tags = spotify_tools.SpotifyAuthorize().generate_metadata(SPOTIFY_TRACK_URL)
meta_tags = spotify_tools.generate_metadata(SPOTIFY_TRACK_URL)
return meta_tags

View File

@@ -1,4 +1,7 @@
from spotdl import spotify_tools
from spotdl import const
import spotipy
import os
import pytest
@@ -6,26 +9,45 @@ import loader
loader.load_defaults()
@pytest.fixture(scope="module")
def spotify():
return spotify_tools.SpotifyAuthorize()
def test_generate_token(spotify):
token = spotify.generate_token()
def test_generate_token():
token = spotify_tools.generate_token()
assert len(token) == 83
def test_refresh_token(spotify):
old_instance = spotify.spotify
spotify.refresh_token()
new_instance = spotify.spotify
assert not old_instance == new_instance
class TestMustBeAuthorizedDecorator:
def test_spotify_instance_is_unset(self):
spotify_tools.spotify = None
@spotify_tools.must_be_authorized
def sample_func():
return True
assert sample_func()
def test_spotify_instance_forces_assertion_error(self):
@spotify_tools.must_be_authorized
def sample_func():
raise AssertionError
with pytest.raises(AssertionError):
sample_func()
def test_fake_token_generator(self, monkeypatch):
spotify_tools.spotify = None
monkeypatch.setattr(spotify_tools, "generate_token", lambda: 123123)
with pytest.raises(spotipy.client.SpotifyException):
spotify_tools.generate_metadata("ncs - spectre")
def test_correct_token(self):
assert spotify_tools.generate_metadata("ncs - spectre")
class TestGenerateMetadata:
@pytest.fixture(scope="module")
def metadata_fixture(self, spotify):
metadata = spotify.generate_metadata("ncs - spectre")
def metadata_fixture(self):
metadata = spotify_tools.generate_metadata("ncs - spectre")
return metadata
def test_len(self, metadata_fixture):
@@ -41,7 +63,7 @@ class TestGenerateMetadata:
assert metadata_fixture["duration"] == 230.634
def test_get_playlists(spotify):
def test_get_playlists():
expect_playlist_ids = [
"34gWCK8gVeYDPKcctB6BQJ",
"04wTU2c2WNQG9XE5oSLYfj",
@@ -53,15 +75,15 @@ def test_get_playlists(spotify):
for playlist_id in expect_playlist_ids
]
playlists = spotify.get_playlists("uqlakumu7wslkoen46s5bulq0")
playlists = spotify_tools.get_playlists("uqlakumu7wslkoen46s5bulq0")
assert playlists == expect_playlists
def test_write_user_playlist(tmpdir, spotify, monkeypatch):
def test_write_user_playlist(tmpdir, monkeypatch):
expect_tracks = 17
text_file = os.path.join(str(tmpdir), "test_us.txt")
monkeypatch.setattr("builtins.input", lambda x: 1)
spotify.write_user_playlist("uqlakumu7wslkoen46s5bulq0", text_file)
spotify_tools.write_user_playlist("uqlakumu7wslkoen46s5bulq0", text_file)
with open(text_file, "r") as f:
tracks = len(f.readlines())
assert tracks == expect_tracks
@@ -69,8 +91,8 @@ def test_write_user_playlist(tmpdir, spotify, monkeypatch):
class TestFetchPlaylist:
@pytest.fixture(scope="module")
def playlist_fixture(self, spotify):
playlist = spotify.fetch_playlist(
def playlist_fixture(self):
playlist = spotify_tools.fetch_playlist(
"https://open.spotify.com/playlist/0fWBMhGh38y0wsYWwmM9Kt"
)
return playlist
@@ -82,10 +104,10 @@ class TestFetchPlaylist:
assert playlist_fixture["tracks"]["total"] == 14
def test_write_playlist(tmpdir, spotify):
def test_write_playlist(tmpdir):
expect_tracks = 14
text_file = os.path.join(str(tmpdir), "test_pl.txt")
spotify.write_playlist(
spotify_tools.write_playlist(
"https://open.spotify.com/playlist/0fWBMhGh38y0wsYWwmM9Kt", text_file
)
with open(text_file, "r") as f:
@@ -96,8 +118,8 @@ def test_write_playlist(tmpdir, spotify):
# XXX: Mock this test off if it fails in future
class TestFetchAlbum:
@pytest.fixture(scope="module")
def album_fixture(self, spotify):
album = spotify.fetch_album(
def album_fixture(self):
album = spotify_tools.fetch_album(
"https://open.spotify.com/album/499J8bIsEnU7DSrosFDJJg"
)
return album
@@ -112,8 +134,8 @@ class TestFetchAlbum:
# XXX: Mock this test off if it fails in future
class TestFetchAlbumsFromArtist:
@pytest.fixture(scope="module")
def albums_from_artist_fixture(self, spotify):
albums = spotify.fetch_albums_from_artist(
def albums_from_artist_fixture(self):
albums = spotify_tools.fetch_albums_from_artist(
"https://open.spotify.com/artist/7oPftvlwr6VrsViSDV7fJY"
)
return albums
@@ -134,10 +156,10 @@ class TestFetchAlbumsFromArtist:
assert albums_from_artist_fixture[0]["total_tracks"] == 12
def test_write_all_albums_from_artist(tmpdir, spotify):
def test_write_all_albums_from_artist(tmpdir):
expect_tracks = 282
text_file = os.path.join(str(tmpdir), "test_ab.txt")
spotify.write_all_albums_from_artist(
spotify_tools.write_all_albums_from_artist(
"https://open.spotify.com/artist/4dpARuHxo51G3z768sgnrY", text_file
)
with open(text_file, "r") as f:
@@ -145,10 +167,10 @@ def test_write_all_albums_from_artist(tmpdir, spotify):
assert tracks == expect_tracks
def test_write_album(tmpdir, spotify):
def test_write_album(tmpdir):
expect_tracks = 15
text_file = os.path.join(str(tmpdir), "test_al.txt")
spotify.write_album(
spotify_tools.write_album(
"https://open.spotify.com/album/499J8bIsEnU7DSrosFDJJg", text_file
)
with open(text_file, "r") as f:

View File

@@ -40,7 +40,7 @@ class TestYouTubeAPIKeys:
@pytest.fixture(scope="module")
def metadata_fixture():
metadata = spotify_tools.SpotifyAuthorize().generate_metadata(TRACK_SEARCH)
metadata = spotify_tools.generate_metadata(TRACK_SEARCH)
return metadata