Add multi-version support to [DurableTask] annotations - #751
Conversation
There was a problem hiding this comment.
Pull request overview
Adds multi-version support to [DurableTask] version annotations across reflection-based registration and the source generator, and updates worker work-item filter generation to respect CurrentOrOlder versioning semantics.
Changes:
- Parse comma-separated
DurableTaskAttribute.Versionvalues into a distinct ordered version set and register tasks under each declared version. - Update the source generator to emit multi-version call helpers that require a
versionparameter with runtime validation. - Narrow auto-generated worker work-item filters to versions
<= workerVersionwhenVersionMatchStrategy.CurrentOrOlderis enabled, with expanded test coverage.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| CHANGELOG.md | Documents the new multi-version attribute support and filter narrowing behavior. |
| src/Abstractions/DurableTaskAttribute.cs | Updates XML docs to describe comma-separated multi-version semantics and validation rules. |
| src/Abstractions/TypeExtensions.cs | Adds reflection-side parsing for multi-version attributes (GetDurableTaskVersions) and preserves GetDurableTaskVersion semantics. |
| src/Abstractions/DurableTaskRegistry.Orchestrators.cs | Fans out orchestrator type/singleton registrations to all declared versions. |
| src/Abstractions/DurableTaskRegistry.Activities.cs | Fans out activity type/singleton registrations to all declared versions. |
| src/Abstractions/DurableTaskRegistry.cs | Adds shared “register across all versions” helper for activities. |
| src/Generators/DurableTaskSourceGenerator.cs | Implements multi-version parsing, deduplication, collision detection, and version-parameter helper generation with runtime guards. |
| src/Worker/Core/DurableTaskWorkerWorkItemFilters.cs | Narrows generated versions for CurrentOrOlder by comparing against worker version. |
| test/Abstractions.Tests/DurableTaskAttributeVersionTests.cs | Adds tests for multi-version parsing, trimming/dedup, unversioned behavior, and whitespace-only entry rejection. |
| test/Abstractions.Tests/DurableTaskRegistryVersioningTests.cs | Adds tests ensuring multi-version types register under each version and collisions are rejected. |
| test/Generators.Tests/VersionedOrchestratorTests.cs | Adds generator snapshot test for multi-version orchestrators producing version-parameter helpers and guards. |
| test/Generators.Tests/VersionedActivityTests.cs | Adds generator snapshot test for multi-version activities producing version-parameter helpers and guards. |
| test/Worker/Core.Tests/DependencyInjection/UseWorkItemFiltersTests.cs | Adds CurrentOrOlder filter tests that validate dropping newer versions and retaining unversioned + older versions. |
| test/Worker/Core.Tests/DurableTaskFactoryVersioningTests.cs | Adds factory resolution test covering coexistence of unversioned and multi-version activity registrations. |
Comments suppressed due to low confidence (1)
src/Worker/Core/DurableTaskWorkerWorkItemFilters.cs:118
- Under MatchStrategy.CurrentOrOlder, workerVersion filtering can remove all versioned registrations for a name, leaving only the unversioned "" entry. The current
normalized.Length == 1 && normalized[0].Length == 0check then returns an empty Versions list (wildcard), which widens the backend filter and can cause newer versioned work items to be streamed (and then rejected) even though the registry originally had versioned registrations (which disable unversioned fallback). Consider emitting the wildcard only when the original registrations for the name were truly unversioned-only (no non-empty versions) before filtering; otherwise return[""]to restrict to unversioned only.
// Unversioned-only: emit the wildcard match-all (empty list) so the backend can deliver
// versioned work items that the factory will resolve via unversioned fallback. Without
// this, callers asking for a specific version would be filtered out at the backend even
// though the worker can handle them.
if (normalized.Length == 1 && normalized[0].Length == 0)
{
return [];
}
| # Changelog | ||
|
|
||
| ## Unreleased | ||
| - Support declaring multiple versions in a single `[DurableTask]` attribute via a comma-separated list (for example `[DurableTask("MyActivity", Version = "1.0.0,1.1.0")]`); each version is registered and plumbed through code generation. Narrow auto-generated work item filters to current-or-older versions under the `CurrentOrOlder` match strategy. |
d324252 to
5c4eda7
Compare
The Version property now accepts a comma-separated list (e.g. "v1,v2"), registering the type under each declared version and emitting call helpers that take a required 'version' parameter validated against the declared set. Single-version and unversioned behavior is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
5c4eda7 to
dd42561
Compare
| static IReadOnlyList<string> GetFilterVersions(IEnumerable<string?> versions, string? workerVersion) | ||
| { | ||
| // Normalize null to "" so an unversioned registration appears consistently. | ||
| string[] normalized = versions | ||
| .Select(version => version ?? string.Empty) | ||
| .Distinct(StringComparer.OrdinalIgnoreCase) | ||
| .OrderBy(version => version, StringComparer.OrdinalIgnoreCase) | ||
| .Where(version => workerVersion == null || TaskOrchestrationVersioningUtils.CompareVersions(workerVersion, version) >= 0) | ||
| .ToArray(); | ||
|
|
berndverst
left a comment
berndverst
left a comment
There was a problem hiding this comment.
Requesting changes. The current head has six blocking issues:
- The existing unresolved comma-semantics thread is a source and behavioral/replay break: a value such as
"v1,v2"used to identify one version, but now registers two versions and changes generated helper signatures. This needs explicit breaking-change approval and migration guidance, or an opt-in representation that preserves legacy values. - Single-version values are now trimmed, changing existing version identity and potentially preventing in-flight instances from resolving after upgrade.
- The existing unresolved
CurrentOrOlderfilter thread is also a material latency/throughput issue: a post-filter empty list is interpreted as a wildcard, so the backend can stream work that the worker will reject. - Multi-version registration is not atomic when a later version collides.
- Generated version validation is case-sensitive even though all runtime version identity/comparison paths are case-insensitive.
- Unescaped version text can produce invalid generated C#.
Compatibility verdict: no binary, protobuf, or JSON-format break was found, but there are definite source and behavioral/replay breaks in the shipped Abstractions and generated consumer API surfaces.
Performance verdict: parsing and registry fan-out are configuration-time operations and otherwise look reasonable. The wildcard filter behavior remains a real backend over-fetch and worker-rejection risk.
The PR also currently conflicts with main in the generator and changelog. When rebasing, preserve the nullable-input generator fix from #763 and revalidate the resulting diff.
| continue; | ||
| } | ||
|
|
||
| string trimmed = segment.Trim(); |
There was a problem hiding this comment.
This trims a single version value that was previously preserved verbatim. For example, [DurableTask(Version = " v1 ")] used to register and stamp " v1 "; after an upgrade this registers as "v1", so existing in-flight instances can no longer resolve to the task. Please preserve the exact value when no comma is present and apply normalization only to the new list syntax. The source-generator parser needs the same compatibility behavior.
| { | ||
| foreach (TaskVersion version in versions) | ||
| { | ||
| this.AddActivity(name, version, factory); |
There was a problem hiding this comment.
This fan-out mutates the registry as it goes. If v2 is already registered and the attribute declares v1,v2, v1 is added before v2 throws, so a failed AddActivity call still changes dispatch state. Please preflight every key before adding any of them; the same atomicity fix is needed in AddOrchestratorAllVersions.
| static string BuildDeclaredVersionGuard(DurableTaskTypeInfo task, string callKind) | ||
| { | ||
| string condition = string.Join( | ||
| " || ", task.TaskVersions.Select(v => $"version == {ToCSharpStringLiteral(v)}")); |
There was a problem hiding this comment.
These generated comparisons are case-sensitive, while TaskVersion.Equals, TaskVersionKey, parser deduplication, and CompareVersions all use case-insensitive semantics. The helper therefore rejects "V1" even though the runtime accepts it for a declared "v1". Please emit string.Equals(..., StringComparison.OrdinalIgnoreCase) checks or canonicalize to the declared value.
| { | ||
| string condition = string.Join( | ||
| " || ", task.TaskVersions.Select(v => $"version == {ToCSharpStringLiteral(v)}")); | ||
| string declaredVersions = string.Join(", ", task.TaskVersions); |
There was a problem hiding this comment.
declaredVersions is later inserted raw into the generated exception-message literal. A valid declaration such as Version = "v1,v\"2" produces malformed generated C# and fails the consumer build. Please construct and escape the complete message with ToCSharpStringLiteral (and keep raw version text out of generated literals).
Summary
What changed?
The Version property now accepts a comma-separated list (e.g. "v1,v2"), registering the type under each declared version and emitting call helpers that take a required 'version' parameter validated against the declared set. Single-version and unversioned behavior is unchanged.
Why is this change needed?
This allows customers to specify multiple versions of a single orchestration/activity without having to duplicate the code in their own codebase.
Issues / work items
Project checklist
release_notes.mdAI-assisted code disclosure (required)
Was an AI tool used? (select one)
If AI was used:
AI verification (required if AI was used):
Testing
Automated tests
Manual validation (only if runtime/behavior changed)
Notes for reviewers