Launchpad itself

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

  1. bug-327575
  2. Merge into devel
Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~jtv/launchpad/bug-327575
Merge into: lp:launchpad
Diff against target: 321 lines (+173/-34)
6 files modified
lib/lp/translations/browser/tests/test_translationgroup.py (+69/-0)
lib/lp/translations/browser/translationgroup.py (+23/-16)
lib/lp/translations/doc/translationgroup.txt (+37/-0)
lib/lp/translations/interfaces/translationgroup.py (+14/-4)
lib/lp/translations/model/translationgroup.py (+27/-12)
lib/lp/translations/templates/translationgroup-index.pt (+3/-2)
To merge this branch: bzr merge lp:~jtv/launchpad/bug-327575
Reviewer Review Type Date Requested Status
Muharem Hrnjadovic (community) code Approve
Review via email: mp+20591@code.launchpad.net

Commit message

Speed up TranslationGroup:+index

To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

= 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

Revision history for this message
Muharem Hrnjadovic (al-maisan) wrote :

Nice branch!

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""View tests for TranslationGroup."""
5
6__metaclass__ = type
7
8import unittest
9
10from zope.component import getUtility
11
12from canonical.testing import LaunchpadZopelessLayer
13
14from canonical.launchpad.webapp.servers import LaunchpadTestRequest
15from lp.services.worlddata.interfaces.language import ILanguageSet
16from lp.translations.browser.translationgroup import TranslationGroupView
17from lp.testing import TestCaseWithFactory
18
19
20class TestTranslationGroupView(TestCaseWithFactory):
21 layer = LaunchpadZopelessLayer
22
23 def setUp(self):
24 super(TestTranslationGroupView, self).setUp()
25
26 def _makeGroup(self):
27 """Create a translation group."""
28 return self.factory.makeTranslationGroup()
29
30 def _makeView(self, group):
31 """Create a view for self.group."""
32 view = TranslationGroupView(group, LaunchpadTestRequest())
33 view.initialize()
34 return view
35
36 def test_translator_list_empty(self):
37 view = self._makeView(self._makeGroup())
38 self.assertEqual([], view.translator_list)
39
40 def test_translator_list(self):
41 # translator_list composes dicts using _makeTranslatorDict.
42 group = self._makeGroup()
43 tr_translator = self.factory.makeTranslator('tr', group)
44 view = self._makeView(group)
45 translator_dict = view._makeTranslatorDict(
46 tr_translator, tr_translator.language, tr_translator.translator)
47 self.assertEqual([translator_dict], list(view.translator_list))
48
49 def test_makeTranslatorDict(self):
50 # _makeTranslatorDict describes a Translator entry to the UI.
51 group = self._makeGroup()
52 xhosa = self.factory.makeTranslator('xh', group)
53 xhosa.style_guide_url = 'http://xh.example.com/'
54 view = self._makeView(group)
55 output = view._makeTranslatorDict(
56 xhosa, xhosa.language, xhosa.translator)
57
58 self.assertEqual(xhosa.translator, output['person'])
59 self.assertEqual('xh', output['code'])
60 self.assertEqual(
61 getUtility(ILanguageSet).getLanguageByCode('xh'),
62 output['language'])
63 self.assertEqual(xhosa.datecreated, output['datecreated'])
64 self.assertEqual(xhosa.style_guide_url, output['style_guide_url'])
65 self.assertEqual(xhosa, output['context'])
66
67
68def test_suite():
69 return unittest.TestLoader().loadTestsFromName(__name__)
070
=== 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 @@
16 'TranslationGroupView',16 'TranslationGroupView',
17 ]17 ]
1818
19import operator
20
21from zope.component import getUtility19from zope.component import getUtility
2220
23from lp.translations.interfaces.translationgroup import (21from lp.translations.interfaces.translationgroup import (
@@ -29,7 +27,7 @@
29from canonical.launchpad.webapp.interfaces import NotFoundError27from canonical.launchpad.webapp.interfaces import NotFoundError
30from canonical.launchpad.webapp import (28from canonical.launchpad.webapp import (
31 action, canonical_url, GetitemNavigation, LaunchpadEditFormView,29 action, canonical_url, GetitemNavigation, LaunchpadEditFormView,
32 LaunchpadFormView)30 LaunchpadFormView, LaunchpadView)
33from canonical.launchpad.webapp.breadcrumb import Breadcrumb31from canonical.launchpad.webapp.breadcrumb import Breadcrumb
3432
3533
@@ -54,9 +52,10 @@
54 label = page_title52 label = page_title
5553
5654
57class TranslationGroupView:55class TranslationGroupView(LaunchpadView):
5856
59 def __init__(self, context, request):57 def __init__(self, context, request):
58 super(TranslationGroupView, self).__init__(context, request)
60 self.context = context59 self.context = context
61 self.request = request60 self.request = request
62 self.translation_groups = getUtility(ITranslationGroupSet)61 self.translation_groups = getUtility(ITranslationGroupSet)
@@ -69,20 +68,28 @@
69 def page_title(self):68 def page_title(self):
70 return self.context.title69 return self.context.title
7170
71 def _makeTranslatorDict(self, translator, language, person):
72 """Compose dict describing a `Translator` for UI use.
73
74 Parameters are the ones found in each item in the sequence
75 returned by `TranslationGroup.fetchTranslatorData`.
76 """
77 return {
78 'person': person,
79 'code': language.code,
80 'language': language,
81 'datecreated': translator.datecreated,
82 'style_guide_url': translator.style_guide_url,
83 'context': translator,
84 }
85
72 @property86 @property
73 def translator_list(self):87 def translator_list(self):
74 result = []88 """List of dicts describing the translation teams."""
75 for item in self.context.translators:89 return [
76 result.append({'lang': item.language.englishname,90 self._makeTranslatorDict(*data)
77 'person': item.translator,91 for data in self.context.fetchTranslatorData()
78 'code': item.language.code,92 ]
79 'language': item.language,
80 'datecreated': item.datecreated,
81 'style_guide_url': item.style_guide_url,
82 'context': item,
83 })
84 result.sort(key=operator.itemgetter('lang'))
85 return result
8693
8794
88class TranslationGroupAddTranslatorView(LaunchpadFormView):95class TranslationGroupAddTranslatorView(LaunchpadFormView):
8996
=== 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 @@
157 cy157 cy
158 foo-translators158 foo-translators
159 None159 None
160
161
162fetchTranslatorData
163-------------------
164
165Use fetchTranslator data to get all members of a translation group,
166with their respective assigned languages, in one go. This saves
167repeated querying.
168
169 >>> group = factory.makeTranslationGroup()
170 >>> list(group.fetchTranslatorData())
171 []
172
173 >>> de_team = factory.makeTeam(name='de-team')
174 >>> nl_team = factory.makeTeam(name='nl-team')
175 >>> la_team = factory.makeTeam(name='la-team')
176 >>> de_translator = factory.makeTranslator('de', group, person=de_team)
177 >>> nl_translator = factory.makeTranslator('nl', group, person=nl_team)
178 >>> la_translator = factory.makeTranslator('la', group, person=la_team)
179
180The method returns tuples of respectively a Translator ("translation
181group membership entry"), its language, and the actual team or person
182assigned to that language.
183
184 >>> for (translator, language, team) in group.fetchTranslatorData():
185 ... print translator.language.code, language.code, team.name
186 nl nl nl-team
187 de de de-team
188 la la la-team
189
190The members are sorted by language name in English.
191
192 >>> for (translator, language, person) in group.fetchTranslatorData():
193 ... print language.englishname
194 Dutch
195 German
196 Latin
160197
=== 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 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4# pylint: disable-msg=E0211,E02134# pylint: disable-msg=E0211,E0213
@@ -148,9 +148,6 @@
148 def query_translator(language):148 def query_translator(language):
149 """Retrieve a translator, or None, based on a Language"""149 """Retrieve a translator, or None, based on a Language"""
150150
151 def __getitem__(languagecode):
152 """Retrieve the translator for the given language in this group."""
153
154 # adding and removing translators151 # adding and removing translators
155 def remove_translator(language):152 def remove_translator(language):
156 """Remove the translator for this language from the group."""153 """Remove the translator for this language from the group."""
@@ -165,6 +162,19 @@
165 number_of_remaining_projects = Attribute(162 number_of_remaining_projects = Attribute(
166 "Count of remaining projects not listed in the `top_projects`.")163 "Count of remaining projects not listed in the `top_projects`.")
167164
165 def __getitem__(language_code):
166 """Retrieve the translator for the given language in this group.
167
168 This is used for navigation through the group.
169 """
170
171 def fetchTranslatorData():
172 """Fetch translators and related data.
173
174 :return: A tuple (`Translator`, `Language`, `Person`), ordered
175 by language name in English.
176 """
177
168178
169class ITranslationGroupSet(Interface):179class ITranslationGroupSet(Interface):
170 """A container for translation groups."""180 """A container for translation groups."""
171181
=== 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 @@
19from storm.expr import Join19from storm.expr import Join
20from storm.store import Store20from storm.store import Store
2121
22from lp.services.worlddata.interfaces.language import ILanguageSet
23from lp.translations.interfaces.translationgroup import (22from lp.translations.interfaces.translationgroup import (
24 ITranslationGroup, ITranslationGroupSet)23 ITranslationGroup, ITranslationGroupSet)
24from lp.registry.model.person import Person
25from lp.registry.model.product import Product25from lp.registry.model.product import Product
26from lp.registry.model.project import Project26from lp.registry.model.project import Project
27from lp.registry.model.teammembership import TeamParticipation27from lp.registry.model.teammembership import TeamParticipation
28from lp.services.worlddata.model.language import Language
28from lp.translations.model.translator import Translator29from lp.translations.model.translator import Translator
29from canonical.launchpad.webapp.interfaces import NotFoundError30from canonical.launchpad.webapp.interfaces import (
31 DEFAULT_FLAVOR, IStoreSelector, MAIN_STORE, NotFoundError)
3032
31from canonical.database.sqlbase import SQLBase33from canonical.database.sqlbase import SQLBase
32from canonical.database.constants import DEFAULT34from canonical.database.constants import DEFAULT
@@ -61,6 +63,21 @@
61 joinColumn='translationgroup')63 joinColumn='translationgroup')
62 translation_guide_url = StringCol(notNull=False, default=None)64 translation_guide_url = StringCol(notNull=False, default=None)
6365
66 def __getitem__(self, language_code):
67 """See `ITranslationGroup`."""
68 store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
69 query = store.find(
70 Translator,
71 Translator.translationgroup == self,
72 Translator.languageID == Language.id,
73 Language.code == language_code)
74
75 translator = query.one()
76 if translator is None:
77 raise NotFoundError, language_code
78
79 return translator
80
64 # used to note additions81 # used to note additions
65 def add(self, content):82 def add(self, content):
66 """See ITranslationGroup."""83 """See ITranslationGroup."""
@@ -117,17 +134,15 @@
117 else:134 else:
118 return 0135 return 0
119136
120137 def fetchTranslatorData(self):
121 # get a translator by code
122 def __getitem__(self, code):
123 """See ITranslationGroup."""138 """See ITranslationGroup."""
124 language_set = getUtility(ILanguageSet)139 store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
125 language = language_set[code]140 translator_data = store.find(
126 result = Translator.selectOneBy(language=language,141 (Translator, Language, Person),
127 translationgroup=self)142 Translator.translationgroup == self,
128 if result is None:143 Language.id == Translator.languageID,
129 raise NotFoundError, code144 Person.id == Translator.translatorID)
130 return result145 return translator_data.order_by(Language.englishname)
131146
132147
133class TranslationGroupSet:148class TranslationGroupSet:
134149
=== 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 @@
71 <td class="translator-link">71 <td class="translator-link">
72 <a tal:condition="translator/style_guide_url"72 <a tal:condition="translator/style_guide_url"
73 tal:attributes="href translator/style_guide_url"73 tal:attributes="href translator/style_guide_url"
74 ><img src="/@@/link" alt="Doc"74 ><span alt="Doc"
75 tal:attributes="title translator/style_guide_url"75 tal:attributes="title translator/style_guide_url"
76 />76 class="external-link sprite"
77 ></span>
77 <tal:link78 <tal:link
78 replace="translator/style_guide_url/fmt:shorten/30"79 replace="translator/style_guide_url/fmt:shorten/30"
79 ></tal:link></a>80 ></tal:link></a>