Launchpad itself

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

  1. remove-get-current-principal
  2. Merge into devel
Proposed by Jonathan Lange
Status: Merged
Approved by: Robert Collins
Approved revision: no longer in the source branch.
Merged at revision: 11114
Proposed branch: lp:~jml/launchpad/remove-get-current-principal
Merge into: lp:launchpad
Diff against target: 114 lines (+20/-26)
2 files modified
lib/canonical/launchpad/webapp/interaction.py (+14/-5)
lib/canonical/launchpad/webapp/launchbag.py (+6/-21)
To merge this branch: bzr merge lp:~jml/launchpad/remove-get-current-principal
Reviewer Review Type Date Requested Status
Robert Collins (community) Approve
Review via email: mp+29542@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/webapp/interaction.py'
2--- lib/canonical/launchpad/webapp/interaction.py 2010-04-16 23:56:47 +0000
3+++ lib/canonical/launchpad/webapp/interaction.py 2010-07-09 12:22:44 +0000
4@@ -30,14 +30,23 @@
5
6
7 def get_current_principal():
8- """Get the principal from the current interaction."""
9+ """Get the principal from the current interaction.
10+
11+ :return: The current principal if there is an interaction, None otherwise.
12+ """
13 interaction = queryInteraction()
14+ if interaction is None:
15+ return None
16 principals = [
17 participation.principal
18- for participation in interaction.participations]
19- assert len(principals) == 1, (
20- "There should be only one principal in the current interaction.")
21- return principals[0]
22+ for participation in interaction.participations
23+ if participation.principal is not None]
24+ if not principals:
25+ return None
26+ elif len(principals) > 1:
27+ raise ValueError('Too many principals')
28+ else:
29+ return principals[0]
30
31
32 def setupInteraction(principal, login=None, participation=None):
33
34=== modified file 'lib/canonical/launchpad/webapp/launchbag.py'
35--- lib/canonical/launchpad/webapp/launchbag.py 2010-02-17 11:13:06 +0000
36+++ lib/canonical/launchpad/webapp/launchbag.py 2010-07-09 12:22:44 +0000
37@@ -12,7 +12,6 @@
38
39 from zope.interface import implements
40 from zope.component import getUtility
41-from zope.security import management
42 from zope import thread
43
44 from canonical.database.sqlbase import block_implicit_flushes
45@@ -20,30 +19,13 @@
46 IAccount, IBug, IDistribution, IDistroSeries, IPerson,
47 IProjectGroup, IProduct, ISourcePackage, IDistroArchSeries,
48 ISpecification, IBugTask, ILaunchpadCelebrities)
49+from canonical.launchpad.webapp.interaction import get_current_principal
50 from canonical.launchpad.webapp.interfaces import (
51 ILaunchBag, ILaunchpadApplication, ILoggedInEvent, IOpenLaunchBag)
52
53 _utc_tz = pytz.timezone('UTC')
54
55
56-def get_principal():
57- """Return the principal for the current interaction."""
58- interaction = management.queryInteraction()
59- if interaction is None:
60- return None
61- principals = [
62- participation.principal
63- for participation in list(interaction.participations)
64- if participation.principal is not None
65- ]
66- if not principals:
67- return None
68- elif len(principals) > 1:
69- raise ValueError('Too many principals')
70- else:
71- return principals[0]
72-
73-
74 class LaunchBag:
75
76 implements(IOpenLaunchBag)
77@@ -84,12 +66,12 @@
78 @property
79 @block_implicit_flushes
80 def account(self):
81- return IAccount(get_principal(), None)
82+ return IAccount(get_current_principal(), None)
83
84 @property
85 @block_implicit_flushes
86 def user(self):
87- return IPerson(get_principal(), None)
88+ return IPerson(get_current_principal(), None)
89
90 def add(self, obj):
91 store = self._store
92@@ -192,6 +174,7 @@
93 else:
94 launchbag.setLogin(loggedinevent.login)
95
96+
97 def set_developer_in_launchbag_before_traversal(event):
98 """Subscriber for IBeforeTraverseEvent
99
100@@ -208,12 +191,14 @@
101 is_developer = user.inTeam(celebrities.launchpad_developers)
102 launchbag.setDeveloper(is_developer)
103
104+
105 def reset_login_in_launchbag_on_logout(event):
106 """Subscriber for ILoggedOutEvent that sets 'login' in launchbag to None.
107 """
108 launchbag = getUtility(IOpenLaunchBag)
109 launchbag.setLogin(None)
110
111+
112 def reset_developer_in_launchbag_on_logout(event):
113 """Subscriber for ILoggedOutEvent that resets the developer flag."""
114 launchbag = getUtility(IOpenLaunchBag)