diff --git a/apprise/plugins/NotifyPushover.py b/apprise/plugins/NotifyPushover.py index 922ead25..18a6c341 100644 --- a/apprise/plugins/NotifyPushover.py +++ b/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[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,80 +361,67 @@ class NotifyPushover(NotifyBase): Perform Pushover Notification """ - # error tracking (used for function return) - has_error = False + if not self.targets: + # There were no services to notify + self.logger.warning( + 'There were no Pushover targets to notify.') + return False - # Create a copy of the devices list - devices = list(self.targets) - while len(devices): - device = devices.pop(0) + # prepare JSON Object + payload = { + 'token': self.token, + 'user': self.user_key, + 'priority': str(self.priority), + 'title': title if title else self.app_desc, + 'message': body, + 'device': ','.join(self.targets), + 'sound': self.sound, + } - if VALIDATE_DEVICE.match(device) is None: - self.logger.warning( - 'The device specified (%s) is invalid.' % device, - ) + if self.supplemental_url: + payload['url'] = self.supplemental_url - # Mark our failure - has_error = True - continue - - # prepare JSON Object - payload = { - 'token': self.token, - 'user': self.user_key, - 'priority': str(self.priority), - 'title': title if title else self.app_desc, - 'message': body, - 'device': device, - '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) - payload['html'] = 1 - - if self.priority == PushoverPriority.EMERGENCY: - payload.update({'retry': self.retry, 'expire': self.expire}) - - if attach and self.attachment_support: - # Create a copy of our payload - _payload = payload.copy() - - # Send with attachments - for no, attachment in enumerate(attach): - if no or not body: - # To handle multiple attachments, clean up our message - _payload['message'] = attachment.name - - if not self._send(_payload, attachment): - # Mark our failure - has_error = True - # clean exit from our attachment loop - break - - # Clear our title if previously set - _payload['title'] = '' - - # No need to alarm for each consecutive attachment uploaded - # afterwards - _payload['sound'] = PushoverSound.NONE + if self.supplemental_url_title: + payload['url_title'] = self.supplemental_url_title - else: - # Simple send - if not self._send(payload): + 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) + payload['html'] = 1 + + if self.priority == PushoverPriority.EMERGENCY: + payload.update({'retry': self.retry, 'expire': self.expire}) + + if attach and self.attachment_support: + # Create a copy of our payload + _payload = payload.copy() + + # Send with attachments + for no, attachment in enumerate(attach): + if no or not body: + # To handle multiple attachments, clean up our message + _payload['message'] = attachment.name + + if not self._send(_payload, attachment): # Mark our failure - has_error = True + return False + + # Clear our title if previously set + _payload['title'] = '' - return not has_error + # No need to alarm for each consecutive attachment uploaded + # afterwards + _payload['sound'] = PushoverSound.NONE + + else: + # Simple send + return self._send(payload) + + 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): """ diff --git a/test/test_plugin_pushover.py b/test/test_plugin_pushover.py index 667fe683..737f9535 100644 --- a/test/test_plugin_pushover.py +++ b/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