← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jtv/launchpad/db-bug-786970 into lp:launchpad/db-devel

 

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/db-bug-786970 into lp:launchpad/db-devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #786970 in Launchpad itself: "Track and display errors in PlainPackageCopyJobs"
  https://bugs.launchpad.net/launchpad/+bug/786970
  Bug #787462 in Launchpad itself: "PackageCopyJob runner should not file an OOPS for CannotCopy errors"
  https://bugs.launchpad.net/launchpad/+bug/787462

For more details, see:
https://code.launchpad.net/~jtv/launchpad/db-bug-786970/+merge/62292

= Summary =

When a PlainPackageCopyJob fails with a CannotCopy error, that's not really meant to be an oops.  It's meant to be an error that the user should address.  But how do we tell the user?  This job is something you "see going on" in the UI, so we want the errors in the UI as well — email is not an answer.


== Proposed fix ==

Julian and I (with feedback from Colin) decided on the following error-reporting mechanism: any one of the PlainPackageCopyJobs we're dealing with resolves a DistroSeriesDifference, which is what's visible in the UI.  And that in turn can have a conversation stream of DistroSeriesDifferenceComments attached.  So the job can throw CannotCopy failure messages into that stream, which also solves the problem of logging historical builds.

Next we'll make the page poll, so that these messages can show up on the fly.


== Pre-implementation notes ==

Multiple package copies for multiple DistroSeriesDifferences can go into a single PlainPackageCopyJob.  We initially thought we could capture enough information from failures that a CannotCopy can be neatly pegged to a DistroSeriesDifference on the page.

But alas.  There are complications: it takes some relatively complicated tracking, and because the checks are to some extent phased, you might see your package failing because some other package broke the whole job.  In the end we decided on a simple fix: only handle one package per job.  This affects two callsites.

Another question is: what Launchpad user will the error messages be attributed to?  An attempt to start a discussion about having a user identity to represent Launchpad in the database for this sort of thing failed miserably.  We thought it might be easy impersonate the requesting user (which will be a little disconcerting because automatically generated messages meant primarily for the user will look as if they'd been entered by that same user).  But the only current place for that information is Job.requester, which… isn't accessible in Python.  In the end Julian gave me the green light to use the Janitor, which fulfills many of the roles that I had wanted a "Launchpad persona" user for.


== Implementation details ==

For once I remembered to test the new code for database privileges.  Which revealed a few that were needed, hopefully saving us some agony later on.

Sadly gone are very similar bits of code that Gavin and I developed in parallel for collating packages by archive, so as to bundle them into the minimum number of PlainPackageCopyJobs.


== Tests ==

{{{
./bin/test -vvc lp.soyuz.tests.test_packagecopyjob
}}}


== Demo and Q/A ==

Request asynchronous package copies on a derived distroseries' +localpackagediffs page.  (Do this by requesting more copies than the max_synchronous_syncs feature flag setting at once).  If any of them fail (and I admit I have yet to figure out how to do that) its error will show up as a comment.


= Launchpad lint =

No lint, but it's complicated.  I touched a file that had pre-existing lint, which I did not fix, and then I undid my changes to that file.
-- 
https://code.launchpad.net/~jtv/launchpad/db-bug-786970/+merge/62292
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/db-bug-786970 into lp:launchpad/db-devel.
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2011-05-25 12:30:44 +0000
+++ database/schema/security.cfg	2011-05-25 13:09:21 +0000
@@ -984,6 +984,7 @@
 
 [sync_packages]
 groups=script
+public.account                                = SELECT
 public.archive                                = SELECT
 public.archivepermission                      = SELECT, INSERT
 public.binarypackagebuild                     = SELECT, INSERT
@@ -1001,12 +1002,20 @@
 public.distributionsourcepackage              = SELECT, INSERT, UPDATE
 public.distroarchseries                       = SELECT, INSERT
 public.distroseries                           = SELECT, UPDATE
+<<<<<<< TREE
 public.distroseriesparent                     = SELECT
+=======
+public.distroseriesdifference                 = SELECT
+public.distroseriesdifferencemessage          = SELECT, INSERT, UPDATE
+public.emailaddress                           = SELECT
+>>>>>>> MERGE-SOURCE
 public.flatpackagesetinclusion                = SELECT, INSERT
 public.gpgkey                                 = SELECT
 public.job                                    = SELECT, INSERT, UPDATE, DELETE
 public.libraryfilealias                       = SELECT, INSERT, UPDATE, DELETE
 public.libraryfilecontent                     = SELECT, INSERT
+public.message                                = SELECT, INSERT, UPDATE
+public.messagechunk                           = SELECT, INSERT, UPDATE
 public.packagebuild                           = SELECT, INSERT
 public.packagecopyjob                         = SELECT
 public.packageset                             = SELECT, INSERT

=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py	2011-05-24 10:08:33 +0000
+++ lib/lp/registry/browser/distroseries.py	2011-05-25 13:09:21 +0000
@@ -1043,17 +1043,14 @@
         """Request sync of packages that can be easily upgraded."""
         target_distroseries = self.context
         target_archive = target_distroseries.main_archive
-        differences_by_archive = (
-            getUtility(IDistroSeriesDifferenceSource)
-                .collateDifferencesByParentArchive(self.getUpgrades()))
-        for source_archive, differences in differences_by_archive.iteritems():
-            source_package_info = [
-                (difference.source_package_name.name,
-                 difference.parent_source_version)
-                for difference in differences]
-            getUtility(IPlainPackageCopyJobSource).create(
-                source_package_info, source_archive, target_archive,
-                target_distroseries, PackagePublishingPocket.UPDATES)
+        job_source = getUtility(IPlainPackageCopyJobSource)
+
+        for dsd in self.getUpgrades():
+            job_source.create(
+                [specify_dsd_package(dsd)], dsd.parent_series.main_archive,
+                target_archive, target_distroseries,
+                PackagePublishingPocket.UPDATES)
+
         self.request.response.addInfoNotification(
             (u"Upgrades of {context.displayname} packages have been "
              u"requested. Please give Launchpad some time to complete "

=== modified file 'lib/lp/registry/interfaces/distroseriesdifference.py'
--- lib/lp/registry/interfaces/distroseriesdifference.py	2011-05-24 15:36:35 +0000
+++ lib/lp/registry/interfaces/distroseriesdifference.py	2011-05-25 13:09:21 +0000
@@ -338,15 +338,3 @@
 
         Blacklisted items are excluded.
         """
-
-    def collateDifferencesByParentArchive(differences):
-        """Collate the given differences by parent archive.
-
-        The given `IDistroSeriesDifference`s are returned in a `dict`, with
-        the parent `Archive` as keys.
-
-        :param differences: An iterable sequence of `IDistroSeriesDifference`.
-
-        :return: A `dict` of iterable sequences of `IDistroSeriesDifference`
-            keyed by their parent `IArchive`.
-        """

=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py	2011-05-24 11:32:34 +0000
+++ lib/lp/registry/model/distroseriesdifference.py	2011-05-25 13:09:21 +0000
@@ -494,17 +494,6 @@
                 DistroSeriesDifference.source_package_name_id)
         return DecoratedResultSet(differences, itemgetter(0))
 
-    @staticmethod
-    def collateDifferencesByParentArchive(differences):
-        by_archive = dict()
-        for difference in differences:
-            archive = difference.parent_series.main_archive
-            if archive in by_archive:
-                by_archive[archive].append(difference)
-            else:
-                by_archive[archive] = [difference]
-        return by_archive
-
     @cachedproperty
     def source_pub(self):
         """See `IDistroSeriesDifference`."""

=== modified file 'lib/lp/registry/tests/test_distroseriesdifference.py'
--- lib/lp/registry/tests/test_distroseriesdifference.py	2011-05-24 15:36:35 +0000
+++ lib/lp/registry/tests/test_distroseriesdifference.py	2011-05-25 13:09:21 +0000
@@ -1129,24 +1129,6 @@
         self.assertContentEqual(
             [], dsd_source.getSimpleUpgrades(dsd.derived_series))
 
-    def test_collateDifferencesByParentArchive(self):
-        dsp1 = self.factory.makeDistroSeriesParent()
-        dsp2 = self.factory.makeDistroSeriesParent()
-        differences = [
-            self.factory.makeDistroSeriesDifference(dsp1.derived_series),
-            self.factory.makeDistroSeriesDifference(dsp2.derived_series),
-            self.factory.makeDistroSeriesDifference(dsp1.derived_series),
-            self.factory.makeDistroSeriesDifference(dsp2.derived_series),
-            ]
-        dsd_source = getUtility(IDistroSeriesDifferenceSource)
-        observed = (
-            dsd_source.collateDifferencesByParentArchive(differences))
-        expected = {
-            dsp1.parent_series.main_archive: differences[0::2],
-            dsp2.parent_series.main_archive: differences[1::2],
-            }
-        self.assertEqual(observed, expected)
-
 
 class TestMostRecentComments(TestCaseWithFactory):
 

=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2011-05-20 07:56:37 +0000
+++ lib/lp/soyuz/browser/archive.py	2011-05-25 13:09:21 +0000
@@ -1282,19 +1282,6 @@
         dest_display_name)
 
 
-def partition_pubs_by_archive(source_pubs):
-    """Group `source_pubs` by archive.
-
-    :param source_pubs: A sequence of `SourcePackagePublishingHistory`.
-    :return: A dict mapping `Archive`s to the list of entries from
-        `source_pubs` that are in that archive.
-    """
-    by_source_archive = {}
-    for spph in source_pubs:
-        by_source_archive.setdefault(spph.archive, []).append(spph)
-    return by_source_archive
-
-
 def name_pubs_with_versions(source_pubs):
     """Annotate each entry from `source_pubs` with its version.
 
@@ -1325,11 +1312,11 @@
             person, dest_archive, dest_series, dest_pocket, spns)
 
     job_source = getUtility(IPlainPackageCopyJobSource)
-    archive_pubs = partition_pubs_by_archive(source_pubs)
-    for source_archive, spphs in archive_pubs.iteritems():
+    for spph in source_pubs:
         job_source.create(
-            name_pubs_with_versions(spphs), source_archive, dest_archive,
+            name_pubs_with_versions([spph]), spph.archive, dest_archive,
             dest_series, dest_pocket, include_binaries=include_binaries)
+
     return structured("""
         <p>Requested sync of %s packages.</p>
         <p>Please allow some time for these to be processed.</p>

=== modified file 'lib/lp/soyuz/browser/tests/test_package_copying_mixin.py'
--- lib/lp/soyuz/browser/tests/test_package_copying_mixin.py	2011-05-20 20:40:19 +0000
+++ lib/lp/soyuz/browser/tests/test_package_copying_mixin.py	2011-05-25 13:09:21 +0000
@@ -19,7 +19,6 @@
     FEATURE_FLAG_MAX_SYNCHRONOUS_SYNCS,
     name_pubs_with_versions,
     PackageCopyingMixin,
-    partition_pubs_by_archive,
     render_cannotcopy_as_html,
     )
 from lp.soyuz.interfaces.archive import CannotCopy
@@ -104,30 +103,6 @@
         packages = [self.getUniqueString() for counter in range(300)]
         self.assertFalse(PackageCopyingMixin().canCopySynchronously(packages))
 
-    def test_partition_pubs_by_archive_maps_archives_to_pubs(self):
-        # partition_pubs_by_archive returns a dict mapping each archive
-        # to a list of SPPHs on that archive.
-        spph = FakeSPPH()
-        self.assertEqual(
-            {spph.archive: [spph]}, partition_pubs_by_archive([spph]))
-
-    def test_partition_pubs_by_archive_splits_by_archive(self):
-        # partition_pubs_by_archive keeps SPPHs for different archives
-        # separate.
-        spphs = [FakeSPPH() for counter in xrange(2)]
-        mapping = partition_pubs_by_archive(spphs)
-        self.assertEqual(
-            dict((spph.archive, [spph]) for spph in spphs), mapping)
-
-    def test_partition_pubs_by_archive_clusters_by_archive(self):
-        # partition_pubs_by_archive bundles SPPHs for the same archive
-        # into a single dict entry.
-        archive = FakeArchive()
-        spphs = [FakeSPPH(archive=archive) for counter in xrange(2)]
-        mapping = partition_pubs_by_archive(spphs)
-        self.assertEqual([archive], mapping.keys())
-        self.assertContentEqual(spphs, mapping[archive])
-
     def test_name_pubs_with_versions_lists_packages_and_versions(self):
         # name_pubs_with_versions returns a list of tuples of source
         # package name and source package version, one per SPPH.

=== modified file 'lib/lp/soyuz/model/packagecopyjob.py'
--- lib/lp/soyuz/model/packagecopyjob.py	2011-05-19 17:20:02 +0000
+++ lib/lp/soyuz/model/packagecopyjob.py	2011-05-25 13:09:21 +0000
@@ -17,6 +17,8 @@
     Reference,
     Unicode,
     )
+import transaction
+from zope.component import getUtility
 from zope.interface import (
     classProvides,
     implements,
@@ -26,13 +28,20 @@
 from canonical.launchpad.components.decoratedresultset import (
     DecoratedResultSet,
     )
+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.interfaces.lpstorm import (
     IMasterStore,
     IStore,
     )
 from lp.app.errors import NotFoundError
+from lp.registry.interfaces.distroseriesdifference import (
+    IDistroSeriesDifferenceSource,
+    )
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.model.distroseries import DistroSeries
+from lp.registry.interfaces.distroseriesdifferencecomment import (
+    IDistroSeriesDifferenceCommentSource,
+    )
 from lp.services.database.stormbase import StormBase
 from lp.services.job.interfaces.job import JobStatus
 from lp.services.job.model.job import Job
@@ -232,6 +241,19 @@
 
     def run(self):
         """See `IRunnableJob`."""
+        try:
+            self.attemptCopy()
+        except CannotCopy, e:
+            self.abort()
+            self.reportFailure(e)
+            self.commit()
+
+    def attemptCopy(self):
+        """Attempt to perform the copy.
+
+        :raise CannotCopy: If the copy fails for a reason that the user
+            can deal with.
+        """
         if self.target_archive.is_ppa:
             if self.target_pocket != PackagePublishingPocket.RELEASE:
                 raise CannotCopy(
@@ -250,6 +272,47 @@
             series=self.target_distroseries, pocket=self.target_pocket,
             include_binaries=self.include_binaries, check_permissions=False)
 
+    def abort(self):
+        """Abort work."""
+        transaction.abort()
+
+    def commit(self):
+        """Commit work."""
+        transaction.commit()
+
+    def findMatchingDSDs(self, package_name=None):
+        """Find any `DistroSeriesDifference`s that this job might resolve."""
+        dsd_source = getUtility(IDistroSeriesDifferenceSource)
+        target_series = self.target_distroseries
+        candidates = dsd_source.getForDistroSeries(
+            distro_series=target_series,
+            source_package_name_filter=package_name)
+        # The job doesn't know what distroseries a given package is
+        # coming from, and the version number in the DSD may have
+        # changed.  We can however filter out DSDs that are from
+        # different distributions, based on the job's target archive.
+        source_distro = self.source_archive.distribution
+        return [
+            dsd
+            for dsd in candidates
+                if dsd.parent_series.distribution == source_distro]
+
+    def reportFailure(self, cannotcopy_exception):
+        """Attempt to report failure to the user."""
+        message = unicode(cannotcopy_exception)
+        dsds = self.findMatchingDSDs(cannotcopy_exception.package_name)
+        comment_source = getUtility(IDistroSeriesDifferenceCommentSource)
+
+        # Register the error comment in the name of the Janitor.  Not a
+        # great choice, but we have no user identity to represent
+        # Launchpad; it's far too costly to create one; and
+        # impersonating the requester can be misleading and would also
+        # involve extra bookkeeping.
+        reporting_persona = getUtility(ILaunchpadCelebrities).janitor
+
+        for dsd in dsds:
+            comment_source.new(dsd, reporting_persona, message)
+
     def __repr__(self):
         """Returns an informative representation of the job."""
         parts = ["%s to copy" % self.__class__.__name__]

=== modified file 'lib/lp/soyuz/tests/test_packagecopyjob.py'
--- lib/lp/soyuz/tests/test_packagecopyjob.py	2011-05-24 14:29:51 +0000
+++ lib/lp/soyuz/tests/test_packagecopyjob.py	2011-05-25 13:09:21 +0000
@@ -8,7 +8,12 @@
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
+from canonical.config import config
+from canonical.launchpad.interfaces.lpstorm import IStore
 from canonical.testing import LaunchpadZopelessLayer
+from lp.registry.model.distroseriesdifferencecomment import (
+    DistroSeriesDifferenceComment,
+    )
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.services.features.testing import FeatureFixture
 from lp.services.job.interfaces.job import JobStatus
@@ -34,15 +39,30 @@
     run_script,
     TestCaseWithFactory,
     )
-
-
-class PlainPackageCopyJobTests(TestCaseWithFactory):
-    """Test case for PlainPackageCopyJob."""
-
-    layer = LaunchpadZopelessLayer
-
-    def makeJob(self, dsd):
+from lp.testing.fakemethod import FakeMethod
+
+
+def get_dsd_comments(dsd):
+    """Retrieve `DistroSeriesDifferenceComment`s for `dsd`."""
+    return IStore(dsd).find(
+        DistroSeriesDifferenceComment,
+        DistroSeriesDifferenceComment.distro_series_difference == dsd)
+
+
+def strip_error_handling_transactionality(job):
+    """Stop `job`'s error handling from committing or aborting."""
+    naked_job = removeSecurityProxy(job)
+    naked_job.commit = FakeMethod()
+    naked_job.abort = FakeMethod()
+
+
+class LocalTestHelper:
+    """Put test helpers that want to be in the test classes here."""
+
+    def makeJob(self, dsd=None):
         """Create a `PlainPackageCopyJob` that would resolve `dsd`."""
+        if dsd is None:
+            dsd = self.factory.makeDistroSeriesDifference()
         source_packages = [specify_dsd_package(dsd)]
         source_archive = dsd.parent_series.main_archive
         target_archive = dsd.derived_series.main_archive
@@ -52,12 +72,21 @@
             source_packages, source_archive, target_archive,
             target_distroseries, target_pocket)
 
+<<<<<<< TREE
     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')
         job.run()
 
+=======
+
+class PlainPackageCopyJobTests(TestCaseWithFactory, LocalTestHelper):
+    """Test case for PlainPackageCopyJob."""
+
+    layer = LaunchpadZopelessLayer
+
+>>>>>>> MERGE-SOURCE
     def test_create(self):
         # A PackageCopyJob can be created and stores its arguments.
         distroseries = self.factory.makeDistroSeries()
@@ -105,23 +134,63 @@
 
     def test_getActiveJobs_only_returns_waiting_jobs(self):
         # getActiveJobs ignores jobs that aren't in the WAITING state.
-        job = self.makeJob(self.factory.makeDistroSeriesDifference())
+        job = self.makeJob()
         removeSecurityProxy(job).job._status = JobStatus.RUNNING
         source = getUtility(IPlainPackageCopyJobSource)
         self.assertContentEqual([], source.getActiveJobs(job.target_archive))
 
-    def test_run_unknown_package(self):
-        # A job properly records failure.
+    def test_run_raises_errors(self):
+        # A job reports unexpected errors as exceptions.
+        class Boom(Exception):
+            pass
+
+        job = self.makeJob()
+        removeSecurityProxy(job).attemptCopy = FakeMethod(failure=Boom())
+
+        self.assertRaises(Boom, job.run)
+
+    def test_run_posts_copy_failure_as_comment(self):
+        # If the job fails with a CannotCopy exception, it swallows the
+        # exception and posts a DistroSeriesDifferenceComment with the
+        # failure message.
+        dsd = self.factory.makeDistroSeriesDifference()
+        self.factory.makeArchive(distribution=dsd.derived_series.distribution)
+        job = self.makeJob(dsd)
+        removeSecurityProxy(job).attemptCopy = FakeMethod(
+            failure=CannotCopy("Server meltdown"))
+
+        # The job's error handling will abort, so commit the objects we
+        # created as would have happened in real life.
+        transaction.commit()
+
+        job.run()
+
+        self.assertEqual(
+            ["Server meltdown"],
+            [comment.body_text for comment in get_dsd_comments(dsd)])
+
+    def test_run_cannot_copy_unknown_package(self):
+        # Attempting to copy an unknown package is reported as a
+        # failure.
         distroseries = self.factory.makeDistroSeries()
         archive1 = self.factory.makeArchive(distroseries.distribution)
         archive2 = self.factory.makeArchive(distroseries.distribution)
-        source = getUtility(IPlainPackageCopyJobSource)
-        job = source.create(
+        job_source = getUtility(IPlainPackageCopyJobSource)
+        job = job_source.create(
             source_packages=[("foo", "1.0-1")], source_archive=archive1,
             target_archive=archive2, target_distroseries=distroseries,
             target_pocket=PackagePublishingPocket.RELEASE,
             include_binaries=False)
+<<<<<<< TREE
         self.assertRaises(CannotCopy, self.runJob, job)
+=======
+        naked_job = removeSecurityProxy(job)
+        naked_job.reportFailure = FakeMethod()
+
+        job.run()
+
+        self.assertEqual(1, naked_job.reportFailure.call_count)
+>>>>>>> MERGE-SOURCE
 
     def test_target_ppa_non_release_pocket(self):
         # When copying to a PPA archive the target must be the release pocket.
@@ -134,7 +203,17 @@
             target_archive=archive2, target_distroseries=distroseries,
             target_pocket=PackagePublishingPocket.UPDATES,
             include_binaries=False)
+<<<<<<< TREE
         self.assertRaises(CannotCopy, self.runJob, job)
+=======
+
+        naked_job = removeSecurityProxy(job)
+        naked_job.reportFailure = FakeMethod()
+
+        job.run()
+
+        self.assertEqual(1, naked_job.reportFailure.call_count)
+>>>>>>> MERGE-SOURCE
 
     def test_run(self):
         # A proper test run synchronizes packages.
@@ -275,8 +354,7 @@
     def test_getPendingJobsPerPackage_ignores_other_distroseries(self):
         # getPendingJobsPerPackage only looks for jobs on the indicated
         # distroseries.
-        dsd = self.factory.makeDistroSeriesDifference()
-        self.makeJob(dsd)
+        self.makeJob()
         other_series = self.factory.makeDistroSeries()
         job_source = getUtility(IPlainPackageCopyJobSource)
         self.assertEqual(
@@ -333,3 +411,69 @@
         job_source = getUtility(IPlainPackageCopyJobSource)
         self.assertEqual(
             {}, job_source.getPendingJobsPerPackage(dsd.derived_series))
+
+    def test_findMatchingDSDs_without_package_matches_all_DSDs_for_job(self):
+        # If given no package name, findMatchingDSDs will find all
+        # matching DSDs for any of the packages in the job.
+        dsd = self.factory.makeDistroSeriesDifference()
+        job = removeSecurityProxy(self.makeJob(dsd))
+        self.assertContentEqual(
+            [dsd], job.findMatchingDSDs(package_name=None))
+
+    def test_findMatchingDSDs_finds_matching_DSD_for_package(self):
+        # If given a package name, findMatchingDSDs will find all
+        # matching DSDs for the job that are for that package name.
+        dsd = self.factory.makeDistroSeriesDifference()
+        job = removeSecurityProxy(self.makeJob(dsd))
+        package_name = dsd.source_package_name.name
+        self.assertContentEqual(
+            [dsd], job.findMatchingDSDs(package_name=package_name))
+
+    def test_findMatchingDSDs_ignores_other_packages(self):
+        # If given a package name, findMatchingDSDs will ignore DSDs
+        # that would otherwise match the job but are for different
+        # packages.
+        job = removeSecurityProxy(self.makeJob())
+        other_package = self.factory.makeSourcePackageName()
+        self.assertContentEqual(
+            [], job.findMatchingDSDs(package_name=other_package.name))
+
+    def test_findMatchingDSDs_ignores_other_source_series(self):
+        # findMatchingDSDs tries to ignore DSDs that are for different
+        # parent series than the job's source series.  (This can't be
+        # done with perfect precision because the job doesn't keep track
+        # of source distroseries, but in practice it should be good
+        # enough).
+        dsd = self.factory.makeDistroSeriesDifference()
+        job = removeSecurityProxy(self.makeJob(dsd))
+
+        # If the dsd differs only in parent series, that's enough to
+        # make it a non-match.
+        removeSecurityProxy(dsd).parent_series = (
+            self.factory.makeDistroSeries())
+
+        self.assertContentEqual(
+            [],
+            job.findMatchingDSDs(package_name=dsd.source_package_name.name))
+
+
+class TestPlainPackageCopyJobPrivileges(TestCaseWithFactory, LocalTestHelper):
+    """Test that `PlainPackageCopyJob` has the privileges it needs.
+
+    This test looks for errors, not failures.  It's here only to see that
+    these operations don't run into any privilege limitations.
+    """
+
+    layer = LaunchpadZopelessLayer
+
+    def test_findMatchingDSDs(self):
+        job = self.makeJob()
+        transaction.commit()
+        self.layer.switchDbUser(config.IPlainPackageCopyJobSource.dbuser)
+        removeSecurityProxy(job).findMatchingDSDs()
+
+    def test_reportFailure(self):
+        job = self.makeJob()
+        transaction.commit()
+        self.layer.switchDbUser(config.IPlainPackageCopyJobSource.dbuser)
+        removeSecurityProxy(job).reportFailure(CannotCopy("Mommy it hurts"))