Proposal: Make AsyncTaskCodeActivity derive from AsyncCodeActivity instead of TaskCodeActivity<object>
fubar-coder opened this issue · 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);
}
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!
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.