Merge lp:~jelmer/launchpad/refactor-permissions into lp:launchpad

[Jelmer Vernooij]

Move archive permission checking functions onto Archive.

This move some of the permission checking functions for archives on the Archive method and removes a couple of duplicate functions in the process.

This is in preparation of exposing these methods over the web API, as requested by james_w.

[Brad Crittenden (bac)]

Hi Jelmer this refactoring looks good. In test_archive.py please end all comments with punctuation.

review: Approve (code)

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2010-04-26 15:02:03 +0000
+++ lib/canonical/launchpad/security.py	2010-05-11 14:12:39 +0000
@@ -12,8 +12,6 @@
 from zope.component import getAdapter, getUtility
 
 from canonical.launchpad.interfaces.account import IAccount
-from lp.archiveuploader.permission import (
-    can_upload_to_archive, check_upload_to_archive)
 from canonical.launchpad.interfaces.emailaddress import IEmailAddress
 from lp.registry.interfaces.announcement import IAnnouncement
 from lp.soyuz.interfaces.archive import IArchive
@@ -1475,9 +1473,9 @@
         # user to retry build if so.
         # strict_component is True because the source package already exists,
         # otherwise, how can they give it back?
-        check_perms = check_upload_to_archive(
+        check_perms = self.obj.archive.checkUpload(
             user.person, self.obj.distroseries,
-            self.obj.sourcepackagerelease.sourcepackagename, self.obj.archive,
+            self.obj.sourcepackagerelease.sourcepackagename, 
             self.obj.current_component, self.obj.pocket,
             strict_component=True)
         return check_perms == None
@@ -1718,7 +1716,8 @@
     # one combination that allows us to upload the corresponding source
     # package.
     for ssp in ssp_list:
-        if can_upload_to_archive(person_role.person, ssp):
+        archive = ssp.sourcepackage.get_default_archive()
+        if archive.canUploadSuiteSourcePackage(person_role.person, ssp):
             return True
     return False
 
@@ -2047,7 +2046,7 @@
             return True
 
         # Uploaders can view private PPAs.
-        if self.obj.is_ppa and self.obj.canUpload(user.person):
+        if self.obj.is_ppa and self.obj.checkArchivePermission(user.person):
             return True
 
         return False
@@ -2062,7 +2061,7 @@
 
     No one can upload to disabled archives.
 
-    PPA upload rights are managed via `IArchive.canUpload`;
+    PPA upload rights are managed via `IArchive.checkArchivePermission`;
 
     Appending to PRIMARY, PARTNER or COPY archives is restricted to owners.
 
@@ -2079,7 +2078,7 @@
         if user.inTeam(self.obj.owner):
             return True
 
-        if self.obj.is_ppa and self.obj.canUpload(user.person):
+        if self.obj.is_ppa and self.obj.checkArchivePermission(user.person):
             return True
 
         celebrities = getUtility(ILaunchpadCelebrities)
 
=== modified file 'lib/lp/archiveuploader/nascentupload.py'
--- lib/lp/archiveuploader/nascentupload.py	2010-02-26 16:52:46 +0000
+++ lib/lp/archiveuploader/nascentupload.py	2010-05-11 14:12:39 +0000
@@ -28,7 +28,6 @@
 from lp.archiveuploader.nascentuploadfile import (
     UploadError, UploadWarning, CustomUploadFile, SourceUploadFile,
     BaseBinaryUploadFile)
-from lp.archiveuploader.permission import check_upload_to_archive
 from lp.archiveuploader.utils import determine_source_file_type
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.sourcepackage import SourcePackageFileType
@@ -489,8 +488,8 @@
         source_name = getUtility(
             ISourcePackageNameSet).queryByName(self.changes.dsc.package)
 
-        rejection_reason = check_upload_to_archive(
-            uploader, self.policy.distroseries, source_name, archive,
+        rejection_reason = archive.checkUpload(
+            uploader, self.policy.distroseries, source_name, 
             self.changes.dsc.component, self.policy.pocket, not self.is_new)
 
         if rejection_reason is not None:
 
=== removed file 'lib/lp/archiveuploader/permission.py'
--- lib/lp/archiveuploader/permission.py	2010-02-02 14:01:02 +0000
+++ lib/lp/archiveuploader/permission.py	1970-01-01 00:00:00 +0000
@@ -1,246 +0,0 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Permissions for uploading a package to an archive."""
-
-__metaclass__ = type
-__all__ = [
-    'CannotUploadToArchive',
-    'CannotUploadToPPA',
-    'can_upload_to_archive',
-    'check_upload_to_archive',
-    'check_upload_to_pocket',
-    'components_valid_for',
-    'verify_upload',
-    ]
-
-from zope.component import getUtility
-
-from lp.registry.interfaces.pocket import PackagePublishingPocket
-from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
-from lp.soyuz.interfaces.archive import ArchivePurpose
-
-
-class CannotUploadToArchive(Exception):
-    """A reason for not being able to upload to an archive."""
-
-    _fmt = '%(person)s has no upload rights to %(archive)s.'
-
-    def __init__(self, **args):
-        """Construct a `CannotUploadToArchive`."""
-        Exception.__init__(self, self._fmt % args)
-
-
-class CannotUploadToPocket(Exception):
-    """Returned when a pocket is closed for uploads."""
-
-    def __init__(self, distroseries, pocket):
-        Exception.__init__(self,
-            "Not permitted to upload to the %s pocket in a series in the "
-            "'%s' state." % (pocket.name, distroseries.status.name))
-
-
-class CannotUploadToPPA(CannotUploadToArchive):
-    """Raised when a person cannot upload to a PPA."""
-
-    _fmt = 'Signer has no upload rights to this PPA.'
-
-
-class NoRightsForArchive(CannotUploadToArchive):
-    """Raised when a person has absolutely no upload rights to an archive."""
-
-    _fmt = (
-        "The signer of this package has no upload rights to this "
-        "distribution's primary archive.  Did you mean to upload to "
-        "a PPA?")
-
-
-class InsufficientUploadRights(CannotUploadToArchive):
-    """Raised when a person has insufficient upload rights."""
-    _fmt = (
-        "The signer of this package is lacking the upload rights for "
-        "the source package, component or package set in question.")
-
-
-class NoRightsForComponent(CannotUploadToArchive):
-    """Raised when a person tries to upload to a component without permission.
-    """
-
-    _fmt = (
-        "Signer is not permitted to upload to the component '%(component)s'.")
-
-    def __init__(self, component):
-        CannotUploadToArchive.__init__(self, component=component.name)
-
-
-class InvalidPocketForPPA(CannotUploadToArchive):
-    """PPAs only support some pockets."""
-
-    _fmt = "PPA uploads must be for the RELEASE pocket."
-
-
-class InvalidPocketForPartnerArchive(CannotUploadToArchive):
-    """Partner archives only support some pockets."""
-
-    _fmt = "Partner uploads must be for the RELEASE or PROPOSED pocket."
-
-
-class ArchiveDisabled(CannotUploadToArchive):
-    """Uploading to a disabled archive is not allowed."""
-
-    _fmt = ("%(archive_name)s is disabled.")
-
-    def __init__(self, archive_name):
-        CannotUploadToArchive.__init__(self, archive_name=archive_name)
-
-
-def components_valid_for(archive, person):
-    """Return the components that 'person' can upload to 'archive'.
-
-    :param archive: The `IArchive` than 'person' wishes to upload to.
-    :param person: An `IPerson` wishing to upload to an archive.
-    :return: A `set` of `IComponent`s that 'person' can upload to.
-    """
-    permission_set = getUtility(IArchivePermissionSet)
-    permissions = permission_set.componentsForUploader(archive, person)
-    return set(permission.component for permission in permissions)
-
-
-def can_upload_to_archive(person, suitesourcepackage, archive=None):
-    """Check if 'person' upload 'suitesourcepackage' to 'archive'.
-
-    :param person: An `IPerson` who might be uploading.
-    :param suitesourcepackage: An `ISuiteSourcePackage` to be uploaded.
-    :param archive: The `IArchive` to upload to. If not provided, defaults
-        to the default archive for the source package. (See
-        `ISourcePackage.get_default_archive`).
-    :return: True if they can, False if they cannot.
-    """
-    sourcepackage = suitesourcepackage.sourcepackage
-    if archive is None:
-        archive = sourcepackage.get_default_archive()
-    pocket = suitesourcepackage.pocket
-    distroseries = sourcepackage.distroseries
-    sourcepackagename = sourcepackage.sourcepackagename
-    component = sourcepackage.latest_published_component
-    # strict_component is True because the source package already exists
-    # (otherwise we couldn't have a suitesourcepackage object) and
-    # nascentupload passes True as a matter of policy when the package exists.
-    reason = check_upload_to_archive(
-        person, distroseries, sourcepackagename, archive, component, pocket,
-        strict_component=True)
-    return reason is None
-
-
-def check_upload_to_pocket(archive, distroseries, pocket):
-    """Check if uploading to a particular pocket in an archive is possible.
-
-    :param archive: A `IArchive`
-    :param distroseries: A `IDistroSeries`
-    :param pocket: A `PackagePublishingPocket`
-    """
-    if archive.purpose == ArchivePurpose.PARTNER:
-        if pocket not in (
-            PackagePublishingPocket.RELEASE,
-            PackagePublishingPocket.PROPOSED):
-            return InvalidPocketForPartnerArchive()
-    elif archive.is_ppa:
-        if pocket != PackagePublishingPocket.RELEASE:
-            return InvalidPocketForPPA()
-    else:
-        # Uploads to the partner archive are allowed in any distroseries
-        # state.
-        # XXX julian 2005-05-29 bug=117557:
-        # This is a greasy hack until bug #117557 is fixed.
-        if not distroseries.canUploadToPocket(pocket):
-            return CannotUploadToPocket(distroseries, pocket)
-
-
-def check_upload_to_archive(person, distroseries, sourcepackagename, archive,
-                            component, pocket, strict_component=True):
-    """Check if 'person' upload 'suitesourcepackage' to 'archive'.
-
-    :param person: An `IPerson` who might be uploading.
-    :param distroseries: The `IDistroSeries` being uploaded to.
-    :param sourcepackagename: The `ISourcePackageName` being uploaded.
-    :param archive: The `IArchive` to upload to. If not provided, defaults
-        to the default archive for the source package. (See
-        `ISourcePackage.get_default_archive`).
-    :param component: The `Component` being uploaded to.
-    :param pocket: The `PackagePublishingPocket` of 'distroseries' being
-        uploaded to.
-    :return: The reason for not being able to upload, None otherwise.
-    """
-    reason = check_upload_to_pocket(archive, distroseries, pocket)
-    if reason is not None:
-        return reason
-    return verify_upload(
-        person, sourcepackagename, archive, component, distroseries,
-        strict_component)
-
-
-def packagesets_valid_for(archive, person):
-    """Return the package sets that 'person' can upload to 'archive'.
-
-    :param archive: The `IArchive` than 'person' wishes to upload to.
-    :param person: An `IPerson` wishing to upload to an archive.
-    :return: A `set` of `IPackageset`s that 'person' can upload to.
-    """
-    permission_set = getUtility(IArchivePermissionSet)
-    permissions = permission_set.packagesetsForUploader(archive, person)
-    return set(permission.packageset for permission in permissions)
-
-
-def verify_upload(person, sourcepackagename, archive, component,
-                  distroseries, strict_component=True):
-    """Can 'person' upload 'sourcepackagename' to 'archive'?
-
-    :param person: The `IPerson` trying to upload to the package. Referred to
-        as 'the signer' in upload code.
-    :param sourcepackagename: The source package being uploaded. None if the
-        package is new.
-    :param archive: The `IArchive` being uploaded to.
-    :param component: The `IComponent` that the source package belongs to.
-    :param distroseries: The upload's target distro series.
-    :param strict_component: True if access to the specific component for the
-        package is needed to upload to it. If False, then access to any
-        package will do.
-    :return: CannotUploadToArchive if 'person' cannot upload to the archive,
-        None otherwise.
-    """
-    if not archive.enabled:
-        return ArchiveDisabled(archive.displayname)
-
-    # For PPAs...
-    if archive.is_ppa:
-        if not archive.canUpload(person):
-            return CannotUploadToPPA()
-        else:
-            return None
-
-    # For any other archive...
-    ap_set = getUtility(IArchivePermissionSet)
-
-    if sourcepackagename is not None:
-        # Check whether user may upload because he holds a permission for
-        #   - the given source package directly
-        #   - a package set in the correct distro series that includes the
-        #     given source package
-        source_allowed = archive.canUpload(person, sourcepackagename)
-        set_allowed = ap_set.isSourceUploadAllowed(
-            archive, sourcepackagename, person, distroseries)
-        if source_allowed or set_allowed:
-            return None
-
-    if not components_valid_for(archive, person):
-        if not packagesets_valid_for(archive, person):
-            return NoRightsForArchive()
-        else:
-            return InsufficientUploadRights()
-
-    if (component is not None
-        and strict_component
-        and not archive.canUpload(person, component)):
-        return NoRightsForComponent(component)
-
-    return None
 
=== modified file 'lib/lp/archiveuploader/tests/nascentupload-packageset.txt'
--- lib/lp/archiveuploader/tests/nascentupload-packageset.txt	2009-10-02 15:00:09 +0000
+++ lib/lp/archiveuploader/tests/nascentupload-packageset.txt	2010-05-11 14:12:39 +0000
@@ -39,8 +39,8 @@
     >>> bar_failed.is_rejected
     True
     >>> print bar_failed.rejection_message
-    The signer of this package has no upload rights to this distribution's
-    primary archive.  Did you mean to upload to a PPA?
+	The signer of this package is lacking the upload rights for the source
+	package, component or package set in question.
 
 
 We can grant selective, package set based upload permissions to the user
 
=== modified file 'lib/lp/archiveuploader/tests/test_permission.py'
--- lib/lp/archiveuploader/tests/test_permission.py	2010-02-08 10:56:18 +0000
+++ lib/lp/archiveuploader/tests/test_permission.py	2010-05-11 14:12:39 +0000
@@ -12,8 +12,6 @@
 
 from canonical.testing import DatabaseFunctionalLayer
 
-from lp.archiveuploader.permission import (
-    check_upload_to_archive, components_valid_for)
 from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.soyuz.interfaces.archive import ArchivePurpose
@@ -29,21 +27,22 @@
         # By default, a person cannot upload to any component of an archive.
         archive = self.factory.makeArchive()
         person = self.factory.makePerson()
-        self.assertEqual(set(), components_valid_for(archive, person))
+        self.assertEqual(set(), 
+            set(archive.getComponentsForUploader(person)))
 
     def test_components_for_person_with_permissions(self):
         # If a person has been explicitly granted upload permissions to a
         # particular component, then those components are included in
-        # components_valid_for.
+        # IArchive.getComponentsForUploader.
         archive = self.factory.makeArchive()
         component = self.factory.makeComponent()
         person = self.factory.makePerson()
         # Only admins or techboard members can add permissions normally. That
         # restriction isn't relevant to this test.
         ap_set = removeSecurityProxy(getUtility(IArchivePermissionSet))
-        ap_set.newComponentUploader(archive, person, component)
-        self.assertEqual(
-            set([component]), components_valid_for(archive, person))
+        ap = ap_set.newComponentUploader(archive, person, component)
+        self.assertEqual(set([ap]),
+            set(archive.getComponentsForUploader(person)))
 
 
 class TestPermission(TestCaseWithFactory):
@@ -67,8 +66,8 @@
         pocket = PackagePublishingPocket.RELEASE
         self.assertIs(
             None,
-            check_upload_to_archive(
-                person, distroseries, spn, archive, component, pocket,
+            archive.checkUpload(
+                person, distroseries, spn, component, pocket,
                 strict_component))
 
     def assertCannotUpload(self, reason, person, spn, archive, component,
@@ -87,8 +86,8 @@
         if distroseries is None:
             distroseries = self.factory.makeDistroSeries()
         pocket = PackagePublishingPocket.RELEASE
-        exception = check_upload_to_archive(
-            person, distroseries, spn, archive, component, pocket)
+        exception = archive.checkUpload(
+            person, distroseries, spn, component, pocket)
         self.assertEqual(reason, str(exception))
 
     def test_random_person_cannot_upload_to_ppa(self):
@@ -137,9 +136,8 @@
         archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
         spn = self.factory.makeSourcePackageName()
         self.assertCannotUpload(
-            ("The signer of this package has no upload rights to this "
-             "distribution's primary archive.  Did you mean to upload to "
-             "a PPA?"),
+            ("The signer of this package is lacking the upload rights for "
+             "the source package, component or package set in question."),
             person, spn, archive, None)
 
     def test_package_specific_rights(self):
 
=== modified file 'lib/lp/bugs/model/bugnomination.py'
--- lib/lp/bugs/model/bugnomination.py	2009-08-26 09:31:57 +0000
+++ lib/lp/bugs/model/bugnomination.py	2010-05-11 14:12:39 +0000
@@ -144,7 +144,7 @@
                     upload_component.component
                     for upload_component in distribution.uploaders)
             for packagename_or_component in bug_packagenames_and_components:
-                if distribution.main_archive.canUpload(
+                if distribution.main_archive.checkArchivePermission(
                     person, packagename_or_component):
                     return True
 
 
=== modified file 'lib/lp/code/model/recipebuilder.py'
--- lib/lp/code/model/recipebuilder.py	2010-04-29 14:54:05 +0000
+++ lib/lp/code/model/recipebuilder.py	2010-05-11 14:12:39 +0000
@@ -15,7 +15,6 @@
 
 from canonical.config import config
 
-from lp.archiveuploader.permission import check_upload_to_pocket
 from lp.buildmaster.interfaces.buildfarmjobbehavior import (
     IBuildFarmJobBehavior)
 from lp.buildmaster.interfaces.builder import CannotBuild
@@ -158,8 +157,8 @@
 
         # This should already have been checked earlier, but just check again
         # here in case of programmer errors.
-        reason = check_upload_to_pocket(
-            build.archive, build.distroseries, build.pocket)
+        reason = build.archive.checkUploadToPocket(
+            build.distroseries, build.pocket)
         assert reason is None, (
                 "%s (%s) can not be built for pocket %s: invalid pocket due "
                 "to the series status of %s." %
 
=== modified file 'lib/lp/code/model/sourcepackagerecipe.py'
--- lib/lp/code/model/sourcepackagerecipe.py	2010-04-30 12:33:16 +0000
+++ lib/lp/code/model/sourcepackagerecipe.py	2010-05-11 14:12:39 +0000
@@ -22,7 +22,6 @@
 from canonical.database.datetimecol import UtcDateTimeCol
 from canonical.launchpad.interfaces.lpstorm import IMasterStore
 
-from lp.archiveuploader.permission import check_upload_to_archive
 from lp.code.interfaces.sourcepackagerecipe import (
     ISourcePackageRecipe, ISourcePackageRecipeSource,
     ISourcePackageRecipeData)
@@ -153,9 +152,9 @@
         if archive.purpose != ArchivePurpose.PPA:
             raise NonPPABuildRequest
         component = getUtility(IComponentSet)["multiverse"]
-        reject_reason = check_upload_to_archive(
-            requester, distroseries, self.sourcepackagename,
-            archive, component, pocket)
+        reject_reason = archive.checkUpload(
+            requester, self.distroseries, self.sourcepackagename,
+            component, pocket)
         if reject_reason is not None:
             raise reject_reason
 
 
=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipe.py	2010-04-30 12:33:16 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipe.py	2010-05-11 14:12:39 +0000
@@ -24,8 +24,8 @@
 from canonical.testing.layers import DatabaseFunctionalLayer, AppServerLayer
 
 from canonical.launchpad.webapp.authorization import check_permission
-from lp.archiveuploader.permission import (
-    ArchiveDisabled, CannotUploadToArchive, InvalidPocketForPPA)
+from lp.soyuz.interfaces.archive import (
+    ArchiveDisabled, ArchivePurpose, CannotUploadToArchive, InvalidPocketForPPA)
 from lp.buildmaster.interfaces.buildqueue import IBuildQueue
 from lp.buildmaster.model.buildqueue import BuildQueue
 from lp.code.interfaces.sourcepackagerecipe import (
@@ -40,7 +40,6 @@
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.services.job.interfaces.job import (
     IJob, JobStatus)
-from lp.soyuz.interfaces.archive import ArchivePurpose
 from lp.testing import (
     ANONYMOUS, launchpadlib_for, login, login_person, person_logged_in,
     TestCaseWithFactory, ws_object)
 
=== modified file 'lib/lp/code/tests/test_branch.py'
--- lib/lp/code/tests/test_branch.py	2010-03-18 17:49:21 +0000
+++ lib/lp/code/tests/test_branch.py	2010-05-11 14:12:39 +0000
@@ -12,7 +12,6 @@
 from canonical.launchpad.webapp.authorization import check_permission
 from canonical.testing import DatabaseFunctionalLayer
 
-from lp.archiveuploader.permission import verify_upload
 from lp.code.enums import (
     BranchSubscriptionDiffSize, BranchSubscriptionNotificationLevel,
     CodeReviewNotificationLevel)
@@ -277,8 +276,8 @@
             distroseries = archive.distribution.currentseries
         self.assertIs(
             None,
-            verify_upload(
-                person, spn, archive, component, distroseries,
+            archive.verifyUpload(
+                person, spn, component, distroseries,
                 strict_component))
 
     def assertCannotUpload(
@@ -295,8 +294,8 @@
         """
         if distroseries is None:
             distroseries = archive.distribution.currentseries
-        exception = verify_upload(
-            person, spn, archive, component, distroseries)
+        exception = archive.verifyUpload(
+            person, spn, component, distroseries)
         self.assertEqual(reason, str(exception))
 
     def test_package_upload_permissions_grant_branch_edit(self):
 
=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2010-04-30 03:10:17 +0000
+++ lib/lp/soyuz/browser/archive.py	2010-05-11 14:12:39 +0000
@@ -1236,7 +1236,8 @@
         # XXX cprov 2009-07-17 bug=385503: copies cannot be properly traced
         # that's why we explicitly don't allow them do be done via the UI
         # in main archives, only PPAs.
-        return self.context.is_ppa and self.context.canUpload(self.user)
+        return (self.context.is_ppa and
+                self.context.checkArchivePermission(self.user))
 
     def createDestinationArchiveField(self):
         """Create the 'destination_archive' field."""
 
=== modified file 'lib/lp/soyuz/doc/archive.txt'
--- lib/lp/soyuz/doc/archive.txt	2010-05-06 10:05:49 +0000
+++ lib/lp/soyuz/doc/archive.txt	2010-05-11 14:12:39 +0000
@@ -276,7 +276,7 @@
 
 Uploads to copy archives are not allowed.
 
-    >>> rebuild_archive.canUpload(cprov)
+    >>> rebuild_archive.checkArchivePermission(cprov)
     Traceback (most recent call last):
     ...
     AssertionError: Uploads to copy archives are not allowed.
@@ -2106,7 +2106,7 @@
 
 == Archive Permission Checking ==
 
-IArchive has two public methods, canUpload() and canAdministerQueue()
+IArchive has two public methods, checkArchivePermission() and canAdministerQueue()
 that check a user's permission to upload and/or administer a
 distroseries upload queue respectively.  See archivepermission.txt
 for more details.
@@ -2114,40 +2114,40 @@
     >>> ubuntu_team = getUtility(IPersonSet).getByName('ubuntu-team')
     >>> carlos = getUtility(IPersonSet).getByName('carlos')
 
-    >>> ubuntu.main_archive.canUpload(carlos, main_component)
+    >>> ubuntu.main_archive.checkArchivePermission(carlos, main_component)
     False
 
     >>> ubuntu.main_archive.canAdministerQueue(carlos, main_component)
     False
 
-    >>> ubuntu.main_archive.canUpload(ubuntu_team, main_component)
+    >>> ubuntu.main_archive.checkArchivePermission(ubuntu_team, main_component)
     True
 
     >>> ubuntu.main_archive.canAdministerQueue(ubuntu_team, main_component)
     True
 
-canUpload() can also check someone's permission to upload a specific
+checkArchivePermission() can also check someone's permission to upload a specific
 source package.  Carlos, who does not have permission to upload to any
 Ubuntu components, has permission to upload "mozilla-firefox".
 
     >>> from canonical.launchpad.interfaces import ISourcePackageNameSet
     >>> mozilla = getUtility(
     ...     ISourcePackageNameSet).queryByName("mozilla-firefox")
-    >>> ubuntu.main_archive.canUpload(carlos, mozilla)
+    >>> ubuntu.main_archive.checkArchivePermission(carlos, mozilla)
     True
 
 Cprov does not have permission, however.
 
-    >>> ubuntu.main_archive.canUpload(cprov, mozilla)
+    >>> ubuntu.main_archive.checkArchivePermission(cprov, mozilla)
     False
 
-canUpload() also works in the same way for PPAs.  By default, it allows
+checkArchivePermission() also works in the same way for PPAs.  By default, it allows
 anyone in the PPA owning team to upload.
 
-    >>> cprov_archive.canUpload(cprov)
+    >>> cprov_archive.checkArchivePermission(cprov)
     True
 
-    >>> cprov_archive.canUpload(carlos)
+    >>> cprov_archive.checkArchivePermission(carlos)
     False
 
 We can also create an ArchivePermission entry for carlos to be able to upload
@@ -2159,7 +2159,7 @@
 
 Carlos can now upload to Joe's PPA:
 
-    >>> joes_ppa.canUpload(carlos)
+    >>> joes_ppa.checkArchivePermission(carlos)
     True
 
 Note that when creating a new permission, trying to specify a component other
 
=== modified file 'lib/lp/soyuz/doc/archivepermission.txt'
--- lib/lp/soyuz/doc/archivepermission.txt	2010-03-08 17:06:41 +0000
+++ lib/lp/soyuz/doc/archivepermission.txt	2010-05-11 14:12:39 +0000
@@ -108,7 +108,7 @@
     ...     ubuntu)
     Traceback (most recent call last):
     ...
-    AssertionError: 'item' is not an IComponent, IPackageset or an
+    AssertionError: 'item' ... is not an IComponent, IPackageset or an
     ISourcePackageName
 
 IArchivePermissionSet also has some helpers to make it very easy to
 
=== modified file 'lib/lp/soyuz/doc/nascentupload.txt'
--- lib/lp/soyuz/doc/nascentupload.txt	2010-03-11 16:09:29 +0000
+++ lib/lp/soyuz/doc/nascentupload.txt	2010-05-11 14:12:39 +0000
@@ -879,9 +879,8 @@
   >>> bar_failed.is_rejected
   True
   >>> print bar_failed.rejection_message
-  The signer of this package has no upload rights to this distribution's
-  primary archive.  Did you mean to upload to a PPA?
-
+  The signer of this package is lacking the upload rights for the source
+  package, component or package set in question.
 
 Even in a rejected upload using 'insecure' policy, the DSC signing key
 and the changesfile sigining key are stored in NascentUpload instance
 
=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2010-03-26 17:30:33 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2010-05-11 14:12:39 +0000
@@ -11,12 +11,15 @@
     'ALLOW_RELEASE_BUILDS',
     'AlreadySubscribed',
     'ArchiveDependencyError',
+    'ArchiveDisabled',
     'ArchiveNotPrivate',
     'ArchivePurpose',
     'ArchiveStatus',
     'CannotCopy',
     'CannotSwitchPrivacy',
     'ComponentNotFound',
+    'CannotUploadToPPA',
+    'CannotUploadToPocket',
     'DistroSeriesNotFound',
     'IArchive',
     'IArchiveAppend',
@@ -26,10 +29,15 @@
     'IArchivePublic',
     'IArchiveSet',
     'IDistributionArchive',
+    'InsufficientUploadRights',
     'InvalidComponent',
+    'InvalidPocketForPartnerArchive',
+    'InvalidPocketForPPA',
     'IPPA',
     'IPPAActivateForm',
     'MAIN_ARCHIVE_PURPOSES',
+    'NoRightsForArchive',
+    'NoRightsForComponent',
     'NoSuchPPA',
     'NoTokensForTeams',
     'PocketNotFound',
@@ -133,6 +141,79 @@
     webservice_error(400) # Bad request.
 
 
+class CannotUploadToArchive(Exception):
+    """A reason for not being able to upload to an archive."""
+
+    _fmt = '%(person)s has no upload rights to %(archive)s.'
+
+    def __init__(self, **args):
+        """Construct a `CannotUploadToArchive`."""
+        Exception.__init__(self, self._fmt % args)
+
+
+class InvalidPocketForPartnerArchive(CannotUploadToArchive):
+    """Partner archives only support some pockets."""
+
+    _fmt = "Partner uploads must be for the RELEASE or PROPOSED pocket."
+
+
+class CannotUploadToPocket(Exception):
+    """Returned when a pocket is closed for uploads."""
+
+    def __init__(self, distroseries, pocket):
+        Exception.__init__(self, 
+            "Not permitted to upload to the %s pocket in a series in the "
+            "'%s' state." % (pocket.name, distroseries.status.name))
+
+
+class CannotUploadToPPA(CannotUploadToArchive):
+    """Raised when a person cannot upload to a PPA."""
+
+    _fmt = 'Signer has no upload rights to this PPA.'
+
+
+class NoRightsForArchive(CannotUploadToArchive):
+    """Raised when a person has absolutely no upload rights to an archive."""
+
+    _fmt = (
+        "The signer of this package has no upload rights to this "
+        "distribution's primary archive.  Did you mean to upload to "
+        "a PPA?")
+
+
+class InsufficientUploadRights(CannotUploadToArchive):
+    """Raised when a person has insufficient upload rights."""
+    _fmt = (
+        "The signer of this package is lacking the upload rights for "
+        "the source package, component or package set in question.")
+
+
+class NoRightsForComponent(CannotUploadToArchive):
+    """Raised when a person tries to upload to a component without permission.
+    """
+
+    _fmt = (
+        "Signer is not permitted to upload to the component '%(component)s'.")
+
+    def __init__(self, component):
+        CannotUploadToArchive.__init__(self, component=component.name)
+
+
+class InvalidPocketForPPA(CannotUploadToArchive):
+    """PPAs only support some pockets."""
+
+    _fmt = "PPA uploads must be for the RELEASE pocket."
+
+
+class ArchiveDisabled(CannotUploadToArchive):
+    """Uploading to a disabled archive is not allowed."""
+
+    _fmt = ("%(archive_name)s is disabled.")
+
+    def __init__(self, archive_name):
+        CannotUploadToArchive.__init__(self, archive_name=archive_name)
+
+
 class IArchivePublic(IHasOwner, IPrivacy):
     """An Archive interface for publicly available operations."""
     id = Attribute("The archive ID.")
@@ -380,7 +461,7 @@
         :return: A list of `IArchivePermission` records.
         """
 
-    def canUpload(person, component_or_package=None):
+    def checkArchivePermission(person, component_or_package=None):
         """Check to see if person is allowed to upload to component.
 
         :param person: An `IPerson` whom should be checked for authentication.
@@ -395,6 +476,53 @@
 
         """
 
+    def canUploadSuiteSourcePackage(person, suitesourcepackage):
+        """Check if 'person' upload 'suitesourcepackage' to 'archive'.
+
+        :param person: An `IPerson` who might be uploading.
+        :param suitesourcepackage: An `ISuiteSourcePackage` to be uploaded.
+        :return: True if they can, False if they cannot.
+        """
+
+    def checkUploadToPocket(distroseries, pocket):
+        """Check if uploading to a particular pocket in an archive is possible.
+
+        :param distroseries: A `IDistroSeries`
+        :param pocket: A `PackagePublishingPocket`
+        :return: Reason why uploading is not possible or None
+        """
+
+    def checkUpload(person, distroseries, sourcepackagename, component, 
+                    pocket, strict_component=True):
+        """Check if 'person' upload 'suitesourcepackage' to 'archive'.
+
+        :param person: An `IPerson` who might be uploading.
+        :param distroseries: The `IDistroSeries` being uploaded to.
+        :param sourcepackagename: The `ISourcePackageName` being uploaded.
+        :param component: The `Component` being uploaded to.
+        :param pocket: The `PackagePublishingPocket` of 'distroseries' being
+            uploaded to.
+        :return: The reason for not being able to upload, None otherwise.
+        """
+
+    def verifyUpload(person, sourcepackagename, component,
+                      distroseries, strict_component=True):
+        """Can 'person' upload 'sourcepackagename' to this archive ?
+
+        :param person: The `IPerson` trying to upload to the package. Referred to
+            as 'the signer' in upload code.
+        :param sourcepackagename: The source package being uploaded. None if the
+            package is new.
+        :param archive: The `IArchive` being uploaded to.
+        :param component: The `IComponent` that the source package belongs to.
+        :param distroseries: The upload's target distro series.
+        :param strict_component: True if access to the specific component for the
+            package is needed to upload to it. If False, then access to any
+            package will do.
+        :return: CannotUploadToArchive if 'person' cannot upload to the archive,
+            None otherwise.
+        """
+
     def canAdministerQueue(person, component):
         """Check to see if person is allowed to administer queue items.
 
@@ -564,6 +692,13 @@
             'person' is allowed to upload to.
         """
 
+    def getComponentsForUploader(person):
+        """Return the components that 'person' can upload to this archive.
+
+        :param person: An `IPerson` wishing to upload to an archive.
+        :return: A `set` of `IComponent`s that 'person' can upload to.
+        """
+
     @operation_parameters(
         sourcepackagename=TextLine(
             title=_("Source package name"), required=True),
 
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2010-05-06 10:05:49 +0000
+++ lib/lp/soyuz/model/archive.py	2010-05-11 14:12:39 +0000
@@ -58,11 +58,14 @@
 from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
 from lp.registry.model.teammembership import TeamParticipation
 from lp.soyuz.interfaces.archive import (
-    AlreadySubscribed, ArchiveDependencyError, ArchiveNotPrivate,
-    ArchivePurpose, ArchiveStatus, CannotCopy, CannotSwitchPrivacy,
+    AlreadySubscribed, ArchiveDependencyError, ArchiveDisabled,
+    ArchiveNotPrivate, ArchivePurpose, ArchiveStatus, CannotCopy,
+    CannotSwitchPrivacy, CannotUploadToPPA, CannotUploadToPocket,
     DistroSeriesNotFound, IArchive, IArchiveSet, IDistributionArchive,
-    InvalidComponent, IPPA, MAIN_ARCHIVE_PURPOSES, NoSuchPPA,
-    NoTokensForTeams, PocketNotFound, VersionRequiresName,
+    InsufficientUploadRights, InvalidPocketForPPA,
+    InvalidPocketForPartnerArchive, InvalidComponent, IPPA,
+    MAIN_ARCHIVE_PURPOSES, NoRightsForArchive, NoRightsForComponent,
+    NoSuchPPA, NoTokensForTeams, PocketNotFound, VersionRequiresName,
     default_name_by_purpose)
 from lp.soyuz.interfaces.archiveauthtoken import IArchiveAuthTokenSet
 from lp.soyuz.interfaces.archivearch import IArchiveArchSet
@@ -954,7 +957,7 @@
             source_ids,
             archive=self)
 
-    def canUpload(self, user, component_or_package=None):
+    def checkArchivePermission(self, user, component_or_package=None):
         """See `IArchive`."""
         assert not self.is_copy, "Uploads to copy archives are not allowed."
         # PPA access is immediately granted if the user is in the PPA
@@ -977,6 +980,87 @@
         return self._authenticate(
             user, component_or_package, ArchivePermissionType.UPLOAD)
 
+    def canUploadSuiteSourcePackage(self, person, suitesourcepackage):
+        """See `IArchive`."""
+        sourcepackage = suitesourcepackage.sourcepackage
+        pocket = suitesourcepackage.pocket
+        distroseries = sourcepackage.distroseries
+        sourcepackagename = sourcepackage.sourcepackagename
+        component = sourcepackage.latest_published_component
+        # strict_component is True because the source package already exists
+        # (otherwise we couldn't have a suitesourcepackage object) and
+        # nascentupload passes True as a matter of policy when the package exists.
+        reason = self.checkUpload(
+            person, distroseries, sourcepackagename, component, pocket,
+            strict_component=True)
+        return reason is None
+
+    def checkUploadToPocket(self, distroseries, pocket):
+        """See `IArchive`."""
+        if self.purpose == ArchivePurpose.PARTNER:
+            if pocket not in (
+                PackagePublishingPocket.RELEASE,
+                PackagePublishingPocket.PROPOSED):
+                return InvalidPocketForPartnerArchive()
+        elif self.is_ppa:
+            if pocket != PackagePublishingPocket.RELEASE:
+                return InvalidPocketForPPA()
+        else:
+            # Uploads to the partner archive are allowed in any distroseries
+            # state.
+            # XXX julian 2005-05-29 bug=117557:
+            # This is a greasy hack until bug #117557 is fixed.
+            if not distroseries.canUploadToPocket(pocket):
+                return CannotUploadToPocket(distroseries, pocket)
+
+    def checkUpload(self, person, distroseries, sourcepackagename, component, 
+                    pocket, strict_component=True):
+        """See `IArchive`."""
+        reason = self.checkUploadToPocket(distroseries, pocket)
+        if reason is not None:
+            return reason
+        return self.verifyUpload(
+            person, sourcepackagename, component, distroseries,
+            strict_component)
+
+    def verifyUpload(self, person, sourcepackagename, component,
+                      distroseries, strict_component=True):
+        """See `IArchive`."""
+        if not self.enabled:
+            return ArchiveDisabled(self.displayname)
+
+        # For PPAs...
+        if self.is_ppa:
+            if not self.checkArchivePermission(person):
+                return CannotUploadToPPA()
+            else:
+                return None
+
+        if sourcepackagename is not None:
+            # Check whether user may upload because they hold a permission for
+            #   - the given source package directly
+            #   - a package set in the correct distro series that includes the
+            #     given source package
+            source_allowed = self.checkArchivePermission(person,
+                                                         sourcepackagename)
+            set_allowed = self.isSourceUploadAllowed(
+                sourcepackagename, person, distroseries)
+            if source_allowed or set_allowed:
+                return None
+
+        if not self.getComponentsForUploader(person):
+            if not self.getPackagesetsForUploader(person):
+                return NoRightsForArchive()
+            else:
+                return InsufficientUploadRights()
+
+        if (component is not None
+            and strict_component
+            and not self.checkArchivePermission(person, component)):
+            return NoRightsForComponent(component)
+
+        return None
+
     def canAdministerQueue(self, user, component):
         """See `IArchive`."""
         return self._authenticate(
@@ -1050,6 +1134,11 @@
         return permission_set.deletePackagesetUploader(
             self, person, packageset, explicit)
 
+    def getComponentsForUploader(self, person):
+        """See `IArchive`."""
+        permission_set = getUtility(IArchivePermissionSet)
+        return permission_set.componentsForUploader(self, person)
+
     def getPackagesetsForUploader(self, person):
         """See `IArchive`."""
         permission_set = getUtility(IArchivePermissionSet)
 
=== modified file 'lib/lp/soyuz/model/archivepermission.py'
--- lib/lp/soyuz/model/archivepermission.py	2010-03-08 17:06:41 +0000
+++ lib/lp/soyuz/model/archivepermission.py	2010-05-11 14:12:39 +0000
@@ -151,8 +151,8 @@
             prejoins.append("packageset")
         else:
             raise AssertionError(
-                "'item' is not an IComponent, IPackageset or an "
-                "ISourcePackageName")
+                "'item' %r is not an IComponent, IPackageset or an "
+                "ISourcePackageName" % item)
 
         query = " AND ".join(clauses)
         auth = ArchivePermission.select(
 
=== modified file 'lib/lp/soyuz/model/binarypackagebuildbehavior.py'
--- lib/lp/soyuz/model/binarypackagebuildbehavior.py	2010-04-12 05:52:01 +0000
+++ lib/lp/soyuz/model/binarypackagebuildbehavior.py	2010-05-11 14:12:39 +0000
@@ -15,7 +15,6 @@
 
 from canonical.launchpad.webapp import urlappend
 
-from lp.archiveuploader.permission import check_upload_to_pocket
 from lp.buildmaster.interfaces.buildfarmjobbehavior import (
     IBuildFarmJobBehavior)
 from lp.buildmaster.model.buildfarmjobbehavior import (
@@ -123,8 +122,8 @@
 
         # This should already have been checked earlier, but just check again 
         # here in case of programmer errors.
-        reason = check_upload_to_pocket(
-            build.archive, build.distroseries, build.pocket)
+        reason = build.archive.checkUploadToPocket(build.distroseries,
+            build.pocket)
         assert reason is None, (
                 "%s (%s) can not be built for pocket %s: invalid pocket due "
                 "to the series status of %s." %
 
=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2010-05-03 07:48:54 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2010-05-11 14:12:39 +0000
@@ -18,13 +18,16 @@
 from lp.buildmaster.interfaces.buildbase import BuildStatus
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.person import IPersonSet
+from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.services.job.interfaces.job import JobStatus
+from lp.soyuz.interfaces.archive import (IArchiveSet, ArchivePurpose,
+    ArchiveStatus, CannotSwitchPrivacy, InvalidPocketForPartnerArchive,
+    InvalidPocketForPPA)
 from lp.services.worlddata.interfaces.country import ICountrySet
-from lp.soyuz.interfaces.archive import (
-    IArchiveSet, ArchivePurpose, ArchiveStatus, CannotSwitchPrivacy)
 from lp.soyuz.interfaces.archivearch import IArchiveArchSet
 from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
 from lp.soyuz.interfaces.binarypackagerelease import BinaryPackageFormat
+from lp.soyuz.interfaces.component import IComponentSet
 from lp.soyuz.interfaces.processor import IProcessorFamilySet
 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
 from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild
@@ -345,6 +348,7 @@
         self.failUnlessEqual(
             self.sourcepackagereleases[0], result[0])
 
+
 class TestCorrespondingDebugArchive(TestCaseWithFactory):
 
     layer = LaunchpadZopelessLayer
@@ -520,6 +524,7 @@
         self.archive.disable()
         self.assertRaises(AssertionError, self.archive.disable)
 
+
 class TestCollectLatestPublishedSources(TestCaseWithFactory):
     """Ensure that the private helper method works as expected."""
 
@@ -566,6 +571,70 @@
         self.assertEqual('0.5.11~ppa1', pubs[0].source_package_version)
 
 
+class TestArchiveCanUpload(TestCaseWithFactory):
+    """Test the various methods that verify whether uploads are allowed to 
+    happen."""
+
+    layer = LaunchpadZopelessLayer
+
+    def test_checkArchivePermission_by_PPA_owner(self):
+        # Uploading to a PPA should be allowed for a user that is the owner 
+        owner = self.factory.makePerson(name="somebody")
+        archive = self.factory.makeArchive(owner=owner)
+        self.assertEquals(True, archive.checkArchivePermission(owner))
+        someone_unrelated = self.factory.makePerson(name="somebody-unrelated")
+        self.assertEquals(False,
+            archive.checkArchivePermission(someone_unrelated))
+
+    def test_checkArchivePermission_distro_archive(self):
+        # Regular users can not upload to ubuntu
+        ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY, 
+                                           distribution=ubuntu)
+        main = getUtility(IComponentSet)["main"]
+        # A regular user doesn't have access
+        somebody = self.factory.makePerson(name="somebody")
+        self.assertEquals(False, 
+            archive.checkArchivePermission(somebody, main))
+        # An ubuntu core developer does have access
+        kamion = getUtility(IPersonSet).getByName('kamion')
+        self.assertEquals(True, archive.checkArchivePermission(kamion, main))
+
+    def test_checkArchivePermission_ppa(self):
+        ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
+        owner = self.factory.makePerson(name="eigenaar")
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA, 
+                                           distribution=ubuntu,
+                                           owner=owner)
+        somebody = self.factory.makePerson(name="somebody")
+        # The owner has access
+        self.assertEquals(True, archive.checkArchivePermission(owner))
+        # Somebody unrelated does not
+        self.assertEquals(False, archive.checkArchivePermission(somebody))
+
+    def test_checkUpload_partner_invalid_pocket(self):
+        # Partner archives only have release and proposed pockets
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.PARTNER)
+        self.assertIsInstance(archive.checkUpload(self.factory.makePerson(), 
+                                self.factory.makeDistroSeries(),
+                                self.factory.makeSourcePackageName(),
+                                self.factory.makeComponent(),
+                                PackagePublishingPocket.UPDATES),
+                                InvalidPocketForPartnerArchive)
+ 
+    def test_checkUpload_ppa_invalid_pocket(self):
+        # PPA archives only have release pockets
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+        self.assertIsInstance(archive.checkUpload(self.factory.makePerson(), 
+                                self.factory.makeDistroSeries(),
+                                self.factory.makeSourcePackageName(),
+                                self.factory.makeComponent(),
+                                PackagePublishingPocket.PROPOSED),
+                                InvalidPocketForPPA)
+
+    # XXX: JRV 20100511: IArchive.canUploadSuiteSourcePackage needs tests
+
+
 class TestUpdatePackageDownloadCount(TestCaseWithFactory):
     """Ensure that updatePackageDownloadCount works as expected."""