Merge lp:~jml/launchpad/remove-get-current-principal into lp:launchpad

[Jonathan Lange]

I was poking around in the login code and noticed that there are two nearly-identical ways for getting the current principal: get_principal in launchbag and get_current_principal in interaction.

Since they were both nearly the same, I decided to delete one. get_current_principal is more widely used, has a better name and is in a better location, so I kept that. But get_principal has the better implementation, so I changed get_current_principal to have get_principal's implementation.

[Robert Collins (lifeless)]

It might be nice if there was an Interaction object. I know it would be global, but it would at least allow nonglobal methods which are easier to test.

review: Approve

=== modified file 'lib/canonical/launchpad/webapp/interaction.py'
--- lib/canonical/launchpad/webapp/interaction.py	2010-04-16 23:56:47 +0000
+++ lib/canonical/launchpad/webapp/interaction.py	2010-07-09 12:22:44 +0000
@@ -30,14 +30,23 @@
 
 
 def get_current_principal():
-    """Get the principal from the current interaction."""
+    """Get the principal from the current interaction.
+
+    :return: The current principal if there is an interaction, None otherwise.
+    """
     interaction = queryInteraction()
+    if interaction is None:
+        return None
     principals = [
         participation.principal
-        for participation in interaction.participations]
-    assert len(principals) == 1, (
-        "There should be only one principal in the current interaction.")
-    return principals[0]
+        for participation in interaction.participations
+        if participation.principal is not None]
+    if not principals:
+        return None
+    elif len(principals) > 1:
+        raise ValueError('Too many principals')
+    else:
+        return principals[0]
 
 
 def setupInteraction(principal, login=None, participation=None):

=== modified file 'lib/canonical/launchpad/webapp/launchbag.py'
--- lib/canonical/launchpad/webapp/launchbag.py	2010-02-17 11:13:06 +0000
+++ lib/canonical/launchpad/webapp/launchbag.py	2010-07-09 12:22:44 +0000
@@ -12,7 +12,6 @@
 
 from zope.interface import implements
 from zope.component import getUtility
-from zope.security import management
 from zope import thread
 
 from canonical.database.sqlbase import block_implicit_flushes
@@ -20,30 +19,13 @@
         IAccount, IBug, IDistribution, IDistroSeries, IPerson,
         IProjectGroup, IProduct, ISourcePackage, IDistroArchSeries,
         ISpecification, IBugTask, ILaunchpadCelebrities)
+from canonical.launchpad.webapp.interaction import get_current_principal
 from canonical.launchpad.webapp.interfaces import (
     ILaunchBag, ILaunchpadApplication, ILoggedInEvent, IOpenLaunchBag)
 
 _utc_tz = pytz.timezone('UTC')
 
 
-def get_principal():
-    """Return the principal for the current interaction."""
-    interaction = management.queryInteraction()
-    if interaction is None:
-        return None
-    principals = [
-        participation.principal
-        for participation in list(interaction.participations)
-        if participation.principal is not None
-        ]
-    if not principals:
-        return None
-    elif len(principals) > 1:
-        raise ValueError('Too many principals')
-    else:
-        return principals[0]
-
-
 class LaunchBag:
 
     implements(IOpenLaunchBag)
@@ -84,12 +66,12 @@
     @property
     @block_implicit_flushes
     def account(self):
-        return IAccount(get_principal(), None)
+        return IAccount(get_current_principal(), None)
 
     @property
     @block_implicit_flushes
     def user(self):
-        return IPerson(get_principal(), None)
+        return IPerson(get_current_principal(), None)
 
     def add(self, obj):
         store = self._store
@@ -192,6 +174,7 @@
     else:
         launchbag.setLogin(loggedinevent.login)
 
+
 def set_developer_in_launchbag_before_traversal(event):
     """Subscriber for IBeforeTraverseEvent
 
@@ -208,12 +191,14 @@
         is_developer = user.inTeam(celebrities.launchpad_developers)
         launchbag.setDeveloper(is_developer)
 
+
 def reset_login_in_launchbag_on_logout(event):
     """Subscriber for ILoggedOutEvent that sets 'login' in launchbag to None.
     """
     launchbag = getUtility(IOpenLaunchBag)
     launchbag.setLogin(None)
 
+
 def reset_developer_in_launchbag_on_logout(event):
     """Subscriber for ILoggedOutEvent that resets the developer flag."""
     launchbag = getUtility(IOpenLaunchBag)