launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06875
[Merge] lp:~stevenk/launchpad/bugs-use-information_type-redux into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/bugs-use-information_type-redux into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/bugs-use-information_type-redux/+merge/99236
Resurrect the rollback of using IBug.information_type that happened in r15006. The original MPs description is as follows:
Rename IBug.{private,security_related} to IBug._{private,security_related} as the next step in switching to information_type. This branch also adds IBug.{private,security_related} back as a property that depends on information_type if it is set.
Fixed some tests that assumed that IBug.{private,security_related} was something they could set directly.
I cleaned up a small amount of lint.
Further to those changes, I have created a new method, convert_to_information_type, which returns the relevant InformationType given a private and security_related parameter, which allows me to remove IBug._setInformationType() and to set the information_type of the bug right at the initial insert. I have added a test which checks that case.
--
https://code.launchpad.net/~stevenk/launchpad/bugs-use-information_type-redux/+merge/99236
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/bugs-use-information_type-redux into lp:launchpad.
=== modified file 'lib/lp/bugs/adapters/bug.py'
--- lib/lp/bugs/adapters/bug.py 2010-08-20 20:31:18 +0000
+++ lib/lp/bugs/adapters/bug.py 2012-03-25 23:40:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 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).
"""Resources having to do with Launchpad bugs."""
@@ -7,11 +7,14 @@
__all__ = [
'bugcomment_to_entry',
'bugtask_to_privacy',
+ 'convert_to_information_type',
]
from lazr.restful.interfaces import IEntry
from zope.component import getMultiAdapter
+from lp.registry.enums import InformationType
+
def bugcomment_to_entry(comment, version):
"""Will adapt to the bugcomment to the real IMessage.
@@ -22,9 +25,21 @@
return getMultiAdapter(
(comment.bugtask.bug.messages[comment.index], version), IEntry)
+
def bugtask_to_privacy(bugtask):
"""Adapt the bugtask to the underlying bug (which implements IPrivacy).
Needed because IBugTask does not implement IPrivacy.
"""
return bugtask.bug
+
+
+def convert_to_information_type(private, security_related):
+ if private and security_related:
+ return InformationType.EMBARGOEDSECURITY
+ elif security_related:
+ return InformationType.UNEMBARGOEDSECURITY
+ elif private:
+ return InformationType.USERDATA
+ else:
+ return InformationType.PUBLIC
=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscription_views.py'
--- lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2012-03-24 04:42:32 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2012-03-25 23:40:26 +0000
@@ -40,10 +40,6 @@
from lp.testing.views import create_initialized_view
-ON = 'on'
-OFF = None
-
-
class BugsubscriptionPrivacyTests(TestCaseWithFactory):
layer = LaunchpadFunctionalLayer
@@ -52,7 +48,7 @@
super(BugsubscriptionPrivacyTests, self).setUp()
self.user = self.factory.makePerson()
self.bug = self.factory.makeBug(owner=self.user)
- removeSecurityProxy(self.bug).private = True
+ removeSecurityProxy(self.bug).setPrivate(True, self.user)
def _assert_subscription_fails(self, team):
with person_logged_in(self.user):
=== modified file 'lib/lp/bugs/mail/tests/test_handler.py'
--- lib/lp/bugs/mail/tests/test_handler.py 2012-03-24 04:42:32 +0000
+++ lib/lp/bugs/mail/tests/test_handler.py 2012-03-25 23:40:26 +0000
@@ -287,7 +287,7 @@
notification = self.getLatestBugNotification()
bug = notification.bug
self.assertEqual('unsecure code', bug.title)
- self.assertEqual(True, bug.security_related)
+ self.assertTrue(bug.security_related)
self.assertEqual(['ajax'], bug.tags)
self.assertEqual(1, len(bug.bugtasks))
self.assertEqual(project, bug.bugtasks[0].target)
@@ -310,7 +310,7 @@
notification = self.getLatestBugNotification()
bug = notification.bug
self.assertEqual('security issue', bug.title)
- self.assertEqual(True, bug.security_related)
+ self.assertTrue(bug.security_related)
self.assertEqual(1, len(bug.bugtasks))
self.assertEqual(project, bug.bugtasks[0].target)
recipients = set()
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2012-03-24 04:42:32 +0000
+++ lib/lp/bugs/model/bug.py 2012-03-25 23:40:26 +0000
@@ -89,6 +89,7 @@
)
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.app.validators import LaunchpadValidationError
+from lp.bugs.adapters.bug import convert_to_information_type
from lp.bugs.adapters.bugchange import (
BranchLinkedToBug,
BranchUnlinkedFromBug,
@@ -157,7 +158,11 @@
)
from lp.code.interfaces.branchcollection import IAllBranches
from lp.hardwaredb.interfaces.hwdb import IHWSubmissionBugSet
-from lp.registry.enums import InformationType
+from lp.registry.enums import (
+ InformationType,
+ PRIVATE_INFORMATION_TYPES,
+ SECURITY_INFORMATION_TYPES,
+ )
from lp.registry.interfaces.distribution import IDistribution
from lp.registry.interfaces.distroseries import IDistroSeries
from lp.registry.interfaces.person import (
@@ -347,12 +352,13 @@
dbName='duplicateof', foreignKey='Bug', default=None)
datecreated = UtcDateTimeCol(notNull=True, default=UTC_NOW)
date_last_updated = UtcDateTimeCol(notNull=True, default=UTC_NOW)
- private = BoolCol(notNull=True, default=False)
+ _private = BoolCol(dbName='private', notNull=True, default=False)
date_made_private = UtcDateTimeCol(notNull=False, default=None)
who_made_private = ForeignKey(
dbName='who_made_private', foreignKey='Person',
storm_validator=validate_public_person, default=None)
- security_related = BoolCol(notNull=True, default=False)
+ _security_related = BoolCol(
+ dbName='security_related', notNull=True, default=False)
information_type = EnumCol(
enum=InformationType, default=InformationType.PUBLIC)
@@ -389,6 +395,20 @@
heat_last_updated = UtcDateTimeCol(default=None)
latest_patch_uploaded = UtcDateTimeCol(default=None)
+ @property
+ def private(self):
+ if self.information_type:
+ return self.information_type in PRIVATE_INFORMATION_TYPES
+ else:
+ return self._private
+
+ @property
+ def security_related(self):
+ if self.information_type:
+ return self.information_type in SECURITY_INFORMATION_TYPES
+ else:
+ return self._security_related
+
@cachedproperty
def _subscriber_cache(self):
"""Caches known subscribers."""
@@ -1697,16 +1717,6 @@
return bugtask
- def _setInformationType(self):
- if self.private and self.security_related:
- self.information_type = InformationType.EMBARGOEDSECURITY
- elif self.private:
- self.information_type = InformationType.USERDATA
- elif self.security_related:
- self.information_type = InformationType.UNEMBARGOEDSECURITY
- else:
- self.information_type = InformationType.PUBLIC
-
def setPrivacyAndSecurityRelated(self, private, security_related, who):
""" See `IBug`."""
private_changed = False
@@ -1734,7 +1744,7 @@
raise BugCannotBePrivate(
"Multi-pillar bugs cannot be private.")
private_changed = True
- self.private = private
+ self._private = private
if private:
self.who_made_private = who
@@ -1750,14 +1760,15 @@
if self.security_related != security_related:
security_related_changed = True
- self.security_related = security_related
+ self._security_related = security_related
if private_changed or security_related_changed:
# Correct the heat for the bug immediately, so that we don't have
# to wait for the next calculation job for the adjusted heat.
self.updateHeat()
- self._setInformationType()
+ self.information_type = convert_to_information_type(
+ self._private, self._security_related)
if private_changed or security_related_changed:
changed_fields = []
@@ -2829,9 +2840,6 @@
bug.subscribe(params.product.bug_supervisor, params.owner)
else:
bug.subscribe(params.product.owner, params.owner)
- else:
- # nothing to do
- pass
# Create the task on a product if one was passed.
if params.product:
@@ -2858,8 +2866,6 @@
if notify_event:
notify(event)
- bug._setInformationType()
-
# Calculate the bug's initial heat.
bug.updateHeat()
@@ -2906,11 +2912,14 @@
date_made_private=params.datecreated,
who_made_private=params.owner)
+ information_type = convert_to_information_type(
+ params.private, params.security_related)
bug = Bug(
title=params.title, description=params.description,
- private=params.private, owner=params.owner,
+ _private=params.private, owner=params.owner,
datecreated=params.datecreated,
- security_related=params.security_related,
+ _security_related=params.security_related,
+ information_type=information_type,
**extra_params)
if params.subscribe_owner:
=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/model/tests/test_bug.py 2012-03-14 12:39:42 +0000
+++ lib/lp/bugs/model/tests/test_bug.py 2012-03-25 23:40:26 +0000
@@ -9,6 +9,7 @@
)
from pytz import UTC
+from storm.expr import Join
from storm.store import Store
from testtools.testcase import ExpectedException
from zope.component import getUtility
@@ -970,6 +971,22 @@
bug.setPrivate(True, bug.owner)
self.assertEqual(InformationType.USERDATA, bug.information_type)
+ def test_information_type_does_not_leak(self):
+ product = self.factory.makeProduct()
+ with person_logged_in(product.owner):
+ product.addSubscription(product.owner, product.owner)
+ reporter = self.factory.makePerson()
+ bug = self.factory.makeBug(
+ private=True, product=product, owner=reporter)
+ recipients = Store.of(bug).using(
+ BugNotificationRecipient,
+ Join(BugNotification, BugNotification.bugID == bug.id)).find(
+ BugNotificationRecipient,
+ BugNotificationRecipient.bug_notificationID ==
+ BugNotification.id)
+ self.assertEqual(
+ [reporter], [recipient.person for recipient in recipients])
+
class TestBugPrivateAndSecurityRelatedUpdatesPrivateProject(
TestBugPrivateAndSecurityRelatedUpdatesMixin, TestCaseWithFactory):
=== modified file 'lib/lp/bugs/tests/test_bug_mirror_access_triggers.py'
--- lib/lp/bugs/tests/test_bug_mirror_access_triggers.py 2012-03-24 04:42:32 +0000
+++ lib/lp/bugs/tests/test_bug_mirror_access_triggers.py 2012-03-25 23:40:26 +0000
@@ -136,7 +136,7 @@
bug = self.makeBugAndPolicies(private=True)
self.assertIsNot(
None, getUtility(IAccessArtifactSource).find([bug]).one())
- bug.private = False
+ removeSecurityProxy(bug).setPrivate(False, bug.owner)
self.assertIs(
None, getUtility(IAccessArtifactSource).find([bug]).one())
@@ -144,7 +144,7 @@
bug = self.makeBugAndPolicies(private=False)
self.assertIs(
None, getUtility(IAccessArtifactSource).find([bug]).one())
- bug.private = True
+ removeSecurityProxy(bug).setPrivate(True, bug.owner)
self.assertIsNot(
None, getUtility(IAccessArtifactSource).find([bug]).one())
self.assertEqual((1, 1), self.assertMirrored(bug))
@@ -158,7 +158,7 @@
self.assertContentEqual(
[InformationType.USERDATA],
self.getPolicyTypesForArtifact(artifact))
- bug.security_related = True
+ removeSecurityProxy(bug).setSecurityRelated(True, bug.owner)
self.assertEqual((1, 1), self.assertMirrored(bug))
self.assertContentEqual(
[InformationType.EMBARGOEDSECURITY],
=== modified file 'lib/lp/registry/enums.py'
--- lib/lp/registry/enums.py 2012-03-24 04:42:32 +0000
+++ lib/lp/registry/enums.py 2012-03-25 23:40:26 +0000
@@ -9,7 +9,9 @@
'DistroSeriesDifferenceType',
'InformationType',
'PersonTransferJobType',
+ 'PRIVATE_INFORMATION_TYPES',
'ProductJobType',
+ 'SECURITY_INFORMATION_TYPES',
'SharingPermission',
]
@@ -61,6 +63,15 @@
""")
+PRIVATE_INFORMATION_TYPES = (
+ InformationType.EMBARGOEDSECURITY, InformationType.USERDATA,
+ InformationType.PROPRIETARY)
+
+
+SECURITY_INFORMATION_TYPES = (
+ InformationType.UNEMBARGOEDSECURITY, InformationType.EMBARGOEDSECURITY)
+
+
class SharingPermission(DBEnumeratedType):
"""Sharing permission.
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2012-03-24 04:42:32 +0000
+++ lib/lp/registry/model/person.py 2012-03-25 23:40:26 +0000
@@ -1744,14 +1744,14 @@
Bug,
Join(BugSubscription, BugSubscription.bug_id == Bug.id)),
where=And(
- Bug.private == True,
+ Bug._private == True,
BugSubscription.person_id == self.id)),
Select(
Bug.id,
tables=(
Bug,
Join(BugTask, BugTask.bugID == Bug.id)),
- where=And(Bug.private == True, BugTask.assignee == self.id)),
+ where=And(Bug._private == True, BugTask.assignee == self.id)),
limit=1))
if private_bugs_involved.rowcount:
raise TeamSubscriptionPolicyError(
=== modified file 'lib/lp/services/feeds/stories/xx-security.txt'
--- lib/lp/services/feeds/stories/xx-security.txt 2012-03-24 04:42:32 +0000
+++ lib/lp/services/feeds/stories/xx-security.txt 2012-03-25 23:40:26 +0000
@@ -1,16 +1,16 @@
-== Feeds do not display private bugs ==
+Feeds do not display private bugs
+=================================
Feeds never contain private bugs, as we are serving feeds over HTTP.
First, set all the bugs to private.
>>> from zope.security.interfaces import Unauthorized
>>> from BeautifulSoup import BeautifulStoneSoup as BSS
+ >>> from lp.services.database.lpstorm import IStore
+ >>> import transaction
>>> from lp.bugs.model.bug import Bug
- >>> bugs = Bug.select()
- >>> for bug in bugs:
- ... bug.private = True
- >>> from lp.services.database.sqlbase import flush_database_updates
- >>> flush_database_updates()
+ >>> IStore(Bug).find(Bug).set(_private=True)
+ >>> transaction.commit()
There should be zero entries in these feeds, since all the bugs are private.
@@ -26,7 +26,8 @@
>>> BSS(browser.contents)('entry')
[]
- >>> browser.open('http://feeds.launchpad.dev/~simple-team/latest-bugs.atom')
+ >>> browser.open(
+ ... 'http://feeds.launchpad.dev/~simple-team/latest-bugs.atom')
>>> BSS(browser.contents)('entry')
[]
@@ -66,7 +67,8 @@
>>> print extract_text(BSS(browser.contents)('tr')[0])
Bugs for Foo Bar
- >>> browser.open('http://feeds.launchpad.dev/~simple-team/latest-bugs.html')
+ >>> browser.open(
+ ... 'http://feeds.launchpad.dev/~simple-team/latest-bugs.html')
>>> len(BSS(browser.contents)('tr'))
1
