Merge ~ilasc/launchpad:move-getPublishedBinaries-to-IArchiveSubscriberView into launchpad:master

[Ioana Lasc]

Move getPublishedBinaries to IArchiveSubscriberView

[Colin Watson (cjwatson)]

This is fine per the investigation of why `getPublishedSources` and `getPublishedBinaries` ended up being different (`getPublishedSources` was moved in response to https://bugs.launchpad.net/launchpad/+bug/599286; apparently we don't necessarily want subscribers to be able to see things like build information, but since they can see the published archive it should be fine for them to search for published binaries).

However, could you please add a test for the changed behaviour? It could go in `lp.soyuz.browser.tests.test_archive_webservice.TestGetPublishedBinaries`.

review: Approve

[Ioana Lasc (ilasc)]

Good point Colin, added unit tests, thanks!

[Colin Watson (cjwatson)]

review: Approve

diff --git a/lib/lp/soyuz/browser/tests/test_archive_webservice.py b/lib/lp/soyuz/browser/tests/test_archive_webservice.py
index c404f6e..96d217f 100644
--- a/lib/lp/soyuz/browser/tests/test_archive_webservice.py
+++ b/lib/lp/soyuz/browser/tests/test_archive_webservice.py
@@ -697,6 +697,32 @@ class TestGetPublishedBinaries(TestCaseWithFactory):
         self.assertEqual(200, response.status)
         self.assertEqual(1, response.jsonBody()["total_size"])
 
+    def test_getPublishedBinaries_private_subscriber(self):
+        # Subscribers can see published binaries for private archives.
+        private_archive = self.factory.makeArchive(private=True)
+        with admin_logged_in():
+            self.factory.makeBinaryPackagePublishingHistory(
+                archive=private_archive)
+        subscriber = self.factory.makePerson()
+        with person_logged_in(private_archive.owner):
+            private_archive.newSubscription(subscriber, private_archive.owner)
+        archive_url = api_url(private_archive)
+        ws = webservice_for_person(
+            subscriber, permission=OAuthPermission.READ_PRIVATE,
+            default_api_version="devel")
+        response = ws.named_get(archive_url, "getPublishedBinaries")
+        self.assertEqual(200, response.status)
+        self.assertEqual(1, response.jsonBody()["total_size"])
+
+    def test_getPublishedBinaries_private_non_subscriber(self):
+        private_archive = self.factory.makeArchive(private=True)
+        archive_url = api_url(private_archive)
+        ws = webservice_for_person(
+            self.factory.makePerson(), permission=OAuthPermission.READ_PRIVATE,
+            default_api_version="devel")
+        response = ws.named_get(archive_url, "getPublishedBinaries")
+        self.assertEqual(401, response.status)
+
     def test_getPublishedBinaries_created_since_date(self):
         datecreated = self.factory.getUniqueDate()
         later_date = datecreated + timedelta(minutes=1)
diff --git a/lib/lp/soyuz/interfaces/archive.py b/lib/lp/soyuz/interfaces/archive.py
index 1752ba3..9f60d5d 100644
--- a/lib/lp/soyuz/interfaces/archive.py
+++ b/lib/lp/soyuz/interfaces/archive.py
@@ -590,6 +590,86 @@ class IArchiveSubscriberView(Interface):
         :return: A new IArchiveAuthToken
         """
 
+    @call_with(eager_load=True)
+    @rename_parameters_as(
+        name="binary_name", distroarchseries="distro_arch_series")
+    @operation_parameters(
+        name=TextLine(title=_("Binary Package Name"), required=False),
+        version=TextLine(title=_("Version"), required=False),
+        status=Choice(
+            title=_("Package Publishing Status"),
+            description=_("The status of this publishing record"),
+            vocabulary=PackagePublishingStatus,
+            required=False),
+        distroarchseries=Reference(
+            # Really IDistroArchSeries, circular import fixed below.
+            Interface,
+            title=_("Distro Arch Series"), required=False),
+        pocket=Choice(
+            title=_("Pocket"),
+            description=_("The pocket into which this entry is published"),
+            vocabulary=PackagePublishingPocket,
+            required=False, readonly=True),
+        exact_match=Bool(
+            description=_("Whether or not to filter binary names by exact "
+                          "matching."),
+            required=False),
+        created_since_date=Datetime(
+            title=_("Created Since Date"),
+            description=_("Return entries whose `date_created` is greater "
+                          "than or equal to this date."),
+            required=False),
+        ordered=Bool(
+            title=_("Ordered"),
+            description=_("Return ordered results by default, but specifying "
+                          "False will return results more quickly."),
+            required=False, readonly=True),
+        order_by_date=Bool(
+            title=_("Order by creation date"),
+            description=_("Return newest results first. This is recommended "
+                          "for applications that need to catch up with "
+                          "publications since their last run."),
+            required=False),
+        )
+    # Really returns IBinaryPackagePublishingHistory, see below for
+    # patch to avoid circular import.
+    @operation_returns_collection_of(Interface)
+    @export_operation_as("getPublishedBinaries")
+    @export_read_operation()
+    def getAllPublishedBinaries(name=None, version=None, status=None,
+                                distroarchseries=None, pocket=None,
+                                exact_match=False, created_since_date=None,
+                                ordered=True, order_by_date=False,
+                                include_removed=True, only_unpublished=False,
+                                eager_load=False):
+        """All `IBinaryPackagePublishingHistory` target to this archive.
+
+        :param name: binary name filter (exact match or SQL LIKE controlled
+                      by 'exact_match' argument).
+        :param version: binary version filter (always exact match).
+        :param status: `PackagePublishingStatus` filter, can be a list.
+        :param distroarchseries: `IDistroArchSeries` filter, can be a list.
+        :param pocket: `PackagePublishingPocket` filter.
+        :param exact_match: either or not filter source names by exact
+                             matching.
+        :param created_since_date: Only return publications created on or
+            after this date.
+        :param ordered: Normally publications are ordered by binary package
+            name and then ID order (creation order).  If this parameter is
+            False then the results will be unordered.  This will make the
+            operation much quicker to return results if you don't care about
+            ordering.
+        :param order_by_date: Order publications by descending creation date
+            and then by descending ID.  This is suitable for applications
+            that need to catch up with publications since their last run.
+        :param include_removed: If True, include publications that have been
+            removed from disk as well as those that have not.
+        :param only_unpublished: If True, only include publications that
+            have never been published to disk.
+
+        :return: A collection containing `BinaryPackagePublishingHistory`.
+        """
+
 
 class IArchiveView(IHasBuildRecords):
     """Archive interface for operations restricted by view privilege."""
@@ -1191,86 +1271,6 @@ class IArchiveView(IHasBuildRecords):
     def getOverridePolicy(distroseries, pocket, phased_update_percentage=None):
         """Returns an instantiated `IOverridePolicy` for the archive."""
 
-    @call_with(eager_load=True)
-    @rename_parameters_as(
-        name="binary_name", distroarchseries="distro_arch_series")
-    @operation_parameters(
-        name=TextLine(title=_("Binary Package Name"), required=False),
-        version=TextLine(title=_("Version"), required=False),
-        status=Choice(
-            title=_("Package Publishing Status"),
-            description=_("The status of this publishing record"),
-            vocabulary=PackagePublishingStatus,
-            required=False),
-        distroarchseries=Reference(
-            # Really IDistroArchSeries, circular import fixed below.
-            Interface,
-            title=_("Distro Arch Series"), required=False),
-        pocket=Choice(
-            title=_("Pocket"),
-            description=_("The pocket into which this entry is published"),
-            vocabulary=PackagePublishingPocket,
-            required=False, readonly=True),
-        exact_match=Bool(
-            description=_("Whether or not to filter binary names by exact "
-                          "matching."),
-            required=False),
-        created_since_date=Datetime(
-            title=_("Created Since Date"),
-            description=_("Return entries whose `date_created` is greater "
-                          "than or equal to this date."),
-            required=False),
-        ordered=Bool(
-            title=_("Ordered"),
-            description=_("Return ordered results by default, but specifying "
-                          "False will return results more quickly."),
-            required=False, readonly=True),
-        order_by_date=Bool(
-            title=_("Order by creation date"),
-            description=_("Return newest results first. This is recommended "
-                          "for applications that need to catch up with "
-                          "publications since their last run."),
-            required=False),
-        )
-    # Really returns IBinaryPackagePublishingHistory, see below for
-    # patch to avoid circular import.
-    @operation_returns_collection_of(Interface)
-    @export_operation_as("getPublishedBinaries")
-    @export_read_operation()
-    def getAllPublishedBinaries(name=None, version=None, status=None,
-                                distroarchseries=None, pocket=None,
-                                exact_match=False, created_since_date=None,
-                                ordered=True, order_by_date=False,
-                                include_removed=True, only_unpublished=False,
-                                eager_load=False):
-        """All `IBinaryPackagePublishingHistory` target to this archive.
-
-        :param name: binary name filter (exact match or SQL LIKE controlled
-                      by 'exact_match' argument).
-        :param version: binary version filter (always exact match).
-        :param status: `PackagePublishingStatus` filter, can be a list.
-        :param distroarchseries: `IDistroArchSeries` filter, can be a list.
-        :param pocket: `PackagePublishingPocket` filter.
-        :param exact_match: either or not filter source names by exact
-                             matching.
-        :param created_since_date: Only return publications created on or
-            after this date.
-        :param ordered: Normally publications are ordered by binary package
-            name and then ID order (creation order).  If this parameter is
-            False then the results will be unordered.  This will make the
-            operation much quicker to return results if you don't care about
-            ordering.
-        :param order_by_date: Order publications by descending creation date
-            and then by descending ID.  This is suitable for applications
-            that need to catch up with publications since their last run.
-        :param include_removed: If True, include publications that have been
-            removed from disk as well as those that have not.
-        :param only_unpublished: If True, only include publications that
-            have never been published to disk.
-
-        :return: A collection containing `BinaryPackagePublishingHistory`.
-        """
-
     @operation_parameters(
         include_needsbuild=Bool(
             title=_("Include builds with state NEEDSBUILD"), required=False))