Person.addMember() does not properly invalidate the person's _inTeam_cache

Bug #125505 reported by Barry Warsaw
6
Affects Status Importance Assigned to Milestone
Launchpad itself
Fix Released
Medium
Christian Reis

Bug Description

The following test suite code doesn't work as expected:

    >>> team_two = personset.newTeam(ddaa, 'team-two', 'Team Two')
    >>> from canonical.lp.dbschema import TeamMembershipStatus
    >>> sabdfl = personset.getByName('sabdfl')
    >>> team_two.addMember(salgado, sabdfl, TeamMembershipStatus.ADMIN)
    >>> flush_database_updates()
    >>> [team.name for team in salgado.getAdministratedTeams()]
    [u'admins', u'rosetta-admins']

Note that salgado is not an administrator for team_two!

The problem is that Person has a _inTeam_cache that doesn't get invalidated by
Person.addMember(). The alternative is to do this:

    >>> from canonical.lp.dbschema import TeamMembershipStatus
    >>> salgado.join(team_two)
    >>> sabdfl = personset.getByName('sabdfl')
    >>> team_two.setMembershipData(salgado, TeamMembershipStatus.ADMIN, sabdfl)
    >>> flush_database_updates()

But this requires that team_two have an OPEN subscription policy. (Extra
dance steps needed if not.)

Person.addMember() should invalidate the _inTeam_cache for the person being
added. Note that .join() is implemented in terms of .addMember() so the
proper fix is probably to move the line:

        self._inTeam_cache = {} # Flush the cache used by the inTeam method

to .addMember(). I found this too late in the 1.1.7 cycle to feel comfortable
about sneaking the fix in.

Revision history for this message
Guilherme Salgado (salgado) wrote :

We probably need to invalidate the cache in setMembershipData(), acceptInvitationToBeMemberOf() and possibly other methods of IPerson.

Changed in launchpad:
status: New → Confirmed
Changed in launchpad:
importance: Undecided → Medium
Revision history for this message
Christian Reis (kiko) wrote :

Have fix in hand.

Changed in launchpad:
assignee: nobody → kiko
status: Confirmed → In Progress
Revision history for this message
Christian Reis (kiko) wrote :

And the cache is already invalidated in ITeamMembership.setStatus() so it's actually only necessary in a few places.

Revision history for this message
Christian Reis (kiko) wrote :

Landed in RF 5256.

Changed in launchpad:
status: In Progress → Fix Committed
Changed in launchpad:
status: Fix Committed → Fix Released
Curtis Hovey (sinzui)
tags: added: tech-debt
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.