From 53c44d9a97ba29ee7606ab82a61f356b5d01ebcc Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Sun, 28 Dec 2014 16:18:51 +0100 Subject: [PATCH 01/56] Added the import_task_created event Improved the IHatePlugin to filter files based on file names --- beets/importer.py | 15 +++ beetsplug/ihate.py | 135 ++++++++++++++++++- docs/dev/plugins.rst | 3 + test/test_ihate.py | 311 ++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 461 insertions(+), 3 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 0877b5508..e07b49f48 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -277,6 +277,8 @@ class ImportSession(object): else: stages = [query_tasks(self)] + stages += [send_import_task_created_event(self)] + if self.config['pretend']: # Only log the imported files and end the pipeline stages += [log_files(self)] @@ -1299,6 +1301,9 @@ def manipulate_files(session, task): def log_files(session, task): """A coroutine (pipeline stage) to log each file which will be imported """ + if task.skip: + return + if isinstance(task, SingletonImportTask): log.info(displayable_path(task.item['path'])) elif task.items: @@ -1306,6 +1311,16 @@ def log_files(session, task): log.info(displayable_path(item['path'])) +@pipeline.mutator_stage +def send_import_task_created_event(session, task): + """A coroutine (pipeline stage) to send the import_task_created event + """ + if task.skip: + return + + plugins.send('import_task_created', session=session, task=task) + + def group_albums(session): """Group the items of a task by albumartist and album name and create a new task for each album. Yield the tasks as a multi message. diff --git a/beetsplug/ihate.py b/beetsplug/ihate.py index b55554d8a..18f5e96fb 100644 --- a/beetsplug/ihate.py +++ b/beetsplug/ihate.py @@ -15,8 +15,11 @@ """Warns you about things you hate (or even blocks import).""" import logging +import os +import re +from beets import config from beets.plugins import BeetsPlugin -from beets.importer import action +from beets.importer import action, SingletonImportTask from beets.library import parse_query_string from beets.library import Item from beets.library import Album @@ -43,11 +46,63 @@ class IHatePlugin(BeetsPlugin): super(IHatePlugin, self).__init__() self.register_listener('import_task_choice', self.import_task_choice_event) + self.register_listener('import_task_created', + self.import_task_created_event) self.config.add({ 'warn': [], 'skip': [], + 'regex_ignore_case': False, + 'regex_invert_folder_result': False, + 'regex_invert_file_result': False, + 'regex_folder_name': '.*', + 'regex_file_name': '.*' }) + flags = re.IGNORECASE if self.config['regex_ignore_case'].get() else 0 + + self.invert_folder_album_result = \ + self.invert_folder_singleton_result = \ + self.config['regex_invert_folder_result'].get() + self.invert_file_album_result = \ + self.invert_file_singleton_result = \ + self.config['regex_invert_file_result'].get() + self.folder_name_album_regex = \ + self.folder_name_singleton_regex = \ + re.compile(self.config['regex_folder_name'].get(), flags) + self.file_name_album_regex = \ + self.file_name_singleton_regex = \ + re.compile(self.config['regex_file_name'].get(), flags) + + if 'album' in self.config: + album_config = self.config['album'] + if 'regex_invert_folder_result' in album_config: + self.invert_folder_album_result = album_config[ + 'regex_invert_folder_result'].get() + if 'regex_invert_file_result' in album_config: + self.invert_file_album_result = album_config[ + 'regex_invert_file_result'].get() + if 'regex_folder_name' in album_config: + self.folder_name_album_regex = re.compile( + album_config['regex_folder_name'].get(), flags) + if 'regex_file_name' in album_config: + self.file_name_album_regex = re.compile( + album_config['regex_file_name'].get(), flags) + + if 'singleton' in self.config: + singleton_config = self.config['singleton'] + if 'regex_invert_folder_result' in singleton_config: + self.invert_folder_singleton_result = singleton_config[ + 'regex_invert_folder_result'].get() + if 'regex_invert_file_result' in singleton_config: + self.invert_file_singleton_result = singleton_config[ + 'regex_invert_file_result'].get() + if 'regex_folder_name' in singleton_config: + self.folder_name_singleton_regex = re.compile( + singleton_config['regex_folder_name'].get(), flags) + if 'regex_file_name' in singleton_config: + self.file_name_singleton_regex = re.compile( + singleton_config['regex_file_name'].get(), flags) + @classmethod def do_i_hate_this(cls, task, action_patterns): """Process group of patterns (warn or skip) and returns True if @@ -82,3 +137,81 @@ class IHatePlugin(BeetsPlugin): self._log.debug(u'[ihate] nothing to do') else: self._log.debug(u'[ihate] user made a decision, nothing to do') + + def import_task_created_event(self, session, task): + if task.items and len(task.items) > 0: + items_to_import = [] + for item in task.items: + if self.file_filter(item['path'], session.paths): + items_to_import.append(item) + if len(items_to_import) > 0: + task.items = items_to_import + else: + task.choice_flag = action.SKIP + elif isinstance(task, SingletonImportTask): + if not self.file_filter(task.item['path'], session.paths): + task.choice_flag = action.SKIP + + def file_filter(self, full_path, base_paths): + """Checks if the configured regular expressions allow the import of the + file given in full_path. + """ + # The folder regex only checks the folder names starting from the + # longest base path. Find this folder. + matched_base_path = '' + for base_path in base_paths: + if full_path.startswith(base_path) and len(base_path) > len( + matched_base_path): + matched_base_path = base_path + relative_path = full_path[len(matched_base_path):] + + if os.path.isdir(full_path): + path = relative_path + file_name = None + else: + path, file_name = os.path.split(relative_path) + path, folder_name = os.path.split(path) + + import_config = dict(config['import']) + if 'singletons' not in import_config or not import_config[ + 'singletons']: + # Album + + # Folder + while len(folder_name) > 0: + matched = self.folder_name_album_regex.match( + folder_name) is not None + matched = not matched if self.invert_folder_album_result else \ + matched + if not matched: + return False + path, folder_name = os.path.split(path) + + # File + matched = self.file_name_album_regex.match( + file_name) is not None + matched = not matched if self.invert_file_album_result else matched + if not matched: + return False + return True + else: + # Singleton + + # Folder + while len(folder_name) > 0: + matched = self.folder_name_singleton_regex.match( + folder_name) is not None + matched = not matched if \ + self.invert_folder_singleton_result else matched + if not matched: + return False + path, folder_name = os.path.split(path) + + # File + matched = self.file_name_singleton_regex.match( + file_name) is not None + matched = not matched if self.invert_file_singleton_result else \ + matched + if not matched: + return False + return True diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index c79d49645..8eb184a5b 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -159,6 +159,9 @@ currently available are: * *after_write*: called with an ``Item`` object after a file's metadata is written to disk (i.e., just after the file on disk is closed). +* *import_task_created*: called after an import task has been created. + Parameters: ``task`` (an `ImportTask`) and ``session`` (an `ImportSession`). + * *import_task_start*: called when before an import task begins processing. Parameters: ``task`` (an `ImportTask`) and ``session`` (an `ImportSession`). diff --git a/test/test_ihate.py b/test/test_ihate.py index 030f5649e..a5061ed95 100644 --- a/test/test_ihate.py +++ b/test/test_ihate.py @@ -1,12 +1,108 @@ """Tests for the 'ihate' plugin""" +import os +import shutil from _common import unittest -from beets import importer +from beets import importer, config from beets.library import Item +from beets.mediafile import MediaFile from beetsplug.ihate import IHatePlugin +from test import _common +from test.helper import capture_log +from test.test_importer import ImportHelper -class IHatePluginTest(unittest.TestCase): +class IHatePluginTest(_common.TestCase, ImportHelper): + def setUp(self): + super(IHatePluginTest, self).setUp() + self.setup_beets() + self.__create_import_dir(2) + self._setup_import_session() + config['import']['pretend'] = True + + self.all_paths = [self.artist_paths[0], self.artist_paths[1], + self.album_paths[0], self.album_paths[1], + self.misc_paths[0], self.misc_paths[1]] + + def tearDown(self): + self.teardown_beets() + + def __copy_file(self, dest_path, metadata): + # Copy files + resource_path = os.path.join(_common.RSRC, 'full.mp3') + shutil.copy(resource_path, dest_path) + medium = MediaFile(dest_path) + # Set metadata + for attr in metadata: + setattr(medium, attr, metadata[attr]) + medium.save() + + def __create_import_dir(self, count): + self.import_dir = os.path.join(self.temp_dir, 'testsrcdir') + if os.path.isdir(self.import_dir): + shutil.rmtree(self.import_dir) + + artist_path = os.path.join(self.import_dir, 'artist') + album_path = os.path.join(artist_path, 'album') + misc_path = os.path.join(self.import_dir, 'misc') + os.makedirs(album_path) + os.makedirs(misc_path) + + metadata = { + 'artist': 'Tag Artist', + 'album': 'Tag Album', + 'albumartist': None, + 'mb_trackid': None, + 'mb_albumid': None, + 'comp': None + } + self.album_paths = [] + for i in range(count): + metadata['track'] = i + 1 + metadata['title'] = 'Tag Title Album %d' % (i + 1) + dest_path = os.path.join(album_path, '%02d - track.mp3' % (i + 1)) + self.__copy_file(dest_path, metadata) + self.album_paths.append(dest_path) + + self.artist_paths = [] + metadata['album'] = None + for i in range(count): + metadata['track'] = i + 10 + metadata['title'] = 'Tag Title Artist %d' % (i + 1) + dest_path = os.path.join(artist_path, 'track_%d.mp3' % (i + 1)) + self.__copy_file(dest_path, metadata) + self.artist_paths.append(dest_path) + + self.misc_paths = [] + for i in range(count): + metadata['artist'] = 'Artist %d' % (i + 42) + metadata['track'] = i + 5 + metadata['title'] = 'Tag Title Misc %d' % (i + 1) + dest_path = os.path.join(misc_path, 'track_%d.mp3' % (i + 1)) + self.__copy_file(dest_path, metadata) + self.misc_paths.append(dest_path) + + def __run(self, expected_lines, singletons=False): + import beetsplug + path = beetsplug.__path__ + print path + self.load_plugins('ihate') + + import_files = [self.import_dir] + self._setup_import_session(singletons=singletons) + self.importer.paths = import_files + + with capture_log() as logs: + self.importer.run() + self.unload_plugins() + IHatePlugin.listeners = None + + logs = [line for line in logs if not line.startswith('Sending event:')] + + self.assertEqual(logs, expected_lines) + + def __reset_config(self): + config['ihate'] = {} def test_hate(self): @@ -42,6 +138,217 @@ class IHatePluginTest(unittest.TestCase): "artist:testartist album:notthis"] self.assertTrue(IHatePlugin.do_i_hate_this(task, match_pattern)) + def test_import_default(self): + """ The default configuration should import everything. + """ + self.__reset_config() + self.__run(self.all_paths) + + def test_import_nothing(self): + self.__reset_config() + config['ihate']['regex_invert_folder_result'] = True + config['ihate']['regex_invert_file_result'] = True + self.__run([]) + + # Global options + def test_import_global_match_folder(self): + self.__reset_config() + config['ihate']['regex_folder_name'] = 'artist' + self.__run([self.artist_paths[0], + self.artist_paths[1]]) + + def test_import_global_invert_folder(self): + self.__reset_config() + config['ihate']['regex_folder_name'] = 'artist' + config['ihate']['regex_invert_folder_result'] = True + self.__run([self.misc_paths[0], + self.misc_paths[1]]) + + def test_import_global_match_file(self): + self.__reset_config() + config['ihate']['regex_file_name'] = '.*2.*' + self.__run([self.artist_paths[1], + self.album_paths[1], + self.misc_paths[1]]) + + def test_import_global_invert_file(self): + self.__reset_config() + config['ihate']['regex_file_name'] = '.*2.*' + config['ihate']['regex_invert_file_result'] = True + self.__run([self.artist_paths[0], + self.album_paths[0], + self.misc_paths[0]]) + + def test_import_global_match_folder_case_sensitive(self): + self.__reset_config() + config['ihate']['regex_folder_name'] = 'Artist' + self.__run([]) + + def test_import_global_match_folder_ignore_case(self): + self.__reset_config() + config['ihate']['regex_ignore_case'] = True + config['ihate']['regex_folder_name'] = 'Artist' + self.__run([self.artist_paths[0], + self.artist_paths[1]]) + + # Album options + def test_import_album_match_folder(self): + self.__reset_config() + config['ihate']['album']['regex_folder_name'] = 'artist' + self.__run([self.artist_paths[0], + self.artist_paths[1]]) + self.__run(self.all_paths, singletons=True) + + def test_import_album_invert_folder(self): + self.__reset_config() + config['ihate']['album']['regex_folder_name'] = 'artist' + config['ihate']['album']['regex_invert_folder_result'] = True + self.__run([self.misc_paths[0], + self.misc_paths[1]]) + self.__run(self.all_paths, singletons=True) + + def test_import_album_match_file(self): + self.__reset_config() + config['ihate']['album']['regex_file_name'] = '.*2.*' + self.__run([self.artist_paths[1], + self.album_paths[1], + self.misc_paths[1]]) + self.__run(self.all_paths, singletons=True) + + def test_import_album_invert_file(self): + self.__reset_config() + config['ihate']['album']['regex_file_name'] = '.*2.*' + config['ihate']['album']['regex_invert_file_result'] = True + self.__run([self.artist_paths[0], + self.album_paths[0], + self.misc_paths[0]]) + self.__run(self.all_paths, singletons=True) + + def test_import_album_match_folder_case_sensitive(self): + self.__reset_config() + config['ihate']['album']['regex_folder_name'] = 'Artist' + self.__run([]) + self.__run(self.all_paths, singletons=True) + + def test_import_album_match_folder_ignore_case(self): + self.__reset_config() + config['ihate']['regex_ignore_case'] = True + config['ihate']['album']['regex_folder_name'] = 'Artist' + self.__run([self.artist_paths[0], + self.artist_paths[1]]) + self.__run(self.all_paths, singletons=True) + + # Singleton options + def test_import_singleton_match_folder(self): + self.__reset_config() + config['ihate']['singleton']['regex_folder_name'] = 'artist' + self.__run([self.artist_paths[0], + self.artist_paths[1]], singletons=True) + self.__run(self.all_paths) + + def test_import_singleton_invert_folder(self): + self.__reset_config() + config['ihate']['singleton']['regex_folder_name'] = 'artist' + config['ihate']['singleton']['regex_invert_folder_result'] = True + self.__run([self.misc_paths[0], + self.misc_paths[1]], singletons=True) + self.__run(self.all_paths) + + def test_import_singleton_match_file(self): + self.__reset_config() + config['ihate']['singleton']['regex_file_name'] = '.*2.*' + self.__run([self.artist_paths[1], + self.album_paths[1], + self.misc_paths[1]], singletons=True) + self.__run(self.all_paths) + + def test_import_singleton_invert_file(self): + self.__reset_config() + config['ihate']['singleton']['regex_file_name'] = '.*2.*' + config['ihate']['singleton']['regex_invert_file_result'] = True + self.__run([self.artist_paths[0], + self.album_paths[0], + self.misc_paths[0]], singletons=True) + self.__run(self.all_paths) + + def test_import_singleton_match_folder_case_sensitive(self): + self.__reset_config() + config['ihate']['singleton']['regex_folder_name'] = 'Artist' + self.__run([], singletons=True) + self.__run(self.all_paths) + + def test_import_singleton_match_folder_ignore_case(self): + self.__reset_config() + config['ihate']['regex_ignore_case'] = True + config['ihate']['singleton']['regex_folder_name'] = 'Artist' + self.__run([self.artist_paths[0], + self.artist_paths[1]], singletons=True) + self.__run(self.all_paths) + + # Album and singleton options + def test_import_both_match_folder(self): + self.__reset_config() + config['ihate']['album']['regex_folder_name'] = 'artist' + config['ihate']['singleton']['regex_folder_name'] = 'misc' + self.__run([self.artist_paths[0], + self.artist_paths[1]]) + self.__run([self.misc_paths[0], + self.misc_paths[1]], singletons=True) + + def test_import_both_invert_folder(self): + self.__reset_config() + config['ihate']['album']['regex_folder_name'] = 'artist' + config['ihate']['album']['regex_invert_folder_result'] = True + config['ihate']['singleton']['regex_folder_name'] = 'misc' + config['ihate']['singleton']['regex_invert_folder_result'] = True + self.__run([self.misc_paths[0], + self.misc_paths[1]]) + self.__run([self.artist_paths[0], + self.artist_paths[1], + self.album_paths[0], + self.album_paths[1]], singletons=True) + + def test_import_both_match_file(self): + self.__reset_config() + config['ihate']['album']['regex_file_name'] = '.*2.*' + config['ihate']['singleton']['regex_file_name'] = '.*1.*' + self.__run([self.artist_paths[1], + self.album_paths[1], + self.misc_paths[1]]) + self.__run([self.artist_paths[0], + self.album_paths[0], + self.misc_paths[0]], singletons=True) + + def test_import_both_invert_file(self): + self.__reset_config() + config['ihate']['album']['regex_file_name'] = '.*2.*' + config['ihate']['album']['regex_invert_file_result'] = True + config['ihate']['singleton']['regex_file_name'] = '.*1.*' + config['ihate']['singleton']['regex_invert_file_result'] = True + self.__run([self.artist_paths[0], + self.album_paths[0], + self.misc_paths[0]]) + self.__run([self.artist_paths[1], + self.album_paths[1], + self.misc_paths[1]], singletons=True) + + def test_import_both_match_folder_case_sensitive(self): + self.__reset_config() + config['ihate']['album']['regex_folder_name'] = 'Artist' + config['ihate']['singleton']['regex_folder_name'] = 'Misc' + self.__run([]) + self.__run([], singletons=True) + + def test_import_both_match_folder_ignore_case(self): + self.__reset_config() + config['ihate']['regex_ignore_case'] = True + config['ihate']['album']['regex_folder_name'] = 'Artist' + config['ihate']['singleton']['regex_folder_name'] = 'Misc' + self.__run([self.artist_paths[0], + self.artist_paths[1]]) + self.__run([self.misc_paths[0], + self.misc_paths[1]], singletons=True) + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From 14543589ed2f96814c323575449125e0ee31dac6 Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Mon, 29 Dec 2014 11:52:33 +0100 Subject: [PATCH 02/56] Removed some testing code which was checked in accidentally --- test/test_ihate.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/test_ihate.py b/test/test_ihate.py index a5061ed95..b6dbd37f5 100644 --- a/test/test_ihate.py +++ b/test/test_ihate.py @@ -83,9 +83,6 @@ class IHatePluginTest(_common.TestCase, ImportHelper): self.misc_paths.append(dest_path) def __run(self, expected_lines, singletons=False): - import beetsplug - path = beetsplug.__path__ - print path self.load_plugins('ihate') import_files = [self.import_dir] From 7674399a452df99413b1169a9277dff6339cd8ad Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Sun, 28 Dec 2014 16:18:51 +0100 Subject: [PATCH 03/56] Added the import_task_created event Improved the IHatePlugin to filter files based on file names --- beets/importer.py | 15 +++ beetsplug/ihate.py | 135 ++++++++++++++++++- docs/dev/plugins.rst | 3 + test/test_ihate.py | 311 ++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 461 insertions(+), 3 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 0877b5508..e07b49f48 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -277,6 +277,8 @@ class ImportSession(object): else: stages = [query_tasks(self)] + stages += [send_import_task_created_event(self)] + if self.config['pretend']: # Only log the imported files and end the pipeline stages += [log_files(self)] @@ -1299,6 +1301,9 @@ def manipulate_files(session, task): def log_files(session, task): """A coroutine (pipeline stage) to log each file which will be imported """ + if task.skip: + return + if isinstance(task, SingletonImportTask): log.info(displayable_path(task.item['path'])) elif task.items: @@ -1306,6 +1311,16 @@ def log_files(session, task): log.info(displayable_path(item['path'])) +@pipeline.mutator_stage +def send_import_task_created_event(session, task): + """A coroutine (pipeline stage) to send the import_task_created event + """ + if task.skip: + return + + plugins.send('import_task_created', session=session, task=task) + + def group_albums(session): """Group the items of a task by albumartist and album name and create a new task for each album. Yield the tasks as a multi message. diff --git a/beetsplug/ihate.py b/beetsplug/ihate.py index b55554d8a..18f5e96fb 100644 --- a/beetsplug/ihate.py +++ b/beetsplug/ihate.py @@ -15,8 +15,11 @@ """Warns you about things you hate (or even blocks import).""" import logging +import os +import re +from beets import config from beets.plugins import BeetsPlugin -from beets.importer import action +from beets.importer import action, SingletonImportTask from beets.library import parse_query_string from beets.library import Item from beets.library import Album @@ -43,11 +46,63 @@ class IHatePlugin(BeetsPlugin): super(IHatePlugin, self).__init__() self.register_listener('import_task_choice', self.import_task_choice_event) + self.register_listener('import_task_created', + self.import_task_created_event) self.config.add({ 'warn': [], 'skip': [], + 'regex_ignore_case': False, + 'regex_invert_folder_result': False, + 'regex_invert_file_result': False, + 'regex_folder_name': '.*', + 'regex_file_name': '.*' }) + flags = re.IGNORECASE if self.config['regex_ignore_case'].get() else 0 + + self.invert_folder_album_result = \ + self.invert_folder_singleton_result = \ + self.config['regex_invert_folder_result'].get() + self.invert_file_album_result = \ + self.invert_file_singleton_result = \ + self.config['regex_invert_file_result'].get() + self.folder_name_album_regex = \ + self.folder_name_singleton_regex = \ + re.compile(self.config['regex_folder_name'].get(), flags) + self.file_name_album_regex = \ + self.file_name_singleton_regex = \ + re.compile(self.config['regex_file_name'].get(), flags) + + if 'album' in self.config: + album_config = self.config['album'] + if 'regex_invert_folder_result' in album_config: + self.invert_folder_album_result = album_config[ + 'regex_invert_folder_result'].get() + if 'regex_invert_file_result' in album_config: + self.invert_file_album_result = album_config[ + 'regex_invert_file_result'].get() + if 'regex_folder_name' in album_config: + self.folder_name_album_regex = re.compile( + album_config['regex_folder_name'].get(), flags) + if 'regex_file_name' in album_config: + self.file_name_album_regex = re.compile( + album_config['regex_file_name'].get(), flags) + + if 'singleton' in self.config: + singleton_config = self.config['singleton'] + if 'regex_invert_folder_result' in singleton_config: + self.invert_folder_singleton_result = singleton_config[ + 'regex_invert_folder_result'].get() + if 'regex_invert_file_result' in singleton_config: + self.invert_file_singleton_result = singleton_config[ + 'regex_invert_file_result'].get() + if 'regex_folder_name' in singleton_config: + self.folder_name_singleton_regex = re.compile( + singleton_config['regex_folder_name'].get(), flags) + if 'regex_file_name' in singleton_config: + self.file_name_singleton_regex = re.compile( + singleton_config['regex_file_name'].get(), flags) + @classmethod def do_i_hate_this(cls, task, action_patterns): """Process group of patterns (warn or skip) and returns True if @@ -82,3 +137,81 @@ class IHatePlugin(BeetsPlugin): self._log.debug(u'[ihate] nothing to do') else: self._log.debug(u'[ihate] user made a decision, nothing to do') + + def import_task_created_event(self, session, task): + if task.items and len(task.items) > 0: + items_to_import = [] + for item in task.items: + if self.file_filter(item['path'], session.paths): + items_to_import.append(item) + if len(items_to_import) > 0: + task.items = items_to_import + else: + task.choice_flag = action.SKIP + elif isinstance(task, SingletonImportTask): + if not self.file_filter(task.item['path'], session.paths): + task.choice_flag = action.SKIP + + def file_filter(self, full_path, base_paths): + """Checks if the configured regular expressions allow the import of the + file given in full_path. + """ + # The folder regex only checks the folder names starting from the + # longest base path. Find this folder. + matched_base_path = '' + for base_path in base_paths: + if full_path.startswith(base_path) and len(base_path) > len( + matched_base_path): + matched_base_path = base_path + relative_path = full_path[len(matched_base_path):] + + if os.path.isdir(full_path): + path = relative_path + file_name = None + else: + path, file_name = os.path.split(relative_path) + path, folder_name = os.path.split(path) + + import_config = dict(config['import']) + if 'singletons' not in import_config or not import_config[ + 'singletons']: + # Album + + # Folder + while len(folder_name) > 0: + matched = self.folder_name_album_regex.match( + folder_name) is not None + matched = not matched if self.invert_folder_album_result else \ + matched + if not matched: + return False + path, folder_name = os.path.split(path) + + # File + matched = self.file_name_album_regex.match( + file_name) is not None + matched = not matched if self.invert_file_album_result else matched + if not matched: + return False + return True + else: + # Singleton + + # Folder + while len(folder_name) > 0: + matched = self.folder_name_singleton_regex.match( + folder_name) is not None + matched = not matched if \ + self.invert_folder_singleton_result else matched + if not matched: + return False + path, folder_name = os.path.split(path) + + # File + matched = self.file_name_singleton_regex.match( + file_name) is not None + matched = not matched if self.invert_file_singleton_result else \ + matched + if not matched: + return False + return True diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index c79d49645..8eb184a5b 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -159,6 +159,9 @@ currently available are: * *after_write*: called with an ``Item`` object after a file's metadata is written to disk (i.e., just after the file on disk is closed). +* *import_task_created*: called after an import task has been created. + Parameters: ``task`` (an `ImportTask`) and ``session`` (an `ImportSession`). + * *import_task_start*: called when before an import task begins processing. Parameters: ``task`` (an `ImportTask`) and ``session`` (an `ImportSession`). diff --git a/test/test_ihate.py b/test/test_ihate.py index 030f5649e..a5061ed95 100644 --- a/test/test_ihate.py +++ b/test/test_ihate.py @@ -1,12 +1,108 @@ """Tests for the 'ihate' plugin""" +import os +import shutil from _common import unittest -from beets import importer +from beets import importer, config from beets.library import Item +from beets.mediafile import MediaFile from beetsplug.ihate import IHatePlugin +from test import _common +from test.helper import capture_log +from test.test_importer import ImportHelper -class IHatePluginTest(unittest.TestCase): +class IHatePluginTest(_common.TestCase, ImportHelper): + def setUp(self): + super(IHatePluginTest, self).setUp() + self.setup_beets() + self.__create_import_dir(2) + self._setup_import_session() + config['import']['pretend'] = True + + self.all_paths = [self.artist_paths[0], self.artist_paths[1], + self.album_paths[0], self.album_paths[1], + self.misc_paths[0], self.misc_paths[1]] + + def tearDown(self): + self.teardown_beets() + + def __copy_file(self, dest_path, metadata): + # Copy files + resource_path = os.path.join(_common.RSRC, 'full.mp3') + shutil.copy(resource_path, dest_path) + medium = MediaFile(dest_path) + # Set metadata + for attr in metadata: + setattr(medium, attr, metadata[attr]) + medium.save() + + def __create_import_dir(self, count): + self.import_dir = os.path.join(self.temp_dir, 'testsrcdir') + if os.path.isdir(self.import_dir): + shutil.rmtree(self.import_dir) + + artist_path = os.path.join(self.import_dir, 'artist') + album_path = os.path.join(artist_path, 'album') + misc_path = os.path.join(self.import_dir, 'misc') + os.makedirs(album_path) + os.makedirs(misc_path) + + metadata = { + 'artist': 'Tag Artist', + 'album': 'Tag Album', + 'albumartist': None, + 'mb_trackid': None, + 'mb_albumid': None, + 'comp': None + } + self.album_paths = [] + for i in range(count): + metadata['track'] = i + 1 + metadata['title'] = 'Tag Title Album %d' % (i + 1) + dest_path = os.path.join(album_path, '%02d - track.mp3' % (i + 1)) + self.__copy_file(dest_path, metadata) + self.album_paths.append(dest_path) + + self.artist_paths = [] + metadata['album'] = None + for i in range(count): + metadata['track'] = i + 10 + metadata['title'] = 'Tag Title Artist %d' % (i + 1) + dest_path = os.path.join(artist_path, 'track_%d.mp3' % (i + 1)) + self.__copy_file(dest_path, metadata) + self.artist_paths.append(dest_path) + + self.misc_paths = [] + for i in range(count): + metadata['artist'] = 'Artist %d' % (i + 42) + metadata['track'] = i + 5 + metadata['title'] = 'Tag Title Misc %d' % (i + 1) + dest_path = os.path.join(misc_path, 'track_%d.mp3' % (i + 1)) + self.__copy_file(dest_path, metadata) + self.misc_paths.append(dest_path) + + def __run(self, expected_lines, singletons=False): + import beetsplug + path = beetsplug.__path__ + print path + self.load_plugins('ihate') + + import_files = [self.import_dir] + self._setup_import_session(singletons=singletons) + self.importer.paths = import_files + + with capture_log() as logs: + self.importer.run() + self.unload_plugins() + IHatePlugin.listeners = None + + logs = [line for line in logs if not line.startswith('Sending event:')] + + self.assertEqual(logs, expected_lines) + + def __reset_config(self): + config['ihate'] = {} def test_hate(self): @@ -42,6 +138,217 @@ class IHatePluginTest(unittest.TestCase): "artist:testartist album:notthis"] self.assertTrue(IHatePlugin.do_i_hate_this(task, match_pattern)) + def test_import_default(self): + """ The default configuration should import everything. + """ + self.__reset_config() + self.__run(self.all_paths) + + def test_import_nothing(self): + self.__reset_config() + config['ihate']['regex_invert_folder_result'] = True + config['ihate']['regex_invert_file_result'] = True + self.__run([]) + + # Global options + def test_import_global_match_folder(self): + self.__reset_config() + config['ihate']['regex_folder_name'] = 'artist' + self.__run([self.artist_paths[0], + self.artist_paths[1]]) + + def test_import_global_invert_folder(self): + self.__reset_config() + config['ihate']['regex_folder_name'] = 'artist' + config['ihate']['regex_invert_folder_result'] = True + self.__run([self.misc_paths[0], + self.misc_paths[1]]) + + def test_import_global_match_file(self): + self.__reset_config() + config['ihate']['regex_file_name'] = '.*2.*' + self.__run([self.artist_paths[1], + self.album_paths[1], + self.misc_paths[1]]) + + def test_import_global_invert_file(self): + self.__reset_config() + config['ihate']['regex_file_name'] = '.*2.*' + config['ihate']['regex_invert_file_result'] = True + self.__run([self.artist_paths[0], + self.album_paths[0], + self.misc_paths[0]]) + + def test_import_global_match_folder_case_sensitive(self): + self.__reset_config() + config['ihate']['regex_folder_name'] = 'Artist' + self.__run([]) + + def test_import_global_match_folder_ignore_case(self): + self.__reset_config() + config['ihate']['regex_ignore_case'] = True + config['ihate']['regex_folder_name'] = 'Artist' + self.__run([self.artist_paths[0], + self.artist_paths[1]]) + + # Album options + def test_import_album_match_folder(self): + self.__reset_config() + config['ihate']['album']['regex_folder_name'] = 'artist' + self.__run([self.artist_paths[0], + self.artist_paths[1]]) + self.__run(self.all_paths, singletons=True) + + def test_import_album_invert_folder(self): + self.__reset_config() + config['ihate']['album']['regex_folder_name'] = 'artist' + config['ihate']['album']['regex_invert_folder_result'] = True + self.__run([self.misc_paths[0], + self.misc_paths[1]]) + self.__run(self.all_paths, singletons=True) + + def test_import_album_match_file(self): + self.__reset_config() + config['ihate']['album']['regex_file_name'] = '.*2.*' + self.__run([self.artist_paths[1], + self.album_paths[1], + self.misc_paths[1]]) + self.__run(self.all_paths, singletons=True) + + def test_import_album_invert_file(self): + self.__reset_config() + config['ihate']['album']['regex_file_name'] = '.*2.*' + config['ihate']['album']['regex_invert_file_result'] = True + self.__run([self.artist_paths[0], + self.album_paths[0], + self.misc_paths[0]]) + self.__run(self.all_paths, singletons=True) + + def test_import_album_match_folder_case_sensitive(self): + self.__reset_config() + config['ihate']['album']['regex_folder_name'] = 'Artist' + self.__run([]) + self.__run(self.all_paths, singletons=True) + + def test_import_album_match_folder_ignore_case(self): + self.__reset_config() + config['ihate']['regex_ignore_case'] = True + config['ihate']['album']['regex_folder_name'] = 'Artist' + self.__run([self.artist_paths[0], + self.artist_paths[1]]) + self.__run(self.all_paths, singletons=True) + + # Singleton options + def test_import_singleton_match_folder(self): + self.__reset_config() + config['ihate']['singleton']['regex_folder_name'] = 'artist' + self.__run([self.artist_paths[0], + self.artist_paths[1]], singletons=True) + self.__run(self.all_paths) + + def test_import_singleton_invert_folder(self): + self.__reset_config() + config['ihate']['singleton']['regex_folder_name'] = 'artist' + config['ihate']['singleton']['regex_invert_folder_result'] = True + self.__run([self.misc_paths[0], + self.misc_paths[1]], singletons=True) + self.__run(self.all_paths) + + def test_import_singleton_match_file(self): + self.__reset_config() + config['ihate']['singleton']['regex_file_name'] = '.*2.*' + self.__run([self.artist_paths[1], + self.album_paths[1], + self.misc_paths[1]], singletons=True) + self.__run(self.all_paths) + + def test_import_singleton_invert_file(self): + self.__reset_config() + config['ihate']['singleton']['regex_file_name'] = '.*2.*' + config['ihate']['singleton']['regex_invert_file_result'] = True + self.__run([self.artist_paths[0], + self.album_paths[0], + self.misc_paths[0]], singletons=True) + self.__run(self.all_paths) + + def test_import_singleton_match_folder_case_sensitive(self): + self.__reset_config() + config['ihate']['singleton']['regex_folder_name'] = 'Artist' + self.__run([], singletons=True) + self.__run(self.all_paths) + + def test_import_singleton_match_folder_ignore_case(self): + self.__reset_config() + config['ihate']['regex_ignore_case'] = True + config['ihate']['singleton']['regex_folder_name'] = 'Artist' + self.__run([self.artist_paths[0], + self.artist_paths[1]], singletons=True) + self.__run(self.all_paths) + + # Album and singleton options + def test_import_both_match_folder(self): + self.__reset_config() + config['ihate']['album']['regex_folder_name'] = 'artist' + config['ihate']['singleton']['regex_folder_name'] = 'misc' + self.__run([self.artist_paths[0], + self.artist_paths[1]]) + self.__run([self.misc_paths[0], + self.misc_paths[1]], singletons=True) + + def test_import_both_invert_folder(self): + self.__reset_config() + config['ihate']['album']['regex_folder_name'] = 'artist' + config['ihate']['album']['regex_invert_folder_result'] = True + config['ihate']['singleton']['regex_folder_name'] = 'misc' + config['ihate']['singleton']['regex_invert_folder_result'] = True + self.__run([self.misc_paths[0], + self.misc_paths[1]]) + self.__run([self.artist_paths[0], + self.artist_paths[1], + self.album_paths[0], + self.album_paths[1]], singletons=True) + + def test_import_both_match_file(self): + self.__reset_config() + config['ihate']['album']['regex_file_name'] = '.*2.*' + config['ihate']['singleton']['regex_file_name'] = '.*1.*' + self.__run([self.artist_paths[1], + self.album_paths[1], + self.misc_paths[1]]) + self.__run([self.artist_paths[0], + self.album_paths[0], + self.misc_paths[0]], singletons=True) + + def test_import_both_invert_file(self): + self.__reset_config() + config['ihate']['album']['regex_file_name'] = '.*2.*' + config['ihate']['album']['regex_invert_file_result'] = True + config['ihate']['singleton']['regex_file_name'] = '.*1.*' + config['ihate']['singleton']['regex_invert_file_result'] = True + self.__run([self.artist_paths[0], + self.album_paths[0], + self.misc_paths[0]]) + self.__run([self.artist_paths[1], + self.album_paths[1], + self.misc_paths[1]], singletons=True) + + def test_import_both_match_folder_case_sensitive(self): + self.__reset_config() + config['ihate']['album']['regex_folder_name'] = 'Artist' + config['ihate']['singleton']['regex_folder_name'] = 'Misc' + self.__run([]) + self.__run([], singletons=True) + + def test_import_both_match_folder_ignore_case(self): + self.__reset_config() + config['ihate']['regex_ignore_case'] = True + config['ihate']['album']['regex_folder_name'] = 'Artist' + config['ihate']['singleton']['regex_folder_name'] = 'Misc' + self.__run([self.artist_paths[0], + self.artist_paths[1]]) + self.__run([self.misc_paths[0], + self.misc_paths[1]], singletons=True) + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From 16abc858e3de58793c206933c94fbd9511254d04 Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Mon, 29 Dec 2014 11:52:33 +0100 Subject: [PATCH 04/56] Removed some testing code which was checked in accidentally --- test/test_ihate.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/test_ihate.py b/test/test_ihate.py index a5061ed95..b6dbd37f5 100644 --- a/test/test_ihate.py +++ b/test/test_ihate.py @@ -83,9 +83,6 @@ class IHatePluginTest(_common.TestCase, ImportHelper): self.misc_paths.append(dest_path) def __run(self, expected_lines, singletons=False): - import beetsplug - path = beetsplug.__path__ - print path self.load_plugins('ihate') import_files = [self.import_dir] From acec078fa3ba90dfc4514c641288d3d0a4a41d99 Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Mon, 29 Dec 2014 13:15:54 +0100 Subject: [PATCH 05/56] The new event changed the log so an old import test failed... --- test/test_importer.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/test_importer.py b/test/test_importer.py index 0ffa9aaf3..37f091b7d 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1567,13 +1567,15 @@ class ImportPretendTest(_common.TestCase, ImportHelper): with capture_log() as logs: self.importer.run() + logs = [line for line in logs if not line.startswith('Sending event:')] + self.assertEqual(len(self.lib.items()), 0) self.assertEqual(len(self.lib.albums()), 0) - self.assertEqual(len(logs), 3) - self.assertEqual(logs[1], os.path.join(import_files[0], + self.assertEqual(len(logs), 2) + self.assertEqual(logs[0], os.path.join(import_files[0], u'track_1.mp3')) - self.assertEqual(logs[2], import_files[1]) + self.assertEqual(logs[1], import_files[1]) def test_import_pretend_empty(self): path = os.path.join(self.temp_dir, 'empty') @@ -1585,11 +1587,13 @@ class ImportPretendTest(_common.TestCase, ImportHelper): with capture_log() as logs: self.importer.run() + logs = [line for line in logs if not line.startswith('Sending event:')] + self.assertEqual(len(self.lib.items()), 0) self.assertEqual(len(self.lib.albums()), 0) - self.assertEqual(len(logs), 2) - self.assertEqual(logs[1], 'No files imported from {0}' + self.assertEqual(len(logs), 1) + self.assertEqual(logs[0], 'No files imported from {0}' .format(displayable_path(path))) From 11008494c3cb4d67a53def412c81a33489aa8a85 Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Tue, 30 Dec 2014 12:45:29 +0100 Subject: [PATCH 06/56] Tests failed on windows --- test/test_ihate.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/test_ihate.py b/test/test_ihate.py index b6dbd37f5..7b4b3de25 100644 --- a/test/test_ihate.py +++ b/test/test_ihate.py @@ -12,9 +12,8 @@ from test.helper import capture_log from test.test_importer import ImportHelper -class IHatePluginTest(_common.TestCase, ImportHelper): +class IHatePluginTest(ImportHelper): def setUp(self): - super(IHatePluginTest, self).setUp() self.setup_beets() self.__create_import_dir(2) self._setup_import_session() From 8addf3ef3971d7aa8677e883a7ea121e9c7ebb76 Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Tue, 30 Dec 2014 14:11:45 +0100 Subject: [PATCH 07/56] Simplified the configuration of the regular expressions for th ihate plugin Added the docs --- beetsplug/ihate.py | 115 ++++--------------------- docs/plugins/ihate.rst | 17 +++- test/test_ihate.py | 189 +++-------------------------------------- 3 files changed, 43 insertions(+), 278 deletions(-) diff --git a/beetsplug/ihate.py b/beetsplug/ihate.py index 18f5e96fb..b7980e02c 100644 --- a/beetsplug/ihate.py +++ b/beetsplug/ihate.py @@ -15,7 +15,6 @@ """Warns you about things you hate (or even blocks import).""" import logging -import os import re from beets import config from beets.plugins import BeetsPlugin @@ -51,57 +50,24 @@ class IHatePlugin(BeetsPlugin): self.config.add({ 'warn': [], 'skip': [], - 'regex_ignore_case': False, - 'regex_invert_folder_result': False, - 'regex_invert_file_result': False, - 'regex_folder_name': '.*', - 'regex_file_name': '.*' + 'path': '.*' }) - flags = re.IGNORECASE if self.config['regex_ignore_case'].get() else 0 - - self.invert_folder_album_result = \ - self.invert_folder_singleton_result = \ - self.config['regex_invert_folder_result'].get() - self.invert_file_album_result = \ - self.invert_file_singleton_result = \ - self.config['regex_invert_file_result'].get() - self.folder_name_album_regex = \ - self.folder_name_singleton_regex = \ - re.compile(self.config['regex_folder_name'].get(), flags) - self.file_name_album_regex = \ - self.file_name_singleton_regex = \ - re.compile(self.config['regex_file_name'].get(), flags) + self.path_album_regex = \ + self.path_singleton_regex = \ + re.compile(self.config['path'].get()) if 'album' in self.config: album_config = self.config['album'] - if 'regex_invert_folder_result' in album_config: - self.invert_folder_album_result = album_config[ - 'regex_invert_folder_result'].get() - if 'regex_invert_file_result' in album_config: - self.invert_file_album_result = album_config[ - 'regex_invert_file_result'].get() - if 'regex_folder_name' in album_config: - self.folder_name_album_regex = re.compile( - album_config['regex_folder_name'].get(), flags) - if 'regex_file_name' in album_config: - self.file_name_album_regex = re.compile( - album_config['regex_file_name'].get(), flags) + if 'path' in album_config: + self.path_album_regex = re.compile( + album_config['path'].get()) if 'singleton' in self.config: singleton_config = self.config['singleton'] - if 'regex_invert_folder_result' in singleton_config: - self.invert_folder_singleton_result = singleton_config[ - 'regex_invert_folder_result'].get() - if 'regex_invert_file_result' in singleton_config: - self.invert_file_singleton_result = singleton_config[ - 'regex_invert_file_result'].get() - if 'regex_folder_name' in singleton_config: - self.folder_name_singleton_regex = re.compile( - singleton_config['regex_folder_name'].get(), flags) - if 'regex_file_name' in singleton_config: - self.file_name_singleton_regex = re.compile( - singleton_config['regex_file_name'].get(), flags) + if 'path' in singleton_config: + self.path_singleton_regex = re.compile( + singleton_config['path'].get()) @classmethod def do_i_hate_this(cls, task, action_patterns): @@ -142,76 +108,25 @@ class IHatePlugin(BeetsPlugin): if task.items and len(task.items) > 0: items_to_import = [] for item in task.items: - if self.file_filter(item['path'], session.paths): + if self.file_filter(item['path']): items_to_import.append(item) if len(items_to_import) > 0: task.items = items_to_import else: task.choice_flag = action.SKIP elif isinstance(task, SingletonImportTask): - if not self.file_filter(task.item['path'], session.paths): + if not self.file_filter(task.item['path']): task.choice_flag = action.SKIP - def file_filter(self, full_path, base_paths): + def file_filter(self, full_path): """Checks if the configured regular expressions allow the import of the file given in full_path. """ - # The folder regex only checks the folder names starting from the - # longest base path. Find this folder. - matched_base_path = '' - for base_path in base_paths: - if full_path.startswith(base_path) and len(base_path) > len( - matched_base_path): - matched_base_path = base_path - relative_path = full_path[len(matched_base_path):] - - if os.path.isdir(full_path): - path = relative_path - file_name = None - else: - path, file_name = os.path.split(relative_path) - path, folder_name = os.path.split(path) - import_config = dict(config['import']) if 'singletons' not in import_config or not import_config[ 'singletons']: # Album - - # Folder - while len(folder_name) > 0: - matched = self.folder_name_album_regex.match( - folder_name) is not None - matched = not matched if self.invert_folder_album_result else \ - matched - if not matched: - return False - path, folder_name = os.path.split(path) - - # File - matched = self.file_name_album_regex.match( - file_name) is not None - matched = not matched if self.invert_file_album_result else matched - if not matched: - return False - return True + return self.path_album_regex.match(full_path) is not None else: # Singleton - - # Folder - while len(folder_name) > 0: - matched = self.folder_name_singleton_regex.match( - folder_name) is not None - matched = not matched if \ - self.invert_folder_singleton_result else matched - if not matched: - return False - path, folder_name = os.path.split(path) - - # File - matched = self.file_name_singleton_regex.match( - file_name) is not None - matched = not matched if self.invert_file_singleton_result else \ - matched - if not matched: - return False - return True + return self.path_singleton_regex.match(full_path) is not None diff --git a/docs/plugins/ihate.rst b/docs/plugins/ihate.rst index f2224bf5a..3fb5e4ffc 100644 --- a/docs/plugins/ihate.rst +++ b/docs/plugins/ihate.rst @@ -4,7 +4,8 @@ IHate Plugin The ``ihate`` plugin allows you to automatically skip things you hate during import or warn you about them. You specify queries (see :doc:`/reference/query`) and the plugin skips (or warns about) albums or items -that match any query. +that match any query. You can also specify regular expressions to filter files +to import regarding of their path and name. To use the ``ihate`` plugin, enable it in your configuration (see :ref:`using-plugins`). @@ -19,6 +20,13 @@ file. The available options are: Default: ``[]`` (empty list). - **warn**: Print a warning message for matches in this list of queries. Default: ``[]``. +- **path**: A regular expression to filter files based on its path and name. + Default: ``.*`` (everything) +- **album** and **singleton**: You may specify different regular expressions + used for imports of albums and singletons. This way, you can automatically + skip singletons when importing albums if the names (and paths) of the files + are distinguishable via a regex. The path regex defined here take precedence + over the global ``path`` option. Here's an example:: @@ -33,5 +41,12 @@ Here's an example:: - genre:polka - artist:manowar - album:christmas + path: .*\d\d[^/]+$ + # will only import files which names start with two digits + album: + path: .*\d\d[^/]+$ + singleton: + path: .*/(?!\d\d)[^/]+$ The plugin trusts your decision in "as-is" imports. + diff --git a/test/test_ihate.py b/test/test_ihate.py index 7b4b3de25..af996e740 100644 --- a/test/test_ihate.py +++ b/test/test_ihate.py @@ -12,7 +12,7 @@ from test.helper import capture_log from test.test_importer import ImportHelper -class IHatePluginTest(ImportHelper): +class IHatePluginTest(unittest.TestCase, ImportHelper): def setUp(self): self.setup_beets() self.__create_import_dir(2) @@ -142,207 +142,42 @@ class IHatePluginTest(ImportHelper): def test_import_nothing(self): self.__reset_config() - config['ihate']['regex_invert_folder_result'] = True - config['ihate']['regex_invert_file_result'] = True + config['ihate']['path'] = 'not_there' self.__run([]) # Global options - def test_import_global_match_folder(self): + def test_import_global(self): self.__reset_config() - config['ihate']['regex_folder_name'] = 'artist' + config['ihate']['path'] = '.*track_1.*\.mp3' self.__run([self.artist_paths[0], - self.artist_paths[1]]) - - def test_import_global_invert_folder(self): - self.__reset_config() - config['ihate']['regex_folder_name'] = 'artist' - config['ihate']['regex_invert_folder_result'] = True - self.__run([self.misc_paths[0], - self.misc_paths[1]]) - - def test_import_global_match_file(self): - self.__reset_config() - config['ihate']['regex_file_name'] = '.*2.*' - self.__run([self.artist_paths[1], - self.album_paths[1], - self.misc_paths[1]]) - - def test_import_global_invert_file(self): - self.__reset_config() - config['ihate']['regex_file_name'] = '.*2.*' - config['ihate']['regex_invert_file_result'] = True - self.__run([self.artist_paths[0], - self.album_paths[0], self.misc_paths[0]]) - - def test_import_global_match_folder_case_sensitive(self): - self.__reset_config() - config['ihate']['regex_folder_name'] = 'Artist' - self.__run([]) - - def test_import_global_match_folder_ignore_case(self): - self.__reset_config() - config['ihate']['regex_ignore_case'] = True - config['ihate']['regex_folder_name'] = 'Artist' self.__run([self.artist_paths[0], - self.artist_paths[1]]) + self.misc_paths[0]], singletons=True) # Album options - def test_import_album_match_folder(self): + def test_import_album(self): self.__reset_config() - config['ihate']['album']['regex_folder_name'] = 'artist' + config['ihate']['album']['path'] = '.*track_1.*\.mp3' self.__run([self.artist_paths[0], - self.artist_paths[1]]) - self.__run(self.all_paths, singletons=True) - - def test_import_album_invert_folder(self): - self.__reset_config() - config['ihate']['album']['regex_folder_name'] = 'artist' - config['ihate']['album']['regex_invert_folder_result'] = True - self.__run([self.misc_paths[0], - self.misc_paths[1]]) - self.__run(self.all_paths, singletons=True) - - def test_import_album_match_file(self): - self.__reset_config() - config['ihate']['album']['regex_file_name'] = '.*2.*' - self.__run([self.artist_paths[1], - self.album_paths[1], - self.misc_paths[1]]) - self.__run(self.all_paths, singletons=True) - - def test_import_album_invert_file(self): - self.__reset_config() - config['ihate']['album']['regex_file_name'] = '.*2.*' - config['ihate']['album']['regex_invert_file_result'] = True - self.__run([self.artist_paths[0], - self.album_paths[0], self.misc_paths[0]]) self.__run(self.all_paths, singletons=True) - def test_import_album_match_folder_case_sensitive(self): - self.__reset_config() - config['ihate']['album']['regex_folder_name'] = 'Artist' - self.__run([]) - self.__run(self.all_paths, singletons=True) - - def test_import_album_match_folder_ignore_case(self): - self.__reset_config() - config['ihate']['regex_ignore_case'] = True - config['ihate']['album']['regex_folder_name'] = 'Artist' - self.__run([self.artist_paths[0], - self.artist_paths[1]]) - self.__run(self.all_paths, singletons=True) - # Singleton options - def test_import_singleton_match_folder(self): + def test_import_singleton(self): self.__reset_config() - config['ihate']['singleton']['regex_folder_name'] = 'artist' + config['ihate']['singleton']['path'] = '.*track_1.*\.mp3' self.__run([self.artist_paths[0], - self.artist_paths[1]], singletons=True) - self.__run(self.all_paths) - - def test_import_singleton_invert_folder(self): - self.__reset_config() - config['ihate']['singleton']['regex_folder_name'] = 'artist' - config['ihate']['singleton']['regex_invert_folder_result'] = True - self.__run([self.misc_paths[0], - self.misc_paths[1]], singletons=True) - self.__run(self.all_paths) - - def test_import_singleton_match_file(self): - self.__reset_config() - config['ihate']['singleton']['regex_file_name'] = '.*2.*' - self.__run([self.artist_paths[1], - self.album_paths[1], - self.misc_paths[1]], singletons=True) - self.__run(self.all_paths) - - def test_import_singleton_invert_file(self): - self.__reset_config() - config['ihate']['singleton']['regex_file_name'] = '.*2.*' - config['ihate']['singleton']['regex_invert_file_result'] = True - self.__run([self.artist_paths[0], - self.album_paths[0], self.misc_paths[0]], singletons=True) self.__run(self.all_paths) - def test_import_singleton_match_folder_case_sensitive(self): - self.__reset_config() - config['ihate']['singleton']['regex_folder_name'] = 'Artist' - self.__run([], singletons=True) - self.__run(self.all_paths) - - def test_import_singleton_match_folder_ignore_case(self): - self.__reset_config() - config['ihate']['regex_ignore_case'] = True - config['ihate']['singleton']['regex_folder_name'] = 'Artist' - self.__run([self.artist_paths[0], - self.artist_paths[1]], singletons=True) - self.__run(self.all_paths) - # Album and singleton options - def test_import_both_match_folder(self): + def test_import_both(self): self.__reset_config() - config['ihate']['album']['regex_folder_name'] = 'artist' - config['ihate']['singleton']['regex_folder_name'] = 'misc' + config['ihate']['album']['path'] = '.*track_1.*\.mp3' + config['ihate']['singleton']['path'] = '.*track_2.*\.mp3' self.__run([self.artist_paths[0], - self.artist_paths[1]]) - self.__run([self.misc_paths[0], - self.misc_paths[1]], singletons=True) - - def test_import_both_invert_folder(self): - self.__reset_config() - config['ihate']['album']['regex_folder_name'] = 'artist' - config['ihate']['album']['regex_invert_folder_result'] = True - config['ihate']['singleton']['regex_folder_name'] = 'misc' - config['ihate']['singleton']['regex_invert_folder_result'] = True - self.__run([self.misc_paths[0], - self.misc_paths[1]]) - self.__run([self.artist_paths[0], - self.artist_paths[1], - self.album_paths[0], - self.album_paths[1]], singletons=True) - - def test_import_both_match_file(self): - self.__reset_config() - config['ihate']['album']['regex_file_name'] = '.*2.*' - config['ihate']['singleton']['regex_file_name'] = '.*1.*' - self.__run([self.artist_paths[1], - self.album_paths[1], - self.misc_paths[1]]) - self.__run([self.artist_paths[0], - self.album_paths[0], - self.misc_paths[0]], singletons=True) - - def test_import_both_invert_file(self): - self.__reset_config() - config['ihate']['album']['regex_file_name'] = '.*2.*' - config['ihate']['album']['regex_invert_file_result'] = True - config['ihate']['singleton']['regex_file_name'] = '.*1.*' - config['ihate']['singleton']['regex_invert_file_result'] = True - self.__run([self.artist_paths[0], - self.album_paths[0], self.misc_paths[0]]) self.__run([self.artist_paths[1], - self.album_paths[1], - self.misc_paths[1]], singletons=True) - - def test_import_both_match_folder_case_sensitive(self): - self.__reset_config() - config['ihate']['album']['regex_folder_name'] = 'Artist' - config['ihate']['singleton']['regex_folder_name'] = 'Misc' - self.__run([]) - self.__run([], singletons=True) - - def test_import_both_match_folder_ignore_case(self): - self.__reset_config() - config['ihate']['regex_ignore_case'] = True - config['ihate']['album']['regex_folder_name'] = 'Artist' - config['ihate']['singleton']['regex_folder_name'] = 'Misc' - self.__run([self.artist_paths[0], - self.artist_paths[1]]) - self.__run([self.misc_paths[0], self.misc_paths[1]], singletons=True) From 0e74c5dbaa5556a8667d6c1fb62ada1e91aeed5f Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Wed, 31 Dec 2014 11:56:50 +0100 Subject: [PATCH 08/56] Rearranged regex config options to reduce hierarchies --- beetsplug/ihate.py | 13 ++++--------- docs/plugins/ihate.rst | 16 +++++++--------- test/test_ihate.py | 8 ++++---- 3 files changed, 15 insertions(+), 22 deletions(-) diff --git a/beetsplug/ihate.py b/beetsplug/ihate.py index b7980e02c..3fdcb26f7 100644 --- a/beetsplug/ihate.py +++ b/beetsplug/ihate.py @@ -57,17 +57,12 @@ class IHatePlugin(BeetsPlugin): self.path_singleton_regex = \ re.compile(self.config['path'].get()) - if 'album' in self.config: - album_config = self.config['album'] - if 'path' in album_config: - self.path_album_regex = re.compile( - album_config['path'].get()) + if 'album_path' in self.config: + self.path_album_regex = re.compile(self.config['album_path'].get()) - if 'singleton' in self.config: - singleton_config = self.config['singleton'] - if 'path' in singleton_config: + if 'singleton_path' in self.config: self.path_singleton_regex = re.compile( - singleton_config['path'].get()) + self.config['singleton_path'].get()) @classmethod def do_i_hate_this(cls, task, action_patterns): diff --git a/docs/plugins/ihate.rst b/docs/plugins/ihate.rst index 3fb5e4ffc..14d1219ec 100644 --- a/docs/plugins/ihate.rst +++ b/docs/plugins/ihate.rst @@ -22,11 +22,11 @@ file. The available options are: Default: ``[]``. - **path**: A regular expression to filter files based on its path and name. Default: ``.*`` (everything) -- **album** and **singleton**: You may specify different regular expressions - used for imports of albums and singletons. This way, you can automatically - skip singletons when importing albums if the names (and paths) of the files - are distinguishable via a regex. The path regex defined here take precedence - over the global ``path`` option. +- **album_path** and **singleton_path**: You may specify different regular + expressions used for imports of albums and singletons. This way, you can + automatically skip singletons when importing albums if the names (and paths) + of the files are distinguishable via a regex. The path regex defined here + take precedence over the global ``path`` option. Here's an example:: @@ -43,10 +43,8 @@ Here's an example:: - album:christmas path: .*\d\d[^/]+$ # will only import files which names start with two digits - album: - path: .*\d\d[^/]+$ - singleton: - path: .*/(?!\d\d)[^/]+$ + album_path: .*\d\d[^/]+$ + singleton_path: .*/(?!\d\d)[^/]+$ The plugin trusts your decision in "as-is" imports. diff --git a/test/test_ihate.py b/test/test_ihate.py index af996e740..7d25f5938 100644 --- a/test/test_ihate.py +++ b/test/test_ihate.py @@ -157,7 +157,7 @@ class IHatePluginTest(unittest.TestCase, ImportHelper): # Album options def test_import_album(self): self.__reset_config() - config['ihate']['album']['path'] = '.*track_1.*\.mp3' + config['ihate']['album_path'] = '.*track_1.*\.mp3' self.__run([self.artist_paths[0], self.misc_paths[0]]) self.__run(self.all_paths, singletons=True) @@ -165,7 +165,7 @@ class IHatePluginTest(unittest.TestCase, ImportHelper): # Singleton options def test_import_singleton(self): self.__reset_config() - config['ihate']['singleton']['path'] = '.*track_1.*\.mp3' + config['ihate']['singleton_path'] = '.*track_1.*\.mp3' self.__run([self.artist_paths[0], self.misc_paths[0]], singletons=True) self.__run(self.all_paths) @@ -173,8 +173,8 @@ class IHatePluginTest(unittest.TestCase, ImportHelper): # Album and singleton options def test_import_both(self): self.__reset_config() - config['ihate']['album']['path'] = '.*track_1.*\.mp3' - config['ihate']['singleton']['path'] = '.*track_2.*\.mp3' + config['ihate']['album_path'] = '.*track_1.*\.mp3' + config['ihate']['singleton_path'] = '.*track_2.*\.mp3' self.__run([self.artist_paths[0], self.misc_paths[0]]) self.__run([self.artist_paths[1], From 7f4a06d12ca3985792c85f7a25c5d30b92967f0a Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Wed, 31 Dec 2014 16:57:09 +0100 Subject: [PATCH 09/56] Instead of emitting the import_task_created event using a pipeline stage, it is fired every time an import task was created. --- beets/importer.py | 32 ++++++++++++++++---------------- test/test_ihate.py | 2 +- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index e07b49f48..e383b5aa0 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -277,8 +277,6 @@ class ImportSession(object): else: stages = [query_tasks(self)] - stages += [send_import_task_created_event(self)] - if self.config['pretend']: # Only log the imported files and end the pipeline stages += [log_files(self)] @@ -1091,6 +1089,8 @@ def read_tasks(session): log.debug(u'extracting archive {0}' .format(displayable_path(toppath))) archive_task = ArchiveImportTask(toppath) + plugins.send('import_task_created', session=session, + task=archive_task) try: archive_task.extract() except Exception as exc: @@ -1103,6 +1103,7 @@ def read_tasks(session): task_factory = ImportTaskFactory(toppath, session) imported = False for t in task_factory.tasks(): + plugins.send('import_task_created', session=session, task=t) imported |= not t.skip yield t @@ -1130,7 +1131,9 @@ def query_tasks(session): if session.config['singletons']: # Search for items. for item in session.lib.items(session.query): - yield SingletonImportTask(None, item) + task = SingletonImportTask(None, item) + plugins.send('import_task_created', session=session, task=task) + yield task else: # Search for albums. @@ -1145,7 +1148,9 @@ def query_tasks(session): item.id = None item.album_id = None - yield ImportTask(None, [album.item_dir()], items) + task = ImportTask(None, [album.item_dir()], items) + plugins.send('import_task_created', session=session, task=task) + yield task @pipeline.mutator_stage @@ -1191,7 +1196,10 @@ def user_query(session, task): # Set up a little pipeline for dealing with the singletons. def emitter(task): for item in task.items: - yield SingletonImportTask(task.toppath, item) + new_task = SingletonImportTask(task.toppath, item) + plugins.send('import_task_created', session=session, + task=new_task) + yield new_task yield SentinelImportTask(task.toppath, task.paths) ipl = pipeline.Pipeline([ @@ -1311,16 +1319,6 @@ def log_files(session, task): log.info(displayable_path(item['path'])) -@pipeline.mutator_stage -def send_import_task_created_event(session, task): - """A coroutine (pipeline stage) to send the import_task_created event - """ - if task.skip: - return - - plugins.send('import_task_created', session=session, task=task) - - def group_albums(session): """Group the items of a task by albumartist and album name and create a new task for each album. Yield the tasks as a multi message. @@ -1335,7 +1333,9 @@ def group_albums(session): continue tasks = [] for _, items in itertools.groupby(task.items, group): - tasks.append(ImportTask(items=list(items))) + new_task = ImportTask(items=list(items)) + plugins.send('import_task_created', session=session, task=new_task) + tasks.append(new_task) tasks.append(SentinelImportTask(task.toppath, task.paths)) task = pipeline.multiple(tasks) diff --git a/test/test_ihate.py b/test/test_ihate.py index 7d25f5938..271a2c083 100644 --- a/test/test_ihate.py +++ b/test/test_ihate.py @@ -143,7 +143,7 @@ class IHatePluginTest(unittest.TestCase, ImportHelper): def test_import_nothing(self): self.__reset_config() config['ihate']['path'] = 'not_there' - self.__run([]) + self.__run(['No files imported from %s' % self.import_dir]) # Global options def test_import_global(self): From 61a20b31634e64423f4234a1997a8923e540c251 Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Wed, 31 Dec 2014 17:15:52 +0100 Subject: [PATCH 10/56] Modified the tests to match the new `--pretend` output. --- test/test_ihate.py | 96 +++++++++++++++++++++++++++++++++------------- 1 file changed, 69 insertions(+), 27 deletions(-) diff --git a/test/test_ihate.py b/test/test_ihate.py index 271a2c083..26f73f780 100644 --- a/test/test_ihate.py +++ b/test/test_ihate.py @@ -6,6 +6,7 @@ from _common import unittest from beets import importer, config from beets.library import Item from beets.mediafile import MediaFile +from beets.util import displayable_path from beetsplug.ihate import IHatePlugin from test import _common from test.helper import capture_log @@ -19,10 +20,6 @@ class IHatePluginTest(unittest.TestCase, ImportHelper): self._setup_import_session() config['import']['pretend'] = True - self.all_paths = [self.artist_paths[0], self.artist_paths[1], - self.album_paths[0], self.album_paths[1], - self.misc_paths[0], self.misc_paths[1]] - def tearDown(self): self.teardown_beets() @@ -41,11 +38,11 @@ class IHatePluginTest(unittest.TestCase, ImportHelper): if os.path.isdir(self.import_dir): shutil.rmtree(self.import_dir) - artist_path = os.path.join(self.import_dir, 'artist') - album_path = os.path.join(artist_path, 'album') - misc_path = os.path.join(self.import_dir, 'misc') - os.makedirs(album_path) - os.makedirs(misc_path) + self.artist_path = os.path.join(self.import_dir, 'artist') + self.album_path = os.path.join(self.artist_path, 'album') + self.misc_path = os.path.join(self.import_dir, 'misc') + os.makedirs(self.album_path) + os.makedirs(self.misc_path) metadata = { 'artist': 'Tag Artist', @@ -59,7 +56,7 @@ class IHatePluginTest(unittest.TestCase, ImportHelper): for i in range(count): metadata['track'] = i + 1 metadata['title'] = 'Tag Title Album %d' % (i + 1) - dest_path = os.path.join(album_path, '%02d - track.mp3' % (i + 1)) + dest_path = os.path.join(self.album_path, '%02d - track.mp3' % (i + 1)) self.__copy_file(dest_path, metadata) self.album_paths.append(dest_path) @@ -68,7 +65,7 @@ class IHatePluginTest(unittest.TestCase, ImportHelper): for i in range(count): metadata['track'] = i + 10 metadata['title'] = 'Tag Title Artist %d' % (i + 1) - dest_path = os.path.join(artist_path, 'track_%d.mp3' % (i + 1)) + dest_path = os.path.join(self.artist_path, 'track_%d.mp3' % (i + 1)) self.__copy_file(dest_path, metadata) self.artist_paths.append(dest_path) @@ -77,7 +74,7 @@ class IHatePluginTest(unittest.TestCase, ImportHelper): metadata['artist'] = 'Artist %d' % (i + 42) metadata['track'] = i + 5 metadata['title'] = 'Tag Title Misc %d' % (i + 1) - dest_path = os.path.join(misc_path, 'track_%d.mp3' % (i + 1)) + dest_path = os.path.join(self.misc_path, 'track_%d.mp3' % (i + 1)) self.__copy_file(dest_path, metadata) self.misc_paths.append(dest_path) @@ -138,7 +135,17 @@ class IHatePluginTest(unittest.TestCase, ImportHelper): """ The default configuration should import everything. """ self.__reset_config() - self.__run(self.all_paths) + self.__run([ + 'Album %s' % displayable_path(self.artist_path), + ' %s' % displayable_path(self.artist_paths[0]), + ' %s' % displayable_path(self.artist_paths[1]), + 'Album %s' % displayable_path(self.album_path), + ' %s' % displayable_path(self.album_paths[0]), + ' %s' % displayable_path(self.album_paths[1]), + 'Album %s' % displayable_path(self.misc_path), + ' %s' % displayable_path(self.misc_paths[0]), + ' %s' % displayable_path(self.misc_paths[1]) + ]) def test_import_nothing(self): self.__reset_config() @@ -149,36 +156,71 @@ class IHatePluginTest(unittest.TestCase, ImportHelper): def test_import_global(self): self.__reset_config() config['ihate']['path'] = '.*track_1.*\.mp3' - self.__run([self.artist_paths[0], - self.misc_paths[0]]) - self.__run([self.artist_paths[0], - self.misc_paths[0]], singletons=True) + self.__run([ + 'Album %s' % displayable_path(self.artist_path), + ' %s' % displayable_path(self.artist_paths[0]), + 'Album %s' % displayable_path(self.misc_path), + ' %s' % displayable_path(self.misc_paths[0]), + ]) + self.__run([ + 'Singleton: %s' % displayable_path(self.artist_paths[0]), + 'Singleton: %s' % displayable_path(self.misc_paths[0]) + ], singletons=True) # Album options def test_import_album(self): self.__reset_config() config['ihate']['album_path'] = '.*track_1.*\.mp3' - self.__run([self.artist_paths[0], - self.misc_paths[0]]) - self.__run(self.all_paths, singletons=True) + self.__run([ + 'Album %s' % displayable_path(self.artist_path), + ' %s' % displayable_path(self.artist_paths[0]), + 'Album %s' % displayable_path(self.misc_path), + ' %s' % displayable_path(self.misc_paths[0]), + ]) + self.__run([ + 'Singleton: %s' % displayable_path(self.artist_paths[0]), + 'Singleton: %s' % displayable_path(self.artist_paths[1]), + 'Singleton: %s' % displayable_path(self.album_paths[0]), + 'Singleton: %s' % displayable_path(self.album_paths[1]), + 'Singleton: %s' % displayable_path(self.misc_paths[0]), + 'Singleton: %s' % displayable_path(self.misc_paths[1]) + ], singletons=True) # Singleton options def test_import_singleton(self): self.__reset_config() config['ihate']['singleton_path'] = '.*track_1.*\.mp3' - self.__run([self.artist_paths[0], - self.misc_paths[0]], singletons=True) - self.__run(self.all_paths) + self.__run([ + 'Singleton: %s' % displayable_path(self.artist_paths[0]), + 'Singleton: %s' % displayable_path(self.misc_paths[0]) + ], singletons=True) + self.__run([ + 'Album %s' % displayable_path(self.artist_path), + ' %s' % displayable_path(self.artist_paths[0]), + ' %s' % displayable_path(self.artist_paths[1]), + 'Album %s' % displayable_path(self.album_path), + ' %s' % displayable_path(self.album_paths[0]), + ' %s' % displayable_path(self.album_paths[1]), + 'Album %s' % displayable_path(self.misc_path), + ' %s' % displayable_path(self.misc_paths[0]), + ' %s' % displayable_path(self.misc_paths[1]) + ]) # Album and singleton options def test_import_both(self): self.__reset_config() config['ihate']['album_path'] = '.*track_1.*\.mp3' config['ihate']['singleton_path'] = '.*track_2.*\.mp3' - self.__run([self.artist_paths[0], - self.misc_paths[0]]) - self.__run([self.artist_paths[1], - self.misc_paths[1]], singletons=True) + self.__run([ + 'Album %s' % displayable_path(self.artist_path), + ' %s' % displayable_path(self.artist_paths[0]), + 'Album %s' % displayable_path(self.misc_path), + ' %s' % displayable_path(self.misc_paths[0]), + ]) + self.__run([ + 'Singleton: %s' % displayable_path(self.artist_paths[1]), + 'Singleton: %s' % displayable_path(self.misc_paths[1]) + ], singletons=True) def suite(): From c560a3d0423c7a2036937b386eb790c2064be3bb Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Wed, 31 Dec 2014 17:20:47 +0100 Subject: [PATCH 11/56] Recent changes broke the flake8 rules... --- test/test_ihate.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/test_ihate.py b/test/test_ihate.py index 26f73f780..04ebf4132 100644 --- a/test/test_ihate.py +++ b/test/test_ihate.py @@ -56,7 +56,8 @@ class IHatePluginTest(unittest.TestCase, ImportHelper): for i in range(count): metadata['track'] = i + 1 metadata['title'] = 'Tag Title Album %d' % (i + 1) - dest_path = os.path.join(self.album_path, '%02d - track.mp3' % (i + 1)) + dest_path = os.path.join(self.album_path, + '%02d - track.mp3' % (i + 1)) self.__copy_file(dest_path, metadata) self.album_paths.append(dest_path) @@ -65,7 +66,8 @@ class IHatePluginTest(unittest.TestCase, ImportHelper): for i in range(count): metadata['track'] = i + 10 metadata['title'] = 'Tag Title Artist %d' % (i + 1) - dest_path = os.path.join(self.artist_path, 'track_%d.mp3' % (i + 1)) + dest_path = os.path.join(self.artist_path, + 'track_%d.mp3' % (i + 1)) self.__copy_file(dest_path, metadata) self.artist_paths.append(dest_path) From 09164c2d3b67480a34182755c90d8118e5e191b3 Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Fri, 2 Jan 2015 17:42:50 +0100 Subject: [PATCH 12/56] All import tasks are now created by the ImportTaskFactory. --- beets/importer.py | 89 +++++++++++++++++++++++++---------------------- 1 file changed, 47 insertions(+), 42 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index c3c0fcb28..e6802cb95 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -1000,47 +1000,60 @@ class ImportTaskFactory(object): for dirs, paths in albums_in_dir(self.toppath): yield (dirs, paths) - def singleton(self, path): - if self.session.already_imported(self.toppath, [path]): - log.debug(u'Skipping previously-imported path: {0}' - .format(displayable_path(path))) - self.skipped += 1 - return None + def singleton(self, path, item=None): + if not item: + if self.session.already_imported(self.toppath, [path]): + log.debug(u'Skipping previously-imported path: {0}' + .format(displayable_path(path))) + self.skipped += 1 + return None + + item = self.read_item(path) - item = self.read_item(path) if item: - return SingletonImportTask(self.toppath, item) + return self.__handle_plugins(SingletonImportTask(self.toppath, + item)) else: return None - def album(self, paths, dirs=None): + def album(self, paths, dirs=None, items=None): """Return `ImportTask` with all media files from paths. `dirs` is a list of parent directories used to record already imported albums. """ - if not paths: - return None + if not items: + if not paths: + return None - if dirs is None: - dirs = list(set(os.path.dirname(p) for p in paths)) + if dirs is None: + dirs = list(set(os.path.dirname(p) for p in paths)) - if self.session.already_imported(self.toppath, dirs): - log.debug(u'Skipping previously-imported path: {0}' - .format(displayable_path(dirs))) - self.skipped += 1 - return None + if self.session.already_imported(self.toppath, dirs): + log.debug(u'Skipping previously-imported path: {0}' + .format(displayable_path(dirs))) + self.skipped += 1 + return None - items = map(self.read_item, paths) - items = [item for item in items if item] + items = map(self.read_item, paths) + items = [item for item in items if item] if items: - return ImportTask(self.toppath, dirs, items) + return self.__handle_plugins(ImportTask(self.toppath, dirs, items)) else: return None def sentinel(self, paths=None): - return SentinelImportTask(self.toppath, paths) + return self.__handle_plugins(SentinelImportTask(self.toppath, paths)) + + def archive(self, path): + return self.__handle_plugins(ArchiveImportTask(path)) + + def __handle_plugins(self, task): + plugins.send('import_task_created', session=self.session, + task=task) + # TODO: Use value(s) returned by plugins. + return task def read_item(self, path): """Return an item created from the path. @@ -1077,6 +1090,7 @@ def read_tasks(session): # Determine if we want to resume import of the toppath session.ask_resume(toppath) user_toppath = toppath + task_factory = ImportTaskFactory(toppath, session) # Extract archives. archive_task = None @@ -1088,9 +1102,7 @@ def read_tasks(session): log.debug(u'extracting archive {0}' .format(displayable_path(toppath))) - archive_task = ArchiveImportTask(toppath) - plugins.send('import_task_created', session=session, - task=archive_task) + archive_task = task_factory.archive(toppath) try: archive_task.extract() except Exception as exc: @@ -1099,11 +1111,10 @@ def read_tasks(session): # Continue reading albums from the extracted directory. toppath = archive_task.toppath + task_factory.toppath = toppath - task_factory = ImportTaskFactory(toppath, session) imported = False for t in task_factory.tasks(): - plugins.send('import_task_created', session=session, task=t) imported |= not t.skip yield t @@ -1128,12 +1139,11 @@ def query_tasks(session): Instead of finding files from the filesystem, a query is used to match items from the library. """ + task_factory = ImportTaskFactory(None) if session.config['singletons']: # Search for items. for item in session.lib.items(session.query): - task = SingletonImportTask(None, item) - plugins.send('import_task_created', session=session, task=task) - yield task + yield task_factory.singleton(None, item) else: # Search for albums. @@ -1148,9 +1158,7 @@ def query_tasks(session): item.id = None item.album_id = None - task = ImportTask(None, [album.item_dir()], items) - plugins.send('import_task_created', session=session, task=task) - yield task + yield task_factory.album(None, [album.item_dir()], items) @pipeline.mutator_stage @@ -1195,12 +1203,10 @@ def user_query(session, task): if task.choice_flag is action.TRACKS: # Set up a little pipeline for dealing with the singletons. def emitter(task): + task_factory = ImportTaskFactory(task.toppath, session) for item in task.items: - new_task = SingletonImportTask(task.toppath, item) - plugins.send('import_task_created', session=session, - task=new_task) - yield new_task - yield SentinelImportTask(task.toppath, task.paths) + yield task_factory.singleton(None, item) + yield task_factory.sentinel(task.paths) ipl = pipeline.Pipeline([ emitter(task), @@ -1334,11 +1340,10 @@ def group_albums(session): if task.skip: continue tasks = [] + task_factory = ImportTaskFactory(task.toppath, session) for _, items in itertools.groupby(task.items, group): - new_task = ImportTask(items=list(items)) - plugins.send('import_task_created', session=session, task=new_task) - tasks.append(new_task) - tasks.append(SentinelImportTask(task.toppath, task.paths)) + tasks.append(task_factory.album(None, None, list(items))) + tasks.append(task_factory.sentinel(task.paths)) task = pipeline.multiple(tasks) From 7e21406ad0e66b7e3cd8830ec5ea9f516d790d5f Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Fri, 2 Jan 2015 18:51:00 +0100 Subject: [PATCH 13/56] Event handler implemented in plugins may now return values. Handler for `import_task_created` can return a list of tasks to substitute the given task. --- beets/importer.py | 57 +++++++++++++++++++++++++++++++---------------- beets/plugins.py | 9 +++++++- 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index e6802cb95..ce407d701 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -975,15 +975,18 @@ class ImportTaskFactory(object): for dirs, paths in self.paths(): if self.session.config['singletons']: for path in paths: - task = self.singleton(path) - if task: - yield task - yield self.sentinel(dirs) + tasks = self.singleton(path) + if tasks: + for task in tasks: + yield task + for task in self.sentinel(dirs): + yield task else: - task = self.album(paths, dirs) - if task: - yield task + tasks = self.album(paths, dirs) + if tasks: + for task in tasks: + yield task def paths(self): """Walk `self.toppath` and yield pairs of directory lists and @@ -1050,10 +1053,11 @@ class ImportTaskFactory(object): return self.__handle_plugins(ArchiveImportTask(path)) def __handle_plugins(self, task): - plugins.send('import_task_created', session=self.session, - task=task) - # TODO: Use value(s) returned by plugins. - return task + tasks = plugins.send('import_task_created', session=self.session, + task=task) + if not tasks: + tasks = [task] + return tasks def read_item(self, path): """Return an item created from the path. @@ -1102,7 +1106,7 @@ def read_tasks(session): log.debug(u'extracting archive {0}' .format(displayable_path(toppath))) - archive_task = task_factory.archive(toppath) + archive_task = task_factory.archive(toppath)[0] try: archive_task.extract() except Exception as exc: @@ -1121,7 +1125,8 @@ def read_tasks(session): # Indicate the directory is finished. # FIXME hack to delete extracted archives if archive_task is None: - yield task_factory.sentinel() + for task in task_factory.sentinel(): + yield task else: yield archive_task @@ -1143,7 +1148,10 @@ def query_tasks(session): if session.config['singletons']: # Search for items. for item in session.lib.items(session.query): - yield task_factory.singleton(None, item) + tasks = task_factory.singleton(None, item) + if tasks: + for task in tasks: + yield task else: # Search for albums. @@ -1158,7 +1166,10 @@ def query_tasks(session): item.id = None item.album_id = None - yield task_factory.album(None, [album.item_dir()], items) + tasks = task_factory.album(None, [album.item_dir()], items) + if tasks: + for task in tasks: + yield task @pipeline.mutator_stage @@ -1205,8 +1216,12 @@ def user_query(session, task): def emitter(task): task_factory = ImportTaskFactory(task.toppath, session) for item in task.items: - yield task_factory.singleton(None, item) - yield task_factory.sentinel(task.paths) + new_tasks = task_factory.singleton(None, item) + if new_tasks: + for t in new_tasks: + yield t + for t in task_factory.sentinel(task.paths): + yield t ipl = pipeline.Pipeline([ emitter(task), @@ -1342,8 +1357,12 @@ def group_albums(session): tasks = [] task_factory = ImportTaskFactory(task.toppath, session) for _, items in itertools.groupby(task.items, group): - tasks.append(task_factory.album(None, None, list(items))) - tasks.append(task_factory.sentinel(task.paths)) + new_tasks = task_factory.album(None, None, list(items)) + if new_tasks: + for t in new_tasks: + tasks.append(t) + for t in task_factory.sentinel(task.paths): + tasks.append(t) task = pipeline.multiple(tasks) diff --git a/beets/plugins.py b/beets/plugins.py index 8611b92a6..3ad2909be 100755 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -399,11 +399,18 @@ def send(event, **arguments): Returns a list of return values from the handlers. """ log.debug(u'Sending event: {0}'.format(event)) + return_values = [] for handler in event_handlers()[event]: # Don't break legacy plugins if we want to pass more arguments argspec = inspect.getargspec(handler).args args = dict((k, v) for k, v in arguments.items() if k in argspec) - handler(**args) + result = handler(**args) + if isinstance(result, list): + return_values += result + else: + return_values.append(result) + + return return_values def feat_tokens(for_artist=True): From 946e7b446dacf18051621d64e1b0bbd086902923 Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Fri, 2 Jan 2015 19:10:57 +0100 Subject: [PATCH 14/56] Take care of None objects when dealing with tasks --- beets/importer.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index ce407d701..59f35e810 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -978,7 +978,8 @@ class ImportTaskFactory(object): tasks = self.singleton(path) if tasks: for task in tasks: - yield task + if task: + yield task for task in self.sentinel(dirs): yield task @@ -986,7 +987,8 @@ class ImportTaskFactory(object): tasks = self.album(paths, dirs) if tasks: for task in tasks: - yield task + if task: + yield task def paths(self): """Walk `self.toppath` and yield pairs of directory lists and @@ -1151,7 +1153,8 @@ def query_tasks(session): tasks = task_factory.singleton(None, item) if tasks: for task in tasks: - yield task + if task: + yield task else: # Search for albums. @@ -1169,7 +1172,8 @@ def query_tasks(session): tasks = task_factory.album(None, [album.item_dir()], items) if tasks: for task in tasks: - yield task + if task: + yield task @pipeline.mutator_stage @@ -1219,7 +1223,8 @@ def user_query(session, task): new_tasks = task_factory.singleton(None, item) if new_tasks: for t in new_tasks: - yield t + if t: + yield t for t in task_factory.sentinel(task.paths): yield t @@ -1360,7 +1365,8 @@ def group_albums(session): new_tasks = task_factory.album(None, None, list(items)) if new_tasks: for t in new_tasks: - tasks.append(t) + if task: + tasks.append(t) for t in task_factory.sentinel(task.paths): tasks.append(t) From 7600d33a5f4114b7f1375174e5369f24d8de74f4 Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Sun, 4 Jan 2015 18:03:03 +0100 Subject: [PATCH 15/56] The return values of plugin event handlers are not flattened any more within the plugin class --- beets/importer.py | 10 ++++++++++ beets/plugins.py | 5 ++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 59f35e810..6a1f71679 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -1059,6 +1059,16 @@ class ImportTaskFactory(object): task=task) if not tasks: tasks = [task] + else: + # The plugins gave us a list of lists of task. Flatten it. + flat_tasks = [] + for inner in tasks: + if isinstance(inner, list): + flat_tasks += inner + else: + flat_tasks.append(inner) + tasks = flat_tasks + return tasks def read_item(self, path): diff --git a/beets/plugins.py b/beets/plugins.py index 3ad2909be..3efe716a4 100755 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -405,9 +405,8 @@ def send(event, **arguments): argspec = inspect.getargspec(handler).args args = dict((k, v) for k, v in arguments.items() if k in argspec) result = handler(**args) - if isinstance(result, list): - return_values += result - else: + # Only append non None return values + if result: return_values.append(result) return return_values From 9da9e84acd1460fc3b23f077aa0382b4a1c96d03 Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Sun, 4 Jan 2015 18:18:08 +0100 Subject: [PATCH 16/56] Factory methods of ImportTaskFactory guaranty that a list is returned and the list does not contain a 'None'. That leads to much cleaner code where the methods are called. --- beets/importer.py | 59 +++++++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 6a1f71679..72b2a32fb 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -976,19 +976,15 @@ class ImportTaskFactory(object): if self.session.config['singletons']: for path in paths: tasks = self.singleton(path) - if tasks: - for task in tasks: - if task: - yield task + for task in tasks: + yield task for task in self.sentinel(dirs): yield task else: tasks = self.album(paths, dirs) - if tasks: - for task in tasks: - if task: - yield task + for task in tasks: + yield task def paths(self): """Walk `self.toppath` and yield pairs of directory lists and @@ -1011,7 +1007,7 @@ class ImportTaskFactory(object): log.debug(u'Skipping previously-imported path: {0}' .format(displayable_path(path))) self.skipped += 1 - return None + return [] item = self.read_item(path) @@ -1019,7 +1015,7 @@ class ImportTaskFactory(object): return self.__handle_plugins(SingletonImportTask(self.toppath, item)) else: - return None + return [] def album(self, paths, dirs=None, items=None): """Return `ImportTask` with all media files from paths. @@ -1029,7 +1025,7 @@ class ImportTaskFactory(object): """ if not items: if not paths: - return None + return [] if dirs is None: dirs = list(set(os.path.dirname(p) for p in paths)) @@ -1038,7 +1034,7 @@ class ImportTaskFactory(object): log.debug(u'Skipping previously-imported path: {0}' .format(displayable_path(dirs))) self.skipped += 1 - return None + return [] items = map(self.read_item, paths) items = [item for item in items if item] @@ -1046,7 +1042,7 @@ class ImportTaskFactory(object): if items: return self.__handle_plugins(ImportTask(self.toppath, dirs, items)) else: - return None + return [] def sentinel(self, paths=None): return self.__handle_plugins(SentinelImportTask(self.toppath, paths)) @@ -1055,6 +1051,17 @@ class ImportTaskFactory(object): return self.__handle_plugins(ArchiveImportTask(path)) def __handle_plugins(self, task): + """ + Sends the 'import_task_created' event to all plugins. Plugins may + return a list of tasks to use instead of the given task. If no plugin + is configured for the event or no plugin returns any value, a list + containing the original task as the only element is returned. + + :param task: The which is intended to create. + :return: A flat list of tasks to create instead of the original task. + The list contains the tasks returned by all plugins. There + will by no None value present at the list. + """ tasks = plugins.send('import_task_created', session=self.session, task=task) if not tasks: @@ -1067,7 +1074,7 @@ class ImportTaskFactory(object): flat_tasks += inner else: flat_tasks.append(inner) - tasks = flat_tasks + tasks = [t for t in flat_tasks if t] return tasks @@ -1161,10 +1168,8 @@ def query_tasks(session): # Search for items. for item in session.lib.items(session.query): tasks = task_factory.singleton(None, item) - if tasks: - for task in tasks: - if task: - yield task + for task in tasks: + yield task else: # Search for albums. @@ -1180,10 +1185,8 @@ def query_tasks(session): item.album_id = None tasks = task_factory.album(None, [album.item_dir()], items) - if tasks: - for task in tasks: - if task: - yield task + for task in tasks: + yield task @pipeline.mutator_stage @@ -1231,10 +1234,8 @@ def user_query(session, task): task_factory = ImportTaskFactory(task.toppath, session) for item in task.items: new_tasks = task_factory.singleton(None, item) - if new_tasks: - for t in new_tasks: - if t: - yield t + for t in new_tasks: + yield t for t in task_factory.sentinel(task.paths): yield t @@ -1373,10 +1374,8 @@ def group_albums(session): task_factory = ImportTaskFactory(task.toppath, session) for _, items in itertools.groupby(task.items, group): new_tasks = task_factory.album(None, None, list(items)) - if new_tasks: - for t in new_tasks: - if task: - tasks.append(t) + for t in new_tasks: + tasks.append(t) for t in task_factory.sentinel(task.paths): tasks.append(t) From d71a8227e28ef422c861fb5051f33dff31e9f6bd Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Sun, 4 Jan 2015 19:02:22 +0100 Subject: [PATCH 17/56] Added documentation and tests --- docs/dev/plugins.rst | 3 ++ test/test_plugins.py | 97 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index 8eb184a5b..bcb6c47d0 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -161,6 +161,9 @@ currently available are: * *import_task_created*: called after an import task has been created. Parameters: ``task`` (an `ImportTask`) and ``session`` (an `ImportSession`). + Return: The event handler may return a single `ImportTask` object or a list + of `ImportTask` objects. The original task will be replaced by the returned + task(s). If nothing is returned, the original task will be created. * *import_task_start*: called when before an import task begins processing. Parameters: ``task`` (an `ImportTask`) and ``session`` (an `ImportSession`). diff --git a/test/test_plugins.py b/test/test_plugins.py index eea162e90..430f90bcd 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -11,15 +11,21 @@ # # The above copyright notice and this permission notice shall be # included in all copies or substantial portions of the Software. +import os from mock import patch +import shutil from _common import unittest +from beets.importer import SingletonImportTask, SentinelImportTask, \ + ArchiveImportTask import helper -from beets import plugins +from beets import plugins, config from beets.library import Item from beets.dbcore import types from beets.mediafile import MediaFile +from test import _common +from test.test_importer import ImportHelper class TestHelper(helper.TestHelper): @@ -151,6 +157,95 @@ class ItemTypeConflictTest(unittest.TestCase, TestHelper): self.assertNotEqual(None, plugins.types(Item)) +class EventsTest(unittest.TestCase, ImportHelper, TestHelper): + + def setUp(self): + self.setup_plugin_loader() + self.setup_beets() + self.__create_import_dir(2) + config['import']['pretend'] = True + + def tearDown(self): + self.teardown_plugin_loader() + self.teardown_beets() + + def __copy_file(self, dest_path, metadata): + # Copy files + resource_path = os.path.join(_common.RSRC, 'full.mp3') + shutil.copy(resource_path, dest_path) + medium = MediaFile(dest_path) + # Set metadata + for attr in metadata: + setattr(medium, attr, metadata[attr]) + medium.save() + + def __create_import_dir(self, count): + self.import_dir = os.path.join(self.temp_dir, 'testsrcdir') + if os.path.isdir(self.import_dir): + shutil.rmtree(self.import_dir) + + self.album_path = os.path.join(self.import_dir, 'album') + os.makedirs(self.album_path) + + metadata = { + 'artist': 'Tag Artist', + 'album': 'Tag Album', + 'albumartist': None, + 'mb_trackid': None, + 'mb_albumid': None, + 'comp': None + } + self.file_paths = [] + for i in range(count): + metadata['track'] = i + 1 + metadata['title'] = 'Tag Title Album %d' % (i + 1) + dest_path = os.path.join(self.album_path, + '%02d - track.mp3' % (i + 1)) + self.__copy_file(dest_path, metadata) + self.file_paths.append(dest_path) + + def test_import_task_created(self): + class ToSingletonPlugin(plugins.BeetsPlugin): + def __init__(self): + super(ToSingletonPlugin, self).__init__() + + self.register_listener('import_task_created', + self.import_task_created_event) + + def import_task_created_event(self, session, task): + if isinstance(task, SingletonImportTask) \ + or isinstance(task, SentinelImportTask)\ + or isinstance(task, ArchiveImportTask): + return task + + new_tasks = [] + for item in task.items: + new_tasks.append(SingletonImportTask(task.toppath, item)) + + return new_tasks + + to_singleton_plugin = ToSingletonPlugin + self.register_plugin(to_singleton_plugin) + + import_files = [self.import_dir] + self._setup_import_session(singletons=False) + self.importer.paths = import_files + + with helper.capture_log() as logs: + self.importer.run() + self.unload_plugins() + + self.assertEqual(logs.count('Sending event: import_task_created'), 2, + 'Only two import_task_created events (one for the ' + 'album and one for the sentinel)') + logs = [line for line in logs if not line.startswith('Sending event:')] + + self.assertEqual(logs, [ + 'Singleton: %s' % self.file_paths[0], + 'Singleton: %s' % self.file_paths[1] + ]) + + class HelpersTest(unittest.TestCase): def test_sanitize_choices(self): From 9fe2bc1a381b27a958b6ddc8d0b67258d290ecb6 Mon Sep 17 00:00:00 2001 From: Andre Miller Date: Sun, 18 Jan 2015 10:23:49 +0200 Subject: [PATCH 18/56] Support for CORS --- beetsplug/web/__init__.py | 11 ++++++ beetsplug/web/crossdomaindec.py | 60 +++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 beetsplug/web/crossdomaindec.py diff --git a/beetsplug/web/__init__.py b/beetsplug/web/__init__.py index a60461689..24bbf271e 100644 --- a/beetsplug/web/__init__.py +++ b/beetsplug/web/__init__.py @@ -22,6 +22,7 @@ from flask import g from werkzeug.routing import BaseConverter, PathConverter import os import json +from crossdomaindec import crossdomain # Utilities. @@ -164,6 +165,7 @@ def before_request(): # Items. @app.route('/item/') +@crossdomain(origin='*') @resource('items') def get_item(id): return g.lib.get_item(id) @@ -171,12 +173,14 @@ def get_item(id): @app.route('/item/') @app.route('/item/query/') +@crossdomain(origin='*') @resource_list('items') def all_items(): return g.lib.items() @app.route('/item//file') +@crossdomain(origin='*') def item_file(item_id): item = g.lib.get_item(item_id) response = flask.send_file(item.path, as_attachment=True, @@ -186,6 +190,7 @@ def item_file(item_id): @app.route('/item/query/') +@crossdomain(origin='*') @resource_query('items') def item_query(queries): return g.lib.items(queries) @@ -194,6 +199,7 @@ def item_query(queries): # Albums. @app.route('/album/') +@crossdomain(origin='*') @resource('albums') def get_album(id): return g.lib.get_album(id) @@ -201,18 +207,21 @@ def get_album(id): @app.route('/album/') @app.route('/album/query/') +@crossdomain(origin='*') @resource_list('albums') def all_albums(): return g.lib.albums() @app.route('/album/query/') +@crossdomain(origin='*') @resource_query('albums') def album_query(queries): return g.lib.albums(queries) @app.route('/album//art') +@crossdomain(origin='*') def album_art(album_id): album = g.lib.get_album(album_id) return flask.send_file(album.artpath) @@ -221,6 +230,7 @@ def album_art(album_id): # Artists. @app.route('/artist/') +@crossdomain(origin='*') def all_artists(): with g.lib.transaction() as tx: rows = tx.query("SELECT DISTINCT albumartist FROM albums") @@ -231,6 +241,7 @@ def all_artists(): # Library information. @app.route('/stats') +@crossdomain(origin='*') def stats(): with g.lib.transaction() as tx: item_rows = tx.query("SELECT COUNT(*) FROM items") diff --git a/beetsplug/web/crossdomaindec.py b/beetsplug/web/crossdomaindec.py new file mode 100644 index 000000000..622b0455c --- /dev/null +++ b/beetsplug/web/crossdomaindec.py @@ -0,0 +1,60 @@ +# Decorator for the HTTP Access Control +# By Armin Ronacher +# http://flask.pocoo.org/snippets/56/ +# +# Cross-site HTTP requests are HTTP requests for resources from a different +# domain than the domain of the resource making the request. +# For instance, a resource loaded from Domain A makes a request for a resource +# on Domain B. The way this is implemented in modern browsers is by using +# HTTP Access Control headers +# +# https://developer.mozilla.org/en/HTTP_access_control +# +# The following view decorator implements this +# + +from datetime import timedelta +from flask import make_response, request, current_app +from functools import update_wrapper + + +def crossdomain(origin=None, methods=None, headers=None, + max_age=21600, attach_to_all=True, + automatic_options=True): + if methods is not None: + methods = ', '.join(sorted(x.upper() for x in methods)) + if headers is not None and not isinstance(headers, basestring): + headers = ', '.join(x.upper() for x in headers) + if not isinstance(origin, basestring): + origin = ', '.join(origin) + if isinstance(max_age, timedelta): + max_age = max_age.total_seconds() + + def get_methods(): + if methods is not None: + return methods + + options_resp = current_app.make_default_options_response() + return options_resp.headers['allow'] + + def decorator(f): + def wrapped_function(*args, **kwargs): + if automatic_options and request.method == 'OPTIONS': + resp = current_app.make_default_options_response() + else: + resp = make_response(f(*args, **kwargs)) + if not attach_to_all and request.method != 'OPTIONS': + return resp + + h = resp.headers + + h['Access-Control-Allow-Origin'] = origin + h['Access-Control-Allow-Methods'] = get_methods() + h['Access-Control-Max-Age'] = str(max_age) + if headers is not None: + h['Access-Control-Allow-Headers'] = headers + return resp + + f.provide_automatic_options = False + return update_wrapper(wrapped_function, f) + return decorator From c91f7f77952e9e77947b225833c8b59884d80aa0 Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Sun, 18 Jan 2015 19:07:20 +0100 Subject: [PATCH 19/56] Archive tasks may be a list of tasks instead of a single task now --- beets/importer.py | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 192a66b07..cd968261a 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -1108,7 +1108,7 @@ def read_tasks(session): task_factory = ImportTaskFactory(toppath, session) # Extract archives. - archive_task = None + archive_tasks = None if ArchiveImportTask.is_archive(syspath(toppath)): if not (session.config['move'] or session.config['copy']): log.warn(u"Archive importing requires either " @@ -1117,16 +1117,17 @@ def read_tasks(session): log.debug(u'extracting archive {0}', displayable_path(toppath)) - archive_task = task_factory.archive(toppath)[0] - try: - archive_task.extract() - except Exception as exc: - log.error(u'extraction failed: {0}', exc) - continue + archive_tasks = task_factory.archive(toppath)[0] + for archive_task in archive_tasks: + try: + archive_task.extract() + except Exception as exc: + log.error(u'extraction failed: {0}', exc) + continue - # Continue reading albums from the extracted directory. - toppath = archive_task.toppath - task_factory.toppath = toppath + # Continue reading albums from the extracted directory. + toppath = archive_task.toppath + task_factory.toppath = toppath imported = False for t in task_factory.tasks(): @@ -1135,11 +1136,12 @@ def read_tasks(session): # Indicate the directory is finished. # FIXME hack to delete extracted archives - if archive_task is None: + if archive_tasks is None or len(archive_tasks) == 0: for task in task_factory.sentinel(): yield task else: - yield archive_task + for archive_task in archive_tasks: + yield archive_task if not imported: log.warn(u'No files imported from {0}', From 0afe0a60a116bb3408336e77cd63ff7995952f8c Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Sun, 18 Jan 2015 19:16:51 +0100 Subject: [PATCH 20/56] Instead of using the list of archive tasks for further importing, only the fist task was used - which is not iterable, of course. --- beets/importer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beets/importer.py b/beets/importer.py index cd968261a..1c5e54e2f 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -1117,7 +1117,7 @@ def read_tasks(session): log.debug(u'extracting archive {0}', displayable_path(toppath)) - archive_tasks = task_factory.archive(toppath)[0] + archive_tasks = task_factory.archive(toppath) for archive_task in archive_tasks: try: archive_task.extract() From cd1564f584fb16f18ffb679f1e969382202abfd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Frederik=20=E2=80=9CFreso=E2=80=9D=20S=2E=20Olesen?= Date: Mon, 19 Jan 2015 18:42:40 +0100 Subject: [PATCH 21/56] changelog: Move lastgenre genre additions to "Features". See https://botbot.me/freenode/beets/msg/29935738/ --- docs/changelog.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 3f82ff6f0..6c6b6df3b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -10,13 +10,15 @@ Features: * A new :ref:`searchlimit` configuration option allows you to specify how many search results you wish to see when looking up releases at MusicBrainz during import. :bug:`1245` +* :doc:`/plugins/lastgenre`: Add *comedy*, *humor*, and *stand-up* to the + built-in whitelist/canonicalization tree. :bug:`1206` +* :doc:`/plugins/lastgenre`: Add classical music to the built-in whitelist and + canonicalization tree. :bug:`1239` :bug:`1240` Fixes: * :doc:`/plugins/lyrics`: Silence a warning about insecure requests in the new MusixMatch backend. :bug:`1204` -* :doc:`/plugins/lastgenre`: Add *comedy*, *humor*, and *stand-up* to the - built-in whitelist/canonicalization tree. :bug:`1206` * Fix a crash when ``beet`` is invoked without arguments. :bug:`1205` :bug:`1207` * :doc:`/plugins/fetchart`: Do not attempt to import directories as album art. @@ -30,8 +32,6 @@ Fixes: * Remove the ``beatport`` plugin. `Beatport`_ has shut off public access to their API and denied our request for an account. We have not heard from the company since 2013, so we are assuming access will not be restored. -* :doc:`/plugins/lastgenre`: Add classical music to the built-in whitelist and - canonicalization tree. :bug:`1239` :bug:`1240` * Incremental imports now (once again) show a "skipped N directories" message. * :doc:`/plugins/embedart`: Handle errors in ImageMagick's output. :bug:`1241` From 4d904e20cf0c5fec05ca0af41900d2f0126ff427 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Frederik=20=E2=80=9CFreso=E2=80=9D=20S=2E=20Olesen?= Date: Mon, 19 Jan 2015 18:44:31 +0100 Subject: [PATCH 22/56] changelog: Combine the two lastgenre genre addition entries. See https://botbot.me/freenode/beets/msg/29935738/ --- docs/changelog.rst | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 6c6b6df3b..2827b1404 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -10,10 +10,9 @@ Features: * A new :ref:`searchlimit` configuration option allows you to specify how many search results you wish to see when looking up releases at MusicBrainz during import. :bug:`1245` -* :doc:`/plugins/lastgenre`: Add *comedy*, *humor*, and *stand-up* to the - built-in whitelist/canonicalization tree. :bug:`1206` -* :doc:`/plugins/lastgenre`: Add classical music to the built-in whitelist and - canonicalization tree. :bug:`1239` :bug:`1240` +* :doc:`/plugins/lastgenre`: Add *comedy*, *humor*, and *stand-up* as well as + a longer list of classical music genre tags to the built-in whitelist and + canonicalization tree. :bug:`1206` :bug:`1239` :bug:`1240` Fixes: From bd63e1e38662191db9d41afbf4e847c2dfa203ba Mon Sep 17 00:00:00 2001 From: Andre Miller Date: Tue, 20 Jan 2015 00:20:26 +0200 Subject: [PATCH 23/56] Made CORS configurable and changed host default to 127.0.0.1 --- beetsplug/web/__init__.py | 28 +++++++++++++++------------- beetsplug/web/crossdomaindec.py | 18 ++++++++++++++---- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/beetsplug/web/__init__.py b/beetsplug/web/__init__.py index 24bbf271e..34d33e01e 100644 --- a/beetsplug/web/__init__.py +++ b/beetsplug/web/__init__.py @@ -22,8 +22,7 @@ from flask import g from werkzeug.routing import BaseConverter, PathConverter import os import json -from crossdomaindec import crossdomain - +from crossdomaindec import crossdomain, set_cors_origin # Utilities. @@ -165,7 +164,7 @@ def before_request(): # Items. @app.route('/item/') -@crossdomain(origin='*') +@crossdomain() @resource('items') def get_item(id): return g.lib.get_item(id) @@ -173,14 +172,14 @@ def get_item(id): @app.route('/item/') @app.route('/item/query/') -@crossdomain(origin='*') +@crossdomain() @resource_list('items') def all_items(): return g.lib.items() @app.route('/item//file') -@crossdomain(origin='*') +@crossdomain() def item_file(item_id): item = g.lib.get_item(item_id) response = flask.send_file(item.path, as_attachment=True, @@ -190,7 +189,7 @@ def item_file(item_id): @app.route('/item/query/') -@crossdomain(origin='*') +@crossdomain() @resource_query('items') def item_query(queries): return g.lib.items(queries) @@ -199,7 +198,7 @@ def item_query(queries): # Albums. @app.route('/album/') -@crossdomain(origin='*') +@crossdomain() @resource('albums') def get_album(id): return g.lib.get_album(id) @@ -207,21 +206,21 @@ def get_album(id): @app.route('/album/') @app.route('/album/query/') -@crossdomain(origin='*') +@crossdomain() @resource_list('albums') def all_albums(): return g.lib.albums() @app.route('/album/query/') -@crossdomain(origin='*') +@crossdomain() @resource_query('albums') def album_query(queries): return g.lib.albums(queries) @app.route('/album//art') -@crossdomain(origin='*') +@crossdomain() def album_art(album_id): album = g.lib.get_album(album_id) return flask.send_file(album.artpath) @@ -230,7 +229,7 @@ def album_art(album_id): # Artists. @app.route('/artist/') -@crossdomain(origin='*') +@crossdomain() def all_artists(): with g.lib.transaction() as tx: rows = tx.query("SELECT DISTINCT albumartist FROM albums") @@ -241,7 +240,7 @@ def all_artists(): # Library information. @app.route('/stats') -@crossdomain(origin='*') +@crossdomain() def stats(): with g.lib.transaction() as tx: item_rows = tx.query("SELECT COUNT(*) FROM items") @@ -265,8 +264,9 @@ class WebPlugin(BeetsPlugin): def __init__(self): super(WebPlugin, self).__init__() self.config.add({ - 'host': u'', + 'host': u'127.0.0.1', 'port': 8337, + 'cors_origin': 'http://127.0.0.1', }) def commands(self): @@ -281,6 +281,8 @@ class WebPlugin(BeetsPlugin): if args: self.config['port'] = int(args.pop(0)) + set_cors_origin(self.config['cors_origin']) + app.config['lib'] = lib app.run(host=self.config['host'].get(unicode), port=self.config['port'].get(int), diff --git a/beetsplug/web/crossdomaindec.py b/beetsplug/web/crossdomaindec.py index 622b0455c..2fde1eae8 100644 --- a/beetsplug/web/crossdomaindec.py +++ b/beetsplug/web/crossdomaindec.py @@ -12,21 +12,31 @@ # # The following view decorator implements this # +# Note that some changes have been made to the original snippet +# to allow changing the CORS origin after the decorator has been attached +# This was done because the flask routing functions are defined before the +# beetsplug hook is called. from datetime import timedelta from flask import make_response, request, current_app from functools import update_wrapper +cors_origin = 'http://127.0.0.1' -def crossdomain(origin=None, methods=None, headers=None, +def set_cors_origin(origin): + global cors_origin + cors_origin = origin + +def get_cors_origin(): + return cors_origin + +def crossdomain(methods=None, headers=None, max_age=21600, attach_to_all=True, automatic_options=True): if methods is not None: methods = ', '.join(sorted(x.upper() for x in methods)) if headers is not None and not isinstance(headers, basestring): headers = ', '.join(x.upper() for x in headers) - if not isinstance(origin, basestring): - origin = ', '.join(origin) if isinstance(max_age, timedelta): max_age = max_age.total_seconds() @@ -48,7 +58,7 @@ def crossdomain(origin=None, methods=None, headers=None, h = resp.headers - h['Access-Control-Allow-Origin'] = origin + h['Access-Control-Allow-Origin'] = get_cors_origin() h['Access-Control-Allow-Methods'] = get_methods() h['Access-Control-Max-Age'] = str(max_age) if headers is not None: From c8880de52ce021f6cea5e9ec535c3528d599f6c7 Mon Sep 17 00:00:00 2001 From: Andre Miller Date: Tue, 20 Jan 2015 00:38:26 +0200 Subject: [PATCH 24/56] Fixes for flake8 validation --- beetsplug/web/__init__.py | 1 + beetsplug/web/crossdomaindec.py | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/beetsplug/web/__init__.py b/beetsplug/web/__init__.py index 34d33e01e..b3114dbab 100644 --- a/beetsplug/web/__init__.py +++ b/beetsplug/web/__init__.py @@ -26,6 +26,7 @@ from crossdomaindec import crossdomain, set_cors_origin # Utilities. + def _rep(obj, expand=False): """Get a flat -- i.e., JSON-ish -- representation of a beets Item or Album object. For Albums, `expand` dictates whether tracks are diff --git a/beetsplug/web/crossdomaindec.py b/beetsplug/web/crossdomaindec.py index 2fde1eae8..8411c6fe8 100644 --- a/beetsplug/web/crossdomaindec.py +++ b/beetsplug/web/crossdomaindec.py @@ -21,15 +21,18 @@ from datetime import timedelta from flask import make_response, request, current_app from functools import update_wrapper -cors_origin = 'http://127.0.0.1' +cors_origin = 'http://127.0.0.1' + def set_cors_origin(origin): global cors_origin cors_origin = origin + def get_cors_origin(): return cors_origin + def crossdomain(methods=None, headers=None, max_age=21600, attach_to_all=True, automatic_options=True): From f47be23658534e4b28d2e7cec3b368a5881c87b3 Mon Sep 17 00:00:00 2001 From: Andre Miller Date: Tue, 20 Jan 2015 15:33:33 +0200 Subject: [PATCH 25/56] CORS support now uses flask-cor extension --- beetsplug/web/__init__.py | 25 +++++------ beetsplug/web/crossdomaindec.py | 73 --------------------------------- setup.py | 2 +- 3 files changed, 12 insertions(+), 88 deletions(-) delete mode 100644 beetsplug/web/crossdomaindec.py diff --git a/beetsplug/web/__init__.py b/beetsplug/web/__init__.py index b3114dbab..9ab7619e0 100644 --- a/beetsplug/web/__init__.py +++ b/beetsplug/web/__init__.py @@ -22,7 +22,6 @@ from flask import g from werkzeug.routing import BaseConverter, PathConverter import os import json -from crossdomaindec import crossdomain, set_cors_origin # Utilities. @@ -165,7 +164,6 @@ def before_request(): # Items. @app.route('/item/') -@crossdomain() @resource('items') def get_item(id): return g.lib.get_item(id) @@ -173,14 +171,12 @@ def get_item(id): @app.route('/item/') @app.route('/item/query/') -@crossdomain() @resource_list('items') def all_items(): return g.lib.items() @app.route('/item//file') -@crossdomain() def item_file(item_id): item = g.lib.get_item(item_id) response = flask.send_file(item.path, as_attachment=True, @@ -190,7 +186,6 @@ def item_file(item_id): @app.route('/item/query/') -@crossdomain() @resource_query('items') def item_query(queries): return g.lib.items(queries) @@ -199,7 +194,6 @@ def item_query(queries): # Albums. @app.route('/album/') -@crossdomain() @resource('albums') def get_album(id): return g.lib.get_album(id) @@ -207,21 +201,18 @@ def get_album(id): @app.route('/album/') @app.route('/album/query/') -@crossdomain() @resource_list('albums') def all_albums(): return g.lib.albums() @app.route('/album/query/') -@crossdomain() @resource_query('albums') def album_query(queries): return g.lib.albums(queries) @app.route('/album//art') -@crossdomain() def album_art(album_id): album = g.lib.get_album(album_id) return flask.send_file(album.artpath) @@ -230,7 +221,6 @@ def album_art(album_id): # Artists. @app.route('/artist/') -@crossdomain() def all_artists(): with g.lib.transaction() as tx: rows = tx.query("SELECT DISTINCT albumartist FROM albums") @@ -241,7 +231,6 @@ def all_artists(): # Library information. @app.route('/stats') -@crossdomain() def stats(): with g.lib.transaction() as tx: item_rows = tx.query("SELECT COUNT(*) FROM items") @@ -267,7 +256,8 @@ class WebPlugin(BeetsPlugin): self.config.add({ 'host': u'127.0.0.1', 'port': 8337, - 'cors_origin': 'http://127.0.0.1', + 'cors': False, + 'cors_origin': '*', }) def commands(self): @@ -282,9 +272,16 @@ class WebPlugin(BeetsPlugin): if args: self.config['port'] = int(args.pop(0)) - set_cors_origin(self.config['cors_origin']) - app.config['lib'] = lib + + ## Enable CORS if required + if self.config['cors']: + from flask.ext.cors import CORS + app.config['CORS_ALLOW_HEADERS'] = "Content-Type" + app.config['CORS_RESOURCES'] = { + r"/*": {"origins": self.config['cors_origin'].get(str)} + } + cors = CORS(app) app.run(host=self.config['host'].get(unicode), port=self.config['port'].get(int), debug=opts.debug, threaded=True) diff --git a/beetsplug/web/crossdomaindec.py b/beetsplug/web/crossdomaindec.py deleted file mode 100644 index 8411c6fe8..000000000 --- a/beetsplug/web/crossdomaindec.py +++ /dev/null @@ -1,73 +0,0 @@ -# Decorator for the HTTP Access Control -# By Armin Ronacher -# http://flask.pocoo.org/snippets/56/ -# -# Cross-site HTTP requests are HTTP requests for resources from a different -# domain than the domain of the resource making the request. -# For instance, a resource loaded from Domain A makes a request for a resource -# on Domain B. The way this is implemented in modern browsers is by using -# HTTP Access Control headers -# -# https://developer.mozilla.org/en/HTTP_access_control -# -# The following view decorator implements this -# -# Note that some changes have been made to the original snippet -# to allow changing the CORS origin after the decorator has been attached -# This was done because the flask routing functions are defined before the -# beetsplug hook is called. - -from datetime import timedelta -from flask import make_response, request, current_app -from functools import update_wrapper - -cors_origin = 'http://127.0.0.1' - - -def set_cors_origin(origin): - global cors_origin - cors_origin = origin - - -def get_cors_origin(): - return cors_origin - - -def crossdomain(methods=None, headers=None, - max_age=21600, attach_to_all=True, - automatic_options=True): - if methods is not None: - methods = ', '.join(sorted(x.upper() for x in methods)) - if headers is not None and not isinstance(headers, basestring): - headers = ', '.join(x.upper() for x in headers) - if isinstance(max_age, timedelta): - max_age = max_age.total_seconds() - - def get_methods(): - if methods is not None: - return methods - - options_resp = current_app.make_default_options_response() - return options_resp.headers['allow'] - - def decorator(f): - def wrapped_function(*args, **kwargs): - if automatic_options and request.method == 'OPTIONS': - resp = current_app.make_default_options_response() - else: - resp = make_response(f(*args, **kwargs)) - if not attach_to_all and request.method != 'OPTIONS': - return resp - - h = resp.headers - - h['Access-Control-Allow-Origin'] = get_cors_origin() - h['Access-Control-Allow-Methods'] = get_methods() - h['Access-Control-Max-Age'] = str(max_age) - if headers is not None: - h['Access-Control-Allow-Headers'] = headers - return resp - - f.provide_automatic_options = False - return update_wrapper(wrapped_function, f) - return decorator diff --git a/setup.py b/setup.py index 36998d39a..a4d35c6bf 100755 --- a/setup.py +++ b/setup.py @@ -102,7 +102,7 @@ setup( 'echonest': ['pyechonest'], 'lastgenre': ['pylast'], 'mpdstats': ['python-mpd'], - 'web': ['flask'], + 'web': ['flask', 'flask-cors'], 'import': ['rarfile'], }, # Non-Python/non-PyPI plugin dependencies: From 47ea4b7d8b835bb2b61d316e9e8925290f66b6b0 Mon Sep 17 00:00:00 2001 From: Andre Miller Date: Tue, 20 Jan 2015 16:06:16 +0200 Subject: [PATCH 26/56] flake8 formatting updates --- beetsplug/web/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/web/__init__.py b/beetsplug/web/__init__.py index 9ab7619e0..5a78b5042 100644 --- a/beetsplug/web/__init__.py +++ b/beetsplug/web/__init__.py @@ -274,14 +274,14 @@ class WebPlugin(BeetsPlugin): app.config['lib'] = lib - ## Enable CORS if required + # Enable CORS if required if self.config['cors']: from flask.ext.cors import CORS app.config['CORS_ALLOW_HEADERS'] = "Content-Type" app.config['CORS_RESOURCES'] = { r"/*": {"origins": self.config['cors_origin'].get(str)} } - cors = CORS(app) + CORS(app) app.run(host=self.config['host'].get(unicode), port=self.config['port'].get(int), debug=opts.debug, threaded=True) From d6a4046245420dc4725a513ebb221de4880bce76 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Tue, 20 Jan 2015 17:34:01 +0100 Subject: [PATCH 27/56] Avoid stacking logger prefixes --- beets/plugins.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/beets/plugins.py b/beets/plugins.py index 2f381c93a..7ca80da0d 100755 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -82,7 +82,9 @@ class BeetsPlugin(object): self._log = log.getChild(self.name) self._log.setLevel(logging.NOTSET) # Use `beets` logger level. if beets.config['verbose']: - self._log.addFilter(PluginLogFilter(self)) + if not any(isinstance(f, PluginLogFilter) + for f in self._log.filters): + self._log.addFilter(PluginLogFilter(self)) def commands(self): """Should return a list of beets.ui.Subcommand objects for From f8151387587dedc2d8bd70b0e5f1fd32ad00a03e Mon Sep 17 00:00:00 2001 From: Andre Miller Date: Tue, 20 Jan 2015 19:53:04 +0200 Subject: [PATCH 28/56] Combined cors and cors_origin config options into one --- beetsplug/web/__init__.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/beetsplug/web/__init__.py b/beetsplug/web/__init__.py index 5a78b5042..cc1666d38 100644 --- a/beetsplug/web/__init__.py +++ b/beetsplug/web/__init__.py @@ -23,8 +23,8 @@ from werkzeug.routing import BaseConverter, PathConverter import os import json -# Utilities. +# Utilities. def _rep(obj, expand=False): """Get a flat -- i.e., JSON-ish -- representation of a beets Item or @@ -256,8 +256,7 @@ class WebPlugin(BeetsPlugin): self.config.add({ 'host': u'127.0.0.1', 'port': 8337, - 'cors': False, - 'cors_origin': '*', + 'cors': '', }) def commands(self): @@ -273,15 +272,16 @@ class WebPlugin(BeetsPlugin): self.config['port'] = int(args.pop(0)) app.config['lib'] = lib - - # Enable CORS if required + # Enable CORS if required. if self.config['cors']: + print "Enabling cors" from flask.ext.cors import CORS app.config['CORS_ALLOW_HEADERS'] = "Content-Type" app.config['CORS_RESOURCES'] = { - r"/*": {"origins": self.config['cors_origin'].get(str)} + r"/*": {"origins": self.config['cors'].get(str)} } CORS(app) + # Start the web application. app.run(host=self.config['host'].get(unicode), port=self.config['port'].get(int), debug=opts.debug, threaded=True) From 293e2512edccdf5788e3b0c89e6e3745410ab7b3 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 20 Jan 2015 09:55:56 -0800 Subject: [PATCH 29/56] Imported count: sensitive to plugin skips Regression from b7125e434315b00584cb6d6abf8baf5d26fb3bf0 pointed out by @mried. --- beets/importer.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index f054d07fe..a00021eee 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -1004,16 +1004,14 @@ class ImportTaskFactory(object): for dirs, paths in self.paths(): if self.session.config['singletons']: for path in paths: - task = self.singleton(path) + task = self._create(self.singleton(path)) if task: - self.imported += 1 yield task yield self.sentinel(dirs) else: - task = self.album(paths, dirs) + task = self._create(self.album(paths, dirs)) if task: - self.imported += 1 yield task # Produce the final sentinel for this toppath to indicate that @@ -1025,6 +1023,19 @@ class ImportTaskFactory(object): else: yield self.sentinel() + def _create(self, task): + """Handle a new task to be emitted by the factory. + + Emit the `import_task_created` event and increment the + `imported` count if the task is not skipped. Return the same + task. If `task` is None, do nothing. + """ + if task: + task.emit_created(self.session) + if not task.skip: + self.imported += 1 + return task + def paths(self): """Walk `self.toppath` and yield `(dirs, files)` pairs where `files` are individual music files and `dirs` the set of @@ -1155,7 +1166,6 @@ def read_tasks(session): # Generate tasks. task_factory = ImportTaskFactory(toppath, session) for t in task_factory.tasks(): - t.emit_created(session) yield t skipped += task_factory.skipped From a62a1520109f302758c90916612035aefa02ac41 Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Tue, 20 Jan 2015 19:50:00 +0100 Subject: [PATCH 30/56] Moved the regular expression file filter into a separate plugin. --- beetsplug/ihate.py | 45 +------ beetsplug/regexfilefilter.py | 68 +++++++++++ docs/plugins/ihate.rst | 15 +-- docs/plugins/index.rst | 2 + docs/plugins/regexfilefilter.rst | 32 +++++ test/test_ihate.py | 186 +---------------------------- test/test_regexfilefilter.py | 197 +++++++++++++++++++++++++++++++ 7 files changed, 303 insertions(+), 242 deletions(-) create mode 100644 beetsplug/regexfilefilter.py create mode 100644 docs/plugins/regexfilefilter.rst create mode 100644 test/test_regexfilefilter.py diff --git a/beetsplug/ihate.py b/beetsplug/ihate.py index 7fdd661a0..62e0b5e58 100644 --- a/beetsplug/ihate.py +++ b/beetsplug/ihate.py @@ -14,10 +14,8 @@ """Warns you about things you hate (or even blocks import).""" -import re -from beets import config from beets.plugins import BeetsPlugin -from beets.importer import action, SingletonImportTask +from beets.importer import action from beets.library import parse_query_string from beets.library import Item from beets.library import Album @@ -42,25 +40,11 @@ class IHatePlugin(BeetsPlugin): super(IHatePlugin, self).__init__() self.register_listener('import_task_choice', self.import_task_choice_event) - self.register_listener('import_task_created', - self.import_task_created_event) self.config.add({ 'warn': [], 'skip': [], - 'path': '.*' }) - self.path_album_regex = \ - self.path_singleton_regex = \ - re.compile(self.config['path'].get()) - - if 'album_path' in self.config: - self.path_album_regex = re.compile(self.config['album_path'].get()) - - if 'singleton_path' in self.config: - self.path_singleton_regex = re.compile( - self.config['singleton_path'].get()) - @classmethod def do_i_hate_this(cls, task, action_patterns): """Process group of patterns (warn or skip) and returns True if @@ -93,30 +77,3 @@ class IHatePlugin(BeetsPlugin): self._log.debug(u'nothing to do') else: self._log.debug(u'user made a decision, nothing to do') - - def import_task_created_event(self, session, task): - if task.items and len(task.items) > 0: - items_to_import = [] - for item in task.items: - if self.file_filter(item['path']): - items_to_import.append(item) - if len(items_to_import) > 0: - task.items = items_to_import - else: - task.choice_flag = action.SKIP - elif isinstance(task, SingletonImportTask): - if not self.file_filter(task.item['path']): - task.choice_flag = action.SKIP - - def file_filter(self, full_path): - """Checks if the configured regular expressions allow the import of the - file given in full_path. - """ - import_config = dict(config['import']) - if 'singletons' not in import_config or not import_config[ - 'singletons']: - # Album - return self.path_album_regex.match(full_path) is not None - else: - # Singleton - return self.path_singleton_regex.match(full_path) is not None diff --git a/beetsplug/regexfilefilter.py b/beetsplug/regexfilefilter.py new file mode 100644 index 000000000..56339fdee --- /dev/null +++ b/beetsplug/regexfilefilter.py @@ -0,0 +1,68 @@ +# This file is part of beets. +# Copyright 2015, Malte Ried. +# +# Permission is hereby granted, free of charge, to any person obtaining +# a copy of this software and associated documentation files (the +# "Software"), to deal in the Software without restriction, including +# without limitation the rights to use, copy, modify, merge, publish, +# distribute, sublicense, and/or sell copies of the Software, and to +# permit persons to whom the Software is furnished to do so, subject to +# the following conditions: +# +# The above copyright notice and this permission notice shall be +# included in all copies or substantial portions of the Software. + +"""Filters the imported files using a regular expression""" + +import re +from beets import config +from beets.plugins import BeetsPlugin +from beets.importer import action, SingletonImportTask + + +class RegexFileFilterPlugin(BeetsPlugin): + def __init__(self): + super(RegexFileFilterPlugin, self).__init__() + self.register_listener('import_task_created', + self.import_task_created_event) + self.config.add({ + 'path': '.*' + }) + + self.path_album_regex = \ + self.path_singleton_regex = \ + re.compile(self.config['path'].get()) + + if 'album_path' in self.config: + self.path_album_regex = re.compile(self.config['album_path'].get()) + + if 'singleton_path' in self.config: + self.path_singleton_regex = re.compile( + self.config['singleton_path'].get()) + + def import_task_created_event(self, session, task): + if task.items and len(task.items) > 0: + items_to_import = [] + for item in task.items: + if self.file_filter(item['path']): + items_to_import.append(item) + if len(items_to_import) > 0: + task.items = items_to_import + else: + task.choice_flag = action.SKIP + elif isinstance(task, SingletonImportTask): + if not self.file_filter(task.item['path']): + task.choice_flag = action.SKIP + + def file_filter(self, full_path): + """Checks if the configured regular expressions allow the import of the + file given in full_path. + """ + import_config = dict(config['import']) + if 'singletons' not in import_config or not import_config[ + 'singletons']: + # Album + return self.path_album_regex.match(full_path) is not None + else: + # Singleton + return self.path_singleton_regex.match(full_path) is not None diff --git a/docs/plugins/ihate.rst b/docs/plugins/ihate.rst index 14d1219ec..f2224bf5a 100644 --- a/docs/plugins/ihate.rst +++ b/docs/plugins/ihate.rst @@ -4,8 +4,7 @@ IHate Plugin The ``ihate`` plugin allows you to automatically skip things you hate during import or warn you about them. You specify queries (see :doc:`/reference/query`) and the plugin skips (or warns about) albums or items -that match any query. You can also specify regular expressions to filter files -to import regarding of their path and name. +that match any query. To use the ``ihate`` plugin, enable it in your configuration (see :ref:`using-plugins`). @@ -20,13 +19,6 @@ file. The available options are: Default: ``[]`` (empty list). - **warn**: Print a warning message for matches in this list of queries. Default: ``[]``. -- **path**: A regular expression to filter files based on its path and name. - Default: ``.*`` (everything) -- **album_path** and **singleton_path**: You may specify different regular - expressions used for imports of albums and singletons. This way, you can - automatically skip singletons when importing albums if the names (and paths) - of the files are distinguishable via a regex. The path regex defined here - take precedence over the global ``path`` option. Here's an example:: @@ -41,10 +33,5 @@ Here's an example:: - genre:polka - artist:manowar - album:christmas - path: .*\d\d[^/]+$ - # will only import files which names start with two digits - album_path: .*\d\d[^/]+$ - singleton_path: .*/(?!\d\d)[^/]+$ The plugin trusts your decision in "as-is" imports. - diff --git a/docs/plugins/index.rst b/docs/plugins/index.rst index a84222fa0..f8ecb8975 100644 --- a/docs/plugins/index.rst +++ b/docs/plugins/index.rst @@ -63,6 +63,7 @@ Each plugin has its own set of options that can be defined in a section bearing play plexupdate random + regexfilefilter replaygain rewrite scrub @@ -151,6 +152,7 @@ Miscellaneous * :doc:`mbcollection`: Maintain your MusicBrainz collection list. * :doc:`missing`: List missing tracks. * :doc:`random`: Randomly choose albums and tracks from your library. +* :doc:`regexfilefilter`: Automatically skip files during the import process based on regular expressions. * :doc:`spotify`: Create Spotify playlists from the Beets library. * :doc:`types`: Declare types for flexible attributes. * :doc:`web`: An experimental Web-based GUI for beets. diff --git a/docs/plugins/regexfilefilter.rst b/docs/plugins/regexfilefilter.rst new file mode 100644 index 000000000..dcec1e052 --- /dev/null +++ b/docs/plugins/regexfilefilter.rst @@ -0,0 +1,32 @@ +RegexFileFilter Plugin +====================== + +The ``regexfilefilter`` plugin allows you to skip files during import using +regular expressions. + +To use the ``regexfilefilter`` plugin, enable it in your configuration (see +:ref:`using-plugins`). + +Configuration +------------- + +To configure the plugin, make an ``regexfilefilter:`` section in your +configuration file. The available options are: + +- **path**: A regular expression to filter files based on its path and name. + Default: ``.*`` (everything) +- **album_path** and **singleton_path**: You may specify different regular + expressions used for imports of albums and singletons. This way, you can + automatically skip singletons when importing albums if the names (and paths) + of the files are distinguishable via a regex. The path regex defined here + take precedence over the global ``path`` option. + +Here's an example:: + + regexfilefilter: + path: .*\d\d[^/]+$ + # will only import files which names start with two digits + album_path: .*\d\d[^/]+$ + singleton_path: .*/(?!\d\d)[^/]+$ + + diff --git a/test/test_ihate.py b/test/test_ihate.py index 04ebf4132..030f5649e 100644 --- a/test/test_ihate.py +++ b/test/test_ihate.py @@ -1,103 +1,12 @@ """Tests for the 'ihate' plugin""" -import os -import shutil from _common import unittest -from beets import importer, config +from beets import importer from beets.library import Item -from beets.mediafile import MediaFile -from beets.util import displayable_path from beetsplug.ihate import IHatePlugin -from test import _common -from test.helper import capture_log -from test.test_importer import ImportHelper -class IHatePluginTest(unittest.TestCase, ImportHelper): - def setUp(self): - self.setup_beets() - self.__create_import_dir(2) - self._setup_import_session() - config['import']['pretend'] = True - - def tearDown(self): - self.teardown_beets() - - def __copy_file(self, dest_path, metadata): - # Copy files - resource_path = os.path.join(_common.RSRC, 'full.mp3') - shutil.copy(resource_path, dest_path) - medium = MediaFile(dest_path) - # Set metadata - for attr in metadata: - setattr(medium, attr, metadata[attr]) - medium.save() - - def __create_import_dir(self, count): - self.import_dir = os.path.join(self.temp_dir, 'testsrcdir') - if os.path.isdir(self.import_dir): - shutil.rmtree(self.import_dir) - - self.artist_path = os.path.join(self.import_dir, 'artist') - self.album_path = os.path.join(self.artist_path, 'album') - self.misc_path = os.path.join(self.import_dir, 'misc') - os.makedirs(self.album_path) - os.makedirs(self.misc_path) - - metadata = { - 'artist': 'Tag Artist', - 'album': 'Tag Album', - 'albumartist': None, - 'mb_trackid': None, - 'mb_albumid': None, - 'comp': None - } - self.album_paths = [] - for i in range(count): - metadata['track'] = i + 1 - metadata['title'] = 'Tag Title Album %d' % (i + 1) - dest_path = os.path.join(self.album_path, - '%02d - track.mp3' % (i + 1)) - self.__copy_file(dest_path, metadata) - self.album_paths.append(dest_path) - - self.artist_paths = [] - metadata['album'] = None - for i in range(count): - metadata['track'] = i + 10 - metadata['title'] = 'Tag Title Artist %d' % (i + 1) - dest_path = os.path.join(self.artist_path, - 'track_%d.mp3' % (i + 1)) - self.__copy_file(dest_path, metadata) - self.artist_paths.append(dest_path) - - self.misc_paths = [] - for i in range(count): - metadata['artist'] = 'Artist %d' % (i + 42) - metadata['track'] = i + 5 - metadata['title'] = 'Tag Title Misc %d' % (i + 1) - dest_path = os.path.join(self.misc_path, 'track_%d.mp3' % (i + 1)) - self.__copy_file(dest_path, metadata) - self.misc_paths.append(dest_path) - - def __run(self, expected_lines, singletons=False): - self.load_plugins('ihate') - - import_files = [self.import_dir] - self._setup_import_session(singletons=singletons) - self.importer.paths = import_files - - with capture_log() as logs: - self.importer.run() - self.unload_plugins() - IHatePlugin.listeners = None - - logs = [line for line in logs if not line.startswith('Sending event:')] - - self.assertEqual(logs, expected_lines) - - def __reset_config(self): - config['ihate'] = {} +class IHatePluginTest(unittest.TestCase): def test_hate(self): @@ -133,97 +42,6 @@ class IHatePluginTest(unittest.TestCase, ImportHelper): "artist:testartist album:notthis"] self.assertTrue(IHatePlugin.do_i_hate_this(task, match_pattern)) - def test_import_default(self): - """ The default configuration should import everything. - """ - self.__reset_config() - self.__run([ - 'Album %s' % displayable_path(self.artist_path), - ' %s' % displayable_path(self.artist_paths[0]), - ' %s' % displayable_path(self.artist_paths[1]), - 'Album %s' % displayable_path(self.album_path), - ' %s' % displayable_path(self.album_paths[0]), - ' %s' % displayable_path(self.album_paths[1]), - 'Album %s' % displayable_path(self.misc_path), - ' %s' % displayable_path(self.misc_paths[0]), - ' %s' % displayable_path(self.misc_paths[1]) - ]) - - def test_import_nothing(self): - self.__reset_config() - config['ihate']['path'] = 'not_there' - self.__run(['No files imported from %s' % self.import_dir]) - - # Global options - def test_import_global(self): - self.__reset_config() - config['ihate']['path'] = '.*track_1.*\.mp3' - self.__run([ - 'Album %s' % displayable_path(self.artist_path), - ' %s' % displayable_path(self.artist_paths[0]), - 'Album %s' % displayable_path(self.misc_path), - ' %s' % displayable_path(self.misc_paths[0]), - ]) - self.__run([ - 'Singleton: %s' % displayable_path(self.artist_paths[0]), - 'Singleton: %s' % displayable_path(self.misc_paths[0]) - ], singletons=True) - - # Album options - def test_import_album(self): - self.__reset_config() - config['ihate']['album_path'] = '.*track_1.*\.mp3' - self.__run([ - 'Album %s' % displayable_path(self.artist_path), - ' %s' % displayable_path(self.artist_paths[0]), - 'Album %s' % displayable_path(self.misc_path), - ' %s' % displayable_path(self.misc_paths[0]), - ]) - self.__run([ - 'Singleton: %s' % displayable_path(self.artist_paths[0]), - 'Singleton: %s' % displayable_path(self.artist_paths[1]), - 'Singleton: %s' % displayable_path(self.album_paths[0]), - 'Singleton: %s' % displayable_path(self.album_paths[1]), - 'Singleton: %s' % displayable_path(self.misc_paths[0]), - 'Singleton: %s' % displayable_path(self.misc_paths[1]) - ], singletons=True) - - # Singleton options - def test_import_singleton(self): - self.__reset_config() - config['ihate']['singleton_path'] = '.*track_1.*\.mp3' - self.__run([ - 'Singleton: %s' % displayable_path(self.artist_paths[0]), - 'Singleton: %s' % displayable_path(self.misc_paths[0]) - ], singletons=True) - self.__run([ - 'Album %s' % displayable_path(self.artist_path), - ' %s' % displayable_path(self.artist_paths[0]), - ' %s' % displayable_path(self.artist_paths[1]), - 'Album %s' % displayable_path(self.album_path), - ' %s' % displayable_path(self.album_paths[0]), - ' %s' % displayable_path(self.album_paths[1]), - 'Album %s' % displayable_path(self.misc_path), - ' %s' % displayable_path(self.misc_paths[0]), - ' %s' % displayable_path(self.misc_paths[1]) - ]) - - # Album and singleton options - def test_import_both(self): - self.__reset_config() - config['ihate']['album_path'] = '.*track_1.*\.mp3' - config['ihate']['singleton_path'] = '.*track_2.*\.mp3' - self.__run([ - 'Album %s' % displayable_path(self.artist_path), - ' %s' % displayable_path(self.artist_paths[0]), - 'Album %s' % displayable_path(self.misc_path), - ' %s' % displayable_path(self.misc_paths[0]), - ]) - self.__run([ - 'Singleton: %s' % displayable_path(self.artist_paths[1]), - 'Singleton: %s' % displayable_path(self.misc_paths[1]) - ], singletons=True) - def suite(): return unittest.TestLoader().loadTestsFromName(__name__) diff --git a/test/test_regexfilefilter.py b/test/test_regexfilefilter.py new file mode 100644 index 000000000..18b077703 --- /dev/null +++ b/test/test_regexfilefilter.py @@ -0,0 +1,197 @@ +"""Tests for the 'regexfilefilter' plugin""" +import os +import shutil + +from _common import unittest +from beets import config +from beets.mediafile import MediaFile +from beets.util import displayable_path +from beetsplug.regexfilefilter import RegexFileFilterPlugin +from test import _common +from test.helper import capture_log +from test.test_importer import ImportHelper + + +class RegexFileFilterPluginTest(unittest.TestCase, ImportHelper): + def setUp(self): + self.setup_beets() + self.__create_import_dir(2) + self._setup_import_session() + config['import']['pretend'] = True + + def tearDown(self): + self.teardown_beets() + + def __copy_file(self, dest_path, metadata): + # Copy files + resource_path = os.path.join(_common.RSRC, 'full.mp3') + shutil.copy(resource_path, dest_path) + medium = MediaFile(dest_path) + # Set metadata + for attr in metadata: + setattr(medium, attr, metadata[attr]) + medium.save() + + def __create_import_dir(self, count): + self.import_dir = os.path.join(self.temp_dir, 'testsrcdir') + if os.path.isdir(self.import_dir): + shutil.rmtree(self.import_dir) + + self.artist_path = os.path.join(self.import_dir, 'artist') + self.album_path = os.path.join(self.artist_path, 'album') + self.misc_path = os.path.join(self.import_dir, 'misc') + os.makedirs(self.album_path) + os.makedirs(self.misc_path) + + metadata = { + 'artist': 'Tag Artist', + 'album': 'Tag Album', + 'albumartist': None, + 'mb_trackid': None, + 'mb_albumid': None, + 'comp': None + } + self.album_paths = [] + for i in range(count): + metadata['track'] = i + 1 + metadata['title'] = 'Tag Title Album %d' % (i + 1) + dest_path = os.path.join(self.album_path, + '%02d - track.mp3' % (i + 1)) + self.__copy_file(dest_path, metadata) + self.album_paths.append(dest_path) + + self.artist_paths = [] + metadata['album'] = None + for i in range(count): + metadata['track'] = i + 10 + metadata['title'] = 'Tag Title Artist %d' % (i + 1) + dest_path = os.path.join(self.artist_path, + 'track_%d.mp3' % (i + 1)) + self.__copy_file(dest_path, metadata) + self.artist_paths.append(dest_path) + + self.misc_paths = [] + for i in range(count): + metadata['artist'] = 'Artist %d' % (i + 42) + metadata['track'] = i + 5 + metadata['title'] = 'Tag Title Misc %d' % (i + 1) + dest_path = os.path.join(self.misc_path, 'track_%d.mp3' % (i + 1)) + self.__copy_file(dest_path, metadata) + self.misc_paths.append(dest_path) + + def __run(self, expected_lines, singletons=False): + self.load_plugins('regexfilefilter') + + import_files = [self.import_dir] + self._setup_import_session(singletons=singletons) + self.importer.paths = import_files + + with capture_log() as logs: + self.importer.run() + self.unload_plugins() + RegexFileFilterPlugin.listeners = None + + logs = [line for line in logs if not line.startswith('Sending event:')] + + self.assertEqual(logs, expected_lines) + + def __reset_config(self): + config['regexfilefilter'] = {} + + def test_import_default(self): + """ The default configuration should import everything. + """ + self.__reset_config() + self.__run([ + 'Album %s' % displayable_path(self.artist_path), + ' %s' % displayable_path(self.artist_paths[0]), + ' %s' % displayable_path(self.artist_paths[1]), + 'Album %s' % displayable_path(self.album_path), + ' %s' % displayable_path(self.album_paths[0]), + ' %s' % displayable_path(self.album_paths[1]), + 'Album %s' % displayable_path(self.misc_path), + ' %s' % displayable_path(self.misc_paths[0]), + ' %s' % displayable_path(self.misc_paths[1]) + ]) + + def test_import_nothing(self): + self.__reset_config() + config['regexfilefilter']['path'] = 'not_there' + self.__run(['No files imported from %s' % self.import_dir]) + + # Global options + def test_import_global(self): + self.__reset_config() + config['regexfilefilter']['path'] = '.*track_1.*\.mp3' + self.__run([ + 'Album %s' % displayable_path(self.artist_path), + ' %s' % displayable_path(self.artist_paths[0]), + 'Album %s' % displayable_path(self.misc_path), + ' %s' % displayable_path(self.misc_paths[0]), + ]) + self.__run([ + 'Singleton: %s' % displayable_path(self.artist_paths[0]), + 'Singleton: %s' % displayable_path(self.misc_paths[0]) + ], singletons=True) + + # Album options + def test_import_album(self): + self.__reset_config() + config['regexfilefilter']['album_path'] = '.*track_1.*\.mp3' + self.__run([ + 'Album %s' % displayable_path(self.artist_path), + ' %s' % displayable_path(self.artist_paths[0]), + 'Album %s' % displayable_path(self.misc_path), + ' %s' % displayable_path(self.misc_paths[0]), + ]) + self.__run([ + 'Singleton: %s' % displayable_path(self.artist_paths[0]), + 'Singleton: %s' % displayable_path(self.artist_paths[1]), + 'Singleton: %s' % displayable_path(self.album_paths[0]), + 'Singleton: %s' % displayable_path(self.album_paths[1]), + 'Singleton: %s' % displayable_path(self.misc_paths[0]), + 'Singleton: %s' % displayable_path(self.misc_paths[1]) + ], singletons=True) + + # Singleton options + def test_import_singleton(self): + self.__reset_config() + config['regexfilefilter']['singleton_path'] = '.*track_1.*\.mp3' + self.__run([ + 'Singleton: %s' % displayable_path(self.artist_paths[0]), + 'Singleton: %s' % displayable_path(self.misc_paths[0]) + ], singletons=True) + self.__run([ + 'Album %s' % displayable_path(self.artist_path), + ' %s' % displayable_path(self.artist_paths[0]), + ' %s' % displayable_path(self.artist_paths[1]), + 'Album %s' % displayable_path(self.album_path), + ' %s' % displayable_path(self.album_paths[0]), + ' %s' % displayable_path(self.album_paths[1]), + 'Album %s' % displayable_path(self.misc_path), + ' %s' % displayable_path(self.misc_paths[0]), + ' %s' % displayable_path(self.misc_paths[1]) + ]) + + # Album and singleton options + def test_import_both(self): + self.__reset_config() + config['regexfilefilter']['album_path'] = '.*track_1.*\.mp3' + config['regexfilefilter']['singleton_path'] = '.*track_2.*\.mp3' + self.__run([ + 'Album %s' % displayable_path(self.artist_path), + ' %s' % displayable_path(self.artist_paths[0]), + 'Album %s' % displayable_path(self.misc_path), + ' %s' % displayable_path(self.misc_paths[0]), + ]) + self.__run([ + 'Singleton: %s' % displayable_path(self.artist_paths[1]), + 'Singleton: %s' % displayable_path(self.misc_paths[1]) + ], singletons=True) + + +def suite(): + return unittest.TestLoader().loadTestsFromName(__name__) + +if __name__ == '__main__': + unittest.main(defaultTest='suite') From 5cf869e0f8d3a4809827baaeffc300a1bd11d214 Mon Sep 17 00:00:00 2001 From: Andre Miller Date: Tue, 20 Jan 2015 20:52:24 +0200 Subject: [PATCH 31/56] Updated web documentation for CORS --- docs/plugins/web.rst | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/docs/plugins/web.rst b/docs/plugins/web.rst index 63c679e2b..66b7f110b 100644 --- a/docs/plugins/web.rst +++ b/docs/plugins/web.rst @@ -20,6 +20,12 @@ flask``. .. _Flask: http://flask.pocoo.org/ +If you require `CORS`_ (Cross-origin resource sharing), then you also +need `flask-cors`_. This can be installed by running ``pip install flask-cors``. + +.. _flask-cors: https://github.com/CoryDolphin/flask-cors +.. _CORS: http://en.wikipedia.org/wiki/Cross-origin_resource_sharing + Finally, enable the ``web`` plugin in your configuration (see :ref:`using-plugins`). @@ -52,10 +58,12 @@ Configuration To configure the plugin, make a ``web:`` section in your configuration file. The available options are: -- **host**: The server hostname. - Default: Bind to all interfaces. +- **host**: The server hostname. Set this to 0.0.0.0 to bind to all interfaces. + Default: Bind to 127.0.0.1. - **port**: The server port. Default: 8337. +- **cors**: The CORS origin. See below. + Default: CORS is disabled. Implementation -------------- @@ -78,6 +86,28 @@ for unsupported formats/browsers. There are a number of options for this: .. _html5media: http://html5media.info/ .. _MediaElement.js: http://mediaelementjs.com/ +Cross-origin resource sharing (CORS) +------------------------------------ + +This is only required if you intend to access the API from a browser using JavaScript and +the JavaScript is not hosted by the beets web server. + +The browser will check if the resources the JavaScript is trying to access is coming from the +same source as the the Script and give an error similar to the following: + +``XMLHttpRequest cannot load http://beets:8337/item/xx. No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://webserver' is therefore not allowed access.`` + +To prevent this, `CORS`_ is used. To enable CORS, set the ``cors`` configuration option to the origin +of your JavaScript or set it to ``'*'`` to enable access from all origins. Note that there are +security implications if you set the origin to ``'*'``, please research this before enabling it. + +For example:: + + web: + host: 0.0.0.0 + cors: 'http://webserver' + + JSON API -------- From a82f6c2d76abd41983a1b92026a0a5be0d6d76c7 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 20 Jan 2015 14:10:33 -0800 Subject: [PATCH 32/56] Docs tweaks and changelog for #1237, fix #1236 --- docs/changelog.rst | 3 +++ docs/plugins/web.rst | 28 +++++++++++++++------------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 2827b1404..374f48b46 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -13,6 +13,9 @@ Features: * :doc:`/plugins/lastgenre`: Add *comedy*, *humor*, and *stand-up* as well as a longer list of classical music genre tags to the built-in whitelist and canonicalization tree. :bug:`1206` :bug:`1239` :bug:`1240` +* :doc:`/plugins/web`: Add support for *cross-origin resource sharing* for + more flexible in-browser clients. Thanks to Andre Miller. :bug:`1236` + :bug:`1237` Fixes: diff --git a/docs/plugins/web.rst b/docs/plugins/web.rst index 66b7f110b..70ed37274 100644 --- a/docs/plugins/web.rst +++ b/docs/plugins/web.rst @@ -62,7 +62,7 @@ configuration file. The available options are: Default: Bind to 127.0.0.1. - **port**: The server port. Default: 8337. -- **cors**: The CORS origin. See below. +- **cors**: The CORS allowed origin (see :ref:`web-cors`, below). Default: CORS is disabled. Implementation @@ -86,26 +86,28 @@ for unsupported formats/browsers. There are a number of options for this: .. _html5media: http://html5media.info/ .. _MediaElement.js: http://mediaelementjs.com/ -Cross-origin resource sharing (CORS) +.. _web-cors: + +Cross-Origin Resource Sharing (CORS) ------------------------------------ -This is only required if you intend to access the API from a browser using JavaScript and -the JavaScript is not hosted by the beets web server. +The ``web`` plugin's API can be used as a backend for an in-browser client. By +default, browsers will only allow access from clients running on the same +server as the API. (You will get an arcane error about ``XMLHttpRequest`` +otherwise.) A technology called `CORS`_ lets you relax this restriction. -The browser will check if the resources the JavaScript is trying to access is coming from the -same source as the the Script and give an error similar to the following: - -``XMLHttpRequest cannot load http://beets:8337/item/xx. No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://webserver' is therefore not allowed access.`` - -To prevent this, `CORS`_ is used. To enable CORS, set the ``cors`` configuration option to the origin -of your JavaScript or set it to ``'*'`` to enable access from all origins. Note that there are -security implications if you set the origin to ``'*'``, please research this before enabling it. +If you want to use an in-browser client hosted elsewhere (or running from +a different server on your machine), set the ``cors`` configuration option to +the "origin" (protocol, host, and optional port number) where the client is +served. Or set it to ``'*'`` to enable access from all origins. Note that +there are security implications if you set the origin to ``'*'``, so please +research this before using it. For example:: web: host: 0.0.0.0 - cors: 'http://webserver' + cors: 'http://example.com' JSON API From ec21fb8af19d6315c44173534d7166a3924f4f49 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 20 Jan 2015 14:18:06 -0800 Subject: [PATCH 33/56] Revert #1186 changes to beets core The changes were: - Return values from events. - A new `import_task_created` event. Both were added preemptively to master. --- beets/importer.py | 334 +++++++++++++++++++++++-------------------- beets/plugins.py | 23 +-- docs/dev/plugins.rst | 6 - 3 files changed, 191 insertions(+), 172 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 1c5e54e2f..a00021eee 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -283,25 +283,28 @@ class ImportSession(object): else: stages = [query_tasks(self)] + # In pretend mode, just log what would otherwise be imported. if self.config['pretend']: - # Only log the imported files and end the pipeline stages += [log_files(self)] else: if self.config['group_albums'] and \ not self.config['singletons']: - # Split directory tasks into one task for each album + # Split directory tasks into one task for each album. stages += [group_albums(self)] + if self.config['autotag']: - # FIXME We should also resolve duplicates when not - # autotagging. This is currently handled in `user_query` stages += [lookup_candidates(self), user_query(self)] else: stages += [import_asis(self)] + stages += [apply_choices(self)] + # Plugin stages. for stage_func in plugins.import_stages(): stages.append(plugin_stage(self, stage_func)) + stages += [manipulate_files(self)] + pl = pipeline.Pipeline(stages) # Run the pipeline. @@ -514,8 +517,6 @@ class ImportTask(object): def cleanup(self, copy=False, delete=False, move=False): """Remove and prune imported paths. """ - # FIXME Maybe the keywords should be task properties. - # Do not delete any files or prune directories when skipping. if self.skip: return @@ -543,6 +544,11 @@ class ImportTask(object): return plugins.send('album_imported', lib=lib, album=self.album) + def emit_created(self, session): + """Send the `import_task_created` event for this task. + """ + plugins.send('import_task_created', session=session, task=self) + def lookup_candidates(self): """Retrieve and store candidates for this album. """ @@ -837,11 +843,11 @@ class SingletonImportTask(ImportTask): # are so many methods which pass. We should introduce a new # BaseImportTask class. class SentinelImportTask(ImportTask): - """This class marks the progress of an import and does not import - any items itself. + """A sentinel task marks the progress of an import and does not + import any items itself. - If only `toppath` is set the task indicats the end of a top-level - directory import. If the `paths` argument is givent, too, the task + If only `toppath` is set the task indicates the end of a top-level + directory import. If the `paths` argument is also given, the task indicates the progress in the `toppath` import. """ @@ -879,9 +885,16 @@ class SentinelImportTask(ImportTask): class ArchiveImportTask(SentinelImportTask): - """Additional methods for handling archives. + """An import task that represents the processing of an archive. - Use when `toppath` points to a `zip`, `tar`, or `rar` archive. + `toppath` must be a `zip`, `tar`, or `rar` archive. Archive tasks + serve two purposes: + - First, it will unarchive the files to a temporary directory and + return it. The client should read tasks from the resulting + directory and send them through the pipeline. + - Second, it will clean up the temporary directory when it proceeds + through the pipeline. The client should send the archive task + after sending the rest of the music tasks to make this work. """ def __init__(self, toppath): @@ -929,6 +942,8 @@ class ArchiveImportTask(SentinelImportTask): """Removes the temporary directory the archive was extracted to. """ if self.extracted: + log.debug(u'Removing extracted directory: {0}', + displayable_path(self.toppath)) shutil.rmtree(self.toppath) def extract(self): @@ -950,136 +965,179 @@ class ArchiveImportTask(SentinelImportTask): class ImportTaskFactory(object): - """Create album and singleton import tasks for all media files in a - directory or path. - - Depending on the session's 'flat' and 'singleton' configuration, it - groups all media files contained in `toppath` into singleton or - album import tasks. + """Generate album and singleton import tasks for all media files + indicated by a path. """ def __init__(self, toppath, session): + """Create a new task factory. + + `toppath` is the user-specified path to search for music to + import. `session` is the `ImportSession`, which controls how + tasks are read from the directory. + """ self.toppath = toppath self.session = session - self.skipped = 0 + self.skipped = 0 # Skipped due to incremental/resume. + self.imported = 0 # "Real" tasks created. + self.is_archive = ArchiveImportTask.is_archive(syspath(toppath)) def tasks(self): - """Yield all import tasks for `self.toppath`. + """Yield all import tasks for music found in the user-specified + path `self.toppath`. Any necessary sentinel tasks are also + produced. - The behavior is configured by the session's 'flat', and - 'singleton' flags. + During generation, update `self.skipped` and `self.imported` + with the number of tasks that were not produced (due to + incremental mode or resumed imports) and the number of concrete + tasks actually produced, respectively. + + If `self.toppath` is an archive, it is adjusted to point to the + extracted data. """ + # Check whether this is an archive. + if self.is_archive: + archive_task = self.unarchive() + if not archive_task: + return + + # Search for music in the directory. for dirs, paths in self.paths(): if self.session.config['singletons']: for path in paths: - tasks = self.singleton(path) - for task in tasks: + task = self._create(self.singleton(path)) + if task: yield task - for task in self.sentinel(dirs): - yield task + yield self.sentinel(dirs) else: - tasks = self.album(paths, dirs) - for task in tasks: + task = self._create(self.album(paths, dirs)) + if task: yield task + # Produce the final sentinel for this toppath to indicate that + # it is finished. This is usually just a SentinelImportTask, but + # for archive imports, send the archive task instead (to remove + # the extracted directory). + if self.is_archive: + yield archive_task + else: + yield self.sentinel() + + def _create(self, task): + """Handle a new task to be emitted by the factory. + + Emit the `import_task_created` event and increment the + `imported` count if the task is not skipped. Return the same + task. If `task` is None, do nothing. + """ + if task: + task.emit_created(self.session) + if not task.skip: + self.imported += 1 + return task + def paths(self): - """Walk `self.toppath` and yield pairs of directory lists and - path lists. + """Walk `self.toppath` and yield `(dirs, files)` pairs where + `files` are individual music files and `dirs` the set of + containing directories where the music was found. + + This can either be a recursive search in the ordinary case, a + single track when `toppath` is a file, a single directory in + `flat` mode. """ if not os.path.isdir(syspath(self.toppath)): - yield ([self.toppath], [self.toppath]) + yield [self.toppath], [self.toppath] elif self.session.config['flat']: paths = [] for dirs, paths_in_dir in albums_in_dir(self.toppath): paths += paths_in_dir - yield ([self.toppath], paths) + yield [self.toppath], paths else: for dirs, paths in albums_in_dir(self.toppath): - yield (dirs, paths) + yield dirs, paths - def singleton(self, path, item=None): - if not item: - if self.session.already_imported(self.toppath, [path]): - log.debug(u'Skipping previously-imported path: {0}', - displayable_path(path)) - self.skipped += 1 - return [] - - item = self.read_item(path) + def singleton(self, path): + """Return a `SingletonImportTask` for the music file. + """ + if self.session.already_imported(self.toppath, [path]): + log.debug(u'Skipping previously-imported path: {0}', + displayable_path(path)) + self.skipped += 1 + return None + item = self.read_item(path) if item: - return self.__handle_plugins(SingletonImportTask(self.toppath, - item)) + return SingletonImportTask(self.toppath, item) else: - return [] + return None - def album(self, paths, dirs=None, items=None): - """Return `ImportTask` with all media files from paths. + def album(self, paths, dirs=None): + """Return a `ImportTask` with all media files from paths. `dirs` is a list of parent directories used to record already imported albums. """ - if not items: - if not paths: - return [] + if not paths: + return None - if dirs is None: - dirs = list(set(os.path.dirname(p) for p in paths)) + if dirs is None: + dirs = list(set(os.path.dirname(p) for p in paths)) - if self.session.already_imported(self.toppath, dirs): - log.debug(u'Skipping previously-imported path: {0}', - displayable_path(dirs)) - self.skipped += 1 - return [] + if self.session.already_imported(self.toppath, dirs): + log.debug(u'Skipping previously-imported path: {0}', + displayable_path(dirs)) + self.skipped += 1 + return None - items = map(self.read_item, paths) - items = [item for item in items if item] + items = map(self.read_item, paths) + items = [item for item in items if item] if items: - return self.__handle_plugins(ImportTask(self.toppath, dirs, items)) + return ImportTask(self.toppath, dirs, items) else: - return [] + return None def sentinel(self, paths=None): - return self.__handle_plugins(SentinelImportTask(self.toppath, paths)) - - def archive(self, path): - return self.__handle_plugins(ArchiveImportTask(path)) - - def __handle_plugins(self, task): + """Return a `SentinelImportTask` indicating the end of a + top-level directory import. """ - Sends the 'import_task_created' event to all plugins. Plugins may - return a list of tasks to use instead of the given task. If no plugin - is configured for the event or no plugin returns any value, a list - containing the original task as the only element is returned. + return SentinelImportTask(self.toppath, paths) - :param task: The which is intended to create. - :return: A flat list of tasks to create instead of the original task. - The list contains the tasks returned by all plugins. There - will by no None value present at the list. + def unarchive(self): + """Extract the archive for this `toppath`. + + Extract the archive to a new directory, adjust `toppath` to + point to the extracted directory, and return an + `ArchiveImportTask`. If extraction fails, return None. """ - tasks = plugins.send('import_task_created', session=self.session, - task=task) - if not tasks: - tasks = [task] - else: - # The plugins gave us a list of lists of task. Flatten it. - flat_tasks = [] - for inner in tasks: - if isinstance(inner, list): - flat_tasks += inner - else: - flat_tasks.append(inner) - tasks = [t for t in flat_tasks if t] + assert self.is_archive - return tasks + if not (self.session.config['move'] or + self.session.config['copy']): + log.warn(u"Archive importing requires either " + "'copy' or 'move' to be enabled.") + return + + log.debug(u'Extracting archive: {0}', + displayable_path(self.toppath)) + archive_task = ArchiveImportTask(self.toppath) + try: + archive_task.extract() + except Exception as exc: + log.error(u'extraction failed: {0}', exc) + return + + # Now read albums from the extracted directory. + self.toppath = archive_task.toppath + log.debug(u'Archive extracted to: {0}', self.toppath) + return archive_task def read_item(self, path): - """Return an item created from the path. + """Return an `Item` read from the path. - If an item could not be read it returns None and logs an error. + If an item cannot be read, return `None` instead and log an + error. """ - # TODO remove this method. Should be handled in ImportTask creation. try: return library.Item.from_path(path) except library.ReadError as exc: @@ -1102,54 +1160,22 @@ def read_tasks(session): """ skipped = 0 for toppath in session.paths: - # Determine if we want to resume import of the toppath + # Check whether we need to resume the import. session.ask_resume(toppath) - user_toppath = toppath + + # Generate tasks. task_factory = ImportTaskFactory(toppath, session) - - # Extract archives. - archive_tasks = None - if ArchiveImportTask.is_archive(syspath(toppath)): - if not (session.config['move'] or session.config['copy']): - log.warn(u"Archive importing requires either " - "'copy' or 'move' to be enabled.") - continue - - log.debug(u'extracting archive {0}', - displayable_path(toppath)) - archive_tasks = task_factory.archive(toppath) - for archive_task in archive_tasks: - try: - archive_task.extract() - except Exception as exc: - log.error(u'extraction failed: {0}', exc) - continue - - # Continue reading albums from the extracted directory. - toppath = archive_task.toppath - task_factory.toppath = toppath - - imported = False for t in task_factory.tasks(): - imported |= not t.skip yield t + skipped += task_factory.skipped - # Indicate the directory is finished. - # FIXME hack to delete extracted archives - if archive_tasks is None or len(archive_tasks) == 0: - for task in task_factory.sentinel(): - yield task - else: - for archive_task in archive_tasks: - yield archive_task - - if not imported: + if not task_factory.imported: log.warn(u'No files imported from {0}', - displayable_path(user_toppath)) + displayable_path(toppath)) - # Show skipped directories. + # Show skipped directories (due to incremental/resume). if skipped: - log.info(u'Skipped {0} directories.', skipped) + log.info(u'Skipped {0} paths.', skipped) def query_tasks(session): @@ -1157,13 +1183,12 @@ def query_tasks(session): Instead of finding files from the filesystem, a query is used to match items from the library. """ - task_factory = ImportTaskFactory(None) if session.config['singletons']: # Search for items. for item in session.lib.items(session.query): - tasks = task_factory.singleton(None, item) - for task in tasks: - yield task + task = SingletonImportTask(None, item) + task.emit_created(session) + yield task else: # Search for albums. @@ -1178,9 +1203,9 @@ def query_tasks(session): item.id = None item.album_id = None - tasks = task_factory.album(None, [album.item_dir()], items) - for task in tasks: - yield task + task = ImportTask(None, [album.item_dir()], items) + task.emit_created(session) + yield task @pipeline.mutator_stage @@ -1225,13 +1250,11 @@ def user_query(session, task): if task.choice_flag is action.TRACKS: # Set up a little pipeline for dealing with the singletons. def emitter(task): - task_factory = ImportTaskFactory(task.toppath, session) for item in task.items: - new_tasks = task_factory.singleton(None, item) - for t in new_tasks: - yield t - for t in task_factory.sentinel(task.paths): - yield t + task = SingletonImportTask(task.toppath, item) + task.emit_created(session) + yield task + yield SentinelImportTask(task.toppath, task.paths) ipl = pipeline.Pipeline([ emitter(task), @@ -1338,7 +1361,7 @@ def manipulate_files(session, task): @pipeline.stage def log_files(session, task): - """A coroutine (pipeline stage) to log each file which will be imported + """A coroutine (pipeline stage) to log each file to be imported. """ if task.skip: return @@ -1346,14 +1369,17 @@ def log_files(session, task): if isinstance(task, SingletonImportTask): log.info(u'Singleton: {0}', displayable_path(task.item['path'])) elif task.items: - log.info(u'Album {0}', displayable_path(task.paths[0])) + log.info(u'Album: {0}', displayable_path(task.paths[0])) for item in task.items: log.info(u' {0}', displayable_path(item['path'])) def group_albums(session): - """Group the items of a task by albumartist and album name and create a new - task for each album. Yield the tasks as a multi message. + """A pipeline stage that groups the items of each task into albums + using their metadata. + + Groups are identified using their artist and album fields. The + pipeline stage emits new album tasks for each discovered group. """ def group(item): return (item.albumartist or item.artist, item.album) @@ -1364,13 +1390,11 @@ def group_albums(session): if task.skip: continue tasks = [] - task_factory = ImportTaskFactory(task.toppath, session) for _, items in itertools.groupby(task.items, group): - new_tasks = task_factory.album(None, None, list(items)) - for t in new_tasks: - tasks.append(t) - for t in task_factory.sentinel(task.paths): - tasks.append(t) + task = ImportTask(items=list(items)) + task.emit_created(session) + tasks.append(task) + tasks.append(SentinelImportTask(task.toppath, task.paths)) task = pipeline.multiple(tasks) diff --git a/beets/plugins.py b/beets/plugins.py index 76804e579..7ca80da0d 100755 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -82,7 +82,9 @@ class BeetsPlugin(object): self._log = log.getChild(self.name) self._log.setLevel(logging.NOTSET) # Use `beets` logger level. if beets.config['verbose']: - self._log.addFilter(PluginLogFilter(self)) + if not any(isinstance(f, PluginLogFilter) + for f in self._log.filters): + self._log.addFilter(PluginLogFilter(self)) def commands(self): """Should return a list of beets.ui.Subcommand objects for @@ -443,24 +445,23 @@ def event_handlers(): def send(event, **arguments): - """Sends an event to all assigned event listeners. Event is the - name of the event to send, all other named arguments go to the - event handler(s). + """Send an event to all assigned event listeners. - Returns a list of return values from the handlers. + `event` is the name of the event to send, all other named arguments + are passed along to the handlers. + + Return a list of non-None values returned from the handlers. """ log.debug(u'Sending event: {0}', event) - return_values = [] + results = [] for handler in event_handlers()[event]: # Don't break legacy plugins if we want to pass more arguments argspec = inspect.getargspec(handler).args args = dict((k, v) for k, v in arguments.items() if k in argspec) result = handler(**args) - # Only append non None return values - if result: - return_values.append(result) - - return return_values + if result is not None: + results.append(result) + return results def feat_tokens(for_artist=True): diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index 56db52673..d81810bde 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -174,12 +174,6 @@ The events currently available are: * *after_write*: called with an ``Item`` object after a file's metadata is written to disk (i.e., just after the file on disk is closed). -* *import_task_created*: called after an import task has been created. - Parameters: ``task`` (an `ImportTask`) and ``session`` (an `ImportSession`). - Return: The event handler may return a single `ImportTask` object or a list - of `ImportTask` objects. The original task will be replaced by the returned - task(s). If nothing is returned, the original task will be created. - * *import_task_start*: called when before an import task begins processing. Parameters: ``task`` (an `ImportTask`) and ``session`` (an `ImportSession`). From 0a8dcadb753a63a1f11dbf4bd1868226f4b798d9 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 20 Jan 2015 14:32:22 -0800 Subject: [PATCH 34/56] Rename regexfilefilter to filefilter (#1186) --- .../{regexfilefilter.py => filefilter.py} | 4 +- .../{regexfilefilter.rst => filefilter.rst} | 4 +- docs/plugins/index.rst | 5 ++- ..._regexfilefilter.py => test_filefilter.py} | 39 +++++++++++++------ 4 files changed, 34 insertions(+), 18 deletions(-) rename beetsplug/{regexfilefilter.py => filefilter.py} (96%) rename docs/plugins/{regexfilefilter.rst => filefilter.rst} (95%) rename test/{test_regexfilefilter.py => test_filefilter.py} (84%) diff --git a/beetsplug/regexfilefilter.py b/beetsplug/filefilter.py similarity index 96% rename from beetsplug/regexfilefilter.py rename to beetsplug/filefilter.py index 56339fdee..84d694ef4 100644 --- a/beetsplug/regexfilefilter.py +++ b/beetsplug/filefilter.py @@ -20,9 +20,9 @@ from beets.plugins import BeetsPlugin from beets.importer import action, SingletonImportTask -class RegexFileFilterPlugin(BeetsPlugin): +class FileFilterPlugin(BeetsPlugin): def __init__(self): - super(RegexFileFilterPlugin, self).__init__() + super(FileFilterPlugin, self).__init__() self.register_listener('import_task_created', self.import_task_created_event) self.config.add({ diff --git a/docs/plugins/regexfilefilter.rst b/docs/plugins/filefilter.rst similarity index 95% rename from docs/plugins/regexfilefilter.rst rename to docs/plugins/filefilter.rst index dcec1e052..cd51b01e4 100644 --- a/docs/plugins/regexfilefilter.rst +++ b/docs/plugins/filefilter.rst @@ -1,5 +1,5 @@ -RegexFileFilter Plugin -====================== +FileFilter Plugin +================= The ``regexfilefilter`` plugin allows you to skip files during import using regular expressions. diff --git a/docs/plugins/index.rst b/docs/plugins/index.rst index f8ecb8975..3a79af4d7 100644 --- a/docs/plugins/index.rst +++ b/docs/plugins/index.rst @@ -63,7 +63,7 @@ Each plugin has its own set of options that can be defined in a section bearing play plexupdate random - regexfilefilter + filefilter replaygain rewrite scrub @@ -152,7 +152,8 @@ Miscellaneous * :doc:`mbcollection`: Maintain your MusicBrainz collection list. * :doc:`missing`: List missing tracks. * :doc:`random`: Randomly choose albums and tracks from your library. -* :doc:`regexfilefilter`: Automatically skip files during the import process based on regular expressions. +* :doc:`filefilter`: Automatically skip files during the import process based + on regular expressions. * :doc:`spotify`: Create Spotify playlists from the Beets library. * :doc:`types`: Declare types for flexible attributes. * :doc:`web`: An experimental Web-based GUI for beets. diff --git a/test/test_regexfilefilter.py b/test/test_filefilter.py similarity index 84% rename from test/test_regexfilefilter.py rename to test/test_filefilter.py index 18b077703..21c080c81 100644 --- a/test/test_regexfilefilter.py +++ b/test/test_filefilter.py @@ -1,4 +1,19 @@ -"""Tests for the 'regexfilefilter' plugin""" +# This file is part of beets. +# Copyright 2015, Malte Ried. +# +# Permission is hereby granted, free of charge, to any person obtaining +# a copy of this software and associated documentation files (the +# "Software"), to deal in the Software without restriction, including +# without limitation the rights to use, copy, modify, merge, publish, +# distribute, sublicense, and/or sell copies of the Software, and to +# permit persons to whom the Software is furnished to do so, subject to +# the following conditions: +# +# The above copyright notice and this permission notice shall be +# included in all copies or substantial portions of the Software. + +"""Tests for the `filefilter` plugin. +""" import os import shutil @@ -6,13 +21,13 @@ from _common import unittest from beets import config from beets.mediafile import MediaFile from beets.util import displayable_path -from beetsplug.regexfilefilter import RegexFileFilterPlugin +from beetsplug.filefilter import FileFilterPlugin from test import _common from test.helper import capture_log from test.test_importer import ImportHelper -class RegexFileFilterPluginTest(unittest.TestCase, ImportHelper): +class FileFilterPluginTest(unittest.TestCase, ImportHelper): def setUp(self): self.setup_beets() self.__create_import_dir(2) @@ -80,7 +95,7 @@ class RegexFileFilterPluginTest(unittest.TestCase, ImportHelper): self.misc_paths.append(dest_path) def __run(self, expected_lines, singletons=False): - self.load_plugins('regexfilefilter') + self.load_plugins('filefilter') import_files = [self.import_dir] self._setup_import_session(singletons=singletons) @@ -89,14 +104,14 @@ class RegexFileFilterPluginTest(unittest.TestCase, ImportHelper): with capture_log() as logs: self.importer.run() self.unload_plugins() - RegexFileFilterPlugin.listeners = None + FileFilterPlugin.listeners = None logs = [line for line in logs if not line.startswith('Sending event:')] self.assertEqual(logs, expected_lines) def __reset_config(self): - config['regexfilefilter'] = {} + config['filefilter'] = {} def test_import_default(self): """ The default configuration should import everything. @@ -116,13 +131,13 @@ class RegexFileFilterPluginTest(unittest.TestCase, ImportHelper): def test_import_nothing(self): self.__reset_config() - config['regexfilefilter']['path'] = 'not_there' + config['filefilter']['path'] = 'not_there' self.__run(['No files imported from %s' % self.import_dir]) # Global options def test_import_global(self): self.__reset_config() - config['regexfilefilter']['path'] = '.*track_1.*\.mp3' + config['filefilter']['path'] = '.*track_1.*\.mp3' self.__run([ 'Album %s' % displayable_path(self.artist_path), ' %s' % displayable_path(self.artist_paths[0]), @@ -137,7 +152,7 @@ class RegexFileFilterPluginTest(unittest.TestCase, ImportHelper): # Album options def test_import_album(self): self.__reset_config() - config['regexfilefilter']['album_path'] = '.*track_1.*\.mp3' + config['filefilter']['album_path'] = '.*track_1.*\.mp3' self.__run([ 'Album %s' % displayable_path(self.artist_path), ' %s' % displayable_path(self.artist_paths[0]), @@ -156,7 +171,7 @@ class RegexFileFilterPluginTest(unittest.TestCase, ImportHelper): # Singleton options def test_import_singleton(self): self.__reset_config() - config['regexfilefilter']['singleton_path'] = '.*track_1.*\.mp3' + config['filefilter']['singleton_path'] = '.*track_1.*\.mp3' self.__run([ 'Singleton: %s' % displayable_path(self.artist_paths[0]), 'Singleton: %s' % displayable_path(self.misc_paths[0]) @@ -176,8 +191,8 @@ class RegexFileFilterPluginTest(unittest.TestCase, ImportHelper): # Album and singleton options def test_import_both(self): self.__reset_config() - config['regexfilefilter']['album_path'] = '.*track_1.*\.mp3' - config['regexfilefilter']['singleton_path'] = '.*track_2.*\.mp3' + config['filefilter']['album_path'] = '.*track_1.*\.mp3' + config['filefilter']['singleton_path'] = '.*track_2.*\.mp3' self.__run([ 'Album %s' % displayable_path(self.artist_path), ' %s' % displayable_path(self.artist_paths[0]), From 8e94419c2af8cb32b0125db79f3b78dbfb2862da Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 20 Jan 2015 14:34:42 -0800 Subject: [PATCH 35/56] Fix filefilter (#1186) tests for consistent colons I added this to the pretend output a few commits ago. --- beetsplug/filefilter.py | 7 ++++--- test/test_filefilter.py | 26 +++++++++++++------------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/beetsplug/filefilter.py b/beetsplug/filefilter.py index 84d694ef4..1a19d46cc 100644 --- a/beetsplug/filefilter.py +++ b/beetsplug/filefilter.py @@ -12,7 +12,8 @@ # The above copyright notice and this permission notice shall be # included in all copies or substantial portions of the Software. -"""Filters the imported files using a regular expression""" +"""Filter imported files using a regular expression. +""" import re from beets import config @@ -55,8 +56,8 @@ class FileFilterPlugin(BeetsPlugin): task.choice_flag = action.SKIP def file_filter(self, full_path): - """Checks if the configured regular expressions allow the import of the - file given in full_path. + """Checks if the configured regular expressions allow the import + of the file given in full_path. """ import_config = dict(config['import']) if 'singletons' not in import_config or not import_config[ diff --git a/test/test_filefilter.py b/test/test_filefilter.py index 21c080c81..0bc3826ca 100644 --- a/test/test_filefilter.py +++ b/test/test_filefilter.py @@ -64,7 +64,7 @@ class FileFilterPluginTest(unittest.TestCase, ImportHelper): 'albumartist': None, 'mb_trackid': None, 'mb_albumid': None, - 'comp': None + 'comp': None, } self.album_paths = [] for i in range(count): @@ -118,13 +118,13 @@ class FileFilterPluginTest(unittest.TestCase, ImportHelper): """ self.__reset_config() self.__run([ - 'Album %s' % displayable_path(self.artist_path), + 'Album: %s' % displayable_path(self.artist_path), ' %s' % displayable_path(self.artist_paths[0]), ' %s' % displayable_path(self.artist_paths[1]), - 'Album %s' % displayable_path(self.album_path), + 'Album: %s' % displayable_path(self.album_path), ' %s' % displayable_path(self.album_paths[0]), ' %s' % displayable_path(self.album_paths[1]), - 'Album %s' % displayable_path(self.misc_path), + 'Album: %s' % displayable_path(self.misc_path), ' %s' % displayable_path(self.misc_paths[0]), ' %s' % displayable_path(self.misc_paths[1]) ]) @@ -139,9 +139,9 @@ class FileFilterPluginTest(unittest.TestCase, ImportHelper): self.__reset_config() config['filefilter']['path'] = '.*track_1.*\.mp3' self.__run([ - 'Album %s' % displayable_path(self.artist_path), + 'Album: %s' % displayable_path(self.artist_path), ' %s' % displayable_path(self.artist_paths[0]), - 'Album %s' % displayable_path(self.misc_path), + 'Album: %s' % displayable_path(self.misc_path), ' %s' % displayable_path(self.misc_paths[0]), ]) self.__run([ @@ -154,9 +154,9 @@ class FileFilterPluginTest(unittest.TestCase, ImportHelper): self.__reset_config() config['filefilter']['album_path'] = '.*track_1.*\.mp3' self.__run([ - 'Album %s' % displayable_path(self.artist_path), + 'Album: %s' % displayable_path(self.artist_path), ' %s' % displayable_path(self.artist_paths[0]), - 'Album %s' % displayable_path(self.misc_path), + 'Album: %s' % displayable_path(self.misc_path), ' %s' % displayable_path(self.misc_paths[0]), ]) self.__run([ @@ -177,13 +177,13 @@ class FileFilterPluginTest(unittest.TestCase, ImportHelper): 'Singleton: %s' % displayable_path(self.misc_paths[0]) ], singletons=True) self.__run([ - 'Album %s' % displayable_path(self.artist_path), + 'Album: %s' % displayable_path(self.artist_path), ' %s' % displayable_path(self.artist_paths[0]), ' %s' % displayable_path(self.artist_paths[1]), - 'Album %s' % displayable_path(self.album_path), + 'Album: %s' % displayable_path(self.album_path), ' %s' % displayable_path(self.album_paths[0]), ' %s' % displayable_path(self.album_paths[1]), - 'Album %s' % displayable_path(self.misc_path), + 'Album: %s' % displayable_path(self.misc_path), ' %s' % displayable_path(self.misc_paths[0]), ' %s' % displayable_path(self.misc_paths[1]) ]) @@ -194,9 +194,9 @@ class FileFilterPluginTest(unittest.TestCase, ImportHelper): config['filefilter']['album_path'] = '.*track_1.*\.mp3' config['filefilter']['singleton_path'] = '.*track_2.*\.mp3' self.__run([ - 'Album %s' % displayable_path(self.artist_path), + 'Album: %s' % displayable_path(self.artist_path), ' %s' % displayable_path(self.artist_paths[0]), - 'Album %s' % displayable_path(self.misc_path), + 'Album: %s' % displayable_path(self.misc_path), ' %s' % displayable_path(self.misc_paths[0]), ]) self.__run([ From 07f99e6ab7ea4b4a2c248d6cf720a84de8932b30 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 20 Jan 2015 14:51:26 -0800 Subject: [PATCH 36/56] Fix up tests for import_task_created (#1186) --- test/test_plugins.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/test/test_plugins.py b/test/test_plugins.py index 3771828b7..c4d8c4d9a 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -235,14 +235,15 @@ class EventsTest(unittest.TestCase, ImportHelper, TestHelper): self.importer.run() self.unload_plugins() - self.assertEqual(logs.count('Sending event: import_task_created'), 2, - 'Only two import_task_created events (one for the ' - 'album and one for the sentinel)') - logs = [line for line in logs if not line.startswith('Sending event:')] + # Exactly one event should have been imported (for the album). + # Sentinels do not get emitted. + self.assertEqual(logs.count('Sending event: import_task_created'), 1) + logs = [line for line in logs if not line.startswith('Sending event:')] self.assertEqual(logs, [ - 'Singleton: %s' % self.file_paths[0], - 'Singleton: %s' % self.file_paths[1] + 'Album: {0}/album'.format(self.import_dir), + ' {0}'.format(self.file_paths[0]), + ' {0}'.format(self.file_paths[1]), ]) From ea2474277a542ba41dcb1df6f0d3a26888406cfc Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 20 Jan 2015 14:55:50 -0800 Subject: [PATCH 37/56] Changelog/typo fix for filefilter (#1186) --- docs/changelog.rst | 3 +++ docs/plugins/filefilter.rst | 4 +--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 374f48b46..903eb4848 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -6,6 +6,9 @@ Changelog Features: +* A new :doc:`/plugins/filefilter` lets you write regular expressions to + automatically avoid importing certain files. Thanks to :user:`mried`. + :bug:`1186` * Stop on invalid queries instead of ignoring the invalid part. * A new :ref:`searchlimit` configuration option allows you to specify how many search results you wish to see when looking up releases at MusicBrainz diff --git a/docs/plugins/filefilter.rst b/docs/plugins/filefilter.rst index cd51b01e4..8feee9096 100644 --- a/docs/plugins/filefilter.rst +++ b/docs/plugins/filefilter.rst @@ -18,7 +18,7 @@ configuration file. The available options are: - **album_path** and **singleton_path**: You may specify different regular expressions used for imports of albums and singletons. This way, you can automatically skip singletons when importing albums if the names (and paths) - of the files are distinguishable via a regex. The path regex defined here + of the files are distinguishable via a regex. The regexes defined here take precedence over the global ``path`` option. Here's an example:: @@ -28,5 +28,3 @@ Here's an example:: # will only import files which names start with two digits album_path: .*\d\d[^/]+$ singleton_path: .*/(?!\d\d)[^/]+$ - - From 899365c4a98fc95b3b0e7a9f836dec0f5a3898a3 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 20 Jan 2015 15:00:28 -0800 Subject: [PATCH 38/56] Finish changing plugin name in docs (#1186) --- docs/plugins/filefilter.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/plugins/filefilter.rst b/docs/plugins/filefilter.rst index 8feee9096..3daa5b9c6 100644 --- a/docs/plugins/filefilter.rst +++ b/docs/plugins/filefilter.rst @@ -1,16 +1,16 @@ FileFilter Plugin ================= -The ``regexfilefilter`` plugin allows you to skip files during import using +The ``filefilter`` plugin allows you to skip files during import using regular expressions. -To use the ``regexfilefilter`` plugin, enable it in your configuration (see +To use the ``filefilter`` plugin, enable it in your configuration (see :ref:`using-plugins`). Configuration ------------- -To configure the plugin, make an ``regexfilefilter:`` section in your +To configure the plugin, make an ``filefilter:`` section in your configuration file. The available options are: - **path**: A regular expression to filter files based on its path and name. @@ -23,7 +23,7 @@ configuration file. The available options are: Here's an example:: - regexfilefilter: + filefilter: path: .*\d\d[^/]+$ # will only import files which names start with two digits album_path: .*\d\d[^/]+$ From b737102008c1d22e92c4e98dd5172587c066c362 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 20 Jan 2015 15:02:13 -0800 Subject: [PATCH 39/56] One-byte typo fix --- docs/plugins/filefilter.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/plugins/filefilter.rst b/docs/plugins/filefilter.rst index 3daa5b9c6..3866faccd 100644 --- a/docs/plugins/filefilter.rst +++ b/docs/plugins/filefilter.rst @@ -10,7 +10,7 @@ To use the ``filefilter`` plugin, enable it in your configuration (see Configuration ------------- -To configure the plugin, make an ``filefilter:`` section in your +To configure the plugin, make a ``filefilter:`` section in your configuration file. The available options are: - **path**: A regular expression to filter files based on its path and name. From 5586fcee671029e85683012034f137d9d4f05658 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 20 Jan 2015 15:11:13 -0800 Subject: [PATCH 40/56] keyfinder: Only write on import when asked to This should really be standard machinery. --- beetsplug/keyfinder.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/beetsplug/keyfinder.py b/beetsplug/keyfinder.py index 9ce81f36e..280710f02 100644 --- a/beetsplug/keyfinder.py +++ b/beetsplug/keyfinder.py @@ -20,6 +20,7 @@ import subprocess from beets import ui from beets import util from beets.plugins import BeetsPlugin +from beets import config class KeyFinderPlugin(BeetsPlugin): @@ -31,8 +32,9 @@ class KeyFinderPlugin(BeetsPlugin): u'auto': True, u'overwrite': False, }) - self.config['auto'].get(bool) - self.import_stages = [self.imported] + + if self.config['auto'].get(bool): + self.import_stages = [self.imported] def commands(self): cmd = ui.Subcommand('keyfinder', @@ -41,13 +43,13 @@ class KeyFinderPlugin(BeetsPlugin): return [cmd] def command(self, lib, opts, args): - self.find_key(lib.items(ui.decargs(args))) + self.find_key(lib.items(ui.decargs(args)), + write=config['import']['write'].get(bool)) def imported(self, session, task): - if self.config['auto'].get(bool): - self.find_key(task.items) + self.find_key(task.items) - def find_key(self, items): + def find_key(self, items, write=False): overwrite = self.config['overwrite'].get(bool) bin = util.bytestring_path(self.config['bin'].get(unicode)) @@ -64,5 +66,7 @@ class KeyFinderPlugin(BeetsPlugin): item['initial_key'] = key self._log.debug(u'added computed initial key {0} for {1}', key, util.displayable_path(item.path)) - item.try_write() + + if write: + item.try_write() item.store() From dae1776165b630b60760d5905d0a219cb29287db Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 20 Jan 2015 15:12:26 -0800 Subject: [PATCH 41/56] Reasonable logging for keyfinder command Previously the command was completely silent. --- beetsplug/keyfinder.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/keyfinder.py b/beetsplug/keyfinder.py index 280710f02..d5b3e9bc7 100644 --- a/beetsplug/keyfinder.py +++ b/beetsplug/keyfinder.py @@ -64,8 +64,8 @@ class KeyFinderPlugin(BeetsPlugin): continue item['initial_key'] = key - self._log.debug(u'added computed initial key {0} for {1}', - key, util.displayable_path(item.path)) + self._log.info(u'added computed initial key {0} for {1}', + key, util.displayable_path(item.path)) if write: item.try_write() From f617d162cf65d93bf8ae471f5bd229f44a089d3c Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 20 Jan 2015 15:17:21 -0800 Subject: [PATCH 42/56] keyfinder: Better output parsing (#1248) We were being sloppy about bytes output from the process. Also, it seems like the tools outputs the path also, so it's necessary to break on whitespace to actually get the key name. --- beetsplug/keyfinder.py | 9 ++++++++- docs/changelog.rst | 2 ++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/beetsplug/keyfinder.py b/beetsplug/keyfinder.py index d5b3e9bc7..882ae3a88 100644 --- a/beetsplug/keyfinder.py +++ b/beetsplug/keyfinder.py @@ -58,11 +58,18 @@ class KeyFinderPlugin(BeetsPlugin): continue try: - key = util.command_output([bin, '-f', item.path]) + output = util.command_output([bin, '-f', item.path]) except (subprocess.CalledProcessError, OSError) as exc: self._log.error(u'execution failed: {0}', exc) continue + key_raw = output.rsplit(None, 1)[-1] + try: + key = key_raw.decode('utf8') + except UnicodeDecodeError: + self._log.error(u'output is invalid UTF-8') + continue + item['initial_key'] = key self._log.info(u'added computed initial key {0} for {1}', key, util.displayable_path(item.path)) diff --git a/docs/changelog.rst b/docs/changelog.rst index 903eb4848..e6c28ee6b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -39,6 +39,8 @@ Fixes: company since 2013, so we are assuming access will not be restored. * Incremental imports now (once again) show a "skipped N directories" message. * :doc:`/plugins/embedart`: Handle errors in ImageMagick's output. :bug:`1241` +* :doc:`/plugins/keyfinder`: Parse the underlying tool's output more robustly. + :bug:`1248` For developers: From 1a151c2272888c5a7485a0eb17a7ebd6abfb8071 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 20 Jan 2015 15:22:59 -0800 Subject: [PATCH 43/56] Remove redundant config rest in tests teardown_beets resets the configuration already. --- test/test_filefilter.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/test/test_filefilter.py b/test/test_filefilter.py index 0bc3826ca..a8a046a37 100644 --- a/test/test_filefilter.py +++ b/test/test_filefilter.py @@ -110,13 +110,9 @@ class FileFilterPluginTest(unittest.TestCase, ImportHelper): self.assertEqual(logs, expected_lines) - def __reset_config(self): - config['filefilter'] = {} - def test_import_default(self): """ The default configuration should import everything. """ - self.__reset_config() self.__run([ 'Album: %s' % displayable_path(self.artist_path), ' %s' % displayable_path(self.artist_paths[0]), @@ -130,13 +126,11 @@ class FileFilterPluginTest(unittest.TestCase, ImportHelper): ]) def test_import_nothing(self): - self.__reset_config() config['filefilter']['path'] = 'not_there' self.__run(['No files imported from %s' % self.import_dir]) # Global options def test_import_global(self): - self.__reset_config() config['filefilter']['path'] = '.*track_1.*\.mp3' self.__run([ 'Album: %s' % displayable_path(self.artist_path), @@ -151,7 +145,6 @@ class FileFilterPluginTest(unittest.TestCase, ImportHelper): # Album options def test_import_album(self): - self.__reset_config() config['filefilter']['album_path'] = '.*track_1.*\.mp3' self.__run([ 'Album: %s' % displayable_path(self.artist_path), @@ -170,7 +163,6 @@ class FileFilterPluginTest(unittest.TestCase, ImportHelper): # Singleton options def test_import_singleton(self): - self.__reset_config() config['filefilter']['singleton_path'] = '.*track_1.*\.mp3' self.__run([ 'Singleton: %s' % displayable_path(self.artist_paths[0]), @@ -190,7 +182,6 @@ class FileFilterPluginTest(unittest.TestCase, ImportHelper): # Album and singleton options def test_import_both(self): - self.__reset_config() config['filefilter']['album_path'] = '.*track_1.*\.mp3' config['filefilter']['singleton_path'] = '.*track_2.*\.mp3' self.__run([ From 770ffad638f6c595cacf83a68a47ec6e322b7ec3 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 20 Jan 2015 15:30:28 -0800 Subject: [PATCH 44/56] Require mutagen 1.27 Avoid incorrect reporting of audio properties for MPEG-4 on earlier versions, as in #1243. --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index a4d35c6bf..61bdd461b 100755 --- a/setup.py +++ b/setup.py @@ -75,7 +75,7 @@ setup( install_requires=[ 'enum34', - 'mutagen>=1.23', + 'mutagen>=1.27', 'munkres', 'unidecode', 'musicbrainzngs>=0.4', From 9b83cef0de0a78c7104e6acef1d8da2db5d1343f Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 20 Jan 2015 16:17:59 -0800 Subject: [PATCH 45/56] Handle encoding misfortunes in logging (#1248) The beets loggers now, as proposed in #1044, do their utmost to avoid crashing when logged values are not valid Unicode or Unicode-convertible. This came up urgently when a `CalledProcessError` could not be logged with `'u{}'.format()` (!!!) so we needed to some manner of conversion, but it should be possible to log paths without `displayable_path`. In fact, it may be time to retire `displayable_path` for the most part. --- beets/logging.py | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/beets/logging.py b/beets/logging.py index 933fdb167..bb60761b0 100644 --- a/beets/logging.py +++ b/beets/logging.py @@ -31,6 +31,34 @@ import sys PY26 = sys.version_info[:2] == (2, 6) +def as_unicode(val): + """Coerce a value to Unicode for displaying in a log message. + + This works around a number of pitfalls when logging objects in + Python 2: + - Logging path names, which must be byte strings, requires + conversion for output. + - Some objects, including some exceptions, will crash when you call + `unicode(v)` while `str(v)` works fine. CalledProcessError is an + example. + """ + if isinstance(val, unicode): + return val + elif isinstance(val, bytes): + # Blindly convert with UTF-8. Eventually, it would be nice to + # (a) only do this for paths, if they can be given a distinct + # type, and (b) warn the developer if they do this for other + # bytestrings. + return val.decode('utf8', 'replace') + else: + try: + return unicode(val) + except UnicodeDecodeError: + # An object with a broken __unicode__ formatter. Use __str__ + # instead. + return str(val).decode('utf8', 'replace') + + class StrFormatLogger(Logger): """A version of `Logger` that uses `str.format`-style formatting instead of %-style formatting. @@ -43,7 +71,9 @@ class StrFormatLogger(Logger): self.kwargs = kwargs def __str__(self): - return self.msg.format(*self.args, **self.kwargs) + args = [as_unicode(a) for a in self.args] + kwargs = dict((k, as_unicode(v)) for (k, v) in self.kwargs.items()) + return self.msg.format(*args, **kwargs) def _log(self, level, msg, args, exc_info=None, extra=None, **kwargs): """Log msg.format(*args, **kwargs)""" From 7e21c684872bea3fc55e2bc29c00a98ee49398e1 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 20 Jan 2015 16:28:42 -0800 Subject: [PATCH 46/56] Logging: pass through non-problematic values 9b83cef was a little too aggressive; we need most objects to go through unconverted so '{0.title}' and such still work in format strings. This is now just a targeted workaround for the case I encountered. --- beets/logging.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/beets/logging.py b/beets/logging.py index bb60761b0..31b12fbb3 100644 --- a/beets/logging.py +++ b/beets/logging.py @@ -24,6 +24,7 @@ from __future__ import absolute_import from copy import copy from logging import * # noqa import sys +import subprocess # We need special hacks for Python 2.6 due to logging.Logger being an @@ -31,8 +32,9 @@ import sys PY26 = sys.version_info[:2] == (2, 6) -def as_unicode(val): - """Coerce a value to Unicode for displaying in a log message. +def logsafe(val): + """Coerce a potentially "problematic" value so it can be formatted + in a Unicode log string. This works around a number of pitfalls when logging objects in Python 2: @@ -42,15 +44,20 @@ def as_unicode(val): `unicode(v)` while `str(v)` works fine. CalledProcessError is an example. """ + # Already Unicode. if isinstance(val, unicode): return val + + # Bytestring: needs decoding. elif isinstance(val, bytes): # Blindly convert with UTF-8. Eventually, it would be nice to # (a) only do this for paths, if they can be given a distinct # type, and (b) warn the developer if they do this for other # bytestrings. return val.decode('utf8', 'replace') - else: + + # A "problem" object: needs a workaround. + elif isinstance(val, subprocess.CalledProcessError): try: return unicode(val) except UnicodeDecodeError: @@ -58,6 +65,11 @@ def as_unicode(val): # instead. return str(val).decode('utf8', 'replace') + # Other objects are used as-is so field access, etc., still works in + # the format string. + else: + return val + class StrFormatLogger(Logger): """A version of `Logger` that uses `str.format`-style formatting @@ -71,8 +83,8 @@ class StrFormatLogger(Logger): self.kwargs = kwargs def __str__(self): - args = [as_unicode(a) for a in self.args] - kwargs = dict((k, as_unicode(v)) for (k, v) in self.kwargs.items()) + args = [logsafe(a) for a in self.args] + kwargs = dict((k, logsafe(v)) for (k, v) in self.kwargs.items()) return self.msg.format(*args, **kwargs) def _log(self, level, msg, args, exc_info=None, extra=None, **kwargs): From 3317580296c04d1baa2289f68df52d9824013f65 Mon Sep 17 00:00:00 2001 From: David Logie Date: Mon, 19 Jan 2015 11:07:21 +0000 Subject: [PATCH 47/56] mbsync: Support custom output format. --- beetsplug/mbsync.py | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/beetsplug/mbsync.py b/beetsplug/mbsync.py index b1a74da28..95ca15c51 100644 --- a/beetsplug/mbsync.py +++ b/beetsplug/mbsync.py @@ -18,6 +18,7 @@ from beets.plugins import BeetsPlugin from beets import autotag, library, ui, util from beets.autotag import hooks from beets import config +from beets.util.functemplate import Template from collections import defaultdict @@ -49,6 +50,8 @@ class MBSyncPlugin(BeetsPlugin): cmd.parser.add_option('-W', '--nowrite', action='store_false', default=config['import']['write'], dest='write', help="don't write updated metadata to files") + cmd.parser.add_option('-f', '--format', action='store', default=None, + help='print with custom format') cmd.func = self.func return [cmd] @@ -59,24 +62,28 @@ class MBSyncPlugin(BeetsPlugin): pretend = opts.pretend write = opts.write query = ui.decargs(args) + tmpl = Template(ui._pick_format(True, opts.format)) - self.singletons(lib, query, move, pretend, write) - self.albums(lib, query, move, pretend, write) + self.singletons(lib, query, move, pretend, write, tmpl) + self.albums(lib, query, move, pretend, write, tmpl) - def singletons(self, lib, query, move, pretend, write): + def singletons(self, lib, query, move, pretend, write, tmpl): """Retrieve and apply info from the autotagger for items matched by query. """ for item in lib.items(query + ['singleton:true']): + item_formatted = item.evaluate_template(tmpl) if not item.mb_trackid: - self._log.info(u'Skipping singleton {0}: has no mb_trackid', - item.title) + self._log.info(u'Skipping singleton with no mb_trackid: {0}', + item_formatted) continue # Get the MusicBrainz recording info. track_info = hooks.track_for_mbid(item.mb_trackid) if not track_info: - self._log.info(u'Recording ID not found: {0}', item.mb_trackid) + self._log.info(u'Recording ID not found: {0} for track {0}', + item.mb_trackid, + item_formatted) continue # Apply. @@ -84,14 +91,16 @@ class MBSyncPlugin(BeetsPlugin): autotag.apply_item_metadata(item, track_info) apply_item_changes(lib, item, move, pretend, write) - def albums(self, lib, query, move, pretend, write): + def albums(self, lib, query, move, pretend, write, tmpl): """Retrieve and apply info from the autotagger for albums matched by query and their items. """ # Process matching albums. for a in lib.albums(query): + album_formatted = a.evaluate_template(tmpl) if not a.mb_albumid: - self._log.info(u'Skipping album {0}: has no mb_albumid', a.id) + self._log.info(u'Skipping album with no mb_albumid: {0}', + album_formatted) continue items = list(a.items()) @@ -99,7 +108,9 @@ class MBSyncPlugin(BeetsPlugin): # Get the MusicBrainz album information. album_info = hooks.album_for_mbid(a.mb_albumid) if not album_info: - self._log.info(u'Release ID not found: {0}', a.mb_albumid) + self._log.info(u'Release ID {0} not found for album {1}', + a.mb_albumid, + album_formatted) continue # Map recording MBIDs to their information. Recordings can appear @@ -147,5 +158,5 @@ class MBSyncPlugin(BeetsPlugin): # Move album art (and any inconsistent items). if move and lib.directory in util.ancestry(items[0].path): - self._log.debug(u'moving album {0}', a.id) + self._log.debug(u'moving album {0}', album_formatted) a.move() From 2fae94a93a6c234c1dd1be36e2d6ea087eabc140 Mon Sep 17 00:00:00 2001 From: David Logie Date: Mon, 19 Jan 2015 13:44:14 +0000 Subject: [PATCH 48/56] Make the Template object an attribute. --- beetsplug/mbsync.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/beetsplug/mbsync.py b/beetsplug/mbsync.py index 95ca15c51..9f22d8fae 100644 --- a/beetsplug/mbsync.py +++ b/beetsplug/mbsync.py @@ -62,17 +62,18 @@ class MBSyncPlugin(BeetsPlugin): pretend = opts.pretend write = opts.write query = ui.decargs(args) - tmpl = Template(ui._pick_format(True, opts.format)) - self.singletons(lib, query, move, pretend, write, tmpl) - self.albums(lib, query, move, pretend, write, tmpl) + self.template = Template(ui._pick_format(True, opts.format)) - def singletons(self, lib, query, move, pretend, write, tmpl): + self.singletons(lib, query, move, pretend, write) + self.albums(lib, query, move, pretend, write) + + def singletons(self, lib, query, move, pretend, write): """Retrieve and apply info from the autotagger for items matched by query. """ for item in lib.items(query + ['singleton:true']): - item_formatted = item.evaluate_template(tmpl) + item_formatted = item.evaluate_template(self.template) if not item.mb_trackid: self._log.info(u'Skipping singleton with no mb_trackid: {0}', item_formatted) @@ -91,13 +92,13 @@ class MBSyncPlugin(BeetsPlugin): autotag.apply_item_metadata(item, track_info) apply_item_changes(lib, item, move, pretend, write) - def albums(self, lib, query, move, pretend, write, tmpl): + def albums(self, lib, query, move, pretend, write): """Retrieve and apply info from the autotagger for albums matched by query and their items. """ # Process matching albums. for a in lib.albums(query): - album_formatted = a.evaluate_template(tmpl) + album_formatted = a.evaluate_template(self.template) if not a.mb_albumid: self._log.info(u'Skipping album with no mb_albumid: {0}', album_formatted) From 1380ee79dce12922f31694d5e8c7702a5b271deb Mon Sep 17 00:00:00 2001 From: David Logie Date: Tue, 20 Jan 2015 08:48:49 +0000 Subject: [PATCH 49/56] Add basic test for custom mbsync formats. --- test/test_mbsync.py | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/test/test_mbsync.py b/test/test_mbsync.py index 8dddf60c0..6ad89fd99 100644 --- a/test/test_mbsync.py +++ b/test/test_mbsync.py @@ -17,7 +17,8 @@ from mock import patch from _common import unittest from helper import TestHelper,\ generate_album_info, \ - generate_track_info + generate_track_info, \ + capture_log from beets.library import Item @@ -41,8 +42,32 @@ class MbsyncCliTest(unittest.TestCase, TestHelper): generate_track_info('singleton track id', {'title': 'singleton info'}) - album_item = Item( + # test album with no mb_albumid + album_invalid_item = Item( + album='album info', + path='' + ) + album_invalid = self.lib.add_album([album_invalid_item]) + + with capture_log('beets.mbsync') as logs: + self.run_command('mbsync', '-f', "'$album'") + e = "mbsync: Skipping album with no mb_albumid: 'album info'" + self.assertEqual(e, logs[0]) + + # test singleton with no mb_trackid + item_invalid = Item( title='old title', + path='', + ) + self.lib.add(item_invalid) + + with capture_log('beets.mbsync') as logs: + self.run_command('mbsync', '-f', "'$title'") + e = "mbsync: Skipping singleton with no mb_trackid: 'old title'" + self.assertEqual(e, logs[0]) + + album_item = Item( + album='old title', mb_albumid='album id', mb_trackid='track id', path='' From 3e9f716b68ff987ca2cbfd5873c4e1f8d843203c Mon Sep 17 00:00:00 2001 From: David Logie Date: Tue, 20 Jan 2015 10:14:04 +0000 Subject: [PATCH 50/56] Remove unused variable. --- test/test_mbsync.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_mbsync.py b/test/test_mbsync.py index 6ad89fd99..569bfd8ff 100644 --- a/test/test_mbsync.py +++ b/test/test_mbsync.py @@ -43,11 +43,11 @@ class MbsyncCliTest(unittest.TestCase, TestHelper): {'title': 'singleton info'}) # test album with no mb_albumid - album_invalid_item = Item( + album_invalid = Item( album='album info', path='' ) - album_invalid = self.lib.add_album([album_invalid_item]) + self.lib.add_album([album_invalid]) with capture_log('beets.mbsync') as logs: self.run_command('mbsync', '-f', "'$album'") From 7db3e1d80c5af5f1d5517f2966cda0a25a13555e Mon Sep 17 00:00:00 2001 From: David Logie Date: Tue, 20 Jan 2015 10:40:06 +0000 Subject: [PATCH 51/56] mbsync: Fix bug where singletons always used the album format. --- beetsplug/mbsync.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/beetsplug/mbsync.py b/beetsplug/mbsync.py index 9f22d8fae..fb3710e55 100644 --- a/beetsplug/mbsync.py +++ b/beetsplug/mbsync.py @@ -62,18 +62,19 @@ class MBSyncPlugin(BeetsPlugin): pretend = opts.pretend write = opts.write query = ui.decargs(args) + format = opts.format - self.template = Template(ui._pick_format(True, opts.format)) + self.singletons(lib, query, move, pretend, write, format) + self.albums(lib, query, move, pretend, write, format) - self.singletons(lib, query, move, pretend, write) - self.albums(lib, query, move, pretend, write) - - def singletons(self, lib, query, move, pretend, write): + def singletons(self, lib, query, move, pretend, write, format): """Retrieve and apply info from the autotagger for items matched by query. """ + template = Template(ui._pick_format(False, format)) + for item in lib.items(query + ['singleton:true']): - item_formatted = item.evaluate_template(self.template) + item_formatted = item.evaluate_template(template) if not item.mb_trackid: self._log.info(u'Skipping singleton with no mb_trackid: {0}', item_formatted) @@ -92,13 +93,15 @@ class MBSyncPlugin(BeetsPlugin): autotag.apply_item_metadata(item, track_info) apply_item_changes(lib, item, move, pretend, write) - def albums(self, lib, query, move, pretend, write): + def albums(self, lib, query, move, pretend, write, format): """Retrieve and apply info from the autotagger for albums matched by query and their items. """ + template = Template(ui._pick_format(True, format)) + # Process matching albums. for a in lib.albums(query): - album_formatted = a.evaluate_template(self.template) + album_formatted = a.evaluate_template(template) if not a.mb_albumid: self._log.info(u'Skipping album with no mb_albumid: {0}', album_formatted) From 11a21e149e883a5438efba2a1d84210cd7141176 Mon Sep 17 00:00:00 2001 From: David Logie Date: Tue, 20 Jan 2015 10:40:38 +0000 Subject: [PATCH 52/56] mbsync: Improve tests. * Move the "skipped item" tests to their own method. * Add a test for the default format --- test/test_mbsync.py | 80 +++++++++++++++++++++++++++++++-------------- 1 file changed, 56 insertions(+), 24 deletions(-) diff --git a/test/test_mbsync.py b/test/test_mbsync.py index 569bfd8ff..56967b7b7 100644 --- a/test/test_mbsync.py +++ b/test/test_mbsync.py @@ -42,30 +42,6 @@ class MbsyncCliTest(unittest.TestCase, TestHelper): generate_track_info('singleton track id', {'title': 'singleton info'}) - # test album with no mb_albumid - album_invalid = Item( - album='album info', - path='' - ) - self.lib.add_album([album_invalid]) - - with capture_log('beets.mbsync') as logs: - self.run_command('mbsync', '-f', "'$album'") - e = "mbsync: Skipping album with no mb_albumid: 'album info'" - self.assertEqual(e, logs[0]) - - # test singleton with no mb_trackid - item_invalid = Item( - title='old title', - path='', - ) - self.lib.add(item_invalid) - - with capture_log('beets.mbsync') as logs: - self.run_command('mbsync', '-f', "'$title'") - e = "mbsync: Skipping singleton with no mb_trackid: 'old title'" - self.assertEqual(e, logs[0]) - album_item = Item( album='old title', mb_albumid='album id', @@ -92,6 +68,62 @@ class MbsyncCliTest(unittest.TestCase, TestHelper): album.load() self.assertEqual(album.album, 'album info') + @patch('beets.autotag.hooks.album_for_mbid') + @patch('beets.autotag.hooks.track_for_mbid') + def test_message_when_skipping(self, track_for_mbid, album_for_mbid): + album_for_mbid.return_value = \ + generate_album_info('album id', ['track id']) + track_for_mbid.return_value = \ + generate_track_info('singleton track id', + {'title': 'singleton info'}) + + # Test album with no mb_albumid. + # The default format for an album include $albumartist so + # set that here, too. + album_invalid = Item( + albumartist='album info', + album='album info', + path='' + ) + self.lib.add_album([album_invalid]) + + # default format + with capture_log('beets.mbsync') as logs: + self.run_command('mbsync') + e = 'mbsync: Skipping album with no mb_albumid: ' + \ + 'album info - album info' + self.assertEqual(e, logs[0]) + + # custom format + with capture_log('beets.mbsync') as logs: + self.run_command('mbsync', '-f', "'$album'") + e = "mbsync: Skipping album with no mb_albumid: 'album info'" + self.assertEqual(e, logs[0]) + + # Test singleton with no mb_trackid. + # The default singleton format includes $artist and $album + # so we need to stub them here + item_invalid = Item( + artist='album info', + album='album info', + title='old title', + path='', + ) + self.lib.add(item_invalid) + + # default format + with capture_log('beets.mbsync') as logs: + self.run_command('mbsync') + e = 'mbsync: Skipping singleton with no mb_trackid: ' + \ + 'album info - album info - old title' + self.assertEqual(e, logs[0]) + + # custom format + with capture_log('beets.mbsync') as logs: + self.run_command('mbsync', '-f', "'$title'") + e = "mbsync: Skipping singleton with no mb_trackid: 'old title'" + self.assertEqual(e, logs[0]) + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From 99e89a29e2494c14bd63590b0206592b20a7a784 Mon Sep 17 00:00:00 2001 From: David Logie Date: Tue, 20 Jan 2015 11:19:52 +0000 Subject: [PATCH 53/56] mbsync: Don't overwrite the `format` built-in. --- beetsplug/mbsync.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/beetsplug/mbsync.py b/beetsplug/mbsync.py index fb3710e55..a4e242026 100644 --- a/beetsplug/mbsync.py +++ b/beetsplug/mbsync.py @@ -62,16 +62,16 @@ class MBSyncPlugin(BeetsPlugin): pretend = opts.pretend write = opts.write query = ui.decargs(args) - format = opts.format + fmt = opts.format - self.singletons(lib, query, move, pretend, write, format) - self.albums(lib, query, move, pretend, write, format) + self.singletons(lib, query, move, pretend, write, fmt) + self.albums(lib, query, move, pretend, write, fmt) - def singletons(self, lib, query, move, pretend, write, format): + def singletons(self, lib, query, move, pretend, write, fmt): """Retrieve and apply info from the autotagger for items matched by query. """ - template = Template(ui._pick_format(False, format)) + template = Template(ui._pick_format(False, fmt)) for item in lib.items(query + ['singleton:true']): item_formatted = item.evaluate_template(template) @@ -93,11 +93,11 @@ class MBSyncPlugin(BeetsPlugin): autotag.apply_item_metadata(item, track_info) apply_item_changes(lib, item, move, pretend, write) - def albums(self, lib, query, move, pretend, write, format): + def albums(self, lib, query, move, pretend, write, fmt): """Retrieve and apply info from the autotagger for albums matched by query and their items. """ - template = Template(ui._pick_format(True, format)) + template = Template(ui._pick_format(True, fmt)) # Process matching albums. for a in lib.albums(query): From 28f616f75d0a5aac0927d32d27daeaa2d1f0bf85 Mon Sep 17 00:00:00 2001 From: David Logie Date: Tue, 20 Jan 2015 16:40:28 +0000 Subject: [PATCH 54/56] mbsync: Remove unnecessary decorators. --- test/test_mbsync.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/test/test_mbsync.py b/test/test_mbsync.py index 56967b7b7..ea5e78155 100644 --- a/test/test_mbsync.py +++ b/test/test_mbsync.py @@ -68,15 +68,7 @@ class MbsyncCliTest(unittest.TestCase, TestHelper): album.load() self.assertEqual(album.album, 'album info') - @patch('beets.autotag.hooks.album_for_mbid') - @patch('beets.autotag.hooks.track_for_mbid') - def test_message_when_skipping(self, track_for_mbid, album_for_mbid): - album_for_mbid.return_value = \ - generate_album_info('album id', ['track id']) - track_for_mbid.return_value = \ - generate_track_info('singleton track id', - {'title': 'singleton info'}) - + def test_message_when_skipping(self): # Test album with no mb_albumid. # The default format for an album include $albumartist so # set that here, too. From f458549e4ee1ce646eed9eb5e8f2294d60ec356b Mon Sep 17 00:00:00 2001 From: David Logie Date: Tue, 20 Jan 2015 16:41:58 +0000 Subject: [PATCH 55/56] mbsync: Set default album/item format for tests. --- test/test_mbsync.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/test_mbsync.py b/test/test_mbsync.py index ea5e78155..a6248023c 100644 --- a/test/test_mbsync.py +++ b/test/test_mbsync.py @@ -20,6 +20,7 @@ from helper import TestHelper,\ generate_track_info, \ capture_log +from beets import config from beets.library import Item @@ -69,6 +70,9 @@ class MbsyncCliTest(unittest.TestCase, TestHelper): self.assertEqual(album.album, 'album info') def test_message_when_skipping(self): + config['list_format_item'] = '$artist - $album - $title' + config['list_format_album'] = '$albumartist - $album' + # Test album with no mb_albumid. # The default format for an album include $albumartist so # set that here, too. From 8ccd385d85584c7fc1856183cfc7fe2209a73ddb Mon Sep 17 00:00:00 2001 From: David Logie Date: Wed, 21 Jan 2015 08:05:12 +0000 Subject: [PATCH 56/56] Add a note to the docs and changelog about the new -f/--format option for mbsync. --- docs/changelog.rst | 2 ++ docs/plugins/mbsync.rst | 3 +++ 2 files changed, 5 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index e6c28ee6b..3d3cbc27c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -19,6 +19,8 @@ Features: * :doc:`/plugins/web`: Add support for *cross-origin resource sharing* for more flexible in-browser clients. Thanks to Andre Miller. :bug:`1236` :bug:`1237` +* :doc:`plugins/mbsync`: Add ``-f/--format`` option for controlling + the output format for unrecognized items. Fixes: diff --git a/docs/plugins/mbsync.rst b/docs/plugins/mbsync.rst index 633725b69..cf89974e4 100644 --- a/docs/plugins/mbsync.rst +++ b/docs/plugins/mbsync.rst @@ -33,3 +33,6 @@ The command has a few command-line options: * If you have the `import.write` configuration option enabled, then this plugin will write new metadata to files' tags. To disable this, use the ``-W`` (``--nowrite``) option. +* To customize the output of unrecognized items, use the ``-f`` + (``--format``) option. The default output is ``list_format_item`` or + ``list_format_album`` for items and albums, respectively.