From 2a824713c35dec1ce6c0a4158f71f70ea89ce711 Mon Sep 17 00:00:00 2001 From: Andy McKay Date: Wed, 15 Aug 2012 11:39:36 -0700 Subject: [PATCH] allow the install of packaged apps (bug 777144) --- apps/files/models.py | 8 ++++--- apps/files/tests/test_models.py | 10 +++++++++ media/js/mkt/apps.js | 11 +++++++++- mkt/downloads/__init__.py | 0 mkt/downloads/tests/__init__.py | 0 mkt/downloads/tests/test_views.py | 35 +++++++++++++++++++++++++++++++ mkt/downloads/urls.py | 11 ++++++++++ mkt/downloads/views.py | 20 ++++++++++++++++++ mkt/site/helpers.py | 11 +++++++--- mkt/site/tests/test_helpers.py | 22 ++++++++++++++----- mkt/urls.py | 4 +++- 11 files changed, 119 insertions(+), 13 deletions(-) create mode 100644 mkt/downloads/__init__.py create mode 100644 mkt/downloads/tests/__init__.py create mode 100644 mkt/downloads/tests/test_views.py create mode 100644 mkt/downloads/urls.py create mode 100644 mkt/downloads/views.py diff --git a/apps/files/models.py b/apps/files/models.py index 0734c4cccf..dc1fa36707 100644 --- a/apps/files/models.py +++ b/apps/files/models.py @@ -99,7 +99,9 @@ class File(amo.models.OnChangeMixin, amo.models.ModelBase): return True def is_mirrorable(self): - if self.version.addon_id and self.version.addon.is_premium(): + if (self.version.addon_id and + (self.version.addon.is_premium() or + self.version.addon.type == amo.ADDON_WEBAPP)): return False return self.status in amo.MIRROR_STATUSES @@ -143,7 +145,7 @@ class File(amo.models.OnChangeMixin, amo.models.ModelBase): # In most cases we have an addon in the calling context. if not addon: addon = self.version.addon - if addon.is_premium(): + if addon.is_premium() and addon.type != amo.ADDON_WEBAPP: url = reverse('downloads.watermarked', args=[self.id]) else: url = reverse('downloads.file', args=[self.id]) @@ -272,7 +274,7 @@ class File(amo.models.OnChangeMixin, amo.models.ModelBase): kw = {'addon_id': addon.pk} if self.platform_id != amo.PLATFORM_ALL.id: kw['platform'] = self.platform_id - if addon.is_premium(): + if addon.is_premium() and addon.type != amo.ADDON_WEBAPP: url = reverse('downloads.watermarked', args=[self.id]) else: url = reverse('downloads.latest', kwargs=kw) diff --git a/apps/files/tests/test_models.py b/apps/files/tests/test_models.py index b630069a40..78834eab4c 100644 --- a/apps/files/tests/test_models.py +++ b/apps/files/tests/test_models.py @@ -329,6 +329,11 @@ class TestFile(amo.tests.TestCase, amo.tests.AMOPaths): f.version.addon.premium_type = amo.ADDON_PREMIUM eq_(f.is_mirrorable(), False) + def test_dont_mirror_apps(self): + f = File.objects.get(pk=67442) + f.version.addon.update(type=amo.ADDON_WEBAPP) + eq_(f.is_mirrorable(), False) + class TestParseXpi(amo.tests.TestCase): fixtures = ['base/apps'] @@ -1124,6 +1129,11 @@ class TestWatermark(amo.tests.TestCase, amo.tests.AMOPaths): self.file.version.addon.update(premium_type=amo.ADDON_PREMIUM) assert url in self.file.latest_xpi_url() + def test_dont_watermark_apps(self): + url = reverse('downloads.watermarked', args=[self.file.id]) + self.file.version.addon.update(type=amo.ADDON_WEBAPP) + assert url not in self.file.latest_xpi_url() + class TestWatermarkCleanup(amo.tests.TestCase, amo.tests.AMOPaths): fixtures = ['base/addon_3615', 'base/users'] diff --git a/media/js/mkt/apps.js b/media/js/mkt/apps.js index 624a7b9444..e16dd27b64 100644 --- a/media/js/mkt/apps.js +++ b/media/js/mkt/apps.js @@ -38,11 +38,20 @@ exports.install = function(product, opt) { var self = apps, errSummary, manifestUrl = product.manifestUrl, + package_url = product.package_url, $def = $.Deferred(); /* Try and install the app. */ if (manifestUrl && opt.navigator.mozApps && opt.navigator.mozApps.install) { - var installRequest = opt.navigator.mozApps.install(manifestUrl, opt.data); + /* TODO: combine the following check with the above. But don't stop + * clients without installPackage from using apps for now. + */ + if (product.is_packaged && !opt.navigator.mozApps.installPackage && !package_url) { + $def.reject(); + } + var installRequest = product.is_packaged ? + opt.navigator.mozApps.installPackage(manifestUrl, opt.data) : + opt.navigator.mozApps.install(package_url, opt.data); installRequest.onsuccess = function() { $def.resolve(product); }; diff --git a/mkt/downloads/__init__.py b/mkt/downloads/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/mkt/downloads/tests/__init__.py b/mkt/downloads/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/mkt/downloads/tests/test_views.py b/mkt/downloads/tests/test_views.py new file mode 100644 index 0000000000..a7d3f4a83e --- /dev/null +++ b/mkt/downloads/tests/test_views.py @@ -0,0 +1,35 @@ +from nose.tools import eq_ + +from addons.models import Addon +import amo +import amo.tests +from amo.urlresolvers import reverse + + +class TestDownload(amo.tests.TestCase): + fixtures = ['base/users', 'webapps/337141-steamcube'] + + def setUp(self): + self.webapp = Addon.objects.get(pk=337141) + self.file = self.webapp.get_latest_file() + self.file.update(is_packaged=True) + self.url = reverse('downloads.file', args=[self.file.pk]) + + def test_download(self): + res = self.client.get(self.url) + eq_(res.status_code, 200) + assert 'x-sendfile' in res._headers + + def test_disabled(self): + self.webapp.update(status=amo.STATUS_DISABLED) + eq_(self.client.get(self.url).status_code, 404) + + def test_disabled_but_owner(self): + self.client.login(username='steamcube@mozilla.com', + password='password') + eq_(self.client.get(self.url).status_code, 200) + + def test_disabled_but_admin(self): + self.client.login(username='admin@mozilla.com', + password='password') + eq_(self.client.get(self.url).status_code, 200) diff --git a/mkt/downloads/urls.py b/mkt/downloads/urls.py new file mode 100644 index 0000000000..9900f823f0 --- /dev/null +++ b/mkt/downloads/urls.py @@ -0,0 +1,11 @@ +from django.conf.urls.defaults import patterns, url + +from mkt.downloads import views + + +urlpatterns = patterns('', + # .* at the end to match filenames. + # /file/:id/type:attachment + url('^file/(?P\d+)(?:/type:(?P\w+))?(?:/.*)?', + views.download_file, name='downloads.file'), +) diff --git a/mkt/downloads/views.py b/mkt/downloads/views.py new file mode 100644 index 0000000000..678c09912c --- /dev/null +++ b/mkt/downloads/views.py @@ -0,0 +1,20 @@ +from django import http +from django.shortcuts import get_object_or_404 + +from access import acl +from addons.models import Addon +import amo +from amo.utils import HttpResponseSendFile +from files.models import File + + +def download_file(request, file_id, type=None): + file = get_object_or_404(File.objects, pk=file_id) + webapp = get_object_or_404(Addon.objects, pk=file.version.addon_id) + + if webapp.is_disabled or file.status == amo.STATUS_DISABLED: + if not acl.check_addon_ownership(request, webapp, viewer=True, + ignore_disabled=True): + raise http.Http404() + + return HttpResponseSendFile(request, file.guarded_file_path) diff --git a/mkt/site/helpers.py b/mkt/site/helpers.py index b8d079ca7c..fcd05dc910 100644 --- a/mkt/site/helpers.py +++ b/mkt/site/helpers.py @@ -65,6 +65,7 @@ def product_as_dict(request, product, purchased=None, receipt_type=None): url = (reverse('receipt.issue', args=[product.app_slug]) if receipt_type else product.get_detail_url('record')) + src = request.GET.get('src', '') ret = { 'id': product.id, 'name': product.name, @@ -73,10 +74,13 @@ def product_as_dict(request, product, purchased=None, receipt_type=None): 'manifestUrl': product.manifest_url, 'preapprovalUrl': reverse('detail.purchase.preapproval', args=[product.app_slug]), - 'recordUrl': urlparams(url, src=request.GET.get('src', '')), + 'recordUrl': urlparams(url, src=src), 'author': author, 'author_url': author_url, - 'iconUrl': product.get_icon_url(64) + 'iconUrl': product.get_icon_url(64), + 'is_packaged': product.has_packaged_files, + 'package_url': (product.current_version.all_files[0].get_url_path(src) + if product.has_packaged_files else ''), } # Add in previews to the dict. @@ -107,7 +111,8 @@ def product_as_dict(request, product, purchased=None, receipt_type=None): # Jinja2 escape everything except this whitelist so that bool is retained # for the JSON encoding. - wl = ('isPurchased', 'price', 'currencies', 'categories', 'previews') + wl = ('isPurchased', 'price', 'currencies', 'categories', 'previews', + 'is_packaged') return dict([k, jinja2.escape(v) if k not in wl else v] for k, v in ret.items()) diff --git a/mkt/site/tests/test_helpers.py b/mkt/site/tests/test_helpers.py index 22814bc166..eb98d18016 100644 --- a/mkt/site/tests/test_helpers.py +++ b/mkt/site/tests/test_helpers.py @@ -25,6 +25,7 @@ class TestMarketButton(amo.tests.TestCase): self.user = UserProfile.objects.get(pk=999) request = mock.Mock() request.amo_user = self.user + request.groups = () request.check_ownership.return_value = False request.GET = {'src': 'foo'} request.groups = () @@ -105,11 +106,9 @@ class TestMarketButton(amo.tests.TestCase): eq_(json.loads(data['currencies'])['USD'], '$1.00') eq_(json.loads(data['currencies'])['CAD'], 'CA$1.00') - def test_reviewers(self): - # Make ourselves a reviewer! - amo_user = UserProfile.objects.get(pk=5497308) - self.context['request'].amo_user = amo_user - self.context['request'].groups = amo_user.groups.all() + @mock.patch('mkt.site.helpers.acl.check_reviewer') + def test_reviewers(self, check_reviewer): + check_reviewer.return_value = True doc = pq(market_tile(self.context, self.webapp)) data = json.loads(doc('.mkt-tile').attr('data-product')) issue = urlparams(reverse('receipt.issue', @@ -123,3 +122,16 @@ class TestMarketButton(amo.tests.TestCase): data = json.loads(doc('.mkt-tile').attr('data-product')) eq_(data['categories'], [str(cat.name) for cat in self.webapp.categories.all()]) + + def test_is_packaged(self): + self.webapp.current_version.files.all()[0].update(is_packaged=True) + doc = pq(market_tile(self.context, self.webapp)) + data = json.loads(doc('a').attr('data-product')) + eq_(data['is_packaged'], True) + assert data['package_url'].startswith('/downloads') + + def test_is_not_packaged(self): + doc = pq(market_tile(self.context, self.webapp)) + data = json.loads(doc('a').attr('data-product')) + eq_(data['is_packaged'], False) + eq_(data['package_url'], '') diff --git a/mkt/urls.py b/mkt/urls.py index 9e082c521e..9337e376ce 100644 --- a/mkt/urls.py +++ b/mkt/urls.py @@ -11,9 +11,9 @@ from apps.users.urls import (detail_patterns as user_detail_patterns, users_patterns as users_users_patterns) from mkt.account.urls import (purchases_patterns, settings_patterns, users_patterns as mkt_users_patterns) +from mkt.developers.views import login from mkt.purchase.urls import bluevia_services_patterns from mkt.themes.urls import theme_patterns -from mkt.developers.views import login admin.autodiscover() @@ -114,6 +114,8 @@ urlpatterns = patterns('', url('^appcache/', include('django_appcache.urls')), + url('^downloads/', include('mkt.downloads.urls')), + # Try and keep urls without a prefix at the bottom of the list for # minor performance reasons.