From b026d60c314683bceaa1a896da1865fa0fc2d659 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Wed, 2 Apr 2014 23:08:15 +0200 Subject: [PATCH 01/14] Add MediaFile.fields() method --- beets/mediafile.py | 8 ++++++++ test/test_mediafile.py | 14 ++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/beets/mediafile.py b/beets/mediafile.py index 301e0f370..3ef7d16f8 100644 --- a/beets/mediafile.py +++ b/beets/mediafile.py @@ -1256,6 +1256,14 @@ class MediaFile(object): for tag in self.mgfile.keys(): del self.mgfile[tag] + @classmethod + def fields(cls): + """Yield the names of all properties that are MediaFields. + """ + for property, descriptor in cls.__dict__.items(): + if isinstance(descriptor, MediaField): + yield property + # Field definitions. diff --git a/test/test_mediafile.py b/test/test_mediafile.py index 19d2ceda8..c710bde1f 100644 --- a/test/test_mediafile.py +++ b/test/test_mediafile.py @@ -692,6 +692,20 @@ class OpusTest(ReadWriteTestBase, unittest.TestCase): } +class MediaFieldTest(unittest.TestCase): + + def test_properties_from_fields(self): + path = os.path.join(_common.RSRC, 'full.mp3') + mediafile = MediaFile(path) + for field in MediaFile.fields(): + self.assertTrue(hasattr(mediafile, field)) + + def test_known_fields(self): + fields = ReadWriteTestBase.empty_tags.keys() + fields.extend(('encoder', 'images', 'genres', 'albumtype')) + self.assertItemsEqual(MediaFile.fields(), fields) + + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From bcb72becf84381564b3101ae5af4713dcfa9abb8 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Wed, 2 Apr 2014 23:22:53 +0200 Subject: [PATCH 02/14] Add MediaFile.update() method to supersede save() --- beets/library.py | 6 ++---- beets/mediafile.py | 20 ++++++++++++++++++++ test/test_mediafile.py | 20 ++++++++++++++++++++ 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/beets/library.py b/beets/library.py index 945594307..82393644c 100644 --- a/beets/library.py +++ b/beets/library.py @@ -420,16 +420,14 @@ class Item(LibModel): Can raise either a `ReadError` or a `WriteError`. """ try: - f = MediaFile(syspath(self.path)) + mediafile = MediaFile(syspath(self.path)) except (OSError, IOError) as exc: raise ReadError(self.path, exc) plugins.send('write', item=self) - for key in ITEM_KEYS_WRITABLE: - setattr(f, key, self[key]) try: - f.save(id3v23=beets.config['id3v23'].get(bool)) + mediafile.update(self, id3v23=beets.config['id3v23'].get(bool)) except (OSError, IOError, MutagenError) as exc: raise WriteError(self.path, exc) diff --git a/beets/mediafile.py b/beets/mediafile.py index 3ef7d16f8..c01c32424 100644 --- a/beets/mediafile.py +++ b/beets/mediafile.py @@ -1219,6 +1219,7 @@ class MediaFile(object): if self.mgfile.tags is None: self.mgfile.add_tags() + # FIXME Deprecated. Use `update` instead def save(self, id3v23=False): """Write the object's tags back to the file. @@ -1264,6 +1265,25 @@ class MediaFile(object): if isinstance(descriptor, MediaField): yield property + def update(self, dict, id3v23=False): + """Update tags from the dictionary and write them to the file. + + For any key in ``dict`` that is also a field to store tags the + method retrieves the corresponding value from ``dict`` and + updates the ``MediaFile``. If any of the tags are changed, these + changes are written to the disk. + + By default, MP3 files are saved with ID3v2.4 tags. You can use + the older ID3v2.3 standard by specifying the `id3v23` option. + """ + updated = False + for field in self.fields(): + if field in dict and getattr(self, field) != dict[field]: + updated = True + setattr(self, field, dict[field]) + if updated: + self.save(id3v23) + # Field definitions. diff --git a/test/test_mediafile.py b/test/test_mediafile.py index c710bde1f..cca299ed6 100644 --- a/test/test_mediafile.py +++ b/test/test_mediafile.py @@ -353,6 +353,15 @@ class ReadWriteTestBase(ArtTestMixin, GenreListTestMixin): mediafile = MediaFile(mediafile.path) self.assertTags(mediafile, tags) + def test_update_empty(self): + mediafile = self._mediafile_fixture('empty') + tags = self._generate_tags() + + mediafile.update(tags) + + mediafile = MediaFile(mediafile.path) + self.assertTags(mediafile, tags) + def test_overwrite_full(self): mediafile = self._mediafile_fixture('full') tags = self._generate_tags() @@ -369,6 +378,17 @@ class ReadWriteTestBase(ArtTestMixin, GenreListTestMixin): mediafile = MediaFile(mediafile.path) self.assertTags(mediafile, tags) + def test_update_full(self): + mediafile = self._mediafile_fixture('full') + tags = self._generate_tags() + + mediafile.update(tags) + # Make sure the tags are already set when writing a second time + mediafile.update(tags) + + mediafile = MediaFile(mediafile.path) + self.assertTags(mediafile, tags) + def test_write_date_components(self): mediafile = self._mediafile_fixture('full') mediafile.year = 2001 From bedad53c2779471e537fea9e5b9c850a5e76a5ca Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Wed, 2 Apr 2014 23:41:49 +0200 Subject: [PATCH 03/14] Test that we can migrate ITEM_KEYS_WRITABLE The test show that we can replace the hard-coded `ITEM_KEYS_WRITABLE` constant with the computed value derived from `MediaField.fields()` and ITEM_KEYS. This will be done in the next commit. --- test/test_mediafile.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/test_mediafile.py b/test/test_mediafile.py index cca299ed6..24fcd5c3b 100644 --- a/test/test_mediafile.py +++ b/test/test_mediafile.py @@ -24,6 +24,7 @@ import time import _common from _common import unittest from beets.mediafile import MediaFile, Image +from beets.library import ITEM_KEYS_WRITABLE, ITEM_KEYS class ArtTestMixin(object): @@ -725,6 +726,10 @@ class MediaFieldTest(unittest.TestCase): fields.extend(('encoder', 'images', 'genres', 'albumtype')) self.assertItemsEqual(MediaFile.fields(), fields) + def test_item_keys_writable_migration(self): + keys_writable = set(MediaFile.fields()).intersection(ITEM_KEYS) + self.assertItemsEqual(keys_writable, ITEM_KEYS_WRITABLE) + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From 5b6305b61028a4a564afcc928c3e35ebd8b7134f Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Wed, 2 Apr 2014 23:46:40 +0200 Subject: [PATCH 04/14] Compute ITEM_KEYS_WRITABLE See 65d64a7d50612df4ee6127ff05e591ae6fcceac0 why this is possible. --- beets/library.py | 126 +++++++++++++++++++++++------------------------ 1 file changed, 63 insertions(+), 63 deletions(-) diff --git a/beets/library.py b/beets/library.py index 82393644c..6a9610430 100644 --- a/beets/library.py +++ b/beets/library.py @@ -122,71 +122,71 @@ class PathType(types.Type): # - Is the field writable? # - Does the field reflect an attribute of a MediaFile? ITEM_FIELDS = [ - ('id', types.Id(), False, False), - ('path', PathType(), False, False), - ('album_id', types.Integer(), False, False), + ('id', types.Id(), False), + ('path', PathType(), False), + ('album_id', types.Integer(), False), - ('title', types.String(), True, True), - ('artist', types.String(), True, True), - ('artist_sort', types.String(), True, True), - ('artist_credit', types.String(), True, True), - ('album', types.String(), True, True), - ('albumartist', types.String(), True, True), - ('albumartist_sort', types.String(), True, True), - ('albumartist_credit', types.String(), True, True), - ('genre', types.String(), True, True), - ('composer', types.String(), True, True), - ('grouping', types.String(), True, True), - ('year', types.PaddedInt(4), True, True), - ('month', types.PaddedInt(2), True, True), - ('day', types.PaddedInt(2), True, True), - ('track', types.PaddedInt(2), True, True), - ('tracktotal', types.PaddedInt(2), True, True), - ('disc', types.PaddedInt(2), True, True), - ('disctotal', types.PaddedInt(2), True, True), - ('lyrics', types.String(), True, True), - ('comments', types.String(), True, True), - ('bpm', types.Integer(), True, True), - ('comp', types.Boolean(), True, True), - ('mb_trackid', types.String(), True, True), - ('mb_albumid', types.String(), True, True), - ('mb_artistid', types.String(), True, True), - ('mb_albumartistid', types.String(), True, True), - ('albumtype', types.String(), True, True), - ('label', types.String(), True, True), - ('acoustid_fingerprint', types.String(), True, True), - ('acoustid_id', types.String(), True, True), - ('mb_releasegroupid', types.String(), True, True), - ('asin', types.String(), True, True), - ('catalognum', types.String(), True, True), - ('script', types.String(), True, True), - ('language', types.String(), True, True), - ('country', types.String(), True, True), - ('albumstatus', types.String(), True, True), - ('media', types.String(), True, True), - ('albumdisambig', types.String(), True, True), - ('disctitle', types.String(), True, True), - ('encoder', types.String(), True, True), - ('rg_track_gain', types.Float(), True, True), - ('rg_track_peak', types.Float(), True, True), - ('rg_album_gain', types.Float(), True, True), - ('rg_album_peak', types.Float(), True, True), - ('original_year', types.PaddedInt(4), True, True), - ('original_month', types.PaddedInt(2), True, True), - ('original_day', types.PaddedInt(2), True, True), + ('title', types.String(), True), + ('artist', types.String(), True), + ('artist_sort', types.String(), True), + ('artist_credit', types.String(), True), + ('album', types.String(), True), + ('albumartist', types.String(), True), + ('albumartist_sort', types.String(), True), + ('albumartist_credit', types.String(), True), + ('genre', types.String(), True), + ('composer', types.String(), True), + ('grouping', types.String(), True), + ('year', types.PaddedInt(4), True), + ('month', types.PaddedInt(2), True), + ('day', types.PaddedInt(2), True), + ('track', types.PaddedInt(2), True), + ('tracktotal', types.PaddedInt(2), True), + ('disc', types.PaddedInt(2), True), + ('disctotal', types.PaddedInt(2), True), + ('lyrics', types.String(), True), + ('comments', types.String(), True), + ('bpm', types.Integer(), True), + ('comp', types.Boolean(), True), + ('mb_trackid', types.String(), True), + ('mb_albumid', types.String(), True), + ('mb_artistid', types.String(), True), + ('mb_albumartistid', types.String(), True), + ('albumtype', types.String(), True), + ('label', types.String(), True), + ('acoustid_fingerprint', types.String(), True), + ('acoustid_id', types.String(), True), + ('mb_releasegroupid', types.String(), True), + ('asin', types.String(), True), + ('catalognum', types.String(), True), + ('script', types.String(), True), + ('language', types.String(), True), + ('country', types.String(), True), + ('albumstatus', types.String(), True), + ('media', types.String(), True), + ('albumdisambig', types.String(), True), + ('disctitle', types.String(), True), + ('encoder', types.String(), True), + ('rg_track_gain', types.Float(), True), + ('rg_track_peak', types.Float(), True), + ('rg_album_gain', types.Float(), True), + ('rg_album_peak', types.Float(), True), + ('original_year', types.PaddedInt(4), True), + ('original_month', types.PaddedInt(2), True), + ('original_day', types.PaddedInt(2), True), - ('length', types.Float(), False, True), - ('bitrate', types.ScaledInt(1000, u'kbps'), False, True), - ('format', types.String(), False, True), - ('samplerate', types.ScaledInt(1000, u'kHz'), False, True), - ('bitdepth', types.Integer(), False, True), - ('channels', types.Integer(), False, True), - ('mtime', DateType(), False, False), - ('added', DateType(), False, False), + ('length', types.Float(), True), + ('bitrate', types.ScaledInt(1000, u'kbps'), True), + ('format', types.String(), True), + ('samplerate', types.ScaledInt(1000, u'kHz'), True), + ('bitdepth', types.Integer(), True), + ('channels', types.Integer(), True), + ('mtime', DateType(), False), + ('added', DateType(), False), ] -ITEM_KEYS_WRITABLE = [f[0] for f in ITEM_FIELDS if f[3] and f[2]] -ITEM_KEYS_META = [f[0] for f in ITEM_FIELDS if f[3]] +ITEM_KEYS_META = [f[0] for f in ITEM_FIELDS if f[2]] ITEM_KEYS = [f[0] for f in ITEM_FIELDS] +ITEM_KEYS_WRITABLE = set(MediaFile.fields()).intersection(ITEM_KEYS) # Database fields for the "albums" table. # The third entry in each tuple indicates whether the field reflects an @@ -328,7 +328,7 @@ class LibModel(dbcore.Model): class Item(LibModel): - _fields = dict((name, typ) for (name, typ, _, _) in ITEM_FIELDS) + _fields = dict((name, typ) for (name, typ, _) in ITEM_FIELDS) _table = 'items' _flex_table = 'item_attributes' _search_fields = ITEM_DEFAULT_FIELDS @@ -357,7 +357,7 @@ class Item(LibModel): elif isinstance(value, buffer): value = str(value) - if key in ITEM_KEYS_WRITABLE: + if key in MediaFile.fields(): self.mtime = 0 # Reset mtime on dirty. super(Item, self).__setitem__(key, value) From a2a8b244d70d611165270397cf8debe0892cc079 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Wed, 2 Apr 2014 23:54:34 +0200 Subject: [PATCH 05/14] Add LazySave tests for MediaFile --- test/test_mediafile.py | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/test/test_mediafile.py b/test/test_mediafile.py index 24fcd5c3b..630cd64e4 100644 --- a/test/test_mediafile.py +++ b/test/test_mediafile.py @@ -162,11 +162,11 @@ class ExtendedImageStructureTestMixin(ImageStructureTestMixin): desc='the composer', type=Image.TYPES.composer) -# TODO include this in ReadWriteTestBase if implemented class LazySaveTestMixin(object): """Mediafile should only write changes when tags have changed """ + @unittest.skip('not yet implemented') def test_unmodified(self): mediafile = self._mediafile_fixture('full') mtime = self._set_past_mtime(mediafile.path) @@ -175,6 +175,7 @@ class LazySaveTestMixin(object): mediafile.save() self.assertEqual(os.stat(mediafile.path).st_mtime, mtime) + @unittest.skip('not yet implemented') def test_same_tag_value(self): mediafile = self._mediafile_fixture('full') mtime = self._set_past_mtime(mediafile.path) @@ -184,6 +185,15 @@ class LazySaveTestMixin(object): mediafile.save() self.assertEqual(os.stat(mediafile.path).st_mtime, mtime) + def test_update_same_tag_value(self): + mediafile = self._mediafile_fixture('full') + mtime = self._set_past_mtime(mediafile.path) + self.assertEqual(os.stat(mediafile.path).st_mtime, mtime) + + mediafile.update({'title': mediafile.title}) + self.assertEqual(os.stat(mediafile.path).st_mtime, mtime) + + @unittest.skip('not yet implemented') def test_tag_value_change(self): mediafile = self._mediafile_fixture('full') mtime = self._set_past_mtime(mediafile.path) @@ -194,6 +204,14 @@ class LazySaveTestMixin(object): mediafile.save() self.assertNotEqual(os.stat(mediafile.path).st_mtime, mtime) + def test_update_changed_tag_value(self): + mediafile = self._mediafile_fixture('full') + mtime = self._set_past_mtime(mediafile.path) + self.assertEqual(os.stat(mediafile.path).st_mtime, mtime) + + mediafile.update({'title': mediafile.title, 'album': 'another'}) + self.assertNotEqual(os.stat(mediafile.path).st_mtime, mtime) + def _set_past_mtime(self, path): mtime = round(time.time() - 10000) os.utime(path, (mtime, mtime)) @@ -234,7 +252,7 @@ class GenreListTestMixin(object): self.assertItemsEqual(mediafile.genres, [u'the genre', u'another']) -class ReadWriteTestBase(ArtTestMixin, GenreListTestMixin): +class ReadWriteTestBase(ArtTestMixin, GenreListTestMixin, LazySaveTestMixin): """Test writing and reading tags. Subclasses must set ``extension`` and ``audio_properties``. """ From 863b9fb4afdf97b77cd3c96a2d46badefd930dd2 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Thu, 3 Apr 2014 00:44:57 +0200 Subject: [PATCH 06/14] MediaFile can be extended with custom fields --- beets/mediafile.py | 19 +++++++++++++++ test/test_mediafile.py | 55 ++++++++++++++++++++++++++++++++++++------ 2 files changed, 67 insertions(+), 7 deletions(-) diff --git a/beets/mediafile.py b/beets/mediafile.py index c01c32424..874a211b9 100644 --- a/beets/mediafile.py +++ b/beets/mediafile.py @@ -1265,6 +1265,25 @@ class MediaFile(object): if isinstance(descriptor, MediaField): yield property + @classmethod + def add_field(cls, name, descriptor): + """Add a field to store custom tags. + + ``name`` is the name of the property the field is accessed + through. It must not already exist for the class. If the name + coincides with the name of a property of ``Item`` it will be set + from the item in ``item.write()``. + + ``descriptor`` must be an instance of ``MediaField``. + """ + if not isinstance(descriptor, MediaField): + raise ValueError( + u'{} must be an instance of MediaField'.format(descriptor)) + if name in cls.__dict__: + raise ValueError( + u'property "{}" already exists on MediaField'.format(name)) + setattr(cls, name, descriptor) + def update(self, dict, id3v23=False): """Update tags from the dictionary and write them to the file. diff --git a/test/test_mediafile.py b/test/test_mediafile.py index 630cd64e4..441d867ff 100644 --- a/test/test_mediafile.py +++ b/test/test_mediafile.py @@ -23,8 +23,10 @@ import time import _common from _common import unittest -from beets.mediafile import MediaFile, Image -from beets.library import ITEM_KEYS_WRITABLE, ITEM_KEYS +from beets.mediafile import MediaFile, MediaField, Image, \ + MP3StorageStyle, StorageStyle, \ + MP4StorageStyle, ASFStorageStyle +from beets.library import ITEM_KEYS_WRITABLE, ITEM_KEYS, Item class ArtTestMixin(object): @@ -252,7 +254,50 @@ class GenreListTestMixin(object): self.assertItemsEqual(mediafile.genres, [u'the genre', u'another']) -class ReadWriteTestBase(ArtTestMixin, GenreListTestMixin, LazySaveTestMixin): +field_extension = MediaField( + MP3StorageStyle('TKEY'), + MP4StorageStyle('----:com.apple.iTunes:initialkey'), + StorageStyle('INITIALKEY'), + ASFStorageStyle('INITIALKEY'), +) +class ExtendedFieldTestMixin(object): + + def test_extended_field_write(self): + MediaFile.add_field('initialkey', field_extension) + + mediafile = self._mediafile_fixture('empty') + mediafile.initialkey = 'F#' + mediafile.save() + + mediafile = MediaFile(mediafile.path) + self.assertEqual(mediafile.initialkey, 'F#') + delattr(MediaFile, 'initialkey') + + def test_extended_attribute_from_item(self): + MediaFile.add_field('initialkey', field_extension) + + mediafile = self._mediafile_fixture('empty') + self.assertEqual(mediafile.initialkey, '') + + item = Item(path=mediafile.path, initialkey='Gb') + item.write() + mediafile = MediaFile(mediafile.path) + self.assertEqual(mediafile.initialkey, 'Gb') + delattr(MediaFile, 'initialkey') + + def test_invalid_descriptor(self): + with self.assertRaises(ValueError) as cm: + MediaFile.add_field('somekey', True) + self.assertIn('must be an instance of MediaField', str(cm.exception)) + + def test_overwrite_property(self): + with self.assertRaises(ValueError) as cm: + MediaFile.add_field('artist', MediaField()) + self.assertIn('property "artist" already exists', str(cm.exception)) + + +class ReadWriteTestBase(ArtTestMixin, GenreListTestMixin, + LazySaveTestMixin, ExtendedFieldTestMixin): """Test writing and reading tags. Subclasses must set ``extension`` and ``audio_properties``. """ @@ -744,10 +789,6 @@ class MediaFieldTest(unittest.TestCase): fields.extend(('encoder', 'images', 'genres', 'albumtype')) self.assertItemsEqual(MediaFile.fields(), fields) - def test_item_keys_writable_migration(self): - keys_writable = set(MediaFile.fields()).intersection(ITEM_KEYS) - self.assertItemsEqual(keys_writable, ITEM_KEYS_WRITABLE) - def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From 9474686d6828af9cc7360481d735e75f215dacf8 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Thu, 3 Apr 2014 01:21:16 +0200 Subject: [PATCH 07/14] Use indexed format with python26 --- beets/mediafile.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beets/mediafile.py b/beets/mediafile.py index 874a211b9..bdffa8da9 100644 --- a/beets/mediafile.py +++ b/beets/mediafile.py @@ -1278,10 +1278,10 @@ class MediaFile(object): """ if not isinstance(descriptor, MediaField): raise ValueError( - u'{} must be an instance of MediaField'.format(descriptor)) + u'{0} must be an instance of MediaField'.format(descriptor)) if name in cls.__dict__: raise ValueError( - u'property "{}" already exists on MediaField'.format(name)) + u'property "{0}" already exists on MediaField'.format(name)) setattr(cls, name, descriptor) def update(self, dict, id3v23=False): From 43ae730a6a3347552f20b1099fc605b40dd3b934 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Thu, 3 Apr 2014 13:46:33 +0200 Subject: [PATCH 08/14] Use readable_fields() to replace ITEM_KEYS_META --- beets/mediafile.py | 11 +++++++++++ test/test_mediafile.py | 18 +++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/beets/mediafile.py b/beets/mediafile.py index bdffa8da9..3f207ef20 100644 --- a/beets/mediafile.py +++ b/beets/mediafile.py @@ -1265,6 +1265,17 @@ class MediaFile(object): if isinstance(descriptor, MediaField): yield property + @classmethod + def readable_fields(cls): + """Yield the elements of ``fields()`` and all additional + properties retrieved from the file + """ + for property in cls.fields(): + yield property + for property in ['length', 'samplerate', 'bitdepth', 'bitrate', + 'channels', 'format']: + yield property + @classmethod def add_field(cls, name, descriptor): """Add a field to store custom tags. diff --git a/test/test_mediafile.py b/test/test_mediafile.py index 441d867ff..acf37db02 100644 --- a/test/test_mediafile.py +++ b/test/test_mediafile.py @@ -26,7 +26,7 @@ from _common import unittest from beets.mediafile import MediaFile, MediaField, Image, \ MP3StorageStyle, StorageStyle, \ MP4StorageStyle, ASFStorageStyle -from beets.library import ITEM_KEYS_WRITABLE, ITEM_KEYS, Item +from beets.library import ITEM_KEYS_META, ITEM_KEYS, Item class ArtTestMixin(object): @@ -784,11 +784,27 @@ class MediaFieldTest(unittest.TestCase): for field in MediaFile.fields(): self.assertTrue(hasattr(mediafile, field)) + def test_properties_from_readable_fields(self): + path = os.path.join(_common.RSRC, 'full.mp3') + mediafile = MediaFile(path) + for field in MediaFile.readable_fields(): + self.assertTrue(hasattr(mediafile, field)) + def test_known_fields(self): fields = ReadWriteTestBase.empty_tags.keys() fields.extend(('encoder', 'images', 'genres', 'albumtype')) self.assertItemsEqual(MediaFile.fields(), fields) + def test_fields_in_readable_fields(self): + readable = MediaFile.readable_fields() + for field in MediaFile.fields(): + self.assertIn(field, readable) + + def test_readable_fields_are_item_meta_keys(self): + readable = MediaFile.readable_fields() + meta_keys = set(readable).intersection(ITEM_KEYS) + self.assertItemsEqual(meta_keys, ITEM_KEYS_META) + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From b262edd9726104a0b4ad32df57d739a66d07ea4d Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Thu, 3 Apr 2014 13:51:57 +0200 Subject: [PATCH 09/14] Migrate ITEM_KEYS_META --- beets/library.py | 122 ++++++++++++++++++++--------------------- test/test_mediafile.py | 7 +-- 2 files changed, 62 insertions(+), 67 deletions(-) diff --git a/beets/library.py b/beets/library.py index 6a9610430..33087de87 100644 --- a/beets/library.py +++ b/beets/library.py @@ -122,71 +122,71 @@ class PathType(types.Type): # - Is the field writable? # - Does the field reflect an attribute of a MediaFile? ITEM_FIELDS = [ - ('id', types.Id(), False), - ('path', PathType(), False), - ('album_id', types.Integer(), False), + ('id', types.Id()), + ('path', PathType()), + ('album_id', types.Integer()), - ('title', types.String(), True), - ('artist', types.String(), True), - ('artist_sort', types.String(), True), - ('artist_credit', types.String(), True), - ('album', types.String(), True), - ('albumartist', types.String(), True), - ('albumartist_sort', types.String(), True), - ('albumartist_credit', types.String(), True), - ('genre', types.String(), True), - ('composer', types.String(), True), - ('grouping', types.String(), True), - ('year', types.PaddedInt(4), True), - ('month', types.PaddedInt(2), True), - ('day', types.PaddedInt(2), True), - ('track', types.PaddedInt(2), True), - ('tracktotal', types.PaddedInt(2), True), - ('disc', types.PaddedInt(2), True), - ('disctotal', types.PaddedInt(2), True), - ('lyrics', types.String(), True), - ('comments', types.String(), True), - ('bpm', types.Integer(), True), - ('comp', types.Boolean(), True), - ('mb_trackid', types.String(), True), - ('mb_albumid', types.String(), True), - ('mb_artistid', types.String(), True), - ('mb_albumartistid', types.String(), True), - ('albumtype', types.String(), True), - ('label', types.String(), True), - ('acoustid_fingerprint', types.String(), True), - ('acoustid_id', types.String(), True), - ('mb_releasegroupid', types.String(), True), - ('asin', types.String(), True), - ('catalognum', types.String(), True), - ('script', types.String(), True), - ('language', types.String(), True), - ('country', types.String(), True), - ('albumstatus', types.String(), True), - ('media', types.String(), True), - ('albumdisambig', types.String(), True), - ('disctitle', types.String(), True), - ('encoder', types.String(), True), - ('rg_track_gain', types.Float(), True), - ('rg_track_peak', types.Float(), True), - ('rg_album_gain', types.Float(), True), - ('rg_album_peak', types.Float(), True), - ('original_year', types.PaddedInt(4), True), - ('original_month', types.PaddedInt(2), True), - ('original_day', types.PaddedInt(2), True), + ('title', types.String()), + ('artist', types.String()), + ('artist_sort', types.String()), + ('artist_credit', types.String()), + ('album', types.String()), + ('albumartist', types.String()), + ('albumartist_sort', types.String()), + ('albumartist_credit', types.String()), + ('genre', types.String()), + ('composer', types.String()), + ('grouping', types.String()), + ('year', types.PaddedInt(4)), + ('month', types.PaddedInt(2)), + ('day', types.PaddedInt(2)), + ('track', types.PaddedInt(2)), + ('tracktotal', types.PaddedInt(2)), + ('disc', types.PaddedInt(2)), + ('disctotal', types.PaddedInt(2)), + ('lyrics', types.String()), + ('comments', types.String()), + ('bpm', types.Integer()), + ('comp', types.Boolean()), + ('mb_trackid', types.String()), + ('mb_albumid', types.String()), + ('mb_artistid', types.String()), + ('mb_albumartistid', types.String()), + ('albumtype', types.String()), + ('label', types.String()), + ('acoustid_fingerprint', types.String()), + ('acoustid_id', types.String()), + ('mb_releasegroupid', types.String()), + ('asin', types.String()), + ('catalognum', types.String()), + ('script', types.String()), + ('language', types.String()), + ('country', types.String()), + ('albumstatus', types.String()), + ('media', types.String()), + ('albumdisambig', types.String()), + ('disctitle', types.String()), + ('encoder', types.String()), + ('rg_track_gain', types.Float()), + ('rg_track_peak', types.Float()), + ('rg_album_gain', types.Float()), + ('rg_album_peak', types.Float()), + ('original_year', types.PaddedInt(4)), + ('original_month', types.PaddedInt(2)), + ('original_day', types.PaddedInt(2)), - ('length', types.Float(), True), - ('bitrate', types.ScaledInt(1000, u'kbps'), True), - ('format', types.String(), True), - ('samplerate', types.ScaledInt(1000, u'kHz'), True), - ('bitdepth', types.Integer(), True), - ('channels', types.Integer(), True), - ('mtime', DateType(), False), - ('added', DateType(), False), + ('length', types.Float()), + ('bitrate', types.ScaledInt(1000, u'kbps')), + ('format', types.String()), + ('samplerate', types.ScaledInt(1000, u'kHz')), + ('bitdepth', types.Integer()), + ('channels', types.Integer()), + ('mtime', DateType()), + ('added', DateType()), ] -ITEM_KEYS_META = [f[0] for f in ITEM_FIELDS if f[2]] ITEM_KEYS = [f[0] for f in ITEM_FIELDS] ITEM_KEYS_WRITABLE = set(MediaFile.fields()).intersection(ITEM_KEYS) +ITEM_KEYS_META = set(MediaFile.readable_fields()).intersection(ITEM_KEYS) # Database fields for the "albums" table. # The third entry in each tuple indicates whether the field reflects an @@ -328,7 +328,7 @@ class LibModel(dbcore.Model): class Item(LibModel): - _fields = dict((name, typ) for (name, typ, _) in ITEM_FIELDS) + _fields = dict((name, typ) for (name, typ) in ITEM_FIELDS) _table = 'items' _flex_table = 'item_attributes' _search_fields = ITEM_DEFAULT_FIELDS diff --git a/test/test_mediafile.py b/test/test_mediafile.py index acf37db02..5aa669bd5 100644 --- a/test/test_mediafile.py +++ b/test/test_mediafile.py @@ -26,7 +26,7 @@ from _common import unittest from beets.mediafile import MediaFile, MediaField, Image, \ MP3StorageStyle, StorageStyle, \ MP4StorageStyle, ASFStorageStyle -from beets.library import ITEM_KEYS_META, ITEM_KEYS, Item +from beets.library import Item class ArtTestMixin(object): @@ -800,11 +800,6 @@ class MediaFieldTest(unittest.TestCase): for field in MediaFile.fields(): self.assertIn(field, readable) - def test_readable_fields_are_item_meta_keys(self): - readable = MediaFile.readable_fields() - meta_keys = set(readable).intersection(ITEM_KEYS) - self.assertItemsEqual(meta_keys, ITEM_KEYS_META) - def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From c4f0928bf5f2ef7a9580e029cad48c13650da64f Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Thu, 3 Apr 2014 14:00:11 +0200 Subject: [PATCH 10/14] Read custom fields into database --- beets/library.py | 2 +- beets/mediafile.py | 3 +++ test/test_mediafile.py | 14 ++++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/beets/library.py b/beets/library.py index 33087de87..789b4c72f 100644 --- a/beets/library.py +++ b/beets/library.py @@ -397,7 +397,7 @@ class Item(LibModel): except (OSError, IOError) as exc: raise ReadError(read_path, exc) - for key in ITEM_KEYS_META: + for key in list(ITEM_KEYS_META) + MediaFile.custom_fields: value = getattr(f, key) if isinstance(value, (int, long)): # Filter values wider than 64 bits (in signed diff --git a/beets/mediafile.py b/beets/mediafile.py index 3f207ef20..7c47e54f2 100644 --- a/beets/mediafile.py +++ b/beets/mediafile.py @@ -1276,6 +1276,8 @@ class MediaFile(object): 'channels', 'format']: yield property + custom_fields = [] + @classmethod def add_field(cls, name, descriptor): """Add a field to store custom tags. @@ -1293,6 +1295,7 @@ class MediaFile(object): if name in cls.__dict__: raise ValueError( u'property "{0}" already exists on MediaField'.format(name)) + cls.custom_fields.append(name) setattr(cls, name, descriptor) def update(self, dict, id3v23=False): diff --git a/test/test_mediafile.py b/test/test_mediafile.py index 5aa669bd5..4334f3c7e 100644 --- a/test/test_mediafile.py +++ b/test/test_mediafile.py @@ -272,6 +272,7 @@ class ExtendedFieldTestMixin(object): mediafile = MediaFile(mediafile.path) self.assertEqual(mediafile.initialkey, 'F#') delattr(MediaFile, 'initialkey') + MediaFile.custom_fields = [] def test_extended_attribute_from_item(self): MediaFile.add_field('initialkey', field_extension) @@ -284,6 +285,19 @@ class ExtendedFieldTestMixin(object): mediafile = MediaFile(mediafile.path) self.assertEqual(mediafile.initialkey, 'Gb') delattr(MediaFile, 'initialkey') + MediaFile.custom_fields = [] + + def test_flexible_attribute_from_file(self): + MediaFile.add_field('initialkey', field_extension) + + mediafile = self._mediafile_fixture('empty') + mediafile.update({'initialkey': 'F#'}) + + item = Item.from_path(mediafile.path) + self.assertEqual(item['initialkey'], 'F#') + + delattr(MediaFile, 'initialkey') + MediaFile.custom_fields = [] def test_invalid_descriptor(self): with self.assertRaises(ValueError) as cm: From 3c7dd13b72d0114aa791dfc49d2d004fcb2eaaa9 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Thu, 3 Apr 2014 23:35:33 +0200 Subject: [PATCH 11/14] Add Item.media_fields This new property controls which fields to read from a media file. --- beets/library.py | 31 +++++++++++++++++++++---------- beets/mediafile.py | 3 --- beets/ui/commands.py | 2 +- test/test_mediafile.py | 12 ++++++------ 4 files changed, 28 insertions(+), 20 deletions(-) diff --git a/beets/library.py b/beets/library.py index 789b4c72f..d040e1c18 100644 --- a/beets/library.py +++ b/beets/library.py @@ -186,7 +186,7 @@ ITEM_FIELDS = [ ] ITEM_KEYS = [f[0] for f in ITEM_FIELDS] ITEM_KEYS_WRITABLE = set(MediaFile.fields()).intersection(ITEM_KEYS) -ITEM_KEYS_META = set(MediaFile.readable_fields()).intersection(ITEM_KEYS) + # Database fields for the "albums" table. # The third entry in each tuple indicates whether the field reflects an @@ -333,6 +333,13 @@ class Item(LibModel): _flex_table = 'item_attributes' _search_fields = ITEM_DEFAULT_FIELDS + media_fields = set(MediaFile.readable_fields()).intersection(ITEM_KEYS) + """Set of property names to read from ``MediaFile``. + + ``item.read()`` will read all properties in this set from + ``MediaFile`` and set them on the item. + """ + @classmethod def _getters(cls): return plugins.item_field_getters() @@ -383,8 +390,11 @@ class Item(LibModel): # Interaction with file metadata. def read(self, read_path=None): - """Read the metadata from the associated file. If read_path is - specified, read metadata from that file instead. + """Read the metadata from the associated file. + + If ``read_path`` is specified, read metadata from that file + instead. Updates all the properties in ``Item.media_fields`` + from the media file. Raises a `ReadError` if the file could not be read. """ @@ -393,20 +403,19 @@ class Item(LibModel): else: read_path = normpath(read_path) try: - f = MediaFile(syspath(read_path)) + mediafile = MediaFile(syspath(read_path)) except (OSError, IOError) as exc: raise ReadError(read_path, exc) - for key in list(ITEM_KEYS_META) + MediaFile.custom_fields: - value = getattr(f, key) + for key in list(self.media_fields): + value = getattr(mediafile, key) if isinstance(value, (int, long)): - # Filter values wider than 64 bits (in signed - # representation). SQLite cannot store them. - # py26: Post transition, we can use: + # Filter values wider than 64 bits (in signed representation). + # SQLite cannot store them. py26: Post transition, we can use: # value.bit_length() > 63 if abs(value) >= 2 ** 63: value = 0 - setattr(self, key, value) + self[key] = value # Database's mtime should now reflect the on-disk value. if read_path == self.path: @@ -417,6 +426,8 @@ class Item(LibModel): def write(self): """Write the item's metadata to the associated file. + Updates the mediafile with properties from itself. + Can raise either a `ReadError` or a `WriteError`. """ try: diff --git a/beets/mediafile.py b/beets/mediafile.py index 7c47e54f2..3f207ef20 100644 --- a/beets/mediafile.py +++ b/beets/mediafile.py @@ -1276,8 +1276,6 @@ class MediaFile(object): 'channels', 'format']: yield property - custom_fields = [] - @classmethod def add_field(cls, name, descriptor): """Add a field to store custom tags. @@ -1295,7 +1293,6 @@ class MediaFile(object): if name in cls.__dict__: raise ValueError( u'property "{0}" already exists on MediaField'.format(name)) - cls.custom_fields.append(name) setattr(cls, name, descriptor) def update(self, dict, id3v23=False): diff --git a/beets/ui/commands.py b/beets/ui/commands.py index d45192be1..024e3a3b7 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -923,7 +923,7 @@ def update_items(lib, query, album, move, pretend): # Check for and display changes. changed = ui.show_model_changes(item, - fields=library.ITEM_KEYS_META) + fields=library.Item.media_fields) # Save changes. if not pretend: diff --git a/test/test_mediafile.py b/test/test_mediafile.py index 4334f3c7e..4f8a1b14c 100644 --- a/test/test_mediafile.py +++ b/test/test_mediafile.py @@ -272,9 +272,8 @@ class ExtendedFieldTestMixin(object): mediafile = MediaFile(mediafile.path) self.assertEqual(mediafile.initialkey, 'F#') delattr(MediaFile, 'initialkey') - MediaFile.custom_fields = [] - def test_extended_attribute_from_item(self): + def test_write_extended_tag_from_item(self): MediaFile.add_field('initialkey', field_extension) mediafile = self._mediafile_fixture('empty') @@ -284,11 +283,12 @@ class ExtendedFieldTestMixin(object): item.write() mediafile = MediaFile(mediafile.path) self.assertEqual(mediafile.initialkey, 'Gb') - delattr(MediaFile, 'initialkey') - MediaFile.custom_fields = [] - def test_flexible_attribute_from_file(self): + delattr(MediaFile, 'initialkey') + + def test_read_flexible_attribute_from_file(self): MediaFile.add_field('initialkey', field_extension) + Item.media_fields.add('initialkey') mediafile = self._mediafile_fixture('empty') mediafile.update({'initialkey': 'F#'}) @@ -297,7 +297,7 @@ class ExtendedFieldTestMixin(object): self.assertEqual(item['initialkey'], 'F#') delattr(MediaFile, 'initialkey') - MediaFile.custom_fields = [] + Item.media_fields.remove('initialkey') def test_invalid_descriptor(self): with self.assertRaises(ValueError) as cm: From 4b1a1e3d65a03d45d61852107b6483e2eddc5a50 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Thu, 3 Apr 2014 23:40:07 +0200 Subject: [PATCH 12/14] Remove ITEM_KEYS_WRITABLE --- beets/library.py | 1 - beets/ui/commands.py | 2 +- beetsplug/bpd/__init__.py | 4 +++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/beets/library.py b/beets/library.py index d040e1c18..5b5eed4e4 100644 --- a/beets/library.py +++ b/beets/library.py @@ -185,7 +185,6 @@ ITEM_FIELDS = [ ('added', DateType()), ] ITEM_KEYS = [f[0] for f in ITEM_FIELDS] -ITEM_KEYS_WRITABLE = set(MediaFile.fields()).intersection(ITEM_KEYS) # Database fields for the "albums" table. diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 024e3a3b7..15f3e9601 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -1250,7 +1250,7 @@ def write_items(lib, query, pretend): # Check for and display changes. changed = ui.show_model_changes(item, clean_item, - library.ITEM_KEYS_WRITABLE, + MediaFile.fields(), always=True) if changed and not pretend: try: diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py index 9306c1645..13eb6fb97 100644 --- a/beetsplug/bpd/__init__.py +++ b/beetsplug/bpd/__init__.py @@ -30,8 +30,9 @@ from beets.plugins import BeetsPlugin import beets.ui from beets import vfs from beets.util import bluelet -from beets.library import ITEM_KEYS_WRITABLE +from beets.library import ITEM_KEYS from beets import dbcore +from beets.mediafile import MediaFile PROTOCOL_VERSION = '0.13.0' BUFSIZE = 1024 @@ -67,6 +68,7 @@ SAFE_COMMANDS = ( u'close', u'commands', u'notcommands', u'password', u'ping', ) +ITEM_KEYS_WRITABLE = set(MediaFile.fields()).intersection(ITEM_KEYS) # Loggers. log = logging.getLogger('beets.bpd') From eb4c323bcb794331a31eab97e5f762fff002290f Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Fri, 4 Apr 2014 00:48:29 +0200 Subject: [PATCH 13/14] Plugins can now extend MediaField --- beets/mediafile.py | 12 +++++----- beets/plugins.py | 14 ++++++++++++ docs/dev/index.rst | 1 + docs/dev/media_file.rst | 21 ++++++++++++++++++ docs/dev/plugins.rst | 49 ++++++++++++++++++++--------------------- test/test_mediafile.py | 13 +++++++---- 6 files changed, 76 insertions(+), 34 deletions(-) create mode 100644 docs/dev/media_file.rst diff --git a/beets/mediafile.py b/beets/mediafile.py index 3f207ef20..733eaec32 100644 --- a/beets/mediafile.py +++ b/beets/mediafile.py @@ -913,12 +913,14 @@ class MediaField(object): def __init__(self, *styles, **kwargs): """Creates a new MediaField. - - `styles`: `StorageStyle` instances that describe the strategy - for reading and writing the field in particular formats. - There must be at least one style for each possible file - format. + - `styles`: `StorageStyle` instances that describe the strategy + for reading and writing the field in particular formats. + There must be at least one style for each possible file + format. + - `out_type`: the type of the value that should be returned when - getting this property. + getting this property. + """ self.out_type = kwargs.get('out_type', unicode) self._styles = styles diff --git a/beets/plugins.py b/beets/plugins.py index 6a58777cd..6b177432f 100755 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -106,6 +106,20 @@ class BeetsPlugin(object): """ return None + def add_media_field(self, name, descriptor): + """Add a field that is synchronized between media files and items. + + When a media field is added ``item.write()`` will set the name + property of the item's MediaFile to ``item[name]`` and save the + changes. Similarly ``item.read()`` will set ``item[name]`` to + the value of the name property of the media file. + + ``descriptor`` must be an instance of ``mediafile.MediaField``. + """ + # Defer impor to prevent circular dependency + from beets import library + mediafile.MediaFile.add_field(name, descriptor) + library.Item.media_fields.add(name) listeners = None diff --git a/docs/dev/index.rst b/docs/dev/index.rst index 2579ecbf4..82651a781 100644 --- a/docs/dev/index.rst +++ b/docs/dev/index.rst @@ -8,3 +8,4 @@ in hacking beets itself or creating plugins for it. plugins api + media_file diff --git a/docs/dev/media_file.rst b/docs/dev/media_file.rst new file mode 100644 index 000000000..c703377d8 --- /dev/null +++ b/docs/dev/media_file.rst @@ -0,0 +1,21 @@ +.. _mediafile: + +MediaFile +--------- + +.. currentmodule:: beets.mediafile + +.. autoclass:: MediaFile + + .. automethod:: __init__ + .. automethod:: fields + .. automethod:: readable_fields + .. automethod:: save + .. automethod:: update + +.. autoclass:: MediaField + + .. automethod:: __init__ + +.. autoclass:: StorageStyle + :members: diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index cdea8938a..cda1ca29e 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -300,38 +300,37 @@ template fields by adding a function accepting an ``Album`` argument to the Extend MediaFile ^^^^^^^^^^^^^^^^ -`MediaFile`_ is the file tag abstraction layer that beets uses to make +:ref:`MediaFile` is the file tag abstraction layer that beets uses to make cross-format metadata manipulation simple. Plugins can add fields to MediaFile to extend the kinds of metadata that they can easily manage. -The ``item_fields`` method on plugins should be overridden to return a -dictionary whose keys are field names and whose values are descriptor objects -that provide the field in question. The descriptors should probably be -``MediaField`` instances (defined in ``beets.mediafile``). Here's an example -plugin that provides a meaningless new field "foo":: +The ``MediaFile`` class uses ``MediaField`` descriptors to provide +access to file tags. Have a look at the ``beets.mediafile`` source code +to learn how to use this descriptor class. If you have created a +descriptor you can add it through your plugins ``add_media_field()`` +method. - from beets import mediafile, plugins, ui - class FooPlugin(plugins.BeetsPlugin): - def item_fields(self): - return { - 'foo': mediafile.MediaField( - mp3 = mediafile.StorageStyle( - 'TXXX', id3_desc=u'Foo Field'), - mp4 = mediafile.StorageStyle( - '----:com.apple.iTunes:Foo Field'), - etc = mediafile.StorageStyle('FOO FIELD') - ), - } +.. automethod:: beets.plugins.BeetsPlugin.add_media_field -Later, the plugin can manipulate this new field by saying something like -``mf.foo = 'bar'`` where ``mf`` is a ``MediaFile`` instance. -Note that, currently, these additional fields are *only* applied to -``MediaFile`` itself. The beets library database schema and the ``Item`` class -are not extended, so the fields are second-class citizens. This may change -eventually. +Here's an example plugin that provides a meaningless new field "foo":: + + class FooPlugin(BeetsPlugin): + def __init__(self): + field = mediafile.MediaField( + mediafile.MP3DescStorageStyle(u'foo') + mediafile.StorageStyle(u'foo') + ) + self.add_media_field('foo', field) + + FooPlugin() + item = Item.from_path('/path/to/foo/tag.mp3') + assert item['foo'] == 'spam' + + item['foo'] == 'ham' + item.write() + # The "foo" tag of the file is now "ham" -.. _MediaFile: https://github.com/sampsyo/beets/wiki/MediaFile Add Import Pipeline Stages ^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/test/test_mediafile.py b/test/test_mediafile.py index 4f8a1b14c..99fbd4cc4 100644 --- a/test/test_mediafile.py +++ b/test/test_mediafile.py @@ -27,6 +27,7 @@ from beets.mediafile import MediaFile, MediaField, Image, \ MP3StorageStyle, StorageStyle, \ MP4StorageStyle, ASFStorageStyle from beets.library import Item +from beets.plugins import BeetsPlugin class ArtTestMixin(object): @@ -263,7 +264,8 @@ field_extension = MediaField( class ExtendedFieldTestMixin(object): def test_extended_field_write(self): - MediaFile.add_field('initialkey', field_extension) + plugin = BeetsPlugin() + plugin.add_media_field('initialkey', field_extension) mediafile = self._mediafile_fixture('empty') mediafile.initialkey = 'F#' @@ -272,9 +274,11 @@ class ExtendedFieldTestMixin(object): mediafile = MediaFile(mediafile.path) self.assertEqual(mediafile.initialkey, 'F#') delattr(MediaFile, 'initialkey') + Item.media_fields.remove('initialkey') def test_write_extended_tag_from_item(self): - MediaFile.add_field('initialkey', field_extension) + plugin = BeetsPlugin() + plugin.add_media_field('initialkey', field_extension) mediafile = self._mediafile_fixture('empty') self.assertEqual(mediafile.initialkey, '') @@ -285,10 +289,11 @@ class ExtendedFieldTestMixin(object): self.assertEqual(mediafile.initialkey, 'Gb') delattr(MediaFile, 'initialkey') + Item.media_fields.remove('initialkey') def test_read_flexible_attribute_from_file(self): - MediaFile.add_field('initialkey', field_extension) - Item.media_fields.add('initialkey') + plugin = BeetsPlugin() + plugin.add_media_field('initialkey', field_extension) mediafile = self._mediafile_fixture('empty') mediafile.update({'initialkey': 'F#'}) From e62d36aa69d1b0a942d9d42cb29fa281aeb7f522 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Fri, 4 Apr 2014 13:40:10 +0200 Subject: [PATCH 14/14] Remove lazy update from MediaFile --- beets/mediafile.py | 12 ++++-------- test/test_mediafile.py | 2 +- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/beets/mediafile.py b/beets/mediafile.py index 733eaec32..c5e5efefa 100644 --- a/beets/mediafile.py +++ b/beets/mediafile.py @@ -1221,7 +1221,6 @@ class MediaFile(object): if self.mgfile.tags is None: self.mgfile.add_tags() - # FIXME Deprecated. Use `update` instead def save(self, id3v23=False): """Write the object's tags back to the file. @@ -1302,19 +1301,16 @@ class MediaFile(object): For any key in ``dict`` that is also a field to store tags the method retrieves the corresponding value from ``dict`` and - updates the ``MediaFile``. If any of the tags are changed, these - changes are written to the disk. + updates the ``MediaFile``. The changes are then written to the + disk. By default, MP3 files are saved with ID3v2.4 tags. You can use the older ID3v2.3 standard by specifying the `id3v23` option. """ - updated = False for field in self.fields(): - if field in dict and getattr(self, field) != dict[field]: - updated = True + if field in dict: setattr(self, field, dict[field]) - if updated: - self.save(id3v23) + self.save(id3v23) # Field definitions. diff --git a/test/test_mediafile.py b/test/test_mediafile.py index 99fbd4cc4..bcf80ccbe 100644 --- a/test/test_mediafile.py +++ b/test/test_mediafile.py @@ -316,7 +316,7 @@ class ExtendedFieldTestMixin(object): class ReadWriteTestBase(ArtTestMixin, GenreListTestMixin, - LazySaveTestMixin, ExtendedFieldTestMixin): + ExtendedFieldTestMixin): """Test writing and reading tags. Subclasses must set ``extension`` and ``audio_properties``. """