Merge pull request #1294 from sampsyo/write-tags

Extend item.write() to embed images without changing item

beets/library.py

@@ -500,12 +500,18 @@ class Item(LibModel):
 
         self.path = read_path
 
-    def write(self, path=None):
+    def write(self, path=None, tags=None):
         """Write the item's metadata to a media file.
 
         All fields in `_media_fields` are written to disk according to
         the values on this object.
 
+        `path` is the path of the mediafile to wirte the data to. It
+        defaults to the item's path.
+
+        `tags` is a dictionary of additional metadata the should be
+        written to the file.
+
         Can raise either a `ReadError` or a `WriteError`.
         """
         if path is None:
@@ -513,8 +519,10 @@ class Item(LibModel):
         else:
             path = normpath(path)
 
-        tags = dict(self)
-        plugins.send('write', item=self, path=path, tags=tags)
+        item_tags = dict(self)
+        if tags is not None:
+            item_tags.update(tags)
+        plugins.send('write', item=self, path=path, tags=item_tags)
 
         try:
             mediafile = MediaFile(syspath(path),
@@ -522,7 +530,7 @@ class Item(LibModel):
         except (OSError, IOError, UnreadableFileError) as exc:
             raise ReadError(self.path, exc)
 
-        mediafile.update(tags)
+        mediafile.update(item_tags)
         try:
             mediafile.save()
         except (OSError, IOError, MutagenError) as exc:
@@ -533,14 +541,14 @@ class Item(LibModel):
             self.mtime = self.current_mtime()
         plugins.send('after_write', item=self, path=path)
 
-    def try_write(self, path=None):
+    def try_write(self, path=None, tags=None):
         """Calls `write()` but catches and logs `FileOperationError`
         exceptions.
 
         Returns `False` an exception was caught and `True` otherwise.
         """
         try:
-            self.write(path)
+            self.write(path, tags)
             return True
         except FileOperationError as exc:
             log.error("{0}", exc)

beetsplug/embedart.py

@@ -148,13 +148,11 @@ class EmbedCoverArtPlugin(BeetsPlugin):
 
         try:
             self._log.debug(u'embedding {0}', displayable_path(imagepath))
-            item['images'] = [self._mediafile_image(imagepath, maxwidth)]
+            image = self._mediafile_image(imagepath, maxwidth)
         except IOError as exc:
             self._log.warning(u'could not read image file: {0}', exc)
-        else:
-            # We don't want to store the image in the database.
-            item.try_write(itempath)
-            del item['images']
+            return
+        item.try_write(path=itempath, tags={'images': [image]})
 
     def embed_album(self, album, maxwidth=None, quiet=False):
         """Embed album art into all of the album's items.

test/test_library.py

@@ -35,6 +35,7 @@ from beets import util
 from beets import plugins
 from beets import config
 from beets.mediafile import MediaFile
+from test.helper import TestHelper
 
 # Shortcut to path normalization.
 np = util.normpath
@@ -1109,37 +1110,49 @@ class UnicodePathTest(_common.LibTestCase):
         self.i.write()
 
 
-class WriteTest(_common.LibTestCase):
+class WriteTest(unittest.TestCase, TestHelper):
+    def setUp(self):
+        self.setup_beets()
+
+    def tearDown(self):
+        self.teardown_beets()
+
     def test_write_nonexistant(self):
-        self.i.path = '/path/does/not/exist'
-        self.assertRaises(beets.library.ReadError, self.i.write)
+        item = self.create_item()
+        item.path = '/path/does/not/exist'
+        with self.assertRaises(beets.library.ReadError):
+            item.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)
+        item = self.add_item_fixture()
+        path = item.path
         os.chmod(path, stat.S_IRUSR)
 
         try:
-            self.i.path = path
-            self.assertRaises(beets.library.WriteError, self.i.write)
+            self.assertRaises(beets.library.WriteError, item.write)
 
         finally:
             # Restore write permissions so the file can be cleaned up.
             os.chmod(path, stat.S_IRUSR | stat.S_IWUSR)
 
     def test_write_with_custom_path(self):
-        custom_path = os.path.join(self.temp_dir, 'file.mp3')
-        self.i.path = os.path.join(self.temp_dir, 'item_file.mp3')
-        shutil.copy(os.path.join(_common.RSRC, 'empty.mp3'), custom_path)
-        shutil.copy(os.path.join(_common.RSRC, 'empty.mp3'), self.i.path)
+        item = self.add_item_fixture()
+        custom_path = os.path.join(self.temp_dir, 'custom.mp3')
+        shutil.copy(item.path, custom_path)
 
-        self.i['artist'] = 'new artist'
+        item['artist'] = 'new artist'
         self.assertNotEqual(MediaFile(custom_path).artist, 'new artist')
-        self.assertNotEqual(MediaFile(self.i.path).artist, 'new artist')
+        self.assertNotEqual(MediaFile(item.path).artist, 'new artist')
 
-        self.i.write(custom_path)
+        item.write(custom_path)
         self.assertEqual(MediaFile(custom_path).artist, 'new artist')
-        self.assertNotEqual(MediaFile(self.i.path).artist, 'new artist')
+        self.assertNotEqual(MediaFile(item.path).artist, 'new artist')
+
+    def test_write_custom_tags(self):
+        item = self.add_item_fixture(artist='old artist')
+        item.write(tags={'artist': 'new artist'})
+        self.assertNotEqual(item.artist, 'new artist')
+        self.assertEqual(MediaFile(item.path).artist, 'new artist')
 
 
 class ItemReadTest(unittest.TestCase):