From 451191175cc7f8f5bcd25a95f0a1960104d19ef1 Mon Sep 17 00:00:00 2001 From: Anusha Kolan Date: Fri, 19 Dec 2025 16:25:05 -0800 Subject: [PATCH 1/4] Bug fix for pagination nested entities resulting key not found error. --- src/Core/Resolvers/SqlQueryEngine.cs | 16 +++-- .../MsSqlGraphQLQueryTests.cs | 62 +++++++++++++++++++ 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/src/Core/Resolvers/SqlQueryEngine.cs b/src/Core/Resolvers/SqlQueryEngine.cs index 7b261ecb2b..6523589532 100644 --- a/src/Core/Resolvers/SqlQueryEngine.cs +++ b/src/Core/Resolvers/SqlQueryEngine.cs @@ -224,12 +224,18 @@ public JsonElement ResolveObject(JsonElement element, ObjectField fieldSchema, r parentMetadata = paginationObjectMetadata; } - PaginationMetadata currentMetadata = parentMetadata.Subqueries[fieldSchema.Name]; - metadata = currentMetadata; - - if (currentMetadata.IsPaginated) + // In some scenarios (for example when RBAC removes a relationship + // or when multiple sibling nested entities are present), we may not + // have pagination metadata for the current field. In those cases we + // should simply return the element as-is instead of throwing. + if (parentMetadata.Subqueries.TryGetValue(fieldSchema.Name, out PaginationMetadata? currentMetadata)) { - return SqlPaginationUtil.CreatePaginationConnectionFromJsonElement(element, currentMetadata); + metadata = currentMetadata; + + if (currentMetadata.IsPaginated) + { + return SqlPaginationUtil.CreatePaginationConnectionFromJsonElement(element, currentMetadata); + } } } diff --git a/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs b/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs index 1d90a4c6f1..070990ec34 100644 --- a/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs +++ b/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs @@ -229,6 +229,68 @@ FOR JSON PATH await InFilterOneToOneJoinQuery(msSqlQuery); } + /// + /// Integration-style regression test for nested sibling relationships + /// under RBAC. This exercises the HotChocolate pipeline, SQL query + /// engine and pagination metadata together. + /// + /// It uses the existing Book graph: + /// Book -> websiteplacement (one) + /// -> reviews (many) + /// -> authors (many) + /// and ensures that querying multiple nested navigation branches in a + /// single GraphQL request while authenticated does not result in a + /// KeyNotFoundException and that nested data is present. + /// + [TestMethod] + public async Task NestedSiblingRelationshipsWithRbac_DoNotThrowAndMaterialize() + { + string graphQLQueryName = "books"; + string graphQLQuery = @"query { + books(first: 2) { + items { + id + title + websiteplacement { + price + } + reviews { + items { + id + content + } + } + authors { + items { + name + } + } + } + } + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync( + graphQLQuery, + graphQLQueryName, + isAuthenticated: true, + clientRoleHeader: "authenticated"); + + string response = actual.ToString(); + + // 1) No KeyNotFoundException (historic bug signature) + Assert.IsFalse( + response.Contains("KeyNotFoundException", StringComparison.OrdinalIgnoreCase) || + response.Contains("The given key", StringComparison.OrdinalIgnoreCase), + "GraphQL response should not contain KeyNotFoundException when resolving nested sibling relationships."); + + // 2) Ensure nested branches are actually materialized + Assert.IsTrue( + response.Contains("\"websiteplacement\"", StringComparison.OrdinalIgnoreCase) && + response.Contains("\"reviews\"", StringComparison.OrdinalIgnoreCase) && + response.Contains("\"authors\"", StringComparison.OrdinalIgnoreCase), + "Expected nested websiteplacement, reviews and authors data to be present in the GraphQL response."); + } + /// /// Test query on One-To-One relationship when the fields defining /// the relationship in the entity include fields that are mapped in From 8b6ee7b90b3cfff38e139795b69d258137e9970e Mon Sep 17 00:00:00 2001 From: Anusha Kolan Date: Fri, 19 Dec 2025 17:10:24 -0800 Subject: [PATCH 2/4] Update src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../GraphQLQueryTests/MsSqlGraphQLQueryTests.cs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs b/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs index 070990ec34..5cbcf35a72 100644 --- a/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs +++ b/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs @@ -275,14 +275,17 @@ public async Task NestedSiblingRelationshipsWithRbac_DoNotThrowAndMaterialize() isAuthenticated: true, clientRoleHeader: "authenticated"); - string response = actual.ToString(); - // 1) No KeyNotFoundException (historic bug signature) - Assert.IsFalse( - response.Contains("KeyNotFoundException", StringComparison.OrdinalIgnoreCase) || - response.Contains("The given key", StringComparison.OrdinalIgnoreCase), - "GraphQL response should not contain KeyNotFoundException when resolving nested sibling relationships."); + if (actual.TryGetProperty("errors", out JsonElement errors)) + { + string errorsText = errors.ToString(); + Assert.IsFalse( + errorsText.Contains("KeyNotFoundException", StringComparison.OrdinalIgnoreCase) || + errorsText.Contains("The given key", StringComparison.OrdinalIgnoreCase), + "GraphQL response should not contain KeyNotFoundException when resolving nested sibling relationships."); + } + string response = actual.ToString(); // 2) Ensure nested branches are actually materialized Assert.IsTrue( response.Contains("\"websiteplacement\"", StringComparison.OrdinalIgnoreCase) && From 5cb80b977863cf7ce6eebb5e1b7b5edf832ac249 Mon Sep 17 00:00:00 2001 From: Anusha Kolan Date: Sat, 20 Dec 2025 00:48:38 -0800 Subject: [PATCH 3/4] Addressed comments --- .../MsSqlGraphQLQueryTests.cs | 185 +++++++++++++++++- 1 file changed, 178 insertions(+), 7 deletions(-) diff --git a/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs b/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs index 5cbcf35a72..c102aff0a5 100644 --- a/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs +++ b/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs @@ -4,7 +4,9 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Text; using System.Text.Json; +using System.Text.Json.Nodes; using System.Threading.Tasks; using Azure.DataApiBuilder.Config.ObjectModel; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -285,13 +287,182 @@ public async Task NestedSiblingRelationshipsWithRbac_DoNotThrowAndMaterialize() "GraphQL response should not contain KeyNotFoundException when resolving nested sibling relationships."); } - string response = actual.ToString(); - // 2) Ensure nested branches are actually materialized - Assert.IsTrue( - response.Contains("\"websiteplacement\"", StringComparison.OrdinalIgnoreCase) && - response.Contains("\"reviews\"", StringComparison.OrdinalIgnoreCase) && - response.Contains("\"authors\"", StringComparison.OrdinalIgnoreCase), - "Expected nested websiteplacement, reviews and authors data to be present in the GraphQL response."); + // 2) Compare the materialized nested branches against an equivalent SQL JSON query + string dbQuery = @" + SELECT TOP 2 [table0].[id] AS [id] + ,[table0].[title] AS [title] + ,JSON_QUERY([website_subq].[data]) AS [websiteplacement] + ,JSON_QUERY(COALESCE([reviews_subq].[data], '[]')) AS [reviews] + ,JSON_QUERY(COALESCE([authors_subq].[data], '[]')) AS [authors] + FROM [dbo].[books] AS [table0] + OUTER APPLY ( + SELECT TOP 1 [w].[price] AS [price] + FROM [dbo].[book_website_placements] AS [w] + WHERE [w].[book_id] = [table0].[id] + ORDER BY [w].[id] ASC + FOR JSON PATH + ,INCLUDE_NULL_VALUES + ,WITHOUT_ARRAY_WRAPPER + ) AS [website_subq]([data]) + OUTER APPLY ( + SELECT [r].[id] AS [id], [r].[content] AS [content] + FROM [dbo].[reviews] AS [r] + WHERE [r].[book_id] = [table0].[id] + ORDER BY [r].[id] ASC + FOR JSON PATH + ,INCLUDE_NULL_VALUES + ) AS [reviews_subq]([data]) + OUTER APPLY ( + SELECT [a].[name] AS [name] + FROM [dbo].[book_author_link] AS [l] + JOIN [dbo].[authors] AS [a] ON [a].[id] = [l].[author_id] + WHERE [l].[book_id] = [table0].[id] + ORDER BY [a].[id] ASC + FOR JSON PATH + ,INCLUDE_NULL_VALUES + ) AS [authors_subq]([data]) + WHERE [table0].[id] IN (1, 2) + ORDER BY [table0].[id] ASC + FOR JSON PATH + ,INCLUDE_NULL_VALUES"; + + string expected = await GetDatabaseResultAsync(dbQuery); + + // Normalize the GraphQL response to match the SQL JSON shape by + // unwrapping connection objects (reviews/authors.items). + JsonArray normalizedBooks = new(); + foreach (JsonElement book in actual.GetProperty("items").EnumerateArray()) + { + JsonObject normalizedBook = new() + { + ["id"] = JsonNode.Parse(book.GetProperty("id").GetRawText()), + ["title"] = JsonNode.Parse(book.GetProperty("title").GetRawText()), + ["websiteplacement"] = JsonNode.Parse(book.GetProperty("websiteplacement").GetRawText()), + ["reviews"] = JsonNode.Parse(book.GetProperty("reviews").GetProperty("items").GetRawText()), + ["authors"] = JsonNode.Parse(book.GetProperty("authors").GetProperty("items").GetRawText()) + }; + + normalizedBooks.Add(normalizedBook); + } + + string normalizedActual = normalizedBooks.ToJsonString(); + + SqlTestHelper.PerformTestEqualJsonStrings( + expected, + normalizedActual); + } + + /// + /// Verifies that the nested reviews connection under books correctly paginates + /// when there are more than 100 reviews for a single book. The first page + /// should contain 100 reviews and the endCursor should encode the id of the + /// last review on that page. + /// + [TestMethod] + public async Task NestedReviewsConnection_PaginatesMoreThanHundredItems() + { + // Seed > 100 reviews for book id 1. Use a distinct id range so we can + // clean up without impacting existing rows used by other tests. + StringBuilder sb = new(); + sb.AppendLine("SET IDENTITY_INSERT reviews ON;"); + sb.AppendLine("INSERT INTO reviews(id, book_id, content) VALUES"); + + for (int id = 2000; id <= 2100; id++) + { + string line = $" ({id}, 1, 'Bulk review {id}')"; + if (id < 2100) + { + line += ","; + } + else + { + line += ";"; + } + + sb.AppendLine(line); + } + + sb.AppendLine("SET IDENTITY_INSERT reviews OFF;"); + string seedReviewsSql = sb.ToString(); + + string cleanupReviewsSql = "DELETE FROM reviews WHERE id BETWEEN 2000 AND 2100;"; + + try + { + // Seed additional data for this test only. + await _queryExecutor.ExecuteQueryAsync( + seedReviewsSql, + dataSourceName: string.Empty, + parameters: null, + dataReaderHandler: null); + + string graphQLQueryName = "books"; + string graphQLQuery = @"query { + books(filter: { id: { eq: 1 } }) { + items { + id + reviews(first: 100) { + items { + id + } + endCursor + hasNextPage + } + } + } + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync( + graphQLQuery, + graphQLQueryName, + isAuthenticated: true, + clientRoleHeader: "authenticated"); + + JsonElement book = actual.GetProperty("items")[0]; + JsonElement reviewsConnection = book.GetProperty("reviews"); + JsonElement reviewItems = reviewsConnection.GetProperty("items"); + + // First page should contain exactly 100 reviews when more than 100 exist. + Assert.AreEqual(100, reviewItems.GetArrayLength(), "Expected first page of reviews to contain 100 items."); + + bool hasNextPage = reviewsConnection.GetProperty("hasNextPage").GetBoolean(); + Assert.IsTrue(hasNextPage, "Expected hasNextPage to be true when more than 100 reviews exist."); + + string endCursor = reviewsConnection.GetProperty("endCursor").GetString(); + Assert.IsFalse(string.IsNullOrEmpty(endCursor), "Expected endCursor to be populated when hasNextPage is true."); + + // Decode the opaque cursor and verify it encodes the id of the + // last review on the first page. + int lastReviewIdOnPage = reviewItems[reviewItems.GetArrayLength() - 1].GetProperty("id").GetInt32(); + + string decodedCursorJson = System.Text.Encoding.UTF8.GetString(Convert.FromBase64String(endCursor)); + using JsonDocument cursorDoc = JsonDocument.Parse(decodedCursorJson); + JsonElement cursorArray = cursorDoc.RootElement; + + bool foundIdFieldInCursor = false; + foreach (JsonElement field in cursorArray.EnumerateArray()) + { + if (field.GetProperty("FieldName").GetString() == "id") + { + int cursorId = field.GetProperty("FieldValue").GetInt32(); + Assert.AreEqual(lastReviewIdOnPage, cursorId, "endCursor should encode the id of the last review on the first page."); + foundIdFieldInCursor = true; + break; + } + } + + Assert.IsTrue(foundIdFieldInCursor, "endCursor payload should include a pagination field for id."); + } + finally + { + // Clean up the seeded reviews so other tests relying on the base + // fixture data remain unaffected. + await _queryExecutor.ExecuteQueryAsync( + cleanupReviewsSql, + dataSourceName: string.Empty, + parameters: null, + dataReaderHandler: null); + } } /// From e8b1c2ecb9a15ff8c49c331f48b4a47d1046219f Mon Sep 17 00:00:00 2001 From: Anusha Kolan Date: Mon, 22 Dec 2025 12:31:21 -0800 Subject: [PATCH 4/4] Update src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs Co-authored-by: RubenCerna2079 <32799214+RubenCerna2079@users.noreply.github.com> --- .../MsSqlGraphQLQueryTests.cs | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs b/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs index c102aff0a5..2091c9b6e6 100644 --- a/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs +++ b/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs @@ -251,22 +251,22 @@ public async Task NestedSiblingRelationshipsWithRbac_DoNotThrowAndMaterialize() string graphQLQuery = @"query { books(first: 2) { items { - id - title - websiteplacement { - price - } - reviews { - items { id - content + title + websiteplacement { + price } - } - authors { - items { - name + reviews { + items { + id + content + } + } + authors { + items { + name + } } - } } } }";