← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~julian-edwards/launchpad/async-copying-part-2 into lp:launchpad

 

Julian Edwards has proposed merging lp:~julian-edwards/launchpad/async-copying-part-2 into lp:launchpad with lp:~julian-edwards/launchpad/async-copying-bug-809805 as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~julian-edwards/launchpad/async-copying-part-2/+merge/67989

This is the second part of my copyPackage(s) change for asynchronous package copying.  This part adds copyPackages() which does multiple packages in one go similar to syncSources().

See the pre-requisite at https://code.launchpad.net/~julian-edwards/launchpad/async-copying-bug-809805/+merge/67941
-- 
https://code.launchpad.net/~julian-edwards/launchpad/async-copying-part-2/+merge/67989
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/launchpad/async-copying-part-2 into lp:launchpad.
=== modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
--- lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2011-07-14 16:44:53 +0000
+++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2011-07-14 16:44:58 +0000
@@ -405,6 +405,8 @@
 patch_plain_parameter_type(IArchive, 'syncSources', 'from_archive', IArchive)
 patch_plain_parameter_type(IArchive, 'syncSource', 'from_archive', IArchive)
 patch_plain_parameter_type(IArchive, 'copyPackage', 'from_archive', IArchive)
+patch_plain_parameter_type(
+    IArchive, 'copyPackages', 'from_archive', IArchive)
 patch_entry_return_type(IArchive, 'newSubscription', IArchiveSubscriber)
 patch_plain_parameter_type(
     IArchive, 'getArchiveDependency', 'dependency', IArchive)

=== modified file 'lib/lp/soyuz/browser/tests/test_archive_webservice.py'
--- lib/lp/soyuz/browser/tests/test_archive_webservice.py	2011-07-14 16:44:53 +0000
+++ lib/lp/soyuz/browser/tests/test_archive_webservice.py	2011-07-14 16:44:58 +0000
@@ -19,7 +19,10 @@
 from canonical.testing.layers import DatabaseFunctionalLayer
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.registry.interfaces.pocket import PackagePublishingPocket
-from lp.soyuz.enums import ArchivePurpose
+from lp.soyuz.enums import (
+    ArchivePurpose,
+    PackagePublishingStatus,
+    )
 from lp.soyuz.interfaces.packagecopyjob import IPlainPackageCopyJobSource
 from lp.soyuz.interfaces.processor import IProcessorFamilySet
 from lp.testing import (
@@ -311,17 +314,16 @@
 
 
 class TestCopyPackage(WebServiceTestCase):
-    """Webservice test cases for the copyPackage method"""
+    """Webservice test cases for the copyPackage/copyPackages methods"""
 
-    def test_copyPackage(self):
-        """Basic smoke test"""
+    def setup_data(self):
         self.ws_version = "devel"
         uploader_dude = self.factory.makePerson()
         source_archive = self.factory.makeArchive()
         target_archive = self.factory.makeArchive(
             purpose=ArchivePurpose.PRIMARY)
         source = self.factory.makeSourcePackagePublishingHistory(
-            archive=source_archive)
+            archive=source_archive, status=PackagePublishingStatus.PUBLISHED)
         source_name = source.source_package_name
         version = source.source_package_version
         to_pocket = PackagePublishingPocket.RELEASE
@@ -330,6 +332,13 @@
         with person_logged_in(target_archive.owner):
             target_archive.newComponentUploader(uploader_dude, "universe")
         transaction.commit()
+        return (source_archive, source_name, target_archive, to_pocket,
+                to_series, uploader_dude, version)
+
+    def test_copyPackage(self):
+        """Basic smoke test"""
+        (source_archive, source_name, target_archive, to_pocket, to_series,
+         uploader_dude, version) = self.setup_data()
 
         ws_target_archive = self.wsObject(target_archive, user=uploader_dude)
         ws_source_archive = self.wsObject(source_archive)
@@ -344,6 +353,23 @@
         copy_job = job_source.getActiveJobs(target_archive).one()
         self.assertEqual(target_archive, copy_job.target_archive)
 
+    def test_copyPackages(self):
+        """Basic smoke test"""
+        (source_archive, source_name, target_archive, to_pocket, to_series,
+         uploader_dude, version) = self.setup_data()
+
+        ws_target_archive = self.wsObject(target_archive, user=uploader_dude)
+        ws_source_archive = self.wsObject(source_archive)
+
+        ws_target_archive.copyPackages(
+            source_names=[source_name], from_archive=ws_source_archive,
+            to_pocket=to_pocket.name, to_series=to_series.name,
+            include_binaries=False)
+        transaction.commit()
+
+        job_source = getUtility(IPlainPackageCopyJobSource)
+        copy_job = job_source.getActiveJobs(target_archive).one()
+        self.assertEqual(target_archive, copy_job.target_archive)
 
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2011-07-14 16:44:53 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2011-07-14 16:44:58 +0000
@@ -1258,6 +1258,51 @@
         :raises CannotCopy: if there is a problem copying.
         """
 
+    @call_with(person=REQUEST_USER)
+    @operation_parameters(
+        source_names=List(
+            title=_("Source package names"),
+            value_type=TextLine()),
+        from_archive=Reference(schema=Interface),
+        #Really IArchive, see below
+        to_pocket=TextLine(title=_("Pocket name")),
+        to_series=TextLine(title=_("Distroseries name"), required=False),
+        include_binaries=Bool(
+            title=_("Include Binaries"),
+            description=_("Whether or not to copy binaries already built for"
+                          " this source"),
+            required=False))
+    @export_write_operation()
+    @operation_for_version('devel')
+    def copyPackages(source_names, from_archive, to_pocket, person,
+                     to_series=None, include_binaries=False):
+        """Atomically copy multiple named sources into this archive from another.
+
+        Asynchronously copy the most recent PUBLISHED versions of the named
+        sources to the destination archive if necessary.  Calls to this
+        method will return immediately if the copy passes basic security
+        checks and the copy will happen sometime later with full checking.
+
+        This operation will only succeed when all requested packages
+        are synchronised between the archives. If any of the requested
+        copies cannot be performed, the whole operation will fail. There
+        will be no partial changes of the destination archive.
+
+        :param source_names: a list of string names of packages to copy.
+        :param from_archive: the source archive from which to copy.
+        :param to_pocket: the target pocket (as a string).
+        :param to_series: the target distroseries (as a string).
+        :param include_binaries: optional boolean, controls whether or not
+            the published binaries for each given source should also be
+            copied along with the source.
+        :param person: the `IPerson` who requests the sync.
+
+        :raises NoSuchSourcePackageName: if the source name is invalid
+        :raises PocketNotFound: if the pocket name is invalid
+        :raises NoSuchDistroSeries: if the distro series name is invalid
+        :raises CannotCopy: if there is a problem copying.
+        """
+
 
 class IArchiveAppend(Interface):
     """Archive interface for operations restricted by append privilege."""

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2011-07-14 16:44:53 +0000
+++ lib/lp/soyuz/model/archive.py	2011-07-14 16:44:58 +0000
@@ -102,6 +102,7 @@
 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
 from lp.registry.model.sourcepackagename import SourcePackageName
 from lp.registry.model.teammembership import TeamParticipation
+from lp.services.database.bulk import load_related
 from lp.services.job.interfaces.job import JobStatus
 from lp.services.propertycache import (
     cachedproperty,
@@ -1550,6 +1551,47 @@
             package_version=version, include_binaries=include_binaries,
             copy_policy=PackageCopyPolicy.INSECURE)
 
+    def copyPackages(self, source_names, from_archive, to_pocket,
+                     person, to_series=None, include_binaries=None):
+        """See `IArchive`."""
+        sources = self._collectLatestPublishedSources(
+            from_archive, source_names)
+        if not sources:
+            raise CannotCopy(
+                "None of the supplied package names are published")
+
+        # Bulk-load the sourcepackagereleases so that the list
+        # comprehension doesn't generate additional queries. The
+        # sourcepackagenames themselves will already have been loaded when
+        # generating the list of source publications in "sources".
+        load_related(
+            SourcePackageRelease, sources, ["sourcepackagereleaseID"])
+        sourcepackagenames = [source.sourcepackagerelease.sourcepackagename
+                              for source in sources]
+
+        # Now do a mass check of permissions.
+        pocket = self._text_to_pocket(to_pocket)
+        series = self._text_to_series(to_series)
+        check_copy_permissions(
+            person, self, series, pocket, sourcepackagenames)
+
+        # If we get this far then we can create the PackageCopyJob.
+        copy_tasks = []
+        for source in sources:
+            task = (
+                source.sourcepackagerelease.sourcepackagename,
+                source.sourcepackagerelease.version,
+                from_archive,
+                self,
+                PackagePublishingPocket.RELEASE
+                )
+            copy_tasks.append(task)
+
+        job_source = getUtility(IPlainPackageCopyJobSource)
+        job_source.createMultiple(
+            series, copy_tasks, copy_policy=PackageCopyPolicy.INSECURE,
+            include_binaries=include_binaries)
+
     def _collectLatestPublishedSources(self, from_archive, source_names):
         """Private helper to collect the latest published sources for an
         archive.
@@ -1557,6 +1599,9 @@
         :raises NoSuchSourcePackageName: If any of the source_names do not
             exist.
         """
+        # XXX bigjools bug=810421
+        # This code is inefficient.  It should try to bulk load all the
+        # sourcepackagenames and publications instead of iterating.
         sources = []
         for name in source_names:
             # Check to see if the source package exists. This will raise

=== modified file 'lib/lp/soyuz/model/packagecopyjob.py'
--- lib/lp/soyuz/model/packagecopyjob.py	2011-07-08 09:06:24 +0000
+++ lib/lp/soyuz/model/packagecopyjob.py	2011-07-14 16:44:58 +0000
@@ -484,7 +484,7 @@
         if pu is not None:
             # A PackageUpload will only exist if the copy job had to be
             # held in the queue because of policy/ancestry checks.  If one
-            # does exist we need to make sure 
+            # does exist we need to make sure it gets moved to DONE.
             pu.setDone()
 
     def abort(self):

=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2011-07-14 16:44:53 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2011-07-14 16:44:58 +0000
@@ -1975,7 +1975,7 @@
         source_archive = self.factory.makeArchive()
         target_archive = self.factory.makeArchive(purpose=target_purpose)
         source = self.factory.makeSourcePackagePublishingHistory(
-            archive=source_archive)
+            archive=source_archive, status=PackagePublishingStatus.PUBLISHED)
         source_name = source.source_package_name
         version = source.source_package_version
         to_pocket = PackagePublishingPocket.RELEASE
@@ -2068,3 +2068,90 @@
             to_pocket.name, to_series=to_series.name, include_binaries=False,
             person=target_archive.owner)
 
+    def test_copyPackages_with_single_package(self):
+        (source, source_archive, source_name, target_archive, to_pocket,
+         to_series, version) = self._setup_copy_data()
+
+        with person_logged_in(target_archive.owner):
+            target_archive.copyPackages(
+                [source_name], source_archive, to_pocket.name,
+                to_series=to_series.name, include_binaries=False,
+                person=target_archive.owner)
+
+        # The source should not be published yet in the target_archive.
+        published = target_archive.getPublishedSources(
+            name=source.source_package_name).any()
+        self.assertIs(None, published)
+
+        # There should be one copy job.
+        job_source = getUtility(IPlainPackageCopyJobSource)
+        copy_job = job_source.getActiveJobs(target_archive).one()
+        self.assertEqual(target_archive, copy_job.target_archive)
+
+    def test_copyPackages_with_multiple_packages(self):
+        (source, source_archive, source_name, target_archive, to_pocket,
+         to_series, version) = self._setup_copy_data()
+        sources = [source]
+        sources.append(self.factory.makeSourcePackagePublishingHistory(
+            archive=source_archive,
+            status=PackagePublishingStatus.PUBLISHED))
+        sources.append(self.factory.makeSourcePackagePublishingHistory(
+            archive=source_archive,
+            status=PackagePublishingStatus.PUBLISHED))
+        names = [source.sourcepackagerelease.sourcepackagename.name
+                 for source in sources]
+
+        with person_logged_in(target_archive.owner):
+            target_archive.copyPackages(
+                names, source_archive, to_pocket.name,
+                to_series=to_series.name, include_binaries=False,
+                person=target_archive.owner)
+
+        # Make sure three copy jobs exist.
+        job_source = getUtility(IPlainPackageCopyJobSource)
+        copy_jobs = job_source.getActiveJobs(target_archive)
+        self.assertEqual(3, copy_jobs.count())
+
+    def test_copyPackages_disallows_non_primary_archive_uploaders(self):
+        # If copying to a primary archive and you're not an uploader for
+        # the package then you can't copy.
+        (source, source_archive, source_name, target_archive, to_pocket,
+         to_series, version) = self._setup_copy_data(
+            target_purpose=ArchivePurpose.PRIMARY)
+        person = self.factory.makePerson()
+        self.assertRaises(
+            CannotCopy,
+            target_archive.copyPackages, [source_name], source_archive,
+            to_pocket.name, to_series=to_series.name, include_binaries=False,
+            person=person)
+
+    def test_copyPackages_allows_primary_archive_uploaders(self):
+        # Copying to a primary archive if you're already an uploader is OK.
+        (source, source_archive, source_name, target_archive, to_pocket,
+         to_series, version) = self._setup_copy_data(
+            target_purpose=ArchivePurpose.PRIMARY)
+        person = self.factory.makePerson()
+        with person_logged_in(target_archive.owner):
+            target_archive.newComponentUploader(person, "universe")
+        target_archive.copyPackages(
+            [source_name], source_archive, to_pocket.name,
+            to_series=to_series.name, include_binaries=False,
+            person=person)
+
+        # There should be one copy job.
+        job_source = getUtility(IPlainPackageCopyJobSource)
+        copy_job = job_source.getActiveJobs(target_archive).one()
+        self.assertEqual(target_archive, copy_job.target_archive)
+
+    def test_copyPackages_disallows_non_PPA_owners(self):
+        # Only people with launchpad.Append are allowed to call copyPackage.
+        (source, source_archive, source_name, target_archive, to_pocket,
+         to_series, version) = self._setup_copy_data()
+        person = self.factory.makePerson()
+        self.assertTrue(target_archive.is_ppa)
+        self.assertRaises(
+            CannotCopy,
+            target_archive.copyPackages, [source_name], source_archive,
+            to_pocket.name, to_series=to_series.name, include_binaries=False,
+            person=person)
+


Follow ups