launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/karma-store into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/karma-store/+merge/60987

While poking around the tree, I discovered that IKarmaCache.getTopContributors() made use of cursor() to query the database. I have rewritten the function to make use of IStore() and Storm.
-- 
https://code.launchpad.net/~stevenk/launchpad/karma-store/+merge/60987
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/karma-store into lp:launchpad.

=== modified file 'lib/lp/registry/model/karma.py'
--- lib/lp/registry/model/karma.py	2010-08-20 20:31:18 +0000
+++ lib/lp/registry/model/karma.py	2011-05-14 08:41:21 +0000
@@ -16,6 +16,10 @@
     'KarmaContextMixin',
     ]
 
+from storm.expr import (
+    Desc,
+    Expr,
+    )
 from sqlobject import (
     ForeignKey,
     IntCol,
@@ -29,11 +33,11 @@
 from canonical.database.constants import UTC_NOW
 from canonical.database.datetimecol import UtcDateTimeCol
 from canonical.database.sqlbase import (
-    cursor,
     SQLBase,
     sqlvalues,
     )
 from canonical.launchpad.event.interfaces import IKarmaAssignedEvent
+from canonical.launchpad.interfaces.lpstorm import IStore
 from lp.app.errors import NotFoundError
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.karma import (
@@ -242,34 +246,22 @@
     def getTopContributors(self, category=None, limit=None):
         """See IKarmaContext."""
         from lp.registry.model.person import Person
+        store = IStore(Person)
         if IProduct.providedBy(self):
-            where_clause = "product = %d" % self.id
+            condition = KarmaCache.productID == self.id
         elif IDistribution.providedBy(self):
-            where_clause = "distribution = %d" % self.id
+            condition = KarmaCache.distributionID == self.id
         elif IProjectGroup.providedBy(self):
-            where_clause = "project = %d" % self.id
+            condition = KarmaCache.projectID == self.id
         else:
             raise AssertionError(
                 "Not a product, project or distribution: %r" % self)
 
         if category is not None:
-            category_filter = " AND category = %s" % sqlvalues(category)
-        else:
-            category_filter = " AND category IS NULL"
-        if limit is not None:
-            limit_filter = " LIMIT %d" % limit
-        query = """
-            SELECT person, karmavalue
-            FROM KarmaCache
-            WHERE %(where_clause)s
-            %(category_filter)s
-            ORDER BY karmavalue DESC
-            %(limit)s
-            """ % {'where_clause': where_clause, 'limit': limit_filter,
-                   'category_filter': category_filter}
-
-        cur = cursor()
-        cur.execute(query)
-        return [(Person.get(person_id), karmavalue)
-                for person_id, karmavalue in cur.fetchall()]
-
+            category = category.id
+        contributors = store.find(
+            (Person, KarmaCache.karmavalue),
+            KarmaCache.personID == Person.id,
+            KarmaCache.categoryID == category, condition).order_by(
+                Desc(KarmaCache.karmavalue)).config(limit=limit)
+        return list(contributors)