Skip to content

Commit c9f5b9f

Browse files
committed
Fix async command event notifications
1 parent 4d9816e commit c9f5b9f

File tree

4 files changed

+81
-27
lines changed

4 files changed

+81
-27
lines changed

CommunityToolkit.Mvvm/Input/AsyncRelayCommand.cs

Lines changed: 26 additions & 10 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/>
@@ -300,9 +313,12 @@ public Task ExecuteAsync(object? parameter)
300313
/// <inheritdoc/>
301314
public void Cancel()
302315
{
303-
this.cancellationTokenSource?.Cancel();
316+
if (this.cancellationTokenSource is CancellationTokenSource { IsCancellationRequested: false } cancellationTokenSource)
317+
{
318+
cancellationTokenSource.Cancel();
304319

305-
PropertyChanged?.Invoke(this, IsCancellationRequestedChangedEventArgs);
306-
PropertyChanged?.Invoke(this, CanBeCanceledChangedEventArgs);
320+
PropertyChanged?.Invoke(this, CanBeCanceledChangedEventArgs);
321+
PropertyChanged?.Invoke(this, IsCancellationRequestedChangedEventArgs);
322+
}
307323
}
308324
}

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

Lines changed: 24 additions & 10 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/>
@@ -311,9 +322,12 @@ public Task ExecuteAsync(object? parameter)
311322
/// <inheritdoc/>
312323
public void Cancel()
313324
{
314-
this.cancellationTokenSource?.Cancel();
325+
if (this.cancellationTokenSource is CancellationTokenSource { IsCancellationRequested: false } cancellationTokenSource)
326+
{
327+
cancellationTokenSource.Cancel();
315328

316-
PropertyChanged?.Invoke(this, AsyncRelayCommand.IsCancellationRequestedChangedEventArgs);
317-
PropertyChanged?.Invoke(this, AsyncRelayCommand.CanBeCanceledChangedEventArgs);
329+
PropertyChanged?.Invoke(this, AsyncRelayCommand.CanBeCanceledChangedEventArgs);
330+
PropertyChanged?.Invoke(this, AsyncRelayCommand.IsCancellationRequestedChangedEventArgs);
331+
}
318332
}
319333
}

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

0 commit comments

Comments
 (0)