denormalize reviews.is_latest and reviews.prev_count for easy queries

2010-06-10 20:00:56 -07:00 · 2010-06-10 20:00:56 -07:00 · 4dadc1d980
--- a/apps/reviews/cron.py
+++ b/apps/reviews/cron.py
@ -0,0 +1,20 @@
+import logging
+
+from celery.messaging import establish_connection
+
+import cronjobs
+from amo.utils import chunked
+
+from . import tasks
+from .models import Review
+
+log = logging.getLogger('z.cron')
+
+
+@cronjobs.register
+def reviews_denorm():
+    """Set is_latest and previous_count for all reviews."""
+    pairs = list(set(Review.objects.values_list('addon', 'user')))
+    with establish_connection() as conn:
+        for chunk in chunked(pairs, 50):
+            tasks.update_denorm.apply_async(args=chunk, connection=conn)
--- a/apps/reviews/fixtures/reviews/three-reviews.json
+++ b/apps/reviews/fixtures/reviews/three-reviews.json
@ -0,0 +1,212 @@
+[
+    {
+        "pk": 6,
+        "model": "versions.license",
+        "fields": {
+            "_custom_text": null,
+            "_name_field": 2,
+            "modified": "2009-04-30 23:48:48",
+            "created": "2009-04-30 23:48:48"
+        }
+    },
+    {
+        "pk": 5,
+        "model": "versions.license",
+        "fields": {
+            "_custom_text": null,
+            "_name_field": 0,
+            "modified": "2009-04-30 22:29:53",
+            "created": "2009-04-30 22:29:53"
+        }
+    },
+    {
+        "pk": 72,
+        "model": "addons.addon",
+        "fields": {
+            "dev_agreement": 1,
+            "eula": null,
+            "last_updated": "2010-02-22 08:50:52",
+            "view_source": 1,
+            "enable_thankyou": 0,
+            "total_downloads": 19093515,
+            "developer_comments": null,
+            "inactive": 0,
+            "average_daily_downloads": 18166,
+            "show_beta": 1,
+            "the_future": null,
+            "trusted": 0,
+            "locale_disambiguation": null,
+            "binary": 0,
+            "guid": "{9f08cb5a-76b1-4bcf-aff9-90e1a5d60b1e}",
+            "weekly_downloads": 16809,
+            "support_url": null,
+            "paypal_id": "kasteo@gmail.com",
+            "average_rating": "4.27",
+            "wants_contributions": 1,
+            "average_daily_users": 842821,
+            "bayesian_rating": 4.2623699999999998,
+            "share_count": 3,
+            "get_satisfaction_company": null,
+            "homepage": null,
+            "support_email": null,
+            "public_stats": 1,
+            "status": 4,
+            "privacy_policy": null,
+            "description": null,
+            "default_locale": "en-US",
+            "target_locale": null,
+            "prerelease": 0,
+            "thankyou_note": null,
+            "admin_review": 0,
+            "external_software": 0,
+            "highest_status": 4,
+            "get_satisfaction_product": null,
+            "name": null,
+            "created": "2004-06-14 18:50:28",
+            "type": 2,
+            "icon_type": "image/png",
+            "annoying": 2,
+            "modified": "2010-05-24 17:50:10",
+            "summary": null,
+            "suggested_amount": "5.00",
+            "site_specific": 0,
+            "_current_version": null,
+            "total_reviews": 623,
+            "the_reason": null,
+            "nomination_date": "2009-03-08 01:30:10"
+        }
+    },
+    {
+        "pk": 96624,
+        "model": "versions.version",
+        "fields": {
+            "license": 6,
+            "created": "2010-02-18 22:35:14",
+            "releasenotes": null,
+            "approvalnotes": "",
+            "modified": "2010-02-18 22:36:07",
+            "version": "3.76",
+            "addon": 72
+        }
+    },
+    {
+        "pk": 67371,
+        "model": "versions.version",
+        "fields": {
+            "license": null,
+            "created": "2009-04-28 20:26:32",
+            "releasenotes": null,
+            "approvalnotes": "",
+            "modified": "2009-04-28 20:30:12",
+            "version": "3.63",
+            "addon": 72
+        }
+    },
+    {
+        "pk": 4735395,
+        "model": "users.userprofile",
+        "fields": {
+            "sandboxshown": 0,
+            "display_collections_fav": 0,
+            "display_collections": 0,
+            "occupation": "",
+            "confirmationcode": "",
+            "location": "",
+            "picture_type": "",
+            "averagerating": "",
+            "homepage": "",
+            "email": "happyreader@gmx.net",
+            "notifycompat": 1,
+            "firstname": "h",
+            "deleted": 0,
+            "lastname": "willems",
+            "emailhidden": 1,
+            "user": null,
+            "bio": null,
+            "password": "sha512$280a921ec6e735ec6a1a76dc8802f3f6aee254f3fd71589010fe85ebce185f7a$c797ddde00731d7660b9e0c887d77b373e782eaa949e6101f17ada9a7236c426db53ac4344c133cdf32e73381bb7c7280efe5fdc5dd6134bbd2f706d994f94ff",
+            "nickname": "Join2",
+            "resetcode_expires": null,
+            "resetcode": "",
+            "created": "2009-05-16 05:29:58",
+            "notes": "",
+            "modified": "2009-07-07 01:13:54",
+            "notifyevents": 1
+        }
+    },
+    {
+        "pk": 72099,
+        "model": "versions.version",
+        "fields": {
+            "license": 5,
+            "created": "2009-07-02 07:33:26",
+            "releasenotes": null,
+            "approvalnotes": "",
+            "modified": "2009-07-02 07:38:22",
+            "version": "3.65",
+            "addon": 72
+        }
+    },
+    {
+        "pk": 130291,
+        "model": "reviews.review",
+        "fields": {
+            "body": null,
+            "rating": 2,
+            "editorreview": 0,
+            "created": "2009-05-16 05:37:27",
+            "title": null,
+            "modified": "2009-05-16 05:37:27",
+            "sandbox": 0,
+            "flag": 0,
+            "version": 67371,
+            "user": 4735395,
+            "reply_to": null,
+            "ip_address": "0.0.0.0",
+            "is_latest": 1,
+            "previous_count": 0,
+            "addon": 72
+        }
+    },
+    {
+        "pk": 130292,
+        "model": "reviews.review",
+        "fields": {
+            "body": null,
+            "rating": 2,
+            "editorreview": 0,
+            "created": "2009-05-16 05:38:02",
+            "title": null,
+            "modified": "2009-05-16 05:38:02",
+            "sandbox": 0,
+            "flag": 0,
+            "version": 67371,
+            "user": 4735395,
+            "reply_to": null,
+            "ip_address": "0.0.0.0",
+            "is_latest": 1,
+            "previous_count": 0,
+            "addon": 72
+        }
+    },
+    {
+        "pk": 142675,
+        "model": "reviews.review",
+        "fields": {
+            "body": null,
+            "rating": 3,
+            "editorreview": 0,
+            "created": "2009-07-07 01:23:52",
+            "title": null,
+            "modified": "2009-07-07 01:23:52",
+            "sandbox": 0,
+            "flag": 0,
+            "version": 72099,
+            "user": 4735395,
+            "reply_to": null,
+            "ip_address": "0.0.0.0",
+            "is_latest": 1,
+            "previous_count": 0,
+            "addon": 72
+        }
+    }
+]
--- a/apps/reviews/models.py
+++ b/apps/reviews/models.py
@ -24,6 +24,13 @@ class Review(TranslatedFieldMixin, amo.models.ModelBase):
    flag = models.BooleanField(default=False)
    sandbox = models.BooleanField(default=False)

+    # Denormalized fields for easy lookup queries.
+    # TODO: index on addon, user, latest
+    is_latest = models.BooleanField(default=True, editable=False,
+        help_text="Is this the user's latest review for the add-on?")
+    previous_count = models.PositiveIntegerField(default=0, editable=False,
+        help_text="How many previous reviews by the user for this add-on?")
+
    class Meta:
        db_table = 'reviews'
        ordering = ('-created',)
@ -46,3 +53,17 @@ class Review(TranslatedFieldMixin, amo.models.ModelBase):
                rv[id] = locales.itervalues().next()

        return rv.values()
+
+    @staticmethod
+    def post_save(sender, instance, created, **kwargs):
+        if created:
+            Review.post_delete(sender, instance)
+
+    @staticmethod
+    def post_delete(sender, instance, **kwargs):
+        from . import tasks
+        pair = instance.addon_id, instance.user_id
+        tasks.update_denorm(pair)
+
+models.signals.post_save.connect(Review.post_save, sender=Review)
+models.signals.post_delete.connect(Review.post_delete, sender=Review)
--- a/apps/reviews/tasks.py
+++ b/apps/reviews/tasks.py
@ -0,0 +1,27 @@
+import logging
+
+from celery.decorators import task
+
+from .models import Review
+
+log = logging.getLogger('z.task')
+
+
+@task(rate_limit='50/m')
+def update_denorm(*pairs, **kw):
+    """
+    Takes a bunch of (addon, user) pairs and sets the denormalized fields for
+    all reviews matching that pair.
+    """
+    log.info('[%s@%s] Updating review denorms.' %
+             (len(pairs), update_denorm.rate_limit))
+    for addon, user in pairs:
+        reviews = list(Review.uncached.filter(addon=addon, user=user)
+                       .filter(reply_to=None).order_by('created'))
+        for idx, review in enumerate(reviews):
+            review.previous_count = idx
+            review.is_latest = False
+        reviews[-1].is_latest = True
+
+        for review in reviews:
+            review.save()
--- a/apps/reviews/tests/test_cron.py
+++ b/apps/reviews/tests/test_cron.py
@ -0,0 +1,39 @@
+import mock
+from nose.tools import eq_
+import test_utils
+
+from reviews import cron, tasks
+from reviews.models import Review
+
+
+class TestDenormalization(test_utils.TestCase):
+    fixtures = ['reviews/three-reviews']
+
+    def setUp(self):
+        Review.objects.update(is_latest=True, previous_count=0)
+
+    def _check(self):
+        reviews = list(Review.objects.order_by('created'))
+        for idx, review in enumerate(reviews[:-1]):
+            eq_(review.is_latest, False)
+            eq_(review.previous_count, idx)
+        r = reviews[-1]
+        r.is_latest = True
+        r.previous_count = len(reviews) - 1
+
+    @mock.patch('reviews.tasks.update_denorm.apply_async')
+    def test_denorms(self, async):
+        cron.reviews_denorm()
+        kwargs = async.call_args[1]
+        tasks.update_denorm(*kwargs['args'])
+        self._check()
+
+    def test_denorm_on_save(self):
+        addon, user = Review.objects.values_list('addon', 'user')[0]
+        Review.objects.create(addon_id=addon, user_id=user)
+        self._check()
+
+    def test_denorm_on_delete(self):
+        r = Review.objects.order_by('created')[1]
+        r.delete()
+        self._check()
--- a/apps/reviews/views.py
+++ b/apps/reviews/views.py
@ -11,9 +11,7 @@ from .models import Review

 def review_list(request, addon_id):
    addon = get_object_or_404(Addon, id=addon_id)
-
-    versions = Version.objects.filter(addon=addon)
-    q = Review.objects.filter(version__in=versions).order_by('-created')
+    q = Review.objects.filter(addon=addon, is_latest=True).order_by('-created')
    reviews = amo.utils.paginate(request, q)
    return jingo.render(request, 'reviews/review_list.html',
                        {'addon': addon, 'reviews': reviews})
--- a/apps/users/models.py
+++ b/apps/users/models.py
@ -62,7 +62,7 @@ class UserProfile(amo.models.ModelBase):
    # This is essentially a "has_picture" flag right now
    picture_type = models.CharField(max_length=75, default='', blank=True)
    resetcode = models.CharField(max_length=255, default='', blank=True)
-    resetcode_expires = models.DateTimeField(default=datetime.now,
+    resetcode_expires = models.DateTimeField(default=datetime.now, null=True,
                                             blank=True)
    sandboxshown = models.BooleanField(default=False)

--- a/migrations/47-reviews-cheat.sql
+++ b/migrations/47-reviews-cheat.sql
@ -0,0 +1,3 @@
+ALTER TABLE `reviews`
+    ADD COLUMN `previous_count` int(11) UNSIGNED DEFAULT 0,
+    ADD COLUMN `is_latest` bool DEFAULT 1;
--- a/settings.py
+++ b/settings.py
@ -495,6 +495,7 @@ SYSLOG_TAG = "http_app_addons"
 # unless propagate: True is set.
 LOGGING = {
    'loggers': {
+        'amqplib': {'handlers': ['null']},
        'caching': {'handlers': ['null']},
        'z.sphinx': {'level': logging.INFO},
        'z.task': {'level': logging.INFO},