launchpad-reviewers team mailing list archive

[Colin Watson <mp+426316@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:getPublishedBinaries-inactive-flag into launchpad:master.

Commit message:
Add active_binaries_only option to SPPH.getPublishedBinaries

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1885545 in Launchpad itself: "SourcePackagePublishingHistory.getPublishedBinaries only reported Published/Pending records"
  https://bugs.launchpad.net/launchpad/+bug/1885545

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/426316

This defaults to True, and can be set to False to ask for binaries with any publication status rather than only those currently Published or Pending.  This was already supported by the underlying `_getSourceBinaryJoinForSources` method, but not exported.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:getPublishedBinaries-inactive-flag into launchpad:master.

diff --git a/lib/lp/soyuz/doc/publishing.rst b/lib/lp/soyuz/doc/publishing.rst
index 1e3e580..fb5cf2b 100644
--- a/lib/lp/soyuz/doc/publishing.rst
+++ b/lib/lp/soyuz/doc/publishing.rst
@@ -518,12 +518,16 @@ previous broken implementation in this area.
 
 Note that even PENDING binary publications are returned by
 getPublishedBinaries(), it considers both PENDING and PUBLISHED status
-as active, SUPERSEDED, DELETED and OBSOLETE are excluded. Differently,
+as active, SUPERSEDED, DELETED and OBSOLETE are excluded (unless we pass
+``active_binaries_only=False``). Differently,
 getBuiltBinaries() follows binaries in any state.
 
     >>> source.getPublishedBinaries().count()
     2
 
+    >>> source.getPublishedBinaries(active_binaries_only=False).count()
+    2
+
     >>> len(source.getBuiltBinaries())
     2
 
@@ -531,8 +535,8 @@ Note that getPublishedBinaries() returns a DecoratedResultSet and
 getBuiltBinaries() returns a list.
 
 When we supersede one of the original binary publications, it gets
-excluded from the getPublishedBinaries() results, but not from the
-getBuiltBinaries() result.
+excluded from the getPublishedBinaries() results, but not if we pass
+``active_binaries_only=False``, and not from the getBuiltBinaries() result.
 
     >>> a_binary = source.getPublishedBinaries()[0]
     >>> a_binary.supersede()
@@ -540,6 +544,9 @@ getBuiltBinaries() result.
     >>> source.getPublishedBinaries().count()
     1
 
+    >>> source.getPublishedBinaries(active_binaries_only=False).count()
+    2
+
     >>> len(source.getBuiltBinaries())
     2
 
@@ -553,6 +560,9 @@ published in the original location.
     >>> source.getPublishedBinaries().count()
     0
 
+    >>> source.getPublishedBinaries(active_binaries_only=False).count()
+    2
+
     >>> len(source.getBuiltBinaries())
     2
 
@@ -568,6 +578,9 @@ verify that the getPublishedBinaries() result is also empty after that.
     >>> copied_source.getPublishedBinaries().count()
     0
 
+    >>> copied_source.getPublishedBinaries(active_binaries_only=False).count()
+    2
+
     >>> len(copied_source.getBuiltBinaries())
     2
 
diff --git a/lib/lp/soyuz/interfaces/publishing.py b/lib/lp/soyuz/interfaces/publishing.py
index be55014..95ddb5d 100644
--- a/lib/lp/soyuz/interfaces/publishing.py
+++ b/lib/lp/soyuz/interfaces/publishing.py
@@ -416,18 +416,24 @@ class ISourcePackagePublishingHistoryPublic(IPublishingView):
             required=False, readonly=True
         ))
 
+    @operation_parameters(
+        active_binaries_only=Bool(
+            title=_("Only return active publications"), required=False))
     # Really IBinaryPackagePublishingHistory, see below.
     @operation_returns_collection_of(Interface)
     @export_read_operation()
     @operation_for_version("beta")
-    def getPublishedBinaries():
+    def getPublishedBinaries(active_binaries_only=True):
         """Return all resulted `IBinaryPackagePublishingHistory`.
 
-        Follow the build record and return every PUBLISHED or PENDING
-        binary publishing record for any `DistroArchSeries` in this
-        `DistroSeries` and in the same `IArchive` and Pocket, ordered
-        by architecture tag.
+        Follow the build record and return every binary publishing record
+        for any `DistroArchSeries` in this `DistroSeries` and in the same
+        `IArchive` and Pocket, ordered by architecture tag.  If
+        `active_binaries_only` is True (the default), then only return
+        PUBLISHED or PENDING binary publishing records.
 
+        :param active_binaries_only: If True, only return PUBLISHED or
+            PENDING publishing records.
         :return: a list with all corresponding publishing records.
         """
 
diff --git a/lib/lp/soyuz/model/publishing.py b/lib/lp/soyuz/model/publishing.py
index b2380e0..b7f7e3f 100644
--- a/lib/lp/soyuz/model/publishing.py
+++ b/lib/lp/soyuz/model/publishing.py
@@ -327,10 +327,11 @@ class SourcePackagePublishingHistory(SQLBase, ArchivePublisherBase):
             return None
         return channel_list_to_string(*self._channel)
 
-    def getPublishedBinaries(self):
+    def getPublishedBinaries(self, active_binaries_only=True):
         """See `ISourcePackagePublishingHistory`."""
         publishing_set = getUtility(IPublishingSet)
-        result_set = publishing_set.getBinaryPublicationsForSources(self)
+        result_set = publishing_set.getBinaryPublicationsForSources(
+            self, active_binaries_only=active_binaries_only)
         return DecoratedResultSet(result_set, result_decorator=itemgetter(1))
 
     def getBuiltBinaries(self, want_files=False):
@@ -1584,7 +1585,8 @@ class PublishingSet:
 
         return result_set
 
-    def getBinaryPublicationsForSources(self, one_or_more_source_publications):
+    def getBinaryPublicationsForSources(self, one_or_more_source_publications,
+                                        active_binaries_only=True):
         """See `IPublishingSet`."""
         source_publication_ids = self._extractIDs(
             one_or_more_source_publications)
@@ -1592,7 +1594,9 @@ class PublishingSet:
         result_set = IStore(SourcePackagePublishingHistory).find(
             (SourcePackagePublishingHistory, BinaryPackagePublishingHistory,
              BinaryPackageRelease, BinaryPackageName, DistroArchSeries),
-            self._getSourceBinaryJoinForSources(source_publication_ids))
+            self._getSourceBinaryJoinForSources(
+                source_publication_ids,
+                active_binaries_only=active_binaries_only))
 
         result_set.order_by(
             SourcePackagePublishingHistory.id,