Merge pull request #27 from wisp3rwind/pr-fix-format-change

Don't corrupt files on format change

CHANGELOG.md

@@ -9,6 +9,8 @@ Change Log
   with current beets)
 * Bugfix: Explicitly write tags after encoding instead of relying on the
   encoder to do so
+* Bugfix: If the `formats` config option is modified, don't move files if the
+  extension would change, but re-encode
 
 ## v0.8.2 - 2015-05-31
 * Fix a bug that made the plugin crash when reading unicode strings

beetsplug/alternatives.py

@@ -119,9 +119,14 @@ def parse_config(self, config):
 
     def matched_item_action(self, item):
         path = self.get_path(item)
-        actions = []
         if path and os.path.isfile(syspath(path)):
             dest = self.destination(item)
+            _, path_ext = os.path.splitext(path)
+            _, dest_ext = os.path.splitext(dest)
+            if not path_ext == dest_ext:
+                # formats config option changed
+                return (item, [self.REMOVE, self.ADD])
+            actions = []
             if not util.samefile(path, dest):
                 actions.append(self.MOVE)
             item_mtime_alt = os.path.getmtime(syspath(path))
@@ -134,9 +139,9 @@ def matched_item_action(self, item):
                         (item_mtime_alt
                          < os.path.getmtime(syspath(album.artpath)))):
                     actions.append(self.SYNC_ART)
+            return (item, actions)
         else:
-            actions.append(self.ADD)
-        return (item, actions)
+            return (item, [self.ADD])
 
     def items_actions(self):
         matched_ids = set()

test/cli_test.py

@@ -306,7 +306,7 @@ def setUp(self):
         self.external_config = self.config['alternatives']['myexternal']
 
     def test_convert(self):
-        item = self.add_track(myexternal='true', format='mp4')
+        item = self.add_track(myexternal='true', format='m4a')
         self.runcli('alt', 'update', 'myexternal')
         item.load()
         converted_path = self.get_path(item)
@@ -324,7 +324,7 @@ def test_convert_and_embed(self):
         self.assertHasEmbeddedArtwork(self.get_path(item))
 
     def test_convert_write_tags(self):
-        item = self.add_track(myexternal='true', format='mp4', title=u'TITLE')
+        item = self.add_track(myexternal='true', format='m4a', title=u'TITLE')
 
         # We "convert" by copying the file. Setting the title simulates
         # a badly behaved converter
@@ -356,6 +356,22 @@ def test_skip_convert_for_alternative_format(self):
         converted_path = self.get_path(item)
         self.assertNotFileTag(converted_path, b'ISOGG')
 
+    def test_no_move_on_extension_change(self):
+        item = self.add_track(myexternal='true', format='m4a')
+        self.runcli('alt', 'update', 'myexternal')
+
+        self.config['convert']['formats'] = {
+            'mp3': 'bash -c "cp \'$source\' \'$dest\';' +
+                   'printf ISMP3 >> \'$dest\'"'
+        }
+        self.config['alternatives']['myexternal']['formats'] = 'mp3'
+
+        # Assert that this re-encodes instead of copying the ogg file
+        self.runcli('alt', 'update', 'myexternal')
+        item.load()
+        converted_path = self.get_path(item)
+        self.assertFileTag(converted_path, b'ISMP3')
+
 
 class ExternalRemovableTest(TestHelper):
 

test/helper.py

@@ -283,11 +283,15 @@ def add_track(self, **kwargs):
             'title': 'track 1',
             'artist': 'artist 1',
             'album': 'album 1',
+            'format': 'mp3',
         }
         values.update(kwargs)
+        assert(values['format'] in 'mp3 m4a ogg'.split())
 
-        item = Item.from_path(os.path.join(self.fixture_dir,
-                                           bytestring_path('min.mp3')))
+        item = Item.from_path(os.path.join(
+            self.fixture_dir,
+            bytestring_path('min.' + values['format'])
+        ))
         item.add(self.lib)
         item.update(values)
         item.move(MoveOperation.COPY)