CoreWF icon indicating copy to clipboard operation
CoreWF copied to clipboard

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

Open fubar-coder opened this issue 3 years ago • 2 comments

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);
}

fubar-coder avatar Feb 06 '22 17:02 fubar-coder

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!

dmetzgar avatar Feb 23 '22 06:02 dmetzgar