launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

On Thu, 2010-09-09 at 19:04 +0000, Robert Collins wrote:
> > The cache is populated with TeamParticipation information. We know
> the db was not hit if no information from the db request was cached
> 
> This is a bit of a surrogate. Can I suggest the following:
> add
> Store.of(self.user).invalidate()
> before your call that shouldn't do db (makes the storm cache invalid
> and also (because we hook in) ICachedProperties are cleared.
> 
> and use
> self.assertFalse(
>     self.assertStatementCount(0, self.user.inTeam, other_user))
> 
> which directly checks that no db statements happen. 

I have incorporated your change by splitting the test into two. The
first to verify the answer is False, and the second to verify the cache
TeamParticipation was not populated. I added the
assertStatementstateCountm, but the answer will not be 0. The method
must get the person (1 call). There is also a contingency that when a
string is passed, to get the team by name (another call).

The issue here is are we querying TeamParticipation when we know the
team in question is in fact a person. In Tim's report, *he happed to
know* this was the case. Launchpad only knows that via a call to
IPerson.teamowner. We expect this to be a faster db lookup than a
failing query against TP. The method builds a dict of TP answers so we
still need to use a surogate to verify that the db call was not to TP.

{{{

=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py	2010-09-09 23:56:01 +0000
+++ lib/lp/registry/tests/test_person.py	2010-09-10 03:04:17 +0000
@@ -11,6 +11,7 @@
 import pytz
 from testtools.matchers import LessThan
 import transaction
+from storm.store import Store
 from zope.component import getUtility
 from zope.interface import providedBy
 from zope.security.proxy import removeSecurityProxy
@@ -198,11 +199,18 @@
             {},
             removeSecurityProxy(self.user)._inTeam_cache)
 
-    def test_inTeam_person(self):
-        # Verify a user cannot be a member of another user and the and
-        # DB call to TeamParticipation was not made to learn this.
+    def test_inTeam_person_is_false(self):
+        # Verify a user cannot be a member of another user.
         other_user = self.factory.makePerson()
         self.assertFalse(self.user.inTeam(other_user))
+
+    def test_inTeam_person_does_not_build_TeamParticipation_cache(self):
+        # Verify when a user is the argument, a DB call to TeamParticipation
+        # was not made to learn this.
+        other_user = self.factory.makePerson()
+        Store.of(self.user).invalidate()
+        self.assertFalse(
+            self.assertStatementstateCount(1, self.user.inTeam, other_user))
         self.assertEqual(
             {},
             removeSecurityProxy(self.user)._inTeam_cache)

}}}
-- 
__Curtis C. Hovey_________
http://launchpad.net/

https://code.launchpad.net/~sinzui/launchpad/inteam-db-calls-0/+merge/34932
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/inteam-db-calls-0 into lp:launchpad/devel.