move, rather than copying, when re-importing

beets/importer.py

@@ -19,6 +19,7 @@ from __future__ import with_statement # Python 2.5
 import os
 import logging
 import pickle
+from collections import defaultdict
 
 from beets import autotag
 from beets import library
@@ -508,6 +509,7 @@ def apply_choices(config):
         task = yield task
         if task.should_skip():
             continue
+        items = task.items if task.is_album else [task.item]
 
         # Change metadata.
         if task.should_write_tags():
@@ -520,12 +522,26 @@ def apply_choices(config):
         if task.is_album:
             _infer_album_fields(task)
 
+        # Find existing item entries that these are replacing. Old
+        # album structures are automatically cleaned up when the
+        # last item is removed.
+        replaced_items = defaultdict(list)
+        for item in items:
+            dup_items = list(lib.items(
+                                library.MatchQuery('path', item.path)
+                        ))
+            for dup_item in dup_items:
+                if dup_item.id != item.id:
+                    replaced_items[item].append(dup_item)
+
         # Move/copy files.
-        items = task.items if task.is_album else [task.item]
         task.old_paths = [item.path for item in items]
         for item in items:
             if config.copy:
-                item.move(lib, True, task.is_album)
+                # If we're replacing an item, then move rather than
+                # copying.
+                do_copy = not bool(replaced_items[item])
+                item.move(lib, do_copy, task.is_album)
             if config.write and task.should_write_tags():
                 item.write()
 
@@ -542,16 +558,10 @@ def apply_choices(config):
                 for item in items:
                     lib.add(item)
 
-            # Remove old entries if we're re-importing old items. Old
+            # Remove old ones.
-            # album structures are automatically cleaned up when the
+            for replaced in replaced_items.itervalues():
-            # last item is removed.
+                for item in replaced:
-            for item, old_path in zip(items, task.old_paths):
+                    lib.remove(item)
-                dup_items = list(lib.items(
-                                    library.MatchQuery('path', old_path)
-                            ))
-                for dup_item in dup_items:
-                    if dup_item.id != item.id:
-                        lib.remove(dup_item)
         finally:
             lib.save()
 

beets/library.py

@@ -22,7 +22,7 @@ import logging
 from beets.mediafile import MediaFile
 from beets import plugins
 from beets import util
-from beets.util import bytestring_path, syspath, normpath
+from beets.util import bytestring_path, syspath, normpath, samefile
 
 MAX_FILENAME_LENGTH = 200
 
@@ -237,7 +237,7 @@ class Item(object):
         # Create necessary ancestry for the move.
         util.mkdirall(dest)
         
-        if not shutil._samefile(syspath(self.path), syspath(dest)):
+        if not samefile(self.path, dest):
             if copy:
                 # copyfile rather than copy will not copy permissions
                 # bits, thus possibly making the copy writable even when
@@ -1213,10 +1213,10 @@ class Album(BaseAlbum):
         oldart = self.artpath
         artdest = self.art_destination(path)
 
-        if oldart and shutil._samefile(syspath(path), syspath(oldart)):
+        if oldart and samefile(path, oldart):
             # Art already set.
             return
-        elif shutil._samefile(syspath(path), syspath(artdest)):
+        elif samefile(path, artdest):
             # Art already in place.
             self.artpath = path
             return

beets/util/__init__.py

@@ -175,6 +175,10 @@ def syspath(path, pathmod=None):
 
     return path
 
+def samefile(p1, p2):
+    """Safer equality for paths."""
+    return shutil._samefile(syspath(p1), syspath(p2))
+
 def soft_remove(path):
     """Remove the file if it exists."""
     path = syspath(path)

test/test_importer.py

@@ -373,6 +373,74 @@ class ApplyExistingItemsTest(unittest.TestCase, _common.ExtraAsserts):
         self.assertTrue('differentTitle' in item.path)
         self.assertExists(item.path)
 
+    def test_apply_existing_item_new_metadata_copy_disabled(self):
+        # Import *without* copying to ensure that the path does *not* change.
+        self.config.copy = False
+        self._apply_asis([self.i])
+        item = self.lib.items().next()
+        new_item = library.Item.from_path(item.path)
+        new_item.title = 'differentTitle'
+        self._apply_asis([new_item])
+
+        item = self.lib.items().next()
+        self.assertFalse('differentTitle' in item.path)
+        self.assertExists(item.path)
+
+    def test_apply_existing_item_new_metadata_removes_old_files(self):
+        self.config.copy = True
+        self._apply_asis([self.i])
+        item = self.lib.items().next()
+        oldpath = item.path
+        new_item = library.Item.from_path(item.path)
+        new_item.title = 'differentTitle'
+        self._apply_asis([new_item])
+
+        item = self.lib.items().next()
+        self.assertNotExists(oldpath)
+
+    def test_apply_existing_item_new_metadata_delete_enabled(self):
+        # The "delete" flag should be ignored -- only the "copy" flag
+        # controls whether files move.
+        self.config.copy = True
+        self.config.delete = True # !
+        self._apply_asis([self.i])
+        item = self.lib.items().next()
+        oldpath = item.path
+        new_item = library.Item.from_path(item.path)
+        new_item.title = 'differentTitle'
+        self._apply_asis([new_item])
+
+        item = self.lib.items().next()
+        self.assertNotExists(oldpath)
+        self.assertTrue('differentTitle' in item.path)
+        self.assertExists(item.path)
+
+    def test_apply_existing_item_preserves_file(self):
+        # With copying enabled, import the item twice with same metadata.
+        self.config.copy = True
+        self._apply_asis([self.i])
+        item = self.lib.items().next()
+        oldpath = item.path
+        new_item = library.Item.from_path(item.path)
+        self._apply_asis([new_item])
+
+        self.assertEqual(len(list(self.lib.items())), 1)
+        item = self.lib.items().next()
+        self.assertEqual(oldpath, item.path)
+        self.assertExists(oldpath)
+
+    def test_apply_existing_item_preserves_file_delete_enabled(self):
+        self.config.copy = True
+        self.config.delete = True # !
+        self._apply_asis([self.i])
+        item = self.lib.items().next()
+        new_item = library.Item.from_path(item.path)
+        self._apply_asis([new_item])
+
+        self.assertEqual(len(list(self.lib.items())), 1)
+        item = self.lib.items().next()
+        self.assertExists(item.path)
+
 class InferAlbumDataTest(unittest.TestCase):
     def setUp(self):
         i1 = _common.item()