Skip to content

Commit 9cfbb41

Browse files
committed
feedback
1 parent 082f487 commit 9cfbb41

File tree

5 files changed

+786
-5
lines changed

5 files changed

+786
-5
lines changed

src/Client/Grpc/ProtoUtils.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ public static class ProtoUtils
4040
/// So replaceableStatus = all terminal statuses MINUS dedupeStatuses.
4141
/// </remarks>
4242
public static P.OrchestrationIdReusePolicy? ConvertDedupeStatusesToReusePolicy(
43-
IEnumerable<P.OrchestrationStatus> dedupeStatuses)
43+
IEnumerable<P.OrchestrationStatus>? dedupeStatuses)
4444
{
4545
ImmutableArray<P.OrchestrationStatus> terminalStatuses = GetTerminalStatuses();
46-
ImmutableHashSet<P.OrchestrationStatus> dedupeStatusSet = dedupeStatuses.ToImmutableHashSet();
46+
ImmutableHashSet<P.OrchestrationStatus> dedupeStatusSet = dedupeStatuses?.ToImmutableHashSet() ?? ImmutableHashSet<P.OrchestrationStatus>.Empty;
4747

4848
P.OrchestrationIdReusePolicy policy = new();
4949

src/Client/OrchestrationServiceClientShim/ShimDurableTaskClient.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,10 +201,8 @@ public override async Task<string> ScheduleNewOrchestrationInstanceAsync(
201201
{
202202
if (!Enum.TryParse<OrchestrationRuntimeStatus>(s, ignoreCase: true, out var status))
203203
{
204-
var validStatuses = string.Join(", ", Enum.GetNames(typeof(OrchestrationRuntimeStatus)));
205204
throw new ArgumentException(
206-
$"Invalid orchestration runtime status: '{s}' for deduplication. Valid values are: {validStatuses}",
207-
nameof(options.DedupeStatuses));
205+
$"Invalid orchestration runtime status: '{s}' for deduplication.");
208206
}
209207
return status.ConvertToCore();
210208
})
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
using Grpc.Core;
5+
using Microsoft.DurableTask.Client;
6+
using Microsoft.Extensions.Logging;
7+
8+
namespace Microsoft.DurableTask.Client.Grpc.Tests;
9+
10+
public class GrpcDurableTaskClientTests
11+
{
12+
readonly Mock<ILogger> loggerMock = new();
13+
14+
GrpcDurableTaskClient CreateClient()
15+
{
16+
var callInvoker = Mock.Of<CallInvoker>();
17+
var options = new GrpcDurableTaskClientOptions
18+
{
19+
CallInvoker = callInvoker,
20+
};
21+
22+
return new GrpcDurableTaskClient("test", options, this.loggerMock.Object);
23+
}
24+
25+
[Fact]
26+
public async Task ScheduleNewOrchestrationInstanceAsync_InvalidDedupeStatus_ThrowsArgumentException()
27+
{
28+
// Arrange
29+
var client = this.CreateClient();
30+
var startOptions = new StartOrchestrationOptions
31+
{
32+
DedupeStatuses = new[] { "InvalidStatus", "AnotherInvalidStatus" },
33+
};
34+
35+
// Act & Assert
36+
Func<Task> act = async () => await client.ScheduleNewOrchestrationInstanceAsync(
37+
new TaskName("TestOrchestration"),
38+
input: null,
39+
startOptions);
40+
41+
var exception = await act.Should().ThrowAsync<ArgumentException>();
42+
exception.Which.Message.Should().Contain("Invalid orchestration runtime status: 'InvalidStatus' for deduplication.");
43+
}
44+
45+
[Fact]
46+
public async Task ScheduleNewOrchestrationInstanceAsync_InvalidDedupeStatus_ContainsInvalidStatusInMessage()
47+
{
48+
// Arrange
49+
var client = this.CreateClient();
50+
var startOptions = new StartOrchestrationOptions
51+
{
52+
DedupeStatuses = new[] { "NonExistentStatus" },
53+
};
54+
55+
// Act & Assert
56+
Func<Task> act = async () => await client.ScheduleNewOrchestrationInstanceAsync(
57+
new TaskName("TestOrchestration"),
58+
input: null,
59+
startOptions);
60+
61+
var exception = await act.Should().ThrowAsync<ArgumentException>();
62+
exception.Which.Message.Should().Contain("'NonExistentStatus'");
63+
exception.Which.Message.Should().Contain("for deduplication");
64+
}
65+
66+
[Fact]
67+
public async Task ScheduleNewOrchestrationInstanceAsync_MixedValidAndInvalidStatus_ThrowsArgumentException()
68+
{
69+
// Arrange
70+
var client = this.CreateClient();
71+
var startOptions = new StartOrchestrationOptions
72+
{
73+
DedupeStatuses = new[] { "Completed", "InvalidStatus", "Failed" },
74+
};
75+
76+
// Act & Assert
77+
Func<Task> act = async () => await client.ScheduleNewOrchestrationInstanceAsync(
78+
new TaskName("TestOrchestration"),
79+
input: null,
80+
startOptions);
81+
82+
var exception = await act.Should().ThrowAsync<ArgumentException>();
83+
exception.Which.Message.Should().Contain("Invalid orchestration runtime status: 'InvalidStatus' for deduplication.");
84+
}
85+
86+
[Fact]
87+
public async Task ScheduleNewOrchestrationInstanceAsync_CaseInsensitiveValidStatus_DoesNotThrowArgumentException()
88+
{
89+
// Arrange
90+
var client = this.CreateClient();
91+
var startOptions = new StartOrchestrationOptions
92+
{
93+
DedupeStatuses = new[] { "completed", "FAILED", "Terminated" },
94+
};
95+
96+
// Act & Assert - Case-insensitive parsing should work, so no ArgumentException should be thrown
97+
// The call will fail at the gRPC level, but validation should pass
98+
Func<Task> act = async () => await client.ScheduleNewOrchestrationInstanceAsync(
99+
new TaskName("TestOrchestration"),
100+
input: null,
101+
startOptions);
102+
103+
// Should not throw ArgumentException for invalid status (case-insensitive parsing works)
104+
// It may throw other exceptions due to gRPC call failure, but not ArgumentException
105+
var exception = await act.Should().ThrowAsync<Exception>();
106+
exception.Which.Should().NotBeOfType<ArgumentException>();
107+
}
108+
109+
[Fact]
110+
public async Task ScheduleNewOrchestrationInstanceAsync_ValidDedupeStatus_DoesNotThrowArgumentException()
111+
{
112+
// Arrange
113+
var client = this.CreateClient();
114+
var startOptions = new StartOrchestrationOptions
115+
{
116+
DedupeStatuses = new[] { "Completed", "Failed" },
117+
};
118+
119+
// Act & Assert - Valid statuses should pass validation
120+
// The call will fail at the gRPC level, but validation should pass
121+
Func<Task> act = async () => await client.ScheduleNewOrchestrationInstanceAsync(
122+
new TaskName("TestOrchestration"),
123+
input: null,
124+
startOptions);
125+
126+
// Should not throw ArgumentException for invalid status since "Completed" and "Failed" are valid
127+
var exception = await act.Should().ThrowAsync<Exception>();
128+
exception.Which.Should().NotBeOfType<ArgumentException>();
129+
}
130+
}
131+

0 commit comments

Comments
 (0)