Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 28 additions & 9 deletions mqttwarn/services/desktopnotify.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,30 @@

import json
import typing as t
import asyncio

from desktop_notifier import DesktopNotifier, Urgency, Button, ReplyField

from mqttwarn.model import Service, ProcessorItem, Struct


notify = DesktopNotifier()
Comment thread
amotl marked this conversation as resolved.


def is_json(msg: t.Union[str, bytes]) -> bool:
try:
json.loads(msg)
except ValueError as e:
return False
except TypeError as e:
return False
return True


async def send_notification(message, title, sound=True):
await notify.send(message=message, title=title, sound=sound)


def plugin(srv: Service, item: ProcessorItem):
# Log
srv.logging.debug("*** MODULE=%s: service=%s, target=%s", __file__, item.service, item.target)
Expand All @@ -30,27 +40,36 @@ def plugin(srv: Service, item: ProcessorItem):
config = item.config

# Play Sound ?
playSound = True
playSound = False
if isinstance(config, dict):
playSound = config.get('sound', True)
playSound = config.get('sound', False)

# Get Message
message = item.message
if message and is_json(message):
data = json.loads(message)
else:
if message is None:
raise ValueError("Message is required")
if is_json(message):
parsed = json.loads(message)
data = parsed if isinstance(parsed, dict) else {
"title": item.get("title", item.topic),
"message": parsed,
}
else:
try:
msg = t.cast(t.Dict, message).get('message')
except:
msg = str(message)
Comment on lines +58 to +61

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Handle dictionary payloads explicitly.

For a dictionary without "message", .get() returns None, so the notification becomes "None" at Line 64. The bare except also masks unrelated errors; use an explicit isinstance/mapping check and define a fallback for missing keys.

🧰 Tools
🪛 Ruff (0.15.20)

[error] 56-56: Do not use bare except

(E722)

Source: Linters/SAST tools

data = {
"title" : item.get('title', item.topic),
"message": message
"message": msg
}
Comment thread
amotl marked this conversation as resolved.

srv.logging.debug("Sending desktop notification")
message = str(data.get('message', ''))
title = str(data.get('title', ''))
try:
# Synchronous Notification (allows no callbacks in OSX)
# Asynchronous would require asyncio and require some changes to the plugin handler
notify.send_sync(message=message, title=title, sound=playSound)
# TODO: Fix issue where this keeps running until the notification is closed.
asyncio.run(send_notification(message=message, title=title, sound=playSound))

except Exception as e:
srv.logging.warning("Invoking desktop notifier failed: %s" % e)
Expand Down
39 changes: 21 additions & 18 deletions tests/services/test_desktopnotify.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# -*- coding: utf-8 -*-
# (c) 2022 The mqttwarn developers
import json
import sys
from unittest import mock
from unittest.mock import Mock, call

import pytest
Expand All @@ -12,7 +14,9 @@

@pytest.fixture
def desktop_notifier_mock(mocker):
notifier = mocker.patch("desktop_notifier.DesktopNotifier", create=True)
if sys.version_info < (3, 8):
raise pytest.skip(reason="mock.AsyncMock not available on Python 3.7 and earlier")
notifier = mocker.patch("desktop_notifier.DesktopNotifier.send", new_callable=mock.AsyncMock, return_value=True)
mocker.patch("desktop_notifier.Urgency", create=True)
mocker.patch("desktop_notifier.Button", create=True)
mocker.patch("desktop_notifier.ReplyField", create=True)
Expand All @@ -32,10 +36,9 @@ def test_desktopnotify_vanilla_success(desktop_notifier_mock, srv, caplog):

outcome = module.plugin(srv, item)

assert desktop_notifier_mock.mock_calls == [
call(),
call().send_sync(message="⚽ Notification message ⚽", title="⚽ Notification title ⚽", sound=True),
]
desktop_notifier_mock.assert_awaited_once_with(
message="⚽ Notification message ⚽", title="⚽ Notification title ⚽", sound=False
)

assert outcome is True
assert "Sending desktop notification" in caplog.messages
Expand All @@ -52,21 +55,21 @@ def test_desktopnotify_vanilla_failure(desktop_notifier_mock, mocker, srv: Servi
# Plugin needs a real `Struct`.
item = Struct(**processor_item.asdict())

# Make the `send_sync` method fail.
# Make the `send` method fail.
notifier_mock: Mock = mocker.patch.object(
module,
"notify",
Mock(
**{ # ty: ignore[invalid-argument-type, unused-ignore-comment, unused-ignore-comment]
"send_sync.side_effect": Exception("Something failed")
"send.side_effect": Exception("Something failed")
}
),
)

outcome = module.plugin(srv, item)

assert notifier_mock.mock_calls == [
call.send_sync(message="⚽ Notification message ⚽", title="⚽ Notification title ⚽", sound=True),
call.send(message="⚽ Notification message ⚽", title="⚽ Notification title ⚽", sound=False),
]

assert outcome is False
Expand All @@ -93,33 +96,33 @@ def test_desktopnotify_json_success(desktop_notifier_mock, srv, caplog):

outcome = module.plugin(srv, item)

assert desktop_notifier_mock.mock_calls == [
call(),
call().send_sync(message="⚽ Notification message ⚽", title="⚽ Notification title ⚽", sound=True),
]
desktop_notifier_mock.assert_awaited_once_with(
message="⚽ Notification message ⚽",
title="⚽ Notification title ⚽",
sound=False,
)

assert outcome is True
assert "Sending desktop notification" in caplog.messages


def test_desktopnotify_no_sound_success(desktop_notifier_mock, srv, caplog):
def test_desktopnotify_with_sound_success(desktop_notifier_mock, srv, caplog):
module = load_module_by_name("mqttwarn.services.desktopnotify")

item = Item(
title="⚽ Notification title ⚽",
message="⚽ Notification message ⚽",
config={"sound": False},
config={"sound": True},
)

# Plugin needs a real `Struct`.
item = Struct(**item.asdict())

outcome = module.plugin(srv, item)

assert desktop_notifier_mock.mock_calls == [
call(),
call().send_sync(message="⚽ Notification message ⚽", title="⚽ Notification title ⚽", sound=False),
]
desktop_notifier_mock.assert_awaited_once_with(
message="⚽ Notification message ⚽", title="⚽ Notification title ⚽", sound=True
)

assert outcome is True
assert "Sending desktop notification" in caplog.messages
Loading