test coverage incomplete; but at least passes

apprise/plugins/smpp.py

@@ -57,7 +57,7 @@ class NotifySMPP(NotifyBase):
 
     requirements = {
         # Define our required packaging in order to work
-        'packages_required': 'python-snpp'
+        'packages_required': 'smpplib'
     }
 
     # The default descriptive name associated with the Notification
@@ -72,6 +72,10 @@ class NotifySMPP(NotifyBase):
     # The default secure protocol
     secure_protocol = 'smpps'
 
+    # Default port setup
+    default_port = 2775
+    default_secure_port = 3550
+
     # A URL that takes you to the setup/help of the specific protocol
     setup_url = 'https://github.com/caronc/apprise/wiki/Notify_SMPP'
 
@@ -80,6 +84,7 @@ class NotifySMPP(NotifyBase):
     title_maxlen = 0
 
     templates = (
+        '{schema}://{user}:{password}@{host}/{from_phone}/{targets}',
         '{schema}://{user}:{password}@{host}:{port}/{from_phone}/{targets}',
     )
 
@@ -105,7 +110,6 @@ class NotifySMPP(NotifyBase):
             'type': 'int',
             'min': 1,
             'max': 65535,
-            'required': True,
         },
         'from_phone': {
             'name': _('From Phone No'),
@@ -135,17 +139,25 @@ class NotifySMPP(NotifyBase):
         super().__init__(**kwargs)
 
         self.source = None
-        if source:
+        result = is_phone_no(source)
-            result = is_phone_no(source)
+        if not result:
-            if not result:
+            msg = 'The Account (From) Phone # specified ' \
-                msg = 'The Account (From) Phone # specified ' \
+                  '({}) is invalid.'.format(source)
-                      '({}) is invalid.'.format(source)
+            self.logger.warning(msg)
-                self.logger.warning(msg)
+            raise TypeError(msg)
 
-                raise TypeError(msg)
+        if not self.user:
+            msg = 'No SMPP user account was specified.'
+            self.logger.warning(msg)
+            raise TypeError(msg)
 
-            # Tidy source
+        if not self.host:
-            self.source = result['full']
+            msg = 'No SMPP host was specified.'
+            self.logger.warning(msg)
+            raise TypeError(msg)
+
+        # Tidy source
+        self.source = result['full']
 
         # Used for URL generation afterwards only
         self._invalid_targets = list()
@@ -186,13 +198,13 @@ class NotifySMPP(NotifyBase):
 
         params = self.url_parameters(privacy=privacy, *args, **kwargs)
 
-        return ('{schema}://{user}:{password}@{host}:{port}/{source}/{targets}'
+        return ('{schema}://{user}:{password}@{host}/{source}/{targets}'
                 '/?{params}').format(
             schema=self.secure_protocol if self.secure else self.protocol,
             user=self.user,
             password=self.password,
-            host=self.host,
+            host='{}:{}'.format(self.host, self.port)
-            port=self.port,
+            if self.port else self.host,
             source=self.source,
             targets='/'.join(
                 [NotifySMPP.quote(t, safe='')
@@ -222,10 +234,21 @@ class NotifySMPP(NotifyBase):
         # error tracking (used for function return)
         has_error = False
 
-        client = smpplib.client.Client(self.host, self.port,
+        port = self.default_port if not self.secure \
-                                       allow_unknown_opt_params=True)
+            else self.default_secure_port
-        client.connect()
+
-        client.bind_transmitter(system_id=self.user, password=self.password)
+        client = smpplib.client.Client(
+            self.host, port, allow_unknown_opt_params=True)
+        try:
+            client.connect()
+            client.bind_transmitter(
+                system_id=self.user, password=self.password)
+
+        except smpplib.exceptions.ConnectionError as e:
+            self.logger.warning(
+                'Failed to establish connection to SMPP server {}: {}'.format(
+                    self.host, e))
+            return False
 
         for target in self.targets:
             parts, encoding, msg_type = smpplib.gsm.make_parts(body)
@@ -264,11 +287,40 @@ class NotifySMPP(NotifyBase):
         Parses the URL and returns enough arguments that can allow
         us to re-instantiate this object.
         """
-        results = NotifyBase.parse_url(url)
+        results = NotifyBase.parse_url(url, verify_host=False)
         if not results:
             # We're done early as we couldn't load the results
             return results
 
+        if not results:
+            # We're done early as we couldn't load the results
+            return results
+
+        # Support the 'from'  and 'source' variable so that we can support
+        # targets this way too.
+        # The 'from' makes it easier to use yaml configuration
+        if 'from' in results['qsd'] and len(results['qsd']['from']):
+            results['source'] = \
+                NotifySMPP.unquote(results['qsd']['from'])
+
+            # hostname will also be a target in this case
+            results['targets'] = [
+                *NotifySMPP.parse_phone_no(results['host']),
+                *NotifySMPP.split_path(results['fullpath'])]
+
+        else:
+            # store our source
+            results['source'] = NotifySMPP.unquote(results['host'])
+
+            # store targets
+            results['targets'] = NotifySMPP.split_path(results['fullpath'])
+
+        # Support the 'to' variable so that we can support targets this way too
+        # The 'to' makes it easier to use yaml configuration
+        if 'to' in results['qsd'] and len(results['qsd']['to']):
+            results['targets'] += \
+                NotifySMPP.parse_phone_no(results['qsd']['to'])
+
         results['targets'] = NotifySMPP.split_path(results['fullpath'])
 
         # Support the 'to' variable so that we can support targets this way too

test/test_plugin_smpp.py

@@ -28,7 +28,7 @@
 
 import logging
 import sys
-
+from unittest import mock
 import pytest
 from apprise import Apprise
 from apprise.plugins.smpp import NotifySMPP
@@ -39,72 +39,68 @@ logging.disable(logging.CRITICAL)
 # Our Testing URLs
 apprise_url_tests = (
     ('smpp://', {
-        'instance': None,
+        'instance': TypeError,
     }),
     ('smpp:///', {
-        'instance': None,
+        'instance': TypeError,
     }),
     ('smpp://@/', {
-        'instance': None,
+        'instance': TypeError,
     }),
     ('smpp://user@/', {
-        'instance': None,
+        'instance': TypeError,
     }),
     ('smpp://user:pass/', {
-        'instance': None,
+        'instance': TypeError,
     }),
     ('smpp://user:pass@/', {
-        'instance': None,
+        'instance': TypeError,
+    }),
+    ('smpp://user@hostname', {
+        'instance': TypeError,
     }),
     ('smpp://user:pass@host:/', {
-        'instance': None,
+        'instance': TypeError,
     }),
-    ('smpp://user:pass@host:port/', {
+    ('smpp://user:pass@host:2775/', {
-        'instance': None,
+        'instance': TypeError,
     }),
-    ('smpp://user:pass@host:port/{}/{}'.format('1' * 10, 'a' * 32), {
+    ('smpp://user:pass@host:2775/{}/{}'.format('1' * 10, 'a' * 32), {
         # valid everything but target numbers
         'instance': NotifySMPP,
         # We have no one to notify
         'notify_response': False,
     }),
-    ('smpp://user:pass@host:port/{}'.format('1' * 10), {
+    ('smpp://user:pass@host:2775/{}'.format('1' * 10), {
         # everything valid
         'instance': NotifySMPP,
         # We have no one to notify
         'notify_response': False,
     }),
-    ('smpp://user:pass@host:port/{}/{}'.format('1' * 10, '1' * 10), {
+    ('smpp://user:pass@host/{}/{}'.format('1' * 10, '1' * 10), {
         'instance': NotifySMPP,
     }),
-    ('smpp://_?&from={}&to={},{}'.format(
+    ('smpps://_?&from={}&to={},{}&user=user&password=pw'.format(
         '1' * 10, '1' * 10, '1' * 10), {
         # use get args to accomplish the same thing
         'instance': NotifySMPP,
     }),
-    ('smpp://user:pass@host:port/{}/{}'.format('1' * 10, '1' * 10), {
-        'instance': NotifySMPP,
-        # throw a bizarre code forcing us to fail to look it up
-        'response': False,
-        'requests_response_code': 999,
-    }),
-    ('smpp://user:pass@host:port/{}/{}'.format('1' * 10, '1' * 10), {
-        'instance': NotifySMPP,
-        # Throws a series of connection and transfer exceptions when this flag
-        # is set and tests that we gracefully handle them
-        'test_requests_exceptions': True,
-    }),
 )
 
 
 @pytest.mark.skipif(
-    'python-smpp' in sys.modules,
+    'smpplib' in sys.modules,
-    reason="Requires that python-smpp NOT be installed")
+    reason="Requires that smpplib NOT be installed")
-def test_plugin_fcm_cryptography_import_error():
+@mock.patch('smpplib.client.Client')
+def test_plugin_smpplib_import_error(mock_client):
     """
-    NotifySimplePush() python-smpp loading failure
+    NotifySMPP() smpplib loading failure
     """
 
+    mock_client.connect.return_value = True
+    mock_client.bind_transmitter.return_value = True
+    mock_client.send_message.return_value = True
+
     # Attempt to instantiate our object
     obj = Apprise.instantiate(
         'smpp://user:pass@host:port/{}/{}'.format('1' * 10, '1' * 10))
@@ -114,11 +110,16 @@ def test_plugin_fcm_cryptography_import_error():
 
 
 @pytest.mark.skipif(
-    'python-smpp' not in sys.modules, reason="Requires python-smpp")
+    'smpplib' not in sys.modules, reason="Requires smpplib")
-def test_plugin_smpp_urls():
+@mock.patch('smpplib.client.Client')
+def test_plugin_smpp_urls(mock_client):
     """
     NotifySMPP() Apprise URLs
     """
 
+    mock_client.connect.return_value = True
+    mock_client.bind_transmitter.return_value = True
+    mock_client.send_message.return_value = True
+
     # Run our general tests
     AppriseURLTester(tests=apprise_url_tests).run_all()