[extensions-web] Fix metadata.json replacement in the zipfile and add tests.

[Jasper St. Pierre <jstpierre src gnome org>]

commit 074b7823eca4091859c96dc8b1bc0660bfefee61
Author: Jasper St. Pierre <jstpierre mecheye net>
Date:   Tue Sep 27 15:22:35 2011 -0400

    Fix metadata.json replacement in the zipfile and add tests.

 sweettooth/extensions/models.py                    |   26 +++++++++-
 .../extensions/testdata/LotsOfFiles/Craig.jpg      |  Bin 0 -> 80711 bytes
 .../testdata/LotsOfFiles/LotsOfFiles.zip           |  Bin 0 -> 82309 bytes
 sweettooth/extensions/testdata/LotsOfFiles/README  |    4 ++
 .../extensions/testdata/LotsOfFiles/extension.js   |    7 +++
 .../extensions/testdata/LotsOfFiles/metadata.json  |    6 ++
 .../testdata/LotsOfFiles/subdirectory/test.txt     |    3 +
 .../LotsOfFiles/subdirectory/test2/test.txt        |    2 +
 sweettooth/extensions/tests.py                     |   50 +++++++++++++++++++-
 9 files changed, 95 insertions(+), 3 deletions(-)
---
diff --git a/sweettooth/extensions/models.py b/sweettooth/extensions/models.py
index 92a9002..0be22d9 100644
--- a/sweettooth/extensions/models.py
+++ b/sweettooth/extensions/models.py
@@ -193,6 +193,9 @@ class ExtensionVersion(models.Model):
         data.update(fields)
         return data
 
+    def make_metadata_json_string(self):
+        return json.dumps(self.make_metadata_json(), sort_keys=True, indent=2)
+
     def get_zipfile(self, mode):
         return ZipFile(self.source.storage.path(self.source.name), mode)
 
@@ -201,9 +204,28 @@ class ExtensionVersion(models.Model):
         In the uploaded extension zipfile, edit metadata.json
         to reflect the new contents.
         """
-        zipfile = self.get_zipfile("a")
+
+        # We can't easily *replace* files in a zipfile
+        # archive. See http://bugs.python.org/issue6818.
+        # Just read all the contents from the old zipfile
+        # into memory and then emit a new one with the
+        # generated metadata.json
+        zipfile_in = self.get_zipfile("r")
+
+        filemap = {}
+        for info in zipfile_in.infolist():
+            if info.filename == "metadata.json":
+                continue
+
+            contents = zipfile_in.read(info)
+            filemap[info] = contents
+
+        zipfile = self.get_zipfile("w")
+        for info, contents in filemap.iteritems():
+            zipfile.writestr(info, contents)
+
         metadata = self.make_metadata_json()
-        zipfile.writestr("metadata.json", json.dumps(metadata, sort_keys=True, indent=2))
+        zipfile.writestr("metadata.json", self.make_metadata_json_string())
         zipfile.close()
 
     def parse_metadata_json(self, metadata):
diff --git a/sweettooth/extensions/testdata/LotsOfFiles/Craig.jpg b/sweettooth/extensions/testdata/LotsOfFiles/Craig.jpg
new file mode 100644
index 0000000..ead9ce9
Binary files /dev/null and b/sweettooth/extensions/testdata/LotsOfFiles/Craig.jpg differ
diff --git a/sweettooth/extensions/testdata/LotsOfFiles/LotsOfFiles.zip b/sweettooth/extensions/testdata/LotsOfFiles/LotsOfFiles.zip
new file mode 100644
index 0000000..51eee66
Binary files /dev/null and b/sweettooth/extensions/testdata/LotsOfFiles/LotsOfFiles.zip differ
diff --git a/sweettooth/extensions/testdata/LotsOfFiles/README b/sweettooth/extensions/testdata/LotsOfFiles/README
new file mode 100644
index 0000000..7b8111f
--- /dev/null
+++ b/sweettooth/extensions/testdata/LotsOfFiles/README
@@ -0,0 +1,4 @@
+This is intended to test the metadata zipfile replacement with binary files and subdirs.
+
+Craig.jpg is an old file on my disk that's my go-to "I need an image to test with" file.
+I have no idea about its origins. if you know, I'd love to know.
diff --git a/sweettooth/extensions/testdata/LotsOfFiles/extension.js b/sweettooth/extensions/testdata/LotsOfFiles/extension.js
new file mode 100644
index 0000000..bf9c324
--- /dev/null
+++ b/sweettooth/extensions/testdata/LotsOfFiles/extension.js
@@ -0,0 +1,7 @@
+// This is a dumb file.
+
+function init() {
+}
+
+var enable, disable;
+enable = disable = init;
diff --git a/sweettooth/extensions/testdata/LotsOfFiles/metadata.json b/sweettooth/extensions/testdata/LotsOfFiles/metadata.json
new file mode 100644
index 0000000..590a368
--- /dev/null
+++ b/sweettooth/extensions/testdata/LotsOfFiles/metadata.json
@@ -0,0 +1,6 @@
+{
+    "uuid": "test-extension gnome org",
+    "name": "Test Extension",
+    "description": "Simple test metadata",
+    "url": "http://test-metadata.gnome.org";
+}
diff --git a/sweettooth/extensions/testdata/LotsOfFiles/subdirectory/test.txt b/sweettooth/extensions/testdata/LotsOfFiles/subdirectory/test.txt
new file mode 100644
index 0000000..89ac2b1
--- /dev/null
+++ b/sweettooth/extensions/testdata/LotsOfFiles/subdirectory/test.txt
@@ -0,0 +1,3 @@
+
+testing subdirs
+
diff --git a/sweettooth/extensions/testdata/LotsOfFiles/subdirectory/test2/test.txt b/sweettooth/extensions/testdata/LotsOfFiles/subdirectory/test2/test.txt
new file mode 100644
index 0000000..0d21fec
--- /dev/null
+++ b/sweettooth/extensions/testdata/LotsOfFiles/subdirectory/test2/test.txt
@@ -0,0 +1,2 @@
+
+testing one more time
diff --git a/sweettooth/extensions/tests.py b/sweettooth/extensions/tests.py
index 0fbb218..c7b4835 100644
--- a/sweettooth/extensions/tests.py
+++ b/sweettooth/extensions/tests.py
@@ -1,9 +1,12 @@
 
 import json
 import os.path
+import tempfile
+from zipfile import ZipFile
 
 from django.test import TestCase
 from django.test.client import Client
+from django.core.files.base import File
 from django.core.urlresolvers import reverse
 from django.contrib.auth.models import User
 from extensions import models
@@ -11,7 +14,13 @@ from extensions import models
 testdata_dir = os.path.join(os.path.dirname(__file__), 'testdata')
 
 def get_test_zipfile(testname):
-    return open(os.path.join(testdata_dir, testname, testname + ".zip"), 'rb')
+    original = open(os.path.join(testdata_dir, testname, testname + ".zip"), 'rb')
+    new_temp = tempfile.NamedTemporaryFile(suffix=testname + ".zip.temp")
+    new_temp.write(original.read())
+    new_temp.seek(0)
+    original.close()
+
+    return new_temp
 
 class UploadTest(TestCase):
     def setUp(self):
@@ -63,6 +72,45 @@ class UploadTest(TestCase):
         self.assertTrue("description" not in extra)
         self.assertTrue("url" not in extra)
 
+class ReplaceMetadataTest(TestCase):
+    def setUp(self):
+        self.username = 'TestUser1'
+        self.email = 'non-existant non-existant tld'
+        self.password = 'a random password'
+        self.user = User.objects.create_user(self.username, self.email, self.password)
+
+    def test_replace_metadata(self):
+        extension = models.Extension(creator=self.user)
+        version = models.ExtensionVersion()
+        version.extension = extension
+
+        old_zip_file = get_test_zipfile('LotsOfFiles')
+
+        metadata = models.parse_zipfile_metadata(old_zip_file)
+        old_zip_file.seek(0)
+
+        version.source = File(old_zip_file)
+
+        version.parse_metadata_json(metadata)
+        version.replace_metadata_json()
+        new_zip = version.get_zipfile('r')
+
+        old_zip = ZipFile(File(old_zip_file), 'r')
+        self.assertEqual(len(old_zip.infolist()), len(new_zip.infolist()))
+        self.assertEqual(new_zip.read("metadata.json"),
+                         version.make_metadata_json_string())
+
+        for old_info in old_zip.infolist():
+            if old_info.filename == "metadata.json":
+                continue
+
+            new_info = new_zip.getinfo(old_info.filename)
+            self.assertEqual(old_zip.read(old_info), new_zip.read(new_info))
+            self.assertEqual(old_info.date_time, new_info.date_time)
+
+        old_zip.close()
+        new_zip.close()
+
 class UploadTest(TestCase):
     def setUp(self):
         self.username = 'TestUser1'