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
1=== added file 'lib/lp/translations/browser/tests/test_translationgroup.py'
2--- lib/lp/translations/browser/tests/test_translationgroup.py 1970-01-01 00:00:00 +0000
3+++ lib/lp/translations/browser/tests/test_translationgroup.py 2010-03-03 20:17:26 +0000
4@@ -0,0 +1,69 @@
5+# Copyright 2010 Canonical Ltd. This software is licensed under the
6+# GNU Affero General Public License version 3 (see the file LICENSE).
7+
8+"""View tests for TranslationGroup."""
9+
10+__metaclass__ = type
11+
12+import unittest
13+
14+from zope.component import getUtility
15+
16+from canonical.testing import LaunchpadZopelessLayer
17+
18+from canonical.launchpad.webapp.servers import LaunchpadTestRequest
19+from lp.services.worlddata.interfaces.language import ILanguageSet
20+from lp.translations.browser.translationgroup import TranslationGroupView
21+from lp.testing import TestCaseWithFactory
22+
23+
24+class TestTranslationGroupView(TestCaseWithFactory):
25+ layer = LaunchpadZopelessLayer
26+
27+ def setUp(self):
28+ super(TestTranslationGroupView, self).setUp()
29+
30+ def _makeGroup(self):
31+ """Create a translation group."""
32+ return self.factory.makeTranslationGroup()
33+
34+ def _makeView(self, group):
35+ """Create a view for self.group."""
36+ view = TranslationGroupView(group, LaunchpadTestRequest())
37+ view.initialize()
38+ return view
39+
40+ def test_translator_list_empty(self):
41+ view = self._makeView(self._makeGroup())
42+ self.assertEqual([], view.translator_list)
43+
44+ def test_translator_list(self):
45+ # translator_list composes dicts using _makeTranslatorDict.
46+ group = self._makeGroup()
47+ tr_translator = self.factory.makeTranslator('tr', group)
48+ view = self._makeView(group)
49+ translator_dict = view._makeTranslatorDict(
50+ tr_translator, tr_translator.language, tr_translator.translator)
51+ self.assertEqual([translator_dict], list(view.translator_list))
52+
53+ def test_makeTranslatorDict(self):
54+ # _makeTranslatorDict describes a Translator entry to the UI.
55+ group = self._makeGroup()
56+ xhosa = self.factory.makeTranslator('xh', group)
57+ xhosa.style_guide_url = 'http://xh.example.com/'
58+ view = self._makeView(group)
59+ output = view._makeTranslatorDict(
60+ xhosa, xhosa.language, xhosa.translator)
61+
62+ self.assertEqual(xhosa.translator, output['person'])
63+ self.assertEqual('xh', output['code'])
64+ self.assertEqual(
65+ getUtility(ILanguageSet).getLanguageByCode('xh'),
66+ output['language'])
67+ self.assertEqual(xhosa.datecreated, output['datecreated'])
68+ self.assertEqual(xhosa.style_guide_url, output['style_guide_url'])
69+ self.assertEqual(xhosa, output['context'])
70+
71+
72+def test_suite():
73+ return unittest.TestLoader().loadTestsFromName(__name__)
74
75=== modified file 'lib/lp/translations/browser/translationgroup.py'
76--- lib/lp/translations/browser/translationgroup.py 2010-02-17 15:50:00 +0000
77+++ lib/lp/translations/browser/translationgroup.py 2010-03-03 20:17:26 +0000
78@@ -16,8 +16,6 @@
79 'TranslationGroupView',
80 ]
81
82-import operator
83-
84 from zope.component import getUtility
85
86 from lp.translations.interfaces.translationgroup import (
87@@ -29,7 +27,7 @@
88 from canonical.launchpad.webapp.interfaces import NotFoundError
89 from canonical.launchpad.webapp import (
90 action, canonical_url, GetitemNavigation, LaunchpadEditFormView,
91- LaunchpadFormView)
92+ LaunchpadFormView, LaunchpadView)
93 from canonical.launchpad.webapp.breadcrumb import Breadcrumb
94
95
96@@ -54,9 +52,10 @@
97 label = page_title
98
99
100-class TranslationGroupView:
101+class TranslationGroupView(LaunchpadView):
102
103 def __init__(self, context, request):
104+ super(TranslationGroupView, self).__init__(context, request)
105 self.context = context
106 self.request = request
107 self.translation_groups = getUtility(ITranslationGroupSet)
108@@ -69,20 +68,28 @@
109 def page_title(self):
110 return self.context.title
111
112+ def _makeTranslatorDict(self, translator, language, person):
113+ """Compose dict describing a `Translator` for UI use.
114+
115+ Parameters are the ones found in each item in the sequence
116+ returned by `TranslationGroup.fetchTranslatorData`.
117+ """
118+ return {
119+ 'person': person,
120+ 'code': language.code,
121+ 'language': language,
122+ 'datecreated': translator.datecreated,
123+ 'style_guide_url': translator.style_guide_url,
124+ 'context': translator,
125+ }
126+
127 @property
128 def translator_list(self):
129- result = []
130- for item in self.context.translators:
131- result.append({'lang': item.language.englishname,
132- 'person': item.translator,
133- 'code': item.language.code,
134- 'language': item.language,
135- 'datecreated': item.datecreated,
136- 'style_guide_url': item.style_guide_url,
137- 'context': item,
138- })
139- result.sort(key=operator.itemgetter('lang'))
140- return result
141+ """List of dicts describing the translation teams."""
142+ return [
143+ self._makeTranslatorDict(*data)
144+ for data in self.context.fetchTranslatorData()
145+ ]
146
147
148 class TranslationGroupAddTranslatorView(LaunchpadFormView):
149
150=== modified file 'lib/lp/translations/doc/translationgroup.txt'
151--- lib/lp/translations/doc/translationgroup.txt 2009-08-25 10:20:25 +0000
152+++ lib/lp/translations/doc/translationgroup.txt 2010-03-03 20:17:26 +0000
153@@ -157,3 +157,40 @@
154 cy
155 foo-translators
156 None
157+
158+
159+fetchTranslatorData
160+-------------------
161+
162+Use fetchTranslator data to get all members of a translation group,
163+with their respective assigned languages, in one go. This saves
164+repeated querying.
165+
166+ >>> group = factory.makeTranslationGroup()
167+ >>> list(group.fetchTranslatorData())
168+ []
169+
170+ >>> de_team = factory.makeTeam(name='de-team')
171+ >>> nl_team = factory.makeTeam(name='nl-team')
172+ >>> la_team = factory.makeTeam(name='la-team')
173+ >>> de_translator = factory.makeTranslator('de', group, person=de_team)
174+ >>> nl_translator = factory.makeTranslator('nl', group, person=nl_team)
175+ >>> la_translator = factory.makeTranslator('la', group, person=la_team)
176+
177+The method returns tuples of respectively a Translator ("translation
178+group membership entry"), its language, and the actual team or person
179+assigned to that language.
180+
181+ >>> for (translator, language, team) in group.fetchTranslatorData():
182+ ... print translator.language.code, language.code, team.name
183+ nl nl nl-team
184+ de de de-team
185+ la la la-team
186+
187+The members are sorted by language name in English.
188+
189+ >>> for (translator, language, person) in group.fetchTranslatorData():
190+ ... print language.englishname
191+ Dutch
192+ German
193+ Latin
194
195=== modified file 'lib/lp/translations/interfaces/translationgroup.py'
196--- lib/lp/translations/interfaces/translationgroup.py 2010-02-02 16:02:34 +0000
197+++ lib/lp/translations/interfaces/translationgroup.py 2010-03-03 20:17:26 +0000
198@@ -1,4 +1,4 @@
199-# Copyright 2009 Canonical Ltd. This software is licensed under the
200+# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
201 # GNU Affero General Public License version 3 (see the file LICENSE).
202
203 # pylint: disable-msg=E0211,E0213
204@@ -148,9 +148,6 @@
205 def query_translator(language):
206 """Retrieve a translator, or None, based on a Language"""
207
208- def __getitem__(languagecode):
209- """Retrieve the translator for the given language in this group."""
210-
211 # adding and removing translators
212 def remove_translator(language):
213 """Remove the translator for this language from the group."""
214@@ -165,6 +162,19 @@
215 number_of_remaining_projects = Attribute(
216 "Count of remaining projects not listed in the `top_projects`.")
217
218+ def __getitem__(language_code):
219+ """Retrieve the translator for the given language in this group.
220+
221+ This is used for navigation through the group.
222+ """
223+
224+ def fetchTranslatorData():
225+ """Fetch translators and related data.
226+
227+ :return: A tuple (`Translator`, `Language`, `Person`), ordered
228+ by language name in English.
229+ """
230+
231
232 class ITranslationGroupSet(Interface):
233 """A container for translation groups."""
234
235=== modified file 'lib/lp/translations/model/translationgroup.py'
236--- lib/lp/translations/model/translationgroup.py 2009-08-25 10:24:51 +0000
237+++ lib/lp/translations/model/translationgroup.py 2010-03-03 20:17:26 +0000
238@@ -19,14 +19,16 @@
239 from storm.expr import Join
240 from storm.store import Store
241
242-from lp.services.worlddata.interfaces.language import ILanguageSet
243 from lp.translations.interfaces.translationgroup import (
244 ITranslationGroup, ITranslationGroupSet)
245+from lp.registry.model.person import Person
246 from lp.registry.model.product import Product
247 from lp.registry.model.project import Project
248 from lp.registry.model.teammembership import TeamParticipation
249+from lp.services.worlddata.model.language import Language
250 from lp.translations.model.translator import Translator
251-from canonical.launchpad.webapp.interfaces import NotFoundError
252+from canonical.launchpad.webapp.interfaces import (
253+ DEFAULT_FLAVOR, IStoreSelector, MAIN_STORE, NotFoundError)
254
255 from canonical.database.sqlbase import SQLBase
256 from canonical.database.constants import DEFAULT
257@@ -61,6 +63,21 @@
258 joinColumn='translationgroup')
259 translation_guide_url = StringCol(notNull=False, default=None)
260
261+ def __getitem__(self, language_code):
262+ """See `ITranslationGroup`."""
263+ store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
264+ query = store.find(
265+ Translator,
266+ Translator.translationgroup == self,
267+ Translator.languageID == Language.id,
268+ Language.code == language_code)
269+
270+ translator = query.one()
271+ if translator is None:
272+ raise NotFoundError, language_code
273+
274+ return translator
275+
276 # used to note additions
277 def add(self, content):
278 """See ITranslationGroup."""
279@@ -117,17 +134,15 @@
280 else:
281 return 0
282
283-
284- # get a translator by code
285- def __getitem__(self, code):
286+ def fetchTranslatorData(self):
287 """See ITranslationGroup."""
288- language_set = getUtility(ILanguageSet)
289- language = language_set[code]
290- result = Translator.selectOneBy(language=language,
291- translationgroup=self)
292- if result is None:
293- raise NotFoundError, code
294- return result
295+ store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
296+ translator_data = store.find(
297+ (Translator, Language, Person),
298+ Translator.translationgroup == self,
299+ Language.id == Translator.languageID,
300+ Person.id == Translator.translatorID)
301+ return translator_data.order_by(Language.englishname)
302
303
304 class TranslationGroupSet:
305
306=== modified file 'lib/lp/translations/templates/translationgroup-index.pt'
307--- lib/lp/translations/templates/translationgroup-index.pt 2009-09-17 11:28:13 +0000
308+++ lib/lp/translations/templates/translationgroup-index.pt 2010-03-03 20:17:26 +0000
309@@ -71,9 +71,10 @@
310 <td class="translator-link">
311 <a tal:condition="translator/style_guide_url"
312 tal:attributes="href translator/style_guide_url"
313- ><img src="/@@/link" alt="Doc"
314+ ><span alt="Doc"
315 tal:attributes="title translator/style_guide_url"
316- />
317+ class="external-link sprite"
318+ ></span>
319 <tal:link
320 replace="translator/style_guide_url/fmt:shorten/30"
321 ></tal:link></a>