Slugify local playlist names to make them safe to use in paths (#217)
This commit is contained in:
parent
078cc72fff
commit
8c9a3d6df2
@ -1,7 +1,9 @@
|
||||
import glob
|
||||
import logging
|
||||
import os
|
||||
import re
|
||||
import shutil
|
||||
import unicodedata
|
||||
|
||||
from mopidy import settings
|
||||
from mopidy.backends import base
|
||||
@ -20,9 +22,8 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
|
||||
self._folder = settings.LOCAL_PLAYLIST_PATH
|
||||
self.refresh()
|
||||
|
||||
# TODO playlist names needs safer handling using a slug function
|
||||
|
||||
def create(self, name):
|
||||
name = self._slugify(name)
|
||||
file_path = os.path.join(self._folder, name + '.m3u')
|
||||
uri = path.path_to_uri(file_path)
|
||||
playlist = Playlist(uri=uri, name=name)
|
||||
@ -68,10 +69,11 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
|
||||
old_playlist = self.lookup(playlist.uri)
|
||||
|
||||
if old_playlist and playlist.name != old_playlist.name:
|
||||
new_name = self._slugify(playlist.name)
|
||||
src = os.path.join(self._folder, old_playlist.name + '.m3u')
|
||||
dst = os.path.join(self._folder, playlist.name + '.m3u')
|
||||
dst = os.path.join(self._folder, new_name + '.m3u')
|
||||
shutil.move(src, dst)
|
||||
playlist = playlist.copy(uri=path.path_to_uri(dst))
|
||||
playlist = playlist.copy(uri=path.path_to_uri(dst), name=new_name)
|
||||
|
||||
self._save_m3u(playlist)
|
||||
|
||||
@ -97,3 +99,16 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider):
|
||||
file_path = playlist.uri[len('file://'):]
|
||||
if os.path.exists(file_path):
|
||||
os.remove(file_path)
|
||||
|
||||
def _slugify(self, value):
|
||||
"""
|
||||
Converts to lowercase, removes non-word characters (alphanumerics and
|
||||
underscores) and converts spaces to hyphens. Also strips leading and
|
||||
trailing whitespace.
|
||||
|
||||
This function is based on Django's slugify implementation.
|
||||
"""
|
||||
value = unicodedata.normalize('NFKD', value)
|
||||
value = value.encode('ascii', 'ignore').decode('ascii')
|
||||
value = re.sub('[^\w\s-]', '', value).strip().lower()
|
||||
return re.sub('[-\s]+', '-', value)
|
||||
|
||||
@ -31,15 +31,15 @@ class StoredPlaylistsControllerTest(object):
|
||||
settings.runtime.clear()
|
||||
|
||||
def test_create_returns_playlist_with_name_set(self):
|
||||
playlist = self.stored.create('test')
|
||||
playlist = self.stored.create(u'test')
|
||||
self.assertEqual(playlist.name, 'test')
|
||||
|
||||
def test_create_returns_playlist_with_uri_set(self):
|
||||
playlist = self.stored.create('test')
|
||||
playlist = self.stored.create(u'test')
|
||||
self.assert_(playlist.uri)
|
||||
|
||||
def test_create_adds_playlist_to_playlists_collection(self):
|
||||
playlist = self.stored.create('test')
|
||||
playlist = self.stored.create(u'test')
|
||||
self.assert_(self.stored.playlists)
|
||||
self.assertIn(playlist, self.stored.playlists)
|
||||
|
||||
@ -50,7 +50,7 @@ class StoredPlaylistsControllerTest(object):
|
||||
self.stored.delete('file:///unknown/playlist')
|
||||
|
||||
def test_delete_playlist_removes_it_from_the_collection(self):
|
||||
playlist = self.stored.create('test')
|
||||
playlist = self.stored.create(u'test')
|
||||
self.assertIn(playlist, self.stored.playlists)
|
||||
|
||||
self.stored.delete(playlist.uri)
|
||||
@ -66,7 +66,7 @@ class StoredPlaylistsControllerTest(object):
|
||||
self.assertRaises(LookupError, test)
|
||||
|
||||
def test_get_with_right_criteria(self):
|
||||
playlist1 = self.stored.create('test')
|
||||
playlist1 = self.stored.create(u'test')
|
||||
playlist2 = self.stored.get(name='test')
|
||||
self.assertEqual(playlist1, playlist2)
|
||||
|
||||
@ -96,7 +96,7 @@ class StoredPlaylistsControllerTest(object):
|
||||
self.assertEqual(u'"name=c" match no playlists', e[0])
|
||||
|
||||
def test_lookup_finds_playlist_by_uri(self):
|
||||
original_playlist = self.stored.create('test')
|
||||
original_playlist = self.stored.create(u'test')
|
||||
|
||||
looked_up_playlist = self.stored.lookup(original_playlist.uri)
|
||||
|
||||
@ -107,10 +107,10 @@ class StoredPlaylistsControllerTest(object):
|
||||
pass
|
||||
|
||||
def test_save_replaces_stored_playlist_with_updated_playlist(self):
|
||||
playlist1 = self.stored.create('test1')
|
||||
playlist1 = self.stored.create(u'test1')
|
||||
self.assertIn(playlist1, self.stored.playlists)
|
||||
|
||||
playlist2 = playlist1.copy(name='test2')
|
||||
playlist2 = playlist1.copy(name=u'test2')
|
||||
playlist2 = self.stored.save(playlist2)
|
||||
self.assertNotIn(playlist1, self.stored.playlists)
|
||||
self.assertIn(playlist2, self.stored.playlists)
|
||||
|
||||
@ -18,22 +18,40 @@ class LocalStoredPlaylistsControllerTest(
|
||||
|
||||
def test_created_playlist_is_persisted(self):
|
||||
path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test.m3u')
|
||||
self.assert_(not os.path.exists(path))
|
||||
self.stored.create('test')
|
||||
self.assert_(os.path.exists(path))
|
||||
self.assertFalse(os.path.exists(path))
|
||||
|
||||
self.stored.create(u'test')
|
||||
self.assertTrue(os.path.exists(path))
|
||||
|
||||
def test_create_slugifies_playlist_name(self):
|
||||
path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test-foo-bar.m3u')
|
||||
self.assertFalse(os.path.exists(path))
|
||||
|
||||
playlist = self.stored.create(u'test FOO baR')
|
||||
self.assertEqual(u'test-foo-bar', playlist.name)
|
||||
self.assertTrue(os.path.exists(path))
|
||||
|
||||
def test_create_slugifies_names_which_tries_to_change_directory(self):
|
||||
path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test-foo-bar.m3u')
|
||||
self.assertFalse(os.path.exists(path))
|
||||
|
||||
playlist = self.stored.create(u'../../test FOO baR')
|
||||
self.assertEqual(u'test-foo-bar', playlist.name)
|
||||
self.assertTrue(os.path.exists(path))
|
||||
|
||||
def test_saved_playlist_is_persisted(self):
|
||||
path1 = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test1.m3u')
|
||||
path2 = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test2.m3u')
|
||||
path2 = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test2-foo-bar.m3u')
|
||||
|
||||
playlist = self.stored.create('test1')
|
||||
playlist = self.stored.create(u'test1')
|
||||
|
||||
self.assertTrue(os.path.exists(path1))
|
||||
self.assertFalse(os.path.exists(path2))
|
||||
|
||||
playlist = playlist.copy(name='test2')
|
||||
playlist = playlist.copy(name=u'test2 FOO baR')
|
||||
playlist = self.stored.save(playlist)
|
||||
|
||||
self.assertEqual(u'test2-foo-bar', playlist.name)
|
||||
self.assertFalse(os.path.exists(path1))
|
||||
self.assertTrue(os.path.exists(path2))
|
||||
|
||||
@ -41,7 +59,7 @@ class LocalStoredPlaylistsControllerTest(
|
||||
path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test.m3u')
|
||||
self.assertFalse(os.path.exists(path))
|
||||
|
||||
playlist = self.stored.create('test')
|
||||
playlist = self.stored.create(u'test')
|
||||
self.assertTrue(os.path.exists(path))
|
||||
|
||||
self.stored.delete(playlist.uri)
|
||||
@ -50,7 +68,7 @@ class LocalStoredPlaylistsControllerTest(
|
||||
def test_playlist_contents_is_written_to_disk(self):
|
||||
track = Track(uri=generate_song(1))
|
||||
track_path = track.uri[len('file://'):]
|
||||
playlist = self.stored.create('test')
|
||||
playlist = self.stored.create(u'test')
|
||||
playlist_path = playlist.uri[len('file://'):]
|
||||
playlist = playlist.copy(tracks=[track])
|
||||
playlist = self.stored.save(playlist)
|
||||
@ -64,7 +82,7 @@ class LocalStoredPlaylistsControllerTest(
|
||||
playlist_path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test.m3u')
|
||||
|
||||
track = Track(uri=path_to_uri(path_to_data_dir('uri2')))
|
||||
playlist = self.stored.create('test')
|
||||
playlist = self.stored.create(u'test')
|
||||
playlist = playlist.copy(tracks=[track])
|
||||
playlist = self.stored.save(playlist)
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user