Attachment consistency across supported notification services (#194)

* some test cleanup

* Attachment handling more consistent across plugins

apprise/plugins/NotifyDiscord.py

@@ -51,6 +51,7 @@ from ..common import NotifyType
 from ..utils import parse_bool
 from ..utils import validate_regex
 from ..AppriseLocale import gettext_lazy as _
+from ..attachment.AttachBase import AttachBase
 
 
 class NotifyDiscord(NotifyBase):
@@ -312,6 +313,19 @@ class NotifyDiscord(NotifyBase):
         # Always call throttle before any remote server i/o is made
         self.throttle()
 
+        # Perform some simple error checking
+        if isinstance(attach, AttachBase):
+            if not attach:
+                # We could not access the attachment
+                self.logger.error(
+                    'Could not access attachment {}.'.format(
+                        attach.url(privacy=True)))
+                return False
+
+            self.logger.debug(
+                'Posting Discord attachment {}'.format(
+                    attach.url(privacy=True)))
+
         # Our attachment path (if specified)
         files = None
         try:

apprise/plugins/NotifyEmail.py

@@ -573,21 +573,22 @@ class NotifyEmail(NotifyBase):
                 # First attach our body to our content as the first element
                 base.attach(content)
 
-                attach_error = False
-
                 # Now store our attachments
                 for attachment in attach:
                     if not attachment:
                         # We could not load the attachment; take an early
                         # exit since this isn't what the end user wanted
 
-                        self.logger.warning(
+                        # We could not access the attachment
-                            'The specified attachment could not be referenced:'
+                        self.logger.error(
-                            ' {}.'.format(attachment.url(privacy=True)))
+                            'Could not access attachment {}.'.format(
+                                attachment.url(privacy=True)))
 
-                        # Mark our failure
+                        return False
-                        attach_error = True
+
-                        break
+                    self.logger.debug(
+                        'Preparing Email attachment {}'.format(
+                            attachment.url(privacy=True)))
 
                     with open(attachment.path, "rb") as abody:
                         app = MIMEApplication(
@@ -600,11 +601,6 @@ class NotifyEmail(NotifyBase):
 
                         base.attach(app)
 
-                if attach_error:
-                    # Mark our error and quit early
-                    has_error = True
-                    break
-
             # bind the socket variable to the current namespace
             socket = None
 

apprise/plugins/NotifyPushBullet.py

@@ -147,6 +147,19 @@ class NotifyPushBullet(NotifyBase):
             # We need to upload our payload first so that we can source it
             # in remaining messages
             for attachment in attach:
+
+                # Perform some simple error checking
+                if not attachment:
+                    # We could not access the attachment
+                    self.logger.error(
+                        'Could not access attachment {}.'.format(
+                            attachment.url(privacy=True)))
+                    return False
+
+                self.logger.debug(
+                    'Preparing PushBullet attachment {}'.format(
+                        attachment.url(privacy=True)))
+
                 # prepare payload
                 payload = {
                     'file_name': attachment.name,
@@ -253,7 +266,7 @@ class NotifyPushBullet(NotifyBase):
                     continue
 
                 self.logger.info(
-                    'Sent PushBullet attachment (%s) to "%s".' % (
+                    'Sent PushBullet attachment ({}) to "{}".'.format(
                         attach_payload['file_name'], recipient))
 
         return not has_error

apprise/plugins/NotifyPushSafer.py

@@ -548,15 +548,22 @@ class NotifyPushSafer(NotifyBase):
                 # prepare payload
                 if not attachment:
                     # We could not access the attachment
-                    self.logger.warning(
+                    self.logger.error(
-                        'Could not access {}.'.format(
+                        'Could not access attachment {}.'.format(
                             attachment.url(privacy=True)))
                     return False
 
                 if not attachment.mimetype.startswith('image/'):
                     # Attachment not supported; continue peacefully
+                    self.logger.debug(
+                        'Ignoring unsupported PushSafer attachment {}.'.format(
+                            attachment.url(privacy=True)))
                     continue
 
+                self.logger.debug(
+                    'Posting PushSafer attachment {}'.format(
+                        attachment.url(privacy=True)))
+
                 try:
                     with io.open(attachment.path, 'rb') as f:
                         # Output must be in a DataURL format (that's what

apprise/plugins/NotifyPushover.py

@@ -364,8 +364,8 @@ class NotifyPushover(NotifyBase):
             # Perform some simple error checking
             if not attach:
                 # We could not access the attachment
-                self.logger.warning(
+                self.logger.error(
-                    'Could not access {}.'.format(
+                    'Could not access attachment {}.'.format(
                         attach.url(privacy=True)))
                 return False
 
@@ -398,6 +398,10 @@ class NotifyPushover(NotifyBase):
 
                 return False
 
+            self.logger.debug(
+                'Posting Pushover attachment {}'.format(
+                    attach.url(privacy=True)))
+
         # Default Header
         headers = {
             'User-Agent': self.app_id,

apprise/plugins/NotifySlack.py

@@ -435,8 +435,18 @@ class NotifySlack(NotifyBase):
         if attach and self.mode is SlackMode.BOT and attach_channel_list:
             # Send our attachments (can only be done in bot mode)
             for attachment in attach:
-                self.logger.info(
+
-                    'Posting Slack Attachment {}'.format(attachment.name))
+                # Perform some simple error checking
+                if not attachment:
+                    # We could not access the attachment
+                    self.logger.error(
+                        'Could not access attachment {}.'.format(
+                            attachment.url(privacy=True)))
+                    return False
+
+                self.logger.debug(
+                    'Posting Slack attachment {}'.format(
+                        attachment.url(privacy=True)))
 
                 # Prepare API Upload Payload
                 _payload = {

apprise/plugins/NotifyTelegram.py

@@ -267,18 +267,22 @@ class NotifyTelegram(NotifyBase):
         path = None
 
         if isinstance(attach, AttachBase):
+            if not attach:
+                # We could not access the attachment
+                self.logger.error(
+                    'Could not access attachment {}.'.format(
+                        attach.url(privacy=True)))
+                return False
+
+            self.logger.debug(
+                'Posting Telegram attachment {}'.format(
+                    attach.url(privacy=True)))
+
             # Store our path to our file
             path = attach.path
             file_name = attach.name
             mimetype = attach.mimetype
 
-            if not path:
-                # We could not access the attachment
-                self.logger.warning(
-                    'Could not access {}.'.format(
-                        attach.url(privacy=True)))
-                return False
-
             # Process our attachment
             function_name, key = \
                 next(((x['function_name'], x['key']) for x in self.mime_lookup
@@ -642,10 +646,10 @@ class NotifyTelegram(NotifyBase):
             if attach:
                 # Send our attachments now (if specified and if it exists)
                 for attachment in attach:
-                    sent_attachment = self.send_media(
+                    if not self.send_media(
-                        payload['chat_id'], notify_type, attach=attachment)
+                            payload['chat_id'], notify_type,
+                            attach=attachment):
 
-                    if not sent_attachment:
                         # We failed; don't continue
                         has_error = True
                         break

test/test_discord_plugin.py

@@ -174,11 +174,8 @@ def test_discord_attachments(mock_post):
     webhook_token = 'D' * 64
 
     # Prepare Mock return object
-    response = mock.Mock()
+    mock_post.return_value = requests.Request()
-    response.status_code = requests.codes.ok
+    mock_post.return_value.status_code = requests.codes.ok
-
-    # Throw an exception on the second call to requests.post()
-    mock_post.side_effect = [response, OSError()]
 
     # Test our markdown
     obj = Apprise.instantiate(
@@ -187,5 +184,26 @@ def test_discord_attachments(mock_post):
     # attach our content
     attach = AppriseAttachment(os.path.join(TEST_VAR_DIR, 'apprise-test.gif'))
 
+    assert obj.notify(
+        body='body', title='title', notify_type=NotifyType.INFO,
+        attach=attach) is True
+
+    # An invalid attachment will cause a failure
+    path = os.path.join(TEST_VAR_DIR, '/invalid/path/to/an/invalid/file.jpg')
+    attach = AppriseAttachment(path)
+    assert obj.notify(
+        body='body', title='title', notify_type=NotifyType.INFO,
+        attach=path) is False
+
+    # Throw an exception on the second call to requests.post()
+    mock_post.return_value = None
+    response = mock.Mock()
+    response.status_code = requests.codes.ok
+    mock_post.side_effect = [response, OSError()]
+
+    # update our attachment to be valid
+    attach = AppriseAttachment(os.path.join(TEST_VAR_DIR, 'apprise-test.gif'))
+    # Test our markdown
+
     # We'll fail now because of an internal exception
     assert obj.send(body="test", attach=attach) is False

test/test_pushbullet.py

@@ -59,6 +59,7 @@ def test_pushbullet_attachments(mock_post):
         "upload_url": "https://upload.pushbullet.com/abcd123",
     })
     response.status_code = requests.codes.ok
+    mock_post.return_value = response
 
     # prepare our attachment
     attach = AppriseAttachment(os.path.join(TEST_VAR_DIR, 'apprise-test.gif'))
@@ -67,7 +68,23 @@ def test_pushbullet_attachments(mock_post):
     obj = Apprise.instantiate(
         'pbul://{}/?format=markdown'.format(access_token))
 
+    # Send a good attachment
+    assert obj.notify(body="test", attach=attach) is True
+
+    # Add another attachment so we drop into the area of the PushBullet code
+    # that sends remaining attachments (if more detected)
+    attach.add(os.path.join(TEST_VAR_DIR, 'apprise-test.gif'))
+
+    # Send our attachments
+    assert obj.notify(body="test", attach=attach) is True
+
+    # An invalid attachment will cause a failure
+    path = os.path.join(TEST_VAR_DIR, '/invalid/path/to/an/invalid/file.jpg')
+    attach = AppriseAttachment(path)
+    assert obj.notify(body="test", attach=attach) is False
+
     # Throw an exception on the first call to requests.post()
+    mock_post.return_value = None
     mock_post.side_effect = requests.RequestException()
 
     # We'll fail now because of an internal exception
@@ -99,3 +116,47 @@ def test_pushbullet_attachments(mock_post):
 
     # We'll fail because of an invalid json object
     assert obj.send(body="test", attach=attach) is False
+
+    #
+    # Test bad responses
+    #
+
+    # Prepare a bad response
+    response.content = dumps({
+        "file_name": "cat.jpg",
+        "file_type": "image/jpeg",
+        "file_url": "https://dl.pushb.com/abc/cat.jpg",
+        "upload_url": "https://upload.pushbullet.com/abcd123",
+    })
+    bad_response = mock.Mock()
+    bad_response.content = response.content
+    bad_response.status_code = 400
+
+    # Throw an exception on the third call to requests.post()
+    mock_post.return_value = bad_response
+
+    # prepare our attachment
+    attach = AppriseAttachment(os.path.join(TEST_VAR_DIR, 'apprise-test.gif'))
+
+    # We'll fail now because we were unable to send the attachment
+    assert obj.send(body="test", attach=attach) is False
+
+    # Throw an exception on the second call
+    mock_post.side_effect = [response, bad_response, response]
+    assert obj.send(body="test", attach=attach) is False
+
+    # Throw an OSError
+    mock_post.side_effect = [response, OSError()]
+    assert obj.send(body="test", attach=attach) is False
+
+    # Throw an exception on the third call
+    mock_post.side_effect = [response, response, bad_response]
+    assert obj.send(body="test", attach=attach) is False
+
+    # Throw an exception on the fourth call
+    mock_post.side_effect = [response, response, response, bad_response]
+    assert obj.send(body="test", attach=attach) is False
+
+    # A good message
+    mock_post.side_effect = [response, response, response, response]
+    assert obj.send(body="test", attach=attach) is True

test/test_pushsafer.py

@@ -40,7 +40,7 @@ TEST_VAR_DIR = os.path.join(os.path.dirname(__file__), 'var')
 
 
 @mock.patch('requests.post')
-def test_notify_pushsafer_plugin(mock_post, tmpdir):
+def test_notify_pushsafer_plugin(mock_post):
     """
     API: NotifyPushSafer() Tests
 

test/test_rest_plugins.py

@@ -4210,11 +4210,11 @@ def test_rest_plugins(mock_post, mock_get):
                 # Check that we were expecting this exception to happen
                 try:
                     if not isinstance(e, response):
-                        raise
+                        raise e
 
                 except TypeError:
                     print('%s Unhandled response %s' % (url, type(e)))
-                    raise
+                    raise e
 
             #
             # Stage 2: without title defined
@@ -4250,7 +4250,7 @@ def test_rest_plugins(mock_post, mock_get):
             except Exception as e:
                 # Check that we were expecting this exception to happen
                 if not isinstance(e, response):
-                    raise
+                    raise e
 
             # Tidy our object and allow any possible defined deconstructors to
             # be executed.
@@ -4265,11 +4265,11 @@ def test_rest_plugins(mock_post, mock_get):
             # Handle our exception
             if instance is None:
                 print('%s %s' % (url, str(e)))
-                raise
+                raise e
 
             if not isinstance(e, instance):
                 print('%s %s' % (url, str(e)))
-                raise
+                raise e
 
 
 @mock.patch('requests.get')

test/test_slack_plugin.py

@@ -22,12 +22,14 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 # THE SOFTWARE.
 
+import os
 import six
 import mock
 import pytest
 import requests
 from apprise import plugins
 from apprise import NotifyType
+from apprise import AppriseAttachment
 
 from json import dumps
 
@@ -35,6 +37,9 @@ from json import dumps
 import logging
 logging.disable(logging.CRITICAL)
 
+# Attachment Directory
+TEST_VAR_DIR = os.path.join(os.path.dirname(__file__), 'var')
+
 
 @mock.patch('requests.post')
 def test_slack_oauth_access_token(mock_post):
@@ -52,6 +57,11 @@ def test_slack_oauth_access_token(mock_post):
     request.content = dumps({
         'ok': True,
         'message': '',
+
+        # Attachment support
+        'file': {
+            'url_private': 'http://localhost',
+        }
     })
     request.status_code = requests.codes.ok
 
@@ -73,6 +83,34 @@ def test_slack_oauth_access_token(mock_post):
     # apprise room was found
     assert obj.send(body="test") is True
 
+    # Test Valid Attachment
+    path = os.path.join(TEST_VAR_DIR, 'apprise-test.gif')
+    attach = AppriseAttachment(path)
+    assert obj.notify(
+        body='body', title='title', notify_type=NotifyType.INFO,
+        attach=attach) is True
+
+    # Test invalid attachment
+    path = os.path.join(TEST_VAR_DIR, '/invalid/path/to/an/invalid/file.jpg')
+    assert obj.notify(
+        body='body', title='title', notify_type=NotifyType.INFO,
+        attach=path) is False
+
+    # Test case where expected return attachment payload is invalid
+    request.content = dumps({
+        'ok': True,
+        'message': '',
+
+        # Attachment support
+        'file': None
+    })
+    path = os.path.join(TEST_VAR_DIR, 'apprise-test.gif')
+    attach = AppriseAttachment(path)
+    # We'll fail because of the bad 'file' response
+    assert obj.notify(
+        body='body', title='title', notify_type=NotifyType.INFO,
+        attach=attach) is False
+
     # Slack requests pay close attention to the response to determine
     # if things go well... this is not a good JSON response:
     request.content = '{'

test/test_telegram.py

@@ -43,7 +43,7 @@ TEST_VAR_DIR = os.path.join(os.path.dirname(__file__), 'var')
 
 @mock.patch('requests.get')
 @mock.patch('requests.post')
-def test_notify_telegram_plugin(mock_post, mock_get, tmpdir):
+def test_notify_telegram_plugin(mock_post, mock_get):
     """
     API: NotifyTelegram() Tests
 
@@ -205,13 +205,14 @@ def test_notify_telegram_plugin(mock_post, mock_get, tmpdir):
     assert len(obj.targets) == 1
     assert obj.targets[0] == '12345'
 
-    path = os.path.join(TEST_VAR_DIR, 'apprise-test.gif')
+    attach = AppriseAttachment(os.path.join(TEST_VAR_DIR, 'apprise-test.gif'))
-    attach = AppriseAttachment(path)
     assert obj.notify(
         body='body', title='title', notify_type=NotifyType.INFO,
         attach=attach) is True
 
+    # An invalid attachment will cause a failure
     path = os.path.join(TEST_VAR_DIR, '/invalid/path/to/an/invalid/file.jpg')
+    attach = AppriseAttachment(path)
     assert obj.notify(
         body='body', title='title', notify_type=NotifyType.INFO,
         attach=path) is False