From e8b8cb179f27f9f61e169d03d96fd502c8542629 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Thu, 15 Sep 2011 16:15:53 -0700 Subject: [PATCH] refactor: move() is a method on Library (not Item) --- beets/importer.py | 2 +- beets/library.py | 92 +++++++++++++++++++++----------------------- beets/ui/commands.py | 6 +-- test/test_files.py | 34 ++++++++-------- 4 files changed, 65 insertions(+), 69 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index a6edfc949..66d35314d 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -628,7 +628,7 @@ def apply_choices(config): # 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) + lib.move(item, do_copy, task.is_album) if config.write and task.should_write_tags(): item.write() diff --git a/beets/library.py b/beets/library.py index 90f7952a3..2d1c1d0cc 100644 --- a/beets/library.py +++ b/beets/library.py @@ -207,51 +207,6 @@ class Item(object): for key in ITEM_KEYS_WRITABLE: setattr(f, key, getattr(self, key)) f.save() - - - # Dealing with files themselves. - - def move(self, library, copy=False, in_album=False, basedir=None): - """Move the item to its designated location within the library - directory (provided by destination()). Subdirectories are - created as needed. If the operation succeeds, the item's path - field is updated to reflect the new location. - - If copy is True, moving the file is copied rather than moved. - - If in_album is True, then the track is treated as part of an - album even if it does not yet have an album_id associated with - it. (This allows items to be moved before they are added to the - database, a performance optimization.) - - basedir overrides the library base directory for the - destination. - - Passes on appropriate exceptions if directories cannot be - created or moving/copying fails. - - Note that one should almost certainly call store() and - library.save() after this method in order to keep on-disk data - consistent. - """ - dest = library.destination(self, in_album=in_album, basedir=basedir) - - # Create necessary ancestry for the move. - util.mkdirall(dest) - - if not samefile(self.path, dest): - if copy: - util.copy(self.path, dest) - else: - util.move(self.path, dest) - - # Either copying or moving succeeded, so update the stored path. - old_path = self.path - self.path = dest - - # Prune vacated directory. - if not copy: - util.prune_dirs(os.path.dirname(old_path), library.directory) # Library queries. @@ -864,13 +819,13 @@ class Library(BaseLibrary): return normpath(os.path.join(basedir, subpath)) - # Main interface. + # Item manipulation. def add(self, item, copy=False): #FIXME make a deep copy of the item? item.library = self if copy: - item.move(self, copy=True) + self.move(item, copy=True) # build essential parts of query columns = ','.join([key for key in ITEM_KEYS if key != 'id']) @@ -962,6 +917,47 @@ class Library(BaseLibrary): if delete: util.soft_remove(item.path) util.prune_dirs(os.path.dirname(item.path), self.directory) + + def move(self, item, copy=False, in_album=False, basedir=None): + """Move the item to its designated location within the library + directory (provided by destination()). Subdirectories are + created as needed. If the operation succeeds, the item's path + field is updated to reflect the new location. + + If copy is True, moving the file is copied rather than moved. + + If in_album is True, then the track is treated as part of an + album even if it does not yet have an album_id associated with + it. (This allows items to be moved before they are added to the + database, a performance optimization.) + + basedir overrides the library base directory for the + destination. + + Passes on appropriate exceptions if directories cannot be + created or moving/copying fails. + + Note that one should almost certainly call store() and + library.save() after this method in order to keep on-disk data + consistent. + """ + dest = self.destination(item, in_album=in_album, basedir=basedir) + + # Create necessary ancestry for the move. + util.mkdirall(dest) + + if copy: + util.copy(item.path, dest) + else: + util.move(item.path, dest) + + # Either copying or moving succeeded, so update the stored path. + old_path = item.path + item.path = dest + + # Prune vacated directory. + if not copy: + util.prune_dirs(os.path.dirname(old_path), self.directory) # Querying. @@ -1166,7 +1162,7 @@ class Album(BaseAlbum): # Move items. items = list(self.items()) for item in items: - item.move(self._library, copy, basedir=basedir) + self._library.move(item, copy, basedir=basedir) newdir = os.path.dirname(items[0].path) # Move art. diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 359549227..ee37de051 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -718,7 +718,7 @@ def update_items(lib, query, album, move, color): # Move the item if it's in the library. if move and lib.directory in ancestry(item.path): - item.move(lib) + lib.move(item) lib.store(item) affected_albums.add(item.album_id) @@ -911,7 +911,7 @@ def modify_items(lib, mods, query, write, move, album, color, confirm): if album: obj.move() else: - obj.move(lib) + lib.move(obj) # When modifying items, we have to store them to the database. if not album: @@ -973,7 +973,7 @@ def move_items(lib, dest, query, copy, album): if album: obj.move(copy, basedir=dest) else: - obj.move(lib, copy, basedir=dest) + lib.move(obj, copy, basedir=dest) lib.store(obj) lib.save() diff --git a/test/test_files.py b/test/test_files.py index 45e3b20d5..00ec59536 100644 --- a/test/test_files.py +++ b/test/test_files.py @@ -57,49 +57,49 @@ class MoveTest(unittest.TestCase, _common.ExtraAsserts): shutil.rmtree(self.otherdir) def test_move_arrives(self): - self.i.move(self.lib) + self.lib.move(self.i) self.assertExists(self.dest) def test_move_to_custom_dir(self): - self.i.move(self.lib, basedir=self.otherdir) + self.lib.move(self.i, basedir=self.otherdir) self.assertExists(join(self.otherdir, 'one', 'two', 'three.mp3')) def test_move_departs(self): - self.i.move(self.lib) + self.lib.move(self.i) self.assertNotExists(self.path) def test_move_in_lib_prunes_empty_dir(self): - self.i.move(self.lib) + self.lib.move(self.i) old_path = self.i.path self.assertExists(old_path) self.i.artist = 'newArtist' - self.i.move(self.lib) + self.lib.move(self.i) self.assertNotExists(old_path) self.assertNotExists(os.path.dirname(old_path)) def test_copy_arrives(self): - self.i.move(self.lib, copy=True) + self.lib.move(self.i, copy=True) self.assertExists(self.dest) def test_copy_does_not_depart(self): - self.i.move(self.lib, copy=True) + self.lib.move(self.i, copy=True) self.assertExists(self.path) def test_move_changes_path(self): - self.i.move(self.lib) + self.lib.move(self.i) self.assertEqual(self.i.path, util.normpath(self.dest)) def test_copy_already_at_destination(self): - self.i.move(self.lib) + self.lib.move(self.i) old_path = self.i.path - self.i.move(self.lib, copy=True) + self.lib.move(self.i, copy=True) self.assertEqual(self.i.path, old_path) def test_move_already_at_destination(self): - self.i.move(self.lib) + self.lib.move(self.i) old_path = self.i.path - self.i.move(self.lib, copy=False) + self.lib.move(self.i, copy=False) self.assertEqual(self.i.path, old_path) def test_read_only_file_copied_writable(self): @@ -107,7 +107,7 @@ class MoveTest(unittest.TestCase, _common.ExtraAsserts): os.chmod(self.path, 0444) try: - self.i.move(self.lib, copy=True) + self.lib.move(self.i, copy=True) self.assertTrue(os.access(self.i.path, os.W_OK)) finally: # Make everything writable so it can be cleaned up. @@ -256,7 +256,7 @@ class ArtFileTest(unittest.TestCase, _common.ExtraAsserts): i2.path = self.i.path i2.artist = 'someArtist' ai = self.lib.add_album((i2,)) - i2.move(self.lib, True) + self.lib.move(i2, True) self.assertEqual(ai.artpath, None) ai.set_art(newart) @@ -272,7 +272,7 @@ class ArtFileTest(unittest.TestCase, _common.ExtraAsserts): i2.path = self.i.path i2.artist = 'someArtist' ai = self.lib.add_album((i2,)) - i2.move(self.lib, True) + self.lib.move(i2, True) ai.set_art(newart) # Set the art again. @@ -286,7 +286,7 @@ class ArtFileTest(unittest.TestCase, _common.ExtraAsserts): i2.path = self.i.path i2.artist = 'someArtist' ai = self.lib.add_album((i2,)) - i2.move(self.lib, True) + self.lib.move(i2, True) # Copy the art to the destination. artdest = ai.art_destination(newart) @@ -308,7 +308,7 @@ class ArtFileTest(unittest.TestCase, _common.ExtraAsserts): i2.path = self.i.path i2.artist = 'someArtist' ai = self.lib.add_album((i2,)) - i2.move(self.lib, True) + self.lib.move(i2, True) ai.set_art(newart) mode = stat.S_IMODE(os.stat(ai.artpath).st_mode)