diff --git a/beets/attachments.py b/beets/attachments.py index 72f56dbd3..9c477d529 100644 --- a/beets/attachments.py +++ b/beets/attachments.py @@ -32,6 +32,11 @@ log = logging.getLogger('beets') AUDIO_EXTENSIONS = ['.mp3', '.ogg', '.mp4', '.m4a', '.mpc', '.wma', '.wv', '.flac', '.aiff', '.ape'] +DEFAULT_TEMPLATE = '${entity_prefix}${basename}' + +def config(key): + from beets import config + return config['attachments'][key] def ref_type(entity): # FIXME prevents circular dependency @@ -73,7 +78,6 @@ class Attachment(dbcore.db.Model): def entity(self): """Return the `Item` or `Album` we are attached to. """ - # TODO cache this for performance if self.ref is None or self.ref_type is None: return None query = dbcore.query.MatchQuery('id', self.ref) @@ -89,7 +93,7 @@ class Attachment(dbcore.db.Model): """ self.ref_type = ref_type(entity) if not entity.id: - raise ValueError('{} must have an id', format(entity)) + raise ValueError('{} must have an id'.format(entity)) self.ref = entity.id def move(self, dest=None, copy=False, overwrite=False): @@ -151,11 +155,10 @@ class Attachment(dbcore.db.Model): @property def basename(self): # TODO doc - if self.ref_type == 'item': + if ref_type(self.entity) == 'item': + # TODO use `path_prefix` prefix = os.path.splitext(self.entity.path)[0] - # FIXME circular dependency - from beets import config - for sep in config['attachment']['track separators'].get(list): + for sep in config('track separators').get(list): if self.path.startswith(prefix + sep): return self.path[(len(prefix) + len(sep)):] @@ -173,9 +176,7 @@ class Attachment(dbcore.db.Model): def _destination_template(self): # TODO template functions - # FIXME circular dependency - from beets import config - for path_spec in reversed(config['attachment']['paths'].get()): + for path_spec in reversed(config('paths').get(list)): if isinstance(path_spec, basestring): return Template(path_spec) @@ -186,6 +187,7 @@ class Attachment(dbcore.db.Model): queries = [MatchQuery(k, v) for k, v in path_spec.items()] if AndQuery(queries).match(self): return Template(template_str) + return Template(DEFAULT_TEMPLATE) def _validate(self): # TODO integrate this into the `store()` method. @@ -234,12 +236,10 @@ class DestinationTemplateMapping(collections.Mapping): For tracks (i.e. items) this is the tracks path without the extension and ` - ` attached, e.g. `/path/to/track - ` """ - if self.attachment.ref_type == 'album': + if ref_type(self.entity) == 'album': return self['entity_dir'] - elif self.attachment.ref_type == 'item': - # FIXME circular dependency - from beets import config - separator = config['attachment']['track separators'].get(list)[0] + elif ref_type(self.entity) == 'item': + separator = config('track separators').get(list)[0] return self['track_base'] + separator @property @@ -251,11 +251,11 @@ class DestinationTemplateMapping(collections.Mapping): For tracks (i.e. items) this is the tracks path without the extension (`track_base`). """ - if self.attachment.ref_type == 'album': + if ref_type(self.entity) == 'album': base = '{0} - {1}'.format(self.entity_mapping['albumartist'], self.entity_mapping['album']) return os.path.join(self['entity_dir'], base) - elif self.attachment.ref_type == 'item': + elif ref_type(self.entity) == 'item': return self['track_base'] @property @@ -265,16 +265,16 @@ class DestinationTemplateMapping(collections.Mapping): The directory includes a trailing slash. """ - if self.attachment.ref_type == 'album': + if ref_type(self.entity) == 'album': return self.entity.item_dir() + os.sep - elif self.attachment.ref_type == 'item': + elif ref_type(self.entity) == 'item': return os.path.dirname(self.entity.path) + os.sep @property def track_base(self): """For tack attachments, return the track path without its extension. """ - if self.attachment.ref_type == 'item': + if ref_type(self.entity) == 'item': return os.path.splitext(self.entity.path)[0] @property @@ -419,12 +419,7 @@ class AttachmentFactory(object): return discovered def _discover_local(self, prefix, local): - # FIXME circular dependency - from beets import config - # TODO add this to config_default.yaml - # config['attachment']['track separators'] = \ - # [os.sep, ' - ', '', ' ', '-', '_', '.'] - seps = config['attachment']['track separators'].get(list) + seps = config('track separators').get(list) if local[0] == '.': seps.append('') for sep in seps: @@ -441,10 +436,12 @@ class AttachmentFactory(object): set retrieves meta data from registered collectors and and adds it as flexible attributes """ + # TODO entity should not be optional attachment = Attachment(db=self._db, path=path, entity=entity, type=type) for key, value in self._collect_meta(type, attachment.path).items(): attachment[key] = value + return attachment def add(self, path, type, entity): @@ -485,8 +482,6 @@ class AttachmentFactory(object): Uses the functions from `register_detector` and the `attachments.types` configuration. """ - # FIXME circular dependency - from beets import config for detector in self._detectors: try: type = detector(path) @@ -496,7 +491,7 @@ class AttachmentFactory(object): # TODO logging? pass - types_config = config['attachments']['types'] + types_config = config('types') if types_config.exists(): for matcher, type in types_config.get(dict).items(): if re.match(matcher, path): diff --git a/beets/config_default.yaml b/beets/config_default.yaml index 689ab44b1..603912514 100644 --- a/beets/config_default.yaml +++ b/beets/config_default.yaml @@ -102,3 +102,6 @@ match: required: [] track_length_grace: 10 track_length_max: 30 + +attachment: + 'track separators': [' - ', ' ', '-', '_', '.'] diff --git a/beets/importer.py b/beets/importer.py index 060a188c9..4ca0106a5 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -35,6 +35,7 @@ from beets import util from beets import config from beets.util import pipeline from beets.util import syspath, normpath, displayable_path +from beets.attachments import AttachmentFactory from enum import Enum from beets import mediafile @@ -180,6 +181,8 @@ class ImportSession(object): self.query = query self.seen_idents = set() self._is_resuming = dict() + self.attachment_factory = AttachmentFactory(lib) + self.attachment_factory.register_plugins(plugins.find_plugins()) # Normalize the paths. if self.paths: @@ -354,6 +357,7 @@ class ImportTask(object): # TODO remove this eventually self.should_remove_duplicates = False self.is_album = True + self.attachments = [] def set_null_candidates(self): """Set the candidates to indicate no album match was found. @@ -606,6 +610,10 @@ class ImportTask(object): for item in self.imported_items(): item.store() + for a in self.attachments: + if move or copy: + a.move(copy=copy) + plugins.send('import_task_files', session=session, task=self) def add(self, lib): @@ -613,7 +621,12 @@ class ImportTask(object): """ with lib.transaction(): self.remove_replaced(lib) + # FIXME set album earlier so we can use it to create + # attachmenst self.album = lib.add_album(self.imported_items()) + for a in self.attachments: + a.entity = self.album + a.add(lib) def remove_replaced(self, lib): """Removes all the items from the library that have the same @@ -648,6 +661,19 @@ class ImportTask(object): item.load() self.album.load() + def discover_attachments(self, factory): + """Return a list of known attachments for files in the album's directory. + + Saves the list in the `attachments` attribute. + """ + # FIXME the album model should already be available so we can + # attach the attachment. This also means attachments must handle + # unpersisted entities. + for album_path in self.paths: + for path in factory.discover(album_path): + self.attachments.extend(factory.detect(path)) + return self.attachments + # Utilities. def prune(self, filename): @@ -722,6 +748,9 @@ class SingletonImportTask(ImportTask): with lib.transaction(): self.remove_replaced(lib) lib.add(self.item) + for a in self.attachments: + a.entity = self.item + a.add(lib) def infer_album_fields(self): raise NotImplementedError @@ -1096,6 +1125,7 @@ def apply_choices(session, task): if task.is_album: task.infer_album_fields() + task.discover_attachments(session.attachment_factory) task.add(session.lib) diff --git a/test/test_attachments.py b/test/test_attachments.py index 035ca0bfc..fc42b82e3 100644 --- a/test/test_attachments.py +++ b/test/test_attachments.py @@ -28,10 +28,10 @@ class AttachmentTestHelper(TestHelper): def setup_beets(self): super(AttachmentTestHelper, self).setup_beets() - # TODO this comes into default config - self.config['attachment']['paths'] = ['${entity_prefix}${basename}'] - self.config['attachment']['track separators'] = \ - [' - ', ' ', '-', '_', '.', os.sep] + self.config['attachments'] = { + 'paths': ['${entity_prefix}${basename}'], + 'track separators': [' - ', ' ', '-', '_', '.', os.sep] + } @property def factory(self): @@ -94,9 +94,11 @@ class AttachmentTestHelper(TestHelper): def ext_detector(path): if path.endswith('.' + ext): return ext + def collector(type, path): if type == ext: return meta + log_plugin = BeetsPlugin() log_plugin.attachment_detector = ext_detector log_plugin.attachment_collector = collector @@ -207,7 +209,7 @@ class AttachmentDestinationTest(unittest.TestCase, AttachmentTestHelper): def test_item_basename(self): self.set_path_template('$basename') - self.config['attachment']['track separators'] = ['--'] + self.config['attachments']['track separators'] = ['--'] attachment = self.create_item_attachment( '/a.ext', track_path='/track.mp3' @@ -219,7 +221,7 @@ class AttachmentDestinationTest(unittest.TestCase, AttachmentTestHelper): # Helper def set_path_template(self, *templates): - self.config['attachment']['paths'] = templates + self.config['attachments']['paths'] = templates class AttachmentTest(unittest.TestCase, AttachmentTestHelper): @@ -437,6 +439,44 @@ class EntityAttachmentsTest(unittest.TestCase, AttachmentTestHelper): self.assertEqual(queried.id, attachments[1].id) +class AttachImportTest(unittest.TestCase, AttachmentTestHelper): + + def setUp(self): + self.setup_beets() + self.add_attachment_plugin('jpg', meta={'covertype': 'front'}) + self.importer = self.create_importer() + + def tearDown(self): + self.teardown_beets() + + def test_add_album_attachment(self): + album_dir = os.path.join(self.importer.paths[0], 'album 0') + self.touch('cover.jpg', dir=album_dir) + self.importer.run() + album = self.lib.albums().get() + attachment = album.attachments().get() + self.assertEqual(attachment.type, 'jpg') + self.assertEqual(attachment['covertype'], 'front') + self.assertEqual(attachment.path, + os.path.join(album.item_dir(), 'cover.jpg')) + + def test_add_singleton_track_attachment(self): + self.config['import']['singletons'] = True + track_prefix = os.path.join(self.importer.paths[0], + 'album 0', 'track 0') + self.touch(track_prefix + '.cover.jpg') + self.importer.run() + item = self.lib.items().get() + attachment = item.attachments().get() + self.assertEqual(attachment.type, 'jpg') + self.assertEqual(attachment['covertype'], 'front') + self.assertEqual( + attachment.path, + os.path.splitext(item.path)[0] + ' - track 0.cover.jpg' + ) + self.skipTest('Basename should not contain "track 0"') + + class AttachCommandTest(unittest.TestCase, AttachmentTestHelper): """Tests the `beet attach FILE QUERY...` command """