Proposal: Make AsyncTaskCodeActivity derive from AsyncCodeActivity instead of TaskCodeActivity<object>

[fubar-coder]

The AsyncTaskCodeActivity should not have a Result property. This causes problems in some scenarios with reflection and validation.

There are two ways to solve thje problem:

Create a TaskCodeActivity as a companion for TaskCodeActivity`1 or let the AsyncTaskCodeActivity derive directly from AsyncCodeActivity.

Here's an example implementation, which uses AsyncCodeActivity directly, as the TaskCodeActivity`1 implementation looks unnecessary even though there is a little bit of code duplication:

/// <summary>
/// Activity that executes a <see cref="Task"/> which allows the usage of async/await.
/// </summary>
public abstract class AsyncTaskCodeActivity : AsyncCodeActivity
{
    /// <inheritdoc />
    protected sealed override IAsyncResult BeginExecute(
        AsyncCodeActivityContext context,
        AsyncCallback callback,
        object state)
    {
        var cancellationTokenSource = new CancellationTokenSource();
        context.UserState = cancellationTokenSource;
        return ApmAsyncFactory.ToBegin(
            ExecuteAsync(context, cancellationTokenSource.Token),
            callback,
            state);
    }

    /// <inheritdoc />
    protected sealed override void EndExecute(
        AsyncCodeActivityContext context,
        IAsyncResult result)
    {
        ((CancellationTokenSource)context.UserState).Dispose();
    }

    /// <inheritdoc />
    protected sealed override void Cancel(AsyncCodeActivityContext context) =>
        ((CancellationTokenSource)context.UserState).Cancel();

    /// <summary>
    /// Called to execute this activity.
    /// </summary>
    /// <param name="context">The context.</param>
    /// <param name="cancellationToken">The <see cref="CancellationToken"/>.</param>
    /// <returns>The task.</returns>
    /// <remarks>
    /// The <see cref="AsyncCodeActivityContext.UserState"/> must not be used/altered.
    /// </remarks>
    protected abstract Task ExecuteAsync(
        AsyncCodeActivityContext context,
        CancellationToken cancellationToken);
}

[dmetzgar]

I'm also a bit confused on why there is an AsyncTaskCodeActivity<TResult> and a TaskCodeActivity<TResult>. The latter implies that I could have a synchronous activity that executes a Task or perhaps an activity that starts a Task without waiting for completion. But since everything inherits from AsyncCodeActivity<TResult> I don't see a difference between the two classes such that I could explain when to use one or the other. The new on the Result property that changes it from an OutArgument to an object that is always null would be a bit strange for reflection. I'm curious how that affects CacheMetadata. A PR would be appreciated here. Thanks!

[Foxtrek64]

Hi all, perhaps I can provide a bit of history here. I'm the author of the AsyncTaskCodeActivity<TResult> type.

AsyncCodeActivity and AsyncCodeActivity<TResult> are types ported from the System.Activities namespace, effectively CoreWF but when it was maintained by Microsoft for NetFx. These types use the old Begin/EndExecute() methods.

My original proposal had both the generic and non-generic AsyncTaskCodeActivities implementing their respective AsyncCodeActivity and translating the old asynchronous methods into TPL-style.

@lbargaoanu had proposed that, in order to help facilitate code deduplication, the TaskCodeActivity type would be implemented. They also added a reference to Nito.AsyncEx to handle the TPL implementation.

---

If this TaskCodeActivity base class is causing issues for consumers, I'm perfectly fine deleting it. This should in theory be a non-breaking change since the AsyncCodeActivity type is an internal implementation, but we will need to be careful to ensure the transition does not affect public-facing code adversely.