launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/api-archives.getByReference into lp:launchpad.

Commit message:
Export ArchiveSet.getByReference on the webservice, returning None if the archive doesn't exist or the user can't see it.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/api-archives.getByReference/+merge/229870

Export ArchiveSet.getByReference on the webservice, returning None if the archive doesn't exist or the user can't see it.
-- 
https://code.launchpad.net/~wgrant/launchpad/api-archives.getByReference/+merge/229870
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/api-archives.getByReference into lp:launchpad.

=== modified file 'lib/lp/app/browser/launchpad.py'
--- lib/lp/app/browser/launchpad.py	2014-05-06 12:54:34 +0000
+++ lib/lp/app/browser/launchpad.py	2014-08-06 21:43:48 +0000
@@ -142,6 +142,7 @@
 from lp.services.webapp.url import urlappend
 from lp.services.worlddata.interfaces.country import ICountrySet
 from lp.services.worlddata.interfaces.language import ILanguageSet
+from lp.soyuz.interfaces.archive import IArchiveSet
 from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
 from lp.soyuz.interfaces.livefs import ILiveFSSet
 from lp.soyuz.interfaces.packageset import IPackagesetSet
@@ -675,6 +676,7 @@
     # hierarchical navigation model.
     stepto_utilities = {
         '+announcements': IAnnouncementSet,
+        'archives': IArchiveSet,
         '+services': IServiceFactory,
         'binarypackagenames': IBinaryPackageNameSet,
         'branches': IBranchSet,

=== modified file 'lib/lp/soyuz/browser/tests/test_archive_webservice.py'
--- lib/lp/soyuz/browser/tests/test_archive_webservice.py	2013-09-13 07:09:55 +0000
+++ lib/lp/soyuz/browser/tests/test_archive_webservice.py	2014-08-06 21:43:48 +0000
@@ -27,7 +27,7 @@
 from lp.soyuz.interfaces.packagecopyjob import IPlainPackageCopyJobSource
 from lp.soyuz.model.archivepermission import ArchivePermission
 from lp.testing import (
-    celebrity_logged_in,
+    admin_logged_in,
     launchpadlib_for,
     person_logged_in,
     record_two_runs,
@@ -36,6 +36,10 @@
     )
 from lp.testing.layers import DatabaseFunctionalLayer
 from lp.testing.matchers import HasQueryCount
+from lp.testing.pages import (
+    LaunchpadWebServiceCaller,
+    webservice_for_person,
+    )
 
 
 class TestArchiveWebservice(TestCaseWithFactory):
@@ -43,7 +47,7 @@
 
     def setUp(self):
         super(TestArchiveWebservice, self).setUp()
-        with celebrity_logged_in('admin'):
+        with admin_logged_in():
             self.archive = self.factory.makeArchive(
                 purpose=ArchivePurpose.PRIMARY)
             distroseries = self.factory.makeDistroSeries(
@@ -88,7 +92,7 @@
         # getAllPermissions has a query count constant in the number of
         # permissions and people.
         def create_permission():
-            with celebrity_logged_in('admin'):
+            with admin_logged_in():
                 ArchivePermission(
                     archive=self.archive, person=self.factory.makePerson(),
                     component=getUtility(IComponentSet)["main"],
@@ -440,3 +444,46 @@
         source = getUtility(IPlainPackageCopyJobSource)
         self.assertEqual(
             None, source.getIncompleteJobsForArchive(self.archive).any())
+
+
+class TestArchiveSet(TestCaseWithFactory):
+    """Test ArchiveSet.getByReference."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_getByReference(self):
+        body = LaunchpadWebServiceCaller('consumer', '').named_get(
+            '/archives', 'getByReference', reference='ubuntu',
+            api_version='devel').jsonBody()
+        self.assertEqual(body['reference'], 'ubuntu')
+
+    def test_getByReference_ppa(self):
+        body = LaunchpadWebServiceCaller('consumer', '').named_get(
+            '/archives', 'getByReference', reference='~cprov/ubuntu/ppa',
+            api_version='devel').jsonBody()
+        self.assertEqual(body['reference'], '~cprov/ubuntu/ppa')
+
+    def test_getByReference_invalid(self):
+        body = LaunchpadWebServiceCaller('consumer', '').named_get(
+            '/archives', 'getByReference', reference='~cprov/ubuntu',
+            api_version='devel').jsonBody()
+        self.assertIs(None, body)
+
+    def test_getByReference_private(self):
+        with admin_logged_in():
+            archive = self.factory.makeArchive(private=True)
+            owner = archive.owner
+            reference = archive.reference
+            random = self.factory.makePerson()
+        body = LaunchpadWebServiceCaller('consumer', '').named_get(
+            '/archives', 'getByReference', reference=reference,
+            api_version='devel').jsonBody()
+        self.assertIs(None, body)
+        body = webservice_for_person(random).named_get(
+            '/archives', 'getByReference', reference=reference,
+            api_version='devel').jsonBody()
+        self.assertIs(None, body)
+        body = webservice_for_person(owner).named_get(
+            '/archives', 'getByReference', reference=reference,
+            api_version='devel').jsonBody()
+        self.assertEqual(body['reference'], reference)

=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2014-07-23 05:19:34 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2014-08-06 21:43:48 +0000
@@ -55,7 +55,9 @@
 from lazr.enum import DBEnumeratedType
 from lazr.restful.declarations import (
     call_with,
+    collection_default_content,
     error_status,
+    export_as_webservice_collection,
     export_as_webservice_entry,
     export_factory_operation,
     export_operation_as,
@@ -2035,8 +2037,14 @@
 class IArchiveSet(Interface):
     """Interface for ArchiveSet"""
 
+    export_as_webservice_collection(IArchive)
+
     title = Attribute('Title')
 
+    @collection_default_content()
+    def empty_list():
+        """There is no default content, but lazr.restful needs some anyway."""
+
     def new(purpose, owner, name=None, displayname=None, distribution=None,
             description=None, enabled=True, require_virtualized=True,
             private=False, suppress_subscription_notifications=False):
@@ -2072,7 +2080,14 @@
     def get(archive_id):
         """Return the IArchive with the given archive_id."""
 
-    def getByReference(reference):
+    @call_with(check_permissions=True, user=REQUEST_USER)
+    @operation_parameters(
+        reference=TextLine(
+            title=_("Archive reference string"), required=True))
+    @operation_returns_entry(schema=IArchive)
+    @export_read_operation()
+    @operation_for_version('devel')
+    def getByReference(reference, check_permissions=False, user=None):
         """Return the IArchive with the given archive reference."""
 
     def getPPAByDistributionAndOwnerName(distribution, person_name, ppa_name):

=== modified file 'lib/lp/soyuz/interfaces/webservice.py'
--- lib/lp/soyuz/interfaces/webservice.py	2014-05-06 11:52:17 +0000
+++ lib/lp/soyuz/interfaces/webservice.py	2014-08-06 21:43:48 +0000
@@ -24,6 +24,7 @@
     'IArchive',
     'IArchiveDependency',
     'IArchivePermission',
+    'IArchiveSet',
     'IArchiveSubscriber',
     'IBinaryPackageBuild',
     'IBinaryPackagePublishingHistory',
@@ -66,6 +67,7 @@
     CannotUploadToPPA,
     ComponentNotFound,
     IArchive,
+    IArchiveSet,
     InsufficientUploadRights,
     InvalidComponent,
     InvalidPocketForPartnerArchive,

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2014-08-04 12:37:41 +0000
+++ lib/lp/soyuz/model/archive.py	2014-08-06 21:43:48 +0000
@@ -36,7 +36,10 @@
     Join,
     )
 from storm.store import Store
-from zope.component import getUtility
+from zope.component import (
+    getAdapter,
+    getUtility,
+    )
 from zope.event import notify
 from zope.interface import (
     alsoProvides,
@@ -45,6 +48,7 @@
 
 from lp.app.errors import NotFoundError
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.app.interfaces.security import IAuthorization
 from lp.app.validators.name import valid_name
 from lp.archivepublisher.debversion import Version
 from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
@@ -2214,7 +2218,7 @@
         """See `IArchiveSet`."""
         return Archive.get(archive_id)
 
-    def getByReference(self, reference):
+    def getByReference(self, reference, check_permissions=False, user=None):
         """See `IArchiveSet`."""
         from lp.registry.interfaces.distribution import IDistributionSet
 
@@ -2231,7 +2235,7 @@
             distro = getUtility(IDistributionSet).getByName(bits[1])
             if distro is None:
                 return None
-            return self.getPPAOwnedByPerson(
+            archive = self.getPPAOwnedByPerson(
                 person, distribution=distro, name=bits[2])
         else:
             # Official archive reference (DISTRO or DISTRO/ARCHIVE)
@@ -2239,11 +2243,19 @@
             if distro is None:
                 return None
             if len(bits) == 1:
-                return distro.main_archive
+                archive = distro.main_archive
             elif len(bits) == 2:
-                return self.getByDistroAndName(distro, bits[1])
+                archive = self.getByDistroAndName(distro, bits[1])
             else:
                 return None
+        if not check_permissions:
+            return archive
+        authz = getAdapter(archive, IAuthorization, 'launchpad.SubscriberView')
+        if ((user is None and authz.checkUnauthenticated()) or
+            (user is not None and authz.checkAuthenticated(
+                IPersonRoles(user)))):
+            return archive
+        return None
 
     def getPPAByDistributionAndOwnerName(self, distribution, person_name,
                                          ppa_name):
@@ -2603,6 +2615,10 @@
 
         return results.order_by(SourcePackagePublishingHistory.id)
 
+    def empty_list(self):
+        """See `IArchiveSet."""
+        return []
+
 
 def get_archive_privacy_filter(user):
     if user is None:

=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2014-08-04 12:37:41 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2014-08-06 21:43:48 +0000
@@ -3162,6 +3162,47 @@
             getUtility(IArchiveSet).getByReference(
                 'that/does/not/make/sense'))
 
+    def test_check_permissions_private(self):
+        private_owner = self.factory.makeTeam(
+            visibility=PersonVisibility.PRIVATE)
+        private = self.factory.makeArchive(owner=private_owner, private=True)
+        with admin_logged_in():
+            private_reference = private.reference
+        self.assertEqual(
+            private,
+            getUtility(IArchiveSet).getByReference(
+                private_reference, check_permissions=False))
+        self.assertIs(
+            None,
+            getUtility(IArchiveSet).getByReference(
+                private_reference, check_permissions=True))
+        self.assertEqual(
+            private,
+            getUtility(IArchiveSet).getByReference(
+                private_reference, check_permissions=True,
+                user=private_owner))
+        self.assertIs(
+            None,
+            getUtility(IArchiveSet).getByReference(
+                private_reference, check_permissions=True,
+                user=self.factory.makePerson()))
+
+    def test_check_permissions_public(self):
+        public = self.factory.makeArchive(private=False)
+        self.assertEqual(
+            public,
+            getUtility(IArchiveSet).getByReference(
+                public.reference, check_permissions=False))
+        self.assertEqual(
+            public,
+            getUtility(IArchiveSet).getByReference(
+                public.reference, check_permissions=True))
+        self.assertEqual(
+            public,
+            getUtility(IArchiveSet).getByReference(
+                public.reference, check_permissions=True,
+                user=self.factory.makePerson()))
+
 
 class TestDisplayName(TestCaseWithFactory):