fixing up /downloads (bug 591028)
* use .no_transforms() for fewer queries on download pages * drop the locale prefix for better caching * watch out for null datestatuschanged * watch out for multiple platform matches * use /_attachments to get files as content-disposition:attachment
This commit is contained in:
Jeff Balogh
Родитель
be05588d42
Коммит
cc058d2474
|
|
@ -170,7 +170,7 @@ class THUNDERBIRD:
|
|||
short = 'thunderbird'
|
||||
shortername = 'tb'
|
||||
pretty = _(u'Thunderbird')
|
||||
browser = True
|
||||
browser = False
|
||||
types = [ADDON_EXTENSION, ADDON_THEME, ADDON_DICT, ADDON_LPAPP,
|
||||
ADDON_PERSONA]
|
||||
guid = '{3550f703-e582-4d05-9a08-453d09bdfdc6}'
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ class TestFile(test_utils.TestCase):
|
|||
def test_latest_url(self):
|
||||
# With platform.
|
||||
f = File.objects.get(id=74797)
|
||||
base = '/en-US/firefox/downloads/latest/'
|
||||
base = '/firefox/downloads/latest/'
|
||||
expected = base + '{0}/platform:3/addon-{0}-latest.xpi'
|
||||
eq_(expected.format(f.version.addon_id), f.latest_xpi_url())
|
||||
|
||||
|
|
|
|||
|
|
@ -1,7 +1,6 @@
|
|||
from datetime import datetime, timedelta
|
||||
from django.conf import settings
|
||||
|
||||
import nose
|
||||
import mock
|
||||
import test_utils
|
||||
from nose.tools import eq_
|
||||
|
|
@ -174,8 +173,11 @@ class TestDownloadsBase(test_utils.TestCase):
|
|||
'%s/%s/%s' % (host, self.addon.id, file_.filename))
|
||||
eq_(response['X-Target-Digest'], file_.hash)
|
||||
|
||||
def assert_served_locally(self, response, file_=None):
|
||||
self.assert_served_by_host(response, settings.LOCAL_MIRROR_URL, file_)
|
||||
def assert_served_locally(self, response, file_=None, attachment=False):
|
||||
host = settings.LOCAL_MIRROR_URL
|
||||
if attachment:
|
||||
host += '/_attachments'
|
||||
self.assert_served_by_host(response, host, file_)
|
||||
|
||||
def assert_served_by_mirror(self, response):
|
||||
self.assert_served_by_host(response, settings.MIRROR_URL)
|
||||
|
|
@ -222,11 +224,11 @@ class TestDownloads(TestDownloadsBase):
|
|||
def test_type_attachment(self):
|
||||
self.assert_served_by_mirror(self.client.get(self.file_url))
|
||||
url = reverse('downloads.file', args=[self.file.id, 'attachment'])
|
||||
self.assert_served_locally(self.client.get(url))
|
||||
self.assert_served_locally(self.client.get(url), attachment=True)
|
||||
|
||||
def test_nonbrowser_app(self):
|
||||
url = self.file_url.replace('firefox', 'sunbird')
|
||||
self.assert_served_locally(self.client.get(url))
|
||||
url = self.file_url.replace('firefox', 'thunderbird')
|
||||
self.assert_served_locally(self.client.get(url), attachment=True)
|
||||
|
||||
def test_mirror_delay(self):
|
||||
self.file.datestatuschanged = datetime.now()
|
||||
|
|
@ -246,6 +248,10 @@ class TestDownloads(TestDownloadsBase):
|
|||
url = reverse('downloads.file', args=[self.beta_file.id])
|
||||
self.assert_served_locally(self.client.get(url), self.beta_file)
|
||||
|
||||
def test_null_datestatuschanged(self):
|
||||
self.file.update(datestatuschanged=None)
|
||||
self.assert_served_locally(self.client.get(self.file_url))
|
||||
|
||||
|
||||
class TestDownloadsLatest(TestDownloadsBase):
|
||||
|
||||
|
|
@ -258,10 +264,11 @@ class TestDownloadsLatest(TestDownloadsBase):
|
|||
r = self.client.get(response['Location'])
|
||||
super(TestDownloadsLatest, self).assert_served_by_mirror(r)
|
||||
|
||||
def assert_served_locally(self, response):
|
||||
def assert_served_locally(self, response, file_=None, attachment=False):
|
||||
# Follow one more hop to hit the downloads.files view.
|
||||
r = self.client.get(response['Location'])
|
||||
super(TestDownloadsLatest, self).assert_served_locally(r)
|
||||
super(TestDownloadsLatest, self).assert_served_locally(
|
||||
r, file_, attachment)
|
||||
|
||||
def test_404(self):
|
||||
url = reverse('downloads.latest', args=[123])
|
||||
|
|
@ -290,20 +297,28 @@ class TestDownloadsLatest(TestDownloadsBase):
|
|||
def test_type(self):
|
||||
url = reverse('downloads.latest',
|
||||
kwargs={'addon_id': self.addon.id, 'type': 'attachment'})
|
||||
self.assert_served_locally(self.client.get(url))
|
||||
self.assert_served_locally(self.client.get(url), attachment=True)
|
||||
|
||||
def test_platform_and_type(self):
|
||||
url = reverse('downloads.latest',
|
||||
kwargs={'addon_id': self.addon.id, 'platform': 5,
|
||||
'type': 'attachment'})
|
||||
self.assert_served_locally(self.client.get(url))
|
||||
self.assert_served_locally(self.client.get(url), attachment=True)
|
||||
|
||||
def test_trailing_filename(self):
|
||||
url = reverse('downloads.latest',
|
||||
kwargs={'addon_id': self.addon.id, 'platform': 5,
|
||||
'type': 'attachment'})
|
||||
url += self.file.filename
|
||||
self.assert_served_locally(self.client.get(url))
|
||||
self.assert_served_locally(self.client.get(url), attachment=True)
|
||||
|
||||
def test_platform_multiple_objects(self):
|
||||
p = Platform.objects.create(id=3)
|
||||
f = File.objects.create(platform=p, version=self.file.version,
|
||||
filename='unst.xpi')
|
||||
url = reverse('downloads.latest',
|
||||
kwargs={'addon_id': self.addon.id, 'platform': 3})
|
||||
self.assert_served_locally(self.client.get(url), file_=f)
|
||||
|
||||
def test_query_params(self):
|
||||
url = self.latest_url + '?src=xxx'
|
||||
|
|
|
|||
|
|
@ -56,7 +56,8 @@ def _find_version_page(qs, addon_id, version_num):
|
|||
# Should accept junk at the end for filename goodness.
|
||||
def download_file(request, file_id, type=None):
|
||||
file = get_object_or_404(File.objects, pk=file_id)
|
||||
addon = get_object_or_404(Addon.objects, pk=file.version.addon_id)
|
||||
addon = get_object_or_404(Addon.objects.all().no_transforms(),
|
||||
pk=file.version.addon_id)
|
||||
|
||||
if (addon.status == amo.STATUS_DISABLED
|
||||
and not acl.check_ownership(request, addon)):
|
||||
|
|
@ -64,10 +65,16 @@ def download_file(request, file_id, type=None):
|
|||
|
||||
attachment = (type == 'attachment' or not request.APP.browser)
|
||||
|
||||
published = datetime.now() - file.datestatuschanged
|
||||
if (not (settings.DEBUG or attachment)
|
||||
and addon.status == file.status == amo.STATUS_PUBLIC
|
||||
and published > timedelta(minutes=settings.MIRROR_DELAY)):
|
||||
if file.datestatuschanged:
|
||||
published = datetime.now() - file.datestatuschanged
|
||||
else:
|
||||
published = timedelta(minutes=0)
|
||||
|
||||
if attachment:
|
||||
host = posixpath.join(settings.LOCAL_MIRROR_URL, '_attachments')
|
||||
elif (addon.status == file.status == amo.STATUS_PUBLIC
|
||||
and published > timedelta(minutes=settings.MIRROR_DELAY)
|
||||
and not settings.DEBUG):
|
||||
host = settings.MIRROR_URL # Send it to the mirrors.
|
||||
else:
|
||||
host = settings.LOCAL_MIRROR_URL
|
||||
|
|
@ -77,16 +84,19 @@ def download_file(request, file_id, type=None):
|
|||
return response
|
||||
|
||||
|
||||
def download_latest(request, addon_id, type=None, platform=None):
|
||||
if type is None:
|
||||
type = 'xpi'
|
||||
def download_latest(request, addon_id, type='xpi', platform=None):
|
||||
addon = get_object_or_404(Addon.objects.all().no_transforms(),
|
||||
pk=addon_id, _current_version__isnull=False)
|
||||
platforms = [amo.PLATFORM_ALL.id]
|
||||
if platform is not None and int(platform)in amo.PLATFORMS:
|
||||
if platform is not None and int(platform) in amo.PLATFORMS:
|
||||
platforms.append(int(platform))
|
||||
addon = get_object_or_404(Addon.objects, pk=addon_id,
|
||||
_current_version__isnull=False)
|
||||
file = get_object_or_404(File.objects, platform__in=platforms,
|
||||
version=addon.current_version)
|
||||
files = File.objects.filter(platform__in=platforms,
|
||||
version=addon._current_version_id)
|
||||
try:
|
||||
# If there's a file matching our platform, it'll float to the end.
|
||||
file = sorted(files, key=lambda f: f.platform_id == platforms[-1])[-1]
|
||||
except IndexError:
|
||||
raise http.Http404()
|
||||
url = posixpath.join(reverse('downloads.file', args=[file.id, type]),
|
||||
file.filename)
|
||||
if request.GET:
|
||||
|
|
|
|||
|
|
@ -146,7 +146,7 @@ SUPPORTED_NONAPPS = ('admin', 'developers', 'editors', 'img',
|
|||
DEFAULT_APP = 'firefox'
|
||||
|
||||
# paths that don't require a locale prefix
|
||||
SUPPORTED_NONLOCALES = ('img', 'media', 'robots.txt', 'services',)
|
||||
SUPPORTED_NONLOCALES = ('img', 'media', 'robots.txt', 'services', 'downloads')
|
||||
|
||||
# Make this unique, and don't share it with anybody.
|
||||
SECRET_KEY = 'r#%9w^o_80)7f%!_ir5zx$tu3mupw9u%&s!)-_q%gy7i+fhx#)'
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче