Skip to content

Commit

Permalink
Improve collections ajax list performance. (bug 1211692)
Browse files Browse the repository at this point in the history
This uses a simple extra query to check if the addon
exists in a collection or not.

This avoids the for-loop and thus n queries on a (possibly) huge
list of addon ids.
  • Loading branch information
EnTeQuAk committed Oct 22, 2015
1 parent 938a015 commit 75d321a
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 7 deletions.
19 changes: 19 additions & 0 deletions apps/bandwagon/models.py
Expand Up @@ -45,10 +45,29 @@ def __set__(self, obj, value):
cache.set(self.key(obj), value, two_days)


class CollectionQuerySet(caching.CachingQuerySet):

def with_has_addon(self, addon_id):
"""Add a `has_addon` property to each collection.
`has_addon` will be `True` if `addon_id` exists in that
particular collection.
"""
has_addon = """
select 1 from addons_collections as ac
where ac.addon_id = %s and ac.collection_id = collections.id
limit 1"""

return self.extra(
select={'has_addon': has_addon},
select_params=(addon_id,))


class CollectionManager(amo.models.ManagerBase):

def get_query_set(self):
qs = super(CollectionManager, self).get_query_set()
qs = qs._clone(klass=CollectionQuerySet)
return qs.transform(Collection.transformer)

def manual(self):
Expand Down
20 changes: 20 additions & 0 deletions apps/bandwagon/tests/test_models.py
Expand Up @@ -214,6 +214,26 @@ def test_can_view_stats(self):
eq_(c.can_view_stats(fake_request), True)


class TestCollectionQuerySet(amo.tests.TestCase):
fixtures = ('base/addon_3615',)

def test_with_has_addon(self):
user = UserProfile.objects.create(username='uhhh', email='uh@hh')
collection = Collection.objects.create(author=user)
addon = Addon.objects.all()[0]

qset = (
Collection.objects
.filter(pk=collection.id)
.with_has_addon(addon.id))

assert not qset.first().has_addon

collection.add_addon(addon)

assert qset.first().has_addon


class TestRecommendations(amo.tests.TestCase):
fixtures = ['base/addon-recs']
ids = [5299, 1843, 2464, 7661, 5369]
Expand Down
11 changes: 4 additions & 7 deletions apps/bandwagon/views.py
Expand Up @@ -369,13 +369,10 @@ def ajax_list(request):
except (KeyError, ValueError):
return http.HttpResponseBadRequest()

# Get collections associated with this user
collections = Collection.objects.publishable_by(request.amo_user)

for collection in collections:
# See if the collections contains the addon
if addon_id in collection.addons.values_list('id', flat=True):
collection.has_addon = True
collections = (
Collection.objects
.publishable_by(request.amo_user)
.with_has_addon(addon_id))

return render(request, 'bandwagon/ajax_list.html',
{'collections': collections})
Expand Down

0 comments on commit 75d321a

Please sign in to comment.