Merge lp:~lifeless/launchpad/bug-279513 into lp:launchpad
- bug-279513
- Merge into devel
Status: | Work in progress |
---|---|
Proposed branch: | lp:~lifeless/launchpad/bug-279513 |
Merge into: | lp:launchpad |
Diff against target: |
427 lines (+63/-173) 11 files modified
lib/lp/bugs/browser/bugtask.py (+28/-15) lib/lp/bugs/browser/tests/bugtask-target-link-titles.txt (+4/-2) lib/lp/bugs/stories/bugs/xx-bug-index.txt (+15/-3) lib/lp/registry/browser/product.py (+2/-2) lib/lp/registry/interfaces/distribution.py (+0/-19) lib/lp/registry/model/distribution.py (+0/-58) lib/lp/registry/model/distributionsourcepackage.py (+4/-2) lib/lp/registry/tests/test_distribution.py (+0/-66) lib/lp/registry/tests/test_distroseries.py (+5/-5) lib/lp/soyuz/doc/closing-bugs-from-changelogs.txt (+4/-1) lib/lp/soyuz/stories/soyuz/xx-distributionsourcepackagerelease-pages.txt (+1/-0) |
To merge this branch: | bzr merge lp:~lifeless/launchpad/bug-279513 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
j.c.sackett (community) | Approve | ||
William Grant | code* | Approve | |
Review via email: mp+51063@code.launchpad.net |
Commit message
[r=jcsackett,
Description of the change
Distribution.
We could alternatively make Distribution.
j.c.sackett (jcsackett) wrote : | # |
This looks good to land.
Robert Collins (lifeless) wrote : | # |
I'm abandoning this - its a bit of a rabbit warren, and the perf wins are relatively small compared to the initial work. We are really broken right now, but this is something to do when things are fast, not right now.
Preview Diff
1 | === modified file 'lib/lp/bugs/browser/bugtask.py' |
2 | --- lib/lp/bugs/browser/bugtask.py 2011-03-01 05:05:26 +0000 |
3 | +++ lib/lp/bugs/browser/bugtask.py 2011-03-02 05:19:17 +0000 |
4 | @@ -250,7 +250,6 @@ |
5 | from lp.bugs.interfaces.malone import IMaloneApplication |
6 | from lp.registry.interfaces.distribution import ( |
7 | IDistribution, |
8 | - IDistributionSet, |
9 | ) |
10 | from lp.registry.interfaces.distributionsourcepackage import ( |
11 | IDistributionSourcePackage, |
12 | @@ -267,6 +266,7 @@ |
13 | from lp.registry.interfaces.productseries import IProductSeries |
14 | from lp.registry.interfaces.projectgroup import IProjectGroup |
15 | from lp.registry.interfaces.sourcepackage import ISourcePackage |
16 | +from lp.registry.model.sourcepackage import SourcePackage |
17 | from lp.registry.vocabularies import MilestoneVocabulary |
18 | from lp.services.fields import PersonChoice |
19 | from lp.services.propertycache import cachedproperty |
20 | @@ -3152,32 +3152,45 @@ |
21 | self.many_bugtasks = len(self.bugtasks) >= 10 |
22 | self.cached_milestone_source = CachedMilestoneSourceFactory() |
23 | self.user_is_subscribed = self.context.isSubscribed(self.user) |
24 | - distro_packages = defaultdict(list) |
25 | distro_series_packages = defaultdict(list) |
26 | for bugtask in self.bugtasks: |
27 | target = bugtask.target |
28 | if IDistributionSourcePackage.providedBy(target): |
29 | - distro_packages[target.distribution].append( |
30 | - target.sourcepackagename) |
31 | + distro_series = target.distribution.currentseries |
32 | + if distro_series is not None: |
33 | + distro_series_packages[distro_series].append( |
34 | + target.sourcepackagename) |
35 | if ISourcePackage.providedBy(target): |
36 | distro_series_packages[target.distroseries].append( |
37 | target.sourcepackagename) |
38 | - distro_set = getUtility(IDistributionSet) |
39 | - self.target_releases = dict(distro_set.getCurrentSourceReleases( |
40 | - distro_packages)) |
41 | distro_series_set = getUtility(IDistroSeriesSet) |
42 | - self.target_releases.update( |
43 | - distro_series_set.getCurrentSourceReleases(distro_series_packages)) |
44 | + self.target_releases = distro_series_set.getCurrentSourceReleases( |
45 | + distro_series_packages) |
46 | + |
47 | + def _get_target_release(self, target): |
48 | + """Get a target release for target.""" |
49 | + error = None |
50 | + if not (IDistributionSourcePackage.providedBy(target) or |
51 | + ISourcePackage.providedBy(target)): |
52 | + return None, None |
53 | + distribution = target.distribution |
54 | + if IDistributionSourcePackage.providedBy(target): |
55 | + target = SourcePackage(target.sourcepackagename, |
56 | + distribution.currentseries) |
57 | + if target.distroseries is not None: |
58 | + current_release = self.target_releases.get(target) |
59 | + else: |
60 | + current_release = None |
61 | + if current_release is None: |
62 | + error = "No current release for this source package in %s" % ( |
63 | + distribution.displayname) |
64 | + return current_release, error |
65 | |
66 | def getTargetLinkTitle(self, target): |
67 | """Return text to put as the title for the link to the target.""" |
68 | - if not (IDistributionSourcePackage.providedBy(target) or |
69 | - ISourcePackage.providedBy(target)): |
70 | - return None |
71 | - current_release = self.target_releases.get(target) |
72 | + current_release, result = self._get_target_release(target) |
73 | if current_release is None: |
74 | - return "No current release for this source package in %s" % ( |
75 | - target.distribution.displayname) |
76 | + return result |
77 | uploader = current_release.creator |
78 | maintainer = current_release.maintainer |
79 | return ( |
80 | |
81 | === modified file 'lib/lp/bugs/browser/tests/bugtask-target-link-titles.txt' |
82 | --- lib/lp/bugs/browser/tests/bugtask-target-link-titles.txt 2010-08-22 18:31:30 +0000 |
83 | +++ lib/lp/bugs/browser/tests/bugtask-target-link-titles.txt 2011-03-02 05:19:17 +0000 |
84 | @@ -89,6 +89,8 @@ |
85 | >>> print view.getTargetLinkTitle(published_distro) |
86 | None |
87 | >>> view = set_up_view(published_evolution) |
88 | + >>> from lp.registry.interfaces.series import SeriesStatus |
89 | + >>> published_distro.currentseries.status = SeriesStatus.FROZEN |
90 | >>> print view.getTargetLinkTitle(published_evolution) |
91 | Latest release: 1.0, uploaded to universe on 2008-07-18 09:30:15+00:00 |
92 | by Foo Bar (name16), maintained by No Privileges Person (no-priv) |
93 | @@ -192,7 +194,7 @@ |
94 | >>> view = BugTasksAndNominationsView(bug, None) |
95 | >>> view.initialize() |
96 | >>> for bug_target in bug_targets: |
97 | - ... release = view.target_releases.get(bug_target) |
98 | + ... release, _ = view._get_target_release(bug_target) |
99 | ... if release is None: |
100 | ... version = 'No releases' |
101 | ... else: |
102 | @@ -201,7 +203,7 @@ |
103 | product: No releases |
104 | product/product-series: No releases |
105 | published-distro: No releases |
106 | - evolution (Published Distro): 2.0 |
107 | + evolution (Published Distro): 1.0 |
108 | Published-distro Published-distro-series: No releases |
109 | evolution (Published-distro Published-distro-series): 1.0 |
110 | unpublished-distro: No releases |
111 | |
112 | === modified file 'lib/lp/bugs/stories/bugs/xx-bug-index.txt' |
113 | --- lib/lp/bugs/stories/bugs/xx-bug-index.txt 2010-05-19 05:47:50 +0000 |
114 | +++ lib/lp/bugs/stories/bugs/xx-bug-index.txt 2011-03-02 05:19:17 +0000 |
115 | @@ -61,11 +61,23 @@ |
116 | 1 |
117 | |
118 | If the context is a distribution package, the package name has a |
119 | -tooltip containing the package details. |
120 | +tooltip containing the package details. The detailed tests for the tooltip |
121 | +contents are in lib/lp/bugs/browser/tests/bugtask-target-link-titles.txt so we |
122 | +just need to check that one is set - that the layers are wired together. |
123 | + |
124 | + >>> from zope.component import getUtility |
125 | + >>> from lp.registry.interfaces.distribution import IDistributionSet |
126 | + >>> ubuntu = getUtility(IDistributionSet).getByName('ubuntu') |
127 | + >>> mozilla = ubuntu.getSourcePackage('mozilla-firefox') |
128 | + >>> mozilla_release = factory.makeSourcePackageRelease( |
129 | + ... sourcepackagename=mozilla.sourcepackagename, |
130 | + ... distroseries=mozilla.distribution.currentseries, |
131 | + ... archive=mozilla.distribution.main_archive, version='0.9') |
132 | + |
133 | |
134 | >>> print anon_browser.getLink('mozilla-firefox (Ubuntu)').attrs['title'] |
135 | - Latest release: 0.9, uploaded to main on 2004-09-27 11:57:13+00:00... |
136 | - by Mark Shuttleworth (mark), maintained by Mark Shuttleworth (mark) |
137 | + Latest release: 0.9, uploaded to main on ... |
138 | + by ..., maintained by ... |
139 | |
140 | >>> print anon_browser.getLink('mozilla-firefox (Debian)').attrs['title'] |
141 | No current release for this source package in Debian |
142 | |
143 | === modified file 'lib/lp/registry/browser/product.py' |
144 | --- lib/lp/registry/browser/product.py 2011-02-05 06:11:48 +0000 |
145 | +++ lib/lp/registry/browser/product.py 2011-03-02 05:19:17 +0000 |
146 | @@ -2038,9 +2038,9 @@ |
147 | # Set the source_package_release attribute on the licenses |
148 | # widget, so that the source package's copyright info can be |
149 | # displayed. |
150 | - ubuntu = getUtility(ILaunchpadCelebrities).ubuntu |
151 | + ubuntu_dev = getUtility(ILaunchpadCelebrities).ubuntu.currentseries |
152 | if self.source_package_name is not None: |
153 | - release_list = ubuntu.getCurrentSourceReleases( |
154 | + release_list = ubuntu_dev.getCurrentSourceReleases( |
155 | [self.source_package_name]) |
156 | if len(release_list) != 0: |
157 | self.widgets['licenses'].source_package_release = ( |
158 | |
159 | === modified file 'lib/lp/registry/interfaces/distribution.py' |
160 | --- lib/lp/registry/interfaces/distribution.py 2011-02-24 15:30:54 +0000 |
161 | +++ lib/lp/registry/interfaces/distribution.py 2011-03-02 05:19:17 +0000 |
162 | @@ -417,16 +417,6 @@ |
163 | Receives a sourcepackagerelease. |
164 | """ |
165 | |
166 | - def getCurrentSourceReleases(source_package_names): |
167 | - """Get the current release of a list of source packages. |
168 | - |
169 | - :param source_package_names: a list of `ISourcePackageName` |
170 | - instances. |
171 | - |
172 | - :return: a dict where the key is a `IDistributionSourcePackage` |
173 | - and the value is a `IDistributionSourcePackageRelease`. |
174 | - """ |
175 | - |
176 | def getDistroSeriesAndPocket(distroseriesname): |
177 | """Return a (distroseries,pocket) tuple which is the given textual |
178 | distroseriesname in this distribution.""" |
179 | @@ -692,15 +682,6 @@ |
180 | members, owner, mugshot=None, logo=None, icon=None): |
181 | """Create a new distribution.""" |
182 | |
183 | - def getCurrentSourceReleases(distro_to_source_packagenames): |
184 | - """Lookup many distribution source package releases. |
185 | - |
186 | - :param distro_to_source_packagenames: A dictionary with |
187 | - its keys being `IDistribution` and its values a list of |
188 | - `ISourcePackageName`. |
189 | - :return: A dict as per `IDistribution.getCurrentSourceReleases` |
190 | - """ |
191 | - |
192 | |
193 | class NoSuchDistribution(NameLookupFailed): |
194 | """Raised when we try to find a distribution that doesn't exist.""" |
195 | |
196 | === modified file 'lib/lp/registry/model/distribution.py' |
197 | --- lib/lp/registry/model/distribution.py 2011-03-01 05:05:26 +0000 |
198 | +++ lib/lp/registry/model/distribution.py 2011-03-02 05:19:17 +0000 |
199 | @@ -775,11 +775,6 @@ |
200 | """See `IDistribution`.""" |
201 | return DistributionSourcePackageRelease(self, sourcepackagerelease) |
202 | |
203 | - def getCurrentSourceReleases(self, source_package_names): |
204 | - """See `IDistribution`.""" |
205 | - return getUtility(IDistributionSet).getCurrentSourceReleases( |
206 | - {self:source_package_names}) |
207 | - |
208 | @property |
209 | def has_any_specifications(self): |
210 | """See `IHasSpecifications`.""" |
211 | @@ -1900,56 +1895,3 @@ |
212 | getUtility(IArchiveSet).new(distribution=distro, |
213 | owner=owner, purpose=ArchivePurpose.PRIMARY) |
214 | return distro |
215 | - |
216 | - def getCurrentSourceReleases(self, distro_source_packagenames): |
217 | - """See `IDistributionSet`.""" |
218 | - # Builds one query for all the distro_source_packagenames. |
219 | - # This may need tuning: its possible that grouping by the common |
220 | - # archives may yield better efficiency: the current code is |
221 | - # just a direct push-down of the previous in-python lookup to SQL. |
222 | - series_clauses = [] |
223 | - distro_lookup = {} |
224 | - for distro, package_names in distro_source_packagenames.items(): |
225 | - source_package_ids = map(attrgetter('id'), package_names) |
226 | - # all_distro_archive_ids is just a list of ints, but it gets |
227 | - # wrapped anyway - and sqlvalues goes boom. |
228 | - archives = removeSecurityProxy( |
229 | - distro.all_distro_archive_ids) |
230 | - clause = """(spr.sourcepackagename IN %s AND |
231 | - spph.archive IN %s AND |
232 | - ds.distribution = %s) |
233 | - """ % sqlvalues(source_package_ids, archives, distro.id) |
234 | - series_clauses.append(clause) |
235 | - distro_lookup[distro.id] = distro |
236 | - if not len(series_clauses): |
237 | - return {} |
238 | - combined_clause = "(" + " OR ".join(series_clauses) + ")" |
239 | - |
240 | - releases = IStore(SourcePackageRelease).find( |
241 | - (SourcePackageRelease, Distribution.id), SQL(""" |
242 | - (SourcePackageRelease.id, Distribution.id) IN ( |
243 | - SELECT DISTINCT ON ( |
244 | - spr.sourcepackagename, ds.distribution) |
245 | - spr.id, ds.distribution |
246 | - FROM |
247 | - SourcePackageRelease AS spr, |
248 | - SourcePackagePublishingHistory AS spph, |
249 | - DistroSeries AS ds |
250 | - WHERE |
251 | - spph.sourcepackagerelease = spr.id |
252 | - AND spph.distroseries = ds.id |
253 | - AND spph.status IN %s |
254 | - AND %s |
255 | - ORDER BY |
256 | - spr.sourcepackagename, ds.distribution, spph.id DESC |
257 | - ) |
258 | - """ |
259 | - % (sqlvalues(active_publishing_status) + (combined_clause,)))) |
260 | - result = {} |
261 | - for sp_release, distro_id in releases: |
262 | - distro = distro_lookup[distro_id] |
263 | - sourcepackage = distro.getSourcePackage( |
264 | - sp_release.sourcepackagename) |
265 | - result[sourcepackage] = DistributionSourcePackageRelease( |
266 | - distro, sp_release) |
267 | - return result |
268 | |
269 | === modified file 'lib/lp/registry/model/distributionsourcepackage.py' |
270 | --- lib/lp/registry/model/distributionsourcepackage.py 2011-01-21 08:12:29 +0000 |
271 | +++ lib/lp/registry/model/distributionsourcepackage.py 2011-03-02 05:19:17 +0000 |
272 | @@ -298,9 +298,11 @@ |
273 | @property |
274 | def currentrelease(self): |
275 | """See `IDistributionSourcePackage`.""" |
276 | - releases = self.distribution.getCurrentSourceReleases( |
277 | + releases = self.distribution.currentseries.getCurrentSourceReleases( |
278 | [self.sourcepackagename]) |
279 | - return releases.get(self) |
280 | + if releases: |
281 | + return releases.values()[0] |
282 | + return None |
283 | |
284 | def bugtasks(self, quantity=None): |
285 | """See `IDistributionSourcePackage`.""" |
286 | |
287 | === modified file 'lib/lp/registry/tests/test_distribution.py' |
288 | --- lib/lp/registry/tests/test_distribution.py 2010-10-24 12:46:23 +0000 |
289 | +++ lib/lp/registry/tests/test_distribution.py 2011-03-02 05:19:17 +0000 |
290 | @@ -6,7 +6,6 @@ |
291 | __metaclass__ = type |
292 | |
293 | from lazr.lifecycle.snapshot import Snapshot |
294 | -from zope.security.proxy import removeSecurityProxy |
295 | |
296 | from canonical.testing.layers import ( |
297 | DatabaseFunctionalLayer, |
298 | @@ -15,13 +14,6 @@ |
299 | from lp.registry.errors import NoSuchDistroSeries |
300 | from lp.registry.interfaces.distribution import IDistribution |
301 | from lp.registry.interfaces.series import SeriesStatus |
302 | -from lp.registry.tests.test_distroseries import ( |
303 | - TestDistroSeriesCurrentSourceReleases, |
304 | - ) |
305 | -from lp.services.propertycache import get_property_cache |
306 | -from lp.soyuz.interfaces.distributionsourcepackagerelease import ( |
307 | - IDistributionSourcePackageRelease, |
308 | - ) |
309 | from lp.testing import TestCaseWithFactory |
310 | |
311 | |
312 | @@ -48,64 +40,6 @@ |
313 | self.assertEqual("'\\u0170-distro'", displayname) |
314 | |
315 | |
316 | -class TestDistributionCurrentSourceReleases( |
317 | - TestDistroSeriesCurrentSourceReleases): |
318 | - """Test for Distribution.getCurrentSourceReleases(). |
319 | - |
320 | - This works in the same way as |
321 | - DistroSeries.getCurrentSourceReleases() works, except that we look |
322 | - for the latest published source across multiple distro series. |
323 | - """ |
324 | - |
325 | - layer = LaunchpadFunctionalLayer |
326 | - release_interface = IDistributionSourcePackageRelease |
327 | - |
328 | - @property |
329 | - def test_target(self): |
330 | - return self.distribution |
331 | - |
332 | - def test_which_distroseries_does_not_matter(self): |
333 | - # When checking for the current release, we only care about the |
334 | - # version numbers. We don't care whether the version is |
335 | - # published in a earlier or later series. |
336 | - self.current_series = self.factory.makeDistroRelease( |
337 | - self.distribution, '1.0', status=SeriesStatus.CURRENT) |
338 | - self.publisher.getPubSource( |
339 | - version='0.9', distroseries=self.current_series) |
340 | - self.publisher.getPubSource( |
341 | - version='1.0', distroseries=self.development_series) |
342 | - self.assertCurrentVersion('1.0') |
343 | - |
344 | - self.publisher.getPubSource( |
345 | - version='1.1', distroseries=self.current_series) |
346 | - self.assertCurrentVersion('1.1') |
347 | - |
348 | - def test_distribution_series_cache(self): |
349 | - distribution = removeSecurityProxy( |
350 | - self.factory.makeDistribution('foo')) |
351 | - |
352 | - cache = get_property_cache(distribution) |
353 | - |
354 | - # Not yet cached. |
355 | - self.assertNotIn("series", cache) |
356 | - |
357 | - # Now cached. |
358 | - series = distribution.series |
359 | - self.assertIs(series, cache.series) |
360 | - |
361 | - # Cache cleared. |
362 | - distribution.newSeries( |
363 | - name='bar', displayname='Bar', title='Bar', summary='', |
364 | - description='', version='1', parent_series=None, |
365 | - owner=self.factory.makePerson()) |
366 | - self.assertNotIn("series", cache) |
367 | - |
368 | - # New cached value. |
369 | - series = distribution.series |
370 | - self.assertEqual(1, len(series)) |
371 | - self.assertIs(series, cache.series) |
372 | - |
373 | - |
374 | class SeriesByStatusTests(TestCaseWithFactory): |
375 | """Test IDistribution.getSeriesByStatus(). |
376 | """ |
377 | |
378 | === modified file 'lib/lp/registry/tests/test_distroseries.py' |
379 | --- lib/lp/registry/tests/test_distroseries.py 2010-10-26 15:47:24 +0000 |
380 | +++ lib/lp/registry/tests/test_distroseries.py 2011-03-02 05:19:17 +0000 |
381 | @@ -152,13 +152,13 @@ |
382 | def test_get_multiple(self): |
383 | # getCurrentSourceReleases() allows you to get information about |
384 | # the current release for multiple packages at the same time. |
385 | - # This is done using a single DB query, making it more efficient |
386 | - # than using IDistributionSource.currentrelease. |
387 | + # This is done using a set queries, making it more efficient than |
388 | + # looking up each package separately. |
389 | self.publisher.getPubSource(version='0.9', sourcename='foo') |
390 | self.publisher.getPubSource(version='1.0', sourcename='bar') |
391 | - foo_package = self.distribution.getSourcePackage('foo') |
392 | - bar_package = self.distribution.getSourcePackage('bar') |
393 | - releases = self.distribution.getCurrentSourceReleases( |
394 | + foo_package = self.publisher.distroseries.getSourcePackage('foo') |
395 | + bar_package = self.publisher.distroseries.getSourcePackage('bar') |
396 | + releases = self.publisher.distroseries.getCurrentSourceReleases( |
397 | [foo_package.sourcepackagename, bar_package.sourcepackagename]) |
398 | self.assertEqual(releases[foo_package].version, '0.9') |
399 | self.assertEqual(releases[bar_package].version, '1.0') |
400 | |
401 | === modified file 'lib/lp/soyuz/doc/closing-bugs-from-changelogs.txt' |
402 | --- lib/lp/soyuz/doc/closing-bugs-from-changelogs.txt 2010-11-01 15:46:48 +0000 |
403 | +++ lib/lp/soyuz/doc/closing-bugs-from-changelogs.txt 2011-03-02 05:19:17 +0000 |
404 | @@ -209,7 +209,10 @@ |
405 | is "NEW". |
406 | |
407 | >>> cdrkit_ubuntu = ubuntu.getSourcePackage('cdrkit') |
408 | - >>> cdrkit_release = cdrkit_ubuntu.currentrelease.sourcepackagerelease |
409 | + >>> cdrkit_release = factory.makeSourcePackageRelease( |
410 | + ... sourcepackagename=cdrkit_ubuntu.sourcepackagename, |
411 | + ... distroseries=cdrkit_ubuntu.distribution['warty'], |
412 | + ... archive=cdrkit_ubuntu.distribution.main_archive, version='1.0') |
413 | |
414 | >>> cdrkit_bug_id = cdrkit_ubuntu.createBug(bug_params).id |
415 | |
416 | |
417 | === modified file 'lib/lp/soyuz/stories/soyuz/xx-distributionsourcepackagerelease-pages.txt' |
418 | --- lib/lp/soyuz/stories/soyuz/xx-distributionsourcepackagerelease-pages.txt 2011-01-12 23:07:40 +0000 |
419 | +++ lib/lp/soyuz/stories/soyuz/xx-distributionsourcepackagerelease-pages.txt 2011-03-02 05:19:17 +0000 |
420 | @@ -12,6 +12,7 @@ |
421 | >>> stp = SoyuzTestPublisher() |
422 | >>> login('foo.bar@canonical.com') |
423 | >>> stp.prepareBreezyAutotest() |
424 | + >>> stp.distroseries = stp.ubuntutest.currentseries |
425 | >>> source = stp.getPubSource('testing-dspr', version='1.0') |
426 | >>> source.setPublished() |
427 | >>> binaries = stp.getPubBinaries(pub_source=source) |
I don't like the DistributionSou rcePackage. currentrelease change, but it's probably cleaner than the alternative. Otherwise this is fine.