From 24be33b895b2f988ee4831e0474073b3e48f144e Mon Sep 17 00:00:00 2001
From: Nicolas ROCHE <nroche@entrouvert.com>
Date: Sun, 26 Apr 2020 15:54:47 +0200
Subject: [PATCH] sms: send SMS asynchronously (#21465)

---
 passerelle/base/models.py | 18 +++++++++++---
 tests/test_orange.py      | 31 +++++++++++++++---------
 tests/test_sms.py         | 50 ++++++++++++++++++++++++++++++++-------
 tests/utils.py            |  4 +++-
 4 files changed, 79 insertions(+), 24 deletions(-)

diff --git a/passerelle/base/models.py b/passerelle/base/models.py
index 9d9365db..d16afde5 100644
--- a/passerelle/base/models.py
+++ b/passerelle/base/models.py
@@ -35,17 +35,17 @@ from jsonfield import JSONField
 from model_utils.managers import InheritanceManager as ModelUtilsInheritanceManager
 
 import jsonfield
 
 import passerelle
 import requests
 from passerelle.compat import json_loads
 from passerelle.utils.api import endpoint
-from passerelle.utils.jsonresponse import APIError
+from passerelle.utils.jsonresponse import APIError, APIRecoverableError
 
 KEYTYPE_CHOICES = (
     ('API', _('API Key')),
     ('SIGN', _('HMAC Signature')),
 )
 
 BASE_EXPORT_FIELDS = (models.TextField, models.CharField, models.SlugField,
                       models.URLField, models.BooleanField, models.IntegerField,
@@ -957,19 +957,31 @@ class SMSResource(BaseResource):
             assert all(map(lambda x: isinstance(x, six.text_type), data['to'])), \
                 'to is not a list of strings'
         except (ValueError, AssertionError) as e:
             raise APIError('Payload error: %s' % e)
         data['message'] = data['message'][:self.max_message_length]
         logging.info('sending message %r to %r with sending number %r',
                      data['message'], data['to'], data['from'])
         stop = not bool('nostop' in request.GET)
-        result = {'data': self.send_msg(data['message'], data['from'], data['to'], stop=stop)}
+        self.add_job('send_job',
+                     text=data['message'], sender=data['from'], destinations=data['to'],
+                     stop=stop)
+        return {'err': 0}
+
+    def send_job(self, *args, **kwargs):
+        try:
+            self.send_msg(**kwargs)
+        except APIRecoverableError as exc:
+            logging.info('recoverable error on message %r to %r with sending number %r: %s',
+                         kwargs['text'], kwargs['sender'], kwargs['destinations'], exc)
+            raise SkipJob(after_timestamp=3600 * 1)
+        except APIError:
+            raise  # not recoverable
         SMSLog.objects.create(appname=self.get_connector_slug(), slug=self.slug)
-        return result
 
     class Meta:
         abstract = True
 
 
 @six.python_2_unicode_compatible
 class SMSLog(models.Model):
     timestamp = models.DateTimeField(auto_now_add=True)
diff --git a/tests/test_orange.py b/tests/test_orange.py
index e803d88f..2ecd213a 100644
--- a/tests/test_orange.py
+++ b/tests/test_orange.py
@@ -20,17 +20,17 @@ import requests
 
 import httmock
 import pytest
 
 from django.contrib.contenttypes.models import ContentType
 from django.utils.encoding import force_text
 
 from passerelle.apps.orange.models import OrangeSMSGateway, OrangeError
-from passerelle.base.models import ApiUser, AccessRight
+from passerelle.base.models import ApiUser, AccessRight, Job
 from passerelle.utils.jsonresponse import APIError, APIRecoverableError
 
 
 NETLOC = 'contact-everyone.orange-business.com'
 JSON_HEADERS = {'content-type': 'application/json'}
 PAYLOAD = {
     'message': 'hello',
     'from': '+33699999999',
@@ -163,25 +163,34 @@ def test_diffusion(app, connector):
 
     with pytest.raises(OrangeError, match='Orange returned Invalid JSON content'):
         with httmock.HTTMock(mocked_response):
             orange.diffusion('my_token', 'gid2', PAYLOAD['to'], PAYLOAD['message'])
 
 
 def test_send_msg(app, connector):
     url = '/%s/%s/send/' % (connector.get_connector_slug(), connector.slug)
-    with httmock.HTTMock(response_token_ok, response_group_ok, response_diffusion_ok):
-        resp = app.post_json(url, params=PAYLOAD, status=200)
+    assert Job.objects.count() == 0
+    resp = app.post_json(url, params=PAYLOAD, status=200)
     assert not resp.json['err']
-    assert resp.json['data']['status'] == "I'm ok"
+    assert Job.objects.count() == 1
+    with httmock.HTTMock(response_token_ok, response_group_ok, response_diffusion_ok):
+        connector.jobs()
+    assert Job.objects.all()[0].status == 'completed'
 
     # not 201
+    resp = app.post_json(url, params=PAYLOAD, status=200)
+    assert not resp.json['err']
+    assert Job.objects.count() == 2
     with httmock.HTTMock(response_token_ok, response_group_ok, response_500):
-        resp = app.post_json(url, params=PAYLOAD, status=200)
-    assert resp.json['err']
-    assert resp.json['err_desc'] == 'Orange fails to send SMS: 500, my_error'
+        connector.jobs()
+    job = Job.objects.all()[1]
+    assert job.status == 'failed'
+    assert 'Orange fails to send SMS: 500, my_error' in job.status_details['error_summary']
 
     # RequestException
+    resp = app.post_json(url, params=PAYLOAD, status=200)
+    assert not resp.json['err']
+    assert Job.objects.count() == 3
     with httmock.HTTMock(request_exception):
-        resp = app.post_json(url, params=PAYLOAD, status=200)
-    assert resp.json['err']
-    assert resp.json['err_desc'] == 'Orange error: POST failed, my_error'
-    assert 'APIRecoverableError' in resp.json['err_class']
+        connector.jobs()
+    job = Job.objects.all()[2]
+    assert job.status == 'registered'
diff --git a/tests/test_sms.py b/tests/test_sms.py
index 53d6f2ca..a5bdb8b9 100644
--- a/tests/test_sms.py
+++ b/tests/test_sms.py
@@ -1,15 +1,17 @@
+import isodate
 import mock
 import pytest
+from requests import RequestException
 
 from django.contrib.contenttypes.models import ContentType
 
 from passerelle.apps.ovh.models import OVHSMSGateway
-from passerelle.base.models import ApiUser, AccessRight, SMSResource, SMSLog
+from passerelle.base.models import ApiUser, AccessRight, Job, SMSResource, SMSLog
 
 from test_manager import login, admin_user
 
 import utils
 
 pytestmark = pytest.mark.django_db
 
 klasses = SMSResource.__subclasses__()
@@ -39,37 +41,65 @@ def connector(request, db):
     # no access check
     AccessRight.objects.create(codename='can_send_messages',
                                apiuser=api,
                                resource_type=obj_type,
                                resource_pk=c.pk)
     return c
 
 
-def test_connectors(app, connector):
+def test_connectors(app, connector, freezer):
     path = '/%s/%s/send/' % (connector.get_connector_slug(), connector.slug)
     result = app.post_json(path, params={})
     assert result.json['err'] == 1
     assert result.json['err_desc'].startswith('Payload error: ')
 
     payload = {
         'message': 'hello',
         'from': '+33699999999',
         'to': ['+33688888888', '+33677777777'],
     }
-    for test_vector in getattr(connector, 'TEST_DEFAULTS', {}).get('test_vectors', []):
+    test_vectors = getattr(connector, 'TEST_DEFAULTS', {}).get('test_vectors', [])
+    total = len(test_vectors)
+    nb_failed = 0
+    assert Job.objects.count() == 0
+    for test_vector in test_vectors:
+
+        # register job
+        freezer.move_to('2019-01-01 00:00:00')
+        result = app.post_json(path, params=payload)
+        assert result.json['err'] == 0
+        job_id = Job.objects.get(status='registered').id
+
+        # transport error
+        freezer.move_to('2019-01-01 00:00:02')
+        with utils.mock_url(connector.URL,
+                            exception=RequestException('connection timed-out')):
+            connector.jobs()
+        job = Job.objects.get(id=job_id)
+        assert job.update_timestamp == isodate.parse_datetime('2019-01-01T00:00:02+00:00')
+        assert job.status == 'registered'
+        assert job.status_details == {}
+
+        # perform job
+        freezer.move_to('2019-01-01 01:00:03')
         with utils.mock_url(
                 connector.URL,
                 test_vector.get('response', ''),
                 test_vector.get('status_code', 200)):
-
-            result = app.post_json(path, params=payload)
-            for key, value in test_vector['result'].items():
-                assert key in result.json
-                assert result.json[key] == value
+            connector.jobs()
+        job = Job.objects.get(id=job_id)
+        if job.status == 'failed':
+            assert len(job.status_details['error_summary']) > 0
+            assert test_vector['result']['err_desc'] in job.status_details['error_summary']
+            nb_failed += 1
+        else:
+            assert job.status == 'completed'
+    assert Job.objects.count() == total
+    assert SMSLog.objects.count() == total - nb_failed
 
 
 def test_manage_views(admin_user, app, connector):
     url = '/%s/%s/' % (connector.get_connector_slug(), connector.slug)
     resp = app.get(url)
     assert 'Endpoints' in resp.text
     assert not 'accessright/add' in resp.text
     app = login(app)
@@ -86,25 +116,27 @@ def test_sms_max_message_length(app, connector):
     payload = {
         'message': message_above_limit,
         'from': '+33699999999',
         'to': ['+33688888888'],
     }
     with mock.patch.object(OVHSMSGateway, 'send_msg') as send_function:
         send_function.return_value = {}
         result = app.post_json(path, params=payload)
-        assert send_function.call_args[0][0] == 'a' * connector.max_message_length
+        connector.jobs()
+        assert send_function.call_args[1]['text'] == 'a' * connector.max_message_length
 
 
 @pytest.mark.parametrize('connector', [OVHSMSGateway], indirect=True)
 def test_sms_log(app, connector):
     path = '/%s/%s/send/' % (connector.get_connector_slug(), connector.slug)
     assert not SMSLog.objects.filter(appname=connector.get_connector_slug(), slug=connector.slug).exists()
 
     payload = {
         'message': 'plop',
         'from': '+33699999999',
         'to': ['+33688888888'],
     }
     with mock.patch.object(OVHSMSGateway, 'send_msg') as send_function:
         send_function.return_value = {}
         result = app.post_json(path, params=payload)
+        connector.jobs()
         assert SMSLog.objects.filter(appname=connector.get_connector_slug(), slug=connector.slug).exists()
diff --git a/tests/utils.py b/tests/utils.py
index 62fc82bc..6af91756 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -27,30 +27,32 @@ def setup_access_rights(obj):
 
 class FakedResponse(mock.Mock):
     headers = {}
 
     def json(self):
         return json_loads(self.content)
 
 
-def mock_url(url=None, response='', status_code=200, headers=None):
+def mock_url(url=None, response='', status_code=200, headers=None, exception=None):
     urlmatch_kwargs = {}
     if url:
         parsed = urlparse.urlparse(url)
         if parsed.netloc:
             urlmatch_kwargs['netloc'] = parsed.netloc
         if parsed.path:
             urlmatch_kwargs['path'] = parsed.path
 
     if not isinstance(response, str):
         response = json.dumps(response)
 
     @httmock.urlmatch(**urlmatch_kwargs)
     def mocked(url, request):
+        if exception:
+            raise exception
         return httmock.response(status_code, response, headers, request=request)
     return httmock.HTTMock(mocked)
 
 
 def make_resource(model_class, **kwargs):
     resource = model_class.objects.create(**kwargs)
     setup_access_rights(resource)
     return resource
-- 
2.26.2

