launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02756
[Merge] lp:~lifeless/launchpad/bug-279513 into lp:launchpad
Robert Collins has proposed merging lp:~lifeless/launchpad/bug-279513 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#279513 Distribution source package tooltip in bugtask table shows most recent SPPH in any series
https://bugs.launchpad.net/bugs/279513
For more details, see:
https://code.launchpad.net/~lifeless/launchpad/bug-279513/+merge/51063
Distribution.getCurrentSourceReleases returns arbitrary uninteresting versions - when folk speak about 'ubuntu' they mean 'ubuntu.currentseries' (and in fact the currentseries attribute explains this). This branch just deletes the inappropriate Distribution.getCurrentSourceReleases and changes the 3 callers to indirect via the currentseries attribute.
We could alternatively make Distribution.getCurrentSourceReleases batch indirect via the relevant distro series, but I think the overall deletion of code is more of a win. We may have some tests that need updating, but I'll wait for ec2 to complain.
--
https://code.launchpad.net/~lifeless/launchpad/bug-279513/+merge/51063
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/bug-279513 into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2011-02-22 08:42:07 +0000
+++ lib/lp/bugs/browser/bugtask.py 2011-02-24 02:49:56 +0000
@@ -250,7 +250,6 @@
from lp.bugs.interfaces.malone import IMaloneApplication
from lp.registry.interfaces.distribution import (
IDistribution,
- IDistributionSet,
)
from lp.registry.interfaces.distributionsourcepackage import (
IDistributionSourcePackage,
@@ -267,6 +266,7 @@
from lp.registry.interfaces.productseries import IProductSeries
from lp.registry.interfaces.projectgroup import IProjectGroup
from lp.registry.interfaces.sourcepackage import ISourcePackage
+from lp.registry.model.sourcepackage import SourcePackage
from lp.registry.vocabularies import MilestoneVocabulary
from lp.services.fields import PersonChoice
from lp.services.propertycache import cachedproperty
@@ -3150,28 +3150,28 @@
self.many_bugtasks = len(self.bugtasks) >= 10
self.cached_milestone_source = CachedMilestoneSourceFactory()
self.user_is_subscribed = self.context.isSubscribed(self.user)
- distro_packages = defaultdict(list)
distro_series_packages = defaultdict(list)
for bugtask in self.bugtasks:
target = bugtask.target
if IDistributionSourcePackage.providedBy(target):
- distro_packages[target.distribution].append(
+ distro_series = target.distribution.currentseries
+ distro_series_packages[distro_series].append(
target.sourcepackagename)
if ISourcePackage.providedBy(target):
distro_series_packages[target.distroseries].append(
target.sourcepackagename)
- distro_set = getUtility(IDistributionSet)
- self.target_releases = dict(distro_set.getCurrentSourceReleases(
- distro_packages))
distro_series_set = getUtility(IDistroSeriesSet)
- self.target_releases.update(
- distro_series_set.getCurrentSourceReleases(distro_series_packages))
+ self.target_releases = distro_series_set.getCurrentSourceReleases(
+ distro_series_packages)
def getTargetLinkTitle(self, target):
"""Return text to put as the title for the link to the target."""
if not (IDistributionSourcePackage.providedBy(target) or
ISourcePackage.providedBy(target)):
return None
+ if IDistributionSourcePackage.providedBy(target):
+ target = SourcePackage(target.sourcepackagename,
+ target.distribution.currentseries)
current_release = self.target_releases.get(target)
if current_release is None:
return "No current release for this source package in %s" % (
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2011-02-05 06:11:48 +0000
+++ lib/lp/registry/browser/product.py 2011-02-24 02:49:56 +0000
@@ -2038,9 +2038,9 @@
# Set the source_package_release attribute on the licenses
# widget, so that the source package's copyright info can be
# displayed.
- ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+ ubuntu_dev = getUtility(ILaunchpadCelebrities).ubuntu.currentseries
if self.source_package_name is not None:
- release_list = ubuntu.getCurrentSourceReleases(
+ release_list = ubuntu_dev.getCurrentSourceReleases(
[self.source_package_name])
if len(release_list) != 0:
self.widgets['licenses'].source_package_release = (
=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py 2011-02-20 22:26:17 +0000
+++ lib/lp/registry/interfaces/distribution.py 2011-02-24 02:49:56 +0000
@@ -417,16 +417,6 @@
Receives a sourcepackagerelease.
"""
- def getCurrentSourceReleases(source_package_names):
- """Get the current release of a list of source packages.
-
- :param source_package_names: a list of `ISourcePackageName`
- instances.
-
- :return: a dict where the key is a `IDistributionSourcePackage`
- and the value is a `IDistributionSourcePackageRelease`.
- """
-
def getDistroSeriesAndPocket(distroseriesname):
"""Return a (distroseries,pocket) tuple which is the given textual
distroseriesname in this distribution."""
@@ -692,15 +682,6 @@
members, owner, mugshot=None, logo=None, icon=None):
"""Create a new distribution."""
- def getCurrentSourceReleases(distro_to_source_packagenames):
- """Lookup many distribution source package releases.
-
- :param distro_to_source_packagenames: A dictionary with
- its keys being `IDistribution` and its values a list of
- `ISourcePackageName`.
- :return: A dict as per `IDistribution.getCurrentSourceReleases`
- """
-
class NoSuchDistribution(NameLookupFailed):
"""Raised when we try to find a distribution that doesn't exist."""
=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py 2011-02-22 08:17:43 +0000
+++ lib/lp/registry/model/distribution.py 2011-02-24 02:49:56 +0000
@@ -775,11 +775,6 @@
"""See `IDistribution`."""
return DistributionSourcePackageRelease(self, sourcepackagerelease)
- def getCurrentSourceReleases(self, source_package_names):
- """See `IDistribution`."""
- return getUtility(IDistributionSet).getCurrentSourceReleases(
- {self:source_package_names})
-
@property
def has_any_specifications(self):
"""See `IHasSpecifications`."""
@@ -1901,56 +1896,3 @@
getUtility(IArchiveSet).new(distribution=distro,
owner=owner, purpose=ArchivePurpose.PRIMARY)
return distro
-
- def getCurrentSourceReleases(self, distro_source_packagenames):
- """See `IDistributionSet`."""
- # Builds one query for all the distro_source_packagenames.
- # This may need tuning: its possible that grouping by the common
- # archives may yield better efficiency: the current code is
- # just a direct push-down of the previous in-python lookup to SQL.
- series_clauses = []
- distro_lookup = {}
- for distro, package_names in distro_source_packagenames.items():
- source_package_ids = map(attrgetter('id'), package_names)
- # all_distro_archive_ids is just a list of ints, but it gets
- # wrapped anyway - and sqlvalues goes boom.
- archives = removeSecurityProxy(
- distro.all_distro_archive_ids)
- clause = """(spr.sourcepackagename IN %s AND
- spph.archive IN %s AND
- ds.distribution = %s)
- """ % sqlvalues(source_package_ids, archives, distro.id)
- series_clauses.append(clause)
- distro_lookup[distro.id] = distro
- if not len(series_clauses):
- return {}
- combined_clause = "(" + " OR ".join(series_clauses) + ")"
-
- releases = IStore(SourcePackageRelease).find(
- (SourcePackageRelease, Distribution.id), SQL("""
- (SourcePackageRelease.id, Distribution.id) IN (
- SELECT DISTINCT ON (
- spr.sourcepackagename, ds.distribution)
- spr.id, ds.distribution
- FROM
- SourcePackageRelease AS spr,
- SourcePackagePublishingHistory AS spph,
- DistroSeries AS ds
- WHERE
- spph.sourcepackagerelease = spr.id
- AND spph.distroseries = ds.id
- AND spph.status IN %s
- AND %s
- ORDER BY
- spr.sourcepackagename, ds.distribution, spph.id DESC
- )
- """
- % (sqlvalues(active_publishing_status) + (combined_clause,))))
- result = {}
- for sp_release, distro_id in releases:
- distro = distro_lookup[distro_id]
- sourcepackage = distro.getSourcePackage(
- sp_release.sourcepackagename)
- result[sourcepackage] = DistributionSourcePackageRelease(
- distro, sp_release)
- return result
=== modified file 'lib/lp/registry/model/distributionsourcepackage.py'
--- lib/lp/registry/model/distributionsourcepackage.py 2011-01-21 08:12:29 +0000
+++ lib/lp/registry/model/distributionsourcepackage.py 2011-02-24 02:49:56 +0000
@@ -298,9 +298,11 @@
@property
def currentrelease(self):
"""See `IDistributionSourcePackage`."""
- releases = self.distribution.getCurrentSourceReleases(
+ releases = self.distribution.currentseries.getCurrentSourceReleases(
[self.sourcepackagename])
- return releases.get(self)
+ if releases:
+ return releases.items()[0]
+ return None
def bugtasks(self, quantity=None):
"""See `IDistributionSourcePackage`."""
=== modified file 'lib/lp/registry/tests/test_distribution.py'
--- lib/lp/registry/tests/test_distribution.py 2010-10-24 12:46:23 +0000
+++ lib/lp/registry/tests/test_distribution.py 2011-02-24 02:49:56 +0000
@@ -6,7 +6,6 @@
__metaclass__ = type
from lazr.lifecycle.snapshot import Snapshot
-from zope.security.proxy import removeSecurityProxy
from canonical.testing.layers import (
DatabaseFunctionalLayer,
@@ -15,13 +14,6 @@
from lp.registry.errors import NoSuchDistroSeries
from lp.registry.interfaces.distribution import IDistribution
from lp.registry.interfaces.series import SeriesStatus
-from lp.registry.tests.test_distroseries import (
- TestDistroSeriesCurrentSourceReleases,
- )
-from lp.services.propertycache import get_property_cache
-from lp.soyuz.interfaces.distributionsourcepackagerelease import (
- IDistributionSourcePackageRelease,
- )
from lp.testing import TestCaseWithFactory
@@ -48,64 +40,6 @@
self.assertEqual("'\\u0170-distro'", displayname)
-class TestDistributionCurrentSourceReleases(
- TestDistroSeriesCurrentSourceReleases):
- """Test for Distribution.getCurrentSourceReleases().
-
- This works in the same way as
- DistroSeries.getCurrentSourceReleases() works, except that we look
- for the latest published source across multiple distro series.
- """
-
- layer = LaunchpadFunctionalLayer
- release_interface = IDistributionSourcePackageRelease
-
- @property
- def test_target(self):
- return self.distribution
-
- def test_which_distroseries_does_not_matter(self):
- # When checking for the current release, we only care about the
- # version numbers. We don't care whether the version is
- # published in a earlier or later series.
- self.current_series = self.factory.makeDistroRelease(
- self.distribution, '1.0', status=SeriesStatus.CURRENT)
- self.publisher.getPubSource(
- version='0.9', distroseries=self.current_series)
- self.publisher.getPubSource(
- version='1.0', distroseries=self.development_series)
- self.assertCurrentVersion('1.0')
-
- self.publisher.getPubSource(
- version='1.1', distroseries=self.current_series)
- self.assertCurrentVersion('1.1')
-
- def test_distribution_series_cache(self):
- distribution = removeSecurityProxy(
- self.factory.makeDistribution('foo'))
-
- cache = get_property_cache(distribution)
-
- # Not yet cached.
- self.assertNotIn("series", cache)
-
- # Now cached.
- series = distribution.series
- self.assertIs(series, cache.series)
-
- # Cache cleared.
- distribution.newSeries(
- name='bar', displayname='Bar', title='Bar', summary='',
- description='', version='1', parent_series=None,
- owner=self.factory.makePerson())
- self.assertNotIn("series", cache)
-
- # New cached value.
- series = distribution.series
- self.assertEqual(1, len(series))
- self.assertIs(series, cache.series)
-
-
class SeriesByStatusTests(TestCaseWithFactory):
"""Test IDistribution.getSeriesByStatus().
"""
