From e5d718f982e0cd3fc85da00d3abdccce1907e488 Mon Sep 17 00:00:00 2001 From: Folker Bernitt Date: Wed, 2 Sep 2015 12:35:33 +0200 Subject: Download attachments from mail store instead of querier - Issue #435 - Improved error handling of attachment resource --- .../pixelated/adapter/mailstore/leap_mailstore.py | 4 +-- service/pixelated/adapter/services/mail_service.py | 4 +-- .../pixelated/resources/attachments_resource.py | 11 ++++++-- .../test/integration/test_retrieve_attachment.py | 33 +++++++++++++++------- .../test/support/integration/app_test_client.py | 6 ++-- .../unit/adapter/mailstore/test_leap_mailstore.py | 6 ++-- service/test/unit/adapter/test_mail_service.py | 9 ++++++ 7 files changed, 51 insertions(+), 22 deletions(-) (limited to 'service') diff --git a/service/pixelated/adapter/mailstore/leap_mailstore.py b/service/pixelated/adapter/mailstore/leap_mailstore.py index a3a5e984..993f413c 100644 --- a/service/pixelated/adapter/mailstore/leap_mailstore.py +++ b/service/pixelated/adapter/mailstore/leap_mailstore.py @@ -18,7 +18,7 @@ from email.header import decode_header import quopri import re from uuid import uuid4 -from leap.mail.adaptors.soledad import SoledadMailAdaptor +from leap.mail.adaptors.soledad import SoledadMailAdaptor, ContentDocWrapper from twisted.internet import defer from pixelated.adapter.mailstore.body_parser import BodyParser from pixelated.adapter.mailstore.mailstore import MailStore, underscore_uuid @@ -182,7 +182,7 @@ class LeapMailStore(MailStore): def get_mail_attachment(self, attachment_id): results = yield self.soledad.get_from_index('by-type-and-payloadhash', 'cnt', attachment_id) if attachment_id else [] if len(results): - content = results[0] + content = ContentDocWrapper(**results[0].content) defer.returnValue({'content-type': content.content_type, 'content': self._try_decode( content.raw, content.content_transfer_encoding)}) else: diff --git a/service/pixelated/adapter/services/mail_service.py b/service/pixelated/adapter/services/mail_service.py index 4ff42046..2c576e98 100644 --- a/service/pixelated/adapter/services/mail_service.py +++ b/service/pixelated/adapter/services/mail_service.py @@ -65,8 +65,8 @@ class MailService(object): def mail(self, mail_id): return self.mail_store.get_mail(mail_id, include_body=True) - def attachment(self, attachment_id, encoding): - return self.querier.attachment(attachment_id, encoding) + def attachment(self, attachment_id): + return self.mail_store.get_mail_attachment(attachment_id) @defer.inlineCallbacks def mail_exists(self, mail_id): diff --git a/service/pixelated/resources/attachments_resource.py b/service/pixelated/resources/attachments_resource.py index 0a903cc4..a78022ec 100644 --- a/service/pixelated/resources/attachments_resource.py +++ b/service/pixelated/resources/attachments_resource.py @@ -18,7 +18,7 @@ import io import re from twisted.protocols.basic import FileSender -from twisted.python.log import err +from twisted.python.log import msg from twisted.web import server from twisted.web.resource import Resource from twisted.internet import defer @@ -34,23 +34,28 @@ class AttachmentResource(Resource): self.mail_service = mail_service def render_GET(self, request): + def error_handler(failure): + msg(failure, 'attachment not found') + request.code = 404 + request.finish() encoding = request.args.get('encoding', [None])[0] filename = request.args.get('filename', [self.attachment_id])[0] request.setHeader(b'Content-Type', b'application/force-download') request.setHeader(b'Content-Disposition', bytes('attachment; filename=' + filename)) d = self._send_attachment(encoding, filename, request) - d.addErrback(err) + d.addErrback(error_handler) return server.NOT_DONE_YET @defer.inlineCallbacks def _send_attachment(self, encoding, filename, request): - attachment = yield self.mail_service.attachment(self.attachment_id, encoding) + attachment = yield self.mail_service.attachment(self.attachment_id) bytes_io = io.BytesIO(attachment['content']) try: + request.code = 200 yield FileSender().beginFileTransfer(bytes_io, request) finally: bytes_io.close() diff --git a/service/test/integration/test_retrieve_attachment.py b/service/test/integration/test_retrieve_attachment.py index 1cf8006f..e7e8670d 100644 --- a/service/test/integration/test_retrieve_attachment.py +++ b/service/test/integration/test_retrieve_attachment.py @@ -13,6 +13,9 @@ # # You should have received a copy of the GNU Affero General Public License # along with Pixelated. If not, see . +from email.mime.application import MIMEApplication +from email.mime.multipart import MIMEMultipart +from email.mime.text import MIMEText from test.support.integration.soledad_test_base import SoledadTestBase from twisted.internet import defer @@ -22,16 +25,26 @@ class RetrieveAttachmentTest(SoledadTestBase): @defer.inlineCallbacks def test_attachment_content_is_retrieved(self): - ident = 'F4E99C1CEC4D300A4223A96CCABBE0304BDBC31C550A5A03E207A5E4C3C71A22' - attachment_dict = {'content-disposition': 'attachment', - 'content-transfer-encoding': '', - 'type': 'cnt', - 'raw': 'cGVxdWVubyBhbmV4byA6RAo=', - 'phash': ident, - 'content_type': 'text/plain; charset=US-ASCII; name="attachment_pequeno.txt"'} + attachment_id, input_mail = self._create_mail_with_attachment() + yield self.mail_store.add_mail('INBOX', input_mail.as_string()) - yield self.add_document_to_soledad(attachment_dict) + attachment, req = yield self.get_attachment(attachment_id, 'base64') - attachment = yield self.get_attachment(ident, 'base64') + self.assertEqual(200, req.code) + self.assertEquals('pretend to be binary attachment data', attachment) - self.assertEquals('pequeno anexo :D\n', attachment) + def _create_mail_with_attachment(self): + input_mail = MIMEMultipart() + input_mail.attach(MIMEText(u'a utf8 message', _charset='utf-8')) + attachment = MIMEApplication('pretend to be binary attachment data') + attachment.add_header('Content-Disposition', 'attachment', filename='filename.txt') + input_mail.attach(attachment) + attachment_id = 'B5B4ED80AC3B894523D72E375DACAA2FC6606C18EDF680FE95903086C8B5E14A' + return attachment_id, input_mail + + @defer.inlineCallbacks + def test_attachment_error_returned_if_id_not_found(self): + attachment, req = yield self.get_attachment('invalid attachment id', 'base64') + + self.assertEqual(404, req.code) + self.assertIsNone(attachment) diff --git a/service/test/support/integration/app_test_client.py b/service/test/support/integration/app_test_client.py index fe9eaffd..51fbf483 100644 --- a/service/test/support/integration/app_test_client.py +++ b/service/test/support/integration/app_test_client.py @@ -193,9 +193,11 @@ class AppTestClient(object): res = yield res defer.returnValue([ResponseMail(m) for m in res['mails']]) + @defer.inlineCallbacks def get_attachment(self, ident, encoding): - res, req = self.get("/attachment/%s" % ident, {'encoding': [encoding]}, as_json=False) - return res + deferred_result, req = self.get("/attachment/%s" % ident, {'encoding': [encoding]}, as_json=False) + res = yield deferred_result + defer.returnValue((res, req)) def put_mail(self, data): res, req = self.put('/mails', data) diff --git a/service/test/unit/adapter/mailstore/test_leap_mailstore.py b/service/test/unit/adapter/mailstore/test_leap_mailstore.py index 7cbd1405..96a862be 100644 --- a/service/test/unit/adapter/mailstore/test_leap_mailstore.py +++ b/service/test/unit/adapter/mailstore/test_leap_mailstore.py @@ -137,7 +137,7 @@ class TestLeapMailStore(TestCase): @defer.inlineCallbacks def test_get_mail_attachment(self): attachment_id = 'AAAA9AAD9E153D24265395203C53884506ABA276394B9FEC02B214BF9E77E48E' - doc = ContentDocWrapper(content_type='foo/bar', raw='asdf') + doc = SoledadDocument(json=json.dumps({'content_type': 'foo/bar', 'raw': 'asdf'})) when(self.soledad).get_from_index('by-type-and-payloadhash', 'cnt', attachment_id).thenReturn(defer.succeed([doc])) store = LeapMailStore(self.soledad) @@ -153,8 +153,8 @@ class TestLeapMailStore(TestCase): ('quoted-printable', 'äsdf', '=C3=A4sdf')] for transfer_encoding, data, encoded_data in encoding_examples: - doc = ContentDocWrapper(content_type='foo/bar', raw=encoded_data, - content_transfer_encoding=transfer_encoding) + doc = SoledadDocument(json=json.dumps({'content_type': 'foo/bar', 'raw': encoded_data, + 'content_transfer_encoding': transfer_encoding})) when(self.soledad).get_from_index('by-type-and-payloadhash', 'cnt', attachment_id).thenReturn(defer.succeed([doc])) store = LeapMailStore(self.soledad) diff --git a/service/test/unit/adapter/test_mail_service.py b/service/test/unit/adapter/test_mail_service.py index 79161a04..30784769 100644 --- a/service/test/unit/adapter/test_mail_service.py +++ b/service/test/unit/adapter/test_mail_service.py @@ -130,3 +130,12 @@ class TestMailService(unittest.TestCase): yield self.mail_service.recover_mail(1) verify(self.mail_store).move_mail_to_mailbox(1, 'INBOX') + + @defer.inlineCallbacks + def test_get_attachment(self): + attachment_dict = {'content': bytearray('data'), 'content-type': 'text/plain'} + when(self.mail_store).get_mail_attachment('some attachment id').thenReturn(defer.succeed(attachment_dict)) + + attachment = yield self.mail_service.attachment('some attachment id') + + self.assertEqual(attachment_dict, attachment) -- cgit v1.2.3