launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00388
[Merge] lp:~adeuring/launchpad/bug-39674-flip-lfa-restricted-flag into lp:launchpad
Abel Deuring has proposed merging lp:~adeuring/launchpad/bug-39674-flip-lfa-restricted-flag into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
This branch is another sequel to fix bug 39674: "Attachments of private bugreports are public".
When an attachment is adde to a private bug, the "restricted" flag of the related Librarian file is set. Similary, the "restricted" flag is changed when Bug.setPrivate() is called.
I removed the method IBug.linkAttachment(), which allows to link an existing LibraryFileAlias record to a bug attachment. Allowing to link existing LFAs to bug attachments could cause inconsistencies for LFA.restricted, if one LFA is linked to a private and to another public bug. (We had also a short discussion about this issue on the LP developers mailing list.)
Bug.linkAttachment() is at present only used in in bug.AddAttachment, so there was no need to explicity change other code. But I removed the section of doc/bugattachments.txt which describes this method. The "side tests", for example the patch flag, are covered elsewhere is that file.
test: ./bin/test -t bugattachments.txt
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/bugs/doc/bugattachments.txt
lib/lp/bugs/interfaces/bug.py
lib/lp/bugs/model/bug.py
./lib/lp/bugs/interfaces/bug.py
375: E301 expected 1 blank line, found 2
1064: E301 expected 1 blank line, found 2
Seems that the linter is confused by comments...
This branch is based on lp:~adeuring/launchpad/bug-39674-use-proxied-lfa which is reviewed but which is still in EC2. The diff against that branch:
=== modified file 'lib/lp/bugs/doc/bugattachments.txt'
--- lib/lp/bugs/doc/bugattachments.txt 2010-07-27 14:12:47 +0000
+++ lib/lp/bugs/doc/bugattachments.txt 2010-08-02 16:46:27 +0000
@@ -521,72 +521,25 @@
True
-Linking existing LibraryFileAliases as attachments
---------------------------------------------------
-
-It's possible to link an existing LibraryFileAliases to a bug as an
-attachment by calling the bug's linkAttachment() method.
-
- >>> from canonical.launchpad.interfaces.librarian import (
- ... ILibraryFileAliasSet)
-
- >>> file_content = "Hello, world"
- >>> content_type = "text/plain"
- >>> file_alias = getUtility(ILibraryFileAliasSet).create(
- ... name='foobar', size=len(file_content),
- ... file=StringIO(file_content), contentType=content_type)
- >>> transaction.commit()
-
- >>> bug = factory.makeBug()
- >>> bug.linkAttachment(
- ... owner=bug.owner, file_alias=file_alias,
- ... comment="Some attachment")
- <BugAttachment ...>
-
- >>> bug.attachments.count()
- 1
- >>> attachment = bug.attachments[0]
- >>> print attachment.title
- foobar
-
-The attachment will have a type of BugAttachmentType.UNSPECIFIED, since
-we didn't specify that it was a patch.
-
- >>> print attachment.type.title
- Unspecified
-
-We can specify that the attachment is a patch and give it a more
-meaningful description.
-
- >>> file_alias = getUtility(ILibraryFileAliasSet).create(
- ... name='anotherfoobar', size=len(file_content),
- ... file=StringIO(file_content), contentType=content_type)
- >>> transaction.commit()
-
- >>> bug.linkAttachment(
- ... owner=bug.owner, file_alias=file_alias,
- ... comment="Some attachment", is_patch=True,
- ... description="An attachment of some sort")
- <BugAttachment ...>
-
- >>> bug.attachments.count()
- 2
- >>> attachment = bug.attachments[1]
- >>> print attachment.title
- An attachment of some sort
-
- >>> print attachment.type.title
- Patch
-
-
Attachments without library files
---------------------------------
It can happen that the LibraryFileContent record of a bug attachment is
deleted, for example. because an admin deleted a privacy sensitive file.
These attachments are not included in Bug.attachments. Our test bug has
-at present two attachments.
+two attachments.
+ >>> bug = factory.makeBug()
+ >>> file_content = "Hello, world"
+ >>> bug.addAttachment(
+ ... owner=bug.owner, data="Hello, world", filename="foobar",
+ ... comment="Some attachment")
+ <BugAttachment ...>
+ >>> bug.addAttachment(
+ ... owner=bug.owner, data="whatever", filename='anotherfoobar',
+ ... comment="Some attachment",
+ ... description="An attachment of some sort")
+ <BugAttachment ...>
>>> [attachment.title for attachment in bug.attachments]
[u'foobar', u'An attachment of some sort']
@@ -599,6 +552,42 @@
[u'foobar']
+Adding bug attachments to private bugs
+--------------------------------------
+
+If an attachment is added to a private bug, the "restricted" flag of
+its Librarian file is set.
+
+ >>> private_bug_owner = factory.makePerson()
+ >>> login_person(private_bug_owner)
+ >>> private_bug = factory.makeBug(private=True, owner=private_bug_owner)
+ >>> private_attachment = private_bug.addAttachment(
+ ... owner=private_bug_owner, data="secret", filename="baz.txt",
+ ... comment="Some attachment")
+ >>> private_attachment.libraryfile.restricted
+ True
+
+But the "restricted" flag of Librarian files elonging to bug attachments
+of public bugs is not set.
+
+ >>> attachment.libraryfile.restricted
+ False
+
+If a private bug becomes public, the restricted flag of the related
+Librarian files are no longer set.
+
+ >>> changed = private_bug.setPrivate(False, private_bug.owner)
+ >>> private_attachment.libraryfile.restricted
+ False
+
+Similary, if a public bug becomes private, the "restricted" flag of
+its Librarian files are set.
+
+ >>> changed = bug.setPrivate(True, bug.owner)
+ >>> attachment.libraryfile.restricted
+ True
+
+
Miscellaneous
-------------
=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py 2010-07-22 12:17:41 +0000
+++ lib/lp/bugs/interfaces/bug.py 2010-08-02 16:51:31 +0000
@@ -135,6 +135,7 @@
except NotFoundError:
return None
+
class IBugBecameQuestionEvent(Interface):
"""A bug became a question."""
@@ -522,18 +523,6 @@
:is_patch: A boolean.
"""
- def linkAttachment(owner, file_alias, comment, is_patch=False,
- description=None):
- """Link an `ILibraryFileAlias` to this bug.
-
- :owner: An IPerson.
- :file_alias: The `ILibraryFileAlias` to link to this bug.
- :description: A brief description of the attachment.
- :comment: An IMessage or string.
- :filename: A string.
- :is_patch: A boolean.
- """
-
def linkCVE(cve, user):
"""Ensure that this CVE is linked to this bug."""
@@ -716,7 +705,7 @@
This may also cause the security contact to be subscribed
if one is registered and if the bug is not private.
-
+
Return True if a change is made, False otherwise.
"""
@@ -825,6 +814,7 @@
Returns True or False.
"""
+
class InvalidDuplicateValue(Exception):
"""A bug cannot be set as the duplicate of another."""
webservice_error(417)
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2010-07-22 20:30:26 +0000
+++ lib/lp/bugs/model/bug.py 2010-08-02 16:50:45 +0000
@@ -694,7 +694,8 @@
if recipients is not None:
for subscriber in dupe_subscribers:
- recipients.addDupeSubscriber(subscriber, dupe_details[subscriber])
+ recipients.addDupeSubscriber(
+ subscriber, dupe_details[subscriber])
return sorted(
dupe_subscribers, key=operator.attrgetter("displayname"))
@@ -959,13 +960,26 @@
filealias = getUtility(ILibraryFileAliasSet).create(
name=filename, size=len(filecontent),
- file=StringIO(filecontent), contentType=content_type)
+ file=StringIO(filecontent), contentType=content_type,
+ restricted=self.private)
return self.linkAttachment(
owner, filealias, comment, is_patch, description)
def linkAttachment(self, owner, file_alias, comment, is_patch=False,
description=None):
+ """Link an `ILibraryFileAlias` to this bug.
+
+ :owner: An IPerson.
+ :file_alias: The `ILibraryFileAlias` to link to this bug.
+ :description: A brief description of the attachment.
+ :comment: An IMessage or string.
+ :is_patch: A boolean.
+
+ This method should only be called by addAttachment(), otherwise
+ we may get inconsistent settings of bug.private and
+ file_alias.restricted.
+ """
if is_patch:
attach_type = BugAttachmentType.PATCH
else:
@@ -1388,6 +1402,9 @@
self.who_made_private = None
self.date_made_private = None
+ for attachment in self.attachments:
+ attachment.libraryfile.restricted = private
+
# 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()
@@ -1941,4 +1958,3 @@
def asDict(self):
"""Return the FileBugData instance as a dict."""
return self.__dict__.copy()
-
--
https://code.launchpad.net/~adeuring/launchpad/bug-39674-flip-lfa-restricted-flag/+merge/31558
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/bug-39674-flip-lfa-restricted-flag into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py 2010-07-22 12:17:41 +0000
+++ lib/lp/bugs/browser/bug.py 2010-08-02 17:19:19 +0000
@@ -49,6 +49,7 @@
from canonical.cachedproperty import cachedproperty
from canonical.launchpad import _
+from canonical.launchpad.browser.librarian import ProxiedLibraryFileAlias
from canonical.launchpad.webapp.interfaces import ILaunchBag, NotFoundError
from lp.bugs.interfaces.bug import IBug, IBugSet
from lp.bugs.interfaces.bugattachment import BugAttachmentType
@@ -472,16 +473,28 @@
@property
def regular_attachments(self):
"""The list of bug attachments that are not patches."""
- return [attachment
- for attachment in self.context.attachments
- if attachment.type != BugAttachmentType.PATCH]
+ return [
+ {
+ 'attachment': attachment,
+ 'file': ProxiedLibraryFileAlias(
+ attachment.libraryfile, attachment),
+ }
+ for attachment in self.context.attachments
+ if attachment.type != BugAttachmentType.PATCH
+ ]
@property
def patches(self):
"""The list of bug attachments that are patches."""
- return [attachment
- for attachment in self.context.attachments
- if attachment.type == BugAttachmentType.PATCH]
+ return [
+ {
+ 'attachment': attachment,
+ 'file': ProxiedLibraryFileAlias(
+ attachment.libraryfile, attachment),
+ }
+ for attachment in self.context.attachments
+ if attachment.type == BugAttachmentType.PATCH
+ ]
class BugView(LaunchpadView, BugViewMixin):
=== modified file 'lib/lp/bugs/browser/bugattachment.py'
--- lib/lp/bugs/browser/bugattachment.py 2010-03-30 20:56:51 +0000
+++ lib/lp/bugs/browser/bugattachment.py 2010-08-02 17:19:19 +0000
@@ -1,5 +1,4 @@
-
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2010 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Bug attachment views."""
@@ -7,6 +6,7 @@
__metaclass__ = type
__all__ = [
'BugAttachmentContentCheck',
+ 'BugAttachmentFileNavigation',
'BugAttachmentSetNavigation',
'BugAttachmentEditView',
'BugAttachmentURL',
@@ -18,8 +18,9 @@
from zope.component import getUtility
from zope.contenttype import guess_content_type
+from canonical.launchpad.browser.librarian import FileNavigationMixin
from canonical.launchpad.webapp import (
- canonical_url, custom_widget, GetitemNavigation)
+ canonical_url, custom_widget, GetitemNavigation, Navigation)
from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
from canonical.launchpad.webapp.interfaces import ILaunchBag
from lp.bugs.interfaces.bugattachment import (
@@ -34,6 +35,8 @@
from canonical.widgets.itemswidgets import LaunchpadBooleanRadioWidget
+from lp.bugs.interfaces.bugattachment import IBugAttachment
+
class BugAttachmentContentCheck:
"""A mixin class that checks the consistency of patch flag and file type.
@@ -222,3 +225,9 @@
def is_patch(self):
"""True if this attachment contains a patch, else False."""
return self.context.type == BugAttachmentType.PATCH
+
+
+class BugAttachmentFileNavigation(Navigation, FileNavigationMixin):
+ """Traversal to +files/${filename}."""
+
+ usedfor = IBugAttachment
=== modified file 'lib/lp/bugs/browser/bugcomment.py'
--- lib/lp/bugs/browser/bugcomment.py 2010-06-22 12:53:50 +0000
+++ lib/lp/bugs/browser/bugcomment.py 2010-08-02 17:19:19 +0000
@@ -26,6 +26,7 @@
from lp.bugs.interfaces.bugmessage import (
IBugComment, IBugMessageSet)
from lp.registry.interfaces.person import IPersonSet
+from canonical.launchpad.browser.librarian import ProxiedLibraryFileAlias
from canonical.launchpad.webapp import canonical_url, LaunchpadView
from canonical.launchpad.webapp.interfaces import ILaunchBag
from canonical.launchpad.webapp.authorization import check_permission
@@ -265,13 +266,22 @@
self.comment.index, self.context.bug.id)
-class BugCommentBoxView(LaunchpadView):
+class BugCommentBoxViewMixin:
+ """A class which provides proxied Librarian URLs for bug attachments."""
+
+ def proxiedUrlOfLibraryFileAlias(self, attachment):
+ """Return the proxied URL for the Librarian file of the attachment."""
+ return ProxiedLibraryFileAlias(
+ attachment.libraryfile, attachment).http_url
+
+
+class BugCommentBoxView(LaunchpadView, BugCommentBoxViewMixin):
"""Render a comment box with reply field collapsed."""
expand_reply_box = False
-class BugCommentBoxExpandedReplyView(LaunchpadView):
+class BugCommentBoxExpandedReplyView(LaunchpadView, BugCommentBoxViewMixin):
"""Render a comment box with reply field expanded."""
expand_reply_box = True
=== modified file 'lib/lp/bugs/browser/configure.zcml'
--- lib/lp/bugs/browser/configure.zcml 2010-06-25 13:34:37 +0000
+++ lib/lp/bugs/browser/configure.zcml 2010-08-02 17:19:19 +0000
@@ -767,7 +767,7 @@
<browser:navigation
module="lp.bugs.browser.bugattachment"
classes="
- BugAttachmentSetNavigation"/>
+ BugAttachmentSetNavigation BugAttachmentFileNavigation"/>
<browser:page
name="+bugsupervisor"
for="lp.bugs.interfaces.bugsupervisor.IHasBugSupervisor"
=== modified file 'lib/lp/bugs/browser/tests/bug-views.txt'
--- lib/lp/bugs/browser/tests/bug-views.txt 2010-07-22 12:17:41 +0000
+++ lib/lp/bugs/browser/tests/bug-views.txt 2010-08-02 17:19:19 +0000
@@ -263,8 +263,11 @@
== Bug Attachments ==
We show bug attachments in two lists: patches and non-patch attachments.
-Sequences of patch and non-patch attachments are provided by the
-properties `patches` and `regular_attachments` of the class BugView.
+Sequences with data about patch and non-patch attachments are provided
+by the properties `patches` and `regular_attachments` of the class
+BugView. The elements of the sequences are dictionaries containing
+the the attachment itself and a ProxiedLibraryFileAlias for the
+librarian file of the attachment.
>>> from lp.bugs.browser.bug import BugView
>>> login('foo.bar@xxxxxxxxxxxxx')
@@ -279,10 +282,18 @@
>>> patch_2 = factory.makeBugAttachment(
... bug=bug_seven, description='patch 2', is_patch=True)
>>> view = BugView(bug_seven, request)
- >>> [attachment.title for attachment in view.regular_attachments]
+ >>> [attachment['attachment'].title
+ ... for attachment in view.regular_attachments]
[u'attachment 1', u'attachment 2']
- >>> [patch.title for patch in view.patches]
+ >>> [patch['attachment'].title for patch in view.patches]
[u'patch 1', u'patch 2']
+ >>> [attachment['file'].http_url
+ ... for attachment in view.regular_attachments]
+ [u'http://bugs.launchpad.dev/firefox/+bug/5/+attachment/1/+files/...',
+ u'http://bugs.launchpad.dev/firefox/+bug/5/+attachment/2/+files/...']
+ >>> [patch['file'].http_url for patch in view.patches]
+ [u'http://bugs.launchpad.dev/firefox/+bug/5/+attachment/3/+files/...',
+ u'http://bugs.launchpad.dev/firefox/+bug/5/+attachment/4/+files/...']
== Bug Navigation ==
=== added file 'lib/lp/bugs/browser/tests/test_bugattachment_file_access.py'
--- lib/lp/bugs/browser/tests/test_bugattachment_file_access.py 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/browser/tests/test_bugattachment_file_access.py 2010-08-02 17:19:19 +0000
@@ -0,0 +1,106 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+import re
+import transaction
+
+from zope.component import getMultiAdapter, getUtility
+from zope.publisher.interfaces import NotFound
+from zope.security.interfaces import Unauthorized
+
+from canonical.launchpad.browser.librarian import (
+ StreamOrRedirectLibraryFileAliasView)
+from canonical.launchpad.interfaces import ILaunchBag
+from canonical.launchpad.interfaces.librarian import (
+ ILibraryFileAliasWithParent)
+from canonical.testing import LaunchpadFunctionalLayer
+from canonical.launchpad.webapp.publisher import RedirectionView
+from canonical.launchpad.webapp.servers import LaunchpadTestRequest
+
+from lp.bugs.browser.bugattachment import BugAttachmentFileNavigation
+from lp.testing import login_person, TestCaseWithFactory
+
+
+class TestAccessToBugAttachmentFiles(TestCaseWithFactory):
+ """Tests of traversal to and access of files of bug attachments."""
+
+ layer = LaunchpadFunctionalLayer
+
+ def setUp(self):
+ super(TestAccessToBugAttachmentFiles, self).setUp()
+ self.bug_owner = self.factory.makePerson()
+ getUtility(ILaunchBag).clear()
+ login_person(self.bug_owner)
+ self.bug = self.factory.makeBug(owner=self.bug_owner)
+ self.bugattachment = self.factory.makeBugAttachment(
+ bug=self.bug, filename='foo.txt', data='file content')
+
+ def test_traversal_to_lfa_of_bug_attachment(self):
+ # Traversing to the URL provided by a ProxiedLibraryFileAlias of a
+ # bug attachament returns a StreamOrRedirectLibraryFileAliasView.
+ request = LaunchpadTestRequest()
+ request.setTraversalStack(['foo.txt'])
+ navigation = BugAttachmentFileNavigation(
+ self.bugattachment, request)
+ view = navigation.publishTraverse(request, '+files')
+ self.assertIsInstance(view, StreamOrRedirectLibraryFileAliasView)
+
+ def test_traversal_to_lfa_of_bug_attachment_wrong_filename(self):
+ # If the filename provided in the URL does not match the
+ # filename of the LibraryFileAlias, a NotFound error is raised.
+ request = LaunchpadTestRequest()
+ request.setTraversalStack(['nonsense'])
+ navigation = BugAttachmentFileNavigation(self.bugattachment, request)
+ self.assertRaises(
+ NotFound, navigation.publishTraverse, request, '+files')
+
+ def test_access_to_unrestricted_file(self):
+ # Requests of unrestricted files are redirected to Librarian URLs.
+ request = LaunchpadTestRequest()
+ request.setTraversalStack(['foo.txt'])
+ navigation = BugAttachmentFileNavigation(
+ self.bugattachment, request)
+ view = navigation.publishTraverse(request, '+files')
+ next_view, traversal_path = view.browserDefault(request)
+ self.assertIsInstance(next_view, RedirectionView)
+ mo = re.match(
+ '^http://localhost:58000/\d+/foo.txt$', next_view.target)
+ self.assertIsNot(None, mo)
+
+ def test_access_to_restircted_file(self):
+ # Requests of restricted files are handled by ProxiedLibraryFileAlias.
+ lfa_with_parent = getMultiAdapter(
+ (self.bugattachment.libraryfile, self.bugattachment),
+ ILibraryFileAliasWithParent)
+ lfa_with_parent.restricted = True
+ self.bug.setPrivate(True, self.bug_owner)
+ transaction.commit()
+ request = LaunchpadTestRequest()
+ request.setTraversalStack(['foo.txt'])
+ navigation = BugAttachmentFileNavigation(self.bugattachment, request)
+ view = navigation.publishTraverse(request, '+files')
+ next_view, traversal_path = view.browserDefault(request)
+ self.assertEqual(view, next_view)
+ file_ = next_view()
+ file_.seek(0)
+ self.assertEqual('file content', file_.read())
+
+ def test_access_to_restircted_file_unauthorized(self):
+ # If a user cannot access the bug attachment itself, he can neither
+ # access the restricted Librarian file.
+ lfa_with_parent = getMultiAdapter(
+ (self.bugattachment.libraryfile, self.bugattachment),
+ ILibraryFileAliasWithParent)
+ lfa_with_parent.restricted = True
+ self.bug.setPrivate(True, self.bug_owner)
+ transaction.commit()
+ user = self.factory.makePerson()
+ login_person(user)
+ self.assertRaises(Unauthorized, getattr, self.bugattachment, 'title')
+ request = LaunchpadTestRequest()
+ request.setTraversalStack(['foo.txt'])
+ navigation = BugAttachmentFileNavigation(self.bugattachment, request)
+ self.assertRaises(
+ Unauthorized, navigation.publishTraverse, request, '+files')
=== modified file 'lib/lp/bugs/browser/tests/test_bugview.py'
--- lib/lp/bugs/browser/tests/test_bugview.py 2010-04-07 11:28:32 +0000
+++ lib/lp/bugs/browser/tests/test_bugview.py 2010-08-02 17:19:19 +0000
@@ -39,7 +39,7 @@
removeSecurityProxy(attachment.libraryfile).content = None
self.assertEqual(
['regular attachment'],
- [attachment.title
+ [attachment['attachment'].title
for attachment in self.view.regular_attachments])
def test_patches_dont_include_invalid_records(self):
@@ -54,7 +54,8 @@
removeSecurityProxy(patch.libraryfile).content = None
self.assertEqual(
['patch'],
- [attachment.title for attachment in self.view.patches])
+ [attachment['attachment'].title
+ for attachment in self.view.patches])
def test_suite():
=== modified file 'lib/lp/bugs/doc/bugattachments.txt'
--- lib/lp/bugs/doc/bugattachments.txt 2010-04-08 14:34:58 +0000
+++ lib/lp/bugs/doc/bugattachments.txt 2010-08-02 17:19:19 +0000
@@ -211,7 +211,8 @@
If the request contains no attachment description the filename should be used.
- >>> filecontent = StringIO("No, sir. That's one bonehead name, but that ain't me any more.")
+ >>> filecontent = StringIO(
+ ... "No, sir. That's one bonehead name, but that ain't me any more.")
>>> filecontent.filename = 'RA.txt'
>>> add_request = LaunchpadTestRequest(
... method="POST",
@@ -520,72 +521,25 @@
True
-Linking existing LibraryFileAliases as attachments
---------------------------------------------------
-
-It's possible to link an existing LibraryFileAliases to a bug as an
-attachment by calling the bug's linkAttachment() method.
-
- >>> from canonical.launchpad.interfaces.librarian import (
- ... ILibraryFileAliasSet)
-
- >>> file_content = "Hello, world"
- >>> content_type = "text/plain"
- >>> file_alias = getUtility(ILibraryFileAliasSet).create(
- ... name='foobar', size=len(file_content),
- ... file=StringIO(file_content), contentType=content_type)
- >>> transaction.commit()
-
- >>> bug = factory.makeBug()
- >>> bug.linkAttachment(
- ... owner=bug.owner, file_alias=file_alias,
- ... comment="Some attachment")
- <BugAttachment ...>
-
- >>> bug.attachments.count()
- 1
- >>> attachment = bug.attachments[0]
- >>> print attachment.title
- foobar
-
-The attachment will have a type of BugAttachmentType.UNSPECIFIED, since
-we didn't specify that it was a patch.
-
- >>> print attachment.type.title
- Unspecified
-
-We can specify that the attachment is a patch and give it a more
-meaningful description.
-
- >>> file_alias = getUtility(ILibraryFileAliasSet).create(
- ... name='anotherfoobar', size=len(file_content),
- ... file=StringIO(file_content), contentType=content_type)
- >>> transaction.commit()
-
- >>> bug.linkAttachment(
- ... owner=bug.owner, file_alias=file_alias,
- ... comment="Some attachment", is_patch=True,
- ... description="An attachment of some sort")
- <BugAttachment ...>
-
- >>> bug.attachments.count()
- 2
- >>> attachment = bug.attachments[1]
- >>> print attachment.title
- An attachment of some sort
-
- >>> print attachment.type.title
- Patch
-
-
Attachments without library files
---------------------------------
It can happen that the LibraryFileContent record of a bug attachment is
deleted, for example. because an admin deleted a privacy sensitive file.
These attachments are not included in Bug.attachments. Our test bug has
-at present two attachments.
+two attachments.
+ >>> bug = factory.makeBug()
+ >>> file_content = "Hello, world"
+ >>> bug.addAttachment(
+ ... owner=bug.owner, data="Hello, world", filename="foobar",
+ ... comment="Some attachment")
+ <BugAttachment ...>
+ >>> bug.addAttachment(
+ ... owner=bug.owner, data="whatever", filename='anotherfoobar',
+ ... comment="Some attachment",
+ ... description="An attachment of some sort")
+ <BugAttachment ...>
>>> [attachment.title for attachment in bug.attachments]
[u'foobar', u'An attachment of some sort']
@@ -596,3 +550,58 @@
>>> removeSecurityProxy(attachment.libraryfile).content = None
>>> [attachment.title for attachment in bug.attachments]
[u'foobar']
+
+
+Adding bug attachments to private bugs
+--------------------------------------
+
+If an attachment is added to a private bug, the "restricted" flag of
+its Librarian file is set.
+
+ >>> private_bug_owner = factory.makePerson()
+ >>> login_person(private_bug_owner)
+ >>> private_bug = factory.makeBug(private=True, owner=private_bug_owner)
+ >>> private_attachment = private_bug.addAttachment(
+ ... owner=private_bug_owner, data="secret", filename="baz.txt",
+ ... comment="Some attachment")
+ >>> private_attachment.libraryfile.restricted
+ True
+
+But the "restricted" flag of Librarian files elonging to bug attachments
+of public bugs is not set.
+
+ >>> attachment.libraryfile.restricted
+ False
+
+If a private bug becomes public, the restricted flag of the related
+Librarian files are no longer set.
+
+ >>> changed = private_bug.setPrivate(False, private_bug.owner)
+ >>> private_attachment.libraryfile.restricted
+ False
+
+Similary, if a public bug becomes private, the "restricted" flag of
+its Librarian files are set.
+
+ >>> changed = bug.setPrivate(True, bug.owner)
+ >>> attachment.libraryfile.restricted
+ True
+
+
+Miscellaneous
+-------------
+
+The method IBugAttachment.getFileByName() returns the Librarian file.
+
+ >>> attachment.libraryfile.filename
+ u'foobar'
+ >>> attachment.getFileByName('foobar')
+ <LibraryFileAlias at...
+
+A NotFoundError is raised if the file name passed to getFileByName()
+does not match the file name of the Librarian file.
+
+ >>> attachment.getFileByName('nonsense')
+ Traceback (most recent call last):
+ ...
+ NotFoundError: 'nonsense'
=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py 2010-07-22 12:17:41 +0000
+++ lib/lp/bugs/interfaces/bug.py 2010-08-02 17:19:19 +0000
@@ -135,6 +135,7 @@
except NotFoundError:
return None
+
class IBugBecameQuestionEvent(Interface):
"""A bug became a question."""
@@ -522,18 +523,6 @@
:is_patch: A boolean.
"""
- def linkAttachment(owner, file_alias, comment, is_patch=False,
- description=None):
- """Link an `ILibraryFileAlias` to this bug.
-
- :owner: An IPerson.
- :file_alias: The `ILibraryFileAlias` to link to this bug.
- :description: A brief description of the attachment.
- :comment: An IMessage or string.
- :filename: A string.
- :is_patch: A boolean.
- """
-
def linkCVE(cve, user):
"""Ensure that this CVE is linked to this bug."""
@@ -716,7 +705,7 @@
This may also cause the security contact to be subscribed
if one is registered and if the bug is not private.
-
+
Return True if a change is made, False otherwise.
"""
@@ -825,6 +814,7 @@
Returns True or False.
"""
+
class InvalidDuplicateValue(Exception):
"""A bug cannot be set as the duplicate of another."""
webservice_error(417)
=== modified file 'lib/lp/bugs/interfaces/bugattachment.py'
--- lib/lp/bugs/interfaces/bugattachment.py 2010-02-02 14:54:24 +0000
+++ lib/lp/bugs/interfaces/bugattachment.py 2010-08-02 17:19:19 +0000
@@ -99,6 +99,14 @@
The library file content for this attachment is set to None.
"""
+ def getFileByName(filename):
+ """Return the `ILibraryFileAlias for the given file name.
+
+ NotFoundError is raised if the given filename does not match
+ libraryfile.filename.
+ """
+
+
# Need to do this here because of circular imports.
IMessage['bugattachments'].value_type.schema = IBugAttachment
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2010-07-22 20:30:26 +0000
+++ lib/lp/bugs/model/bug.py 2010-08-02 17:19:19 +0000
@@ -694,7 +694,8 @@
if recipients is not None:
for subscriber in dupe_subscribers:
- recipients.addDupeSubscriber(subscriber, dupe_details[subscriber])
+ recipients.addDupeSubscriber(
+ subscriber, dupe_details[subscriber])
return sorted(
dupe_subscribers, key=operator.attrgetter("displayname"))
@@ -959,13 +960,26 @@
filealias = getUtility(ILibraryFileAliasSet).create(
name=filename, size=len(filecontent),
- file=StringIO(filecontent), contentType=content_type)
+ file=StringIO(filecontent), contentType=content_type,
+ restricted=self.private)
return self.linkAttachment(
owner, filealias, comment, is_patch, description)
def linkAttachment(self, owner, file_alias, comment, is_patch=False,
description=None):
+ """Link an `ILibraryFileAlias` to this bug.
+
+ :owner: An IPerson.
+ :file_alias: The `ILibraryFileAlias` to link to this bug.
+ :description: A brief description of the attachment.
+ :comment: An IMessage or string.
+ :is_patch: A boolean.
+
+ This method should only be called by addAttachment(), otherwise
+ we may get inconsistent settings of bug.private and
+ file_alias.restricted.
+ """
if is_patch:
attach_type = BugAttachmentType.PATCH
else:
@@ -1388,6 +1402,9 @@
self.who_made_private = None
self.date_made_private = None
+ for attachment in self.attachments:
+ attachment.libraryfile.restricted = private
+
# 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()
@@ -1941,4 +1958,3 @@
def asDict(self):
"""Return the FileBugData instance as a dict."""
return self.__dict__.copy()
-
=== modified file 'lib/lp/bugs/model/bugattachment.py'
--- lib/lp/bugs/model/bugattachment.py 2010-01-20 17:09:40 +0000
+++ lib/lp/bugs/model/bugattachment.py 2010-08-02 17:19:19 +0000
@@ -59,6 +59,12 @@
self.libraryfile.content = None
super(BugAttachment, self).destroySelf()
+ def getFileByName(self, filename):
+ """See IBugAttachment."""
+ if filename == self.libraryfile.filename:
+ return self.libraryfile
+ raise NotFoundError(filename)
+
class BugAttachmentSet:
"""A set for bug attachments."""
=== modified file 'lib/lp/bugs/stories/bugattachments/10-add-bug-attachment.txt'
--- lib/lp/bugs/stories/bugattachments/10-add-bug-attachment.txt 2010-04-06 06:05:25 +0000
+++ lib/lp/bugs/stories/bugattachments/10-add-bug-attachment.txt 2010-08-02 17:19:19 +0000
@@ -41,7 +41,7 @@
>>> link = user_browser.getLink('Some information')
>>> link.url
- 'http://localhost:58000/.../foo.txt'
+ 'http://bugs.launchpad.dev/firefox/+bug/1/+attachment/1/+files/foo.txt'
>>> 'Added some information' in user_browser.contents
True
=== modified file 'lib/lp/bugs/stories/bugattachments/20-edit-bug-attachment.txt'
--- lib/lp/bugs/stories/bugattachments/20-edit-bug-attachment.txt 2010-03-30 20:56:51 +0000
+++ lib/lp/bugs/stories/bugattachments/20-edit-bug-attachment.txt 2010-08-02 17:19:19 +0000
@@ -1,7 +1,8 @@
We can also edit the attachment details, let's navigate to that page.
+ >>> import re
>>> user_browser.open('http://bugs.launchpad.dev/firefox/+bug/1')
- >>> user_browser.getLink(url='+attachment/1').click()
+ >>> user_browser.getLink(url=re.compile(r'[+]attachment/1$')).click()
>>> user_browser.url
'http://bugs.launchpad.dev/firefox/+bug/1/+attachment/1'
@@ -31,7 +32,7 @@
We can edit the attachment to be a patch.
- >>> user_browser.getLink(url='+attachment/1').click()
+ >>> user_browser.getLink(url=re.compile(r'[+]attachment/1$')).click()
>>> patch_control = user_browser.getControl(
... 'This attachment contains a solution (patch) for this bug')
>>> patch_control.selected = True
=== modified file 'lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.txt'
--- lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.txt 2010-03-15 14:34:49 +0000
+++ lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.txt 2010-08-02 17:19:19 +0000
@@ -50,7 +50,7 @@
text/plain)
>>> link = browser.getLink('sample data')
>>> print link.url
- http://localhost:58000/.../test.txt
+ http://bugs.launchpad.dev/jokosher/+bug/11/+attachment/1/+files/test.txt
== Patch attachments are shown before non-patch attachments ==
=== modified file 'lib/lp/bugs/stories/bugattachments/xx-delete-bug-attachment.txt'
--- lib/lp/bugs/stories/bugattachments/xx-delete-bug-attachment.txt 2009-11-27 13:09:06 +0000
+++ lib/lp/bugs/stories/bugattachments/xx-delete-bug-attachment.txt 2010-08-02 17:19:19 +0000
@@ -36,7 +36,7 @@
attachment.
>>> import re
- >>> user_browser.getLink(url=re.compile(r'[+]attachment/\d')).click()
+ >>> user_browser.getLink(url=re.compile(r'[+]attachment/\d$')).click()
>>> print user_browser.title
Bug #2...
>>> user_browser.getControl('Delete Attachment') is not None
=== modified file 'lib/lp/bugs/stories/guided-filebug/xx-filebug-attachments.txt'
--- lib/lp/bugs/stories/guided-filebug/xx-filebug-attachments.txt 2009-09-23 09:03:22 +0000
+++ lib/lp/bugs/stories/guided-filebug/xx-filebug-attachments.txt 2010-08-02 17:19:19 +0000
@@ -48,7 +48,7 @@
A description of the attachment
>>> user_browser.getLink('A description of the attachment').url
- 'http://localhost:58000/.../example.txt'
+ 'http://bugs.launchpad.dev/firefox/+bug/.../+attachment/1/+files/ex...'
== Empty Attachment Fields ==
=== modified file 'lib/lp/bugs/templates/bug-portlet-attachments.pt'
--- lib/lp/bugs/templates/bug-portlet-attachments.pt 2010-03-15 03:29:51 +0000
+++ lib/lp/bugs/templates/bug-portlet-attachments.pt 2010-08-02 17:19:19 +0000
@@ -8,13 +8,13 @@
<ul>
<li class="download-attachment"
tal:repeat="attachment view/patches">
- <a tal:attributes="href attachment/libraryfile/http_url"
- tal:content="attachment/title"
+ <a tal:attributes="href attachment/file/http_url"
+ tal:content="attachment/attachment/title"
class="sprite haspatch-icon">
Attachment Title
</a>
<small>
- (<a tal:attributes="href attachment/fmt:url">edit</a>)
+ (<a tal:attributes="href attachment/attachment/fmt:url">edit</a>)
</small>
</li>
</ul>
@@ -31,13 +31,13 @@
<ul>
<li class="download-attachment"
tal:repeat="attachment view/regular_attachments">
- <a tal:attributes="href attachment/libraryfile/http_url"
- tal:content="attachment/title"
+ <a tal:attributes="href attachment/file/http_url"
+ tal:content="attachment/attachment/title"
class="sprite download-icon">
Attachment Title
</a>
<small>
- (<a tal:attributes="href attachment/fmt:url">edit</a>)
+ (<a tal:attributes="href attachment/attachment/fmt:url">edit</a>)
</small>
</li>
</ul>
=== modified file 'lib/lp/bugs/templates/bugcomment-box.pt'
--- lib/lp/bugs/templates/bugcomment-box.pt 2010-06-22 12:53:50 +0000
+++ lib/lp/bugs/templates/bugcomment-box.pt 2010-08-02 17:19:19 +0000
@@ -58,7 +58,7 @@
<ul tal:condition="comment/bugattachments" style="margin-bottom: 1em">
<li tal:repeat="attachment comment/bugattachments"
class="download-attachment">
- <a tal:attributes="href attachment/libraryfile/http_url"
+ <a tal:attributes="href python: view.proxiedUrlOfLibraryFileAlias(attachment)"
tal:content="attachment/title"
class="sprite download-icon">foo.txt</a>
(<span
@@ -69,7 +69,7 @@
<ul tal:condition="comment/patches" style="margin-bottom: 1em">
<li tal:repeat="attachment comment/patches" class="download-attachment">
- <a tal:attributes="href attachment/libraryfile/http_url"
+ <a tal:attributes="href python: view.proxiedUrlOfLibraryFileAlias(attachment)"
tal:content="attachment/title" class="sprite haspatch-icon">foo.txt</a>
(<span
tal:replace="attachment/libraryfile/content/filesize/fmt:bytes" />,
