Skip to content

Commit 53edba9

Browse files
CopilotJerryNixon
andcommitted
Address review comments for timeout bounds and test refactoring
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
1 parent 79f7e36 commit 53edba9

2 files changed

Lines changed: 7 additions & 6 deletions

File tree

src/Azure.DataApiBuilder.Mcp/BuiltInTools/AggregateRecordsTool.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public class AggregateRecordsTool : IMcpTool
3636
public ToolType ToolType { get; } = ToolType.BuiltIn;
3737

3838
private static readonly HashSet<string> _validFunctions = new(StringComparer.OrdinalIgnoreCase) { "count", "avg", "sum", "min", "max" };
39+
// Supported HAVING operators for post-aggregation filtering.
3940
private static readonly HashSet<string> _validHavingOperators = new(StringComparer.OrdinalIgnoreCase) { "eq", "neq", "gt", "gte", "lt", "lte", "in" };
4041

4142
public Tool GetToolMetadata()
@@ -253,7 +254,7 @@ public async Task<CallToolResult> ExecuteAsync(
253254
return McpResponseBuilder.BuildErrorResult(
254255
toolName,
255256
"InvalidArguments",
256-
$"Unsupported having operator '{prop.Name}'. Supported operators are: eq, neq, gt, gte, lt, lte, in.",
257+
$"Unsupported having operator '{prop.Name}'. Supported operators are: {string.Join(", ", _validHavingOperators)}.",
257258
logger);
258259
}
259260

src/Service.Tests/Mcp/AggregateRecordsToolTests.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,10 @@ public async Task AggregateRecords_DisabledConfigurations_ReturnToolDisabledErro
111111
[DataRow("{\"entity\": \"Book\", \"function\": \"count\", \"field\": \"*\", \"having\": { \"between\": 10 }}", "Unsupported having operator", DisplayName = "Unsupported having operator returns InvalidArguments")]
112112
[DataRow("{\"entity\": \"Book\", \"function\": \"count\", \"field\": \"*\", \"having\": { \"eq\": \"10\" }}", "must be a numeric value", DisplayName = "Non-numeric having value returns InvalidArguments")]
113113
[DataRow("{\"entity\": \"Book\", \"function\": \"count\", \"field\": \"*\", \"having\": { \"in\": [1, \"x\"] }}", "must contain only numeric values", DisplayName = "Non-numeric having.in value returns InvalidArguments")]
114-
public async Task AggregateRecords_InvalidArguments_ReturnInvalidArguments(string? argsJson, string? expectedMessageFragment)
114+
public async Task AggregateRecords_InvalidArguments_ReturnInvalidArguments(string? argumentsJson, string? expectedMessageFragment)
115115
{
116116
RuntimeConfig config = CreateConfig();
117-
CallToolResult result = await ExecuteWithConfigAsync(config, argsJson);
117+
CallToolResult result = await ExecuteWithConfigAsync(config, argumentsJson);
118118

119119
Assert.IsTrue(result.IsError == true);
120120
JsonElement content = ParseContent(result);
@@ -1132,12 +1132,12 @@ private static JsonElement ParseContent(CallToolResult result)
11321132
return JsonDocument.Parse(firstContent.Text).RootElement;
11331133
}
11341134

1135-
private static async Task<CallToolResult> ExecuteWithConfigAsync(RuntimeConfig config, string? argsJson, CancellationToken cancellationToken = default)
1135+
private static async Task<CallToolResult> ExecuteWithConfigAsync(RuntimeConfig config, string? argumentsJson, CancellationToken cancellationToken = default)
11361136
{
11371137
IServiceProvider serviceProvider = CreateServiceProvider(config);
11381138
AggregateRecordsTool tool = new();
1139-
JsonDocument? args = argsJson is null ? null : JsonDocument.Parse(argsJson);
1140-
return await tool.ExecuteAsync(args, serviceProvider, cancellationToken);
1139+
JsonDocument? arguments = argumentsJson is not null ? JsonDocument.Parse(argumentsJson) : null;
1140+
return await tool.ExecuteAsync(arguments, serviceProvider, cancellationToken);
11411141
}
11421142

11431143
private static void AssertToolDisabledError(JsonElement content)

0 commit comments

Comments
 (0)