Merge lp:~adeuring/launchpad/bug-39674-flip-lfa-restricted-flag into lp:launchpad/db-devel

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()
-