Launchpad itself

Merge lp:~bac/launchpad/bug-5927 into lp:launchpad

  1. bug-5927
  2. Merge into devel
Proposed by Brad Crittenden
Status: Merged
Approved by: Gavin Panella
Approved revision: no longer in the source branch.
Merged at revision: 12218
Proposed branch: lp:~bac/launchpad/bug-5927
Merge into: lp:launchpad
Diff against target: 681 lines (+285/-63)
9 files modified
lib/lp/bugs/configure.zcml (+5/-0)
lib/lp/bugs/doc/bug.txt (+25/-19)
lib/lp/bugs/model/bug.py (+37/-12)
lib/lp/bugs/model/bugtask.py (+52/-18)
lib/lp/bugs/stories/bugtask-searches/xx-searching-by-tags.txt (+1/-1)
lib/lp/bugs/tests/test_bugtask_search.py (+76/-5)
lib/lp/bugs/tests/test_bugvisibility.py (+81/-0)
lib/lp/registry/browser/tests/test_person_view.py (+6/-3)
lib/lp/registry/tests/test_product.py (+2/-5)
To merge this branch: bzr merge lp:~bac/launchpad/bug-5927
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Review via email: mp+45403@code.launchpad.net

Commit message

[r=allenap][ui=none][bug=5927] Allow bugtask assignees to see private bugs.

Description of the change

= Summary =

Bug assignees should be allowed to see private bugs.

== Proposed fix ==

Since assignees already get bugnotifications, all that is required is to
change userCanView to explicitly allow bugtask assignees to view the
bug. When the assignee is changed there is no residual subscription,
which would have been a problem if the assignee were given visibility
through the subscription model as is done for the bug supervisor.

== Pre-implementation notes ==

Talks with Curtis.

== Implementation details ==

As above.

== Tests ==

bin/test -vvm lp.bugs -t test_bugvisibility -t bugnotification

== Demo and Q/A ==

Mark a bug with no assignee private. Ensure user 'bac' cannot see the
bug. Assign bac to the bug. Ensure bac can now see the bug. Unassign
bac. Ensure bac can no longer see the bug. Repeat.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/tests/test_bugvisibility.py
  lib/lp/bugs/model/bug.py
  lib/lp/registry/tests/test_product.py

To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) wrote :

Nice change. I wonder how we ever managed without this!

[1]

- # This is a private bug. Only explicit subscribers may view it.
+ # This is a private bug. Explicit subscribers may view it.
             for subscription in self.subscriptions:
                 if user.inTeam(subscription.person):
                     self._known_viewers.add(user.id)
                     return True
+ # Assignees to bugtasks can also see the private bug.
+ for bugtask in self.bugtasks:
+ if user.inTeam(bugtask.assignee):
+ self._known_viewers.add(user.id)
+ return True

As there are almost always fewer bugtasks than subscriptions, and
assignees are often not set and not usually teams, might this work out
more efficient to loop through bugtask assigness before looping
through subscriptions?

Mmm, I guess it's not that clear-cut. Anyway, just a thought.

[2]

+ def test_privateBugOwner(self):
+ # A regular (non-privileged) user can not view a private bug.
+ self.assertTrue(self.bug.userCanView(self.owner))

I think the comment here is wrong.

[3]

+ def test_privateBugSupervisorPrivateBugsByDefault(self):
+ # A member of the bug supervisor team can see a private bug if the
+ # product is set to have private bugs by default.

I think the reason for this might be that the bug supervisor is
subscribed to new bugs for private-by-default projects. There isn't
any code in userCanView() that provides this behaviour.

If this is the case, the question is what to do with this test. It
probably makes sense to change it into a test for subscriber
visibility.

[4]

+ def test_privateBugAssignee(self):
+ # The bug assignee can see the private bug.
+ bug_assignee = self.factory.makePerson(name="bugassignee")
+ with person_logged_in(self.product.owner):
+ self.bug.default_bugtask.transitionToAssignee(bug_assignee)
+ self.assertTrue(self.bug.userCanView(bug_assignee))

Is it worth asserting that userCanView() returns False before setting
the assignee?

[5]

There might be some gnarled doctest somewhere that does bug visibility
testing. Could you try and remove some of that, or file a bug to trim
it. Here are a few promising looking places:

  lib/lp/bugs/doc/bug.txt
  lib/lp/bugs/stories/bug-privacy
  lib/lp/bugs/stories/upstream-bugprivacy ?

review: Approve
Revision history for this message
Robert Collins (lifeless) wrote :

This needs eager loading in bug queries or we'll throw performance
out; we also need to change bug searches to consider assignee as
granting accessability to bugs.

So this is good, but incomplete and landing it is likely to cause a
performance regression on bugs with many bug tasks.

Revision history for this message
Gavin Panella (allenap) wrote :
Download full text (4.3 KiB)

Hi Brad,

The search methods for bugs/bugtasks are horrible to work on, and
eager loading doesn't make it any prettier... you got the short straw
on this one :-/

I found a few issues, one fairly major, but none too hard to fix.

Gavin.

[6]

In lib/lp/bugs/model/bug.py:

- FROM Bug, BugSubscription, TeamParticipation
- WHERE Bug.id = BugSubscription.bug AND
- TeamParticipation.person = %(personid)s AND
- BugSubscription.person = TeamParticipation.team))
+ FROM Bug, BugSubscription, BugTask, TeamParticipation
+ WHERE (Bug.id = BugSubscription.bug AND
+ TeamParticipation.person = %(personid)s AND
+ BugSubscription.person = TeamParticipation.team)
+ OR (
+ BugTask.bug = Bug.id AND
+ TeamParticipation.person = %(personid)s AND
+ TeamParticipation.team = BugTask.assignee)
+ ))

The new join to BugTask combined with the first of the ORed clauses:

    Bug.id = BugSubscription.bug AND
    TeamParticipation.person = %(personid)s AND
    BugSubscription.person = TeamParticipation.team

results in a cartestian product against BugTask.

I think this could be expressed as a UNION:

    SELECT Bug.id
      FROM Bug, BugSubscription, TeamParticipation
     WHERE Bug.id = BugSubscription.bug AND
           TeamParticipation.person = %(personid)s AND
           BugSubscription.person = TeamParticipation.team)
    UNION
    SELECT Bug.id
      FROM Bug, BugTask, TeamParticipation
     WHERE BugTask.bug = Bug.id AND
           TeamParticipation.person = %(personid)s AND
           TeamParticipation.team = BugTask.assignee

In addition, the join to Bug could be removed from both queries:

    SELECT BugSubscription.bug
      FROM BugSubscription, TeamParticipation
     WHERE TeamParticipation.person = %(personid)s AND
           BugSubscription.person = TeamParticipation.team
    UNION
    SELECT BugTask.bug
      FROM BugTask, TeamParticipation
     WHERE TeamParticipation.person = %(personid)s AND
           TeamParticipation.team = BugTask.assignee

[7]

In lib/lp/bugs/model/bugtask.py:

- FROM BugSubscription, TeamParticipation
- WHERE TeamParticipation.person = %(personid)s AND
+ FROM BugSubscription, TeamParticipation, BugTask
+ WHERE (TeamParticipation.person = %(personid)s AND
                    BugSubscription.person = TeamParticipation.team AND
- BugSubscription.bug = Bug.id))
+ BugSubscription.bug = Bug.id) OR
+ (BugTask.bug = Bug.id AND
+ TeamParticipation.person = %(personid)s AND
+ TeamParticipation.team = BugTask.assignee)
+ ))

Similar problem to [6].

[8]

+ def cache_assignees(rows):
+ # The decorator must be called to turn the rows of the result into
+ # a list of BugTasks, which can be used for caching the assigne...

Read more...

review: Needs Fixing
Revision history for this message
Brad Crittenden (bac) wrote :

Thanks for the suggestions Gavin. I've incorporated them all. Have another look?

Revision history for this message
Gavin Panella (allenap) wrote :

Cool :) +1

A couple more really really minor things.

[12]

+ def _get_bug_tasks(self):
+ store = Store.of(self.bug)
+ return store.find(
+ BugTask, BugTask.bug == self.bug)

Perhaps a docstring to make it obvious what this is for?

[13]

+ bugtasks = [bugtask.assignee.name for bugtask in bugtasks]

Strictly this is a list of assignee names.

This appears twice.

review: Approve
Revision history for this message
Robert Collins (lifeless) wrote :

Something you might be interested in - John Meinel did some profiling
of generator vs lists in constructors and short lived lists are
sometimes faster :)... perf is tricky.

Revision history for this message
Robert Collins (lifeless) wrote :

Following up on recent search performance, I noticed a couple of odd things introduced in this branch. Firstly, the specialised BugTaskResultSet was unnecessary - the addition of assignee to the permission lookups on BugTask searching has an immediate effect of precaching assignee *access* to the bug; none of our bug views show the assignee at the moment, so precaching actual assignee objects is a pessimism. As such I'm rolling that part of the patch backout.

The other odd thing was that I saw the initial patch *had* a pre_iter_hook parameter, which is actually more general and more compact than creating a class : as we get more complex query interactions being able to to compose and reuse eager loading requirements will become more important: I don't think we want a proliferation of result set /types/ to achieve that: simple functions are going to be generally more powerful.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/configure.zcml'
2--- lib/lp/bugs/configure.zcml 2010-11-11 11:55:53 +0000
3+++ lib/lp/bugs/configure.zcml 2011-01-14 11:09:14 +0000
4@@ -26,6 +26,11 @@
5 <lp:help-folder
6 folder="help" type="lp.bugs.publisher.BugsLayer" />
7
8+ <class class="lp.bugs.model.bugtask.BugTaskResultSet">
9+ <allow interface="storm.zope.interfaces.IResultSet" />
10+ <allow attributes="__getslice__" />
11+ </class>
12+
13 <class
14 class="lp.bugs.model.bugactivity.BugActivity">
15 <allow
16
17=== modified file 'lib/lp/bugs/doc/bug.txt'
18--- lib/lp/bugs/doc/bug.txt 2010-10-18 22:24:59 +0000
19+++ lib/lp/bugs/doc/bug.txt 2011-01-14 11:09:14 +0000
20@@ -228,7 +228,8 @@
21 subscribed to the bug can see it. Launchpad admins can always view and
22 modify private bugs.
23
24-=== Marking Bugs Private ===
25+Marking Bugs Private
26+....................
27
28 For the purposes of demonstration, we'll make the firefox crashing bug
29 private. A bug cannot be made private by an anonymous user.
30@@ -298,7 +299,8 @@
31 >>> firefox_crashes.setPrivate(True, current_user())
32 False
33
34-=== How Privacy Affects Access to a Bug ===
35+How Privacy Affects Access to a Bug
36+...................................
37
38 Once a bug is made private, it can only be accessed by the users that
39 are directly subscribed to the bug and Launchpad admins.
40@@ -323,6 +325,7 @@
41 ... user=current_user()))
42 ... return sorted(all_bugs - found_bugs)
43
44+ >>> login("test@canonical.com")
45 >>> hidden_bugs()
46 [14]
47
48@@ -344,6 +347,16 @@
49 >>> hidden_bugs()
50 []
51
52+If a bug is private, bugtask assignees can see the bug. Previously
53+sample person could not see bug 14. But after making him the
54+assignee it is visible.
55+
56+ >>> bug14 = bugset.get(14)
57+ >>> bug14.default_bugtask.transitionToAssignee(sample_person)
58+ >>> login("test@canonical.com")
59+ >>> hidden_bugs()
60+ []
61+
62 As one would expect, the permissions are team aware. So, let's retrieve a bug
63 and set it private (as Foo Bar again who, of course, is an admin.)
64
65@@ -409,20 +422,9 @@
66 >>> hidden_bugs()
67 [2, 6, 14]
68
69-IBug.userCanView() offers a quick means by which to verify that a
70-given user can see a given bug.
71-
72- >>> no_priv = personset.getByEmail('no-priv@canonical.com')
73- >>> bug_6 = getUtility(IBugSet).get(6)
74- >>> bug_6.userCanView(no_priv)
75- False
76-
77- >>> foobar = personset.getByEmail('foo.bar@canonical.com')
78- >>> bug_6.userCanView(foobar)
79- True
80-
81-
82-=== Filing Public vs. Private Bugs ===
83+
84+Filing Public vs. Private Bugs
85+..............................
86
87 Let's log back in as Foo Bar to continue our examples:
88
89@@ -430,6 +432,7 @@
90
91 When a public bug is filed:
92
93+ >>> foobar = personset.getByEmail('foo.bar@canonical.com')
94 >>> params = CreateBugParams(
95 ... title="test firefox bug", comment="blah blah blah", owner=foobar)
96 >>> params.setBugTarget(product=firefox)
97@@ -1122,7 +1125,8 @@
98
99 >>> chunks = bug_two.getMessageChunks()
100 >>> for chunk in sorted(chunks, key=lambda x:x.id):
101- ... chunk.id, chunk.message.id, chunk.message.owner.id, chunk.content[:30]
102+ ... (chunk.id, chunk.message.id, chunk.message.owner.id,
103+ ... chunk.content[:30])
104 (4, 1, 16, u'Problem exists between chair a')
105 (7, 5, 12, u'This would be a real killer fe')
106 (8, 6, 12, u'Oddly enough the bug system se')
107@@ -1131,7 +1135,7 @@
108 information, too:
109
110 XXX: bug=https://bugs.launchpad.net/storm/+bug/619017 means that this sometimes
111-does 3 queries, depending on the precise state of the storm cache. To avoid
112+does 3 queries, depending on the precise state of the storm cache. To avoid
113 spurious failures it has been changed to tolerate this additional query.
114
115 >>> len(CursorWrapper.last_executed_sql) - queries <= 3
116@@ -1276,7 +1280,8 @@
117 If there is another dup of the master bug, filed by someone else, the
118 master bug's affected count with dups increases.
119
120- >>> dupe_affected_other_user = factory.makePerson(name='napoleon-bonaparte')
121+ >>> dupe_affected_other_user = factory.makePerson(
122+ ... name='napoleon-bonaparte')
123 >>> dupe_three = factory.makeBug(owner=dupe_affected_other_user)
124 >>> dupe_three.markAsDuplicate(test_bug)
125 >>> test_bug.users_affected_count_with_dupes
126@@ -1425,6 +1430,7 @@
127 >>> new_bug_2.setPrivate(True, foobar)
128 True
129
130+ >>> no_priv = personset.getByEmail('no-priv@canonical.com')
131 >>> matching_bugs = getUtility(IBugSet).getDistinctBugsForBugTasks(
132 ... bug_tasks, user=no_priv)
133
134
135=== modified file 'lib/lp/bugs/model/bug.py'
136--- lib/lp/bugs/model/bug.py 2010-12-24 11:02:19 +0000
137+++ lib/lp/bugs/model/bug.py 2011-01-14 11:09:14 +0000
138@@ -1,4 +1,4 @@
139-# Copyright 2009 Canonical Ltd. This software is licensed under the
140+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
141 # GNU Affero General Public License version 3 (see the file LICENSE).
142
143 # pylint: disable-msg=E0611,W0212
144@@ -1773,17 +1773,24 @@
145 def _known_viewers(self):
146 """A set of known persons able to view this bug.
147
148- Seed it by including the list of all owners of bugtasks for the bug.
149+ Seed it by including the list of all owners of pillars for bugtasks
150+ for the bug.
151 """
152- bugtask_owners = [bt.pillar.owner.id for bt in self.bugtasks]
153- return set(bugtask_owners)
154+ pillar_owners = [bt.pillar.owner.id for bt in self.bugtasks]
155+ return set(pillar_owners)
156
157 def userCanView(self, user):
158 """See `IBug`.
159
160 Note that Editing is also controlled by this check,
161 because we permit editing of any bug one can see.
162+
163+ If bug privacy rights are changed here, corresponding changes need
164+ to be made to the queries which screen for privacy. See
165+ Bug.searchAsUser and BugTask.get_bug_privacy_filter_with_decorator.
166 """
167+ assert user is not None, "User may not be None"
168+
169 if user.id in self._known_viewers:
170 return True
171 if not self.private:
172@@ -1793,7 +1800,15 @@
173 # Admins can view all bugs.
174 return True
175 else:
176- # This is a private bug. Only explicit subscribers may view it.
177+ # At this point we know the bug is private and the user is
178+ # unprivileged.
179+
180+ # Assignees to bugtasks can see the private bug.
181+ for bugtask in self.bugtasks:
182+ if user.inTeam(bugtask.assignee):
183+ self._known_viewers.add(user.id)
184+ return True
185+ # Explicit subscribers may also view it.
186 for subscription in self.subscriptions:
187 if user.inTeam(subscription.person):
188 self._known_viewers.add(user.id)
189@@ -2185,13 +2200,23 @@
190 # allowed to see.
191 where_clauses.append("""
192 (Bug.private = FALSE OR
193- Bug.id in (
194- SELECT Bug.id
195- FROM Bug, BugSubscription, TeamParticipation
196- WHERE Bug.id = BugSubscription.bug AND
197- TeamParticipation.person = %(personid)s AND
198- BugSubscription.person = TeamParticipation.team))
199- """ % sqlvalues(personid=user.id))
200+ Bug.id in (
201+ -- Users who have a subscription to this bug.
202+ SELECT BugSubscription.bug
203+ FROM BugSubscription, TeamParticipation
204+ WHERE
205+ TeamParticipation.person = %(personid)s AND
206+ BugSubscription.person = TeamParticipation.team
207+ UNION
208+ -- Users who are the assignee for one of the bug's
209+ -- bugtasks.
210+ SELECT BugTask.bug
211+ FROM BugTask, TeamParticipation
212+ WHERE
213+ TeamParticipation.person = %(personid)s AND
214+ TeamParticipation.team = BugTask.assignee
215+ )
216+ )""" % sqlvalues(personid=user.id))
217 else:
218 # Anonymous user; filter to include only public bugs in
219 # the search results.
220
221=== modified file 'lib/lp/bugs/model/bugtask.py'
222--- lib/lp/bugs/model/bugtask.py 2011-01-14 00:35:43 +0000
223+++ lib/lp/bugs/model/bugtask.py 2011-01-14 11:09:14 +0000
224@@ -1,4 +1,4 @@
225-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
226+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
227 # GNU Affero General Public License version 3 (see the file LICENSE).
228
229 # pylint: disable-msg=E0611,W0212
230@@ -9,6 +9,7 @@
231
232 __all__ = [
233 'BugTaskDelta',
234+ 'BugTaskResultSet',
235 'BugTaskToBugAdapter',
236 'BugTaskMixin',
237 'BugTask',
238@@ -424,6 +425,21 @@
239 self.bug.id, self.bugtargetdisplayname, self.bug.title)
240
241
242+class BugTaskResultSet(DecoratedResultSet):
243+ """Decorated results with cached assignees."""
244+
245+ def __iter__(self, *args, **kwargs):
246+ """Iter with caching of assignees.
247+
248+ Assumes none of the decorators will need to access the assignees or
249+ there is not benefit.
250+ """
251+ bugtasks = list(
252+ super(BugTaskResultSet, self).__iter__(*args, **kwargs))
253+ BugTaskSet._cache_assignees(bugtasks)
254+ return iter(bugtasks)
255+
256+
257 def BugTaskToBugAdapter(bugtask):
258 """Adapt an IBugTask to an IBug."""
259 return bugtask.bug
260@@ -1058,12 +1074,20 @@
261 # The assignee is being cleared, so clear the date_assigned
262 # value.
263 self.date_assigned = None
264+ # The bugtask is unassigned, so clear the _known_viewer cached
265+ # property for the bug.
266+ get_property_cache(self.bug)._known_viewers = set()
267 if not self.assignee and assignee:
268 # The task is going from not having an assignee to having
269 # one, so record when this happened
270 self.date_assigned = now
271
272 self.assignee = assignee
273+ # Invalidate the old visibility cache for this bug and replace it with
274+ # the new assignee.
275+ if self.assignee is not None:
276+ get_property_cache(self.bug)._known_viewers = set(
277+ [self.assignee.id])
278
279 def transitionToTarget(self, target):
280 """See `IBugTask`.
281@@ -1347,8 +1371,15 @@
282 SELECT BugSubscription.bug
283 FROM BugSubscription, TeamParticipation
284 WHERE TeamParticipation.person = %(personid)s AND
285- BugSubscription.person = TeamParticipation.team AND
286- BugSubscription.bug = Bug.id))
287+ TeamParticipation.team = BugSubscription.person AND
288+ BugSubscription.bug = Bug.id
289+ UNION
290+ SELECT BugTask.bug
291+ FROM BugTask, TeamParticipation
292+ WHERE TeamParticipation.person = %(personid)s AND
293+ TeamParticipation.team = BugTask.assignee AND
294+ BugTask.bug = Bug.id
295+ ))
296 """ % sqlvalues(personid=user.id),
297 _make_cache_user_can_view_bug(user))
298
299@@ -2305,7 +2336,7 @@
300 origin.append(table)
301 return origin
302
303- def _search(self, resultrow, prejoins, params, *args, **kw):
304+ def _search(self, resultrow, prejoins, params, *args):
305 """Return a Storm result set for the given search parameters.
306
307 :param resultrow: The type of data returned by the query.
308@@ -2314,6 +2345,7 @@
309 :param params: A BugTaskSearchParams instance.
310 :param args: optional additional BugTaskSearchParams instances,
311 """
312+
313 store = IStore(BugTask)
314 [query, clauseTables, orderby, bugtask_decorator, join_tables,
315 has_duplicate_results] = self.buildQuery(params)
316@@ -2372,7 +2404,8 @@
317
318 result = store.using(*origin).find(resultrow)
319 result.order_by(orderby)
320- return DecoratedResultSet(result, result_decorator=decorator)
321+ return BugTaskResultSet(
322+ result, result_decorator=decorator)
323
324 def search(self, params, *args, **kwargs):
325 """See `IBugTaskSet`.
326@@ -2412,6 +2445,15 @@
327 """See `IBugTaskSet`."""
328 return self._search(BugTask.bugID, [], params).result_set
329
330+ @staticmethod
331+ def _cache_assignees(rows):
332+ assignee_ids = set(
333+ bug_task.assigneeID for bug_task in rows)
334+ assignees = getUtility(IPersonSet).getPrecachedPersonsFromIDs(
335+ assignee_ids, need_validity=True)
336+ # Execute query to load storm cache.
337+ list(assignees)
338+
339 def getPrecachedNonConjoinedBugTasks(self, user, milestone):
340 """See `IBugTaskSet`."""
341 params = BugTaskSearchParams(
342@@ -2420,16 +2462,8 @@
343 omit_dupes=True, exclude_conjoined_tasks=True)
344 non_conjoined_slaves = self.search(params)
345
346- def cache_people(rows):
347- assignee_ids = set(
348- bug_task.assigneeID for bug_task in rows)
349- assignees = getUtility(IPersonSet).getPrecachedPersonsFromIDs(
350- assignee_ids, need_validity=True)
351- # Execute query to load storm cache.
352- list(assignees)
353-
354 return DecoratedResultSet(
355- non_conjoined_slaves, pre_iter_hook=cache_people)
356+ non_conjoined_slaves, pre_iter_hook=BugTaskSet._cache_assignees)
357
358 def createTask(self, bug, owner, product=None, productseries=None,
359 distribution=None, distroseries=None,
360@@ -2581,7 +2615,7 @@
361 bug_privacy_filter = "AND " + bug_privacy_filter
362 unconfirmed_bug_condition = self._getUnconfirmedBugCondition()
363 (target_join, target_clause) = self._getTargetJoinAndClause(target)
364- expirable_bugtasks = BugTask.select("""
365+ query = """
366 BugTask.bug = Bug.id
367 AND BugTask.id IN (
368 SELECT BugTask.id
369@@ -2600,13 +2634,13 @@
370 AND Bug.date_last_updated < CURRENT_TIMESTAMP
371 AT TIME ZONE 'UTC' - interval '%s days'
372 AND BugWatch.id IS NULL
373- )""" % sqlvalues(BugTaskStatus.INCOMPLETE, min_days_old) +
374- unconfirmed_bug_condition,
375+ )""" % sqlvalues(BugTaskStatus.INCOMPLETE, min_days_old)
376+ expirable_bugtasks = BugTask.select(
377+ query + unconfirmed_bug_condition,
378 clauseTables=['Bug'],
379 orderBy='Bug.date_last_updated')
380 if limit is not None:
381 expirable_bugtasks = expirable_bugtasks.limit(limit)
382-
383 return expirable_bugtasks
384
385 def _getUnconfirmedBugCondition(self):
386
387=== modified file 'lib/lp/bugs/stories/bugtask-searches/xx-searching-by-tags.txt'
388--- lib/lp/bugs/stories/bugtask-searches/xx-searching-by-tags.txt 2010-10-18 22:24:59 +0000
389+++ lib/lp/bugs/stories/bugtask-searches/xx-searching-by-tags.txt 2011-01-14 11:09:14 +0000
390@@ -46,7 +46,7 @@
391 Summary Importance Status Heat
392 16 test bug a Undecided New
393 17 test bug b Undecided New
394-
395+
396 Same works for user related bugs:
397
398 >>> anon_browser.open('http://launchpad.dev/~name16/+bugs?advanced=1')
399
400=== modified file 'lib/lp/bugs/tests/test_bugtask_search.py'
401--- lib/lp/bugs/tests/test_bugtask_search.py 2010-12-24 15:32:32 +0000
402+++ lib/lp/bugs/tests/test_bugtask_search.py 2011-01-14 11:09:14 +0000
403@@ -1,4 +1,4 @@
404-# Copyright 2010 Canonical Ltd. This software is licensed under the
405+# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
406 # GNU Affero General Public License version 3 (see the file LICENSE).
407
408 __metaclass__ = type
409@@ -9,7 +9,11 @@
410 )
411 from new import classobj
412 import sys
413-from testtools.matchers import Equals
414+from testtools.matchers import (
415+ Equals,
416+ LessThan,
417+ Not,
418+ )
419 import unittest
420
421 import pytz
422@@ -32,7 +36,7 @@
423 BugTaskStatus,
424 IBugTaskSet,
425 )
426-from lp.bugs.model.bugtask import BugTask
427+from lp.bugs.model.bugtask import BugTask, BugTaskResultSet
428 from lp.registry.interfaces.distribution import IDistribution
429 from lp.registry.interfaces.distributionsourcepackage import (
430 IDistributionSourcePackage,
431@@ -87,17 +91,25 @@
432 # If the user is subscribed to the bug, it is included in the
433 # search result.
434 user = self.factory.makePerson()
435- with person_logged_in(self.owner):
436+ admin = getUtility(IPersonSet).getByEmail('foo.bar@canonical.com')
437+ with person_logged_in(admin):
438 bug = self.bugtasks[-1].bug
439 bug.subscribe(user, self.owner)
440 params = self.getBugTaskSearchParams(user=user)
441 self.assertSearchFinds(params, self.bugtasks)
442
443 # Private bugs are included in search results for admins.
444- admin = getUtility(IPersonSet).getByEmail('foo.bar@canonical.com')
445 params = self.getBugTaskSearchParams(user=admin)
446 self.assertSearchFinds(params, self.bugtasks)
447
448+ # Private bugs are included in search results for the assignee.
449+ user = self.factory.makePerson()
450+ bugtask = self.bugtasks[-1]
451+ with person_logged_in(admin):
452+ bugtask.transitionToAssignee(user)
453+ params = self.getBugTaskSearchParams(user=user)
454+ self.assertSearchFinds(params, self.bugtasks)
455+
456 def test_search_by_bug_reporter(self):
457 # Search results can be limited to bugs filed by a given person.
458 bugtask = self.bugtasks[0]
459@@ -976,6 +988,65 @@
460 return [bugtask.bug.id for bugtask in expected_bugtasks]
461
462
463+class TestCachingAssignees(TestCaseWithFactory):
464+ """Searching bug tasks should pre-cache the bugtask assignees."""
465+
466+ layer = LaunchpadFunctionalLayer
467+
468+ def setUp(self):
469+ super(TestCachingAssignees, self).setUp()
470+ self.owner = self.factory.makePerson(name="bug-owner")
471+ with person_logged_in(self.owner):
472+ # Create some bugs with assigned bugtasks.
473+ self.bug = self.factory.makeBug(
474+ owner=self.owner)
475+ self.bug.default_bugtask.transitionToAssignee(
476+ self.factory.makePerson())
477+
478+ for i in xrange(9):
479+ bugtask = self.factory.makeBugTask(
480+ bug=self.bug)
481+ bugtask.transitionToAssignee(
482+ self.factory.makePerson())
483+ self.bug.setPrivate(True, self.owner)
484+
485+ def _get_bug_tasks(self):
486+ """Get the bugtasks for a bug.
487+
488+ This method is used rather than Bug.bugtasks since the later does
489+ prejoining which would spoil the test.
490+ """
491+ store = Store.of(self.bug)
492+ return store.find(
493+ BugTask, BugTask.bug == self.bug)
494+
495+ def test_no_precaching(self):
496+ bugtasks = self._get_bug_tasks()
497+ Store.of(self.bug).invalidate()
498+ with person_logged_in(self.owner):
499+ with StormStatementRecorder() as recorder:
500+ # Access the assignees to trigger a query.
501+ names = [bugtask.assignee.name for bugtask in bugtasks]
502+ # With no caching, the number of queries is roughly twice the
503+ # number of bugtasks.
504+ query_count_floor = len(names) * 2
505+ self.assertThat(
506+ recorder, HasQueryCount(Not(LessThan(query_count_floor))))
507+
508+ def test_precaching(self):
509+ bugtasks = self._get_bug_tasks()
510+ Store.of(self.bug).invalidate()
511+ with person_logged_in(self.owner):
512+ with StormStatementRecorder() as recorder:
513+ bugtasks = BugTaskResultSet(bugtasks)
514+ # Access the assignees to trigger a query if not properly
515+ # cached.
516+ names = [bugtask.assignee.name for bugtask in bugtasks]
517+ # With caching the number of queries is two, one for the
518+ # bugtask and one for all of the assignees at once.
519+ self.assertThat(recorder, HasQueryCount(Equals(2)))
520+
521+
522 def test_suite():
523 module = sys.modules[__name__]
524 for bug_target_search_type_class in (
525
526=== added file 'lib/lp/bugs/tests/test_bugvisibility.py'
527--- lib/lp/bugs/tests/test_bugvisibility.py 1970-01-01 00:00:00 +0000
528+++ lib/lp/bugs/tests/test_bugvisibility.py 2011-01-14 11:09:14 +0000
529@@ -0,0 +1,81 @@
530+# Copyright 2011 Canonical Ltd. This software is licensed under the
531+# GNU Affero General Public License version 3 (see the file LICENSE).
532+
533+"""Tests for visibility of a bug."""
534+
535+from canonical.testing.layers import LaunchpadFunctionalLayer
536+from lp.testing import (
537+ celebrity_logged_in,
538+ person_logged_in,
539+ TestCaseWithFactory,
540+ )
541+
542+
543+class TestPublicBugVisibility(TestCaseWithFactory):
544+ """Test visibility for a public bug."""
545+
546+ layer = LaunchpadFunctionalLayer
547+
548+ def setUp(self):
549+ super(TestPublicBugVisibility, self).setUp()
550+ owner = self.factory.makePerson(name="bugowner")
551+ self.bug = self.factory.makeBug(owner=owner)
552+
553+ def test_publicBugAnonUser(self):
554+ # userCanView does not get called for anonymous users.
555+ self.assertRaises(AssertionError, self.bug.userCanView, None)
556+
557+ def test_publicBugRegularUser(self):
558+ # A regular (non-privileged) user can view a public bug.
559+ user = self.factory.makePerson()
560+ self.assertTrue(self.bug.userCanView(user))
561+
562+
563+class TestPrivateBugVisibility(TestCaseWithFactory):
564+ """Test visibility for a private bug."""
565+
566+ layer = LaunchpadFunctionalLayer
567+
568+ def setUp(self):
569+ super(TestPrivateBugVisibility, self).setUp()
570+ self.owner = self.factory.makePerson(name="bugowner")
571+ self.product_owner = self.factory.makePerson(name="productowner")
572+ self.product = self.factory.makeProduct(
573+ name="regular-product", owner=self.product_owner)
574+ self.bug_team = self.factory.makeTeam(
575+ name="bugteam", owner=self.product.owner)
576+ self.bug_team_member = self.factory.makePerson(name="bugteammember")
577+ with person_logged_in(self.product.owner):
578+ self.bug_team.addMember(self.bug_team_member, self.product.owner)
579+ self.product.setBugSupervisor(
580+ bug_supervisor=self.bug_team,
581+ user=self.product.owner)
582+ self.bug = self.factory.makeBug(
583+ owner=self.owner, private=True, product=self.product)
584+
585+ def test_privateBugRegularUser(self):
586+ # A regular (non-privileged) user can not view a private bug.
587+ user = self.factory.makePerson()
588+ self.assertFalse(self.bug.userCanView(user))
589+
590+ def test_privateBugOwner(self):
591+ # The bug submitter may view a private bug.
592+ self.assertTrue(self.bug.userCanView(self.owner))
593+
594+ def test_privateBugSupervisor(self):
595+ # A member of the bug supervisor team can not see a private bug.
596+ self.assertFalse(self.bug.userCanView(self.bug_team_member))
597+
598+ def test_privateBugSubscriber(self):
599+ # A person subscribed to a private bug can see it.
600+ user = self.factory.makePerson()
601+ with person_logged_in(self.owner):
602+ self.bug.subscribe(user, self.owner)
603+ self.assertTrue(self.bug.userCanView(user))
604+
605+ def test_privateBugAssignee(self):
606+ # The bug assignee can see the private bug.
607+ bug_assignee = self.factory.makePerson(name="bugassignee")
608+ with person_logged_in(self.product.owner):
609+ self.bug.default_bugtask.transitionToAssignee(bug_assignee)
610+ self.assertTrue(self.bug.userCanView(bug_assignee))
611
612=== modified file 'lib/lp/registry/browser/tests/test_person_view.py'
613--- lib/lp/registry/browser/tests/test_person_view.py 2011-01-12 21:42:48 +0000
614+++ lib/lp/registry/browser/tests/test_person_view.py 2011-01-14 11:09:14 +0000
615@@ -6,7 +6,10 @@
616 import transaction
617 from storm.expr import LeftJoin
618 from storm.store import Store
619-from testtools.matchers import Equals
620+from testtools.matchers import (
621+ Equals,
622+ LessThan,
623+ )
624 from zope.component import getUtility
625
626 from canonical.config import config
627@@ -858,7 +861,7 @@
628 prejoins=[(Person, LeftJoin(Person, BugTask.owner==Person.id))]
629 bugtasks = view.searchUnbatched(prejoins=prejoins)
630 [bugtask.owner for bugtask in bugtasks]
631- self.assertThat(recorder, HasQueryCount(Equals(1)))
632+ self.assertThat(recorder, HasQueryCount(LessThan(3)))
633
634 def test_getMilestoneWidgetValues(self):
635 view = create_initialized_view(self.person, self.view_name)
636@@ -876,7 +879,7 @@
637 Store.of(milestones[0]).invalidate()
638 with StormStatementRecorder() as recorder:
639 self.assertEqual(expected, view.getMilestoneWidgetValues())
640- self.assertThat(recorder, HasQueryCount(Equals(1)))
641+ self.assertThat(recorder, HasQueryCount(LessThan(3)))
642
643
644 class TestPersonRelatedBugTaskSearchListingView(
645
646=== modified file 'lib/lp/registry/tests/test_product.py'
647--- lib/lp/registry/tests/test_product.py 2010-12-09 07:14:34 +0000
648+++ lib/lp/registry/tests/test_product.py 2011-01-14 11:09:14 +0000
649@@ -1,4 +1,4 @@
650-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
651+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
652 # GNU Affero General Public License version 3 (see the file LICENSE).
653
654 __metaclass__ = type
655@@ -10,7 +10,6 @@
656 from lazr.lifecycle.snapshot import Snapshot
657 import pytz
658 import transaction
659-from zope.component import getUtility
660
661 from canonical.launchpad.ftests import syncUpdate
662 from canonical.launchpad.testing.pages import (
663@@ -22,7 +21,6 @@
664 DatabaseFunctionalLayer,
665 LaunchpadFunctionalLayer,
666 )
667-from lp.registry.interfaces.person import IPersonSet
668 from lp.registry.interfaces.product import (
669 IProduct,
670 License,
671@@ -345,9 +343,8 @@
672 def testPersonCanSetSelfAsSupervisor(self):
673 # A person can set themselves as bug supervisor for a product.
674 # This is a regression test for bug 438985.
675- user = getUtility(IPersonSet).getByName(self.person.name)
676 self.product.setBugSupervisor(
677- bug_supervisor=self.person, user=user)
678+ bug_supervisor=self.person, user=self.person)
679
680 self.assertEqual(
681 self.product.bug_supervisor, self.person,