← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~julian-edwards/launchpad/copies-must-use-queue-bug-789502 into lp:launchpad

 

Julian Edwards has proposed merging lp:~julian-edwards/launchpad/copies-must-use-queue-bug-789502 into lp:launchpad with lp:~stevenk/launchpad/copies-use-notify as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #789052 in Launchpad itself: "PackageCopyJob should hold some copies in PackageUpload"
  https://bugs.launchpad.net/launchpad/+bug/789052

For more details, see:
https://code.launchpad.net/~julian-edwards/launchpad/copies-must-use-queue-bug-789502/+merge/63713

Plumb PackageCopyJob into PackageUpload so that copying to a distro makes the package observe the normal queue rules.  The branch is slightly over budget but it has some verbose tests.

Breaking this down into more detailed changes:
 1. lib/lp/soyuz/adapters/tests/test_copypolicy.py gets an extra test for no auto approval on a frozen distro and approving for an upload to a non-frozen distro.
 2. lib/lp/soyuz/model/packagecopyjob.py - the run() method is now putting the upload into the NEW or the UNAPPROVED queue depending on the results of the policy and ancestry checks, or calling do_copy() directly if the package doesn't need to be held.
 3. lib/lp/soyuz/tests/test_packagecopyjob.py - lots of test fixes and new tests for the new scenarios as above in (2)
 4. Add getSourceOverride and addSourceOverride methods to PlainPackageCopyJob which adds metadata to set overrides on component and section.
 5. Fix the packagecopier (do_copy()) so that it allows the caller to specify overrides to the copied source.
 6. Pass the copy policy's send_email boolean to do_copy() to make sure email is only sent as the policy requires.
-- 
https://code.launchpad.net/~julian-edwards/launchpad/copies-must-use-queue-bug-789502/+merge/63713
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/launchpad/copies-must-use-queue-bug-789502 into lp:launchpad.
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2011-06-10 03:46:57 +0000
+++ database/schema/security.cfg	2011-06-10 11:54:22 +0000
@@ -1031,7 +1031,7 @@
 public.packagesetgroup                        = SELECT, INSERT
 public.packagesetinclusion                    = SELECT, INSERT
 public.packagesetsources                      = SELECT, INSERT
-public.packageupload                          = SELECT
+public.packageupload                          = SELECT, INSERT
 public.packageuploadsource                    = SELECT
 public.packaging                              = SELECT, INSERT
 public.person                                 = SELECT

=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py	2011-06-08 15:27:40 +0000
+++ lib/lp/registry/model/distroseries.py	2011-06-10 11:54:22 +0000
@@ -1626,20 +1626,23 @@
                 "changesfilename and changesfilecontent must be supplied "
                 "if there is no package_copy_job")
 
-        # The PGP signature is stripped from all changesfiles
-        # to avoid replay attacks (see bugs 159304 and 451396).
-        signed_message = signed_message_from_string(changesfilecontent)
-        if signed_message is not None:
-            # Overwrite `changesfilecontent` with the text stripped
-            # of the PGP signature.
-            new_content = signed_message.signedContent
-            if new_content is not None:
-                changesfilecontent = signed_message.signedContent
+        if package_copy_job is None:
+            # The PGP signature is stripped from all changesfiles
+            # to avoid replay attacks (see bugs 159304 and 451396).
+            signed_message = signed_message_from_string(changesfilecontent)
+            if signed_message is not None:
+                # Overwrite `changesfilecontent` with the text stripped
+                # of the PGP signature.
+                new_content = signed_message.signedContent
+                if new_content is not None:
+                    changesfilecontent = signed_message.signedContent
 
-        changes_file = getUtility(ILibraryFileAliasSet).create(
-            changesfilename, len(changesfilecontent),
-            StringIO(changesfilecontent), 'text/plain',
-            restricted=archive.private)
+            changes_file = getUtility(ILibraryFileAliasSet).create(
+                changesfilename, len(changesfilecontent),
+                StringIO(changesfilecontent), 'text/plain',
+                restricted=archive.private)
+        else:
+            changes_file = None
 
         return PackageUpload(
             distroseries=self, status=PackageUploadStatus.NEW,

=== modified file 'lib/lp/soyuz/interfaces/queue.py'
--- lib/lp/soyuz/interfaces/queue.py	2011-06-03 10:38:25 +0000
+++ lib/lp/soyuz/interfaces/queue.py	2011-06-10 11:54:22 +0000
@@ -633,6 +633,13 @@
     def getSourceBySourcePackageReleaseIDs(spr_ids):
         """Return `PackageUploadSource`s for the sourcepackagerelease IDs."""
 
+    def getByPackageCopyJobIDs(pcj_ids):
+        """Return `PackageUpload`s using `PackageCopyJob`s.
+
+        :param pcj_ids: A list of `PackageCopyJob` IDs.
+        :return: all the `PackageUpload`s that reference the supplied IDs.
+        """
+
 
 class IHasQueueItems(Interface):
     """An Object that has queue items"""

=== modified file 'lib/lp/soyuz/model/packagecopyjob.py'
--- lib/lp/soyuz/model/packagecopyjob.py	2011-06-06 11:33:26 +0000
+++ lib/lp/soyuz/model/packagecopyjob.py	2011-06-10 11:54:22 +0000
@@ -17,6 +17,7 @@
     Unicode,
     )
 import transaction
+
 from zope.component import getUtility
 from zope.interface import (
     classProvides,
@@ -44,10 +45,17 @@
     IDistroSeriesDifferenceCommentSource,
     )
 from lp.services.database.stormbase import StormBase
-from lp.services.job.interfaces.job import JobStatus
+from lp.services.job.interfaces.job import (
+    JobStatus,
+    SuspendJobException,
+    )
 from lp.services.job.model.job import Job
 from lp.services.job.runner import BaseRunnableJob
-from lp.soyuz.adapters.overrides import SourceOverride
+from lp.soyuz.adapters.overrides import (
+    FromExistingOverridePolicy,
+    SourceOverride,
+    UnknownOverridePolicy,
+    )
 from lp.soyuz.enums import PackageCopyPolicy
 from lp.soyuz.interfaces.archive import CannotCopy
 from lp.soyuz.interfaces.component import IComponentSet
@@ -58,6 +66,7 @@
     IPlainPackageCopyJobSource,
     PackageCopyJobType,
     )
+from lp.soyuz.interfaces.queue import IPackageUploadSet
 from lp.soyuz.interfaces.section import ISectionSet
 from lp.soyuz.model.archive import Archive
 from lp.soyuz.scripts.packagecopier import do_copy
@@ -313,25 +322,83 @@
     def include_binaries(self):
         return self.metadata['include_binaries']
 
+    def _createPackageUpload(self, unapproved=False):
+        pu = self.target_distroseries.createQueueEntry(
+            pocket=self.target_pocket, archive=self.target_archive,
+            package_copy_job=self.context)
+        if unapproved:
+            pu.setUnapproved()
+
     def addSourceOverride(self, override):
         """Add an `ISourceOverride` to the metadata."""
+        component = ""
+        section = ""
+        if override.component is not None:
+            component = override.component.name
+        if override.section is not None:
+            section = override.section.name
         metadata_dict = dict(
-            component_override=override.component.name,
-            section_override=override.section.name)
+            component_override=component,
+            section_override=section)
         self.context.extendMetadata(metadata_dict)
 
     def getSourceOverride(self):
         """Fetch an `ISourceOverride` from the metadata."""
-        # There's only one package per job; although the schema allows
-        # multiple we're not using that.
         name = self.package_name
         component_name = self.metadata.get("component_override")
         section_name = self.metadata.get("section_override")
         source_package_name = getUtility(ISourcePackageNameSet)[name]
-        component = getUtility(IComponentSet)[component_name]
-        section = getUtility(ISectionSet)[section_name]
+        try:
+            component = getUtility(IComponentSet)[component_name]
+        except NotFoundError:
+            component = None
+        try:
+            section = getUtility(ISectionSet)[section_name]
+        except NotFoundError:
+            section = None
+
         return SourceOverride(source_package_name, component, section)
 
+    def _checkPolicies(self, source_name):
+        # This helper will only return if it's safe to carry on with the
+        # copy, otherwise it raises SuspendJobException to tell the job
+        # runner to suspend the job.
+        override_policy = FromExistingOverridePolicy()
+        ancestry = override_policy.calculateSourceOverrides(
+            self.target_archive, self.target_distroseries,
+            self.target_pocket, [source_name])
+
+        copy_policy = self.getPolicyImplementation()
+
+        if len(ancestry) == 0:
+            # We need to get the default overrides and put them in the
+            # metadata.
+            defaults = UnknownOverridePolicy().calculateSourceOverrides(
+                self.target_archive, self.target_distroseries,
+                self.target_pocket, [source_name])
+            self.addSourceOverride(defaults[0])
+
+            approve_new = copy_policy.autoApproveNew(
+                self.target_archive, self.target_distroseries,
+                self.target_pocket)
+
+            if not approve_new:
+                # There's no existing package with the same name and the
+                # policy says unapproved, so we poke it in the NEW queue.
+                self._createPackageUpload()
+                raise SuspendJobException
+        else:
+            # Put the existing override in the metadata.
+            self.addSourceOverride(ancestry[0])
+
+        # The package is not new (it has ancestry) so check the copy
+        # policy for existing packages.
+        approve_existing = copy_policy.autoApprove(
+            self.target_archive, self.target_distroseries, self.target_pocket)
+        if not approve_existing:
+            self._createPackageUpload(unapproved=True)
+            raise SuspendJobException
+
     def run(self):
         """See `IRunnableJob`."""
         try:
@@ -357,11 +424,26 @@
             name=name, version=version, exact_match=True).first()
         if source_package is None:
             raise CannotCopy("Package %r %r not found." % (name, version))
-
+        source_name = getUtility(ISourcePackageNameSet)[name]
+
+        # If there's a PackageUpload associated with this job then this
+        # job has just been released by an archive admin from the queue.
+        # We don't need to check any policies, but the admin may have
+        # set overrides which we will get from the job's metadata.
+        pu = getUtility(IPackageUploadSet).getByPackageCopyJobIDs(
+            [self.context.id])
+        if not pu.any():
+            self._checkPolicies(source_name)
+
+        # The package is free to go right in, so just copy it now.
+        override = self.getSourceOverride()
+        copy_policy = self.getPolicyImplementation()
+        send_email = copy_policy.send_email(self.target_archive)
         do_copy(
             sources=[source_package], archive=self.target_archive,
             series=self.target_distroseries, pocket=self.target_pocket,
-            include_binaries=self.include_binaries, check_permissions=False)
+            include_binaries=self.include_binaries, check_permissions=False,
+            overrides=[override], send_email=send_email)
 
     def abort(self):
         """Abort work."""

=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py	2011-06-03 10:38:25 +0000
+++ lib/lp/soyuz/model/queue.py	2011-06-10 11:54:22 +0000
@@ -29,7 +29,10 @@
     Join,
     Reference,
     )
-from storm.store import Store
+from storm.store import (
+    EmptyResultSet,
+    Store,
+    )
 from zope.component import getUtility
 from zope.interface import implements
 
@@ -41,7 +44,10 @@
     SQLBase,
     sqlvalues,
     )
-from canonical.launchpad.interfaces.lpstorm import IMasterStore
+from canonical.launchpad.interfaces.lpstorm import (
+    IMasterStore,
+    IStore,
+    )
 from canonical.librarian.interfaces import DownloadFailed
 from canonical.librarian.utils import copy_and_close
 from lp.app.errors import NotFoundError
@@ -478,7 +484,7 @@
     @property
     def is_delayed_copy(self):
         """See `IPackageUpload`."""
-        return self.changesfile is None
+        return self.changesfile is None and self.package_copy_job is None
 
     def _isSingleSourceUpload(self):
         """Return True if this upload contains only a single source."""
@@ -1365,3 +1371,13 @@
         return PackageUploadSource.select("""
             PackageUploadSource.sourcepackagerelease IN %s
             """ % sqlvalues(spr_ids))
+
+    def getByPackageCopyJobIDs(self, pcj_ids):
+        """See `IPackageUploadSet`."""
+        if pcj_ids is None or len(pcj_ids) == 0:
+            return EmptyResultSet()
+
+        return IStore(PackageUpload).find(
+            PackageUpload,
+            PackageUpload.package_copy_job_id.is_in(pcj_ids))
+

=== modified file 'lib/lp/soyuz/scripts/packagecopier.py'
--- lib/lp/soyuz/scripts/packagecopier.py	2011-06-07 23:33:42 +0000
+++ lib/lp/soyuz/scripts/packagecopier.py	2011-06-10 11:54:22 +0000
@@ -511,7 +511,7 @@
 
 def do_copy(sources, archive, series, pocket, include_binaries=False,
             allow_delayed_copies=True, person=None, check_permissions=True,
-            send_email=False):
+            overrides=None, send_email=False):
     """Perform the complete copy of the given sources incrementally.
 
     Verifies if each copy can be performed using `CopyChecker` and
@@ -536,6 +536,12 @@
     :param person: the requester `IPerson`.
     :param check_permissions: boolean indicating whether or not the
         requester's permissions to copy should be checked.
+    :param overrides: A list of `IOverride` as returned from one of the copy
+        policies which will be used as a manual override insyead of using the
+        default override returned by IArchive.getOverridePolicy().  There
+        must be the same number of overrides as there are sources and each
+        override must be for the corresponding source in the sources list.
+        Overrides will be ignored for delayed copies.
     :param send_email: Should we notify for the copy performed?
 
     :raise CannotCopy when one or more copies were not allowed. The error
@@ -565,6 +571,7 @@
     if len(errors) != 0:
         raise CannotCopy("\n".join(errors))
 
+    overrides_index = 0
     for source in copy_checker.getCheckedCopies():
         if series is None:
             destination_series = source.distroseries
@@ -576,21 +583,26 @@
                 include_binaries)
             sub_copies = [delayed_copy]
         else:
+            override = None
+            if overrides:
+                override = overrides[overrides_index]
             sub_copies = _do_direct_copy(
                 source, archive, destination_series, pocket,
-                include_binaries)
+                include_binaries, override)
             if send_email:
                 notify(
                     person, source.sourcepackagerelease, [], [], archive,
                     destination_series, pocket, changes=None,
                     action='accepted')
 
+        overrides_index += 1
         copies.extend(sub_copies)
 
     return copies
 
 
-def _do_direct_copy(source, archive, series, pocket, include_binaries):
+def _do_direct_copy(source, archive, series, pocket, include_binaries,
+                    override=None):
     """Copy publishing records to another location.
 
     Copy each item of the given list of `SourcePackagePublishingHistory`
@@ -608,6 +620,7 @@
     :param include_binaries: optional boolean, controls whether or
         not the published binaries for each given source should be also
         copied along with the source.
+    :param override: An `IOverride` as per do_copy().
 
     :return: a list of `ISourcePackagePublishingHistory` and
         `BinaryPackagePublishingHistory` corresponding to the copied
@@ -623,9 +636,12 @@
         distroseries=series, pocket=pocket)
     policy = archive.getOverridePolicy()
     if source_in_destination.is_empty():
-        override = None
-        if policy is not None:
+        # If no manual overrides were specified and the archive has an
+        # override policy then use that policy to get overrides.
+        if override is None and policy is not None:
             package_names = (source.sourcepackagerelease.sourcepackagename,)
+            # Only one override can be returned so take the first
+            # element of the returned list.
             overrides = policy.calculateSourceOverrides(
                 archive, series, pocket, package_names)
             # Only one override can be returned so take the first

=== modified file 'lib/lp/soyuz/scripts/tests/test_copypackage.py'
--- lib/lp/soyuz/scripts/tests/test_copypackage.py	2011-06-03 07:50:41 +0000
+++ lib/lp/soyuz/scripts/tests/test_copypackage.py	2011-06-10 11:54:22 +0000
@@ -15,6 +15,7 @@
 from testtools.matchers import (
     Equals,
     LessThan,
+    MatchesStructure,
     )
 import transaction
 from zope.component import getUtility
@@ -41,6 +42,7 @@
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.log.logger import BufferLogger
+from lp.soyuz.adapters.overrides import SourceOverride
 from lp.soyuz.adapters.packagelocation import PackageLocationError
 from lp.soyuz.enums import (
     ArchivePermissionType,
@@ -1382,6 +1384,31 @@
         [copied_source] = self.doCopy(
             source, target_archive, dsp.derived_series, source.pocket, False)
 
+    def test_copy_with_override(self):
+        # Test the override parameter for do_copy and by extension
+        # _do_direct_copy.
+        archive = self.test_publisher.ubuntutest.main_archive
+        source = self.test_publisher.getPubSource(
+            archive=archive, version='1.0-2', architecturehintlist='any')
+        dsp = self.factory.makeDistroSeriesParent()
+        target_archive = dsp.derived_series.main_archive
+        override = SourceOverride(
+            source.sourcepackagerelease.sourcepackagename,
+            self.factory.makeComponent(),
+            self.factory.makeSection())
+        getUtility(ISourcePackageFormatSelectionSet).add(
+            dsp.derived_series, SourcePackageFormat.FORMAT_1_0)
+        self.layer.txn.commit()
+        self.layer.switchDbUser('archivepublisher')
+        [copied_source] = do_copy(
+            [source], target_archive, dsp.derived_series, source.pocket,
+            check_permissions=False, overrides=[override])
+
+        matcher = MatchesStructure(
+            component=Equals(override.component),
+            section=Equals(override.section))
+        self.assertThat(copied_source, matcher)
+
     def test_copy_ppa_generates_notification(self):
         # When a copy into a PPA is performed, a notification is sent.
         archive = self.test_publisher.ubuntutest.main_archive

=== modified file 'lib/lp/soyuz/tests/test_packagecopyjob.py'
--- lib/lp/soyuz/tests/test_packagecopyjob.py	2011-06-06 08:37:36 +0000
+++ lib/lp/soyuz/tests/test_packagecopyjob.py	2011-06-10 11:54:22 +0000
@@ -12,6 +12,8 @@
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
+from storm.store import Store
+
 from canonical.config import config
 from canonical.launchpad.interfaces.lpstorm import IStore
 from canonical.launchpad.webapp.testing import verifyObject
@@ -20,17 +22,23 @@
     DistroSeriesDifferenceComment,
     )
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.registry.interfaces.series import SeriesStatus
 from lp.services.features.testing import FeatureFixture
-from lp.services.job.interfaces.job import JobStatus
+from lp.services.job.interfaces.job import (
+    JobStatus,
+    SuspendJobException,
+    )
 from lp.soyuz.adapters.overrides import SourceOverride
 from lp.soyuz.enums import (
     ArchivePurpose,
     PackageCopyPolicy,
+    PackageUploadStatus,
     SourcePackageFormat,
     )
 from lp.soyuz.model.distroseriesdifferencejob import (
     FEATURE_FLAG_ENABLE_MODULE,
     )
+from lp.soyuz.model.queue import PackageUpload
 from lp.soyuz.interfaces.archive import CannotCopy
 from lp.soyuz.interfaces.component import IComponentSet
 from lp.soyuz.interfaces.packagecopyjob import (
@@ -39,6 +47,7 @@
     IPlainPackageCopyJobSource,
     )
 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
+from lp.soyuz.interfaces.queue import IPackageUploadSet
 from lp.soyuz.interfaces.section import ISectionSet
 from lp.soyuz.interfaces.sourcepackageformat import (
     ISourcePackageFormatSelectionSet,
@@ -48,6 +57,7 @@
     run_script,
     TestCaseWithFactory,
     )
+from lp.testing.mail_helpers import pop_notifications
 from lp.testing.fakemethod import FakeMethod
 
 
@@ -61,6 +71,8 @@
 class LocalTestHelper:
     """Put test helpers that want to be in the test classes here."""
 
+    dbuser = config.IPlainPackageCopyJobSource.dbuser
+
     def makeJob(self, dsd=None, **kwargs):
         """Create a `PlainPackageCopyJob` that would resolve `dsd`."""
         if dsd is None:
@@ -77,7 +89,7 @@
     def runJob(self, job):
         """Helper to switch to the right DB user and run the job."""
         self.layer.txn.commit()
-        self.layer.switchDbUser('sync_packages')
+        self.layer.switchDbUser(self.dbuser)
         job.run()
 
 
@@ -284,6 +296,12 @@
             distroseries=distroseries, sourcename="libc",
             version="2.8-1", status=PackagePublishingStatus.PUBLISHED,
             archive=breezy_archive)
+        # The target archive needs ancestry so the package is
+        # auto-accepted.
+        ancestry = publisher.getPubSource(
+            distroseries=target_series, sourcename="libc",
+            version="2.8-0", status=PackagePublishingStatus.PUBLISHED,
+            archive=target_archive)
 
         source = getUtility(IPlainPackageCopyJobSource)
         job = source.create(
@@ -298,15 +316,18 @@
         # Make sure everything hits the database, switching db users
         # aborts.
         transaction.commit()
-        # XXX: GavinPanella 2011-04-20 bug=770297: The sync_packages database
-        # user should be renamed to copy_packages.
-        self.layer.switchDbUser('sync_packages')
+        self.layer.switchDbUser(self.dbuser)
         job.run()
 
-        published_sources = target_archive.getPublishedSources()
-        spr = published_sources.one().sourcepackagerelease
-        self.assertEquals("libc", spr.name)
-        self.assertEquals("2.8-1", spr.version)
+        published_sources = target_archive.getPublishedSources(
+            name="libc", version="2.8-1")
+        self.assertIsNot(None, published_sources.any())
+
+        # The copy should have sent an email too. (see
+        # soyuz/scripts/tests/test_copypackage.py for detailed
+        # notification tests)
+        emails = pop_notifications()
+        self.assertTrue(len(emails) > 0)
 
         # Switch back to a db user that has permission to clean up
         # featureflag.
@@ -455,6 +476,260 @@
         self.assertEqual(
             {}, job_source.getPendingJobsPerPackage(dsd.derived_series))
 
+    def test_copying_to_main_archive_ancestry_overrides(self):
+        # The job will complete right away for auto-approved copies to a
+        # main archive and apply any ancestry overrides.
+        publisher = SoyuzTestPublisher()
+        publisher.prepareBreezyAutotest()
+        distroseries = publisher.breezy_autotest
+
+        target_archive = self.factory.makeArchive(
+            distroseries.distribution, purpose=ArchivePurpose.PRIMARY)
+        source_archive = self.factory.makeArchive()
+
+        # Publish a package in the source archive with some overridable
+        # properties set to known values.
+        source_package = publisher.getPubSource(
+            distroseries=distroseries, sourcename="libc",
+            component='universe', section='web',
+            version="2.8-1", status=PackagePublishingStatus.PUBLISHED,
+            archive=source_archive)
+
+        # Now put the same named package in the target archive with
+        # different override values.
+        ancestry_package = publisher.getPubSource(
+            distroseries=distroseries, sourcename="libc",
+            component='restricted', section='games',
+            version="2.8-0", status=PackagePublishingStatus.PUBLISHED,
+            archive=target_archive)
+
+        # Now, run the copy job, which should auto-approve the copy and
+        # override the package with the existing values in the
+        # target_archive.
+
+        source = getUtility(IPlainPackageCopyJobSource)
+        job = source.create(
+            package_name="libc",
+            package_version="2.8-1",
+            source_archive=source_archive,
+            target_archive=target_archive,
+            target_distroseries=distroseries,
+            target_pocket=PackagePublishingPocket.RELEASE,
+            include_binaries=False)
+
+        self.runJob(job)
+
+        new_publication = target_archive.getPublishedSources(
+            name='libc', version='2.8-1').one()
+        self.assertEqual('restricted', new_publication.component.name)
+        self.assertEqual('games', new_publication.section.name)
+
+    def test_copying_to_main_archive_manual_overrides(self):
+        # Test processing a packagecopyjob that has manual overrides.
+        publisher = SoyuzTestPublisher()
+        publisher.prepareBreezyAutotest()
+        distroseries = publisher.breezy_autotest
+
+        target_archive = self.factory.makeArchive(
+            distroseries.distribution, purpose=ArchivePurpose.PRIMARY)
+        source_archive = self.factory.makeArchive()
+
+        # Publish a package in the source archive with some overridable
+        # properties set to known values.
+        source_package = publisher.getPubSource(
+            distroseries=distroseries, sourcename="copyme",
+            component='universe', section='web',
+            version="2.8-1", status=PackagePublishingStatus.PUBLISHED,
+            archive=source_archive)
+
+        # Now, run the copy job, which should raise an error because
+        # there's no ancestry.
+        source = getUtility(IPlainPackageCopyJobSource)
+        job = source.create(
+            package_name="copyme",
+            package_version="2.8-1",
+            source_archive=source_archive,
+            target_archive=target_archive,
+            target_distroseries=distroseries,
+            target_pocket=PackagePublishingPocket.RELEASE,
+            include_binaries=False)
+
+        self.assertRaises(SuspendJobException, self.runJob, job)
+        # Simulate the job runner suspending after getting a
+        # SuspendJobException
+        job.suspend()
+        self.layer.txn.commit()
+        self.layer.switchDbUser("launchpad_main")
+
+        # Add some overrides to the job.
+        package = source_package.sourcepackagerelease.sourcepackagename
+        restricted = getUtility(IComponentSet)['restricted']
+        editors = getUtility(ISectionSet)['editors']
+        override = SourceOverride(package, restricted, editors)
+        job.addSourceOverride(override)
+
+        # Accept the upload to release the job then run it.
+        pu = getUtility(IPackageUploadSet).getByPackageCopyJobIDs(
+            [removeSecurityProxy(job).context.id]).one()
+        pu.acceptFromQueue()
+        self.runJob(job)
+
+        # The copied source should have the manual overrides, not the
+        # original values.
+        new_publication = target_archive.getPublishedSources(
+            name='copyme', version='2.8-1').one()
+        self.assertEqual('restricted', new_publication.component.name)
+        self.assertEqual('editors', new_publication.section.name)
+
+    def test_copying_to_main_archive_with_no_ancestry(self):
+        # The job should suspend itself and create a packageupload with
+        # a reference to the package_copy_job.
+        publisher = SoyuzTestPublisher()
+        publisher.prepareBreezyAutotest()
+        distroseries = publisher.breezy_autotest
+
+        target_archive = self.factory.makeArchive(
+            distroseries.distribution, purpose=ArchivePurpose.PRIMARY)
+        source_archive = self.factory.makeArchive()
+
+        # Publish a package in the source archive with some overridable
+        # properties set to known values.
+        source_package = publisher.getPubSource(
+            distroseries=distroseries, sourcename="copyme",
+            component='multiverse', section='web',
+            version="2.8-1", status=PackagePublishingStatus.PUBLISHED,
+            archive=source_archive)
+
+        # There is no package of the same name already in the target
+        # archive.
+        existing_sources = target_archive.getPublishedSources(name='copyme')
+        self.assertEqual(None, existing_sources.any())
+
+        # Now, run the copy job.
+
+        source = getUtility(IPlainPackageCopyJobSource)
+        job = source.create(
+            package_name="copyme",
+            package_version="2.8-1",
+            source_archive=source_archive,
+            target_archive=target_archive,
+            target_distroseries=distroseries,
+            target_pocket=PackagePublishingPocket.RELEASE,
+            include_binaries=False)
+
+        # The job should be suspended and there's a PackageUpload with
+        # its package_copy_job set.
+        self.assertRaises(SuspendJobException, self.runJob, job)
+        pu = Store.of(target_archive).find(
+            PackageUpload,
+            PackageUpload.package_copy_job_id == job.id).one()
+        pcj = removeSecurityProxy(job).context
+        self.assertEqual(pcj, pu.package_copy_job)
+
+        # The job metadata should contain default overrides from the
+        # UnknownOverridePolicy policy.
+        self.assertEqual('universe', pcj.metadata['component_override'])
+
+    def test_copying_to_main_archive_unapproved(self):
+        # Uploading to a series that is in a state that precludes auto
+        # approval will cause the job to suspend and a packageupload
+        # created in the UNAPPROVED state.
+        publisher = SoyuzTestPublisher()
+        publisher.prepareBreezyAutotest()
+        distroseries = publisher.breezy_autotest
+        # The series is frozen so it won't auto-approve new packages.
+        distroseries.status = SeriesStatus.FROZEN
+
+        target_archive = self.factory.makeArchive(
+            distroseries.distribution, purpose=ArchivePurpose.PRIMARY)
+        source_archive = self.factory.makeArchive()
+
+        # Publish a package in the source archive.
+        source_package = publisher.getPubSource(
+            distroseries=distroseries, sourcename="copyme",
+            version="2.8-1", status=PackagePublishingStatus.PUBLISHED,
+            component='multiverse', section='web',
+            archive=source_archive)
+
+        # Now put the same named package in the target archive so it has
+        # ancestry.
+        ancestry_package = publisher.getPubSource(
+            distroseries=distroseries, sourcename="copyme",
+            version="2.8-0", status=PackagePublishingStatus.PUBLISHED,
+            component='main', section='games',
+            archive=target_archive)
+
+        # Now, run the copy job.
+        source = getUtility(IPlainPackageCopyJobSource)
+        job = source.create(
+            package_name="copyme",
+            package_version="2.8-1",
+            source_archive=source_archive,
+            target_archive=target_archive,
+            target_distroseries=distroseries,
+            target_pocket=PackagePublishingPocket.RELEASE,
+            include_binaries=False)
+
+        # The job should be suspended and there's a PackageUpload with
+        # its package_copy_job set in the UNAPPROVED queue.
+        self.assertRaises(SuspendJobException, self.runJob, job)
+
+        pu = Store.of(target_archive).find(
+            PackageUpload,
+            PackageUpload.package_copy_job_id == job.id).one()
+        pcj = removeSecurityProxy(job).context
+        self.assertEqual(pcj, pu.package_copy_job)
+        self.assertEqual(PackageUploadStatus.UNAPPROVED, pu.status)
+
+        # The job's metadata should contain the override ancestry from
+        # the target archive.
+        self.assertEqual('main', pcj.metadata['component_override'])
+
+    def test_copying_after_job_released(self):
+        # The first pass of the job may have created a PackageUpload and
+        # suspended the job.  Here we test the second run to make sure
+        # that it actually copies the package.
+        publisher = SoyuzTestPublisher()
+        publisher.prepareBreezyAutotest()
+        distroseries = publisher.breezy_autotest
+
+        target_archive = self.factory.makeArchive(
+            distroseries.distribution, purpose=ArchivePurpose.PRIMARY)
+        source_archive = self.factory.makeArchive()
+
+        # Publish a package in the source archive.
+        source_package = publisher.getPubSource(
+            distroseries=distroseries, sourcename="copyme",
+            version="2.8-1", status=PackagePublishingStatus.PUBLISHED,
+            archive=source_archive)
+
+        source = getUtility(IPlainPackageCopyJobSource)
+        job = source.create(
+            package_name="copyme",
+            package_version="2.8-1",
+            source_archive=source_archive,
+            target_archive=target_archive,
+            target_distroseries=distroseries,
+            target_pocket=PackagePublishingPocket.RELEASE,
+            include_binaries=False)
+
+        # Run the job so it gains a PackageUpload.
+        self.assertRaises(SuspendJobException, self.runJob, job)
+        # Simulate the job runner suspending after getting a
+        # SuspendJobException
+        job.suspend()
+        self.layer.txn.commit()
+        self.layer.switchDbUser("launchpad_main")
+
+        # Accept the upload to release the job then run it.
+        pu = getUtility(IPackageUploadSet).getByPackageCopyJobIDs(
+            [removeSecurityProxy(job).context.id]).one()
+        pu.acceptFromQueue()
+        self.runJob(job)
+
+        existing_sources = target_archive.getPublishedSources(name='copyme')
+        self.assertIsNot(None, existing_sources.any())
+
     def test_findMatchingDSDs_matches_all_DSDs_for_job(self):
         # findMatchingDSDs finds matching DSDs for any of the packages
         # in the job.
@@ -555,11 +830,11 @@
     def test_findMatchingDSDs(self):
         job = self.makeJob()
         transaction.commit()
-        self.layer.switchDbUser(config.IPlainPackageCopyJobSource.dbuser)
+        self.layer.switchDbUser(self.dbuser)
         removeSecurityProxy(job).findMatchingDSDs()
 
     def test_reportFailure(self):
         job = self.makeJob()
         transaction.commit()
-        self.layer.switchDbUser(config.IPlainPackageCopyJobSource.dbuser)
+        self.layer.switchDbUser(self.dbuser)
         removeSecurityProxy(job).reportFailure(CannotCopy("Mommy it hurts"))

=== modified file 'lib/lp/soyuz/tests/test_packageupload.py'
--- lib/lp/soyuz/tests/test_packageupload.py	2011-06-03 16:10:52 +0000
+++ lib/lp/soyuz/tests/test_packageupload.py	2011-06-10 11:54:22 +0000
@@ -366,6 +366,13 @@
 
         self.assertEqual(pcj, pu.package_copy_job)
 
+    def test_getByPackageCopyJobIDs(self):
+        pcj = removeSecurityProxy(
+            self.factory.makePlainPackageCopyJob()).context
+        pu = self.factory.makePackageUpload(package_copy_job=pcj)
+        result = getUtility(IPackageUploadSet).getByPackageCopyJobIDs([pcj.id])
+        self.assertEqual(pu, result.one())
+
     def test_overrideSource_with_copy_job(self):
         # The overrides should be stored in the job's metadata.
         plain_copy_job = self.factory.makePlainPackageCopyJob()