From 1fceefd637f81836bcd9d1b0adf0e83f50905f32 Mon Sep 17 00:00:00 2001 From: Alexander Zaytsev Date: Fri, 13 Apr 2018 17:49:01 +1200 Subject: [PATCH] Fix persistent collections' CopyTo implementations Fixes #1660 --- .../PersistentCollectionsFixture.cs | 40 ++++++++++++++++ .../PersistentGenericMapFixture.cs | 46 +++++++++++++++++++ .../Generic/PersistentGenericBag.cs | 21 +++++---- .../Generic/PersistentGenericIdentifierBag.cs | 18 +++++--- .../Generic/PersistentGenericList.cs | 21 +++++---- .../Generic/PersistentGenericMap.cs | 33 ++++++------- .../Generic/PersistentGenericSet.cs | 1 - 7 files changed, 134 insertions(+), 46 deletions(-) create mode 100644 src/NHibernate.Test/CollectionTest/PersistentCollectionsFixture.cs create mode 100644 src/NHibernate.Test/CollectionTest/PersistentGenericMapFixture.cs diff --git a/src/NHibernate.Test/CollectionTest/PersistentCollectionsFixture.cs b/src/NHibernate.Test/CollectionTest/PersistentCollectionsFixture.cs new file mode 100644 index 00000000000..5a7f9679773 --- /dev/null +++ b/src/NHibernate.Test/CollectionTest/PersistentCollectionsFixture.cs @@ -0,0 +1,40 @@ +using System.Collections.Generic; +using System.Linq; +using NHibernate.Collection.Generic; +using NUnit.Framework; + +namespace NHibernate.Test.CollectionTest +{ + [TestFixture] + public class PersistentCollectionsFixture + { + [Test] + public void SelectManyWorksCorrectly() + { + var bags = new IEnumerable[] + { + new List {"A"}, + new PersistentGenericBag(null, new[] {"B"}), + new PersistentIdentifierBag(null, new[] {"C"}), + new PersistentGenericList(null, new[] {"D"}), + new PersistentGenericSet(null, new HashSet {"E"}) + }; + + var items = bags.SelectMany(b => b).ToArray(); + + Assert.That(items, Is.EqualTo(new[] {"A", "B", "C", "D", "E"})); + } + + [Test] + public void AddRangeWorksCorrectly() + { + var items = new List {"A"}; + items.AddRange(new PersistentGenericBag(null, new[] {"B"})); + items.AddRange(new PersistentIdentifierBag(null, new[] {"C"})); + items.AddRange(new PersistentGenericList(null, new[] {"D"})); + items.AddRange(new PersistentGenericSet(null, new HashSet {"E"})); + + Assert.That(items, Is.EqualTo(new[] {"A", "B", "C", "D", "E"})); + } + } +} diff --git a/src/NHibernate.Test/CollectionTest/PersistentGenericMapFixture.cs b/src/NHibernate.Test/CollectionTest/PersistentGenericMapFixture.cs new file mode 100644 index 00000000000..f7e76ba2d40 --- /dev/null +++ b/src/NHibernate.Test/CollectionTest/PersistentGenericMapFixture.cs @@ -0,0 +1,46 @@ +using System.Collections.Generic; +using System.Linq; +using NHibernate.Collection.Generic; +using NUnit.Framework; + +namespace NHibernate.Test.CollectionTest +{ + [TestFixture] + public class PersistentGenericMapFixture + { + [Test] + public void SelectManyWorksCorrectly() + { + var bags = new[] + { + new PersistentGenericMap( + null, + new Dictionary {{"A", "B"}}), + new PersistentGenericMap( + null, + new Dictionary {{"C", "D"}}) + }; + + var items = bags.SelectMany(b => b).ToArray(); + + Assert.That( + items, + Is.EquivalentTo(new[] {new KeyValuePair("A", "B"), new KeyValuePair("C", "D")})); + } + + [Test] + public void AddRangeWorksCorrectly() + { + var items = new List> {new KeyValuePair("A", "B")}; + + items.AddRange( + new PersistentGenericMap( + null, + new Dictionary {{"C", "D"}})); + + Assert.That( + items, + Is.EquivalentTo(new[] {new KeyValuePair("A", "B"), new KeyValuePair("C", "D")})); + } + } +} diff --git a/src/NHibernate/Collection/Generic/PersistentGenericBag.cs b/src/NHibernate/Collection/Generic/PersistentGenericBag.cs index 51dbeaf98b9..14cd9f7688b 100644 --- a/src/NHibernate/Collection/Generic/PersistentGenericBag.cs +++ b/src/NHibernate/Collection/Generic/PersistentGenericBag.cs @@ -92,11 +92,17 @@ bool ICollection.IsSynchronized get { return false; } } - void ICollection.CopyTo(Array array, int index) + void ICollection.CopyTo(Array array, int arrayIndex) { - for (var i = index; i < Count; i++) + Read(); + if (_gbag is ICollection collection) + { + collection.CopyTo(array, arrayIndex); + } + else { - array.SetValue(this[i], i); + foreach (var item in _gbag) + array.SetValue(item, arrayIndex++); } } @@ -197,16 +203,13 @@ public void Clear() public bool Contains(T item) { - var exists = ReadElementExistence(item); - return !exists.HasValue ? _gbag.Contains(item) : exists.Value; + return ReadElementExistence(item) ?? _gbag.Contains(item); } public void CopyTo(T[] array, int arrayIndex) { - for (var i = arrayIndex; i < Count; i++) - { - array.SetValue(this[i], i); - } + Read(); + _gbag.CopyTo(array, arrayIndex); } public bool Remove(T item) diff --git a/src/NHibernate/Collection/Generic/PersistentGenericIdentifierBag.cs b/src/NHibernate/Collection/Generic/PersistentGenericIdentifierBag.cs index 85bb25d93c5..e80eac9bd17 100644 --- a/src/NHibernate/Collection/Generic/PersistentGenericIdentifierBag.cs +++ b/src/NHibernate/Collection/Generic/PersistentGenericIdentifierBag.cs @@ -390,11 +390,17 @@ public int Count get { return ReadSize() ? CachedSize : _values.Count; } } - void ICollection.CopyTo(Array array, int index) + void ICollection.CopyTo(Array array, int arrayIndex) { - for (int i = index; i < Count; i++) + Read(); + if (_values is ICollection collection) + { + collection.CopyTo(array, arrayIndex); + } + else { - array.SetValue(this[i], i); + foreach (var item in _values) + array.SetValue(item, arrayIndex++); } } @@ -449,10 +455,8 @@ public bool Contains(T item) public void CopyTo(T[] array, int arrayIndex) { - for (int i = arrayIndex; i < Count; i++) - { - array.SetValue(this[i], i); - } + Read(); + _values.CopyTo(array, arrayIndex); } public bool Remove(T item) diff --git a/src/NHibernate/Collection/Generic/PersistentGenericList.cs b/src/NHibernate/Collection/Generic/PersistentGenericList.cs index e913a9d539d..8c676663f56 100644 --- a/src/NHibernate/Collection/Generic/PersistentGenericList.cs +++ b/src/NHibernate/Collection/Generic/PersistentGenericList.cs @@ -407,11 +407,17 @@ public T this[int index] #region ICollection Members - void ICollection.CopyTo(Array array, int index) + void ICollection.CopyTo(Array array, int arrayIndex) { - for (int i = index; i < Count; i++) + Read(); + if (WrappedList is ICollection collection) + { + collection.CopyTo(array, arrayIndex); + } + else { - array.SetValue(this[i], i); + foreach (var item in WrappedList) + array.SetValue(item, arrayIndex++); } } @@ -450,16 +456,13 @@ public void Add(T item) public bool Contains(T item) { - bool? exists = ReadElementExistence(item); - return !exists.HasValue ? WrappedList.Contains(item) : exists.Value; + return ReadElementExistence(item) ?? WrappedList.Contains(item); } public void CopyTo(T[] array, int arrayIndex) { - for (int i = arrayIndex; i < Count; i++) - { - array.SetValue(this[i], i); - } + Read(); + WrappedList.CopyTo(array, arrayIndex); } bool ICollection.IsReadOnly { diff --git a/src/NHibernate/Collection/Generic/PersistentGenericMap.cs b/src/NHibernate/Collection/Generic/PersistentGenericMap.cs index 1f34cd7f06a..994f71e0d44 100644 --- a/src/NHibernate/Collection/Generic/PersistentGenericMap.cs +++ b/src/NHibernate/Collection/Generic/PersistentGenericMap.cs @@ -408,24 +408,8 @@ public bool Contains(KeyValuePair item) public void CopyTo(KeyValuePair[] array, int arrayIndex) { - int c = Count; - var keys = new TKey[c]; - var values = new TValue[c]; - if (Keys != null) - { - Keys.CopyTo(keys, arrayIndex); - } - if (Values != null) - { - Values.CopyTo(values, arrayIndex); - } - for (int i = arrayIndex; i < c; i++) - { - if (keys[i] != null || values[i] != null) - { - array.SetValue(new KeyValuePair(keys[i], values[i]), i); - } - } + Read(); + WrappedMap.CopyTo(array, arrayIndex); } public bool Remove(KeyValuePair item) @@ -453,9 +437,18 @@ public bool IsReadOnly #region ICollection Members - public void CopyTo(Array array, int index) + public void CopyTo(Array array, int arrayIndex) { - CopyTo((KeyValuePair[]) array, index); + Read(); + if (WrappedMap is ICollection collection) + { + collection.CopyTo(array, arrayIndex); + } + else + { + foreach (var item in WrappedMap) + array.SetValue(item, arrayIndex++); + } } public object SyncRoot diff --git a/src/NHibernate/Collection/Generic/PersistentGenericSet.cs b/src/NHibernate/Collection/Generic/PersistentGenericSet.cs index f7b6110f578..35b50f36f4a 100644 --- a/src/NHibernate/Collection/Generic/PersistentGenericSet.cs +++ b/src/NHibernate/Collection/Generic/PersistentGenericSet.cs @@ -469,7 +469,6 @@ public void Clear() public void CopyTo(T[] array, int arrayIndex) { - // NH : we really need to initialize the set ? Read(); WrappedSet.CopyTo(array, arrayIndex); }