Skip to content

Commit

Permalink
Fixed Slack file upload deprecation (#1130)
Browse files Browse the repository at this point in the history
  • Loading branch information
@delenamalan
delenamalan authored May 28, 2024
1 parent d3c13fd commit da14b3a
Show file tree
Hide file tree
Showing 3 changed files with 231 additions and 167 deletions.
154 changes: 84 additions & 70 deletions apprise/plugins/slack.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -603,16 +603,17 @@ def send(self, body, title='', notify_type=NotifyType.INFO, attach=None,
channel if channel[0] == '#' \
else '#{}'.format(channel)

# Store the valid and massaged payload that is recognizable by
# slack. This list is used for sending attachments later.
attach_channel_list.append(payload['channel'])

response = self._send(url, payload)
if not response:
# Handle any error
has_error = True
continue

# Store the valid channel or chat ID (for DMs) that will
# be accepted by Slack's attachment method later.
if response.get('channel'):
attach_channel_list.append(response.get('channel'))

self.logger.info(
'Sent Slack notification{}.'.format(
' to {}'.format(channel)
Expand All @@ -635,21 +636,58 @@ def send(self, body, title='', notify_type=NotifyType.INFO, attach=None,
'Posting Slack attachment {}'.format(
attachment.url(privacy=True)))

# Prepare API Upload Payload
_payload = {
# Get the URL to which to upload the file.
# https://api.slack.com/methods/files.getUploadURLExternal
_params = {
'filename': attachment.name,
'channels': ','.join(attach_channel_list)
'length': len(attachment),
}

# Our URL
_url = self.api_url.format('files.upload')

response = self._send(_url, _payload, attach=attachment)
if not (response and response.get('file') and
response['file'].get('url_private')):
# We failed to post our attachments, take an early exit
_url = self.api_url.format('files.getUploadURLExternal')
response = self._send(
_url, {}, http_method='get', params=_params
)
if not (
response and response.get('file_id')
and response.get('upload_url')
):
self.logger.error('Could retrieve file upload URL.')
# We failed to get an upload URL, take an early exit
return False

file_id = response.get('file_id')
upload_url = response.get('upload_url')

# Upload file
response = self._send(upload_url, {}, attach=attachment)

# Send file to channels
# https://api.slack.com/methods/files.completeUploadExternal
for channel_id in attach_channel_list:
_payload = {
'files': [{
"id": file_id,
"title": attachment.name,
}],
'channel_id': channel_id
}
_url = self.api_url.format('files.completeUploadExternal')
response = self._send(_url, _payload)
# Expected response
# {
# "ok": true,
# "files": [
# {
# "id": "F123ABC456",
# "title": "slack-test"
# }
# ]
# }
if not (response and response.get('files')):
self.logger.error('Failed to send file to channel.')
# We failed to send the file to the channel,
# take an early exit
return False

return not has_error

def lookup_userid(self, email):
Expand Down Expand Up @@ -808,7 +846,8 @@ def lookup_userid(self, email):

return user_id

def _send(self, url, payload, attach=None, **kwargs):
def _send(self, url, payload, attach=None, http_method='post', params=None,
**kwargs):
"""
Wrapper to the requests (post) object
"""
Expand Down Expand Up @@ -842,13 +881,15 @@ def _send(self, url, payload, attach=None, **kwargs):
if attach:
files = {'file': (attach.name, open(attach.path, 'rb'))}

r = requests.post(
r = requests.request(
http_method,
url,
data=payload if attach else dumps(payload),
headers=headers,
files=files,
verify=self.verify_certificate,
timeout=self.request_timeout,
params=params if params else None,
)

# Posts return a JSON string
Expand All @@ -866,11 +907,22 @@ def _send(self, url, payload, attach=None, **kwargs):
# 'ok': False,
# 'error': 'not_in_channel',
# }
#
# The text 'ok' is returned if this is a Webhook request
# So the below captures that as well.
status_okay = (response and response.get('ok', False)) \
if self.mode is SlackMode.BOT else r.content == b'ok'
status_okay = False
if self.mode is SlackMode.BOT:
status_okay = (
(response and response.get('ok', False)) or
# Responses for file uploads look like this
# 'OK - <file length>'
(
r.content and
isinstance(r.content, bytes) and
b'OK' in r.content
)
)
elif r.content == b'ok':
# The text 'ok' is returned if this is a Webhook request
# So the below captures that as well.
status_okay = True

if r.status_code != requests.codes.ok or not status_okay:
# We had a problem
Expand All @@ -879,9 +931,9 @@ def _send(self, url, payload, attach=None, **kwargs):
r.status_code, SLACK_HTTP_ERROR_MAP)

self.logger.warning(
'Failed to send {}to Slack: '
'Failed to send{} to Slack: '
'{}{}error={}.'.format(
attach.name if attach else '',
(' ' + attach.name) if attach else '',
status_str,
', ' if status_str else '',
r.status_code))
Expand Down Expand Up @@ -913,53 +965,15 @@ def _send(self, url, payload, attach=None, **kwargs):
# "username": "Apprise"
# }

# File Attachment Responses look like this
# files.completeUploadExternal responses look like this:
# {
# "file": {
# "channels": [],
# "comments_count": 0,
# "created": 1573617523,
# "display_as_bot": false,
# "editable": false,
# "external_type": "",
# "filetype": "png",
# "groups": [],
# "has_rich_preview": false,
# "id": "FQJJLDAHM",
# "image_exif_rotation": 1,
# "ims": [],
# "is_external": false,
# "is_public": false,
# "is_starred": false,
# "mimetype": "image/png",
# "mode": "hosted",
# "name": "apprise-test.png",
# "original_h": 640,
# "original_w": 640,
# "permalink": "https://{name}.slack.com/files/...
# "permalink_public": "https://slack-files.com/...
# "pretty_type": "PNG",
# "public_url_shared": false,
# "shares": {},
# "size": 238810,
# "thumb_160": "https://files.slack.com/files-tmb/...
# "thumb_360": "https://files.slack.com/files-tmb/...
# "thumb_360_h": 360,
# "thumb_360_w": 360,
# "thumb_480": "https://files.slack.com/files-tmb/...
# "thumb_480_h": 480,
# "thumb_480_w": 480,
# "thumb_64": "https://files.slack.com/files-tmb/...
# "thumb_80": "https://files.slack.com/files-tmb/...
# "thumb_tiny": abcd...
# "timestamp": 1573617523,
# "title": "apprise-test",
# "url_private": "https://files.slack.com/files-pri/...
# "url_private_download": "https://files.slack.com/files-...
# "user": "UADKLLMJT",
# "username": ""
# },
# "ok": true
# "ok": true,
# "files": [
# {
# "id": "F123ABC456",
# "title": "slack-test"
# }
# ]
# }
except requests.RequestException as e:
self.logger.warning(
Expand Down
28 changes: 25 additions & 3 deletions test/helpers/rest.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ def run_all(self):

@mock.patch('requests.get')
@mock.patch('requests.post')
def run(self, url, meta, mock_post, mock_get):
@mock.patch('requests.request')
def run(self, url, meta, mock_request, mock_post, mock_get):
"""
Run a specific test
"""
Expand Down Expand Up @@ -150,6 +151,7 @@ def run(self, url, meta, mock_post, mock_get):
robj.content = u''
mock_get.return_value = robj
mock_post.return_value = robj
mock_request.return_value = robj

try:
# We can now instantiate our object:
Expand Down Expand Up @@ -276,8 +278,21 @@ def run(self, url, meta, mock_post, mock_get):
@mock.patch('requests.put')
@mock.patch('requests.delete')
@mock.patch('requests.patch')
def __notify(self, url, obj, meta, asset, mock_patch, mock_del, mock_put,
mock_head, mock_post, mock_get):
@mock.patch('requests.request')
def __notify(
self,
url,
obj,
meta,
asset,
mock_request,
mock_patch,
mock_del,
mock_put,
mock_head,
mock_post,
mock_get
):
"""
Perform notification testing against object specified
"""
Expand Down Expand Up @@ -342,6 +357,7 @@ def __notify(self, url, obj, meta, asset, mock_patch, mock_del, mock_put,
mock_patch.return_value = robj
mock_del.return_value = robj
mock_put.return_value = robj
mock_request.return_value = robj

if test_requests_exceptions is False:
# Handle our default response
Expand All @@ -351,6 +367,7 @@ def __notify(self, url, obj, meta, asset, mock_patch, mock_del, mock_put,
mock_post.return_value.status_code = requests_response_code
mock_get.return_value.status_code = requests_response_code
mock_patch.return_value.status_code = requests_response_code
mock_request.return_value.status_code = requests_response_code

# Handle our default text response
mock_get.return_value.content = requests_response_content
Expand All @@ -359,13 +376,15 @@ def __notify(self, url, obj, meta, asset, mock_patch, mock_del, mock_put,
mock_put.return_value.content = requests_response_content
mock_head.return_value.content = requests_response_content
mock_patch.return_value.content = requests_response_content
mock_request.return_value.content = requests_response_content

mock_get.return_value.text = requests_response_text
mock_post.return_value.text = requests_response_text
mock_put.return_value.text = requests_response_text
mock_del.return_value.text = requests_response_text
mock_head.return_value.text = requests_response_text
mock_patch.return_value.text = requests_response_text
mock_request.return_value.text = requests_response_text

# Ensure there is no side effect set
mock_post.side_effect = None
Expand All @@ -374,6 +393,7 @@ def __notify(self, url, obj, meta, asset, mock_patch, mock_del, mock_put,
mock_head.side_effect = None
mock_get.side_effect = None
mock_patch.side_effect = None
mock_request.side_effect = None

else:
# Handle exception testing; first we turn the boolean flag
Expand Down Expand Up @@ -532,6 +552,7 @@ def __notify(self, url, obj, meta, asset, mock_patch, mock_del, mock_put,
mock_put.side_effect = _exception
mock_get.side_effect = _exception
mock_patch.side_effect = _exception
mock_request.side_effect = _exception

try:
assert obj.notify(
Expand Down Expand Up @@ -577,6 +598,7 @@ def __notify(self, url, obj, meta, asset, mock_patch, mock_del, mock_put,
mock_head.side_effect = _exception
mock_get.side_effect = _exception
mock_patch.side_effect = _exception
mock_request.side_effect = _exception

try:
assert obj.notify(
Expand Down
Loading

0 comments on commit da14b3a

Please sign in to comment.