launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06487
[Merge] lp:~stevenk/launchpad/less-bug-leakage into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/less-bug-leakage into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/less-bug-leakage/+merge/94495
Plug a large number of leaks in IBug. Do not be fooled by the small diff size in the ZCML, this branch moves 29 attributes on IBug from zope.Public to launchpad.View.
I have fixed the factory by papering over it with removeSecurityProxy(), but have fixed other tests by either grabbing the task in an interaction, moving it out to where Zope lookups are not hard to deal with, or in the cases the test was already using rSP(), fixing the call.
--
https://code.launchpad.net/~stevenk/launchpad/less-bug-leakage/+merge/94495
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/less-bug-leakage into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py 2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py 2012-02-24 04:06:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for Bug Views."""
@@ -26,6 +26,7 @@
from lp.services.webapp.servers import LaunchpadTestRequest
from lp.testing import (
BrowserTestCase,
+ celebrity_logged_in,
login_person,
person_logged_in,
TestCaseWithFactory,
@@ -316,11 +317,12 @@
# blocked from doing so.
view = self.createInitializedSecrecyView()
bug = view.context.bug
+ with celebrity_logged_in('admin'):
+ task = bug.default_bugtask
self.assertEqual(1, len(view.request.response.notifications))
notification = view.request.response.notifications[0].message
- mute_url = canonical_url(bug.default_bugtask, view_name='+mute')
- subscribe_url = canonical_url(
- bug.default_bugtask, view_name='+subscribe')
+ mute_url = canonical_url(task, view_name='+mute')
+ subscribe_url = canonical_url(task, view_name='+subscribe')
self.assertIn(mute_url, notification)
self.assertIn(subscribe_url, notification)
@@ -386,7 +388,7 @@
'Discussion', subscription_data['bug_notification_level'])
[subscriber_data] = result_data['subscription_data']
- subscriber = removeSecurityProxy(bug.default_bugtask).pillar.owner
+ subscriber = removeSecurityProxy(bug).default_bugtask.pillar.owner
self.assertEqual(
subscriber.name, subscriber_data['subscriber']['name'])
self.assertEqual('Discussion', subscriber_data['subscription_level'])
=== modified file 'lib/lp/bugs/browser/tests/test_bugcomment.py'
--- lib/lp/bugs/browser/tests/test_bugcomment.py 2012-02-01 15:26:32 +0000
+++ lib/lp/bugs/browser/tests/test_bugcomment.py 2012-02-24 04:06:20 +0000
@@ -16,10 +16,8 @@
HTMLContains,
Tag,
)
-from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
-from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.bugs.interfaces.bugmessage import IBugComment
from lp.bugs.browser.bugcomment import (
BugComment,
@@ -36,7 +34,6 @@
BrowserTestCase,
celebrity_logged_in,
login_person,
- person_logged_in,
TestCase,
TestCaseWithFactory,
)
@@ -239,19 +236,17 @@
def getContext(self, comment_owner=None):
"""Required by the mixin."""
- administrator = getUtility(ILaunchpadCelebrities).admin.teamowner
bug = self.factory.makeBug()
- with person_logged_in(administrator):
+ with celebrity_logged_in('admin'):
self.factory.makeBugComment(bug=bug, owner=comment_owner)
return bug
def getView(self, context, user=None, no_login=False):
"""Required by the mixin."""
- view = self.getViewBrowser(
- context=context.default_bugtask,
- user=user,
- no_login=no_login)
- return view
+ with celebrity_logged_in('admin'):
+ task = context.default_bugtask
+ return self.getViewBrowser(
+ context=task, user=user, no_login=no_login)
def _test_hide_link_visible(self, context, user):
view = self.getView(context=context, user=user)
=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py 2012-02-15 01:59:43 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py 2012-02-24 04:06:20 +0000
@@ -163,27 +163,27 @@
source_branch=source_branch)
def test_rendered_query_counts_reduced_with_branches(self):
- f = self.factory
- owner = f.makePerson()
- ds = f.makeDistroSeries()
- bug = f.makeBug()
+ owner = self.factory.makePerson()
+ ds = self.factory.makeDistroSeries()
+ bug = self.factory.makeBug()
sourcepackages = [
- f.makeSourcePackage(distroseries=ds, publish=True)
+ self.factory.makeSourcePackage(distroseries=ds, publish=True)
for i in range(5)]
for sp in sourcepackages:
- f.makeBugTask(bug=bug, owner=owner, target=sp)
- url = canonical_url(bug.default_bugtask)
+ self.factory.makeBugTask(bug=bug, owner=owner, target=sp)
+ task = bug.default_bugtask
+ url = canonical_url(task)
recorder = QueryCollector()
recorder.register()
self.addCleanup(recorder.unregister)
- self.invalidate_caches(bug.default_bugtask)
+ self.invalidate_caches(task)
self.getUserBrowser(url, owner)
# At least 20 of these should be removed.
self.assertThat(recorder, HasQueryCount(LessThan(107)))
count_with_no_branches = recorder.count
for sp in sourcepackages:
self.makeLinkedBranchMergeProposal(sp, bug, owner)
- self.invalidate_caches(bug.default_bugtask)
+ self.invalidate_caches(task)
self.getUserBrowser(url, owner) # This triggers the query recorder.
# Ideally this should be much fewer, but this tries to keep a win of
# removing more than half of these.
@@ -1797,7 +1797,8 @@
owner = factory.makePerson()
bug = factory.makeBug(
owner=owner, private=True, security_related=True)
- bugtask = bug.default_bugtask
+ with person_logged_in(owner):
+ bugtask = bug.default_bugtask
bug_task_set = getUtility(IBugTaskSet)
bug_badge_properties = bug_task_set.getBugTaskBadgeProperties(
[bugtask])
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2012-02-15 12:30:20 +0000
+++ lib/lp/bugs/configure.zcml 2012-02-24 04:06:20 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+<!-- Copyright 2009-2012 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
@@ -722,6 +722,10 @@
attributes="
id
private
+ userCanView"/>
+ <require
+ permission="launchpad.View"
+ attributes="
bugtasks
default_bugtask
affected_pillars
@@ -741,7 +745,6 @@
official_tags
who_made_private
date_made_private
- userCanView
userCanSetCommentVisibility
personIsDirectSubscriber
personIsAlsoNotifiedSubscriber
@@ -751,10 +754,7 @@
heat_last_updated
has_patches
latest_patch
- latest_patch_uploaded"/>
- <require
- permission="launchpad.View"
- attributes="
+ latest_patch_uploaded
datecreated
name
title
@@ -821,8 +821,7 @@
isUserAffected
getHWSubmissions
isExpirable
- isMuted
- "/>
+ isMuted"/>
<require
permission="launchpad.Edit"
attributes="
=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/model/tests/test_bug.py 2012-01-06 11:08:30 +0000
+++ lib/lp/bugs/model/tests/test_bug.py 2012-02-24 04:06:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -613,7 +613,7 @@
bugtask_b = self.factory.makeBugTask(bug=bug, target=product_series_b)
naked_bugtask_a = removeSecurityProxy(bugtask_a)
naked_bugtask_b = removeSecurityProxy(bugtask_b)
- naked_default_bugtask = removeSecurityProxy(bug.default_bugtask)
+ naked_default_bugtask = removeSecurityProxy(bug).default_bugtask
return (bug, bug_owner, naked_bugtask_a, naked_bugtask_b,
naked_default_bugtask)
@@ -748,7 +748,7 @@
bug.setPrivacyAndSecurityRelated(
private=True, security_related=False, who=who)
subscribers = bug.getDirectSubscribers()
- naked_bugtask = removeSecurityProxy(bug.default_bugtask)
+ naked_bugtask = removeSecurityProxy(bug).default_bugtask
self.assertContentEqual(
set((naked_bugtask.pillar.owner, bug_owner, who)),
subscribers)
@@ -763,7 +763,7 @@
bug.setPrivacyAndSecurityRelated(
private=False, security_related=True, who=who)
subscribers = bug.getDirectSubscribers()
- naked_bugtask = removeSecurityProxy(bug.default_bugtask)
+ naked_bugtask = removeSecurityProxy(bug).default_bugtask
self.assertContentEqual(
set((naked_bugtask.pillar.owner, bug_owner)),
subscribers)
=== modified file 'lib/lp/bugs/stories/bugs/xx-bug-personal-subscriptions-advanced-features.txt'
--- lib/lp/bugs/stories/bugs/xx-bug-personal-subscriptions-advanced-features.txt 2011-12-24 17:49:30 +0000
+++ lib/lp/bugs/stories/bugs/xx-bug-personal-subscriptions-advanced-features.txt 2012-02-24 04:06:20 +0000
@@ -8,9 +8,9 @@
>>> from lp.testing.sampledata import USER_EMAIL
>>> login(USER_EMAIL)
>>> bug = factory.makeBug()
+ >>> task = bug.default_bugtask
>>> logout()
- >>> user_browser.open(
- ... canonical_url(bug.default_bugtask, view_name='+subscribe'))
+ >>> user_browser.open(canonical_url(task, view_name='+subscribe'))
>>> bug_notification_level_control = user_browser.getControl(
... name='field.bug_notification_level')
>>> for control in bug_notification_level_control.controls:
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2012-02-20 02:07:55 +0000
+++ lib/lp/testing/factory.py 2012-02-24 04:06:20 +0000
@@ -1679,7 +1679,7 @@
# fromText() creates a bug watch associated with the bug.
with person_logged_in(owner):
getUtility(IBugWatchSet).fromText(bug_watch_url, bug, owner)
- bugtask = bug.default_bugtask
+ bugtask = removeSecurityProxy(bug).default_bugtask
if date_closed is not None:
with person_logged_in(owner):
bugtask.transitionToStatus(
@@ -1716,7 +1716,7 @@
# Find and return the existing target if one exists.
if bug is not None and target is not None:
- existing_bugtask = bug.getBugTask(target)
+ existing_bugtask = removeSecurityProxy(bug).getBugTask(target)
if existing_bugtask is not None:
return existing_bugtask
@@ -1755,7 +1755,8 @@
distroseries=target.distribution.currentseries,
sourcepackagename=target.sourcepackagename)
if prerequisite_target is not None:
- prerequisite = bug and bug.getBugTask(prerequisite_target)
+ prerequisite = bug and removeSecurityProxy(bug).getBugTask(
+ prerequisite_target)
if prerequisite is None:
prerequisite = self.makeBugTask(
bug, prerequisite_target, publish=publish)
