Skip to content

Commit e347cab

Browse files
committed
revert signatures
1 parent 192c6d4 commit e347cab

3 files changed

Lines changed: 87 additions & 20 deletions

File tree

src/Core/Resolvers/SqlPaginationUtil.cs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -641,13 +641,14 @@ public static string BuildQueryStringWithAfterToken(NameValueCollection? querySt
641641
}
642642

643643
/// <summary>
644-
/// Builds the next-page URI used for cursor-based pagination.
644+
/// Returns the next-page link for cursor-based pagination as a <see cref="JsonElement"/>
645+
/// wrapping a single-element array of the form <c>[ { "nextLink": "..." } ]</c>.
645646
/// </summary>
646647
/// <param name="baseUri">The base pagination URI.</param>
647648
/// <param name="queryString">The query string with the $after value already merged in.</param>
648649
/// <param name="isNextLinkRelative">True to return only the path + query (no host); false for an absolute URL.</param>
649-
/// <returns>The next-page URI as a string.</returns>
650-
public static string BuildNextLinkUri(string baseUri, string queryString, bool isNextLinkRelative = false)
650+
/// <returns>JsonElement wrapping the next-page URL.</returns>
651+
public static JsonElement GetConsolidatedNextLinkForPagination(string baseUri, string queryString, bool isNextLinkRelative = false)
651652
{
652653
UriBuilder uriBuilder = new(baseUri)
653654
{
@@ -656,9 +657,15 @@ public static string BuildNextLinkUri(string baseUri, string queryString, bool i
656657
};
657658

658659
// Construct final link- absolute or relative
659-
return isNextLinkRelative
660+
string nextLinkValue = isNextLinkRelative
660661
? uriBuilder.Uri.PathAndQuery // returns just "/api/<Entity>?$after...", no host
661662
: uriBuilder.Uri.AbsoluteUri; // returns full URL
663+
664+
string jsonString = JsonSerializer.Serialize(new[]
665+
{
666+
new { nextLink = nextLinkValue }
667+
});
668+
return JsonSerializer.Deserialize<JsonElement>(jsonString);
662669
}
663670

664671
/// <summary>

src/Core/Resolvers/SqlResponseHelpers.cs

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,14 @@ public static OkObjectResult FormatFindResult(
111111
string queryString = SqlPaginationUtil.BuildQueryStringWithAfterToken(
112112
queryStringParameters: context.ParsedQueryString,
113113
newAfterPayload: after);
114-
string nextLink = SqlPaginationUtil.BuildNextLinkUri(
115-
baseUri: basePaginationUri,
116-
queryString: queryString,
117-
isNextLinkRelative: runtimeConfig.NextLinkRelative());
114+
UriBuilder uriBuilder = new(basePaginationUri)
115+
{
116+
// Form final link by appending the query string
117+
Query = queryString
118+
};
119+
string nextLink = runtimeConfig.NextLinkRelative()
120+
? uriBuilder.Uri.PathAndQuery // returns just "/api/<Entity>?$after...", no host
121+
: uriBuilder.Uri.AbsoluteUri; // returns full URL
118122

119123
return new OkObjectResult(new
120124
{
@@ -208,11 +212,19 @@ private static JsonElement RemoveExtraFieldsInResponseWithSingleItem(JsonElement
208212
/// shape of <paramref name="jsonResult"/>. <see cref="FormatFindResult"/> attaches those fields
209213
/// out-of-band when needed. This avoids confusing array-typed column values (e.g. SQL Server
210214
/// JSON arrays, vector/collection types) with a pagination sentinel.
215+
///
216+
/// <paramref name="isMcpRequest"/> is accepted for source compatibility with prior versions
217+
/// of <c>Microsoft.DataApiBuilder.Core</c> but is no longer used: the envelope shape produced
218+
/// here is identical for REST and MCP, and pagination metadata is built by
219+
/// <see cref="FormatFindResult"/>.
211220
/// </summary>
212221
/// <param name="jsonResult">Value representing the Json results of the client's request.</param>
222+
/// <param name="isMcpRequest">Unused; preserved for backwards-compatible call sites.</param>
213223
/// <returns>Correctly formatted OkObjectResult.</returns>
214-
public static OkObjectResult OkResponse(JsonElement jsonResult)
224+
public static OkObjectResult OkResponse(JsonElement jsonResult, bool? isMcpRequest = null)
215225
{
226+
_ = isMcpRequest; // intentionally unused; kept for source compatibility.
227+
216228
// For consistency we always return the payload as an array under "value".
217229
List<JsonElement> rows = jsonResult.ValueKind is JsonValueKind.Array
218230
? jsonResult.EnumerateArray().ToList()

src/Service.Tests/UnitTests/SqlResponseHelpersUnitTests.cs

Lines changed: 59 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,17 @@ namespace Azure.DataApiBuilder.Service.Tests.UnitTests
1818
/// <summary>
1919
/// Focused unit tests for <see cref="SqlResponseHelpers.FormatFindResult"/>.
2020
///
21-
/// These tests pin the response envelope shape across the five paths the method takes:
21+
/// Tests 1-5 pin the response envelope shape across each path the method takes:
2222
/// (1) empty result, (2) single object, (3) collection without next page,
2323
/// (4) collection with next page (REST), (5) collection with next page (MCP).
2424
///
25-
/// Test 6 is a regression guard against confusing array-typed column values with the old
26-
/// pagination "shape sentinel" — it pins that a row containing an array-valued property
27-
/// (e.g. a SQL Server JSON array, vector, or other collection-typed column) is returned
28-
/// unmodified and is NOT misinterpreted as a pagination marker.
25+
/// Test 6 documents that rows containing array-valued columns (the shape enabled by
26+
/// SQL Server's JSON/vector types) round-trip through the response pipeline unchanged.
27+
///
28+
/// Test 7 is the load-bearing regression guard for the shape-sentinel removal:
29+
/// it pins that a result whose last top-level element is itself a JSON array — the
30+
/// exact trigger the pre-refactor <see cref="SqlResponseHelpers.OkResponse"/> used to
31+
/// detect a pagination sentinel — is now correctly treated as ordinary data.
2932
/// </summary>
3033
[TestClass]
3134
public class SqlResponseHelpersUnitTests
@@ -176,14 +179,15 @@ public void FormatFindResult_CollectionWithNextPage_Mcp_ReturnsAfterEnvelope()
176179
}
177180

178181
/// <summary>
179-
/// Regression guard for the shape-sentinel removal:
180-
/// a row whose last column is an array-valued JSON value (e.g. a SQL Server JSON array
181-
/// or vector/collection column) must be returned verbatim and must NOT be confused
182-
/// with a pagination marker. Pre-refactor, the in-band sentinel detection in
183-
/// <see cref="SqlResponseHelpers.OkResponse"/> would have misclassified this shape.
182+
/// Pins that rows containing array-valued columns (e.g. a SQL Server JSON array, vector,
183+
/// or other collection-typed column) round-trip through the response pipeline unchanged.
184+
/// This is forward-looking coverage for query shapes enabled by SQL Server's JSON/vector
185+
/// types: the array values live inside object-shaped rows, so this case did not actually
186+
/// trigger the pre-refactor shape sentinel — but it documents the supported shape and
187+
/// guards against future regressions in extra-field stripping or envelope construction.
184188
/// </summary>
185189
[TestMethod]
186-
public void FormatFindResult_RowWithArrayColumn_NotMisclassifiedAsPagination()
190+
public void FormatFindResult_RowWithArrayColumn_RoundTripsUnchanged()
187191
{
188192
// Two rows with array-valued "tags" column. first=5 so HasNext=false.
189193
JsonElement input = ParseJson(@"[
@@ -214,6 +218,50 @@ public void FormatFindResult_RowWithArrayColumn_NotMisclassifiedAsPagination()
214218
Assert.AreEqual("fantasy", value[1].GetProperty("tags")[0].GetString());
215219
}
216220

221+
/// <summary>
222+
/// Regression guard for the actual shape-sentinel failure mode: when the result list's
223+
/// last top-level element is itself a JSON array (a non-object row, as could be produced
224+
/// by future query shapes that project array-typed values at the row level), the response
225+
/// must be returned verbatim under <c>value</c>. Pre-refactor, <see cref="SqlResponseHelpers.OkResponse"/>
226+
/// inspected <c>JsonValueKind.Array</c> on the last element and would have attempted to
227+
/// unpack it as a <c>{ "nextLink" }</c> / <c>{ "after" }</c> sentinel, producing an
228+
/// incorrect envelope. With shape-based detection removed, the array element is now
229+
/// correctly treated as ordinary data.
230+
/// </summary>
231+
[TestMethod]
232+
public void FormatFindResult_TopLevelArrayTailRow_IsNotMisclassifiedAsPaginationSentinel()
233+
{
234+
// Last top-level element is a JSON array — the exact shape the old in-band sentinel
235+
// detection used as its trigger. first=10 keeps HasNext=false so the no-pagination
236+
// path is taken; without the refactor, OkResponse would have misfired here.
237+
JsonElement input = ParseJson(@"[
238+
{ ""id"": 1 },
239+
{ ""id"": 2 },
240+
[ 1, 2, 3 ]
241+
]");
242+
FindRequestContext context = CreateContext(
243+
fieldsToBeReturned: new List<string> { "id" },
244+
first: 10);
245+
246+
OkObjectResult result = SqlResponseHelpers.FormatFindResult(
247+
findOperationResponse: input,
248+
context: context,
249+
sqlMetadataProvider: Mock.Of<ISqlMetadataProvider>(),
250+
runtimeConfig: CreateRuntimeConfig(),
251+
httpContext: new DefaultHttpContext());
252+
253+
JsonElement envelope = SerializeValue(result);
254+
AssertHasNoPaginationFields(envelope);
255+
256+
JsonElement value = envelope.GetProperty("value");
257+
Assert.AreEqual(3, value.GetArrayLength(),
258+
"All three top-level elements must be preserved; the trailing array must NOT be unpacked as a pagination sentinel.");
259+
Assert.AreEqual(JsonValueKind.Object, value[0].ValueKind);
260+
Assert.AreEqual(JsonValueKind.Object, value[1].ValueKind);
261+
Assert.AreEqual(JsonValueKind.Array, value[2].ValueKind);
262+
Assert.AreEqual(3, value[2].GetArrayLength());
263+
}
264+
217265
#endregion
218266

219267
#region Helpers

0 commit comments

Comments
 (0)