launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/launchpad/policy-upload-checker-bug-777767 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #777767 in Launchpad itself: "Write a generic upload policy checker"
  https://bugs.launchpad.net/launchpad/+bug/777767

For more details, see:
https://code.launchpad.net/~julian-edwards/launchpad/policy-upload-checker-bug-777767/+merge/60174

= Summary =
The beginnings of a generic package copying and upload checker.

== Proposed fix ==
There are 2 main ways to get a new package into Launchpad:
1. Upload it, where it's dealt with by the upload processor.
2. Copy it from another archive, where it's dealt with by the package copier.

These two areas of code implement almost identical checks but in entirely different ways.  This branch is the start of the unification by re-implementing the InsecureUploadPolicy defined in archiveuploader/uploadpolicy.py which will check whether a package should be auto-approved or held in a queue.

Initially just the copy checker will use this code but in the future the upload processor will use it too.

== Pre-implementation notes ==
Discussed with wgrant and we're eventually looking to make uploads just work
like a copy from nowhere inside LP.

The end intention is to make policies that pull information from the database
so that each distro in LP can set its own policies.  Since we only have Ubuntu thus far, the decisions are hard-coded for now.

== Implementation details ==
I've added a BaseCopyPolicy that is an abstract base class.  The only sub-class to use it is the InsecureCopyPolicy which represents a package from an insecure source.

== Tests ==
bin/test -cvv test_copypolicy

== Demo and Q/A ==
n/a


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/soyuz/tests/test_copypolicy.py
  lib/lp/soyuz/adapters/copypolicy.py
-- 
https://code.launchpad.net/~julian-edwards/launchpad/policy-upload-checker-bug-777767/+merge/60174
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/launchpad/policy-upload-checker-bug-777767 into lp:launchpad.

=== added file 'lib/lp/soyuz/adapters/copypolicy.py'
--- lib/lp/soyuz/adapters/copypolicy.py	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/adapters/copypolicy.py	2011-05-06 12:18:57 +0000
@@ -0,0 +1,55 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Copy Policy Classes.
+
+The classes contain various policies about copying packages that can be
+decided at runtime, such as whether to auto-accept a package or not.
+"""
+
+__metaclass__ = type
+
+__all__ = [
+    "InsecureCopyPolicy",
+    ]
+
+
+from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.registry.interfaces.series import SeriesStatus
+
+
+class BaseCopyPolicy:
+    """Encapsulation of the policies for copying a package in Launchpad."""
+
+    def autoApprove(self, packageupload):
+        """Decide if the packageupload can be approved automatically or
+        should be held in the queue.
+        """
+        raise AssertionError("Subclass must provide autoApprove")
+
+    def autoApproveNew(self, packageupload):
+        """Decide if a previously unknown package is approved automatically
+        or should be held in the queue.
+        """
+        raise AssertionError("Subclass must provide autoApproveNew")
+
+
+class InsecureCopyPolicy(BaseCopyPolicy):
+    """A policy for copying from insecure sources."""
+
+    def autoApproveNew(self, packageupload):
+        if packageupload.isPPA():
+            return True
+        return False
+
+    def autoApprove(self, packageupload):
+        if packageupload.isPPA():
+            return True
+
+        # This check is orthogonal to the
+        # IDistroSeries.canUploadToPocket check.
+        if (packageupload.pocket == PackagePublishingPocket.RELEASE and
+            packageupload.distroseries.status != SeriesStatus.FROZEN):
+            return True
+
+        return False

=== added file 'lib/lp/soyuz/tests/test_copypolicy.py'
--- lib/lp/soyuz/tests/test_copypolicy.py	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/tests/test_copypolicy.py	2011-05-06 12:18:57 +0000
@@ -0,0 +1,56 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from canonical.testing.layers import LaunchpadZopelessLayer
+from lp.testing import TestCaseWithFactory
+
+from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.registry.interfaces.series import SeriesStatus
+from lp.soyuz.adapters.copypolicy import InsecureCopyPolicy
+from lp.soyuz.enums import ArchivePurpose
+
+
+class TestCopyPolicy(TestCaseWithFactory):
+
+    # makePackageUpload() needs the librarian.
+    layer = LaunchpadZopelessLayer
+
+    def _getPackageUpload(self, archive_purpose):
+        archive = self.factory.makeArchive(purpose=archive_purpose)
+        packageupload = self.factory.makePackageUpload(archive=archive)
+        return packageupload
+
+    def assertApproved(self, archive_purpose, method):
+        packageupload = self._getPackageUpload(archive_purpose)
+        approved = method(packageupload)
+        self.assertTrue(approved)
+
+    def assertUnapproved(self, archive_purpose, method):
+        packageupload = self._getPackageUpload(archive_purpose)
+        approved = method(packageupload)
+        self.assertFalse(approved)
+
+    def test_insecure_holds_new_distro_package(self):
+        cp = InsecureCopyPolicy()
+        self.assertUnapproved(ArchivePurpose.PRIMARY, cp.autoApproveNew)
+
+    def test_insecure_approves_new_ppa_packages(self):
+        cp = InsecureCopyPolicy()
+        self.assertApproved(ArchivePurpose.PPA, cp.autoApproveNew)
+
+    def test_insecure_approves_existing_distro_package(self):
+        cp = InsecureCopyPolicy()
+        self.assertApproved(ArchivePurpose.PRIMARY, cp.autoApprove)
+
+    def test_insecure_holds_copy_to_release_pocket_in_frozen_series(self):
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
+        packageupload = self.factory.makePackageUpload(
+            archive=archive, pocket=PackagePublishingPocket.RELEASE)
+        packageupload.distroseries.status = SeriesStatus.FROZEN
+        cp = InsecureCopyPolicy()
+        approve = cp.autoApprove(packageupload)
+        self.assertFalse(approve)
+
+    def test_insecure_approves_existing_ppa_package(self):
+        cp = InsecureCopyPolicy()
+        self.assertApproved(ArchivePurpose.PPA, cp.autoApprove)