diff --git a/beets/importer/tasks.py b/beets/importer/tasks.py index 0667083168..bfa9d00ff9 100644 --- a/beets/importer/tasks.py +++ b/beets/importer/tasks.py @@ -270,15 +270,30 @@ def duplicate_items(self, lib: library.Library): return duplicate_items def remove_duplicates(self, lib: library.Library): - duplicate_items = self.duplicate_items(lib) - log.debug("removing {} old duplicated items", len(duplicate_items)) - for item in duplicate_items: - item.remove() - if lib.directory in util.ancestry(item.path): - log.debug("deleting duplicate {.filepath}", item) - util.remove(item.path) + duplicate_albums = self.find_duplicates(lib) + log.debug("removing {} old duplicate albums", len(duplicate_albums)) + + for album in duplicate_albums: + artpath = album.artpath + + for item in album.items(): + item.remove(with_album=False) + if lib.directory in util.ancestry(item.path): + log.debug("deleting duplicate {.filepath}", item) + util.remove(item.path) + util.prune_dirs( + os.path.dirname(item.path), + lib.directory, + clutter=config["clutter"].as_str_seq(), + ) + + album.remove(with_items=False) + + if artpath and lib.directory in util.ancestry(artpath): + log.debug("deleting duplicate album art {}", artpath) + util.remove(artpath) util.prune_dirs( - os.path.dirname(item.path), + os.path.dirname(artpath), lib.directory, clutter=config["clutter"].as_str_seq(), ) @@ -716,6 +731,20 @@ def find_duplicates(self, lib: library.Library) -> list[library.Item]: # type: duplicate_items = find_duplicates + def remove_duplicates(self, lib: library.Library): + duplicate_items = self.find_duplicates(lib) + log.debug("removing {} old duplicated items", len(duplicate_items)) + for item in duplicate_items: + item.remove() + if lib.directory in util.ancestry(item.path): + log.debug("deleting duplicate {.filepath}", item) + util.remove(item.path) + util.prune_dirs( + os.path.dirname(item.path), + lib.directory, + clutter=config["clutter"].as_str_seq(), + ) + def add(self, lib): with lib.transaction(): self.record_replaced(lib) diff --git a/beets/library/models.py b/beets/library/models.py index e3e84cd342..750403ddc1 100644 --- a/beets/library/models.py +++ b/beets/library/models.py @@ -540,9 +540,9 @@ def set_art(self, path, copy=True): return # Normal operation. - if oldart == artdest: + if oldart: util.remove(oldart) - artdest = util.unique_path(artdest) + util.remove(artdest) if copy: util.copy(path, artdest) else: diff --git a/docs/changelog.rst b/docs/changelog.rst index 238cf7ad23..9b22d26497 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -48,6 +48,10 @@ Bug fixes item or album-art paths were stored as SQLite ``TEXT`` values instead of bytes, so upgrading to the portable-path storage format no longer fails for those libraries. :bug:`6561` +- :ref:`import-cmd` Fix duplicate album art files (e.g. ``cover.2.jpg``) being + created when re-importing albums with the :doc:`plugins/fetchart` plugin + enabled. Old album art is now properly removed when replacing duplicate albums + during import. :bug:`1264` :bug:`6205` .. For plugin developers diff --git a/test/test_files.py b/test/test_files.py index db7c3c5618..9b525bc686 100644 --- a/test/test_files.py +++ b/test/test_files.py @@ -414,7 +414,7 @@ def test_setart_to_existing_but_unset_art_works(self): ai.set_art(artdest) assert ai.art_filepath.exists() - def test_setart_to_conflicting_file_gets_new_path(self): + def test_setart_to_conflicting_file_replaces_it(self): newart = os.path.join(self.libdir, b"newart.jpg") touch(newart) i2 = item() @@ -427,10 +427,33 @@ def test_setart_to_conflicting_file_gets_new_path(self): artdest = ai.art_destination(newart) touch(artdest) - # Set the art. + # Set the art - should replace the existing file, not create a suffixed + # duplicate like cover.2.jpg. ai.set_art(newart) - assert artdest != ai.artpath - assert os.path.dirname(artdest) == os.path.dirname(ai.artpath) + assert artdest == ai.artpath + + def test_setart_replaces_old_art_at_different_path(self): + newart = os.path.join(self.libdir, b"newart.png") + touch(newart) + i2 = item() + i2.path = self.i.path + i2.artist = "someArtist" + ai = self.lib.add_album((i2,)) + i2.move(operation=MoveOperation.COPY) + + # Set initial art. + ai.set_art(newart) + old_artpath = ai.artpath + assert os.path.exists(syspath(old_artpath)) + + # Set new art with a different extension. + another_art = os.path.join(self.libdir, b"another.jpg") + touch(another_art) + ai.set_art(another_art) + + # Old art should be removed. + assert not os.path.exists(syspath(old_artpath)) + assert ai.art_filepath.exists() def test_setart_sets_permissions(self): util.remove(self.art) diff --git a/test/test_importer.py b/test/test_importer.py index 31ec6535cb..5252132dfc 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1058,6 +1058,21 @@ def test_remove_duplicate_album(self): item = self.lib.items().get() assert item.title == "new title" + def test_remove_duplicate_album_deletes_art(self): + album = self.lib.albums().get() + art_source = os.path.join(_common.RSRC, b"abbey.jpg") + album.set_art(art_source) + album.store() + old_artpath = album.art_filepath + + assert old_artpath.exists() + + self.importer.default_resolution = self.importer.Resolution.REMOVE + self.importer.run() + + assert not old_artpath.exists() + assert len(self.lib.albums()) == 1 + def test_no_autotag_removes_duplicate_album(self): config["import"]["autotag"] = False album = self.lib.albums().get()