Ensure SyncRoot is not null (#132)

Author: twsouthwick

src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericCollectionWrapper.cs

@@ -9,6 +9,7 @@ namespace Microsoft.AspNetCore.SystemWebAdapters.Internal
 {
     internal class NonGenericCollectionWrapper<T> : ICollection
     {
+        private object? _syncRoot;
         private readonly ICollection<T> _collection;
 
         public NonGenericCollectionWrapper(ICollection<T> collection)
@@ -18,9 +19,9 @@ public NonGenericCollectionWrapper(ICollection<T> collection)
 
         public int Count => _collection.Count;
 
-        public bool IsSynchronized => false;
+        public bool IsSynchronized => (_collection as ICollection)?.IsSynchronized ?? false;
 
-        public object SyncRoot => null!;
+        public object SyncRoot => _syncRoot ??= (_collection as ICollection)?.SyncRoot ?? new object();
 
         public void CopyTo(Array array, int index)
         {

src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericDictionaryWrapper.cs

@@ -11,6 +11,7 @@ internal class NonGenericDictionaryWrapper : IDictionary
     {
         private readonly IDictionary<object, object?> _original;
 
+        private object? _syncRoot;
         private ICollection? _keys;
         private ICollection? _values;
 
@@ -57,9 +58,9 @@ public ICollection Values
 
         public int Count => _original.Count;
 
-        public bool IsSynchronized => false;
+        public bool IsSynchronized => (_original as ICollection)?.IsSynchronized ?? false;
 
-        public object SyncRoot => null!;
+        public object SyncRoot => _syncRoot ??= (_original as ICollection)?.SyncRoot ?? new object();
 
         public void Add(object key, object? value) => _original.Add(key, value);
 

test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Internal/NonGenericCollectionWrapperTests.cs

@@ -1,7 +1,10 @@
 using System;
+using System.Collections;
+using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using AutoFixture;
+using Moq;
 using Xunit;
 
 namespace Microsoft.AspNetCore.SystemWebAdapters.Internal
@@ -34,13 +37,74 @@ public void Empty()
             Assert.Empty(wrapped);
         }
 
+        [Theory]
+        [InlineData(null)]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void IsSynchronized(bool? isSynchronized)
+        {
+            // Arrange
+            var original = new Mock<ICollection<object>>();
+
+            if (isSynchronized.HasValue)
+            {
+                var collection = original.As<ICollection>();
+                collection.Setup(c => c.IsSynchronized).Returns(isSynchronized.Value);
+            }
+
+            var wrapped = new NonGenericCollectionWrapper<object>(original.Object);
+
+            // Act
+            var result = wrapped.IsSynchronized;
+
+            // Assert
+            Assert.Equal(isSynchronized ?? false, result);
+        }
+
+        [Fact]
+        public void NoSyncRoot()
+        {
+            // Arrange
+            var original = new Mock<ICollection<object>>();
+            var wrapped = new NonGenericCollectionWrapper<object>(original.Object);
+
+            // Act
+            var result1 = wrapped.SyncRoot;
+            var result2 = wrapped.SyncRoot;
+
+            // Assert
+            Assert.Same(result1, result2);
+        }
+
+        [Fact]
+        public void HasSyncRoot()
+        {
+            // Arrange
+            var original = new Mock<ICollection<object>>();
+            var syncRoot = new object();
+
+            var collection = original.As<ICollection>();
+            collection.Setup(c => c.SyncRoot).Returns(syncRoot);
+
+            var wrapped = new NonGenericCollectionWrapper<object>(original.Object);
+
+            // Act
+            var result1 = wrapped.SyncRoot;
+            var result2 = wrapped.SyncRoot;
+
+            // Assert
+            Assert.Same(syncRoot, result1);
+            Assert.Same(syncRoot, result2);
+        }
+
         [Fact]
         public void SyncProperties()
         {
-            var wrapped = new NonGenericCollectionWrapper<object>(Array.Empty<object>());
+            var list = new List<object>();
+            var wrapped = new NonGenericCollectionWrapper<object>(list);
 
             Assert.False(wrapped.IsSynchronized);
-            Assert.Null(wrapped.SyncRoot);
+            Assert.Equal(((ICollection)list).SyncRoot, wrapped.SyncRoot);
         }
 
         [Fact]

test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Internal/NonGenericDictionaryWrapperTests.cs

@@ -18,6 +18,7 @@ public NonGenericDictionaryWrapperTests()
             _fixture = new Fixture();
         }
 
+
         [Fact]
         public void IsFixedSize()
         {
@@ -27,22 +28,64 @@ public void IsFixedSize()
             Assert.False(wrapped.IsFixedSize);
         }
 
+        [Theory]
+        [InlineData(null)]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void IsSynchronized(bool? isSynchronized)
+        {
+            // Arrange
+            var dictionary = new Mock<IDictionary<object, object?>>();
+
+            if (isSynchronized.HasValue)
+            {
+                var collection = dictionary.As<ICollection>();
+                collection.Setup(c => c.IsSynchronized).Returns(isSynchronized.Value);
+            }
+
+            var wrapped = new NonGenericDictionaryWrapper(dictionary.Object);
+
+            // Act
+            var result = wrapped.IsSynchronized;
+
+            // Assert
+            Assert.Equal(isSynchronized ?? false, result);
+        }
+
         [Fact]
-        public void IsSynchronized()
+        public void NoSyncRoot()
         {
-            var dictionary = new Dictionary<object, object?>();
-            var wrapped = new NonGenericDictionaryWrapper(dictionary);
+            // Arrange
+            var dictionary = new Mock<IDictionary<object, object?>>();
+            var wrapped = new NonGenericDictionaryWrapper(dictionary.Object);
+
+            // Act
+            var result1 = wrapped.SyncRoot;
+            var result2 = wrapped.SyncRoot;
 
-            Assert.False(wrapped.IsSynchronized);
+            // Assert
+            Assert.Same(result1, result2);
         }
 
         [Fact]
-        public void SyncRoot()
+        public void HasSyncRoot()
         {
-            var dictionary = new Dictionary<object, object?>();
-            var wrapped = new NonGenericDictionaryWrapper(dictionary);
+            // Arrange
+            var dictionary = new Mock<IDictionary<object, object?>>();
+            var syncRoot = new object();
+
+            var collection = dictionary.As<ICollection>();
+            collection.Setup(c => c.SyncRoot).Returns(syncRoot);
+
+            var wrapped = new NonGenericDictionaryWrapper(dictionary.Object);
+
+            // Act
+            var result1 = wrapped.SyncRoot;
+            var result2 = wrapped.SyncRoot;
 
-            Assert.Null(wrapped.SyncRoot);
+            // Assert
+            Assert.Same(syncRoot, result1);
+            Assert.Same(syncRoot, result2);
         }
 
         [Theory]