Skip to content

Conversation

@cgillum
Copy link
Member

@cgillum cgillum commented Dec 22, 2025

Summary

What changed?

Added new overloads for func-based activity and orchestrator registration.

For example, instead of doing this all the time:

static string SendEmail(TaskActivityContext context, string message)
{
    // ...
}

registry.AddActivityFunc<string, string>(nameof(SendEmail), SendEmail);

I can now do this:

[DurableTask(nameof(SendEmail))] // <-- optional
static string SendEmail(TaskActivityContext context, string message)
{
    // ...
}

registry.AddActivityFunc<string, string>(SendEmail);

Why is this change needed?

The current methods are verbose and require writing repetitive code. I found it quite annoying when creating samples.

Issues / work items

There are no work items associated with this PR

Project checklist

  • Release notes are not required for the next release
    • Otherwise: Notes added to release_notes.md
  • Backport is not required
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • All required tests have been added/updated (unit tests, E2E tests)
  • Breaking change? (no)
    • If yes:
      • Impact:
      • Migration guidance:

AI-assisted code disclosure (required)

Was an AI tool used? (select one)

  • No
  • Yes, AI helped write parts of this PR (e.g., GitHub Copilot)
  • Yes, an AI agent generated most of this PR

If AI was used:

  • Tool(s): GitHub Copilot /w Claude Opus 4.5
  • AI-assisted areas/files: I don't know what this means?
  • What you changed after AI output: Nothing that I can specifically remember

AI verification (required if AI was used):

  • I understand the code and can explain it
  • I verified referenced APIs/types exist and are correct
  • I reviewed edge cases/failure paths (timeouts, retries, cancellation, exceptions)
  • I reviewed concurrency/async behavior
  • I checked for unintended breaking or behavior changes

Testing

Automated tests

  • Result: Passed

Manual validation (only if runtime/behavior changed)

N/A


Notes for reviewers

  • N/A

@cgillum cgillum requested review from halspang and jviau December 22, 2025 22:03
@cgillum cgillum self-assigned this Dec 22, 2025
Copilot AI review requested due to automatic review settings December 22, 2025 22:03
DurableTaskAttribute? attribute = method.GetCustomAttribute<DurableTaskAttribute>();
if (attribute?.Name.Name is not null and not "")
{
return attribute.Name;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really see how it's possible for attribute to ever be null here, assuming it's not shared state that can be accessed (and mutated) by multiple threads concurrently (something we're not doing).

DurableTaskAttribute? attribute = method.GetCustomAttribute<DurableTaskAttribute>();
if (attribute?.Name.Name is not null and not "")
{
return attribute.Name;
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds simplified overloads for func-based activity and orchestrator registration, allowing developers to omit the explicit name parameter. The name is automatically inferred from either a [DurableTask] attribute on the method or the method name itself, reducing boilerplate when registering tasks.

Key changes:

  • Added 8 new overloads each for AddOrchestratorFunc and AddActivityFunc that infer task names from delegates
  • Extended DurableTaskAttribute to support method-level application for custom naming
  • Comprehensive test coverage for all new overload variations including lambda detection

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/Abstractions/DurableTaskRegistry.Orchestrators.cs Added 8 new AddOrchestratorFunc overloads with name inference and GetTaskNameFromDelegate helper method
src/Abstractions/DurableTaskRegistry.Activities.cs Added 8 new AddActivityFunc overloads with name inference and GetActivityNameFromDelegate helper method
src/Abstractions/DurableTaskAttribute.cs Extended AttributeUsage to support methods and updated documentation to reflect method usage
test/Worker/Core.Tests/DurableTaskRegistryTests.Orchestrators.cs Added 10 new test cases covering all orchestrator overload variations and error conditions
test/Worker/Core.Tests/DurableTaskRegistryTests.Activities.cs Added 10 new test cases covering all activity overload variations and error conditions
src/Abstractions/Entities/TaskEntity.cs Added string type validation for entity state (unrelated to PR purpose)
misc/misc.csproj Added Directory.Packages.* to project includes (unrelated to PR purpose)

Comment on lines +151 to +156
// Throw if TState is a string, since strings are immutable and don't support dynamic creation of new instances.
if (typeof(TState) == typeof(string))
{
throw new InvalidOperationException("Entity state cannot be a string. Use a class or struct instead.");
}

Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This validation for string entity state appears to be unrelated to the PR's stated purpose of adding simplified func-based activity and orchestrator registration. This change should be removed or moved to a separate PR focused on entity state validation.

Suggested change
// Throw if TState is a string, since strings are immutable and don't support dynamic creation of new instances.
if (typeof(TState) == typeof(string))
{
throw new InvalidOperationException("Entity state cannot be a string. Use a class or struct instead.");
}

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something I ran into when doing some unrelated prototyping and wanted to create a fix while it was on the top of my mind. I could make it into a separate PR, but not sure if it's really worth it.

Comment on lines +379 to +401
static TaskName GetTaskNameFromDelegate(Delegate @delegate)
{
MethodInfo method = @delegate.Method;

// Check for DurableTaskAttribute on the method
DurableTaskAttribute? attribute = method.GetCustomAttribute<DurableTaskAttribute>();
if (attribute?.Name.Name is not null and not "")
{
return attribute.Name;
}

// Fall back to method name
string? methodName = method.Name;
if (string.IsNullOrEmpty(methodName) || methodName.StartsWith("<", StringComparison.Ordinal))
{
throw new ArgumentException(
"Cannot infer orchestrator name from the delegate. The delegate must either have a " +
"[DurableTask] attribute with a name, or be a named method (not a lambda or anonymous delegate).",
nameof(@delegate));
}

return new TaskName(methodName);
}
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GetTaskNameFromDelegate and GetActivityNameFromDelegate methods have identical implementations. Consider extracting this logic into a shared private static method in a common location to reduce code duplication and improve maintainability.

Copilot uses AI. Check for mistakes.
/// <see cref="TaskOrchestrator{TInput, TOutput}"/>, <see cref="TaskActivity{TInput, TOutput}"/>,
/// or TaskEntity{TState} from the Microsoft.DurableTask.Entities namespace.
/// It can also be applied to methods used with <see cref="DurableTaskRegistry.AddOrchestratorFunc{TInput, TOutput}(System.Func{TaskOrchestrationContext, TInput, System.Threading.Tasks.Task{TOutput}})"/>
/// or similar overloads to specify a custom name for the orchestrator.
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation mentions "orchestrator" specifically but this attribute can now also be applied to activity methods. The documentation should mention both orchestrators and activities to be more accurate and helpful.

Suggested change
/// or similar overloads to specify a custom name for the orchestrator.
/// or similar overloads to specify a custom name for the orchestrator or activity.

Copilot uses AI. Check for mistakes.
<None Include="$(RepoRoot).gitignore" />
<None Include="$(RepoRoot)azure-pipelines-release.yml" />
<None Include="$(RepoRoot)Directory.Build.*" />
<None Include="$(RepoRoot)Directory.Packages.*" />
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change to include Directory.Packages.* appears unrelated to the PR's stated purpose of adding simplified func-based activity and orchestrator registration. This should be removed or moved to a separate PR if it's intended to be a general maintenance improvement.

Suggested change
<None Include="$(RepoRoot)Directory.Packages.*" />

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to keep this in here since it's not worth creating a separate PR for.

@cgillum cgillum requested a review from YunchuWang December 22, 2025 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants