Fix Extensions.Find<T> (#168)

Extensions.Find incorrectly compares the kvp.Key to the T type. It is using an `is` operator, which is a cast check. This is incorrect because the Key is always a System.Type. Instead it should be comparing the Key to typeof(T).
Azure · Oct 30, 2020 · de7083e · de7083e
1 parent 86f022e
commit de7083e
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 6 deletions.
diff --git a/src/AmqpExtensions.cs b/src/AmqpExtensions.cs
@@ -663,15 +663,13 @@ public static ArraySegment<byte> AsSegment(this ByteBuffer buffer)
         /// <returns>The object matching the type, or default(T) if not found.</returns>
         public static T Find<T>(this IDictionary<Type, object> extensions)
         {
-            foreach (var kvp in extensions)
+            if (extensions.TryGetValue(typeof(T), out object value) &&
+                value is T typedValue)
             {
-                if (kvp.Key is T)
-                {
-                    return (T)kvp.Value;
-                }
+                return typedValue;
             }
 
-            return default(T);
+            return default;
         }
     }
 }
diff --git a/test/Test.Microsoft.Amqp/Test.Microsoft.Amqp.csproj b/test/Test.Microsoft.Amqp/Test.Microsoft.Amqp.csproj
@@ -22,6 +22,7 @@
     <Compile Include="..\TestAmqpBroker\TestAmqpBroker.cs" Link="TestAmqpBroker.cs" />
     <Compile Include="..\TestCases\AmqpCodecTests.cs" Link="TestCases\AmqpCodecTests.cs" />
     <Compile Include="..\TestCases\AmqpConnectionListenerTests.cs" Link="TestCases\AmqpConnectionListenerTests.cs" />
+    <Compile Include="..\TestCases\AmqpExtensionsTests.cs" Link="TestCases\AmqpExtensionsTests.cs" />
     <Compile Include="..\TestCases\AmqpLinkTests.cs" Link="TestCases\AmqpLinkTests.cs" />
     <Compile Include="..\TestCases\AmqpMessageTests.cs" Link="TestCases\AmqpMessageTests.cs" />
     <Compile Include="..\TestCases\AmqpSamples.cs" Link="TestCases\AmqpSamples.cs" />

diff --git a/test/TestCases/AmqpExtensionsTests.cs b/test/TestCases/AmqpExtensionsTests.cs
@@ -0,0 +1,34 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+namespace Test.Microsoft.Azure.Amqp
+{
+    using global::Microsoft.Azure.Amqp;
+    using System;
+    using System.Collections.Generic;
+    using Xunit;
+
+    [Trait("Category", TestCategory.Current)]
+    public class AmqpExtensionsTests
+    {
+        [Fact]
+        public void TestFind()
+        {
+            Dictionary<Type, object> dictionary = new Dictionary<Type, object>();
+
+            Assert.Null(dictionary.Find<TestClass>());
+
+            var testValue = new TestClass();
+            dictionary.Add(typeof(TestClass), testValue);
+            Assert.Same(testValue, dictionary.Find<TestClass>());
+
+            dictionary.Remove(typeof(TestClass));
+
+            Assert.Null(dictionary.Find<TestClass>());
+        }
+
+        private class TestClass
+        {
+        }
+    }
+}