Merge lp:~lifeless/launchpad/prejoin into lp:launchpad

Proposed by Robert Collins
Status: Merged
Approved by: Robert Collins
Approved revision: no longer in the source branch.
Merged at revision: 11956
Proposed branch: lp:~lifeless/launchpad/prejoin
Merge into: lp:launchpad
Diff against target: 455 lines (+30/-259)
7 files modified
lib/lp/code/model/branch.py (+6/-3)
lib/lp/registry/model/product.py (+4/-2)
lib/lp/services/database/configure.zcml (+0/-4)
lib/lp/services/database/prejoin.py (+0/-118)
lib/lp/services/database/tests/test_prejoin.py (+0/-118)
lib/lp/translations/model/potemplate.py (+7/-3)
lib/lp/translations/model/translationgroup.py (+13/-11)
To merge this branch: bzr merge lp:~lifeless/launchpad/prejoin
Reviewer Review Type Date Requested Status
Jonathan Lange (community) Approve
Review via email: mp+39720@code.launchpad.net

Commit message

Replace lp.services.database.prejoin with DecoratedResultSet

Description of the change

Delete unused code.

To post a comment you must log in.
Revision history for this message
Jonathan Lange (jml) wrote :

I guess this isn't actually used anywhere, so my one qualm – that this is not a duplicate of DecoratedResultSet but instead a near-duplicate with extra logic – is not valid.

review: Approve
Revision history for this message
Robert Collins (lifeless) wrote :

Bah, my grep was not powerful enough. Theres at least 'a' user. Will investigate.

Revision history for this message
Robert Collins (lifeless) wrote :

The 'extra' logic was used in one call site - to return a slice. We can, if we want, add it to DRS eventually - but for now lambda x:x[slice(0,3)] should do the job.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/model/branch.py'
2--- lib/lp/code/model/branch.py 2010-11-18 16:34:35 +0000
3+++ lib/lp/code/model/branch.py 2010-11-19 22:50:20 +0000
4@@ -10,6 +10,7 @@
5 ]
6
7 from datetime import datetime
8+import operator
9 import simplejson
10
11 from bzrlib import urlutils
12@@ -58,6 +59,9 @@
13 sqlvalues,
14 )
15 from canonical.launchpad import _
16+from canonical.launchpad.components.decoratedresultset import (
17+ DecoratedResultSet,
18+ )
19 from canonical.launchpad.interfaces.launchpad import (
20 ILaunchpadCelebrities,
21 IPrivacy,
22@@ -129,7 +133,6 @@
23 validate_person,
24 validate_public_person,
25 )
26-from lp.services.database.prejoin import prejoin
27 from lp.services.job.interfaces.job import JobStatus
28 from lp.services.job.model.job import Job
29 from lp.services.mail.notificationrecipientset import NotificationRecipientSet
30@@ -283,7 +286,7 @@
31 Revision.id == BranchRevision.revision_id,
32 BranchRevision.sequence != None)
33 result = result.order_by(Desc(BranchRevision.sequence))
34- return prejoin(result, return_slice=slice(0, 1))
35+ return DecoratedResultSet(result, operator.itemgetter(0))
36
37 subscriptions = SQLMultipleJoin(
38 'BranchSubscription', joinColumn='branch', orderBy='id')
39@@ -593,7 +596,7 @@
40 Revision.revision_date > timestamp)
41 result = result.order_by(Desc(BranchRevision.sequence))
42 # Return BranchRevision but prejoin Revision as well.
43- return prejoin(result, slice(0, 1))
44+ return DecoratedResultSet(result, operator.itemgetter(0))
45
46 def canBeDeleted(self):
47 """See `IBranch`."""
48
49=== modified file 'lib/lp/registry/model/product.py'
50--- lib/lp/registry/model/product.py 2010-11-08 01:08:15 +0000
51+++ lib/lp/registry/model/product.py 2010-11-19 22:50:20 +0000
52@@ -55,6 +55,9 @@
53 SQLBase,
54 sqlvalues,
55 )
56+from canonical.launchpad.components.decoratedresultset import (
57+ DecoratedResultSet,
58+ )
59 from canonical.launchpad.interfaces.launchpad import (
60 IHasIcon,
61 IHasLogo,
62@@ -157,7 +160,6 @@
63 from lp.registry.model.structuralsubscription import (
64 StructuralSubscriptionTargetMixin,
65 )
66-from lp.services.database.prejoin import prejoin
67 from lp.services.propertycache import (
68 cachedproperty,
69 get_property_cache,
70@@ -1576,7 +1578,7 @@
71
72 # We only want Product - the other tables are just to populate
73 # the cache.
74- return prejoin(results)
75+ return DecoratedResultSet(results, operator.itemgetter(0))
76
77 def featuredTranslatables(self, maximumproducts=8):
78 """See `IProductSet`"""
79
80=== modified file 'lib/lp/services/database/configure.zcml'
81--- lib/lp/services/database/configure.zcml 2010-03-11 15:57:43 +0000
82+++ lib/lp/services/database/configure.zcml 2010-11-19 22:50:20 +0000
83@@ -8,10 +8,6 @@
84 xmlns:i18n="http://namespaces.zope.org/i18n"
85 i18n_domain="launchpad">
86
87- <class class="lp.services.database.prejoin.PrejoinResultSet">
88- <allow interface="storm.zope.interfaces.IResultSet" />
89- <allow attributes="__getslice__" />
90- </class>
91 <class class="storm.references.BoundIndirectReferenceSet">
92 <allow attributes="add remove clear" />
93 </class>
94
95=== removed file 'lib/lp/services/database/prejoin.py'
96--- lib/lp/services/database/prejoin.py 2010-09-07 04:53:35 +0000
97+++ lib/lp/services/database/prejoin.py 1970-01-01 00:00:00 +0000
98@@ -1,118 +0,0 @@
99-# Copyright 2009 Canonical Ltd. This software is licensed under the
100-# GNU Affero General Public License version 3 (see the file LICENSE).
101-
102-"""Prejoin for Storm.
103-
104-XXX bug=632150 This is duplicated with DecoratedResultSet. please use that.
105-RBC 20100907.
106-"""
107-
108-__metaclass__ = type
109-__all__ = ['prejoin']
110-
111-from lazr.delegates import delegates
112-from storm.zope.interfaces import IResultSet
113-
114-
115-class PrejoinResultSet:
116- """Prejoin support.
117-
118- Wrap a ResultSet, trimming unwanted items. The unwanted items
119- are still pulled from the database and populate the Storm caches.
120-
121- This is a temporary solution, as as count() performs an unnecessary
122- join making it slower and __contains__() is not implemented at all.
123- The preferred solution is support in Storm core, so we can just do
124- something like:
125-
126- # Select Product, but prejoin the owner as well.
127- >>> join = store.find((Product, Person), Product.name == name)
128- >>> results = prejoin(join, slice(0, 1))
129- """
130- delegates(IResultSet, context='result_set')
131-
132- def __init__(self, result_set, return_slice=slice(0, 1)):
133- self.result_set = result_set
134- self.return_slice = return_slice
135-
136- def _chain(self, result_set):
137- return PrejoinResultSet(result_set, self.return_slice)
138-
139- def _chomp(self, row):
140- if row is None:
141- return None
142- elems = row[self.return_slice]
143- if len(elems) == 1:
144- return elems[0]
145- else:
146- return elems
147-
148- def copy(self):
149- """See `IResultSet`."""
150- return self._chain(self.result_set.copy())
151-
152- def config(self, distinct=None, offset=None, limit=None):
153- """See `IResultSet`."""
154- self.result_set.config(distinct, offset, limit)
155- return self
156-
157- def order_by(self, *args):
158- """See `IResultSet`."""
159- return self._chain(self.result_set.order_by(*args))
160-
161- def difference(self, *args, **kw):
162- """See `IResultSet`."""
163- raise NotImplementedError("difference")
164-
165- def group_by(self, *args, **kw):
166- """See `IResultSet`."""
167- raise NotImplementedError("group_by")
168-
169- def having(self, *args, **kw):
170- """See `IResultSet`."""
171- raise NotImplementedError("having")
172-
173- def intersection(self, *args, **kw):
174- """See `IResultSet`."""
175- raise NotImplementedError("intersection")
176-
177- def union(self, *args, **kw):
178- """See `IResultSet`."""
179- raise NotImplementedError("union")
180-
181- def __iter__(self):
182- """See `IResultSet`."""
183- return (self._chomp(row) for row in self.result_set)
184-
185- def __getitem__(self, index):
186- """See `IResultSet`."""
187- if isinstance(index, slice):
188- return self._chain(self.result_set[index])
189- else:
190- return self._chomp(self.result_set[index])
191-
192- def __contains__(self, item):
193- """See `IResultSet`."""
194- raise NotImplementedError("__contains__")
195-
196- def any(self):
197- """See `IResultSet`."""
198- return self._chomp(self.result_set.any())
199-
200- def first(self):
201- """See `IResultSet`."""
202- return self._chomp(self.result_set.first())
203-
204- def last(self):
205- """See `IResultSet`."""
206- return self._chomp(self.result_set.last())
207-
208- def one(self):
209- """See `IResultSet`."""
210- return self._chomp(self.result_set.one())
211-
212- def cached(self):
213- """See `IResultSet`."""
214- raise NotImplementedError("cached")
215-
216-prejoin = PrejoinResultSet
217
218=== removed file 'lib/lp/services/database/tests/test_prejoin.py'
219--- lib/lp/services/database/tests/test_prejoin.py 2010-10-20 20:51:26 +0000
220+++ lib/lp/services/database/tests/test_prejoin.py 1970-01-01 00:00:00 +0000
221@@ -1,118 +0,0 @@
222-# Copyright 2009 Canonical Ltd. This software is licensed under the
223-# GNU Affero General Public License version 3 (see the file LICENSE).
224-
225-"""prejoin module tests."""
226-
227-__metaclass__ = type
228-__all__ = []
229-
230-import unittest
231-
232-from canonical.launchpad.interfaces.lpstorm import IMasterStore
233-from canonical.testing.layers import LaunchpadZopelessLayer
234-from lp.registry.model.person import Person
235-from lp.registry.model.product import Product
236-from lp.services.database.prejoin import (
237- prejoin,
238- PrejoinResultSet,
239- )
240-from lp.testing import TestCase
241-
242-
243-class PrejoinTestCase(TestCase):
244- layer = LaunchpadZopelessLayer
245-
246- def setUp(self):
247- super(PrejoinTestCase, self).setUp()
248- self.store = IMasterStore(Product)
249-
250- # All products
251- self.standard_result = self.store.find(Product).order_by(Product.name)
252-
253- # All products, with owner and preferred email address prejoined.
254- # Note that because some Product owners have multiple email
255- # addresses, this query returns more results. prejoin needs
256- # to hide this from callsites.
257- self.unwrapped_result = self.store.find(
258- (Product, Person),
259- Product._ownerID == Person.id).order_by(Product.name)
260- self.prejoin_result = prejoin(self.unwrapped_result)
261-
262- def verify(self, prejoined, normal):
263- # Ensure our prejoined result really is a PrejoinResultSet.
264- # One of our methods might not be sticky, and we have ended up
265- # with a normal Storm ResultSet.
266- self.assertTrue(
267- isinstance(prejoined, PrejoinResultSet),
268- "Expected a PrejoinResultSet, got %s" % repr(prejoined))
269-
270- # Confirm the two result sets return identical results.
271- self.assertEqual(list(normal), list(prejoined))
272-
273- def test_count(self):
274- self.assertEqual(
275- self.standard_result.count(),
276- self.prejoin_result.count())
277-
278- def test_copy(self):
279- copy = self.prejoin_result.copy()
280- self.verify(copy, self.standard_result)
281-
282- def test_config(self):
283- self.prejoin_result.config(offset=3, limit=2)
284- self.standard_result.config(offset=3, limit=2)
285- self.verify(self.prejoin_result, self.standard_result)
286-
287- def test_config_returnvalue(self):
288- prejoin_rv = self.prejoin_result.config(offset=3, limit=2)
289- standard_rv = self.standard_result.config(offset=3, limit=2)
290- self.verify(prejoin_rv, standard_rv)
291-
292- def test_order_by(self):
293- self.verify(
294- self.prejoin_result.order_by(Product.id),
295- self.standard_result.order_by(Product.id))
296-
297- def test_slice(self):
298- self.verify(
299- self.prejoin_result[4:6],
300- self.standard_result[4:6])
301-
302- def test_any(self):
303- self.assertIn(self.prejoin_result.any(), self.standard_result)
304-
305- def test_first(self):
306- self.assertEqual(
307- self.standard_result.first(), self.prejoin_result.first())
308-
309- def test_one(self):
310- standard_result = self.store.find(Product, Product.name == 'firefox')
311- prejoin_result = prejoin(self.store.find(
312- (Product, Person),
313- Person.id == Product._ownerID,
314- Product.name == 'firefox'))
315- self.assertEqual(standard_result.one(), prejoin_result.one())
316-
317- def test_one_empty(self):
318- # For an empty result (None), one returns None, too.
319- name = "none-existent-name"
320- prejoin_result = prejoin(self.store.find(
321- (Product, Person),
322- Person.id == Product._ownerID,
323- Product.name == name))
324- self.assertIs(None, prejoin_result.one())
325-
326- def test_cache_populated(self):
327- # Load a row.
328- product = self.prejoin_result.first()
329-
330- # Destroy our data, without telling Storm. This way we can
331- # tell if we are accessing an object from the cache, or if it
332- # was retrieved from the database.
333- self.store.execute("UPDATE Person SET displayname='foo'")
334-
335- self.failIfEqual('foo', product.owner.displayname)
336-
337-
338-def test_suite():
339- return unittest.TestLoader().loadTestsFromName(__name__)
340
341=== modified file 'lib/lp/translations/model/potemplate.py'
342--- lib/lp/translations/model/potemplate.py 2010-11-11 06:16:33 +0000
343+++ lib/lp/translations/model/potemplate.py 2010-11-19 22:50:20 +0000
344@@ -17,6 +17,7 @@
345
346 import datetime
347 import logging
348+import operator
349 import os
350 import re
351
352@@ -56,6 +57,9 @@
353 sqlvalues,
354 )
355 from canonical.launchpad import helpers
356+from canonical.launchpad.components.decoratedresultset import (
357+ DecoratedResultSet,
358+ )
359 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
360 from canonical.launchpad.interfaces.lpstorm import (
361 IMasterStore,
362@@ -65,7 +69,6 @@
363 from lp.registry.interfaces.person import validate_public_person
364 from lp.registry.model.sourcepackagename import SourcePackageName
365 from lp.services.database.collection import Collection
366-from lp.services.database.prejoin import prejoin
367 from lp.services.propertycache import cachedproperty
368 from lp.services.worlddata.model.language import Language
369 from lp.translations.enums import RosettaImportStatus
370@@ -1066,10 +1069,11 @@
371 else:
372 store = Store.of(self.distroseries)
373 if do_prejoin:
374- query = prejoin(store.find(
375+ query = DecoratedResultSet(store.find(
376 (POTemplate, SourcePackageName),
377 (POTemplate.sourcepackagenameID ==
378- SourcePackageName.id), condition))
379+ SourcePackageName.id), condition),
380+ operator.itemgetter(0))
381 else:
382 query = store.find(POTemplate, condition)
383
384
385=== modified file 'lib/lp/translations/model/translationgroup.py'
386--- lib/lp/translations/model/translationgroup.py 2010-09-03 10:59:24 +0000
387+++ lib/lp/translations/model/translationgroup.py 2010-11-19 22:50:20 +0000
388@@ -9,6 +9,8 @@
389 'TranslationGroupSet',
390 ]
391
392+import operator
393+
394 from sqlobject import (
395 ForeignKey,
396 SQLMultipleJoin,
397@@ -26,6 +28,9 @@
398 from canonical.database.constants import DEFAULT
399 from canonical.database.datetimecol import UtcDateTimeCol
400 from canonical.database.sqlbase import SQLBase
401+from canonical.launchpad.components.decoratedresultset import (
402+ DecoratedResultSet,
403+ )
404 from canonical.launchpad.database.librarian import (
405 LibraryFileAlias,
406 LibraryFileContent,
407@@ -41,7 +46,6 @@
408 )
409 from lp.registry.model.projectgroup import ProjectGroup
410 from lp.registry.model.teammembership import TeamParticipation
411-from lp.services.database.prejoin import prejoin
412 from lp.services.worlddata.model.language import Language
413 from lp.translations.interfaces.translationgroup import (
414 ITranslationGroup,
415@@ -171,10 +175,9 @@
416 Translator.translationgroup == self,
417 Language.id == Translator.languageID,
418 Person.id == Translator.translatorID)
419-
420- return prejoin(
421- translator_data.order_by(Language.englishname),
422- slice(0, 3))
423+ translator_data = translator_data.order_by(Language.englishname)
424+ mapper = lambda row:row[slice(0,3)]
425+ return DecoratedResultSet(translator_data, mapper)
426
427 def fetchProjectsForDisplay(self):
428 """See `ITranslationGroup`."""
429@@ -218,10 +221,9 @@
430 project_data = ISlaveStore(ProjectGroup).using(*using).find(
431 tables,
432 ProjectGroup.translationgroupID == self.id,
433- ProjectGroup.active == True)
434+ ProjectGroup.active == True).order_by(ProjectGroup.displayname)
435
436- return prejoin(
437- project_data.order_by(ProjectGroup.displayname), slice(0, 1))
438+ return DecoratedResultSet(project_data, operator.itemgetter(0))
439
440 def fetchDistrosForDisplay(self):
441 """See `ITranslationGroup`."""
442@@ -239,10 +241,10 @@
443 LibraryFileContent,
444 )
445 distro_data = ISlaveStore(Distribution).using(*using).find(
446- tables, Distribution.translationgroupID == self.id)
447+ tables, Distribution.translationgroupID == self.id).order_by(
448+ Distribution.displayname)
449
450- return prejoin(
451- distro_data.order_by(Distribution.displayname), slice(0, 1))
452+ return DecoratedResultSet(distro_data, operator.itemgetter(0))
453
454
455 class TranslationGroupSet: