From 18ed7c627951cbde418d02825e306fea138dc002 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 29 Jul 2013 00:08:32 +0200 Subject: [PATCH] local: Convert mopidy-local extension to local: uris. - Use local:playlist:, local:track: and local:search uris - Adds LocalPlaybackProvider which translates to file uris. - Switches to storing actual uris in playlists - so local: urls and not file:// or plain paths. - Moved file:// to streaming plugin - Cleaned up tests and imports for these changes. --- mopidy/backends/base.py | 3 +- mopidy/backends/local/actor.py | 5 ++-- mopidy/backends/local/library.py | 8 ++++-- mopidy/backends/local/playback.py | 19 +++++++++++++ mopidy/backends/local/playlists.py | 38 +++++++++++-------------- mopidy/backends/local/translator.py | 24 +++++++--------- mopidy/backends/stream/ext.conf | 1 + mopidy/core/playback.py | 1 + mopidy/frontends/mpd/dispatcher.py | 1 + mopidy/scanner.py | 3 +- mopidy/utils/path.py | 1 - tests/backends/base/library.py | 23 ++++++--------- tests/backends/local/__init__.py | 5 +--- tests/backends/local/playback_test.py | 15 +++++----- tests/backends/local/playlists_test.py | 12 ++++---- tests/backends/local/translator_test.py | 24 +++++++--------- tests/data/library_tag_cache | 12 ++++---- 17 files changed, 98 insertions(+), 97 deletions(-) create mode 100644 mopidy/backends/local/playback.py diff --git a/mopidy/backends/base.py b/mopidy/backends/base.py index 207edb3a..3c1bbbf0 100644 --- a/mopidy/backends/base.py +++ b/mopidy/backends/base.py @@ -157,7 +157,8 @@ class BasePlaybackProvider(object): """ self.audio.prepare_change() # TODO: add .get() to this? self.change_track(track) - return self.audio.start_playback().get() + self.audio.start_playback().get() + return True def change_track(self, track): """ diff --git a/mopidy/backends/local/actor.py b/mopidy/backends/local/actor.py index 8f53af4d..f3611891 100644 --- a/mopidy/backends/local/actor.py +++ b/mopidy/backends/local/actor.py @@ -10,6 +10,7 @@ from mopidy.utils import encoding, path from .library import LocalLibraryProvider from .playlists import LocalPlaylistsProvider +from .playback import LocalPlaybackProvider logger = logging.getLogger('mopidy.backends.local') @@ -23,10 +24,10 @@ class LocalBackend(pykka.ThreadingActor, base.Backend): self.check_dirs_and_files() self.library = LocalLibraryProvider(backend=self) - self.playback = base.BasePlaybackProvider(audio=audio, backend=self) + self.playback = LocalPlaybackProvider(audio=audio, backend=self) self.playlists = LocalPlaylistsProvider(backend=self) - self.uri_schemes = ['file'] + self.uri_schemes = ['local'] def check_dirs_and_files(self): if not os.path.isdir(self.config['local']['media_dir']): diff --git a/mopidy/backends/local/library.py b/mopidy/backends/local/library.py index c80e95fe..9dd112e9 100644 --- a/mopidy/backends/local/library.py +++ b/mopidy/backends/local/library.py @@ -81,7 +81,8 @@ class LocalLibraryProvider(base.BaseLibraryProvider): result_tracks = filter(any_filter, result_tracks) else: raise LookupError('Invalid lookup field: %s' % field) - return SearchResult(uri='file:search', tracks=result_tracks) + # TODO: add local:search: + return SearchResult(uri='local:search', tracks=result_tracks) def search(self, query=None, uris=None): # TODO Only return results within URI roots given by ``uris`` @@ -122,7 +123,8 @@ class LocalLibraryProvider(base.BaseLibraryProvider): result_tracks = filter(any_filter, result_tracks) else: raise LookupError('Invalid lookup field: %s' % field) - return SearchResult(uri='file:search', tracks=result_tracks) + # TODO: add local:search: + return SearchResult(uri='local:search', tracks=result_tracks) def _validate_query(self, query): for (_, values) in query.iteritems(): @@ -135,7 +137,7 @@ class LocalLibraryProvider(base.BaseLibraryProvider): # TODO: rename and move to tagcache extension. class LocalLibraryUpdateProvider(base.BaseLibraryProvider): - uri_schemes = ['file'] + uri_schemes = ['local'] def __init__(self, config): self._tracks = {} diff --git a/mopidy/backends/local/playback.py b/mopidy/backends/local/playback.py new file mode 100644 index 00000000..8c40cb9e --- /dev/null +++ b/mopidy/backends/local/playback.py @@ -0,0 +1,19 @@ +from __future__ import unicode_literals + +import logging +import os + +from mopidy.backends import base +from mopidy.utils import path + +logger = logging.getLogger('mopidy.backends.spotify') + + +class LocalPlaybackProvider(base.BasePlaybackProvider): + def change_track(self, track): + media_dir = self.backend.config['local']['media_dir'] + # TODO: check that type is correct. + file_path = path.uri_to_path(track.uri).split(':', 1)[1] + file_path = os.path.join(media_dir, file_path) + track = track.copy(uri=path.path_to_uri(file_path)) + return super(LocalPlaybackProvider, self).change_track(track) diff --git a/mopidy/backends/local/playlists.py b/mopidy/backends/local/playlists.py index cd370eaa..af3814ae 100644 --- a/mopidy/backends/local/playlists.py +++ b/mopidy/backends/local/playlists.py @@ -24,7 +24,7 @@ class LocalPlaylistsProvider(base.BasePlaylistsProvider): def create(self, name): name = formatting.slugify(name) - uri = path.path_to_uri(self._get_m3u_path(name)) + uri = 'local:playlist:%s.m3u' % name playlist = Playlist(uri=uri, name=name) return self.save(playlist) @@ -37,6 +37,7 @@ class LocalPlaylistsProvider(base.BasePlaylistsProvider): self._delete_m3u(playlist.uri) def lookup(self, uri): + # TODO: store as {uri: playlist}? for playlist in self._playlists: if playlist.uri == uri: return playlist @@ -45,8 +46,8 @@ class LocalPlaylistsProvider(base.BasePlaylistsProvider): playlists = [] for m3u in glob.glob(os.path.join(self._playlists_dir, '*.m3u')): - uri = path.path_to_uri(m3u) name = os.path.splitext(os.path.basename(m3u))[0] + uri = 'local:playlist:%s' % name tracks = [] for track_uri in parse_m3u(m3u, self._media_dir): @@ -61,6 +62,7 @@ class LocalPlaylistsProvider(base.BasePlaylistsProvider): playlists.append(playlist) self.playlists = playlists + # TODO: send what scheme we loaded them for? listener.BackendListener.send('playlists_loaded') logger.info( @@ -86,38 +88,30 @@ class LocalPlaylistsProvider(base.BasePlaylistsProvider): return playlist - def _get_m3u_path(self, name): - name = formatting.slugify(name) - file_path = os.path.join(self._playlists_dir, name + '.m3u') + def _m3u_uri_to_path(self, uri): + # TODO: create uri handling helpers for local uri types. + file_path = path.uri_to_path(uri).split(':', 1)[1] + file_path = os.path.join(self._playlists_dir, file_path) path.check_file_path_is_inside_base_dir(file_path, self._playlists_dir) return file_path def _save_m3u(self, playlist): - file_path = path.uri_to_path(playlist.uri) - path.check_file_path_is_inside_base_dir(file_path, self._playlists_dir) + file_path = self._m3u_uri_to_path(playlist.uri) with open(file_path, 'w') as file_handle: for track in playlist.tracks: - if track.uri.startswith('file://'): - uri = path.uri_to_path(track.uri) - else: - uri = track.uri - file_handle.write(uri + '\n') + file_handle.write(track.uri + '\n') def _delete_m3u(self, uri): - file_path = path.uri_to_path(uri) - path.check_file_path_is_inside_base_dir(file_path, self._playlists_dir) + file_path = self._m3u_uri_to_path(uri) if os.path.exists(file_path): os.remove(file_path) def _rename_m3u(self, playlist): - src_file_path = path.uri_to_path(playlist.uri) - path.check_file_path_is_inside_base_dir( - src_file_path, self._playlists_dir) + dst_name = formatting.slugify(playlist.name) + dst_uri = 'local:playlist:%s.m3u' % dst_name - dst_file_path = self._get_m3u_path(playlist.name) - path.check_file_path_is_inside_base_dir( - dst_file_path, self._playlists_dir) + src_file_path = self._m3u_uri_to_path(playlist.uri) + dst_file_path = self._m3u_uri_to_path(dst_uri) shutil.move(src_file_path, dst_file_path) - - return playlist.copy(uri=path.path_to_uri(dst_file_path)) + return playlist.copy(uri=dst_uri) diff --git a/mopidy/backends/local/translator.py b/mopidy/backends/local/translator.py index 344e8ad7..b8e98dd3 100644 --- a/mopidy/backends/local/translator.py +++ b/mopidy/backends/local/translator.py @@ -3,6 +3,7 @@ from __future__ import unicode_literals import logging import os import urllib +import urlparse from mopidy.models import Track, Artist, Album from mopidy.utils.encoding import locale_decode @@ -31,7 +32,6 @@ def parse_m3u(file_path, media_dir): - m3u files are latin-1. - This function does not bother with Extended M3U directives. """ - # TODO: uris as bytes uris = [] try: @@ -47,9 +47,11 @@ def parse_m3u(file_path, media_dir): if line.startswith('#'): continue - # FIXME what about other URI types? - if line.startswith('file://'): + if urlparse.urlsplit(line).scheme: uris.append(line) + elif os.path.normpath(line) == os.path.abspath(line): + path = path_to_uri(line) + uris.append(path) else: path = path_to_uri(os.path.join(media_dir, line)) uris.append(path) @@ -57,6 +59,7 @@ def parse_m3u(file_path, media_dir): return uris +# TODO: remove music_dir from API def parse_mpd_tag_cache(tag_cache, music_dir=''): """ Converts a MPD tag_cache into a lists of tracks, artists and albums. @@ -87,17 +90,17 @@ def parse_mpd_tag_cache(tag_cache, music_dir=''): key, value = line.split(b': ', 1) if key == b'key': - _convert_mpd_data(current, tracks, music_dir) + _convert_mpd_data(current, tracks) current.clear() current[key.lower()] = value.decode('utf-8') - _convert_mpd_data(current, tracks, music_dir) + _convert_mpd_data(current, tracks) return tracks -def _convert_mpd_data(data, tracks, music_dir): +def _convert_mpd_data(data, tracks): if not data: return @@ -161,15 +164,8 @@ def _convert_mpd_data(data, tracks, music_dir): path = data['file'][1:] else: path = data['file'] - path = urllib.unquote(path.encode('utf-8')) - - if isinstance(music_dir, unicode): - music_dir = music_dir.encode('utf-8') - - # Make sure we only pass bytestrings to path_to_uri to avoid implicit - # decoding of bytestrings to unicode strings - track_kwargs['uri'] = path_to_uri(os.path.join(music_dir, path)) + track_kwargs['uri'] = 'local:track:%s' % path track_kwargs['length'] = int(data.get('time', 0)) * 1000 track = Track(**track_kwargs) diff --git a/mopidy/backends/stream/ext.conf b/mopidy/backends/stream/ext.conf index 9caafac1..dc0287da 100644 --- a/mopidy/backends/stream/ext.conf +++ b/mopidy/backends/stream/ext.conf @@ -1,6 +1,7 @@ [stream] enabled = true protocols = + file http https mms diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 2e79827a..2f296751 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -401,6 +401,7 @@ class PlaybackController(object): if self.random and self._shuffled: self._shuffled.remove(tl_track) if on_error_step == 1: + # TODO: can cause an endless loop for single track repeat. self.next() elif on_error_step == -1: self.previous() diff --git a/mopidy/frontends/mpd/dispatcher.py b/mopidy/frontends/mpd/dispatcher.py index 6590897d..0e55271d 100644 --- a/mopidy/frontends/mpd/dispatcher.py +++ b/mopidy/frontends/mpd/dispatcher.py @@ -266,6 +266,7 @@ class MpdContext(object): for playlist in self.core.playlists.playlists.get(): if not playlist.name: continue + # TODO: add scheme to name perhaps 'foo (spotify)' etc. name = self.create_unique_name(playlist.name) self._playlist_uri_from_name[name] = playlist.uri self._playlist_name_from_uri[playlist.uri] = name diff --git a/mopidy/scanner.py b/mopidy/scanner.py index 2a901910..1752ece9 100644 --- a/mopidy/scanner.py +++ b/mopidy/scanner.py @@ -27,7 +27,6 @@ pygst.require('0.10') import gst from mopidy import config as config_lib, ext -from mopidy.audio import dummy as dummy_audio from mopidy.models import Track, Artist, Album from mopidy.utils import log, path, versioning @@ -59,7 +58,7 @@ def main(): updaters = {} for e in extensions: for updater_class in e.get_library_updaters(): - if updater_class and 'file' in updater_class.uri_schemes: + if updater_class and 'local' in updater_class.uri_schemes: updaters[e.ext_name] = updater_class if not updaters: diff --git a/mopidy/utils/path.py b/mopidy/utils/path.py index af1f38b1..602b2569 100644 --- a/mopidy/utils/path.py +++ b/mopidy/utils/path.py @@ -2,7 +2,6 @@ from __future__ import unicode_literals import logging import os -import re # pylint: disable = W0402 import string # pylint: enable = W0402 diff --git a/tests/backends/base/library.py b/tests/backends/base/library.py index 4bc525c8..23c76f38 100644 --- a/tests/backends/base/library.py +++ b/tests/backends/base/library.py @@ -7,8 +7,6 @@ import pykka from mopidy import core from mopidy.models import Track, Album, Artist -from tests import path_to_data_dir - class LibraryControllerTest(object): artists = [Artist(name='artist1'), Artist(name='artist2'), Artist()] @@ -17,13 +15,10 @@ class LibraryControllerTest(object): Album(name='album2', artists=artists[1:2]), Album()] tracks = [ - Track( - uri='file://' + path_to_data_dir('uri1'), name='track1', - artists=artists[:1], album=albums[0], date='2001-02-03', - length=4000), - Track( - uri='file://' + path_to_data_dir('uri2'), name='track2', - artists=artists[1:2], album=albums[1], date='2002', length=4000), + Track(uri='local:track:path1', name='track1', artists=artists[:1], + album=albums[0], date='2001-02-03', length=4000), + Track(uri='local:track:path2', name='track2', artists=artists[1:2], + album=albums[1], date='2002', length=4000), Track()] config = {} @@ -66,11 +61,11 @@ class LibraryControllerTest(object): self.assertEqual(list(result[0].tracks), []) def test_find_exact_uri(self): - track_1_uri = 'file://' + path_to_data_dir('uri1') + track_1_uri = 'local:track:path1' result = self.library.find_exact(uri=track_1_uri) self.assertEqual(list(result[0].tracks), self.tracks[:1]) - track_2_uri = 'file://' + path_to_data_dir('uri2') + track_2_uri = 'local:track:path2' result = self.library.find_exact(uri=track_2_uri) self.assertEqual(list(result[0].tracks), self.tracks[1:2]) @@ -136,10 +131,10 @@ class LibraryControllerTest(object): self.assertEqual(list(result[0].tracks), []) def test_search_uri(self): - result = self.library.search(uri=['RI1']) + result = self.library.search(uri=['TH1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) - result = self.library.search(uri=['RI2']) + result = self.library.search(uri=['TH2']) self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_search_track(self): @@ -183,7 +178,7 @@ class LibraryControllerTest(object): self.assertEqual(list(result[0].tracks), self.tracks[:1]) result = self.library.search(any=['Bum1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) - result = self.library.search(any=['RI1']) + result = self.library.search(any=['TH1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) def test_search_wrong_type(self): diff --git a/tests/backends/local/__init__.py b/tests/backends/local/__init__.py index 684e12d8..ca93cdc0 100644 --- a/tests/backends/local/__init__.py +++ b/tests/backends/local/__init__.py @@ -2,8 +2,5 @@ from __future__ import unicode_literals from mopidy.utils.path import path_to_uri -from tests import path_to_data_dir - -song = path_to_data_dir('song%s.wav') -generate_song = lambda i: path_to_uri(song % i) +generate_song = lambda i: 'local:track:song%s.wav' % i diff --git a/tests/backends/local/playback_test.py b/tests/backends/local/playback_test.py index 4c699699..b12464bd 100644 --- a/tests/backends/local/playback_test.py +++ b/tests/backends/local/playback_test.py @@ -5,11 +5,10 @@ import unittest from mopidy.backends.local import actor from mopidy.core import PlaybackState from mopidy.models import Track -from mopidy.utils.path import path_to_uri -from tests import path_to_data_dir from tests.backends.base.playback import PlaybackControllerTest from tests.backends.local import generate_song +from tests import path_to_data_dir class LocalPlaybackControllerTest(PlaybackControllerTest, unittest.TestCase): @@ -24,25 +23,25 @@ class LocalPlaybackControllerTest(PlaybackControllerTest, unittest.TestCase): tracks = [ Track(uri=generate_song(i), length=4464) for i in range(1, 4)] - def add_track(self, path): - uri = path_to_uri(path_to_data_dir(path)) + def add_track(self, uri): track = Track(uri=uri, length=4464) self.tracklist.add([track]) def test_uri_scheme(self): - self.assertIn('file', self.core.uri_schemes) + self.assertNotIn('file', self.core.uri_schemes) + self.assertIn('local', self.core.uri_schemes) def test_play_mp3(self): - self.add_track('blank.mp3') + self.add_track('local:track:blank.mp3') self.playback.play() self.assertEqual(self.playback.state, PlaybackState.PLAYING) def test_play_ogg(self): - self.add_track('blank.ogg') + self.add_track('local:track:blank.ogg') self.playback.play() self.assertEqual(self.playback.state, PlaybackState.PLAYING) def test_play_flac(self): - self.add_track('blank.flac') + self.add_track('local:track:blank.flac') self.playback.play() self.assertEqual(self.playback.state, PlaybackState.PLAYING) diff --git a/tests/backends/local/playlists_test.py b/tests/backends/local/playlists_test.py index 2882e476..591a9d1d 100644 --- a/tests/backends/local/playlists_test.py +++ b/tests/backends/local/playlists_test.py @@ -7,7 +7,7 @@ import unittest from mopidy.backends.local import actor from mopidy.models import Track -from mopidy.utils.path import path_to_uri +from mopidy.utils.path import path_to_uri, uri_to_path from tests import path_to_data_dir from tests.backends.base.playlists import ( @@ -89,21 +89,20 @@ class LocalPlaylistsControllerTest( def test_playlist_contents_is_written_to_disk(self): track = Track(uri=generate_song(1)) - track_path = track.uri[len('file://'):] playlist = self.core.playlists.create('test') - playlist_path = playlist.uri[len('file://'):] + playlist_path = os.path.join(self.playlists_dir, 'test.m3u') playlist = playlist.copy(tracks=[track]) playlist = self.core.playlists.save(playlist) with open(playlist_path) as playlist_file: contents = playlist_file.read() - self.assertEqual(track_path, contents.strip()) + self.assertEqual(track.uri, contents.strip()) def test_playlists_are_loaded_at_startup(self): playlist_path = os.path.join(self.playlists_dir, 'test.m3u') - track = Track(uri=path_to_uri(path_to_data_dir('uri2'))) + track = Track(uri='local:track:path2') playlist = self.core.playlists.create('test') playlist = playlist.copy(tracks=[track]) playlist = self.core.playlists.save(playlist) @@ -112,8 +111,7 @@ class LocalPlaylistsControllerTest( self.assert_(backend.playlists.playlists) self.assertEqual( - path_to_uri(playlist_path), - backend.playlists.playlists[0].uri) + 'local:playlist:test', backend.playlists.playlists[0].uri) self.assertEqual( playlist.name, backend.playlists.playlists[0].name) self.assertEqual( diff --git a/tests/backends/local/translator_test.py b/tests/backends/local/translator_test.py index 4f958232..5ed07fca 100644 --- a/tests/backends/local/translator_test.py +++ b/tests/backends/local/translator_test.py @@ -98,7 +98,7 @@ expected_tracks = [] def generate_track(path, ident): - uri = path_to_uri(path_to_data_dir(path)) + uri = 'local:track:%s' % path track = Track( uri=uri, name='trackname', artists=expected_artists, album=expected_albums[0], track_no=1, date='2006', length=4000, @@ -126,11 +126,10 @@ class MPDTagCacheToTracksTest(unittest.TestCase): def test_simple_cache(self): tracks = parse_mpd_tag_cache( path_to_data_dir('simple_tag_cache'), path_to_data_dir('')) - uri = path_to_uri(path_to_data_dir('song1.mp3')) track = Track( - uri=uri, name='trackname', artists=expected_artists, track_no=1, - album=expected_albums[0], date='2006', length=4000, - last_modified=1272319626) + uri='local:track:song1.mp3', name='trackname', + artists=expected_artists, track_no=1, album=expected_albums[0], + date='2006', length=4000, last_modified=1272319626) self.assertEqual(set([track]), tracks) def test_advanced_cache(self): @@ -142,12 +141,11 @@ class MPDTagCacheToTracksTest(unittest.TestCase): tracks = parse_mpd_tag_cache( path_to_data_dir('utf8_tag_cache'), path_to_data_dir('')) - uri = path_to_uri(path_to_data_dir('song1.mp3')) artists = [Artist(name='æøå')] album = Album(name='æøå', artists=artists) track = Track( - uri=uri, name='æøå', artists=artists, album=album, length=4000, - last_modified=1272319626) + uri='local:track:song1.mp3', name='æøå', artists=artists, + album=album, length=4000, last_modified=1272319626) self.assertEqual(track, list(tracks)[0]) @@ -159,8 +157,8 @@ class MPDTagCacheToTracksTest(unittest.TestCase): def test_cache_with_blank_track_info(self): tracks = parse_mpd_tag_cache( path_to_data_dir('blank_tag_cache'), path_to_data_dir('')) - uri = path_to_uri(path_to_data_dir('song1.mp3')) - expected = Track(uri=uri, length=4000, last_modified=1272319626) + expected = Track( + uri='local:track:song1.mp3', length=4000, last_modified=1272319626) self.assertEqual(set([expected]), tracks) def test_musicbrainz_tagcache(self): @@ -183,10 +181,10 @@ class MPDTagCacheToTracksTest(unittest.TestCase): def test_albumartist_tag_cache(self): tracks = parse_mpd_tag_cache( path_to_data_dir('albumartist_tag_cache'), path_to_data_dir('')) - uri = path_to_uri(path_to_data_dir('song1.mp3')) artist = Artist(name='albumartistname') album = expected_albums[0].copy(artists=[artist]) track = Track( - uri=uri, name='trackname', artists=expected_artists, track_no=1, - album=album, date='2006', length=4000, last_modified=1272319626) + uri='local:track:song1.mp3', name='trackname', + artists=expected_artists, track_no=1, album=album, date='2006', + length=4000, last_modified=1272319626) self.assertEqual(track, list(tracks)[0]) diff --git a/tests/data/library_tag_cache b/tests/data/library_tag_cache index 50771a0a..9dc11777 100644 --- a/tests/data/library_tag_cache +++ b/tests/data/library_tag_cache @@ -3,22 +3,22 @@ mpd_version: 0.14.2 fs_charset: UTF-8 info_end songList begin -key: uri1 -file: /uri1 +key: key1 +file: /path1 Artist: artist1 Title: track1 Album: album1 Date: 2001-02-03 Time: 4 -key: uri2 -file: /uri2 +key: key1 +file: /path2 Artist: artist2 Title: track2 Album: album2 Date: 2002 Time: 4 -key: uri3 -file: /uri3 +key: key3 +file: /path3 Artist: artist3 Title: track3 Album: album3