← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~cjohnston/launchpad/ppa-rtm-updates into lp:launchpad

 

Chris Johnston has proposed merging lp:~cjohnston/launchpad/ppa-rtm-updates into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjohnston/launchpad/ppa-rtm-updates/+merge/230142
-- 
https://code.launchpad.net/~cjohnston/launchpad/ppa-rtm-updates/+merge/230142
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjohnston/launchpad/ppa-rtm-updates into lp:launchpad.
=== modified file 'lib/lp/code/javascript/tests/test_requestbuild_overlay.html'
--- lib/lp/code/javascript/tests/test_requestbuild_overlay.html	2012-10-26 09:54:28 +0000
+++ lib/lp/code/javascript/tests/test_requestbuild_overlay.html	2014-08-08 17:27:51 +0000
@@ -1,6 +1,6 @@
 <!DOCTYPE html>
 <!--
-Copyright 2012 Canonical Ltd.  This software is licensed under the
+Copyright 2012-2014 Canonical Ltd.  This software is licensed under the
 GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -73,7 +73,7 @@
         <script type="text/x-template" id="requestbuilds-form-template">
           <div class="form" id="launchpad-form-widgets">
             <select id="field.archive" name="field.archive" size="1" >
-              <option selected="selected" value="mark/ppa">
+              <option selected="selected" value="~mark/ubuntu/ppa">
                 PPA for Mark Shuttleworth</option>
             </select>
             <input name="field.archive-empty-marker" type="hidden" value="1" />

=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2014-08-01 08:47:00 +0000
+++ lib/lp/soyuz/browser/archive.py	2014-08-08 17:27:51 +0000
@@ -1391,6 +1391,7 @@
     for archive in archives:
         label = '%s [%s]' % (archive.displayname, archive.reference)
         terms.append(SimpleTerm(archive, archive.reference, label))
+    terms.sort(key=lambda x: x.value.reference)
     return SimpleVocabulary(terms)
 
 
@@ -1627,10 +1628,8 @@
                     canonical_url(dependency), archive_dependency.title)
             else:
                 dependency_label = archive_dependency.title
-            dependency_token = '%s/%s' % (
-                dependency.owner.name, dependency.name)
             term = SimpleTerm(
-                dependency, dependency_token, dependency_label)
+                dependency, dependency.reference, dependency_label)
             terms.append(term)
         return form.Fields(
             List(__name__='selected_dependencies',

=== modified file 'lib/lp/soyuz/browser/tests/archive-views.txt'
--- lib/lp/soyuz/browser/tests/archive-views.txt	2014-07-24 09:37:03 +0000
+++ lib/lp/soyuz/browser/tests/archive-views.txt	2014-08-08 17:27:51 +0000
@@ -716,7 +716,7 @@
 
 Let's emulate a dependency addition. Note that the form contains, a
 empty 'selected_dependencies' (as it was rendered in the empty
-request) and 'dependency_candidate' contains a valid PPA owner name.
+request) and 'dependency_candidate' contains a valid PPA name.
 Validation checks are documented in
 pagetests/ppa/xx-edit-dependencies.txt.
 
@@ -724,7 +724,7 @@
     ...     cprov.archive, name="+edit-dependencies",
     ...     form={
     ...         'field.selected_dependencies': [],
-    ...         'field.dependency_candidate': 'mark/ppa',
+    ...         'field.dependency_candidate': '~mark/ubuntu/ppa',
     ...         'field.primary_dependencies': 'UPDATES',
     ...         'field.primary_components': 'ALL_COMPONENTS',
     ...         'field.actions.save': 'Save',
@@ -762,7 +762,7 @@
     PPA for Mark Shuttleworth
 
     >>> print dependency.token
-    mark/ppa
+    ~mark/ubuntu/ppa
 
     >>> print dependency.title.escapedtext
     <a href="http://launchpad.dev/~mark/+archive/ubuntu/ppa";>PPA for Mark
@@ -794,7 +794,7 @@
     PPA for Mark Shuttleworth
 
     >>> print dependency.token
-    mark/ppa
+    ~mark/ubuntu/ppa
 
     >>> print dependency.title
     PPA for Mark Shuttleworth
@@ -805,7 +805,7 @@
     >>> view = create_initialized_view(
     ...     cprov.archive, name="+edit-dependencies",
     ...     form={
-    ...         'field.selected_dependencies': ['mark/ppa'],
+    ...         'field.selected_dependencies': ['~mark/ubuntu/ppa'],
     ...         'field.dependency_candidate': '',
     ...         'field.primary_dependencies': 'UPDATES',
     ...         'field.primary_components': 'ALL_COMPONENTS',
@@ -849,7 +849,7 @@
     >>> view.widgets.get('primary_dependencies')._getCurrentValue()
     <DBItem PackagePublishingPocket.UPDATES, (20) Updates>
 
-A similar widget is used for the primary archive component overrides ,
+A similar widget is used for the primary archive component overrides,
 which contains two pre-defined options. By default all PPAs use all
 ubuntu components available to satisfy build dependencies, i.e. the
 'multiverse' component.
@@ -965,7 +965,7 @@
     <DBItem PackagePublishingPocket.UPDATES, (20) Updates>
 
 Dependencies on private PPAs can be only set if the user performing
-the action also have permission to view the private PPA and if the
+the action also has permission to view the private PPA and if the
 context PPA is also private.
 
 The latter guarantee that the P3A buildd_secret won't get exposed in
@@ -988,7 +988,7 @@
 
     >>> add_private_form = {
     ...     'field.selected_dependencies': [],
-    ...     'field.dependency_candidate': 'pirulito-team/ppa',
+    ...     'field.dependency_candidate': '~pirulito-team/ubuntu/ppa',
     ...     'field.primary_dependencies': 'UPDATES',
     ...     'field.primary_components': 'FOLLOW_PRIMARY',
     ...     'field.actions.save': 'Save',

=== modified file 'lib/lp/soyuz/doc/vocabularies.txt'
--- lib/lp/soyuz/doc/vocabularies.txt	2013-07-12 06:14:22 +0000
+++ lib/lp/soyuz/doc/vocabularies.txt	2014-08-08 17:27:51 +0000
@@ -189,13 +189,13 @@
  * value: the IArchive object;
  * title: the first line of the PPA description text.
 
-    >>> cprov_term = vocabulary.getTermByToken('cprov/ppa')
+    >>> cprov_term = vocabulary.getTermByToken('~cprov/ubuntu/ppa')
 
     >>> print cprov_term.token
-    cprov/ppa
+    ~cprov/ubuntu/ppa
 
     >>> print cprov_term.value
-    <Archive ...>
+    <... lp.soyuz.model.archive.Archive instance ...>
 
     >>> print cprov_term.title
     packages to help my friends.
@@ -216,15 +216,15 @@
 
     >>> cprov_search = vocabulary.search(u'cprov')
     >>> print_search_results(cprov_search)
-    cprov/ppa: packages to help my friends.
+    ~cprov/ubuntu/ppa: packages to help my friends.
 
     >>> celso_search = vocabulary.search(u'celso')
     >>> print_search_results(celso_search)
-    cprov/ppa: packages to help my friends.
+    ~cprov/ubuntu/ppa: packages to help my friends.
 
     >>> friends_search = vocabulary.search(u'friends')
     >>> print_search_results(friends_search)
-    cprov/ppa: packages to help my friends.
+    ~cprov/ubuntu/ppa: packages to help my friends.
 
 We will create an additional PPA for Celso named 'testing'
 
@@ -242,15 +242,15 @@
 
     >>> cprov_search = vocabulary.search(u'cprov')
     >>> print_search_results(cprov_search)
-    cprov/ppa: packages to help my friends.
-    cprov/testing: testing packages.
+    ~cprov/ubuntu/ppa: packages to help my friends.
+    ~cprov/ubuntu/testing: testing packages.
 
 The vocabulary search also supports specific named PPA lookups
 follwing the same combined syntax used to build unique tokens.
 
-    >>> named_search = vocabulary.search(u'cprov/testing')
+    >>> named_search = vocabulary.search(u'~cprov/ubuntu/testing')
     >>> print_search_results(named_search)
-    cprov/testing: testing packages.
+    ~cprov/ubuntu/testing: testing packages.
 
 As mentioned the PPA vocabulary term title only contains the first
 line of the PPA description.
@@ -258,14 +258,14 @@
     >>> cprov.archive.description = "Single line."
     >>> flush_database_updates()
 
-    >>> cprov_term = vocabulary.getTermByToken('cprov/ppa')
+    >>> cprov_term = vocabulary.getTermByToken('~cprov/ubuntu/ppa')
     >>> print cprov_term.title
     Single line.
 
     >>> cprov.archive.description = "First line\nSecond line."
     >>> flush_database_updates()
 
-    >>> cprov_term = vocabulary.getTermByToken('cprov/ppa')
+    >>> cprov_term = vocabulary.getTermByToken('~cprov/ubuntu/ppa')
     >>> print cprov_term.title
     First line
 
@@ -274,7 +274,7 @@
     >>> cprov.archive.description = None
     >>> flush_database_updates()
 
-    >>> cprov_term = vocabulary.getTermByToken('cprov/ppa')
+    >>> cprov_term = vocabulary.getTermByToken('~cprov/ubuntu/ppa')
     >>> print cprov_term.title
     No description available
 

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2014-08-07 07:53:27 +0000
+++ lib/lp/soyuz/model/archive.py	2014-08-08 17:27:51 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2014 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Database class for table Archive."""
@@ -1077,7 +1077,13 @@
                 raise ArchiveDependencyError(
                     "Non-primary archives only support the '%s' component." %
                     dependency.default_component.name)
+        if dependency.distribution != self.distribution:
+            raise ArchiveDependencyError(
+                "This dependency uses a different archive.")
 
+        if not dependency.enabled:
+            raise ArchiveDependencyError(
+                "This dependency is not active.")
         return ArchiveDependency(
             archive=self, dependency=dependency, pocket=pocket,
             component=component)

=== modified file 'lib/lp/soyuz/stories/ppa/xx-edit-dependencies.txt'
--- lib/lp/soyuz/stories/ppa/xx-edit-dependencies.txt	2014-07-24 09:37:03 +0000
+++ lib/lp/soyuz/stories/ppa/xx-edit-dependencies.txt	2014-08-08 17:27:51 +0000
@@ -112,7 +112,7 @@
 is rendered on top of the page and the list of dependencies available
 for removal is updated.
 
-    >>> admin_browser.getControl("Add PPA dependency").value = 'mark/ppa'
+    >>> admin_browser.getControl("Add PPA dependency").value = '~mark/ubuntu/ppa'
     >>> admin_browser.getControl("Save").click()
     >>> print_feedback_messages(admin_browser.contents)
     Dependency added: PPA for Mark Shuttleworth
@@ -123,7 +123,7 @@
 
 Trying to add a dependency that is already recorded results in a error.
 
-    >>> admin_browser.getControl("Add PPA dependency").value = 'mark/ppa'
+    >>> admin_browser.getControl("Add PPA dependency").value = '~mark/ubuntu/ppa'
     >>> admin_browser.getControl("Save").click()
     >>> print_feedback_messages(admin_browser.contents)
     There is 1 error.
@@ -132,7 +132,7 @@
 Trying to add a dependency for the context PPA itself also results in
 a error.
 
-    >>> admin_browser.getControl("Add PPA dependency").value = 'cprov/ppa'
+    >>> admin_browser.getControl("Add PPA dependency").value = '~cprov/ubuntu/ppa'
     >>> admin_browser.getControl("Save").click()
     >>> print_feedback_messages(admin_browser.contents)
     There is 1 error.
@@ -140,7 +140,7 @@
 
 If it's a new dependency everything is fine.
 
-    >>> admin_browser.getControl("Add PPA dependency").value = 'no-priv/ppa'
+    >>> admin_browser.getControl("Add PPA dependency").value = '~no-priv/ubuntu/ppa'
     >>> admin_browser.getControl("Save").click()
     >>> print_feedback_messages(admin_browser.contents)
     Dependency added: PPA for No Privileges Person
@@ -152,7 +152,7 @@
     PPA for Mark Shuttleworth
     PPA for No Privileges Person
 
-The dependencies are presented in a separated section (bellow the
+The dependencies are presented in a separated section (below the
 sources.list widget).
 
     >>> user_browser.open('http://launchpad.dev/~cprov/+archive/ubuntu/ppa')
@@ -196,7 +196,7 @@
     http://launchpad.dev/~no-priv/+archive/ubuntu/ppa
 
 When accessed by their owners, a PPA depending on disabled archives
-will additionally show an warning for uploaders . This way PPA
+will additionally show an warning for uploaders. This way a PPA
 maintainer can react to this problem.
 
     >>> cprov_browser.open('http://launchpad.dev/~cprov/+archive/ubuntu/ppa')
@@ -234,7 +234,7 @@
 
     >>> admin_browser.getControl(
     ...     name="field.selected_dependencies").value = [
-    ...     'mark/ppa', 'no-priv/ppa']
+    ...     '~mark/ubuntu/ppa', '~no-priv/ubuntu/ppa']
     >>> admin_browser.getControl("Save").click()
     >>> print_feedback_messages(admin_browser.contents)
     Dependencies removed:
@@ -258,6 +258,43 @@
     ...     user_browser.contents, 'archive-dependencies')
     None
 
+We should also make sure that a user is unable to add a disabled PPA as a
+dependency.
+
+    # Disable Mark's PPA.
+    >>> login('foo.bar@xxxxxxxxxxxxx')
+    >>> from zope.component import getUtility
+    >>> from lp.registry.interfaces.person import IPersonSet
+    >>> mark = getUtility(IPersonSet).getByName('mark')
+    >>> mark.archive.disable()
+    >>> logout()
+
+    # Attempt to add Mark's PPA
+    >>> admin_browser.getControl("Add PPA dependency").value = '~mark/ubuntu/ppa'
+    >>> admin_browser.getControl("Save").click()
+    >>> print_feedback_messages(admin_browser.contents)
+    There is 1 error.
+    Invalid value
+
+    # When the page is reloaded, there shouldn't be any dependencies.
+    >>> admin_browser.reload()
+    >>> print_ppa_dependencies(admin_browser.contents)
+    No dependencies recorded for this PPA yet.
+
+Re-enable Mark's PPA for subsequent tests.
+
+    >>> login('foo.bar@xxxxxxxxxxxxx')
+    >>> mark.archive.enable()
+    >>> logout()
+
+Clear the page.
+
+    >>> admin_browser.getControl("Add PPA dependency").value = ''
+    >>> admin_browser.getControl("Save").click()
+    >>> admin_browser.reload()
+    >>> print_ppa_dependencies(admin_browser.contents)
+    No dependencies recorded for this PPA yet.
+
 == Primary dependencies ==
 
 A user can modify how a PPA depends on its corresponding
@@ -418,9 +455,9 @@
 
 The form can perform multiple actions in a single submit.
 
-First we will create a PPA dependency for No privileged' PPA.
+First we will create a PPA dependency for 'No privileged' PPA.
 
-    >>> admin_browser.getControl("Add PPA dependency").value = 'no-priv/ppa'
+    >>> admin_browser.getControl("Add PPA dependency").value = '~no-priv/ubuntu/ppa'
     >>> admin_browser.getControl("Save").click()
     >>> print_feedback_messages(admin_browser.contents)
     Dependency added: PPA for No Privileges Person
@@ -450,12 +487,12 @@
 RELEASE.
 
     >>> admin_browser.getControl(
-    ...     name="field.selected_dependencies").value = ['no-priv/ppa']
+    ...     name="field.selected_dependencies").value = ['~no-priv/ubuntu/ppa']
 
     >>> admin_browser.getControl(
     ...     "Use all Ubuntu components available.").selected = True
 
-    >>> admin_browser.getControl("Add PPA dependency").value = 'mark/ppa'
+    >>> admin_browser.getControl("Add PPA dependency").value = '~mark/ubuntu/ppa'
 
     >>> admin_browser.getControl(
     ...     "Basic (only released packages).").selected = True
@@ -513,9 +550,9 @@
 
     >>> admin_browser.getLink('Edit PPA dependencies').click()
 
-    >>> admin_browser.getControl("Add PPA dependency").value = 'no-priv/ppa'
+    >>> admin_browser.getControl("Add PPA dependency").value = '~no-priv/ubuntu/ppa'
     >>> admin_browser.getControl(
-    ...     name="field.selected_dependencies").value = ['mark/ppa']
+    ...     name="field.selected_dependencies").value = ['~mark/ubuntu/ppa']
     >>> admin_browser.getControl(
     ...     "Default (security dependencies and recommended updates)."
     ...     ).selected = True

=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2014-08-07 07:53:27 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2014-08-08 17:27:51 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2014 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test Archive features."""
@@ -1415,6 +1415,30 @@
                 PackagePublishingPocket.RELEASE)
             self.assertContentEqual(archive.dependencies, [archive_dependency])
 
+    def test_dependency_has_different_distribution(self):
+        # A public archive may not depend on a private archive.
+        archive = self.factory.makeArchive()
+        distro = self.factory.makeDistribution()
+        dependency = self.factory.makeArchive(
+            distribution=distro, owner=archive.owner)
+        with person_logged_in(archive.owner):
+            with ExpectedException(
+                ArchiveDependencyError,
+                "This dependency uses a different archive."):
+                archive.addArchiveDependency(
+                    dependency, PackagePublishingPocket.RELEASE)
+
+    def test_dependency_is_disabled(self):
+        # A public archive may not depend on a private archive.
+        archive = self.factory.makeArchive()
+        dependency = self.factory.makeArchive(
+            owner=archive.owner, enabled=False)
+        with person_logged_in(archive.owner):
+            with ExpectedException(
+                ArchiveDependencyError,
+                "This dependency is not active."):
+                archive.addArchiveDependency(
+                    dependency, PackagePublishingPocket.RELEASE)
 
 class TestArchiveDependencies(TestCaseWithFactory):
 

=== modified file 'lib/lp/soyuz/tests/test_vocabularies.py'
--- lib/lp/soyuz/tests/test_vocabularies.py	2014-06-11 08:29:40 +0000
+++ lib/lp/soyuz/tests/test_vocabularies.py	2014-08-08 17:27:51 +0000
@@ -22,5 +22,5 @@
         term = vocab.toTerm(archive)
         self.assertThat(term, MatchesStructure.byEquality(
             value=archive,
-            token='%s/%s' % (archive.owner.name, archive.name),
+            token=archive.reference,
             title='No description available'))

=== modified file 'lib/lp/soyuz/vocabularies.py'
--- lib/lp/soyuz/vocabularies.py	2013-09-10 06:28:26 +0000
+++ lib/lp/soyuz/vocabularies.py	2014-08-08 17:27:51 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the GNU
+# Copyright 2009-2014 Canonical Ltd.  This software is licensed under the GNU
 # Affero General Public License version 3 (see the file LICENSE).
 
 """Soyuz vocabularies."""
@@ -20,6 +20,7 @@
 from zope.component import getUtility
 from zope.interface import implements
 from zope.schema.vocabulary import SimpleTerm
+from zope.security.interfaces import Unauthorized
 
 from lp.registry.model.distroseries import DistroSeries
 from lp.registry.model.person import Person
@@ -32,6 +33,7 @@
     SQLObjectVocabularyBase,
     )
 from lp.soyuz.enums import ArchivePurpose
+from lp.soyuz.interfaces.archive import IArchiveSet
 from lp.soyuz.model.archive import Archive
 from lp.soyuz.model.component import Component
 from lp.soyuz.model.distroarchseries import DistroArchSeries
@@ -88,6 +90,7 @@
     _orderBy = ['Person.name, Archive.name']
     _clauseTables = ['Person']
     _filter = And(
+        Archive._enabled == True,
         Person.q.id == Archive.q.ownerID,
         Archive.q.purpose == ArchivePurpose.PPA)
     displayname = 'Select a PPA'
@@ -95,35 +98,33 @@
 
     def toTerm(self, archive):
         """See `IVocabulary`."""
-        description = archive.description
-        if description:
-            summary = description.splitlines()[0]
-        else:
-            summary = "No description available"
+        try:
+            description = archive.description
+            if description:
+                summary = description.splitlines()[0]
+            else:
+                summary = "No description available"
+        except Unauthorized:
+            summary = None
 
-        token = '%s/%s' % (archive.owner.name, archive.name)
+        token = archive.reference
 
         return SimpleTerm(archive, token, summary)
 
     def getTermByToken(self, token):
         """See `IVocabularyTokenized`."""
         try:
-            owner_name, archive_name = token.split('/')
+            owner_name, distro_name, archive_name = token.split('/')
         except ValueError:
             raise LookupError(token)
 
-        clause = And(
-            self._filter,
-            Person.name == owner_name,
-            Archive.name == archive_name)
-
-        obj = self._table.selectOne(
-            clause, clauseTables=self._clauseTables)
-
+        obj = getUtility(IArchiveSet).getByReference(token)
         if obj is None:
-            raise LookupError(token)
-        else:
+            return LookupError(token)
+        elif obj.enabled:
             return self.toTerm(obj)
+        else:
+            raise LookupError(token)
 
     def search(self, query, vocab_filter=None):
         """Return a resultset of archives.
@@ -135,8 +136,14 @@
 
         query = query.lower()
 
+        if query.startswith('~'):
+            query = query.strip('~')
         try:
-            owner_name, archive_name = query.split('/')
+            query_split = query.split('/')
+            if len(query_split) == 3:
+                owner_name, distro_name, archive_name = query_split
+            else:
+                owner_name, archive_name = query_split
         except ValueError:
             clause = And(
                 self._filter,


Follow ups