From 574903e9861d1330c85c6052b7bf1652502daff7 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Mon, 24 Mar 2014 14:22:27 +0100 Subject: [PATCH] Add FileOperationError and handling in item.write() --- beets/importer.py | 6 +++++- beets/library.py | 46 ++++++++++++++++++++------------------------ beets/ui/commands.py | 12 ++++++++++-- docs/dev/plugins.rst | 5 +++-- test/test_library.py | 16 +++++++++++++++ 5 files changed, 55 insertions(+), 30 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 93d2bbed5..c002779df 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -894,7 +894,11 @@ def manipulate_files(session): item.move(True) if config['import']['write'] and task.should_write_tags(): - item.write() + try: + item.write() + except library.FileOperationError as exc: + log.error(u'could not write {0}: {1}'.format( + util.displayable_path(item.path), exc)) # Save new paths. with session.lib.transaction(): diff --git a/beets/library.py b/beets/library.py index 100c0201b..48806405e 100644 --- a/beets/library.py +++ b/beets/library.py @@ -23,7 +23,7 @@ import unicodedata import traceback import time from unidecode import unidecode -from beets.mediafile import MediaFile +from beets.mediafile import MediaFile, MutagenError from beets import plugins from beets import util from beets.util import bytestring_path, syspath, normpath, samefile @@ -283,6 +283,16 @@ class LibModel(dbcore.Model): super(LibModel, self).add(lib) plugins.send('database_change', lib=self._db) +class FileOperationError(Exception): + """Raised by ``item.write()`` to indicate an error when interacting + with the file. + """ + +class ReadError(FileOperationError): + pass + +class WriteError(FileOperationError): + pass class Item(LibModel): _fields = dict((name, typ) for (name, typ, _, _) in ITEM_FIELDS) @@ -342,6 +352,8 @@ class Item(LibModel): def read(self, read_path=None): """Read the metadata from the associated file. If read_path is specified, read metadata from that file instead. + + Raises ``ReadError`` if the file could not be read. """ if read_path is None: read_path = self.path @@ -350,8 +362,7 @@ class Item(LibModel): try: f = MediaFile(syspath(read_path)) except (OSError, IOError) as exc: - raise util.FilesystemError(exc, 'read', (read_path,), - traceback.format_exc()) + raise ReadError(exc.message) for key in ITEM_KEYS_META: value = getattr(f, key) @@ -371,42 +382,27 @@ class Item(LibModel): self.path = read_path def write(self): - """Try to write the item's metadata to the associated file. + """Write the item's metadata to the associated file. - Returns ``True`` if the write was successful. The method catches - file system read and write exceptions and logs an error message. - If any of 'write' event handlers returns a truthy value the - write will not be performed and an error message is logged. + Raises ``ReadError`` or ``WriteError``. """ - if any(plugins.send('write', item=self)): - log.error(u'plugin aborted writing {0}'.format( - util.displayable_path(item.path))) - return - - try: f = MediaFile(syspath(self.path)) except (OSError, IOError) as exc: - log.error(u'could not read {0}: {1}'.format( - util.displayable_path(item.path), exc - )) - return + raise ReadError(str(exc)) + + plugins.send('write', item=self) for key in ITEM_KEYS_WRITABLE: setattr(f, key, self[key]) - try: f.save(id3v23=beets.config['id3v23'].get(bool)) - except (OSError, IOError) as exc: - log.error(u'could not write {0}: {1}'.format( - util.displayable_path(item.path), exc - )) - return + except (OSError, IOError, MutagenError) as exc: + raise WriteError(str(exc)) # The file has a new mtime. self.mtime = self.current_mtime() plugins.send('after_write', item=self) - return True # Files themselves. diff --git a/beets/ui/commands.py b/beets/ui/commands.py index d75e43e77..40c13a998 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -1134,7 +1134,11 @@ def modify_items(lib, mods, query, write, move, album, confirm): else: changed_items = changed for item in changed_items: - item.write() + try: + item.write() + except library.FileOperationError as exc: + log.error(u'could not write {0}: {1}'.format( + util.displayable_path(item.path), exc)) modify_cmd = ui.Subcommand('modify', help='change metadata fields', aliases=('mod',)) @@ -1231,7 +1235,11 @@ def write_items(lib, query, pretend): changed = ui.show_model_changes(item, clean_item, library.ITEM_KEYS_WRITABLE, always=True) if changed and not pretend: - item.write() + try: + item.write() + except library.FileOperationError as exc: + log.error(u'could not write {0}: {1}'.format( + util.displayable_path(item.path), exc)) write_cmd = ui.Subcommand('write', help='write tag information to files') write_cmd.parser.add_option('-p', '--pretend', action='store_true', diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index 408c002c3..cdea8938a 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -142,8 +142,9 @@ currently available are: * *write*: called with an ``Item`` object just before a file's metadata is written to disk (i.e., just before the file on disk is opened). Event - handlers may return a truthy value to prevent beets from writing the - file. In that case make sure that you log an appropriate message. + handlers may raise a ``library.FileOperationError`` exception to abort + the write operation. Beets will catch that exception, print an error + message and continue. * *after_write*: called with an ``Item`` object after a file's metadata is written to disk (i.e., just after the file on disk is closed). diff --git a/test/test_library.py b/test/test_library.py index 8eabb4122..3fa3d1173 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -15,6 +15,8 @@ """Tests for non-query database functions of Item. """ import os +import os.path +import stat import shutil import re import unicodedata @@ -942,6 +944,20 @@ class TemplateTest(_common.LibTestCase): self.album.store() self.assertEqual(self.i.evaluate_template('$foo'), 'baz') +class WriteTest(_common.LibTestCase): + + def test_write_nonexistant(self): + self.i.path = '/path/does/not/exist' + self.assertRaises(beets.library.ReadError, self.i.write) + + def test_no_write_permission(self): + path = os.path.join(self.temp_dir, 'file.mp3') + shutil.copy(os.path.join(_common.RSRC, 'empty.mp3'), path) + os.chmod(path, stat.S_IRUSR) + + self.i.path = path + self.assertRaises(beets.library.WriteError, self.i.write) + def suite(): return unittest.TestLoader().loadTestsFromName(__name__)