launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/das-filter-webservice into lp:launchpad with lp:~cjwatson/launchpad/das-filter-initialize-ds as a prerequisite.

Commit message:
Export IDistroArchSeries.setFilter and IDistroArchSeries.removeFilter on the webservice.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1842658 in Launchpad itself: "Support central filtering of which packages build for some architectures"
  https://bugs.launchpad.net/launchpad/+bug/1842658

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/das-filter-webservice/+merge/372265
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/das-filter-webservice into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/tests/test_distroarchseries_webservice.py'
--- lib/lp/soyuz/browser/tests/test_distroarchseries_webservice.py	2019-07-30 11:38:18 +0000
+++ lib/lp/soyuz/browser/tests/test_distroarchseries_webservice.py	2019-09-04 14:19:10 +0000
@@ -11,12 +11,14 @@
     BadRequest,
     Unauthorized,
     )
+from testtools.matchers import MatchesStructure
 from zope.security.management import endInteraction
 
 from lp.buildmaster.enums import BuildBaseImageType
 from lp.registry.enums import PersonVisibility
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.services.features.testing import FeatureFixture
+from lp.soyuz.enums import DistroArchSeriesFilterSense
 from lp.soyuz.interfaces.livefs import LIVEFS_FEATURE_FLAG
 from lp.testing import (
     api_url,
@@ -291,3 +293,56 @@
             image_type="LXD image")
         self.assertIsNone(das.getChroot(image_type=BuildBaseImageType.CHROOT))
         self.assertEqual(lfa, das.getChroot(image_type=BuildBaseImageType.LXD))
+
+    def test_setFilter_removeFilter_random_user(self):
+        # Random users are not allowed to set or remove filters.
+        das = self.factory.makeDistroArchSeries()
+        packageset = self.factory.makePackageset(distroseries=das.distroseries)
+        user = self.factory.makePerson()
+        packageset_url = api_url(packageset)
+        webservice = launchpadlib_for("testing", user, version="devel")
+        ws_das = ws_object(webservice, das)
+        self.assertRaises(
+            Unauthorized, ws_das.setFilter,
+            packageset=packageset_url, sense="Include")
+        self.assertRaises(Unauthorized, ws_das.removeFilter)
+
+    def test_setFilter_wrong_distroseries(self):
+        # Trying to set a filter using a packageset for the wrong
+        # distroseries returns an error.
+        das = self.factory.makeDistroArchSeries()
+        packageset = self.factory.makePackageset()
+        user = das.distroseries.distribution.main_archive.owner
+        packageset_url = api_url(packageset)
+        webservice = launchpadlib_for("testing", user, version="devel")
+        ws_das = ws_object(webservice, das)
+        e = self.assertRaises(
+            BadRequest, ws_das.setFilter,
+            packageset=packageset_url, sense="Include")
+        self.assertEqual(
+            "The requested package set is for %s and cannot be set as a "
+            "filter for %s %s." % (
+                packageset.distroseries.fullseriesname,
+                das.distroseries.fullseriesname, das.architecturetag),
+            e.content)
+
+    def test_setFilter_removeFilter(self):
+        das = self.factory.makeDistroArchSeries()
+        packageset = self.factory.makePackageset(distroseries=das.distroseries)
+        user = das.distroseries.distribution.main_archive.owner
+        packageset_url = api_url(packageset)
+        webservice = launchpadlib_for("testing", user, version="devel")
+        ws_das = ws_object(webservice, das)
+        ws_das.setFilter(packageset=packageset_url, sense="Include")
+        with person_logged_in(user):
+            dasf = das.getFilter()
+        self.assertThat(dasf, MatchesStructure.byEquality(
+            packageset=packageset, sense=DistroArchSeriesFilterSense.INCLUDE))
+        ws_das.setFilter(packageset=packageset_url, sense="Exclude")
+        with person_logged_in(user):
+            dasf = das.getFilter()
+        self.assertThat(dasf, MatchesStructure.byEquality(
+            packageset=packageset, sense=DistroArchSeriesFilterSense.EXCLUDE))
+        ws_das.removeFilter()
+        with person_logged_in(user):
+            self.assertIsNone(das.getFilter())

=== modified file 'lib/lp/soyuz/interfaces/distroarchseries.py'
--- lib/lp/soyuz/interfaces/distroarchseries.py	2019-09-04 14:19:10 +0000
+++ lib/lp/soyuz/interfaces/distroarchseries.py	2019-09-04 14:19:10 +0000
@@ -16,6 +16,7 @@
 import httplib
 
 from lazr.restful.declarations import (
+    call_with,
     error_status,
     export_as_webservice_entry,
     export_read_operation,
@@ -23,6 +24,7 @@
     exported,
     operation_for_version,
     operation_parameters,
+    REQUEST_USER,
     )
 from lazr.restful.fields import (
     Reference,
@@ -49,6 +51,7 @@
 from lp.registry.interfaces.person import IPerson
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.role import IHasOwner
+from lp.soyuz.enums import DistroArchSeriesFilterSense
 from lp.soyuz.interfaces.buildrecords import IHasBuildRecords
 
 
@@ -292,6 +295,15 @@
         tarball".
         """
 
+    @operation_parameters(
+        # Really IPackageset, patched in _schema_circular_imports.py.
+        packageset=Reference(Interface, title=_("Package set"), required=True),
+        sense=Choice(
+            vocabulary=DistroArchSeriesFilterSense,
+            title=_("Sense"), required=True))
+    @call_with(creator=REQUEST_USER)
+    @export_write_operation()
+    @operation_for_version("devel")
     def setFilter(packageset, sense, creator):
         """Set a filter for packages to build for this architecture.
 
@@ -314,6 +326,8 @@
         :param creator: The `IPerson` who is creating this filter.
         """
 
+    @export_write_operation()
+    @operation_for_version("devel")
     def removeFilter():
         """Remove any filter for packages to build for this architecture.