Merge pull request #2687 from zigarrre/issue2682

Refactored move functions for clarity according to #2682
This commit is contained in:
Adrian Sampson 2017-09-12 23:11:58 -04:00 committed by GitHub
commit 0a1db4218f
10 changed files with 142 additions and 101 deletions

View file

@ -37,7 +37,7 @@ from beets import dbcore
from beets import plugins
from beets import util
from beets import config
from beets.util import pipeline, sorted_walk, ancestry
from beets.util import pipeline, sorted_walk, ancestry, MoveOperation
from beets.util import syspath, normpath, displayable_path
from enum import Enum
from beets import mediafile
@ -675,20 +675,28 @@ class ImportTask(BaseImportTask):
for item in self.items:
item.update(changes)
def manipulate_files(self, move=False, copy=False, write=False,
link=False, hardlink=False, session=None):
def manipulate_files(self, operation=None, write=False, session=None):
""" Copy, move, link or hardlink (depending on `operation`) the files
as well as write metadata.
`operation` should be an instance of `util.MoveOperation`.
If `write` is `True` metadata is written to the files.
"""
items = self.imported_items()
# 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]
for item in items:
if move or copy or link or hardlink:
if operation is not None:
# In copy and link modes, treat re-imports specially:
# move in-library files. (Out-of-library files are
# copied/moved as usual).
old_path = item.path
if (copy or link or hardlink) and self.replaced_items[item] \
and session.lib.directory in util.ancestry(old_path):
if (operation != MoveOperation.MOVE
and self.replaced_items[item]
and session.lib.directory in util.ancestry(old_path)):
item.move()
# We moved the item, so remove the
# now-nonexistent file from old_paths.
@ -696,7 +704,7 @@ class ImportTask(BaseImportTask):
else:
# A normal import. Just copy files and keep track of
# old paths.
item.move(copy, link, hardlink)
item.move(operation)
if write and (self.apply or self.choice_flag == action.RETAG):
item.try_write()
@ -1450,12 +1458,20 @@ def manipulate_files(session, task):
if task.should_remove_duplicates:
task.remove_duplicates(session.lib)
if session.config['move']:
operation = MoveOperation.MOVE
elif session.config['copy']:
operation = MoveOperation.COPY
elif session.config['link']:
operation = MoveOperation.LINK
elif session.config['hardlink']:
operation = MoveOperation.HARDLINK
else:
operation = None
task.manipulate_files(
move=session.config['move'],
copy=session.config['copy'],
operation,
write=session.config['write'],
link=session.config['link'],
hardlink=session.config['hardlink'],
session=session,
)

View file

@ -28,7 +28,8 @@ from beets import logging
from beets.mediafile import MediaFile, UnreadableFileError
from beets import plugins
from beets import util
from beets.util import bytestring_path, syspath, normpath, samefile
from beets.util import bytestring_path, syspath, normpath, samefile, \
MoveOperation
from beets.util.functemplate import Template
from beets import dbcore
from beets.dbcore import types
@ -685,31 +686,34 @@ class Item(LibModel):
# Files themselves.
def move_file(self, dest, copy=False, link=False, hardlink=False):
"""Moves or copies the item's file, updating the path value if
the move succeeds. If a file exists at ``dest``, then it is
slightly modified to be unique.
def move_file(self, dest, operation=MoveOperation.MOVE):
"""Move, copy, link or hardlink the item's depending on `operation`,
updating the path value if the move succeeds.
If a file exists at `dest`, then it is slightly modified to be unique.
`operation` should be an instance of `util.MoveOperation`.
"""
if not util.samefile(self.path, dest):
dest = util.unique_path(dest)
if copy:
util.copy(self.path, dest)
plugins.send("item_copied", item=self, source=self.path,
destination=dest)
elif link:
util.link(self.path, dest)
plugins.send("item_linked", item=self, source=self.path,
destination=dest)
elif hardlink:
util.hardlink(self.path, dest)
plugins.send("item_hardlinked", item=self, source=self.path,
destination=dest)
else:
if operation == MoveOperation.MOVE:
plugins.send("before_item_moved", item=self, source=self.path,
destination=dest)
util.move(self.path, dest)
plugins.send("item_moved", item=self, source=self.path,
destination=dest)
elif operation == MoveOperation.COPY:
util.copy(self.path, dest)
plugins.send("item_copied", item=self, source=self.path,
destination=dest)
elif operation == MoveOperation.LINK:
util.link(self.path, dest)
plugins.send("item_linked", item=self, source=self.path,
destination=dest)
elif operation == MoveOperation.HARDLINK:
util.hardlink(self.path, dest)
plugins.send("item_hardlinked", item=self, source=self.path,
destination=dest)
# Either copying or moving succeeded, so update the stored path.
self.path = dest
@ -756,29 +760,27 @@ class Item(LibModel):
self._db._memotable = {}
def move(self, copy=False, link=False, hardlink=False, basedir=None,
def move(self, operation=MoveOperation.MOVE, basedir=None,
with_album=True, store=True):
"""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.
Similarly, `link` creates a symlink instead, and `hardlink`
creates a hardlink.
Instead of moving the item it can also be copied, linked or hardlinked
depending on `operation` which should be an instance of
`util.MoveOperation`.
basedir overrides the library base directory for the
destination.
`basedir` overrides the library base directory for the destination.
If the item is in an album, the album is given an opportunity to
move its art. (This can be disabled by passing
with_album=False.)
If the item is in an album and `with_album` is `True`, the album is
given an opportunity to move its art.
By default, the item is stored to the database if it is in the
database, so any dirty fields prior to the move() call will be written
as a side effect. You probably want to call save() to commit the DB
transaction. If `store` is true however, the item won't be stored, and
you'll have to manually store it after invoking this method.
as a side effect.
If `store` is `False` however, the item won't be stored and you'll
have to manually store it after invoking this method.
"""
self._check_db()
dest = self.destination(basedir=basedir)
@ -788,7 +790,7 @@ class Item(LibModel):
# Perform the move and store the change.
old_path = self.path
self.move_file(dest, copy, link, hardlink)
self.move_file(dest, operation)
if store:
self.store()
@ -796,12 +798,12 @@ class Item(LibModel):
if with_album:
album = self.get_album()
if album:
album.move_art(copy)
album.move_art(operation)
if store:
album.store()
# Prune vacated directory.
if not copy:
if operation == MoveOperation.MOVE:
util.prune_dirs(os.path.dirname(old_path), self._db.directory)
# Templating.
@ -1008,9 +1010,12 @@ class Album(LibModel):
for item in self.items():
item.remove(delete, False)
def move_art(self, copy=False, link=False, hardlink=False):
"""Move or copy any existing album art so that it remains in the
same directory as the items.
def move_art(self, operation=MoveOperation.MOVE):
"""Move, copy, link or hardlink (depending on `operation`) any
existing album art so that it remains in the same directory as
the items.
`operation` should be an instance of `util.MoveOperation`.
"""
old_art = self.artpath
if not old_art:
@ -1024,29 +1029,29 @@ class Album(LibModel):
log.debug(u'moving album art {0} to {1}',
util.displayable_path(old_art),
util.displayable_path(new_art))
if copy:
util.copy(old_art, new_art)
elif link:
util.link(old_art, new_art)
elif hardlink:
util.hardlink(old_art, new_art)
else:
if operation == MoveOperation.MOVE:
util.move(old_art, new_art)
util.prune_dirs(os.path.dirname(old_art), self._db.directory)
elif operation == MoveOperation.COPY:
util.copy(old_art, new_art)
elif operation == MoveOperation.LINK:
util.link(old_art, new_art)
elif operation == MoveOperation.HARDLINK:
util.hardlink(old_art, new_art)
self.artpath = new_art
# Prune old path when moving.
if not copy:
util.prune_dirs(os.path.dirname(old_art),
self._db.directory)
def move(self, operation=MoveOperation.MOVE, basedir=None, store=True):
"""Move, copy, link or hardlink (depending on `operation`)
all items to their destination. Any album art moves along with them.
def move(self, copy=False, link=False, hardlink=False, basedir=None,
store=True):
"""Moves (or copies) all items to their destination. Any album
art moves along with them. basedir overrides the library base
directory for the destination. By default, the album is stored to the
database, persisting any modifications to its metadata. If `store` is
true however, the album is not stored automatically, and you'll have
to manually store it after invoking this method.
`basedir` overrides the library base directory for the destination.
`operation` should be an instance of `util.MoveOperation`.
By default, the album is stored to the database, persisting any
modifications to its metadata. If `store` is `False` however,
the album is not stored automatically, and you'll have to manually
store it after invoking this method.
"""
basedir = basedir or self._db.directory
@ -1058,11 +1063,11 @@ class Album(LibModel):
# Move items.
items = list(self.items())
for item in items:
item.move(copy, link, hardlink, basedir=basedir, with_album=False,
item.move(operation, basedir=basedir, with_album=False,
store=store)
# Move art.
self.move_art(copy, link, hardlink)
self.move_art(operation)
if store:
self.store()

View file

@ -34,7 +34,8 @@ from beets.autotag import hooks
from beets import plugins
from beets import importer
from beets import util
from beets.util import syspath, normpath, ancestry, displayable_path
from beets.util import syspath, normpath, ancestry, displayable_path, \
MoveOperation
from beets import library
from beets import config
from beets import logging
@ -1499,10 +1500,14 @@ def move_items(lib, dest, query, copy, album, pretend, confirm=False,
if export:
# Copy without affecting the database.
obj.move(True, basedir=dest, store=False)
obj.move(operation=MoveOperation.COPY, basedir=dest,
store=False)
else:
# Ordinary move/copy: store the new path.
obj.move(copy, basedir=dest)
if copy:
obj.move(operation=MoveOperation.COPY, basedir=dest)
else:
obj.move(operation=MoveOperation.MOVE, basedir=dest)
def move_func(lib, opts, args):

View file

@ -31,6 +31,7 @@ import shlex
from beets.util import hidden
import six
from unidecode import unidecode
from enum import Enum
MAX_FILENAME_LENGTH = 200
@ -124,6 +125,15 @@ class FilesystemError(HumanReadableException):
return u'{0} {1}'.format(self._reasonstr(), clause)
class MoveOperation(Enum):
"""The file operations that e.g. various move functions can carry out.
"""
MOVE = 0
COPY = 1
LINK = 2
HARDLINK = 3
def normpath(path):
"""Provide the canonical form of the path suitable for storing in
the database.

View file

@ -22,7 +22,7 @@ import shlex
from beets.plugins import BeetsPlugin
from beets.ui import decargs, print_, Subcommand, UserError
from beets.util import command_output, displayable_path, subprocess, \
bytestring_path
bytestring_path, MoveOperation
from beets.library import Item, Album
import six
@ -175,10 +175,10 @@ class DuplicatesPlugin(BeetsPlugin):
"""
print_(format(item, fmt))
if copy:
item.move(basedir=copy, copy=True)
item.move(basedir=copy, operation=MoveOperation.COPY)
item.store()
if move:
item.move(basedir=move, copy=False)
item.move(basedir=move)
item.store()
if delete:
item.remove(delete=True)
@ -312,7 +312,7 @@ class DuplicatesPlugin(BeetsPlugin):
objs[0],
displayable_path(o.path),
displayable_path(missing.destination()))
missing.move(copy=True)
missing.move(operation=MoveOperation.COPY)
return objs
def _merge(self, objs):

View file

@ -50,6 +50,9 @@ For developers:
missing values into type-specific null-like values. This should help in
cases where a string field is unexpectedly `None` sometimes instead of just
showing up as an empty string. :bug:`2605`
* Refactored the move functions in library.py and the `manipulate_files` function
in importer.py to use a single parameter describing the file operation instead
of multiple boolean flags. :bug:`2682`
1.4.5 (June 20, 2017)

View file

@ -5,8 +5,8 @@ logging-clear-handlers=1
[flake8]
min-version=2.7
accept-encodings=utf-8
# Default pyflakes errors we ignore:
# - E241: missing whitespace after ',' (used to align visually)
# Errors we ignore:
# - E121,E123,E126,E226,E24,E704,W503,W504 flake8 default ignores (have to be listed here to not be overridden)
# - E221: multiple spaces before operator (used to align visually)
# - E731: do not assign a lambda expression, use a def
# - F405 object may be undefined, or defined from star imports
@ -19,4 +19,4 @@ accept-encodings=utf-8
# - FI53: `__future__` import "print_function" present
# - FI14: `__future__` import "unicode_literals" missing
# - FI15: `__future__` import "generator_stop" missing
ignore=E305,C901,E241,E221,E731,F405,FI50,FI51,FI12,FI53,FI14,FI15
ignore=E121,E123,E126,E226,E24,E704,W503,W504,E305,C901,E221,E731,F405,FI50,FI51,FI12,FI53,FI14,FI15

View file

@ -52,6 +52,7 @@ from beets import importer
from beets.autotag.hooks import AlbumInfo, TrackInfo
from beets.mediafile import MediaFile, Image
from beets import util
from beets.util import MoveOperation
# TODO Move AutotagMock here
from test import _common
@ -349,7 +350,7 @@ class TestHelper(object):
item['path'] = os.path.join(_common.RSRC,
util.bytestring_path('min.' + extension))
item.add(self.lib)
item.move(copy=True)
item.move(operation=MoveOperation.COPY)
item.store()
return item
@ -370,7 +371,7 @@ class TestHelper(object):
# mtime needs to be set last since other assignments reset it.
item.mtime = 12345
item.add(self.lib)
item.move(copy=True)
item.move(operation=MoveOperation.COPY)
item.store()
items.append(item)
return items
@ -387,7 +388,7 @@ class TestHelper(object):
# mtime needs to be set last since other assignments reset it.
item.mtime = 12345
item.add(self.lib)
item.move(copy=True)
item.move(operation=MoveOperation.COPY)
item.store()
items.append(item)
return self.lib.add_album(items)

View file

@ -27,6 +27,7 @@ from test import _common
from test._common import item, touch
import beets.library
from beets import util
from beets.util import MoveOperation
class MoveTest(_common.TestCase):
@ -78,11 +79,11 @@ class MoveTest(_common.TestCase):
self.assertNotExists(os.path.dirname(old_path))
def test_copy_arrives(self):
self.i.move(copy=True)
self.i.move(operation=MoveOperation.COPY)
self.assertExists(self.dest)
def test_copy_does_not_depart(self):
self.i.move(copy=True)
self.i.move(operation=MoveOperation.COPY)
self.assertExists(self.path)
def test_move_changes_path(self):
@ -92,13 +93,13 @@ class MoveTest(_common.TestCase):
def test_copy_already_at_destination(self):
self.i.move()
old_path = self.i.path
self.i.move(copy=True)
self.i.move(operation=MoveOperation.COPY)
self.assertEqual(self.i.path, old_path)
def test_move_already_at_destination(self):
self.i.move()
old_path = self.i.path
self.i.move(copy=False)
self.i.move()
self.assertEqual(self.i.path, old_path)
def test_read_only_file_copied_writable(self):
@ -106,7 +107,7 @@ class MoveTest(_common.TestCase):
os.chmod(self.path, 0o444)
try:
self.i.move(copy=True)
self.i.move(operation=MoveOperation.COPY)
self.assertTrue(os.access(self.i.path, os.W_OK))
finally:
# Make everything writable so it can be cleaned up.
@ -126,24 +127,24 @@ class MoveTest(_common.TestCase):
@unittest.skipUnless(_common.HAVE_SYMLINK, "need symlinks")
def test_link_arrives(self):
self.i.move(link=True)
self.i.move(operation=MoveOperation.LINK)
self.assertExists(self.dest)
self.assertTrue(os.path.islink(self.dest))
self.assertEqual(os.readlink(self.dest), self.path)
@unittest.skipUnless(_common.HAVE_SYMLINK, "need symlinks")
def test_link_does_not_depart(self):
self.i.move(link=True)
self.i.move(operation=MoveOperation.LINK)
self.assertExists(self.path)
@unittest.skipUnless(_common.HAVE_SYMLINK, "need symlinks")
def test_link_changes_path(self):
self.i.move(link=True)
self.i.move(operation=MoveOperation.LINK)
self.assertEqual(self.i.path, util.normpath(self.dest))
@unittest.skipUnless(_common.HAVE_HARDLINK, "need hardlinks")
def test_hardlink_arrives(self):
self.i.move(hardlink=True)
self.i.move(operation=MoveOperation.HARDLINK)
self.assertExists(self.dest)
s1 = os.stat(self.path)
s2 = os.stat(self.dest)
@ -154,12 +155,12 @@ class MoveTest(_common.TestCase):
@unittest.skipUnless(_common.HAVE_HARDLINK, "need hardlinks")
def test_hardlink_does_not_depart(self):
self.i.move(hardlink=True)
self.i.move(operation=MoveOperation.HARDLINK)
self.assertExists(self.path)
@unittest.skipUnless(_common.HAVE_HARDLINK, "need hardlinks")
def test_hardlink_changes_path(self):
self.i.move(hardlink=True)
self.i.move(operation=MoveOperation.HARDLINK)
self.assertEqual(self.i.path, util.normpath(self.dest))
@ -236,7 +237,7 @@ class AlbumFileTest(_common.TestCase):
def test_albuminfo_move_copies_file(self):
oldpath = self.i.path
self.ai.album = u'newAlbumName'
self.ai.move(True)
self.ai.move(operation=MoveOperation.COPY)
self.ai.store()
self.i.load()
@ -311,7 +312,7 @@ class ArtFileTest(_common.TestCase):
i2.path = self.i.path
i2.artist = u'someArtist'
ai = self.lib.add_album((i2,))
i2.move(True)
i2.move(operation=MoveOperation.COPY)
self.assertEqual(ai.artpath, None)
ai.set_art(newart)
@ -327,7 +328,7 @@ class ArtFileTest(_common.TestCase):
i2.path = self.i.path
i2.artist = u'someArtist'
ai = self.lib.add_album((i2,))
i2.move(True)
i2.move(operation=MoveOperation.COPY)
ai.set_art(newart)
# Set the art again.
@ -341,7 +342,7 @@ class ArtFileTest(_common.TestCase):
i2.path = self.i.path
i2.artist = u'someArtist'
ai = self.lib.add_album((i2,))
i2.move(True)
i2.move(operation=MoveOperation.COPY)
# Copy the art to the destination.
artdest = ai.art_destination(newart)
@ -358,7 +359,7 @@ class ArtFileTest(_common.TestCase):
i2.path = self.i.path
i2.artist = u'someArtist'
ai = self.lib.add_album((i2,))
i2.move(True)
i2.move(operation=MoveOperation.COPY)
# Make a file at the destination.
artdest = ai.art_destination(newart)
@ -382,7 +383,7 @@ class ArtFileTest(_common.TestCase):
i2.path = self.i.path
i2.artist = u'someArtist'
ai = self.lib.add_album((i2,))
i2.move(True)
i2.move(operation=MoveOperation.COPY)
ai.set_art(newart)
mode = stat.S_IMODE(os.stat(ai.artpath).st_mode)

View file

@ -40,7 +40,7 @@ from beets import config
from beets import plugins
from beets.util.confit import ConfigError
from beets import util
from beets.util import syspath
from beets.util import syspath, MoveOperation
class ListTest(unittest.TestCase):
@ -126,7 +126,7 @@ class RemoveTest(_common.TestCase):
item_path = os.path.join(_common.RSRC, b'full.mp3')
self.i = library.Item.from_path(item_path)
self.lib.add(self.i)
self.i.move(True)
self.i.move(operation=MoveOperation.COPY)
def test_remove_items_no_delete(self):
self.io.addinput('y')
@ -509,7 +509,7 @@ class UpdateTest(_common.TestCase):
item_path = os.path.join(_common.RSRC, b'full.mp3')
self.i = library.Item.from_path(item_path)
self.lib.add(self.i)
self.i.move(True)
self.i.move(operation=MoveOperation.COPY)
self.album = self.lib.add_album([self.i])
# Album art.