Skip to content

Commit df3b529

Browse files
Sergio0694Sergio0694
authored
Merge pull request #122 from CommunityToolkit/dev/async-command-events-fixup
Fix async command event notifications
2 parents 4d9816e + 9c0a1b3 commit df3b529

File tree

5 files changed

+127
-31
lines changed

5 files changed

+127
-31
lines changed

‎CommunityToolkit.Mvvm/Input/AsyncRelayCommand.cs

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -207,9 +207,18 @@ private set
207207

208208
PropertyChanged?.Invoke(this, ExecutionTaskChangedEventArgs);
209209
PropertyChanged?.Invoke(this, IsRunningChangedEventArgs);
210-
PropertyChanged?.Invoke(this, CanBeCanceledChangedEventArgs);
211210

212-
if (value?.IsCompleted ?? true)
211+
bool isAlreadyCompletedOrNull = value?.IsCompleted ?? true;
212+
213+
if (this.cancellationTokenSource is not null)
214+
{
215+
PropertyChanged?.Invoke(this, CanBeCanceledChangedEventArgs);
216+
PropertyChanged?.Invoke(this, IsCancellationRequestedChangedEventArgs);
217+
}
218+
219+
// The branch is on a condition evaluated before raising the events above if
220+
// needed, to avoid race conditions with a task completing right after them.
221+
if (isAlreadyCompletedOrNull)
213222
{
214223
return;
215224
}
@@ -222,7 +231,11 @@ static async void MonitorTask(AsyncRelayCommand @this, Task task)
222231
{
223232
@this.PropertyChanged?.Invoke(@this, ExecutionTaskChangedEventArgs);
224233
@this.PropertyChanged?.Invoke(@this, IsRunningChangedEventArgs);
225-
@this.PropertyChanged?.Invoke(@this, CanBeCanceledChangedEventArgs);
234+
235+
if (@this.cancellationTokenSource is not null)
236+
{
237+
@this.PropertyChanged?.Invoke(@this, CanBeCanceledChangedEventArgs);
238+
}
226239
}
227240
}
228241

@@ -231,13 +244,13 @@ static async void MonitorTask(AsyncRelayCommand @this, Task task)
231244
}
232245

233246
/// <inheritdoc/>
234-
public bool CanBeCanceled => this.cancelableExecute is not null && IsRunning;
247+
public bool CanBeCanceled => IsRunning && this.cancellationTokenSource is { IsCancellationRequested: false };
235248

236249
/// <inheritdoc/>
237-
public bool IsCancellationRequested => this.cancellationTokenSource?.IsCancellationRequested == true;
250+
public bool IsCancellationRequested => this.cancellationTokenSource is { IsCancellationRequested: true };
238251

239252
/// <inheritdoc/>
240-
public bool IsRunning => ExecutionTask?.IsCompleted == false;
253+
public bool IsRunning => ExecutionTask is { IsCompleted: false };
241254

242255
/// <inheritdoc/>
243256
public void NotifyCanExecuteChanged()
@@ -251,7 +264,7 @@ public bool CanExecute(object? parameter)
251264
{
252265
bool canExecute = this.canExecute?.Invoke() != false;
253266

254-
return canExecute && (this.allowConcurrentExecutions || ExecutionTask?.IsCompleted != false);
267+
return canExecute && (this.allowConcurrentExecutions || ExecutionTask is not { IsCompleted: false });
255268
}
256269

257270
/// <inheritdoc/>
@@ -279,8 +292,6 @@ public Task ExecuteAsync(object? parameter)
279292

280293
CancellationTokenSource cancellationTokenSource = this.cancellationTokenSource = new();
281294

282-
PropertyChanged?.Invoke(this, IsCancellationRequestedChangedEventArgs);
283-
284295
// Invoke the cancelable command delegate with a new linked token
285296
executionTask = ExecutionTask = this.cancelableExecute!(cancellationTokenSource.Token);
286297
}
@@ -300,9 +311,12 @@ public Task ExecuteAsync(object? parameter)
300311
/// <inheritdoc/>
301312
public void Cancel()
302313
{
303-
this.cancellationTokenSource?.Cancel();
314+
if (this.cancellationTokenSource is CancellationTokenSource { IsCancellationRequested: false } cancellationTokenSource)
315+
{
316+
cancellationTokenSource.Cancel();
304317

305-
PropertyChanged?.Invoke(this, IsCancellationRequestedChangedEventArgs);
306-
PropertyChanged?.Invoke(this, CanBeCanceledChangedEventArgs);
318+
PropertyChanged?.Invoke(this, CanBeCanceledChangedEventArgs);
319+
PropertyChanged?.Invoke(this, IsCancellationRequestedChangedEventArgs);
320+
}
307321
}
308322
}

‎CommunityToolkit.Mvvm/Input/AsyncRelayCommand{T}.cs

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -191,9 +191,16 @@ private set
191191

192192
PropertyChanged?.Invoke(this, AsyncRelayCommand.ExecutionTaskChangedEventArgs);
193193
PropertyChanged?.Invoke(this, AsyncRelayCommand.IsRunningChangedEventArgs);
194-
PropertyChanged?.Invoke(this, AsyncRelayCommand.CanBeCanceledChangedEventArgs);
195194

196-
if (value?.IsCompleted ?? true)
195+
bool isAlreadyCompletedOrNull = value?.IsCompleted ?? true;
196+
197+
if (this.cancellationTokenSource is not null)
198+
{
199+
PropertyChanged?.Invoke(this, AsyncRelayCommand.CanBeCanceledChangedEventArgs);
200+
PropertyChanged?.Invoke(this, AsyncRelayCommand.IsCancellationRequestedChangedEventArgs);
201+
}
202+
203+
if (isAlreadyCompletedOrNull)
197204
{
198205
return;
199206
}
@@ -206,7 +213,11 @@ static async void MonitorTask(AsyncRelayCommand<T> @this, Task task)
206213
{
207214
@this.PropertyChanged?.Invoke(@this, AsyncRelayCommand.ExecutionTaskChangedEventArgs);
208215
@this.PropertyChanged?.Invoke(@this, AsyncRelayCommand.IsRunningChangedEventArgs);
209-
@this.PropertyChanged?.Invoke(@this, AsyncRelayCommand.CanBeCanceledChangedEventArgs);
216+
217+
if (@this.cancellationTokenSource is not null)
218+
{
219+
@this.PropertyChanged?.Invoke(@this, AsyncRelayCommand.CanBeCanceledChangedEventArgs);
220+
}
210221
}
211222
}
212223

@@ -215,13 +226,13 @@ static async void MonitorTask(AsyncRelayCommand<T> @this, Task task)
215226
}
216227

217228
/// <inheritdoc/>
218-
public bool CanBeCanceled => this.cancelableExecute is not null && IsRunning;
229+
public bool CanBeCanceled => IsRunning && this.cancellationTokenSource is { IsCancellationRequested: false };
219230

220231
/// <inheritdoc/>
221-
public bool IsCancellationRequested => this.cancellationTokenSource?.IsCancellationRequested == true;
232+
public bool IsCancellationRequested => this.cancellationTokenSource is { IsCancellationRequested: true };
222233

223234
/// <inheritdoc/>
224-
public bool IsRunning => ExecutionTask?.IsCompleted == false;
235+
public bool IsRunning => ExecutionTask is { IsCompleted: false };
225236

226237
/// <inheritdoc/>
227238
public void NotifyCanExecuteChanged()
@@ -235,7 +246,7 @@ public bool CanExecute(T? parameter)
235246
{
236247
bool canExecute = this.canExecute?.Invoke(parameter) != false;
237248

238-
return canExecute && (this.allowConcurrentExecutions || ExecutionTask?.IsCompleted != false);
249+
return canExecute && (this.allowConcurrentExecutions || ExecutionTask is not { IsCompleted: false });
239250
}
240251

241252
/// <inheritdoc/>
@@ -284,8 +295,6 @@ public Task ExecuteAsync(T? parameter)
284295

285296
CancellationTokenSource cancellationTokenSource = this.cancellationTokenSource = new();
286297

287-
PropertyChanged?.Invoke(this, AsyncRelayCommand.IsCancellationRequestedChangedEventArgs);
288-
289298
// Invoke the cancelable command delegate with a new linked token
290299
executionTask = ExecutionTask = this.cancelableExecute!(parameter, cancellationTokenSource.Token);
291300
}
@@ -311,9 +320,12 @@ public Task ExecuteAsync(object? parameter)
311320
/// <inheritdoc/>
312321
public void Cancel()
313322
{
314-
this.cancellationTokenSource?.Cancel();
323+
if (this.cancellationTokenSource is CancellationTokenSource { IsCancellationRequested: false } cancellationTokenSource)
324+
{
325+
cancellationTokenSource.Cancel();
315326

316-
PropertyChanged?.Invoke(this, AsyncRelayCommand.IsCancellationRequestedChangedEventArgs);
317-
PropertyChanged?.Invoke(this, AsyncRelayCommand.CanBeCanceledChangedEventArgs);
327+
PropertyChanged?.Invoke(this, AsyncRelayCommand.CanBeCanceledChangedEventArgs);
328+
PropertyChanged?.Invoke(this, AsyncRelayCommand.IsCancellationRequestedChangedEventArgs);
329+
}
318330
}
319331
}

‎CommunityToolkit.Mvvm/Input/Interfaces/IAsyncRelayCommand.cs

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,32 @@ public interface IAsyncRelayCommand : IRelayCommand, INotifyPropertyChanged
1919
Task? ExecutionTask { get; }
2020

2121
/// <summary>
22-
/// Gets a value indicating whether running operations for this command can be canceled.
22+
/// Gets a value indicating whether a running operation for this command can currently be canceled.
2323
/// </summary>
24+
/// <remarks>
25+
/// The exact sequence of events that types implementing this interface should raise is as follows:
26+
/// <list type="bullet">
27+
/// <item>
28+
/// The command is initially not running: <see cref="IsRunning"/>, <see cref="CanBeCanceled"/>
29+
/// and <see cref="IsCancellationRequested"/> are <see langword="false"/>.
30+
/// </item>
31+
/// <item>
32+
/// The command starts running: <see cref="IsRunning"/> and <see cref="CanBeCanceled"/> switch to
33+
/// <see langword="true"/>. <see cref="IsCancellationRequested"/> is set to <see langword="false"/>.
34+
/// </item>
35+
/// <item>
36+
/// If the operation is canceled: <see cref="CanBeCanceled"/> switches to <see langword="false"/>
37+
/// and <see cref="IsCancellationRequested"/> switches to <see langword="true"/>.
38+
/// </item>
39+
/// <item>
40+
/// The operation completes: <see cref="IsRunning"/> and <see cref="CanBeCanceled"/> switch
41+
/// to <see langword="false"/>. The state of <see cref="IsCancellationRequested"/> is undefined.
42+
/// </item>
43+
/// </list>
44+
/// This only applies if the underlying logic for the command actually supports cancelation. If that is
45+
/// not the case, then <see cref="CanBeCanceled"/> and <see cref="IsCancellationRequested"/> will always remain
46+
/// <see langword="false"/> regardless of the current state of the command.
47+
/// </remarks>
2448
bool CanBeCanceled { get; }
2549

2650
/// <summary>

‎tests/CommunityToolkit.Mvvm.UnitTests/Test_AsyncRelayCommand.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -151,18 +151,18 @@ public async Task Test_AsyncRelayCommand_WithCancellation()
151151

152152
// Validate the various event args for all the properties that were updated when executing the command
153153
Assert.AreEqual(args.Count, 4);
154-
Assert.AreEqual(args[0].PropertyName, nameof(IAsyncRelayCommand.IsCancellationRequested));
155-
Assert.AreEqual(args[1].PropertyName, nameof(IAsyncRelayCommand.ExecutionTask));
156-
Assert.AreEqual(args[2].PropertyName, nameof(IAsyncRelayCommand.IsRunning));
157-
Assert.AreEqual(args[3].PropertyName, nameof(IAsyncRelayCommand.CanBeCanceled));
154+
Assert.AreEqual(args[0].PropertyName, nameof(IAsyncRelayCommand.ExecutionTask));
155+
Assert.AreEqual(args[1].PropertyName, nameof(IAsyncRelayCommand.IsRunning));
156+
Assert.AreEqual(args[2].PropertyName, nameof(IAsyncRelayCommand.CanBeCanceled));
157+
Assert.AreEqual(args[3].PropertyName, nameof(IAsyncRelayCommand.IsCancellationRequested));
158158

159159
command.Cancel();
160160

161161
// Verify that these two properties raised notifications correctly when canceling the command too.
162162
// We need to ensure all command properties support notifications so that users can bind to them.
163163
Assert.AreEqual(args.Count, 6);
164-
Assert.AreEqual(args[4].PropertyName, nameof(IAsyncRelayCommand.IsCancellationRequested));
165-
Assert.AreEqual(args[5].PropertyName, nameof(IAsyncRelayCommand.CanBeCanceled));
164+
Assert.AreEqual(args[4].PropertyName, nameof(IAsyncRelayCommand.CanBeCanceled));
165+
Assert.AreEqual(args[5].PropertyName, nameof(IAsyncRelayCommand.IsCancellationRequested));
166166

167167
Assert.IsTrue(command.IsCancellationRequested);
168168

‎tests/CommunityToolkit.Mvvm.UnitTests/Test_AsyncRelayCommand{T}.cs

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

55
using System;
6+
using System.Collections.Generic;
7+
using System.ComponentModel;
68
using System.Threading.Tasks;
79
using CommunityToolkit.Mvvm.Input;
810
using CommunityToolkit.Mvvm.UnitTests.Helpers;
@@ -134,6 +136,50 @@ public void Test_AsyncRelayCommandOfT_NullWithValueType()
134136
_ = Assert.ThrowsException<NullReferenceException>(() => command.Execute(null));
135137
}
136138

139+
[TestMethod]
140+
public async Task Test_AsyncRelayCommandOfT_WithCancellation()
141+
{
142+
// See comments in Test_AsyncRelayCommand_WithCancellation for the logic below
143+
TaskCompletionSource<object?> tcs = new();
144+
AsyncRelayCommand<string> command = new((s, token) => tcs.Task);
145+
146+
List<PropertyChangedEventArgs> args = new();
147+
148+
command.PropertyChanged += (s, e) => args.Add(e);
149+
150+
Assert.IsTrue(command.CanExecute(null));
151+
Assert.IsTrue(command.CanExecute("Hello"));
152+
153+
Assert.IsFalse(command.CanBeCanceled);
154+
Assert.IsFalse(command.IsCancellationRequested);
155+
156+
command.Execute(null);
157+
158+
Assert.IsTrue(command.CanBeCanceled);
159+
Assert.IsFalse(command.IsCancellationRequested);
160+
161+
Assert.AreEqual(args.Count, 4);
162+
Assert.AreEqual(args[0].PropertyName, nameof(IAsyncRelayCommand.ExecutionTask));
163+
Assert.AreEqual(args[1].PropertyName, nameof(IAsyncRelayCommand.IsRunning));
164+
Assert.AreEqual(args[2].PropertyName, nameof(IAsyncRelayCommand.CanBeCanceled));
165+
Assert.AreEqual(args[3].PropertyName, nameof(IAsyncRelayCommand.IsCancellationRequested));
166+
167+
command.Cancel();
168+
169+
Assert.AreEqual(args.Count, 6);
170+
Assert.AreEqual(args[4].PropertyName, nameof(IAsyncRelayCommand.CanBeCanceled));
171+
Assert.AreEqual(args[5].PropertyName, nameof(IAsyncRelayCommand.IsCancellationRequested));
172+
173+
Assert.IsTrue(command.IsCancellationRequested);
174+
175+
tcs.SetResult(null);
176+
177+
await command.ExecutionTask!;
178+
179+
Assert.IsFalse(command.CanBeCanceled);
180+
Assert.IsTrue(command.IsCancellationRequested);
181+
}
182+
137183
[TestMethod]
138184
public async Task Test_AsyncRelayCommandOfT_AllowConcurrentExecutions_Disabled()
139185
{

0 commit comments

Comments
 (0)