Repository navigation
Proposal: Make AsyncTaskCodeActivity derive from AsyncCodeActivity instead of TaskCodeActivity<object> #186
Description
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);
}Reacted by TheBrambleShark
Activity
dmetzgar commentedon Feb 23, 2022
I'm also a bit confused on why there is an
AsyncTaskCodeActivity<TResult>and aTaskCodeActivity<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 fromAsyncCodeActivity<TResult>I don't see a difference between the two classes such that I could explain when to use one or the other. Thenewon 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!TheBrambleShark commentedon Mar 21, 2023
Hi all, perhaps I can provide a bit of history here. I'm the author of the
AsyncTaskCodeActivity<TResult>type.AsyncCodeActivityandAsyncCodeActivity<TResult>are types ported from theSystem.Activitiesnamespace, effectively CoreWF but when it was maintained by Microsoft for NetFx. These types use the oldBegin/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
TaskCodeActivitytype 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.