Merge lp:~abentley/launchpad/ppa-api into lp:launchpad

[Aaron Bentley]

Summary
=======
This code was approved and merged, but then rolled back because there was validation in the browser code that was missing from the model code. The changes from the previous version are here:

http://pastebin.ubuntu.com/620212/

Original summary: Fix bug #776444 and #776449 about missing APIs

Proposed change
===============
Export APIs

Implementation Details
======================
Implemented _addArchiveDependency as a wrapper, because we do not want to export IComponent.

Also fixed the permissions of addArchiveDependency and removeArchiveDependency.

Moved the validation code from the view to the model, so that the API respects it.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/launchpad/interfaces/_schema_circular_imports.py
  lib/lp/soyuz/browser/archive.py
  lib/lp/soyuz/browser/tests/archive-views.txt
  lib/lp/soyuz/browser/tests/test_archive_webservice.py
  lib/lp/soyuz/doc/archive-dependencies.txt
  lib/lp/soyuz/doc/archive.txt
  lib/lp/soyuz/interfaces/archive.py
  lib/lp/soyuz/model/archive.py
  lib/lp/soyuz/stories/webservice/xx-archive.txt
  lib/lp/soyuz/tests/test_archive.py
  lib/lp/testing/factory.py

./lib/lp/soyuz/browser/tests/archive-views.txt
       1: narrative uses a moin header.
       9: narrative uses a moin header.
     486: narrative uses a moin header.
     529: narrative uses a moin header.
     656: narrative uses a moin header.
    1040: narrative uses a moin header.
    1317: narrative uses a moin header.
    1383: narrative uses a moin header.
./lib/lp/soyuz/doc/archive-dependencies.txt
       1: narrative uses a moin header.
       8: narrative uses a moin header.
      30: narrative uses a moin header.
      81: narrative uses a moin header.
     156: narrative uses a moin header.
     330: narrative uses a moin header.
     449: narrative uses a moin header.
     479: narrative uses a moin header.
./lib/lp/soyuz/stories/webservice/xx-archive.txt
      20: want exceeds 78 characters.
      87: want exceeds 78 characters.
     113: source exceeds 78 characters.
     120: narrative uses a moin header.
     131: source exceeds 78 characters.
     140: want exceeds 78 characters.
     145: source exceeds 78 characters.
     155: want exceeds 78 characters.
     160: source exceeds 78 characters.
     170: want exceeds 78 characters.
     175: source exceeds 78 characters.
     185: want exceeds 78 characters.
     189: narrative uses a moin header.
     272: want exceeds 78 characters.
     333: want exceeds 78 characters.
     341: narrative has trailing whitespace.
     346: source has trailing whitespace.
     374: source has trailing whitespace.
     464: want exceeds 78 characters.
     490: narrative uses a moin header.
     497: source exceeds 78 characters.
     506: source exceeds 78 characters.
     514: source exceeds 78 characters.
     520: narrative exceeds 78 characters.
     523: source exceeds 78 characters.
     530: narrative uses a moin header.
     558: narrative uses a moin header.
     587: narrative uses a moin header.
     766: narrative uses a moin header.
     768: narrative exceeds 78 characters.
     787: source exceeds 78 characters.
     797: narrative uses a moin header.
     824: want exceeds 78 characters.
     840: want exceeds 78 characters.
     854: narrative uses a moin header.
     943: narrative uses a moin header.
     956: narrative uses a moin header.
    1004: narrative uses a moin header.

Checking for conflicts and issues in changed files.

[Gavin Panella (allenap)]

Looks good :)

[1]

+ with ExpectedException(LRUnauthorized, '.*'):
+ ws_archive.lp_save()

Oh wow, that's neat.

[2]

+class InvalidExternalDependencies(Exception):
+ """Tried to set external dependencies to an invalid value."""
+
+ webservice_error(400) # Bad request.
+
+ def __init__(self, errors):
+ error_msg = 'Invalid external dependencies:\n%s\n' % '\n'.join(errors)
+ Exception.__init__(self, error_msg)

Although it's a single line of inheritance, Exception is a new style
class, so please use super() here.

review: Approve

[Aaron Bentley (abentley)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 11-06-02 11:43 AM, Gavin Panella wrote:
> Review: Approve
> Looks good :)
>
>
> [1]
>
> + with ExpectedException(LRUnauthorized, '.*'):
> + ws_archive.lp_save()
>
> Oh wow, that's neat.

Thanks. I added that to testtools myself. I like that it preserves the
normal way of writing function calls, so it's easier to read than
assertRaises.

> [2]
>
> +class InvalidExternalDependencies(Exception):
> + """Tried to set external dependencies to an invalid value."""
> +
> + webservice_error(400) # Bad request.
> +
> + def __init__(self, errors):
> + error_msg = 'Invalid external dependencies:\n%s\n' % '\n'.join(errors)
> + Exception.__init__(self, error_msg)
>
> Although it's a single line of inheritance, Exception is a new style
> class, so please use super() here.

Thanks. I guess I missed that when we switched to python 2.5.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk3nx1kACgkQ0F+nu1YWqI3i0gCggzwz0ihRu1BrK/hkHFoxTxD/
wWMAn2WOlUv7dDndqxNDmcfPEVg2+HbS
=tvir
-----END PGP SIGNATURE-----

[Graham Binns (gmb)]

r=me on the latest changes.

review: Approve (code)

=== modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
--- lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2011-06-09 08:07:52 +0000
+++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2011-06-09 17:59:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Update the interface schema values due to circular imports.
@@ -434,6 +434,14 @@
     IArchive, 'getUploadersForPackageset', 'packageset', IPackageset)
 patch_plain_parameter_type(
     IArchive, 'deletePackagesetUploader', 'packageset', IPackageset)
+patch_plain_parameter_type(
+    IArchive, 'removeArchiveDependency', 'dependency', IArchive)
+patch_plain_parameter_type(
+    IArchive, '_addArchiveDependency', 'dependency', IArchive)
+patch_choice_parameter_type(
+    IArchive, '_addArchiveDependency', 'pocket', PackagePublishingPocket)
+patch_entry_return_type(
+    IArchive, '_addArchiveDependency', IArchiveDependency)
 
 
 # IBuildFarmJob

=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2011-06-09 08:07:52 +0000
+++ lib/lp/soyuz/browser/archive.py	2011-06-09 17:59:33 +0000
@@ -33,7 +33,6 @@
     datetime,
     timedelta,
     )
-from urlparse import urlparse
 
 import pytz
 from sqlobject import SQLObjectNotFound
@@ -138,11 +137,13 @@
     PackagePublishingStatus,
     )
 from lp.soyuz.interfaces.archive import (
+    ArchiveDependencyError,
     CannotCopy,
     IArchive,
     IArchiveEditDependenciesForm,
     IArchiveSet,
     NoSuchPPA,
+    validate_external_dependencies,
     )
 from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
 from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet
@@ -1864,45 +1865,6 @@
         self._messages.append(structured(
             '<p>Primary dependency added: %s</p>', primary_dependency.title))
 
-    def validate(self, data):
-        """Validate dependency configuration changes.
-
-        Skip checks if no dependency candidate was sent in the form.
-
-        Validate if the requested PPA dependency is sane (different than
-        the context PPA and not yet registered).
-
-        Also check if the dependency candidate is private, if so, it can
-        only be set if the user has 'launchpad.View' permission on it and
-        the context PPA is also private (this way P3A credentials will be
-        sanitized from buildlogs).
-        """
-        dependency_candidate = data.get('dependency_candidate')
-
-        if dependency_candidate is None:
-            return
-
-        if dependency_candidate == self.context:
-            self.setFieldError('dependency_candidate',
-                               "An archive should not depend on itself.")
-            return
-
-        if self.context.getArchiveDependency(dependency_candidate):
-            self.setFieldError('dependency_candidate',
-                               "This dependency is already registered.")
-            return
-
-        if not check_permission('launchpad.View', dependency_candidate):
-            self.setFieldError(
-                'dependency_candidate',
-                "You don't have permission to use this dependency.")
-            return
-
-        if dependency_candidate.private and not self.context.private:
-            self.setFieldError(
-                'dependency_candidate',
-                "Public PPAs cannot depend on private ones.")
-
     @action(_("Save"), name="save")
     def save_action(self, action, data):
         """Save dependency configuration changes.
@@ -1914,18 +1876,21 @@
         refreshing. And render a page notification with the summary of the
         changes made.
         """
-        # Redirect after POST.
-        self.next_url = self.request.URL
-
         # Process the form.
         self._add_primary_dependencies(data)
-        self._add_ppa_dependencies(data)
+        try:
+            self._add_ppa_dependencies(data)
+        except ArchiveDependencyError as e:
+            self.setFieldError('dependency_candidate', str(e))
+            return
         self._remove_dependencies(data)
 
         # Issue a notification if anything was changed.
         if len(self.messages) > 0:
             self.request.response.addNotification(
                 structured(self.messages))
+        # Redirect after POST.
+        self.next_url = self.request.URL
 
 
 class ArchiveActivateView(LaunchpadFormView):
@@ -2128,7 +2093,7 @@
         # Check the external_dependencies field.
         ext_deps = data.get('external_dependencies')
         if ext_deps is not None:
-            errors = self.validate_external_dependencies(ext_deps)
+            errors = validate_external_dependencies(ext_deps)
             if len(errors) != 0:
                 error_text = "\n".join(errors)
                 self.setFieldError('external_dependencies', error_text)
@@ -2138,31 +2103,6 @@
                 'commercial',
                 'Can only set commericial for private archives.')
 
-    def validate_external_dependencies(self, ext_deps):
-        """Validate the external_dependencies field.
-
-        :param ext_deps: The dependencies form field to check.
-        """
-        errors = []
-        # The field can consist of multiple entries separated by
-        # newlines, so process each in turn.
-        for dep in ext_deps.splitlines():
-            try:
-                deb, url, suite, components = dep.split(" ", 3)
-            except ValueError:
-                errors.append(
-                    "'%s' is not a complete and valid sources.list entry"
-                        % dep)
-                continue
-
-            if deb != "deb":
-                errors.append("%s: Must start with 'deb'" % dep)
-            url_components = urlparse(url)
-            if not url_components[0] or not url_components[1]:
-                errors.append("%s: Invalid URL" % dep)
-
-        return errors
-
     @property
     def owner_is_private_team(self):
         """Is the owner a private team?

=== modified file 'lib/lp/soyuz/browser/tests/archive-views.txt'
--- lib/lp/soyuz/browser/tests/archive-views.txt	2011-06-06 02:00:42 +0000
+++ lib/lp/soyuz/browser/tests/archive-views.txt	2011-06-09 17:59:33 +0000
@@ -753,7 +753,8 @@
     mark/ppa
 
     >>> print dependency.title.escapedtext
-    <a href="http://launchpad.dev/~mark/+archive/ppa">PPA for Mark Shuttleworth</a>
+    <a href="http://launchpad.dev/~mark/+archive/ppa">PPA for Mark
+    Shuttleworth</a>
 
 The form focus, now that we have a recorded dependencies, is set to the
 first listed dependency.
@@ -1387,32 +1388,34 @@
     >>> ppa_archive_view = create_initialized_view(
     ...     cprov.archive, name="+admin")
 
-The validate_external_dependencies() method is called when validating and will
-return a list of errors if the data dis not validate.  A valid entry is of the
-form:
+    >>> from lp.soyuz.interfaces.archive import validate_external_dependencies
+
+The validate_external_dependencies() function is called when validating and
+will return a list of errors if the data dis not validate.  A valid entry is
+of the form:
     deb scheme://domain/ suite component[s]
 
-    >>> print ppa_archive_view.validate_external_dependencies(
+    >>> print validate_external_dependencies(
     ...     "deb http://example.com/ karmic main")
     []
 
 Multiple entries are valid, separated by newlines:
 
-    >>> print ppa_archive_view.validate_external_dependencies(
+    >>> print validate_external_dependencies(
     ...     "deb http://example.com/ karmic main\n"
     ...     "deb http://example.com/ karmic restricted")
     []
 
 If the line does not start with the word "deb" it fails:
 
-    >>> print ppa_archive_view.validate_external_dependencies(
+    >>> print validate_external_dependencies(
     ...     "deb http://example.com/ karmic universe\n"
     ...     "dab http://example.com/ karmic main")
     ["dab http://example.com/ karmic main: Must start with 'deb'"]
 
 If the line has too few parts it fails.  Here we're missing a suite:
 
-    >>> print ppa_archive_view.validate_external_dependencies(
+    >>> print validate_external_dependencies(
     ...     "deb http://example.com/ karmic universe\n"
     ...     "deb http://example.com/ main")
     ["'deb http://example.com/ main'
@@ -1420,7 +1423,7 @@
 
 If the URL looks invalid, it fails:
 
-    >>> print ppa_archive_view.validate_external_dependencies(
+    >>> print validate_external_dependencies(
     ...     "deb http://example.com/ karmic universe\n"
     ...     "deb example.com/ karmic main")
     ['deb example.com/ karmic main: Invalid URL']

=== modified file 'lib/lp/soyuz/browser/tests/test_archive_webservice.py'
--- lib/lp/soyuz/browser/tests/test_archive_webservice.py	2011-06-06 02:00:42 +0000
+++ lib/lp/soyuz/browser/tests/test_archive_webservice.py	2011-06-09 17:59:33 +0000
@@ -1,19 +1,30 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
 
 import unittest
 
-from lazr.restfulclient.errors import HTTPError
+from lazr.restfulclient.errors import (
+    BadRequest,
+    HTTPError,
+    Unauthorized as LRUnauthorized,
+)
+from testtools import ExpectedException
+import transaction
+from zope.component import getUtility
 
 from canonical.launchpad.testing.pages import LaunchpadWebServiceCaller
 from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.soyuz.enums import ArchivePurpose
 from lp.testing import (
     celebrity_logged_in,
     launchpadlib_for,
+    person_logged_in,
     TestCaseWithFactory,
+    WebServiceTestCase,
     )
 
 
@@ -67,5 +78,111 @@
             "in the 'DEVELOPMENT' state.", e.content)
 
 
+class TestExternalDependencies(WebServiceTestCase):
+
+    def test_external_dependencies_random_user(self):
+        """Normal users can look but not touch."""
+        archive = self.factory.makeArchive()
+        transaction.commit()
+        ws_archive = self.wsObject(archive)
+        self.assertIs(None, ws_archive.external_dependencies)
+        ws_archive.external_dependencies = "random"
+        with ExpectedException(LRUnauthorized, '.*'):
+            ws_archive.lp_save()
+
+    def test_external_dependencies_owner(self):
+        """Normal archive owners can look but not touch."""
+        archive = self.factory.makeArchive()
+        transaction.commit()
+        ws_archive = self.wsObject(archive, archive.owner)
+        self.assertIs(None, ws_archive.external_dependencies)
+        ws_archive.external_dependencies = "random"
+        with ExpectedException(LRUnauthorized, '.*'):
+            ws_archive.lp_save()
+
+    def test_external_dependencies_commercial_owner_invalid(self):
+        """Commercial admins can look and touch."""
+        commercial = getUtility(ILaunchpadCelebrities).commercial_admin
+        owner = self.factory.makePerson(member_of=[commercial])
+        archive = self.factory.makeArchive(owner=owner)
+        transaction.commit()
+        ws_archive = self.wsObject(archive, archive.owner)
+        self.assertIs(None, ws_archive.external_dependencies)
+        ws_archive.external_dependencies = "random"
+        regex = '(\n|.)*Invalid external dependencies(\n|.)*'
+        with ExpectedException(BadRequest, regex):
+            ws_archive.lp_save()
+
+    def test_external_dependencies_commercial_owner_valid(self):
+        """Commercial admins can look and touch."""
+        commercial = getUtility(ILaunchpadCelebrities).commercial_admin
+        owner = self.factory.makePerson(member_of=[commercial])
+        archive = self.factory.makeArchive(owner=owner)
+        transaction.commit()
+        ws_archive = self.wsObject(archive, archive.owner)
+        self.assertIs(None, ws_archive.external_dependencies)
+        ws_archive.external_dependencies = (
+            "deb http://example.org suite components")
+        ws_archive.lp_save()
+
+
+class TestArchiveDependencies(WebServiceTestCase):
+
+    def test_addArchiveDependency_random_user(self):
+        """Normal users cannot add archive dependencies."""
+        archive = self.factory.makeArchive()
+        dependency = self.factory.makeArchive()
+        transaction.commit()
+        ws_archive = self.wsObject(archive)
+        ws_dependency = self.wsObject(dependency)
+        self.assertContentEqual([], ws_archive.dependencies)
+        failure_regex = '(.|\n)*addArchiveDependency.*launchpad.Edit(.|\n)*'
+        with ExpectedException(LRUnauthorized, failure_regex):
+            dependency = ws_archive.addArchiveDependency(
+                dependency=ws_dependency, pocket='Release', component='main')
+
+    def test_addArchiveDependency_owner(self):
+        """Normal users cannot add archive dependencies."""
+        archive = self.factory.makeArchive()
+        dependency = self.factory.makeArchive()
+        transaction.commit()
+        ws_archive = self.wsObject(archive, archive.owner)
+        ws_dependency = self.wsObject(dependency)
+        self.assertContentEqual([], ws_archive.dependencies)
+        with ExpectedException(BadRequest, '(.|\n)*asdf(.|\n)*'):
+            ws_archive.addArchiveDependency(
+                dependency=ws_dependency, pocket='Release', component='asdf')
+        dependency = ws_archive.addArchiveDependency(
+            dependency=ws_dependency, pocket='Release', component='main')
+        self.assertContentEqual([dependency], ws_archive.dependencies)
+
+    def test_removeArchiveDependency_random_user(self):
+        """Normal users can remove archive dependencies."""
+        archive = self.factory.makeArchive()
+        dependency = self.factory.makeArchive()
+        with person_logged_in(archive.owner):
+            archive.addArchiveDependency(
+                dependency, PackagePublishingPocket.RELEASE)
+        transaction.commit()
+        ws_archive = self.wsObject(archive)
+        ws_dependency = self.wsObject(dependency)
+        failure_regex = '(.|\n)*remove.*Dependency.*launchpad.Edit(.|\n)*'
+        with ExpectedException(LRUnauthorized, failure_regex):
+            ws_archive.removeArchiveDependency(dependency=ws_dependency)
+
+    def test_removeArchiveDependency_owner(self):
+        """Normal users can remove archive dependencies."""
+        archive = self.factory.makeArchive()
+        dependency = self.factory.makeArchive()
+        with person_logged_in(archive.owner):
+            archive.addArchiveDependency(
+                dependency, PackagePublishingPocket.RELEASE)
+        transaction.commit()
+        ws_archive = self.wsObject(archive, archive.owner)
+        ws_dependency = self.wsObject(dependency)
+        ws_archive.removeArchiveDependency(dependency=ws_dependency)
+        self.assertContentEqual([], ws_archive.dependencies)
+
+
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/soyuz/doc/archive-dependencies.txt'
--- lib/lp/soyuz/doc/archive-dependencies.txt	2011-06-07 00:16:31 +0000
+++ lib/lp/soyuz/doc/archive-dependencies.txt	2011-06-09 17:59:33 +0000
@@ -281,115 +281,17 @@
     deb http://archive.launchpad.dev/ubuntu hoary-updates
         main restricted universe multiverse
 
-The authentication information gets added for private PPA
-dependencies. We'll create a private PPA and then update cprov's PPA,
-removing the dependency on Mark's public PPA and adding one for the
-new private PPA.
-
-    >>> private_ppa = factory.makeArchive(
-    ...     owner=mark, name='p3a', private=True, distribution=ubuntu)
-    >>> pub_binaries = test_publisher.getPubBinaries(
-    ...     binaryname='dep-bin', archive=private_ppa,
-    ...     status=PackagePublishingStatus.PUBLISHED)
     >>> cprov.archive.removeArchiveDependency(mark.archive)
-    >>> archive_dependency = cprov.archive.addArchiveDependency(
-    ...     private_ppa, PackagePublishingPocket.RELEASE,
-    ...     getUtility(IComponentSet)['main'])
-
-    >>> print_building_sources_list(a_build)
-    deb http://ppa.launchpad.dev/cprov/ppa/ubuntu hoary main
-    deb http://buildd:[e-mail]/mark/p3a/ubuntu
-        hoary main
-    deb http://archive.launchpad.dev/ubuntu hoary
-        main restricted universe multiverse
-    deb http://archive.launchpad.dev/ubuntu hoary-security
-        main restricted universe multiverse
-    deb http://archive.launchpad.dev/ubuntu hoary-updates
-        main restricted universe multiverse
-
-Remove the private PPA dependency before continuing.
-
-    >>> cprov.archive.removeArchiveDependency(private_ppa)
-
-    >>> cprov.archive.external_dependencies is None
-    True
-    >>> cprov.archive.enabled
-    True
-
-What happens when we fake external dependencies?
-
-    >>> cprov.archive.external_dependencies = (
-    ...     "Malformed format string here --> %(series)")
-
-The error caused by the malformed format string is caught and the other
-dependencies are returned as normal.
-
-    >>> print_building_sources_list(a_build)
-    ERROR:root:Exception during external dependency processing:
-    Traceback (most recent call last):
-      File ... in get_sources_list_for_building
-        {'series': distroarchseries.distroseries.name})
-    ValueError: incomplete format
-    <BLANKLINE>
-    deb http://ppa.launchpad.dev/cprov/ppa/ubuntu hoary main
-    deb http://archive.launchpad.dev/ubuntu hoary
-        main restricted universe multiverse
-    deb http://archive.launchpad.dev/ubuntu hoary-security
-        main restricted universe multiverse
-    deb http://archive.launchpad.dev/ubuntu hoary-updates
-        main restricted universe multiverse
-
-However, in order to avoid the problem going forward (and to allow the PPA
-owner to correct the external dependencies) the PPA will be disabled.
-
-    >>> cprov.archive.enabled
-    False
-
-Restore the 'external_dependencies' property to its original state and
-re-enable the PPA.
-
-    >>> cprov.archive.external_dependencies = None
-    >>> cprov.archive.enable()
-
-    >>> cprov.archive.external_dependencies is None
-    True
-    >>> cprov.archive.enabled
-    True
-
-What happens when we fake external dependencies?
-
-    >>> cprov.archive.external_dependencies = (
-    ...     "Malformed format string here --> %(series)")
-
-The error caused by the malformed format string is caught and the other
-dependencies are returned as normal.
-
-    >>> print_building_sources_list(a_build)
-    ERROR:root:Exception during external dependency processing:
-    Traceback (most recent call last):
-      File ... in get_sources_list_for_building
-        {'series': distroarchseries.distroseries.name})
-    ValueError: incomplete format
-    <BLANKLINE>
-    deb http://ppa.launchpad.dev/cprov/ppa/ubuntu hoary main
-    deb http://archive.launchpad.dev/ubuntu hoary
-        main restricted universe multiverse
-    deb http://archive.launchpad.dev/ubuntu hoary-security
-        main restricted universe multiverse
-    deb http://archive.launchpad.dev/ubuntu hoary-updates
-        main restricted universe multiverse
-
-However, in order to avoid the problem going forward (and to allow the PPA
-owner to correct the external dependencies) the PPA will be disabled.
-
-    >>> cprov.archive.enabled
-    False
-
-Restore the 'external_dependencies' property to its original state and
-re-enable the PPA.
-
-    >>> cprov.archive.external_dependencies = None
-    >>> cprov.archive.enable()
+
+What when we supply invalid dependencies, an exception is raised.
+
+    >>> cprov.archive.external_dependencies = (
+    ...     "Malformed format string here --> %(series)")
+    Traceback (most recent call last):
+    InvalidExternalDependencies: (InvalidExternalDependencies(...),
+    "Invalid external dependencies:\nMalformed format string here
+    --> %(series): Must start with 'deb'\nMalformed format string here
+    --> %(series): Invalid URL\n")
 
 
 == Overriding default primary archive dependencies ==

=== modified file 'lib/lp/soyuz/doc/archive.txt'
--- lib/lp/soyuz/doc/archive.txt	2011-06-06 02:00:42 +0000
+++ lib/lp/soyuz/doc/archive.txt	2011-06-09 17:59:33 +0000
@@ -101,7 +101,7 @@
 
     >>> login("[e-mail]")
     >>> cprov_archive.relative_build_score = 100
-    >>> cprov_archive.external_dependencies = "test"
+    >>> cprov_archive.external_dependencies = "deb http://foo hardy bar"
 
 The buildd_secret is used by the slave scanner when generating a
 sources.list entry for the builder to access a private archive.  It is
@@ -1043,6 +1043,7 @@
     >>> print_dependencies(no_priv.archive)
     No dependencies recorded.
 
+    >>> login_person(no_priv)
     >>> archive_dependency = no_priv.archive.addArchiveDependency(
     ...     cprov.archive, release_pocket, main_component)
 
@@ -1058,8 +1059,7 @@
     ...     cprov.archive, release_pocket, main_component)
     Traceback (most recent call last):
     ...
-    ArchiveDependencyError: Only one dependency record per archive is
-    supported.
+    ArchiveDependencyError: This dependency is already registered.
 
 'dependency' and target archive are the same.
 
@@ -1099,8 +1099,7 @@
     ...     getUtility(IComponentSet)['main'])
     Traceback (most recent call last):
     ...
-    ArchiveDependencyError: Only one dependency record per archive is
-    supported.
+    ArchiveDependencyError: This dependency is already registered.
 
 Thus archive dependency removal can be performed simply by passing the
 dependency target.

=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2011-06-07 00:16:31 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2011-06-09 17:59:33 +0000
@@ -32,6 +32,7 @@
     'IDistributionArchive',
     'InsufficientUploadRights',
     'InvalidComponent',
+    'InvalidExternalDependencies',
     'InvalidPocketForPartnerArchive',
     'InvalidPocketForPPA',
     'IPPA',
@@ -43,8 +44,12 @@
     'PocketNotFound',
     'VersionRequiresName',
     'default_name_by_purpose',
+    'validate_external_dependencies',
     ]
 
+
+from urlparse import urlparse
+
 from lazr.enum import DBEnumeratedType
 from lazr.restful.declarations import (
     call_with,
@@ -54,6 +59,7 @@
     export_read_operation,
     export_write_operation,
     exported,
+    operation_for_version,
     operation_parameters,
     operation_returns_collection_of,
     operation_returns_entry,
@@ -114,59 +120,59 @@
 
 class CannotCopy(Exception):
     """Exception raised when a copy cannot be performed."""
-    webservice_error(400) #Bad request.
+    webservice_error(400)  # Bad request.
 
 
 class CannotSwitchPrivacy(Exception):
     """Raised when switching the privacy of an archive that has
     publishing records."""
-    webservice_error(400) # Bad request.
+    webservice_error(400)  # Bad request.
 
 
 class PocketNotFound(Exception):
     """Invalid pocket."""
-    webservice_error(400) #Bad request.
+    webservice_error(400)  # Bad request.
 
 
 class DistroSeriesNotFound(Exception):
     """Invalid distroseries."""
-    webservice_error(400) #Bad request.
+    webservice_error(400)  # Bad request.
 
 
 class AlreadySubscribed(Exception):
     """Raised when creating a subscription for a subscribed person."""
-    webservice_error(400) # Bad request.
+    webservice_error(400)  # Bad request.
 
 
 class ArchiveNotPrivate(Exception):
     """Raised when creating an archive subscription for a public archive."""
-    webservice_error(400) # Bad request.
+    webservice_error(400)  # Bad request.
 
 
 class NoTokensForTeams(Exception):
     """Raised when creating a token for a team, rather than a person."""
-    webservice_error(400) # Bad request.
+    webservice_error(400)  # Bad request.
 
 
 class ComponentNotFound(Exception):
     """Invalid source name."""
-    webservice_error(400) #Bad request.
+    webservice_error(400)  # Bad request.
 
 
 class InvalidComponent(Exception):
     """Invalid component name."""
-    webservice_error(400) #Bad request.
+    webservice_error(400)  # Bad request.
 
 
 class NoSuchPPA(NameLookupFailed):
     """Raised when we try to look up an PPA that doesn't exist."""
-    webservice_error(400) #Bad request.
+    webservice_error(400)  # Bad request.
     _message_prefix = "No such ppa"
 
 
 class VersionRequiresName(Exception):
     """Raised on some queries when version is specified but name is not."""
-    webservice_error(400) # Bad request.
+    webservice_error(400)  # Bad request.
 
 
 class CannotRestrictArchitectures(Exception):
@@ -175,7 +181,7 @@
 
 class CannotUploadToArchive(Exception):
     """A reason for not being able to upload to an archive."""
-    webservice_error(403) # Forbidden.
+    webservice_error(403)  # Forbidden.
 
     _fmt = '%(person)s has no upload rights to %(archive)s.'
 
@@ -192,7 +198,7 @@
 
 class CannotUploadToPocket(Exception):
     """Returned when a pocket is closed for uploads."""
-    webservice_error(403) # Forbidden.
+    webservice_error(403)  # Forbidden.
 
     def __init__(self, distroseries, pocket):
         Exception.__init__(self,
@@ -248,6 +254,17 @@
         CannotUploadToArchive.__init__(self, archive_name=archive_name)
 
 
+class InvalidExternalDependencies(Exception):
+    """Tried to set external dependencies to an invalid value."""
+
+    webservice_error(400)  # Bad request.
+
+    def __init__(self, errors):
+        error_msg = 'Invalid external dependencies:\n%s\n' % '\n'.join(errors)
+        super(Exception, self).__init__(self, error_msg)
+        self.errors = errors
+
+
 class IArchivePublic(IHasOwner, IPrivacy):
     """An Archive interface for publicly available operations."""
     id = Attribute("The archive ID.")
@@ -332,7 +349,7 @@
 
     distribution = exported(
         Reference(
-            Interface, # Redefined to IDistribution later.
+            Interface,  # Redefined to IDistribution later.
             title=_("The distribution that uses or is used by this "
                     "archive.")))
 
@@ -412,9 +429,9 @@
             "A delta to apply to all build scores for the archive. Builds "
             "with a higher score will build sooner."))
 
-    external_dependencies = Text(
-        title=_("External dependencies"), required=False, readonly=False,
-        description=_(
+    external_dependencies = exported(
+        Text(title=_("External dependencies"), required=False,
+        readonly=False, description=_(
             "Newline-separated list of repositories to be used to retrieve "
             "any external build dependencies when building packages in the "
             "archive, in the format:\n"
@@ -422,7 +439,7 @@
                 "[components]\n"
             "The series variable is replaced with the series name of the "
             "context build.\n"
-            "NOTE: This is for migration of OEM PPAs only!"))
+            "NOTE: This is for migration of OEM PPAs only!")))
 
     enabled_restricted_families = CollectionField(
             title=_("Enabled restricted families"),
@@ -520,27 +537,6 @@
             records.
         """
 
-    def removeArchiveDependency(dependency):
-        """Remove the `IArchiveDependency` record for the given dependency.
-
-        :param dependency: is an `IArchive` object.
-        """
-
-    def addArchiveDependency(dependency, pocket, component=None):
-        """Record an archive dependency record for the context archive.
-
-        :param dependency: is an `IArchive` object.
-        :param pocket: is an `PackagePublishingPocket` enum.
-        :param component: is an optional `IComponent` object, if not given
-            the archive dependency will be tied to the component used
-            for a corresponding source in primary archive.
-
-        :raise: `ArchiveDependencyError` if given 'dependency' does not fit
-            the context archive.
-        :return: a `IArchiveDependency` object targeted to the context
-            `IArchive` requiring 'dependency' `IArchive`.
-        """
-
     def getPermissions(person, item, perm_type):
         """Get the `IArchivePermission` record with the supplied details.
 
@@ -892,7 +888,8 @@
         :return: True if the person is allowed to upload the source package.
         """
 
-    num_pkgs_building = Attribute("Tuple of packages building and waiting to build")
+    num_pkgs_building = Attribute(
+        "Tuple of packages building and waiting to build")
 
     def getSourcePackageReleases(build_status=None):
         """Return the releases for this archive.
@@ -944,7 +941,8 @@
     dependencies = exported(
         CollectionField(
             title=_("Archive dependencies recorded for this archive."),
-            value_type=Reference(schema=Interface), #Really IArchiveDependency
+            value_type=Reference(schema=Interface),
+            # Really IArchiveDependency
             readonly=True))
 
     description = exported(
@@ -1111,8 +1109,8 @@
         """
 
     @operation_parameters(
-        dependency=Reference(schema=Interface)) #Really IArchive. See below.
-    @operation_returns_entry(schema=Interface) #Really IArchiveDependency.
+        dependency=Reference(schema=Interface))  # Really IArchive. See below.
+    @operation_returns_entry(schema=Interface)  # Really IArchiveDependency.
     @export_read_operation()
     def getArchiveDependency(dependency):
         """Return the `IArchiveDependency` object for the given dependency.
@@ -1233,7 +1231,8 @@
         source_names=List(
             title=_("Source package names"),
             value_type=TextLine()),
-        from_archive=Reference(schema=Interface), #Really IArchive, see below
+        from_archive=Reference(schema=Interface),
+        #Really IArchive, see below
         to_pocket=TextLine(title=_("Pocket name")),
         to_series=TextLine(title=_("Distroseries name"), required=False),
         include_binaries=Bool(
@@ -1275,7 +1274,8 @@
     @operation_parameters(
         source_name=TextLine(title=_("Source package name")),
         version=TextLine(title=_("Version")),
-        from_archive=Reference(schema=Interface), #Really IArchive, see below
+        from_archive=Reference(schema=Interface),
+        # Really IArchive, see below
         to_pocket=TextLine(title=_("Pocket name")),
         to_series=TextLine(title=_("Distroseries name"), required=False),
         include_binaries=Bool(
@@ -1314,7 +1314,7 @@
 
     @call_with(registrant=REQUEST_USER)
     @operation_parameters(
-        subscriber = PublicPersonChoice(
+        subscriber=PublicPersonChoice(
             title=_("Subscriber"),
             required=True,
             vocabulary='ValidPersonOrTeam',
@@ -1458,6 +1458,61 @@
         processed.
         """
 
+    def addArchiveDependency(dependency, pocket, component=None):
+        """Record an archive dependency record for the context archive.
+
+        :param dependency: is an `IArchive` object.
+        :param pocket: is an `PackagePublishingPocket` enum.
+        :param component: is an optional `IComponent` object, if not given
+            the archive dependency will be tied to the component used
+            for a corresponding source in primary archive.
+
+        :raise: `ArchiveDependencyError` if given 'dependency' does not fit
+            the context archive.
+        :return: a `IArchiveDependency` object targeted to the context
+            `IArchive` requiring 'dependency' `IArchive`.
+        """
+
+    @operation_parameters(
+        dependency=Reference(schema=Interface, required=True),
+        #  Really IArchive
+        pocket=Choice(
+            title=_("Pocket"),
+            description=_("The pocket into which this entry is published"),
+            # Really PackagePublishingPocket.
+            vocabulary=DBEnumeratedType,
+            required=True),
+        component=TextLine(title=_("Component"), required=False),
+        )
+    @export_operation_as('addArchiveDependency')
+    @export_factory_operation(Interface, [])  # Really IArchiveDependency
+    @operation_for_version('devel')
+    def _addArchiveDependency(dependency, pocket, component=None):
+        """Record an archive dependency record for the context archive.
+
+        :param dependency: is an `IArchive` object.
+        :param pocket: is an `PackagePublishingPocket` enum.
+        :param component: is the name of a component.  If not given,
+            the archive dependency will be tied to the component used
+            for a corresponding source in primary archive.
+
+        :raise: `ArchiveDependencyError` if given 'dependency' does not fit
+            the context archive.
+        :return: a `IArchiveDependency` object targeted to the context
+            `IArchive` requiring 'dependency' `IArchive`.
+        """
+    @operation_parameters(
+        dependency=Reference(schema=Interface, required=True),
+        # Really IArchive
+    )
+    @export_write_operation()
+    @operation_for_version('devel')
+    def removeArchiveDependency(dependency):
+        """Remove the `IArchiveDependency` record for the given dependency.
+
+        :param dependency: is an `IArchive` object.
+        """
+
 
 class IArchive(IArchivePublic, IArchiveAppend, IArchiveEdit, IArchiveView):
     """Main Archive interface."""
@@ -1690,3 +1745,29 @@
     )
 
 # Circular dependency issues fixed in _schema_circular_imports.py
+
+
+def validate_external_dependencies(ext_deps):
+    """Validate the external_dependencies field.
+
+    :param ext_deps: The dependencies form field to check.
+    """
+    errors = []
+    # The field can consist of multiple entries separated by
+    # newlines, so process each in turn.
+    for dep in ext_deps.splitlines():
+        try:
+            deb, url, suite, components = dep.split(" ", 3)
+        except ValueError:
+            errors.append(
+                "'%s' is not a complete and valid sources.list entry"
+                    % dep)
+            continue
+
+        if deb != "deb":
+            errors.append("%s: Must start with 'deb'" % dep)
+        url_components = urlparse(url)
+        if not url_components[0] or not url_components[1]:
+            errors.append("%s: Invalid URL" % dep)
+
+    return errors

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2011-06-07 00:16:31 +0000
+++ lib/lp/soyuz/model/archive.py	2011-06-09 17:59:33 +0000
@@ -68,6 +68,7 @@
     ISlaveStore,
     IStore,
     )
+from canonical.launchpad.webapp.authorization import check_permission
 from canonical.launchpad.webapp.interfaces import (
     DEFAULT_FLAVOR,
     IStoreSelector,
@@ -125,6 +126,7 @@
     CannotSwitchPrivacy,
     CannotUploadToPocket,
     CannotUploadToPPA,
+    ComponentNotFound,
     default_name_by_purpose,
     DistroSeriesNotFound,
     FULL_COMPONENT_SUPPORT,
@@ -133,6 +135,7 @@
     IDistributionArchive,
     InsufficientUploadRights,
     InvalidComponent,
+    InvalidExternalDependencies,
     InvalidPocketForPartnerArchive,
     InvalidPocketForPPA,
     IPPA,
@@ -143,6 +146,7 @@
     NoTokensForTeams,
     PocketNotFound,
     VersionRequiresName,
+    validate_external_dependencies,
     )
 from lp.soyuz.interfaces.archivearch import IArchiveArchSet
 from lp.soyuz.interfaces.archiveauthtoken import IArchiveAuthTokenSet
@@ -197,6 +201,14 @@
 from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
 
 
+def storm_validate_external_dependencies(archive, attr, value):
+    assert attr == 'external_dependencies'
+    errors = validate_external_dependencies(value)
+    if len(errors) > 0:
+        raise InvalidExternalDependencies(errors)
+    return value
+
+
 class Archive(SQLBase):
     implements(IArchive, IHasOwner, IHasBuildRecords)
     _table = 'Archive'
@@ -306,7 +318,8 @@
     # Launchpad and should be re-examined in October 2010 to see if it
     # is still relevant.
     external_dependencies = StringCol(
-        dbName='external_dependencies', notNull=False, default=None)
+        dbName='external_dependencies', notNull=False, default=None,
+        storm_validator=storm_validate_external_dependencies)
 
     commercial = BoolCol(
         dbName='commercial', notNull=True, default=False)
@@ -483,7 +496,7 @@
 
         if name is not None:
             if exact_match:
-                storm_clauses.append(SourcePackageName.name==name)
+                storm_clauses.append(SourcePackageName.name == name)
             else:
                 clauses.append(
                     "SourcePackageName.name LIKE '%%%%' || %s || '%%%%'"
@@ -494,7 +507,7 @@
                 raise VersionRequiresName(
                     "The 'version' parameter can be used only together with"
                     " the 'name' parameter.")
-            storm_clauses.append(SourcePackageRelease.version==version)
+            storm_clauses.append(SourcePackageRelease.version == version)
         else:
             orderBy.insert(1, Desc(SourcePackageRelease.version))
 
@@ -514,7 +527,7 @@
 
         if pocket is not None:
             storm_clauses.append(
-                SourcePackagePublishingHistory.pocket==pocket)
+                SourcePackagePublishingHistory.pocket == pocket)
 
         if created_since_date is not None:
             clauses.append(
@@ -529,6 +542,7 @@
             *orderBy)
         if not eager_load:
             return resultset
+
         # Its not clear that this eager load is necessary or sufficient, it
         # replaces a prejoin that had pathological query plans.
         def eager_load(rows):
@@ -609,7 +623,7 @@
         clauseTables = ['SourcePackageRelease', 'SourcePackageName']
 
         order_const = "SourcePackageRelease.version"
-        desc_version_order = SQLConstant(order_const+" DESC")
+        desc_version_order = SQLConstant(order_const + " DESC")
         orderBy = ['SourcePackageName.name', desc_version_order,
                    '-SourcePackagePublishingHistory.id']
 
@@ -953,7 +967,14 @@
         a_dependency = self.getArchiveDependency(dependency)
         if a_dependency is not None:
             raise ArchiveDependencyError(
-                "Only one dependency record per archive is supported.")
+                "This dependency is already registered.")
+        if not check_permission('launchpad.View', dependency):
+            raise ArchiveDependencyError(
+                "You don't have permission to use this dependency.")
+            return
+        if dependency.private and not self.private:
+            raise ArchiveDependencyError(
+                "Public PPAs cannot depend on private ones.")
 
         if dependency.is_ppa:
             if pocket is not PackagePublishingPocket.RELEASE:
@@ -969,6 +990,15 @@
             archive=self, dependency=dependency, pocket=pocket,
             component=component)
 
+    def _addArchiveDependency(self, dependency, pocket, component=None):
+        """See `IArchive`."""
+        if isinstance(component, basestring):
+            try:
+                component = getUtility(IComponentSet)[component]
+            except NotFoundError as e:
+                raise ComponentNotFound(e)
+        return self.addArchiveDependency(dependency, pocket, component)
+
     def getPermissions(self, user, item, perm_type):
         """See `IArchive`."""
         permission_set = getUtility(IArchivePermissionSet)

=== modified file 'lib/lp/soyuz/stories/webservice/xx-archive.txt'
--- lib/lp/soyuz/stories/webservice/xx-archive.txt	2011-06-06 02:00:42 +0000
+++ lib/lp/soyuz/stories/webservice/xx-archive.txt	2011-06-09 17:59:33 +0000
@@ -21,6 +21,7 @@
     description: u'packages to help my friends.'
     displayname: u'PPA for Celso Providelo'
     distribution_link: u'http://.../ubuntu'
+    external_dependencies: None
     name: u'ppa'
     owner_link: u'http://.../~cprov'
     private: False
@@ -87,6 +88,7 @@
     description: None
     displayname: u'Primary Archive for Ubuntu Test'
     distribution_link: u'http://.../ubuntutest'
+    external_dependencies: None
     name: u'primary'
     owner_link: u'http://.../~ubuntu-team'
     private: False
@@ -823,6 +825,7 @@
     description: u'tag:launchpad.net:2008:redacted'
     displayname: u'PPA named p3a for Celso Providelo'
     distribution_link: u'http://.../ubuntu'
+    external_dependencies: None
     name: u'p3a'
     owner_link: u'http://.../~cprov'
     private: True
@@ -838,6 +841,7 @@
     description: u'packages to help my friends.'
     displayname: u'PPA named p3a for Celso Providelo'
     distribution_link: u'http://.../ubuntu'
+    external_dependencies: None
     name: u'p3a'
     owner_link: u'http://.../~cprov'
     private: True

=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2011-06-03 11:00:55 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2011-06-09 17:59:33 +0000
@@ -10,7 +10,9 @@
     )
 import doctest
 
-from testtools.matchers import DocTestMatches
+from testtools.matchers import DocTestMatches, MatchesRegex
+from testtools.testcase import ExpectedException
+
 import transaction
 from zope.component import getUtility
 from zope.security.interfaces import Unauthorized
@@ -48,6 +50,7 @@
     PackagePublishingStatus,
     )
 from lp.soyuz.interfaces.archive import (
+    ArchiveDependencyError,
     ArchiveDisabled,
     CannotRestrictArchitectures,
     CannotUploadToPocket,
@@ -223,7 +226,7 @@
         # Calling series_with_sources returns all series with publishings.
         distribution = self.factory.makeDistribution()
         archive = self.factory.makeArchive(distribution=distribution)
-        series_with_no_sources = self.factory.makeDistroSeries(
+        self.factory.makeDistroSeries(
             distribution=distribution, version="0.5")
         series_with_sources1 = self.factory.makeDistroSeries(
             distribution=distribution, version="1")
@@ -283,7 +286,7 @@
                 source_package_release=sourcepackagerelease,
                 archive=archive, status=status)
             sprs.append(sourcepackagerelease)
-        unlinked_spr = self.factory.makeSourcePackageRelease()
+        self.factory.makeSourcePackageRelease()
         return archive, sprs
 
     def test_getSourcePackageReleases_with_no_params(self):
@@ -425,7 +428,7 @@
         other_spn = self.factory.makeSourcePackageName(name="bar")
         archive = self.factory.makeArchive()
         self.makePublishedSources(archive,
-            [PackagePublishingStatus.PUBLISHED]*3,
+            [PackagePublishingStatus.PUBLISHED] * 3,
             ["1.0", "1.1", "2.0"],
             [sourcepackagename, sourcepackagename, other_spn])
         pubs = removeSecurityProxy(archive)._collectLatestPublishedSources(
@@ -1299,6 +1302,69 @@
         self.assertTrue(self.archive.build_debug_symbols)
 
 
+class TestAddArchiveDependencies(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_add_hidden_dependency(self):
+        # The user cannot add a dependency on an archive they cannot see.
+        archive = self.factory.makeArchive(private=True)
+        dependency = self.factory.makeArchive(private=True)
+        with person_logged_in(archive.owner):
+            with ExpectedException(
+                ArchiveDependencyError,
+                "You don't have permission to use this dependency."):
+                archive.addArchiveDependency(dependency, 'foo')
+
+    def test_private_dependency_public_archive(self):
+        # A public archive may not depend on a private archive.
+        archive = self.factory.makeArchive()
+        dependency = self.factory.makeArchive(
+            private=True, owner=archive.owner)
+        with person_logged_in(archive.owner):
+            with ExpectedException(
+                ArchiveDependencyError,
+                "Public PPAs cannot depend on private ones."):
+                archive.addArchiveDependency(dependency, 'foo')
+
+    def test_add_private_dependency(self):
+        # The user can add a dependency on private archive they can see.
+        archive = self.factory.makeArchive(private=True)
+        dependency = self.factory.makeArchive(
+            private=True, owner=archive.owner)
+        with person_logged_in(archive.owner):
+            archive_dependency = archive.addArchiveDependency(dependency,
+                PackagePublishingPocket.RELEASE)
+            self.assertContentEqual(
+                archive.dependencies, [archive_dependency])
+
+
+class TestArchiveDependencies(TestCaseWithFactory):
+
+    layer = LaunchpadZopelessLayer
+
+    def test_private_sources_list(self):
+        """Entries for private dependencies include credentials."""
+        p3a = self.factory.makeArchive(name='p3a', private=True)
+        dependency = self.factory.makeArchive(
+            name='dependency', private=True, owner=p3a.owner)
+        with person_logged_in(p3a.owner):
+            bpph = self.factory.makeBinaryPackagePublishingHistory(
+                archive=dependency, status=PackagePublishingStatus.PUBLISHED)
+            p3a.addArchiveDependency(dependency,
+                PackagePublishingPocket.RELEASE)
+            build = self.factory.makeBinaryPackageBuild(archive=p3a,
+                distroarchseries=bpph.distroarchseries)
+            sources_list = get_sources_list_for_building(
+                build, build.distro_arch_series,
+                build.source_package_release.name)
+            sources_list_str = '\n'.join(sources_list)
+            matches = MatchesRegex(
+                "deb http://buildd:[e-mail]/"
+                "person-name-.*/dependency/ubuntu distroseries-.* main")
+            self.assertThat(sources_list[0], matches)
+
+
 class TestFindDepCandidates(TestCaseWithFactory):
     """Tests for Archive.findDepCandidates."""
 
@@ -1346,7 +1412,7 @@
 
     def test_does_not_find_pending_publication(self):
         # A pending candidate in the same archive should not be found.
-        bins = self.publisher.getPubBinaries(
+        self.publisher.getPubBinaries(
             binaryname='foo', archive=self.archive)
         self.assertDep('i386', 'foo', [])
 
@@ -1572,7 +1638,7 @@
     def test_two_ppas_with_team(self):
         team = self.factory.makeTeam(
             subscription_policy=TeamSubscriptionPolicy.MODERATED)
-        ppa = self.factory.makeArchive(owner=team, name='ppa')
+        self.factory.makeArchive(owner=team, name='ppa')
         self.assertEqual("%s already has a PPA named 'ppa'." % (
             team.displayname), Archive.validatePPA(team, 'ppa'))
 

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2011-06-09 08:07:52 +0000
+++ lib/lp/testing/factory.py	2011-06-09 17:59:33 +0000
@@ -595,7 +595,7 @@
         self, email=None, name=None, password=None,
         email_address_status=None, hide_email_addresses=False,
         displayname=None, time_zone=None, latitude=None, longitude=None,
-        selfgenerated_bugnotifications=False):
+        selfgenerated_bugnotifications=False, member_of=()):
         """Create and return a new, arbitrary Person.
 
         :param email: The email address for the new person.
@@ -660,6 +660,10 @@
 
         self.makeOpenIdIdentifier(person.account)
 
+        for team in member_of:
+            with person_logged_in(team.teamowner):
+                team.addMember(person, team.teamowner)
+
         # Ensure updated ValidPersonCache
         flush_database_updates()
         return person