From 3109b89111ca3e6a1b9ebf86d4dbb47d6906e402 Mon Sep 17 00:00:00 2001 From: aaronburtle Date: Tue, 5 May 2026 15:07:50 -0700 Subject: [PATCH 1/4] refactor pagination response --- src/Core/Resolvers/SqlPaginationUtil.cs | 22 ++-- src/Core/Resolvers/SqlResponseHelpers.cs | 146 ++++++++--------------- 2 files changed, 60 insertions(+), 108 deletions(-) diff --git a/src/Core/Resolvers/SqlPaginationUtil.cs b/src/Core/Resolvers/SqlPaginationUtil.cs index 852138ef09..92e3b26e8f 100644 --- a/src/Core/Resolvers/SqlPaginationUtil.cs +++ b/src/Core/Resolvers/SqlPaginationUtil.cs @@ -644,13 +644,13 @@ public static string BuildQueryStringWithAfterToken(NameValueCollection? querySt } /// - /// Gets a consolidated next link for pagination in JSON format. + /// Builds the next-page URI used for cursor-based pagination. /// - /// The base Pagination Uri - /// The query string with after value - /// True, if the next link should be relative - /// - public static JsonElement GetConsolidatedNextLinkForPagination(string baseUri, string queryString, bool isNextLinkRelative = false) + /// The base pagination URI. + /// The query string with the $after value already merged in. + /// True to return only the path + query (no host); false for an absolute URL. + /// The next-page URI as a string. + public static string BuildNextLinkUri(string baseUri, string queryString, bool isNextLinkRelative = false) { UriBuilder uriBuilder = new(baseUri) { @@ -659,17 +659,9 @@ public static JsonElement GetConsolidatedNextLinkForPagination(string baseUri, s }; // Construct final link- absolute or relative - string nextLinkValue = isNextLinkRelative + return isNextLinkRelative ? uriBuilder.Uri.PathAndQuery // returns just "/api/?$after...", no host : uriBuilder.Uri.AbsoluteUri; // returns full URL - - // Return serialized JSON object - string jsonString = JsonSerializer.Serialize(new[] - { - new { nextLink = nextLinkValue } - }); - - return JsonSerializer.Deserialize(jsonString); } /// diff --git a/src/Core/Resolvers/SqlResponseHelpers.cs b/src/Core/Resolvers/SqlResponseHelpers.cs index 3736054d7f..fa5a5ee497 100644 --- a/src/Core/Resolvers/SqlResponseHelpers.cs +++ b/src/Core/Resolvers/SqlResponseHelpers.cs @@ -22,11 +22,11 @@ public class SqlResponseHelpers { /// - /// Format the results from a Find operation. Check if there is a requirement - /// for a nextLink/after, and if so, add this value to the array of JsonElements to - /// be used as part of the response. + /// Format the results from a Find operation. If a nextLink/after token is required for + /// pagination, the envelope is built directly via an anonymous response object so that + /// pagination metadata is carried out-of-band rather than encoded into the row collection. /// - /// The JsonDocument from the query. + /// The JsonElement from the query (object for single-row, array for collections). /// The RequestContext. /// The metadataprovider. /// Runtimeconfig object @@ -41,7 +41,6 @@ public static OkObjectResult FormatFindResult( HttpContext httpContext, bool? isMcpRequest = null) { - // When there are no rows returned from the database, the jsonElement will be an empty array. // In that case, the response is returned as is. if (findOperationResponse.ValueKind is JsonValueKind.Array && findOperationResponse.GetArrayLength() == 0) @@ -49,41 +48,42 @@ public static OkObjectResult FormatFindResult( return OkResponse(findOperationResponse); } - HashSet extraFieldsInResponse = (findOperationResponse.ValueKind is not JsonValueKind.Array) - ? DetermineExtraFieldsInResponse(findOperationResponse, context.FieldsToBeReturned) - : DetermineExtraFieldsInResponse(findOperationResponse.EnumerateArray().First(), context.FieldsToBeReturned); + bool isCollection = findOperationResponse.ValueKind is JsonValueKind.Array; + + // Compute additional fields that were fetched for cursor/$orderby computation but + // are not part of $select and so should be stripped from the response payload. + JsonElement firstRowProbe = isCollection ? findOperationResponse.EnumerateArray().First() : findOperationResponse; + HashSet extraFieldsInResponse = DetermineExtraFieldsInResponse(firstRowProbe, context.FieldsToBeReturned); uint defaultPageSize = runtimeConfig.DefaultPageSize(); uint maxPageSize = runtimeConfig.MaxPageSize(); + bool hasNext = isCollection && SqlPaginationUtil.HasNext(findOperationResponse, context.First, defaultPageSize, maxPageSize); - // If the results are not a collection or if the query does not have a next page - // no nextLink/after is needed. So, the response is returned after removing the extra fields. - if (findOperationResponse.ValueKind is not JsonValueKind.Array || !SqlPaginationUtil.HasNext(findOperationResponse, context.First, defaultPageSize, maxPageSize)) + // No-pagination path: single object, or a collection without a next page. + if (!hasNext) { - // If there are no additional fields present, the response is returned directly. When there - // are extra fields, they are removed before returning the response. if (extraFieldsInResponse.Count == 0) { return OkResponse(findOperationResponse); } - else - { - return findOperationResponse.ValueKind is JsonValueKind.Array ? OkResponse(JsonSerializer.SerializeToElement(RemoveExtraFieldsInResponseWithMultipleItems(findOperationResponse.EnumerateArray().ToList(), extraFieldsInResponse))) - : OkResponse(RemoveExtraFieldsInResponseWithSingleItem(findOperationResponse, extraFieldsInResponse)); - } + + return isCollection + ? OkResponse(JsonSerializer.SerializeToElement(RemoveExtraFieldsInResponseWithMultipleItems(findOperationResponse.EnumerateArray().ToList(), extraFieldsInResponse))) + : OkResponse(RemoveExtraFieldsInResponseWithSingleItem(findOperationResponse, extraFieldsInResponse)); } - List rootEnumerated = findOperationResponse.EnumerateArray().ToList(); + // Paginated path. + List rows = findOperationResponse.EnumerateArray().ToList(); // More records exist than requested, we know this by requesting 1 extra record, // that extra record is removed here. - rootEnumerated.RemoveAt(rootEnumerated.Count - 1); + rows.RemoveAt(rows.Count - 1); // The fields such as primary keys, fields in $orderby clause that are retrieved in addition to the // fields requested in the $select clause are required for calculating the $after element which is part of nextLink. // So, the extra fields are removed post the calculation of $after element. string after = SqlPaginationUtil.MakeCursorFromJsonElement( - element: rootEnumerated[rootEnumerated.Count - 1], + element: rows[rows.Count - 1], orderByColumns: context.OrderByClauseOfBackingColumns, primaryKey: sqlMetadataProvider.GetSourceDefinition(context.EntityName).PrimaryKey, entityName: context.EntityName, @@ -94,40 +94,34 @@ public static OkObjectResult FormatFindResult( // When there are extra fields present, they are removed before returning the response. if (extraFieldsInResponse.Count > 0) { - rootEnumerated = RemoveExtraFieldsInResponseWithMultipleItems(rootEnumerated, extraFieldsInResponse); + rows = RemoveExtraFieldsInResponseWithMultipleItems(rows, extraFieldsInResponse); } - // Create an 'after' object if the request comes from MCP endpoint. + // MCP endpoint: { value: [...], after: "" } if (isMcpRequest is true) { - string jsonString = JsonSerializer.Serialize(new[] + return new OkObjectResult(new { - new { after = after } + value = rows, + after = after }); - JsonElement afterElement = JsonSerializer.Deserialize(jsonString); - - rootEnumerated.Add(afterElement); } - // Create a 'nextLink' object if the request comes from REST endpoint. - else - { - string basePaginationUri = SqlPaginationUtil.ConstructBaseUriForPagination(httpContext, runtimeConfig.Runtime?.BaseRoute); - // Build the query string with the $after token. - string queryString = SqlPaginationUtil.BuildQueryStringWithAfterToken( - queryStringParameters: context!.ParsedQueryString, - newAfterPayload: after); + // REST endpoint: { value: [...], nextLink: "" } + string basePaginationUri = SqlPaginationUtil.ConstructBaseUriForPagination(httpContext, runtimeConfig.Runtime?.BaseRoute); + string queryString = SqlPaginationUtil.BuildQueryStringWithAfterToken( + queryStringParameters: context!.ParsedQueryString, + newAfterPayload: after); + string nextLink = SqlPaginationUtil.BuildNextLinkUri( + baseUri: basePaginationUri, + queryString: queryString, + isNextLinkRelative: runtimeConfig.NextLinkRelative()); - // Get the final consolidated nextLink for the pagination. - JsonElement nextLink = SqlPaginationUtil.GetConsolidatedNextLinkForPagination( - baseUri: basePaginationUri, - queryString: queryString, - isNextLinkRelative: runtimeConfig.NextLinkRelative()); - - rootEnumerated.Add(nextLink); - } - - return OkResponse(JsonSerializer.SerializeToElement(rootEnumerated), isMcpRequest); + return new OkObjectResult(new + { + value = rows, + nextLink = nextLink + }); } /// @@ -200,60 +194,26 @@ private static JsonElement RemoveExtraFieldsInResponseWithSingleItem(JsonElement } /// - /// Helper function returns an OkObjectResult with provided arguments in a - /// form that complies with vNext Api guidelines. + /// Helper function returns an OkObjectResult that wraps a single JsonElement (object or array) + /// into the standard { "value": [ ... ] } envelope used by REST/MCP responses. + /// + /// Pagination metadata (nextLink/after) is intentionally NOT inferred from the + /// shape of . attaches those fields + /// out-of-band when needed. This avoids confusing array-typed column values (e.g. SQL Server + /// JSON arrays, vector/collection types) with a pagination sentinel. /// /// Value representing the Json results of the client's request. - /// True if request is done through MCP endpoint. /// Correctly formatted OkObjectResult. - public static OkObjectResult OkResponse(JsonElement jsonResult, bool? isMcpRequest = null) + public static OkObjectResult OkResponse(JsonElement jsonResult) { - // For consistency we return all values as type Array - if (jsonResult.ValueKind != JsonValueKind.Array) - { - string jsonString = $"[{JsonSerializer.Serialize(jsonResult)}]"; - jsonResult = JsonSerializer.Deserialize(jsonString); - } + // For consistency we always return the payload as an array under "value". + List rows = jsonResult.ValueKind == JsonValueKind.Array + ? jsonResult.EnumerateArray().ToList() + : new List { jsonResult }; - List resultEnumerated = jsonResult.EnumerateArray().ToList(); - // More than 0 records, and the last element is of type array, then we have pagination - if (resultEnumerated.Count > 0 && resultEnumerated[resultEnumerated.Count - 1].ValueKind == JsonValueKind.Array) - { - // Get the 'nextLink' or 'after' - // resultEnumerated will be an array of the form - // [{object1}, {object2},...{objectlimit}, [{nextLinkObject/afterObject}]] - // if the last element is of type array, we know it is 'nextLink' - // if the request is done through the REST endpoint and it is - // 'after' if the request is done through the MCP endpoint, - // we strip the "[" and "]" and then save the element - // into a dictionary with a key of "nextLinkAfter" and a value that - // represents the nextLink/after data we require. - string nextLinkAfterJsonString = JsonSerializer.Serialize(resultEnumerated[resultEnumerated.Count - 1]); - Dictionary nextLinkAfter = JsonSerializer.Deserialize>(nextLinkAfterJsonString[1..^1])!; - IEnumerable value = resultEnumerated.Take(resultEnumerated.Count - 1); - - // Check 'after' object if request is done through MCP endpoint. - if (isMcpRequest is true) - { - return new OkObjectResult(new - { - value = value, - after = nextLinkAfter["after"] - }); - } - - // Check 'nextLink' object if request is done through REST endpoint. - return new OkObjectResult(new - { - value = value, - @nextLink = nextLinkAfter["nextLink"] - }); - } - - // no pagination, do not need nextLink return new OkObjectResult(new { - value = resultEnumerated + value = rows }); } From e0de2b2a99fcd61eb1c99c5b877f1b1fed366bd9 Mon Sep 17 00:00:00 2001 From: aaronburtle Date: Thu, 7 May 2026 02:15:02 -0700 Subject: [PATCH 2/4] cleanup and add unit tests --- src/Core/Resolvers/SqlPaginationUtil.cs | 5 +- src/Core/Resolvers/SqlResponseHelpers.cs | 19 +- .../UnitTests/SqlResponseHelpersUnitTests.cs | 292 ++++++++++++++++++ 3 files changed, 306 insertions(+), 10 deletions(-) create mode 100644 src/Service.Tests/UnitTests/SqlResponseHelpersUnitTests.cs diff --git a/src/Core/Resolvers/SqlPaginationUtil.cs b/src/Core/Resolvers/SqlPaginationUtil.cs index 92e3b26e8f..07c433ea0a 100644 --- a/src/Core/Resolvers/SqlPaginationUtil.cs +++ b/src/Core/Resolvers/SqlPaginationUtil.cs @@ -608,8 +608,7 @@ public static string ConstructBaseUriForPagination(HttpContext httpContext, stri /// /// Builds a query string by appending or replacing the $after token with the specified value. /// - /// This method does not include the in the returned query - /// string. It only processes and formats the query string parameters. + /// This method returns only the query string portion (no path or host). /// A collection of existing query string parameters. If , an empty collection is used. /// The $after parameter, if present, will be removed before appending the new token. /// The new value for the $after token. If this value is , empty, or whitespace, no @@ -638,8 +637,6 @@ public static string BuildQueryStringWithAfterToken(NameValueCollection? querySt queryString += $"{afterPrefix}{RequestParser.AFTER_URL}={newAfterPayload}"; } - // Construct final link - // return $"{path}{queryString}"; return queryString; } diff --git a/src/Core/Resolvers/SqlResponseHelpers.cs b/src/Core/Resolvers/SqlResponseHelpers.cs index fa5a5ee497..b5266a49f6 100644 --- a/src/Core/Resolvers/SqlResponseHelpers.cs +++ b/src/Core/Resolvers/SqlResponseHelpers.cs @@ -31,7 +31,7 @@ public class SqlResponseHelpers /// The metadataprovider. /// Runtimeconfig object /// HTTP context associated with the API request - /// True if request is done through MCP endpoint + /// true if invoked from the MCP endpoint (emit after); false or null for REST (emit nextLink). /// An OkObjectResult from a Find operation that has been correctly formatted. public static OkObjectResult FormatFindResult( JsonElement findOperationResponse, @@ -41,8 +41,7 @@ public static OkObjectResult FormatFindResult( HttpContext httpContext, bool? isMcpRequest = null) { - // When there are no rows returned from the database, the jsonElement will be an empty array. - // In that case, the response is returned as is. + // Empty result set: return the standard envelope { "value": [] } and skip extra-field/cursor work. if (findOperationResponse.ValueKind is JsonValueKind.Array && findOperationResponse.GetArrayLength() == 0) { return OkResponse(findOperationResponse); @@ -110,7 +109,7 @@ public static OkObjectResult FormatFindResult( // REST endpoint: { value: [...], nextLink: "" } string basePaginationUri = SqlPaginationUtil.ConstructBaseUriForPagination(httpContext, runtimeConfig.Runtime?.BaseRoute); string queryString = SqlPaginationUtil.BuildQueryStringWithAfterToken( - queryStringParameters: context!.ParsedQueryString, + queryStringParameters: context.ParsedQueryString, newAfterPayload: after); string nextLink = SqlPaginationUtil.BuildNextLinkUri( baseUri: basePaginationUri, @@ -133,9 +132,17 @@ public static OkObjectResult FormatFindResult( /// /// Response json retrieved from the database /// List of fields to be returned in the response. - /// Additional fields that are present in the response + /// Additional fields that are present in the response. Returns an empty set when is not a JSON object (e.g. a scalar or array-typed row value), since there are no named properties to filter. private static HashSet DetermineExtraFieldsInResponse(JsonElement response, List fieldsToBeReturned) { + // Guard: a result row is normally a JSON object, but with database engines that can return + // array/scalar/collection-typed shapes at the row level there is nothing to enumerate. In that + // case there are no extra-field columns to strip, so return an empty set rather than throwing. + if (response.ValueKind is not JsonValueKind.Object) + { + return new HashSet(); + } + HashSet fieldsPresentInResponse = new(); foreach (JsonProperty property in response.EnumerateObject()) @@ -207,7 +214,7 @@ private static JsonElement RemoveExtraFieldsInResponseWithSingleItem(JsonElement public static OkObjectResult OkResponse(JsonElement jsonResult) { // For consistency we always return the payload as an array under "value". - List rows = jsonResult.ValueKind == JsonValueKind.Array + List rows = jsonResult.ValueKind is JsonValueKind.Array ? jsonResult.EnumerateArray().ToList() : new List { jsonResult }; diff --git a/src/Service.Tests/UnitTests/SqlResponseHelpersUnitTests.cs b/src/Service.Tests/UnitTests/SqlResponseHelpersUnitTests.cs new file mode 100644 index 0000000000..ec87ce5ab6 --- /dev/null +++ b/src/Service.Tests/UnitTests/SqlResponseHelpersUnitTests.cs @@ -0,0 +1,292 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Collections.Generic; +using System.Text.Json; +using Azure.DataApiBuilder.Config.DatabasePrimitives; +using Azure.DataApiBuilder.Config.ObjectModel; +using Azure.DataApiBuilder.Core.Models; +using Azure.DataApiBuilder.Core.Resolvers; +using Azure.DataApiBuilder.Core.Services; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Azure.DataApiBuilder.Service.Tests.UnitTests +{ + /// + /// Focused unit tests for . + /// + /// These tests pin the response envelope shape across the five paths the method takes: + /// (1) empty result, (2) single object, (3) collection without next page, + /// (4) collection with next page (REST), (5) collection with next page (MCP). + /// + /// Test 6 is a regression guard against confusing array-typed column values with the old + /// pagination "shape sentinel" — it pins that a row containing an array-valued property + /// (e.g. a SQL Server JSON array, vector, or other collection-typed column) is returned + /// unmodified and is NOT misinterpreted as a pagination marker. + /// + [TestClass] + public class SqlResponseHelpersUnitTests + { + private const string ENTITY_NAME = "Book"; + + #region Tests + + /// + /// An empty result set returns the standard envelope { "value": [] } and no + /// pagination metadata. + /// + [TestMethod] + public void FormatFindResult_EmptyArray_ReturnsValueOnlyEnvelope() + { + JsonElement input = ParseJson("[]"); + FindRequestContext context = CreateContext(fieldsToBeReturned: new List()); + + OkObjectResult result = SqlResponseHelpers.FormatFindResult( + findOperationResponse: input, + context: context, + sqlMetadataProvider: Mock.Of(), + runtimeConfig: CreateRuntimeConfig(), + httpContext: new DefaultHttpContext()); + + JsonElement envelope = SerializeValue(result); + AssertHasNoPaginationFields(envelope); + Assert.AreEqual(0, envelope.GetProperty("value").GetArrayLength()); + } + + /// + /// A single-object result (FindById) is wrapped into { "value": [ { ... } ] } + /// with no pagination metadata. + /// + [TestMethod] + public void FormatFindResult_SingleObject_ReturnsValueOnlyEnvelope() + { + JsonElement input = ParseJson(@"{ ""id"": 1, ""title"": ""Dune"" }"); + FindRequestContext context = CreateContext(fieldsToBeReturned: new List { "id", "title" }); + + OkObjectResult result = SqlResponseHelpers.FormatFindResult( + findOperationResponse: input, + context: context, + sqlMetadataProvider: Mock.Of(), + runtimeConfig: CreateRuntimeConfig(), + httpContext: new DefaultHttpContext()); + + JsonElement envelope = SerializeValue(result); + AssertHasNoPaginationFields(envelope); + + JsonElement value = envelope.GetProperty("value"); + Assert.AreEqual(1, value.GetArrayLength()); + Assert.AreEqual(1, value[0].GetProperty("id").GetInt32()); + Assert.AreEqual("Dune", value[0].GetProperty("title").GetString()); + } + + /// + /// A collection result that does NOT exceed $first has no next page; the envelope + /// is { "value": [ ... ] } with no pagination metadata. + /// + [TestMethod] + public void FormatFindResult_CollectionWithoutNextPage_ReturnsValueOnlyEnvelope() + { + // first = 5, only 2 rows: HasNext is false. + JsonElement input = ParseJson(@"[ { ""id"": 1 }, { ""id"": 2 } ]"); + FindRequestContext context = CreateContext( + fieldsToBeReturned: new List { "id" }, + first: 5); + + OkObjectResult result = SqlResponseHelpers.FormatFindResult( + findOperationResponse: input, + context: context, + sqlMetadataProvider: Mock.Of(), + runtimeConfig: CreateRuntimeConfig(), + httpContext: new DefaultHttpContext()); + + JsonElement envelope = SerializeValue(result); + AssertHasNoPaginationFields(envelope); + Assert.AreEqual(2, envelope.GetProperty("value").GetArrayLength()); + } + + /// + /// REST: a collection with more rows than requested produces the + /// { "value": [ ... ], "nextLink": "..." } envelope and trims the +1 probe row. + /// + [TestMethod] + public void FormatFindResult_CollectionWithNextPage_Rest_ReturnsNextLinkEnvelope() + { + // first = 1, 2 rows: HasNext is true, last (probe) row is dropped. + JsonElement input = ParseJson(@"[ { ""id"": 1 }, { ""id"": 2 } ]"); + FindRequestContext context = CreateContext( + fieldsToBeReturned: new List { "id" }, + first: 1); + + DefaultHttpContext httpContext = new(); + httpContext.Request.Scheme = "https"; + httpContext.Request.Host = new HostString("localhost"); + httpContext.Request.Path = "/api/Book"; + + OkObjectResult result = SqlResponseHelpers.FormatFindResult( + findOperationResponse: input, + context: context, + sqlMetadataProvider: CreateMetadataProviderWithIdPrimaryKey(), + runtimeConfig: CreateRuntimeConfig(), + httpContext: httpContext); + + JsonElement envelope = SerializeValue(result); + JsonElement value = envelope.GetProperty("value"); + Assert.AreEqual(1, value.GetArrayLength(), "Probe row should have been removed."); + Assert.AreEqual(1, value[0].GetProperty("id").GetInt32()); + + Assert.IsTrue(envelope.TryGetProperty("nextLink", out JsonElement nextLink), + "REST paginated response must carry a 'nextLink' field."); + Assert.IsTrue(nextLink.GetString()!.Contains("$after="), "nextLink should encode the $after cursor."); + Assert.IsFalse(envelope.TryGetProperty("after", out _), + "REST paginated response must NOT carry an 'after' field."); + } + + /// + /// MCP: a collection with more rows than requested produces the + /// { "value": [ ... ], "after": "..." } envelope and trims the +1 probe row. + /// + [TestMethod] + public void FormatFindResult_CollectionWithNextPage_Mcp_ReturnsAfterEnvelope() + { + JsonElement input = ParseJson(@"[ { ""id"": 1 }, { ""id"": 2 } ]"); + FindRequestContext context = CreateContext( + fieldsToBeReturned: new List { "id" }, + first: 1); + + OkObjectResult result = SqlResponseHelpers.FormatFindResult( + findOperationResponse: input, + context: context, + sqlMetadataProvider: CreateMetadataProviderWithIdPrimaryKey(), + runtimeConfig: CreateRuntimeConfig(), + httpContext: new DefaultHttpContext(), + isMcpRequest: true); + + JsonElement envelope = SerializeValue(result); + JsonElement value = envelope.GetProperty("value"); + Assert.AreEqual(1, value.GetArrayLength(), "Probe row should have been removed."); + + Assert.IsTrue(envelope.TryGetProperty("after", out JsonElement after), + "MCP paginated response must carry an 'after' field."); + Assert.IsFalse(string.IsNullOrEmpty(after.GetString()), "after cursor should be populated."); + Assert.IsFalse(envelope.TryGetProperty("nextLink", out _), + "MCP paginated response must NOT carry a 'nextLink' field."); + } + + /// + /// Regression guard for the shape-sentinel removal: + /// a row whose last column is an array-valued JSON value (e.g. a SQL Server JSON array + /// or vector/collection column) must be returned verbatim and must NOT be confused + /// with a pagination marker. Pre-refactor, the in-band sentinel detection in + /// would have misclassified this shape. + /// + [TestMethod] + public void FormatFindResult_RowWithArrayColumn_NotMisclassifiedAsPagination() + { + // Two rows with array-valued "tags" column. first=5 so HasNext=false. + JsonElement input = ParseJson(@"[ + { ""id"": 1, ""tags"": [ ""sci-fi"", ""classic"" ] }, + { ""id"": 2, ""tags"": [ ""fantasy"" ] } + ]"); + FindRequestContext context = CreateContext( + fieldsToBeReturned: new List { "id", "tags" }, + first: 5); + + OkObjectResult result = SqlResponseHelpers.FormatFindResult( + findOperationResponse: input, + context: context, + sqlMetadataProvider: Mock.Of(), + runtimeConfig: CreateRuntimeConfig(), + httpContext: new DefaultHttpContext()); + + JsonElement envelope = SerializeValue(result); + AssertHasNoPaginationFields(envelope); + + JsonElement value = envelope.GetProperty("value"); + Assert.AreEqual(2, value.GetArrayLength(), "Both rows must be returned, including the array-valued column."); + + // Pin that the array column survived intact and the row count is correct. + Assert.AreEqual(2, value[0].GetProperty("tags").GetArrayLength()); + Assert.AreEqual("sci-fi", value[0].GetProperty("tags")[0].GetString()); + Assert.AreEqual(1, value[1].GetProperty("tags").GetArrayLength()); + Assert.AreEqual("fantasy", value[1].GetProperty("tags")[0].GetString()); + } + + #endregion + + #region Helpers + + private static JsonElement ParseJson(string json) + { + return JsonDocument.Parse(json).RootElement.Clone(); + } + + /// + /// Serializes the OkObjectResult.Value (an anonymous envelope object) to JSON and + /// returns it as a JsonElement so individual fields can be asserted. + /// + private static JsonElement SerializeValue(OkObjectResult result) + { + string json = JsonSerializer.Serialize(result.Value); + return JsonDocument.Parse(json).RootElement.Clone(); + } + + private static void AssertHasNoPaginationFields(JsonElement envelope) + { + Assert.IsFalse(envelope.TryGetProperty("nextLink", out _), "Envelope unexpectedly contains 'nextLink'."); + Assert.IsFalse(envelope.TryGetProperty("after", out _), "Envelope unexpectedly contains 'after'."); + } + + private static FindRequestContext CreateContext(List fieldsToBeReturned, int? first = null) + { + SourceDefinition sourceDef = new() { PrimaryKey = new List { "id" } }; + sourceDef.SourceEntityRelationshipMap.Add(ENTITY_NAME, new()); + DatabaseObject dbObject = new DatabaseTable(schemaName: "dbo", tableName: ENTITY_NAME) + { + TableDefinition = sourceDef + }; + + FindRequestContext context = new(entityName: ENTITY_NAME, dbo: dbObject, isList: true) + { + First = first + }; + context.FieldsToBeReturned = fieldsToBeReturned; + return context; + } + + /// + /// Builds a metadata provider that maps any (entity, "id") to the exposed column "id" + /// so that can produce a cursor. + /// + private static ISqlMetadataProvider CreateMetadataProviderWithIdPrimaryKey() + { + Mock mock = new(); + + SourceDefinition sourceDef = new() { PrimaryKey = new List { "id" } }; + mock.Setup(m => m.GetSourceDefinition(It.IsAny())).Returns(sourceDef); + + string exposedName = "id"; + mock.Setup(m => m.TryGetExposedColumnName(It.IsAny(), It.IsAny(), out exposedName)) + .Returns(true); + + return mock.Object; + } + + private static RuntimeConfig CreateRuntimeConfig() + { + return new RuntimeConfig( + Schema: "test-schema", + DataSource: new DataSource(DatabaseType: DatabaseType.MSSQL, ConnectionString: "", Options: null), + Entities: new RuntimeEntities(new Dictionary()), + Runtime: new RuntimeOptions( + Rest: new(), + GraphQL: new(), + Mcp: null, + Host: new(Cors: null, Authentication: null, Mode: HostMode.Development))); + } + + #endregion + } +} From 781f323cb3e59dff8301a42b4cbc1ad166cf2be2 Mon Sep 17 00:00:00 2001 From: aaronburtle Date: Thu, 7 May 2026 02:50:27 -0700 Subject: [PATCH 3/4] format --- src/Service.Tests/UnitTests/SqlResponseHelpersUnitTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Service.Tests/UnitTests/SqlResponseHelpersUnitTests.cs b/src/Service.Tests/UnitTests/SqlResponseHelpersUnitTests.cs index ec87ce5ab6..5563095bfa 100644 --- a/src/Service.Tests/UnitTests/SqlResponseHelpersUnitTests.cs +++ b/src/Service.Tests/UnitTests/SqlResponseHelpersUnitTests.cs @@ -250,9 +250,9 @@ private static FindRequestContext CreateContext(List fieldsToBeReturned, FindRequestContext context = new(entityName: ENTITY_NAME, dbo: dbObject, isList: true) { - First = first + First = first, + FieldsToBeReturned = fieldsToBeReturned }; - context.FieldsToBeReturned = fieldsToBeReturned; return context; } From e347cab4025c46f6838467a41325ae07bbfef878 Mon Sep 17 00:00:00 2001 From: aaronburtle Date: Thu, 7 May 2026 04:14:59 -0700 Subject: [PATCH 4/4] revert signatures --- src/Core/Resolvers/SqlPaginationUtil.cs | 15 ++-- src/Core/Resolvers/SqlResponseHelpers.cs | 22 ++++-- .../UnitTests/SqlResponseHelpersUnitTests.cs | 70 ++++++++++++++++--- 3 files changed, 87 insertions(+), 20 deletions(-) diff --git a/src/Core/Resolvers/SqlPaginationUtil.cs b/src/Core/Resolvers/SqlPaginationUtil.cs index 07c433ea0a..6b7dce7c90 100644 --- a/src/Core/Resolvers/SqlPaginationUtil.cs +++ b/src/Core/Resolvers/SqlPaginationUtil.cs @@ -641,13 +641,14 @@ public static string BuildQueryStringWithAfterToken(NameValueCollection? querySt } /// - /// Builds the next-page URI used for cursor-based pagination. + /// Returns the next-page link for cursor-based pagination as a + /// wrapping a single-element array of the form [ { "nextLink": "..." } ]. /// /// The base pagination URI. /// The query string with the $after value already merged in. /// True to return only the path + query (no host); false for an absolute URL. - /// The next-page URI as a string. - public static string BuildNextLinkUri(string baseUri, string queryString, bool isNextLinkRelative = false) + /// JsonElement wrapping the next-page URL. + public static JsonElement GetConsolidatedNextLinkForPagination(string baseUri, string queryString, bool isNextLinkRelative = false) { UriBuilder uriBuilder = new(baseUri) { @@ -656,9 +657,15 @@ public static string BuildNextLinkUri(string baseUri, string queryString, bool i }; // Construct final link- absolute or relative - return isNextLinkRelative + string nextLinkValue = isNextLinkRelative ? uriBuilder.Uri.PathAndQuery // returns just "/api/?$after...", no host : uriBuilder.Uri.AbsoluteUri; // returns full URL + + string jsonString = JsonSerializer.Serialize(new[] + { + new { nextLink = nextLinkValue } + }); + return JsonSerializer.Deserialize(jsonString); } /// diff --git a/src/Core/Resolvers/SqlResponseHelpers.cs b/src/Core/Resolvers/SqlResponseHelpers.cs index b5266a49f6..1ba0c8b639 100644 --- a/src/Core/Resolvers/SqlResponseHelpers.cs +++ b/src/Core/Resolvers/SqlResponseHelpers.cs @@ -111,10 +111,14 @@ public static OkObjectResult FormatFindResult( string queryString = SqlPaginationUtil.BuildQueryStringWithAfterToken( queryStringParameters: context.ParsedQueryString, newAfterPayload: after); - string nextLink = SqlPaginationUtil.BuildNextLinkUri( - baseUri: basePaginationUri, - queryString: queryString, - isNextLinkRelative: runtimeConfig.NextLinkRelative()); + UriBuilder uriBuilder = new(basePaginationUri) + { + // Form final link by appending the query string + Query = queryString + }; + string nextLink = runtimeConfig.NextLinkRelative() + ? uriBuilder.Uri.PathAndQuery // returns just "/api/?$after...", no host + : uriBuilder.Uri.AbsoluteUri; // returns full URL return new OkObjectResult(new { @@ -208,11 +212,19 @@ private static JsonElement RemoveExtraFieldsInResponseWithSingleItem(JsonElement /// shape of . attaches those fields /// out-of-band when needed. This avoids confusing array-typed column values (e.g. SQL Server /// JSON arrays, vector/collection types) with a pagination sentinel. + /// + /// is accepted for source compatibility with prior versions + /// of Microsoft.DataApiBuilder.Core but is no longer used: the envelope shape produced + /// here is identical for REST and MCP, and pagination metadata is built by + /// . /// /// Value representing the Json results of the client's request. + /// Unused; preserved for backwards-compatible call sites. /// Correctly formatted OkObjectResult. - public static OkObjectResult OkResponse(JsonElement jsonResult) + public static OkObjectResult OkResponse(JsonElement jsonResult, bool? isMcpRequest = null) { + _ = isMcpRequest; // intentionally unused; kept for source compatibility. + // For consistency we always return the payload as an array under "value". List rows = jsonResult.ValueKind is JsonValueKind.Array ? jsonResult.EnumerateArray().ToList() diff --git a/src/Service.Tests/UnitTests/SqlResponseHelpersUnitTests.cs b/src/Service.Tests/UnitTests/SqlResponseHelpersUnitTests.cs index 5563095bfa..1cfee324c6 100644 --- a/src/Service.Tests/UnitTests/SqlResponseHelpersUnitTests.cs +++ b/src/Service.Tests/UnitTests/SqlResponseHelpersUnitTests.cs @@ -18,14 +18,17 @@ namespace Azure.DataApiBuilder.Service.Tests.UnitTests /// /// Focused unit tests for . /// - /// These tests pin the response envelope shape across the five paths the method takes: + /// Tests 1-5 pin the response envelope shape across each path the method takes: /// (1) empty result, (2) single object, (3) collection without next page, /// (4) collection with next page (REST), (5) collection with next page (MCP). /// - /// Test 6 is a regression guard against confusing array-typed column values with the old - /// pagination "shape sentinel" — it pins that a row containing an array-valued property - /// (e.g. a SQL Server JSON array, vector, or other collection-typed column) is returned - /// unmodified and is NOT misinterpreted as a pagination marker. + /// Test 6 documents that rows containing array-valued columns (the shape enabled by + /// SQL Server's JSON/vector types) round-trip through the response pipeline unchanged. + /// + /// Test 7 is the load-bearing regression guard for the shape-sentinel removal: + /// it pins that a result whose last top-level element is itself a JSON array — the + /// exact trigger the pre-refactor used to + /// detect a pagination sentinel — is now correctly treated as ordinary data. /// [TestClass] public class SqlResponseHelpersUnitTests @@ -176,14 +179,15 @@ public void FormatFindResult_CollectionWithNextPage_Mcp_ReturnsAfterEnvelope() } /// - /// Regression guard for the shape-sentinel removal: - /// a row whose last column is an array-valued JSON value (e.g. a SQL Server JSON array - /// or vector/collection column) must be returned verbatim and must NOT be confused - /// with a pagination marker. Pre-refactor, the in-band sentinel detection in - /// would have misclassified this shape. + /// Pins that rows containing array-valued columns (e.g. a SQL Server JSON array, vector, + /// or other collection-typed column) round-trip through the response pipeline unchanged. + /// This is forward-looking coverage for query shapes enabled by SQL Server's JSON/vector + /// types: the array values live inside object-shaped rows, so this case did not actually + /// trigger the pre-refactor shape sentinel — but it documents the supported shape and + /// guards against future regressions in extra-field stripping or envelope construction. /// [TestMethod] - public void FormatFindResult_RowWithArrayColumn_NotMisclassifiedAsPagination() + public void FormatFindResult_RowWithArrayColumn_RoundTripsUnchanged() { // Two rows with array-valued "tags" column. first=5 so HasNext=false. JsonElement input = ParseJson(@"[ @@ -214,6 +218,50 @@ public void FormatFindResult_RowWithArrayColumn_NotMisclassifiedAsPagination() Assert.AreEqual("fantasy", value[1].GetProperty("tags")[0].GetString()); } + /// + /// Regression guard for the actual shape-sentinel failure mode: when the result list's + /// last top-level element is itself a JSON array (a non-object row, as could be produced + /// by future query shapes that project array-typed values at the row level), the response + /// must be returned verbatim under value. Pre-refactor, + /// inspected JsonValueKind.Array on the last element and would have attempted to + /// unpack it as a { "nextLink" } / { "after" } sentinel, producing an + /// incorrect envelope. With shape-based detection removed, the array element is now + /// correctly treated as ordinary data. + /// + [TestMethod] + public void FormatFindResult_TopLevelArrayTailRow_IsNotMisclassifiedAsPaginationSentinel() + { + // Last top-level element is a JSON array — the exact shape the old in-band sentinel + // detection used as its trigger. first=10 keeps HasNext=false so the no-pagination + // path is taken; without the refactor, OkResponse would have misfired here. + JsonElement input = ParseJson(@"[ + { ""id"": 1 }, + { ""id"": 2 }, + [ 1, 2, 3 ] + ]"); + FindRequestContext context = CreateContext( + fieldsToBeReturned: new List { "id" }, + first: 10); + + OkObjectResult result = SqlResponseHelpers.FormatFindResult( + findOperationResponse: input, + context: context, + sqlMetadataProvider: Mock.Of(), + runtimeConfig: CreateRuntimeConfig(), + httpContext: new DefaultHttpContext()); + + JsonElement envelope = SerializeValue(result); + AssertHasNoPaginationFields(envelope); + + JsonElement value = envelope.GetProperty("value"); + Assert.AreEqual(3, value.GetArrayLength(), + "All three top-level elements must be preserved; the trailing array must NOT be unpacked as a pagination sentinel."); + Assert.AreEqual(JsonValueKind.Object, value[0].ValueKind); + Assert.AreEqual(JsonValueKind.Object, value[1].ValueKind); + Assert.AreEqual(JsonValueKind.Array, value[2].ValueKind); + Assert.AreEqual(3, value[2].GetArrayLength()); + } + #endregion #region Helpers