6
\$\begingroup\$

I've just started working with Workflow (WF4) and have been playing with an idea of using it in MVC3.0 controller actions to see if it improves the maintainability of complex actions; also potentially giving the ability to make multiple DB calls in parallel to populate the output model. Along with this I've been looking at Async controller actions but I'm not sure I've really got my head around those. I know you can load workflow into a WorkflowApplication and call BeginRun(...) to get that to run asynchronously but I'm thinking that's the wrong approach; from looking online I've come up with the following implementation which does run as expected but I'm wondering if there's anything wrong with the code below or it will cause other issues that I've not thought about.

    [HttpPost]
    public void ConfigureAsync(ConfigureExportInputModel inputModel)
    {
        if (inputModel == null)
        {
            throw new ArgumentNullException("inputModel");
        }

        var arguments = new Dictionary<string, object> { { "ExportType", inputModel.ExportType } };
        var wf = new ErrorHandledExportWorkflow();

        AsyncManager.OutstandingOperations.Increment();
        ThreadPool.QueueUserWorkItem(o =>
                {
                    AsyncManager.Parameters["result"] = WorkflowInvoker.Invoke(wf, arguments)["MvcOutput"] as IDefineMvcOutput;
                    AsyncManager.OutstandingOperations.Decrement();
                });
    }

    public ActionResult ConfigureCompleted(IDefineMvcOutput result)
    {
        return this.ActionResultFactory.Create(this, result);
    }
\$\endgroup\$
  • 2
    \$\begingroup\$ Just FYI - C# 4 introduced the Code Contracts library so you can do Contract.Requires(inputModel != null) which is more semantic and has interesting AOP possibilities. For example you can have all these checks fire in development and testing and disabled to boost performance in production. There is also a VS extension that will attempt to do some basic analysis on the code contracts while your coding and point out gaps in logic. \$\endgroup\$ – George Mauer Feb 7 '12 at 16:28
  • \$\begingroup\$ Thanks for the comment, I've seen code contracts and taken a quick look but not used them in any real code yet - they do sound really nice. I keep meaning to see if there's a way to unit test that a method implements a particular contract. I think you can put them on as attributes so it would be possible to do it that way; reflect over the method to confirm it has the expected contract(s) in place. \$\endgroup\$ – Paul Hadfield Feb 7 '12 at 18:54
  • \$\begingroup\$ I think code contracts are complementary to unit tests, they don't need to and shouldn't be included by them. From what I understand they are inspired by projects like Spec# so they aim to eventually be an extension of the type system. As far as using them goes though, it takes like 5 seconds to learn, I very much recommend it. \$\endgroup\$ – George Mauer Feb 7 '12 at 19:24
5
\$\begingroup\$

I cannot believe this went unanswered for 5 and a half years (I guess I can, it is a difficult question to answer) - I'm going to try to answer it from the respect of early 2012, and the respect of today (mid 2017).


2012

The biggest concern I see comes from this line:

AsyncManager.Parameters["result"] = WorkflowInvoker.Invoke(wf, arguments)["MvcOutput"] as IDefineMvcOutput;

I don't know how WorkflowInvoker works, but I assume that it has the capability of throwing an exception, which means that your AsyncManager.OutstandingOperations.Decrement(); line would not be reached, and I wonder what other issues that might cause for your application. (Looking for operations that aren't there, for example.) I would consider a try/finally block, or decrement before your operation gets invoked. (You can probably consider "In Progress" as a non-outstanding operation.)

The other issue I see with your Lambda method is regarding the idea of "closures", and captured variables. In .NET the wf and arguments variables are still part of the local method and are simply referring to the local copy, which means if you modify either variable after queuing up the worker, you could end up with a different result. This may not be an issue with your infrastructure, but I feel it's worth pointing out none-the-less.


2017

Let's fast-forward five years and six months (which is an agonizingly long time) and examine what language features might make this a different process. I'm just going to post the form that takes advantage of the new language features, then we'll discuss where async/await might help you.

[HttpPost]
public void ConfigureAsync(ConfigureExportInputModel inputModel)
{
    if (inputModel == null)
    {
        throw new ArgumentNullException("inputModel");
    }

    var arguments = new Dictionary<string, object> { [nameof(inputModel.ExportType)] = inputModel.ExportType };
    var wf = new ErrorHandledExportWorkflow();

    AsyncManager.OutstandingOperations.Increment();
    ThreadPool.QueueUserWorkItem(o =>
            {
                AsyncManager.Parameters["result"] = WorkflowInvoker.Invoke(wf, arguments)["MvcOutput"] as IDefineMvcOutput;
                AsyncManager.OutstandingOperations.Decrement();
            });
}

public ActionResult ConfigureCompleted(IDefineMvcOutput result) =>
    this.ActionResultFactory.Create(this, result);

So it doesn't look much different, but you can see that it's just a bit shorter. The expression-bodied method syntax, the dictionary initializer, and the nameof operator. But what really makes a difference is the async/await of the TPL. This starts making our code come alive.

With the proper implementation, you can leverage async/await to really bring a good flow to the asynchronous nature of your code. If properly implemented on the WorkflowInvoker.Invoke method, you might get away with something like:

[HttpPost]
public Task<ActionResult> RunAsync(ConfigureExportInputModel inputModel)
{
    if (inputModel == null)
    {
        throw new ArgumentNullException("inputModel");
    }

    var arguments = new Dictionary<string, object> { [nameof(inputModel.ExportType)] = inputModel.ExportType };
    var wf = new ErrorHandledExportWorkflow();

    var result = await WorkflowInvoker.InvokeAsync(wf, arguments);
    return this.ActionResultFactory.Create(this, result["MvcOutput"] as IDefineMvcOutput);
}

This would allow you to await the whole method, and return the thread to the pool until done. By leveraging what .NET built-in with C#5.0, you can write succinct asynchronous code that doesn't have to worry about the overhead of performing the async operations. The framework and language really take that burden away from you.

Of course, this expects an InvokeAsync method on the WorkflowInvoker, which may or may not exist, and as such this is pseudo-/hypothetical code, but the idea should remain the same. You can read the MSDN article for more explanation of what is possible with this design.

\$\endgroup\$
  • \$\begingroup\$ Thank you for your detailed response (and yes, it is a blast from the past question!). \$\endgroup\$ – Paul Hadfield Aug 17 '17 at 18:55
  • 2
    \$\begingroup\$ @PaulHadfield I'm actually pleasantly surprised you're still around. :) \$\endgroup\$ – 410_Gone Aug 17 '17 at 19:02
  • \$\begingroup\$ Three thoughts: 1) Did you want to quote the first snippet which is OP code and not the second one which is yours? 2) Didn't you forget the async keyword on the method signature in your implementation? 3) Wouldn't we now return BadRequest() instead of throwing an ArgumentNullException? \$\endgroup\$ – t3chb0t Aug 18 '17 at 4:29
  • \$\begingroup\$ @t3chb0t 1) No, the first block is my code, the second is pseudo-C# code, so I quoted it to mean it's not usable. 2) Yes, I forgot that async modifier - part of th reason for the quote. ;) 3) Yes, and again, part of the reason for pseudo- quote. \$\endgroup\$ – 410_Gone Aug 18 '17 at 13:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.