Skip to content

Commit

Permalink
Merge pull request #127 from CommunityToolkit/dev/default-async-comma…
Browse files Browse the repository at this point in the history
…nd-concurrency-false

Switch async commands to default to no concurrent execution
  • Loading branch information
Sergio0694 authored Feb 28, 2022
2 parents c71eb35 + 0e26a29 commit 90cf344
Show file tree
Hide file tree
Showing 7 changed files with 133 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,10 @@ public static ImmutableArray<MemberDeclarationSyntax> GetSyntax(CommandInfo comm
commandCreationArguments.Add(Argument(canExecuteExpression));
}

// Disable concurrent executions, if requested
if (!commandInfo.AllowConcurrentExecutions)
// Enable concurrent executions, if requested
if (commandInfo.AllowConcurrentExecutions)
{
commandCreationArguments.Add(Argument(LiteralExpression(SyntaxKind.FalseLiteralExpression)));
commandCreationArguments.Add(Argument(LiteralExpression(SyntaxKind.TrueLiteralExpression)));
}

// Construct the generated property as follows (the explicit delegate cast is needed to avoid overload resolution conflicts):
Expand Down Expand Up @@ -366,7 +366,7 @@ private static bool TryMapCommandTypesFromMethod(
/// <param name="attributeData">The <see cref="AttributeData"/> instance the method was annotated with.</param>
/// <param name="commandClassType">The command class type name.</param>
/// <param name="diagnostics">The current collection of gathered diagnostics.</param>
/// <param name="allowConcurrentExecutions">Whether or not concurrent executions have been disabled.</param>
/// <param name="allowConcurrentExecutions">Whether or not concurrent executions have been enabled.</param>
/// <returns>Whether or not a value for <paramref name="allowConcurrentExecutions"/> could be retrieved successfully.</returns>
private static bool TryGetAllowConcurrentExecutionsSwitch(
IMethodSymbol methodSymbol,
Expand All @@ -375,11 +375,10 @@ private static bool TryGetAllowConcurrentExecutionsSwitch(
ImmutableArray<Diagnostic>.Builder diagnostics,
out bool allowConcurrentExecutions)
{
// Try to get the custom switch for concurrent executions. If the switch is not present, the
// default value is set to true, to avoid breaking backwards compatibility with the first release.
// Try to get the custom switch for concurrent executions (the default is false)
if (!attributeData.TryGetNamedArgument("AllowConcurrentExecutions", out allowConcurrentExecutions))
{
allowConcurrentExecutions = true;
allowConcurrentExecutions = false;

return true;
}
Expand Down
12 changes: 4 additions & 8 deletions CommunityToolkit.Mvvm/Input/AsyncRelayCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public sealed class AsyncRelayCommand : IAsyncRelayCommand
public event EventHandler? CanExecuteChanged;

/// <summary>
/// Initializes a new instance of the <see cref="AsyncRelayCommand"/> class that can always execute.
/// Initializes a new instance of the <see cref="AsyncRelayCommand"/> class.
/// </summary>
/// <param name="execute">The execution logic.</param>
/// <exception cref="System.ArgumentNullException">Thrown if <paramref name="execute"/> is <see langword="null"/>.</exception>
Expand All @@ -81,11 +81,10 @@ public AsyncRelayCommand(Func<Task> execute)
ArgumentNullException.ThrowIfNull(execute);

this.execute = execute;
this.allowConcurrentExecutions = true;
}

/// <summary>
/// Initializes a new instance of the <see cref="AsyncRelayCommand"/> class that can always execute.
/// Initializes a new instance of the <see cref="AsyncRelayCommand"/> class.
/// </summary>
/// <param name="execute">The execution logic.</param>
/// <param name="allowConcurrentExecutions">Whether or not to allow concurrent executions of the command.</param>
Expand All @@ -99,7 +98,7 @@ public AsyncRelayCommand(Func<Task> execute, bool allowConcurrentExecutions)
}

/// <summary>
/// Initializes a new instance of the <see cref="AsyncRelayCommand"/> class that can always execute.
/// Initializes a new instance of the <see cref="AsyncRelayCommand"/> class.
/// </summary>
/// <param name="cancelableExecute">The cancelable execution logic.</param>
/// <exception cref="System.ArgumentNullException">Thrown if <paramref name="cancelableExecute"/> is <see langword="null"/>.</exception>
Expand All @@ -108,11 +107,10 @@ public AsyncRelayCommand(Func<CancellationToken, Task> cancelableExecute)
ArgumentNullException.ThrowIfNull(cancelableExecute);

this.cancelableExecute = cancelableExecute;
this.allowConcurrentExecutions = true;
}

/// <summary>
/// Initializes a new instance of the <see cref="AsyncRelayCommand"/> class that can always execute.
/// Initializes a new instance of the <see cref="AsyncRelayCommand"/> class.
/// </summary>
/// <param name="cancelableExecute">The cancelable execution logic.</param>
/// <param name="allowConcurrentExecutions">Whether or not to allow concurrent executions of the command.</param>
Expand All @@ -138,7 +136,6 @@ public AsyncRelayCommand(Func<Task> execute, Func<bool> canExecute)

this.execute = execute;
this.canExecute = canExecute;
this.allowConcurrentExecutions = true;
}

/// <summary>
Expand Down Expand Up @@ -171,7 +168,6 @@ public AsyncRelayCommand(Func<CancellationToken, Task> cancelableExecute, Func<b

this.cancelableExecute = cancelableExecute;
this.canExecute = canExecute;
this.allowConcurrentExecutions = true;
}

/// <summary>
Expand Down
12 changes: 4 additions & 8 deletions CommunityToolkit.Mvvm/Input/AsyncRelayCommand{T}.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public sealed class AsyncRelayCommand<T> : IAsyncRelayCommand<T>
public event EventHandler? CanExecuteChanged;

/// <summary>
/// Initializes a new instance of the <see cref="AsyncRelayCommand{T}"/> class that can always execute.
/// Initializes a new instance of the <see cref="AsyncRelayCommand{T}"/> class.
/// </summary>
/// <param name="execute">The execution logic.</param>
/// <remarks>See notes in <see cref="RelayCommand{T}(Action{T})"/>.</remarks>
Expand All @@ -58,11 +58,10 @@ public AsyncRelayCommand(Func<T?, Task> execute)
ArgumentNullException.ThrowIfNull(execute);

this.execute = execute;
this.allowConcurrentExecutions = true;
}

/// <summary>
/// Initializes a new instance of the <see cref="AsyncRelayCommand{T}"/> class that can always execute.
/// Initializes a new instance of the <see cref="AsyncRelayCommand{T}"/> class.
/// </summary>
/// <param name="execute">The execution logic.</param>
/// <param name="allowConcurrentExecutions">Whether or not to allow concurrent executions of the command.</param>
Expand All @@ -77,7 +76,7 @@ public AsyncRelayCommand(Func<T?, Task> execute, bool allowConcurrentExecutions)
}

/// <summary>
/// Initializes a new instance of the <see cref="AsyncRelayCommand{T}"/> class that can always execute.
/// Initializes a new instance of the <see cref="AsyncRelayCommand{T}"/> class.
/// </summary>
/// <param name="cancelableExecute">The cancelable execution logic.</param>
/// <remarks>See notes in <see cref="RelayCommand{T}(Action{T})"/>.</remarks>
Expand All @@ -87,11 +86,10 @@ public AsyncRelayCommand(Func<T?, CancellationToken, Task> cancelableExecute)
ArgumentNullException.ThrowIfNull(cancelableExecute);

this.cancelableExecute = cancelableExecute;
this.allowConcurrentExecutions = true;
}

/// <summary>
/// Initializes a new instance of the <see cref="AsyncRelayCommand{T}"/> class that can always execute.
/// Initializes a new instance of the <see cref="AsyncRelayCommand{T}"/> class.
/// </summary>
/// <param name="cancelableExecute">The cancelable execution logic.</param>
/// <param name="allowConcurrentExecutions">Whether or not to allow concurrent executions of the command.</param>
Expand Down Expand Up @@ -119,7 +117,6 @@ public AsyncRelayCommand(Func<T?, Task> execute, Predicate<T?> canExecute)

this.execute = execute;
this.canExecute = canExecute;
this.allowConcurrentExecutions = true;
}

/// <summary>
Expand Down Expand Up @@ -154,7 +151,6 @@ public AsyncRelayCommand(Func<T?, CancellationToken, Task> cancelableExecute, Pr

this.cancelableExecute = cancelableExecute;
this.canExecute = canExecute;
this.allowConcurrentExecutions = true;
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,5 @@ public sealed class ICommandAttribute : Attribute
/// when an execution is invoked while a previous one is still running. It is the same as creating an instance of
/// these command types with a constructor such as <see cref="AsyncRelayCommand(Func{System.Threading.Tasks.Task}, bool)"/>.
/// </summary>
public bool AllowConcurrentExecutions { get; init; } = true;
public bool AllowConcurrentExecutions { get; init; }
}
77 changes: 75 additions & 2 deletions tests/CommunityToolkit.Mvvm.UnitTests/Test_AsyncRelayCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,84 @@ public async Task Test_AsyncRelayCommand_WithCancellation()
}

[TestMethod]
public async Task Test_AsyncRelayCommand_WithConcurrencyControl()
public async Task Test_AsyncRelayCommand_AllowConcurrentExecutions_Enable()
{
int index = 0;
TaskCompletionSource<object?>[] cancellationTokenSources = new TaskCompletionSource<object?>[]
{
new TaskCompletionSource<object?>(),
new TaskCompletionSource<object?>()
};

AsyncRelayCommand? command = new(() => cancellationTokenSources[index++].Task, allowConcurrentExecutions: true);

Assert.IsTrue(command.CanExecute(null));
Assert.IsTrue(command.CanExecute(new object()));

Assert.IsFalse(command.CanBeCanceled);
Assert.IsFalse(command.IsCancellationRequested);

(object?, EventArgs?) args = default;

command.CanExecuteChanged += (s, e) => args = (s, e);

command.NotifyCanExecuteChanged();

Assert.AreSame(args.Item1, command);
Assert.AreSame(args.Item2, EventArgs.Empty);

Assert.IsNull(command.ExecutionTask);
Assert.IsFalse(command.IsRunning);

Task task = command.ExecuteAsync(null);

Assert.IsNotNull(command.ExecutionTask);
Assert.AreSame(command.ExecutionTask, task);
Assert.AreSame(command.ExecutionTask, cancellationTokenSources[0].Task);
Assert.IsTrue(command.IsRunning);

// The command can still be executed now
Assert.IsTrue(command.CanExecute(null));
Assert.IsTrue(command.CanExecute(new object()));

Assert.IsFalse(command.CanBeCanceled);
Assert.IsFalse(command.IsCancellationRequested);

Task newTask = command.ExecuteAsync(null);

// A new task was returned
Assert.AreSame(command.ExecutionTask, newTask);
Assert.AreSame(command.ExecutionTask, cancellationTokenSources[1].Task);

cancellationTokenSources[0].SetResult(null);
cancellationTokenSources[1].SetResult(null);

_ = await Task.WhenAll(cancellationTokenSources[0].Task, cancellationTokenSources[1].Task);

Assert.IsFalse(command.IsRunning);
}

[TestMethod]
public async Task Test_AsyncRelayCommand_AllowConcurrentExecutions_Disabled()
{
await Test_AsyncRelayCommand_AllowConcurrentExecutions_TestLogic(static task => new(async () => await task, allowConcurrentExecutions: false));
}

[TestMethod]
public async Task Test_AsyncRelayCommand_AllowConcurrentExecutions_Default()
{
await Test_AsyncRelayCommand_AllowConcurrentExecutions_TestLogic(static task => new(async () => await task));
}

/// <summary>
/// Shared logic for <see cref="Test_AsyncRelayCommand_AllowConcurrentExecutions_Disabled"/> and <see cref="Test_AsyncRelayCommand_AllowConcurrentExecutions_Default"/>.
/// </summary>
/// <param name="factory">A factory to create the <see cref="AsyncRelayCommand"/> instance to test.</param>
private static async Task Test_AsyncRelayCommand_AllowConcurrentExecutions_TestLogic(Func<Task, AsyncRelayCommand> factory)
{
TaskCompletionSource<object?> tcs = new();

AsyncRelayCommand? command = new(async () => await tcs.Task, allowConcurrentExecutions: false);
AsyncRelayCommand? command = factory(tcs.Task);

Assert.IsTrue(command.CanExecute(null));
Assert.IsTrue(command.CanExecute(new object()));
Expand Down
19 changes: 17 additions & 2 deletions tests/CommunityToolkit.Mvvm.UnitTests/Test_AsyncRelayCommand{T}.cs
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,26 @@ public void Test_AsyncRelayCommandOfT_NullWithValueType()
}

[TestMethod]
public async Task Test_AsyncRelayCommandOfT_WithConcurrencyControl()
public async Task Test_AsyncRelayCommandOfT_AllowConcurrentExecutions_Disabled()
{
await Test_AsyncRelayCommandOfT_AllowConcurrentExecutions_TestLogic(static task => new(async _ => await task, allowConcurrentExecutions: false));
}

[TestMethod]
public async Task Test_AsyncRelayCommandOfT_AllowConcurrentExecutions_Default()
{
await Test_AsyncRelayCommandOfT_AllowConcurrentExecutions_TestLogic(static task => new(async _ => await task));
}

/// <summary>
/// Shared logic for <see cref="Test_AsyncRelayCommandOfT_AllowConcurrentExecutions_Disabled"/> and <see cref="Test_AsyncRelayCommandOfT_AllowConcurrentExecutions_Default"/>.
/// </summary>
/// <param name="factory">A factory to create the <see cref="AsyncRelayCommand{T}"/> instance to test.</param>
private static async Task Test_AsyncRelayCommandOfT_AllowConcurrentExecutions_TestLogic(Func<Task, AsyncRelayCommand<string>> factory)
{
TaskCompletionSource<object?> tcs = new();

AsyncRelayCommand<string> command = new(async s => await tcs.Task, allowConcurrentExecutions: false);
AsyncRelayCommand<string> command = factory(tcs.Task);

Assert.IsTrue(command.CanExecute(null));
Assert.IsTrue(command.CanExecute("1"));
Expand Down
27 changes: 26 additions & 1 deletion tests/CommunityToolkit.Mvvm.UnitTests/Test_ICommandAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,23 @@ public async Task Test_ICommandAttribute_RelayCommand()
CollectionAssert.AreEqual(model.Values, Enumerable.Range(0, 10).ToArray());

await Task.WhenAll(tasks);

tasks.Clear();

for (int i = 10; i < 20; i++)
{
tasks.Add(model.AddValueToListAndDelayWithDefaultConcurrencyCommand.ExecuteAsync(i));
}

Assert.AreEqual(10, tasks.Count);

// Only the first item should have been added
CollectionAssert.AreEqual(model.Values, Enumerable.Range(0, 11).ToArray());

for (int i = 1; i < tasks.Count; i++)
{
Assert.AreSame(Task.CompletedTask, tasks[i]);
}
}

[TestMethod]
Expand Down Expand Up @@ -366,14 +383,22 @@ private async Task DelayAndIncrementCounterAsync()
Counter += 1;
}

[ICommand]
[ICommand(AllowConcurrentExecutions = true)]
private async Task AddValueToListAndDelayAsync(int value)
{
Values.Add(value);

await Task.Delay(100);
}

[ICommand]
private async Task AddValueToListAndDelayWithDefaultConcurrencyAsync(int value)
{
Values.Add(value);

await Task.Delay(1000);
}

#region Test region

/// <summary>
Expand Down

0 comments on commit 90cf344

Please sign in to comment.