15

I've been Googling and even Bing-ing and I haven't come up with anything that is satisfying.

I have a ViewModel which has some commands, such as: SaveCommand, NewCommand and DeleteCommand. My SaveCommand executes a save to a file operation, which I want to be an async operation so that the UI doesn't wait for it.

My SaveCommand is an instance of AsyncCommand, which implements ICommand.

 SaveCommand = new AsyncCommand(
  async param =>
        {
            Connection con = await Connection.GetInstanceAsync(m_configurationPath);
            con.Shoppe.Configurations = new List<CouchDbConfig>(m_configurations);
            await con.SaveConfigurationAsync(m_configurationPath);
            //now that its saved, we reload the Data.
            await LoadDataAsync(m_configurationPath);
        }, 
 ...etc

Now I'm building a test for my ViewModel. In it, I create a new thing with the NewCommand, I modify it and then use the SaveCommand.

vm.SaveCommand.Execute(null);
Assert.IsFalse(vm.SaveCommand.CanExecute(null));

My CanExecute method (not shown) of the SaveCommand should return False just after the item has been saved (there's no point saving an unchanged item). However, the Assert shown above fails all the time because I am not waiting for the SaveCommand to finish executing.

Now, I can't wait for it to finish executing because I can't. The ICommand.Execute doesn't return a Task. And if I change the AsyncCommand to have its Execute return a Task then it won't implement the ICommand interface properly.

So, the only thing I think I can do now, for testing purposes, is for the AsynCommand to have a new function:

public async Task ExecuteAsync(object param) { ... }

And thus, my test will run (and await) the ExecuteAsync function and the XAML UI will run the ICommand.Execute method in which it does not await.

I don't feel happy about doing my proposed solution method as I think, and hope, and wish that there is a better way.

Is what I suggest, reasonable? Is there a better way?

Igor Popov
  • 9,795
  • 7
  • 55
  • 68
Peter pete
  • 704
  • 1
  • 7
  • 17
  • Do you have any properties in your command that says "Running" or "Executing"? Maybe one way is to add an Executing flag to the command. Might also fix why the test fails, you can use the Executing flag in CanExecute to make sure they don't run the command twice. – Ron Beyer Apr 28 '15 at 12:58
  • I see what you mean. Let me try that out. *backsoon – Peter pete Apr 28 '15 at 13:01
  • Actually, AsyncCommand already has the Executing flag. When I initially found the problem, TBH, I wasn't using AsyncCommand, instead another thing which didn't. Maybe I don't actually have a problem now ! .. testing – Peter pete Apr 28 '15 at 13:03
  • @RonBeyer: The Assert in the Unit Test now passes, and my UI works too, thanks to the Executing flag. However, when the unit test ends I get a ThreadAbortException from my SaveCommand still saving the item to a file. Although, "not a real problem", I wonder if I can have it all end cleanly. – Peter pete Apr 28 '15 at 13:11
  • 1
    I'd just put a while(vm.SaveCommand.Executing); under the assert to verify that the operation finishes. It'll make that test run longer but will probably execute cleanly. – Ron Beyer Apr 28 '15 at 13:12
  • Ah! Great suggestion Ron. I like it. I like it indeed. Please replay them as an answer :) – Peter pete Apr 28 '15 at 13:23

3 Answers3

13

What you suggest is reasonable, and is exactly what the AsyncCommand implementation created by Stephen Cleary does (he is one of the foremost experts on the subject of async code IMHO)

Here is a full implementation of the code from the article (plus a few tweaks I made for the use case I was using.)

AsyncCommand.cs

/*
 * Based on the article: Patterns for Asynchronous MVVM Applications: Commands
 * http://msdn.microsoft.com/en-us/magazine/dn630647.aspx
 * 
 * Modified by Scott Chamberlain 11-19-2014
 * - Added parameter support 
 * - Added the ability to shut off the single invocation restriction.
 * - Made a non-generic version of the class that called the generic version with a <object> return type.
 */
using System;
using System.ComponentModel;
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;
using System.Windows.Input;

namespace Infrastructure
{
    public class AsyncCommand : AsyncCommand<object>
    {
        public AsyncCommand(Func<object, Task> command) 
            : base(async (parmater, token) => { await command(parmater); return null; }, null)
        {
        }

        public AsyncCommand(Func<object, Task> command, Func<object, bool> canExecute)
            : base(async (parmater, token) => { await command(parmater); return null; }, canExecute)
        {
        }

        public AsyncCommand(Func<object, CancellationToken, Task> command)
            : base(async (parmater, token) => { await command(parmater, token); return null; }, null)
        {
        }

        public AsyncCommand(Func<object, CancellationToken, Task> command, Func<object, bool> canExecute)
            : base(async (parmater, token) => { await command(parmater, token); return null; }, canExecute)
        {
        }
    }

    public class AsyncCommand<TResult> : AsyncCommandBase, INotifyPropertyChanged
    {
        private readonly Func<object, CancellationToken, Task<TResult>> _command;
        private readonly CancelAsyncCommand _cancelCommand;
        private readonly Func<object, bool> _canExecute;
        private NotifyTaskCompletion<TResult> _execution;
        private bool _allowMultipleInvocations;

        public AsyncCommand(Func<object, Task<TResult>> command)
            : this((parmater, token) => command(parmater), null)
        {
        }

        public AsyncCommand(Func<object, Task<TResult>> command, Func<object, bool> canExecute)
            : this((parmater, token) => command(parmater), canExecute)
        {
        }

        public AsyncCommand(Func<object, CancellationToken, Task<TResult>> command)
            : this(command, null)
        {
        }

        public AsyncCommand(Func<object, CancellationToken, Task<TResult>> command, Func<object, bool> canExecute)
        {
            _command = command;
            _canExecute = canExecute;
            _cancelCommand = new CancelAsyncCommand();
        }


        public override bool CanExecute(object parameter)
        {
            var canExecute = _canExecute == null || _canExecute(parameter);
            var executionComplete = (Execution == null || Execution.IsCompleted);

            return canExecute && (AllowMultipleInvocations || executionComplete);
        }

        public override async Task ExecuteAsync(object parameter)
        {
            _cancelCommand.NotifyCommandStarting();
            Execution = new NotifyTaskCompletion<TResult>(_command(parameter, _cancelCommand.Token));
            RaiseCanExecuteChanged();
            await Execution.TaskCompletion;
            _cancelCommand.NotifyCommandFinished();
            RaiseCanExecuteChanged();
        }

        public bool AllowMultipleInvocations
        {
            get { return _allowMultipleInvocations; }
            set
            {
                if (_allowMultipleInvocations == value)
                    return;

                _allowMultipleInvocations = value;
                OnPropertyChanged();
            }
        }

        public ICommand CancelCommand
        {
            get { return _cancelCommand; }
        }

        public NotifyTaskCompletion<TResult> Execution
        {
            get { return _execution; }
            private set
            {
                _execution = value;
                OnPropertyChanged();
            }
        }

        public event PropertyChangedEventHandler PropertyChanged;
        protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
        {
            PropertyChangedEventHandler handler = PropertyChanged;
            if (handler != null)
                handler(this, new PropertyChangedEventArgs(propertyName));
        }

        private sealed class CancelAsyncCommand : ICommand
        {
            private CancellationTokenSource _cts = new CancellationTokenSource();
            private bool _commandExecuting;

            public CancellationToken Token { get { return _cts.Token; } }

            public void NotifyCommandStarting()
            {
                _commandExecuting = true;
                if (!_cts.IsCancellationRequested)
                    return;
                _cts = new CancellationTokenSource();
                RaiseCanExecuteChanged();
            }

            public void NotifyCommandFinished()
            {
                _commandExecuting = false;
                RaiseCanExecuteChanged();
            }

            bool ICommand.CanExecute(object parameter)
            {
                return _commandExecuting && !_cts.IsCancellationRequested;
            }

            void ICommand.Execute(object parameter)
            {
                _cts.Cancel();
                RaiseCanExecuteChanged();
            }

            public event EventHandler CanExecuteChanged
            {
                add { CommandManager.RequerySuggested += value; }
                remove { CommandManager.RequerySuggested -= value; }
            }

            private void RaiseCanExecuteChanged()
            {
                CommandManager.InvalidateRequerySuggested();
            }
        }
    }
}

AsyncCommandBase.cs

/*
 * Based on the article: Patterns for Asynchronous MVVM Applications: Commands
 * http://msdn.microsoft.com/en-us/magazine/dn630647.aspx
 */
using System;
using System.Threading.Tasks;
using System.Windows.Input;

namespace Infrastructure
{
    public abstract class AsyncCommandBase : IAsyncCommand
    {
        public abstract bool CanExecute(object parameter);

        public abstract Task ExecuteAsync(object parameter);

        public async void Execute(object parameter)
        {
            await ExecuteAsync(parameter);
        }

        public event EventHandler CanExecuteChanged
        {
            add { CommandManager.RequerySuggested += value; }
            remove { CommandManager.RequerySuggested -= value; }
        }

        protected void RaiseCanExecuteChanged()
        {
            CommandManager.InvalidateRequerySuggested();
        }
    }
}

NotifyTaskCompletion.cs

/*
 * Based on the article: Patterns for Asynchronous MVVM Applications: Commands
 * http://msdn.microsoft.com/en-us/magazine/dn630647.aspx
 * 
 * Modifed by Scott Chamberlain on 12/03/2014
 * Split in to two classes, one that does not return a result and a 
 * derived class that does.
 */

using System;
using System.ComponentModel;
using System.Threading.Tasks;

namespace Infrastructure
{
    public sealed class NotifyTaskCompletion<TResult> : NotifyTaskCompletion
    {
        public NotifyTaskCompletion(Task<TResult> task)
            : base(task)
        {
        }

        public TResult Result
        {
            get
            {
                return (Task.Status == TaskStatus.RanToCompletion) ?
                    ((Task<TResult>)Task).Result : default(TResult);
            }
        }
    }

    public class NotifyTaskCompletion : INotifyPropertyChanged
    {
        public NotifyTaskCompletion(Task task)
        {
            Task = task;
            if (!task.IsCompleted)
                TaskCompletion = WatchTaskAsync(task);
            else
                TaskCompletion = Task;
        }

        private async Task WatchTaskAsync(Task task)
        {
            try
            {
                await task;
            }
            catch
            {
                //This catch is intentionally empty, the errors will be handled lower on the "task.IsFaulted" branch.
            }
            var propertyChanged = PropertyChanged;
            if (propertyChanged == null)
                return;
            propertyChanged(this, new PropertyChangedEventArgs("Status"));
            propertyChanged(this, new PropertyChangedEventArgs("IsCompleted"));
            propertyChanged(this, new PropertyChangedEventArgs("IsNotCompleted"));
            if (task.IsCanceled)
            {
                propertyChanged(this, new PropertyChangedEventArgs("IsCanceled"));
            }
            else if (task.IsFaulted)
            {
                propertyChanged(this, new PropertyChangedEventArgs("IsFaulted"));
                propertyChanged(this, new PropertyChangedEventArgs("Exception"));
                propertyChanged(this, new PropertyChangedEventArgs("InnerException"));
                propertyChanged(this, new PropertyChangedEventArgs("ErrorMessage"));
            }
            else
            {
                propertyChanged(this, new PropertyChangedEventArgs("IsSuccessfullyCompleted"));
                propertyChanged(this, new PropertyChangedEventArgs("Result"));
            }
        }

        public Task Task { get; private set; }
        public Task TaskCompletion { get; private set; }
        public TaskStatus Status { get { return Task.Status; } }
        public bool IsCompleted { get { return Task.IsCompleted; } }
        public bool IsNotCompleted { get { return !Task.IsCompleted; } }
        public bool IsSuccessfullyCompleted
        {
            get
            {
                return Task.Status ==
                    TaskStatus.RanToCompletion;
            }
        }
        public bool IsCanceled { get { return Task.IsCanceled; } }
        public bool IsFaulted { get { return Task.IsFaulted; } }
        public AggregateException Exception { get { return Task.Exception; } }
        public Exception InnerException
        {
            get
            {
                return (Exception == null) ?
                    null : Exception.InnerException;
            }
        }
        public string ErrorMessage
        {
            get
            {
                return (InnerException == null) ?
                    null : InnerException.Message;
            }
        }
        public event PropertyChangedEventHandler PropertyChanged;
    }
}
Community
  • 1
  • 1
Scott Chamberlain
  • 124,994
  • 33
  • 282
  • 431
  • Wowsers. That seems to do everything including the Kitchen sink! I agree that's a seemingly very complete implementation including cancelling, etc. Now, my question is, which answer do I mark correct? For my immediate needs, and probably all I will need for a while is the while loop in my test case. But I understand this answer is most complete and ultimately solves my immediate problem and probably all that i'll have in the future. – Peter pete Apr 28 '15 at 14:00
3

Looks like the answer is using a flag with the AsyncCommand object. Using the Executing flag of the AsyncCommand in the CanExecute method will make sure that the user cannot execute the command while another instance is running.

Also with your unit test, you can make it wait after the assert by using a while loop:

while (vm.SaveCommand.Executing) ;

So that the test exits cleanly.

Ron Beyer
  • 11,003
  • 1
  • 19
  • 37
  • So you mean this is cleaner than adding another method which returns a `Task`? – Sriram Sakthivel Apr 28 '15 at 13:31
  • Yes, if it means that the `Task` is only used for testing purposes. I would avoid writing additional code to pass a test, as it kind of defeats the purpose of a test. – Ron Beyer Apr 28 '15 at 13:32
  • I think it is cleaner. Since there aren't two confusing Execute methods. – Peter pete Apr 28 '15 at 13:32
  • @RonBeyer Your method is no better. You are also introducing a property just for testing. Is that good for you? Check [this article](https://msdn.microsoft.com/en-us/magazine/dn630647.aspx) stephen recommends to add another method which returns a Task which is nicely encapsulated in another interface. – Sriram Sakthivel Apr 28 '15 at 13:36
  • @SriramSakthivel I'm not creating a property, it already exists on the AsyncCommand object, take a look at the comments to the question. – Ron Beyer Apr 28 '15 at 13:38
  • @RonBeyer In the [link OP provided](http://enumeratethis.com/2012/06/14/asynchronous-commands-in-metro-wpf-silverlight/) it isn't. It is just a private field. Also note that you have to expose the `SaveCommand` property as `AsyncCommand` not just `ICommand`. So better to add another method IMHO. – Sriram Sakthivel Apr 28 '15 at 13:42
  • @SriramSakthivel: Yeah, it was a private field but I simply promoted it up. And thats true, I am exposing it AsyncCommand and not ICommand. But, even if there was an Execute() and ExecuteAsync methods around, I'd still need to expose AsyncCommand so that my unit test can see ExecuteAsync, and await on it. – Peter pete Apr 28 '15 at 13:44
  • I like the Executing/IsExecuting property solution and the loop, because I also need to add some code to force the dispatcher to pump, and I can do that in the loop. [http://stackoverflow.com/questions/1106881/using-the-wpf-dispatcher-in-unit-tests] – Peter pete Apr 28 '15 at 14:09
2

Review of the other answers

Doing while (vm.SaveCommand.Executing) ; seems like busy waiting and I prefer to avoid that.

The other solution using AsyncCommand from Stephen Cleary seems a bit overkill for such a simple task.

My proposed way doesn't break encapsulation - the Save method doesn't expose any internals. It just offers another way of accessing the same functionality.

My solution seems to cover everything that's needed in a simple and straightforward way.

Suggestion

I would suggest refactoring this code:

SaveCommand = new AsyncCommand(
    async param =>
    {
        Connection con = await Connection.GetInstanceAsync(m_configurationPath);
        con.Shoppe.Configurations = new List<CouchDbConfig>(m_configurations);
        await con.SaveConfigurationAsync(m_configurationPath);
        //now that its saved, we reload the Data.
        await LoadDataAsync(m_configurationPath);
    });

to:

SaveCommand = new RelayCommand(async param => await Save(param));

public async Task Save(object param)
{
    Connection con = await Connection.GetInstanceAsync(m_configurationPath);
    con.Shoppe.Configurations = new List<CouchDbConfig>(m_configurations);
    await con.SaveConfigurationAsync(m_configurationPath);
    //now that its saved, we reload the Data.
    await LoadDataAsync(m_configurationPath);
}

Just a note: I changed AsyncCommand to RelayCommand which can be found in any MVVM framework. It just receives an action as parameter and runs that when the ICommand.Execute method is called.

The unit tests

I made an example using the NUnit framework which has support for async tests:

[Test]
public async Task MyViewModelWithAsyncCommandsTest()
{
    // Arrange
    // do view model initialization here

    // Act
    await vm.Save(param);

    // Assert
    // verify that what what you expected actually happened
}

and in the view bind the command like you would do normally:

Command="{Binding SaveCommand}"
Igor Popov
  • 9,795
  • 7
  • 55
  • 68
  • I think the Stephen Cleary Way™ would be to use `IAsyncCommand` in your view model and then `await vm.SaveCommand.ExecuteAsync()` in your test. (Not all `AsyncCommand` implementations have this method (e.g., DevExpress does not), but [Cleary's does](http://dotnetapis.com/pkg/Nito.Mvvm.Async/1.0.0-pre-03/netstandard2.0/doc/Nito.Mvvm.IAsyncCommand).) – metal Nov 14 '18 at 21:51
  • 1
    Just a minor addition: I would mark the Save method as 'internal' and add [assembly: InternalsVisibleTo("YourAssembly.Tests")] in AssemblyInfo.cs of YourAssembly (where Save method exists). I'm not a big fan of having things 'public' just for the sake of unit testing. – thomasgalliker Jul 16 '20 at 08:39