From 629a80a1d12818cfa0de0b8b8b8a0e24d753215a Mon Sep 17 00:00:00 2001 From: reiv Date: Sun, 1 Nov 2015 01:43:48 +0100 Subject: [PATCH 1/8] Importer: Delete orphaned album art on re-imports First step towards fixing #314. --- beets/importer.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/beets/importer.py b/beets/importer.py index 95163b6fd..ba1c47c94 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -547,6 +547,23 @@ class ImportTask(BaseImportTask): items = self.imported_items() + # If any original album art files were reused, leave them untouched. + for item in items: + album = item.get_album() + if album: + self.old_art_paths.discard(album.artpath) + + # Delete remaining original album art files. + for old_art_path in self.old_art_paths: + log.debug('Deleting orphaned album art: {0}', old_art_path) + util.remove(syspath(old_art_path), True) + # XXX: Ideally the directory would get pruned with self.prune() + # below, but when calling `beet import -L`, self.toppath is set + # to None, making self.prune() a no-op; hence this workaround of + # calling util.prune_dirs() directly. + util.prune_dirs(os.path.dirname(old_art_path), + clutter=config['clutter'].as_str_seq()) + # When copying and deleting originals, delete old files. if copy and delete: new_paths = [os.path.realpath(item.path) for item in items] @@ -652,6 +669,11 @@ class ImportTask(BaseImportTask): # Save the original paths of all items for deletion and pruning # in the next step (finalization). self.old_paths = [item.path for item in items] + + # Keep track of paths of all original album art for the same reason. + self.old_art_paths = set(filter(bool, + (album.artpath for album in self.replaced_albums.values()))) + for item in items: if move or copy or link: # In copy and link modes, treat re-imports specially: From 413fe6bbfd695725c9ca7d306dfe408d20ad3904 Mon Sep 17 00:00:00 2001 From: reiv Date: Sun, 1 Nov 2015 04:33:36 +0100 Subject: [PATCH 2/8] PEP8 amendments --- beets/importer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index ba1c47c94..8aecf54a3 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -671,8 +671,8 @@ class ImportTask(BaseImportTask): self.old_paths = [item.path for item in items] # Keep track of paths of all original album art for the same reason. - self.old_art_paths = set(filter(bool, - (album.artpath for album in self.replaced_albums.values()))) + self.old_art_paths = set(filter( + bool, (album.artpath for album in self.replaced_albums.values()))) for item in items: if move or copy or link: From 33ea9f1c101183ff1d8fba68950120b967cf14a8 Mon Sep 17 00:00:00 2001 From: reiv Date: Tue, 3 Nov 2015 11:25:04 +0100 Subject: [PATCH 3/8] Revert all changes on this branch thus far Revert "PEP8 amendments" This reverts commit 413fe6bbfd695725c9ca7d306dfe408d20ad3904. Revert "Importer: Delete orphaned album art on..." This reverts commit 629a80a1d12818cfa0de0b8b8b8a0e24d753215a. --- beets/importer.py | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 8aecf54a3..95163b6fd 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -547,23 +547,6 @@ class ImportTask(BaseImportTask): items = self.imported_items() - # If any original album art files were reused, leave them untouched. - for item in items: - album = item.get_album() - if album: - self.old_art_paths.discard(album.artpath) - - # Delete remaining original album art files. - for old_art_path in self.old_art_paths: - log.debug('Deleting orphaned album art: {0}', old_art_path) - util.remove(syspath(old_art_path), True) - # XXX: Ideally the directory would get pruned with self.prune() - # below, but when calling `beet import -L`, self.toppath is set - # to None, making self.prune() a no-op; hence this workaround of - # calling util.prune_dirs() directly. - util.prune_dirs(os.path.dirname(old_art_path), - clutter=config['clutter'].as_str_seq()) - # When copying and deleting originals, delete old files. if copy and delete: new_paths = [os.path.realpath(item.path) for item in items] @@ -669,11 +652,6 @@ class ImportTask(BaseImportTask): # Save the original paths of all items for deletion and pruning # in the next step (finalization). self.old_paths = [item.path for item in items] - - # Keep track of paths of all original album art for the same reason. - self.old_art_paths = set(filter( - bool, (album.artpath for album in self.replaced_albums.values()))) - for item in items: if move or copy or link: # In copy and link modes, treat re-imports specially: From 21f926fb8928bc6a45a5c6879521095b337658d9 Mon Sep 17 00:00:00 2001 From: reiv Date: Tue, 3 Nov 2015 22:38:13 +0100 Subject: [PATCH 4/8] Add test for #314 Ensure that album art is preserved when an album is re-imported. --- test/test_importer.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/test_importer.py b/test/test_importer.py index 7b07ea331..b01a93459 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1592,6 +1592,18 @@ class ReimportTest(unittest.TestCase, ImportHelper): self.importer.run() self.assertEqual(self._item().added, 4747.0) + def test_reimported_item_preserves_art(self): + self._setup_session() + artpath = os.path.join(_common.RSRC, 'abbey.jpg') + replaced_album = self._album() + replaced_album.set_art(artpath) + replaced_album.store() + self.importer.run() + new_album = self._album() + new_artpath = new_album.art_destination(artpath) + self.assertEqual(new_album.artpath, new_artpath) + self.assertTrue(os.path.exists(new_artpath)) + class ImportPretendTest(_common.TestCase, ImportHelper): """ Test the pretend commandline option From 32934bd16a61b8b676e54a8c2699ccf936ae6e50 Mon Sep 17 00:00:00 2001 From: reiv Date: Tue, 3 Nov 2015 22:43:05 +0100 Subject: [PATCH 5/8] Preserve album art on re-import (fixes #314) Copy the replaced album's artpath attribute to the new album. This causes the image file to be moved along with the music files. --- beets/importer.py | 1 + 1 file changed, 1 insertion(+) diff --git a/beets/importer.py b/beets/importer.py index 95163b6fd..85d6e0824 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -718,6 +718,7 @@ class ImportTask(BaseImportTask): if replaced_album: self.album.added = replaced_album.added self.album.update(replaced_album._values_flex) + self.album.artpath = replaced_album.artpath self.album.store() log.debug( u'Reimported album: added {0}, flexible ' From 314dd0e6bc13870f954f80c316ac6b93436e3eab Mon Sep 17 00:00:00 2001 From: reiv Date: Tue, 3 Nov 2015 23:10:01 +0100 Subject: [PATCH 6/8] Update re-import test to leave no orphaned art Make sure that when an album is re-imported and its files are moved, the artwork isn't left behind in the old folder. --- test/test_importer.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/test_importer.py b/test/test_importer.py index b01a93459..675ac7796 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1594,15 +1594,18 @@ class ReimportTest(unittest.TestCase, ImportHelper): def test_reimported_item_preserves_art(self): self._setup_session() - artpath = os.path.join(_common.RSRC, 'abbey.jpg') + art_source = os.path.join(_common.RSRC, 'abbey.jpg') replaced_album = self._album() - replaced_album.set_art(artpath) + replaced_album.set_art(art_source) replaced_album.store() + old_artpath = replaced_album.artpath self.importer.run() new_album = self._album() - new_artpath = new_album.art_destination(artpath) + new_artpath = new_album.art_destination(art_source) self.assertEqual(new_album.artpath, new_artpath) self.assertTrue(os.path.exists(new_artpath)) + if new_artpath != old_artpath: + self.assertFalse(os.path.exists(old_artpath)) class ImportPretendTest(_common.TestCase, ImportHelper): From 0ebc4c799d8f50da3c986723f8b2a9fa25f3f826 Mon Sep 17 00:00:00 2001 From: reiv Date: Wed, 4 Nov 2015 15:08:42 +0100 Subject: [PATCH 7/8] fetchart: in auto, ignore albums with art When re-importing an album, we don't want fetchart to interfere with any existing album art. --- beetsplug/fetchart.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index b807891e0..2c20dd698 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -430,6 +430,9 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): def fetch_art(self, session, task): """Find art for the album being imported.""" if task.is_album: # Only fetch art for full albums. + if task.album.artpath and os.path.isfile(task.album.artpath): + # Album already has art (probably a re-import); skip it. + return if task.choice_flag == importer.action.ASIS: # For as-is imports, don't search Web sources for art. local = True From e32dda78b892289de4e4d4790144bdffe1b6b69f Mon Sep 17 00:00:00 2001 From: reiv Date: Wed, 4 Nov 2015 18:33:52 +0100 Subject: [PATCH 8/8] Changelog for #1681 --- docs/changelog.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 4eafe9c90..d71efac17 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -16,6 +16,10 @@ Fixes: when there were not. :bug:`1652` * :doc:`plugins/lastgenre`: Clean up the reggae related genres somewhat. Thanks to :user:`Freso`. :bug:`1661` +* The importer now correctly moves album art files when re-importing. + :bug:`314` +* :doc:`/plugins/fetchart`: In auto mode, skips albums that already have + art attached to them so as not to interfere with re-imports. :bug:`314` 1.3.15 (October 17, 2015)