From 9cbbc35a95c97efda8ec09679677ad5aafce3218 Mon Sep 17 00:00:00 2001 From: Bert Besser Date: Thu, 6 May 2021 19:55:12 +0200 Subject: [PATCH 1/4] persist set_fields to media files --- beets/importer.py | 47 ++++++++++++++++++++++----------------- docs/changelog.rst | 2 ++ docs/reference/config.rst | 3 +++ test/test_importer.py | 12 +++++++++- 4 files changed, 42 insertions(+), 22 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index b7bfdb156..f28e6822d 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -572,18 +572,22 @@ class ImportTask(BaseImportTask): util.prune_dirs(os.path.dirname(item.path), lib.directory) - def set_fields(self): + def set_fields(self, lib): """Sets the fields given at CLI or configuration to the specified - values. + values, for both the album and all its items. """ - for field, view in config['import']['set_fields'].items(): - value = view.get() - log.debug(u'Set field {1}={2} for {0}', - displayable_path(self.paths), - field, - value) - self.album[field] = value - self.album.store() + with lib.transaction(): + for field, view in config['import']['set_fields'].items(): + value = view.get() + log.debug(u'Set field {1}={2} for {0}', + displayable_path(self.paths), + field, + value) + self.album[field] = value + for item in self.imported_items(): + item[field] = value + item.store() + self.album.store() def finalize(self, session): """Save progress, clean up files, and emit plugin event. @@ -946,18 +950,19 @@ class SingletonImportTask(ImportTask): def reload(self): self.item.load() - def set_fields(self): + def set_fields(self, lib): """Sets the fields given at CLI or configuration to the specified - values. + values, for the singleton item. """ - for field, view in config['import']['set_fields'].items(): - value = view.get() - log.debug(u'Set field {1}={2} for {0}', - displayable_path(self.paths), - field, - value) - self.item[field] = value - self.item.store() + with lib.transaction(): + for field, view in config['import']['set_fields'].items(): + value = view.get() + log.debug(u'Set field {1}={2} for {0}', + displayable_path(self.paths), + field, + value) + self.item[field] = value + self.item.store() # FIXME The inheritance relationships are inverted. This is why there @@ -1510,7 +1515,7 @@ def apply_choice(session, task): # because then the ``ImportTask`` won't have an `album` for which # it can set the fields. if config['import']['set_fields']: - task.set_fields() + task.set_fields(session.lib) @pipeline.mutator_stage diff --git a/docs/changelog.rst b/docs/changelog.rst index 72a4b56d5..6c76b95c4 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -216,6 +216,8 @@ Other new things: ``check_on_import`` config option. * :doc:`/plugins/export`: big speedups when `--include-keys` option is used Thanks to :user:`ssssam`. +* The `importer` persists all fields set using :ref:`set_fields` to the + mediafiles of all imported tracks. Fixes: diff --git a/docs/reference/config.rst b/docs/reference/config.rst index 455639be0..aabe732c2 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -683,6 +683,9 @@ Here's an example:: Other field/value pairs supplied via the ``--set`` option on the command-line override any settings here for fields with the same name. +Fields are set on both the album and each individual track of the album. +Fields are persisted to the media files of each track. + Default: ``{}`` (empty). .. _musicbrainz-config: diff --git a/test/test_importer.py b/test/test_importer.py index 3418d4628..fcdd00125 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -734,10 +734,12 @@ class ImportTest(_common.TestCase, ImportHelper): def test_set_fields(self): genre = u"\U0001F3B7 Jazz" collection = u"To Listen" + comments = u"managed by beets" config['import']['set_fields'] = { u'collection': collection, - u'genre': genre + u'genre': genre, + u'comments': comments } # As-is album import. @@ -749,6 +751,10 @@ class ImportTest(_common.TestCase, ImportHelper): album.load() # TODO: Not sure this is necessary. self.assertEqual(album.genre, genre) self.assertEqual(album.collection, collection) + for item in album.items(): + self.assertEqual(item.genre, genre) + self.assertEqual(item.collection, collection) + self.assertEqual(item.comments, comments) # Remove album from library to test again with APPLY choice. album.remove() @@ -762,6 +768,10 @@ class ImportTest(_common.TestCase, ImportHelper): album.load() self.assertEqual(album.genre, genre) self.assertEqual(album.collection, collection) + for item in album.items(): + self.assertEqual(item.genre, genre) + self.assertEqual(item.collection, collection) + self.assertEqual(item.comments, comments) class ImportTracksTest(_common.TestCase, ImportHelper): From e10b2754c39258cc2a86a5d1630e844733ace8fb Mon Sep 17 00:00:00 2001 From: Bert Besser Date: Fri, 7 May 2021 07:11:41 +0200 Subject: [PATCH 2/4] optimize set_fields loop --- beets/importer.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index f28e6822d..4cd0db08a 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -577,6 +577,7 @@ class ImportTask(BaseImportTask): values, for both the album and all its items. """ with lib.transaction(): + items = self.imported_items() for field, view in config['import']['set_fields'].items(): value = view.get() log.debug(u'Set field {1}={2} for {0}', @@ -584,9 +585,10 @@ class ImportTask(BaseImportTask): field, value) self.album[field] = value - for item in self.imported_items(): + for item in items: item[field] = value - item.store() + for item in items: + item.store() self.album.store() def finalize(self, session): From e98f78e29b5be56c56b7ce703ef26824004ac4fc Mon Sep 17 00:00:00 2001 From: Bert Besser Date: Thu, 13 May 2021 15:07:54 +0200 Subject: [PATCH 3/4] fix: transactions and stricter tests --- beets/importer.py | 37 ++++++++++++++++++------------------- test/test_importer.py | 15 ++++++--------- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index bd16c244e..3e288c271 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -576,17 +576,17 @@ class ImportTask(BaseImportTask): """Sets the fields given at CLI or configuration to the specified values, for both the album and all its items. """ + items = self.imported_items() + for field, view in config['import']['set_fields'].items(): + value = view.get() + log.debug(u'Set field {1}={2} for {0}', + displayable_path(self.paths), + field, + value) + self.album[field] = value + for item in items: + item[field] = value with lib.transaction(): - items = self.imported_items() - for field, view in config['import']['set_fields'].items(): - value = view.get() - log.debug(u'Set field {1}={2} for {0}', - displayable_path(self.paths), - field, - value) - self.album[field] = value - for item in items: - item[field] = value for item in items: item.store() self.album.store() @@ -956,15 +956,14 @@ class SingletonImportTask(ImportTask): """Sets the fields given at CLI or configuration to the specified values, for the singleton item. """ - with lib.transaction(): - for field, view in config['import']['set_fields'].items(): - value = view.get() - log.debug(u'Set field {1}={2} for {0}', - displayable_path(self.paths), - field, - value) - self.item[field] = value - self.item.store() + for field, view in config['import']['set_fields'].items(): + value = view.get() + log.debug(u'Set field {1}={2} for {0}', + displayable_path(self.paths), + field, + value) + self.item[field] = value + self.item.store() # FIXME The inheritance relationships are inverted. This is why there diff --git a/test/test_importer.py b/test/test_importer.py index a80fc9de8..e581f282a 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -743,7 +743,6 @@ class ImportTest(_common.TestCase, ImportHelper): comments = u"managed by beets" config['import']['set_fields'] = { - u'collection': collection, u'genre': genre, u'comments': comments } @@ -756,11 +755,10 @@ class ImportTest(_common.TestCase, ImportHelper): for album in self.lib.albums(): album.load() # TODO: Not sure this is necessary. self.assertEqual(album.genre, genre) - self.assertEqual(album.collection, collection) + self.assertEqual(album.comments, comments) for item in album.items(): - self.assertEqual(item.genre, genre) - self.assertEqual(item.collection, collection) - self.assertEqual(item.comments, comments) + self.assertEqual(item.get("genre", with_album=False), genre) + self.assertEqual(item.get("comments", with_album=False), comments) # Remove album from library to test again with APPLY choice. album.remove() @@ -773,11 +771,10 @@ class ImportTest(_common.TestCase, ImportHelper): for album in self.lib.albums(): album.load() self.assertEqual(album.genre, genre) - self.assertEqual(album.collection, collection) + self.assertEqual(album.comments, comments) for item in album.items(): - self.assertEqual(item.genre, genre) - self.assertEqual(item.collection, collection) - self.assertEqual(item.comments, comments) + self.assertEqual(item.get("genre", with_album=False), genre) + self.assertEqual(item.get("comments", with_album=False), comments) class ImportTracksTest(_common.TestCase, ImportHelper): From 8dc960b1f6d78d4f8ee9fc59f8bad398b1d642f2 Mon Sep 17 00:00:00 2001 From: Bert Besser Date: Fri, 14 May 2021 10:19:59 +0200 Subject: [PATCH 4/4] fix: formatting --- test/test_importer.py | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/test/test_importer.py b/test/test_importer.py index e581f282a..16881a152 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -744,6 +744,7 @@ class ImportTest(_common.TestCase, ImportHelper): config['import']['set_fields'] = { u'genre': genre, + u'collection': collection, u'comments': comments } @@ -757,8 +758,15 @@ class ImportTest(_common.TestCase, ImportHelper): self.assertEqual(album.genre, genre) self.assertEqual(album.comments, comments) for item in album.items(): - self.assertEqual(item.get("genre", with_album=False), genre) - self.assertEqual(item.get("comments", with_album=False), comments) + self.assertEqual( + item.get("genre", with_album=False), + genre) + self.assertEqual( + item.get("collection", with_album=False), + collection) + self.assertEqual( + item.get("comments", with_album=False), + comments) # Remove album from library to test again with APPLY choice. album.remove() @@ -773,8 +781,15 @@ class ImportTest(_common.TestCase, ImportHelper): self.assertEqual(album.genre, genre) self.assertEqual(album.comments, comments) for item in album.items(): - self.assertEqual(item.get("genre", with_album=False), genre) - self.assertEqual(item.get("comments", with_album=False), comments) + self.assertEqual( + item.get("genre", with_album=False), + genre) + self.assertEqual( + item.get("collection", with_album=False), + collection) + self.assertEqual( + item.get("comments", with_album=False), + comments) class ImportTracksTest(_common.TestCase, ImportHelper):