Merge lp:~cody-somerville/launchpad/fix-bug-297709 into lp:launchpad

[Cody A.W. Somerville]

Fix latest_overall_publication property of DistributionSourcePackage to return the correct publication even when obsolete publications exist for a package.

[Brad Crittenden (bac)]

Hi Cody,

Thanks for taking on this branch -- we really appreciate the contribution.

In the future note we do like a merge proposal to include a cover letter with some background information on the task. See https://code.edge.launchpad.net/~bac/launchpad/bug-483607/+merge/14925 for an example. There is a bzr plugin at https://edge.launchpad.net/lpreview-body which makes creating the cover letter very easy. Just a FYI, now on to the review.

* We try to use en_US spellings, though we don't follow it strictly, so please s/favour/favor. Also I note another typo that is not your but it would be great for you to fix s/independant/independent.

* At line 74 could you explicitly set the pocket for ff_pub_warty so the reader doesn't have to know the default?

* The test has a lot of trailing whitespace. Please delete those.

* In the code you have:

from lp.registry.model.sourcepackage import (SourcePackage,
    SourcePackageQuestionTargetMixin)

Please move that first package to the next line, which is part of our coding standards. All of the packages should be sorted alphabetically too, which they are.

* Please remove the unnecessary parens from:

from lp.soyuz.model.publishing import (SourcePackagePublishingHistory)
from lp.soyuz.model.sourcepackagerelease import (SourcePackageRelease)

* Thanks a lot for getting rid of the magic numbers!

Again, thanks for contributing this fix. The change looks great overall and will be ready to land after making the minor changes. I'm marking the proposal 'Approved' since the changes are so straightforward and the changes don't need to be re-reviewed. I'm not sure if you have permission to land this yourself but I'll be happy to do so if you cannot.

review: Approve (code)

[Brad Crittenden (bac)]

Cody,

Thanks for making the requested changes.

I've tried to land this branch for you but have run into some problems. Since you wrote your branch we have updated Launchpad to run with Python 2.5.

In order to me to land your branch and ensure you get credit for it I'll need you to do a 'rocketfuel-get' and in your branch merge from trunk and then re-push to Launchpad. If you run into any problems please ask on #launchpad-dev.

Thanks,
Brad

[Brad Crittenden (bac)]

Cody,

I tried something different and now have your branch playing on PQM. For the moment, please disregard the previous request.

=== modified file 'lib/lp/registry/doc/distribution-sourcepackage.txt'
--- lib/lp/registry/doc/distribution-sourcepackage.txt	2009-09-19 08:14:43 +0000
+++ lib/lp/registry/doc/distribution-sourcepackage.txt	2009-11-17 21:21:15 +0000
@@ -1,7 +1,7 @@
 = Distribution Source Packages =
 
 A distribution source package represents a named source package in a
-distribution, independant of any particular release of that source
+distribution, independent of any particular release of that source
 package.
 
 This is useful for, among other things, tracking bugs in source
@@ -59,36 +59,53 @@
 
 Current overall publications:
 
-    >>> alsa_utils = ubuntu.getSourcePackage("alsa-utils")
-    >>> pub = alsa_utils.latest_overall_publication
-    >>> (pub.sourcepackagerelease.sourcepackagename.name,
-    ...  pub.sourcepackagerelease.version,
-    ...  pub.distroseries.name,
-    ...  pub.status)
-    (u'alsa-utils',
-     u'1.0.9a-4ubuntu1',
-     u'hoary',
-     <DBItem PackagePublishingStatus.PUBLISHED, (2) Published>)
-    >>> linux_source = ubuntu.getSourcePackage("linux-source-2.6.15")
-    >>> pub = linux_source.latest_overall_publication
-    >>> (pub.sourcepackagerelease.sourcepackagename.name,
-    ...  pub.sourcepackagerelease.version,
-    ...  pub.distroseries.name,
-    ...  pub.status)
-    (u'linux-source-2.6.15',
-     u'2.6.15.3',
-     u'hoary',
-     <DBItem PackagePublishingStatus.PUBLISHED, (2) Published>)
-    >>> commercial = ubuntu.getSourcePackage("commercialpackage")
-    >>> pub = linux_source.latest_overall_publication
-    >>> (pub.sourcepackagerelease.sourcepackagename.name,
-    ...  pub.sourcepackagerelease.version,
-    ...  pub.distroseries.name,
-    ...  pub.status)
-    (u'linux-source-2.6.15',
-     u'2.6.15.3',
-     u'hoary',
-     <DBItem PackagePublishingStatus.PUBLISHED, (2) Published>)
+A DistributionSourcePackage has a property called 'latest_overall_publication'
+which returns the latest overall relevant publication. Relevant is currently
+defined as being either published or obsolete with published being preferred,
+sorted by distroseries and component with publications in proposed and backports
+excluded.
+
+    >>> from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
+    >>> from lp.soyuz.interfaces.publishing import PackagePublishingStatus
+    >>> publisher = SoyuzTestPublisher()
+    >>> publisher.prepareBreezyAutotest()
+    >>> warty = ubuntu['warty']
+    >>> hoary = ubuntu['hoary']
+
+This demonstrates the scenario where a newer distroseries becomes obsolete 
+before an older distroseries. The latest_overall_publication property will
+return the publication from the older distroseries because a published
+publication is considered more relevant than an obsolete publication.
+
+Note that the component of the package in the newer obsolete distroseries 
+is 'main' and in the older distroseries it is 'universe'.
+
+    >>> compiz_publication_warty = publisher.getPubSource(
+    ...     sourcename='compiz', version='0.01-1ubuntu1', distroseries=warty,
+    ...     status=PackagePublishingStatus.PUBLISHED, component='universe')
+    >>> compiz_publication_hoary = publisher.getPubSource(
+    ...     sourcename='compiz', version='0.01-1ubuntu1', distroseries=hoary,
+    ...     status=PackagePublishingStatus.OBSOLETE, component='main')
+    >>> compiz = ubuntu.getSourcePackage('compiz')
+    >>> print compiz.latest_overall_publication.component.name
+    universe
+
+When more than one published publication exists in a single distroseries, 
+latest_overall_publication will favor updates over security and security over
+release.
+
+    >>> from lp.registry.interfaces.pocket import PackagePublishingPocket
+    >>> firefox_publication_warty = publisher.getPubSource(
+    ...     sourcename='firefox', version='0.01-1ubuntu1', distroseries=hoary,
+    ...     status=PackagePublishingStatus.PUBLISHED, component='main',
+    ...     pocket=PackagePublishingPocket.RELEASE)
+    >>> firefox_publication_hoary = publisher.getPubSource(
+    ...     sourcename='firefox', version='0.01-1ubuntu1.1', distroseries=hoary,
+    ...     status=PackagePublishingStatus.PUBLISHED, component='main',
+    ...     pocket=PackagePublishingPocket.SECURITY)
+    >>> firefox = ubuntu.getSourcePackage('firefox')
+    >>> print firefox.latest_overall_publication.pocket.name
+    SECURITY
 
 === Release-related properties ===
 
@@ -164,7 +181,6 @@
     ...     print pub.distroseries.name, pub.status.name, is_removed
     breezy-autotest DELETED True
 
-
 == __hash__ ==
 
 DistributionSourcePackage defines a custom __hash__ method, so that
@@ -192,7 +208,6 @@
     >>> mapping[ubuntu.getSourcePackage('mozilla-firefox')] is firefox_marker
     True
 
-
 == Upstream links ==
 
 DistributionSourcePackages can be linked to upstream Products. You can
@@ -209,7 +224,6 @@
     >>> print pmount.upstream_product
     None
 
-
 == Finding archives where this package is published ==
 
 A distribution source package can also find which archives

=== modified file 'lib/lp/registry/model/distributionsourcepackage.py'
--- lib/lp/registry/model/distributionsourcepackage.py	2009-10-26 18:41:50 +0000
+++ lib/lp/registry/model/distributionsourcepackage.py	2009-11-17 21:21:15 +0000
@@ -20,33 +20,30 @@
 from storm.locals import Int, Reference, Store, Storm, Unicode
 from zope.interface import implements
 
+from canonical.database.sqlbase import sqlvalues
+from canonical.launchpad.database.emailaddress import EmailAddress
+from canonical.launchpad.database.structuralsubscription import (
+    StructuralSubscriptionTargetMixin)
+from canonical.launchpad.interfaces.lpstorm import IStore
+from canonical.lazr.utils import smartquote
 from lp.answers.interfaces.questiontarget import IQuestionTarget
-from lp.registry.interfaces.product import IDistributionSourcePackage
-from canonical.database.sqlbase import sqlvalues
 from lp.bugs.model.bug import BugSet, get_bug_tags_open_count
 from lp.bugs.model.bugtarget import BugTargetBase
 from lp.bugs.model.bugtask import BugTask
 from lp.code.model.hasbranches import HasBranchesMixin, HasMergeProposalsMixin
+from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.registry.interfaces.product import IDistributionSourcePackage
+from lp.registry.model.karma import KarmaTotalCache
+from lp.registry.model.person import Person
+from lp.registry.model.sourcepackage import (
+    SourcePackage, SourcePackageQuestionTargetMixin)
 from lp.soyuz.interfaces.archive import ArchivePurpose
 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
 from lp.soyuz.model.archive import Archive
 from lp.soyuz.model.distributionsourcepackagerelease import (
     DistributionSourcePackageRelease)
-from lp.soyuz.model.publishing import (
-    SourcePackagePublishingHistory)
-from lp.soyuz.model.sourcepackagerelease import (
-    SourcePackageRelease)
-from lp.registry.model.karma import KarmaTotalCache
-from lp.registry.model.person import Person
-from lp.registry.model.sourcepackage import (
-    SourcePackage, SourcePackageQuestionTargetMixin)
-from canonical.launchpad.database.emailaddress import EmailAddress
-from canonical.launchpad.database.structuralsubscription import (
-    StructuralSubscriptionTargetMixin)
-from canonical.launchpad.interfaces.lpstorm import IStore
-
-from canonical.lazr.utils import smartquote
-
+from lp.soyuz.model.publishing import SourcePackagePublishingHistory
+from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
 
 class DistributionSourcePackage(BugTargetBase,
                                 SourcePackageQuestionTargetMixin,
@@ -151,15 +148,19 @@
                 SourcePackageRelease.id AND
             SourcePackageRelease.sourcepackagename = %s AND
             SourcePackagePublishingHistory.archive IN %s AND
-            pocket NOT IN (30, 40) AND
-            status in (2,5)""" %
+            pocket NOT IN (%s, %s) AND
+            status in (%s, %s)""" %
                 sqlvalues(self.distribution,
                           self.sourcepackagename,
-                          self.distribution.all_distro_archive_ids),
+                          self.distribution.all_distro_archive_ids,
+                          PackagePublishingPocket.PROPOSED,
+                          PackagePublishingPocket.BACKPORTS,
+                          PackagePublishingStatus.PUBLISHED,
+                          PackagePublishingStatus.OBSOLETE),
             clauseTables=["SourcePackagePublishingHistory",
                           "SourcePackageRelease", 
                           "DistroSeries"],
-            orderBy=["-status",
+            orderBy=["status",
                      SQLConstant(
                         "to_number(DistroSeries.version, '99.99') DESC"),
                      "-pocket"])