update ids.json immediately before signing (bug 873600)

apps/files/models.py

@@ -512,14 +512,6 @@ class File(amo.models.OnChangeMixin, amo.models.ModelBase):
         zf = SafeUnzip(self.file_path, mode='a')
         zf.is_valid()
 
-        # Check if there's already a META-INF/ids.json.
-        try:
-            zf.zip.getinfo(filename)
-            zf.close()
-            return
-        except KeyError:
-            pass  # Not found, we need to add it.
-
         zf.zip.writestr(filename, json.dumps(ids))
         zf.close()
 

apps/versions/models.py

@@ -122,9 +122,6 @@ class Version(amo.models.ModelBase):
                 eta=datetime.datetime.now() +
                     datetime.timedelta(seconds=settings.NFS_LAG_DELAY))
 
-            if addon.is_packaged:
-                f.inject_ids()
-
         v.disable_old_files()
         # After the upload has been copied to all platforms, remove the upload.
         storage.delete(upload.path)

lib/crypto/packaged.py

@@ -136,6 +136,8 @@ def sign(version_id, reviewer=False, resign=False, **kw):
         log.info('Already signed app exists.')
         return path
 
+    file_obj.inject_ids()
+
     with statsd.timer('services.sign.app'):
         try:
             sign_app(file_obj.file_path, path, reviewer)

lib/crypto/tests.py

@@ -2,6 +2,7 @@
 import json
 import os
 import shutil
+import zipfile
 
 from django.conf import settings  # For mocking.
 from django.core.files.storage import default_storage as storage
@@ -176,3 +177,20 @@ class TestPackaged(PackagedApp, amo.tests.TestCase):
             endpoint = packaged._get_endpoint(reviewer=True)
         assert endpoint.startswith('http://review.me'), (
             'Unexpected endpoint returned.')
+
+    @mock.patch('lib.crypto.packaged.sign_app')
+    def test_inject_ids(self, sign):
+        packaged.sign(self.version.pk)
+        zf = zipfile.ZipFile(self.file.file_path, mode='r')
+        ids_data = zf.read('META-INF/ids.json')
+        eq_(sorted(json.loads(ids_data).keys()), ['id', 'version'])
+
+    @mock.patch('lib.crypto.packaged.sign_app')
+    def test_inject_ids_replace(self, sign):
+        zf = zipfile.ZipFile(self.file.file_path, mode='a')
+        zf.writestr('META-INF/ids.json', '{}')
+        zf.close()
+        packaged.sign(self.version.pk)
+        zf = zipfile.ZipFile(self.file.file_path, mode='r')
+        ids_data = zf.read('META-INF/ids.json')
+        eq_(sorted(json.loads(ids_data).keys()), ['id', 'version'])

mkt/webapps/models.py

@@ -776,7 +776,6 @@ class Webapp(Addon):
         f.size = storage.size(f.file_path)
         f.hash = f.generate_hash(f.file_path)
         f.save()
-        f.inject_ids()
         self.sign_if_packaged(v.pk)
         self.status = amo.STATUS_BLOCKED
         self._current_version = v