Fix matrix v3 attachments

apprise/plugins/matrix.py

@@ -81,6 +81,9 @@ class MatrixDiscoveryException(AppriseException):
     re.I,
 )
 
+# Matrix is_image check
+IS_IMAGE = re.compile(r"^image/.*", re.I)
+
 
 class MatrixMessageType:
     """The Matrix Message types."""
@@ -692,6 +695,9 @@ def _send_server_notification(
             # Get our room
             room = rooms.pop(0)
 
+            # Set method according to MatrixVersion
+            method = "PUT" if self.version == MatrixVersion.V3 else "POST"
+
             # Get our room_id from our response
             room_id = self._room_join(room)
             if not room_id:
@@ -717,40 +723,48 @@ def _send_server_notification(
                     f"/rooms/{NotifyMatrix.quote(room_id)}/send/m.room.message"
                 )
 
-            if self.version == MatrixVersion.V2:
-                #
-                # Attachments don't work beyond V2 at this time
-                #
-                if image_url:
-                    # Define our payload
-                    image_payload = {
-                        "msgtype": "m.image",
-                        "url": image_url,
-                        "body": f"{title if title else notify_type}",
-                    }
-
-                    # Post our content
+            if image_url and self.version == MatrixVersion.V2:
+                # Define our payload
+                image_payload = {
+                    "msgtype": "m.image",
+                    "url": image_url,
+                    "body": f"{title if title else notify_type}",
+                }
+
+                # Post our content
+                postokay, response = self._fetch(
+                    path, payload=image_payload)
+                if not postokay:
+                    # Mark our failure
+                    has_error = True
+                    continue
+
+            if attachments:
+                for attachment in attachments:
+                    attachment["room_id"] = room_id
+                    attachment["type"] = "m.room.message"
+
                     postokay, response = self._fetch(
-                        path, payload=image_payload
-                    )
+                        path, payload=attachment, method=method)
+
+                    # Increment the transaction ID to avoid future messages
+                    # being recognized as retransmissions and ignored
+                    if self.version == MatrixVersion.V3 \
+                       and self.access_token != self.password:
+                        self.transaction_id += 1
+                        self.store.set(
+                            "transaction_id", self.transaction_id,
+                            expires=self.default_cache_expiry_sec)
+                        path = "/rooms/{}/send/m.room.message/{}".format(
+                            NotifyMatrix.quote(room_id),
+                            self.transaction_id,
+                        )
+
                     if not postokay:
                         # Mark our failure
                         has_error = True
                         continue
 
-                if attachments:
-                    for attachment in attachments:
-                        attachment["room_id"] = room_id
-                        attachment["type"] = "m.room.message"
-
-                        postokay, response = self._fetch(
-                            path, payload=attachment
-                        )
-                        if not postokay:
-                            # Mark our failure
-                            has_error = True
-                            continue
-
             # Define our payload
             payload = {
                 "msgtype": f"m.{self.msgtype}",
@@ -790,7 +804,6 @@ def _send_server_notification(
                 })
 
             # Post our content
-            method = "PUT" if self.version == MatrixVersion.V3 else "POST"
             postokay, response = self._fetch(
                 path, payload=payload, method=method
             )
@@ -824,18 +837,14 @@ def _send_attachments(self, attach):
         """Posts all of the provided attachments."""
 
         payloads = []
-        if self.version != MatrixVersion.V2:
-            self.logger.warning(
-                "Add ?v=2 to Apprise URL to support Attachments"
-            )
-            return next((False for a in attach if not a), [])
 
         for attachment in attach:
             if not attachment:
                 # invalid attachment (bad file)
                 return False
 
-            if not re.match(r"^image/", attachment.mimetype, re.I):
+            if not IS_IMAGE.match(attachment.mimetype) \
+               and self.version == MatrixVersion.V2:
                 # unsuppored at this time
                 continue
 
@@ -849,38 +858,32 @@ def _send_attachments(self, attach):
             #     "content_uri": "mxc://example.com/a-unique-key"
             # }
 
-            # FUTURE if self.version == MatrixVersion.V3:
-            # FUTURE     # Prepare our payload
-            # FUTURE     payloads.append({
-            # FUTURE         "body": attachment.name,
-            # FUTURE         "info": {
-            # FUTURE             "mimetype": attachment.mimetype,
-            # FUTURE             "size": len(attachment),
-            # FUTURE         },
-            # FUTURE         "msgtype": "m.image",
-            # FUTURE         "url": response.get('content_uri'),
-            # FUTURE     })
-
-            # FUTURE else:
-            # FUTURE     # Prepare our payload
-            # FUTURE     payloads.append({
-            # FUTURE         "info": {
-            # FUTURE             "mimetype": attachment.mimetype,
-            # FUTURE         },
-            # FUTURE         "msgtype": "m.image",
-            # FUTURE         "body": "tta.webp",
-            # FUTURE         "url": response.get('content_uri'),
-            # FUTURE     })
-
-            # Prepare our payload
-            payloads.append({
-                "info": {
-                    "mimetype": attachment.mimetype,
-                },
-                "msgtype": "m.image",
-                "body": "tta.webp",
-                "url": response.get("content_uri"),
-            })
+            if self.version == MatrixVersion.V3:
+                # Prepare our payload
+                is_image = IS_IMAGE.match(attachment.mimetype)
+                payloads.append({
+                    "body": attachment.name,
+                    "info": {
+                        "mimetype": attachment.mimetype,
+                        "size": len(attachment),
+                    },
+                    "msgtype": "m.image" if is_image else "m.file",
+                    "url": response.get("content_uri"),
+                })
+                if not is_image:
+                    # Setup `m.file'
+                    payloads[-1]["filename"] = attachment.name
+
+            else:
+                # Prepare our payload
+                payloads.append({
+                    "info": {
+                        "mimetype": attachment.mimetype,
+                    },
+                    "msgtype": "m.image",
+                    "body": "tta.webp",
+                    "url": response.get("content_uri"),
+                })
 
         return payloads
 
@@ -1335,12 +1338,11 @@ def _fetch(
         status_code = requests.codes.internal_server_error
 
         if path == "/upload":
-            # FUTURE if self.version == MatrixVersion.V3:
-            # FUTURE     url += MATRIX_V3_MEDIA_PATH + path
+            if self.version == MatrixVersion.V3:
+                url += MATRIX_V3_MEDIA_PATH + path
 
-            # FUTURE else:
-            # FUTURE     url += MATRIX_V2_MEDIA_PATH + path
-            url += MATRIX_V2_MEDIA_PATH + path
+            else:
+                url += MATRIX_V2_MEDIA_PATH + path
 
             params.update({"filename": attachment.name})
             with open(attachment.path, "rb") as fp:

tests/test_plugin_matrix.py

@@ -1002,9 +1002,8 @@ def mock_function_handing(url, data, **kwargs):
 
 
 @mock.patch("requests.put")
-@mock.patch("requests.get")
 @mock.patch("requests.post")
-def test_plugin_matrix_attachments_api_v3(mock_post, mock_get, mock_put):
+def test_plugin_matrix_attachments_api_v3(mock_post, mock_put):
     """NotifyMatrix() Attachment Checks (v3)"""
 
     # Prepare a good response
@@ -1018,7 +1017,6 @@ def test_plugin_matrix_attachments_api_v3(mock_post, mock_get, mock_put):
 
     # Prepare Mock return object
     mock_post.return_value = response
-    mock_get.return_value = response
     mock_put.return_value = response
 
     # Instantiate our object
@@ -1040,23 +1038,22 @@ def test_plugin_matrix_attachments_api_v3(mock_post, mock_get, mock_put):
     attach = AppriseAttachment(os.path.join(TEST_VAR_DIR, "apprise-test.gif"))
 
     # Test our call count
-    assert mock_put.call_count == 1
-    assert mock_post.call_count == 2
-    assert (
-        mock_post.call_args_list[0][0][0]
-        == "http://localhost/_matrix/client/v3/login"
-    )
-    assert (
-        mock_post.call_args_list[1][0][0]
-        == "http://localhost/_matrix/client/v3/join/%23general%3Alocalhost"
-    )
-    assert (
-        mock_put.call_args_list[0][0][0]
-        == "http://localhost/_matrix/client/v3/rooms/%21abc123%3Alocalhost/"
+    assert mock_put.call_count == 2
+    assert mock_post.call_count == 3
+    assert mock_post.call_args_list[0][0][0] == \
+        "http://localhost/_matrix/client/v3/login"
+    assert mock_post.call_args_list[1][0][0] == \
+        "http://localhost/_matrix/media/v3/upload"
+    assert mock_post.call_args_list[2][0][0] == \
+        "http://localhost/_matrix/client/v3/join/%23general%3Alocalhost"
+    assert mock_put.call_args_list[0][0][0] == \
+        "http://localhost/_matrix/client/v3/rooms/%21abc123%3Alocalhost/" \
         "send/m.room.message/0"
-    )
+    assert mock_put.call_args_list[1][0][0] == \
+        "http://localhost/_matrix/client/v3/rooms/%21abc123%3Alocalhost/" \
+        "send/m.room.message/1"
 
-    # Attach an unsupported file type (it's just skipped)
+    # Attach a zip file type
     attach = AppriseAttachment(
         os.path.join(TEST_VAR_DIR, "apprise-archive.zip")
     )
@@ -1086,28 +1083,37 @@ def test_plugin_matrix_attachments_api_v3(mock_post, mock_get, mock_put):
     # update our attachment to be valid
     attach = AppriseAttachment(os.path.join(TEST_VAR_DIR, "apprise-test.gif"))
 
+    mock_put.return_value = None
     mock_post.return_value = None
+
     # Throw an exception on the first call to requests.post()
     for side_effect in (requests.RequestException(), OSError(), bad_response):
+        # Reset our value
+        mock_put.reset_mock()
+        mock_post.reset_mock()
+
         mock_post.side_effect = [side_effect]
 
-        # We'll never fail because files are not attached
-        assert obj.send(body="test", attach=attach) is True
+        assert obj.send(body="test", attach=attach) is False
 
     # Throw an exception on the second call to requests.post()
     for side_effect in (requests.RequestException(), OSError(), bad_response):
-        mock_post.side_effect = [response, side_effect]
+        # Reset our value
+        mock_put.reset_mock()
+        mock_post.reset_mock()
 
-        # Attachment support does not exist vor v3 at time, so this will
-        # work nicely
-        assert obj.send(body="test", attach=attach) is True
+        mock_put.side_effect = [side_effect, response]
+        mock_post.side_effect = [response, side_effect, response]
+
+        # We'll fail now because of our error handling
+        assert obj.send(body="test", attach=attach) is False
 
     # handle a bad response
+    mock_put.side_effect = [bad_response, response]
     mock_post.side_effect = [response, bad_response, response]
 
-    # Attachment support does not exist vor v3 at time, so this will
-    # work nicely
-    assert obj.send(body="test", attach=attach) is True
+    # We'll fail now because of an internal exception
+    assert obj.send(body="test", attach=attach) is False
 
     # Force a object removal (thus a logout call)
     del obj