launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-personmerge into lp:launchpad with lp:~cjwatson/launchpad/snap-mail as a prerequisite.

Commit message:
Handle snap packages during person merging.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1476405 in Launchpad itself: "Add support for building snaps"
  https://bugs.launchpad.net/launchpad/+bug/1476405

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-personmerge/+merge/266283

Handle snap packages during person merging.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-personmerge into lp:launchpad.

=== modified file 'lib/lp/registry/personmerge.py'
--- lib/lp/registry/personmerge.py	2015-07-23 12:56:00 +0000
+++ lib/lp/registry/personmerge.py	2015-07-29 17:58:23 +0000
@@ -33,6 +33,7 @@
     )
 from lp.services.identity.interfaces.emailaddress import IEmailAddressSet
 from lp.services.mail.helpers import get_email_template
+from lp.snappy.interfaces.snap import ISnapSet
 from lp.soyuz.enums import ArchiveStatus
 from lp.soyuz.interfaces.archive import IArchiveSet
 from lp.soyuz.interfaces.livefs import ILiveFSSet
@@ -616,6 +617,24 @@
         IStore(livefses[0]).flush()
 
 
+def _mergeSnap(cur, from_person, to_person):
+    # This shouldn't use removeSecurityProxy.
+    snaps = getUtility(ISnapSet).getByPerson(from_person)
+    existing_names = [
+        s.name for s in getUtility(ISnapSet).getByPerson(to_person)]
+    for snap in snaps:
+        new_name = snap.name
+        count = 1
+        while new_name in existing_names:
+            new_name = '%s-%s' % (snap.name, count)
+            count += 1
+        naked_snap = removeSecurityProxy(snap)
+        naked_snap.owner = to_person
+        naked_snap.name = new_name
+    if not snaps.is_empty():
+        IStore(snaps[0]).flush()
+
+
 def _purgeUnmergableTeamArtifacts(from_team, to_team, reviewer):
     """Purge team artifacts that cannot be merged, but can be removed."""
     # A team cannot have more than one mailing list.
@@ -713,9 +732,6 @@
         ('latestpersonsourcepackagereleasecache', 'maintainer'),
         # Obsolete table.
         ('branchmergequeue', 'owner'),
-        # This needs handling before we deploy the snap code, but can be
-        # ignored for the purpose of deploying the database tables.
-        ('snap', 'owner'),
         ]
 
     references = list(postgresql.listReferences(cur, 'person', 'id'))
@@ -848,6 +864,9 @@
     _mergeLiveFS(cur, from_person, to_person)
     skip.append(('livefs', 'owner'))
 
+    _mergeSnap(cur, from_person, to_person)
+    skip.append(('snap', 'owner'))
+
     # Sanity check. If we have a reference that participates in a
     # UNIQUE index, it must have already been handled by this point.
     # We can tell this by looking at the skip list.

=== modified file 'lib/lp/registry/tests/test_personmerge.py'
--- lib/lp/registry/tests/test_personmerge.py	2015-05-14 13:57:51 +0000
+++ lib/lp/registry/tests/test_personmerge.py	2015-07-29 17:58:23 +0000
@@ -5,6 +5,7 @@
 
 from datetime import datetime
 
+from operator import attrgetter
 import pytz
 from testtools.matchers import MatchesStructure
 import transaction
@@ -40,6 +41,10 @@
     EmailAddressStatus,
     IEmailAddressSet,
     )
+from lp.snappy.interfaces.snap import (
+    ISnapSet,
+    SNAP_FEATURE_FLAG,
+    )
 from lp.soyuz.enums import ArchiveStatus
 from lp.soyuz.interfaces.livefs import (
     ILiveFSSet,
@@ -559,6 +564,45 @@
         self.assertEqual(['TO', 'FROM'], project_names)
         self.assertEqual(u'foo-1', livefses[1].name)
 
+    def test_merge_moves_snaps(self):
+        # When person/teams are merged, snap packages owned by the from
+        # person are moved.
+        self.useFixture(FeatureFixture({SNAP_FEATURE_FLAG: u"on"}))
+        duplicate = self.factory.makePerson()
+        mergee = self.factory.makePerson()
+        self.factory.makeSnap(registrant=duplicate, owner=duplicate)
+        self._do_premerge(duplicate, mergee)
+        login_person(mergee)
+        duplicate, mergee = self._do_merge(duplicate, mergee)
+        self.assertEqual(1, getUtility(ISnapSet).getByPerson(mergee).count())
+
+    def test_merge_with_duplicated_snaps(self):
+        # If both the from and to people have snap packages with the same
+        # name, merging renames the duplicate from the from person's side.
+        self.useFixture(FeatureFixture({SNAP_FEATURE_FLAG: u"on"}))
+        duplicate = self.factory.makePerson()
+        mergee = self.factory.makePerson()
+        branch = self.factory.makeAnyBranch()
+        [ref] = self.factory.makeGitRefs()
+        self.factory.makeSnap(
+            registrant=duplicate, owner=duplicate, name=u'foo', branch=branch)
+        self.factory.makeSnap(
+            registrant=mergee, owner=mergee, name=u'foo', git_ref=ref)
+        self._do_premerge(duplicate, mergee)
+        login_person(mergee)
+        duplicate, mergee = self._do_merge(duplicate, mergee)
+        snaps = sorted(
+            getUtility(ISnapSet).getByPerson(mergee), key=attrgetter("name"))
+        self.assertEqual(2, len(snaps))
+        self.assertIsNone(snaps[0].branch)
+        self.assertEqual(ref.repository, snaps[0].git_repository)
+        self.assertEqual(ref.path, snaps[0].git_path)
+        self.assertEqual(u'foo', snaps[0].name)
+        self.assertEqual(branch, snaps[1].branch)
+        self.assertIsNone(snaps[1].git_repository)
+        self.assertIsNone(snaps[1].git_path)
+        self.assertEqual(u'foo-1', snaps[1].name)
+
 
 class TestMergeMailingListSubscriptions(TestCaseWithFactory):