Skip to content

Commit f80fdd2

Browse files
CopilotJerryNixon
andcommitted
Finalize review feedback responses and logging refinements
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
1 parent 53edba9 commit f80fdd2

3 files changed

Lines changed: 30 additions & 8 deletions

File tree

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

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,24 @@ public async Task<CallToolResult> ExecuteAsync(
402402
// Apply pagination if first is specified with groupby
403403
if (first.HasValue && groupby.Count > 0)
404404
{
405-
PaginationResult paginatedResult = ApplyPagination(aggregatedResults, first.Value, after, logger);
405+
if (!string.IsNullOrWhiteSpace(after))
406+
{
407+
try
408+
{
409+
byte[] cursorBytes = Convert.FromBase64String(after);
410+
string cursorDecoded = System.Text.Encoding.UTF8.GetString(cursorBytes);
411+
if (!int.TryParse(cursorDecoded, out _))
412+
{
413+
logger?.LogWarning("Invalid pagination cursor '{Cursor}' provided to aggregate_records; starting from beginning.", after);
414+
}
415+
}
416+
catch (FormatException)
417+
{
418+
logger?.LogWarning("Invalid pagination cursor '{Cursor}' provided to aggregate_records; starting from beginning.", after);
419+
}
420+
}
421+
422+
PaginationResult paginatedResult = ApplyPagination(aggregatedResults, first.Value, after);
406423
return McpResponseBuilder.BuildSuccessResult(
407424
new Dictionary<string, object?>
408425
{
@@ -606,8 +623,7 @@ internal sealed class PaginationResult
606623
internal static PaginationResult ApplyPagination(
607624
List<Dictionary<string, object?>> allResults,
608625
int first,
609-
string? after,
610-
ILogger? logger = null)
626+
string? after)
611627
{
612628
int startIndex = 0;
613629

@@ -624,7 +640,7 @@ internal static PaginationResult ApplyPagination(
624640
}
625641
catch (FormatException)
626642
{
627-
logger?.LogWarning("Invalid pagination cursor provided to aggregate_records; starting from beginning.");
643+
// Invalid cursor format; start from beginning.
628644
}
629645
}
630646

src/Service.Tests/Mcp/AggregateRecordsToolTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1132,11 +1132,11 @@ 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? argumentsJson, CancellationToken cancellationToken = default)
1135+
private static async Task<CallToolResult> ExecuteWithConfigAsync(RuntimeConfig config, string? argumentsJsonString, CancellationToken cancellationToken = default)
11361136
{
11371137
IServiceProvider serviceProvider = CreateServiceProvider(config);
11381138
AggregateRecordsTool tool = new();
1139-
JsonDocument? arguments = argumentsJson is not null ? JsonDocument.Parse(argumentsJson) : null;
1139+
JsonDocument? arguments = argumentsJsonString is not null ? JsonDocument.Parse(argumentsJsonString) : null;
11401140
return await tool.ExecuteAsync(arguments, serviceProvider, cancellationToken);
11411141
}
11421142

src/Service.Tests/Mcp/McpQueryTimeoutTests.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,10 @@ await McpTelemetryHelper.ExecuteWithTelemetryAsync(
222222
tool, "test_tool", null, sp, CancellationToken.None);
223223
});
224224

225-
Assert.IsTrue(ex.Message.Contains($"{McpRuntimeOptions.MAX_QUERY_TIMEOUT_SECONDS}"));
225+
Assert.AreEqual(
226+
$"MCP query-timeout must be within 1 and {McpRuntimeOptions.MAX_QUERY_TIMEOUT_SECONDS} seconds. " +
227+
$"Provided value: {McpRuntimeOptions.MAX_QUERY_TIMEOUT_SECONDS + 1}.",
228+
ex.Message);
226229
}
227230

228231
#endregion
@@ -290,7 +293,10 @@ public void ValidateMcpUri_Throws_WhenTimeoutExceedsMaximum()
290293
RuntimeConfigValidator validator = CreateRuntimeConfigValidator();
291294

292295
DataApiBuilderException ex = Assert.ThrowsException<DataApiBuilderException>(() => validator.ValidateMcpUri(config));
293-
Assert.IsTrue(ex.Message.Contains($"{McpRuntimeOptions.MAX_QUERY_TIMEOUT_SECONDS}"));
296+
Assert.AreEqual(
297+
$"MCP query-timeout must be within 1 and {McpRuntimeOptions.MAX_QUERY_TIMEOUT_SECONDS} seconds. " +
298+
$"Provided value: {McpRuntimeOptions.MAX_QUERY_TIMEOUT_SECONDS + 1}.",
299+
ex.Message);
294300
}
295301

296302
[TestMethod]

0 commit comments

Comments
 (0)