From 67eb8cf9fd83c31af590f43d088a7e072dca97ed Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 6 Jul 2014 14:14:08 +0200 Subject: [PATCH] Integrate and test attachments into library --- beets/attachments.py | 201 +++++++++++++++++++++------------------ beets/library.py | 4 +- test/test_attachments.py | 16 +++- 3 files changed, 125 insertions(+), 96 deletions(-) diff --git a/beets/attachments.py b/beets/attachments.py index 13324e88e..f88216034 100644 --- a/beets/attachments.py +++ b/beets/attachments.py @@ -15,23 +15,22 @@ import re import urlparse -import json -import os.path from argparse import ArgumentParser -from beets import library from beets import dbcore +from beets.dbcore.query import Query, AndQuery class Attachment(dbcore.db.Model): """Represents an attachment in the database. An attachment has four properties that correspond to fields in the - database: ``url``, ``type``, ``ref``, and ``ref_type``. Flexible - attributes are accessed as ``attachment[key]``. + database: `url`, `type`, `ref`, and `ref_type`. Flexible + attributes are accessed as `attachment[key]`. """ _fields = { + 'id': dbcore.types.Id(), 'url': dbcore.types.String(), 'ref': dbcore.types.Integer(), 'ref_type': dbcore.types.String(), @@ -39,30 +38,16 @@ class Attachment(dbcore.db.Model): } _table = 'attachments' _flex_table = 'attachment_metadata' - # FIXME do we need a _search_fields property? - def __init__(self, db=None, libdir=None, **values): + def __init__(self, db=None, libdir=None, path=None, **values): + if path is not None: + values['url'] = path super(Attachment, self).__init__(db, **values) self.libdir = libdir - @classmethod - def _getters(cls): - return [] - - @property - def location(self): - """Return an url string with the ``file`` scheme omitted and - resolved to an absolute path. - """ - url = self.resolve() - if url.scheme == 'file': - return url.path - else: - return urlparse.urlunparse(url) - @property def entity(self): - """Return the ``Item`` or ``Album`` we are attached to. + """Return the `Item` or `Album` we are attached to. """ if self.ref is None or self.ref_type is None: return None @@ -74,59 +59,70 @@ class Attachment(dbcore.db.Model): @entity.setter def entity(self, entity): - """Set the ``ref`` and ``ref_type`` properties so that - ``self.entity() == entity``. + """Set the `ref` and `ref_type` properties so that + `self.entity == entity`. """ - if isinstance(entity, library.Item): + # FIXME prevents circular dependency + from beets.library import Item, Album + if isinstance(entity, Item): self.ref_type = 'item' - elif isinstance(entity, library.Album): + elif isinstance(entity, Album): self.ref_type = 'album' else: raise ValueError('{} must be a Item or Album'.format(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, destination=None, copy=False, force=False): - """Moves the attachment from its original ``url`` to its + """Moves the attachment from its original `url` to its destination URL. - If `destination` is given it must be a path. If the path is relative, it is - treated relative to the ``libdir``. + If `destination` is given it must be a path. If the path is + relative, it is treated relative to the `libdir`. If the destination is `None` the method retrieves a template from a `type -> template` map using the attachements type. It then evaluates the template in the context of the attachment and its associated entity. - The method tries to retrieve the resource from ``self.url`` and - saves it to ``destination``. If the destination already exists - and ``force`` is ``False`` it raises an error. Otherwise the - destination is overwritten and ``self.url`` is set to - ``destination``. + The method tries to retrieve the resource from `self.url` and + saves it to `destination`. If the destination already exists and + `force` is `False` it raises an error. Otherwise the destination + is overwritten and `self.url` is set to `destination`. - If ``copy`` is ``False`` and the original ``url`` pointed to a - local file it removes that file. + If `copy` is `False` and the original `url` pointed to a local + file it removes that file. """ # TODO implement raise NotImplementedError + @property + def path(self): + if self.resolve().scheme == 'file': + return self.resolve().path + + @path.setter + def path(self, value): + self.url = value + def resolve(self): - """Return a url structure for the ``url`` property. + """Return a url structure for the `url` property. - This is similar to ``urlparse(attachment.url)``. If ``url`` has - no schema it defaults to ``file``. If the schema is ``file`` and - the path is relative it is resolved relative to the ``libdir``. + This is similar to `urlparse(attachment.url)`. If `url` has + no schema it defaults to `file`. If the schema is `file` and + the path is relative it is resolved relative to the `libdir`. - The return value is an instance of ``urlparse.ParseResult``. + The return value is an instance of `urlparse.ParseResult`. """ (scheme, netloc, path, params, query, fragment) = \ - urlparse.urlparse(self.url, scheme='file') - if not os.path.isabs(path): - assert os.path.isabs(beetsdir) - path = os.path.normpath(os.path.join(beetsdir, path)) - return urlparse.ParseResult(scheme, netloc, path, params, query, fragment) + urlparse.urlparse(self.url, scheme='file') + # if not os.path.isabs(path): + # assert os.path.isabs(beetsdir) + # path = os.path.normpath(os.path.join(beetsdir, path)) + return urlparse.ParseResult(scheme, netloc, path, + params, query, fragment) def _validate(self): # TODO integrate this into the `store()` method. @@ -138,17 +134,23 @@ class Attachment(dbcore.db.Model): if key in self._fields.keys(): return self[key] else: - return object.__getattr__(self, key) + object.__getattr__(self, key) def __setattr__(self, key, value): + # Unlike dbcore.Model we do not provide attribute setters for + # flexible fields. if key in self._fields.keys(): self[key] = value else: object.__setattr__(self, key, value) + @classmethod + def _getters(cls): + return [] + class AttachmentFactory(object): - """Factory that creates or finds attachments in the database. + """Create and find attachments in the database. Using this factory is the prefered way of creating attachments as it allows plugins to provide additional data. @@ -161,17 +163,29 @@ class AttachmentFactory(object): self._collectors = [] def find(self, attachment_query=None, entity_query=None): - """See Library documentation""" - self._db.attachments(attachment_query, entity_query) + """Yield all attachments in the library matching + `attachment_query` and their associated items matching + `entity_query`. + + Calling `attachments(None, entity_query)` is equivalent to:: + + library.albums(entity_query).attachments() + \ + library.items(entity_query).attachments() + """ + # FIXME make this faster with joins + queries = [AttachmentEntityQuery(entity_query)] + if attachment_query: + queries.append(attachment_query) + return self._db._fetch(Attachment, AndQuery(queries)) def discover(self, url, entity=None): - """Yield a list of attachments for types registered with that url. + """Yield a list of attachments for types registered with the url. The method uses the registered type discoverer functions to get - a list of types for ``path``. For each type it yields an - attachment created with `create_with_type`. + a list of types for `url`. For each type it yields an attachment + created with `create_with_type`. - The scheme of the url defaults to ``file``. + The scheme of the url defaults to `file`. """ url = urlparse.urlparse(url, scheme='file') if url.scheme != 'file': @@ -180,19 +194,19 @@ class AttachmentFactory(object): # discoverers for general URLs. return - for type in self._discover_types(url.path): + for type in self._discover_types(url.path): yield self.create(url.path, type, entity) def create(self, url, type, entity=None): - """Return a populated ``Attachment`` instance. + """Return a populated `Attachment` instance. - The ``url``, ``type``, and ``entity`` properties of the - attachment are set corresponding to the arguments. The method - also populates the ``meta`` property with data retrieved from - all registered collectors. + The `url`, `type`, and `entity` properties of the attachment are + set corresponding to the arguments. The method also set + flexible attributes for metadata retrieved from all registered + collectors. """ # TODO extend this to handle general urls - attachment = Attachment(db=self._db, beetsdir=self._libdir, + attachment = Attachment(db=self._db, libdir=self._libdir, url=url, type=type) if entity is not None: attachment.entity = entity @@ -203,34 +217,44 @@ class AttachmentFactory(object): def register_discoverer(self, discover): """`discover` is a callable accepting the path of an attachment as its only argument. If it was able to determine the type it - returns its name as a string. Otherwise it must return ``None`` + returns its name as a string. Otherwise it must return `None` """ self._discoverers.append(discover) def register_collector(self, collector): + """`collector` is a callable accepting the type and path of an + attachment as its arguments. The `collector` should return a + dictionary of metadata it was able to retrieve from the source + or `None`. + """ self._collectors.append(collector) def register_plugin(self, plugin): self.register_discoverer(plugin.attachment_discoverer) self.register_collector(plugin.attachment_collector) - def _discover_types(self, url): + def _discover_types(self, path): types = [] for discover in self._discoverers: try: - type = discover(url) + type = discover(path) if type: types.append(type) except: + # TODO logging? pass return types - def _collect_meta(self, type, url): + def _collect_meta(self, type, path): all_meta = {} for collector in self._collectors: - meta = collector(type, url) - if isinstance(meta, dict): - all_meta.update(meta) + try: + meta = collector(type, path) + if isinstance(meta, dict): + all_meta.update(meta) + except: + # TODO logging? + pass return all_meta @@ -241,8 +265,8 @@ class AttachmentCommand(ArgumentParser): name = None """Invoke the command if this string is given as the subcommand. - If ``name`` is "myplugin" the command is run when using ``beet - myplugin`` on the command line. + If `name` is "myplugin" the command is run when using `beet + myplugin` on the command line. """ aliases = [] @@ -250,7 +274,7 @@ class AttachmentCommand(ArgumentParser): """ factory = None - """Instance of ``AtachmentFactory``. + """Instance of `AtachmentFactory`. This property will be set by beets before running the command. """ @@ -261,41 +285,34 @@ class AttachmentCommand(ArgumentParser): def run(self, arguments): """Execute the command. - :param arguments: A namespace object as returned by ``parse_args()``. + :param arguments: A namespace object as returned by `parse_args()`. """ raise NotImplementedError def add_arguments(self, arguments): - """Adds custom arguments with ``ArgumentParser.add_argument()``. + """Adds custom arguments with `ArgumentParser.add_argument()`. - The method is called by beets prior to calling ``parse_args``. + The method is called by beets prior to calling `parse_args`. """ pass -class LibraryMixin(object): - """Extends ``beets.library.Library`` with attachment queries. - """ - def attachments(self, attachment_query=None, entity_query=None): - """Yield all attachments in the library matching - ``attachment_query`` and their associated items matching - ``entity_query``. +class AttachmentEntityQuery(Query): - Calling `attachments(None, entity_query)` is equivalent to:: + def __init__(self, query): + self.query = query - library.albums(entity_query).attachments() + \ - library.items(entity_query).attachments() - """ - # TODO implement - raise NotImplementedError + def match(self, attachment): + if self.query is not None: + return self.query.match(attachment.entity) + else: + return True class LibModelMixin(object): - """Extends ``beets.library.LibModel`` with attachment queries. + """Get associated attachments of `beets.library.LibModel` instances. """ def attachments(self): - """Return a list of attachements associated to this model. - """ # TODO implement raise NotImplementedError diff --git a/beets/library.py b/beets/library.py index 8496a844e..7b46c1f77 100644 --- a/beets/library.py +++ b/beets/library.py @@ -30,7 +30,7 @@ from beets.util.functemplate import Template from beets import dbcore from beets.dbcore import types import beets - +from beets.attachments import Attachment log = logging.getLogger('beets') @@ -957,7 +957,7 @@ def get_query(val, model_cls): class Library(dbcore.Database): """A database of music containing songs and albums. """ - _models = (Item, Album) + _models = (Item, Album, Attachment) def __init__(self, path='library.blb', directory='~/Music', diff --git a/test/test_attachments.py b/test/test_attachments.py index 8305b42a3..1d8045f30 100644 --- a/test/test_attachments.py +++ b/test/test_attachments.py @@ -18,13 +18,14 @@ from _common import unittest from beets.attachments import AttachmentFactory from beets.library import Library, Album + class AttachmentFactoryTest(unittest.TestCase): def setUp(self): self.lib = Library(':memory:') self.factory = AttachmentFactory(self.lib) - def test_create(self): + def test_create_with_url_and_type(self): attachment = self.factory.create('/path/to/attachment', 'coverart') self.assertEqual(attachment.url, '/path/to/attachment') self.assertEqual(attachment.type, 'coverart') @@ -38,13 +39,24 @@ class AttachmentFactoryTest(unittest.TestCase): self.assertEqual(attachment.ref_type, 'album') def test_create_populates_metadata(self): - def collector(type, url): + def collector(type, path): return {'mime': 'image/'} self.factory.register_collector(collector) attachment = self.factory.create('/path/to/attachment', 'coverart') self.assertEqual(attachment['mime'], 'image/') + def test_find_all_attachments(self): + self.factory.create('/path', 'atype').add() + self.factory.create('/another_path', 'asecondtype').add() + + all_attachments = self.factory.find() + self.assertEqual(len(all_attachments), 2) + + attachment = all_attachments.get() + self.assertEqual(attachment.path, '/path') + self.assertEqual(attachment.type, 'atype') + def suite(): return unittest.TestLoader().loadTestsFromName(__name__)