Merge lp:~wgrant/launchpad/bug-728174 into lp:launchpad

[William Grant]

[r=lifeless][bug=728174] Optionally preload Language.translators_count in LanguageSet.get(All|Default)Languages.

Webservice Language collections returned by LanguageSet were timing out on repeated heavy queries to calculate Language.translators_count. This branch calculates them all in a single query, running only slightly slower than a single individual query.

[Robert Collins (lifeless)]

ids = [language.id for language in languages]

I would setify this. Possibly not needed, but belt and braces.

review: Approve

=== modified file 'lib/lp/services/worlddata/interfaces/language.py'
--- lib/lp/services/worlddata/interfaces/language.py	2010-12-02 16:13:51 +0000
+++ lib/lp/services/worlddata/interfaces/language.py	2011-03-07 07:51:04 +0000
@@ -19,6 +19,7 @@
     )
 from lazr.lifecycle.snapshot import doNotSnapshot
 from lazr.restful.declarations import (
+    call_with,
     collection_default_content,
     export_as_webservice_collection,
     export_as_webservice_entry,
@@ -176,11 +177,12 @@
 
     @export_read_operation()
     @operation_returns_collection_of(ILanguage)
-    def getAllLanguages():
+    @call_with(want_translators_count=True)
+    def getAllLanguages(want_translators_count=False):
         """Return a result set of all ILanguages from Launchpad."""
 
-    @collection_default_content()
-    def getDefaultLanguages():
+    @collection_default_content(want_translators_count=True)
+    def getDefaultLanguages(want_translators_count=False):
         """An API wrapper for `common_languages`"""
 
     common_languages = Attribute(

=== modified file 'lib/lp/services/worlddata/model/language.py'
--- lib/lp/services/worlddata/model/language.py	2010-12-02 16:13:51 +0000
+++ lib/lp/services/worlddata/model/language.py	2011-03-07 07:51:04 +0000
@@ -1,5 +1,6 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
+# Copyright 2009-2010 Canonical Ltd.  This software is licensed under
+# the GNU Affero General Public License version 3 (see the file
+# LICENSE).
 
 # pylint: disable-msg=E0611,W0212
 
@@ -17,17 +18,35 @@
     SQLRelatedJoin,
     StringCol,
     )
-from storm.locals import Or
+from storm.expr import (
+    And,
+    Count,
+    Desc,
+    Join,
+    LeftJoin,
+    Or,
+    )
 from zope.interface import implements
 
 from canonical.database.enumcol import EnumCol
-from canonical.database.sqlbase import (
-    SQLBase,
-    sqlvalues,
+from canonical.database.sqlbase import SQLBase
+from canonical.launchpad.components.decoratedresultset import (
+    DecoratedResultSet,
     )
 from canonical.launchpad.helpers import ensure_unicode
-from canonical.launchpad.interfaces.lpstorm import ISlaveStore
+from canonical.launchpad.interfaces.lpstorm import (
+    ISlaveStore,
+    IStore,
+    )
 from lp.app.errors import NotFoundError
+from lp.registry.model.karma import (
+    KarmaCache,
+    KarmaCategory,
+    )
+from lp.services.propertycache import (
+    cachedproperty,
+    get_property_cache,
+    )
 from lp.services.worlddata.interfaces.language import (
     ILanguage,
     ILanguageSet,
@@ -39,6 +58,7 @@
 from lp.services.worlddata.model.spokenin import SpokenIn
 SpokenIn
 
+
 class Language(SQLBase):
     implements(ILanguage)
 
@@ -140,34 +160,51 @@
     @property
     def translators(self):
         """See `ILanguage`."""
+        from lp.registry.model.person import (
+            Person,
+            PersonLanguage,
+            )
+        return IStore(Language).using(
+            Join(
+                Person,
+                LanguageSet._getTranslatorJoins(),
+                Person.id == PersonLanguage.personID),
+            ).find(
+                Person,
+                PersonLanguage.language == self,
+            ).order_by(Desc(KarmaCache.karmavalue))
+
+    @cachedproperty
+    def translators_count(self):
+        """See `ILanguage`."""
+        return self.translators.count()
+
+
+class LanguageSet:
+    implements(ILanguageSet)
+
+    @staticmethod
+    def _getTranslatorJoins():
         # XXX CarlosPerelloMarin 2007-03-31 bug=102257:
         # The KarmaCache table doesn't have a field to store karma per
         # language, so we are actually returning the people with the most
         # translation karma that have this language selected in their
         # preferences.
-        from lp.registry.model.person import Person
-        return Person.select('''
-            PersonLanguage.person = Person.id AND
-            PersonLanguage.language = %s AND
-            KarmaCache.person = Person.id AND
-            KarmaCache.product IS NULL AND
-            KarmaCache.project IS NULL AND
-            KarmaCache.sourcepackagename IS NULL AND
-            KarmaCache.distribution IS NULL AND
-            KarmaCache.category = KarmaCategory.id AND
-            KarmaCategory.name = 'translations'
-            ''' % sqlvalues(self), orderBy=['-KarmaCache.karmavalue'],
-            clauseTables=[
-                'PersonLanguage', 'KarmaCache', 'KarmaCategory'])
-
-    @property
-    def translators_count(self):
-        """See `ILanguage`."""
-        return self.translators.count()
-
-
-class LanguageSet:
-    implements(ILanguageSet)
+        from lp.registry.model.person import PersonLanguage
+        return Join(
+            PersonLanguage,
+            Join(
+                KarmaCache,
+                KarmaCategory,
+                And(
+                    KarmaCategory.name == 'translations',
+                    KarmaCache.categoryID == KarmaCategory.id,
+                    KarmaCache.productID == None,
+                    KarmaCache.projectID == None,
+                    KarmaCache.sourcepackagenameID == None,
+                    KarmaCache.distributionID == None)),
+            PersonLanguage.personID ==
+                KarmaCache.personID)
 
     @property
     def _visible_languages(self):
@@ -180,14 +217,39 @@
         """See `ILanguageSet`."""
         return iter(self._visible_languages)
 
-    def getDefaultLanguages(self):
+    def getDefaultLanguages(self, want_translators_count=False):
         """See `ILanguageSet`."""
-        return self._visible_languages
+        return self.getAllLanguages(
+            want_translators_count=want_translators_count,
+            only_visible=True)
 
-    def getAllLanguages(self):
+    def getAllLanguages(self, want_translators_count=False,
+                        only_visible=False):
         """See `ILanguageSet`."""
-        return ISlaveStore(Language).find(Language).order_by(
+        result = IStore(Language).find(
+                Language,
+                Language.visible == True if only_visible else True,
+            ).order_by(
             Language.englishname)
+        if want_translators_count:
+            def preload_translators_count(languages):
+                from lp.registry.model.person import PersonLanguage
+                ids = set(language.id for language in languages).difference(
+                    set([None]))
+                counts = IStore(Language).using(
+                    LeftJoin(
+                        Language,
+                        self._getTranslatorJoins(),
+                        PersonLanguage.languageID == Language.id),
+                    ).find(
+                        (Language, Count(PersonLanguage)),
+                        Language.id.is_in(ids),
+                    ).group_by(Language)
+                for language, count in counts:
+                    get_property_cache(language).translators_count = count
+            return DecoratedResultSet(
+                result, pre_iter_hook=preload_translators_count)
+        return result
 
     def __iter__(self):
         """See `ILanguageSet`."""

=== modified file 'lib/lp/services/worlddata/tests/test_language.py'
--- lib/lp/services/worlddata/tests/test_language.py	2010-12-02 16:13:51 +0000
+++ lib/lp/services/worlddata/tests/test_language.py	2011-03-07 07:51:04 +0000
@@ -4,10 +4,26 @@
 __metaclass__ = type
 
 from lazr.lifecycle.interfaces import IDoNotSnapshot
+from testtools.matchers import Equals
+from zope.component import getUtility
 
-from canonical.testing import DatabaseFunctionalLayer
-from lp.services.worlddata.interfaces.language import ILanguage
-from lp.testing import TestCaseWithFactory
+from canonical.launchpad.interfaces.lpstorm import IStore
+from canonical.testing import (
+    DatabaseFunctionalLayer,
+    LaunchpadZopelessLayer,
+    )
+from lp.registry.interfaces.karma import IKarmaCacheManager
+from lp.registry.model.karma import KarmaCategory
+from lp.services.worlddata.interfaces.language import (
+    ILanguage,
+    ILanguageSet,
+    )
+from lp.testing import (
+    TestCaseWithFactory,
+    StormStatementRecorder,
+    )
+from lp.testing.dbuser import dbuser
+from lp.testing.matchers import HasQueryCount
 
 
 class TestLanguageWebservice(TestCaseWithFactory):
@@ -29,3 +45,44 @@
     def test_guessed_pluralforms_knows(self):
         language = self.factory.makeLanguage(pluralforms=3)
         self.assertEqual(language.pluralforms, language.guessed_pluralforms)
+
+
+class TestTranslatorsCounts(TestCaseWithFactory):
+    """Test preloading of Language.translators_counts."""
+
+    layer = LaunchpadZopelessLayer
+
+    def setUp(self):
+        super(TestTranslatorsCounts, self).setUp()
+        self.translated_lang = self.factory.makeLanguage(pluralforms=None)
+        self.untranslated_lang = self.factory.makeLanguage(pluralforms=None)
+        for i in range(3):
+            translator = self.factory.makePerson()
+            translator.addLanguage(self.translated_lang)
+            with dbuser('karma'):
+                translations_category = IStore(KarmaCategory).find(
+                    KarmaCategory, name='translations').one()
+                getUtility(IKarmaCacheManager).new(
+                    person_id=translator.id,
+                    category_id=translations_category.id,
+                    value=100)
+
+    def test_translators_count(self):
+        # translators_count works.
+        self.assertEquals(3, self.translated_lang.translators_count)
+        self.assertEquals(0, self.untranslated_lang.translators_count)
+
+    def test_translators_count_queries(self):
+        # translators_count issues a single query.
+        with StormStatementRecorder() as recorder:
+            self.translated_lang.translators_count
+        self.assertThat(recorder, HasQueryCount(Equals(1)))
+
+    def test_getAllLanguages_can_preload_translators_count(self):
+        # LanguageSet.getAllLanguages() can preload translators_count.
+        list(getUtility(ILanguageSet).getAllLanguages(
+            want_translators_count=True))
+        with StormStatementRecorder() as recorder:
+            self.assertEquals(3, self.translated_lang.translators_count)
+            self.assertEquals(0, self.untranslated_lang.translators_count)
+        self.assertThat(recorder, HasQueryCount(Equals(0)))