launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02872
[Merge] lp:~wgrant/launchpad/bug-728174 into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/bug-728174 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#728174 LanguageSet:CollectionResource:#languages and LanguageSet:CollectionResource:#languages:getAllLanguages timeouts
https://bugs.launchpad.net/bugs/728174
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-728174/+merge/52374
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.
--
https://code.launchpad.net/~wgrant/launchpad/bug-728174/+merge/52374
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-728174 into lp:launchpad.
=== 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 06:51:33 +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 06:51:33 +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,32 @@
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 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 +55,7 @@
from lp.services.worlddata.model.spokenin import SpokenIn
SpokenIn
+
class Language(SQLBase):
implements(ILanguage)
@@ -140,34 +157,51 @@
@property
def translators(self):
"""See `ILanguage`."""
+ from lp.registry.model.person import (
+ Person,
+ PersonLanguage,
+ )
+ return ISlaveStore(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 +214,38 @@
"""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 = ISlaveStore(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 = [language.id for language in languages]
+ counts = ISlaveStore(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 06:51:33 +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,47 @@
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.
+ slave_langs = list(getUtility(ILanguageSet).getAllLanguages(
+ want_translators_count=True))
+ with StormStatementRecorder() as recorder:
+ for lang in slave_langs:
+ if lang == self.translated_lang:
+ self.assertEquals(3, lang.translators_count)
+ if lang == self.untranslated_lang:
+ self.assertEquals(0, lang.translators_count)
+ self.assertThat(recorder, HasQueryCount(Equals(0)))
