Skip to content

Commit cb00c1d

Browse files
RosuavioRosuavio
committed
Move async lock to handle entire load and update
opertation. Added and cleaned up tests.
1 parent 81a8009 commit cb00c1d

File tree

3 files changed

+159
-46
lines changed

3 files changed

+159
-46
lines changed

‎Microsoft.Toolkit.Uwp/IncrementalLoadingCollection/IncrementalLoadingCollection.cs‎

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -228,36 +228,28 @@ public Task RefreshAsync()
228228
/// </returns>
229229
protected virtual async Task<IEnumerable<IType>> LoadDataAsync(CancellationToken cancellationToken)
230230
{
231-
// TODO (2021.05.05): Make use common AsyncMutex class.
232-
// AsyncMutex is located at Microsoft.Toolkit.Uwp.UI.Media/Extensions/System.Threading.Tasks/AsyncMutex.cs at the time of this note.
233-
await _mutex.WaitAsync();
234-
try
235-
{
236-
var result = await Source.GetPagedItemsAsync(CurrentPageIndex, ItemsPerPage, cancellationToken)
237-
.ContinueWith(
238-
t =>
231+
var result = await Source.GetPagedItemsAsync(CurrentPageIndex, ItemsPerPage, cancellationToken)
232+
.ContinueWith(
233+
t =>
234+
{
235+
if (t.Status == TaskStatus.RanToCompletion)
239236
{
240-
if (t.Status == TaskStatus.RanToCompletion)
241-
{
242-
CurrentPageIndex += 1;
243-
}
237+
CurrentPageIndex += 1;
238+
}
244239

245-
return t.Result;
246-
}, cancellationToken);
240+
return t.Result;
241+
}, cancellationToken);
247242

248-
return result;
249-
}
250-
finally
251-
{
252-
_mutex.Release();
253-
}
243+
return result;
254244
}
255245

256246
private async Task<LoadMoreItemsResult> LoadMoreItemsAsync(uint count, CancellationToken cancellationToken)
257247
{
258248
uint resultCount = 0;
259249
_cancellationToken = cancellationToken;
260-
250+
// TODO (2021.05.05): Make use common AsyncMutex class.
251+
// AsyncMutex is located at Microsoft.Toolkit.Uwp.UI.Media/Extensions/System.Threading.Tasks/AsyncMutex.cs at the time of this note.
252+
await _mutex.WaitAsync();
261253
try
262254
{
263255
if (!_cancellationToken.IsCancellationRequested)
@@ -301,6 +293,8 @@ private async Task<LoadMoreItemsResult> LoadMoreItemsAsync(uint count, Cancellat
301293
_refreshOnLoad = false;
302294
await RefreshAsync();
303295
}
296+
297+
_mutex.Release();
304298
}
305299

306300
return new LoadMoreItemsResult { Count = resultCount };

‎UnitTests/UnitTests.UWP/UI/Collection/DataSource.cs‎

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// See the LICENSE file in the project root for more information.
44

55
using Microsoft.Toolkit.Collections;
6+
using System;
67
using System.Collections.Generic;
78
using System.Linq;
89
using System.Threading;
@@ -12,32 +13,47 @@ namespace UnitTests.UI
1213
{
1314
public class DataSource<T> : IIncrementalSource<T>
1415
{
15-
private readonly IEnumerable<T> _data;
16+
private readonly IEnumerable<T> items;
17+
private readonly Queue<PageOperation> pageRequestOperations;
1618

17-
public DataSource(IEnumerable<T> items)
19+
public delegate IEnumerable<T> PageOperation(IEnumerable<T> page);
20+
21+
public DataSource(IEnumerable<T> items, IEnumerable<PageOperation> pageOps)
22+
: this(items, new Queue<PageOperation>(pageOps))
1823
{
19-
_data = items;
2024
}
2125

22-
public async Task<IEnumerable<T>> GetPagedItemsAsync(int pageIndex, int pageSize, CancellationToken cancellationToken = default(CancellationToken))
26+
public DataSource(IEnumerable<T> items, params PageOperation[] pageOps)
27+
: this(items, new Queue<PageOperation>(pageOps))
2328
{
24-
// Gets items from the collection according to pageIndex and pageSize parameters.
25-
var result = (from p in _data
26-
select p).Skip(pageIndex * pageSize).Take(pageSize);
29+
}
2730

28-
// Simulates a longer request...
29-
// Make sure the list is still in order after a refresh,
30-
// even if the first page takes longer to load
31-
if (pageIndex == 0)
32-
{
33-
await Task.Delay(2500);
34-
}
35-
else
31+
public DataSource(IEnumerable<T> items, Queue<PageOperation> pageOps = default)
32+
{
33+
this.items = items ?? throw new ArgumentNullException(nameof(items));
34+
this.pageRequestOperations = pageOps ?? new Queue<PageOperation>();
35+
}
36+
37+
public static PageOperation MakeDelayOp(int delay)
38+
=> new (page =>
3639
{
37-
await Task.Delay(1000);
38-
}
40+
Thread.Sleep(delay);
41+
return page;
42+
});
43+
44+
public static IEnumerable<T> ThrowException(IEnumerable<T> page) => throw new Exception();
45+
46+
public static IEnumerable<T> PassThrough(IEnumerable<T> page) => page;
47+
48+
public async Task<IEnumerable<T>> GetPagedItemsAsync(int pageIndex, int pageSize, CancellationToken cancellationToken = default)
49+
{
50+
// Gets items from the collection according to pageIndex and pageSize parameters.
51+
var result = (from p in items
52+
select p).Skip(pageIndex * pageSize).Take(pageSize);
3953

40-
return result;
54+
return this.pageRequestOperations.TryDequeue(out var op)
55+
? await Task.Factory.StartNew(new Func<object, IEnumerable<T>>(o => op(o as IEnumerable<T>)), state: result)
56+
: result;
4157
}
4258
}
4359
}

‎UnitTests/UnitTests.UWP/UI/Collection/Test_IncrementalLoadingCollection.cs‎

Lines changed: 110 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System;
66
using System.Collections.Generic;
77
using System.Linq;
8+
using System.Threading;
89
using System.Threading.Tasks;
910
using Microsoft.Toolkit.Uwp;
1011
using Microsoft.VisualStudio.TestTools.UnitTesting;
@@ -14,27 +15,45 @@ namespace UnitTests.UI
1415
[TestClass]
1516
public class Test_IncrementalLoadingCollection
1617
{
18+
private static readonly DataSource<int>.PageOperation[] FailPassSequence
19+
= new DataSource<int>.PageOperation[]
20+
{
21+
DataSource<int>.ThrowException, DataSource<int>.PassThrough,
22+
DataSource<int>.ThrowException, DataSource<int>.PassThrough,
23+
DataSource<int>.ThrowException, DataSource<int>.PassThrough,
24+
DataSource<int>.ThrowException, DataSource<int>.PassThrough,
25+
DataSource<int>.ThrowException, DataSource<int>.PassThrough,
26+
};
27+
28+
[DataRow(2500, 1000, 1000, 1000, 1000)]
29+
[DataRow]
1730
[TestMethod]
18-
public async Task SequentialRequests()
31+
public async Task Requests(params int[] pageDelays)
1932
{
2033
const int pageSize = 20;
2134
const int pages = 5;
22-
var collection = new IncrementalLoadingCollection<DataSource<int>, int>(new DataSource<int>(Enumerable.Range(0, pageSize * pages)), pageSize);
35+
36+
var source = new DataSource<int>(Enumerable.Range(0, pageSize * pages), pageDelays.Select(DataSource<int>.MakeDelayOp));
37+
var collection = new IncrementalLoadingCollection<DataSource<int>, int>(source, pageSize);
2338

2439
for (int pageNum = 1; pageNum <= pages; pageNum++)
2540
{
2641
var rez1 = await collection.LoadMoreItemsAsync(0);
2742
Assert.AreEqual((uint)pageSize, rez1.Count);
28-
CollectionAssert.AreEquivalent(Enumerable.Range(0, pageSize * pageNum).ToArray(), collection);
43+
CollectionAssert.AreEqual(Enumerable.Range(0, pageSize * pageNum).ToArray(), collection);
2944
}
3045
}
3146

47+
[DataRow(2500, 1000, 1000, 1000, 1000)]
48+
[DataRow]
3249
[TestMethod]
33-
public void ConcurentRequests()
50+
public async Task RequestsAsync(params int[] pageDelays)
3451
{
35-
const int pageSize = 20;
52+
const int pageSize = 20;
3653
const int pages = 5;
37-
var collection = new IncrementalLoadingCollection<DataSource<int>, int>(new DataSource<int>(Enumerable.Range(0, pageSize * pages)), pageSize);
54+
55+
var source = new DataSource<int>(Enumerable.Range(0, pageSize * pages), pageDelays.Select(DataSource<int>.MakeDelayOp));
56+
var collection = new IncrementalLoadingCollection<DataSource<int>, int>(source, pageSize);
3857

3958
var requests = new List<Task>();
4059

@@ -47,8 +66,92 @@ public void ConcurentRequests()
4766
}));
4867
}
4968

50-
Task.WaitAll(requests.ToArray());
69+
await Task.WhenAll(requests);
70+
71+
CollectionAssert.AreEqual(Enumerable.Range(0, pageSize * pages).ToArray(), collection);
72+
}
73+
74+
[TestMethod]
75+
public async Task FirstRequestFails()
76+
{
77+
const int pageSize = 20;
78+
const int pages = 5;
79+
80+
var source = new DataSource<int>(Enumerable.Range(0, pageSize * pages), DataSource<int>.ThrowException);
81+
var collection = new IncrementalLoadingCollection<DataSource<int>, int>(source, pageSize);
82+
83+
await Assert.ThrowsExceptionAsync<AggregateException>(async () => await collection.LoadMoreItemsAsync(0));
84+
85+
Assert.IsTrue(!collection.Any());
86+
87+
var requests = new List<Task>();
88+
89+
for (int pageNum = 1; pageNum <= pages; pageNum++)
90+
{
91+
requests.Add(collection.LoadMoreItemsAsync(0).AsTask());
92+
}
93+
94+
await Task.WhenAll(requests);
95+
96+
CollectionAssert.AreEqual(Enumerable.Range(0, pageSize * pages).ToArray(), collection);
97+
}
98+
99+
[TestMethod]
100+
public async Task EveryOtherRequestFails()
101+
{
102+
const int pageSize = 20;
103+
const int pages = 5;
104+
105+
var source = new DataSource<int>(Enumerable.Range(0, pageSize * pages), FailPassSequence);
106+
var collection = new IncrementalLoadingCollection<DataSource<int>, int>(source, pageSize);
107+
108+
var willFail = true;
109+
for (int submitedRequests = 0; submitedRequests < 10; submitedRequests++)
110+
{
111+
if (willFail)
112+
{
113+
await collection.LoadMoreItemsAsync(0).AsTask().ContinueWith(t => Assert.AreEqual(TaskStatus.Faulted, t.Status));
114+
}
115+
else
116+
{
117+
await collection.LoadMoreItemsAsync(0).AsTask().ContinueWith(t => Assert.AreEqual(TaskStatus.RanToCompletion, t.Status));
118+
}
119+
120+
willFail = !willFail;
121+
}
122+
51123
CollectionAssert.AreEquivalent(Enumerable.Range(0, pageSize * pages).ToArray(), collection);
52124
}
125+
126+
[TestMethod]
127+
public async Task EveryOtherRequestFailsAsync()
128+
{
129+
const int pageSize = 20;
130+
const int pages = 5;
131+
132+
var source = new DataSource<int>(Enumerable.Range(0, pageSize * pages), FailPassSequence);
133+
var collection = new IncrementalLoadingCollection<DataSource<int>, int>(source, pageSize);
134+
135+
var requests = new List<Task>();
136+
137+
var willFail = true;
138+
for (int submitedRequests = 0; submitedRequests < 10; submitedRequests++)
139+
{
140+
if (willFail)
141+
{
142+
requests.Add(Assert.ThrowsExceptionAsync<AggregateException>(() => collection.LoadMoreItemsAsync(0).AsTask()));
143+
}
144+
else
145+
{
146+
requests.Add(collection.LoadMoreItemsAsync(0).AsTask());
147+
}
148+
149+
willFail = !willFail;
150+
}
151+
152+
await Task.WhenAll(requests);
153+
154+
CollectionAssert.AreEqual(Enumerable.Range(0, pageSize * pages).ToArray(), collection);
155+
}
53156
}
54157
}

0 commit comments

Comments
 (0)