launchpad-reviewers team mailing list archive

[Stefano Rivera <launchpad@xxxxxxxxxxxxx>]

Stefano Rivera has proposed merging lp:~stefanor/launchpad/edit-packagesets into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #891862 in Launchpad itself: "Changing the owner of a packageset requires SQL"
  https://bugs.launchpad.net/launchpad/+bug/891862

For more details, see:
https://code.launchpad.net/~stefanor/launchpad/edit-packagesets/+merge/124555

== Summary ==

The developer membership board owns the Ubuntu packagesets, allowing the DMB to add and remove packages from them. However, the DMB cannot modify packageset descriptions or delete packagesets. These changes have have to be made by a LP admin with SQL access.
The DMB has some packagesets that are no longer needed, and has descriptions of packagesets that we'd like to set.

== Proposed Fix ==

This proposed branch exposes write access to the name, description, and owner attributes of packagesets to the API, and adds an (exposed) deletion function.

The patches for this are trivial, and minimal.

I believe that allowing packageset deletion is safe, although there are a couple of corner cases, that I could use some guidance on:

1. I emptied packagesets on deletion, but alternatively, that could abort deletion.
2. Question 1 again, but for hierarchy and upload rights. I didn't do any cleaning up here, and assumed the DB's transactional integrity would abort / cleanup here.
3. Should we disallow deletion of packagesets in stable series?
4. I believe allowing renames is safe?
5. If we change the owner of a packageset, who should own its associated packageset group? This is simple in the case of a standalone packageset, but there is no obvious owner if the group contains other packagesets.

== Complexity rationale ==

I re-wrote the packagesets' doctests as unit tests, resulting in a nett LoC reduction, and less doctests \o/

== Tests ==

In lib/lp/soyuz/stories/webservice/xx-packageset.txt and lib/lp/soyuz/tests/test_packageset.py

== QA ==

edit-acl in ubuntu-archive-tools can create packagesets, but can't be conveniently extended to modify / delete them.
Just play around with lp-shell.
-- 
https://code.launchpad.net/~stefanor/launchpad/edit-packagesets/+merge/124555
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stefanor/launchpad/edit-packagesets into lp:launchpad.

=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml	2012-07-11 13:27:35 +0000
+++ lib/lp/soyuz/configure.zcml	2012-09-15 16:53:22 +0000
@@ -801,7 +801,8 @@
             interface="lp.soyuz.interfaces.packageset.IPackagesetRestricted"/>
         <require
             permission="launchpad.Edit"
-            interface="lp.soyuz.interfaces.packageset.IPackagesetEdit"/>
+            interface="lp.soyuz.interfaces.packageset.IPackagesetEdit"
+            set_attributes="name description owner"/>
         <require
             permission="launchpad.Moderate"
             set_schema="lp.soyuz.interfaces.packageset.IPackagesetRestricted"/>

=== removed file 'lib/lp/soyuz/doc/packageset.txt'
--- lib/lp/soyuz/doc/packageset.txt	2012-04-10 14:01:17 +0000
+++ lib/lp/soyuz/doc/packageset.txt	1970-01-01 00:00:00 +0000
@@ -1,1156 +0,0 @@
-= Package sets =
-
-The `Packageset` table allows the specification of a package set. These
-facilitate the grouping of packages for purposes like the control of upload
-permissions, the calculation of build and runtime package dependencies etc.
-
-Initially, package sets will be used to enforce upload permissions to source
-packages. Later they may be put to other uses as well. Please see also the
-following URL for user stories and scenarios:
-https://dev.launchpad.net/VersionThreeDotO/Soyuz/StoryCards#packagesetacl
-
-It is also possible to define hierarchical relationships between package
-sets i.e. include package sets into other package sets and remove them
-respectively.
-
-This effectively allows the users to arrange package sets in a directed
-acyclic graph (DAG, http://en.wikipedia.org/wiki/Directed_acyclic_graph)
-where each arc/edge (A,B) carries the following meaning:
-package set 'A' includes another package set 'B' as a subset.
-
-The following passage may also make it easier to understand the nomenclature
-used (from http://en.wikipedia.org/wiki/Glossary_of_graph_theory):
-
-    "If v is reachable from u, then u is a predecessor of v and v is a
-     successor of u. If there is an arc/edge from u to v, then u is a direct
-     predecessor of v, and v is a direct successor of u."
-
-
-== Package set basics ==
-
-So, let's start by creating a few package sets.
-
-    >>> from zope.component import getUtility
-    >>> from lp.soyuz.interfaces.packageset import (
-    ...     IPackagesetSet)
-
-    >>> login('foo.bar@xxxxxxxxxxxxx')
-
-    >>> person1 = factory.makePerson(
-    ...     name='hacker', displayname=u'Happy Hacker')
-    >>> person2 = factory.makePerson(
-    ...     name='juergen', displayname=u'J\xc3\xbcrgen Schmidt',
-    ...     email='js@xxxxxxxxxxx')
-    >>> ps_factory = getUtility(IPackagesetSet)
-    >>> umbrella_ps = ps_factory.new(
-    ...     u'umbrella', u'Umbrella set, contains all packages', person1)
-    >>> kernel_ps = ps_factory.new(
-    ...     u'kernel', u'Contains all OS kernel packages', person2)
-
-Now 'juergen' and 'hacker' have a package set each.
-
-    >>> ps_factory.getByOwner(person1).count() == 1
-    True
-    >>> ps_factory.getByOwner(person2).count() == 1
-    True
-
-We need to define a few functions that make it easy to look at package set
-related data.
-
-    >>> import operator
-    >>> def sort_by_id(iterable):
-    ...     return sorted(iterable, key=operator.attrgetter('id'))
-    >>> def print_data(iterable):
-    ...     for datum in sort_by_id(iterable):
-    ...          print('%3d -> %s' % (datum.id, datum.name))
-    >>> def resultsets_are_equal(rs1, rs2):
-    ...     rs1 = list(rs1.order_by('name'))
-    ...     rs2 = list(rs2.order_by('name'))
-    ...     return rs1 == rs2
-
-
-Package sets can be looked up by name as follows:
-
-    >>> umbrella = ps_factory['umbrella']
-    >>> print_data((umbrella,))
-    1 -> umbrella
-
-In order to facilitate utilisation of package set related functionality
-via the web services API we need to have a get() method that returns the
-first N (N=50 by default) package sets sorted by name.
-Since we only have 2 package sets at this point only these will be shown.
-
-    >>> for datum in ps_factory.get():
-    ...     print('%3d -> %s' % (datum.id, datum.name))
-    2 -> kernel
-    1 -> umbrella
-
-In a next step we will associate source package names with the package sets
-just created.
-
-    >>> from lp.services.webapp.interfaces import (
-    ...     IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR, MASTER_FLAVOR)
-    >>> from lp.registry.model.sourcepackagename import SourcePackageName
-    >>> store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
-
-First associate *all* source package names with the umbrella package set.
-
-    >>> all_spns = store.find(SourcePackageName)
-    >>> umbrella_ps.add(all_spns)
-
-Let's see what we got:
-
-    >>> umbrella_spns = umbrella_ps.sourcesIncluded(direct_inclusion=True)
-    >>> umbrella_src_names = sorted(spn.name for spn in umbrella_spns)
-    >>> print_data(umbrella_spns)
-     1 -> mozilla-firefox
-     9 -> evolution
-    10 -> netapplet
-    14 -> pmount
-    15 -> a52dec
-    16 -> mozilla
-    17 -> at
-    18 -> thunderbird
-    19 -> alsa-utils
-    20 -> cnews
-    21 -> libstdc++
-    22 -> linux-source-2.6.15
-    23 -> foobar
-    24 -> cdrkit
-    25 -> language-pack-de
-    26 -> iceweasel
-    27 -> commercialpackage
-
-Now let's put a few selected source package names into the 'kernel' package
-set:
-
-    >>> kernel_spns = store.find(
-    ...     SourcePackageName, SourcePackageName.name.like('li%'))
-    >>> kernel_ps.add(kernel_spns)
-    >>> kernel_spns = kernel_ps.sourcesIncluded(direct_inclusion=True)
-    >>> print_data(kernel_spns)
-    21 -> libstdc++
-    22 -> linux-source-2.6.15
-
-Adding source package names to a package set repeatedly has no effect.
-
-    >>> umbrella_ps.add(kernel_spns)
-    >>> umbrella_spns2 = umbrella_ps.sourcesIncluded(direct_inclusion=True)
-    >>> print_data(umbrella_spns2)
-     1 -> mozilla-firefox
-     9 -> evolution
-    10 -> netapplet
-    14 -> pmount
-    15 -> a52dec
-    16 -> mozilla
-    17 -> at
-    18 -> thunderbird
-    19 -> alsa-utils
-    20 -> cnews
-    21 -> libstdc++
-    22 -> linux-source-2.6.15
-    23 -> foobar
-    24 -> cdrkit
-    25 -> language-pack-de
-    26 -> iceweasel
-    27 -> commercialpackage
-
-Removing source package names is easy.
-
-    >>> umbrella_ps.remove(kernel_spns)
-    >>> remaining_spns = umbrella_ps.sourcesIncluded(direct_inclusion=True)
-
-The 'umbrella' package set includes all source names. From that set the
-'kernel' package set (that includes merely two source names) is deducted.
-What the following shows is that the package set substraction method works.
-
-    >>> print_data(remaining_spns)
-     1 -> mozilla-firefox
-     9 -> evolution
-    10 -> netapplet
-    14 -> pmount
-    15 -> a52dec
-    16 -> mozilla
-    17 -> at
-    18 -> thunderbird
-    19 -> alsa-utils
-    20 -> cnews
-    23 -> foobar
-    24 -> cdrkit
-    25 -> language-pack-de
-    26 -> iceweasel
-    27 -> commercialpackage
-
-Trying to remove source package names that are *not* associated with a
-package set from the latter has no effect.
-
-    >>> umbrella_ps.remove(kernel_spns)
-    >>> remaining_spns = umbrella_ps.sourcesIncluded(direct_inclusion=True)
-    >>> print_data(remaining_spns)
-     1 -> mozilla-firefox
-     9 -> evolution
-    10 -> netapplet
-    14 -> pmount
-    15 -> a52dec
-    16 -> mozilla
-    17 -> at
-    18 -> thunderbird
-    19 -> alsa-utils
-    20 -> cnews
-    23 -> foobar
-    24 -> cdrkit
-    25 -> language-pack-de
-    26 -> iceweasel
-    27 -> commercialpackage
-
-Add the removed source package names back to 'umbrella'.
-
-    >>> umbrella_ps.add(kernel_spns)
-
-
-== Package set hierarchies ==
-
-The next step in organizing package sets is to arrange them in a hierarchy
-i.e. for package sets to include others as subsets.
-
-We need more package sets to play with however.
-
-    >>> gnome_ps = ps_factory.new(
-    ...     u'gnome', u'Contains all gnome desktop packages', person2)
-    >>> mozilla_ps = ps_factory.new(
-    ...     u'mozilla', u'Contains all mozilla packages', person2)
-    >>> firefox_ps = ps_factory.new(
-    ...     u'firefox', u'Contains all firefox packages', person2)
-    >>> thunderbird_ps = ps_factory.new(
-    ...     u'thunderbird', u'Contains all thunderbird packages', person2)
-    >>> languagepack_ps = ps_factory.new(
-    ...     u'languagepack', u'Contains all language packs', person2)
-    >>> store.commit()
-
-Now we can set up the package set hierarchy.
-
-    >>> umbrella_ps.add((gnome_ps,))
-    >>> mozilla_ps.add((firefox_ps, thunderbird_ps, languagepack_ps))
-    >>> umbrella_ps.add((mozilla_ps,))
-    >>> gnome_ps.add((languagepack_ps,))
-
-The 'umbrella' package set has two *direct* successors..
-
-    >>> print_data(umbrella_ps.setsIncluded(direct_inclusion=True))
-     3 -> gnome
-     4 -> mozilla
-
-.. but five successors in total. The 'firefox' and 'thunderbird' package
-sets are included via 'mozilla' whereas the 'languagepack' package set comes
-in via 'gnome' and/or 'mozilla'.
-
-    >>> u_successors = umbrella_ps.setsIncluded()
-    >>> print_data(u_successors)
-     3 -> gnome
-     4 -> mozilla
-     5 -> firefox
-     6 -> thunderbird
-     7 -> languagepack
-
-These are the *direct* predecessors of the 'languagepack' package set.
-
-    >>> print_data(languagepack_ps.setsIncludedBy(direct_inclusion=True))
-     3 -> gnome
-     4 -> mozilla
-
-These are *all* predecessors of the 'languagepack' package set. Please not
-that the 'umbrella' package set is listed as well.
-
-    >>> print_data(languagepack_ps.setsIncludedBy())
-     1 -> umbrella
-     3 -> gnome
-     4 -> mozilla
-
-When 'mozilla' stops including 'languagepack' it is still included by
-'umbrella' (via the 'gnome' package set).
-
-    >>> mozilla_ps.remove((languagepack_ps,))
-    >>> print_data(mozilla_ps.setsIncluded())
-      5 -> firefox
-      6 -> thunderbird
-
-    >>> print_data(languagepack_ps.setsIncludedBy(direct_inclusion=True))
-     3 -> gnome
-    >>> print_data(languagepack_ps.setsIncludedBy())
-     1 -> umbrella
-     3 -> gnome
-
-The 'umbrella' successors are still the same ('languagepack' is still
-included via 'gnome').
-
-    >>> sort_by_id(u_successors) == sort_by_id(umbrella_ps.setsIncluded())
-    True
-
-Removing direct successors is pretty tolerant i.e. if you try to remove 'Q'
-from 'P' and 'Q' is *not* a *direct* successor of 'P' nothing will happen.
-
-    >>> umbrella_ps.remove((languagepack_ps,))
-    >>> sort_by_id(u_successors) == sort_by_id(umbrella_ps.setsIncluded())
-    True
-
-What happens if we remove a package set from the database?
-
-    >>> print_data(umbrella_ps.setsIncluded())
-      3 -> gnome
-      4 -> mozilla
-      5 -> firefox
-      6 -> thunderbird
-      7 -> languagepack
-
-    >>> store = getUtility(IStoreSelector).get(MAIN_STORE, MASTER_FLAVOR)
-    >>> store.remove(gnome_ps)
-
-We removed the 'gnome' package set and see that all the relationships it
-participated in were cleaned up as well.
-
-It does not show up as a predecessor for the 'languagepack' package set
-any more.
-
-    >>> print_data(languagepack_ps.setsIncludedBy(direct_inclusion=True))
-    >>> print_data(languagepack_ps.setsIncludedBy())
-
-It is also not included by the 'umbrella' package set any longer. Please
-note that 'languagepack' also ceased to be included by 'umbrella' because
-the link between them ('gnome') is gone.
-
-    >>> print_data(umbrella_ps.setsIncluded())
-      4 -> mozilla
-      5 -> firefox
-      6 -> thunderbird
-
-
-== Package set hierarchies and source names ==
-
-In order to demonstrate how included package sets play together
-with source package names we'll "populate" the former.
-
-    >>> def populate(packageset, pattern):
-    ...     rs = store.find(
-    ...         SourcePackageName, SourcePackageName.name.like(pattern))
-    ...     packageset.add(rs)
-
-    >>> populate(mozilla_ps, 'moz%a')
-    >>> print_data(mozilla_ps.sourcesIncluded(direct_inclusion=True))
-    16 -> mozilla
-
-    >>> populate(firefox_ps, '%fire%')
-    >>> populate(firefox_ps, '%ice%')
-    >>> print_data(firefox_ps.sourcesIncluded(direct_inclusion=True))
-     1 -> mozilla-firefox
-    26 -> iceweasel
-
-If we wanted the string source names as opposed to the `ISourcePackageName`
-instances we could get them as follows:
-
-    >>> sorted(firefox_ps.getSourcesIncluded(direct_inclusion=True))
-    [u'iceweasel', u'mozilla-firefox']
-
-Populate the 'thunderbird' package set with sources.
-
-    >>> populate(thunderbird_ps , '%thunder%')
-    >>> populate(thunderbird_ps , '%ice%')
-    >>> print_data(thunderbird_ps.sourcesIncluded(direct_inclusion=True))
-    18 -> thunderbird
-    26 -> iceweasel
-
-When looking at *all* source package names the 'mozilla' package set is
-associated with we see
-
-    * 'mozilla' i.e. the source package name it is *directly* associated
-      with but also
-    * the union set of source package names of its successor package sets
-
-    >>> print_data(mozilla_ps.sourcesIncluded())
-     1 -> mozilla-firefox
-    16 -> mozilla
-    18 -> thunderbird
-    26 -> iceweasel
-
-We can get the string source package names as follows:
-
-    >>> sorted(mozilla_ps.getSourcesIncluded())
-    [u'iceweasel', u'mozilla', u'mozilla-firefox', u'thunderbird']
-
-We extend the package set hierarchy by including 'languagepack' into
-'thunderbird' ..
-
-    >>> populate(languagepack_ps , 'lang%')
-    >>> thunderbird_ps.add((languagepack_ps,))
-
-.. and see that the 'thunderbird' package set is (indirectly) associated
-with the 'language-pack-de' source package name.
-
-    >>> print_data(thunderbird_ps.sourcesIncluded(direct_inclusion=True))
-    18 -> thunderbird
-    26 -> iceweasel
-    >>> print_data(thunderbird_ps.sourcesIncluded())
-    18 -> thunderbird
-    25 -> language-pack-de
-    26 -> iceweasel
-
-Furthermore, the 'language-pack-de' source package name is picked up by its
-predecessor 'mozilla' as well.
-
-    >>> print_data(mozilla_ps.sourcesIncluded())
-     1 -> mozilla-firefox
-    16 -> mozilla
-    18 -> thunderbird
-    25 -> language-pack-de
-    26 -> iceweasel
-
-Let's see what sources the 'umbrella' and the 'mozilla' package set have in
-common:
-
-    >>> print_data(umbrella_ps.sourcesSharedBy(mozilla_ps))
-      1 -> mozilla-firefox
-     16 -> mozilla
-     18 -> thunderbird
-     25 -> language-pack-de
-     26 -> iceweasel
-
-The same shared sources (but in string form) are obtained as follows:
-
-    >>> sorted(umbrella_ps.getSourcesSharedBy(mozilla_ps))
-    [u'iceweasel', u'language-pack-de', u'mozilla', u'mozilla-firefox',
-     u'thunderbird']
-
-If we ask the question the other way around the answer should be the same.
-
-    >>> print_data(mozilla_ps.sourcesSharedBy(umbrella_ps))
-      1 -> mozilla-firefox
-     16 -> mozilla
-     18 -> thunderbird
-     25 -> language-pack-de
-     26 -> iceweasel
-
-Now we only want to see the directly included sources they have in common.
-
-    >>> print_data(
-    ...     umbrella_ps.sourcesSharedBy(mozilla_ps, direct_inclusion=True))
-     16 -> mozilla
-
-The same shared sources (but in string form) are obtained as follows:
-
-    >>> sorted(
-    ...     umbrella_ps.getSourcesSharedBy(mozilla_ps, direct_inclusion=True))
-    [u'mozilla']
-
-Again, asking the question the other way around works as well.
-
-    >>> print_data(
-    ...     mozilla_ps.sourcesSharedBy(umbrella_ps, direct_inclusion=True))
-     16 -> mozilla
-
-How many sources are in the 'mozilla' package set but not in 'umbrella'?
-
-    >>> mozilla_ps.sourcesNotSharedBy(umbrella_ps).count()
-    0
-
-    >>> len(list(mozilla_ps.getSourcesNotSharedBy(umbrella_ps)))
-    0
-
-What sources are included by the 'mozilla' package set but not by 'firefox'?
-
-    >>> print_data(mozilla_ps.sourcesNotSharedBy(firefox_ps))
-     16 -> mozilla
-     18 -> thunderbird
-     25 -> language-pack-de
-
-     >>> sorted(mozilla_ps.getSourcesNotSharedBy(firefox_ps))
-     [u'language-pack-de', u'mozilla', u'thunderbird']
-
-What sources are *directly* included by 'mozilla' but not by 'firefox'?
-
-    >>> print_data(
-    ...     mozilla_ps.sourcesNotSharedBy(firefox_ps, direct_inclusion=True))
-     16 -> mozilla
-
-    >>> sorted(
-    ...     mozilla_ps.getSourcesNotSharedBy(
-    ...         firefox_ps, direct_inclusion=True))
-    [u'mozilla']
-
-Sometimes it's interesting to see what package sets include a certain
-source package name.
-
-    >>> from lp.soyuz.interfaces.packageset import (
-    ...     IPackagesetSet)
-    >>> from lp.registry.interfaces.sourcepackagename import (
-    ...     ISourcePackageNameSet)
-
-Which package sets include 'mozilla-firefox' either directly or indirectly?
-
-    >>> firefox_spn = getUtility(ISourcePackageNameSet)['mozilla-firefox']
-    >>> ps_set = getUtility(IPackagesetSet)
-    >>> print_data(ps_set.setsIncludingSource(firefox_spn))
-      1 -> umbrella
-      4 -> mozilla
-      5 -> firefox
-
-Which package sets include 'mozilla-firefox' directly? Remember that
-'umbrella' includes *all* source package names directly.
-
-    >>> print_data(
-    ...     ps_set.setsIncludingSource(firefox_spn, direct_inclusion=True))
-      1 -> umbrella
-      5 -> firefox
-
-We can filter the package sets by series:
-
-    >>> from lp.registry.interfaces.distribution import IDistributionSet
-    >>> ubuntu = getUtility(IDistributionSet)['ubuntu']
-    >>> print_data(
-    ...     ps_set.setsIncludingSource(firefox_spn,
-    ...                                distroseries=ubuntu['hoary']))
-      1 -> umbrella
-      4 -> mozilla
-      5 -> firefox
-    >>> print_data(
-    ...     ps_set.setsIncludingSource(firefox_spn,
-    ...                                distroseries=ubuntu['warty']))
-   
-It is also possible to ask the same question by providing the mere name of
-the source package.
-
-    >>> print_data(
-    ...     ps_set.setsIncludingSource('mozilla-firefox',
-    ...     direct_inclusion=True))
-      1 -> umbrella
-      5 -> firefox
-
-If the source package for a given name cannot be found an exception is
-raised.
-
-    >>> print_data(ps_set.setsIncludingSource('this-will-fail'))
-    Traceback (most recent call last):
-    ...
-    NoSuchSourcePackageName: No such source package: 'this-will-fail'.
-
-    >>> store.commit()
-
-
-== Various errors ==
-
-Here's what happens if we try to add something that is not a source package
-name or package set:
-
-    >>> mozilla_ps.add('This will fail'.split())
-    Traceback (most recent call last):
-    ...
-    AssertionError: Not all data was handled.
-
-Likewise for removal:
-
-    >>> mozilla_ps.remove(range(10))
-    Traceback (most recent call last):
-    ...
-    AssertionError: Not all data was handled.
-
-An attempt to add cycles to the package set graph also results in a failure:
-
-    >>> mozilla_ps.add((umbrella_ps,))
-    Traceback (most recent call last):
-    ...
-    InternalError: Package set umbrella already includes mozilla.
-    Adding (mozilla -> umbrella) would introduce a cycle in the
-    package set graph (DAG).
-    <BLANKLINE>
-
-    >>> store.rollback()
-
-== Amending package sets ==
-
-There are some methods that will enable the caller to add and delete
-package sets.  They currently require launchpad.Edit permission to
-use, which enforces the user to be an admin or a member of the "techboard"
-(Ubuntu Technical Board) team.
-
-    >>> from zope.security.checker import canAccess
-    >>> from zope.security.proxy import removeSecurityProxy
-    >>> from lp.app.interfaces.launchpad import ILaunchpadCelebrities
-    >>> from lp.registry.interfaces.person import IPersonSet
-    >>> from lp.registry.interfaces.teammembership import (
-    ...     TeamMembershipStatus)
-    >>> restricted_methods = ('new',)
-    >>> techboard = getUtility(ILaunchpadCelebrities).ubuntu_techboard
-    >>> techboard = removeSecurityProxy(techboard)
-
-Ordinary users have no access:
-
-    >>> login('js@xxxxxxxxxxx')
-    >>> canAccess(ps_factory, 'new')
-    False
-
-Admins have access:
-
-    >>> login("foo.bar@xxxxxxxxxxxxx")
-    >>> canAccess(ps_factory, 'new')
-    True
-
-Now add "test@xxxxxxxxxxxxx" to the techboard team and log in as him.
-
-    >>> ignored = techboard.addMember(
-    ...     person2, reviewer=person2, status=TeamMembershipStatus.APPROVED,
-    ...     force_team_add=True)
-    >>> ignored = login_person(person2)
-
-Create a new package set.
-
-    >>> kde_ps = ps_factory.new(
-    ...     u'kde', u'Contains all KDE packages', person2)
-
-
-== Package sets and permissions ==
-
-As it stands package sets will first be used for governing source package
-uploads i.e. in conjunction with `ArchivePermission` data.
-
-    >>> from lp.soyuz.enums import ArchivePermissionType
-    >>> from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
-    >>> ap_set = getUtility(IArchivePermissionSet)
-
-So, let's assign upload permissions for the 'mozilla' package set to our
-happy hacker.
-
-    >>> def print_permission(result_set):
-    ...     for perm in result_set.order_by(
-    ...         'person, permission, packageset, explicit'):
-    ...         person = perm.person.name
-    ...         pset = perm.packageset.name
-    ...         permission = perm.permission.name
-    ...         archive = perm.archive.name
-    ...         if perm.explicit == True:
-    ...             ptype = 'explicit'
-    ...         else:
-    ...             ptype = 'implicit'
-    ...         print(
-    ...             '%10s %10s: %12s -> %16s (%s)'
-    ...             % (archive, person, pset, permission, ptype))
-
-Introduce a copy archive that will be used to disambiguate archive
-permissions.
-
-    >>> from lp.soyuz.enums import ArchivePurpose
-    >>> from lp.soyuz.interfaces.archive import IArchiveSet
-    >>> rebuild_archive = getUtility(IArchiveSet).new(
-    ...     owner=person1, purpose=ArchivePurpose.COPY,
-    ...     distribution=ubuntu, name='copy-archive',
-    ...     enabled=False, require_virtualized=False)
-
-Since we just created the copy archive there will be no permissions
-associated with it.
-
-    >>> permissions = ap_set.packagesetsForUploader(rebuild_archive, person1)
-    >>> print_permission(permissions)
-
-Next we set up a permission for the Ubuntu main archive.
-
-    >>> ubuntu_archive = ubuntu.main_archive
-    >>> ignore_this = ap_set.newPackagesetUploader(
-    ...     ubuntu_archive, person1, mozilla_ps)
-
-Now we see that person 'hacker' has upload permissions to the 'mozilla'
-package set.
-
-    >>> permissions = ap_set.packagesetsForUploader(
-    ...     ubuntu_archive, person1)
-    >>> print_permission(permissions)
-    primary     hacker:      mozilla ->           UPLOAD (implicit)
-
-The generic checkAuthenticated() method works as well.
-
-    >>> permissions = ap_set.checkAuthenticated(
-    ...     person1, ubuntu_archive, ArchivePermissionType.UPLOAD,
-    ...     mozilla_ps)
-    >>> permission = permissions[0]
-
-    >>> print permission.person.name
-    hacker
-    >>> print permission.package_set_name
-    mozilla
-    >>> print permission.permission.name
-    UPLOAD
-    >>> permission.explicit
-    False
-
-Since the permission above was granted for 'hacker' on the main Ubuntu
-archive the copy archive still has no permissions associated with it.
-
-    >>> nothing = ap_set.packagesetsForUploader(rebuild_archive, person1)
-    >>> print_permission(nothing)
-
-'juergen' is now getting granted upload permissions for the same package set
-but for the copy archive.
-
-    >>> ignore_this = ap_set.newPackagesetUploader(
-    ...     rebuild_archive, person2, mozilla_ps)
-
-    >>> print_permission(ap_set.uploadersForPackageset(
-    ...     rebuild_archive, mozilla_ps))
-    copy-archive    juergen:      mozilla ->           UPLOAD (implicit)
-
-Now 'hacker' may upload packages associated with the 'mozilla' package set
-in the Ubuntu main archive ..
-
-    >>> ap_set.isSourceUploadAllowed(
-    ...     ubuntu_archive, 'mozilla-firefox', person1)
-    True
-
-but not in the copy archive.
-
-    >>> ap_set.isSourceUploadAllowed(
-    ...     rebuild_archive, 'mozilla-firefox', person1)
-    False
-
-Conversely 'juergen' is entitled to uploading 'mozilla' packages in the copy
-archive ..
-
-    >>> ap_set.isSourceUploadAllowed(
-    ...     rebuild_archive, 'mozilla-firefox', person2)
-    True
-
-but not in the Ubuntu main archive.
-
-    >>> ap_set.isSourceUploadAllowed(
-    ...     ubuntu_archive, 'mozilla-firefox', person2)
-    False
-
-The 'package_set_name' property allows easy access to the package set
-name.
-
-    >>> [archp] = permissions
-    >>> archp.package_set_name
-    u'mozilla'
-
-'hacker' is also listed as one of the 'mozilla' uploaders. Please note that
-the upload privilege to the same package set but in a different archive does
-not show in the listing below since we only want to see permissions that
-apply to the Ubuntu archive.
-
-    >>> ignore_this = ap_set.newPackagesetUploader(
-    ...     rebuild_archive, person1, mozilla_ps)
-
-    >>> print_permission(ap_set.uploadersForPackageset(
-    ...     ubuntu_archive, mozilla_ps))
-    primary     hacker:      mozilla ->           UPLOAD (implicit)
-
-    >>> print_permission(
-    ...     ap_set.packagesetsForUploader(ubuntu_archive, person1))
-    primary     hacker:      mozilla ->           UPLOAD (implicit)
-
-    >>> print_permission(
-    ...     ap_set.packagesetsForSourceUploader(
-    ...         ubuntu_archive, 'mozilla-firefox', person1))
-    primary     hacker:      mozilla ->           UPLOAD (implicit)
-
-'hacker' has upload privileges for 'mozilla' in the copy archive.
-
-    >>> print_permission(ap_set.uploadersForPackageset(
-    ...     rebuild_archive, mozilla_ps))
-    copy-archive     hacker:      mozilla ->           UPLOAD (implicit)
-    copy-archive    juergen:      mozilla ->           UPLOAD (implicit)
-
-Now we delete them..
-
-    >>> ap_set.deletePackagesetUploader(
-    ...     rebuild_archive, person1, mozilla_ps)
-
-.. and the 'hacker' privileges are gone.
-
-    >>> print_permission(ap_set.uploadersForPackageset(
-    ...     rebuild_archive, mozilla_ps))
-    copy-archive    juergen:      mozilla ->           UPLOAD (implicit)
-
-The analogous permissions for the Ubuntu archive are unaffected by the
-deletion.
-
-    >>> print_permission(ap_set.uploadersForPackageset(
-    ...     ubuntu_archive, mozilla_ps))
-    primary     hacker:      mozilla ->           UPLOAD (implicit)
-
-Furthermore, 'hacker' will be listed as an uploader for any included
-package set as long as the latter does not have its own permission with
-the 'explicit' flag set.
-
-    >>> print_data(mozilla_ps.setsIncluded())
-      5 -> firefox
-      6 -> thunderbird
-      7 -> languagepack
-
-When we ask for the 'firefox' uploaders, 'hacker' will not be listed although
-'mozilla' includes 'firefox'.
-
-    >>> print_permission(ap_set.uploadersForPackageset(
-    ...     ubuntu_archive, firefox_ps))
-
-If we ask for uploaders while considering the inclusions between package
-sets, 'hacker' will be listed as an uploader for 'firefox' by virtue of
-the fact that the latter is included by 'mozilla' and 'hacker' is an
-uploader for mozilla.
-
-    >>> print_permission(
-    ...     ap_set.uploadersForPackageset(
-    ...         ubuntu_archive, firefox_ps, direct_permissions=False))
-    primary     hacker:      mozilla ->           UPLOAD (implicit)
-
-If we add a permission for 'firefox' things will stay the same i.e. 'hacker'
-is still listed as an uploader. Please note the different package sets
-('mozilla' and 'firefox') although we are looking for 'firefox' uploaders.
-
-    >>> ignore_this = ap_set.newPackagesetUploader(
-    ...     ubuntu_archive, person2, firefox_ps)
-
-Please note also how any permissions granted for the copy archive are
-ignored in the listing below since we are asking for permissions applying
-to the Ubuntu archive.
-
-    >>> ignore_this = ap_set.newPackagesetUploader(
-    ...     rebuild_archive, person2, firefox_ps)
-    >>> print_permission(
-    ...     ap_set.uploadersForPackageset(
-    ...         ubuntu_archive, firefox_ps, direct_permissions=False))
-    primary     hacker:      mozilla ->           UPLOAD (implicit)
-    primary    juergen:      firefox ->           UPLOAD (implicit)
-
-Once 'mozilla' stops including 'firefox', user 'hacker' is not listed as
-an uploader for 'firefox'.
-
-    >>> mozilla_ps.remove((firefox_ps,))
-    >>> print_data(mozilla_ps.setsIncluded())
-      6 -> thunderbird
-      7 -> languagepack
-
-    >>> print_permission(
-    ...     ap_set.uploadersForPackageset(
-    ...         ubuntu_archive, firefox_ps, direct_permissions=False))
-    primary    juergen:      firefox ->           UPLOAD (implicit)
-
-Ulploaders with explicit permissions will be listed along with the other
-uploaders.
-
-    >>> mark = getUtility(IPersonSet).getByName("mark")
-    >>> ignore_this = ap_set.newPackagesetUploader(
-    ...     ubuntu_archive, mark, firefox_ps, True)
-    >>> print_permission(
-    ...     ap_set.uploadersForPackageset(ubuntu_archive, firefox_ps))
-    primary     mark:      firefox ->           UPLOAD (explicit)
-    primary    juergen:      firefox ->           UPLOAD (implicit)
-
-Persons can have both explicit or non-explicit permissions for package sets.
-
-    >>> ignore_this = ap_set.newPackagesetUploader(
-    ...     ubuntu_archive, mark, thunderbird_ps)
-    >>> print_permission(
-    ...     ap_set.packagesetsForUploader(ubuntu_archive, mark))
-    primary     mark:      firefox ->           UPLOAD (explicit)
-    primary     mark:  thunderbird ->           UPLOAD (implicit)
-
-Sometimes it's handy to know what permissions apply for a given source
-package irrespective of the person.
-
-    >>> print_permission(
-    ...     ap_set.packagesetsForSource(ubuntu_archive, 'mozilla-firefox'))
-    primary     mark:      firefox ->           UPLOAD (explicit)
-    primary    juergen:      firefox ->           UPLOAD (implicit)
-
-If we make the 'mozilla' package set include 'firefox' again and ask the same
-question without insisiting on direct permissions we also see the permission
-granting 'hacker' upload privileges to 'mozilla' (since the latter is now
-a parent package set of 'firefox' and that includes the 'mozilla-firefox'
-source package (directly)).
-
-    >>> mozilla_ps.add((firefox_ps,))
-    >>> print_permission(
-    ...     ap_set.packagesetsForSource(
-    ...         ubuntu_archive, 'mozilla-firefox', direct_permissions=False))
-    primary     mark:      firefox ->           UPLOAD (explicit)
-    primary     hacker:      mozilla ->           UPLOAD (implicit)
-    primary    juergen:      firefox ->           UPLOAD (implicit)
-
-Attempting to create an explicit permission when a non-explicit one exists
-already will fail.
-
-    >>> ignore_this = ap_set.newPackagesetUploader(
-    ...     ubuntu_archive, mark, thunderbird_ps, True)
-    Traceback (most recent call last):
-    ...
-    ValueError: Permission for package set 'thunderbird' already exists for
-    mark but with a different 'explicit' flag value (False).
-
-And the other way around:
-
-    >>> ignore_this = ap_set.newPackagesetUploader(
-    ...     ubuntu_archive, mark, firefox_ps)
-    Traceback (most recent call last):
-    ...
-    ValueError: Permission for package set 'firefox' already exists for
-    mark but with a different 'explicit' flag value (True).
-
-Removing package set based permissions is straight-forward.
-
-    >>> ap_set.deletePackagesetUploader(
-    ...     ubuntu_archive, mark, firefox_ps, True)
-    >>> print_permission(
-    ...     ap_set.packagesetsForUploader(ubuntu_archive, mark))
-    primary     mark:  thunderbird ->           UPLOAD (implicit)
-
-
-    >>> ap_set.deletePackagesetUploader(
-    ...     ubuntu_archive, mark, thunderbird_ps)
-    >>> print_permission(ap_set.packagesetsForUploader(
-    ...     ubuntu_archive, mark))
-
-An attempt to look up package set based permission by something else than
-by a package set (name) results in a failure.
-
-    >>> print_permission(ap_set.uploadersForPackageset(
-    ...     ubuntu_archive, 12345))
-    Traceback (most recent call last):
-    ...
-    ValueError: Not a package set: int
-
-Create a package set based permission for a team.
-
-    >>> techboardp = (
-    ...     ap_set.newPackagesetUploader(
-    ...         ubuntu_archive, techboard, kde_ps))
-
-An attempt to create a new permission for a 'techboard' team member
-  * for the same package set
-  * but with a conflicting 'explicit' flag value
-will fail.
-
-    >>> ignore_this = ap_set.newPackagesetUploader(
-    ...     ubuntu_archive, person2, kde_ps, True)
-    Traceback (most recent call last):
-    ...
-    ValueError: Permission for package set 'kde' already exists for
-    techboard but with a different 'explicit' flag value (False).
-
-An attempt to create the same permission repeatedly should just return
-the existing one.
-
-    >>> sameone = (
-    ...     ap_set.newPackagesetUploader(
-    ...         ubuntu_archive, techboard, kde_ps))
-    >>> techboardp.id == sameone.id
-    True
-
-Creating a "compatible" permission for person 'juergen' succeeds although
-there is one in place for 'techboard' already (and 'juergen' belongs to
-the 'techboard' team).
-
-    >>> print_permission(
-    ...     ap_set.uploadersForPackageset(ubuntu_archive, kde_ps))
-    primary  techboard:          kde ->           UPLOAD (implicit)
-
-
-    >>> ignore_this = ap_set.newPackagesetUploader(
-    ...     ubuntu_archive, person2, kde_ps)
-
-    >>> print_permission(
-    ...     ap_set.uploadersForPackageset(ubuntu_archive, kde_ps))
-    primary  techboard:          kde ->           UPLOAD (implicit)
-    primary    juergen:          kde ->           UPLOAD (implicit)
-
-
-== Checking package set based upload permissions ==
-
-The 'mozilla' package set includes the 'firefox' subset and hence the
-'mozilla-firefox' source package name indirectly.
-
-    >>> mozilla_ps.add((firefox_ps,))
-    >>> print_data(mozilla_ps.sourcesIncluded())
-      1 -> mozilla-firefox
-     16 -> mozilla
-     18 -> thunderbird
-     25 -> language-pack-de
-     26 -> iceweasel
-    >>> print_data(mozilla_ps.sourcesIncluded(direct_inclusion=True))
-     16 -> mozilla
-
-'hacker' is authorized to upload to the 'mozilla' package set..
-
-    >>> print_permission(
-    ...     ap_set.uploadersForPackageset(ubuntu_archive, mozilla_ps))
-    primary     hacker:      mozilla ->           UPLOAD (implicit)
-
-.. and hence listed as a *potential* uploader of the 'mozilla-firefox'
-source package.
-
-    >>> print_permission(
-    ...     ap_set.packagesetsForSourceUploader(
-    ...         ubuntu_archive, 'mozilla-firefox', person1))
-    primary     hacker:      mozilla ->           UPLOAD (implicit)
-
-'juergen' is allowed to upload to the 'firefox' package set that includes
-the 'mozilla-firefox' source package name directly ..
-
-    >>> print_data(firefox_ps.sourcesIncluded(direct_inclusion=True))
-      1 -> mozilla-firefox
-     26 -> iceweasel
-
-    >>> print_permission(
-    ...     ap_set.uploadersForPackageset(ubuntu_archive, firefox_ps))
-    primary    juergen:      firefox ->           UPLOAD (implicit)
-
-.. and thus also listed as a possible uploader of the 'mozilla-firefox'
-source package.
-
-    >>> print_permission(
-    ...     ap_set.packagesetsForSourceUploader(
-    ...         ubuntu_archive, 'mozilla-firefox', person2))
-    primary    juergen:      firefox ->           UPLOAD (implicit)
-
-Fetching the permissions for a non-existent source package name will fail
-as follows:
-
-    >>> print_permission(
-    ...     ap_set.packagesetsForSourceUploader(
-    ...         ubuntu_archive, 'vapour-ware', person2))
-    Traceback (most recent call last):
-    ...
-    NoSuchSourcePackageName: No such source package: 'vapour-ware'.
-
-Now for the verdict: is 'hacker' allowed to upload 'mozilla-firefox'?
-
-    >>> ap_set.isSourceUploadAllowed(
-    ...     ubuntu_archive, 'mozilla-firefox', person1)
-    True
-
-How about 'juergen'?
-
-    >>> ap_set.isSourceUploadAllowed(
-    ...     ubuntu_archive, 'mozilla-firefox', person2)
-    True
-
-And the unprivileged user?
-
-    >>> unprivileged = getUtility(IPersonSet).getByName("no-priv")
-    >>> ap_set.isSourceUploadAllowed(
-    ...     ubuntu_archive, 'mozilla-firefox', unprivileged)
-    False
-
-So, what we see is that the package set inclusion hierarchy is honored as
-long as there are no explicit permissions for the package sets involved.
-
-Let's
-
-  * create a super-special package set
-  * populate it with the 'mozilla-firefox' package
-  * grant an explicit permission to the unprivileged person
-
-and see how that changes things.
-
-    >>> login("foo.bar@xxxxxxxxxxxxx")
-    >>> specialist_ps = ps_factory.new(
-    ...     u'specialists-only', u'Packages that require special care.',
-    ...     person1)
-    >>> store.commit()
-
-    >>> specialist_ps.add((firefox_spn,))
-    >>> print_data(
-    ...     ps_set.setsIncludingSource('mozilla-firefox',
-    ...     direct_inclusion=True))
-      1 -> umbrella
-      5 -> firefox
-      9 -> specialists-only
-
-    >>> ignore_this = ap_set.newPackagesetUploader(
-    ...     ubuntu_archive, unprivileged, specialist_ps, True)
-
-Please note that there's a package set now that includes 'mozilla-firefox'
-and has an explicit permission.
-
-    >>> for ps in sort_by_id(ps_set.setsIncludingSource(
-    ...     'mozilla-firefox', direct_inclusion=True)):
-    ...     print_permission(
-    ...         ap_set.uploadersForPackageset(ubuntu_archive, ps))
-    primary    juergen:      firefox     ->           UPLOAD (implicit)
-    primary    no-priv: specialists-only ->           UPLOAD (explicit)
-
-Is 'hacker' still allowed to upload 'mozilla-firefox'?
-
-    >>> ap_set.isSourceUploadAllowed(
-    ...     ubuntu_archive, 'mozilla-firefox', person1)
-    False
-
-How about 'juergen'?
-
-    >>> ap_set.isSourceUploadAllowed(
-    ...     ubuntu_archive, 'mozilla-firefox', person2)
-    False
-
-Now neither 'hacker' nor 'juergen' are allowed to upload. All non-explicit
-permissions are ignored in the presence of explicit ones.
-
-The 'unprivileged' person who holds an explicit permission for the
-'specialists-only' package set (including the 'mozilla-firefox' source)
-is allowed to upload.
-
-    >>> ap_set.isSourceUploadAllowed(
-    ...     ubuntu_archive, 'mozilla-firefox', unprivileged)
-    True
-
-
-== Methods for the Launchpad web services API ==
-
-The following methods are used to expose the package set functionality
-via the Launchpad web services API.
-
-Let's create a few package sets first.
-
-    >>> gnome_ps = ps_factory.new(
-    ...     u'gnome', u'Contains all gnome desktop packages', person2)
-    >>> xwin_ps = ps_factory.new(
-    ...     u'x-win', u'Contains all X windows packages', person2)
-    >>> universe_ps = ps_factory.new(
-    ...     u'universe', u'Contains all universe packages', person2)
-    >>> multiverse_ps = ps_factory.new(
-    ...     u'multiverse', u'Contains all multiverse packages', person2)
-
-The new package sets are added to 'umbrella' by passing their names. Please
-note that non-existent package sets (e.g. 'not-there') are simply ignored.
-
-    >>> to_be_added = (
-    ...     u'gnome', u'x-win', u'universe', u'multiverse', u'not-there')
-    >>> umbrella_ps.addSubsets(to_be_added)
-    >>> print_data(umbrella_ps.setsIncluded(direct_inclusion=True))
-      4 -> mozilla
-     10 -> gnome
-     11 -> x-win
-     12 -> universe
-     13 -> multiverse
-
-Package subsets can be removed in a similar fashion. Non-existent sets
-or sets which are not (direct) subsets are ignored again.
-
-    >>> to_be_removed = (u'umbrella', u'universe', u'multiverse', u'not-mine')
-    >>> umbrella_ps.removeSubsets(to_be_removed)
-    >>> print_data(umbrella_ps.setsIncluded(direct_inclusion=True))
-      4 -> mozilla
-     10 -> gnome
-     11 -> x-win
-
-Source package names can be added by merely specifying their names.
-
-    >>> print_data(mozilla_ps.sourcesIncluded(direct_inclusion=True))
-     16 -> mozilla
-
-    >>> mozilla_ps.addSources(('cdrkit', 'foobar', 'emacs'))
-    >>> print_data(mozilla_ps.sourcesIncluded(direct_inclusion=True))
-     16 -> mozilla
-     23 -> foobar
-     24 -> cdrkit
-
-It also possible to remove source package names by their names.
-
-    >>> mozilla_ps.removeSources(('mozilla', 'zope', 'firefox'))
-    >>> print_data(mozilla_ps.sourcesIncluded(direct_inclusion=True))
-     23 -> foobar
-     24 -> cdrkit

=== modified file 'lib/lp/soyuz/interfaces/packageset.py'
--- lib/lp/soyuz/interfaces/packageset.py	2012-05-17 16:24:42 +0000
+++ lib/lp/soyuz/interfaces/packageset.py	2012-09-15 16:53:22 +0000
@@ -22,6 +22,7 @@
     export_as_webservice_collection,
     export_as_webservice_entry,
     export_factory_operation,
+    export_operation_as,
     export_read_operation,
     export_write_operation,
     exported,
@@ -71,7 +72,7 @@
         description=_("The creation date/time for the package set at hand.")))
 
     owner = exported(Reference(
-        IPerson, title=_("Person"), required=True, readonly=True,
+        IPerson, title=_("Person"), required=True,
         description=_("The person who owns this package set.")))
 
     name = exported(TextLine(
@@ -79,7 +80,7 @@
         required=True, constraint=name_validator))
 
     description = exported(TextLine(
-        title=_("Description"), required=True, readonly=True,
+        title=_("Description"), required=True,
         description=_("The description for the package set at hand.")))
 
     distroseries = exported(Reference(
@@ -347,6 +348,12 @@
         :param names: an iterable with string package set names
         """
 
+    @export_write_operation()
+    @export_operation_as('delete')
+    @operation_for_version('devel')
+    def destroySelf():
+        """Delete the package set."""
+
 
 class IPackagesetRestricted(Interface):
     """A writeable interface for restricted attributes of package sets."""

=== modified file 'lib/lp/soyuz/model/packageset.py'
--- lib/lp/soyuz/model/packageset.py	2012-06-13 11:22:35 +0000
+++ lib/lp/soyuz/model/packageset.py	2012-09-15 16:53:22 +0000
@@ -329,6 +329,16 @@
             Packageset.id != self.id)
         return _order_result_set(result_set)
 
+    def destroySelf(self):
+        store = IStore(Packageset)
+        sources = store.find(
+            PackagesetSources,
+            PackagesetSources.packageset == self)
+        sources.remove()
+        store.remove(self)
+        if self.relatedSets().is_empty():
+            store.remove(self.packagesetgroup)
+
 
 class PackagesetSet:
     """See `IPackagesetSet`."""

=== modified file 'lib/lp/soyuz/stories/webservice/xx-packageset.txt'
--- lib/lp/soyuz/stories/webservice/xx-packageset.txt	2012-06-11 23:57:35 +0000
+++ lib/lp/soyuz/stories/webservice/xx-packageset.txt	2012-09-15 16:53:22 +0000
@@ -14,7 +14,7 @@
 The actual package set *functionality* is tested in much greater detail
 here:
 
-    lib/lp/soyuz/doc/packageset.txt
+    lb/lp/soyuz/tests/test_packageset.py
 
 Please refer to the tests contained in the file above if you are really
 interested in package sets and the complete functionality they offer.
@@ -83,6 +83,37 @@
     HTTP/1.1 404 Not Found
     ...
 
+Let's create another set.
+
+    >>> response = webservice.named_post(
+    ...     '/package-sets', 'new', {},
+    ...     name=u'shortlived', description=u'An ephemeral packageset',
+    ...     owner=name12['self_link'])
+    >>> print response
+    HTTP/1.1 201 Created
+    ...
+
+We can modify it, and even give it away.
+
+    >>> from simplejson import dumps
+    >>> name16 = webservice.get("/~name16").jsonBody()
+    >>> patch = {u'name': 'renamed',
+    ...          u'description': u'Repurposed packageset',
+    ...          u'owner_link': name16['self_link']}
+    >>> response = webservice.patch(
+    ...     '/package-sets/hoary/shortlived', 'application/json', dumps(patch))
+    >>> print response
+    HTTP/1.1 301 Moved Permanently
+    ...
+
+And then delete it.
+
+    >>> response = webservice.named_post(
+    ...     '/package-sets/hoary/renamed', 'delete', {}, api_version='devel')
+    >>> print response
+    HTTP/1.1 200 Ok
+    ...
+
 Populate the 'umbrella' package set with source packages.
 
     >>> from lp.services.webapp.interfaces import (

=== modified file 'lib/lp/soyuz/tests/test_packageset.py'
--- lib/lp/soyuz/tests/test_packageset.py	2012-01-01 02:58:52 +0000
+++ lib/lp/soyuz/tests/test_packageset.py	2012-09-15 16:53:22 +0000
@@ -4,21 +4,41 @@
 """Test Packageset features."""
 
 from zope.component import getUtility
+from zope.security.interfaces import Unauthorized
 
+from lp.app.errors import NotFoundError
+from lp.registry.errors import NoSuchSourcePackageName
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.series import SeriesStatus
+from lp.services.database.lpstorm import IStore
+from lp.soyuz.enums import ArchivePermissionType
+from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
 from lp.soyuz.interfaces.packageset import (
     DuplicatePackagesetName,
     IPackagesetSet,
-    )
-from lp.testing import TestCaseWithFactory
-from lp.testing.layers import ZopelessDatabaseLayer
+    NoSuchPackageSet,
+    )
+from lp.soyuz.model.packagesetgroup import PackagesetGroup
+from lp.testing import (
+    TestCaseWithFactory,
+    admin_logged_in,
+    celebrity_logged_in,
+    person_logged_in,
+    )
+from lp.testing.layers import (
+    DatabaseFunctionalLayer,
+    ZopelessDatabaseLayer,
+    )
 
 
 class TestPackagesetSet(TestCaseWithFactory):
 
     layer = ZopelessDatabaseLayer
 
+    def setUp(self):
+        super(TestPackagesetSet, self).setUp()
+        self.ps_set = getUtility(IPackagesetSet)
+
     def getUbuntu(self):
         """Get the Ubuntu `Distribution`."""
         return getUtility(IDistributionSet).getByName('ubuntu')
@@ -32,7 +52,7 @@
     def test_new_defaults_to_current_distroseries(self):
         # If the distroseries is not provided, the current development
         # distroseries will be assumed.
-        packageset = getUtility(IPackagesetSet).new(
+        packageset = self.ps_set.new(
             self.factory.getUniqueUnicode(), self.factory.getUniqueUnicode(),
             self.factory.makePerson())
         self.failUnlessEqual(
@@ -41,7 +61,7 @@
     def test_new_with_specified_distroseries(self):
         # A distroseries can be provided when creating a package set.
         experimental_series = self.makeExperimentalSeries()
-        packageset = getUtility(IPackagesetSet).new(
+        packageset = self.ps_set.new(
             self.factory.getUniqueUnicode(), self.factory.getUniqueUnicode(),
             self.factory.makePerson(), distroseries=experimental_series)
         self.failUnlessEqual(experimental_series, packageset.distroseries)
@@ -51,7 +71,7 @@
         # group with the same owner.
         owner = self.factory.makePerson()
         experimental_series = self.makeExperimentalSeries()
-        packageset = getUtility(IPackagesetSet).new(
+        packageset = self.ps_set.new(
             self.factory.getUniqueUnicode(), self.factory.getUniqueUnicode(),
             owner, distroseries=experimental_series)
         self.failUnlessEqual(owner, packageset.packagesetgroup.owner)
@@ -63,7 +83,7 @@
         name = self.factory.getUniqueUnicode()
         self.factory.makePackageset(name, distroseries=distroseries)
         self.assertRaises(
-            DuplicatePackagesetName, getUtility(IPackagesetSet).new,
+            DuplicatePackagesetName, self.ps_set.new,
             name, self.factory.getUniqueUnicode(), self.factory.makePerson(),
             distroseries=distroseries)
 
@@ -72,7 +92,7 @@
         # series is no problem.
         name = self.factory.getUniqueUnicode()
         packageset1 = self.factory.makePackageset(name)
-        packageset2 = getUtility(IPackagesetSet).new(
+        packageset2 = self.ps_set.new(
             name, self.factory.getUniqueUnicode(), self.factory.makePerson(),
             distroseries=self.factory.makeDistroSeries())
         self.assertEqual(packageset1.name, packageset2.name)
@@ -97,7 +117,7 @@
         self.factory.makePackageset(
             name, distroseries=self.makeExperimentalSeries(),
             related_set=pset1)
-        self.assertEqual(pset1, getUtility(IPackagesetSet).getByName(name))
+        self.assertEqual(pset1, self.ps_set.getByName(name))
 
     def test_get_by_name_in_specified_distroseries(self):
         # IPackagesetSet.getByName() will return the package set in the
@@ -107,7 +127,7 @@
         pset1 = self.factory.makePackageset(name)
         pset2 = self.factory.makePackageset(
             name, distroseries=experimental_series, related_set=pset1)
-        pset_found = getUtility(IPackagesetSet).getByName(
+        pset_found = self.ps_set.getByName(
             name, distroseries=experimental_series)
         self.assertEqual(pset2, pset_found)
 
@@ -120,8 +140,7 @@
             distroseries=self.makeExperimentalSeries())
         self.assertContentEqual(
             package_sets_for_current_ubuntu,
-            getUtility(IPackagesetSet).getBySeries(
-                self.getUbuntu().currentseries))
+            self.ps_set.getBySeries(self.getUbuntu().currentseries))
 
     def test_getForPackages_returns_packagesets(self):
         # getForPackages finds package sets for given source package
@@ -133,7 +152,7 @@
         packageset.addSources([package.name])
         self.assertEqual(
             {package.id: [packageset]},
-            getUtility(IPackagesetSet).getForPackages(series, [package.id]))
+            self.ps_set.getForPackages(series, [package.id]))
 
     def test_getForPackages_filters_by_distroseries(self):
         # getForPackages does not return packagesets for different
@@ -144,9 +163,7 @@
         package = self.factory.makeSourcePackageName()
         packageset.addSources([package.name])
         self.assertEqual(
-            {},
-            getUtility(IPackagesetSet).getForPackages(
-                other_series, [package.id]))
+            {}, self.ps_set.getForPackages(other_series, [package.id]))
 
     def test_getForPackages_filters_by_sourcepackagename(self):
         # getForPackages does not return packagesets for different
@@ -157,9 +174,106 @@
         other_package = self.factory.makeSourcePackageName()
         packageset.addSources([package.name])
         self.assertEqual(
-            {},
-            getUtility(IPackagesetSet).getForPackages(
-                series, [other_package.id]))
+            {}, self.ps_set.getForPackages(series, [other_package.id]))
+
+    def test_getByOwner(self):
+        # Sets can be looked up by owner
+        person = self.factory.makePerson()
+        self.factory.makePackageset(owner=person)
+        self.assertEqual(self.ps_set.getByOwner(person).count(), 1)
+
+    def test_dict_access(self):
+        # The packagesetset acts as a dictionary
+        packageset = self.factory.makePackageset()
+        self.assertEqual(self.ps_set[packageset.name], packageset)
+
+    def test_list(self):
+        # get returns the first N (N=50 by default) package sets sorted by name
+        # for iterating packagesets over the web services API
+        psets = [self.factory.makePackageset() for i in range(5)]
+        psets.sort(key=lambda p: p.name)
+
+        self.assertEqual(list(self.ps_set.get()), psets)
+
+    def buildSimpleHierarchy(self, series=None):
+        parent = self.factory.makePackageset(distroseries=series)
+        child = self.factory.makePackageset(distroseries=series)
+        package = self.factory.makeSourcePackageName()
+        parent.add((child,))
+        child.add((package,))
+        return parent, child, package
+
+    def test_sets_including_source(self):
+        # Returns the list of sets including a source package
+        parent, child, package = self.buildSimpleHierarchy()
+        self.assertEqual(
+            sorted(self.ps_set.setsIncludingSource(package)),
+            sorted((parent, child)))
+
+        # And can be limited to direct inclusion
+        result = self.ps_set.setsIncludingSource(
+            package, direct_inclusion=True)
+        self.assertEqual(list(result), [child])
+
+    def test_sets_including_source_same_series(self):
+        # setsIncludingSource by default searches the current series, but a
+        # series can be specified
+        series = self.factory.makeDistroSeries()
+        parent, child, package = self.buildSimpleHierarchy(series)
+        result = self.ps_set.setsIncludingSource(
+            package, distroseries=series)
+        self.assertEqual(sorted(result), sorted([parent, child]))
+
+    def test_sets_including_source_different_series(self):
+        # searches are limited to one series
+        parent, child, package = self.buildSimpleHierarchy()
+        series = self.factory.makeDistroSeries()
+        result = self.ps_set.setsIncludingSource(
+            package, distroseries=series)
+        self.assertTrue(result.is_empty())
+
+    def test_sets_including_source_by_name(self):
+        # Returns the list osf sets including a source package
+        parent, child, package = self.buildSimpleHierarchy()
+        self.assertEqual(
+            sorted(self.ps_set.setsIncludingSource(package.name)),
+            sorted([parent, child]))
+
+    def test_sets_including_source_unknown_name(self):
+        # A non-existent package name will throw an exception
+        self.assertRaises(
+            NoSuchSourcePackageName,
+            self.ps_set.setsIncludingSource, 'this-will-fail')
+
+
+class TestPackagesetSetPermissions(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestPackagesetSetPermissions, self).setUp()
+        self.ps_set = getUtility(IPackagesetSet)
+
+    def test_create_packageset_as_user(self):
+        # Normal users can't create packagesets
+        with person_logged_in(self.factory.makePerson()):
+            self.assertRaises(Unauthorized, getattr, self.ps_set, 'new')
+
+    def test_create_packagset_as_techboard(self):
+        # Ubuntu techboard members can create packagesets
+        with celebrity_logged_in('ubuntu_techboard'):
+            self.ps_set.new(
+                self.factory.getUniqueUnicode(),
+                self.factory.getUniqueUnicode(),
+                self.factory.makePerson())
+
+    def test_create_packagset_as_admin(self):
+        # Admins can create packagesets
+        with admin_logged_in():
+            self.ps_set.new(
+                self.factory.getUniqueUnicode(),
+                self.factory.getUniqueUnicode(),
+                self.factory.makePerson())
 
 
 class TestPackageset(TestCaseWithFactory):
@@ -226,3 +340,681 @@
         # Unsurprisingly, the unrelated package set is not associated with any
         # other package set.
         self.failUnlessEqual(pset3.relatedSets().count(), 0)
+
+    def test_destroy(self):
+        pset = self.packageset_set.new(
+            u'kernel', u'Contains all OS kernel packages', self.person1)
+        pset.destroySelf()
+        self.assertRaises(NoSuchPackageSet, self.packageset_set.getByName,
+                          u'kernel')
+
+        # Did we clean up the single packagesetgroup?
+        store = IStore(PackagesetGroup)
+        result_set = store.find(PackagesetGroup)
+        self.assertTrue(result_set.is_empty())
+
+    def test_destroy_with_ancestor(self):
+        ancestor = self.packageset_set.new(
+            u'kernel', u'Contains all OS kernel packages', self.person1)
+        pset = self.packageset_set.new(
+            u'kernel', u'Contains all OS kernel packages', self.person1,
+            distroseries=self.distroseries_experimental, related_set=ancestor)
+        pset.destroySelf()
+        self.assertRaises(
+            NoSuchPackageSet, self.packageset_set.getByName,
+            u'kernel', distroseries=self.distroseries_experimental)
+
+    def test_destroy_with_packages(self):
+        pset = self.packageset_set.new(
+            u'kernel', u'Contains all OS kernel packages', self.person1)
+        package = self.factory.makeSourcePackageName()
+        pset.addSources([package.name])
+
+        pset.destroySelf()
+        self.assertRaises(NoSuchPackageSet, self.packageset_set.getByName,
+                          u'kernel')
+
+    def test_destroy_child(self):
+        parent = self.packageset_set.new(
+            u'core', u'Contains all the important packages', self.person1)
+        child = self.packageset_set.new(
+            u'kernel', u'Contains all OS kernel packages', self.person1)
+        parent.add((child,))
+
+        child.destroySelf()
+        self.assertRaises(NoSuchPackageSet, self.packageset_set.getByName,
+                          u'kernel')
+        self.assertTrue(parent.setsIncluded(direct_inclusion=True).is_empty())
+
+    def test_destroy_parent(self):
+        parent = self.packageset_set.new(
+            u'core', u'Contains all the important packages', self.person1)
+        child = self.packageset_set.new(
+            u'kernel', u'Contains all OS kernel packages', self.person1)
+        parent.add((child,))
+
+        parent.destroySelf()
+        self.assertRaises(NoSuchPackageSet, self.packageset_set.getByName,
+                          u'core')
+        self.assertTrue(child.setsIncludedBy(direct_inclusion=True).is_empty())
+
+    def test_destroy_intermidate(self):
+        # Destroying an intermediate packageset severs the indirect inclusion
+        parent = self.factory.makePackageset()
+        child = self.factory.makePackageset()
+        grandchild = self.factory.makePackageset()
+        parent.add((child,))
+        child.add((grandchild,))
+        self.assertEqual(parent.setsIncluded().count(), 2)
+
+        child.destroySelf()
+        self.assertRaises(NoSuchPackageSet, self.packageset_set.getByName,
+                          child.name)
+        self.assertTrue(parent.setsIncluded().is_empty())
+
+    def buildSet(self, size=5):
+        packageset = self.factory.makePackageset()
+        packages = [self.factory.makeSourcePackageName() for i in range(size)]
+        packageset.add(packages)
+        return packageset, packages
+
+    def test_sources_included(self):
+        # Lists the source packages included in a set
+        packageset, packages = self.buildSet()
+        self.assertEqual(
+            sorted(packageset.sourcesIncluded()), sorted(packages))
+
+    def test_get_sources_included(self):
+        # Lists the names of source packages included in a set
+        packageset, packages = self.buildSet()
+        self.assertEqual(
+            sorted(packageset.getSourcesIncluded()),
+            sorted(p.name for p in packages))
+
+    def test_sources_included_indirect(self):
+        # sourcesIncluded traverses the set tree, by default
+        packageset1, packages1 = self.buildSet()
+        packageset2, packages2 = self.buildSet()
+        packageset1.add((packageset2,))
+        self.assertEqual(
+            sorted(packageset1.sourcesIncluded()),
+            sorted(packages1 + packages2))
+
+        # direct_inclusion disables traversal
+        self.assertEqual(
+            sorted(packageset1.sourcesIncluded(direct_inclusion=True)),
+            sorted(packages1))
+
+    def test_sources_multiply_included(self):
+        # Source packages included in multiple packagesets in a tree are only
+        # listed once.
+        packageset1, packages1 = self.buildSet(5)
+        packageset2, packages2 = self.buildSet(5)
+        packageset1.add(packages2[:2])
+        packageset1.add((packageset2,))
+        self.assertEqual(
+            sorted(packageset1.sourcesIncluded(direct_inclusion=True)),
+            sorted(packages1 + packages2[:2]))
+        self.assertEqual(
+            sorted(packageset1.sourcesIncluded()),
+            sorted(packages1 + packages2))
+
+    def test_add_already_included_sources(self):
+        # Adding source packages to a package set repeatedly has no effect
+        packageset, packages = self.buildSet()
+        packageset.add(packages)
+        self.assertEqual(
+            sorted(packageset.sourcesIncluded()), sorted(packages))
+
+    def test_remove_sources(self):
+        # Source packages can be removed from a set
+        packageset, packages = self.buildSet(5)
+        packageset.remove(packages[:2])
+        self.assertEqual(
+            sorted(packageset.sourcesIncluded()), sorted(packages[2:]))
+
+    def test_remove_non_preset_sources(self):
+        # Trying to remove source packages that are *not* in the set, has no
+        # effect.
+        packageset, packages = self.buildSet()
+        packageset.remove([self.factory.makeSourcePackageName()])
+        self.assertTrue(sorted(packageset.sourcesIncluded()), sorted(packages))
+
+    def test_sets_included(self):
+        # Returns the sets included in a set
+        parent = self.factory.makePackageset()
+        child = self.factory.makePackageset()
+        parent.add((child,))
+        grandchild = self.factory.makePackageset()
+        child.add((grandchild,))
+        self.assertEqual(
+            sorted(parent.setsIncluded()), sorted([child, grandchild]))
+        self.assertEqual(
+            list(parent.setsIncluded(direct_inclusion=True)), [child])
+
+    def test_sets_included_multipath(self):
+        # A set can be included by multiple paths, but will only appear once in
+        # setsIncluded
+        parent = self.factory.makePackageset()
+        child = self.factory.makePackageset()
+        child2 = self.factory.makePackageset()
+        parent.add((child, child2))
+        grandchild = self.factory.makePackageset()
+        child.add((grandchild,))
+        child2.add((grandchild,))
+        self.assertEqual(
+            sorted(parent.setsIncluded()), sorted([child, child2, grandchild]))
+
+    def test_sets_included_by(self):
+        # Returns the set of sets including a set
+        parent = self.factory.makePackageset()
+        child = self.factory.makePackageset()
+        parent.add((child,))
+        grandchild = self.factory.makePackageset()
+        child.add((grandchild,))
+        self.assertEqual(
+            sorted(grandchild.setsIncludedBy()), sorted([child, parent]))
+        self.assertEqual(
+            list(grandchild.setsIncludedBy(direct_inclusion=True)), [child])
+
+    def test_remove_subset(self):
+        # A set can be removed from another set
+        parent = self.factory.makePackageset()
+        child = self.factory.makePackageset()
+        child2 = self.factory.makePackageset()
+        parent.add((child, child2))
+        self.assertEqual(
+            sorted(parent.setsIncluded()), sorted([child, child2]))
+        parent.remove((child,))
+        self.assertEqual(list(parent.setsIncluded()), [child2])
+
+    def test_remove_indirect_subset(self):
+        # Removing indirect successors has no effect.
+        parent = self.factory.makePackageset()
+        child = self.factory.makePackageset()
+        grandchild = self.factory.makePackageset()
+        parent.add((child,))
+        child.add((grandchild,))
+        self.assertEqual(
+            sorted(parent.setsIncluded()), sorted([child, grandchild]))
+        parent.remove((grandchild,))
+        self.assertEqual(
+            sorted(parent.setsIncluded()), sorted([child, grandchild]))
+
+    def test_sources_shared_by(self):
+        # Lists the source packages shared between two packagesets
+        pset1, pkgs1 = self.buildSet(5)
+        pset2, pkgs2 = self.buildSet(5)
+        self.assertTrue(pset1.sourcesSharedBy(pset2).is_empty())
+
+        pset1.add(pkgs2[:2])
+        pset2.add(pkgs1[:2])
+        self.assertEqual(
+            sorted(pset1.sourcesSharedBy(pset2)),
+            sorted(pkgs1[:2] + pkgs2[:2]))
+
+    def test_get_sources_shared_by(self):
+        # List the names of source packages shared between two packagesets
+        pset1, pkgs1 = self.buildSet(5)
+        pset2, pkgs2 = self.buildSet(5)
+        self.assertEqual(pset1.getSourcesSharedBy(pset2), [])
+
+        pset1.add(pkgs2[:2])
+        pset2.add(pkgs1[:2])
+        self.assertEqual(
+            sorted(pset1.getSourcesSharedBy(pset2)),
+            sorted(p.name for p in (pkgs1[:2] + pkgs2[:2])))
+
+    def test_sources_shared_by_subset(self):
+        # sourcesSharedBy takes subsets into account, unless told not to
+        pset1, pkgs1 = self.buildSet()
+        pset2, pkgs2 = self.buildSet()
+        self.assertTrue(pset1.sourcesSharedBy(pset2).is_empty())
+
+        pset1.add((pset2,))
+        self.assertEqual(sorted(pset1.sourcesSharedBy(pset2)), sorted(pkgs2))
+        self.assertTrue(
+            pset1.sourcesSharedBy(pset2, direct_inclusion=True).is_empty())
+
+    def test_sources_shared_by_symmetric(self):
+        # sourcesSharedBy is symmetric
+        pset1, pkgs1 = self.buildSet(5)
+        pset2, pkgs2 = self.buildSet(5)
+        pset3, pkgs3 = self.buildSet(5)
+        self.assertTrue(pset1.sourcesSharedBy(pset2).is_empty())
+
+        pset1.add(pkgs2[:2] + pkgs3)
+        pset2.add(pkgs1[:2] + [pset3])
+        self.assertEqual(
+            sorted(pset1.sourcesSharedBy(pset2)),
+            sorted(pkgs1[:2] + pkgs2[:2] + pkgs3))
+        self.assertEqual(
+            sorted(pset1.sourcesSharedBy(pset2)),
+            sorted(pset2.sourcesSharedBy(pset1)))
+
+    def test_sources_not_shared_by(self):
+        # Lists source packages in the first set, but not the second
+        pset1, pkgs1 = self.buildSet(5)
+        pset2, pkgs2 = self.buildSet(5)
+        self.assertEqual(
+            sorted(pset1.sourcesNotSharedBy(pset2)), sorted(pkgs1))
+        pset1.add(pkgs2[:2])
+        pset2.add(pkgs1[:2])
+        self.assertEqual(
+            sorted(pset1.sourcesNotSharedBy(pset2)), sorted(pkgs1[2:]))
+
+    def test_get_sources_not_shared_by(self):
+        # List the names of source packages in the first set, but not the
+        # second
+        pset1, pkgs1 = self.buildSet(5)
+        pset2, pkgs2 = self.buildSet(5)
+        self.assertEqual(
+            sorted(pset1.getSourcesNotSharedBy(pset2)),
+            sorted(p.name for p in pkgs1))
+
+        pset1.add(pkgs2[:2])
+        pset2.add(pkgs1[:2])
+        self.assertEqual(
+            sorted(pset1.getSourcesNotSharedBy(pset2)),
+            sorted(p.name for p in pkgs1[2:]))
+
+    def test_sources_not_shared_by_subset(self):
+        # sourcesNotSharedBy takes subsets into account, unless told not to
+        pset1, pkgs1 = self.buildSet()
+        pset2, pkgs2 = self.buildSet()
+        self.assertTrue(sorted(pset1.sourcesNotSharedBy(pset2)), sorted(pkgs1))
+
+        pset2.add((pset1,))
+        self.assertTrue(pset1.sourcesNotSharedBy(pset2).is_empty())
+        self.assertTrue(
+            sorted(pset1.sourcesNotSharedBy(pset2, direct_inclusion=True)),
+            sorted(pkgs1))
+
+    def test_add_unknown_name(self):
+        # Adding an unknown package name will raise an error
+        pset = self.factory.makePackageset()
+        self.assertRaises(
+            AssertionError, pset.add, [self.factory.getUniqueUnicode()])
+
+    def test_remove_unknown_name(self):
+        # Removing an unknown package name will raise an error
+        pset = self.factory.makePackageset()
+        self.assertRaises(
+            AssertionError, pset.remove, [self.factory.getUniqueUnicode()])
+
+    def test_add_cycle(self):
+        # Adding cycles to the graph will raise an error
+        parent = self.factory.makePackageset()
+        child = self.factory.makePackageset()
+        parent.add((child,))
+        self.assertRaises(Exception, child.add, (parent,))
+
+    def test_add_indirect_cycle(self):
+        # Adding indirect cycles to the graph will raise an error
+        parent = self.factory.makePackageset()
+        child = self.factory.makePackageset()
+        grandchild = self.factory.makePackageset()
+        parent.add((child,))
+        child.add((grandchild,))
+        self.assertRaises(Exception, grandchild.add, (parent,))
+
+
+class TestPackagesetPermissions(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestPackagesetPermissions, self).setUp()
+        self.person = self.factory.makePerson()
+        self.person2 = self.factory.makePerson()
+        self.packageset = self.factory.makePackageset(owner=self.person)
+        self.packageset2 = self.factory.makePackageset(owner=self.person)
+        self.package = self.factory.makeSourcePackageName()
+
+    def test_user_modify_packageset(self):
+        # Normal users may not modify packagesets
+        with person_logged_in(self.person2):
+            self.assertRaises(
+                Unauthorized, setattr, self.packageset, 'name', u'renamed')
+            self.assertRaises(
+                Unauthorized, setattr, self.packageset, 'description',
+                u'Re-described')
+            self.assertRaises(
+                Unauthorized, setattr, self.packageset, 'owner', self.person2)
+            self.assertRaises(
+                Unauthorized, getattr, self.packageset, 'add')
+            self.assertRaises(
+                Unauthorized, getattr, self.packageset, 'remove')
+            self.assertRaises(
+                Unauthorized, getattr, self.packageset, 'addSources')
+            self.assertRaises(
+                Unauthorized, getattr, self.packageset, 'removeSources')
+            self.assertRaises(
+                Unauthorized, getattr, self.packageset, 'addSubsets')
+            self.assertRaises(
+                Unauthorized, getattr, self.packageset, 'removeSubsets')
+
+    def modifyPackageset(self):
+        self.packageset.name = u'renamed'
+        self.packageset.description = u'Re-described'
+        self.packageset.add((self.package,))
+        self.packageset.remove((self.package,))
+        self.packageset.addSources((self.package.name,))
+        self.packageset.removeSources((self.package.name,))
+        self.packageset.add((self.packageset2,))
+        self.packageset.remove((self.packageset2,))
+        self.packageset.addSubsets((self.packageset2.name,))
+        self.packageset.removeSubsets((self.packageset2.name,))
+        self.packageset.owner = self.person2
+
+    def test_owner_modify_packageset(self):
+        # Packageset owners can modify their packagesets
+        with person_logged_in(self.person):
+            self.modifyPackageset()
+
+    def test_admin_modify_packageset(self):
+        # Admins can modify packagesets
+        with admin_logged_in():
+            self.modifyPackageset()
+
+
+class TestArchivePermissionSet(TestCaseWithFactory):
+
+    layer = ZopelessDatabaseLayer
+
+    def setUp(self):
+        super(TestArchivePermissionSet, self).setUp()
+        self.ap_set = getUtility(IArchivePermissionSet)
+        self.archive = self.factory.makeArchive()
+        self.packageset = self.factory.makePackageset()
+        self.person = self.factory.makePerson()
+
+    def test_packagesets_for_uploader_empty(self):
+        # A new archive will have no upload permissions
+        self.assertTrue(
+            self.ap_set.packagesetsForUploader(
+                self.archive, self.person).is_empty())
+
+    # TODO: More tests here
+    def test_new_packageset_uploader_simple(self):
+        # newPackagesetUploader grants upload rights to a packagset
+        permission = self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+
+        self.assertEqual(permission.archive, self.archive)
+        self.assertEqual(permission.person, self.person)
+        self.assertEqual(permission.packageset, self.packageset)
+        self.assertEqual(permission.permission, ArchivePermissionType.UPLOAD)
+        self.assertFalse(permission.explicit)
+        # Convenience property:
+        self.assertEqual(permission.package_set_name, self.packageset.name)
+
+    def test_new_packageset_uploader_repeated(self):
+        # Creating the same permission repeatedly should return the re-use the
+        # existing permission.
+        permission1 = self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+        permission2 = self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+        self.assertEqual(permission1.id, permission2.id)
+
+    def test_new_packageset_uploader_repeated_explicit(self):
+        # Attempting to create an explicit permission when a non-explicit one
+        # exists already will fail.
+        self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+        self.assertRaises(ValueError, self.ap_set.newPackagesetUploader,
+            self.archive, self.person, self.packageset, True)
+
+    def test_new_packageset_uploader_repeated_implicit(self):
+        # Attempting to create an implicit permission when an explicit one
+        # exists already will fail.
+        self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset, True)
+        self.assertRaises(ValueError, self.ap_set.newPackagesetUploader,
+            self.archive, self.person, self.packageset)
+
+    def test_new_packageset_uploader_teammember(self):
+        # If a team member already has upload rights through a team, they can
+        # be granted again, individually
+        team = self.factory.makeTeam(self.person)
+        self.ap_set.newPackagesetUploader(
+            self.archive, team, self.packageset)
+        self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+
+        # Unless the explicit flag conflicts
+        self.assertRaises(ValueError, self.ap_set.newPackagesetUploader,
+            self.archive, self.person, self.packageset, True)
+
+    def test_packagesets_for_uploader(self):
+        # packagesetsForUploader returns the packageset upload archive
+        # permissions granted to a person
+        self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+
+        permission = self.ap_set.packagesetsForUploader(
+            self.archive, self.person).one()
+        self.assertEqual(permission.archive, self.archive)
+        self.assertEqual(permission.person, self.person)
+        self.assertEqual(permission.packageset, self.packageset)
+        self.assertEqual(permission.permission, ArchivePermissionType.UPLOAD)
+        self.assertFalse(permission.explicit)
+
+    def test_packagesets_for_source_uploader(self):
+        # packagesetsForSourceUploader returns the packageset upload archive
+        # permissions granted to a person affecting a given package
+        self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+        package = self.factory.makeSourcePackageName()
+        self.packageset.add((package,))
+
+        permission = self.ap_set.packagesetsForSourceUploader(
+            self.archive, package, self.person).one()
+        self.assertEqual(permission.archive, self.archive)
+        self.assertEqual(permission.person, self.person)
+        self.assertEqual(permission.packageset, self.packageset)
+        self.assertEqual(permission.permission, ArchivePermissionType.UPLOAD)
+        self.assertFalse(permission.explicit)
+
+    def test_packagesets_for_source_uploader_by_name(self):
+        # packagesetsForSourceUploader can take a package name
+        self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+        package = self.factory.makeSourcePackageName()
+        self.packageset.add((package,))
+
+        self.assertFalse(self.ap_set.packagesetsForSourceUploader(
+            self.archive, package.name, self.person).is_empty())
+
+        # and will raise an exception if the name is invalid
+        self.assertRaises(
+            NoSuchSourcePackageName, self.ap_set.packagesetsForSourceUploader,
+            self.archive, self.factory.getUniqueUnicode(), self.person)
+
+    def test_packagesets_for_source(self):
+        # packagesetsForSource returns the packageset upload archive
+        # permissions affecting a given package
+        self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+        package = self.factory.makeSourcePackageName()
+        self.packageset.add((package,))
+
+        permission = self.ap_set.packagesetsForSource(
+            self.archive, package).one()
+        self.assertEqual(permission.archive, self.archive)
+        self.assertEqual(permission.person, self.person)
+        self.assertEqual(permission.packageset, self.packageset)
+        self.assertEqual(permission.permission, ArchivePermissionType.UPLOAD)
+        self.assertFalse(permission.explicit)
+
+    def test_uploaders_for_packageset(self):
+        # uploadersForPackageset returns the people with upload rigts for a
+        # packageset in a given archive
+        self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+
+        permission = self.ap_set.uploadersForPackageset(
+            self.archive, self.packageset).one()
+        self.assertEqual(permission.archive, self.archive)
+        self.assertEqual(permission.person, self.person)
+        self.assertEqual(permission.packageset, self.packageset)
+        self.assertEqual(permission.permission, ArchivePermissionType.UPLOAD)
+        self.assertFalse(permission.explicit)
+
+    def test_uploaders_for_packageset_subpackagesets(self):
+        # archive permissions apply to children of a packageset, unless they
+        # have their own permissions with the "explicit" flag set
+        child = self.factory.makePackageset()
+        self.packageset.add((child,))
+        self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+
+        # uploadersForPackageset will not list them:
+        self.assertTrue(
+            self.ap_set.uploadersForPackageset(self.archive, child).is_empty())
+
+        # unless told to:
+        self.assertFalse(
+            self.ap_set.uploadersForPackageset(
+                self.archive, child, direct_permissions=False).is_empty())
+
+    def test_uploaders_for_packageset_explicit(self):
+        # people can have both explicit and implicit upload rights to a
+        # packageset
+        child = self.factory.makePackageset()
+        self.packageset.add((child,))
+        implicit_parent = self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+        explicit_child = self.ap_set.newPackagesetUploader(
+            self.archive, self.person, child, True)
+
+        self.assertEqual(
+            sorted(self.ap_set.uploadersForPackageset(
+                self.archive, child, direct_permissions=False)),
+            sorted((implicit_parent, explicit_child)))
+
+    def test_uploaders_for_packageset_subpackagesets_removed(self):
+        # archive permissions cease to apply to removed child packagesets
+        child = self.factory.makePackageset()
+        self.packageset.add((child,))
+        self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+        self.assertFalse(
+            self.ap_set.uploadersForPackageset(
+                self.archive, child, direct_permissions=False).is_empty())
+
+        self.packageset.remove((child,))
+        self.assertTrue(
+            self.ap_set.uploadersForPackageset(
+                self.archive, child, direct_permissions=False).is_empty())
+
+    def test_uploaders_for_packageset_by_name(self):
+        # a packageset name that doesn't exist will throw an error
+        self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+        # A correct name will give us a result:
+        self.assertFalse(self.ap_set.uploadersForPackageset(
+            self.archive, self.packageset.name).is_empty())
+        # An incorrect one will raise an exception
+        self.assertRaises(
+            NotFoundError, self.ap_set.uploadersForPackageset,
+            self.archive, self.factory.getUniqueUnicode())
+        # An incorrect type will raise a ValueError
+        self.assertRaises(
+            ValueError, self.ap_set.uploadersForPackageset,
+            self.archive, 42)
+
+    def test_archive_permission_per_archive(self):
+        # archive permissions are limited to an archive
+        archive2 = self.factory.makeArchive()
+        self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+        self.assertTrue(
+            self.ap_set.packagesetsForUploader(
+                archive2, self.person).is_empty())
+
+    def test_check_authenticated_packageset(self):
+        # checkAuthenticated is a generic way to look up archive permissions
+        self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+
+        permissions = self.ap_set.checkAuthenticated(
+            self.person, self.archive, ArchivePermissionType.UPLOAD,
+            self.packageset)
+
+        self.assertEqual(permissions.count(), 1)
+        permission = permissions[0]
+
+        self.assertEqual(permission.archive, self.archive)
+        self.assertEqual(permission.person, self.person)
+        self.assertEqual(permission.packageset, self.packageset)
+        self.assertEqual(permission.permission, ArchivePermissionType.UPLOAD)
+        self.assertFalse(permission.explicit)
+
+    def test_is_source_upload_allowed(self):
+        # isSourceUploadAllowed indicates whether a user has any archive
+        # permissinos granting them upload access to a specific source package
+        # (excepting component permissions)
+        self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+        package = self.factory.makeSourcePackageName()
+        self.packageset.add((package,))
+
+        self.assertTrue(self.ap_set.isSourceUploadAllowed(
+            self.archive, package, self.person))
+
+    def test_is_source_upload_allowed_denied(self):
+        # isSourceUploadAllowed should return false when a user has no
+        # packageset/PPU permission granting upload rights
+        self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+        package = self.factory.makeSourcePackageName()
+
+        self.assertFalse(self.ap_set.isSourceUploadAllowed(
+            self.archive, package, self.person))
+
+    def test_explicit_packageset_upload_rights(self):
+        # If a package is covered by a packageset with explicit upload rights,
+        # they disable all implicit upload rights to that package through other
+        # packagesets.
+        self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+        package = self.factory.makeSourcePackageName()
+        package2 = self.factory.makeSourcePackageName()
+        self.packageset.add((package, package2))
+
+        self.assertTrue(self.ap_set.isSourceUploadAllowed(
+            self.archive, package, self.person))
+        self.assertTrue(self.ap_set.isSourceUploadAllowed(
+            self.archive, package2, self.person))
+
+        # Create a packageset with explicit rights to package
+        special_person = self.factory.makePerson()
+        special_packageset = self.factory.makePackageset()
+        special_packageset.add((package,))
+        self.ap_set.newPackagesetUploader(
+            self.archive, special_person, special_packageset, True)
+
+        self.assertFalse(self.ap_set.isSourceUploadAllowed(
+            self.archive, package, self.person))
+        self.assertTrue(self.ap_set.isSourceUploadAllowed(
+            self.archive, package2, self.person))
+        self.assertTrue(self.ap_set.isSourceUploadAllowed(
+            self.archive, package, special_person))
+        self.assertFalse(self.ap_set.isSourceUploadAllowed(
+            self.archive, package2, special_person))
+
+    def test_delete_packageset_uploader(self):
+        # deletePackagesetUploader removes upload rights
+        self.ap_set.newPackagesetUploader(
+            self.archive, self.person, self.packageset)
+        self.assertFalse(
+            self.ap_set.packagesetsForUploader(
+                self.archive, self.person).is_empty())
+
+        self.ap_set.deletePackagesetUploader(
+            self.archive, self.person, self.packageset)
+        self.assertTrue(
+            self.ap_set.packagesetsForUploader(
+                self.archive, self.person).is_empty())