launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/launchpad/sync-packages-bug-880246 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #880246 in Launchpad itself: "+synchronised-packages showing syncs to distros derived past the one I sync'd to"
  https://bugs.launchpad.net/launchpad/+bug/880246

For more details, see:
https://code.launchpad.net/~rvb/launchpad/sync-packages-bug-880246/+merge/80889

This branch fixes the package copier so that it can be told to set creator=None for newly created packages.  This is used by the initialization code to create packages with creator=None instead of the default (which is to populate creator with the related spr's creator).

Note: this won't fix the publishings already created with a creator instead of None (the ones mentioned on the bug report).

= Tests =

./bin/test -vvc test_person test_getLatestSynchronisedPublishings_derived_series
./bin/test -vvc test_initialize_distroseries test_copied_publishings_creator_None_cloner
./bin/test -vvc test_initialize_distroseries test_copied_publishings_creator_None_copier

= Q/A =

Create a package in distroseries A, copy it to distroseries B and make sure you get creadited for that on the +synchronised-packages page.  Then initialize a new series from B and make sure you don't see the newly created packages on the +synchronised-packages page.
-- 
https://code.launchpad.net/~rvb/launchpad/sync-packages-bug-880246/+merge/80889
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/sync-packages-bug-880246 into lp:launchpad.

=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py	2011-09-28 23:26:44 +0000
+++ lib/lp/registry/tests/test_person.py	2011-11-01 11:26:35 +0000
@@ -38,6 +38,7 @@
 from canonical.launchpad.testing.pages import LaunchpadWebServiceCaller
 from canonical.testing.layers import (
     DatabaseFunctionalLayer,
+    LaunchpadZopelessLayer,
     reconnect_stores,
     )
 from lp.answers.model.answercontact import AnswerContact
@@ -77,6 +78,10 @@
     ArchivePurpose,
     ArchiveStatus,
     )
+from lp.soyuz.scripts.initialize_distroseries import InitializeDistroSeries
+from lp.soyuz.scripts.tests.test_initialize_distroseries import (
+    InitializationHelperTestCase,
+    )
 from lp.testing import (
     ANONYMOUS,
     celebrity_logged_in,
@@ -439,14 +444,14 @@
         self.assertThat(recorder, HasQueryCount(Equals(1)))
 
     def createCopiedPackage(self, spph, copier, dest_distroseries=None,
-                            dest_archive=None):
+                            dest_archive=None,
+                            dest_pocket=PackagePublishingPocket.UPDATES):
         if dest_distroseries is None:
             dest_distroseries = self.factory.makeDistroSeries()
         if dest_archive is None:
             dest_archive = dest_distroseries.main_archive
         return spph.copyTo(
-            dest_distroseries, creator=copier,
-            pocket=PackagePublishingPocket.UPDATES,
+            dest_distroseries, creator=copier, pocket=dest_pocket,
             archive=dest_archive)
 
     def test_getLatestSynchronisedPublishings_most_recent_first(self):
@@ -521,6 +526,38 @@
             synchronised_spphs.count())
 
 
+class TestPersonInitializedSeries(TestPerson, InitializationHelperTestCase):
+
+    layer = LaunchpadZopelessLayer
+
+    def test_getLatestSynchronisedPublishings_derived_series(self):
+        # The publishings copied over when a new series is initialized
+        # are not picked up by getLatestSynchronisedPublishings.
+        copier = self.factory.makePerson()
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            pocket=PackagePublishingPocket.RELEASE, creator=copier)
+        parent, unused = self.setupParent(
+            packages={}, arch_tag='i386')
+        self.createCopiedPackage(spph, copier, parent)
+        copied_spph2 = self.createCopiedPackage(
+            spph, copier, parent,
+            dest_pocket=PackagePublishingPocket.RELEASE)
+
+        # Initialize a new series from 'parent'.
+        child = self.factory.makeDistroSeries(previous_series=parent)
+        ids = InitializeDistroSeries(child, [parent.id])
+        ids.check()
+        ids.initialize()
+        synchronised_spphs = copier.getLatestSynchronisedPublishings()
+
+        # The publishings copied from the parent are not picked up by
+        # getLatestSynchronisedPublishings because creator=None for
+        # these new publishings.
+        self.assertContentEqual(
+            [copied_spph2],
+            synchronised_spphs)
+
+
 class TestPersonStates(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer

=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py	2011-10-23 02:58:56 +0000
+++ lib/lp/soyuz/model/publishing.py	2011-11-01 11:26:35 +0000
@@ -1416,6 +1416,14 @@
     return expanded
 
 
+# A simple one-off object meaning than we should use the creator from
+# the releted spr (sourcepackagerelease) when we create a new
+# publication.  This covers 99% of the cases but we still need a way to
+# set the creator to None in some cases, hence the
+# DEFAULT_CREATOR_FROM_SPR object.
+DEFAULT_CREATOR_FROM_SPR = object()
+
+
 class PublishingSet:
     """Utilities for manipulating publications in batches."""
 
@@ -1561,13 +1569,13 @@
     def newSourcePublication(self, archive, sourcepackagerelease,
                              distroseries, component, section, pocket,
                              ancestor=None, create_dsd_job=True,
-                             creator=None):
+                             creator=DEFAULT_CREATOR_FROM_SPR):
         """See `IPublishingSet`."""
         # Avoid circular import.
         from lp.registry.model.distributionsourcepackage import (
             DistributionSourcePackage)
 
-        if creator is None:
+        if creator is DEFAULT_CREATOR_FROM_SPR:
             creator = sourcepackagerelease.creator
 
         pub = SourcePackagePublishingHistory(

=== modified file 'lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py'
--- lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py	2011-10-13 10:45:09 +0000
+++ lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py	2011-11-01 11:26:35 +0000
@@ -1249,6 +1249,31 @@
         self.assertFalse(
             ids._use_cloner(target_archive, target_archive))
 
+    def test_copied_publishings_creator_None_cloner(self):
+        # The new publishings, copied over from the parents, have their
+        # 'creator' field set to None.  This tests that behaviour when
+        # the cloner is used to perform the initialization.
+        parent, unused = self.setupParent(packages={u'p1': u'1.2'})
+        child = self.setUpSeriesWithPreviousSeries(previous_parents=[parent])
+        self.factory.makeSourcePackagePublishingHistory(distroseries=child)
+        self._fullInitialize([parent], child=child)
+
+        published_sources = child.main_archive.getPublishedSources(
+            distroseries=child)
+        self.assertEqual(None, published_sources[0].creator)
+
+    def test_copied_publishings_creator_None_copier(self):
+        # The new publishings, copied over from the parents, have their
+        # 'creator' field set to None.  This tests that behaviour when
+        # the copier is used to perform the initialization.
+        parent, unused = self.setupParent(packages={u'p1': u'1.2'})
+        child = self.setUpSeriesWithPreviousSeries(previous_parents=[parent])
+        self._fullInitialize([parent], child=child)
+
+        published_sources = child.main_archive.getPublishedSources(
+            distroseries=child)
+        self.assertEqual(None, published_sources[0].creator)
+
     def test__has_same_parents_as_previous_series_explicit(self):
         # IDS._has_same_parents_as_previous_series returns True if the
         # parents for the series to be initialized are the same as