From 73bba295f09d195ce83bda189996af4a4a84cee1 Mon Sep 17 00:00:00 2001 From: Roger Kratz Date: Thu, 6 Dec 2018 11:24:27 +0100 Subject: [PATCH 1/8] Failing test for GH1920 --- .../GH1920/EntityWithBatchSize.cs | 10 ++++ .../NHSpecificTest/GH1920/Fixture.cs | 51 +++++++++++++++++++ .../NHSpecificTest/GH1920/Mappings.hbm.xml | 9 ++++ 3 files changed, 70 insertions(+) create mode 100644 src/NHibernate.Test/NHSpecificTest/GH1920/EntityWithBatchSize.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/GH1920/Fixture.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/GH1920/Mappings.hbm.xml diff --git a/src/NHibernate.Test/NHSpecificTest/GH1920/EntityWithBatchSize.cs b/src/NHibernate.Test/NHSpecificTest/GH1920/EntityWithBatchSize.cs new file mode 100644 index 00000000000..8cb470eaf51 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH1920/EntityWithBatchSize.cs @@ -0,0 +1,10 @@ +using System; + +namespace NHibernate.Test.NHSpecificTest.GH1920 +{ + public class EntityWithBatchSize + { + public virtual Guid Id { get; set; } + public virtual string Name { get; set; } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH1920/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/GH1920/Fixture.cs new file mode 100644 index 00000000000..d40b2b8cf51 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH1920/Fixture.cs @@ -0,0 +1,51 @@ +using System; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.GH1920 +{ + [TestFixture] + public class Fixture : BugTestCase + { + private Guid entityId; + private Guid someOtherEntityId; + + protected override void OnSetUp() + { + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + entityId = (Guid) session.Save(new EntityWithBatchSize {Name = "some name"}); + someOtherEntityId = (Guid) session.Save(new EntityWithBatchSize()); + + transaction.Commit(); + } + } + + [TestCase(true)] + [TestCase(false)] + public void CanLoadEntity(bool loadProxyOfOtherEntity) + { + using (var session = OpenSession()) + using (session.BeginTransaction()) + { + if(loadProxyOfOtherEntity) + session.Load(someOtherEntityId); + + var result = session.Get(entityId); + + Assert.That(result.Name, Is.Not.Null); + } + } + + + protected override void OnTearDown() + { + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + session.CreateQuery("delete from EntityWithBatchSize").ExecuteUpdate(); + transaction.Commit(); + } + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH1920/Mappings.hbm.xml b/src/NHibernate.Test/NHSpecificTest/GH1920/Mappings.hbm.xml new file mode 100644 index 00000000000..602b6c8cad5 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH1920/Mappings.hbm.xml @@ -0,0 +1,9 @@ + + + + + + + + From 1bef5439fc093ba9f0023f7ac3334269ff013834 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Thu, 6 Dec 2018 13:04:31 +0100 Subject: [PATCH 2/8] Fix Get failing with a null exception --- .../Async/NHSpecificTest/GH1920/Fixture.cs | 62 +++++++++++++++++++ .../NHSpecificTest/GH1920/Fixture.cs | 11 ++-- .../Async/Engine/BatchFetchQueue.cs | 5 +- src/NHibernate/Engine/BatchFetchQueue.cs | 9 ++- 4 files changed, 77 insertions(+), 10 deletions(-) create mode 100644 src/NHibernate.Test/Async/NHSpecificTest/GH1920/Fixture.cs diff --git a/src/NHibernate.Test/Async/NHSpecificTest/GH1920/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/GH1920/Fixture.cs new file mode 100644 index 00000000000..75693b26430 --- /dev/null +++ b/src/NHibernate.Test/Async/NHSpecificTest/GH1920/Fixture.cs @@ -0,0 +1,62 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by AsyncGenerator. +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + + +using System; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.GH1920 +{ + using System.Threading.Tasks; + using System.Threading; + [TestFixture] + public class FixtureAsync : BugTestCase + { + private Guid entityId; + private Guid someOtherEntityId; + + protected override void OnSetUp() + { + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + entityId = (Guid) session.Save(new EntityWithBatchSize { Name = "some name" }); + someOtherEntityId = (Guid) session.Save(new EntityWithBatchSize()); + + transaction.Commit(); + } + } + + [TestCase(true)] + [TestCase(false)] + public async Task CanLoadEntityAsync(bool loadProxyOfOtherEntity, CancellationToken cancellationToken = default(CancellationToken)) + { + using (var session = OpenSession()) + using (session.BeginTransaction()) + { + if (loadProxyOfOtherEntity) + await (session.LoadAsync(someOtherEntityId, cancellationToken)); + + var result = await (session.GetAsync(entityId, cancellationToken)); + + Assert.That(result.Name, Is.Not.Null); + } + } + + protected override void OnTearDown() + { + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + session.CreateQuery("delete from EntityWithBatchSize").ExecuteUpdate(); + transaction.Commit(); + } + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH1920/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/GH1920/Fixture.cs index d40b2b8cf51..e8f21e9e436 100644 --- a/src/NHibernate.Test/NHSpecificTest/GH1920/Fixture.cs +++ b/src/NHibernate.Test/NHSpecificTest/GH1920/Fixture.cs @@ -8,13 +8,13 @@ public class Fixture : BugTestCase { private Guid entityId; private Guid someOtherEntityId; - + protected override void OnSetUp() { using (var session = OpenSession()) using (var transaction = session.BeginTransaction()) { - entityId = (Guid) session.Save(new EntityWithBatchSize {Name = "some name"}); + entityId = (Guid) session.Save(new EntityWithBatchSize { Name = "some name" }); someOtherEntityId = (Guid) session.Save(new EntityWithBatchSize()); transaction.Commit(); @@ -28,16 +28,15 @@ public void CanLoadEntity(bool loadProxyOfOtherEntity) using (var session = OpenSession()) using (session.BeginTransaction()) { - if(loadProxyOfOtherEntity) + if (loadProxyOfOtherEntity) session.Load(someOtherEntityId); - + var result = session.Get(entityId); Assert.That(result.Name, Is.Not.Null); } } - - + protected override void OnTearDown() { using (var session = OpenSession()) diff --git a/src/NHibernate/Async/Engine/BatchFetchQueue.cs b/src/NHibernate/Async/Engine/BatchFetchQueue.cs index d8a1807a043..200bcdc79e2 100644 --- a/src/NHibernate/Async/Engine/BatchFetchQueue.cs +++ b/src/NHibernate/Async/Engine/BatchFetchQueue.cs @@ -295,7 +295,10 @@ async Task CheckCacheAndProcessResultAsync() ? entityKeys.Count - Math.Min(batchSize, entityKeys.Count) : 0; var toIndex = entityKeys.Count - 1; - var indexes = GetSortedKeyIndexes(entityKeys, idIndex.Value, fromIndex, toIndex); + // In case of an ISession.Get on an entity not already loaded as an uninitialized proxy, + // it will not be found in entities awaiting a load, and idIndex will be null. Providing + // -1 then allows to take the entities most recently loaded as uninitialized proxies. + var indexes = GetSortedKeyIndexes(entityKeys, idIndex ?? -1, fromIndex, toIndex); if (batchableCache == null) { for (var j = 0; j < entityKeys.Count; j++) diff --git a/src/NHibernate/Engine/BatchFetchQueue.cs b/src/NHibernate/Engine/BatchFetchQueue.cs index 05426554fb5..b2d8daff434 100644 --- a/src/NHibernate/Engine/BatchFetchQueue.cs +++ b/src/NHibernate/Engine/BatchFetchQueue.cs @@ -455,7 +455,10 @@ bool CheckCacheAndProcessResult() ? entityKeys.Count - Math.Min(batchSize, entityKeys.Count) : 0; var toIndex = entityKeys.Count - 1; - var indexes = GetSortedKeyIndexes(entityKeys, idIndex.Value, fromIndex, toIndex); + // In case of an ISession.Get on an entity not already loaded as an uninitialized proxy, + // it will not be found in entities awaiting a load, and idIndex will be null. Providing + // -1 then allows to take the entities most recently loaded as uninitialized proxies. + var indexes = GetSortedKeyIndexes(entityKeys, idIndex ?? -1, fromIndex, toIndex); if (batchableCache == null) { for (var j = 0; j < entityKeys.Count; j++) @@ -612,11 +615,11 @@ private bool[] AreCached(List - /// Sorts the given keys by thier indexes, where the keys that are after the demanded key will be located + /// Sorts the given keys by their indexes, where the keys that are after the demanded key will be located /// at the start and the remaining indexes at the end of the returned array. /// /// The type of the key - /// The list of pairs of keys and thier indexes. + /// The list of pairs of keys and their indexes. /// The index of the demanded key /// The index where the sorting will begin. /// The index where the sorting will end. From d2d49b5dc6a3c5f4c7b7991ab53ea60ce8673fea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Thu, 6 Dec 2018 14:29:38 +0100 Subject: [PATCH 3/8] Ensure the trouble will not occur for collections --- src/NHibernate/Async/Engine/BatchFetchQueue.cs | 4 +++- src/NHibernate/Engine/BatchFetchQueue.cs | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/NHibernate/Async/Engine/BatchFetchQueue.cs b/src/NHibernate/Async/Engine/BatchFetchQueue.cs index 200bcdc79e2..9f8dc3abb2a 100644 --- a/src/NHibernate/Async/Engine/BatchFetchQueue.cs +++ b/src/NHibernate/Async/Engine/BatchFetchQueue.cs @@ -102,7 +102,9 @@ async Task CheckCacheAndProcessResultAsync() ? collectionKeys.Count - Math.Min(batchSize, collectionKeys.Count) : 0; var toIndex = collectionKeys.Count - 1; - var indexes = GetSortedKeyIndexes(collectionKeys, keyIndex.Value, fromIndex, toIndex); + // In case the collection having triggered the load was not registered for batching, load + // most recently registered collections. + var indexes = GetSortedKeyIndexes(collectionKeys, keyIndex ?? -1, fromIndex, toIndex); if (batchableCache == null) { for (var j = 0; j < collectionKeys.Count; j++) diff --git a/src/NHibernate/Engine/BatchFetchQueue.cs b/src/NHibernate/Engine/BatchFetchQueue.cs index b2d8daff434..ad23b80e528 100644 --- a/src/NHibernate/Engine/BatchFetchQueue.cs +++ b/src/NHibernate/Engine/BatchFetchQueue.cs @@ -271,7 +271,9 @@ bool CheckCacheAndProcessResult() ? collectionKeys.Count - Math.Min(batchSize, collectionKeys.Count) : 0; var toIndex = collectionKeys.Count - 1; - var indexes = GetSortedKeyIndexes(collectionKeys, keyIndex.Value, fromIndex, toIndex); + // In case the collection having triggered the load was not registered for batching, load + // most recently registered collections. + var indexes = GetSortedKeyIndexes(collectionKeys, keyIndex ?? -1, fromIndex, toIndex); if (batchableCache == null) { for (var j = 0; j < collectionKeys.Count; j++) From 41fad6b809501462f9b0a77f0b82681495e220f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Thu, 6 Dec 2018 15:43:06 +0100 Subject: [PATCH 4/8] Improve the fix --- src/NHibernate/Async/Engine/BatchFetchQueue.cs | 9 ++------- src/NHibernate/Engine/BatchFetchQueue.cs | 13 +++++-------- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/NHibernate/Async/Engine/BatchFetchQueue.cs b/src/NHibernate/Async/Engine/BatchFetchQueue.cs index 9f8dc3abb2a..074c823c9b0 100644 --- a/src/NHibernate/Async/Engine/BatchFetchQueue.cs +++ b/src/NHibernate/Async/Engine/BatchFetchQueue.cs @@ -102,9 +102,7 @@ async Task CheckCacheAndProcessResultAsync() ? collectionKeys.Count - Math.Min(batchSize, collectionKeys.Count) : 0; var toIndex = collectionKeys.Count - 1; - // In case the collection having triggered the load was not registered for batching, load - // most recently registered collections. - var indexes = GetSortedKeyIndexes(collectionKeys, keyIndex ?? -1, fromIndex, toIndex); + var indexes = GetSortedKeyIndexes(collectionKeys, keyIndex, fromIndex, toIndex); if (batchableCache == null) { for (var j = 0; j < collectionKeys.Count; j++) @@ -297,10 +295,7 @@ async Task CheckCacheAndProcessResultAsync() ? entityKeys.Count - Math.Min(batchSize, entityKeys.Count) : 0; var toIndex = entityKeys.Count - 1; - // In case of an ISession.Get on an entity not already loaded as an uninitialized proxy, - // it will not be found in entities awaiting a load, and idIndex will be null. Providing - // -1 then allows to take the entities most recently loaded as uninitialized proxies. - var indexes = GetSortedKeyIndexes(entityKeys, idIndex ?? -1, fromIndex, toIndex); + var indexes = GetSortedKeyIndexes(entityKeys, idIndex, fromIndex, toIndex); if (batchableCache == null) { for (var j = 0; j < entityKeys.Count; j++) diff --git a/src/NHibernate/Engine/BatchFetchQueue.cs b/src/NHibernate/Engine/BatchFetchQueue.cs index ad23b80e528..0202507ad7c 100644 --- a/src/NHibernate/Engine/BatchFetchQueue.cs +++ b/src/NHibernate/Engine/BatchFetchQueue.cs @@ -271,9 +271,7 @@ bool CheckCacheAndProcessResult() ? collectionKeys.Count - Math.Min(batchSize, collectionKeys.Count) : 0; var toIndex = collectionKeys.Count - 1; - // In case the collection having triggered the load was not registered for batching, load - // most recently registered collections. - var indexes = GetSortedKeyIndexes(collectionKeys, keyIndex ?? -1, fromIndex, toIndex); + var indexes = GetSortedKeyIndexes(collectionKeys, keyIndex, fromIndex, toIndex); if (batchableCache == null) { for (var j = 0; j < collectionKeys.Count; j++) @@ -457,10 +455,7 @@ bool CheckCacheAndProcessResult() ? entityKeys.Count - Math.Min(batchSize, entityKeys.Count) : 0; var toIndex = entityKeys.Count - 1; - // In case of an ISession.Get on an entity not already loaded as an uninitialized proxy, - // it will not be found in entities awaiting a load, and idIndex will be null. Providing - // -1 then allows to take the entities most recently loaded as uninitialized proxies. - var indexes = GetSortedKeyIndexes(entityKeys, idIndex ?? -1, fromIndex, toIndex); + var indexes = GetSortedKeyIndexes(entityKeys, idIndex, fromIndex, toIndex); if (batchableCache == null) { for (var j = 0; j < entityKeys.Count; j++) @@ -626,13 +621,15 @@ private bool[] AreCached(ListThe index where the sorting will begin. /// The index where the sorting will end. /// An array of sorted key indexes. - private static int[] GetSortedKeyIndexes(List> keys, int keyIndex, int fromIndex, int toIndex) + private static int[] GetSortedKeyIndexes(List> keys, int? keyIndex, int fromIndex, int toIndex) { var result = new int[Math.Abs(toIndex - fromIndex) + 1]; var lowerIndexes = new List(); var i = 0; for (var j = fromIndex; j <= toIndex; j++) { + // If the index was not found (null), this test will be falsy and it will take the most recently + // registered entities or collections. if (keys[j].Value < keyIndex) { lowerIndexes.Add(j); From 25cba06cd3ed2585ed0c5de33a2a4f6d179484f6 Mon Sep 17 00:00:00 2001 From: maca88 Date: Thu, 6 Dec 2018 16:09:10 +0100 Subject: [PATCH 5/8] Fix null exception in BatchFetchQueue --- .../Async/CacheTest/BatchableCacheFixture.cs | 124 +++++++++++++----- .../CacheTest/BatchableCacheFixture.cs | 124 +++++++++++++----- .../Async/Engine/BatchFetchQueue.cs | 22 ++-- src/NHibernate/Engine/BatchFetchQueue.cs | 22 ++-- 4 files changed, 206 insertions(+), 86 deletions(-) diff --git a/src/NHibernate.Test/Async/CacheTest/BatchableCacheFixture.cs b/src/NHibernate.Test/Async/CacheTest/BatchableCacheFixture.cs index 69bd5a578e4..f36ba9fdc8e 100644 --- a/src/NHibernate.Test/Async/CacheTest/BatchableCacheFixture.cs +++ b/src/NHibernate.Test/Async/CacheTest/BatchableCacheFixture.cs @@ -219,21 +219,23 @@ public async Task MultipleGetReadOnlyTestAsync() var persister = Sfi.GetEntityPersister(typeof(ReadOnly).FullName); Assert.That(persister.Cache.Cache, Is.Not.Null); Assert.That(persister.Cache.Cache, Is.TypeOf()); - var ids = new List(); + int[] getIds; + int[] loadIds; using (var s = Sfi.OpenSession()) using (var tx = s.BeginTransaction()) { var items = await (s.Query().ToListAsync()); - ids.AddRange(items.OrderBy(o => o.Id).Select(o => o.Id)); + loadIds = getIds = items.OrderBy(o => o.Id).Select(o => o.Id).ToArray(); await (tx.CommitAsync()); } // Batch size 3 - var parentTestCases = new List>> + var parentTestCases = new List>> { // When the cache is empty, GetMultiple will be called two times. One time in type // DefaultLoadEventListener and the other time in BatchingEntityLoader. - new Tuple>( + new Tuple>( + loadIds, 0, new[] { @@ -245,7 +247,8 @@ public async Task MultipleGetReadOnlyTestAsync() ), // When there are not enough uninitialized entities after the demanded one to fill the batch, // the nearest before the demanded entity are added. - new Tuple>( + new Tuple>( + loadIds, 4, new[] { @@ -255,7 +258,8 @@ public async Task MultipleGetReadOnlyTestAsync() new[] {3, 4, 5}, null ), - new Tuple>( + new Tuple>( + loadIds, 5, new[] { @@ -265,7 +269,8 @@ public async Task MultipleGetReadOnlyTestAsync() new[] {3, 4, 5}, null ), - new Tuple>( + new Tuple>( + loadIds, 0, new[] { @@ -274,7 +279,8 @@ public async Task MultipleGetReadOnlyTestAsync() null, (i) => i % 2 == 0 // Cache all even indexes before loading ), - new Tuple>( + new Tuple>( + loadIds, 1, new[] { @@ -284,7 +290,8 @@ public async Task MultipleGetReadOnlyTestAsync() new[] {1, 3, 5}, (i) => i % 2 == 0 ), - new Tuple>( + new Tuple>( + loadIds, 5, new[] { @@ -294,7 +301,8 @@ public async Task MultipleGetReadOnlyTestAsync() new[] {1, 3, 5}, (i) => i % 2 == 0 ), - new Tuple>( + new Tuple>( + loadIds, 0, new[] { @@ -304,7 +312,8 @@ public async Task MultipleGetReadOnlyTestAsync() new[] {0, 2, 4}, (i) => i % 2 != 0 ), - new Tuple>( + new Tuple>( + loadIds, 4, new[] { @@ -313,12 +322,56 @@ public async Task MultipleGetReadOnlyTestAsync() }, new[] {0, 2, 4}, (i) => i % 2 != 0 + ), + // Tests by loading different ids + new Tuple>( + loadIds.Where((v, i) => i != 0).ToArray(), + 0, + new[] + { + new[] {0, 5, 4}, // triggered by LoadFromSecondLevelCache method of DefaultLoadEventListener type + new[] {3, 4, 5}, // triggered by Load method of BatchingEntityLoader type + }, + new[] {0, 4, 5}, + null + ), + new Tuple>( + loadIds.Where((v, i) => i != 4).ToArray(), + 4, + new[] + { + new[] {4, 5, 3}, + new[] {5, 3, 2}, + }, + new[] {3, 4, 5}, + null + ), + new Tuple>( + loadIds.Where((v, i) => i != 0).ToArray(), + 0, + new[] + { + new[] {0, 5, 4} // 0 get assembled and no further processing is done + }, + null, + (i) => i % 2 == 0 // Cache all even indexes before loading + ), + new Tuple>( + loadIds.Where((v, i) => i != 1).ToArray(), + 1, + new[] + { + new[] {1, 5, 4}, // 4 gets assembled inside LoadFromSecondLevelCache + new[] {5, 3, 2} + }, + new[] {1, 3, 5}, + (i) => i % 2 == 0 ) }; foreach (var tuple in parentTestCases) { - await (AssertMultipleCacheCallsAsync(ids, tuple.Item1, tuple.Item2, tuple.Item3, tuple.Item4)); + await (AssertMultipleCacheCallsAsync(tuple.Item1, getIds, tuple.Item2, tuple.Item3, tuple.Item4, tuple.Item5)); } } @@ -328,21 +381,23 @@ public async Task MultipleGetReadOnlyItemTestAsync() var persister = Sfi.GetEntityPersister(typeof(ReadOnlyItem).FullName); Assert.That(persister.Cache.Cache, Is.Not.Null); Assert.That(persister.Cache.Cache, Is.TypeOf()); - var ids = new List(); + int[] getIds; + int[] loadIds; using (var s = Sfi.OpenSession()) using (var tx = s.BeginTransaction()) { var items = await (s.Query().Take(6).ToListAsync()); - ids.AddRange(items.OrderBy(o => o.Id).Select(o => o.Id)); + loadIds = getIds = items.OrderBy(o => o.Id).Select(o => o.Id).ToArray(); await (tx.CommitAsync()); } // Batch size 4 - var parentTestCases = new List>> + var parentTestCases = new List>> { // When the cache is empty, GetMultiple will be called two times. One time in type // DefaultLoadEventListener and the other time in BatchingEntityLoader. - new Tuple>( + new Tuple>( + loadIds, 0, new[] { @@ -354,7 +409,8 @@ public async Task MultipleGetReadOnlyItemTestAsync() ), // When there are not enough uninitialized entities after the demanded one to fill the batch, // the nearest before the demanded entity are added. - new Tuple>( + new Tuple>( + loadIds, 4, new[] { @@ -364,7 +420,8 @@ public async Task MultipleGetReadOnlyItemTestAsync() new[] {2, 3, 4, 5}, null ), - new Tuple>( + new Tuple>( + loadIds, 5, new[] { @@ -374,7 +431,8 @@ public async Task MultipleGetReadOnlyItemTestAsync() new[] {2, 3, 4, 5}, null ), - new Tuple>( + new Tuple>( + loadIds, 0, new[] { @@ -383,7 +441,8 @@ public async Task MultipleGetReadOnlyItemTestAsync() null, (i) => i % 2 == 0 // Cache all even indexes before loading ), - new Tuple>( + new Tuple>( + loadIds, 1, new[] { @@ -393,7 +452,8 @@ public async Task MultipleGetReadOnlyItemTestAsync() new[] {1, 3, 5}, (i) => i % 2 == 0 ), - new Tuple>( + new Tuple>( + loadIds, 5, new[] { @@ -403,7 +463,8 @@ public async Task MultipleGetReadOnlyItemTestAsync() new[] {1, 3, 5}, (i) => i % 2 == 0 ), - new Tuple>( + new Tuple>( + loadIds, 0, new[] { @@ -413,7 +474,8 @@ public async Task MultipleGetReadOnlyItemTestAsync() new[] {0, 2, 4}, (i) => i % 2 != 0 ), - new Tuple>( + new Tuple>( + loadIds, 4, new[] { @@ -427,7 +489,7 @@ public async Task MultipleGetReadOnlyItemTestAsync() foreach (var tuple in parentTestCases) { - await (AssertMultipleCacheCallsAsync(ids, tuple.Item1, tuple.Item2, tuple.Item3, tuple.Item4)); + await (AssertMultipleCacheCallsAsync(tuple.Item1, getIds, tuple.Item2, tuple.Item3, tuple.Item4, tuple.Item5)); } } @@ -764,7 +826,8 @@ public async Task QueryCacheTestAsync() } } - private async Task AssertMultipleCacheCallsAsync(List ids, int idIndex, int[][] fetchedIdIndexes, int[] putIdIndexes, Func cacheBeforeLoadFn = null, CancellationToken cancellationToken = default(CancellationToken)) + private async Task AssertMultipleCacheCallsAsync(IEnumerable loadIds, IReadOnlyList getIds, int idIndex, + int[][] fetchedIdIndexes, int[] putIdIndexes, Func cacheBeforeLoadFn = null, CancellationToken cancellationToken = default(CancellationToken)) where TEntity : CacheEntity { var persister = Sfi.GetEntityPersister(typeof(TEntity).FullName); @@ -776,7 +839,7 @@ public async Task QueryCacheTestAsync() using (var s = Sfi.OpenSession()) using (var tx = s.BeginTransaction()) { - foreach (var id in ids.Where((o, i) => cacheBeforeLoadFn(i))) + foreach (var id in getIds.Where((o, i) => cacheBeforeLoadFn(i))) { await (s.GetAsync(id, cancellationToken)); } @@ -788,12 +851,11 @@ public async Task QueryCacheTestAsync() using (var tx = s.BeginTransaction()) { cache.ClearStatistics(); - - foreach (var id in ids) + foreach (var id in loadIds) { await (s.LoadAsync(id, cancellationToken)); } - var item = await (s.GetAsync(ids[idIndex], cancellationToken)); + var item = await (s.GetAsync(getIds[idIndex], cancellationToken)); Assert.That(item, Is.Not.Null); Assert.That(cache.GetCalls, Has.Count.EqualTo(0)); Assert.That(cache.PutCalls, Has.Count.EqualTo(0)); @@ -807,14 +869,14 @@ public async Task QueryCacheTestAsync() Assert.That(cache.PutMultipleCalls, Has.Count.EqualTo(1)); Assert.That( cache.PutMultipleCalls[0].OfType().Select(o => (int) o.Key), - Is.EquivalentTo(putIdIndexes.Select(o => ids[o]))); + Is.EquivalentTo(putIdIndexes.Select(o => getIds[o]))); } for (int i = 0; i < fetchedIdIndexes.GetLength(0); i++) { Assert.That( cache.GetMultipleCalls[i].OfType().Select(o => (int) o.Key), - Is.EquivalentTo(fetchedIdIndexes[i].Select(o => ids[o]))); + Is.EquivalentTo(fetchedIdIndexes[i].Select(o => getIds[o]))); } await (tx.CommitAsync(cancellationToken)); diff --git a/src/NHibernate.Test/CacheTest/BatchableCacheFixture.cs b/src/NHibernate.Test/CacheTest/BatchableCacheFixture.cs index 7a85ccbcff9..9cce9657daf 100644 --- a/src/NHibernate.Test/CacheTest/BatchableCacheFixture.cs +++ b/src/NHibernate.Test/CacheTest/BatchableCacheFixture.cs @@ -207,21 +207,23 @@ public void MultipleGetReadOnlyTest() var persister = Sfi.GetEntityPersister(typeof(ReadOnly).FullName); Assert.That(persister.Cache.Cache, Is.Not.Null); Assert.That(persister.Cache.Cache, Is.TypeOf()); - var ids = new List(); + int[] getIds; + int[] loadIds; using (var s = Sfi.OpenSession()) using (var tx = s.BeginTransaction()) { var items = s.Query().ToList(); - ids.AddRange(items.OrderBy(o => o.Id).Select(o => o.Id)); + loadIds = getIds = items.OrderBy(o => o.Id).Select(o => o.Id).ToArray(); tx.Commit(); } // Batch size 3 - var parentTestCases = new List>> + var parentTestCases = new List>> { // When the cache is empty, GetMultiple will be called two times. One time in type // DefaultLoadEventListener and the other time in BatchingEntityLoader. - new Tuple>( + new Tuple>( + loadIds, 0, new[] { @@ -233,7 +235,8 @@ public void MultipleGetReadOnlyTest() ), // When there are not enough uninitialized entities after the demanded one to fill the batch, // the nearest before the demanded entity are added. - new Tuple>( + new Tuple>( + loadIds, 4, new[] { @@ -243,7 +246,8 @@ public void MultipleGetReadOnlyTest() new[] {3, 4, 5}, null ), - new Tuple>( + new Tuple>( + loadIds, 5, new[] { @@ -253,7 +257,8 @@ public void MultipleGetReadOnlyTest() new[] {3, 4, 5}, null ), - new Tuple>( + new Tuple>( + loadIds, 0, new[] { @@ -262,7 +267,8 @@ public void MultipleGetReadOnlyTest() null, (i) => i % 2 == 0 // Cache all even indexes before loading ), - new Tuple>( + new Tuple>( + loadIds, 1, new[] { @@ -272,7 +278,8 @@ public void MultipleGetReadOnlyTest() new[] {1, 3, 5}, (i) => i % 2 == 0 ), - new Tuple>( + new Tuple>( + loadIds, 5, new[] { @@ -282,7 +289,8 @@ public void MultipleGetReadOnlyTest() new[] {1, 3, 5}, (i) => i % 2 == 0 ), - new Tuple>( + new Tuple>( + loadIds, 0, new[] { @@ -292,7 +300,8 @@ public void MultipleGetReadOnlyTest() new[] {0, 2, 4}, (i) => i % 2 != 0 ), - new Tuple>( + new Tuple>( + loadIds, 4, new[] { @@ -301,12 +310,56 @@ public void MultipleGetReadOnlyTest() }, new[] {0, 2, 4}, (i) => i % 2 != 0 + ), + // Tests by loading different ids + new Tuple>( + loadIds.Where((v, i) => i != 0).ToArray(), + 0, + new[] + { + new[] {0, 5, 4}, // triggered by LoadFromSecondLevelCache method of DefaultLoadEventListener type + new[] {3, 4, 5}, // triggered by Load method of BatchingEntityLoader type + }, + new[] {0, 4, 5}, + null + ), + new Tuple>( + loadIds.Where((v, i) => i != 4).ToArray(), + 4, + new[] + { + new[] {4, 5, 3}, + new[] {5, 3, 2}, + }, + new[] {3, 4, 5}, + null + ), + new Tuple>( + loadIds.Where((v, i) => i != 0).ToArray(), + 0, + new[] + { + new[] {0, 5, 4} // 0 get assembled and no further processing is done + }, + null, + (i) => i % 2 == 0 // Cache all even indexes before loading + ), + new Tuple>( + loadIds.Where((v, i) => i != 1).ToArray(), + 1, + new[] + { + new[] {1, 5, 4}, // 4 gets assembled inside LoadFromSecondLevelCache + new[] {5, 3, 2} + }, + new[] {1, 3, 5}, + (i) => i % 2 == 0 ) }; foreach (var tuple in parentTestCases) { - AssertMultipleCacheCalls(ids, tuple.Item1, tuple.Item2, tuple.Item3, tuple.Item4); + AssertMultipleCacheCalls(tuple.Item1, getIds, tuple.Item2, tuple.Item3, tuple.Item4, tuple.Item5); } } @@ -316,21 +369,23 @@ public void MultipleGetReadOnlyItemTest() var persister = Sfi.GetEntityPersister(typeof(ReadOnlyItem).FullName); Assert.That(persister.Cache.Cache, Is.Not.Null); Assert.That(persister.Cache.Cache, Is.TypeOf()); - var ids = new List(); + int[] getIds; + int[] loadIds; using (var s = Sfi.OpenSession()) using (var tx = s.BeginTransaction()) { var items = s.Query().Take(6).ToList(); - ids.AddRange(items.OrderBy(o => o.Id).Select(o => o.Id)); + loadIds = getIds = items.OrderBy(o => o.Id).Select(o => o.Id).ToArray(); tx.Commit(); } // Batch size 4 - var parentTestCases = new List>> + var parentTestCases = new List>> { // When the cache is empty, GetMultiple will be called two times. One time in type // DefaultLoadEventListener and the other time in BatchingEntityLoader. - new Tuple>( + new Tuple>( + loadIds, 0, new[] { @@ -342,7 +397,8 @@ public void MultipleGetReadOnlyItemTest() ), // When there are not enough uninitialized entities after the demanded one to fill the batch, // the nearest before the demanded entity are added. - new Tuple>( + new Tuple>( + loadIds, 4, new[] { @@ -352,7 +408,8 @@ public void MultipleGetReadOnlyItemTest() new[] {2, 3, 4, 5}, null ), - new Tuple>( + new Tuple>( + loadIds, 5, new[] { @@ -362,7 +419,8 @@ public void MultipleGetReadOnlyItemTest() new[] {2, 3, 4, 5}, null ), - new Tuple>( + new Tuple>( + loadIds, 0, new[] { @@ -371,7 +429,8 @@ public void MultipleGetReadOnlyItemTest() null, (i) => i % 2 == 0 // Cache all even indexes before loading ), - new Tuple>( + new Tuple>( + loadIds, 1, new[] { @@ -381,7 +440,8 @@ public void MultipleGetReadOnlyItemTest() new[] {1, 3, 5}, (i) => i % 2 == 0 ), - new Tuple>( + new Tuple>( + loadIds, 5, new[] { @@ -391,7 +451,8 @@ public void MultipleGetReadOnlyItemTest() new[] {1, 3, 5}, (i) => i % 2 == 0 ), - new Tuple>( + new Tuple>( + loadIds, 0, new[] { @@ -401,7 +462,8 @@ public void MultipleGetReadOnlyItemTest() new[] {0, 2, 4}, (i) => i % 2 != 0 ), - new Tuple>( + new Tuple>( + loadIds, 4, new[] { @@ -415,7 +477,7 @@ public void MultipleGetReadOnlyItemTest() foreach (var tuple in parentTestCases) { - AssertMultipleCacheCalls(ids, tuple.Item1, tuple.Item2, tuple.Item3, tuple.Item4); + AssertMultipleCacheCalls(tuple.Item1, getIds, tuple.Item2, tuple.Item3, tuple.Item4, tuple.Item5); } } @@ -752,7 +814,8 @@ public void QueryCacheTest() } } - private void AssertMultipleCacheCalls(List ids, int idIndex, int[][] fetchedIdIndexes, int[] putIdIndexes, Func cacheBeforeLoadFn = null) + private void AssertMultipleCacheCalls(IEnumerable loadIds, IReadOnlyList getIds, int idIndex, + int[][] fetchedIdIndexes, int[] putIdIndexes, Func cacheBeforeLoadFn = null) where TEntity : CacheEntity { var persister = Sfi.GetEntityPersister(typeof(TEntity).FullName); @@ -764,7 +827,7 @@ private void AssertMultipleCacheCalls(List ids, int idIndex, int[] using (var s = Sfi.OpenSession()) using (var tx = s.BeginTransaction()) { - foreach (var id in ids.Where((o, i) => cacheBeforeLoadFn(i))) + foreach (var id in getIds.Where((o, i) => cacheBeforeLoadFn(i))) { s.Get(id); } @@ -776,12 +839,11 @@ private void AssertMultipleCacheCalls(List ids, int idIndex, int[] using (var tx = s.BeginTransaction()) { cache.ClearStatistics(); - - foreach (var id in ids) + foreach (var id in loadIds) { s.Load(id); } - var item = s.Get(ids[idIndex]); + var item = s.Get(getIds[idIndex]); Assert.That(item, Is.Not.Null); Assert.That(cache.GetCalls, Has.Count.EqualTo(0)); Assert.That(cache.PutCalls, Has.Count.EqualTo(0)); @@ -795,14 +857,14 @@ private void AssertMultipleCacheCalls(List ids, int idIndex, int[] Assert.That(cache.PutMultipleCalls, Has.Count.EqualTo(1)); Assert.That( cache.PutMultipleCalls[0].OfType().Select(o => (int) o.Key), - Is.EquivalentTo(putIdIndexes.Select(o => ids[o]))); + Is.EquivalentTo(putIdIndexes.Select(o => getIds[o]))); } for (int i = 0; i < fetchedIdIndexes.GetLength(0); i++) { Assert.That( cache.GetMultipleCalls[i].OfType().Select(o => (int) o.Key), - Is.EquivalentTo(fetchedIdIndexes[i].Select(o => ids[o]))); + Is.EquivalentTo(fetchedIdIndexes[i].Select(o => getIds[o]))); } tx.Commit(); diff --git a/src/NHibernate/Async/Engine/BatchFetchQueue.cs b/src/NHibernate/Async/Engine/BatchFetchQueue.cs index 074c823c9b0..83cbbd1a928 100644 --- a/src/NHibernate/Async/Engine/BatchFetchQueue.cs +++ b/src/NHibernate/Async/Engine/BatchFetchQueue.cs @@ -116,7 +116,6 @@ async Task CheckCacheAndProcessResultAsync() else { var results = await (AreCachedAsync(collectionKeys, indexes, collectionPersister, batchableCache, checkCache, cancellationToken)).ConfigureAwait(false); - var k = toIndex; for (var j = 0; j < results.Length; j++) { if (!results[j] && await (ProcessKeyAsync(collectionKeys[indexes[j]].Key, true)).ConfigureAwait(false)) @@ -153,7 +152,7 @@ Task ProcessKeyAsync(KeyValuePair return Task.FromResult(false); } - if (checkForEnd && (index >= keyIndex.Value + batchSize || index == map.Count)) + if (checkForEnd && (index == map.Count || index >= keyIndex.Value + batchSize)) { return Task.FromResult(true); } @@ -167,7 +166,7 @@ Task ProcessKeyAsync(KeyValuePair } else if (!checkCache || batchableCache == null) { - if (!keyIndex.HasValue || index < keyIndex.Value) + if (index < map.Count && (!keyIndex.HasValue || index < keyIndex.Value)) { collectionKeys.Add(new KeyValuePair, int>(me, index)); return Task.FromResult(false); @@ -204,10 +203,10 @@ Task ProcessKeyAsync(KeyValuePair if (i == batchSize) { i = 1; // End of array, start filling again from start - if (keyIndex.HasValue) + if (index == map.Count || keyIndex.HasValue) { checkForEnd = true; - return Task.FromResult(index >= keyIndex.Value + batchSize || index == map.Count); + return Task.FromResult(index == map.Count || index >= keyIndex.Value + batchSize); } } return Task.FromResult(false); @@ -309,7 +308,6 @@ async Task CheckCacheAndProcessResultAsync() else { var results = await (AreCachedAsync(entityKeys, indexes, persister, batchableCache, checkCache, cancellationToken)).ConfigureAwait(false); - var k = toIndex; for (var j = 0; j < results.Length; j++) { if (!results[j] && await (ProcessKeyAsync(entityKeys[indexes[j]].Key, true)).ConfigureAwait(false)) @@ -329,7 +327,7 @@ async Task CheckCacheAndProcessResultAsync() Task ProcessKeyAsync(EntityKey key, bool ignoreCache = false) { //TODO: this needn't exclude subclasses... - if (checkForEnd && (index >= idIndex.Value + batchSize || index == set.Count)) + if (checkForEnd && (index == set.Count || index >= idIndex.Value + batchSize)) { return Task.FromResult(true); } @@ -339,7 +337,7 @@ Task ProcessKeyAsync(EntityKey key, bool ignoreCache = false) } else if (!checkCache || batchableCache == null) { - if (!idIndex.HasValue || index < idIndex.Value) + if (index < set.Count && (!idIndex.HasValue || index < idIndex.Value)) { entityKeys.Add(new KeyValuePair(key, index)); return Task.FromResult(false); @@ -368,10 +366,10 @@ Task ProcessKeyAsync(EntityKey key, bool ignoreCache = false) if (i == batchSize) { i = 1; // End of array, start filling again from start - if (idIndex.HasValue) + if (index == set.Count || idIndex.HasValue) { checkForEnd = true; - return Task.FromResult(index >= idIndex.Value + batchSize || index == set.Count); + return Task.FromResult(index == set.Count || index >= idIndex.Value + batchSize); } } return Task.FromResult(false); @@ -381,7 +379,7 @@ Task ProcessKeyAsync(EntityKey key, bool ignoreCache = false) /// /// Checks whether the given entity key indexes are cached. /// - /// The list of pairs of entity keys and thier indexes. + /// The list of pairs of entity keys and their indexes. /// The array of indexes of that have to be checked. /// The entity persister. /// The batchable cache. @@ -419,7 +417,7 @@ private async Task AreCachedAsync(List> ent /// /// Checks whether the given collection key indexes are cached. /// - /// The list of pairs of collection entries and thier indexes. + /// The list of pairs of collection entries and their indexes. /// The array of indexes of that have to be checked. /// The collection persister. /// The batchable cache. diff --git a/src/NHibernate/Engine/BatchFetchQueue.cs b/src/NHibernate/Engine/BatchFetchQueue.cs index 0202507ad7c..59790c5d97a 100644 --- a/src/NHibernate/Engine/BatchFetchQueue.cs +++ b/src/NHibernate/Engine/BatchFetchQueue.cs @@ -285,7 +285,6 @@ bool CheckCacheAndProcessResult() else { var results = AreCached(collectionKeys, indexes, collectionPersister, batchableCache, checkCache); - var k = toIndex; for (var j = 0; j < results.Length; j++) { if (!results[j] && ProcessKey(collectionKeys[indexes[j]].Key, true)) @@ -322,7 +321,7 @@ bool ProcessKey(KeyValuePair me, bool ig return false; } - if (checkForEnd && (index >= keyIndex.Value + batchSize || index == map.Count)) + if (checkForEnd && (index == map.Count || index >= keyIndex.Value + batchSize)) { return true; } @@ -336,7 +335,7 @@ bool ProcessKey(KeyValuePair me, bool ig } else if (!checkCache || batchableCache == null) { - if (!keyIndex.HasValue || index < keyIndex.Value) + if (index < map.Count && (!keyIndex.HasValue || index < keyIndex.Value)) { collectionKeys.Add(new KeyValuePair, int>(me, index)); return false; @@ -373,10 +372,10 @@ bool ProcessKey(KeyValuePair me, bool ig if (i == batchSize) { i = 1; // End of array, start filling again from start - if (keyIndex.HasValue) + if (index == map.Count || keyIndex.HasValue) { checkForEnd = true; - return index >= keyIndex.Value + batchSize || index == map.Count; + return index == map.Count || index >= keyIndex.Value + batchSize; } } return false; @@ -469,7 +468,6 @@ bool CheckCacheAndProcessResult() else { var results = AreCached(entityKeys, indexes, persister, batchableCache, checkCache); - var k = toIndex; for (var j = 0; j < results.Length; j++) { if (!results[j] && ProcessKey(entityKeys[indexes[j]].Key, true)) @@ -489,7 +487,7 @@ bool CheckCacheAndProcessResult() bool ProcessKey(EntityKey key, bool ignoreCache = false) { //TODO: this needn't exclude subclasses... - if (checkForEnd && (index >= idIndex.Value + batchSize || index == set.Count)) + if (checkForEnd && (index == set.Count || index >= idIndex.Value + batchSize)) { return true; } @@ -499,7 +497,7 @@ bool ProcessKey(EntityKey key, bool ignoreCache = false) } else if (!checkCache || batchableCache == null) { - if (!idIndex.HasValue || index < idIndex.Value) + if (index < set.Count && (!idIndex.HasValue || index < idIndex.Value)) { entityKeys.Add(new KeyValuePair(key, index)); return false; @@ -528,10 +526,10 @@ bool ProcessKey(EntityKey key, bool ignoreCache = false) if (i == batchSize) { i = 1; // End of array, start filling again from start - if (idIndex.HasValue) + if (index == set.Count || idIndex.HasValue) { checkForEnd = true; - return index >= idIndex.Value + batchSize || index == set.Count; + return index == set.Count || index >= idIndex.Value + batchSize; } } return false; @@ -541,7 +539,7 @@ bool ProcessKey(EntityKey key, bool ignoreCache = false) /// /// Checks whether the given entity key indexes are cached. /// - /// The list of pairs of entity keys and thier indexes. + /// The list of pairs of entity keys and their indexes. /// The array of indexes of that have to be checked. /// The entity persister. /// The batchable cache. @@ -577,7 +575,7 @@ private bool[] AreCached(List> entityKeys, int[] ke /// /// Checks whether the given collection key indexes are cached. /// - /// The list of pairs of collection entries and thier indexes. + /// The list of pairs of collection entries and their indexes. /// The array of indexes of that have to be checked. /// The collection persister. /// The batchable cache. From 80821d375e71618df76b31d900b0e4fb364b4449 Mon Sep 17 00:00:00 2001 From: maca88 Date: Thu, 6 Dec 2018 17:10:33 +0100 Subject: [PATCH 6/8] Fixed the keyIndex condition to return the most recent keys --- src/NHibernate/Engine/BatchFetchQueue.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NHibernate/Engine/BatchFetchQueue.cs b/src/NHibernate/Engine/BatchFetchQueue.cs index 59790c5d97a..1fe0ae2ab6e 100644 --- a/src/NHibernate/Engine/BatchFetchQueue.cs +++ b/src/NHibernate/Engine/BatchFetchQueue.cs @@ -628,7 +628,7 @@ private static int[] GetSortedKeyIndexes(List> keys, int { // If the index was not found (null), this test will be falsy and it will take the most recently // registered entities or collections. - if (keys[j].Value < keyIndex) + if (!keyIndex.HasValue || keys[j].Value < keyIndex) { lowerIndexes.Add(j); } From f379039b8bf188cb8d9865d1d7683f7dd1ecb50e Mon Sep 17 00:00:00 2001 From: maca88 Date: Thu, 6 Dec 2018 23:20:35 +0100 Subject: [PATCH 7/8] Added more tests with a different batch size --- .../Async/CacheTest/BatchableCacheFixture.cs | 46 ++++++++++++++++++- .../CacheTest/BatchableCacheFixture.cs | 46 ++++++++++++++++++- 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/src/NHibernate.Test/Async/CacheTest/BatchableCacheFixture.cs b/src/NHibernate.Test/Async/CacheTest/BatchableCacheFixture.cs index f36ba9fdc8e..cb918341eef 100644 --- a/src/NHibernate.Test/Async/CacheTest/BatchableCacheFixture.cs +++ b/src/NHibernate.Test/Async/CacheTest/BatchableCacheFixture.cs @@ -484,7 +484,51 @@ public async Task MultipleGetReadOnlyItemTestAsync() }, new[] {0, 2, 4}, (i) => i % 2 != 0 - ) + ), + // Tests by loading different ids + new Tuple>( + loadIds.Where((v, i) => i != 0).ToArray(), + 0, + new[] + { + new[] {0, 5, 4, 3}, // triggered by LoadFromSecondLevelCache method of DefaultLoadEventListener type + new[] {5, 4, 3, 2}, // triggered by Load method of BatchingEntityLoader type + }, + new[] {0, 5, 4, 3}, + null + ), + new Tuple>( + loadIds.Where((v, i) => i != 5).ToArray(), + 5, + new[] + { + new[] {5, 4, 3, 2}, + new[] {4, 3, 2, 1}, + }, + new[] {2, 3, 4, 5}, + null + ), + new Tuple>( + loadIds.Where((v, i) => i != 0).ToArray(), + 0, + new[] + { + new[] {0, 5, 4, 3} // 0 get assembled and no further processing is done + }, + null, + (i) => i % 2 == 0 // Cache all even indexes before loading + ), + new Tuple>( + loadIds.Where((v, i) => i != 1).ToArray(), + 1, + new[] + { + new[] {1, 5, 4, 3}, // 4 get assembled inside LoadFromSecondLevelCache + new[] {5, 3, 2, 0} + }, + new[] {1, 3, 5}, + (i) => i % 2 == 0 + ), }; foreach (var tuple in parentTestCases) diff --git a/src/NHibernate.Test/CacheTest/BatchableCacheFixture.cs b/src/NHibernate.Test/CacheTest/BatchableCacheFixture.cs index 9cce9657daf..6b175e546f0 100644 --- a/src/NHibernate.Test/CacheTest/BatchableCacheFixture.cs +++ b/src/NHibernate.Test/CacheTest/BatchableCacheFixture.cs @@ -472,7 +472,51 @@ public void MultipleGetReadOnlyItemTest() }, new[] {0, 2, 4}, (i) => i % 2 != 0 - ) + ), + // Tests by loading different ids + new Tuple>( + loadIds.Where((v, i) => i != 0).ToArray(), + 0, + new[] + { + new[] {0, 5, 4, 3}, // triggered by LoadFromSecondLevelCache method of DefaultLoadEventListener type + new[] {5, 4, 3, 2}, // triggered by Load method of BatchingEntityLoader type + }, + new[] {0, 5, 4, 3}, + null + ), + new Tuple>( + loadIds.Where((v, i) => i != 5).ToArray(), + 5, + new[] + { + new[] {5, 4, 3, 2}, + new[] {4, 3, 2, 1}, + }, + new[] {2, 3, 4, 5}, + null + ), + new Tuple>( + loadIds.Where((v, i) => i != 0).ToArray(), + 0, + new[] + { + new[] {0, 5, 4, 3} // 0 get assembled and no further processing is done + }, + null, + (i) => i % 2 == 0 // Cache all even indexes before loading + ), + new Tuple>( + loadIds.Where((v, i) => i != 1).ToArray(), + 1, + new[] + { + new[] {1, 5, 4, 3}, // 4 get assembled inside LoadFromSecondLevelCache + new[] {5, 3, 2, 0} + }, + new[] {1, 3, 5}, + (i) => i % 2 == 0 + ), }; foreach (var tuple in parentTestCases) From 285b6bd13a990e98ae852a1bf62fee570c9ec0b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Fri, 7 Dec 2018 12:27:53 +0100 Subject: [PATCH 8/8] Remove comment irrelevant since previous code change --- src/NHibernate/Engine/BatchFetchQueue.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/NHibernate/Engine/BatchFetchQueue.cs b/src/NHibernate/Engine/BatchFetchQueue.cs index 1fe0ae2ab6e..2a4c61160f4 100644 --- a/src/NHibernate/Engine/BatchFetchQueue.cs +++ b/src/NHibernate/Engine/BatchFetchQueue.cs @@ -626,8 +626,6 @@ private static int[] GetSortedKeyIndexes(List> keys, int var i = 0; for (var j = fromIndex; j <= toIndex; j++) { - // If the index was not found (null), this test will be falsy and it will take the most recently - // registered entities or collections. if (!keyIndex.HasValue || keys[j].Value < keyIndex) { lowerIndexes.Add(j);