launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24364
[Merge] ~cjwatson/launchpad:bug-addattachment-bytes into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:bug-addattachment-bytes into launchpad:master.
Commit message:
Make Bug.addAttachment take data as bytes
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/379648
More explicit, and needed for Python 3.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:bug-addattachment-bytes into launchpad:master.
diff --git a/lib/lp/bugs/browser/bugmessage.py b/lib/lp/bugs/browser/bugmessage.py
index e8128cf..7bf7612 100644
--- a/lib/lp/bugs/browser/bugmessage.py
+++ b/lib/lp/bugs/browser/bugmessage.py
@@ -8,7 +8,7 @@ __all__ = [
'BugMessageAddFormView',
]
-from StringIO import StringIO
+from io import BytesIO
from zope.component import getUtility
from zope.formlib.widget import CustomWidgetFactory
@@ -125,7 +125,7 @@ class BugMessageAddFormView(LaunchpadFormView, BugAttachmentContentCheck):
else:
is_patch = data['patch']
attachment = bug.addAttachment(
- owner=self.user, data=StringIO(data['filecontent']),
+ owner=self.user, data=BytesIO(data['filecontent']),
filename=filename, description=file_description,
comment=message, is_patch=is_patch)
diff --git a/lib/lp/bugs/browser/bugtarget.py b/lib/lp/bugs/browser/bugtarget.py
index 13e45a1..5755c1b 100644
--- a/lib/lp/bugs/browser/bugtarget.py
+++ b/lib/lp/bugs/browser/bugtarget.py
@@ -19,9 +19,9 @@ __all__ = [
"product_to_productbugconfiguration",
]
-from cStringIO import StringIO
from datetime import datetime
from functools import partial
+from io import BytesIO
from operator import itemgetter
from lazr.restful.interface import copy_field
@@ -617,7 +617,7 @@ class FileBugViewBase(LaunchpadFormView):
file_description = filename
bug.addAttachment(
- owner=self.user, data=StringIO(data['filecontent']),
+ owner=self.user, data=BytesIO(data['filecontent']),
filename=filename, description=file_description,
comment=attachment_comment, is_patch=data['patch'])
diff --git a/lib/lp/bugs/browser/tests/test_bugattachment_edit_view.py b/lib/lp/bugs/browser/tests/test_bugattachment_edit_view.py
index 8c90b34..2710d05 100644
--- a/lib/lp/bugs/browser/tests/test_bugattachment_edit_view.py
+++ b/lib/lp/bugs/browser/tests/test_bugattachment_edit_view.py
@@ -33,7 +33,7 @@ class TestBugAttachmentEditView(TestCaseWithFactory):
login_person(self.bug_owner)
self.bug = self.factory.makeBug(owner=self.bug_owner)
self.bugattachment = self.factory.makeBugAttachment(
- bug=self.bug, filename='foo.diff', data='file content',
+ bug=self.bug, filename='foo.diff', data=b'file content',
description='attachment description', content_type='text/plain',
is_patch=False)
# The Librarian server should know about the new file before
diff --git a/lib/lp/bugs/browser/tests/test_bugattachment_file_access.py b/lib/lp/bugs/browser/tests/test_bugattachment_file_access.py
index 30e6cca..2f68b38 100644
--- a/lib/lp/bugs/browser/tests/test_bugattachment_file_access.py
+++ b/lib/lp/bugs/browser/tests/test_bugattachment_file_access.py
@@ -53,7 +53,7 @@ class TestAccessToBugAttachmentFiles(TestCaseWithFactory):
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')
+ bug=self.bug, filename='foo.txt', data=b'file content')
def test_traversal_to_lfa_of_bug_attachment(self):
# Traversing to the URL provided by a ProxiedLibraryFileAlias of a
@@ -132,7 +132,7 @@ class TestWebserviceAccessToBugAttachmentFiles(TestCaseWithFactory):
login_person(self.bug_owner)
self.bug = self.factory.makeBug(owner=self.bug_owner)
self.factory.makeBugAttachment(
- bug=self.bug, filename='foo.txt', data='file content')
+ bug=self.bug, filename='foo.txt', data=b'file content')
self.bug_url = api_url(self.bug)
def test_anon_access_to_public_bug_attachment(self):
diff --git a/lib/lp/bugs/doc/bug-export.txt b/lib/lp/bugs/doc/bug-export.txt
index ecb3076..3df8a3b 100644
--- a/lib/lp/bugs/doc/bug-export.txt
+++ b/lib/lp/bugs/doc/bug-export.txt
@@ -131,11 +131,13 @@ Attachments are included in the XML dump. First add an attachment to
bug #1. We need to commit here so that the librarian can later serve
the file when we later serialise the bug:
+ >>> from io import BytesIO
+
>>> login('test@xxxxxxxxxxxxx')
>>> bug4 = getUtility(IBugSet).get(4)
>>> sampleperson = getUtility(IPersonSet).getByEmail('test@xxxxxxxxxxxxx')
>>> bug4.addAttachment(
- ... sampleperson, StringIO('Hello World'), 'Added attachment',
+ ... sampleperson, BytesIO(b'Hello World'), 'Added attachment',
... 'hello.txt', description='"Hello World" attachment')
<BugAttachment ...>
diff --git a/lib/lp/bugs/doc/bugattachments.txt b/lib/lp/bugs/doc/bugattachments.txt
index bbd23bd..0bac3b7 100644
--- a/lib/lp/bugs/doc/bugattachments.txt
+++ b/lib/lp/bugs/doc/bugattachments.txt
@@ -66,7 +66,7 @@ ObjectCreatedEvent in order to trigger email notifications:
'Unspecified'
IBug.addAttachment's comment parameter can also be a string. The data
-passed in is often a file-like object, but can be a string too.
+passed in is often a file-like object, but can be bytes too.
>>> data = filecontent
>>> attachment_from_strings = bug_four.addAttachment(
diff --git a/lib/lp/bugs/doc/bugcomment.txt b/lib/lp/bugs/doc/bugcomment.txt
index 8aa5ae0..f98907b 100644
--- a/lib/lp/bugs/doc/bugcomment.txt
+++ b/lib/lp/bugs/doc/bugcomment.txt
@@ -64,10 +64,10 @@ first comment.
If we add an attachment to the first comment, this comment is included
in activity_and_comments...
- >>> import StringIO
+ >>> from io import BytesIO
>>> login("test@xxxxxxxxxxxxx")
>>> attachment = bug_11.addAttachment(
- ... owner=None, data=StringIO.StringIO('whatever'),
+ ... owner=None, data=BytesIO(b'whatever'),
... comment=bug_11.initial_message, filename='test.txt',
... is_patch=False, content_type='text/plain',
... description='sample data')
@@ -194,7 +194,6 @@ attachments to a bug to see this in action:
>>> from lp.services.webapp.interfaces import ILaunchBag
>>> from lp.registry.interfaces.person import IPersonSet
- >>> import StringIO
>>> user = getUtility(ILaunchBag).user
>>> different_user = getUtility(IPersonSet).getByName('name16')
@@ -208,7 +207,7 @@ attachments to a bug to see this in action:
... owner=user, subject="Ho", content="Hello there")
>>> m4 = bug_three.newMessage(
... owner=user, subject="Ho", content="Hello there")
- >>> file_ = StringIO.StringIO("Bogus content makes the world go round")
+ >>> file_ = BytesIO(b"Bogus content makes the world go round")
>>> a1 = bug_three.addAttachment(
... owner=user, data=file_, description="Ho",
... filename="munchy", comment="Hello there")
@@ -386,22 +385,22 @@ not included in BugComment.patches.
>>> bug_task = factory.makeBugTask(owner=user)
>>> bug = bug_task.bug
>>> attachment_1 = bug.addAttachment(
- ... owner=None, data=StringIO.StringIO('whatever'),
+ ... owner=None, data=BytesIO(b'whatever'),
... comment=bug.initial_message, filename='file1',
... is_patch=False, content_type='text/plain',
... description='sample data 1')
>>> attachment_2 = bug.addAttachment(
- ... owner=None, data=StringIO.StringIO('whatever'),
+ ... owner=None, data=BytesIO(b'whatever'),
... comment=bug.initial_message, filename='file2',
... is_patch=False, content_type='text/plain',
... description='sample data 2')
>>> patch_1 = bug.addAttachment(
- ... owner=None, data=StringIO.StringIO('whatever'),
+ ... owner=None, data=BytesIO(b'whatever'),
... comment=bug.initial_message, filename='patch1',
... is_patch=True, content_type='text/plain',
... description='patch 1')
>>> patch_2 = bug.addAttachment(
- ... owner=None, data=StringIO.StringIO('whatever'),
+ ... owner=None, data=BytesIO(b'whatever'),
... comment=bug.initial_message, filename='patch2',
... is_patch=True, content_type='text/plain',
... description='patch 2')
diff --git a/lib/lp/bugs/interfaces/bug.py b/lib/lp/bugs/interfaces/bug.py
index 2d9aa49..79397ef 100644
--- a/lib/lp/bugs/interfaces/bug.py
+++ b/lib/lp/bugs/interfaces/bug.py
@@ -698,7 +698,7 @@ class IBugAppend(Interface):
"""Attach a file to this bug.
:owner: An IPerson.
- :data: A file-like object, or a `str`.
+ :data: A file-like object, or a `bytes`.
:description: A brief description of the attachment.
:comment: An IMessage or string.
:filename: A string.
diff --git a/lib/lp/bugs/model/bug.py b/lib/lp/bugs/model/bug.py
index 4339892..be4bb07 100644
--- a/lib/lp/bugs/model/bug.py
+++ b/lib/lp/bugs/model/bug.py
@@ -19,9 +19,9 @@ __all__ = [
]
-from cStringIO import StringIO
from email.utils import make_msgid
from functools import wraps
+from io import BytesIO
from itertools import chain
import operator
import re
@@ -1299,7 +1299,7 @@ class Bug(SQLBase, InformationTypeMixin):
# wrongly encoded.
if from_api:
data = get_raw_form_value_from_current_request(data, 'data')
- if isinstance(data, str):
+ if isinstance(data, bytes):
filecontent = data
else:
filecontent = data.read()
@@ -1313,7 +1313,7 @@ class Bug(SQLBase, InformationTypeMixin):
filealias = getUtility(ILibraryFileAliasSet).create(
name=filename, size=len(filecontent),
- file=StringIO(filecontent), contentType=content_type,
+ file=BytesIO(filecontent), contentType=content_type,
restricted=self.private)
return self.linkAttachment(
diff --git a/lib/lp/bugs/model/tests/test_bugtasksearch.py b/lib/lp/bugs/model/tests/test_bugtasksearch.py
index fddce7f..60524b8 100644
--- a/lib/lp/bugs/model/tests/test_bugtasksearch.py
+++ b/lib/lp/bugs/model/tests/test_bugtasksearch.py
@@ -280,10 +280,10 @@ class OnceTests:
# a given type.
with person_logged_in(self.owner):
self.bugtasks[0].bug.addAttachment(
- owner=self.owner, data='filedata', comment='a comment',
+ owner=self.owner, data=b'filedata', comment='a comment',
filename='file1.txt', is_patch=False)
self.bugtasks[1].bug.addAttachment(
- owner=self.owner, data='filedata', comment='a comment',
+ owner=self.owner, data=b'filedata', comment='a comment',
filename='file1.txt', is_patch=True)
# We can search for bugs with non-patch attachments...
params = self.getBugTaskSearchParams(
diff --git a/lib/lp/bugs/scripts/tests/test_bugnotification.py b/lib/lp/bugs/scripts/tests/test_bugnotification.py
index 41b7ba4..1b3f4cb 100644
--- a/lib/lp/bugs/scripts/tests/test_bugnotification.py
+++ b/lib/lp/bugs/scripts/tests/test_bugnotification.py
@@ -958,7 +958,7 @@ class TestEmailNotificationsAttachments(
# another five minutes. Therefore, we send out separate
# change notifications.
return self.bug.addAttachment(
- self.person, 'content', 'a comment', 'stuff.txt')
+ self.person, b'content', 'a comment', 'stuff.txt')
old = cachedproperty('old')(_attachment)
new = cachedproperty('new')(_attachment)
diff --git a/lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.txt b/lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.txt
index eaf7ac8..2bf6a34 100644
--- a/lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.txt
+++ b/lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.txt
@@ -20,7 +20,7 @@ of bug #11 is:
If we add an attachment to the bug report...
- >>> import StringIO
+ >>> from io import BytesIO
>>> from zope.component import getUtility
>>> from lp.testing import login, logout
>>> from lp.services.webapp.interfaces import ILaunchBag
@@ -30,7 +30,7 @@ If we add an attachment to the bug report...
>>> launchbag = getUtility(ILaunchBag)
>>> launchbag.clear()
>>> attachment = bug_11.addAttachment(
- ... owner=None, data=StringIO.StringIO('whatever'),
+ ... owner=None, data=BytesIO(b'whatever'),
... comment=bug_11.initial_message, filename='test.txt',
... is_patch=False, content_type='text/plain',
... description='sample data')
@@ -64,7 +64,7 @@ Patch attachments are shown before non-patch attachments
>>> login("test@xxxxxxxxxxxxx")
>>> launchbag.clear()
>>> patch_attachment = bug_11.addAttachment(
- ... owner=None, data=StringIO.StringIO('patchy patch patch'),
+ ... owner=None, data=BytesIO(b'patchy patch patch'),
... comment=bug_11.initial_message, filename='patch.txt',
... is_patch=True, content_type='text/plain',
... description='a patch')
diff --git a/lib/lp/bugs/stories/bugs/xx-bug-text-pages.txt b/lib/lp/bugs/stories/bugs/xx-bug-text-pages.txt
index 3f418d1..5a53e48 100644
--- a/lib/lp/bugs/stories/bugs/xx-bug-text-pages.txt
+++ b/lib/lp/bugs/stories/bugs/xx-bug-text-pages.txt
@@ -7,7 +7,7 @@ To demonstrate this feature, we'll use bug 1.
We'll start by adding some attachments to the bug:
- >>> import StringIO
+ >>> from io import BytesIO
>>> from lp.services.database.sqlbase import flush_database_updates
>>> from lp.testing import login, logout
>>> from lp.bugs.model.bug import Bug
@@ -16,14 +16,14 @@ We'll start by adding some attachments to the bug:
>>> mark = Person.selectOneBy(name="mark")
>>> mark.display_name = u"M\xe1rk Sh\xfattlew\xf2rth"
>>> bug = Bug.get(1)
- >>> content = StringIO.StringIO("<html><body>bogus</body></html>")
+ >>> content = BytesIO(b"<html><body>bogus</body></html>")
>>> a1 = bug.addAttachment(mark, content, "comment for file a",
... "file_a.txt", content_type="text/html")
- >>> content = StringIO.StringIO("do we need to")
+ >>> content = BytesIO(b"do we need to")
>>> a2 = bug.addAttachment(mark, content, "comment for file with space",
... "file with space.txt",
... content_type='text/plain;\n name="file with space.txt"')
- >>> content = StringIO.StringIO("Yes we can!")
+ >>> content = BytesIO(b"Yes we can!")
>>> a3 = bug.addAttachment(mark, content, "comment for patch",
... "bug-patch.diff", is_patch=True, content_type='text/plain',
... description="a patch")
diff --git a/lib/lp/bugs/stories/bugtask-searches/xx-listing-basics.txt b/lib/lp/bugs/stories/bugtask-searches/xx-listing-basics.txt
index 38dacdb..7300d25 100644
--- a/lib/lp/bugs/stories/bugtask-searches/xx-listing-basics.txt
+++ b/lib/lp/bugs/stories/bugtask-searches/xx-listing-basics.txt
@@ -232,9 +232,9 @@ relevant listings:
Patches also appear as badges in bug listings.
+ >>> from io import BytesIO
>>> from lp.testing import login, logout
>>> from zope.component import getUtility
- >>> from StringIO import StringIO
>>> from lp.services.messages.interfaces.message import IMessageSet
>>> from lp.bugs.interfaces.bug import IBugSet
>>> from lp.registry.interfaces.person import IPersonSet
@@ -249,7 +249,7 @@ Patches also appear as badges in bug listings.
>>> bug_one = bugset.get(1)
>>> bug_one.addAttachment(
... owner=foobar,
- ... data=StringIO("file data"),
+ ... data=BytesIO(b"file data"),
... filename="foo.bar",
... description="this fixes the bug",
... comment=message,
diff --git a/lib/lp/bugs/stories/webservice/xx-bug.txt b/lib/lp/bugs/stories/webservice/xx-bug.txt
index 9154525..119fb43 100644
--- a/lib/lp/bugs/stories/webservice/xx-bug.txt
+++ b/lib/lp/bugs/stories/webservice/xx-bug.txt
@@ -1244,7 +1244,7 @@ An attachment can be added to the bug:
>>> response = webservice.named_post(
... bug_one['self_link'], 'addAttachment',
- ... data="12345", filename="numbers.txt", content_type='foo/bar',
+ ... data=b"12345", filename="numbers.txt", content_type='foo/bar',
... comment="The numbers you asked for.")
>>> print(response)
HTTP/1.1 201 Created...
diff --git a/lib/lp/bugs/tests/test_bugs_webservice.py b/lib/lp/bugs/tests/test_bugs_webservice.py
index b795785..2de90c3 100644
--- a/lib/lp/bugs/tests/test_bugs_webservice.py
+++ b/lib/lp/bugs/tests/test_bugs_webservice.py
@@ -392,7 +392,7 @@ class TestErrorHandling(TestCaseWithFactory):
launchpad = launchpadlib_for('test', owner)
lp_bug = launchpad.load(api_url(bug))
self.assertRaises(
- BadRequest, lp_bug.addAttachment, comment='foo', data='foo',
+ BadRequest, lp_bug.addAttachment, comment='foo', data=b'foo',
filename='/home/foo/bar.txt')
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index bac9b17..ed3e163 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -485,6 +485,10 @@ class ObjectFactory:
string = "%s-%s" % (prefix, self.getUniqueInteger())
return string
+ # XXX cjwatson 2020-02-20: We should disentangle this; most uses of
+ # getUniqueString should probably use getUniqueUnicode instead.
+ getUniqueBytes = getUniqueString
+
def getUniqueUnicode(self, prefix=None):
return self.getUniqueString(prefix=prefix).decode('latin-1')
@@ -2157,7 +2161,7 @@ class BareLaunchpadObjectFactory(ObjectFactory):
if owner is None:
owner = self.makePerson()
if data is None:
- data = self.getUniqueString()
+ data = self.getUniqueBytes()
if description is None:
description = self.getUniqueString()
if comment is None:
