Skip to content
This repository has been archived by the owner on Mar 15, 2018. It is now read-only.

Commit

Permalink
Updated to write ids.json to packaged apps on upload (bug 814131)
Browse files Browse the repository at this point in the history
  • Loading branch information
robhudson committed Dec 7, 2012
1 parent addc666 commit 95d40e4
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 1 deletion.
27 changes: 27 additions & 0 deletions apps/files/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,33 @@ def watermark(self, user):

return dest

def inject_ids(self):
"""
For packaged webapps, adds a META-INF/ids.json file with a JSON
document like the following:
{"app_id": "6106d3b3-881a-4ddf-9dec-6b2bf8ff8341",
"version_id": 42}
"""
app = self.version.addon
if not (app.type == amo.ADDON_WEBAPP and app.is_packaged):
return

filename = 'META-INF/ids.json'
ids = {
'app_id': app.guid,
'version_id': self.version_id,
}
zf = SafeUnzip(self.file_path, mode='a')
zf.is_valid()

# Check if there's already a META-INF/ids.json.
if filename in [zi.filename for zi in zf.zip.filelist]:

This comment has been minimized.

Copy link
@andymckay

andymckay Dec 7, 2012

Contributor

I thought you could access the file directly with zf.zip.getinfo(filename)

This comment has been minimized.

Copy link
@robhudson

robhudson Dec 7, 2012

Author Member

Oh? That might be more efficient. I can test that out tomorrow morning.

zf.zip.close()

This comment has been minimized.

Copy link
@andymckay

andymckay Dec 7, 2012

Contributor

What happens if someone uploads a packaged app with META-INF/ids.json already existing, we just trust it?

This comment has been minimized.

Copy link
@robhudson

robhudson Dec 7, 2012

Author Member

bug 819173

This comment has been minimized.

Copy link
@andymckay

andymckay Dec 7, 2012

Contributor

good job on filing that

return

zf.zip.writestr('META-INF/ids.json', json.dumps(ids))

This comment has been minimized.

Copy link
@andymckay

andymckay Dec 7, 2012

Contributor

zf.zip.writestr(filename, json.dumps(ids))

zf.zip.close()

This comment has been minimized.

Copy link
@andymckay

andymckay Dec 7, 2012

Contributor

there a close on SafeUnzip now



@receiver(models.signals.post_save, sender=File,
dispatch_uid='cache_localpicker')
Expand Down
4 changes: 3 additions & 1 deletion apps/versions/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ def from_upload(cls, upload, addon, platforms, send_signal=True):
platforms = cls._make_safe_platform_files(platforms)

for platform in platforms:
File.from_upload(upload, v, platform, parse_data=data)
f = File.from_upload(upload, v, platform, parse_data=data)
if addon.type == amo.ADDON_WEBAPP and addon.is_packaged:

This comment has been minimized.

Copy link
@andymckay

andymckay Dec 7, 2012

Contributor

this check is done in inject_ids as well

This comment has been minimized.

Copy link
@robhudson

robhudson Dec 7, 2012

Author Member

Yeah, but this one avoids extra queries since it has addon available already.

This comment has been minimized.

Copy link
@andymckay

andymckay Dec 7, 2012

Contributor

ah, then i'll argue you can move this as a method onto Addon and write a test for it :)

f.inject_ids()

v.disable_old_files()
# After the upload has been copied to all platforms, remove the upload.
Expand Down
16 changes: 16 additions & 0 deletions mkt/developers/tests/test_views_versions.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import json
import zipfile

import mock
from nose.tools import eq_
from pyquery import PyQuery as pq
Expand Down Expand Up @@ -192,6 +195,19 @@ def test_blocklist_on_new_version(self):
assert EscalationQueue.objects.filter(addon=self.app).exists(), (
'App not in escalation queue')

def test_new_version_gets_ids_file(self):
self.app.current_version.update(version='0.9',
created=self.days_ago(1))
self._post(302)
file_ = self.app.versions.latest().files.latest()
filename = 'META-INF/ids.json'

This comment has been minimized.

Copy link
@andymckay

andymckay Dec 7, 2012

Contributor

that could be constant somewhere

zf = zipfile.ZipFile(file_.file_path)
assert filename in [zi.filename for zi in zf.filelist], (
'Expected %s in zip archive but not found.' % filename)
ids = json.loads(zf.read(filename))
eq_(ids['app_id'], self.app.guid)
eq_(ids['version_id'], file_.version_id)


class TestEditVersion(amo.tests.TestCase):
fixtures = ['base/users']
Expand Down
12 changes: 12 additions & 0 deletions mkt/submit/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import json
import os
import shutil
import zipfile

from django.conf import settings
from django.core.files.storage import default_storage as storage
Expand Down Expand Up @@ -461,6 +462,17 @@ def test_packaged_app_not_unique_by_domain(self, _verify):
assert not _verify.called, ('`verify_app_domain` should not be called'
' for packaged apps.')

def test_packaged_app_has_ids_file(self):
app = self.post_addon(data={'packaged': True, 'free': ['free-os']})
file_ = app.versions.latest().files.latest()
filename = 'META-INF/ids.json'
zf = zipfile.ZipFile(file_.file_path)
assert filename in [zi.filename for zi in zf.filelist], (
'Expected %s in zip archive but not found.' % filename)

This comment has been minimized.

Copy link
@andymckay

andymckay Dec 7, 2012

Contributor

both these tests might be a bit faster if you use getinfo as well

ids = json.loads(zf.read(filename))
eq_(ids['app_id'], app.guid)
eq_(ids['version_id'], file_.version_id)


class TestDetails(TestSubmit):
fixtures = ['base/apps', 'base/users', 'webapps/337141-steamcube']
Expand Down

0 comments on commit 95d40e4

Please sign in to comment.