launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/new-archive-refs into lp:launchpad.

Commit message:
Introduce Archive.reference, a string that uniquely identifies an archive in Launchpad, and ArchiveSet.getByReference to look archives up by their reference.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/new-archive-refs/+merge/225950

Introduce Archive.reference, a string that uniquely identifies an archive in Launchpad, and ArchiveSet.getByReference to look archives up by their reference.
-- 
https://code.launchpad.net/~wgrant/launchpad/new-archive-refs/+merge/225950
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/new-archive-refs into lp:launchpad.

=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2014-07-04 08:46:39 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2014-07-08 10:56:10 +0000
@@ -317,6 +317,11 @@
             title=_("Display name"), required=True,
             description=_("A short title for the archive.")))
 
+    reference = exported(
+        TextLine(
+            title=_("Reference"), required=True, readonly=True,
+            description=_("A string to uniquely identify the archive.")))
+
     distribution = exported(
         Reference(
             Interface,  # Redefined to IDistribution later.
@@ -2064,6 +2069,9 @@
     def get(archive_id):
         """Return the IArchive with the given archive_id."""
 
+    def getByReference(reference):
+        """Return the IArchive with the given archive reference."""
+
     def getPPAByDistributionAndOwnerName(distribution, person_name, ppa_name):
         """Return a single PPA.
 

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2014-07-05 00:23:24 +0000
+++ lib/lp/soyuz/model/archive.py	2014-07-08 10:56:10 +0000
@@ -205,6 +205,14 @@
 from lp.soyuz.scripts.packagecopier import check_copy_permissions
 
 
+ARCHIVE_REFERENCE_TEMPLATES = {
+    ArchivePurpose.PRIMARY: u'%(distribution)s',
+    ArchivePurpose.PPA: u'~%(owner)s/%(distribution)s/%(archive)s',
+    ArchivePurpose.PARTNER: u'%(distribution)s/%(archive)s',
+    ArchivePurpose.COPY: u'%(distribution)s/%(archive)s',
+    }
+
+
 def storm_validate_external_dependencies(archive, attr, value):
     assert attr == 'external_dependencies'
     errors = validate_external_dependencies(value)
@@ -396,6 +404,16 @@
         return self.status == ArchiveStatus.ACTIVE
 
     @property
+    def reference(self):
+        template = ARCHIVE_REFERENCE_TEMPLATES.get(self.purpose)
+        if template is None:
+            raise AssertionError(
+                "No archive reference template for %s." % self.purpose.name)
+        return template % {
+            'archive': self.name, 'owner': self.owner.name,
+            'distribution': self.distribution.name}
+
+    @property
     def series_with_sources(self):
         """See `IArchive`."""
         # Import DistroSeries here to avoid circular imports.
@@ -2123,6 +2141,37 @@
         """See `IArchiveSet`."""
         return Archive.get(archive_id)
 
+    def getByReference(self, reference):
+        """See `IArchiveSet`."""
+        from lp.registry.interfaces.distribution import IDistributionSet
+
+        bits = reference.split(u'/')
+        if len(bits) < 1:
+            return None
+        if bits[0].startswith(u'~'):
+            # PPA reference (~OWNER/DISTRO/ARCHIVE)
+            if len(bits) != 3:
+                return None
+            person = getUtility(IPersonSet).getByName(bits[0][1:])
+            if person is None:
+                return None
+            distro = getUtility(IDistributionSet).getByName(bits[1])
+            if distro is None:
+                return None
+            return self.getPPAOwnedByPerson(
+                person, distribution=distro, name=bits[2])
+        else:
+            # Official archive reference (DISTRO or DISTRO/ARCHIVE)
+            distro = getUtility(IDistributionSet).getByName(bits[0])
+            if distro is None:
+                return None
+            if len(bits) == 1:
+                return distro.main_archive
+            elif len(bits) == 2:
+                return self.getByDistroAndName(distro, bits[1])
+            else:
+                return None
+
     def getPPAByDistributionAndOwnerName(self, distribution, person_name,
                                          ppa_name):
         """See `IArchiveSet`"""

=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2014-07-08 06:34:37 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2014-07-08 10:56:10 +0000
@@ -3010,6 +3010,123 @@
             NoSuchPPA, self.person.getPPAByName, None, "aap")
 
 
+class TestArchiveReference(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def assertReferenceIntegrity(self, reference, archive):
+        """Assert that the archive's reference matches in both directions."""
+        self.assertEqual(reference, archive.reference)
+        self.assertEqual(
+            archive, getUtility(IArchiveSet).getByReference(reference))
+
+    def test_primary(self):
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
+        self.assertReferenceIntegrity(archive.distribution.name, archive)
+
+    def test_partner(self):
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.PARTNER)
+        self.assertReferenceIntegrity(
+            '%s/%s' % (archive.distribution.name, archive.name),
+            archive)
+
+    def test_copy(self):
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.COPY)
+        self.assertReferenceIntegrity(
+            '%s/%s' % (archive.distribution.name, archive.name),
+            archive)
+
+    def test_ppa(self):
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+        self.assertReferenceIntegrity(
+            '~%s/%s/%s' % (
+                archive.owner.name, archive.distribution.name, archive.name),
+            archive)
+
+
+class TestArchiveSetGetByReference(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestArchiveSetGetByReference, self).setUp()
+        self.set = getUtility(IArchiveSet)
+
+    def test_ppa(self):
+        owner = self.factory.makePerson(name='pwner')
+        twoner = self.factory.makePerson(name='twoner')
+        twobuntu = self.factory.makeDistribution(name='two')
+        threebuntu = self.factory.makeDistribution(name='three')
+        ppa1 = self.factory.makeArchive(
+            owner=owner, distribution=twobuntu, name='ppa',
+            purpose=ArchivePurpose.PPA)
+        ppa2 = self.factory.makeArchive(
+            owner=owner, distribution=twobuntu, name='qpa',
+            purpose=ArchivePurpose.PPA)
+        ppa3 = self.factory.makeArchive(
+            owner=owner, distribution=threebuntu, name='ppa',
+            purpose=ArchivePurpose.PPA)
+        ppa4 = self.factory.makeArchive(
+            owner=twoner, distribution=twobuntu, name='ppa',
+            purpose=ArchivePurpose.PPA)
+
+        self.assertEqual(ppa1, self.set.getByReference('~pwner/two/ppa'))
+        self.assertEqual(ppa2, self.set.getByReference('~pwner/two/qpa'))
+        self.assertEqual(ppa3, self.set.getByReference('~pwner/three/ppa'))
+        self.assertEqual(ppa4, self.set.getByReference('~twoner/two/ppa'))
+
+        # Bad combinations give None.
+        self.assertIs(None, self.set.getByReference('~pwner/three/qpa'))
+        self.assertIs(None, self.set.getByReference('~twoner/two/qpa'))
+        self.assertIs(None, self.set.getByReference('~pwner/two/rpa'))
+
+        # Nonexistent names give None.
+        self.assertIs(None, self.set.getByReference('~pwner/enoent/ppa'))
+        self.assertIs(None, self.set.getByReference('~whoisthis/two/ppa'))
+
+        # Invalid formats give None.
+        self.assertIs(None, self.set.getByReference('~whoisthis/two/w/t'))
+        self.assertIs(None, self.set.getByReference('~whoisthis/two'))
+        self.assertIs(None, self.set.getByReference('~whoisthis'))
+
+    def test_distro(self):
+        twobuntu = self.factory.makeDistribution(name='two')
+        threebuntu = self.factory.makeDistribution(name='three')
+        two_primary = self.factory.makeArchive(
+            distribution=twobuntu, purpose=ArchivePurpose.PRIMARY)
+        two_partner = self.factory.makeArchive(
+            distribution=twobuntu, purpose=ArchivePurpose.PARTNER)
+        three_primary = self.factory.makeArchive(
+            distribution=threebuntu, purpose=ArchivePurpose.PRIMARY)
+        three_copy = self.factory.makeArchive(
+            distribution=threebuntu, purpose=ArchivePurpose.COPY,
+            name='rebuild')
+
+        self.assertEqual(two_primary, self.set.getByReference('two'))
+        self.assertEqual(two_partner, self.set.getByReference('two/partner'))
+        self.assertEqual(three_primary, self.set.getByReference('three'))
+        self.assertEqual(three_copy, self.set.getByReference('three/rebuild'))
+
+        # Bad combinations give None.
+        self.assertIs(None, self.set.getByReference('three/partner'))
+        self.assertIs(None, self.set.getByReference('two/rebuild'))
+
+        # Nonexistent names give None.
+        self.assertIs(None, self.set.getByReference('three/enoent'))
+        self.assertIs(None, self.set.getByReference('enodist'))
+        self.assertIs(None, self.set.getByReference('enodist/partner'))
+
+        # Invalid formats give None.
+        self.assertIs(None, self.set.getByReference('two/partner/idonteven'))
+
+    def test_nonsense(self):
+        self.assertIs(None, getUtility(IArchiveSet).getByReference(''))
+        self.assertIs(
+            None,
+            getUtility(IArchiveSet).getByReference(
+                'that/does/not/make/sense'))
+
+
 class TestDisplayName(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer