From c54bf01c172da3f22bda71d53cce446f81ccf398 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Wed, 12 Aug 2020 17:52:17 +0200 Subject: [PATCH 01/38] Enabled shared object[] pool for Messenger --- Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 33 ++++++++++++------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index 9c27badd255..ef99fe971b3 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -222,7 +222,7 @@ public void UnregisterAll(object recipient, TToken token) where TToken : IEquatable { bool lockTaken = false; - IDictionarySlim>[]? maps = null; + object[]? maps = null; int i = 0; // We use an explicit try/finally block here instead of the lock syntax so that we can use a single @@ -243,11 +243,16 @@ public void UnregisterAll(object recipient, TToken token) return; } - // Copy the candidate mappings for the target recipient to a local - // array, as we can't modify the contents of the set while iterating it. - // The rented buffer is oversized and will also include mappings for - // handlers of messages that are registered through a different token. - maps = ArrayPool>>.Shared.Rent(set!.Count); + // Copy the candidate mappings for the target recipient to a local array, as we can't modify the + // contents of the set while iterating it. The rented buffer is oversized and will also include + // mappings for handlers of messages that are registered through a different token. Note that + // we're using just an object array to minimize the number of total rented buffers, that would + // just remain in the shared pool unused, other than when they are rented here. Instead, we're + // using a type that would possibly also be used by the users of the library, which increases + // the opportunities to reuse existing buffers for both. When we need to reference an item + // stored in the buffer with the type we know it will have, we use Unsafe.As to avoid the + // expensive type check in the cast, since we already know the assignment will be valid. + maps = ArrayPool.Shared.Rent(set!.Count); foreach (IMapping item in set) { @@ -265,8 +270,10 @@ public void UnregisterAll(object recipient, TToken token) // without having to know the concrete type in advance, and without having // to deal with reflection: we can just check if the type of the closed interface // matches with the token type currently in use, and operate on those instances. - foreach (IDictionarySlim> map in maps.AsSpan(0, i)) + foreach (object obj in maps.AsSpan(0, i)) { + var map = Unsafe.As>>(obj); + // We don't need whether or not the map contains the recipient, as the // sequence of maps has already been copied from the set containing all // the mappings for the target recipients: it is guaranteed to be here. @@ -324,7 +331,7 @@ public void UnregisterAll(object recipient, TToken token) { maps.AsSpan(0, i).Clear(); - ArrayPool>>.Shared.Return(maps); + ArrayPool.Shared.Return(maps); } } } @@ -382,7 +389,7 @@ public TMessage Send(TMessage message, TToken token) where TMessage : class where TToken : IEquatable { - Action[] entries; + object[] entries; int i = 0; lock (this.recipientsMap) @@ -398,7 +405,7 @@ public TMessage Send(TMessage message, TToken token) // inside one of the currently existing handlers. We can use memory pooling // to reuse arrays, to minimize the average memory usage. In practice, // we usually just need to pay the small overhead of copying the items. - entries = ArrayPool>.Shared.Rent(mapping!.TotalHandlersCount); + entries = ArrayPool.Shared.Rent(mapping!.TotalHandlersCount); // Copy the handlers to the local collection. // Both types being enumerate expose a struct enumerator, @@ -431,7 +438,9 @@ public TMessage Send(TMessage message, TToken token) // Invoke all the necessary handlers on the local copy of entries foreach (var entry in entries.AsSpan(0, i)) { - entry(message); + // We're doing an unsafe cast to skip the type checks again. + // See the comments in the UnregisterALl method for more info. + Unsafe.As>(entry)(message); } } finally @@ -440,7 +449,7 @@ public TMessage Send(TMessage message, TToken token) // lasting memory leaks due to leftover references being stored in the pool. entries.AsSpan(0, i).Clear(); - ArrayPool>.Shared.Return(entries); + ArrayPool.Shared.Return(entries); } return message; From 72b6520a9cc32a485873638ac05230180d94e5ba Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Wed, 12 Aug 2020 17:59:17 +0200 Subject: [PATCH 02/38] Minor performance improvement --- Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 12 +++++++++++- Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj | 1 + 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index ef99fe971b3..45a1031c4eb 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -6,6 +6,7 @@ using System.Buffers; using System.Collections.Generic; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Threading; using Microsoft.Collections.Extensions; @@ -407,6 +408,8 @@ public TMessage Send(TMessage message, TToken token) // we usually just need to pay the small overhead of copying the items. entries = ArrayPool.Shared.Rent(mapping!.TotalHandlersCount); + ref object entriesRef = ref MemoryMarshal.GetReference(entries.AsSpan()); + // Copy the handlers to the local collection. // Both types being enumerate expose a struct enumerator, // so we're not actually allocating the enumerator here. @@ -427,7 +430,14 @@ public TMessage Send(TMessage message, TToken token) // Only select the ones with a matching token if (pairsEnumerator.Key.Equals(token)) { - entries[i++] = pairsEnumerator.Value; + unsafe + { + // We spend quite a bit of time in this busy loop as we go through all + // the existing mappings and registrations to find the handlers we're + // interested in. We can save some time by skipping the bounds checks + // when indexing the array (as the size is already verified anyway). + Unsafe.Add(ref entriesRef, (IntPtr)(void*)(uint)i++) = pairsEnumerator.Value; + } } } } diff --git a/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj b/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj index 25371e3abc3..05c24469bfe 100644 --- a/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj +++ b/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj @@ -4,6 +4,7 @@ netstandard2.0;netstandard2.1 8.0 enable + true Windows Community Toolkit MVVM Toolkit This package includes a .NET Standard MVVM library with helpers such as: From 6ecdc053a98f79f3471c4c15e4e938d1ab686426 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 13 Aug 2020 01:26:26 +0200 Subject: [PATCH 03/38] Fixed a typo and tweaked a comment --- Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index 45a1031c4eb..4e5e780d4dc 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -432,10 +432,10 @@ public TMessage Send(TMessage message, TToken token) { unsafe { - // We spend quite a bit of time in this busy loop as we go through all - // the existing mappings and registrations to find the handlers we're - // interested in. We can save some time by skipping the bounds checks - // when indexing the array (as the size is already verified anyway). + // We spend quite a bit of time in these two busy loops as we go through all the + // existing mappings and registrations to find the handlers we're interested in. + // We can manually offset here to skip the bounds checks in this inner loop when + // indexing the array (the size is already verified and guaranteed to be enough). Unsafe.Add(ref entriesRef, (IntPtr)(void*)(uint)i++) = pairsEnumerator.Value; } } @@ -449,7 +449,7 @@ public TMessage Send(TMessage message, TToken token) foreach (var entry in entries.AsSpan(0, i)) { // We're doing an unsafe cast to skip the type checks again. - // See the comments in the UnregisterALl method for more info. + // See the comments in the UnregisterAll method for more info. Unsafe.As>(entry)(message); } } From 5066dff24fe88a29ddc28696656d7e414449d186 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 17 Aug 2020 11:22:56 +0200 Subject: [PATCH 04/38] Added MessageHandler delegate, removed closures --- .../Messaging/IMessenger.cs | 15 +++- Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 70 +++++++++++-------- .../Messaging/MessengerExtensions.cs | 10 +-- 3 files changed, 60 insertions(+), 35 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs index 39e38d5af01..d1a6e1513df 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs @@ -7,6 +7,17 @@ namespace Microsoft.Toolkit.Mvvm.Messaging { + /// + /// A used to represent actions to invoke when a message is received. + /// The recipient is given as an input argument to allow message registrations to avoid creating + /// closures: if an instance method on a recipient needs to be invoked it is possible to just + /// cast the recipient to the right type and then access the local method from that instance. + /// + /// The type of message to receive. + /// The recipient that is receiving the message. + /// The message being received. + public delegate void MessageHandler(object recipient, TMessage message); + /// /// An interface for a type providing the ability to exchange messages between different objects. /// @@ -32,9 +43,9 @@ bool IsRegistered(object recipient, TToken token) /// The type of token to use to pick the messages to receive. /// The recipient that will receive the messages. /// A token used to determine the receiving channel to use. - /// The to invoke when a message is received. + /// The to invoke when a message is received. /// Thrown when trying to register the same message twice. - void Register(object recipient, TToken token, Action action) + void Register(object recipient, TToken token, MessageHandler action) where TMessage : class where TToken : IEquatable; diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index 4e5e780d4dc..9049cdd1593 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -23,11 +23,16 @@ namespace Microsoft.Toolkit.Mvvm.Messaging /// /// Then, register your a recipient for this message: /// - /// Messenger.Default.Register<LoginCompletedMessage>(this, m => + /// Messenger.Default.Register<LoginCompletedMessage>(this, (r, m) => /// { /// // Handle the message here... /// }); /// + /// The message handler here is a lambda expression taking two parameters: the recipient and the message. + /// This is done to avoid the allocations for the closures that would've been generated if the expression + /// had captured the current instance - instead it is possible to just cast the recipient to the right type + /// and access private instance members from the handler directly. This allows the message handler to be a + /// static method, which enables the C# to perform a number of additional memory optimizations. /// Finally, send a message when needed, like so: /// /// Messenger.Default.Send<LoginCompletedMessage>(); @@ -62,7 +67,7 @@ public sealed class Messenger : IMessenger // | ________(recipients registrations)___________\________/ / __/ // | / _______(channel registrations)_____\___________________/ / // | / / \ / - // DictionarySlim>> mapping = Mapping + // DictionarySlim>> mapping = Mapping // / / \ / / // ___(Type2.tToken)____/ / \______/___________________/ // /________________(Type2.tMessage)____/ / @@ -133,7 +138,7 @@ public bool IsRegistered(object recipient, TToken token) } /// - public void Register(object recipient, TToken token, Action action) + public void Register(object recipient, TToken token, MessageHandler action) where TMessage : class where TToken : IEquatable { @@ -142,12 +147,12 @@ public void Register(object recipient, TToken token, Action registration list for this recipient Mapping mapping = GetOrAddMapping(); var key = new Recipient(recipient); - ref DictionarySlim>? map = ref mapping.GetOrAddValueRef(key); + ref DictionarySlim>? map = ref mapping.GetOrAddValueRef(key); - map ??= new DictionarySlim>(); + map ??= new DictionarySlim>(); // Add the new registration entry - ref Action? handler = ref map.GetOrAddValueRef(token); + ref MessageHandler? handler = ref map.GetOrAddValueRef(token); if (!(handler is null)) { @@ -352,7 +357,7 @@ public void Unregister(object recipient, TToken token) var key = new Recipient(recipient); - if (!mapping!.TryGetValue(key, out DictionarySlim>? dictionary)) + if (!mapping!.TryGetValue(key, out DictionarySlim>? dictionary)) { return; } @@ -386,11 +391,14 @@ public void Unregister(object recipient, TToken token) } /// - public TMessage Send(TMessage message, TToken token) + public unsafe TMessage Send(TMessage message, TToken token) where TMessage : class where TToken : IEquatable { - object[] entries; + object[] handlers; + object[] recipients; + ref object handlersRef = ref Unsafe.AsRef(null); + ref object recipientsRef = ref Unsafe.AsRef(null); int i = 0; lock (this.recipientsMap) @@ -406,9 +414,12 @@ public TMessage Send(TMessage message, TToken token) // inside one of the currently existing handlers. We can use memory pooling // to reuse arrays, to minimize the average memory usage. In practice, // we usually just need to pay the small overhead of copying the items. - entries = ArrayPool.Shared.Rent(mapping!.TotalHandlersCount); + int totalHandlersCount = mapping!.TotalHandlersCount; - ref object entriesRef = ref MemoryMarshal.GetReference(entries.AsSpan()); + handlers = ArrayPool.Shared.Rent(totalHandlersCount); + recipients = ArrayPool.Shared.Rent(totalHandlersCount); + handlersRef = ref handlers[0]; + recipientsRef = ref recipients[0]; // Copy the handlers to the local collection. // Both types being enumerate expose a struct enumerator, @@ -423,6 +434,7 @@ public TMessage Send(TMessage message, TToken token) // that doesn't expose the single standard Current property. while (mappingEnumerator.MoveNext()) { + object recipient = mappingEnumerator.Key.Target; var pairsEnumerator = mappingEnumerator.Value.GetEnumerator(); while (pairsEnumerator.MoveNext()) @@ -430,14 +442,12 @@ public TMessage Send(TMessage message, TToken token) // Only select the ones with a matching token if (pairsEnumerator.Key.Equals(token)) { - unsafe - { - // We spend quite a bit of time in these two busy loops as we go through all the - // existing mappings and registrations to find the handlers we're interested in. - // We can manually offset here to skip the bounds checks in this inner loop when - // indexing the array (the size is already verified and guaranteed to be enough). - Unsafe.Add(ref entriesRef, (IntPtr)(void*)(uint)i++) = pairsEnumerator.Value; - } + // We spend quite a bit of time in these two busy loops as we go through all the + // existing mappings and registrations to find the handlers we're interested in. + // We can manually offset here to skip the bounds checks in this inner loop when + // indexing the array (the size is already verified and guaranteed to be enough). + Unsafe.Add(ref handlersRef, (IntPtr)(void*)(uint)i) = pairsEnumerator.Value; + Unsafe.Add(ref recipientsRef, (IntPtr)(void*)(uint)i++) = recipient; } } } @@ -446,20 +456,24 @@ public TMessage Send(TMessage message, TToken token) try { // Invoke all the necessary handlers on the local copy of entries - foreach (var entry in entries.AsSpan(0, i)) + for (int j = 0; j < i; j++) { // We're doing an unsafe cast to skip the type checks again. // See the comments in the UnregisterAll method for more info. - Unsafe.As>(entry)(message); + MessageHandler handler = Unsafe.As>(Unsafe.Add(ref handlersRef, (IntPtr)(void*)(uint)j)); + + handler(Unsafe.Add(ref recipientsRef, (IntPtr)(void*)(uint)j), message); } } finally { // As before, we also need to clear it first to avoid having potentially long // lasting memory leaks due to leftover references being stored in the pool. - entries.AsSpan(0, i).Clear(); + handlers.AsSpan(0, i).Clear(); + recipients.AsSpan(0, i).Clear(); - ArrayPool.Shared.Return(entries); + ArrayPool.Shared.Return(handlers); + ArrayPool.Shared.Return(recipients); } return message; @@ -534,7 +548,7 @@ private Mapping GetOrAddMapping() /// This type is defined for simplicity and as a workaround for the lack of support for using type aliases /// over open generic types in C# (using type aliases can only be used for concrete, closed types). /// - private sealed class Mapping : DictionarySlim>>, IMapping + private sealed class Mapping : DictionarySlim>>, IMapping where TMessage : class where TToken : IEquatable { @@ -586,7 +600,7 @@ private interface IMapping : IDictionarySlim /// /// The registered recipient. /// - private readonly object target; + public readonly object Target; /// /// Initializes a new instance of the struct. @@ -595,14 +609,14 @@ private interface IMapping : IDictionarySlim [MethodImpl(MethodImplOptions.AggressiveInlining)] public Recipient(object target) { - this.target = target; + Target = target; } /// [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool Equals(Recipient other) { - return ReferenceEquals(this.target, other.target); + return ReferenceEquals(Target, other.Target); } /// @@ -615,7 +629,7 @@ public override bool Equals(object? obj) [MethodImpl(MethodImplOptions.AggressiveInlining)] public override int GetHashCode() { - return RuntimeHelpers.GetHashCode(this.target); + return RuntimeHelpers.GetHashCode(this.Target); } } diff --git a/Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.cs b/Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.cs index a344b76a4f3..5e2473db331 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.cs @@ -174,7 +174,7 @@ static Action GetRegistrationAction(Type type, Metho public static void Register(this IMessenger messenger, IRecipient recipient) where TMessage : class { - messenger.Register(recipient, default, recipient.Receive); + messenger.Register(recipient, default, (r, m) => Unsafe.As>(r).Receive(m)); } /// @@ -191,7 +191,7 @@ public static void Register(this IMessenger messenger, IRecipi where TMessage : class where TToken : IEquatable { - messenger.Register(recipient, token, recipient.Receive); + messenger.Register(recipient, token, (r, m) => Unsafe.As>(r).Receive(m)); } /// @@ -200,13 +200,13 @@ public static void Register(this IMessenger messenger, IRecipi /// The type of message to receive. /// The instance to use to register the recipient. /// The recipient that will receive the messages. - /// The to invoke when a message is received. + /// The to invoke when a message is received. /// Thrown when trying to register the same message twice. /// This method will use the default channel to perform the requested registration. - public static void Register(this IMessenger messenger, object recipient, Action action) + public static void Register(this IMessenger messenger, object recipient, MessageHandler handler) where TMessage : class { - messenger.Register(recipient, default(Unit), action); + messenger.Register(recipient, default(Unit), handler); } /// From f886f2878fdfdfb0554d6fef6628ee06e8cf1885 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 17 Aug 2020 11:30:22 +0200 Subject: [PATCH 05/38] Fixed some XML comments --- Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index 9049cdd1593..01da10a831c 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -42,7 +42,7 @@ namespace Microsoft.Toolkit.Mvvm.Messaging /// in the current scope. This is helpful to keep the registration and handling logic separate. /// Following up from the previous example, consider a class having this method: /// - /// private void Receive(LoginCompletedMessage message) + /// private static void Receive(object recipient, LoginCompletedMessage message) /// { /// // Handle the message there /// } @@ -51,8 +51,8 @@ namespace Microsoft.Toolkit.Mvvm.Messaging /// /// Messenger.Default.Register<LoginCompletedMessage>(this, Receive); /// - /// The C# compiler will automatically convert that expression to an instance - /// compatible with the method. + /// The C# compiler will automatically convert that expression to a instance + /// compatible with the method. /// This will also work if multiple overloads of that method are available, each handling a different /// message type: the C# compiler will automatically pick the right one for the current message type. /// For info on the other available features, check the interface. From c45bb99dd30f27e2dfa2d6784d705ddfd040f182 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 17 Aug 2020 11:32:16 +0200 Subject: [PATCH 06/38] Updated unit tests --- .../Mvvm/Test_Messenger.Request.cs | 28 +++++++++---------- .../UnitTests.Shared/Mvvm/Test_Messenger.cs | 22 +++++++-------- .../Mvvm/Test_ObservableRecipient.cs | 4 +-- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs index 07fef8d9300..e47855a3c0f 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs @@ -20,7 +20,7 @@ public void Test_Messenger_RequestMessage_Ok() var messenger = new Messenger(); var recipient = new object(); - void Receive(NumberRequestMessage m) + void Receive(object recipient, NumberRequestMessage m) { Assert.IsFalse(m.HasReceivedResponse); @@ -54,7 +54,7 @@ public void Test_Messenger_RequestMessage_Fail_MultipleReplies() var messenger = new Messenger(); var recipient = new object(); - void Receive(NumberRequestMessage m) + void Receive(object recipient, NumberRequestMessage m) { m.Reply(42); m.Reply(42); @@ -76,7 +76,7 @@ public async Task Test_Messenger_AsyncRequestMessage_Ok_Sync() var messenger = new Messenger(); var recipient = new object(); - void Receive(AsyncNumberRequestMessage m) + void Receive(object recipient, AsyncNumberRequestMessage m) { Assert.IsFalse(m.HasReceivedResponse); @@ -106,7 +106,7 @@ async Task GetNumberAsync() return 42; } - void Receive(AsyncNumberRequestMessage m) + void Receive(object recipient, AsyncNumberRequestMessage m) { Assert.IsFalse(m.HasReceivedResponse); @@ -140,7 +140,7 @@ public async Task Test_Messenger_AsyncRequestMessage_Fail_MultipleReplies() var messenger = new Messenger(); var recipient = new object(); - void Receive(AsyncNumberRequestMessage m) + void Receive(object recipient, AsyncNumberRequestMessage m) { m.Reply(42); m.Reply(42); @@ -162,7 +162,7 @@ public void Test_Messenger_CollectionRequestMessage_Ok_NoReplies() var messenger = new Messenger(); var recipient = new object(); - void Receive(NumbersCollectionRequestMessage m) + void Receive(object recipient, NumbersCollectionRequestMessage m) { } @@ -183,9 +183,9 @@ public void Test_Messenger_CollectionRequestMessage_Ok_MultipleReplies() recipient2 = new object(), recipient3 = new object(); - void Receive1(NumbersCollectionRequestMessage m) => m.Reply(1); - void Receive2(NumbersCollectionRequestMessage m) => m.Reply(2); - void Receive3(NumbersCollectionRequestMessage m) => m.Reply(3); + void Receive1(object recipient, NumbersCollectionRequestMessage m) => m.Reply(1); + void Receive2(object recipient, NumbersCollectionRequestMessage m) => m.Reply(2); + void Receive3(object recipient, NumbersCollectionRequestMessage m) => m.Reply(3); messenger.Register(recipient1, Receive1); messenger.Register(recipient2, Receive2); @@ -212,7 +212,7 @@ public async Task Test_Messenger_AsyncCollectionRequestMessage_Ok_NoReplies() var messenger = new Messenger(); var recipient = new object(); - void Receive(AsyncNumbersCollectionRequestMessage m) + void Receive(object recipient, AsyncNumbersCollectionRequestMessage m) { } @@ -241,10 +241,10 @@ async Task GetNumberAsync() return 3; } - void Receive1(AsyncNumbersCollectionRequestMessage m) => m.Reply(1); - void Receive2(AsyncNumbersCollectionRequestMessage m) => m.Reply(Task.FromResult(2)); - void Receive3(AsyncNumbersCollectionRequestMessage m) => m.Reply(GetNumberAsync()); - void Receive4(AsyncNumbersCollectionRequestMessage m) => m.Reply(_ => GetNumberAsync()); + void Receive1(object recipient, AsyncNumbersCollectionRequestMessage m) => m.Reply(1); + void Receive2(object recipient, AsyncNumbersCollectionRequestMessage m) => m.Reply(Task.FromResult(2)); + void Receive3(object recipient, AsyncNumbersCollectionRequestMessage m) => m.Reply(GetNumberAsync()); + void Receive4(object recipient, AsyncNumbersCollectionRequestMessage m) => m.Reply(_ => GetNumberAsync()); messenger.Register(recipient1, Receive1); messenger.Register(recipient2, Receive2); diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs index 33d6e8cd19b..96818789f5e 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs @@ -58,7 +58,7 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageType() var messenger = new Messenger(); var recipient = new object(); - messenger.Register(recipient, m => { }); + messenger.Register(recipient, (r, m) => { }); messenger.Unregister(recipient); @@ -72,7 +72,7 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageTypeAndToken var messenger = new Messenger(); var recipient = new object(); - messenger.Register(recipient, nameof(MessageA), m => { }); + messenger.Register(recipient, nameof(MessageA), (r, m) => { }); messenger.Unregister(recipient, nameof(MessageA)); @@ -86,7 +86,7 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithToken() var messenger = new Messenger(); var recipient = new object(); - messenger.Register(recipient, nameof(MessageA), m => { }); + messenger.Register(recipient, nameof(MessageA), (r, m) => { }); messenger.UnregisterAll(recipient, nameof(MessageA)); @@ -100,7 +100,7 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithRecipient() var messenger = new Messenger(); var recipient = new object(); - messenger.Register(recipient, nameof(MessageA), m => { }); + messenger.Register(recipient, nameof(MessageA), (r, m) => { }); messenger.UnregisterAll(recipient); @@ -116,7 +116,7 @@ public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithN Assert.IsFalse(Messenger.Default.IsRegistered(a)); string result = null; - Messenger.Default.Register(a, m => result = m.Text); + Messenger.Default.Register(a, (r, m) => result = m.Text); Assert.IsTrue(Messenger.Default.IsRegistered(a)); @@ -143,7 +143,7 @@ public void Test_Messenger_IsRegistered_Register_Send_UnregisterRecipient_WithNo Assert.IsFalse(Messenger.Default.IsRegistered(a)); string result = null; - Messenger.Default.Register(a, m => result = m.Text); + Messenger.Default.Register(a, (r, m) => result = m.Text); Assert.IsTrue(Messenger.Default.IsRegistered(a)); @@ -170,7 +170,7 @@ public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithT Assert.IsFalse(Messenger.Default.IsRegistered(a)); string result = null; - Messenger.Default.Register(a, nameof(MessageA), m => result = m.Text); + Messenger.Default.Register(a, nameof(MessageA), (r, m) => result = m.Text); Assert.IsTrue(Messenger.Default.IsRegistered(a, nameof(MessageA))); @@ -195,11 +195,11 @@ public void Test_Messenger_DuplicateRegistrationWithMessageType() var messenger = new Messenger(); var recipient = new object(); - messenger.Register(recipient, m => { }); + messenger.Register(recipient, (r, m) => { }); Assert.ThrowsException(() => { - messenger.Register(recipient, m => { }); + messenger.Register(recipient, (r, m) => { }); }); } @@ -210,11 +210,11 @@ public void Test_Messenger_DuplicateRegistrationWithMessageTypeAndToken() var messenger = new Messenger(); var recipient = new object(); - messenger.Register(recipient, nameof(MessageA), m => { }); + messenger.Register(recipient, nameof(MessageA), (r, m) => { }); Assert.ThrowsException(() => { - messenger.Register(recipient, nameof(MessageA), m => { }); + messenger.Register(recipient, nameof(MessageA), (r, m) => { }); }); } diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs index d5e330e1e53..76ff145d86f 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs @@ -59,7 +59,7 @@ public void Test_ObservableRecipient_Broadcast() PropertyChangedMessage message = null; - messenger.Register>(messenger, m => message = m); + messenger.Register>(messenger, (r, m) => message = m); viewmodel.Data = 42; @@ -97,7 +97,7 @@ protected override void OnActivated() { IsActivatedCheck = true; - Messenger.Register(this, m => { }); + Messenger.Register(this, (r, m) => { }); } protected override void OnDeactivated() From eef4d0af8e809fda7d3a7dc5a57ca64ef90337df Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 17 Aug 2020 12:50:27 +0200 Subject: [PATCH 07/38] Fixed an index exception in Messenger.Send --- Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index 01da10a831c..e85f1ef4bc2 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -6,7 +6,6 @@ using System.Buffers; using System.Collections.Generic; using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; using System.Threading; using Microsoft.Collections.Extensions; @@ -404,17 +403,19 @@ public unsafe TMessage Send(TMessage message, TToken token) lock (this.recipientsMap) { // Check whether there are any registered recipients - if (!TryGetMapping(out Mapping? mapping)) - { - return message; - } + _ = TryGetMapping(out Mapping? mapping); // We need to make a local copy of the currently registered handlers, // since users might try to unregister (or register) new handlers from // inside one of the currently existing handlers. We can use memory pooling // to reuse arrays, to minimize the average memory usage. In practice, // we usually just need to pay the small overhead of copying the items. - int totalHandlersCount = mapping!.TotalHandlersCount; + int totalHandlersCount = mapping?.TotalHandlersCount ?? 0; + + if (totalHandlersCount == 0) + { + return message; + } handlers = ArrayPool.Shared.Rent(totalHandlersCount); recipients = ArrayPool.Shared.Rent(totalHandlersCount); @@ -428,7 +429,7 @@ public unsafe TMessage Send(TMessage message, TToken token) // handlers for different tokens. We can reuse the same variable // to count the number of matching handlers to invoke later on. // This will be the array slice with valid actions in the rented buffer. - var mappingEnumerator = mapping.GetEnumerator(); + var mappingEnumerator = mapping!.GetEnumerator(); // Explicit enumerator usage here as we're using a custom one // that doesn't expose the single standard Current property. From dea8b8913f27bf56c107a24057f9deaa09ab7287 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 17 Aug 2020 13:17:15 +0200 Subject: [PATCH 08/38] Tweaked XML docs and comments --- Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs | 4 ++-- Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs index d1a6e1513df..a3838d2b117 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs @@ -43,9 +43,9 @@ bool IsRegistered(object recipient, TToken token) /// The type of token to use to pick the messages to receive. /// The recipient that will receive the messages. /// A token used to determine the receiving channel to use. - /// The to invoke when a message is received. + /// The to invoke when a message is received. /// Thrown when trying to register the same message twice. - void Register(object recipient, TToken token, MessageHandler action) + void Register(object recipient, TToken token, MessageHandler handler) where TMessage : class where TToken : IEquatable; diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index e85f1ef4bc2..8ead8d4b73b 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -36,7 +36,7 @@ namespace Microsoft.Toolkit.Mvvm.Messaging /// /// Messenger.Default.Send<LoginCompletedMessage>(); /// - /// Additionally, the method group syntax can also be used to specify the action + /// Additionally, the method group syntax can also be used to specify the message handler /// to invoke when receiving a message, if a method with the right signature is available /// in the current scope. This is helpful to keep the registration and handling logic separate. /// Following up from the previous example, consider a class having this method: @@ -137,7 +137,7 @@ public bool IsRegistered(object recipient, TToken token) } /// - public void Register(object recipient, TToken token, MessageHandler action) + public void Register(object recipient, TToken token, MessageHandler handler) where TMessage : class where TToken : IEquatable { @@ -151,14 +151,14 @@ public void Register(object recipient, TToken token, MessageHa map ??= new DictionarySlim>(); // Add the new registration entry - ref MessageHandler? handler = ref map.GetOrAddValueRef(token); + ref MessageHandler? registeredHandler = ref map.GetOrAddValueRef(token); - if (!(handler is null)) + if (!(registeredHandler is null)) { ThrowInvalidOperationExceptionForDuplicateRegistration(); } - handler = action; + registeredHandler = handler; // Update the total counter for handlers for the current type parameters mapping.TotalHandlersCount++; @@ -428,7 +428,7 @@ public unsafe TMessage Send(TMessage message, TToken token) // The array is oversized at this point, since it also includes // handlers for different tokens. We can reuse the same variable // to count the number of matching handlers to invoke later on. - // This will be the array slice with valid actions in the rented buffer. + // This will be the array slice with valid handler in the rented buffer. var mappingEnumerator = mapping!.GetEnumerator(); // Explicit enumerator usage here as we're using a custom one From 02790ab33f862981bf641375d9924d64d4c78686 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 17 Aug 2020 23:33:55 +0200 Subject: [PATCH 09/38] Added IMessenger.Cleanup API (to help with extensibility) --- Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs | 8 ++++++++ Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 10 ++++++++++ 2 files changed, 18 insertions(+) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs index a3838d2b117..e046d056c07 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs @@ -94,6 +94,14 @@ TMessage Send(TMessage message, TToken token) where TMessage : class where TToken : IEquatable; + /// + /// Performs a cleanup on the current messenger. + /// Invoking this method does not unregister any of the currently registered + /// recipient, and it can be used to perform cleanup operations such as + /// trimming the internal data structures of a messenger implementation. + /// + void Cleanup(); + /// /// Resets the instance and unregisters all the existing recipients. /// diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index 8ead8d4b73b..f352fa364b2 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -480,6 +480,16 @@ public unsafe TMessage Send(TMessage message, TToken token) return message; } + /// + void IMessenger.Cleanup() + { + // The current implementation doesn't require any kind of cleanup operation, as + // all the internal data structures are already kept in sync whenever a recipient + // is added or removed. This method is implemented through an explicit interface + // implementation so that developers using this type directly will not see it in + // the API surface (as it wouldn't be useful anyway, since it's a no-op here). + } + /// public void Reset() { From e514869295465859e56ca720ecc94ee353b9911c Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 25 Aug 2020 12:13:37 +0200 Subject: [PATCH 10/38] Added missing contravariant type argument modifier --- Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs index e046d056c07..f008ba14a5a 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs @@ -16,7 +16,7 @@ namespace Microsoft.Toolkit.Mvvm.Messaging /// The type of message to receive. /// The recipient that is receiving the message. /// The message being received. - public delegate void MessageHandler(object recipient, TMessage message); + public delegate void MessageHandler(object recipient, TMessage message); /// /// An interface for a type providing the ability to exchange messages between different objects. From d86f7948e19233433d97a84c1e3b255bb4887d84 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sat, 5 Sep 2020 16:45:47 +0200 Subject: [PATCH 11/38] Added MessageHandler "covariance" --- .../Messaging/IMessenger.cs | 11 ++-- Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 50 ++++++++++++------- .../Messaging/MessengerExtensions.cs | 42 ++++++++++++++-- .../UnitTests.Shared/Mvvm/Test_Messenger.cs | 17 +++++++ 4 files changed, 94 insertions(+), 26 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs index f008ba14a5a..006ce132141 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs @@ -13,10 +13,13 @@ namespace Microsoft.Toolkit.Mvvm.Messaging /// closures: if an instance method on a recipient needs to be invoked it is possible to just /// cast the recipient to the right type and then access the local method from that instance. /// + /// The type of recipient for the message. /// The type of message to receive. /// The recipient that is receiving the message. /// The message being received. - public delegate void MessageHandler(object recipient, TMessage message); + public delegate void MessageHandler(TRecipient recipient, TMessage message) + where TRecipient : class + where TMessage : class; /// /// An interface for a type providing the ability to exchange messages between different objects. @@ -39,13 +42,15 @@ bool IsRegistered(object recipient, TToken token) /// /// Registers a recipient for a given type of message. /// + /// The type of recipient for the message. /// The type of message to receive. /// The type of token to use to pick the messages to receive. /// The recipient that will receive the messages. /// A token used to determine the receiving channel to use. - /// The to invoke when a message is received. + /// The to invoke when a message is received. /// Thrown when trying to register the same message twice. - void Register(object recipient, TToken token, MessageHandler handler) + void Register(TRecipient recipient, TToken token, MessageHandler handler) + where TRecipient : class where TMessage : class where TToken : IEquatable; diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index f352fa364b2..60a3003e36d 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -50,8 +50,8 @@ namespace Microsoft.Toolkit.Mvvm.Messaging /// /// Messenger.Default.Register<LoginCompletedMessage>(this, Receive); /// - /// The C# compiler will automatically convert that expression to a instance - /// compatible with the method. + /// The C# compiler will automatically convert that expression to a instance + /// compatible with . /// This will also work if multiple overloads of that method are available, each handling a different /// message type: the C# compiler will automatically pick the right one for the current message type. /// For info on the other available features, check the interface. @@ -66,18 +66,23 @@ public sealed class Messenger : IMessenger // | ________(recipients registrations)___________\________/ / __/ // | / _______(channel registrations)_____\___________________/ / // | / / \ / - // DictionarySlim>> mapping = Mapping - // / / \ / / - // ___(Type2.tToken)____/ / \______/___________________/ + // DictionarySlim>> mapping = Mapping + // / / \ / / + // ___(Type2.tToken)____/ / \______/_________________________/ // /________________(Type2.tMessage)____/ / // / ________________________________________/ // / / // DictionarySlim typesMap; // -------------------------------------------------------------------------------------------------------- - // Each combination of results in a concrete Mapping type, which holds - // the references from registered recipients to handlers. The handlers are stored in a > - // dictionary, so that each recipient can have up to one registered handler for a given token, for each - // message type. Each mapping is stored in the types map, which associates each pair of concrete types to its + // Each combination of results in a concrete Mapping type, which holds the references + // from registered recipients to handlers. The handlers are stored in a > + // dictionary, so that each recipient can have up to one registered handler for a given token, for each message type. + // Note that the registered handlers are unsafe-cast to a MessageHandler instance even if they were + // actually of type MessageHandler. This allows the messenger to track and invoke type-specific + // handlers without using reflection and without having to capture the input handler in a proxy delegate, causing one + // extra memory allocations and adding overhead. The type conversion is guaranteed to be respected due to how the + // messenger type itself works - as registered handlers are always invoked on their respective recipients. + // Each mapping is stored in the types map, which associates each pair of concrete types to its // mapping instance. Mapping instances are exposed as IMapping items, as each will be a closed type over // a different combination of TMessage and TToken generic type parameters. Each existing recipient is also stored in // the main recipients map, along with a set of all the existing dictionaries of handlers for that recipient (for all @@ -137,7 +142,8 @@ public bool IsRegistered(object recipient, TToken token) } /// - public void Register(object recipient, TToken token, MessageHandler handler) + public void Register(TRecipient recipient, TToken token, MessageHandler handler) + where TRecipient : class where TMessage : class where TToken : IEquatable { @@ -146,19 +152,20 @@ public void Register(object recipient, TToken token, MessageHa // Get the registration list for this recipient Mapping mapping = GetOrAddMapping(); var key = new Recipient(recipient); - ref DictionarySlim>? map = ref mapping.GetOrAddValueRef(key); + ref DictionarySlim>? map = ref mapping.GetOrAddValueRef(key); - map ??= new DictionarySlim>(); + map ??= new DictionarySlim>(); // Add the new registration entry - ref MessageHandler? registeredHandler = ref map.GetOrAddValueRef(token); + ref MessageHandler? registeredHandler = ref map.GetOrAddValueRef(token); if (!(registeredHandler is null)) { ThrowInvalidOperationExceptionForDuplicateRegistration(); } - registeredHandler = handler; + // Treat the input delegate as if it was covariant (see comments below in the Send method) + registeredHandler = Unsafe.As>(handler); // Update the total counter for handlers for the current type parameters mapping.TotalHandlersCount++; @@ -356,7 +363,7 @@ public void Unregister(object recipient, TToken token) var key = new Recipient(recipient); - if (!mapping!.TryGetValue(key, out DictionarySlim>? dictionary)) + if (!mapping!.TryGetValue(key, out DictionarySlim>? dictionary)) { return; } @@ -461,9 +468,14 @@ public unsafe TMessage Send(TMessage message, TToken token) { // We're doing an unsafe cast to skip the type checks again. // See the comments in the UnregisterAll method for more info. - MessageHandler handler = Unsafe.As>(Unsafe.Add(ref handlersRef, (IntPtr)(void*)(uint)j)); - - handler(Unsafe.Add(ref recipientsRef, (IntPtr)(void*)(uint)j), message); + object handler = Unsafe.Add(ref handlersRef, (IntPtr)(void*)(uint)j); + object recipient = Unsafe.Add(ref recipientsRef, (IntPtr)(void*)(uint)j); + + // Here we perform an unsafe cast to enable covariance for delegate types. + // We know that the input recipient will always respect the type constraints + // of each original input delegate, and doing so allows us to still invoke + // them all from here without worrying about specific generic type arguments. + Unsafe.As>(handler)(recipient, message); } } finally @@ -559,7 +571,7 @@ private Mapping GetOrAddMapping() /// This type is defined for simplicity and as a workaround for the lack of support for using type aliases /// over open generic types in C# (using type aliases can only be used for concrete, closed types). /// - private sealed class Mapping : DictionarySlim>>, IMapping + private sealed class Mapping : DictionarySlim>>, IMapping where TMessage : class where TToken : IEquatable { diff --git a/Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.cs b/Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.cs index 5e2473db331..2a6a0383bec 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.cs @@ -174,7 +174,7 @@ static Action GetRegistrationAction(Type type, Metho public static void Register(this IMessenger messenger, IRecipient recipient) where TMessage : class { - messenger.Register(recipient, default, (r, m) => Unsafe.As>(r).Receive(m)); + messenger.Register, TMessage, Unit>(recipient, default, (r, m) => r.Receive(m)); } /// @@ -191,7 +191,7 @@ public static void Register(this IMessenger messenger, IRecipi where TMessage : class where TToken : IEquatable { - messenger.Register(recipient, token, (r, m) => Unsafe.As>(r).Receive(m)); + messenger.Register, TMessage, TToken>(recipient, token, (r, m) => r.Receive(m)); } /// @@ -200,15 +200,49 @@ public static void Register(this IMessenger messenger, IRecipi /// The type of message to receive. /// The instance to use to register the recipient. /// The recipient that will receive the messages. - /// The to invoke when a message is received. + /// The to invoke when a message is received. /// Thrown when trying to register the same message twice. /// This method will use the default channel to perform the requested registration. - public static void Register(this IMessenger messenger, object recipient, MessageHandler handler) + public static void Register(this IMessenger messenger, object recipient, MessageHandler handler) where TMessage : class { messenger.Register(recipient, default(Unit), handler); } + /// + /// Registers a recipient for a given type of message. + /// + /// The type of recipient for the message. + /// The type of message to receive. + /// The instance to use to register the recipient. + /// The recipient that will receive the messages. + /// The to invoke when a message is received. + /// Thrown when trying to register the same message twice. + /// This method will use the default channel to perform the requested registration. + public static void Register(this IMessenger messenger, TRecipient recipient, MessageHandler handler) + where TRecipient : class + where TMessage : class + { + messenger.Register(recipient, default(Unit), handler); + } + + /// + /// Registers a recipient for a given type of message. + /// + /// The type of message to receive. + /// The type of token to use to pick the messages to receive. + /// The instance to use to register the recipient. + /// The recipient that will receive the messages. + /// A token used to determine the receiving channel to use. + /// The to invoke when a message is received. + /// Thrown when trying to register the same message twice. + public static void Register(this IMessenger messenger, object recipient, TToken token, MessageHandler handler) + where TMessage : class + where TToken : IEquatable + { + messenger.Register(recipient, token, handler); + } + /// /// Unregisters a recipient from messages of a given type. /// diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs index 96818789f5e..41b073e3f81 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs @@ -297,8 +297,25 @@ public void Test_Messenger_IRecipient_SomeMessages_WithToken() Assert.IsFalse(messenger.IsRegistered(recipient)); } + [TestCategory("Mvvm")] + [TestMethod] + public void Test_Messenger_RegisterWithTypeParameter() + { + var messenger = new Messenger(); + var recipient = new RecipientWithNoMessages { Number = 42 }; + + int number = 0; + + messenger.Register(recipient, (r, m) => number = r.Number); + + messenger.Send(); + + Assert.AreEqual(number, 42); + } + public sealed class RecipientWithNoMessages { + public int Number { get; set; } } public sealed class RecipientWithSomeMessages From af91276723f5c877c3986bec61f7df9035f36f5d Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 6 Sep 2020 19:50:09 +0200 Subject: [PATCH 12/38] Tweaked IMessenger and Messenger XML docs --- .../Messaging/IMessenger.cs | 48 +++++++++++++++++++ Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 48 ++----------------- 2 files changed, 53 insertions(+), 43 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs index 006ce132141..9968d79cb2c 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs @@ -23,6 +23,54 @@ public delegate void MessageHandler(TRecipient recip /// /// An interface for a type providing the ability to exchange messages between different objects. + /// This can be useful to decouple different modules of an application without having to keep strong + /// references to types being referenced. It is also possible to send messages to specific channels, uniquely + /// identified by a token, and to have different messengers in different sections of an applications. + /// In order to use the functionalities, first define a message type, like so: + /// + /// public sealed class LoginCompletedMessage { } + /// + /// Then, register your a recipient for this message: + /// + /// Messenger.Default.Register<MyRecipientType, LoginCompletedMessage>(this, (r, m) => + /// { + /// // Handle the message here... + /// }); + /// + /// The message handler here is a lambda expression taking two parameters: the recipient and the message. + /// This is done to avoid the allocations for the closures that would've been generated if the expression + /// had captured the current instance. The recipient type parameter is used so that the recipient can be + /// directly accessed within the handler without the need to manually perform type casts. This allows the + /// code to be less verbose and more reliable, as all the checks are done just at build time. If the handler + /// is defined within the same type as the recipient, it is also possible to directly access private members. + /// This allows the message handler to be a static method, which enables the C# compiler to perform a number + /// of additional memory optimizations (such as caching the delegate, avoiding unnecessary memory allocations). + /// Finally, send a message when needed, like so: + /// + /// Messenger.Default.Send<LoginCompletedMessage>(); + /// + /// Additionally, the method group syntax can also be used to specify the message handler + /// to invoke when receiving a message, if a method with the right signature is available + /// in the current scope. This is helpful to keep the registration and handling logic separate. + /// Following up from the previous example, consider a class having this method: + /// + /// private static void Receive(MyRecipientType recipient, LoginCompletedMessage message) + /// { + /// // Handle the message there + /// } + /// + /// The registration can then be performed in a single line like so: + /// + /// Messenger.Default.Register(this, Receive); + /// + /// The C# compiler will automatically convert that expression to a instance + /// compatible with . + /// This will also work if multiple overloads of that method are available, each handling a different + /// message type: the C# compiler will automatically pick the right one for the current message type. + /// It is also possible to register message handlers explicitly using the interface. + /// To do so, the recipient just needs to implement the interface and then call the + /// extension, which will automatically register + /// all the handlers that are declared by the recipient type. Registration for individual handlers is supported as well. /// public interface IMessenger { diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index 60a3003e36d..5e6d4728ae1 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -12,50 +12,12 @@ namespace Microsoft.Toolkit.Mvvm.Messaging { /// - /// A type that can be used to exchange messages between different objects. - /// This can be useful to decouple different modules of an application without having to keep strong - /// references to types being referenced. It is also possible to send messages to specific channels, uniquely - /// identified by a token, and to have different messengers in different sections of an applications. - /// In order to use the functionalities, first define a message type, like so: - /// - /// public sealed class LoginCompletedMessage { } - /// - /// Then, register your a recipient for this message: - /// - /// Messenger.Default.Register<LoginCompletedMessage>(this, (r, m) => - /// { - /// // Handle the message here... - /// }); - /// - /// The message handler here is a lambda expression taking two parameters: the recipient and the message. - /// This is done to avoid the allocations for the closures that would've been generated if the expression - /// had captured the current instance - instead it is possible to just cast the recipient to the right type - /// and access private instance members from the handler directly. This allows the message handler to be a - /// static method, which enables the C# to perform a number of additional memory optimizations. - /// Finally, send a message when needed, like so: - /// - /// Messenger.Default.Send<LoginCompletedMessage>(); - /// - /// Additionally, the method group syntax can also be used to specify the message handler - /// to invoke when receiving a message, if a method with the right signature is available - /// in the current scope. This is helpful to keep the registration and handling logic separate. - /// Following up from the previous example, consider a class having this method: - /// - /// private static void Receive(object recipient, LoginCompletedMessage message) - /// { - /// // Handle the message there - /// } - /// - /// The registration can then be performed in a single line like so: - /// - /// Messenger.Default.Register<LoginCompletedMessage>(this, Receive); - /// - /// The C# compiler will automatically convert that expression to a instance - /// compatible with . - /// This will also work if multiple overloads of that method are available, each handling a different - /// message type: the C# compiler will automatically pick the right one for the current message type. - /// For info on the other available features, check the interface. + /// A class providing a reference implementation for the interface. /// + /// + /// This implementation uses strong references to track the registered + /// recipients, so it is necessary to manually unregister them when they're no longer needed. + /// public sealed class Messenger : IMessenger { // The Messenger class uses the following logic to link stored instances together: From 3293fa6da031afccc98a53b4800be3f376667e02 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 8 Sep 2020 00:31:58 +0200 Subject: [PATCH 13/38] Minor code refactoring --- Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 46 ++++++++++--------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index 5e6d4728ae1..c93d12430f3 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -24,26 +24,28 @@ public sealed class Messenger : IMessenger // -------------------------------------------------------------------------------------------------------- // DictionarySlim> recipientsMap; // | \________________[*]IDictionarySlim> - // | \___ / / / - // | ________(recipients registrations)___________\________/ / __/ - // | / _______(channel registrations)_____\___________________/ / - // | / / \ / - // DictionarySlim>> mapping = Mapping - // / / \ / / - // ___(Type2.tToken)____/ / \______/_________________________/ - // /________________(Type2.tMessage)____/ / - // / ________________________________________/ + // | \____________/_________ / + // | ________(recipients registrations)____________________/ \ / + // | / ____(channel registrations)________________\____________/ + // | / / \ + // DictionarySlim>> mapping = Mapping + // / / / + // ___(Type2.tToken)____/ / / + // /________________(Type2.tMessage)________________________/ / + // / ____________________________________________________________/ // / / // DictionarySlim typesMap; // -------------------------------------------------------------------------------------------------------- // Each combination of results in a concrete Mapping type, which holds the references - // from registered recipients to handlers. The handlers are stored in a > - // dictionary, so that each recipient can have up to one registered handler for a given token, for each message type. - // Note that the registered handlers are unsafe-cast to a MessageHandler instance even if they were - // actually of type MessageHandler. This allows the messenger to track and invoke type-specific - // handlers without using reflection and without having to capture the input handler in a proxy delegate, causing one - // extra memory allocations and adding overhead. The type conversion is guaranteed to be respected due to how the - // messenger type itself works - as registered handlers are always invoked on their respective recipients. + // from registered recipients to handlers. The handlers are stored in a > dictionary, + // so that each recipient can have up to one registered handler for a given token, for each message type. + // Note that the registered handlers are only stored as object references, even if they were actually of type + // MessageHandler, to avoid unnecessary unsafe casts. Each handler is also generic with respect to the + // recipient type, in order to allow the messenger to track and invoke type-specific handlers without using reflection and + // without having to capture the input handler in a proxy delegate, causing one extra memory allocations and adding overhead. + // This allows users to retain type information on each registered recipient, instead of having to manually cast each recipient + // to the right type within the handler. The type conversion is guaranteed to be respected due to how the messenger type + // itself works - as registered handlers are always invoked on their respective recipients. // Each mapping is stored in the types map, which associates each pair of concrete types to its // mapping instance. Mapping instances are exposed as IMapping items, as each will be a closed type over // a different combination of TMessage and TToken generic type parameters. Each existing recipient is also stored in @@ -114,12 +116,12 @@ public void Register(TRecipient recipient, TToken // Get the registration list for this recipient Mapping mapping = GetOrAddMapping(); var key = new Recipient(recipient); - ref DictionarySlim>? map = ref mapping.GetOrAddValueRef(key); + ref DictionarySlim? map = ref mapping.GetOrAddValueRef(key); - map ??= new DictionarySlim>(); + map ??= new DictionarySlim(); // Add the new registration entry - ref MessageHandler? registeredHandler = ref map.GetOrAddValueRef(token); + ref object? registeredHandler = ref map.GetOrAddValueRef(token); if (!(registeredHandler is null)) { @@ -127,7 +129,7 @@ public void Register(TRecipient recipient, TToken } // Treat the input delegate as if it was covariant (see comments below in the Send method) - registeredHandler = Unsafe.As>(handler); + registeredHandler = handler; // Update the total counter for handlers for the current type parameters mapping.TotalHandlersCount++; @@ -325,7 +327,7 @@ public void Unregister(object recipient, TToken token) var key = new Recipient(recipient); - if (!mapping!.TryGetValue(key, out DictionarySlim>? dictionary)) + if (!mapping!.TryGetValue(key, out DictionarySlim? dictionary)) { return; } @@ -533,7 +535,7 @@ private Mapping GetOrAddMapping() /// This type is defined for simplicity and as a workaround for the lack of support for using type aliases /// over open generic types in C# (using type aliases can only be used for concrete, closed types). /// - private sealed class Mapping : DictionarySlim>>, IMapping + private sealed class Mapping : DictionarySlim>, IMapping where TMessage : class where TToken : IEquatable { From 93456ae0b72dbf51eda4ebe2b3380062af019ddd Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 10 Sep 2020 19:33:14 +0200 Subject: [PATCH 14/38] Added tests for correct recipients --- .../Mvvm/Test_Messenger.Request.cs | 33 ++++++++++++++++--- .../UnitTests.Shared/Mvvm/Test_Messenger.cs | 12 ++++++- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs index e47855a3c0f..9a4329e06d3 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs @@ -19,9 +19,12 @@ public void Test_Messenger_RequestMessage_Ok() { var messenger = new Messenger(); var recipient = new object(); + object test = null; void Receive(object recipient, NumberRequestMessage m) { + test = recipient; + Assert.IsFalse(m.HasReceivedResponse); m.Reply(42); @@ -33,6 +36,7 @@ void Receive(object recipient, NumberRequestMessage m) int result = messenger.Send(); + Assert.AreSame(test, recipient); Assert.AreEqual(result, 42); } @@ -181,11 +185,28 @@ public void Test_Messenger_CollectionRequestMessage_Ok_MultipleReplies() object recipient1 = new object(), recipient2 = new object(), - recipient3 = new object(); + recipient3 = new object(), + r1 = null, + r2 = null, + r3 = null; - void Receive1(object recipient, NumbersCollectionRequestMessage m) => m.Reply(1); - void Receive2(object recipient, NumbersCollectionRequestMessage m) => m.Reply(2); - void Receive3(object recipient, NumbersCollectionRequestMessage m) => m.Reply(3); + void Receive1(object recipient, NumbersCollectionRequestMessage m) + { + r1 = recipient; + m.Reply(1); + } + + void Receive2(object recipient, NumbersCollectionRequestMessage m) + { + r2 = recipient; + m.Reply(2); + } + + void Receive3(object recipient, NumbersCollectionRequestMessage m) + { + r3 = recipient; + m.Reply(3); + } messenger.Register(recipient1, Receive1); messenger.Register(recipient2, Receive2); @@ -198,6 +219,10 @@ public void Test_Messenger_CollectionRequestMessage_Ok_MultipleReplies() responses.Add(response); } + Assert.AreSame(r1, recipient1); + Assert.AreSame(r2, recipient2); + Assert.AreSame(r3, recipient3); + CollectionAssert.AreEquivalent(responses, new[] { 1, 2, 3 }); } diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs index 41b073e3f81..9e83f85ab4b 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs @@ -115,22 +115,32 @@ public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithN Assert.IsFalse(Messenger.Default.IsRegistered(a)); + object recipient = null; string result = null; - Messenger.Default.Register(a, (r, m) => result = m.Text); + + Messenger.Default.Register(a, (r, m) => + { + recipient = r; + result = m.Text; + }); Assert.IsTrue(Messenger.Default.IsRegistered(a)); Messenger.Default.Send(new MessageA { Text = nameof(MessageA) }); + Assert.AreSame(recipient, a); Assert.AreEqual(result, nameof(MessageA)); Messenger.Default.Unregister(a); Assert.IsFalse(Messenger.Default.IsRegistered(a)); + recipient = null; result = null; + Messenger.Default.Send(new MessageA { Text = nameof(MessageA) }); + Assert.IsNull(recipient); Assert.IsNull(result); } From b0f2edd2e3ae157c58a0dd94fa92bd9b99cf1732 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sat, 12 Sep 2020 02:32:18 +0200 Subject: [PATCH 15/38] Fixed accidental O(logn) to O(n) lookup slowdown --- Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index c93d12430f3..82bc094ea38 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -394,8 +394,6 @@ public unsafe TMessage Send(TMessage message, TToken token) recipientsRef = ref recipients[0]; // Copy the handlers to the local collection. - // Both types being enumerate expose a struct enumerator, - // so we're not actually allocating the enumerator here. // The array is oversized at this point, since it also includes // handlers for different tokens. We can reuse the same variable // to count the number of matching handlers to invoke later on. @@ -407,20 +405,14 @@ public unsafe TMessage Send(TMessage message, TToken token) while (mappingEnumerator.MoveNext()) { object recipient = mappingEnumerator.Key.Target; - var pairsEnumerator = mappingEnumerator.Value.GetEnumerator(); - while (pairsEnumerator.MoveNext()) + // Pick the target handler, if the token is a match for the recipient + if (mappingEnumerator.Value.TryGetValue(token, out object? handler)) { - // Only select the ones with a matching token - if (pairsEnumerator.Key.Equals(token)) - { - // We spend quite a bit of time in these two busy loops as we go through all the - // existing mappings and registrations to find the handlers we're interested in. - // We can manually offset here to skip the bounds checks in this inner loop when - // indexing the array (the size is already verified and guaranteed to be enough). - Unsafe.Add(ref handlersRef, (IntPtr)(void*)(uint)i) = pairsEnumerator.Value; - Unsafe.Add(ref recipientsRef, (IntPtr)(void*)(uint)i++) = recipient; - } + // We can manually offset here to skip the bounds checks in this inner loop when + // indexing the array (the size is already verified and guaranteed to be enough). + Unsafe.Add(ref handlersRef, (IntPtr)(void*)(uint)i) = handler!; + Unsafe.Add(ref recipientsRef, (IntPtr)(void*)(uint)i++) = recipient; } } } From 89efbea051e85489350d46cd61c4c814d152b517 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sat, 12 Sep 2020 02:40:33 +0200 Subject: [PATCH 16/38] Small code refactoring and improvements --- Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 43 ++++++------------- 1 file changed, 12 insertions(+), 31 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index 82bc094ea38..6f6bf62a047 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -131,9 +131,6 @@ public void Register(TRecipient recipient, TToken // Treat the input delegate as if it was covariant (see comments below in the Send method) registeredHandler = handler; - // Update the total counter for handlers for the current type parameters - mapping.TotalHandlersCount++; - // Make sure this registration map is tracked for the current recipient ref HashSet? set = ref this.recipientsMap.GetOrAddValueRef(key); @@ -171,8 +168,6 @@ public void UnregisterAll(object recipient) // The handlers map is the IDictionary instance for the mapping. int handlersCount = Unsafe.As(handlersMap).Count; - mapping.TotalHandlersCount -= handlersCount; - if (mapping.Count == 0) { // Maps here are really of type Mapping<,> and with unknown type arguments. @@ -259,15 +254,6 @@ public void UnregisterAll(object recipient, TToken token) // for the current message type (unknown from here). if (holder.Remove(token)) { - // As above, we need to update the total number of registered handlers for the map. - // In this case we also know that the current TToken type parameter is of interest - // for the current method, as we're only unsubscribing handlers using that token. - // This is because we're already working on the final mapping, - // which associates a single handler with a given token, for a given recipient. - // This means that we don't have to retrieve the count to subtract in this case, - // we're just removing a single handler at a time. So, we just decrement the total. - Unsafe.As(map).TotalHandlersCount--; - if (holder.Count == 0) { // If the map is empty, remove the recipient entirely from its container @@ -335,9 +321,6 @@ public void Unregister(object recipient, TToken token) // Remove the target handler if (dictionary!.Remove(token)) { - // Decrement the total count, as above - mapping.TotalHandlersCount--; - // If the map is empty, it means that the current recipient has no remaining // registered handlers for the current combination, regardless, // of the specific token value (ie. the channel used to receive messages of that type). @@ -376,12 +359,18 @@ public unsafe TMessage Send(TMessage message, TToken token) // Check whether there are any registered recipients _ = TryGetMapping(out Mapping? mapping); - // We need to make a local copy of the currently registered handlers, - // since users might try to unregister (or register) new handlers from - // inside one of the currently existing handlers. We can use memory pooling - // to reuse arrays, to minimize the average memory usage. In practice, - // we usually just need to pay the small overhead of copying the items. - int totalHandlersCount = mapping?.TotalHandlersCount ?? 0; + // We need to make a local copy of the currently registered handlers, since users might + // try to unregister (or register) new handlers from inside one of the currently existing + // handlers. We can use memory pooling to reuse arrays, to minimize the average memory + // usage. In practice, we usually just need to pay the small overhead of copying the items. + // The current mapping contains all the currently registered recipients and handlers for + // the combination in use. In the worst case scenario, all recipients + // will have a registered handler with a token matching the input one, meaning that we could + // have at worst a number of pending handlers to invoke equal to the total number of recipient + // in the mapping. This relies on the fact that tokens are unique, and that there is only + // one handler associated with a given token. We can use this upper bound as the requested + // size for each array rented from the pool, which guarantees that we'll have enough space. + int totalHandlersCount = mapping?.Count ?? 0; if (totalHandlersCount == 0) { @@ -541,9 +530,6 @@ public Mapping() /// public Type2 TypeArguments { get; } - - /// - public int TotalHandlersCount { get; set; } } /// @@ -556,11 +542,6 @@ private interface IMapping : IDictionarySlim /// Gets the instance representing the current type arguments. /// Type2 TypeArguments { get; } - - /// - /// Gets or sets the total number of handlers in the current instance. - /// - int TotalHandlersCount { get; set; } } /// From cf515e6cbabf16a25084886528aff77a6a5b87dd Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sat, 12 Sep 2020 15:34:07 +0200 Subject: [PATCH 17/38] Added WeakRefMessenger type --- .../DictionarySlim{TKey,TValue}.cs | 2 +- .../IDictionarySlim{TKey}.cs | 9 +- .../Messaging/WeakRefMessenger.cs | 484 ++++++++++++++++++ 3 files changed, 493 insertions(+), 2 deletions(-) create mode 100644 Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs b/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs index ab4706de5f7..b171a8a17be 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs @@ -130,7 +130,7 @@ public void Clear() this.entries = InitialEntries; } - /// + /// public bool ContainsKey(TKey key) { Entry[] entries = this.entries; diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/IDictionarySlim{TKey}.cs b/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/IDictionarySlim{TKey}.cs index 7de2505148a..efb201ffbc0 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/IDictionarySlim{TKey}.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/IDictionarySlim{TKey}.cs @@ -13,12 +13,19 @@ namespace Microsoft.Collections.Extensions internal interface IDictionarySlim : IDictionarySlim where TKey : IEquatable { + /// + /// Checks whether or not the dictionary contains a pair with a specified key. + /// + /// The key to look for. + /// Whether or not the key was present in the dictionary. + public bool ContainsKey(TKey key); + /// /// Tries to remove a value with a specified key. /// /// The key of the value to remove. /// The removed value, if it was present. - /// .Whether or not the key was present. + /// Whether or not the key was present. bool TryRemove(TKey key, out object? result); /// diff --git a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs new file mode 100644 index 00000000000..91ed0800868 --- /dev/null +++ b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs @@ -0,0 +1,484 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +#if NETSTANDARD2_1 + +using System; +using System.Buffers; +using System.Collections.Generic; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using Microsoft.Collections.Extensions; + +namespace Microsoft.Toolkit.Mvvm.Messaging +{ + /// + /// A class providing a reference implementation for the interface. + /// + /// + /// This implementation uses weak references to track the registered + /// recipients, so it is not necessary to manually unregister them when they're no longer needed. + /// + public sealed class WeakRefMessenger : IMessenger + { + // The WeakRefMessenger class uses the following logic to link stored instances together: + // -------------------------------------------------------------------------------------------------------- + // DictionarySlim> mapping + // / / / + // ___(Type2.TToken)___/ / / + // /_________________(Type2.TMessage)______________________/ / + // / ___________________________/ + // / / + // DictionarySlim> recipientsMap; + // -------------------------------------------------------------------------------------------------------- + // Just like in the strong reference variant, each pair of message and token types is used as a key in the + // recipients map. In this case, the values in the dictionary are ConditionalWeakTable<,> instances, that + // link each registered recipient to a map of currently registered handlers, through a weak reference. + // The value in each conditional table is Dictionary>, using + // the same unsafe cast as before to allow the generic handler delegates to be invoked without knowing + // what type each recipient was registered with, and without the need to use reflection. + + /// + /// The map of currently registered recipients for all message types. + /// + private readonly DictionarySlim> recipientsMap + = new DictionarySlim>(); + + /// + /// Gets the default instance. + /// + public static WeakRefMessenger Default { get; } = new WeakRefMessenger(); + + /// + public bool IsRegistered(object recipient, TToken token) + where TMessage : class + where TToken : IEquatable + { + lock (this.recipientsMap) + { + Type2 type2 = new Type2(typeof(TMessage), typeof(TToken)); + + // Get the conditional table associated with the target recipient, for the current pair + // of token and message types. If it exists, check if there is a matching token. + return + this.recipientsMap.TryGetValue(type2, out ConditionalWeakTable? table) && + table!.TryGetValue(recipient, out IDictionarySlim mapping) && + Unsafe.As>(mapping).ContainsKey(token); + } + } + + /// + public void Register(TRecipient recipient, TToken token, MessageHandler handler) + where TRecipient : class + where TMessage : class + where TToken : IEquatable + { + lock (this.recipientsMap) + { + Type2 type2 = new Type2(typeof(TMessage), typeof(TToken)); + + // Get the conditional table for the pair of type arguments, or create it if it doesn't exist + ref ConditionalWeakTable? mapping = ref this.recipientsMap.GetOrAddValueRef(type2); + + mapping ??= new ConditionalWeakTable(); + + // Get or create the handlers dictionary for the target recipient + var map = Unsafe.As>(mapping.GetValue(recipient, _ => new DictionarySlim())); + + // Add the new registration entry + ref object? registeredHandler = ref map.GetOrAddValueRef(token); + + if (!(registeredHandler is null)) + { + ThrowInvalidOperationExceptionForDuplicateRegistration(); + } + + // Store the input handler + registeredHandler = handler; + } + } + + /// + public void UnregisterAll(object recipient) + { + lock (this.recipientsMap) + { + var enumerator = this.recipientsMap.GetEnumerator(); + + // Traverse all the existing conditional tables and remove all the ones + // with the target recipient as key. We don't perform a cleanup here, + // as that is responsability of a separate method defined below. + while (enumerator.MoveNext()) + { + enumerator.Value.Remove(recipient); + } + } + } + + /// + public void UnregisterAll(object recipient, TToken token) + where TToken : IEquatable + { + lock (this.recipientsMap) + { + var enumerator = this.recipientsMap.GetEnumerator(); + + // Same as above, with the difference being that this time we only go through + // the conditional tables having a matching token type as key, and that we + // only try to remove handlers with a matching token, if any. + while (enumerator.MoveNext()) + { + if (enumerator.Key.TToken == typeof(TToken) && + enumerator.Value.TryGetValue(recipient, out IDictionarySlim mapping)) + { + Unsafe.As>(mapping).TryRemove(token, out _); + } + } + } + } + + /// + public void Unregister(object recipient, TToken token) + where TMessage : class + where TToken : IEquatable + { + lock (this.recipientsMap) + { + var type2 = new Type2(typeof(TMessage), typeof(TToken)); + var enumerator = this.recipientsMap.GetEnumerator(); + + // Traverse all the existing token and message pairs matching the current type + // arguments, and remove all the handlers with a matching token, as above. + while (enumerator.MoveNext()) + { + if (enumerator.Key.Equals(type2) && + enumerator.Value.TryGetValue(recipient, out IDictionarySlim mapping)) + { + Unsafe.As>(mapping).TryRemove(token, out _); + } + } + } + } + + /// + public TMessage Send(TMessage message, TToken token) + where TMessage : class + where TToken : IEquatable + { + ArrayPoolBufferWriter recipients; + ArrayPoolBufferWriter handlers; + + lock (this.recipientsMap) + { + Type2 type2 = new Type2(typeof(TMessage), typeof(TToken)); + + // Try to get the target table + if (!this.recipientsMap.TryGetValue(type2, out ConditionalWeakTable? table)) + { + return message; + } + + recipients = new ArrayPoolBufferWriter(); + handlers = new ArrayPoolBufferWriter(); + + // We need a local, temporary copy of all the pending recipients and handlers to + // invoke, to avoid issues with handlers unregistering from messages while we're + // holding the lock. To do this, we can just traverse the conditional table in use + // to enumerate all the existing recipients for the token and message types pair + // corresponding to the generic arguments for this invocation, and then track the + // handlers with a matching token, and their corresponding recipients. + foreach (KeyValuePair pair in table!) + { + var map = Unsafe.As>(pair.Value); + + if (map.TryGetValue(token, out object? handler)) + { + recipients.Add(pair.Key); + handlers.Add(handler!); + } + } + } + + try + { + ReadOnlySpan + recipientsSpan = recipients.Span, + handlersSpan = handlers.Span; + int handlersCount = recipients.Count; + + for (int i = 0; i < handlersCount; i++) + { + // Just like in the other messenger, here we need an unsafe cast to be able to + // invoke a generic delegate with a contravariant input argument, with a less + // derived reference, without reflection. This is guaranteed to work by how the + // messenger tracks registered recipients and their associated handlers, so the + // type conversion will always be valid (the recipients are the rigth instances). + Unsafe.As>(handlersSpan[i])(recipientsSpan![i], message); + } + } + finally + { + recipients.Dispose(); + handlers.Dispose(); + } + + return message; + } + + /// + void IMessenger.Cleanup() + { + lock (this.recipientsMap) + { + using ArrayPoolBufferWriter type2s = new ArrayPoolBufferWriter(); + using ArrayPoolBufferWriter emptyRecipients = new ArrayPoolBufferWriter(); + + var enumerator = this.recipientsMap.GetEnumerator(); + + // First, we go through all the currently registered pairs of token and message types. + // These represents all the combinations of generic arguments with at least one registered + // handler, with the exception of those with recipients that have already been collected. + while (enumerator.MoveNext()) + { + emptyRecipients.Reset(); + + bool hasAtLeastOneHandler = false; + + // Go through the currently alive recipients to look for those with no handlers left. We track + // the ones we find to remove them outside of the loop (can't modify during enumeration). + foreach (KeyValuePair pair in enumerator.Value) + { + if (pair.Value.Count == 0) + { + emptyRecipients.Add(pair.Key); + } + else + { + hasAtLeastOneHandler = true; + } + } + + // Remove the handler maps for recipients that are still alive but with no handlers + foreach (object recipient in emptyRecipients.Span) + { + enumerator.Value.Remove(recipient); + } + + // Track the type combinations with no recipients or handlers left + if (!hasAtLeastOneHandler) + { + type2s.Add(enumerator.Key); + } + } + + // Remove all the mappings with no handlers left + foreach (Type2 key in type2s.Span) + { + this.recipientsMap.Remove(key); + } + } + } + + /// + public void Reset() + { + lock (this.recipientsMap) + { + this.recipientsMap.Clear(); + } + } + + /// + /// A simple buffer writer implementation using pooled arrays. + /// + /// The type of items to store in the list. + public sealed class ArrayPoolBufferWriter : IDisposable + { + /// + /// The default buffer size to use to expand empty arrays. + /// + private const int DefaultInitialBufferSize = 128; + + /// + /// The underlying array. + /// + private T[] array; + + /// + /// The starting offset within . + /// + private int index; + + /// + /// Initializes a new instance of the class. + /// + public ArrayPoolBufferWriter() + { + this.array = ArrayPool.Shared.Rent(DefaultInitialBufferSize); + this.index = 0; + } + + /// + /// Gets a with the current items. + /// + public ReadOnlySpan Span + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get + { + T[] array = this.array; + + if (this.index > 0) + { + return MemoryMarshal.CreateReadOnlySpan(ref array[0], this.index); + } + + return default; + } + } + + /// + /// Gets the current number of items. + /// + public int Count + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => this.index; + } + + /// + /// Adds a new item to the current collection. + /// + /// The item to add. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void Add(T item) + { + if (this.index == this.array.Length) + { + ResizeBuffer(); + } + + this.array[this.index++] = item; + } + + /// + /// Resets the underlying array and the stored items. + /// + public void Reset() + { + Array.Clear(this.array, 0, this.index); + + this.index = 0; + } + + /// + /// Resizes when there is no space left for new items. + /// + [MethodImpl(MethodImplOptions.NoInlining)] + private void ResizeBuffer() + { + T[] rent = ArrayPool.Shared.Rent(this.index << 2); + + Array.Copy(this.array, 0, rent, 0, this.index); + Array.Clear(this.array, 0, this.index); + + ArrayPool.Shared.Return(this.array); + + this.array = rent; + } + + /// + public void Dispose() + { + Array.Clear(this.array, 0, this.index); + + ArrayPool.Shared.Return(this.array); + } + } + + /// + /// A simple type representing an immutable pair of types. + /// + /// + /// This type replaces a simple as it's faster in its + /// and methods, and because + /// unlike a value tuple it exposes its fields as immutable. Additionally, the + /// and fields provide additional clarity reading + /// the code compared to and . + /// + private readonly struct Type2 : IEquatable + { + /// + /// The type of registered message. + /// + public readonly Type TMessage; + + /// + /// The type of registration token. + /// + public readonly Type TToken; + + /// + /// Initializes a new instance of the struct. + /// + /// The type of registered message. + /// The type of registration token. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public Type2(Type tMessage, Type tToken) + { + TMessage = tMessage; + TToken = tToken; + } + + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public bool Equals(Type2 other) + { + // We can't just use reference equality, as that's technically not guaranteed + // to work and might fail in very rare cases (eg. with type forwarding between + // different assemblies). Instead, we can use the == operator to compare for + // equality, which still avoids the callvirt overhead of calling Type.Equals, + // and is also implemented as a JIT intrinsic on runtimes such as .NET Core. + return + TMessage == other.TMessage && + TToken == other.TToken; + } + + /// + public override bool Equals(object? obj) + { + return obj is Type2 other && Equals(other); + } + + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public override int GetHashCode() + { + unchecked + { + // To combine the two hashes, we can simply use the fast djb2 hash algorithm. + // This is not a problem in this case since we already know that the base + // RuntimeHelpers.GetHashCode method is providing hashes with a good enough distribution. + int hash = RuntimeHelpers.GetHashCode(TMessage); + + hash = (hash << 5) + hash; + + hash += RuntimeHelpers.GetHashCode(TToken); + + return hash; + } + } + } + + /// + /// Throws an when trying to add a duplicate handler. + /// + private static void ThrowInvalidOperationExceptionForDuplicateRegistration() + { + throw new InvalidOperationException("The target recipient has already subscribed to the target message"); + } + } +} + +#endif From 5840ad0559e9f544d140217c65b303c79e707ce8 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sat, 12 Sep 2020 15:48:12 +0200 Subject: [PATCH 18/38] Added support for multiple messenger types to tests --- .../Mvvm/Test_Messenger.Request.cs | 55 ++++---- .../UnitTests.Shared/Mvvm/Test_Messenger.cs | 124 ++++++++++-------- .../Mvvm/Test_ObservableRecipient.cs | 30 ++++- 3 files changed, 128 insertions(+), 81 deletions(-) diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs index 9a4329e06d3..e1f99621b29 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs @@ -15,9 +15,10 @@ public partial class Test_Messenger { [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_RequestMessage_Ok() + [DataRow(typeof(Messenger))] + public void Test_Messenger_RequestMessage_Ok(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new object(); object test = null; @@ -42,20 +43,22 @@ void Receive(object recipient, NumberRequestMessage m) [TestCategory("Mvvm")] [TestMethod] + [DataRow(typeof(Messenger))] [ExpectedException(typeof(InvalidOperationException))] - public void Test_Messenger_RequestMessage_Fail_NoReply() + public void Test_Messenger_RequestMessage_Fail_NoReply(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); int result = messenger.Send(); } [TestCategory("Mvvm")] [TestMethod] + [DataRow(typeof(Messenger))] [ExpectedException(typeof(InvalidOperationException))] - public void Test_Messenger_RequestMessage_Fail_MultipleReplies() + public void Test_Messenger_RequestMessage_Fail_MultipleReplies(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new object(); void Receive(object recipient, NumberRequestMessage m) @@ -75,9 +78,10 @@ public class NumberRequestMessage : RequestMessage [TestCategory("Mvvm")] [TestMethod] - public async Task Test_Messenger_AsyncRequestMessage_Ok_Sync() + [DataRow(typeof(Messenger))] + public async Task Test_Messenger_AsyncRequestMessage_Ok_Sync(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new object(); void Receive(object recipient, AsyncNumberRequestMessage m) @@ -98,9 +102,10 @@ void Receive(object recipient, AsyncNumberRequestMessage m) [TestCategory("Mvvm")] [TestMethod] - public async Task Test_Messenger_AsyncRequestMessage_Ok_Async() + [DataRow(typeof(Messenger))] + public async Task Test_Messenger_AsyncRequestMessage_Ok_Async(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new object(); async Task GetNumberAsync() @@ -128,20 +133,22 @@ void Receive(object recipient, AsyncNumberRequestMessage m) [TestCategory("Mvvm")] [TestMethod] + [DataRow(typeof(Messenger))] [ExpectedException(typeof(InvalidOperationException))] - public async Task Test_Messenger_AsyncRequestMessage_Fail_NoReply() + public async Task Test_Messenger_AsyncRequestMessage_Fail_NoReply(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); int result = await messenger.Send(); } [TestCategory("Mvvm")] [TestMethod] + [DataRow(typeof(Messenger))] [ExpectedException(typeof(InvalidOperationException))] - public async Task Test_Messenger_AsyncRequestMessage_Fail_MultipleReplies() + public async Task Test_Messenger_AsyncRequestMessage_Fail_MultipleReplies(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new object(); void Receive(object recipient, AsyncNumberRequestMessage m) @@ -161,9 +168,10 @@ public class AsyncNumberRequestMessage : AsyncRequestMessage [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_CollectionRequestMessage_Ok_NoReplies() + [DataRow(typeof(Messenger))] + public void Test_Messenger_CollectionRequestMessage_Ok_NoReplies(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new object(); void Receive(object recipient, NumbersCollectionRequestMessage m) @@ -179,9 +187,10 @@ void Receive(object recipient, NumbersCollectionRequestMessage m) [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_CollectionRequestMessage_Ok_MultipleReplies() + [DataRow(typeof(Messenger))] + public void Test_Messenger_CollectionRequestMessage_Ok_MultipleReplies(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); object recipient1 = new object(), recipient2 = new object(), @@ -232,9 +241,10 @@ public class NumbersCollectionRequestMessage : CollectionRequestMessage [TestCategory("Mvvm")] [TestMethod] - public async Task Test_Messenger_AsyncCollectionRequestMessage_Ok_NoReplies() + [DataRow(typeof(Messenger))] + public async Task Test_Messenger_AsyncCollectionRequestMessage_Ok_NoReplies(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new object(); void Receive(object recipient, AsyncNumbersCollectionRequestMessage m) @@ -250,9 +260,10 @@ void Receive(object recipient, AsyncNumbersCollectionRequestMessage m) [TestCategory("Mvvm")] [TestMethod] - public async Task Test_Messenger_AsyncCollectionRequestMessage_Ok_MultipleReplies() + [DataRow(typeof(Messenger))] + public async Task Test_Messenger_AsyncCollectionRequestMessage_Ok_MultipleReplies(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); object recipient1 = new object(), recipient2 = new object(), diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs index 9e83f85ab4b..bbbba617720 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs @@ -13,9 +13,10 @@ public partial class Test_Messenger { [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_UnregisterRecipientWithMessageType() + [DataRow(typeof(Messenger))] + public void Test_Messenger_UnregisterRecipientWithMessageType(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new object(); messenger.Unregister(recipient); @@ -23,9 +24,10 @@ public void Test_Messenger_UnregisterRecipientWithMessageType() [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_UnregisterRecipientWithMessageTypeAndToken() + [DataRow(typeof(Messenger))] + public void Test_Messenger_UnregisterRecipientWithMessageTypeAndToken(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new object(); messenger.Unregister(recipient, nameof(MessageA)); @@ -33,9 +35,10 @@ public void Test_Messenger_UnregisterRecipientWithMessageTypeAndToken() [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_UnregisterRecipientWithToken() + [DataRow(typeof(Messenger))] + public void Test_Messenger_UnregisterRecipientWithToken(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new object(); messenger.UnregisterAll(recipient, nameof(MessageA)); @@ -43,9 +46,10 @@ public void Test_Messenger_UnregisterRecipientWithToken() [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_UnregisterRecipientWithRecipient() + [DataRow(typeof(Messenger))] + public void Test_Messenger_UnregisterRecipientWithRecipient(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new object(); messenger.UnregisterAll(recipient); @@ -53,9 +57,10 @@ public void Test_Messenger_UnregisterRecipientWithRecipient() [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageType() + [DataRow(typeof(Messenger))] + public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageType(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new object(); messenger.Register(recipient, (r, m) => { }); @@ -67,9 +72,10 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageType() [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageTypeAndToken() + [DataRow(typeof(Messenger))] + public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageTypeAndToken(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new object(); messenger.Register(recipient, nameof(MessageA), (r, m) => { }); @@ -81,9 +87,10 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageTypeAndToken [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_RegisterAndUnregisterRecipientWithToken() + [DataRow(typeof(Messenger))] + public void Test_Messenger_RegisterAndUnregisterRecipientWithToken(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new object(); messenger.Register(recipient, nameof(MessageA), (r, m) => { }); @@ -95,9 +102,10 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithToken() [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_RegisterAndUnregisterRecipientWithRecipient() + [DataRow(typeof(Messenger))] + public void Test_Messenger_RegisterAndUnregisterRecipientWithRecipient(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new object(); messenger.Register(recipient, nameof(MessageA), (r, m) => { }); @@ -109,36 +117,38 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithRecipient() [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithNoToken() + [DataRow(typeof(Messenger))] + public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithNoToken(Type type) { + var messenger = (IMessenger)Activator.CreateInstance(type); object a = new object(); - Assert.IsFalse(Messenger.Default.IsRegistered(a)); + Assert.IsFalse(messenger.IsRegistered(a)); object recipient = null; string result = null; - Messenger.Default.Register(a, (r, m) => + messenger.Register(a, (r, m) => { recipient = r; result = m.Text; }); - Assert.IsTrue(Messenger.Default.IsRegistered(a)); + Assert.IsTrue(messenger.IsRegistered(a)); - Messenger.Default.Send(new MessageA { Text = nameof(MessageA) }); + messenger.Send(new MessageA { Text = nameof(MessageA) }); Assert.AreSame(recipient, a); Assert.AreEqual(result, nameof(MessageA)); - Messenger.Default.Unregister(a); + messenger.Unregister(a); - Assert.IsFalse(Messenger.Default.IsRegistered(a)); + Assert.IsFalse(messenger.IsRegistered(a)); recipient = null; result = null; - Messenger.Default.Send(new MessageA { Text = nameof(MessageA) }); + messenger.Send(new MessageA { Text = nameof(MessageA) }); Assert.IsNull(recipient); Assert.IsNull(result); @@ -146,63 +156,68 @@ public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithN [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_IsRegistered_Register_Send_UnregisterRecipient_WithNoToken() + [DataRow(typeof(Messenger))] + public void Test_Messenger_IsRegistered_Register_Send_UnregisterRecipient_WithNoToken(Type type) { + var messenger = (IMessenger)Activator.CreateInstance(type); object a = new object(); - Assert.IsFalse(Messenger.Default.IsRegistered(a)); + Assert.IsFalse(messenger.IsRegistered(a)); string result = null; - Messenger.Default.Register(a, (r, m) => result = m.Text); + messenger.Register(a, (r, m) => result = m.Text); - Assert.IsTrue(Messenger.Default.IsRegistered(a)); + Assert.IsTrue(messenger.IsRegistered(a)); - Messenger.Default.Send(new MessageA { Text = nameof(MessageA) }); + messenger.Send(new MessageA { Text = nameof(MessageA) }); Assert.AreEqual(result, nameof(MessageA)); - Messenger.Default.UnregisterAll(a); + messenger.UnregisterAll(a); - Assert.IsFalse(Messenger.Default.IsRegistered(a)); + Assert.IsFalse(messenger.IsRegistered(a)); result = null; - Messenger.Default.Send(new MessageA { Text = nameof(MessageA) }); + messenger.Send(new MessageA { Text = nameof(MessageA) }); Assert.IsNull(result); } [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithToken() + [DataRow(typeof(Messenger))] + public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithToken(Type type) { + var messenger = (IMessenger)Activator.CreateInstance(type); object a = new object(); - Assert.IsFalse(Messenger.Default.IsRegistered(a)); + Assert.IsFalse(messenger.IsRegistered(a)); string result = null; - Messenger.Default.Register(a, nameof(MessageA), (r, m) => result = m.Text); + messenger.Register(a, nameof(MessageA), (r, m) => result = m.Text); - Assert.IsTrue(Messenger.Default.IsRegistered(a, nameof(MessageA))); + Assert.IsTrue(messenger.IsRegistered(a, nameof(MessageA))); - Messenger.Default.Send(new MessageA { Text = nameof(MessageA) }, nameof(MessageA)); + messenger.Send(new MessageA { Text = nameof(MessageA) }, nameof(MessageA)); Assert.AreEqual(result, nameof(MessageA)); - Messenger.Default.Unregister(a, nameof(MessageA)); + messenger.Unregister(a, nameof(MessageA)); - Assert.IsFalse(Messenger.Default.IsRegistered(a, nameof(MessageA))); + Assert.IsFalse(messenger.IsRegistered(a, nameof(MessageA))); result = null; - Messenger.Default.Send(new MessageA { Text = nameof(MessageA) }, nameof(MessageA)); + messenger.Send(new MessageA { Text = nameof(MessageA) }, nameof(MessageA)); Assert.IsNull(result); } [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_DuplicateRegistrationWithMessageType() + [DataRow(typeof(Messenger))] + public void Test_Messenger_DuplicateRegistrationWithMessageType(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new object(); messenger.Register(recipient, (r, m) => { }); @@ -215,9 +230,10 @@ public void Test_Messenger_DuplicateRegistrationWithMessageType() [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_DuplicateRegistrationWithMessageTypeAndToken() + [DataRow(typeof(Messenger))] + public void Test_Messenger_DuplicateRegistrationWithMessageTypeAndToken(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new object(); messenger.Register(recipient, nameof(MessageA), (r, m) => { }); @@ -230,9 +246,10 @@ public void Test_Messenger_DuplicateRegistrationWithMessageTypeAndToken() [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_IRecipient_NoMessages() + [DataRow(typeof(Messenger))] + public void Test_Messenger_IRecipient_NoMessages(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new RecipientWithNoMessages(); messenger.RegisterAll(recipient); @@ -243,9 +260,10 @@ public void Test_Messenger_IRecipient_NoMessages() [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_IRecipient_SomeMessages_NoToken() + [DataRow(typeof(Messenger))] + public void Test_Messenger_IRecipient_SomeMessages_NoToken(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new RecipientWithSomeMessages(); messenger.RegisterAll(recipient); @@ -274,9 +292,10 @@ public void Test_Messenger_IRecipient_SomeMessages_NoToken() [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_IRecipient_SomeMessages_WithToken() + [DataRow(typeof(Messenger))] + public void Test_Messenger_IRecipient_SomeMessages_WithToken(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new RecipientWithSomeMessages(); var token = nameof(Test_Messenger_IRecipient_SomeMessages_WithToken); @@ -309,9 +328,10 @@ public void Test_Messenger_IRecipient_SomeMessages_WithToken() [TestCategory("Mvvm")] [TestMethod] - public void Test_Messenger_RegisterWithTypeParameter() + [DataRow(typeof(Messenger))] + public void Test_Messenger_RegisterWithTypeParameter(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var recipient = new RecipientWithNoMessages { Number = 42 }; int number = 0; diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs index 76ff145d86f..a67a52b2344 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; using Microsoft.Toolkit.Mvvm.ComponentModel; using Microsoft.Toolkit.Mvvm.Messaging; using Microsoft.Toolkit.Mvvm.Messaging.Messages; @@ -14,9 +15,11 @@ public class Test_ObservableRecipient { [TestCategory("Mvvm")] [TestMethod] - public void Test_ObservableRecipient_Activation() + [DataRow(typeof(Messenger))] + public void Test_ObservableRecipient_Activation(Type type) { - var viewmodel = new SomeRecipient(); + var messenger = (IMessenger)Activator.CreateInstance(type); + var viewmodel = new SomeRecipient(messenger); Assert.IsFalse(viewmodel.IsActivatedCheck); @@ -33,7 +36,18 @@ public void Test_ObservableRecipient_Activation() [TestCategory("Mvvm")] [TestMethod] - public void Test_ObservableRecipient_Defaults() + [DataRow(typeof(Messenger))] + public void Test_ObservableRecipient_IsSame(Type type) + { + var messenger = (IMessenger)Activator.CreateInstance(type); + var viewmodel = new SomeRecipient(messenger); + + Assert.AreSame(viewmodel.CurrentMessenger, messenger); + } + + [TestCategory("Mvvm")] + [TestMethod] + public void Test_ObservableRecipient_Default() { var viewmodel = new SomeRecipient(); @@ -42,9 +56,10 @@ public void Test_ObservableRecipient_Defaults() [TestCategory("Mvvm")] [TestMethod] - public void Test_ObservableRecipient_Injection() + [DataRow(typeof(Messenger))] + public void Test_ObservableRecipient_Injection(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var viewmodel = new SomeRecipient(messenger); Assert.AreSame(viewmodel.CurrentMessenger, messenger); @@ -52,9 +67,10 @@ public void Test_ObservableRecipient_Injection() [TestCategory("Mvvm")] [TestMethod] - public void Test_ObservableRecipient_Broadcast() + [DataRow(typeof(Messenger))] + public void Test_ObservableRecipient_Broadcast(Type type) { - var messenger = new Messenger(); + var messenger = (IMessenger)Activator.CreateInstance(type); var viewmodel = new SomeRecipient(messenger); PropertyChangedMessage message = null; From d68f9d2ccd26d7340623a2e5179e13dc09afc787 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sat, 12 Sep 2020 15:54:25 +0200 Subject: [PATCH 19/38] Enabled tests for WeakRefMessenger --- .../Mvvm/Test_Messenger.Request.cs | 33 ++++++++++++ .../UnitTests.Shared/Mvvm/Test_Messenger.cs | 51 +++++++++++++++++++ .../Mvvm/Test_ObservableRecipient.cs | 12 +++++ 3 files changed, 96 insertions(+) diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs index e1f99621b29..0d3c9b48d60 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs @@ -16,6 +16,9 @@ public partial class Test_Messenger [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_RequestMessage_Ok(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -44,6 +47,9 @@ void Receive(object recipient, NumberRequestMessage m) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif [ExpectedException(typeof(InvalidOperationException))] public void Test_Messenger_RequestMessage_Fail_NoReply(Type type) { @@ -55,6 +61,9 @@ public void Test_Messenger_RequestMessage_Fail_NoReply(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif [ExpectedException(typeof(InvalidOperationException))] public void Test_Messenger_RequestMessage_Fail_MultipleReplies(Type type) { @@ -79,6 +88,9 @@ public class NumberRequestMessage : RequestMessage [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public async Task Test_Messenger_AsyncRequestMessage_Ok_Sync(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -103,6 +115,9 @@ void Receive(object recipient, AsyncNumberRequestMessage m) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public async Task Test_Messenger_AsyncRequestMessage_Ok_Async(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -134,6 +149,9 @@ void Receive(object recipient, AsyncNumberRequestMessage m) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif [ExpectedException(typeof(InvalidOperationException))] public async Task Test_Messenger_AsyncRequestMessage_Fail_NoReply(Type type) { @@ -145,6 +163,9 @@ public async Task Test_Messenger_AsyncRequestMessage_Fail_NoReply(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif [ExpectedException(typeof(InvalidOperationException))] public async Task Test_Messenger_AsyncRequestMessage_Fail_MultipleReplies(Type type) { @@ -169,6 +190,9 @@ public class AsyncNumberRequestMessage : AsyncRequestMessage [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_CollectionRequestMessage_Ok_NoReplies(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -188,6 +212,9 @@ void Receive(object recipient, NumbersCollectionRequestMessage m) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_CollectionRequestMessage_Ok_MultipleReplies(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -242,6 +269,9 @@ public class NumbersCollectionRequestMessage : CollectionRequestMessage [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public async Task Test_Messenger_AsyncCollectionRequestMessage_Ok_NoReplies(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -261,6 +291,9 @@ void Receive(object recipient, AsyncNumbersCollectionRequestMessage m) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public async Task Test_Messenger_AsyncCollectionRequestMessage_Ok_MultipleReplies(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs index bbbba617720..a429baa7552 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs @@ -14,6 +14,9 @@ public partial class Test_Messenger [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_UnregisterRecipientWithMessageType(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -25,6 +28,9 @@ public void Test_Messenger_UnregisterRecipientWithMessageType(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_UnregisterRecipientWithMessageTypeAndToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -36,6 +42,9 @@ public void Test_Messenger_UnregisterRecipientWithMessageTypeAndToken(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_UnregisterRecipientWithToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -47,6 +56,9 @@ public void Test_Messenger_UnregisterRecipientWithToken(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_UnregisterRecipientWithRecipient(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -58,6 +70,9 @@ public void Test_Messenger_UnregisterRecipientWithRecipient(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageType(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -73,6 +88,9 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageType(Type ty [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageTypeAndToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -88,6 +106,9 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageTypeAndToken [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_RegisterAndUnregisterRecipientWithToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -103,6 +124,9 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithToken(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_RegisterAndUnregisterRecipientWithRecipient(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -118,6 +142,9 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithRecipient(Type type [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithNoToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -157,6 +184,9 @@ public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithN [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_IsRegistered_Register_Send_UnregisterRecipient_WithNoToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -186,6 +216,9 @@ public void Test_Messenger_IsRegistered_Register_Send_UnregisterRecipient_WithNo [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -215,6 +248,9 @@ public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithT [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_DuplicateRegistrationWithMessageType(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -231,6 +267,9 @@ public void Test_Messenger_DuplicateRegistrationWithMessageType(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_DuplicateRegistrationWithMessageTypeAndToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -247,6 +286,9 @@ public void Test_Messenger_DuplicateRegistrationWithMessageTypeAndToken(Type typ [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_IRecipient_NoMessages(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -261,6 +303,9 @@ public void Test_Messenger_IRecipient_NoMessages(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_IRecipient_SomeMessages_NoToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -293,6 +338,9 @@ public void Test_Messenger_IRecipient_SomeMessages_NoToken(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_IRecipient_SomeMessages_WithToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -329,6 +377,9 @@ public void Test_Messenger_IRecipient_SomeMessages_WithToken(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_Messenger_RegisterWithTypeParameter(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs index a67a52b2344..be41663f116 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs @@ -16,6 +16,9 @@ public class Test_ObservableRecipient [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_ObservableRecipient_Activation(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -37,6 +40,9 @@ public void Test_ObservableRecipient_Activation(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_ObservableRecipient_IsSame(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -57,6 +63,9 @@ public void Test_ObservableRecipient_Default() [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_ObservableRecipient_Injection(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -68,6 +77,9 @@ public void Test_ObservableRecipient_Injection(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] +#if NETCOREAPP3_1 + [DataRow(typeof(WeakRefMessenger))] +#endif public void Test_ObservableRecipient_Broadcast(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); From 78bbd6e094916ffdbfbaebdeec78b279d97795e4 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sat, 12 Sep 2020 16:47:30 +0200 Subject: [PATCH 20/38] Removed leftover code no longer needed --- Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index 6f6bf62a047..20ed40b52ab 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -156,18 +156,8 @@ public void UnregisterAll(object recipient) // Removes all the lists of registered handlers for the recipient foreach (IMapping mapping in set!) { - if (mapping.TryRemove(key, out object? handlersMap)) + if (mapping.TryRemove(key, out _)) { - // If this branch is taken, it means the target recipient to unregister - // had at least one registered handler for the current - // pair of type parameters, which here is masked out by the IMapping interface. - // Before removing the handlers, we need to retrieve the count of how many handlers - // are being removed, in order to update the total counter for the mapping. - // Just casting the dictionary to the base interface and accessing the Count - // property directly gives us O(1) access time to retrieve this count. - // The handlers map is the IDictionary instance for the mapping. - int handlersCount = Unsafe.As(handlersMap).Count; - if (mapping.Count == 0) { // Maps here are really of type Mapping<,> and with unknown type arguments. From 98095223b784fcf6efec297d5a77c6bad1d310ec Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sat, 12 Sep 2020 17:38:18 +0200 Subject: [PATCH 21/38] Code refactoring, removed duplicate types --- .../ComponentModel/ObservableRecipient.cs | 2 +- .../Messaging/IMessenger.cs | 4 +- ...rExtensions.cs => IMessengerExtensions.cs} | 7 +- .../DictionarySlim{TKey,TValue}.cs | 0 .../HashHelpers.cs | 0 .../IDictionarySlim.cs | 0 .../IDictionarySlim{TKey,TValue}.cs | 0 .../IDictionarySlim{TKey}.cs | 0 .../Messaging/Internals/Type2.cs | 79 +++++++++++++++++++ .../Messaging/Internals/Unit.cs | 31 ++++++++ Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 79 +------------------ .../Messaging/MessengerExtensions.Unit.cs | 41 ---------- .../Messaging/WeakRefMessenger.cs | 75 +----------------- 13 files changed, 121 insertions(+), 197 deletions(-) rename Microsoft.Toolkit.Mvvm/Messaging/{MessengerExtensions.cs => IMessengerExtensions.cs} (98%) rename Microsoft.Toolkit.Mvvm/Messaging/{ => Internals}/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs (100%) rename Microsoft.Toolkit.Mvvm/Messaging/{ => Internals}/Microsoft.Collections.Extensions/HashHelpers.cs (100%) rename Microsoft.Toolkit.Mvvm/Messaging/{ => Internals}/Microsoft.Collections.Extensions/IDictionarySlim.cs (100%) rename Microsoft.Toolkit.Mvvm/Messaging/{ => Internals}/Microsoft.Collections.Extensions/IDictionarySlim{TKey,TValue}.cs (100%) rename Microsoft.Toolkit.Mvvm/Messaging/{ => Internals}/Microsoft.Collections.Extensions/IDictionarySlim{TKey}.cs (100%) create mode 100644 Microsoft.Toolkit.Mvvm/Messaging/Internals/Type2.cs create mode 100644 Microsoft.Toolkit.Mvvm/Messaging/Internals/Unit.cs delete mode 100644 Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.Unit.cs diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs index ca847383b87..81b87dffb90 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs @@ -79,7 +79,7 @@ public bool IsActive /// /// The base implementation registers all messages for this recipients that have been declared /// explicitly through the interface, using the default channel. - /// For more details on how this works, see the method. + /// For more details on how this works, see the method. /// If you need more fine tuned control, want to register messages individually or just prefer /// the lambda-style syntax for message registration, override this method and register manually. /// diff --git a/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs index 9968d79cb2c..2e41916e950 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/IMessenger.cs @@ -64,12 +64,12 @@ public delegate void MessageHandler(TRecipient recip /// Messenger.Default.Register(this, Receive); /// /// The C# compiler will automatically convert that expression to a instance - /// compatible with . + /// compatible with . /// This will also work if multiple overloads of that method are available, each handling a different /// message type: the C# compiler will automatically pick the right one for the current message type. /// It is also possible to register message handlers explicitly using the interface. /// To do so, the recipient just needs to implement the interface and then call the - /// extension, which will automatically register + /// extension, which will automatically register /// all the handlers that are declared by the recipient type. Registration for individual handlers is supported as well. /// public interface IMessenger diff --git a/Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.cs b/Microsoft.Toolkit.Mvvm/Messaging/IMessengerExtensions.cs similarity index 98% rename from Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.cs rename to Microsoft.Toolkit.Mvvm/Messaging/IMessengerExtensions.cs index 2a6a0383bec..72caa9fb7dd 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/IMessengerExtensions.cs @@ -8,19 +8,20 @@ using System.Linq.Expressions; using System.Reflection; using System.Runtime.CompilerServices; +using Microsoft.Toolkit.Mvvm.Messaging.Internals; namespace Microsoft.Toolkit.Mvvm.Messaging { /// /// Extensions for the type. /// - public static partial class MessengerExtensions + public static class IMessengerExtensions { /// /// A class that acts as a container to load the instance linked to /// the method. /// This class is needed to avoid forcing the initialization code in the static constructor to run as soon as - /// the type is referenced, even if that is done just to use methods + /// the type is referenced, even if that is done just to use methods /// that do not actually require this instance to be available. /// We're effectively using this type to leverage the lazy loading of static constructors done by the runtime. /// @@ -32,7 +33,7 @@ private static class MethodInfos static MethodInfos() { RegisterIRecipient = ( - from methodInfo in typeof(MessengerExtensions).GetMethods() + from methodInfo in typeof(IMessengerExtensions).GetMethods() where methodInfo.Name == nameof(Register) && methodInfo.IsGenericMethod && methodInfo.GetGenericArguments().Length == 2 diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs similarity index 100% rename from Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs rename to Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/HashHelpers.cs b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/HashHelpers.cs similarity index 100% rename from Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/HashHelpers.cs rename to Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/HashHelpers.cs diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/IDictionarySlim.cs b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/IDictionarySlim.cs similarity index 100% rename from Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/IDictionarySlim.cs rename to Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/IDictionarySlim.cs diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/IDictionarySlim{TKey,TValue}.cs b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/IDictionarySlim{TKey,TValue}.cs similarity index 100% rename from Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/IDictionarySlim{TKey,TValue}.cs rename to Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/IDictionarySlim{TKey,TValue}.cs diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/IDictionarySlim{TKey}.cs b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/IDictionarySlim{TKey}.cs similarity index 100% rename from Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/IDictionarySlim{TKey}.cs rename to Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/IDictionarySlim{TKey}.cs diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Internals/Type2.cs b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Type2.cs new file mode 100644 index 00000000000..0b254562e95 --- /dev/null +++ b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Type2.cs @@ -0,0 +1,79 @@ +using System; +using System.Runtime.CompilerServices; + +namespace Microsoft.Toolkit.Mvvm.Messaging.Internals +{ + /// + /// A simple type representing an immutable pair of types. + /// + /// + /// This type replaces a simple as it's faster in its + /// and methods, and because + /// unlike a value tuple it exposes its fields as immutable. Additionally, the + /// and fields provide additional clarity reading + /// the code compared to and . + /// + internal readonly struct Type2 : IEquatable + { + /// + /// The type of registered message. + /// + public readonly Type TMessage; + + /// + /// The type of registration token. + /// + public readonly Type TToken; + + /// + /// Initializes a new instance of the struct. + /// + /// The type of registered message. + /// The type of registration token. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public Type2(Type tMessage, Type tToken) + { + TMessage = tMessage; + TToken = tToken; + } + + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public bool Equals(Type2 other) + { + // We can't just use reference equality, as that's technically not guaranteed + // to work and might fail in very rare cases (eg. with type forwarding between + // different assemblies). Instead, we can use the == operator to compare for + // equality, which still avoids the callvirt overhead of calling Type.Equals, + // and is also implemented as a JIT intrinsic on runtimes such as .NET Core. + return + TMessage == other.TMessage && + TToken == other.TToken; + } + + /// + public override bool Equals(object? obj) + { + return obj is Type2 other && Equals(other); + } + + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public override int GetHashCode() + { + unchecked + { + // To combine the two hashes, we can simply use the fast djb2 hash algorithm. + // This is not a problem in this case since we already know that the base + // RuntimeHelpers.GetHashCode method is providing hashes with a good enough distribution. + int hash = RuntimeHelpers.GetHashCode(TMessage); + + hash = (hash << 5) + hash; + + hash += RuntimeHelpers.GetHashCode(TToken); + + return hash; + } + } + } +} diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Internals/Unit.cs b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Unit.cs new file mode 100644 index 00000000000..5748643cf83 --- /dev/null +++ b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Unit.cs @@ -0,0 +1,31 @@ +using System; +using System.Runtime.CompilerServices; + +namespace Microsoft.Toolkit.Mvvm.Messaging.Internals +{ + /// + /// An empty type representing a generic token with no specific value. + /// + internal readonly struct Unit : IEquatable + { + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public bool Equals(Unit other) + { + return true; + } + + /// + public override bool Equals(object? obj) + { + return obj is Unit; + } + + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public override int GetHashCode() + { + return 0; + } + } +} diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index 6f6bf62a047..3a7bed003e7 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -8,6 +8,7 @@ using System.Runtime.CompilerServices; using System.Threading; using Microsoft.Collections.Extensions; +using Microsoft.Toolkit.Mvvm.Messaging.Internals; namespace Microsoft.Toolkit.Mvvm.Messaging { @@ -30,8 +31,8 @@ public sealed class Messenger : IMessenger // | / / \ // DictionarySlim>> mapping = Mapping // / / / - // ___(Type2.tToken)____/ / / - // /________________(Type2.tMessage)________________________/ / + // ___(Type2.TToken)____/ / / + // /________________(Type2.TMessage)________________________/ / // / ____________________________________________________________/ // / / // DictionarySlim typesMap; @@ -593,80 +594,6 @@ public override int GetHashCode() } } - /// - /// A simple type representing an immutable pair of types. - /// - /// - /// This type replaces a simple as it's faster in its - /// and methods, and because - /// unlike a value tuple it exposes its fields as immutable. Additionally, the - /// and fields provide additional clarity reading - /// the code compared to and . - /// - private readonly struct Type2 : IEquatable - { - /// - /// The type of registered message. - /// - private readonly Type tMessage; - - /// - /// The type of registration token. - /// - private readonly Type tToken; - - /// - /// Initializes a new instance of the struct. - /// - /// The type of registered message. - /// The type of registration token. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public Type2(Type tMessage, Type tToken) - { - this.tMessage = tMessage; - this.tToken = tToken; - } - - /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public bool Equals(Type2 other) - { - // We can't just use reference equality, as that's technically not guaranteed - // to work and might fail in very rare cases (eg. with type forwarding between - // different assemblies). Instead, we can use the == operator to compare for - // equality, which still avoids the callvirt overhead of calling Type.Equals, - // and is also implemented as a JIT intrinsic on runtimes such as .NET Core. - return - this.tMessage == other.tMessage && - this.tToken == other.tToken; - } - - /// - public override bool Equals(object? obj) - { - return obj is Type2 other && Equals(other); - } - - /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public override int GetHashCode() - { - unchecked - { - // To combine the two hashes, we can simply use the fast djb2 hash algorithm. - // This is not a problem in this case since we already know that the base - // RuntimeHelpers.GetHashCode method is providing hashes with a good enough distribution. - int hash = RuntimeHelpers.GetHashCode(this.tMessage); - - hash = (hash << 5) + hash; - - hash += RuntimeHelpers.GetHashCode(this.tToken); - - return hash; - } - } - } - /// /// Throws an when trying to add a duplicate handler. /// diff --git a/Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.Unit.cs b/Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.Unit.cs deleted file mode 100644 index 67dfbc1c7ea..00000000000 --- a/Microsoft.Toolkit.Mvvm/Messaging/MessengerExtensions.Unit.cs +++ /dev/null @@ -1,41 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System; -using System.Runtime.CompilerServices; - -namespace Microsoft.Toolkit.Mvvm.Messaging -{ - /// - /// Extensions for the type. - /// - public static partial class MessengerExtensions - { - /// - /// An empty type representing a generic token with no specific value. - /// - private readonly struct Unit : IEquatable - { - /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public bool Equals(Unit other) - { - return true; - } - - /// - public override bool Equals(object? obj) - { - return obj is Unit; - } - - /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public override int GetHashCode() - { - return 0; - } - } - } -} diff --git a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs index 91ed0800868..643038edeeb 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs @@ -10,6 +10,7 @@ using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using Microsoft.Collections.Extensions; +using Microsoft.Toolkit.Mvvm.Messaging.Internals; namespace Microsoft.Toolkit.Mvvm.Messaging { @@ -397,80 +398,6 @@ public void Dispose() } } - /// - /// A simple type representing an immutable pair of types. - /// - /// - /// This type replaces a simple as it's faster in its - /// and methods, and because - /// unlike a value tuple it exposes its fields as immutable. Additionally, the - /// and fields provide additional clarity reading - /// the code compared to and . - /// - private readonly struct Type2 : IEquatable - { - /// - /// The type of registered message. - /// - public readonly Type TMessage; - - /// - /// The type of registration token. - /// - public readonly Type TToken; - - /// - /// Initializes a new instance of the struct. - /// - /// The type of registered message. - /// The type of registration token. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public Type2(Type tMessage, Type tToken) - { - TMessage = tMessage; - TToken = tToken; - } - - /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public bool Equals(Type2 other) - { - // We can't just use reference equality, as that's technically not guaranteed - // to work and might fail in very rare cases (eg. with type forwarding between - // different assemblies). Instead, we can use the == operator to compare for - // equality, which still avoids the callvirt overhead of calling Type.Equals, - // and is also implemented as a JIT intrinsic on runtimes such as .NET Core. - return - TMessage == other.TMessage && - TToken == other.TToken; - } - - /// - public override bool Equals(object? obj) - { - return obj is Type2 other && Equals(other); - } - - /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public override int GetHashCode() - { - unchecked - { - // To combine the two hashes, we can simply use the fast djb2 hash algorithm. - // This is not a problem in this case since we already know that the base - // RuntimeHelpers.GetHashCode method is providing hashes with a good enough distribution. - int hash = RuntimeHelpers.GetHashCode(TMessage); - - hash = (hash << 5) + hash; - - hash += RuntimeHelpers.GetHashCode(TToken); - - return hash; - } - } - } - /// /// Throws an when trying to add a duplicate handler. /// From c6553d1f41482c27e7ef6a2b53246e0eee77d8f2 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 13 Sep 2020 12:55:56 +0200 Subject: [PATCH 22/38] Removed some unnecessary bounds checks --- Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs index 643038edeeb..04125dbb2ef 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs @@ -206,16 +206,15 @@ public TMessage Send(TMessage message, TToken token) ReadOnlySpan recipientsSpan = recipients.Span, handlersSpan = handlers.Span; - int handlersCount = recipients.Count; - for (int i = 0; i < handlersCount; i++) + for (int i = 0; i < recipientsSpan.Length; i++) { // Just like in the other messenger, here we need an unsafe cast to be able to // invoke a generic delegate with a contravariant input argument, with a less // derived reference, without reflection. This is guaranteed to work by how the // messenger tracks registered recipients and their associated handlers, so the // type conversion will always be valid (the recipients are the rigth instances). - Unsafe.As>(handlersSpan[i])(recipientsSpan![i], message); + Unsafe.As>(handlersSpan[i])(recipientsSpan[i], message); } } finally From 6f0f7c3ac0b1c6c927465773f1cb9f8c1b41a076 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 13 Sep 2020 13:06:01 +0200 Subject: [PATCH 23/38] Minor code refactoring --- Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 54 +++++++++---------- .../DictionarySlim{TKey,TValue}.cs | 7 --- .../IDictionarySlim{TKey}.cs | 9 +--- 3 files changed, 26 insertions(+), 44 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index 20ed40b52ab..af3e60e17ad 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -168,13 +168,13 @@ public void UnregisterAll(object recipient) // dictionary (a hashed collection) only costs O(1) in the best case, while // if we had tried to iterate the whole dictionary every time we would have // paid an O(n) minimum cost for each single remove operation. - this.typesMap.Remove(mapping.TypeArguments); + this.typesMap.TryRemove(mapping.TypeArguments, out _); } } } // Remove the associated set in the recipients map - this.recipientsMap.Remove(key); + this.recipientsMap.TryRemove(key, out _); } } @@ -242,26 +242,24 @@ public void UnregisterAll(object recipient, TToken token) // Try to remove the registered handler for the input token, // for the current message type (unknown from here). - if (holder.Remove(token)) + if (holder.TryRemove(token, out _) && + holder.Count == 0) { - if (holder.Count == 0) - { - // If the map is empty, remove the recipient entirely from its container - map.Remove(key); + // If the map is empty, remove the recipient entirely from its container + map.TryRemove(key, out _); - if (map.Count == 0) + if (map.Count == 0) + { + // If no handlers are left at all for the recipient, across all + // message types and token types, remove the set of mappings + // entirely for the current recipient, and lost the strong + // reference to it as well. This is the same situation that + // would've been achieved by just calling UnregisterAll(recipient). + set.Remove(Unsafe.As(map)); + + if (set.Count == 0) { - // If no handlers are left at all for the recipient, across all - // message types and token types, remove the set of mappings - // entirely for the current recipient, and lost the strong - // reference to it as well. This is the same situation that - // would've been achieved by just calling UnregisterAll(recipient). - set.Remove(Unsafe.As(map)); - - if (set.Count == 0) - { - this.recipientsMap.Remove(key); - } + this.recipientsMap.TryRemove(key, out _); } } } @@ -309,25 +307,23 @@ public void Unregister(object recipient, TToken token) } // Remove the target handler - if (dictionary!.Remove(token)) + if (dictionary!.TryRemove(token, out _) && + dictionary.Count == 0) { // If the map is empty, it means that the current recipient has no remaining // registered handlers for the current combination, regardless, // of the specific token value (ie. the channel used to receive messages of that type). // We can remove the map entirely from this container, and remove the link to the map itself // to the current mapping between existing registered recipients (or entire recipients too). - if (dictionary.Count == 0) - { - mapping.Remove(key); + mapping.TryRemove(key, out _); - HashSet set = this.recipientsMap[key]; + HashSet set = this.recipientsMap[key]; - set.Remove(mapping); + set.Remove(mapping); - if (set.Count == 0) - { - this.recipientsMap.Remove(key); - } + if (set.Count == 0) + { + this.recipientsMap.TryRemove(key, out _); } } } diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs b/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs index ab4706de5f7..a117912073b 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs @@ -218,13 +218,6 @@ public bool TryRemove(TKey key, out object? result) return false; } - /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public bool Remove(TKey key) - { - return TryRemove(key, out _); - } - /// /// Gets the value for the specified key, or, if the key is not present, /// adds an entry and returns the value by ref. This makes it possible to diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/IDictionarySlim{TKey}.cs b/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/IDictionarySlim{TKey}.cs index 7de2505148a..04b2fcdf358 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/IDictionarySlim{TKey}.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Microsoft.Collections.Extensions/IDictionarySlim{TKey}.cs @@ -14,18 +14,11 @@ internal interface IDictionarySlim : IDictionarySlim where TKey : IEquatable { /// - /// Tries to remove a value with a specified key. + /// Tries to remove a value with a specified key, if present. /// /// The key of the value to remove. /// The removed value, if it was present. /// .Whether or not the key was present. bool TryRemove(TKey key, out object? result); - - /// - /// Removes an item from the dictionary with the specified key, if present. - /// - /// The key of the item to remove. - /// Whether or not an item was removed. - bool Remove(TKey key); } } From 9d8cff68e295ab513776919090de15d181cc9012 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 13 Sep 2020 20:51:37 +0200 Subject: [PATCH 24/38] Fixed a merge error, minor code refactoring --- .../DictionarySlim{TKey,TValue}.cs | 19 +++++++++++++++++-- .../IDictionarySlim{TKey}.cs | 10 +--------- Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 6 +++--- .../Messaging/WeakRefMessenger.cs | 8 ++++---- 4 files changed, 25 insertions(+), 18 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs index 1eecd25f275..11012a1e350 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs @@ -130,7 +130,11 @@ public void Clear() this.entries = InitialEntries; } - /// + /// + /// Checks whether or not the dictionary contains a pair with a specified key. + /// + /// The key to look for. + /// Whether or not the key was present in the dictionary. public bool ContainsKey(TKey key) { Entry[] entries = this.entries; @@ -176,7 +180,18 @@ public bool TryGetValue(TKey key, out TValue? value) } /// - public bool TryRemove(TKey key, out object? result) + public bool TryRemove(TKey key) + { + return TryRemove(key, out _); + } + + /// + /// Tries to remove a value with a specified key, if present. + /// + /// The key of the value to remove. + /// The removed value, if it was present. + /// Whether or not the key was present. + public bool TryRemove(TKey key, out TValue? result) { Entry[] entries = this.entries; int bucketIndex = key.GetHashCode() & (this.buckets.Length - 1); diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/IDictionarySlim{TKey}.cs b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/IDictionarySlim{TKey}.cs index 30d4677360a..eeb4fa75f60 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/IDictionarySlim{TKey}.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/IDictionarySlim{TKey}.cs @@ -13,19 +13,11 @@ namespace Microsoft.Collections.Extensions internal interface IDictionarySlim : IDictionarySlim where TKey : IEquatable { - /// - /// Checks whether or not the dictionary contains a pair with a specified key. - /// - /// The key to look for. - /// Whether or not the key was present in the dictionary. - public bool ContainsKey(TKey key); - /// /// Tries to remove a value with a specified key, if present. /// /// The key of the value to remove. - /// The removed value, if it was present. /// Whether or not the key was present. - bool TryRemove(TKey key, out object? result); + bool TryRemove(TKey key); } } diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index dac82acc0c1..eb993b0731d 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -157,7 +157,7 @@ public void UnregisterAll(object recipient) // Removes all the lists of registered handlers for the recipient foreach (IMapping mapping in set!) { - if (mapping.TryRemove(key, out _)) + if (mapping.TryRemove(key)) { if (mapping.Count == 0) { @@ -243,11 +243,11 @@ public void UnregisterAll(object recipient, TToken token) // Try to remove the registered handler for the input token, // for the current message type (unknown from here). - if (holder.TryRemove(token, out _) && + if (holder.TryRemove(token) && holder.Count == 0) { // If the map is empty, remove the recipient entirely from its container - map.TryRemove(key, out _); + map.TryRemove(key); if (map.Count == 0) { diff --git a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs index 04125dbb2ef..13d9d3dd4a1 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs @@ -65,7 +65,7 @@ public bool IsRegistered(object recipient, TToken token) return this.recipientsMap.TryGetValue(type2, out ConditionalWeakTable? table) && table!.TryGetValue(recipient, out IDictionarySlim mapping) && - Unsafe.As>(mapping).ContainsKey(token); + Unsafe.As>(mapping).ContainsKey(token); } } @@ -133,7 +133,7 @@ public void UnregisterAll(object recipient, TToken token) if (enumerator.Key.TToken == typeof(TToken) && enumerator.Value.TryGetValue(recipient, out IDictionarySlim mapping)) { - Unsafe.As>(mapping).TryRemove(token, out _); + Unsafe.As>(mapping).TryRemove(token, out _); } } } @@ -156,7 +156,7 @@ public void Unregister(object recipient, TToken token) if (enumerator.Key.Equals(type2) && enumerator.Value.TryGetValue(recipient, out IDictionarySlim mapping)) { - Unsafe.As>(mapping).TryRemove(token, out _); + Unsafe.As>(mapping).TryRemove(token, out _); } } } @@ -275,7 +275,7 @@ void IMessenger.Cleanup() // Remove all the mappings with no handlers left foreach (Type2 key in type2s.Span) { - this.recipientsMap.Remove(key); + this.recipientsMap.TryRemove(key, out _); } } } From 15a0c3b59ec6a57cf34a25a6609a1caf31942b61 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 14 Sep 2020 00:15:28 +0200 Subject: [PATCH 25/38] Minor code tweaks --- .../Messaging/WeakRefMessenger.cs | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs index 13d9d3dd4a1..fa7091e3a5e 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs @@ -8,7 +8,6 @@ using System.Buffers; using System.Collections.Generic; using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; using Microsoft.Collections.Extensions; using Microsoft.Toolkit.Mvvm.Messaging.Internals; @@ -293,7 +292,7 @@ public void Reset() /// A simple buffer writer implementation using pooled arrays. /// /// The type of items to store in the list. - public sealed class ArrayPoolBufferWriter : IDisposable + private sealed class ArrayPoolBufferWriter : IDisposable { /// /// The default buffer size to use to expand empty arrays. @@ -325,17 +324,7 @@ public ArrayPoolBufferWriter() public ReadOnlySpan Span { [MethodImpl(MethodImplOptions.AggressiveInlining)] - get - { - T[] array = this.array; - - if (this.index > 0) - { - return MemoryMarshal.CreateReadOnlySpan(ref array[0], this.index); - } - - return default; - } + get => this.array.AsSpan(0, this.index); } /// From 1cf168b3929d25d298654fc7622afb6873fae834 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 14 Sep 2020 00:39:26 +0200 Subject: [PATCH 26/38] Backported weak messenger to .NET Standard 2.0 --- .../Messaging/WeakRefMessenger.cs | 103 ++++++++++++++++-- 1 file changed, 92 insertions(+), 11 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs index fa7091e3a5e..974d17e3e4e 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs @@ -2,14 +2,17 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#if NETSTANDARD2_1 - using System; using System.Buffers; using System.Collections.Generic; using System.Runtime.CompilerServices; using Microsoft.Collections.Extensions; using Microsoft.Toolkit.Mvvm.Messaging.Internals; +#if NETSTANDARD2_1 +using RecipientsTable = System.Runtime.CompilerServices.ConditionalWeakTable; +#else +using RecipientsTable = Microsoft.Toolkit.Mvvm.Messaging.WeakRefMessenger.ConditionalWeakTable; +#endif namespace Microsoft.Toolkit.Mvvm.Messaging { @@ -42,8 +45,7 @@ public sealed class WeakRefMessenger : IMessenger /// /// The map of currently registered recipients for all message types. /// - private readonly DictionarySlim> recipientsMap - = new DictionarySlim>(); + private readonly DictionarySlim recipientsMap = new DictionarySlim(); /// /// Gets the default instance. @@ -62,8 +64,8 @@ public bool IsRegistered(object recipient, TToken token) // Get the conditional table associated with the target recipient, for the current pair // of token and message types. If it exists, check if there is a matching token. return - this.recipientsMap.TryGetValue(type2, out ConditionalWeakTable? table) && - table!.TryGetValue(recipient, out IDictionarySlim mapping) && + this.recipientsMap.TryGetValue(type2, out RecipientsTable? table) && + table!.TryGetValue(recipient, out IDictionarySlim? mapping) && Unsafe.As>(mapping).ContainsKey(token); } } @@ -79,9 +81,9 @@ public void Register(TRecipient recipient, TToken Type2 type2 = new Type2(typeof(TMessage), typeof(TToken)); // Get the conditional table for the pair of type arguments, or create it if it doesn't exist - ref ConditionalWeakTable? mapping = ref this.recipientsMap.GetOrAddValueRef(type2); + ref RecipientsTable? mapping = ref this.recipientsMap.GetOrAddValueRef(type2); - mapping ??= new ConditionalWeakTable(); + mapping ??= new RecipientsTable(); // Get or create the handlers dictionary for the target recipient var map = Unsafe.As>(mapping.GetValue(recipient, _ => new DictionarySlim())); @@ -174,7 +176,7 @@ public TMessage Send(TMessage message, TToken token) Type2 type2 = new Type2(typeof(TMessage), typeof(TToken)); // Try to get the target table - if (!this.recipientsMap.TryGetValue(type2, out ConditionalWeakTable? table)) + if (!this.recipientsMap.TryGetValue(type2, out RecipientsTable? table)) { return message; } @@ -288,6 +290,87 @@ public void Reset() } } +#if !NETSTANDARD2_1 + /// + /// A wrapper for + /// that backports the enumerable support to .NET Standard 2.0 through an auxiliary list. + /// + /// Tke key of items to store in the table. + /// The values to store in the table. + internal sealed class ConditionalWeakTable + where TKey : class + where TValue : class? + { + /// + /// The underlying instance. + /// + private readonly System.Runtime.CompilerServices.ConditionalWeakTable table; + + /// + /// A supporting linked list to store keys in . This is needed to expose + /// the ability to enumerate existing keys when there is no support for that in the BCL. + /// + private readonly LinkedList> keys; + + /// + /// Initializes a new instance of the class. + /// + public ConditionalWeakTable() + { + this.table = new System.Runtime.CompilerServices.ConditionalWeakTable(); + this.keys = new LinkedList>(); + } + + /// + public bool TryGetValue(TKey key, out TValue value) + { + return this.table.TryGetValue(key, out value); + } + + /// + public TValue GetValue(TKey key, System.Runtime.CompilerServices.ConditionalWeakTable.CreateValueCallback createValueCallback) + { + // Get or create the value. When this method returns, the key will be present in the table + TValue value = this.table.GetValue(key, createValueCallback); + + // Check if the list of keys contains the given key. + // If it does, we can just stop here and return the result. + foreach (WeakReference node in this.keys) + { + if (node.TryGetTarget(out TKey? target) && + ReferenceEquals(target, key)) + { + return value; + } + } + + // Add the key to the list of weak references to track it + this.keys.AddFirst(new WeakReference(key)); + + return value; + } + + /// + public bool Remove(TKey key) + { + return this.table.Remove(key); + } + + /// + public IEnumerator> GetEnumerator() + { + foreach (WeakReference node in this.keys) + { + if (node.TryGetTarget(out TKey? target) && + this.table.TryGetValue(target, out TValue value)) + { + yield return new KeyValuePair(target, value); + } + } + } + } +#endif + /// /// A simple buffer writer implementation using pooled arrays. /// @@ -395,5 +478,3 @@ private static void ThrowInvalidOperationExceptionForDuplicateRegistration() } } } - -#endif From 9e7f12adb9b6d8d475dc97ad5b671162f5be41a1 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 14 Sep 2020 00:43:51 +0200 Subject: [PATCH 27/38] Enabled weak messenger tests on .NET Standard 2.0 --- .../Mvvm/Test_Messenger.Request.cs | 22 ------------ .../UnitTests.Shared/Mvvm/Test_Messenger.cs | 34 ------------------- .../Mvvm/Test_ObservableRecipient.cs | 8 ----- 3 files changed, 64 deletions(-) diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs index 0d3c9b48d60..6d116009a1f 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs @@ -16,9 +16,7 @@ public partial class Test_Messenger [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_RequestMessage_Ok(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -47,9 +45,7 @@ void Receive(object recipient, NumberRequestMessage m) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif [ExpectedException(typeof(InvalidOperationException))] public void Test_Messenger_RequestMessage_Fail_NoReply(Type type) { @@ -61,9 +57,7 @@ public void Test_Messenger_RequestMessage_Fail_NoReply(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif [ExpectedException(typeof(InvalidOperationException))] public void Test_Messenger_RequestMessage_Fail_MultipleReplies(Type type) { @@ -88,9 +82,7 @@ public class NumberRequestMessage : RequestMessage [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public async Task Test_Messenger_AsyncRequestMessage_Ok_Sync(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -115,9 +107,7 @@ void Receive(object recipient, AsyncNumberRequestMessage m) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public async Task Test_Messenger_AsyncRequestMessage_Ok_Async(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -149,9 +139,7 @@ void Receive(object recipient, AsyncNumberRequestMessage m) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif [ExpectedException(typeof(InvalidOperationException))] public async Task Test_Messenger_AsyncRequestMessage_Fail_NoReply(Type type) { @@ -163,9 +151,7 @@ public async Task Test_Messenger_AsyncRequestMessage_Fail_NoReply(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif [ExpectedException(typeof(InvalidOperationException))] public async Task Test_Messenger_AsyncRequestMessage_Fail_MultipleReplies(Type type) { @@ -190,9 +176,7 @@ public class AsyncNumberRequestMessage : AsyncRequestMessage [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_CollectionRequestMessage_Ok_NoReplies(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -212,9 +196,7 @@ void Receive(object recipient, NumbersCollectionRequestMessage m) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_CollectionRequestMessage_Ok_MultipleReplies(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -269,9 +251,7 @@ public class NumbersCollectionRequestMessage : CollectionRequestMessage [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public async Task Test_Messenger_AsyncCollectionRequestMessage_Ok_NoReplies(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -291,9 +271,7 @@ void Receive(object recipient, AsyncNumbersCollectionRequestMessage m) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public async Task Test_Messenger_AsyncCollectionRequestMessage_Ok_MultipleReplies(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs index a429baa7552..7ed62e3bc00 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs @@ -14,9 +14,7 @@ public partial class Test_Messenger [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_UnregisterRecipientWithMessageType(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -28,9 +26,7 @@ public void Test_Messenger_UnregisterRecipientWithMessageType(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_UnregisterRecipientWithMessageTypeAndToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -42,9 +38,7 @@ public void Test_Messenger_UnregisterRecipientWithMessageTypeAndToken(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_UnregisterRecipientWithToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -56,9 +50,7 @@ public void Test_Messenger_UnregisterRecipientWithToken(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_UnregisterRecipientWithRecipient(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -70,9 +62,7 @@ public void Test_Messenger_UnregisterRecipientWithRecipient(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageType(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -88,9 +78,7 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageType(Type ty [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageTypeAndToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -106,9 +94,7 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageTypeAndToken [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_RegisterAndUnregisterRecipientWithToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -124,9 +110,7 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithToken(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_RegisterAndUnregisterRecipientWithRecipient(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -142,9 +126,7 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithRecipient(Type type [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithNoToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -184,9 +166,7 @@ public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithN [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_IsRegistered_Register_Send_UnregisterRecipient_WithNoToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -216,9 +196,7 @@ public void Test_Messenger_IsRegistered_Register_Send_UnregisterRecipient_WithNo [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -248,9 +226,7 @@ public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithT [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_DuplicateRegistrationWithMessageType(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -267,9 +243,7 @@ public void Test_Messenger_DuplicateRegistrationWithMessageType(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_DuplicateRegistrationWithMessageTypeAndToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -286,9 +260,7 @@ public void Test_Messenger_DuplicateRegistrationWithMessageTypeAndToken(Type typ [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_IRecipient_NoMessages(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -303,9 +275,7 @@ public void Test_Messenger_IRecipient_NoMessages(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_IRecipient_SomeMessages_NoToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -338,9 +308,7 @@ public void Test_Messenger_IRecipient_SomeMessages_NoToken(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_IRecipient_SomeMessages_WithToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -377,9 +345,7 @@ public void Test_Messenger_IRecipient_SomeMessages_WithToken(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_Messenger_RegisterWithTypeParameter(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs index be41663f116..5a3c91505b5 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs @@ -16,9 +16,7 @@ public class Test_ObservableRecipient [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_ObservableRecipient_Activation(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -40,9 +38,7 @@ public void Test_ObservableRecipient_Activation(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_ObservableRecipient_IsSame(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -63,9 +59,7 @@ public void Test_ObservableRecipient_Default() [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_ObservableRecipient_Injection(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -77,9 +71,7 @@ public void Test_ObservableRecipient_Injection(Type type) [TestCategory("Mvvm")] [TestMethod] [DataRow(typeof(Messenger))] -#if NETCOREAPP3_1 [DataRow(typeof(WeakRefMessenger))] -#endif public void Test_ObservableRecipient_Broadcast(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); From 8212a9580d2b4cf97842f78a5ee6e2327c0c3e0c Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 14 Sep 2020 01:09:29 +0200 Subject: [PATCH 28/38] Improved cleanup on .NET Standard 2.0 polyfill --- .../Messaging/WeakRefMessenger.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs index 974d17e3e4e..136966dacd2 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs @@ -271,6 +271,11 @@ void IMessenger.Cleanup() { type2s.Add(enumerator.Key); } + +#if !NETSTANDARD2_1 + // Do the cleanup for the list of keys that are weakly tracked too + enumerator.Value.Cleanup(); +#endif } // Remove all the mappings with no handlers left @@ -368,6 +373,25 @@ public IEnumerator> GetEnumerator() } } } + + /// + /// Trims the list of tracked instances. + /// + public void Cleanup() + { + for (LinkedListNode>? node = this.keys.First; !(node is null);) + { + LinkedListNode>? next = node.Next; + + // Remove all the nodes with a target that has been collected + if (!node.Value.TryGetTarget(out _)) + { + this.keys.Remove(node); + } + + node = next; + } + } } #endif From 0e9e7dbd8a5575583474ab49a58f087e88ac496d Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 14 Sep 2020 01:17:48 +0200 Subject: [PATCH 29/38] Removed explicit interface implementation --- Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs index 136966dacd2..92caa1f216f 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs @@ -228,7 +228,7 @@ public TMessage Send(TMessage message, TToken token) } /// - void IMessenger.Cleanup() + public void Cleanup() { lock (this.recipientsMap) { From 72a2236fe87b7c646065bfe9f95e32d8b897a84e Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 14 Sep 2020 12:58:46 +0200 Subject: [PATCH 30/38] Minor code refactoring --- Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs | 50 ++++++++----------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs index eb993b0731d..bf7d01932a2 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs @@ -157,20 +157,18 @@ public void UnregisterAll(object recipient) // Removes all the lists of registered handlers for the recipient foreach (IMapping mapping in set!) { - if (mapping.TryRemove(key)) + if (mapping.TryRemove(key) && + mapping.Count == 0) { - if (mapping.Count == 0) - { - // Maps here are really of type Mapping<,> and with unknown type arguments. - // If after removing the current recipient a given map becomes empty, it means - // that there are no registered recipients at all for a given pair of message - // and token types. In that case, we also remove the map from the types map. - // The reason for keeping a key in each mapping is that removing items from a - // dictionary (a hashed collection) only costs O(1) in the best case, while - // if we had tried to iterate the whole dictionary every time we would have - // paid an O(n) minimum cost for each single remove operation. - this.typesMap.TryRemove(mapping.TypeArguments, out _); - } + // Maps here are really of type Mapping<,> and with unknown type arguments. + // If after removing the current recipient a given map becomes empty, it means + // that there are no registered recipients at all for a given pair of message + // and token types. In that case, we also remove the map from the types map. + // The reason for keeping a key in each mapping is that removing items from a + // dictionary (a hashed collection) only costs O(1) in the best case, while + // if we had tried to iterate the whole dictionary every time we would have + // paid an O(n) minimum cost for each single remove operation. + this.typesMap.TryRemove(mapping.TypeArguments, out _); } } @@ -249,19 +247,16 @@ public void UnregisterAll(object recipient, TToken token) // If the map is empty, remove the recipient entirely from its container map.TryRemove(key); - if (map.Count == 0) + // If no handlers are left at all for the recipient, across all + // message types and token types, remove the set of mappings + // entirely for the current recipient, and lost the strong + // reference to it as well. This is the same situation that + // would've been achieved by just calling UnregisterAll(recipient). + if (map.Count == 0 && + set.Remove(Unsafe.As(map)) && + set.Count == 0) { - // If no handlers are left at all for the recipient, across all - // message types and token types, remove the set of mappings - // entirely for the current recipient, and lost the strong - // reference to it as well. This is the same situation that - // would've been achieved by just calling UnregisterAll(recipient). - set.Remove(Unsafe.As(map)); - - if (set.Count == 0) - { - this.recipientsMap.TryRemove(key, out _); - } + this.recipientsMap.TryRemove(key, out _); } } } @@ -320,9 +315,8 @@ public void Unregister(object recipient, TToken token) HashSet set = this.recipientsMap[key]; - set.Remove(mapping); - - if (set.Count == 0) + if (set.Remove(mapping) && + set.Count == 0) { this.recipientsMap.TryRemove(key, out _); } From f583a66548f04618bc14145884f7d41b0a6bf649 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 14 Sep 2020 13:03:17 +0200 Subject: [PATCH 31/38] Added auto trimming to CVT<,> polyfill --- .../Messaging/WeakRefMessenger.cs | 30 +++++-------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs index 92caa1f216f..fd020b4c1be 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs @@ -271,11 +271,6 @@ public void Cleanup() { type2s.Add(enumerator.Key); } - -#if !NETSTANDARD2_1 - // Do the cleanup for the list of keys that are weakly tracked too - enumerator.Value.Cleanup(); -#endif } // Remove all the mappings with no handlers left @@ -363,29 +358,20 @@ public bool Remove(TKey key) /// public IEnumerator> GetEnumerator() - { - foreach (WeakReference node in this.keys) - { - if (node.TryGetTarget(out TKey? target) && - this.table.TryGetValue(target, out TValue value)) - { - yield return new KeyValuePair(target, value); - } - } - } - - /// - /// Trims the list of tracked instances. - /// - public void Cleanup() { for (LinkedListNode>? node = this.keys.First; !(node is null);) { LinkedListNode>? next = node.Next; - // Remove all the nodes with a target that has been collected - if (!node.Value.TryGetTarget(out _)) + // Get the key and value for the current node + if (node.Value.TryGetTarget(out TKey? target) && + this.table.TryGetValue(target!, out TValue value)) + { + yield return new KeyValuePair(target, value); + } + else { + // If the current key has been collected, trim the list this.keys.Remove(node); } From 8c17b0af7157ae0bc4c1e2169c0c96cb26afe3b1 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 14 Sep 2020 13:51:33 +0200 Subject: [PATCH 32/38] Added recipient garbage collect test --- .../UnitTests.Shared/Mvvm/Test_Messenger.cs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs index 7ed62e3bc00..08f843c2e08 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs @@ -360,6 +360,36 @@ public void Test_Messenger_RegisterWithTypeParameter(Type type) Assert.AreEqual(number, 42); } + [TestCategory("Mvvm")] + [TestMethod] + [DataRow(typeof(Messenger), false)] + [DataRow(typeof(WeakRefMessenger), true)] + public void Test_Messenger_Collect_Test(Type type, bool isWeak) + { + var messenger = (IMessenger)Activator.CreateInstance(type); + + WeakReference weakRecipient; + + void Test() + { + var recipient = new RecipientWithNoMessages { Number = 42 }; + weakRecipient = new WeakReference(recipient); + + messenger.Register(recipient, (r, m) => { }); + + Assert.IsTrue(messenger.IsRegistered(recipient)); + Assert.IsTrue(weakRecipient.IsAlive); + + GC.KeepAlive(recipient); + } + + Test(); + + GC.Collect(); + + Assert.AreEqual(!isWeak, weakRecipient.IsAlive); + } + public sealed class RecipientWithNoMessages { public int Number { get; set; } From 603e9d8612491eeb17e3440afbc62f8c0b88bf79 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 14 Sep 2020 19:15:33 +0200 Subject: [PATCH 33/38] Removed unnecessary property --- Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs index fd020b4c1be..1ed74093c38 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs @@ -420,15 +420,6 @@ public ReadOnlySpan Span get => this.array.AsSpan(0, this.index); } - /// - /// Gets the current number of items. - /// - public int Count - { - [MethodImpl(MethodImplOptions.AggressiveInlining)] - get => this.index; - } - /// /// Adds a new item to the current collection. /// From 2613c566281e9158e5bde2c3b3033b69794677ec Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 14 Sep 2020 19:28:59 +0200 Subject: [PATCH 34/38] Improved test coverage --- .../Mvvm/Test_AsyncRelayCommand{T}.cs | 141 ++++++++++++++++++ .../UnitTests.Shared/Mvvm/Test_Messenger.cs | 127 +++++++++++++++- .../Mvvm/Test_RelayCommand{T}.cs | 20 ++- .../UnitTests.Shared.projitems | 1 + 4 files changed, 285 insertions(+), 4 deletions(-) create mode 100644 UnitTests/UnitTests.Shared/Mvvm/Test_AsyncRelayCommand{T}.cs diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_AsyncRelayCommand{T}.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_AsyncRelayCommand{T}.cs new file mode 100644 index 00000000000..bf6320fa083 --- /dev/null +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_AsyncRelayCommand{T}.cs @@ -0,0 +1,141 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; +using Microsoft.Toolkit.Mvvm.Input; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace UnitTests.Mvvm +{ + [TestClass] + [SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1649", Justification = "Generic type")] + public class Test_AsyncRelayCommandOfT + { + [TestCategory("Mvvm")] + [TestMethod] + public async Task Test_AsyncRelayCommandOfT_AlwaysEnabled() + { + int ticks = 0; + + var command = new AsyncRelayCommand(async s => + { + await Task.Delay(1000); + ticks = int.Parse(s); + await Task.Delay(1000); + }); + + Assert.IsTrue(command.CanExecute(null)); + Assert.IsTrue(command.CanExecute("1")); + + (object, EventArgs) args = default; + + command.CanExecuteChanged += (s, e) => args = (s, e); + + command.NotifyCanExecuteChanged(); + + Assert.AreSame(args.Item1, command); + Assert.AreSame(args.Item2, EventArgs.Empty); + + Assert.IsNull(command.ExecutionTask); + Assert.IsFalse(command.IsRunning); + + Task task = command.ExecuteAsync((object)"42"); + + Assert.IsNotNull(command.ExecutionTask); + Assert.AreSame(command.ExecutionTask, task); + Assert.IsTrue(command.IsRunning); + + await task; + + Assert.IsFalse(command.IsRunning); + + Assert.AreEqual(ticks, 42); + + command.Execute("2"); + + await command.ExecutionTask!; + + Assert.AreEqual(ticks, 2); + + Assert.ThrowsException(() => command.Execute(new object())); + } + + [TestCategory("Mvvm")] + [TestMethod] + public void Test_AsyncRelayCommandOfT_WithCanExecuteFunctionTrue() + { + int ticks = 0; + + var command = new AsyncRelayCommand( + s => + { + ticks = int.Parse(s); + return Task.CompletedTask; + }, s => true); + + Assert.IsTrue(command.CanExecute(null)); + Assert.IsTrue(command.CanExecute("1")); + + command.Execute("42"); + + Assert.AreEqual(ticks, 42); + + command.Execute("2"); + + Assert.AreEqual(ticks, 2); + } + + [TestCategory("Mvvm")] + [TestMethod] + public void Test_AsyncRelayCommandOfT_WithCanExecuteFunctionFalse() + { + int ticks = 0; + + var command = new AsyncRelayCommand( + s => + { + ticks = int.Parse(s); + return Task.CompletedTask; + }, s => false); + + Assert.IsFalse(command.CanExecute(null)); + Assert.IsFalse(command.CanExecute("1")); + + command.Execute("2"); + + Assert.AreEqual(ticks, 0); + + command.Execute("42"); + + Assert.AreEqual(ticks, 0); + } + + [TestCategory("Mvvm")] + [TestMethod] + public void Test_AsyncRelayCommandOfT_NullWithValueType() + { + int n = 0; + + var command = new AsyncRelayCommand(i => + { + n = i; + return Task.CompletedTask; + }); + + // Special case for null value types + Assert.IsTrue(command.CanExecute(null)); + + command = new AsyncRelayCommand( + i => + { + n = i; + return Task.CompletedTask; + }, i => i > 0); + + Assert.ThrowsException(() => command.CanExecute(null)); + } + } +} diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs index 08f843c2e08..2b20d0f9ab5 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs @@ -3,6 +3,8 @@ // See the LICENSE file in the project root for more information. using System; +using System.Linq; +using System.Reflection; using Microsoft.Toolkit.Mvvm.Messaging; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -390,13 +392,134 @@ void Test() Assert.AreEqual(!isWeak, weakRecipient.IsAlive); } + [TestCategory("Mvvm")] + [TestMethod] + [DataRow(typeof(Messenger))] + [DataRow(typeof(WeakRefMessenger))] + public void Test_Messenger_Reset(Type type) + { + var messenger = (IMessenger)Activator.CreateInstance(type); + var recipient = new RecipientWithSomeMessages(); + + messenger.RegisterAll(recipient); + + Assert.IsTrue(messenger.IsRegistered(recipient)); + Assert.IsTrue(messenger.IsRegistered(recipient)); + + messenger.Reset(); + + Assert.IsFalse(messenger.IsRegistered(recipient)); + Assert.IsFalse(messenger.IsRegistered(recipient)); + } + + [TestCategory("Mvvm")] + [TestMethod] + [DataRow(typeof(Messenger))] + [DataRow(typeof(WeakRefMessenger))] + public void Test_Messenger_Default(Type type) + { + PropertyInfo defaultInfo = type.GetProperty("Default"); + + var default1 = defaultInfo!.GetValue(null); + var default2 = defaultInfo!.GetValue(null); + + Assert.IsNotNull(default1); + Assert.IsNotNull(default2); + Assert.AreSame(default1, default2); + } + + [TestCategory("Mvvm")] + [TestMethod] + [DataRow(typeof(Messenger))] + [DataRow(typeof(WeakRefMessenger))] + public void Test_Messenger_Cleanup(Type type) + { + var messenger = (IMessenger)Activator.CreateInstance(type); + var recipient = new RecipientWithSomeMessages(); + + messenger.Register(recipient); + + Assert.IsTrue(messenger.IsRegistered(recipient)); + + void Test() + { + var recipient2 = new RecipientWithSomeMessages(); + + messenger.Register(recipient2); + + Assert.IsTrue(messenger.IsRegistered(recipient2)); + + GC.KeepAlive(recipient2); + } + + Test(); + + GC.Collect(); + + // Here we just check that calling Cleanup doesn't alter the state + // of the messenger. This method shouldn't really do anything visible + // to consumers, it's just a way for messengers to compact their data. + messenger.Cleanup(); + + Assert.IsTrue(messenger.IsRegistered(recipient)); + } + + [TestCategory("Mvvm")] + [TestMethod] + [DataRow(typeof(Messenger))] + [DataRow(typeof(WeakRefMessenger))] + public void Test_Messenger_ManyRecipients(Type type) + { + var messenger = (IMessenger)Activator.CreateInstance(type); + + void Test() + { + var recipients = Enumerable.Range(0, 512).Select(_ => new RecipientWithSomeMessages()).ToArray(); + + foreach (var recipient in recipients) + { + messenger.RegisterAll(recipient); + } + + foreach (var recipient in recipients) + { + Assert.IsTrue(messenger.IsRegistered(recipient)); + Assert.IsTrue(messenger.IsRegistered(recipient)); + } + + messenger.Send(); + messenger.Send(); + messenger.Send(); + + foreach (var recipient in recipients) + { + Assert.AreEqual(recipient.As, 1); + Assert.AreEqual(recipient.Bs, 2); + } + + foreach (ref var recipient in recipients.AsSpan()) + { + recipient = null; + } + } + + Test(); + + GC.Collect(); + + // Just invoke a final cleanup to improve coverage, this is unrelated to this test in particular + messenger.Cleanup(); + } + public sealed class RecipientWithNoMessages { public int Number { get; set; } } - public sealed class RecipientWithSomeMessages - : IRecipient, ICloneable, IRecipient + public sealed class RecipientWithSomeMessages : + IRecipient, + IRecipient, + ICloneable { public int As { get; private set; } diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_RelayCommand{T}.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_RelayCommand{T}.cs index d2e13e7ed21..1c606b4b884 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_RelayCommand{T}.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_RelayCommand{T}.cs @@ -35,7 +35,7 @@ public void Test_RelayCommandOfT_AlwaysEnabled() Assert.AreSame(args.Item1, command); Assert.AreSame(args.Item2, EventArgs.Empty); - command.Execute("Hello"); + command.Execute((object)"Hello"); Assert.AreEqual(text, "Hello"); @@ -57,7 +57,7 @@ public void Test_RelayCommand_WithCanExecuteFunction() Assert.ThrowsException(() => command.CanExecute(new object())); - command.Execute("Hello"); + command.Execute((object)"Hello"); Assert.AreEqual(text, "Hello"); @@ -65,5 +65,21 @@ public void Test_RelayCommand_WithCanExecuteFunction() Assert.AreEqual(text, "Hello"); } + + [TestCategory("Mvvm")] + [TestMethod] + public void Test_RelayCommand_NullWithValueType() + { + int n = 0; + + var command = new RelayCommand(i => n = i); + + // Special case for null value types + Assert.IsTrue(command.CanExecute(null)); + + command = new RelayCommand(i => n = i, i => i > 0); + + Assert.ThrowsException(() => command.CanExecute(null)); + } } } diff --git a/UnitTests/UnitTests.Shared/UnitTests.Shared.projitems b/UnitTests/UnitTests.Shared/UnitTests.Shared.projitems index 14147cede7e..bf9d2c9745f 100644 --- a/UnitTests/UnitTests.Shared/UnitTests.Shared.projitems +++ b/UnitTests/UnitTests.Shared/UnitTests.Shared.projitems @@ -23,6 +23,7 @@ + From d8854e09453382a084dcfe56e7f65190e367f8f6 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 15 Sep 2020 20:28:11 +0200 Subject: [PATCH 35/38] Renamed messenger types --- .../ComponentModel/ObservableRecipient.cs | 4 +- ...ssenger.cs => StrongReferenceMessenger.cs} | 8 +- ...Messenger.cs => WeakReferenceMessenger.cs} | 10 +-- UnitTests/UnitTests.Shared/Mvvm/Test_Ioc.cs | 2 +- .../Mvvm/Test_Messenger.Request.cs | 44 +++++----- .../UnitTests.Shared/Mvvm/Test_Messenger.cs | 88 +++++++++---------- .../Mvvm/Test_ObservableRecipient.cs | 18 ++-- 7 files changed, 87 insertions(+), 87 deletions(-) rename Microsoft.Toolkit.Mvvm/Messaging/{Messenger.cs => StrongReferenceMessenger.cs} (98%) rename Microsoft.Toolkit.Mvvm/Messaging/{WeakRefMessenger.cs => WeakReferenceMessenger.cs} (98%) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs index 81b87dffb90..85c178cfa7e 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs @@ -26,11 +26,11 @@ public abstract class ObservableRecipient : ObservableObject /// Initializes a new instance of the class. /// /// - /// This constructor will produce an instance that will use the instance + /// This constructor will produce an instance that will use the instance /// to perform requested operations. It will also be available locally through the property. /// protected ObservableRecipient() - : this(Messaging.Messenger.Default) + : this(WeakReferenceMessenger.Default) { } diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/StrongReferenceMessenger.cs similarity index 98% rename from Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs rename to Microsoft.Toolkit.Mvvm/Messaging/StrongReferenceMessenger.cs index bf7d01932a2..5f4d5dccdec 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/StrongReferenceMessenger.cs @@ -19,9 +19,9 @@ namespace Microsoft.Toolkit.Mvvm.Messaging /// This implementation uses strong references to track the registered /// recipients, so it is necessary to manually unregister them when they're no longer needed. /// - public sealed class Messenger : IMessenger + public sealed class StrongReferenceMessenger : IMessenger { - // The Messenger class uses the following logic to link stored instances together: + // This messenger uses the following logic to link stored instances together: // -------------------------------------------------------------------------------------------------------- // DictionarySlim> recipientsMap; // | \________________[*]IDictionarySlim> @@ -84,9 +84,9 @@ public sealed class Messenger : IMessenger private readonly DictionarySlim typesMap = new DictionarySlim(); /// - /// Gets the default instance. + /// Gets the default instance. /// - public static Messenger Default { get; } = new Messenger(); + public static StrongReferenceMessenger Default { get; } = new StrongReferenceMessenger(); /// public bool IsRegistered(object recipient, TToken token) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs similarity index 98% rename from Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs rename to Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs index 1ed74093c38..0a3a8717c0e 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/WeakRefMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs @@ -11,7 +11,7 @@ #if NETSTANDARD2_1 using RecipientsTable = System.Runtime.CompilerServices.ConditionalWeakTable; #else -using RecipientsTable = Microsoft.Toolkit.Mvvm.Messaging.WeakRefMessenger.ConditionalWeakTable; +using RecipientsTable = Microsoft.Toolkit.Mvvm.Messaging.WeakReferenceMessenger.ConditionalWeakTable; #endif namespace Microsoft.Toolkit.Mvvm.Messaging @@ -23,9 +23,9 @@ namespace Microsoft.Toolkit.Mvvm.Messaging /// This implementation uses weak references to track the registered /// recipients, so it is not necessary to manually unregister them when they're no longer needed. /// - public sealed class WeakRefMessenger : IMessenger + public sealed class WeakReferenceMessenger : IMessenger { - // The WeakRefMessenger class uses the following logic to link stored instances together: + // This messenger uses the following logic to link stored instances together: // -------------------------------------------------------------------------------------------------------- // DictionarySlim> mapping // / / / @@ -48,9 +48,9 @@ public sealed class WeakRefMessenger : IMessenger private readonly DictionarySlim recipientsMap = new DictionarySlim(); /// - /// Gets the default instance. + /// Gets the default instance. /// - public static WeakRefMessenger Default { get; } = new WeakRefMessenger(); + public static WeakReferenceMessenger Default { get; } = new WeakReferenceMessenger(); /// public bool IsRegistered(object recipient, TToken token) diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_Ioc.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_Ioc.cs index 44fbf61301e..5dc0bd92230 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_Ioc.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_Ioc.cs @@ -62,7 +62,7 @@ public void Test_Ioc_LambdaInitialization_ConcreteType() public void Test_Ioc_LambdaInitialization_ConstructorInjection() { var ioc = new Ioc(); - var messenger = new Messenger(); + var messenger = new StrongReferenceMessenger(); ioc.ConfigureServices(services => { diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs index 6d116009a1f..24ef9067ff5 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.Request.cs @@ -15,8 +15,8 @@ public partial class Test_Messenger { [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_RequestMessage_Ok(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -44,8 +44,8 @@ void Receive(object recipient, NumberRequestMessage m) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] [ExpectedException(typeof(InvalidOperationException))] public void Test_Messenger_RequestMessage_Fail_NoReply(Type type) { @@ -56,8 +56,8 @@ public void Test_Messenger_RequestMessage_Fail_NoReply(Type type) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] [ExpectedException(typeof(InvalidOperationException))] public void Test_Messenger_RequestMessage_Fail_MultipleReplies(Type type) { @@ -81,8 +81,8 @@ public class NumberRequestMessage : RequestMessage [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public async Task Test_Messenger_AsyncRequestMessage_Ok_Sync(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -106,8 +106,8 @@ void Receive(object recipient, AsyncNumberRequestMessage m) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public async Task Test_Messenger_AsyncRequestMessage_Ok_Async(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -138,8 +138,8 @@ void Receive(object recipient, AsyncNumberRequestMessage m) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] [ExpectedException(typeof(InvalidOperationException))] public async Task Test_Messenger_AsyncRequestMessage_Fail_NoReply(Type type) { @@ -150,8 +150,8 @@ public async Task Test_Messenger_AsyncRequestMessage_Fail_NoReply(Type type) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] [ExpectedException(typeof(InvalidOperationException))] public async Task Test_Messenger_AsyncRequestMessage_Fail_MultipleReplies(Type type) { @@ -175,8 +175,8 @@ public class AsyncNumberRequestMessage : AsyncRequestMessage [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_CollectionRequestMessage_Ok_NoReplies(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -195,8 +195,8 @@ void Receive(object recipient, NumbersCollectionRequestMessage m) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_CollectionRequestMessage_Ok_MultipleReplies(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -250,8 +250,8 @@ public class NumbersCollectionRequestMessage : CollectionRequestMessage [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public async Task Test_Messenger_AsyncCollectionRequestMessage_Ok_NoReplies(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -270,8 +270,8 @@ void Receive(object recipient, AsyncNumbersCollectionRequestMessage m) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public async Task Test_Messenger_AsyncCollectionRequestMessage_Ok_MultipleReplies(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs index 2b20d0f9ab5..f0939b8859b 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs @@ -15,8 +15,8 @@ public partial class Test_Messenger { [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_UnregisterRecipientWithMessageType(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -27,8 +27,8 @@ public void Test_Messenger_UnregisterRecipientWithMessageType(Type type) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_UnregisterRecipientWithMessageTypeAndToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -39,8 +39,8 @@ public void Test_Messenger_UnregisterRecipientWithMessageTypeAndToken(Type type) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_UnregisterRecipientWithToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -51,8 +51,8 @@ public void Test_Messenger_UnregisterRecipientWithToken(Type type) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_UnregisterRecipientWithRecipient(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -63,8 +63,8 @@ public void Test_Messenger_UnregisterRecipientWithRecipient(Type type) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageType(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -79,8 +79,8 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageType(Type ty [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageTypeAndToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -95,8 +95,8 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithMessageTypeAndToken [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_RegisterAndUnregisterRecipientWithToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -111,8 +111,8 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithToken(Type type) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_RegisterAndUnregisterRecipientWithRecipient(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -127,8 +127,8 @@ public void Test_Messenger_RegisterAndUnregisterRecipientWithRecipient(Type type [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithNoToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -167,8 +167,8 @@ public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithN [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_IsRegistered_Register_Send_UnregisterRecipient_WithNoToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -197,8 +197,8 @@ public void Test_Messenger_IsRegistered_Register_Send_UnregisterRecipient_WithNo [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -227,8 +227,8 @@ public void Test_Messenger_IsRegistered_Register_Send_UnregisterOfTMessage_WithT [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_DuplicateRegistrationWithMessageType(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -244,8 +244,8 @@ public void Test_Messenger_DuplicateRegistrationWithMessageType(Type type) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_DuplicateRegistrationWithMessageTypeAndToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -261,8 +261,8 @@ public void Test_Messenger_DuplicateRegistrationWithMessageTypeAndToken(Type typ [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_IRecipient_NoMessages(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -276,8 +276,8 @@ public void Test_Messenger_IRecipient_NoMessages(Type type) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_IRecipient_SomeMessages_NoToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -309,8 +309,8 @@ public void Test_Messenger_IRecipient_SomeMessages_NoToken(Type type) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_IRecipient_SomeMessages_WithToken(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -346,8 +346,8 @@ public void Test_Messenger_IRecipient_SomeMessages_WithToken(Type type) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_RegisterWithTypeParameter(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -364,8 +364,8 @@ public void Test_Messenger_RegisterWithTypeParameter(Type type) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger), false)] - [DataRow(typeof(WeakRefMessenger), true)] + [DataRow(typeof(StrongReferenceMessenger), false)] + [DataRow(typeof(WeakReferenceMessenger), true)] public void Test_Messenger_Collect_Test(Type type, bool isWeak) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -394,8 +394,8 @@ void Test() [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_Reset(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -414,8 +414,8 @@ public void Test_Messenger_Reset(Type type) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_Default(Type type) { PropertyInfo defaultInfo = type.GetProperty("Default"); @@ -430,8 +430,8 @@ public void Test_Messenger_Default(Type type) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_Cleanup(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -466,8 +466,8 @@ void Test() [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_Messenger_ManyRecipients(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs index 5a3c91505b5..5ef526ba758 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs @@ -15,8 +15,8 @@ public class Test_ObservableRecipient { [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_ObservableRecipient_Activation(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -37,8 +37,8 @@ public void Test_ObservableRecipient_Activation(Type type) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_ObservableRecipient_IsSame(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -53,13 +53,13 @@ public void Test_ObservableRecipient_Default() { var viewmodel = new SomeRecipient(); - Assert.AreSame(viewmodel.CurrentMessenger, Messenger.Default); + Assert.AreSame(viewmodel.CurrentMessenger, StrongReferenceMessenger.Default); } [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_ObservableRecipient_Injection(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); @@ -70,8 +70,8 @@ public void Test_ObservableRecipient_Injection(Type type) [TestCategory("Mvvm")] [TestMethod] - [DataRow(typeof(Messenger))] - [DataRow(typeof(WeakRefMessenger))] + [DataRow(typeof(StrongReferenceMessenger))] + [DataRow(typeof(WeakReferenceMessenger))] public void Test_ObservableRecipient_Broadcast(Type type) { var messenger = (IMessenger)Activator.CreateInstance(type); From 2a3cf84de1a67f6cda10a669e7d1a2045806825c Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 15 Sep 2020 21:22:51 +0200 Subject: [PATCH 36/38] Added missing file headers --- Microsoft.Toolkit.Mvvm/Messaging/Internals/Type2.cs | 6 +++++- Microsoft.Toolkit.Mvvm/Messaging/Internals/Unit.cs | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Internals/Type2.cs b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Type2.cs index 0b254562e95..411b8809ba6 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Internals/Type2.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Type2.cs @@ -1,4 +1,8 @@ -using System; +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; using System.Runtime.CompilerServices; namespace Microsoft.Toolkit.Mvvm.Messaging.Internals diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Internals/Unit.cs b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Unit.cs index 5748643cf83..6f136210b34 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Internals/Unit.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Unit.cs @@ -1,4 +1,8 @@ -using System; +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; using System.Runtime.CompilerServices; namespace Microsoft.Toolkit.Mvvm.Messaging.Internals From ff64983b2528ec690a0d90f309a86d0a70ba670f Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 15 Sep 2020 22:08:27 +0200 Subject: [PATCH 37/38] Fixed unit test (changed type after refactoring) --- UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs index 5ef526ba758..e40776f4244 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableRecipient.cs @@ -53,7 +53,7 @@ public void Test_ObservableRecipient_Default() { var viewmodel = new SomeRecipient(); - Assert.AreSame(viewmodel.CurrentMessenger, StrongReferenceMessenger.Default); + Assert.AreSame(viewmodel.CurrentMessenger, WeakReferenceMessenger.Default); } [TestCategory("Mvvm")] From 32656db1cdd4ddc25e3a88c297ce4062fe64d2ad Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 20 Sep 2020 18:17:14 +0200 Subject: [PATCH 38/38] Minor memory usage improvement --- .../Messaging/WeakReferenceMessenger.cs | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs index 0a3a8717c0e..72b1683f577 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs @@ -5,6 +5,7 @@ using System; using System.Buffers; using System.Collections.Generic; +using System.Diagnostics.Contracts; using System.Runtime.CompilerServices; using Microsoft.Collections.Extensions; using Microsoft.Toolkit.Mvvm.Messaging.Internals; @@ -181,8 +182,8 @@ public TMessage Send(TMessage message, TToken token) return message; } - recipients = new ArrayPoolBufferWriter(); - handlers = new ArrayPoolBufferWriter(); + recipients = ArrayPoolBufferWriter.Create(); + handlers = ArrayPoolBufferWriter.Create(); // We need a local, temporary copy of all the pending recipients and handlers to // invoke, to avoid issues with handlers unregistering from messages while we're @@ -232,8 +233,8 @@ public void Cleanup() { lock (this.recipientsMap) { - using ArrayPoolBufferWriter type2s = new ArrayPoolBufferWriter(); - using ArrayPoolBufferWriter emptyRecipients = new ArrayPoolBufferWriter(); + using ArrayPoolBufferWriter type2s = ArrayPoolBufferWriter.Create(); + using ArrayPoolBufferWriter emptyRecipients = ArrayPoolBufferWriter.Create(); var enumerator = this.recipientsMap.GetEnumerator(); @@ -385,7 +386,12 @@ public IEnumerator> GetEnumerator() /// A simple buffer writer implementation using pooled arrays. /// /// The type of items to store in the list. - private sealed class ArrayPoolBufferWriter : IDisposable + /// + /// This type is a to avoid the object allocation and to + /// enable the pattern-based support. We aren't worried with consumers not + /// using this type correctly since it's private and only accessible within the parent type. + /// + private ref struct ArrayPoolBufferWriter { /// /// The default buffer size to use to expand empty arrays. @@ -403,12 +409,13 @@ private sealed class ArrayPoolBufferWriter : IDisposable private int index; /// - /// Initializes a new instance of the class. + /// Creates a new instance of the struct. /// - public ArrayPoolBufferWriter() + [Pure] + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static ArrayPoolBufferWriter Create() { - this.array = ArrayPool.Shared.Rent(DefaultInitialBufferSize); - this.index = 0; + return new ArrayPoolBufferWriter { array = ArrayPool.Shared.Rent(DefaultInitialBufferSize) }; } /// @@ -461,7 +468,7 @@ private void ResizeBuffer() this.array = rent; } - /// + /// public void Dispose() { Array.Clear(this.array, 0, this.index);