do all the filtering in python for better caching and less query overhead
This commit is contained in:
Jeff Balogh
Родитель
0d45383a1c
Коммит
30b2d412ab
|
|
@ -63,7 +63,7 @@ class AddonManager(amo.models.ManagerBase):
|
|||
|
||||
# XXX: handle personas (no versions) and listed (no files)
|
||||
return self.filter(inactive=False, status__in=status,
|
||||
versions__applicationsversions__application=app.id,
|
||||
versions__apps__application=app.id,
|
||||
versions__files__status__in=status).distinct()
|
||||
|
||||
def compatible_with_app(self, app, version=None):
|
||||
|
|
@ -74,18 +74,15 @@ class AddonManager(amo.models.ManagerBase):
|
|||
E.g. amo.FIREFOX and '3.5'
|
||||
"""
|
||||
qs = self.filter(
|
||||
versions__applicationsversions__min__application=app.id)
|
||||
versions__apps__min__application=app.id)
|
||||
|
||||
if version is not None:
|
||||
|
||||
version_int = search_utils.convert_version(version)
|
||||
qs = qs.filter(
|
||||
versions__applicationsversions__min__version_int__lte=
|
||||
version_int,
|
||||
versions__applicationsversions__max__version_int__gte=
|
||||
version_int).distinct()
|
||||
versions__apps__min__version_int__lte=version_int,
|
||||
versions__apps__max__version_int__gte=version_int)
|
||||
|
||||
return qs.distinct()
|
||||
return qs
|
||||
|
||||
def compatible_with_platform(self, platform):
|
||||
"""
|
||||
|
|
@ -99,12 +96,10 @@ class AddonManager(amo.models.ManagerBase):
|
|||
platform = amo.PLATFORM_DICT.get(platform, amo.PLATFORM_ALL)
|
||||
|
||||
if platform != amo.PLATFORM_ALL:
|
||||
return (self.filter(
|
||||
Q(versions__files__platform=platform.id) |
|
||||
Q(versions__files__platform=amo.PLATFORM_ALL.id))
|
||||
.distinct())
|
||||
|
||||
return self.distinct()
|
||||
platforms = [platform.id, amo.PLATFORM_ALL.id]
|
||||
return self.filter(versions__files__platform__in=platforms)
|
||||
else:
|
||||
return self.all()
|
||||
|
||||
|
||||
class Addon(amo.models.ModelBase):
|
||||
|
|
@ -270,7 +265,6 @@ class Addon(amo.models.ModelBase):
|
|||
for addon_id, users in itertools.groupby(q, key=lambda u: u.addon_id):
|
||||
addon_dict[addon_id].listed_authors = list(users)
|
||||
|
||||
|
||||
@amo.cached_property
|
||||
def current_beta_version(self):
|
||||
"""Retrieves the latest version of an addon, in the beta channel."""
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@
|
|||
<version>{{ addon.current_version.version }}</version>
|
||||
<status id="{{ addon.status }}">{{ amo.STATUS_CHOICES[addon.status] }}</status>
|
||||
<authors>
|
||||
{% for author in addon.authors.filter(addonuser__listed=True) -%}
|
||||
{% for author in addon.listed_authors -%}
|
||||
{%- if api_version >= 1.5 -%}
|
||||
<author id="{{ author.id }}">
|
||||
<name>{{ author.display_name }}</name>
|
||||
|
|
@ -22,7 +22,7 @@
|
|||
<icon>{{ addon.icon_url }}</icon>
|
||||
<compatible_applications>
|
||||
{%- if addon.current_version -%}
|
||||
{%- for app in addon.current_version.applicationsversions_set.all() -%}
|
||||
{%- for app in addon.current_version.compatible_apps.values() %}
|
||||
{%- if amo.APP_IDS.get(app.application_id) -%}
|
||||
<application>
|
||||
<name>{{ amo.APP_IDS[app.application_id].pretty }}</name>
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
<?xml version="1.0" encoding="utf-8" ?>
|
||||
<addons>
|
||||
{% for addon in addons %}
|
||||
{% include 'api/includes/addon.xml' %}
|
||||
{% include 'api/includes/addon.xml' %}
|
||||
{% endfor %}
|
||||
</addons>
|
||||
|
|
|
|||
|
|
@ -321,8 +321,8 @@ class ListTest(TestCase):
|
|||
E.g.
|
||||
/list/new/all/1/mac/4.0 gives us nothing
|
||||
"""
|
||||
request = make_call('list/new/all/1/all/4.0')
|
||||
self.assertNotContains(request, "<addon id")
|
||||
response = make_call('list/new/1/1/all/4.0')
|
||||
self.assertNotContains(response, "<addon id")
|
||||
|
||||
def test_backfill(self):
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -1,9 +1,10 @@
|
|||
"""
|
||||
API views
|
||||
"""
|
||||
from datetime import date, timedelta
|
||||
import itertools
|
||||
import json
|
||||
import random
|
||||
import datetime
|
||||
import urllib
|
||||
|
||||
from django.http import HttpResponse, HttpResponsePermanentRedirect
|
||||
|
|
@ -18,6 +19,7 @@ from amo.urlresolvers import get_url_prefix
|
|||
import api
|
||||
from addons.models import Addon
|
||||
from search.client import Client as SearchClient, SearchError
|
||||
from search import utils as search_utils
|
||||
|
||||
ERROR = 'error'
|
||||
OUT_OF_DATE = ugettext_lazy(
|
||||
|
|
@ -29,6 +31,19 @@ old_finalize = xml_env.finalize
|
|||
xml_env.finalize = lambda x: amo.helpers.strip_controls(old_finalize(x))
|
||||
|
||||
|
||||
# Hard limit of 30. The buffer is to try for locale-specific add-ons.
|
||||
MAX_LIMIT, BUFFER = 30, 10
|
||||
|
||||
# "New" is arbitrarily defined as 10 days old.
|
||||
NEW_DAYS = 10
|
||||
|
||||
|
||||
def partition(seq, key):
|
||||
"""Group a sequence based into buckets by key(x)."""
|
||||
groups = itertools.groupby(sorted(seq, key=key), key=key)
|
||||
return ((k, list(v)) for k, v in groups)
|
||||
|
||||
|
||||
def render_xml(request, template, context={}, **kwargs):
|
||||
"""Safely renders xml, stripping out nasty control characters."""
|
||||
if not jingo._helpers_loaded:
|
||||
|
|
@ -122,6 +137,7 @@ class SearchView(APIView):
|
|||
This queries sphinx with `query` and serves the results in xml.
|
||||
"""
|
||||
sc = SearchClient()
|
||||
limit = min(MAX_LIMIT, int(limit))
|
||||
|
||||
opts = {'app': self.request.APP.id}
|
||||
|
||||
|
|
@ -155,7 +171,7 @@ class SearchView(APIView):
|
|||
# This fails if the string is already UTF-8.
|
||||
pass
|
||||
try:
|
||||
results = sc.query(query, limit=int(limit), **opts)
|
||||
results = sc.query(query, limit=limit, **opts)
|
||||
except SearchError:
|
||||
return self.render_msg('Could not connect to Sphinx search.',
|
||||
ERROR, status=503, mimetype=self.mimetype)
|
||||
|
|
@ -168,7 +184,7 @@ class SearchView(APIView):
|
|||
class ListView(APIView):
|
||||
|
||||
def process_request(self, list_type='recommended', addon_type='ALL',
|
||||
limit=3, platform='ALL', version=None):
|
||||
limit=10, platform='ALL', version=None):
|
||||
"""
|
||||
This generates a list of addons that can be served to the
|
||||
AddonManager.
|
||||
|
|
@ -176,46 +192,59 @@ class ListView(APIView):
|
|||
We generate the lists by making empty queries to Sphinx, but with
|
||||
using parameters to influence the order.
|
||||
"""
|
||||
limit = min(MAX_LIMIT, int(limit))
|
||||
APP, platform = self.request.APP, platform.lower()
|
||||
qs = Addon.objects.listed(APP)
|
||||
|
||||
if list_type == 'newest':
|
||||
# "New" is arbitrarily defined as 10 days old.
|
||||
qs = Addon.objects.filter(
|
||||
created__gte=(datetime.date.today() -
|
||||
datetime.timedelta(days=10)))
|
||||
new = date.today() - timedelta(days=NEW_DAYS)
|
||||
addons = (qs.filter(created__gte=new)
|
||||
.order_by('-created'))[:limit + BUFFER]
|
||||
else:
|
||||
qs = Addon.objects.featured(self.request.APP)
|
||||
addons = Addon.objects.featured(APP).distinct() & qs
|
||||
|
||||
if addon_type.upper() != 'ALL':
|
||||
try:
|
||||
addon_type = int(addon_type)
|
||||
if addon_type:
|
||||
qs = qs.filter(type=addon_type)
|
||||
|
||||
addons = [a for a in addons if a.type_id == addon_type]
|
||||
except ValueError:
|
||||
# `addon_type` is ALL or a type id. Otherwise we ignore it.
|
||||
pass
|
||||
|
||||
if platform.lower() != 'all':
|
||||
qs = (qs.distinct() &
|
||||
Addon.objects.compatible_with_platform(platform))
|
||||
# Take out personas since they don't have versions.
|
||||
groups = dict(partition(addons,
|
||||
lambda x: x.type_id == amo.ADDON_PERSONA))
|
||||
personas, addons = groups.get(True, []), groups.get(False, [])
|
||||
|
||||
if platform != 'all' and platform in amo.PLATFORM_DICT:
|
||||
pid = amo.PLATFORM_DICT[platform]
|
||||
f = lambda ps: pid in ps or amo.PLATFORM_ALL in ps
|
||||
addons = [a for a in addons
|
||||
if f(a.current_version.supported_platforms)]
|
||||
|
||||
if version is not None:
|
||||
qs = qs.distinct() & Addon.objects.compatible_with_app(
|
||||
self.request.APP, version)
|
||||
v = search_utils.convert_version(version)
|
||||
f = lambda app: app.min.version_int < v < app.max.version_int
|
||||
addons = [a for a in addons if f(a.compatible_apps[APP])]
|
||||
|
||||
locale_filter = Addon.objects.filter(
|
||||
description__locale=translation.get_language())
|
||||
addons = list(locale_filter.distinct() & qs.distinct())
|
||||
random.shuffle(addons)
|
||||
# Put personas back in.
|
||||
addons.extend(personas)
|
||||
|
||||
if len(addons) < limit:
|
||||
# We need to backfill. So do the previous query without
|
||||
# a language requirement, and exclude the addon ids we've found
|
||||
moar_addons = list(qs.exclude(id__in=[a.id for a in addons]))
|
||||
random.shuffle(moar_addons)
|
||||
addons.extend(moar_addons)
|
||||
# We prefer add-ons that support the current locale.
|
||||
lang = translation.get_language()
|
||||
groups = dict(partition(addons,
|
||||
lambda x: x.description.locale == lang))
|
||||
good, others = groups.get(True, []), groups.get(False, [])
|
||||
|
||||
random.shuffle(good)
|
||||
random.shuffle(others)
|
||||
|
||||
if len(good) < limit:
|
||||
good.extend(others[:limit - len(good)])
|
||||
|
||||
if self.format == 'xml':
|
||||
return self.render('api/list.xml', {'addons': addons[:int(limit)]})
|
||||
return self.render('api/list.xml', {'addons': good})
|
||||
|
||||
|
||||
# pylint: disable-msg=W0613
|
||||
|
|
|
|||
|
|
@ -275,9 +275,9 @@ class FrontendSearchTest(SphinxTestCase):
|
|||
resp = self.get_response()
|
||||
doc = pq(resp.content)
|
||||
num_actual_results = len(Addon.objects.filter(
|
||||
versions__applicationsversions__application=amo.FIREFOX.id,
|
||||
versions__apps__application=amo.FIREFOX.id,
|
||||
versions__files__gt=0, versions__files__platform=1))
|
||||
# Verify that we have at least 10 results listed.
|
||||
# Verify that we have the expected number of results.
|
||||
eq_(doc('.item').length, num_actual_results)
|
||||
|
||||
# We should count the number of expected results and match.
|
||||
|
|
@ -401,6 +401,7 @@ class TestSearchForm(test_utils.TestCase):
|
|||
# So you added a new appversion and this broke? Sorry about that.
|
||||
eq_(actual, expected)
|
||||
|
||||
|
||||
def test_showing_helper():
|
||||
tpl = "{{ showing(query, tag, pager) }}"
|
||||
pager = Mock()
|
||||
|
|
|
|||
|
|
@ -35,7 +35,7 @@ class Version(amo.models.ModelBase):
|
|||
@amo.cached_property(writable=True)
|
||||
def compatible_apps(self):
|
||||
"""Get a mapping of {APP: ApplicationVersion}."""
|
||||
avs = self.applicationsversions_set.select_related(depth=1)
|
||||
avs = self.apps.select_related(depth=1)
|
||||
return self._compat_map(avs)
|
||||
|
||||
@amo.cached_property(writable=True)
|
||||
|
|
@ -171,7 +171,7 @@ class VersionSummary(amo.models.ModelBase):
|
|||
class ApplicationsVersions(caching.base.CachingMixin, models.Model):
|
||||
|
||||
application = models.ForeignKey(Application)
|
||||
version = models.ForeignKey(Version)
|
||||
version = models.ForeignKey(Version, related_name='apps')
|
||||
min = models.ForeignKey(AppVersion, db_column='min',
|
||||
related_name='min_set')
|
||||
max = models.ForeignKey(AppVersion, db_column='max',
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче