Merge lp:~edwin-grubbs/launchpad/bug-664828-teammembership-delete-timeout into lp:launchpad

[Edwin Grubbs]

[r=mars][ui=none][bug=664828] Fixed timeout when deactivating a member of a team.

Summary
-------

This branch fixes the timeout when deactivating a member of a team that
was caused by the many queries necessary to traverse the TeamMembership
table to determine what entries need to be removed from the
TeamParticipation table.

Implementation details
----------------------

Used a postgresql 8.4 recursive query to traverse the TeamMembership
table to determine which entries should be kept in TeamParticipation.
    lib/lp/registry/model/teammembership.py

Extended tests to verify that there are no extra deletions from the
TeamParticipation table as a side-effect. Added test to make sure that
the number of queries doesn't grow.
    lib/lp/registry/tests/test_teammembership.py
    lib/lp/registry/browser/tests/test_teammembership.py

Minor edits:
    lib/lp/registry/doc/teammembership.txt
    lib/lp/registry/interfaces/teammembership.py

Tests
-----

./bin/test -vv -t 'doc/teammembership.txt|registry.tests.test_teammembership'

Demo and Q/A
------------

* Open http://launchpad.dev/...

Lint
----

No lint.

[Robert Collins (lifeless)]

I'm confused - why is a recursive query needed?

Isn't the same thing used to do the +participations able to serve the
response in ~ 2 queries?

[Edwin Grubbs (edwin-grubbs)]

> I'm confused - why is a recursive query needed?
>
> Isn't the same thing used to do the +participations able to serve the
> response in ~ 2 queries?

If Team B's membership in Team A is deactivated, then Team B and all of its participants may or may not still participate in Team A and its super teams. The only way to tell if the super teams still have participants via another path is by recursing the membership tree. This could either be done as a recursive query, which I chose, or in python.

[Māris Fogels (mars)]

Hi Edwin,

The code in this change looks good to me, and the SQL is nicely documented. My first question is, where does the magic number '12' come from as the query count in test_deactivate_member_query_count? I know the test is for performance testing, but that one test and one number don't detail how the query is expected to perform using different data.

On a related note, is there already a test for the case you mentioned to Robert? "The only way to tell if the super teams still have participants via another path is by recursing the membership tree."

Maris

review: Needs Fixing

[Edwin Grubbs (edwin-grubbs)]

Hi Maris,

Thanks for the review. An incremental diff is included at the bottom.

> Hi Edwin,
>
> The code in this change looks good to me, and the SQL is nicely documented.
> My first question is, where does the magic number '12' come from as the query
> count in test_deactivate_member_query_count? I know the test is for
> performance testing, but that one test and one number don't detail how the
> query is expected to perform using different data.

I have added a comment explaining the queries. I was able to reduce the number of queries by one by moving create_view() outside of the with-statement.

> On a related note, is there already a test for the case you mentioned to
> Robert? "The only way to tell if the super teams still have participants via
> another path is by recursing the membership tree."

lp/registry/tests/test_teammembership.py already has the TestTeamParticipationMesh, TestTeamParticipationTable, and TestTeamParticipationHierarchy tests that verify that the TeamParticipation table is updated correctly.

Incremental diff:

=== modified file 'lib/lp/registry/browser/tests/test_teammembership.py'
--- lib/lp/registry/browser/tests/test_teammembership.py 2011-01-11 01:40:09 +0000
+++ lib/lp/registry/browser/tests/test_teammembership.py 2011-01-11 20:45:04 +0000
@@ -32,6 +32,25 @@
         self.member = self.factory.makeTeam()

     def test_deactivate_member_query_count(self):
+ # Only these queries should be run, no matter what the
+ # membership tree looks like, although the number of queries
+ # could change slightly if a different user is logged in.
+ # 1. Check whether the user is the team owner.
+ # 2. Deactivate the membership in the TeamMembership table.
+ # 3. Delete from TeamParticipation table.
+ # (Queries #4, #5, #6, #7, and #10 are run because the storm
+ # objects have been invalidated.)
+ # 4. Get the TeamMembership entry.
+ # 5. Verify that the member exists in the db, but don't load
+ # the refresh the rest of its data, since we just need the id.
+ # 6. Verify that the user exists in the db.
+ # 7. Verify that the team exists in the db.
+ # 8. Insert into Job table.
+ # 9. Insert into PersonTransferJob table to schedule sending
+ # email. (This requires the data from queries #5, #6, and
+ # #7.)
+ # 10.Query the rest of the team data for the invalidated
+ # object in order to generate the canonical url.
         self.team.addMember(
             self.member, self.team.teamowner, force_team_add=True)
         form = {
@@ -41,9 +60,10 @@
             }
         membership = self.membership_set.getByPersonAndTeam(
             self.member, self.team)
+ view = create_view(
+ membership, "+index", method='POST', form=form)
         with StormStatementRecorder() as recorder:
- view = create_view(membership, "+index", method='POST', form=form)
             view.processForm()
         self.assertEqual('', view.errormessage)
         self.assertEqual(TeamMembershipStatus.DEACTIVATED, membership.status)
- self...

[Māris Fogels (mars)]

> Hi Maris,
>
> Thanks for the review. An incremental diff is included at the bottom.
>

Thanks Edwin, your code looks good. r=mars

review: Approve

=== added file 'lib/lp/registry/browser/tests/test_teammembership.py'
--- lib/lp/registry/browser/tests/test_teammembership.py	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/browser/tests/test_teammembership.py	2011-01-11 20:48:40 +0000
@@ -0,0 +1,69 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from testtools.matchers import LessThan
+from zope.component import getUtility
+
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.testing.matchers import HasQueryCount
+from lp.registry.interfaces.teammembership import (
+    ITeamMembershipSet,
+    TeamMembershipStatus,
+    )
+from lp.testing import (
+    login_celebrity,
+    StormStatementRecorder,
+    TestCaseWithFactory,
+    )
+from lp.testing.views import create_view
+
+
+class TestTeamMenu(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestTeamMenu, self).setUp()
+        login_celebrity('admin')
+        self.membership_set = getUtility(ITeamMembershipSet)
+        self.team = self.factory.makeTeam()
+        self.member = self.factory.makeTeam()
+
+    def test_deactivate_member_query_count(self):
+        # Only these queries should be run, no matter what the
+        # membership tree looks like, although the number of queries
+        # could change slightly if a different user is logged in.
+        #   1. Check whether the user is the team owner.
+        #   2. Deactivate the membership in the TeamMembership table.
+        #   3. Delete from TeamParticipation table.
+        #   (Queries #4, #5, #6, #7, and #10 are run because the storm
+        #    objects have been invalidated.)
+        #   4. Get the TeamMembership entry.
+        #   5. Verify that the member exists in the db, but don't load
+        #   the refresh the rest of its data, since we just need the id.
+        #   6. Verify that the user exists in the db.
+        #   7. Verify that the team exists in the db.
+        #   8. Insert into Job table.
+        #   9. Insert into PersonTransferJob table to schedule sending
+        #      email. (This requires the data from queries #5, #6, and
+        #      #7.)
+        #   10.Query the rest of the team data for the invalidated
+        #      object in order to generate the canonical url.
+        self.team.addMember(
+            self.member, self.team.teamowner, force_team_add=True)
+        form = {
+            'editactive': 1,
+            'expires': 'never',
+            'deactivate': 'Deactivate',
+            }
+        membership = self.membership_set.getByPersonAndTeam(
+            self.member, self.team)
+        view = create_view(
+            membership, "+index", method='POST', form=form)
+        with StormStatementRecorder() as recorder:
+            view.processForm()
+        self.assertEqual('', view.errormessage)
+        self.assertEqual(TeamMembershipStatus.DEACTIVATED, membership.status)
+        self.assertThat(recorder, HasQueryCount(LessThan(11)))
 
=== modified file 'lib/lp/registry/doc/teammembership.txt'
--- lib/lp/registry/doc/teammembership.txt	2010-10-24 12:46:23 +0000
+++ lib/lp/registry/doc/teammembership.txt	2011-01-11 20:48:40 +0000
@@ -131,6 +131,8 @@
     ...     ubuntu_team, TeamMembershipStatus.DEACTIVATED, t2.teamowner)
     >>> ubuntu_team in t2.activemembers
     False
+    >>> [m.displayname for m in t2.allmembers]
+    [u'No Privileges Person']
     >>> login(ANONYMOUS)
 
 Another API for adding members is ITeam.addMember(), which ensures the given
 
=== modified file 'lib/lp/registry/interfaces/teammembership.py'
--- lib/lp/registry/interfaces/teammembership.py	2010-12-10 14:58:31 +0000
+++ lib/lp/registry/interfaces/teammembership.py	2011-01-11 20:48:40 +0000
@@ -116,7 +116,11 @@
 
 
 class ITeamMembership(Interface):
-    """TeamMembership for Users"""
+    """TeamMembership for Users.
+
+    This table includes *direct* team members only.  Indirect memberships are
+    handled by the TeamParticipation table.
+    """
     export_as_webservice_entry()
 
     id = Int(title=_('ID'), required=True, readonly=True)
 
=== modified file 'lib/lp/registry/model/teammembership.py'
--- lib/lp/registry/model/teammembership.py	2010-12-10 14:58:31 +0000
+++ lib/lp/registry/model/teammembership.py	2011-01-11 20:48:40 +0000
@@ -21,7 +21,7 @@
     ForeignKey,
     StringCol,
     )
-from storm.locals import Store
+from storm.store import Store
 from zope.component import getUtility
 from zope.interface import implements
 
@@ -498,167 +498,85 @@
     person = ForeignKey(dbName='person', foreignKey='Person', notNull=True)
 
 
-def _cleanTeamParticipation(person, team):
-    """Remove relevant entries in TeamParticipation for <person> and <team>.
-
-    Remove all tuples "person, team" from TeamParticipation for the given
-    person and team (together with all its superteams), unless this person is
-    an indirect member of the given team. More information on how to use the
-    TeamParticipation table can be found in the TeamParticipationUsage spec or
-    the teammembership.txt system doctest.
-    """
-    query = """
-        SELECT EXISTS(
-            SELECT 1 FROM TeamParticipation
-            WHERE person = %(person_id)s AND team IN (
-                    SELECT person
-                    FROM TeamParticipation JOIN Person ON
-                        (person = Person.id)
-                    WHERE team = %(team_id)s
-                        AND person NOT IN (%(team_id)s, %(person_id)s)
-                        AND teamowner IS NOT NULL
-                 )
-        )
-        """ % dict(team_id=team.id, person_id=person.id)
-    store = Store.of(person)
-    (result, ) = store.execute(query).get_one()
-    if result:
-        # The person is a participant in this team by virtue of a membership
-        # in another one, so don't attempt to remove anything.
-        return
-
-    # First of all, we remove <person> from <team> (and its superteams).
-    _removeParticipantFromTeamAndSuperTeams(person, team)
-    if not person.is_team:
-        # Nothing else to do.
-        return
-
-    store = Store.of(person)
-
-    # Clean the participation of all our participant subteams, that are
-    # not a direct members of the target team.
-    query = """
-        -- All of my participant subteams...
-        SELECT person
-        FROM TeamParticipation JOIN Person ON (person = Person.id)
-        WHERE team = %(person_id)s AND person != %(person_id)s
-            AND teamowner IS NOT NULL
-        EXCEPT
-        -- that aren't a direct member of the team.
-        SELECT person
-        FROM TeamMembership
-        WHERE team = %(team_id)s AND status IN %(active_states)s
-        """ % dict(
-            person_id=person.id, team_id=team.id,
-            active_states=sqlvalues(ACTIVE_STATES)[0])
-
-    # Avoid circular import.
-    from lp.registry.model.person import Person
-    for subteam in store.find(Person, "id IN (%s)" % query):
-        _cleanTeamParticipation(subteam, team)
-
-    # Then clean-up all the non-team participants. We can remove those
-    # in a single query when the team graph is up to date.
-    _removeAllIndividualParticipantsFromTeamAndSuperTeams(person, team)
-
-
-def _removeParticipantFromTeamAndSuperTeams(person, team):
-    """Remove participation of person in team.
-
-    If <person> is a participant (that is, has a TeamParticipation entry)
-    of any team that is a subteam of <team>, then <person> should be kept as
-    a participant of <team> and (as a consequence) all its superteams.
-    Otherwise, <person> is removed from <team> and we repeat this process for
-    each superteam of <team>.
-    """
-    # Check if the person is a member of the given team through another team.
-    query = """
-        SELECT EXISTS(
-            SELECT 1
-            FROM TeamParticipation, TeamMembership
-            WHERE
-                TeamMembership.team = %(team_id)s AND
-                TeamMembership.person = TeamParticipation.team AND
-                TeamParticipation.person = %(person_id)s AND
-                TeamMembership.status IN %(active_states)s)
-        """ % dict(team_id=team.id, person_id=person.id,
-                   active_states=sqlvalues(ACTIVE_STATES)[0])
-    store = Store.of(person)
-    (result, ) = store.execute(query).get_one()
-    if result:
-        # The person is a participant by virtue of a membership on another
-        # team, so don't remove.
-        return
-    store.find(TeamParticipation, (
-        (TeamParticipation.team == team) &
-        (TeamParticipation.person == person))).remove()
-
-    for superteam in _getSuperTeamsExcludingDirectMembership(person, team):
-        _removeParticipantFromTeamAndSuperTeams(person, superteam)
-
-
-def _removeAllIndividualParticipantsFromTeamAndSuperTeams(team, target_team):
-    """Remove all non-team participants in <team> from <target_team>.
-
-    All the non-team participants of <team> are removed from <target_team>
-    and its super teams, unless they participate in <target_team> also from
-    one of its sub team.
-    """
-    query = """
+def _cleanTeamParticipation(child, parent):
+    """Remove child from team and clean up child's subteams.
+
+    A participant of child is removed from parent's TeamParticipation
+    entries if the only path from the participant to parent is via
+    child.
+    """
+    # Delete participation entries for the child and the child's
+    # direct/indirect members in other ancestor teams, unless those
+    # ancestor teams have another path the the child besides the
+    # membership that has just been deactivated.
+    store = Store.of(parent)
+    store.execute("""
         DELETE FROM TeamParticipation
-        WHERE team = %(target_team_id)s AND person IN (
-            -- All the individual participants.
-            SELECT person
-            FROM TeamParticipation JOIN Person ON (person = Person.id)
-            WHERE team = %(team_id)s AND teamowner IS NULL
-            EXCEPT
-            -- people participating through a subteam of target_team;
-            SELECT person
+        USING (
+            /* Get all the participation entries that might need to be
+             * deleted, i.e. all the entries where the
+             * TeamParticipation.person is a participant of child, and
+             * where child participated in TeamParticipation.team until
+             * child was removed from parent.
+             */
+            SELECT person, team
             FROM TeamParticipation
-            WHERE team IN (
-                -- The subteams of target_team.
-                SELECT person
-                FROM TeamParticipation JOIN Person ON (person = Person.id)
-                WHERE team = %(target_team_id)s
-                    AND person NOT IN (%(target_team_id)s, %(team_id)s)
-                    AND teamowner IS NOT NULL
-                 )
-            -- or people directly a member of the target team.
-            EXCEPT
-            SELECT person
-            FROM TeamMembership
-            WHERE team = %(target_team_id)s AND status IN %(active_states)s
-        )
-        """ % dict(
-            team_id=team.id, target_team_id=target_team.id,
-            active_states=sqlvalues(ACTIVE_STATES)[0])
-    store = Store.of(team)
-    store.execute(query)
-
-    super_teams = _getSuperTeamsExcludingDirectMembership(team, target_team)
-    for superteam in super_teams:
-        _removeAllIndividualParticipantsFromTeamAndSuperTeams(team, superteam)
-
-
-def _getSuperTeamsExcludingDirectMembership(person, team):
-    """Return all the super teams of <team> where person isn't a member."""
-    query = """
-        -- All the super teams...
-        SELECT team
-        FROM TeamParticipation
-        WHERE person = %(team_id)s AND team != %(team_id)s
-        EXCEPT
-        -- The one where person has an active membership.
-        SELECT team
-        FROM TeamMembership
-        WHERE person = %(person_id)s AND status IN %(active_states)s
-        """ % dict(
-            person_id=person.id, team_id=team.id,
-            active_states=sqlvalues(ACTIVE_STATES)[0])
-
-    # Avoid circular import.
-    from lp.registry.model.person import Person
-    return Store.of(person).find(Person, "id IN (%s)" % query)
+            WHERE person IN (
+                    SELECT person
+                    FROM TeamParticipation
+                    WHERE team = %(child)s
+                )
+                AND team IN (
+                    SELECT team
+                    FROM TeamParticipation
+                    WHERE person = %(child)s
+                        AND team != %(child)s
+                )
+
+
+            EXCEPT (
+
+                /* Compute the TeamParticipation entries that we need to
+                 * keep by walking the tree in the TeamMembership table.
+                 */
+                WITH RECURSIVE parent(person, team) AS (
+                    /* Start by getting all the ancestors of the child
+                     * from the TeamParticipation table, then get those
+                     * ancestors' direct members to recurse through the
+                     * tree from the top.
+                     */
+                    SELECT ancestor.person, ancestor.team
+                    FROM TeamMembership ancestor
+                    WHERE ancestor.status IN %(active_states)s
+                        AND ancestor.team IN (
+                            SELECT team
+                            FROM TeamParticipation
+                            WHERE person = %(child)s
+                        )
+
+                    UNION
+
+                    /* Find the next level of direct members, but hold
+                     * onto the parent.team, since we want the top and
+                     * bottom of the hierarchy to calculate the
+                     * TeamParticipation. The query above makes sure
+                     * that we do this for all the ancestors.
+                     */
+                    SELECT child.person, parent.team
+                    FROM TeamMembership child
+                        JOIN parent ON child.team = parent.person
+                    WHERE child.status IN %(active_states)s
+                )
+                SELECT person, team
+                FROM parent
+            )
+        ) AS keeping
+        WHERE TeamParticipation.person = keeping.person
+            AND TeamParticipation.team = keeping.team
+        """ % sqlvalues(
+            child=child.id,
+            active_states=ACTIVE_STATES))
+    store.invalidate()
 
 
 def _fillTeamParticipation(member, accepting_team):
 
=== modified file 'lib/lp/registry/tests/test_teammembership.py'
--- lib/lp/registry/tests/test_teammembership.py	2010-12-02 16:13:51 +0000
+++ lib/lp/registry/tests/test_teammembership.py	2011-01-11 20:48:40 +0000
@@ -9,7 +9,10 @@
     )
 import re
 import subprocess
-import unittest
+from unittest import (
+    TestCase,
+    TestLoader,
+    )
 
 import pytz
 from zope.component import getUtility
@@ -24,6 +27,7 @@
     login,
     login_person,
     )
+from canonical.launchpad.interfaces.lpstorm import IStore
 from canonical.launchpad.testing.systemdocs import (
     default_optionflags,
     LayeredDocFileSuite,
@@ -40,11 +44,14 @@
     ITeamMembershipSet,
     TeamMembershipStatus,
     )
-from lp.registry.model.teammembership import TeamMembership
-from lp.testing.factory import LaunchpadObjectFactory
-
-
-class TestTeamMembershipSet(unittest.TestCase):
+from lp.registry.model.teammembership import (
+    TeamMembership,
+    TeamParticipation,
+    )
+from lp.testing import TestCaseWithFactory
+
+
+class TestTeamMembershipSet(TestCase):
     layer = DatabaseFunctionalLayer
 
     def setUp(self):
@@ -155,11 +162,12 @@
         self.failIf(sample_person.inTeam(motu))
 
 
-class TeamParticipationTestCase(unittest.TestCase):
+class TeamParticipationTestCase(TestCaseWithFactory):
     """Tests for team participation using 5 teams."""
     layer = DatabaseFunctionalLayer
 
     def setUp(self):
+        super(TeamParticipationTestCase, self).setUp()
         login('[e-mail]')
         person_set = getUtility(IPersonSet)
         self.foo_bar = person_set.getByEmail('[e-mail]')
@@ -177,6 +185,9 @@
             sorted(participant_names),
             sorted([participant.name for participant in team.allmembers]))
 
+    def getTeamParticipationCount(self):
+        return IStore(TeamParticipation).find(TeamParticipation).count()
+
 
 class TestTeamParticipationHierarchy(TeamParticipationTestCase):
     """Participation management tests using 5 nested teams.
@@ -221,6 +232,7 @@
 
         This is similar to what was experienced in bug 261915.
         """
+        previous_count = self.getTeamParticipationCount()
         self.team2.setMembershipData(
             self.team3, TeamMembershipStatus.DEACTIVATED, self.foo_bar)
         self.assertParticipantsEquals(['name16', 'team2'], self.team1)
@@ -230,11 +242,15 @@
         self.assertParticipantsEquals(
             ['name16', 'no-priv', 'team5'], self.team4)
         self.assertParticipantsEquals(['name16', 'no-priv'], self.team5)
+        self.assertEqual(
+            previous_count-8,
+            self.getTeamParticipationCount())
 
     def testRemovingLeafTeam(self):
         """Make sure that participations are updated correctly when removing
         the leaf team.
         """
+        previous_count = self.getTeamParticipationCount()
         self.team4.setMembershipData(
             self.team5, TeamMembershipStatus.DEACTIVATED, self.foo_bar)
         self.assertParticipantsEquals(
@@ -244,6 +260,9 @@
         self.assertParticipantsEquals(['name16', 'team4'], self.team3)
         self.assertParticipantsEquals(['name16'], self.team4)
         self.assertParticipantsEquals(['name16', 'no-priv'], self.team5)
+        self.assertEqual(
+            previous_count-8,
+            self.getTeamParticipationCount())
 
 
 class TestTeamParticipationTree(TeamParticipationTestCase):
@@ -252,9 +271,10 @@
     Create a team hierarchy looking like this:
         team1
            team2
-         team5  team3
-                  team4
-                     team5
+              team5
+              team3
+                 team4
+                    team5
                        no-priv
     """
     layer = DatabaseFunctionalLayer
@@ -269,6 +289,10 @@
         self.team3.addMember(self.team4, self.foo_bar, force_team_add=True)
         self.team4.addMember(self.team5, self.foo_bar, force_team_add=True)
 
+    def tearDown(self):
+        super(TestTeamParticipationTree, self).tearDown()
+        self.layer.force_dirty_database()
+
     def testTeamParticipationSetUp(self):
         """Make sure that the TeamParticipation are sane after setUp."""
         self.assertParticipantsEquals(
@@ -284,6 +308,7 @@
             ['name16', 'no-priv'], self.team5)
 
     def testRemoveTeam3FromTeam2(self):
+        previous_count = self.getTeamParticipationCount()
         self.team2.setMembershipData(
             self.team3, TeamMembershipStatus.DEACTIVATED, self.foo_bar)
         self.assertParticipantsEquals(
@@ -295,8 +320,12 @@
         self.assertParticipantsEquals(
             ['name16', 'no-priv', 'team5'], self.team4)
         self.assertParticipantsEquals(['name16', 'no-priv'], self.team5)
+        self.assertEqual(
+            previous_count-4,
+            self.getTeamParticipationCount())
 
     def testRemoveTeam5FromTeam4(self):
+        previous_count = self.getTeamParticipationCount()
         self.team4.setMembershipData(
             self.team5, TeamMembershipStatus.DEACTIVATED, self.foo_bar)
         self.assertParticipantsEquals(
@@ -308,6 +337,40 @@
             ['name16', 'team4'], self.team3)
         self.assertParticipantsEquals(['name16'], self.team4)
         self.assertParticipantsEquals(['name16', 'no-priv'], self.team5)
+        self.assertEqual(
+            previous_count-4,
+            self.getTeamParticipationCount())
+
+
+class TestParticipationCleanup(TeamParticipationTestCase):
+    """Test deletion of a member from a team with many superteams.
+    Create a team hierarchy looking like this:
+        team1
+           team2
+              team3
+                 team4
+                    team5
+                       no-priv
+    """
+
+    def setUp(self):
+        """Setup the team hierarchy."""
+        super(TestParticipationCleanup, self).setUp()
+        self.team1.addMember(self.team2, self.foo_bar, force_team_add=True)
+        self.team2.addMember(self.team3, self.foo_bar, force_team_add=True)
+        self.team3.addMember(self.team4, self.foo_bar, force_team_add=True)
+        self.team4.addMember(self.team5, self.foo_bar, force_team_add=True)
+        self.team5.addMember(self.no_priv, self.foo_bar)
+
+    def testMemberRemoval(self):
+        """Remove the member from the last team.
+
+        The number of db queries should be constant not O(depth).
+        """
+        self.assertStatementCount(
+            7,
+            self.team5.setMembershipData, self.no_priv,
+            TeamMembershipStatus.DEACTIVATED, self.team5.teamowner)
 
 
 class TestTeamParticipationMesh(TeamParticipationTestCase):
@@ -315,12 +378,15 @@
     branches.
 
     Create a team hierarchy looking like this:
-        team1    /--team6
-            team2        \
-             |  team3    |
-             \--- team4-/
-                     team5
-                       no-priv
+        team1     team6
+           \     /  |
+            team2   |
+           /    |   |
+        team3   |   |
+             \  |  /
+              team4
+                 team5
+                    no-priv
     """
     layer = DatabaseFunctionalLayer
 
@@ -338,6 +404,10 @@
         self.team6.addMember(self.team2, self.foo_bar, force_team_add=True)
         self.team6.addMember(self.team4, self.foo_bar, force_team_add=True)
 
+    def tearDown(self):
+        super(TestTeamParticipationMesh, self).tearDown()
+        self.layer.force_dirty_database()
+
     def testTeamParticipationSetUp(self):
         """Make sure that the TeamParticipation are sane after setUp."""
         self.assertParticipantsEquals(
@@ -355,6 +425,7 @@
             self.team6)
 
     def testRemoveTeam3FromTeam2(self):
+        previous_count = self.getTeamParticipationCount()
         self.team2.setMembershipData(
             self.team3, TeamMembershipStatus.DEACTIVATED, self.foo_bar)
         self.assertParticipantsEquals(
@@ -368,8 +439,12 @@
         self.assertParticipantsEquals(['name16', 'no-priv'], self.team5)
         self.assertParticipantsEquals(
             ['name16', 'no-priv', 'team2', 'team4', 'team5'], self.team6)
+        self.assertEqual(
+            previous_count-3,
+            self.getTeamParticipationCount())
 
     def testRemoveTeam5FromTeam4(self):
+        previous_count = self.getTeamParticipationCount()
         self.team4.setMembershipData(
             self.team5, TeamMembershipStatus.DEACTIVATED, self.foo_bar)
         self.assertParticipantsEquals(
@@ -381,9 +456,12 @@
         self.assertParticipantsEquals(['name16', 'no-priv'], self.team5)
         self.assertParticipantsEquals(
             ['name16', 'team2', 'team3', 'team4'], self.team6)
-
-
-class TestTeamMembership(unittest.TestCase):
+        self.assertEqual(
+            previous_count-10,
+            self.getTeamParticipationCount())
+
+
+class TestTeamMembership(TestCaseWithFactory):
     layer = DatabaseFunctionalLayer
 
     def test_teams_not_kicked_from_themselves_bug_248498(self):
@@ -400,12 +478,11 @@
         This test will make sure that doesn't happen in the future.
         """
         login('[e-mail]')
-        factory = LaunchpadObjectFactory()
-        person = factory.makePerson()
+        person = self.factory.makePerson()
         login_person(person) # Now login with the future owner of the teams.
-        teamA = factory.makeTeam(
+        teamA = self.factory.makeTeam(
             person, subscription_policy=TeamSubscriptionPolicy.MODERATED)
-        teamB = factory.makeTeam(
+        teamB = self.factory.makeTeam(
             person, subscription_policy=TeamSubscriptionPolicy.MODERATED)
         self.failUnless(
             teamA.inTeam(teamA), "teamA is not a participant of itself")
@@ -444,21 +521,21 @@
         self.assertEqual(new_status, TeamMembershipStatus.DEACTIVATED.value)
 
 
-class TestTeamMembershipSetStatus(unittest.TestCase):
+class TestTeamMembershipSetStatus(TestCaseWithFactory):
     """Test the behaviour of TeamMembership's setStatus()."""
     layer = DatabaseFunctionalLayer
 
     def setUp(self):
+        super(TestTeamMembershipSetStatus, self).setUp()
         login('[e-mail]')
         self.foobar = getUtility(IPersonSet).getByName('name16')
         self.no_priv = getUtility(IPersonSet).getByName('no-priv')
         self.ubuntu_team = getUtility(IPersonSet).getByName('ubuntu-team')
         self.admins = getUtility(IPersonSet).getByName('admins')
         # Create a bunch of arbitrary teams to use in the tests.
-        factory = LaunchpadObjectFactory()
-        self.team1 = factory.makeTeam(self.foobar)
-        self.team2 = factory.makeTeam(self.foobar)
-        self.team3 = factory.makeTeam(self.foobar)
+        self.team1 = self.factory.makeTeam(self.foobar)
+        self.team2 = self.factory.makeTeam(self.foobar)
+        self.team3 = self.factory.makeTeam(self.foobar)
 
     def test_proponent_is_stored(self):
         for status in [TeamMembershipStatus.DEACTIVATED,
@@ -698,7 +775,7 @@
         self.assertEqual(TeamMembershipStatus.DEACTIVATED, tm.status)
 
 
-class TestCheckTeamParticipationScript(unittest.TestCase):
+class TestCheckTeamParticipationScript(TestCase):
     layer = DatabaseFunctionalLayer
 
     def _runScript(self, expected_returncode=0):
@@ -803,7 +880,7 @@
 
 
 def test_suite():
-    suite = unittest.TestLoader().loadTestsFromName(__name__)
+    suite = TestLoader().loadTestsFromName(__name__)
     bug_249185 = LayeredDocFileSuite(
         'bug-249185.txt', optionflags=default_optionflags,
         layer=DatabaseFunctionalLayer, setUp=setUp, tearDown=tearDown)