Skip to content

Commit

Permalink
Merge pull request #139 from mattfredwill/develop
Browse files Browse the repository at this point in the history
Make load calls respect configured timeout
  • Loading branch information
kingosticks authored Jun 7, 2017
2 parents 42a4365 + 9d375fd commit 7398c43
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 40 deletions.
37 changes: 21 additions & 16 deletions mopidy_spotify/browse.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ def browse(config, session, uri):
if uri == ROOT_DIR.uri:
return _ROOT_DIR_CONTENTS
elif uri.startswith('spotify:user:'):
return _browse_playlist(session, uri)
return _browse_playlist(session, uri, config)
elif uri.startswith('spotify:album:'):
return _browse_album(session, uri)
return _browse_album(session, uri, config)
elif uri.startswith('spotify:artist:'):
return _browse_artist(session, uri)
return _browse_artist(session, uri, config)
elif uri.startswith('spotify:top:'):
parts = uri.replace('spotify:top:', '').split(':')
if len(parts) == 1:
Expand All @@ -61,25 +61,28 @@ def browse(config, session, uri):
return []


def _browse_playlist(session, uri):
def _browse_playlist(session, uri, config):
sp_playlist = session.get_playlist(uri)
sp_playlist.load()
return list(translator.to_track_refs(sp_playlist.tracks))
sp_playlist.load(config['timeout'])
return list(translator.to_track_refs(
sp_playlist.tracks, timeout=config['timeout']))


def _browse_album(session, uri):
def _browse_album(session, uri, config):
sp_album_browser = session.get_album(uri).browse()
sp_album_browser.load()
return list(translator.to_track_refs(sp_album_browser.tracks))
sp_album_browser.load(config['timeout'])
return list(translator.to_track_refs(
sp_album_browser.tracks, timeout=config['timeout']))


def _browse_artist(session, uri):
def _browse_artist(session, uri, config):
sp_artist_browser = session.get_artist(uri).browse(
type=spotify.ArtistBrowserType.NO_TRACKS)
sp_artist_browser.load()
sp_artist_browser.load(config['timeout'])
top_tracks = list(translator.to_track_refs(
sp_artist_browser.tophit_tracks))
albums = list(translator.to_album_refs(sp_artist_browser.albums))
sp_artist_browser.tophit_tracks, timeout=config['timeout']))
albums = list(translator.to_album_refs(
sp_artist_browser.albums, timeout=config['timeout']))
return top_tracks + albums


Expand Down Expand Up @@ -119,16 +122,18 @@ def _browse_toplist(config, session, variant, region):
return []

if session.connection.state is spotify.ConnectionState.LOGGED_IN:
sp_toplist.load()
sp_toplist.load(config['timeout'])

if not sp_toplist.is_loaded:
return []

if variant == 'tracks':
return list(translator.to_track_refs(sp_toplist.tracks))
elif variant == 'albums':
return list(translator.to_album_refs(sp_toplist.albums))
return list(translator.to_album_refs(
sp_toplist.albums, timeout=config['timeout']))
elif variant == 'artists':
return list(translator.to_artist_refs(sp_toplist.artists))
return list(translator.to_artist_refs(
sp_toplist.artists, timeout=config['timeout']))
else:
return []
4 changes: 2 additions & 2 deletions mopidy_spotify/distinct.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,10 @@ def _get_playlist_tracks(config, session):
for playlist in session.playlist_container:
if not isinstance(playlist, spotify.Playlist):
continue
playlist.load()
playlist.load(config['timeout'])
for track in playlist.tracks:
try:
track.load()
track.load(config['timeout'])
yield track
except spotify.Error: # TODO Why did we get "General error"?
continue
14 changes: 7 additions & 7 deletions mopidy_spotify/lookup.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def lookup(config, session, uri):

def _lookup_track(config, sp_link):
sp_track = sp_link.as_track()
sp_track.load()
sp_track.load(config['timeout'])
track = translator.to_track(sp_track, bitrate=config['bitrate'])
if track is not None:
yield track
Expand All @@ -54,7 +54,7 @@ def _lookup_track(config, sp_link):
def _lookup_album(config, sp_link):
sp_album = sp_link.as_album()
sp_album_browser = sp_album.browse()
sp_album_browser.load()
sp_album_browser.load(config['timeout'])
for sp_track in sp_album_browser.tracks:
track = translator.to_track(
sp_track, bitrate=config['bitrate'])
Expand All @@ -66,13 +66,13 @@ def _lookup_artist(config, sp_link):
sp_artist = sp_link.as_artist()
sp_artist_browser = sp_artist.browse(
type=spotify.ArtistBrowserType.NO_TRACKS)
sp_artist_browser.load()
sp_artist_browser.load(config['timeout'])

# Get all album browsers we need first, so they can start retrieving
# data in the background.
sp_album_browsers = []
for sp_album in sp_artist_browser.albums:
sp_album.load()
sp_album.load(config['timeout'])
if not sp_album.is_available:
continue
if sp_album.type is spotify.AlbumType.COMPILATION:
Expand All @@ -82,7 +82,7 @@ def _lookup_artist(config, sp_link):
sp_album_browsers.append(sp_album.browse())

for sp_album_browser in sp_album_browsers:
sp_album_browser.load()
sp_album_browser.load(config['timeout'])
for sp_track in sp_album_browser.tracks:
track = translator.to_track(
sp_track, bitrate=config['bitrate'])
Expand All @@ -92,9 +92,9 @@ def _lookup_artist(config, sp_link):

def _lookup_playlist(config, sp_link):
sp_playlist = sp_link.as_playlist()
sp_playlist.load()
sp_playlist.load(config['timeout'])
for sp_track in sp_playlist.tracks:
sp_track.load()
sp_track.load(config['timeout'])
track = translator.to_track(
sp_track, bitrate=config['bitrate'])
if track is not None:
Expand Down
3 changes: 2 additions & 1 deletion mopidy_spotify/playlists.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class SpotifyPlaylistsProvider(backend.PlaylistsProvider):

def __init__(self, backend):
self._backend = backend
self._timeout = self._backend._config['spotify']['timeout']

def as_list(self):
with utils.time_logger('playlists.as_list()'):
Expand Down Expand Up @@ -62,7 +63,7 @@ def _get_playlist(self, uri, as_items=False):
if not sp_playlist.is_loaded:
logger.debug(
'Waiting for Spotify playlist to load: %s', sp_playlist)
sp_playlist.load()
sp_playlist.load(self._timeout)

username = self._backend._session.user_name
return translator.to_playlist(
Expand Down
12 changes: 6 additions & 6 deletions mopidy_spotify/translator.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ def to_artist_ref(sp_artist):
return models.Ref.artist(uri=sp_artist.link.uri, name=sp_artist.name)


def to_artist_refs(sp_artists):
def to_artist_refs(sp_artists, timeout=None):
for sp_artist in sp_artists:
sp_artist.load()
sp_artist.load(timeout)
ref = to_artist_ref(sp_artist)
if ref is not None:
yield ref
Expand Down Expand Up @@ -88,9 +88,9 @@ def to_album_ref(sp_album):
return models.Ref.album(uri=sp_album.link.uri, name=name)


def to_album_refs(sp_albums):
def to_album_refs(sp_albums, timeout=None):
for sp_album in sp_albums:
sp_album.load()
sp_album.load(timeout)
ref = to_album_ref(sp_album)
if ref is not None:
yield ref
Expand Down Expand Up @@ -142,9 +142,9 @@ def to_track_ref(sp_track):
return models.Ref.track(uri=sp_track.link.uri, name=sp_track.name)


def to_track_refs(sp_tracks):
def to_track_refs(sp_tracks, timeout=None):
for sp_track in sp_tracks:
sp_track.load()
sp_track.load(timeout)
ref = to_track_ref(sp_track)
if ref is not None:
yield ref
Expand Down
12 changes: 6 additions & 6 deletions tests/test_lookup.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def test_lookup_of_track_uri(session_mock, sp_track_mock, provider):

session_mock.get_link.assert_called_once_with('spotify:track:abc')
sp_track_mock.link.as_track.assert_called_once_with()
sp_track_mock.load.assert_called_once_with()
sp_track_mock.load.assert_called_once_with(10)

assert len(results) == 1
track = results[0]
Expand All @@ -66,7 +66,7 @@ def test_lookup_of_album_uri(session_mock, sp_album_browser_mock, provider):
sp_album_mock.link.as_album.assert_called_once_with()

sp_album_mock.browse.assert_called_once_with()
sp_album_browser_mock.load.assert_called_once_with()
sp_album_browser_mock.load.assert_called_once_with(10)

assert len(results) == 2
track = results[0]
Expand All @@ -88,7 +88,7 @@ def test_lookup_of_artist_uri(

sp_artist_mock.browse.assert_called_once_with(
type=spotify.ArtistBrowserType.NO_TRACKS)
sp_artist_browser_mock.load.assert_called_once_with()
sp_artist_browser_mock.load.assert_called_once_with(10)

assert sp_album_mock.browse.call_count == 2
assert sp_album_browser_mock.load.call_count == 2
Expand Down Expand Up @@ -143,8 +143,8 @@ def test_lookup_of_playlist_uri(session_mock, sp_playlist_mock, provider):

session_mock.get_link.assert_called_once_with('spotify:playlist:alice:foo')
sp_playlist_mock.link.as_playlist.assert_called_once_with()
sp_playlist_mock.load.assert_called_once_with()
sp_playlist_mock.tracks[0].load.assert_called_once_with()
sp_playlist_mock.load.assert_called_once_with(10)
sp_playlist_mock.tracks[0].load.assert_called_once_with(10)

assert len(results) == 1
track = results[0]
Expand All @@ -160,7 +160,7 @@ def test_lookup_of_starred_uri(session_mock, sp_starred_mock, provider):

session_mock.get_link.assert_called_once_with('spotify:user:alice:starred')
sp_starred_mock.link.as_playlist.assert_called_once_with()
sp_starred_mock.load.assert_called_once_with()
sp_starred_mock.load.assert_called_once_with(10)

assert len(results) == 2
track = results[0]
Expand Down
5 changes: 3 additions & 2 deletions tests/test_playlists.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ def session_mock(


@pytest.fixture
def backend_mock(session_mock):
def backend_mock(session_mock, config):
backend_mock = mock.Mock(spec=backend.SpotifyBackend)
backend_mock._config = config
backend_mock._session = session_mock
backend_mock._bitrate = 160
return backend_mock
Expand Down Expand Up @@ -131,7 +132,7 @@ def test_lookup_loads_playlist_when_a_playlist_isnt_loaded(

playlist = provider.lookup('spotify:user:alice:playlist:foo')

sp_playlist_mock.load.assert_called_once_with()
sp_playlist_mock.load.assert_called_once_with(10)
assert playlist.uri == 'spotify:user:alice:playlist:foo'
assert playlist.name == 'Foo'

Expand Down

0 comments on commit 7398c43

Please sign in to comment.