From 63ddfed98e38c6e89f1b9ccc08f6f41e823d2076 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Tue, 26 Jul 2022 18:28:12 -0700 Subject: [PATCH 1/3] Ensure SyncRoot is not null --- .../Internal/NonGenericCollectionWrapper.cs | 2 +- .../Internal/NonGenericDictionaryWrapper.cs | 2 +- .../Internal/NonGenericCollectionWrapperTests.cs | 7 +++++-- .../Internal/NonGenericDictionaryWrapperTests.cs | 3 ++- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericCollectionWrapper.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericCollectionWrapper.cs index ec5b3c41bb..375488cb5d 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericCollectionWrapper.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericCollectionWrapper.cs @@ -20,7 +20,7 @@ public NonGenericCollectionWrapper(ICollection collection) public bool IsSynchronized => false; - public object SyncRoot => null!; + public object SyncRoot => _collection; public void CopyTo(Array array, int index) { diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericDictionaryWrapper.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericDictionaryWrapper.cs index 79d8fbd81c..1a509689e6 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericDictionaryWrapper.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericDictionaryWrapper.cs @@ -59,7 +59,7 @@ public ICollection Values public bool IsSynchronized => false; - public object SyncRoot => null!; + public object SyncRoot => _original; public void Add(object key, object? value) => _original.Add(key, value); diff --git a/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Internal/NonGenericCollectionWrapperTests.cs b/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Internal/NonGenericCollectionWrapperTests.cs index af06103292..396ce78b91 100644 --- a/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Internal/NonGenericCollectionWrapperTests.cs +++ b/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Internal/NonGenericCollectionWrapperTests.cs @@ -1,4 +1,6 @@ using System; +using System.Collections; +using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Linq; using AutoFixture; @@ -37,10 +39,11 @@ public void Empty() [Fact] public void SyncProperties() { - var wrapped = new NonGenericCollectionWrapper(Array.Empty()); + var list = new List(); + var wrapped = new NonGenericCollectionWrapper(list); Assert.False(wrapped.IsSynchronized); - Assert.Null(wrapped.SyncRoot); + Assert.Equal(((ICollection)list).SyncRoot, wrapped.SyncRoot); } [Fact] diff --git a/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Internal/NonGenericDictionaryWrapperTests.cs b/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Internal/NonGenericDictionaryWrapperTests.cs index d44c4f0dbc..db31afb159 100644 --- a/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Internal/NonGenericDictionaryWrapperTests.cs +++ b/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Internal/NonGenericDictionaryWrapperTests.cs @@ -18,6 +18,7 @@ public NonGenericDictionaryWrapperTests() _fixture = new Fixture(); } + [Fact] public void IsFixedSize() { @@ -42,7 +43,7 @@ public void SyncRoot() var dictionary = new Dictionary(); var wrapped = new NonGenericDictionaryWrapper(dictionary); - Assert.Null(wrapped.SyncRoot); + Assert.Equal(((ICollection)dictionary).SyncRoot, wrapped.SyncRoot); } [Theory] From 0b85b49512c44a63faef9fa86596d47a62ba7d10 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Thu, 28 Jul 2022 09:42:57 -0700 Subject: [PATCH 2/3] create new syncroot if not exists --- .../Internal/NonGenericCollectionWrapper.cs | 3 ++- .../Internal/NonGenericDictionaryWrapper.cs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericCollectionWrapper.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericCollectionWrapper.cs index 375488cb5d..e9f4027c70 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericCollectionWrapper.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericCollectionWrapper.cs @@ -9,6 +9,7 @@ namespace Microsoft.AspNetCore.SystemWebAdapters.Internal { internal class NonGenericCollectionWrapper : ICollection { + private object? _syncRoot; private readonly ICollection _collection; public NonGenericCollectionWrapper(ICollection collection) @@ -20,7 +21,7 @@ public NonGenericCollectionWrapper(ICollection collection) public bool IsSynchronized => false; - public object SyncRoot => _collection; + public object SyncRoot => _syncRoot ??= ((ICollection)_collection).SyncRoot ?? new object(); public void CopyTo(Array array, int index) { diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericDictionaryWrapper.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericDictionaryWrapper.cs index 1a509689e6..60d8689447 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericDictionaryWrapper.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericDictionaryWrapper.cs @@ -11,6 +11,7 @@ internal class NonGenericDictionaryWrapper : IDictionary { private readonly IDictionary _original; + private object? _syncRoot; private ICollection? _keys; private ICollection? _values; @@ -59,7 +60,7 @@ public ICollection Values public bool IsSynchronized => false; - public object SyncRoot => _original; + public object SyncRoot => _syncRoot ??= ((ICollection)_original).SyncRoot ?? new object(); public void Add(object key, object? value) => _original.Add(key, value); From 7c09af11b4a05b2f2f819b4bd9eb20b067e3c28d Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Thu, 28 Jul 2022 10:14:03 -0700 Subject: [PATCH 3/3] handle non-icollection usage --- .../Internal/NonGenericCollectionWrapper.cs | 4 +- .../Internal/NonGenericDictionaryWrapper.cs | 4 +- .../NonGenericCollectionWrapperTests.cs | 61 +++++++++++++++++++ .../NonGenericDictionaryWrapperTests.cs | 58 +++++++++++++++--- 4 files changed, 115 insertions(+), 12 deletions(-) diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericCollectionWrapper.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericCollectionWrapper.cs index e9f4027c70..edfa22205b 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericCollectionWrapper.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericCollectionWrapper.cs @@ -19,9 +19,9 @@ public NonGenericCollectionWrapper(ICollection collection) public int Count => _collection.Count; - public bool IsSynchronized => false; + public bool IsSynchronized => (_collection as ICollection)?.IsSynchronized ?? false; - public object SyncRoot => _syncRoot ??= ((ICollection)_collection).SyncRoot ?? new object(); + public object SyncRoot => _syncRoot ??= (_collection as ICollection)?.SyncRoot ?? new object(); public void CopyTo(Array array, int index) { diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericDictionaryWrapper.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericDictionaryWrapper.cs index 60d8689447..2ddeca5f56 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericDictionaryWrapper.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Internal/NonGenericDictionaryWrapper.cs @@ -58,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 => _syncRoot ??= ((ICollection)_original).SyncRoot ?? new object(); + public object SyncRoot => _syncRoot ??= (_original as ICollection)?.SyncRoot ?? new object(); public void Add(object key, object? value) => _original.Add(key, value); diff --git a/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Internal/NonGenericCollectionWrapperTests.cs b/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Internal/NonGenericCollectionWrapperTests.cs index 396ce78b91..bad6bd4a6d 100644 --- a/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Internal/NonGenericCollectionWrapperTests.cs +++ b/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Internal/NonGenericCollectionWrapperTests.cs @@ -4,6 +4,7 @@ using System.Diagnostics.CodeAnalysis; using System.Linq; using AutoFixture; +using Moq; using Xunit; namespace Microsoft.AspNetCore.SystemWebAdapters.Internal @@ -36,6 +37,66 @@ public void Empty() Assert.Empty(wrapped); } + [Theory] + [InlineData(null)] + [InlineData(true)] + [InlineData(false)] + public void IsSynchronized(bool? isSynchronized) + { + // Arrange + var original = new Mock>(); + + if (isSynchronized.HasValue) + { + var collection = original.As(); + collection.Setup(c => c.IsSynchronized).Returns(isSynchronized.Value); + } + + var wrapped = new NonGenericCollectionWrapper(original.Object); + + // Act + var result = wrapped.IsSynchronized; + + // Assert + Assert.Equal(isSynchronized ?? false, result); + } + + [Fact] + public void NoSyncRoot() + { + // Arrange + var original = new Mock>(); + var wrapped = new NonGenericCollectionWrapper(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>(); + var syncRoot = new object(); + + var collection = original.As(); + collection.Setup(c => c.SyncRoot).Returns(syncRoot); + + var wrapped = new NonGenericCollectionWrapper(original.Object); + + // Act + var result1 = wrapped.SyncRoot; + var result2 = wrapped.SyncRoot; + + // Assert + Assert.Same(syncRoot, result1); + Assert.Same(syncRoot, result2); + } + [Fact] public void SyncProperties() { diff --git a/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Internal/NonGenericDictionaryWrapperTests.cs b/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Internal/NonGenericDictionaryWrapperTests.cs index db31afb159..78f4f78031 100644 --- a/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Internal/NonGenericDictionaryWrapperTests.cs +++ b/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Internal/NonGenericDictionaryWrapperTests.cs @@ -28,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>(); + + if (isSynchronized.HasValue) + { + var collection = dictionary.As(); + 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(); - var wrapped = new NonGenericDictionaryWrapper(dictionary); + // Arrange + var dictionary = new Mock>(); + 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(); - var wrapped = new NonGenericDictionaryWrapper(dictionary); + // Arrange + var dictionary = new Mock>(); + var syncRoot = new object(); + + var collection = dictionary.As(); + collection.Setup(c => c.SyncRoot).Returns(syncRoot); + + var wrapped = new NonGenericDictionaryWrapper(dictionary.Object); + + // Act + var result1 = wrapped.SyncRoot; + var result2 = wrapped.SyncRoot; - Assert.Equal(((ICollection)dictionary).SyncRoot, wrapped.SyncRoot); + // Assert + Assert.Same(syncRoot, result1); + Assert.Same(syncRoot, result2); } [Theory]