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/Core/Services/ExecutionHelper.cs b/src/Core/Services/ExecutionHelper.cs index a0a81f02cc..ece32cac1b 100644 --- a/src/Core/Services/ExecutionHelper.cs +++ b/src/Core/Services/ExecutionHelper.cs @@ -4,6 +4,7 @@ using System.Diagnostics; using System.Globalization; using System.Net; +using System.Text; using System.Text.Json; using Azure.DataApiBuilder.Config.ObjectModel; using Azure.DataApiBuilder.Core.Configurations; @@ -534,7 +535,24 @@ public static InputObjectType InputObjectTypeFromIInputField(IInputValueDefiniti // /books/items/items[idx]/authors -> Depth: 3 (0-indexed) which maps to the // pagination metadata for the "authors/items" subquery. string paginationObjectParentName = GetMetadataKey(context.Path) + "::" + context.Path.Parent.Depth(); - return (IMetadata?)context.ContextData[paginationObjectParentName]; + + // For nested list fields under relationships (e.g. reviews.items, authors.items), + // include the relationship path suffix so we look up the same key that + // SetNewMetadataChildren stored ("::depth::relationshipPath"). + string relationshipPath = GetRelationshipPathSuffix(context.Path.Parent); + if (!string.IsNullOrEmpty(relationshipPath)) + { + paginationObjectParentName = paginationObjectParentName + "::" + relationshipPath; + } + + if (context.ContextData.TryGetValue(key: paginationObjectParentName, out object? itemsPaginationMetadata) && itemsPaginationMetadata is not null) + { + return (IMetadata)itemsPaginationMetadata; + } + + // If metadata is missing (e.g. Cosmos DB or pruned relationship), return an empty + // pagination metadata object to avoid KeyNotFoundException. + return PaginationMetadata.MakeEmptyPaginationMetadata(); } // This section would be reached when processing a Cosmos query of the form: @@ -582,7 +600,24 @@ private static IMetadata GetMetadataObjectField(IResolverContext context) // pagination metadata from context.ContextData // The PaginationMetadata fetched has subquery metadata for "authors" from path "/books/items/authors" string objectParentName = GetMetadataKey(context.Path) + "::" + context.Path.Parent.Parent.Depth(); - return (IMetadata)context.ContextData[objectParentName]!; + + // Include relationship path suffix (for example, "addresses" or "phoneNumbers") so + // we look up the same key that SetNewMetadataChildren stored + // ("::depth::relationshipPath"). + string relationshipPath = GetRelationshipPathSuffix(context.Path.Parent.Parent); + if (!string.IsNullOrEmpty(relationshipPath)) + { + objectParentName = objectParentName + "::" + relationshipPath; + } + + if (context.ContextData.TryGetValue(objectParentName, out object? indexerMetadata) && indexerMetadata is not null) + { + return (IMetadata)indexerMetadata; + } + + // If no metadata is present (for example, for non-paginated relationships or when + // RBAC prunes a branch), return an empty pagination metadata object. + return PaginationMetadata.MakeEmptyPaginationMetadata(); } if (!context.Path.IsRootField() && ((NamePathSegment)context.Path.Parent).Name != PURE_RESOLVER_CONTEXT_SUFFIX) @@ -592,12 +627,35 @@ private static IMetadata GetMetadataObjectField(IResolverContext context) // e.g. metadata for index 4 will not exist. only 3. // Depth: / 0 / 1 / 2 / 3 / 4 // Path: /books/items/items[0]/publishers/books + // + // To handle arbitrary nesting depths with sibling relationships, we need to include + // the relationship field path in the key. For example: + // - /entity/items[0]/rel1/nested uses key ::3::rel1 + // - /entity/items[0]/rel2/nested uses key ::3::rel2 + // - /entity/items[0]/rel1/nested/deeper uses key ::4::rel1 + // - /entity/items[0]/rel1/nested2/deeper uses key ::4::rel1::nested2 string objectParentName = GetMetadataKey(context.Path.Parent) + "::" + context.Path.Parent.Depth(); - return (IMetadata)context.ContextData[objectParentName]!; + string relationshipPath = GetRelationshipPathSuffix(context.Path.Parent); + if (!string.IsNullOrEmpty(relationshipPath)) + { + objectParentName = objectParentName + "::" + relationshipPath; + } + + if (context.ContextData.TryGetValue(objectParentName, out object? nestedMetadata) && nestedMetadata is not null) + { + return (IMetadata)nestedMetadata; + } + + return PaginationMetadata.MakeEmptyPaginationMetadata(); } string metadataKey = GetMetadataKey(context.Path) + "::" + context.Path.Depth(); - return (IMetadata)context.ContextData[metadataKey]!; + if (context.ContextData.TryGetValue(metadataKey, out object? rootMetadata) && rootMetadata is not null) + { + return (IMetadata)rootMetadata; + } + + return PaginationMetadata.MakeEmptyPaginationMetadata(); } private static string GetMetadataKey(HotChocolate.Path path) @@ -614,6 +672,50 @@ private static string GetMetadataKey(HotChocolate.Path path) return GetMetadataKey(path: path.Parent); } + /// + /// Builds a suffix representing the relationship path from the IndexerPathSegment (items[n]) + /// up to (but not including) the current path segment. This is used to create unique metadata + /// keys for sibling relationships at any nesting depth. + /// + /// The path to build the suffix for + /// + /// A string like "rel1" for /entity/items[0]/rel1, + /// or "rel1::nested" for /entity/items[0]/rel1/nested, + /// or empty string if no IndexerPathSegment is found in the path ancestry. + /// + private static string GetRelationshipPathSuffix(HotChocolate.Path path) + { + List pathParts = new(); + HotChocolate.Path? current = path; + + // Walk up the path collecting relationship field names until we hit an IndexerPathSegment + while (current is not null && !current.IsRoot) + { + if (current is IndexerPathSegment) + { + // We've reached items[n], stop here + break; + } + + if (current is NamePathSegment nameSegment) + { + pathParts.Add(nameSegment.Name); + } + + current = current.Parent; + } + + // If we didn't find an IndexerPathSegment, return empty (this handles root-level queries) + if (current is not IndexerPathSegment) + { + return string.Empty; + } + + // Reverse because we walked up the tree, but we want the path from root to leaf + pathParts.Reverse(); + return string.Join("::", pathParts); + } + /// /// Resolves the name of the root object of a selection set to /// use as the beginning of a key used to index pagination metadata in the @@ -655,7 +757,25 @@ private static void SetNewMetadataChildren(IResolverContext context, IMetadata? // When context.Path takes the form: "/entity/items[index]/nestedEntity" HC counts the depth as // if the path took the form: "/entity/items/items[index]/nestedEntity" -> Depth of "nestedEntity" // is 3 because depth is 0-indexed. - string contextKey = GetMetadataKey(context.Path) + "::" + context.Path.Depth(); + StringBuilder contextKeyBuilder = new(); + contextKeyBuilder + .Append(GetMetadataKey(context.Path)) + .Append("::") + .Append(context.Path.Depth()); + + // For relationship fields at any depth, include the relationship path suffix to distinguish + // between sibling relationships. This handles arbitrary nesting depths. + // e.g., "/entity/items[0]/rel1" gets key ::3::rel1 + // e.g., "/entity/items[0]/rel1/nested" gets key ::4::rel1::nested + string relationshipPath = GetRelationshipPathSuffix(context.Path); + if (!string.IsNullOrEmpty(relationshipPath)) + { + contextKeyBuilder + .Append("::") + .Append(relationshipPath); + } + + string contextKey = contextKeyBuilder.ToString(); // It's okay to overwrite the context when we are visiting a different item in items e.g. books/items/items[1]/publishers since // context for books/items/items[0]/publishers processing is done and that context isn't needed anymore. diff --git a/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs b/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs index 1d90a4c6f1..876424f0dd 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; @@ -229,6 +231,199 @@ FOR JSON PATH await InFilterOneToOneJoinQuery(msSqlQuery); } + /// + /// Verifies that the nested reviews connection under books correctly paginates + /// when there are more than 100 reviews for a single book, while also + /// exercising sibling navigation properties (websiteplacement and authors) + /// under RBAC in the same request. 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_WithSiblings_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 + title + websiteplacement { + price + } + reviews(first: 100) { + items { + id + } + endCursor + hasNextPage + } + authors { + items { + name + } + } + } + } + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync( + graphQLQuery, + graphQLQueryName, + isAuthenticated: true, + clientRoleHeader: "authenticated"); + + JsonElement book = actual.GetProperty("items")[0]; + JsonElement websiteplacement = book.GetProperty("websiteplacement"); + JsonElement authorsConnection = book.GetProperty("authors"); + 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."); + + // Also validate that sibling navigation properties under RBAC are + // materialized as part of the same query and that their results + // match the outcome of equivalent SQL JSON queries. + Assert.AreEqual(JsonValueKind.Object, websiteplacement.ValueKind, "Expected websiteplacement object to be materialized."); + + JsonElement authorsItems = authorsConnection.GetProperty("items"); + Assert.IsTrue(authorsItems.GetArrayLength() > 0, "Expected authors collection to be materialized with at least one author."); + + // Compare the full nested tree (id, title, websiteplacement, reviews.items, authors.items) + // against an equivalent SQL JSON query. Shape the SQL JSON to match the GraphQL + // "book" object so we can use the actual result directly for comparison. + string fullTreeSql = @" + SELECT + [b].[id] AS [id], + [b].[title] AS [title], + JSON_QUERY(( + SELECT TOP 1 [wp].[price] AS [price] + FROM [dbo].[book_website_placements] AS [wp] + WHERE [wp].[book_id] = [b].[id] + ORDER BY [wp].[id] ASC + FOR JSON PATH, INCLUDE_NULL_VALUES, WITHOUT_ARRAY_WRAPPER + )) AS [websiteplacement], + JSON_QUERY(( + SELECT + JSON_QUERY(( + SELECT TOP 100 [r].[id] AS [id] + FROM [dbo].[reviews] AS [r] + WHERE [r].[book_id] = [b].[id] + ORDER BY [r].[id] ASC + FOR JSON PATH, INCLUDE_NULL_VALUES + )) AS [items] + FOR JSON PATH, INCLUDE_NULL_VALUES, WITHOUT_ARRAY_WRAPPER + )) AS [reviews], + JSON_QUERY(( + SELECT + JSON_QUERY(( + SELECT [a].[name] AS [name] + FROM [dbo].[authors] AS [a] + INNER JOIN [dbo].[book_author_link] AS [bal] + ON [bal].[author_id] = [a].[id] + WHERE [bal].[book_id] = [b].[id] + ORDER BY [a].[id] ASC + FOR JSON PATH, INCLUDE_NULL_VALUES + )) AS [items] + FOR JSON PATH, INCLUDE_NULL_VALUES, WITHOUT_ARRAY_WRAPPER + )) AS [authors] + FROM [dbo].[books] AS [b] + WHERE [b].[id] = 1 + FOR JSON PATH, INCLUDE_NULL_VALUES, WITHOUT_ARRAY_WRAPPER"; + + string expectedFullTreeJson = await GetDatabaseResultAsync(fullTreeSql); + + // Use the actual GraphQL "book" object for comparison, trimming pagination metadata + // (endCursor, hasNextPage) that cannot be reproduced via SQL. + JsonNode actualBookNode = JsonNode.Parse(book.ToString()); + if (actualBookNode is JsonObject bookObject && + bookObject["reviews"] is JsonObject reviewsObject) + { + reviewsObject.Remove("endCursor"); + reviewsObject.Remove("hasNextPage"); + } + + string actualComparableJson = actualBookNode.ToJsonString(); + + SqlTestHelper.PerformTestEqualJsonStrings( + expectedFullTreeJson, + actualComparableJson); + } + 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); + } + } + /// /// Test query on One-To-One relationship when the fields defining /// the relationship in the entity include fields that are mapped in