diff --git a/beets/importer.py b/beets/importer.py index 00510e47d..a01c21d1e 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -895,13 +895,9 @@ def manipulate_files(session): if config['import']['write'] and task.should_write_tags(): try: item.write() - except mediafile.UnreadableFileError as exc: - log.error(u'error while writing ({0}): {0}'.format( - exc, - util.displayable_path(item.path) - )) - except util.FilesystemError as exc: - exc.log(log) + 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 49ba4a9ca..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,24 +382,23 @@ class Item(LibModel): self.path = read_path def write(self): - """Writes the item's metadata to the associated file. - """ - plugins.send('write', item=self) + """Write the item's metadata to the associated file. + Raises ``ReadError`` or ``WriteError``. + """ try: f = MediaFile(syspath(self.path)) except (OSError, IOError) as exc: - raise util.FilesystemError(exc, 'read', (self.path,), - traceback.format_exc()) + 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: - raise util.FilesystemError(exc, 'write', (self.path,), - traceback.format_exc()) + except (OSError, IOError, MutagenError) as exc: + raise WriteError(str(exc)) # The file has a new mtime. self.mtime = self.current_mtime() diff --git a/beets/plugins.py b/beets/plugins.py index 813084ca9..6a58777cd 100755 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -353,10 +353,7 @@ def send(event, **arguments): name of the event to send, all other named arguments go to the event handler(s). - Returns the number of handlers called. + Returns a list of return values from the handlers. """ log.debug('Sending event: %s' % event) - handlers = event_handlers()[event] - for handler in handlers: - handler(**arguments) - return len(handlers) + return [handler(**arguments) for handler in event_handlers()[event]] diff --git a/beets/ui/commands.py b/beets/ui/commands.py index de6349532..40c13a998 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -77,7 +77,6 @@ def _do_query(lib, query, album, also_items=True): return items, albums - # fields: Shows a list of available fields for queries and format strings. fields_cmd = ui.Subcommand('fields', @@ -1135,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',)) @@ -1234,11 +1237,9 @@ def write_items(lib, query, pretend): if changed and not pretend: try: item.write() - except Exception as exc: + except library.FileOperationError as exc: log.error(u'could not write {0}: {1}'.format( - util.displayable_path(item.path), exc - )) - continue + 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 f21f41a38..cdea8938a 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -141,7 +141,10 @@ currently available are: deleted from disk). * *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). + written to disk (i.e., just before the file on disk is opened). Event + 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__)