Skip to content

Commit de944c2

Browse files
fredericDelaportehazzik
fredericDelaporte
authored and
hazzik
committed
Fix failure of Linq aggregates with Future
1 parent c1e909c commit de944c2

File tree

9 files changed

+217
-125
lines changed

9 files changed

+217
-125
lines changed

‎src/NHibernate.Test/Async/NHSpecificTest/NH3850/Fixture.cs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -194,10 +194,14 @@ public async Task AggregateMutableSeedGBaseAsync()
194194
// (And moreover, with current dataset, selected values are same whatever the classes.)
195195
var query = session.Query<DomainClassGExtendedByH>()
196196
.OrderBy(dc => dc.Id);
197-
var result = query.Aggregate(new StringBuilder(), (s, dc) => s.Append(dc.Name).Append(","));
197+
var seed = new StringBuilder();
198+
var result = query.Aggregate(seed, (s, dc) => s.Append(dc.Name).Append(","));
198199
var expectedResult = _searchName1 + "," + _searchName2 + "," + _searchName1 + "," + _searchName2 + ",";
199200
Assert.That(result.ToString(), Is.EqualTo(expectedResult));
200-
var futureQuery = query.ToFutureValue(qdc => qdc.Aggregate(new StringBuilder(), (s, dc) => s.Append(dc.Name).Append(",")));
201+
// We are dodging another bug here: the seed is cached in query plan... So giving another seed to Future
202+
// keeps re-using the seed used for non future above.
203+
seed.Clear();
204+
var futureQuery = query.ToFutureValue(qdc => qdc.Aggregate(seed, (s, dc) => s.Append(dc.Name).Append(",")));
201205
Assert.That((await (futureQuery.GetValueAsync())).ToString(), Is.EqualTo(expectedResult), "Future");
202206
}
203207
}
@@ -1136,7 +1140,7 @@ public Task MaxGBaseAsync()
11361140
"Non nullable decimal max has failed");
11371141
var futureNonNullableDec = dcQuery.ToFutureValue(qdc => qdc.Max(dc => dc.NonNullableDecimal));
11381142
Assert.That(() => futureNonNullableDec.GetValueAsync(cancellationToken),
1139-
Throws.InstanceOf<ArgumentNullException>(),
1143+
Throws.TargetInvocationException.And.InnerException.InstanceOf<InvalidOperationException>(),
11401144
"Future non nullable decimal max has failed");
11411145
}
11421146
}
@@ -1249,7 +1253,7 @@ public Task MinGBaseAsync()
12491253
"Non nullable decimal min has failed");
12501254
var futureNonNullableDec = dcQuery.ToFutureValue(qdc => qdc.Min(dc => dc.NonNullableDecimal));
12511255
Assert.That(() => futureNonNullableDec.GetValueAsync(cancellationToken),
1252-
Throws.InstanceOf<ArgumentNullException>(),
1256+
Throws.TargetInvocationException.And.InnerException.InstanceOf<InvalidOperationException>(),
12531257
"Future non nullable decimal min has failed");
12541258
}
12551259
}
@@ -1517,7 +1521,7 @@ public async Task SumObjectAsync()
15171521
"Non nullable decimal sum has failed");
15181522
var futureNonNullableDec = dcQuery.ToFutureValue(qdc => qdc.Sum(dc => dc.NonNullableDecimal));
15191523
Assert.That(() => futureNonNullableDec.GetValueAsync(cancellationToken),
1520-
Throws.InstanceOf<ArgumentNullException>(),
1524+
Throws.TargetInvocationException.And.InnerException.InstanceOf<InvalidOperationException>(),
15211525
"Future non nullable decimal sum has failed");
15221526
}
15231527
}

‎src/NHibernate.Test/NHSpecificTest/NH3850/Fixture.cs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -182,10 +182,14 @@ public void AggregateMutableSeedGBase()
182182
// (And moreover, with current dataset, selected values are same whatever the classes.)
183183
var query = session.Query<DomainClassGExtendedByH>()
184184
.OrderBy(dc => dc.Id);
185-
var result = query.Aggregate(new StringBuilder(), (s, dc) => s.Append(dc.Name).Append(","));
185+
var seed = new StringBuilder();
186+
var result = query.Aggregate(seed, (s, dc) => s.Append(dc.Name).Append(","));
186187
var expectedResult = _searchName1 + "," + _searchName2 + "," + _searchName1 + "," + _searchName2 + ",";
187188
Assert.That(result.ToString(), Is.EqualTo(expectedResult));
188-
var futureQuery = query.ToFutureValue(qdc => qdc.Aggregate(new StringBuilder(), (s, dc) => s.Append(dc.Name).Append(",")));
189+
// We are dodging another bug here: the seed is cached in query plan... So giving another seed to Future
190+
// keeps re-using the seed used for non future above.
191+
seed.Clear();
192+
var futureQuery = query.ToFutureValue(qdc => qdc.Aggregate(seed, (s, dc) => s.Append(dc.Name).Append(",")));
189193
Assert.That(futureQuery.Value.ToString(), Is.EqualTo(expectedResult), "Future");
190194
}
191195
}
@@ -1124,7 +1128,7 @@ private void Max<DC>(int? expectedResult) where DC : DomainClassBase
11241128
"Non nullable decimal max has failed");
11251129
var futureNonNullableDec = dcQuery.ToFutureValue(qdc => qdc.Max(dc => dc.NonNullableDecimal));
11261130
Assert.That(() => futureNonNullableDec.Value,
1127-
Throws.InstanceOf<ArgumentNullException>(),
1131+
Throws.TargetInvocationException.And.InnerException.InstanceOf<InvalidOperationException>(),
11281132
"Future non nullable decimal max has failed");
11291133
}
11301134
}
@@ -1237,7 +1241,7 @@ private void Min<DC>(int? expectedResult) where DC : DomainClassBase
12371241
"Non nullable decimal min has failed");
12381242
var futureNonNullableDec = dcQuery.ToFutureValue(qdc => qdc.Min(dc => dc.NonNullableDecimal));
12391243
Assert.That(() => futureNonNullableDec.Value,
1240-
Throws.InstanceOf<ArgumentNullException>(),
1244+
Throws.TargetInvocationException.And.InnerException.InstanceOf<InvalidOperationException>(),
12411245
"Future non nullable decimal min has failed");
12421246
}
12431247
}
@@ -1505,7 +1509,7 @@ private void Sum<DC>(int? expectedResult) where DC : DomainClassBase
15051509
"Non nullable decimal sum has failed");
15061510
var futureNonNullableDec = dcQuery.ToFutureValue(qdc => qdc.Sum(dc => dc.NonNullableDecimal));
15071511
Assert.That(() => futureNonNullableDec.Value,
1508-
Throws.InstanceOf<ArgumentNullException>(),
1512+
Throws.TargetInvocationException.And.InnerException.InstanceOf<InvalidOperationException>(),
15091513
"Future non nullable decimal sum has failed");
15101514
}
15111515
}

‎src/NHibernate/Async/Impl/FutureBatch.cs

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,29 +8,39 @@
88
//------------------------------------------------------------------------------
99

1010

11+
using System;
1112
using System.Collections;
1213
using System.Collections.Generic;
1314
using System.Linq;
14-
using System.Threading;
15-
using System.Threading.Tasks;
15+
using NHibernate.Transform;
1616

1717
namespace NHibernate.Impl
1818
{
19+
using System.Threading.Tasks;
20+
using System.Threading;
1921
public abstract partial class FutureBatch<TQueryApproach, TMultiApproach>
2022
{
2123

2224
private async Task<IList> GetResultsAsync(CancellationToken cancellationToken)
2325
{
2426
cancellationToken.ThrowIfCancellationRequested();
2527
if (results != null)
26-
{
2728
return results;
28-
}
29+
2930
var multiApproach = CreateMultiApproach(isCacheable, cacheRegion);
30-
for (int i = 0; i < queries.Count; i++)
31+
var needTransformer = false;
32+
foreach (var query in queries)
3133
{
32-
AddTo(multiApproach, queries[i], resultTypes[i]);
34+
AddTo(multiApproach, query.Query, query.ResultType);
35+
if (query.Future?.ExecuteOnEval != null)
36+
needTransformer = true;
3337
}
38+
39+
if (needTransformer)
40+
AddResultTransformer(
41+
multiApproach,
42+
new FutureResultsTransformer(queries));
43+
3444
results = await (GetResultsFromAsync(multiApproach, cancellationToken)).ConfigureAwait(false);
3545
ClearCurrentFutureBatch();
3646
return results;

‎src/NHibernate/Async/Impl/FutureQueryBatch.cs

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,22 @@
99

1010

1111
using System.Collections;
12+
using NHibernate.Transform;
1213

1314
namespace NHibernate.Impl
1415
{
15-
using System.Threading.Tasks;
16-
using System.Threading;
17-
public partial class FutureQueryBatch : FutureBatch<IQuery, IMultiQuery>
18-
{
16+
using System.Threading.Tasks;
17+
using System.Threading;
18+
public partial class FutureQueryBatch : FutureBatch<IQuery, IMultiQuery>
19+
{
1920

20-
protected override Task<IList> GetResultsFromAsync(IMultiQuery multiApproach, CancellationToken cancellationToken)
21-
{
22-
if (cancellationToken.IsCancellationRequested)
23-
{
24-
return Task.FromCanceled<IList>(cancellationToken);
25-
}
21+
protected override Task<IList> GetResultsFromAsync(IMultiQuery multiApproach, CancellationToken cancellationToken)
22+
{
23+
if (cancellationToken.IsCancellationRequested)
24+
{
25+
return Task.FromCanceled<IList>(cancellationToken);
26+
}
2627
return multiApproach.ListAsync(cancellationToken);
27-
}
28-
}
28+
}
29+
}
2930
}

‎src/NHibernate/Impl/DelayedEnumerator.cs

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Collections;
33
using System.Collections.Generic;
4+
using System.Linq;
45
using System.Threading;
56
using System.Threading.Tasks;
67

@@ -25,8 +26,6 @@ public DelayedEnumerator(GetResult result, GetResultAsync resultAsync)
2526
public IEnumerable<T> GetEnumerable()
2627
{
2728
var value = _result();
28-
if (ExecuteOnEval != null)
29-
value = (IEnumerable<T>) ExecuteOnEval.DynamicInvoke(value);
3029
foreach (T item in value)
3130
{
3231
yield return item;
@@ -59,27 +58,29 @@ public Task<IEnumerable<T>> GetEnumerableAsync(CancellationToken cancellationTok
5958
}
6059
try
6160
{
62-
if (ExecuteOnEval == null)
63-
return _resultAsync(cancellationToken);
64-
return getEnumerableAsync();
61+
return _resultAsync(cancellationToken);
6562
}
6663
catch (Exception ex)
6764
{
6865
return Task.FromException<IEnumerable<T>>(ex);
6966
}
70-
71-
async Task<IEnumerable<T>> getEnumerableAsync()
72-
{
73-
var result = await _resultAsync(cancellationToken).ConfigureAwait(false);
74-
return (IEnumerable<T>)ExecuteOnEval.DynamicInvoke(result);
75-
}
7667
}
7768

7869
#endregion
70+
71+
public IList TransformList(IList collection)
72+
{
73+
if (ExecuteOnEval == null)
74+
return collection;
75+
76+
return ((IEnumerable) ExecuteOnEval.DynamicInvoke(collection)).Cast<T>().ToList();
77+
}
7978
}
8079

8180
internal interface IDelayedValue
8281
{
8382
Delegate ExecuteOnEval { get; set; }
83+
84+
IList TransformList(IList collection);
8485
}
8586
}

‎src/NHibernate/Impl/FutureBatch.cs

Lines changed: 86 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,21 @@
1+
using System;
12
using System.Collections;
23
using System.Collections.Generic;
34
using System.Linq;
4-
using System.Threading;
5-
using System.Threading.Tasks;
5+
using NHibernate.Transform;
66

77
namespace NHibernate.Impl
88
{
99
public abstract partial class FutureBatch<TQueryApproach, TMultiApproach>
1010
{
11-
private readonly List<TQueryApproach> queries = new List<TQueryApproach>();
12-
private readonly IList<System.Type> resultTypes = new List<System.Type>();
11+
private class BatchedQuery
12+
{
13+
public TQueryApproach Query { get; set; }
14+
public System.Type ResultType { get; set; }
15+
public IDelayedValue Future { get; set; }
16+
}
17+
18+
private readonly List<BatchedQuery> queries = new List<BatchedQuery>();
1319
private int index;
1420
private IList results;
1521
private bool isCacheable = true;
@@ -29,8 +35,7 @@ public void Add<TResult>(TQueryApproach query)
2935
cacheRegion = CacheRegion(query);
3036
}
3137

32-
queries.Add(query);
33-
resultTypes.Add(typeof(TResult));
38+
queries.Add(new BatchedQuery { Query = query, ResultType = typeof(TResult) });
3439
index = queries.Count - 1;
3540
isCacheable = isCacheable && IsQueryCacheable(query);
3641
isCacheable = isCacheable && (cacheRegion == CacheRegion(query));
@@ -44,26 +49,44 @@ public void Add(TQueryApproach query)
4449
public IFutureValue<TResult> GetFutureValue<TResult>()
4550
{
4651
int currentIndex = index;
47-
return new FutureValue<TResult>(() => GetCurrentResult<TResult>(currentIndex), cancellationToken => GetCurrentResultAsync<TResult>(currentIndex, cancellationToken));
52+
var future = new FutureValue<TResult>(
53+
() => GetCurrentResult<TResult>(currentIndex),
54+
cancellationToken => GetCurrentResultAsync<TResult>(currentIndex, cancellationToken));
55+
var query = queries[currentIndex];
56+
query.Future = future;
57+
return future;
4858
}
4959

5060
public IFutureEnumerable<TResult> GetEnumerator<TResult>()
5161
{
5262
var currentIndex = index;
53-
return new DelayedEnumerator<TResult>(() => GetCurrentResult<TResult>(currentIndex), cancellationToken => GetCurrentResultAsync<TResult>(currentIndex, cancellationToken));
63+
var future = new DelayedEnumerator<TResult>(
64+
() => GetCurrentResult<TResult>(currentIndex),
65+
cancellationToken => GetCurrentResultAsync<TResult>(currentIndex, cancellationToken));
66+
var query = queries[currentIndex];
67+
query.Future = future;
68+
return future;
5469
}
5570

5671
private IList GetResults()
5772
{
5873
if (results != null)
59-
{
6074
return results;
61-
}
75+
6276
var multiApproach = CreateMultiApproach(isCacheable, cacheRegion);
63-
for (int i = 0; i < queries.Count; i++)
77+
var needTransformer = false;
78+
foreach (var query in queries)
6479
{
65-
AddTo(multiApproach, queries[i], resultTypes[i]);
80+
AddTo(multiApproach, query.Query, query.ResultType);
81+
if (query.Future?.ExecuteOnEval != null)
82+
needTransformer = true;
6683
}
84+
85+
if (needTransformer)
86+
AddResultTransformer(
87+
multiApproach,
88+
new FutureResultsTransformer(queries));
89+
6790
results = GetResultsFrom(multiApproach);
6891
ClearCurrentFutureBatch();
6992
return results;
@@ -80,5 +103,56 @@ private IEnumerable<TResult> GetCurrentResult<TResult>(int currentIndex)
80103
protected abstract void ClearCurrentFutureBatch();
81104
protected abstract bool IsQueryCacheable(TQueryApproach query);
82105
protected abstract string CacheRegion(TQueryApproach query);
106+
107+
protected virtual void AddResultTransformer(
108+
TMultiApproach multiApproach,
109+
IResultTransformer futureResulsTransformer)
110+
{
111+
// Only Linq set ExecuteOnEval, so only FutureQueryBatch needs to support it, not FutureCriteriaBatch.
112+
throw new NotSupportedException();
113+
}
114+
115+
// ResultTransformer are usually re-usable, this is not the case of this one, which will
116+
// be built for each multi-query requiring it.
117+
// It also usually ends in query cache, but this is not the case either for multi-query.
118+
[Serializable]
119+
private class FutureResultsTransformer : IResultTransformer
120+
{
121+
private readonly List<BatchedQuery> _batchedQueries;
122+
private int _currentIndex;
123+
124+
public FutureResultsTransformer(List<BatchedQuery> batchedQueries)
125+
{
126+
_batchedQueries = batchedQueries;
127+
}
128+
129+
public object TransformTuple(object[] tuple, string[] aliases)
130+
{
131+
return tuple.Length == 1 ? tuple[0] : tuple;
132+
}
133+
134+
public IList TransformList(IList collection)
135+
{
136+
if (_currentIndex >= _batchedQueries.Count)
137+
throw new InvalidOperationException(
138+
$"Transformer have been called more times ({_currentIndex + 1}) than it has queries to transform.");
139+
140+
var batchedQuery = _batchedQueries[_currentIndex];
141+
_currentIndex++;
142+
143+
return batchedQuery.Future?.TransformList(collection) ?? collection;
144+
}
145+
146+
// We do not really need to override them since this one does not ends in query cache, but a test forces us to.
147+
public override bool Equals(object obj)
148+
{
149+
return ReferenceEquals(this, obj);
150+
}
151+
152+
public override int GetHashCode()
153+
{
154+
return base.GetHashCode();
155+
}
156+
}
83157
}
84158
}

0 commit comments

Comments
 (0)