Refactored Pushover to better handle devices (less remote queries) (#964)

apprise/plugins/NotifyPushover.py

@@ -32,6 +32,7 @@
 
 import re
 import requests
+from itertools import chain
 
 from .NotifyBase import NotifyBase
 from ..common import NotifyType
@@ -46,7 +47,7 @@ from ..attachment.AttachBase import AttachBase
 PUSHOVER_SEND_TO_ALL = 'ALL_DEVICES'
 
 # Used to detect a Device
-VALIDATE_DEVICE = re.compile(r'^[a-z0-9_]{1,25}$', re.I)
+VALIDATE_DEVICE = re.compile(r'^\s*(?P<device>[a-z0-9_-]{1,25})\s*$', re.I)
 
 
 # Priorities
@@ -204,7 +205,7 @@ class NotifyPushover(NotifyBase):
         'target_device': {
             'name': _('Target Device'),
             'type': 'string',
-            'regex': (r'^[a-z0-9_]{1,25}$', 'i'),
+            'regex': (r'^[a-z0-9_-]{1,25}$', 'i'),
             'map_to': 'targets',
         },
         'targets': {
@@ -279,10 +280,30 @@ class NotifyPushover(NotifyBase):
             self.logger.warning(msg)
             raise TypeError(msg)
 
-        self.targets = parse_list(targets)
-        if len(self.targets) == 0:
+        # Track our valid devices
+        targets = parse_list(targets)
+
+        # Track any invalid entries
+        self.invalid_targets = list()
+
+        if len(targets) == 0:
             self.targets = (PUSHOVER_SEND_TO_ALL, )
 
+        else:
+            self.targets = []
+            for target in targets:
+                result = VALIDATE_DEVICE.match(target)
+                if result:
+                    # Store device information
+                    self.targets.append(result.group('device'))
+                    continue
+
+                self.logger.warning(
+                    'Dropped invalid Pushover device '
+                    '({}) specified.'.format(target),
+                )
+                self.invalid_targets.append(target)
+
         # Setup supplemental url
         self.supplemental_url = supplemental_url
         self.supplemental_url_title = supplemental_url_title
@@ -340,22 +361,11 @@ class NotifyPushover(NotifyBase):
         Perform Pushover Notification
         """
 
-        # error tracking (used for function return)
-        has_error = False
-
-        # Create a copy of the devices list
-        devices = list(self.targets)
-        while len(devices):
-            device = devices.pop(0)
-
-            if VALIDATE_DEVICE.match(device) is None:
+        if not self.targets:
+            # There were no services to notify
             self.logger.warning(
-                    'The device specified (%s) is invalid.' % device,
-                )
-
-                # Mark our failure
-                has_error = True
-                continue
+                'There were no Pushover targets to notify.')
+            return False
 
         # prepare JSON Object
         payload = {
@@ -364,18 +374,20 @@ class NotifyPushover(NotifyBase):
             'priority': str(self.priority),
             'title': title if title else self.app_desc,
             'message': body,
-                'device': device,
+            'device': ','.join(self.targets),
             'sound': self.sound,
         }
 
         if self.supplemental_url:
             payload['url'] = self.supplemental_url
+
         if self.supplemental_url_title:
             payload['url_title'] = self.supplemental_url_title
 
         if self.notify_format == NotifyFormat.HTML:
             # https://pushover.net/api#html
             payload['html'] = 1
+
         elif self.notify_format == NotifyFormat.MARKDOWN:
             payload['message'] = convert_between(
                 NotifyFormat.MARKDOWN, NotifyFormat.HTML, body)
@@ -396,9 +408,7 @@ class NotifyPushover(NotifyBase):
 
                 if not self._send(_payload, attachment):
                     # Mark our failure
-                        has_error = True
-                        # clean exit from our attachment loop
-                        break
+                    return False
 
                 # Clear our title if previously set
                 _payload['title'] = ''
@@ -409,11 +419,9 @@ class NotifyPushover(NotifyBase):
 
         else:
             # Simple send
-                if not self._send(payload):
-                    # Mark our failure
-                    has_error = True
+            return self._send(payload)
 
-        return not has_error
+        return True
 
     def _send(self, payload, attach=None):
         """
@@ -567,8 +575,9 @@ class NotifyPushover(NotifyBase):
         params.update(self.url_parameters(privacy=privacy, *args, **kwargs))
 
         # Escape our devices
-        devices = '/'.join([NotifyPushover.quote(x, safe='')
-                            for x in self.targets])
+        devices = '/'.join(
+            [NotifyPushover.quote(x, safe='')
+             for x in chain(self.targets, self.invalid_targets)])
 
         if devices == PUSHOVER_SEND_TO_ALL:
             # keyword is reserved for internal usage only; it's safe to remove
@@ -582,12 +591,6 @@ class NotifyPushover(NotifyBase):
             devices=devices,
             params=NotifyPushover.urlencode(params))
 
-    def __len__(self):
-        """
-        Returns the number of targets associated with this notification
-        """
-        return len(self.targets)
-
     @staticmethod
     def parse_url(url):
         """

test/test_plugin_pushover.py

@@ -87,7 +87,7 @@ apprise_url_tests = (
         'instance': NotifyPushover,
     }),
     # API Key + Valid User + 2 Devices
-    ('pover://%s@%s/DEVICE1/DEVICE2/' % ('u' * 30, 'a' * 30), {
+    ('pover://%s@%s/DEVICE1/Device-with-dash/' % ('u' * 30, 'a' * 30), {
         'instance': NotifyPushover,
 
         # Our expected url(privacy=True) startswith() response:
@@ -345,12 +345,13 @@ def test_plugin_pushover_edge_cases(mock_post):
     obj = NotifyPushover(
         user_key=user_key, token=token, targets=devices)
     assert isinstance(obj, NotifyPushover) is True
-    assert len(obj.targets) == 3
+    # Our invalid device is ignored
+    assert len(obj.targets) == 2
 
-    # This call fails because there is 1 invalid device
+    # We notify the 2 devices loaded
     assert obj.notify(
         body='body', title='title',
-        notify_type=apprise.NotifyType.INFO) is False
+        notify_type=apprise.NotifyType.INFO) is True
 
     obj = NotifyPushover(user_key=user_key, token=token)
     assert isinstance(obj, NotifyPushover) is True
@@ -446,4 +447,9 @@ def test_plugin_pushover_config_files(mock_post):
         PushoverPriority.NORMAL
 
     # Notifications work
+    # We test 'pushover_str_int' and 'low' which only matches 1 end point
+    assert aobj.notify(
+        title="title", body="body", tag=[('pushover_str_int', 'low')]) is True
+
+    # Notify everything loaded
     assert aobj.notify(title="title", body="body") is True