MSG91 Complete Refactor to Accommodate Upstream Changes (#966)

README.md

@@ -141,7 +141,7 @@ The table below identifies the services this tool supports and some example serv
 | [DingTalk](https://github.com/caronc/apprise/wiki/Notify_dingtalk)  | dingtalk://   | (TCP) 443   | dingtalk://token/<br />dingtalk://token/ToPhoneNo<br />dingtalk://token/ToPhoneNo1/ToPhoneNo2/ToPhoneNo1/
 | [Kavenegar](https://github.com/caronc/apprise/wiki/Notify_kavenegar) | kavenegar://  | (TCP) 443   | kavenegar://ApiKey/ToPhoneNo<br/>kavenegar://FromPhoneNo@ApiKey/ToPhoneNo<br/>kavenegar://ApiKey/ToPhoneNo1/ToPhoneNo2/ToPhoneNoN
 | [MessageBird](https://github.com/caronc/apprise/wiki/Notify_messagebird) | msgbird://  | (TCP) 443   | msgbird://ApiKey/FromPhoneNo<br/>msgbird://ApiKey/FromPhoneNo/ToPhoneNo<br/>msgbird://ApiKey/FromPhoneNo/ToPhoneNo1/ToPhoneNo2/ToPhoneNoN/
-| [MSG91](https://github.com/caronc/apprise/wiki/Notify_msg91) | msg91://  | (TCP) 443   | msg91://AuthKey/ToPhoneNo<br/>msg91://SenderID@AuthKey/ToPhoneNo<br/>msg91://AuthKey/ToPhoneNo1/ToPhoneNo2/ToPhoneNoN/
+| [MSG91](https://github.com/caronc/apprise/wiki/Notify_msg91) | msg91://  | (TCP) 443   | msg91://TemplateID@AuthKey/ToPhoneNo<br/>msg91://TemplateID@AuthKey/ToPhoneNo1/ToPhoneNo2/ToPhoneNoN/
 | [Signal API](https://github.com/caronc/apprise/wiki/Notify_signal) | signal://  or signals:// | (TCP) 80 or 443  | signal://hostname:port/FromPhoneNo<br/>signal://hostname:port/FromPhoneNo/ToPhoneNo<br/>signal://hostname:port/FromPhoneNo/ToPhoneNo1/ToPhoneNo2/ToPhoneNoN/
 | [Sinch](https://github.com/caronc/apprise/wiki/Notify_sinch) | sinch://  | (TCP) 443   | sinch://ServicePlanId:ApiToken@FromPhoneNo<br/>sinch://ServicePlanId:ApiToken@FromPhoneNo/ToPhoneNo<br/>sinch://ServicePlanId:ApiToken@FromPhoneNo/ToPhoneNo1/ToPhoneNo2/ToPhoneNoN/<br/>sinch://ServicePlanId:ApiToken@ShortCode/ToPhoneNo<br/>sinch://ServicePlanId:ApiToken@ShortCode/ToPhoneNo1/ToPhoneNo2/ToPhoneNoN/
 | [SMSEagle](https://github.com/caronc/apprise/wiki/Notify_smseagle) | smseagle://  or smseagles:// | (TCP) 80 or 443  | smseagles://hostname:port/ToPhoneNo<br/>smseagles://hostname:port/@ToContact<br/>smseagles://hostname:port/#ToGroup<br/>smseagles://hostname:port/ToPhoneNo1/#ToGroup/@ToContact/

apprise/plugins/NotifyMSG91.py

@@ -35,50 +35,31 @@
 # Get your (authkey) from the dashboard here:
 #   - https://world.msg91.com/user/index.php#api
 #
+# Note: You will need to define a template for this to work
+#
 # Get details on the API used in this plugin here:
-#   - https://world.msg91.com/apidoc/textsms/send-sms.php
+#   - https://docs.msg91.com/reference/send-sms
-
+import re
 import requests
-
+from json import dumps
 from .NotifyBase import NotifyBase
 from ..common import NotifyType
 from ..utils import is_phone_no
-from ..utils import parse_phone_no
+from ..utils import parse_phone_no, parse_bool
 from ..utils import validate_regex
 from ..AppriseLocale import gettext_lazy as _
 
 
-class MSG91Route:
+class MSG91PayloadField:
     """
-    Transactional SMS Routes
+    Identifies the fields available in the JSON Payload
-    route=1 for promotional, route=4 for transactional SMS.
     """
-    PROMOTIONAL = 1
+    BODY = 'body'
-    TRANSACTIONAL = 4
+    MESSAGETYPE = 'type'
 
 
-# Used for verification
+# Add entries here that are reserved
-MSG91_ROUTES = (
+RESERVED_KEYWORDS = ('mobiles', )
-    MSG91Route.PROMOTIONAL,
-    MSG91Route.TRANSACTIONAL,
-)
-
-
-class MSG91Country:
-    """
-    Optional value that can be specified on the MSG91 api
-    """
-    INTERNATIONAL = 0
-    USA = 1
-    INDIA = 91
-
-
-# Used for verification
-MSG91_COUNTRIES = (
-    MSG91Country.INTERNATIONAL,
-    MSG91Country.USA,
-    MSG91Country.INDIA,
-)
 
 
 class NotifyMSG91(NotifyBase):
@@ -99,7 +80,7 @@ class NotifyMSG91(NotifyBase):
     setup_url = 'https://github.com/caronc/apprise/wiki/Notify_msg91'
 
     # MSG91 uses the http protocol with JSON requests
-    notify_url = 'https://world.msg91.com/api/sendhttp.php'
+    notify_url = 'https://control.msg91.com/api/v5/flow/'
 
     # The maximum length of the body
     body_maxlen = 160
@@ -108,14 +89,24 @@ class NotifyMSG91(NotifyBase):
     # cause any title (if defined) to get placed into the message body.
     title_maxlen = 0
 
+    # Our supported mappings and component keys
+    component_key_re = re.compile(
+        r'(?P<key>((?P<id>[a-z0-9_-])?|(?P<map>body|type)))', re.IGNORECASE)
+
     # Define object templates
     templates = (
-        '{schema}://{authkey}/{targets}',
+        '{schema}://{template}@{authkey}/{targets}',
-        '{schema}://{sender}@{authkey}/{targets}',
     )
 
     # Define our template tokens
     template_tokens = dict(NotifyBase.template_tokens, **{
+        'template': {
+            'name': _('Template ID'),
+            'type': 'string',
+            'required': True,
+            'private': True,
+            'regex': (r'^[a-z0-9 _-]+$', 'i'),
+        },
         'authkey': {
             'name': _('Authentication Key'),
             'type': 'string',
@@ -135,10 +126,6 @@ class NotifyMSG91(NotifyBase):
             'type': 'list:string',
             'required': True,
         },
-        'sender': {
-            'name': _('Sender ID'),
-            'type': 'string',
-        },
     })
 
     # Define our template arguments
@@ -146,21 +133,23 @@ class NotifyMSG91(NotifyBase):
         'to': {
             'alias_of': 'targets',
         },
-        'route': {
+        'short_url': {
-            'name': _('Route'),
+            'name': _('Short URL'),
-            'type': 'choice:int',
+            'type': 'bool',
-            'values': MSG91_ROUTES,
+            'default': False,
-            'default': MSG91Route.TRANSACTIONAL,
-        },
-        'country': {
-            'name': _('Country'),
-            'type': 'choice:int',
-            'values': MSG91_COUNTRIES,
         },
     })
 
-    def __init__(self, authkey, targets=None, sender=None, route=None,
+    # Define any kwargs we're using
-                 country=None, **kwargs):
+    template_kwargs = {
+        'template_mapping': {
+            'name': _('Template Mapping'),
+            'prefix': ':',
+        },
+    }
+
+    def __init__(self, template, authkey, targets=None, short_url=None,
+                 template_mapping=None, **kwargs):
         """
         Initialize MSG91 Object
         """
@@ -175,39 +164,20 @@ class NotifyMSG91(NotifyBase):
             self.logger.warning(msg)
             raise TypeError(msg)
 
-        if route is None:
+        # Template ID
-            self.route = self.template_args['route']['default']
+        self.template = validate_regex(
-
+            template, *self.template_tokens['template']['regex'])
-        else:
+        if not self.template:
-            try:
+            msg = 'An invalid MSG91 Template ID ' \
-                self.route = int(route)
+                  '({}) was specified.'.format(template)
-                if self.route not in MSG91_ROUTES:
-                    # Let outer except catch thi
-                    raise ValueError()
-
-            except (ValueError, TypeError):
-                msg = 'The MSG91 route specified ({}) is invalid.'\
-                    .format(route)
             self.logger.warning(msg)
             raise TypeError(msg)
 
-        if country:
+        if short_url is None:
-            try:
+            self.short_url = self.template_args['short_url']['default']
-                self.country = int(country)
-                if self.country not in MSG91_COUNTRIES:
-                    # Let outer except catch thi
-                    raise ValueError()
 
-            except (ValueError, TypeError):
-                msg = 'The MSG91 country specified ({}) is invalid.'\
-                    .format(country)
-                self.logger.warning(msg)
-                raise TypeError(msg)
         else:
-            self.country = country
+            self.short_url = parse_bool(short_url)
-
-        # Store our sender
-        self.sender = sender
 
         # Parse our targets
         self.targets = list()
@@ -225,6 +195,11 @@ class NotifyMSG91(NotifyBase):
             # store valid phone number
             self.targets.append(result['full'])
 
+        self.template_mapping = {}
+        if template_mapping:
+            # Store our extra payload entries
+            self.template_mapping.update(template_mapping)
+
         return
 
     def send(self, body, title='', notify_type=NotifyType.INFO, **kwargs):
@@ -240,23 +215,55 @@ class NotifyMSG91(NotifyBase):
         # Prepare our headers
         headers = {
             'User-Agent': self.app_id,
-            'Content-Type': 'application/x-www-form-urlencoded',
+            'Content-Type': 'application/json',
+            'authkey': self.authkey,
         }
 
+        # Base
+        recipient_payload = {
+            'mobiles': None,
+            # Keyword Tokens
+            MSG91PayloadField.BODY: body,
+            MSG91PayloadField.MESSAGETYPE: notify_type,
+        }
+
+        # Prepare Recipient Payload Object
+        for key, value in self.template_mapping.items():
+
+            if key in RESERVED_KEYWORDS:
+                self.logger.warning(
+                    'Ignoring MSG91 custom payload entry %s', key)
+                continue
+
+            if key in recipient_payload:
+                if not value:
+                    # Do not store element in payload response
+                    del recipient_payload[key]
+
+                else:
+                    # Re-map
+                    recipient_payload[value] = recipient_payload[key]
+                    del recipient_payload[key]
+
+            else:
+                # Append entry
+                recipient_payload[key] = value
+
+        # Prepare our recipients
+        recipients = []
+        for target in self.targets:
+            recipient = recipient_payload.copy()
+            recipient['mobiles'] = target
+            recipients.append(recipient)
+
         # Prepare our payload
         payload = {
-            'sender': self.sender if self.sender else self.app_id,
+            'template_id': self.template,
-            'authkey': self.authkey,
+            'short_url': 1 if self.short_url else 0,
-            'message': body,
-            'response': 'json',
             # target phone numbers are sent with a comma delimiter
-            'mobiles': ','.join(self.targets),
+            'recipients': recipients,
-            'route': str(self.route),
         }
 
-        if self.country:
-            payload['country'] = str(self.country)
-
         # Some Debug Logging
         self.logger.debug('MSG91 POST URL: {} (cert_verify={})'.format(
             self.notify_url, self.verify_certificate))
@@ -268,7 +275,7 @@ class NotifyMSG91(NotifyBase):
         try:
             r = requests.post(
                 self.notify_url,
-                data=payload,
+                data=dumps(payload),
                 headers=headers,
                 verify=self.verify_certificate,
                 timeout=self.request_timeout,
@@ -314,17 +321,20 @@ class NotifyMSG91(NotifyBase):
 
         # Define any URL parameters
         params = {
-            'route': str(self.route),
+            'short_url': str(self.short_url),
         }
 
         # Extend our parameters
         params.update(self.url_parameters(privacy=privacy, *args, **kwargs))
 
-        if self.country:
+        # Payload body extras prefixed with a ':' sign
-            params['country'] = str(self.country)
+        # Append our payload extras into our parameters
+        params.update(
+            {':{}'.format(k): v for k, v in self.template_mapping.items()})
 
-        return '{schema}://{authkey}/{targets}/?{params}'.format(
+        return '{schema}://{template}@{authkey}/{targets}/?{params}'.format(
             schema=self.secure_protocol,
+            template=self.pprint(self.template, privacy, safe=''),
             authkey=self.pprint(self.authkey, privacy, safe=''),
             targets='/'.join(
                 [NotifyMSG91.quote(x, safe='') for x in self.targets]),
@@ -334,7 +344,8 @@ class NotifyMSG91(NotifyBase):
         """
         Returns the number of targets associated with this notification
         """
-        return len(self.targets)
+        targets = len(self.targets)
+        return targets if targets > 0 else 1
 
     @staticmethod
     def parse_url(url):
@@ -356,11 +367,11 @@ class NotifyMSG91(NotifyBase):
         # The hostname is our authentication key
         results['authkey'] = NotifyMSG91.unquote(results['host'])
 
-        if 'route' in results['qsd'] and len(results['qsd']['route']):
+        # The template id is kept in the user field
-            results['route'] = results['qsd']['route']
+        results['template'] = NotifyMSG91.unquote(results['user'])
 
-        if 'country' in results['qsd'] and len(results['qsd']['country']):
+        if 'short_url' in results['qsd'] and len(results['qsd']['short_url']):
-            results['country'] = results['qsd']['country']
+            results['short_url'] = parse_bool(results['qsd']['short_url'])
 
         # Support the 'to' variable so that we can support targets this way too
         # The 'to' makes it easier to use yaml configuration
@@ -368,4 +379,10 @@ class NotifyMSG91(NotifyBase):
             results['targets'] += \
                 NotifyMSG91.parse_phone_no(results['qsd']['to'])
 
+        # store any additional payload extra's defined
+        results['template_mapping'] = {
+            NotifyMSG91.unquote(x): NotifyMSG91.unquote(y)
+            for x, y in results['qsd:'].items()
+        }
+
         return results

test/test_plugin_msg91.py

@@ -34,7 +34,8 @@ from unittest import mock
 
 import pytest
 import requests
-
+from json import loads
+from apprise import Apprise
 from apprise.plugins.NotifyMSG91 import NotifyMSG91
 from helpers import AppriseURLTester
 
@@ -53,69 +54,49 @@ apprise_url_tests = (
         'instance': TypeError,
     }),
     ('msg91://{}'.format('a' * 23), {
-        # valid AuthKey
+        # valid AuthKey but no Template ID
+        'instance': TypeError,
+    }),
+    ('msg91://{}@{}'.format('t' * 20, 'a' * 23), {
+        # Valid entry but no targets
         'instance': NotifyMSG91,
         # Since there are no targets specified we expect a False return on
         # send()
         'notify_response': False,
     }),
-    ('msg91://{}/123'.format('a' * 23), {
+    ('msg91://{}@{}/abcd'.format('t' * 20, 'a' * 23), {
-        # invalid phone number
-        'instance': NotifyMSG91,
-        # Since there are no targets specified we expect a False return on
-        # send()
-        'notify_response': False,
-    }),
-    ('msg91://{}/abcd'.format('a' * 23), {
         # No number to notify
         'instance': NotifyMSG91,
         # Since there are no targets specified we expect a False return on
         # send()
         'notify_response': False,
     }),
-    ('msg91://{}/15551232000/?country=invalid'.format('a' * 23), {
+    ('msg91://{}@{}/15551232000'.format('t' * 20, 'a' * 23), {
-        # invalid country
-        'instance': TypeError,
-    }),
-    ('msg91://{}/15551232000/?country=99'.format('a' * 23), {
-        # invalid country
-        'instance': TypeError,
-    }),
-    ('msg91://{}/15551232000/?route=invalid'.format('a' * 23), {
-        # invalid route
-        'instance': TypeError,
-    }),
-    ('msg91://{}/15551232000/?route=99'.format('a' * 23), {
-        # invalid route
-        'instance': TypeError,
-    }),
-    ('msg91://{}/15551232000'.format('a' * 23), {
         # a valid message
         'instance': NotifyMSG91,
 
         # Our expected url(privacy=True) startswith() response:
-        'privacy_url': 'msg91://a...a/15551232000',
+        'privacy_url': 'msg91://t...t@a...a/15551232000',
     }),
-    ('msg91://{}/?to=15551232000'.format('a' * 23), {
+    ('msg91://{}@{}/?to=15551232000&short_url=no'.format('t' * 20, 'a' * 23), {
         # a valid message
         'instance': NotifyMSG91,
     }),
-    ('msg91://{}/15551232000?country=91&route=1'.format('a' * 23), {
+    ('msg91://{}@{}/15551232000?short_url=yes'.format('t' * 20, 'a' * 23), {
-        # using phone no with no target - we text ourselves in
+        # testing short_url
-        # this case
         'instance': NotifyMSG91,
     }),
-    ('msg91://{}/15551232000'.format('a' * 23), {
+    ('msg91://{}@{}/15551232000'.format('t' * 20, 'a' * 23), {
         # use get args to acomplish the same thing
         'instance': NotifyMSG91,
     }),
-    ('msg91://{}/15551232000'.format('a' * 23), {
+    ('msg91://{}@{}/15551232000'.format('t' * 20, 'a' * 23), {
         'instance': NotifyMSG91,
         # throw a bizzare code forcing us to fail to look it up
         'response': False,
         'requests_response_code': 999,
     }),
-    ('msg91://{}/15551232000'.format('a' * 23), {
+    ('msg91://{}@{}/15551232000'.format('t' * 20, 'a' * 23), {
         'instance': NotifyMSG91,
         # Throws a series of connection and transfer exceptions when this flag
         # is set and tests that we gracfully handle them
@@ -149,11 +130,89 @@ def test_plugin_msg91_edge_cases(mock_post):
     mock_post.return_value = response
 
     # Initialize some generic (but valid) tokens
-    # authkey = '{}'.format('a' * 24)
     target = '+1 (555) 123-3456'
 
     # No authkey specified
     with pytest.raises(TypeError):
-        NotifyMSG91(authkey=None, targets=target)
+        NotifyMSG91(template="1234", authkey=None, targets=target)
     with pytest.raises(TypeError):
-        NotifyMSG91(authkey="    ", targets=target)
+        NotifyMSG91(template="1234", authkey="    ", targets=target)
+    with pytest.raises(TypeError):
+        NotifyMSG91(template="     ", authkey='a' * 23, targets=target)
+    with pytest.raises(TypeError):
+        NotifyMSG91(template=None, authkey='a' * 23, targets=target)
+
+
+@mock.patch('requests.post')
+def test_plugin_msg91_keywords(mock_post):
+    """
+    NotifyMSG91() Templating
+
+    """
+
+    response = mock.Mock()
+    response.content = ''
+    response.status_code = requests.codes.ok
+
+    # Prepare Mock
+    mock_post.return_value = response
+
+    target = '+1 (555) 123-3456'
+    template = '12345'
+    authkey = '{}'.format('b' * 32)
+
+    message_contents = "test"
+
+    # Variation of initialization without API key
+    obj = Apprise.instantiate(
+        'msg91://{}@{}/{}?:key=value&:mobiles=ignored'
+        .format(template, authkey, target))
+    assert isinstance(obj, NotifyMSG91) is True
+    assert isinstance(obj.url(), str) is True
+
+    # Send Notification
+    assert obj.send(body=message_contents) is True
+
+    # Validate expected call parameters
+    assert mock_post.call_count == 1
+    first_call = mock_post.call_args_list[0]
+
+    # URL and message parameters are the same for both calls
+    assert first_call[0][0] == 'https://control.msg91.com/api/v5/flow/'
+    response = loads(first_call[1]['data'])
+    assert response['template_id'] == template
+    assert response['short_url'] == 0
+    assert len(response['recipients']) == 1
+    # mobiles is not over-ridden as it is a special reserved token
+    assert response['recipients'][0]['mobiles'] == '15551233456'
+
+    # Our base tokens
+    assert response['recipients'][0]['body'] == message_contents
+    assert response['recipients'][0]['type'] == 'info'
+    assert response['recipients'][0]['key'] == 'value'
+
+    mock_post.reset_mock()
+
+    # Play with mapping
+    obj = Apprise.instantiate(
+        'msg91://{}@{}/{}?:body&:type=cat'.format(template, authkey, target))
+    assert isinstance(obj, NotifyMSG91) is True
+    assert isinstance(obj.url(), str) is True
+
+    # Send Notification
+    assert obj.send(body=message_contents) is True
+
+    # Validate expected call parameters
+    assert mock_post.call_count == 1
+    first_call = mock_post.call_args_list[0]
+
+    # URL and message parameters are the same for both calls
+    assert first_call[0][0] == 'https://control.msg91.com/api/v5/flow/'
+    response = loads(first_call[1]['data'])
+    assert response['template_id'] == template
+    assert response['short_url'] == 0
+    assert len(response['recipients']) == 1
+    assert response['recipients'][0]['mobiles'] == '15551233456'
+    assert 'body' not in response['recipients'][0]
+    assert 'type' not in response['recipients'][0]
+    assert response['recipients'][0]['cat'] == 'info'