launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/launchpad/copyjob-dsp-permission-bug-787485 into lp:launchpad/db-devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #787485 in Launchpad itself: "PackageCopyJob has no permission to write to DistributionSourcePackage"
  https://bugs.launchpad.net/launchpad/+bug/787485

For more details, see:
https://code.launchpad.net/~julian-edwards/launchpad/copyjob-dsp-permission-bug-787485/+merge/62140

This branch fixes a test for PackageCopyJob that was not doing a full distro -> derived distro package sync and thus was not hitting all the database tables the code can reach during this operation.

The consequent test failures enabled me to add the right permissions to security.cfg.
-- 
https://code.launchpad.net/~julian-edwards/launchpad/copyjob-dsp-permission-bug-787485/+merge/62140
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/launchpad/copyjob-dsp-permission-bug-787485 into lp:launchpad/db-devel.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2011-05-23 09:13:32 +0000
+++ database/schema/security.cfg	2011-05-24 14:39:18 +0000
@@ -997,9 +997,11 @@
 public.component                              = SELECT
 public.componentselection                     = SELECT, INSERT
 public.distribution                           = SELECT
-public.distributionjob                        = SELECT
+public.distributionjob                        = SELECT , INSERT, UPDATE
 public.distroarchseries                       = SELECT, INSERT
+public.distributionsourcepackage              = SELECT, INSERT, UPDATE
 public.distroseries                           = SELECT, UPDATE
+public.distroseriesparent                     = SELECT
 public.flatpackagesetinclusion                = SELECT, INSERT
 public.gpgkey                                 = SELECT
 public.job                                    = SELECT, INSERT, UPDATE, DELETE
@@ -1012,6 +1014,7 @@
 public.packagesetinclusion                    = SELECT, INSERT
 public.packagesetsources                      = SELECT, INSERT
 public.packageupload                          = SELECT
+public.packageuploadsource                    = SELECT
 public.packaging                              = SELECT, INSERT
 public.person                                 = SELECT
 public.pocketchroot                           = SELECT

=== modified file 'lib/lp/soyuz/tests/test_packagecopyjob.py'
--- lib/lp/soyuz/tests/test_packagecopyjob.py	2011-05-19 17:20:02 +0000
+++ lib/lp/soyuz/tests/test_packagecopyjob.py	2011-05-24 14:39:18 +0000
@@ -10,13 +10,24 @@
 
 from canonical.testing import LaunchpadZopelessLayer
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.services.features.testing import FeatureFixture
 from lp.services.job.interfaces.job import JobStatus
+from lp.soyuz.enums import (
+    ArchivePurpose,
+    SourcePackageFormat,
+    )
+from lp.soyuz.model.distroseriesdifferencejob import (
+    FEATURE_FLAG_ENABLE_MODULE,
+    )
 from lp.soyuz.interfaces.archive import CannotCopy
 from lp.soyuz.interfaces.packagecopyjob import (
     IPackageCopyJob,
     IPlainPackageCopyJobSource,
     )
 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
+from lp.soyuz.interfaces.sourcepackageformat import (
+    ISourcePackageFormatSelectionSet,
+    )
 from lp.soyuz.model.packagecopyjob import specify_dsd_package
 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
 from lp.testing import (
@@ -41,6 +52,12 @@
             source_packages, source_archive, target_archive,
             target_distroseries, target_pocket)
 
+    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()
+
     def test_create(self):
         # A PackageCopyJob can be created and stores its arguments.
         distroseries = self.factory.makeDistroSeries()
@@ -104,7 +121,7 @@
             target_archive=archive2, target_distroseries=distroseries,
             target_pocket=PackagePublishingPocket.RELEASE,
             include_binaries=False)
-        self.assertRaises(CannotCopy, job.run)
+        self.assertRaises(CannotCopy, self.runJob, job)
 
     def test_target_ppa_non_release_pocket(self):
         # When copying to a PPA archive the target must be the release pocket.
@@ -117,26 +134,38 @@
             target_archive=archive2, target_distroseries=distroseries,
             target_pocket=PackagePublishingPocket.UPDATES,
             include_binaries=False)
-        self.assertRaises(CannotCopy, job.run)
+        self.assertRaises(CannotCopy, self.runJob, job)
 
     def test_run(self):
         # A proper test run synchronizes packages.
+
+        # Turn on DSD jobs.
+        self.useFixture(FeatureFixture({FEATURE_FLAG_ENABLE_MODULE: 'on'}))
+
         publisher = SoyuzTestPublisher()
         publisher.prepareBreezyAutotest()
         distroseries = publisher.breezy_autotest
 
-        archive1 = self.factory.makeArchive(distroseries.distribution)
-        archive2 = self.factory.makeArchive(distroseries.distribution)
+        # Synchronise from breezy-autotest to a brand new distro derived
+        # from breezy.
+        breezy_archive = self.factory.makeArchive(
+            distroseries.distribution, purpose=ArchivePurpose.PRIMARY)
+        dsp = self.factory.makeDistroSeriesParent(parent_series=distroseries)
+        target_series = dsp.derived_series
+        target_archive = self.factory.makeArchive(
+            target_series.distribution, purpose=ArchivePurpose.PRIMARY)
+        getUtility(ISourcePackageFormatSelectionSet).add(
+            target_series, SourcePackageFormat.FORMAT_1_0)
 
         source_package = publisher.getPubSource(
             distroseries=distroseries, sourcename="libc",
             version="2.8-1", status=PackagePublishingStatus.PUBLISHED,
-            archive=archive1)
+            archive=breezy_archive)
 
         source = getUtility(IPlainPackageCopyJobSource)
         job = source.create(
-            source_packages=[("libc", "2.8-1")], source_archive=archive1,
-            target_archive=archive2, target_distroseries=distroseries,
+            source_packages=[("libc", "2.8-1")], source_archive=breezy_archive,
+            target_archive=target_archive, target_distroseries=target_series,
             target_pocket=PackagePublishingPocket.RELEASE,
             include_binaries=False)
         self.assertContentEqual(
@@ -150,11 +179,15 @@
         self.layer.switchDbUser('sync_packages')
         job.run()
 
-        published_sources = archive2.getPublishedSources()
+        published_sources = target_archive.getPublishedSources()
         spr = published_sources.one().sourcepackagerelease
         self.assertEquals("libc", spr.name)
         self.assertEquals("2.8-1", spr.version)
 
+        # Switch back to a db user that has permission to clean up
+        # featureflag.
+        self.layer.switchDbUser('launchpad_main')
+
     def test_getOopsVars(self):
         distroseries = self.factory.makeDistroSeries()
         archive1 = self.factory.makeArchive(distroseries.distribution)