From 908584bde8a8809ab9c82c257dadc15f13c795f4 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 20 May 2014 16:24:40 -0700 Subject: [PATCH] Revamp FormattedItemMapping This subclass was not cleanly conforming to the Mapping abstract base. Now we're in business -- the thing looks like a dict. Brought up by #782. --- beets/library.py | 39 ++++++++++++++++++--------------- setup.cfg | 2 +- test/test_dbcore.py | 10 +++++++++ test/test_library.py | 51 ++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 82 insertions(+), 20 deletions(-) diff --git a/beets/library.py b/beets/library.py index 44ca3540d..2c3d4b37a 100644 --- a/beets/library.py +++ b/beets/library.py @@ -577,42 +577,47 @@ class FormattedItemMapping(dbcore.db.FormattedMapping): ``item[key]`` or ``album[key]``. Here `album` is the album associated to `item` if it exists. """ - # FIXME This class should be in the same module as `FormattedMapping` - def __init__(self, item, for_path=False): - self.for_path = for_path - self.model = item - self.model_keys = item.keys(True) + super(FormattedItemMapping, self).__init__(item, for_path) self.album = item.get_album() self.album_keys = [] if self.album: for key in self.album.keys(True): if key in Album.item_keys or key not in item._fields.keys(): self.album_keys.append(key) + self.all_keys = set(self.model_keys).union(self.album_keys) - def get(self, key, default=None): + def _get(self, key): + """Get the value for a key, either from the album or the item. + Raise a KeyError for invalid keys. + """ if key in self.album_keys: return self.album._get_formatted(key, self.for_path) elif key in self.model_keys: return self.model._get_formatted(key, self.for_path) else: - if default is None: - raise KeyError(key) - else: - return default + raise KeyError(key) def __getitem__(self, key): - value = self.get(key) + """Get the value for a key. Certain unset values are remapped. + """ + value = self._get(key) + # `artist` and `albumartist` fields fall back to one another. + # This is helpful in path formats when the album artist is unset + # on as-is imports. if key == 'artist' and not value: - return self.get('albumartist') - if key == 'albumartist' and not value: - return self.get('artist') + return self._get('albumartist') + elif key == 'albumartist' and not value: + return self._get('artist') + else: + return value - return value + def __iter__(self): + return iter(self.all_keys) - def __contains__(self, key): - return key in self.model_keys or key in self.album_keys + def __len__(self): + return len(self.all_keys) class Album(LibModel): diff --git a/setup.cfg b/setup.cfg index 870a7c1fb..cde4869ac 100644 --- a/setup.cfg +++ b/setup.cfg @@ -9,4 +9,4 @@ ignore=E241,E221 # List of files that have not been cleand up yet. We will try to reduce # this with each commit -exclude=test/test_config_command.py,test/test_files.py,test/test_art.py,test/test_dbcore.py,test/test_web.py,test/test_zero.py,test/rsrc/beetsplug/test.py,test/test_template.py,test/test_echonest.py,test/test_datequery.py,test/test_mb.py,test/test_convert.py,test/testall.py,test/test_player.py,test/test_query.py,test/test_mediafile.py,test/test_keyfinder.py,test/test_the.py,test/test_library.py,test/test_lyrics.py,test/test_ihate.py,test/test_vfs.py,test/test_replaygain.py,test/__init__.py,test/_common.py,test/test_mediafile_edge.py,test/test_ui.py +exclude=test/test_config_command.py,test/test_files.py,test/test_art.py,test/test_web.py,test/test_zero.py,test/rsrc/beetsplug/test.py,test/test_template.py,test/test_echonest.py,test/test_datequery.py,test/test_mb.py,test/test_convert.py,test/testall.py,test/test_player.py,test/test_query.py,test/test_mediafile.py,test/test_keyfinder.py,test/test_the.py,test/test_lyrics.py,test/test_ihate.py,test/test_vfs.py,test/test_replaygain.py,test/__init__.py,test/_common.py,test/test_mediafile_edge.py,test/test_ui.py diff --git a/test/test_dbcore.py b/test/test_dbcore.py index 20fa67510..97218eef8 100644 --- a/test/test_dbcore.py +++ b/test/test_dbcore.py @@ -281,6 +281,16 @@ class FormattedMappingTest(_common.TestCase): with self.assertRaises(KeyError): formatted['other_field'] + def test_get_method_with_none_default(self): + model = TestModel1() + formatted = model._formatted_mapping() + self.assertIsNone(formatted.get('other_field')) + + def test_get_method_with_specified_default(self): + model = TestModel1() + formatted = model._formatted_mapping() + self.assertEqual(formatted.get('other_field', 'default'), 'default') + class ParseTest(_common.TestCase): def test_parse_fixed_field(self): diff --git a/test/test_library.py b/test/test_library.py index eea513454..9c40e8823 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -447,6 +447,53 @@ class DestinationTest(_common.TestCase): self.assertEqual(dest, u'foo.caf\xe9') +class ItemFormattedMappingTest(_common.LibTestCase): + def test_formatted_item_value(self): + formatted = self.i._formatted_mapping() + self.assertEqual(formatted['artist'], 'the artist') + + def test_get_unset_field(self): + formatted = self.i._formatted_mapping() + with self.assertRaises(KeyError): + formatted['other_field'] + + def test_get_method_with_none_default(self): + formatted = self.i._formatted_mapping() + self.assertIsNone(formatted.get('other_field')) + + def test_get_method_with_specified_default(self): + formatted = self.i._formatted_mapping() + self.assertEqual(formatted.get('other_field', 'default'), 'default') + + def test_album_field_overrides_item_field(self): + # Make the album inconsistent with the item. + album = self.lib.add_album([self.i]) + album.album = 'foo' + album.store() + self.i.album = 'bar' + self.i.store() + + # Ensure the album takes precedence. + formatted = self.i._formatted_mapping() + self.assertEqual(formatted['album'], 'foo') + + def test_artist_falls_back_to_albumartist(self): + self.i.artist = '' + formatted = self.i._formatted_mapping() + self.assertEqual(formatted['artist'], 'the album artist') + + def test_albumartist_falls_back_to_artist(self): + self.i.albumartist = '' + formatted = self.i._formatted_mapping() + self.assertEqual(formatted['albumartist'], 'the artist') + + def test_both_artist_and_albumartist_empty(self): + self.i.artist = '' + self.i.albumartist = '' + formatted = self.i._formatted_mapping() + self.assertEqual(formatted['albumartist'], '') + + class PathFormattingMixin(object): """Utilities for testing path formatting.""" def _setf(self, fmt): @@ -723,8 +770,8 @@ class AlbumInfoTest(_common.TestCase): def test_album_items_consistent(self): ai = self.lib.get_album(self.i) - for item in ai.items(): - if item.id == self.i.id: + for i in ai.items(): + if i.id == self.i.id: break else: self.fail("item not found")