Async Check for Domain Fullname With WPF and Caliburn.Micro

Question

I have a small WPF program built using Caliburn.Micro which checks Active Directory for a user with the current username of the logged-on user. I have a Bootstrapper method:

protected async override void OnStartup(object sender, StartupEventArgs e)
{
    await ShellViewModel.CollectFullnameAsync();
    DisplayRootViewFor<ShellViewModel>();
}

This starts the ShellViewModel and kicks off an async method to start looking for the currently logged on user on the domain

My ShellViewModel looks like this:

public ShellViewModel()
{
    Computername = EnvironmentHelpers.ComputerName;
    Fullname = _fullnameplaceholder;
}

public static async Task CollectFullnameAsync()
{
    _fullnameplaceholder = ADHelpers.GetFullADName();

    while(_fullnameplaceholder == "Failed" && _attemptsToCollectFullName < 5)
    {
        _attemptsToCollectFullName++;
        await Task.Delay(2000);
        _fullnameplaceholder = ADHelpers.GetFullADName();
    }
}

So my code is meant to:

1. Look for the user on the domain
2. Check 5 times for the user, waiting 2 seconds between each attempt.
3. Display the ShellView with either "Failed" or the Fullname returned from the domain

Can I please get some insight into how I am performing this task? Is this the correct way to do this or am I barking up the wrong tree?

Answer 1

I'll start off by saying I don't have Caliburn.Micro experience but do have other MVVM experience.

I'm going to assume the ShellView is the shell for your entire application. I don't know what other code that is running but I can tell you as a user I would not want to fire off your application and wait for over 10 seconds, because we are waiting for 10 seconds plus the time it takes for the other calls to come back, to be able to use the application. Worst yet is even if the screen hasn't shown. Then I'm likely to fire it off multiple times thinking it stuck or I didn't double click it.

In general it's not best practice to do async void, so I would question why Task.Delay instead of Thread.Sleep since the delay is the only async code.

If we are leaving it as async it should be wrapped in a try/catch otherwise if there is an exception you will need to handle it in Application.DispatcherUnhandledException or AppDomain.UnhandledException. Which brings up a point if there is an exception I assume you want just Failed as the user name?

Now to a different idea. You could remove this from startup, or try once to get it in startup, Then setup a timer to recheck if it failed. The application state could be based on if it has a name yet or not. For Example: The user can start entering data but can't save it until we retrieved the AD user.

If just want to wait until there is user name or not it still better to give the user an idea what's going on so they can see the application isn't just hung and that's it's checking network activity. Create an initial view of just gather application information and part of that is gathering network user information.

Answer 2

In general what you are doing is polling a service to ask for a result or to ask if the result is ready. This is considered bad practice. Asking is always considered bad practice. Polling will always waste valuable resources for nothing.

Ancient polling can always be replaced by a modern event driven or asynchronous approach.

In your case, you simply have to convert your service ADHelpers to a full asynchronous service.
The simplest would be if you are using an Active Directory API that supports asynchronous operations. From your attempt I can see, that you are trying to make the service asynchronous, therfore I assume, that the API doesn't support asynchronous calls. So, we have to make asynchrouns ourself.

The key is to add events to the ADHelpers to notify a listener when the result is ready to consume. With the helpof TaskCompletionSource we can convert the event driven pattern to an asynchronous pattern.

The following code shows how to implement an asynchronous service to replace polling. The new class also supports optional cancellation and an optional timeout:

ADHelpers.cs

class ADHelpers : IDisposable
{
  public ADHelpers()
  {
    // Initilaize the timeout timer, but don't start it (set interval to Timeout.Infinite)
    this.TmeoutTimer = new System.Threading.Timer(ExecuteTimeout, null, Timeout.Infinite , Timeout.Infinite);
  }

  // Method supports timeout which will be disabled by default (TimeSpan.Zero)
  public async Task<Fullname> GetFullAdNameAsync(TimeSpan timeoutDuration = TimeSpan.Zero)
  {
    this.FullNameReady += CompleteAsyncOperation;
    this.taskCompletionSource = new TaskCompletionSource<FullName>(TaskCreationOptions.LongRunning); 

    // Only enable the timeout timer when enabled via the parameters
    if (timeoutDuration != TimeSpan.Zero)
    {
      this.TmeoutTimer.Change(TimeSpan.Zero, timeoutDuration);
    }

    GetFullAdName();
    return this.taskCompletionSource.Task;
  }

  // Overload that additionally supports cancellation (and timeout)
  public async Task<Fullname> GetFullAdNameAsync(CancellatioToken cancellationToken, TimeSpan timeoutDuration = TimeSpan.Zero)
  {
    this.CancellationToken = cancellationToken;
    return await GetFullAdNameAsync(timeoutDuration);
  }

  private void GetFullAdName()
  {
    // The long running operation
    FullName fullAdName = QueryAd();

    // Complete the asynchronous operation
    OnFullNameReady(fullAdName);    
  }

  // To support cancellation, periodically invoke CancellationToken.ThrowIfCancellationRequested()
  // as often as possible to give the operation chances to cancel    
  private FullName QueryAd()
  {
    try
    {
      this.CancellationToken?.ThrowIfCancellationRequested();

      FullName result;

      // Do something

      this.CancellationToken?.ThrowIfCancellationRequested();

      // Do something more

      this.CancellationToken?.ThrowIfCancellationRequested();  

      return result;  
    }
    finally
    {      
      // Always stop the timer, 
      // even when exception was thrown or the task was cancelled.
      this.TmeoutTimer.Change(Timeout.Infinite , Timeout.Infinite);
    }
  }

  private void CompleteAsyncOperation(FullNameReadyEventArgs args)
  {
    // Return the result to the awaiting caller. 
    // Ends the asynchronous call by setting Task.Status to TaskStatus.RanToCompletion
    this.taskCompletionSource.TrySetResult(args.FullAdName);
  }

  private void ExecuteTimeout(object stateInfo)
  {
    // Ends the asynchronous call by setting Task.Status to TaskStatus.Canceled
    this.taskCompletionSource.TrySetCanceled();
  }

  #region Implementation of IDisposable

  // Flag: Has Dispose already been called?
  bool disposed = false;

  // Instantiate a SafeHandle instance.
  SafeHandle handle = new SafeFileHandle(IntPtr.Zero, true);

  // Public implementation of Dispose pattern callable by consumers.
  public void Dispose()
  { 
    Dispose(true);
    GC.SuppressFinalize(this);           
  }

  // Protected implementation of Dispose pattern.
  protected virtual void Dispose(bool disposing)
  {
    if (disposed)
      return; 

    if (disposing) 
    {
      handle.Dispose();

      // Free any other managed objects here.
      this.TimeoutTimer?.Dispose();
    }

    disposed = true;
  }

  #endregion // IDisposable implementaion

  private TaskCompletionSource<FullName> taskCompletionSource { get; set; }
  private CancellationToken CancellationToken { get; set; }
  private System.Threading.Timer TimeoutTimer { get; set; }

  private event EventHandler<FullNameReadyEventArgs> FullNameReady;
  protected virtual void OnFullNameReady(Fullname fullADName) 
  {
    var eventArgs = new FullNameReadyEventArgs(fullADName);
    this.FullNameReady?.Invoke(eventArgs);
  }
}

Usage Example

// ADHelpers is now a IDisposable (because of the internal System.ThreadingTimer)
// Make sure that the instance is disposed properly. using-statement is recommended.
using (var adHelpers = new ADHelpers())
{
  // Optional (if cancellation is required). 
  // Use 'cancellationTokenSource.Cancel()' to abort the asynchronous operation.
  var cancellationTokenSource = new CancellationTokenSource();

  // Optional: use a 10s timeout (the timeout is disabled by default)
  var timeoutDuration = TimeSpan.FromSeconds(10);

  FullName fullAdName = await adHelpers.GetFullAdNameAsync(cancellationTokenSource.Token, timeoutDuration);
}

---

Remarks
The code is not tested, but should compile. If not, please tell me so that I can fix the example.