Merge lp:~lifeless/launchpad/registry into lp:launchpad

[Robert Collins]

Drop the use of ValidPersonCache from Person._all_members as we usually link EmailAddress in anyway so we can get a more efficient query this way. (6000ms->35ms) for ubuntu-dev

So the ValidPersonCache component turns out to turn a 35ms thing into a 6000ms thing. Not so good: this unpacks the view, and performs snappily, fixing timeouts for this on staging.

[Jeroen T. Vermeulen (jtv)]

Two changes discussed on IRC:
 * Indentation—don't mix "right after (" with "tab-indented on following lines."
 * AND should be And.

With that, you have my blessing.

review: Approve

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2010-08-17 13:53:04 +0000
+++ lib/lp/registry/model/person.py	2010-08-18 05:26:48 +0000
@@ -190,6 +190,12 @@
     """Flags if a Person is active and usable in Launchpad.
 
     This is readonly, as this is a view in the database.
+
+    Note that it performs poorly at least some of the time, and if
+    EmailAddress and Person are already being queried, its probably better to
+    query Account directly. See bug
+    https://bugs.edge.launchpad.net/launchpad-registry/+bug/615237 for some
+    corroborating information.
     """
 
 
@@ -1489,7 +1495,7 @@
             columns.append(KarmaTotalCache)
         if need_ubuntu_coc:
             columns.append(Alias(Exists(Select(SignedCodeOfConduct,
-                AND(Person._is_ubuntu_coc_signer_condition(),
+                And(Person._is_ubuntu_coc_signer_condition(),
                     SignedCodeOfConduct.ownerID == Person.id))),
                 name='is_ubuntu_coc_signer'))
         if need_location:
@@ -1509,8 +1515,9 @@
                 Archive.id == Select(Min(Archive.id),
                     Archive.owner == Person.id, Archive),
                 Archive.purpose == ArchivePurpose.PPA)))
-        if need_preferred_email:
-            # Teams don't have email.
+        # checking validity requires having a preferred email.
+        if need_preferred_email or need_validity:
+            # Teams don't have email, so a left join
             origin.append(
                 LeftJoin(EmailAddress, EmailAddress.person == Person.id))
             columns.append(EmailAddress)
@@ -1518,10 +1525,14 @@
                 Or(EmailAddress.status == None,
                     EmailAddress.status == EmailAddressStatus.PREFERRED))
         if need_validity:
-            # May find invalid persons
+            # May find teams (teams are not valid people)
             origin.append(
-                LeftJoin(ValidPersonCache, ValidPersonCache.id == Person.id))
-            columns.append(ValidPersonCache)
+                LeftJoin(Account, Person.account == Account.id))
+            columns.append(Account)
+            conditions = And(conditions,
+                Or(
+                    Account.status == None,
+                    Account.status == AccountStatus.ACTIVE))
         if len(columns) == 1:
             columns = columns[0]
             # Return a simple ResultSet
@@ -1562,8 +1573,14 @@
                 email = row[index]
                 index += 1
                 cache_property(result, '_preferredemail_cached', email)
+            #-- validity caching
             if need_validity:
-                valid = row[index] is not None
+                # valid if:
+                valid = (
+                    # -- valid account found
+                    row[index] is not None
+                    # -- preferred email found
+                    and result.preferredemail is not None)
                 index += 1
                 cache_property(result, '_is_valid_person_cached', valid)
             return result
@@ -2409,7 +2426,7 @@
         """See `IPerson`."""
         # Also assigned to by self._all_members.
         store = Store.of(self)
-        query = AND(SignedCodeOfConduct.ownerID == self.id,
+        query = And(SignedCodeOfConduct.ownerID == self.id,
             Person._is_ubuntu_coc_signer_condition())
         # TODO: Using exists would be faster than count().
         return bool(store.find(SignedCodeOfConduct, query).count())
@@ -2419,7 +2436,7 @@
         """Generate a Storm Expr for determing the coc signing status."""
         sigset = getUtility(ISignedCodeOfConductSet)
         lastdate = sigset.getLastAcceptedDate()
-        return AND(SignedCodeOfConduct.active == True,
+        return And(SignedCodeOfConduct.active == True,
             SignedCodeOfConduct.datecreated >= lastdate)
 
     @property