Fixed ArgumentException when showing multiple notifications with the same content (#19774)

* Add unit test for notifications

* Fixed ArgumentException when showing multiple notifications with the same content, additional unit tests

* fixes after review

* IManagedNotificationManager.Show fix

src/Avalonia.Controls/Notifications/NotificationCard.cs

@@ -1,6 +1,5 @@
 using System;
 using System.Linq;
-using Avalonia.Reactive;
 using Avalonia.Controls.Metadata;
 using Avalonia.Interactivity;
 using Avalonia.LogicalTree;

src/Avalonia.Controls/Notifications/WindowNotificationManager.cs

@@ -2,7 +2,6 @@ using System;
 using System.Collections;
 using System.Collections.Generic;
 using System.Linq;
-using System.Threading.Tasks;
 using Avalonia.Controls.Metadata;
 using Avalonia.Controls.Primitives;
 using Avalonia.Threading;
@@ -17,8 +16,12 @@ namespace Avalonia.Controls.Notifications
     [PseudoClasses(":topleft", ":topright", ":bottomleft", ":bottomright", ":topcenter", ":bottomcenter")]
     public class WindowNotificationManager : TemplatedControl, IManagedNotificationManager
     {
-        private IList? _items;
+        private IList _items = new List<Control>();
-        private readonly Dictionary<object, NotificationCard> _notificationCards = new ();
+
+        /// <summary>
+        /// Currently active notifications.
+        /// </summary>
+        internal IEnumerable<NotificationCard> Notifications => _items.OfType<NotificationCard>();
 
         /// <summary>
         /// Defines the <see cref="Position"/> property.
@@ -81,9 +84,13 @@ namespace Avalonia.Controls.Notifications
         protected override void OnApplyTemplate(TemplateAppliedEventArgs e)
         {
             base.OnApplyTemplate(e);
-            
+
             var itemsControl = e.NameScope.Find<Panel>("PART_Items");
-            _items = itemsControl?.Children;
+
+            if (itemsControl?.Children is not null)
+            {
+                _items = itemsControl.Children;
+            }
         }
 
         /// <inheritdoc/>
@@ -114,11 +121,11 @@ namespace Avalonia.Controls.Notifications
         /// <param name="onClick">an Action to be run when the notification is clicked</param>
         /// <param name="onClose">an Action to be run when the notification is closed</param>
         /// <param name="classes">style classes to apply</param>
-        public async void Show(object content, 
+        public void Show(object content,
-            NotificationType type, 
+            NotificationType type,
             TimeSpan? expiration = null,
-            Action? onClick = null, 
+            Action? onClick = null,
-            Action? onClose = null, 
+            Action? onClose = null,
             string[]? classes = null)
         {
             Dispatcher.UIThread.VerifyAccess();
@@ -141,9 +148,7 @@ namespace Avalonia.Controls.Notifications
             notificationControl.NotificationClosed += (sender, args) =>
             {
                 onClose?.Invoke();
-
+                _items.Remove(notificationControl);
-                _items?.Remove(sender);
-                _notificationCards.Remove(content);
             };
 
             notificationControl.PointerPressed += (sender, args) =>
@@ -153,49 +158,31 @@ namespace Avalonia.Controls.Notifications
                 (sender as NotificationCard)?.Close();
             };
 
-            Dispatcher.UIThread.Post(() =>
+            _items.Add(notificationControl);
-            {
-                _items?.Add(notificationControl);
-                _notificationCards.Add(content, notificationControl);
 
-                if (_items?.OfType<NotificationCard>().Count(i => !i.IsClosing) > MaxItems)
+            if (Notifications.Count(i => !i.IsClosing) > MaxItems)
-                {
+            {
-                    _items.OfType<NotificationCard>().First(i => !i.IsClosing).Close();
+                Notifications.First(i => !i.IsClosing).Close();
-                }
+            }
-            });
 
             if (expiration == TimeSpan.Zero)
             {
                 return;
             }
 
-            await Task.Delay(expiration ?? TimeSpan.FromSeconds(5));
+            DispatcherTimer.RunOnce(notificationControl.Close, expiration ?? TimeSpan.FromSeconds(5));
-
-            notificationControl.Close();
- 
-            _notificationCards.Remove(content);
         }
 
         /// <inheritdoc/>
-        public void Close(INotification notification)
+        public void Close(INotification notification) => Close(notification as object);
-        {
-            Dispatcher.UIThread.VerifyAccess();
-
-            if (_notificationCards.Remove(notification, out var notificationCard))
-            {
-                notificationCard.Close();
-            }
-        }
 
         /// <inheritdoc/>
         public void Close(object content)
         {
             Dispatcher.UIThread.VerifyAccess();
 
-            if (_notificationCards.Remove(content, out var notificationCard))
+            var cardsToClose = Notifications.Where(x => x.Content?.Equals(content) ?? false);
-            {
+            cardsToClose?.Do(x => x.Close());
-                notificationCard.Close();
-            }
         }
 
         /// <inheritdoc/>
@@ -203,12 +190,7 @@ namespace Avalonia.Controls.Notifications
         {
             Dispatcher.UIThread.VerifyAccess();
 
-            foreach (var kvp in _notificationCards)
+            Notifications.Do(x => x.Close());
-            {
-                kvp.Value.Close();
-            }
-            
-            _notificationCards.Clear();
         }
 
         protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs change)
@@ -225,7 +207,7 @@ namespace Avalonia.Controls.Notifications
         {
             base.OnDetachedFromVisualTree(e);
 
-            _notificationCards.Clear();
+            _items.Clear();
         }
 
         /// <summary>
@@ -249,7 +231,7 @@ namespace Avalonia.Controls.Notifications
                 adornerLayer.Children.Remove(this);
                 AdornerLayer.SetAdornedElement(this, null);
             }
-            
+
             // Reinstall notification manager on template reapplied.
             var topLevel = (TopLevel)sender!;
             topLevel.TemplateApplied -= TopLevelOnTemplateApplied;

tests/Avalonia.Controls.UnitTests/NotificationsTests.cs

@@ -0,0 +1,116 @@
+using System.Linq;
+using Avalonia.Controls.Notifications;
+using Avalonia.UnitTests;
+using Xunit;
+using Notification = Avalonia.Controls.Notifications.Notification;
+
+namespace Avalonia.Controls.UnitTests
+{
+    public class WindowNotificationManagerTests : ScopedTestBase
+    {
+        [Fact]
+        public void Show_Notifications_With_Same_String()
+        {
+            WindowNotificationManager manager = new();
+
+            manager.Show("Notification text");
+            manager.Show("Notification text");
+            manager.Show("Notification text");
+
+            Assert.Equal(3, manager.Notifications.Count());
+        }
+
+        [Fact]
+        public void Show_And_Close_Notification()
+        {
+            WindowNotificationManager manager = new();
+
+            manager.Show("Notification text");
+
+            Assert.Equal(1, manager.Notifications.Count());
+
+            manager.Close("Notification text");
+
+            Assert.True(!manager.Notifications.Any(x => !x.IsClosing));
+        }
+
+        [Fact]
+        public void Show_And_Close_All_Notifications()
+        {
+            WindowNotificationManager manager = new();
+
+            manager.Show("Notification 1");
+            manager.Show("Notification 2");
+
+            Assert.Equal(2, manager.Notifications.Count());
+
+            manager.CloseAll();
+
+            Assert.True(!manager.Notifications.Any(x => !x.IsClosing));
+        }
+    }
+
+    public class INotificationManagerTests : ScopedTestBase
+    {
+        [Fact]
+        public void Show_Notifications_With_Same_Content()
+        {
+            INotificationManager manager = new WindowNotificationManager();
+
+            Notification notification = new()
+            {
+                Message = "Notification text"
+            };
+
+            manager.Show(notification);
+            manager.Show(notification);
+            manager.Show(notification);
+
+            Assert.Equal(3, ((WindowNotificationManager)manager).Notifications.Count());
+        }
+
+        [Fact]
+        public void Show_And_Close_Notification()
+        {
+            INotificationManager manager = new WindowNotificationManager();
+
+            Notification notification = new()
+            {
+                Message = "Notification text"
+            };
+
+            manager.Show(notification);
+
+            Assert.Equal(1, ((WindowNotificationManager)manager).Notifications.Count());
+
+            manager.Close(notification);
+
+            Assert.True(!((WindowNotificationManager)manager).Notifications.Any(x => !x.IsClosing));
+        }
+
+        [Fact]
+        public void Show_And_Close_All_Notifications()
+        {
+            INotificationManager manager = new WindowNotificationManager();
+
+            Notification notification1 = new()
+            {
+                Message = "Notification text"
+            };
+
+            Notification notification2 = new()
+            {
+                Message = "Notification text"
+            };
+
+            manager.Show(notification1);
+            manager.Show(notification2);
+
+            Assert.Equal(2, ((WindowNotificationManager)manager).Notifications.Count());
+
+            manager.CloseAll();
+
+            Assert.True(!((WindowNotificationManager)manager).Notifications.Any(x => !x.IsClosing));
+        }
+    }
+}