Merge lp:~jtv/launchpad/bug-327575 into lp:launchpad

[Jeroen T. Vermeulen]

Speed up TranslationGroup:+index

[Jeroen T. Vermeulen (jtv)]

= Bug 327575 =

We're seeing some timeouts (and a robust ranking in the top-10 query counts) for the TranslationGroup:+index page.

The culprit seems to be lots of Person queries, which can only be the translation teams in the group. There are also repeated queries for Language.

Here I introduce a view method that batch-fetches this information. It's an ideal candidate for batch-fetching: a simple three-way 1-to-1 join, no repetition whatsoever, no UI batching, and no selectivity beyond the "front gate" of the query.

In other news, the TranslationGroupView becomes an adopted child of LaunchpadView, and an <img> in the template becomes a sprite.

{{{
./bin/test -vv -t TranslationGroupView -t /translationgroup.txt
}}}

Jeroen

[Muharem Hrnjadovic (al-maisan)]

Nice branch!

review: Approve (code)

=== added file 'lib/lp/translations/browser/tests/test_translationgroup.py'
--- lib/lp/translations/browser/tests/test_translationgroup.py	1970-01-01 00:00:00 +0000
+++ lib/lp/translations/browser/tests/test_translationgroup.py	2010-03-03 20:17:26 +0000
@@ -0,0 +1,69 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""View tests for TranslationGroup."""
+
+__metaclass__ = type
+
+import unittest
+
+from zope.component import getUtility
+
+from canonical.testing import LaunchpadZopelessLayer
+
+from canonical.launchpad.webapp.servers import LaunchpadTestRequest
+from lp.services.worlddata.interfaces.language import ILanguageSet
+from lp.translations.browser.translationgroup import TranslationGroupView
+from lp.testing import TestCaseWithFactory
+
+
+class TestTranslationGroupView(TestCaseWithFactory):
+    layer = LaunchpadZopelessLayer
+
+    def setUp(self):
+        super(TestTranslationGroupView, self).setUp()
+
+    def _makeGroup(self):
+        """Create a translation group."""
+        return self.factory.makeTranslationGroup()
+
+    def _makeView(self, group):
+        """Create a view for self.group."""
+        view = TranslationGroupView(group, LaunchpadTestRequest())
+        view.initialize()
+        return view
+
+    def test_translator_list_empty(self):
+        view = self._makeView(self._makeGroup())
+        self.assertEqual([], view.translator_list)
+
+    def test_translator_list(self):
+        # translator_list composes dicts using _makeTranslatorDict.
+        group = self._makeGroup()
+        tr_translator = self.factory.makeTranslator('tr', group)
+        view = self._makeView(group)
+        translator_dict = view._makeTranslatorDict(
+            tr_translator, tr_translator.language, tr_translator.translator)
+        self.assertEqual([translator_dict], list(view.translator_list))
+
+    def test_makeTranslatorDict(self):
+        # _makeTranslatorDict describes a Translator entry to the UI.
+        group = self._makeGroup()
+        xhosa = self.factory.makeTranslator('xh', group)
+        xhosa.style_guide_url = 'http://xh.example.com/'
+        view = self._makeView(group)
+        output = view._makeTranslatorDict(
+            xhosa, xhosa.language, xhosa.translator)
+
+        self.assertEqual(xhosa.translator, output['person'])
+        self.assertEqual('xh', output['code'])
+        self.assertEqual(
+            getUtility(ILanguageSet).getLanguageByCode('xh'),
+            output['language'])
+        self.assertEqual(xhosa.datecreated, output['datecreated'])
+        self.assertEqual(xhosa.style_guide_url, output['style_guide_url'])
+        self.assertEqual(xhosa, output['context'])
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/translations/browser/translationgroup.py'
--- lib/lp/translations/browser/translationgroup.py	2010-02-17 15:50:00 +0000
+++ lib/lp/translations/browser/translationgroup.py	2010-03-03 20:17:26 +0000
@@ -16,8 +16,6 @@
     'TranslationGroupView',
     ]
 
-import operator
-
 from zope.component import getUtility
 
 from lp.translations.interfaces.translationgroup import (
@@ -29,7 +27,7 @@
 from canonical.launchpad.webapp.interfaces import NotFoundError
 from canonical.launchpad.webapp import (
     action, canonical_url, GetitemNavigation, LaunchpadEditFormView,
-    LaunchpadFormView)
+    LaunchpadFormView, LaunchpadView)
 from canonical.launchpad.webapp.breadcrumb import Breadcrumb
 
 
@@ -54,9 +52,10 @@
     label = page_title
 
 
-class TranslationGroupView:
+class TranslationGroupView(LaunchpadView):
 
     def __init__(self, context, request):
+        super(TranslationGroupView, self).__init__(context, request)
         self.context = context
         self.request = request
         self.translation_groups = getUtility(ITranslationGroupSet)
@@ -69,20 +68,28 @@
     def page_title(self):
         return self.context.title
 
+    def _makeTranslatorDict(self, translator, language, person):
+        """Compose dict describing a `Translator` for UI use.
+
+        Parameters are the ones found in each item in the sequence
+        returned by `TranslationGroup.fetchTranslatorData`.
+        """
+        return {
+                'person': person,
+                'code': language.code,
+                'language': language,
+                'datecreated': translator.datecreated,
+                'style_guide_url': translator.style_guide_url,
+                'context': translator,
+            }
+
     @property
     def translator_list(self):
-        result = []
-        for item in self.context.translators:
-            result.append({'lang': item.language.englishname,
-                           'person': item.translator,
-                           'code': item.language.code,
-                           'language': item.language,
-                           'datecreated': item.datecreated,
-                           'style_guide_url': item.style_guide_url,
-                           'context': item,
-                           })
-        result.sort(key=operator.itemgetter('lang'))
-        return result
+        """List of dicts describing the translation teams."""
+        return [
+            self._makeTranslatorDict(*data)
+            for data in self.context.fetchTranslatorData()
+            ]
 
 
 class TranslationGroupAddTranslatorView(LaunchpadFormView):

=== modified file 'lib/lp/translations/doc/translationgroup.txt'
--- lib/lp/translations/doc/translationgroup.txt	2009-08-25 10:20:25 +0000
+++ lib/lp/translations/doc/translationgroup.txt	2010-03-03 20:17:26 +0000
@@ -157,3 +157,40 @@
     cy
     foo-translators
     None
+
+
+fetchTranslatorData
+-------------------
+
+Use fetchTranslator data to get all members of a translation group,
+with their respective assigned languages, in one go.  This saves
+repeated querying.
+
+    >>> group = factory.makeTranslationGroup()
+    >>> list(group.fetchTranslatorData())
+    []
+
+    >>> de_team = factory.makeTeam(name='de-team')
+    >>> nl_team = factory.makeTeam(name='nl-team')
+    >>> la_team = factory.makeTeam(name='la-team')
+    >>> de_translator = factory.makeTranslator('de', group, person=de_team)
+    >>> nl_translator = factory.makeTranslator('nl', group, person=nl_team)
+    >>> la_translator = factory.makeTranslator('la', group, person=la_team)
+
+The method returns tuples of respectively a Translator ("translation
+group membership entry"), its language, and the actual team or person
+assigned to that language.
+
+    >>> for (translator, language, team) in group.fetchTranslatorData():
+    ...     print translator.language.code, language.code, team.name
+    nl nl nl-team
+    de de de-team
+    la la la-team
+
+The members are sorted by language name in English.
+
+    >>> for (translator, language, person) in group.fetchTranslatorData():
+    ...     print language.englishname
+    Dutch
+    German
+    Latin

=== modified file 'lib/lp/translations/interfaces/translationgroup.py'
--- lib/lp/translations/interfaces/translationgroup.py	2010-02-02 16:02:34 +0000
+++ lib/lp/translations/interfaces/translationgroup.py	2010-03-03 20:17:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# 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=E0211,E0213
@@ -148,9 +148,6 @@
     def query_translator(language):
         """Retrieve a translator, or None, based on a Language"""
 
-    def __getitem__(languagecode):
-        """Retrieve the translator for the given language in this group."""
-
     # adding and removing translators
     def remove_translator(language):
         """Remove the translator for this language from the group."""
@@ -165,6 +162,19 @@
     number_of_remaining_projects = Attribute(
         "Count of remaining projects not listed in the `top_projects`.")
 
+    def __getitem__(language_code):
+        """Retrieve the translator for the given language in this group.
+
+        This is used for navigation through the group.
+        """
+
+    def fetchTranslatorData():
+        """Fetch translators and related data.
+
+        :return: A tuple (`Translator`, `Language`, `Person`), ordered
+            by language name in English.
+        """
+
 
 class ITranslationGroupSet(Interface):
     """A container for translation groups."""

=== modified file 'lib/lp/translations/model/translationgroup.py'
--- lib/lp/translations/model/translationgroup.py	2009-08-25 10:24:51 +0000
+++ lib/lp/translations/model/translationgroup.py	2010-03-03 20:17:26 +0000
@@ -19,14 +19,16 @@
 from storm.expr import Join
 from storm.store import Store
 
-from lp.services.worlddata.interfaces.language import ILanguageSet
 from lp.translations.interfaces.translationgroup import (
     ITranslationGroup, ITranslationGroupSet)
+from lp.registry.model.person import Person
 from lp.registry.model.product import Product
 from lp.registry.model.project import Project
 from lp.registry.model.teammembership import TeamParticipation
+from lp.services.worlddata.model.language import Language
 from lp.translations.model.translator import Translator
-from canonical.launchpad.webapp.interfaces import NotFoundError
+from canonical.launchpad.webapp.interfaces import (
+    DEFAULT_FLAVOR, IStoreSelector, MAIN_STORE, NotFoundError)
 
 from canonical.database.sqlbase import SQLBase
 from canonical.database.constants import DEFAULT
@@ -61,6 +63,21 @@
                                   joinColumn='translationgroup')
     translation_guide_url = StringCol(notNull=False, default=None)
 
+    def __getitem__(self, language_code):
+        """See `ITranslationGroup`."""
+        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
+        query = store.find(
+            Translator,
+            Translator.translationgroup == self,
+            Translator.languageID == Language.id,
+            Language.code == language_code)
+        
+        translator = query.one()
+        if translator is None:
+            raise NotFoundError, language_code
+
+        return translator
+
     # used to note additions
     def add(self, content):
         """See ITranslationGroup."""
@@ -117,17 +134,15 @@
         else:
             return 0
 
-
-    # get a translator by code
-    def __getitem__(self, code):
+    def fetchTranslatorData(self):
         """See ITranslationGroup."""
-        language_set = getUtility(ILanguageSet)
-        language = language_set[code]
-        result = Translator.selectOneBy(language=language,
-                                        translationgroup=self)
-        if result is None:
-            raise NotFoundError, code
-        return result
+        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
+        translator_data = store.find(
+            (Translator, Language, Person),
+            Translator.translationgroup == self,
+            Language.id == Translator.languageID,
+            Person.id == Translator.translatorID)
+        return translator_data.order_by(Language.englishname)
 
 
 class TranslationGroupSet:

=== modified file 'lib/lp/translations/templates/translationgroup-index.pt'
--- lib/lp/translations/templates/translationgroup-index.pt	2009-09-17 11:28:13 +0000
+++ lib/lp/translations/templates/translationgroup-index.pt	2010-03-03 20:17:26 +0000
@@ -71,9 +71,10 @@
                 <td class="translator-link">
                   <a tal:condition="translator/style_guide_url"
                      tal:attributes="href translator/style_guide_url"
-                     ><img src="/@@/link" alt="Doc"
+                     ><span alt="Doc"
                            tal:attributes="title translator/style_guide_url"
-                           />
+                           class="external-link sprite"
+                           ></span>
                     <tal:link
                        replace="translator/style_guide_url/fmt:shorten/30"
                        ></tal:link></a>