Launchpad itself

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

  1. prejoin
  2. Merge into devel
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
=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py 2010-11-18 16:34:35 +0000
+++ lib/lp/code/model/branch.py 2010-11-19 22:50:20 +0000
@@ -10,6 +10,7 @@
10 ]10 ]
1111
12from datetime import datetime12from datetime import datetime
13import operator
13import simplejson14import simplejson
1415
15from bzrlib import urlutils16from bzrlib import urlutils
@@ -58,6 +59,9 @@
58 sqlvalues,59 sqlvalues,
59 )60 )
60from canonical.launchpad import _61from canonical.launchpad import _
62from canonical.launchpad.components.decoratedresultset import (
63 DecoratedResultSet,
64 )
61from canonical.launchpad.interfaces.launchpad import (65from canonical.launchpad.interfaces.launchpad import (
62 ILaunchpadCelebrities,66 ILaunchpadCelebrities,
63 IPrivacy,67 IPrivacy,
@@ -129,7 +133,6 @@
129 validate_person,133 validate_person,
130 validate_public_person,134 validate_public_person,
131 )135 )
132from lp.services.database.prejoin import prejoin
133from lp.services.job.interfaces.job import JobStatus136from lp.services.job.interfaces.job import JobStatus
134from lp.services.job.model.job import Job137from lp.services.job.model.job import Job
135from lp.services.mail.notificationrecipientset import NotificationRecipientSet138from lp.services.mail.notificationrecipientset import NotificationRecipientSet
@@ -283,7 +286,7 @@
283 Revision.id == BranchRevision.revision_id,286 Revision.id == BranchRevision.revision_id,
284 BranchRevision.sequence != None)287 BranchRevision.sequence != None)
285 result = result.order_by(Desc(BranchRevision.sequence))288 result = result.order_by(Desc(BranchRevision.sequence))
286 return prejoin(result, return_slice=slice(0, 1))289 return DecoratedResultSet(result, operator.itemgetter(0))
287290
288 subscriptions = SQLMultipleJoin(291 subscriptions = SQLMultipleJoin(
289 'BranchSubscription', joinColumn='branch', orderBy='id')292 'BranchSubscription', joinColumn='branch', orderBy='id')
@@ -593,7 +596,7 @@
593 Revision.revision_date > timestamp)596 Revision.revision_date > timestamp)
594 result = result.order_by(Desc(BranchRevision.sequence))597 result = result.order_by(Desc(BranchRevision.sequence))
595 # Return BranchRevision but prejoin Revision as well.598 # Return BranchRevision but prejoin Revision as well.
596 return prejoin(result, slice(0, 1))599 return DecoratedResultSet(result, operator.itemgetter(0))
597600
598 def canBeDeleted(self):601 def canBeDeleted(self):
599 """See `IBranch`."""602 """See `IBranch`."""
600603
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2010-11-08 01:08:15 +0000
+++ lib/lp/registry/model/product.py 2010-11-19 22:50:20 +0000
@@ -55,6 +55,9 @@
55 SQLBase,55 SQLBase,
56 sqlvalues,56 sqlvalues,
57 )57 )
58from canonical.launchpad.components.decoratedresultset import (
59 DecoratedResultSet,
60 )
58from canonical.launchpad.interfaces.launchpad import (61from canonical.launchpad.interfaces.launchpad import (
59 IHasIcon,62 IHasIcon,
60 IHasLogo,63 IHasLogo,
@@ -157,7 +160,6 @@
157from lp.registry.model.structuralsubscription import (160from lp.registry.model.structuralsubscription import (
158 StructuralSubscriptionTargetMixin,161 StructuralSubscriptionTargetMixin,
159 )162 )
160from lp.services.database.prejoin import prejoin
161from lp.services.propertycache import (163from lp.services.propertycache import (
162 cachedproperty,164 cachedproperty,
163 get_property_cache,165 get_property_cache,
@@ -1576,7 +1578,7 @@
15761578
1577 # We only want Product - the other tables are just to populate1579 # We only want Product - the other tables are just to populate
1578 # the cache.1580 # the cache.
1579 return prejoin(results)1581 return DecoratedResultSet(results, operator.itemgetter(0))
15801582
1581 def featuredTranslatables(self, maximumproducts=8):1583 def featuredTranslatables(self, maximumproducts=8):
1582 """See `IProductSet`"""1584 """See `IProductSet`"""
15831585
=== modified file 'lib/lp/services/database/configure.zcml'
--- lib/lp/services/database/configure.zcml 2010-03-11 15:57:43 +0000
+++ lib/lp/services/database/configure.zcml 2010-11-19 22:50:20 +0000
@@ -8,10 +8,6 @@
8 xmlns:i18n="http://namespaces.zope.org/i18n"8 xmlns:i18n="http://namespaces.zope.org/i18n"
9 i18n_domain="launchpad">9 i18n_domain="launchpad">
1010
11 <class class="lp.services.database.prejoin.PrejoinResultSet">
12 <allow interface="storm.zope.interfaces.IResultSet" />
13 <allow attributes="__getslice__" />
14 </class>
15 <class class="storm.references.BoundIndirectReferenceSet">11 <class class="storm.references.BoundIndirectReferenceSet">
16 <allow attributes="add remove clear" />12 <allow attributes="add remove clear" />
17 </class>13 </class>
1814
=== removed file 'lib/lp/services/database/prejoin.py'
--- lib/lp/services/database/prejoin.py 2010-09-07 04:53:35 +0000
+++ lib/lp/services/database/prejoin.py 1970-01-01 00:00:00 +0000
@@ -1,118 +0,0 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Prejoin for Storm.
5
6XXX bug=632150 This is duplicated with DecoratedResultSet. please use that.
7RBC 20100907.
8"""
9
10__metaclass__ = type
11__all__ = ['prejoin']
12
13from lazr.delegates import delegates
14from storm.zope.interfaces import IResultSet
15
16
17class PrejoinResultSet:
18 """Prejoin support.
19
20 Wrap a ResultSet, trimming unwanted items. The unwanted items
21 are still pulled from the database and populate the Storm caches.
22
23 This is a temporary solution, as as count() performs an unnecessary
24 join making it slower and __contains__() is not implemented at all.
25 The preferred solution is support in Storm core, so we can just do
26 something like:
27
28 # Select Product, but prejoin the owner as well.
29 >>> join = store.find((Product, Person), Product.name == name)
30 >>> results = prejoin(join, slice(0, 1))
31 """
32 delegates(IResultSet, context='result_set')
33
34 def __init__(self, result_set, return_slice=slice(0, 1)):
35 self.result_set = result_set
36 self.return_slice = return_slice
37
38 def _chain(self, result_set):
39 return PrejoinResultSet(result_set, self.return_slice)
40
41 def _chomp(self, row):
42 if row is None:
43 return None
44 elems = row[self.return_slice]
45 if len(elems) == 1:
46 return elems[0]
47 else:
48 return elems
49
50 def copy(self):
51 """See `IResultSet`."""
52 return self._chain(self.result_set.copy())
53
54 def config(self, distinct=None, offset=None, limit=None):
55 """See `IResultSet`."""
56 self.result_set.config(distinct, offset, limit)
57 return self
58
59 def order_by(self, *args):
60 """See `IResultSet`."""
61 return self._chain(self.result_set.order_by(*args))
62
63 def difference(self, *args, **kw):
64 """See `IResultSet`."""
65 raise NotImplementedError("difference")
66
67 def group_by(self, *args, **kw):
68 """See `IResultSet`."""
69 raise NotImplementedError("group_by")
70
71 def having(self, *args, **kw):
72 """See `IResultSet`."""
73 raise NotImplementedError("having")
74
75 def intersection(self, *args, **kw):
76 """See `IResultSet`."""
77 raise NotImplementedError("intersection")
78
79 def union(self, *args, **kw):
80 """See `IResultSet`."""
81 raise NotImplementedError("union")
82
83 def __iter__(self):
84 """See `IResultSet`."""
85 return (self._chomp(row) for row in self.result_set)
86
87 def __getitem__(self, index):
88 """See `IResultSet`."""
89 if isinstance(index, slice):
90 return self._chain(self.result_set[index])
91 else:
92 return self._chomp(self.result_set[index])
93
94 def __contains__(self, item):
95 """See `IResultSet`."""
96 raise NotImplementedError("__contains__")
97
98 def any(self):
99 """See `IResultSet`."""
100 return self._chomp(self.result_set.any())
101
102 def first(self):
103 """See `IResultSet`."""
104 return self._chomp(self.result_set.first())
105
106 def last(self):
107 """See `IResultSet`."""
108 return self._chomp(self.result_set.last())
109
110 def one(self):
111 """See `IResultSet`."""
112 return self._chomp(self.result_set.one())
113
114 def cached(self):
115 """See `IResultSet`."""
116 raise NotImplementedError("cached")
117
118prejoin = PrejoinResultSet
1190
=== removed file 'lib/lp/services/database/tests/test_prejoin.py'
--- lib/lp/services/database/tests/test_prejoin.py 2010-10-20 20:51:26 +0000
+++ lib/lp/services/database/tests/test_prejoin.py 1970-01-01 00:00:00 +0000
@@ -1,118 +0,0 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""prejoin module tests."""
5
6__metaclass__ = type
7__all__ = []
8
9import unittest
10
11from canonical.launchpad.interfaces.lpstorm import IMasterStore
12from canonical.testing.layers import LaunchpadZopelessLayer
13from lp.registry.model.person import Person
14from lp.registry.model.product import Product
15from lp.services.database.prejoin import (
16 prejoin,
17 PrejoinResultSet,
18 )
19from lp.testing import TestCase
20
21
22class PrejoinTestCase(TestCase):
23 layer = LaunchpadZopelessLayer
24
25 def setUp(self):
26 super(PrejoinTestCase, self).setUp()
27 self.store = IMasterStore(Product)
28
29 # All products
30 self.standard_result = self.store.find(Product).order_by(Product.name)
31
32 # All products, with owner and preferred email address prejoined.
33 # Note that because some Product owners have multiple email
34 # addresses, this query returns more results. prejoin needs
35 # to hide this from callsites.
36 self.unwrapped_result = self.store.find(
37 (Product, Person),
38 Product._ownerID == Person.id).order_by(Product.name)
39 self.prejoin_result = prejoin(self.unwrapped_result)
40
41 def verify(self, prejoined, normal):
42 # Ensure our prejoined result really is a PrejoinResultSet.
43 # One of our methods might not be sticky, and we have ended up
44 # with a normal Storm ResultSet.
45 self.assertTrue(
46 isinstance(prejoined, PrejoinResultSet),
47 "Expected a PrejoinResultSet, got %s" % repr(prejoined))
48
49 # Confirm the two result sets return identical results.
50 self.assertEqual(list(normal), list(prejoined))
51
52 def test_count(self):
53 self.assertEqual(
54 self.standard_result.count(),
55 self.prejoin_result.count())
56
57 def test_copy(self):
58 copy = self.prejoin_result.copy()
59 self.verify(copy, self.standard_result)
60
61 def test_config(self):
62 self.prejoin_result.config(offset=3, limit=2)
63 self.standard_result.config(offset=3, limit=2)
64 self.verify(self.prejoin_result, self.standard_result)
65
66 def test_config_returnvalue(self):
67 prejoin_rv = self.prejoin_result.config(offset=3, limit=2)
68 standard_rv = self.standard_result.config(offset=3, limit=2)
69 self.verify(prejoin_rv, standard_rv)
70
71 def test_order_by(self):
72 self.verify(
73 self.prejoin_result.order_by(Product.id),
74 self.standard_result.order_by(Product.id))
75
76 def test_slice(self):
77 self.verify(
78 self.prejoin_result[4:6],
79 self.standard_result[4:6])
80
81 def test_any(self):
82 self.assertIn(self.prejoin_result.any(), self.standard_result)
83
84 def test_first(self):
85 self.assertEqual(
86 self.standard_result.first(), self.prejoin_result.first())
87
88 def test_one(self):
89 standard_result = self.store.find(Product, Product.name == 'firefox')
90 prejoin_result = prejoin(self.store.find(
91 (Product, Person),
92 Person.id == Product._ownerID,
93 Product.name == 'firefox'))
94 self.assertEqual(standard_result.one(), prejoin_result.one())
95
96 def test_one_empty(self):
97 # For an empty result (None), one returns None, too.
98 name = "none-existent-name"
99 prejoin_result = prejoin(self.store.find(
100 (Product, Person),
101 Person.id == Product._ownerID,
102 Product.name == name))
103 self.assertIs(None, prejoin_result.one())
104
105 def test_cache_populated(self):
106 # Load a row.
107 product = self.prejoin_result.first()
108
109 # Destroy our data, without telling Storm. This way we can
110 # tell if we are accessing an object from the cache, or if it
111 # was retrieved from the database.
112 self.store.execute("UPDATE Person SET displayname='foo'")
113
114 self.failIfEqual('foo', product.owner.displayname)
115
116
117def test_suite():
118 return unittest.TestLoader().loadTestsFromName(__name__)
1190
=== modified file 'lib/lp/translations/model/potemplate.py'
--- lib/lp/translations/model/potemplate.py 2010-11-11 06:16:33 +0000
+++ lib/lp/translations/model/potemplate.py 2010-11-19 22:50:20 +0000
@@ -17,6 +17,7 @@
1717
18import datetime18import datetime
19import logging19import logging
20import operator
20import os21import os
21import re22import re
2223
@@ -56,6 +57,9 @@
56 sqlvalues,57 sqlvalues,
57 )58 )
58from canonical.launchpad import helpers59from canonical.launchpad import helpers
60from canonical.launchpad.components.decoratedresultset import (
61 DecoratedResultSet,
62 )
59from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities63from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
60from canonical.launchpad.interfaces.lpstorm import (64from canonical.launchpad.interfaces.lpstorm import (
61 IMasterStore,65 IMasterStore,
@@ -65,7 +69,6 @@
65from lp.registry.interfaces.person import validate_public_person69from lp.registry.interfaces.person import validate_public_person
66from lp.registry.model.sourcepackagename import SourcePackageName70from lp.registry.model.sourcepackagename import SourcePackageName
67from lp.services.database.collection import Collection71from lp.services.database.collection import Collection
68from lp.services.database.prejoin import prejoin
69from lp.services.propertycache import cachedproperty72from lp.services.propertycache import cachedproperty
70from lp.services.worlddata.model.language import Language73from lp.services.worlddata.model.language import Language
71from lp.translations.enums import RosettaImportStatus74from lp.translations.enums import RosettaImportStatus
@@ -1066,10 +1069,11 @@
1066 else:1069 else:
1067 store = Store.of(self.distroseries)1070 store = Store.of(self.distroseries)
1068 if do_prejoin:1071 if do_prejoin:
1069 query = prejoin(store.find(1072 query = DecoratedResultSet(store.find(
1070 (POTemplate, SourcePackageName),1073 (POTemplate, SourcePackageName),
1071 (POTemplate.sourcepackagenameID ==1074 (POTemplate.sourcepackagenameID ==
1072 SourcePackageName.id), condition))1075 SourcePackageName.id), condition),
1076 operator.itemgetter(0))
1073 else:1077 else:
1074 query = store.find(POTemplate, condition)1078 query = store.find(POTemplate, condition)
10751079
10761080
=== modified file 'lib/lp/translations/model/translationgroup.py'
--- lib/lp/translations/model/translationgroup.py 2010-09-03 10:59:24 +0000
+++ lib/lp/translations/model/translationgroup.py 2010-11-19 22:50:20 +0000
@@ -9,6 +9,8 @@
9 'TranslationGroupSet',9 'TranslationGroupSet',
10 ]10 ]
1111
12import operator
13
12from sqlobject import (14from sqlobject import (
13 ForeignKey,15 ForeignKey,
14 SQLMultipleJoin,16 SQLMultipleJoin,
@@ -26,6 +28,9 @@
26from canonical.database.constants import DEFAULT28from canonical.database.constants import DEFAULT
27from canonical.database.datetimecol import UtcDateTimeCol29from canonical.database.datetimecol import UtcDateTimeCol
28from canonical.database.sqlbase import SQLBase30from canonical.database.sqlbase import SQLBase
31from canonical.launchpad.components.decoratedresultset import (
32 DecoratedResultSet,
33 )
29from canonical.launchpad.database.librarian import (34from canonical.launchpad.database.librarian import (
30 LibraryFileAlias,35 LibraryFileAlias,
31 LibraryFileContent,36 LibraryFileContent,
@@ -41,7 +46,6 @@
41 )46 )
42from lp.registry.model.projectgroup import ProjectGroup47from lp.registry.model.projectgroup import ProjectGroup
43from lp.registry.model.teammembership import TeamParticipation48from lp.registry.model.teammembership import TeamParticipation
44from lp.services.database.prejoin import prejoin
45from lp.services.worlddata.model.language import Language49from lp.services.worlddata.model.language import Language
46from lp.translations.interfaces.translationgroup import (50from lp.translations.interfaces.translationgroup import (
47 ITranslationGroup,51 ITranslationGroup,
@@ -171,10 +175,9 @@
171 Translator.translationgroup == self,175 Translator.translationgroup == self,
172 Language.id == Translator.languageID,176 Language.id == Translator.languageID,
173 Person.id == Translator.translatorID)177 Person.id == Translator.translatorID)
174178 translator_data = translator_data.order_by(Language.englishname)
175 return prejoin(179 mapper = lambda row:row[slice(0,3)]
176 translator_data.order_by(Language.englishname),180 return DecoratedResultSet(translator_data, mapper)
177 slice(0, 3))
178181
179 def fetchProjectsForDisplay(self):182 def fetchProjectsForDisplay(self):
180 """See `ITranslationGroup`."""183 """See `ITranslationGroup`."""
@@ -218,10 +221,9 @@
218 project_data = ISlaveStore(ProjectGroup).using(*using).find(221 project_data = ISlaveStore(ProjectGroup).using(*using).find(
219 tables,222 tables,
220 ProjectGroup.translationgroupID == self.id,223 ProjectGroup.translationgroupID == self.id,
221 ProjectGroup.active == True)224 ProjectGroup.active == True).order_by(ProjectGroup.displayname)
222225
223 return prejoin(226 return DecoratedResultSet(project_data, operator.itemgetter(0))
224 project_data.order_by(ProjectGroup.displayname), slice(0, 1))
225227
226 def fetchDistrosForDisplay(self):228 def fetchDistrosForDisplay(self):
227 """See `ITranslationGroup`."""229 """See `ITranslationGroup`."""
@@ -239,10 +241,10 @@
239 LibraryFileContent,241 LibraryFileContent,
240 )242 )
241 distro_data = ISlaveStore(Distribution).using(*using).find(243 distro_data = ISlaveStore(Distribution).using(*using).find(
242 tables, Distribution.translationgroupID == self.id)244 tables, Distribution.translationgroupID == self.id).order_by(
245 Distribution.displayname)
243246
244 return prejoin(247 return DecoratedResultSet(distro_data, operator.itemgetter(0))
245 distro_data.order_by(Distribution.displayname), slice(0, 1))
246248
247249
248class TranslationGroupSet:250class TranslationGroupSet: