Email sender: improve the Date header, add a Received header

- Set the Date to the message's sent-at header value
- Add a Received header because FMN acts as a "gateway"

Signed-off-by: Aurélien Bompard <[email protected]>

fmn/core/amqp.py

@@ -2,10 +2,15 @@
 #
 # SPDX-License-Identifier: MIT
 
+import logging
 import ssl
+from datetime import datetime, timezone
 
 from aio_pika.abc import SSLOptions
 from aio_pika.connection import URL
+from fedora_messaging.message import Message
+
+log = logging.getLogger(__name__)
 
 
 def get_url_from_config(config: dict):
@@ -21,3 +26,16 @@ def get_url_from_config(config: dict):
             )
         )
     return url
+
+
+def get_sent_datetime(message: Message) -> datetime:
+    sent_at = message._headers.get("sent-at", None)
+    if sent_at:
+        # fromisoformat doesn't parse Z suffix (yet) see:
+        # https://discuss.python.org/t/parse-z-timezone-suffix-in-datetime/2220
+        try:
+            return datetime.fromisoformat(sent_at.replace("Z", "+00:00"))
+        except ValueError:
+            log.exception("Failed to parse sent-at timestamp value")
+    # Default to now
+    return datetime.now(tz=timezone.utc)

fmn/database/model/destination.py

@@ -3,13 +3,15 @@
 # SPDX-License-Identifier: MIT
 
 import logging
+from email.utils import format_datetime
 from typing import TYPE_CHECKING
 
 from httpx import AsyncClient, HTTPStatusError
 from sqlalchemy import Column, ForeignKey, Integer, String, UnicodeText, select
 from sqlalchemy.ext.asyncio import async_object_session
 from sqlalchemy.orm import relationship
 
+from ...core.amqp import get_sent_datetime
 from ...core.config import get_settings
 from ..main import Base
 from .generation_rule import GenerationRule
@@ -56,6 +58,7 @@ async def generate(self, message: "Message") -> "Notification.content":
                 "headers": {
                     "To": self.address,
                     "Subject": f"{app_name}{message.summary}",
+                    "Date": format_datetime(get_sent_datetime(message)),
                 },
                 "body": body,
                 "footer": f"Sent by Fedora Notifications: {settings.public_url}/rules/{rule_id}",

fmn/sender/email.py

@@ -3,6 +3,7 @@
 # SPDX-License-Identifier: MIT
 
 import logging
+import socket
 from email.message import EmailMessage
 from email.utils import localtime
 
@@ -28,7 +29,9 @@ async def handle(self, message):
         # Test with `python -m smtpd -c DebuggingServer -n`
         notif = EmailMessage()
         notif["From"] = self._config["from"]
-        notif["Date"] = localtime()
+        notif.add_header(
+            "Received", f"from fedora-messaging by FMN ({socket.getfqdn()}) ; {localtime()}"
+        )
         for name, value in message["headers"].items():
             notif[name] = value
         body = message["body"]

tests/core/test_amqp.py

@@ -3,10 +3,12 @@
 # SPDX-License-Identifier: MIT
 
 import ssl
+from datetime import datetime, timezone
 
+import pytest
 from aio_pika.connection import URL
 
-from fmn.core.amqp import get_url_from_config
+from fmn.core.amqp import get_sent_datetime, get_url_from_config
 
 
 async def test_get_url_from_config_with_ssl(mocker):
@@ -28,3 +30,27 @@ async def test_get_url_from_config_with_ssl(mocker):
         }
     )
     assert get_url_from_config(config) == expected
+
+
+FAKE_NOW = datetime(2021, 1, 1, 0, 0, 0, tzinfo=timezone.utc)
+
+
+@pytest.mark.parametrize(
+    "sent_at,expected",
+    [
+        (None, FAKE_NOW),
+        ("2021-07-27T04:22:42Z", datetime(2021, 7, 27, 4, 22, 42, tzinfo=timezone.utc)),
+        ("2021-07-27T04:22:42JUNK", FAKE_NOW),
+    ],
+)
+def test_get_sent_datetime(make_mocked_message, mocker, sent_at, expected):
+    message = make_mocked_message(
+        topic="dummy",
+        body={"summary": "dummy summary"},
+    )
+    message._properties.headers["sent-at"] = sent_at
+
+    fake_datetime = mocker.patch("fmn.core.amqp.datetime")
+    fake_datetime.now.return_value = FAKE_NOW
+    fake_datetime.fromisoformat = datetime.fromisoformat
+    assert get_sent_datetime(message) == expected

tests/database/model/test_destination.py

@@ -2,6 +2,8 @@
 #
 # SPDX-License-Identifier: MIT
 
+from datetime import datetime
+from email.utils import format_datetime
 from unittest import mock
 
 import httpx
@@ -42,8 +44,13 @@ async def test_email(make_mocked_message, db_async_session, rule_obj):
         },
     )
     result = await d.generate(message)
+    date_header = format_datetime(datetime.fromisoformat(message._headers["sent-at"]))
     assert result == {
-        "headers": {"To": "[email protected]", "Subject": "[dummy] dummy summary"},
+        "headers": {
+            "To": "[email protected]",
+            "Subject": "[dummy] dummy summary",
+            "Date": date_header,
+        },
         "body": "dummy content\nhttps://dummy.org/dummylink",
         "footer": "Sent by Fedora Notifications: https://notifications.fedoraproject.org/rules/1",
     }
@@ -72,8 +79,13 @@ async def test_email_with_extra(make_mocked_message, mocker, db_async_session, r
 
     result = await d.generate(message)
 
+    date_header = format_datetime(datetime.fromisoformat(message._headers["sent-at"]))
     assert result == {
-        "headers": {"To": "[email protected]", "Subject": "[dummy] dummy summary"},
+        "headers": {
+            "To": "[email protected]",
+            "Subject": "[dummy] dummy summary",
+            "Date": date_header,
+        },
         "body": "dummy content\nhttps://dummy.org/dummylink\nDUMMY EXTRA",
         "footer": "Sent by Fedora Notifications: https://notifications.fedoraproject.org/rules/1",
     }

tests/sender/test_email.py

@@ -2,8 +2,6 @@
 #
 # SPDX-License-Identifier: MIT
 
-from datetime import date
-from email.utils import parsedate_to_datetime
 from unittest.mock import MagicMock
 
 from aiosmtplib import SMTP, SMTPServerDisconnected
@@ -40,9 +38,8 @@ async def test_email_handle():
     sent = smtp.send_message.call_args[0][0]
     assert sent["To"] == "[email protected]"
     assert sent["Subject"] == "Testing"
-    assert "Date" in sent
-    sent_date = parsedate_to_datetime(sent["Date"])
-    assert sent_date.date() == date.today()
+    assert "Received" in sent
+    assert sent["Received"].startswith("from fedora-messaging by FMN")
     assert sent.get_body().get_content() == "This is a test\n"
     assert sent.get_content_type() == "text/plain"