Make notify thread-safe (#3275)

* Make notify thread-safe

* test fixes

* docstring

* Update tests/notifications/test_app_notifications.py

Co-authored-by: Dave Pearson <davep@davep.org>

---------

Co-authored-by: Dave Pearson <davep@davep.org>

CHANGELOG.md

@@ -11,6 +11,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - Fixed a crash when removing an option from an `OptionList` while the mouse is hovering over the last option https://github.com/Textualize/textual/issues/3270
 
+### Changed
+
+- Widget.notify and App.notify are now thread-safe https://github.com/Textualize/textual/pull/3275
+- Breaking change: Widget.notify and App.notify now return None https://github.com/Textualize/textual/pull/3275
+- App.unnotify is now private (renamed to App._unnotify) https://github.com/Textualize/textual/pull/3275
+
 ## [0.36.0] - 2023-09-05
 
 ### Added

src/textual/app.py

@@ -90,7 +90,7 @@ from .keys import (
     _get_unicode_name_from_key,
 )
 from .messages import CallbackType
-from .notifications import Notification, Notifications, SeverityLevel
+from .notifications import Notification, Notifications, Notify, SeverityLevel
 from .reactive import Reactive
 from .renderables.blank import Blank
 from .screen import Screen, ScreenResultCallbackType, ScreenResultType
@@ -2931,18 +2931,20 @@ class App(Generic[ReturnType], DOMNode):
         title: str = "",
         severity: SeverityLevel = "information",
         timeout: float = Notification.timeout,
-    ) -> Notification:
+    ) -> None:
         """Create a notification.
 
+        !!! tip
+
+            This method is thread-safe.
+
+
         Args:
             message: The message for the notification.
             title: The title for the notification.
             severity: The severity of the notification.
             timeout: The timeout for the notification.
 
-        Returns:
-            The new notification.
-
         The `notify` method is used to create an application-wide
         notification, shown in a [`Toast`][textual.widgets._toast.Toast],
         normally originating in the bottom right corner of the display.
@@ -2977,11 +2979,14 @@ class App(Generic[ReturnType], DOMNode):
             ```
         """
         notification = Notification(message, title, severity, timeout)
-        self._notifications.add(notification)
-        self._refresh_notifications()
-        return notification
+        self.post_message(Notify(notification))
 
-    def unnotify(self, notification: Notification, refresh: bool = True) -> None:
+    def _on_notify(self, event: Notify) -> None:
+        """Handle notification message."""
+        self._notifications.add(event.notification)
+        self._refresh_notifications()
+
+    def _unnotify(self, notification: Notification, refresh: bool = True) -> None:
         """Remove a notification from the notification collection.
 
         Args:

src/textual/notifications.py

@@ -10,10 +10,19 @@ from uuid import uuid4
 from rich.repr import Result
 from typing_extensions import Literal, Self, TypeAlias
 
+from .message import Message
+
 SeverityLevel: TypeAlias = Literal["information", "warning", "error"]
 """The severity level for a notification."""
 
 
+@dataclass
+class Notify(Message, bubble=False):
+    """Message to show a notification."""
+
+    notification: Notification
+
+
 @dataclass
 class Notification:
     """Holds the details of a notification."""

src/textual/widget.py

@@ -3391,18 +3391,19 @@ class Widget(DOMNode):
         title: str = "",
         severity: SeverityLevel = "information",
         timeout: float = Notification.timeout,
-    ) -> Notification:
+    ) -> None:
         """Create a notification.
 
+        !!! tip
+
+            This method is thread-safe.
+
         Args:
             message: The message for the notification.
             title: The title for the notification.
             severity: The severity of the notification.
             timeout: The timeout for the notification.
 
-        Returns:
-            The new notification.
-
         See [`App.notify`][textual.app.App.notify] for the full
         documentation for this method.
         """

src/textual/widgets/_toast.py

@@ -128,7 +128,7 @@ class Toast(Static, inherit_css=False):
         # the notification that caused us to exist. Note that we tell the
         # app to not bother refreshing the display on our account, we're
         # about to handle that anyway.
-        self.app.unnotify(self._notification, refresh=False)
+        self.app._unnotify(self._notification, refresh=False)
         # Note that we attempt to remove our parent, because we're wrapped
         # inside an alignment container. The testing that we are is as much
         # to keep type checkers happy as anything else.

tests/notifications/test_app_notifications.py

@@ -17,15 +17,17 @@ async def test_app_with_notifications() -> None:
     """An app with notifications should have notifications in the list."""
     async with NotificationApp().run_test() as pilot:
         pilot.app.notify("test")
+        await pilot.pause()
         assert len(pilot.app._notifications) == 1
 
 
 async def test_app_with_removing_notifications() -> None:
     """An app with notifications should have notifications in the list, which can be removed."""
     async with NotificationApp().run_test() as pilot:
-        notification = pilot.app.notify("test")
+        pilot.app.notify("test")
+        await pilot.pause()
         assert len(pilot.app._notifications) == 1
-        pilot.app.unnotify(notification)
+        pilot.app._unnotify(list(pilot.app._notifications)[0])
         assert len(pilot.app._notifications) == 0
 
 
@@ -34,6 +36,7 @@ async def test_app_with_notifications_that_expire() -> None:
     async with NotificationApp().run_test() as pilot:
         for n in range(100):
             pilot.app.notify("test", timeout=(0.5 if bool(n % 2) else 60))
+        await pilot.pause()
         assert len(pilot.app._notifications) == 100
         sleep(0.6)
         assert len(pilot.app._notifications) == 50
@@ -44,6 +47,7 @@ async def test_app_clearing_notifications() -> None:
     async with NotificationApp().run_test() as pilot:
         for _ in range(100):
             pilot.app.notify("test", timeout=120)
+        await pilot.pause()
         assert len(pilot.app._notifications) == 100
         pilot.app.clear_notifications()
         assert len(pilot.app._notifications) == 0