From b3ebf4a6cdbbe21adbaab191075347dda5b18e60 Mon Sep 17 00:00:00 2001 From: Vinod Kurup Date: Tue, 21 Apr 2015 12:29:12 -0400 Subject: [PATCH] Only send one message per request to modem The MultiModem can only accept one message per request, so iterate over identities. In order to prevent good identities from blocking bad ones, keep a list of failed identities and raise an error with those after all messages have been processed. Code stolen from @reyrodrigues --- rapidsms_multimodem/outgoing.py | 24 ++++--- rapidsms_multimodem/tests/test_outgoing.py | 81 +++++++++++++++++++++- 2 files changed, 94 insertions(+), 11 deletions(-) diff --git a/rapidsms_multimodem/outgoing.py b/rapidsms_multimodem/outgoing.py index 6a73693..8dfdc6a 100644 --- a/rapidsms_multimodem/outgoing.py +++ b/rapidsms_multimodem/outgoing.py @@ -55,11 +55,19 @@ def prepare_querystring(self, id_, text, identities, context): def send(self, id_, text, identities, context={}): logger.debug('Sending message: %s' % text) - query_string = self.prepare_querystring(id_, text, identities, context) - r = requests.get(url=self.sendsms_url, params=query_string) - if r.status_code != requests.codes.ok: - r.raise_for_status() - if "Err" in r.text: - logger.error("Send API failed with %s" % r.text) - logger.error("URL: %s" % r.url) - logger.debug("URL: %s" % r.url) + failures = {} + for identity in identities: + query_string = self.prepare_querystring(id_, text, [identity], context) + r = requests.get(url=self.sendsms_url, params=query_string) + if r.status_code != requests.codes.ok: + # HTTP Error + err_msg = '{0}: HTTP Error {1}: {2}'.format(identity, r.status_code, r.reason) + logger.error(err_msg) + failures[identity] = err_msg + elif "Err" in r.text: + # Multimodem Error + err_msg = '{0}: Multimodem Error: {1}'.format(identity, r.text) + logger.error(err_msg) + failures[identity] = err_msg + if failures: + raise Exception('Multimodem failed to send some messages.', failures) diff --git a/rapidsms_multimodem/tests/test_outgoing.py b/rapidsms_multimodem/tests/test_outgoing.py index bf288d1..cc6c851 100644 --- a/rapidsms_multimodem/tests/test_outgoing.py +++ b/rapidsms_multimodem/tests/test_outgoing.py @@ -1,6 +1,6 @@ # coding: utf-8 from __future__ import unicode_literals -from mock import patch +from mock import patch, MagicMock from django.test import TestCase @@ -50,10 +50,85 @@ def test_prepare_unicode_querystring(self): self.assertIn('text=', query_string) @patch('rapidsms_multimodem.outgoing.requests') - def test_send(self, mock_requests): + def test_send_one(self, mock_requests): message = self.create_outgoing_message(data={'text': 'a message'}) + # Fake an OK response + mock_requests.get.return_value.status_code = mock_requests.codes.ok self.backend.send(id_=message.id, text=message.text, - identities=message.connections[0].identity, + identities=[message.connections[0].identity], context={}) self.assertTrue(mock_requests.get.called) + self.assertEqual(mock_requests.get.call_count, 1) + + @patch('rapidsms_multimodem.outgoing.requests') + def test_send_multiple(self, mock_requests): + conn1 = self.create_connection() + conn2 = self.create_connection(data={'backend': conn1.backend}) + message = self.create_outgoing_message(data={'text': 'a message', + 'connections': [conn1, conn2]}) + # Fake an OK response + mock_requests.get.return_value.status_code = mock_requests.codes.ok + self.backend.send(id_=message.id, + text=message.text, + identities=[conn.identity for conn in message.connections], + context={}) + self.assertTrue(mock_requests.get.called) + # Since multimodem only accepts 1 identity per request, it should get called twice + self.assertEqual(mock_requests.get.call_count, 2) + + @patch('rapidsms_multimodem.outgoing.requests') + def test_send_multiple_with_one_http_error(self, mock_requests): + conn1 = self.create_connection() + conn2 = self.create_connection(data={'backend': conn1.backend}) + message = self.create_outgoing_message(data={'text': 'a message', + 'connections': [conn1, conn2]}) + # Fake one BAD (400) and one OK (200) response + rsp1 = MagicMock(status_code=mock_requests.codes.bad) + rsp2 = MagicMock(status_code=mock_requests.codes.ok) + # each time requests is called, mock will return the next side_effect in the list + # https://docs.python.org/3/library/unittest.mock.html#unittest.mock.Mock + mock_requests.get.side_effect = [rsp1, rsp2] + with self.assertRaises(Exception) as cm: + self.backend.send(id_=message.id, + text=message.text, + identities=[conn.identity for conn in message.connections], + context={}) + # even though the first message raised an error, requests gets called twice + # i.e. bad message doesn't block the good message + self.assertEqual(mock_requests.get.call_count, 2) + error_string, failures = cm.exception.args + self.assertEqual(len(failures), 1) + self.assertIn(conn1.identity, failures) + self.assertNotIn(conn2.identity, failures) + + @patch('rapidsms_multimodem.outgoing.requests') + def test_send_multiple_with_one_multimodem_error(self, mock_requests): + conn1 = self.create_connection() + conn2 = self.create_connection(data={'backend': conn1.backend}) + message = self.create_outgoing_message(data={'text': 'a message', + 'connections': [conn1, conn2]}) + # Fake one multimodem error and one multimodem success response + rsp1 = MagicMock( + status_code=mock_requests.codes.ok, + text="Err: blah" + ) + rsp2 = MagicMock( + status_code=mock_requests.codes.ok, + text="Success" + ) + # each time requests is called, mock will return the next side_effect in the list + # https://docs.python.org/3/library/unittest.mock.html#unittest.mock.Mock + mock_requests.get.side_effect = [rsp1, rsp2] + with self.assertRaises(Exception) as cm: + self.backend.send(id_=message.id, + text=message.text, + identities=[conn.identity for conn in message.connections], + context={}) + # even though the first message raised an error, requests gets called twice + # i.e. bad message doesn't block the good message + self.assertEqual(mock_requests.get.call_count, 2) + error_string, failures = cm.exception.args + self.assertEqual(len(failures), 1) + self.assertIn(conn1.identity, failures) + self.assertNotIn(conn2.identity, failures)