Skip to content

Commit ee2ce9e

Browse files
anushakolanCopilotRubenCerna2079
authored
Bug fix for pagination nested entities resulting key not found error. (#3029)
## Why make this change? We are addressing 2 related issues in this PR: Issue #2374 – Nested sibling relationships under books (websiteplacement, reviews, authors) **Problem**: A nested query on books where a parent has multiple sibling relationships (for example, websiteplacement, reviews, and authors) could throw a `KeyNotFoundException` when RBAC or shape changes were involved. Pagination metadata was stored using only the root and the depth in the path, so different sibling relationships at the same depth could overwrite each other or look up the wrong entry. **Solution**: We now key pagination metadata by both depth and the full relationship path (for example, “books → items → reviews” vs “books → items → authors”), so each sibling branch gets its own unique entry. Reads use the same full-path key, and if metadata for a branch is missing, we return an “empty” `PaginationMetadata` instead of throwing. This prevents collisions between sibling relationships and avoids runtime errors when a particular branch has no metadata. Issue #3026 – Person's graph (AddressType / PhoneNumberType) **Problem**: In the persons graph, a query selecting persons → addresses.items.AddressType and persons → phoneNumbers.items.PhoneNumberType could also throw a `KeyNotFoundException`. In some cases (for example, when RBAC removes a relationship or when that relationship is not paginated at all), there is legitimately no pagination metadata for that nested field, but the code assumed it always existed and indexed into the dictionary directly. **Solution**: Metadata handling is now defensive in two places: In the GraphQL execution helper, metadata lookups for object and list fields use safe TryGet-style access; if an entry isn’t present, we fall back to an empty PaginationMetadata instead of failing. In the SQL query engine’s object resolver, we first check whether there is a subquery metadata entry for the field. If there isn’t, we treat the field as non‑paginated and return the JSON as-is rather than throwing. Together, these changes fix both issues by (a) using full path-based keys, so sibling branches don’t conflict, (b) treating missing metadata as “no pagination here” rather than as a fatal error. ## What is this change? 1. In `SqlQueryEngine.ResolveObject`, instead of always doing `parentMetadata.Subqueries[fieldName]` (which crashed when RBAC caused that entry to be missing), it now uses `TryGetValue` and: - If metadata exists and `IsPaginated` is true -> wrap the JSON as a pagination connection. - If metadata is missing -> just return the JSON as-is (no exception). 2. Introduced `GetRelationshipPathSuffix(HotChocolate.Path path)` to build a relationship path suffix like: - `rel1` for `/entity/items[0]/rel1` - `rel1::nested` for `/entity/items[0]/rel1/nested` 3. `SetNewMetadataChildren`, now stores child metadata under keys of the form - `root_PURE_RESOLVER_CTX::depth::relationshipPath`, ensuring siblings at the same depth get distinct entries. 5. `GetMetadata` (used for list items fields): - For `Selection.ResponseName == "items"` and non-root paths, now looks up: a. `GetMetadataKey(context.Path) + "::" + context.Path.Parent.Depth()` plus the relationship suffix from `GetRelationshipPathSuffix(context.Path.Parent)`. b. Uses `ContextData.TryGetValue(...)` and falls back to `PaginationMetadata.MakeEmptyPaginationMetadata()` when metadata is missing (e.g. Cosmos, pruned relationships). 6. `GetMetadataObjectField` (used for object fields like addresses, AddressType, PhoneNumberType): Updated all branches (indexer, nested non-root, root) to: - Append the relationship suffix to the base key (so keys align with `SetNewMetadataChildren`). - Use `ContextData.TryGetValue(...)` instead of direct indexing, return `PaginationMetadata.MakeEmptyPaginationMetadata()` when no metadata exists, instead of throwing. 7. Added a new test case in `MsSqlGraphQLQueryTests`, an integration test which queries books with multiple sibling nested relationships (websiteplacement, reviews, authors) under the authenticated role to: - Assert no KeyNotFoundException, - Verify all nested branches return data. ## How was this tested? Tested both manually and added an integration test (NestedReviewsConnection_WithSiblings_PaginatesMoreThanHundredItems). Manually if we run this query without the bug fix: `query { persons { items { PersonID FirstName LastName addresses { items { AddressID City AddressType { AddressTypeID TypeName } } } phoneNumbers { items { PhoneNumberID PhoneNumber PhoneNumberType { PhoneNumberTypeID TypeName } } } } } }` We get the following response: `{ "errors": [ { "message": "The given key 'AddressType' was not present in the dictionary.", "locations": [ { "line": 11, "column": 11 } ], "path": [ "persons", "items", 0, "addresses", "items", 1, "AddressType" ] }, { "message": "The given key 'AddressType' was not present in the dictionary.", "locations": [ { "line": 11, "column": 11 } ], "path": [ "persons", "items", 0, "addresses", "items", 0, "AddressType" ] }, { "message": "The given key 'AddressType' was not present in the dictionary.", "locations": [ { "line": 11, "column": 11 } ], "path": [ "persons", "items", 1, "addresses", "items", 0, "AddressType" ] } ], "data": { "persons": { "items": [ { "PersonID": 1, "FirstName": "John", "LastName": "Doe", "addresses": { "items": [ { "AddressID": 1, "City": "New York", "AddressType": null }, { "AddressID": 2, "City": "New York", "AddressType": null } ] }, "phoneNumbers": { "items": [ { "PhoneNumberID": 1, "PhoneNumber": "123-456-7890", "PhoneNumberType": { "PhoneNumberTypeID": 1, "TypeName": "Mobile" } }, { "PhoneNumberID": 2, "PhoneNumber": "111-222-3333", "PhoneNumberType": { "PhoneNumberTypeID": 3, "TypeName": "Work" } } ] } }, { "PersonID": 2, "FirstName": "Jane", "LastName": "Smith", "addresses": { "items": [ { "AddressID": 3, "City": "Los Angeles", "AddressType": null } ] }, "phoneNumbers": { "items": [ { "PhoneNumberID": 3, "PhoneNumber": "987-654-3210", "PhoneNumberType": { "PhoneNumberTypeID": 2, "TypeName": "Home" } } ] } } ] } } }` After the bug fix, we get, `{ "data": { "persons": { "items": [ { "PersonID": 1, "FirstName": "John", "LastName": "Doe", "addresses": { "items": [ { "AddressID": 1, "City": "New York", "AddressType": { "AddressTypeID": 1, "TypeName": "Home" } }, { "AddressID": 2, "City": "New York", "AddressType": { "AddressTypeID": 2, "TypeName": "Work" } } ] }, "phoneNumbers": { "items": [ { "PhoneNumberID": 1, "PhoneNumber": "123-456-7890", "PhoneNumberType": { "PhoneNumberTypeID": 1, "TypeName": "Mobile" } }, { "PhoneNumberID": 2, "PhoneNumber": "111-222-3333", "PhoneNumberType": { "PhoneNumberTypeID": 3, "TypeName": "Work" } } ] } }, { "PersonID": 2, "FirstName": "Jane", "LastName": "Smith", "addresses": { "items": [ { "AddressID": 3, "City": "Los Angeles", "AddressType": { "AddressTypeID": 1, "TypeName": "Home" } } ] }, "phoneNumbers": { "items": [ { "PhoneNumberID": 3, "PhoneNumber": "987-654-3210", "PhoneNumberType": { "PhoneNumberTypeID": 2, "TypeName": "Home" } } ] } } ] } } }` ## Sample Request(s) - Example REST and/or GraphQL request to demonstrate modifications - Example of CLI usage to demonstrate modifications --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: RubenCerna2079 <[email protected]>
1 parent 5b43e2b commit ee2ce9e

3 files changed

Lines changed: 331 additions & 10 deletions

File tree

‎src/Core/Resolvers/SqlQueryEngine.cs‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -224,12 +224,18 @@ public JsonElement ResolveObject(JsonElement element, ObjectField fieldSchema, r
224224
parentMetadata = paginationObjectMetadata;
225225
}
226226

227-
PaginationMetadata currentMetadata = parentMetadata.Subqueries[fieldSchema.Name];
228-
metadata = currentMetadata;
229-
230-
if (currentMetadata.IsPaginated)
227+
// In some scenarios (for example when RBAC removes a relationship
228+
// or when multiple sibling nested entities are present), we may not
229+
// have pagination metadata for the current field. In those cases we
230+
// should simply return the element as-is instead of throwing.
231+
if (parentMetadata.Subqueries.TryGetValue(fieldSchema.Name, out PaginationMetadata? currentMetadata))
231232
{
232-
return SqlPaginationUtil.CreatePaginationConnectionFromJsonElement(element, currentMetadata);
233+
metadata = currentMetadata;
234+
235+
if (currentMetadata.IsPaginated)
236+
{
237+
return SqlPaginationUtil.CreatePaginationConnectionFromJsonElement(element, currentMetadata);
238+
}
233239
}
234240
}
235241

‎src/Core/Services/ExecutionHelper.cs‎

Lines changed: 125 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Diagnostics;
55
using System.Globalization;
66
using System.Net;
7+
using System.Text;
78
using System.Text.Json;
89
using Azure.DataApiBuilder.Config.ObjectModel;
910
using Azure.DataApiBuilder.Core.Configurations;
@@ -534,7 +535,24 @@ public static InputObjectType InputObjectTypeFromIInputField(IInputValueDefiniti
534535
// /books/items/items[idx]/authors -> Depth: 3 (0-indexed) which maps to the
535536
// pagination metadata for the "authors/items" subquery.
536537
string paginationObjectParentName = GetMetadataKey(context.Path) + "::" + context.Path.Parent.Depth();
537-
return (IMetadata?)context.ContextData[paginationObjectParentName];
538+
539+
// For nested list fields under relationships (e.g. reviews.items, authors.items),
540+
// include the relationship path suffix so we look up the same key that
541+
// SetNewMetadataChildren stored ("::depth::relationshipPath").
542+
string relationshipPath = GetRelationshipPathSuffix(context.Path.Parent);
543+
if (!string.IsNullOrEmpty(relationshipPath))
544+
{
545+
paginationObjectParentName = paginationObjectParentName + "::" + relationshipPath;
546+
}
547+
548+
if (context.ContextData.TryGetValue(key: paginationObjectParentName, out object? itemsPaginationMetadata) && itemsPaginationMetadata is not null)
549+
{
550+
return (IMetadata)itemsPaginationMetadata;
551+
}
552+
553+
// If metadata is missing (e.g. Cosmos DB or pruned relationship), return an empty
554+
// pagination metadata object to avoid KeyNotFoundException.
555+
return PaginationMetadata.MakeEmptyPaginationMetadata();
538556
}
539557

540558
// This section would be reached when processing a Cosmos query of the form:
@@ -582,7 +600,24 @@ private static IMetadata GetMetadataObjectField(IResolverContext context)
582600
// pagination metadata from context.ContextData
583601
// The PaginationMetadata fetched has subquery metadata for "authors" from path "/books/items/authors"
584602
string objectParentName = GetMetadataKey(context.Path) + "::" + context.Path.Parent.Parent.Depth();
585-
return (IMetadata)context.ContextData[objectParentName]!;
603+
604+
// Include relationship path suffix (for example, "addresses" or "phoneNumbers") so
605+
// we look up the same key that SetNewMetadataChildren stored
606+
// ("::depth::relationshipPath").
607+
string relationshipPath = GetRelationshipPathSuffix(context.Path.Parent.Parent);
608+
if (!string.IsNullOrEmpty(relationshipPath))
609+
{
610+
objectParentName = objectParentName + "::" + relationshipPath;
611+
}
612+
613+
if (context.ContextData.TryGetValue(objectParentName, out object? indexerMetadata) && indexerMetadata is not null)
614+
{
615+
return (IMetadata)indexerMetadata;
616+
}
617+
618+
// If no metadata is present (for example, for non-paginated relationships or when
619+
// RBAC prunes a branch), return an empty pagination metadata object.
620+
return PaginationMetadata.MakeEmptyPaginationMetadata();
586621
}
587622

588623
if (!context.Path.IsRootField() && ((NamePathSegment)context.Path.Parent).Name != PURE_RESOLVER_CONTEXT_SUFFIX)
@@ -592,12 +627,35 @@ private static IMetadata GetMetadataObjectField(IResolverContext context)
592627
// e.g. metadata for index 4 will not exist. only 3.
593628
// Depth: / 0 / 1 / 2 / 3 / 4
594629
// Path: /books/items/items[0]/publishers/books
630+
//
631+
// To handle arbitrary nesting depths with sibling relationships, we need to include
632+
// the relationship field path in the key. For example:
633+
// - /entity/items[0]/rel1/nested uses key ::3::rel1
634+
// - /entity/items[0]/rel2/nested uses key ::3::rel2
635+
// - /entity/items[0]/rel1/nested/deeper uses key ::4::rel1::nested
636+
// - /entity/items[0]/rel1/nested2/deeper uses key ::4::rel1::nested2
595637
string objectParentName = GetMetadataKey(context.Path.Parent) + "::" + context.Path.Parent.Depth();
596-
return (IMetadata)context.ContextData[objectParentName]!;
638+
string relationshipPath = GetRelationshipPathSuffix(context.Path.Parent);
639+
if (!string.IsNullOrEmpty(relationshipPath))
640+
{
641+
objectParentName = objectParentName + "::" + relationshipPath;
642+
}
643+
644+
if (context.ContextData.TryGetValue(objectParentName, out object? nestedMetadata) && nestedMetadata is not null)
645+
{
646+
return (IMetadata)nestedMetadata;
647+
}
648+
649+
return PaginationMetadata.MakeEmptyPaginationMetadata();
597650
}
598651

599652
string metadataKey = GetMetadataKey(context.Path) + "::" + context.Path.Depth();
600-
return (IMetadata)context.ContextData[metadataKey]!;
653+
if (context.ContextData.TryGetValue(metadataKey, out object? rootMetadata) && rootMetadata is not null)
654+
{
655+
return (IMetadata)rootMetadata;
656+
}
657+
658+
return PaginationMetadata.MakeEmptyPaginationMetadata();
601659
}
602660

603661
private static string GetMetadataKey(HotChocolate.Path path)
@@ -614,6 +672,50 @@ private static string GetMetadataKey(HotChocolate.Path path)
614672
return GetMetadataKey(path: path.Parent);
615673
}
616674

675+
/// <summary>
676+
/// Builds a suffix representing the relationship path from the IndexerPathSegment (items[n])
677+
/// up to (but not including) the current path segment. This is used to create unique metadata
678+
/// keys for sibling relationships at any nesting depth.
679+
/// </summary>
680+
/// <param name="path">The path to build the suffix for</param>
681+
/// <returns>
682+
/// A string like "rel1" for /entity/items[0]/rel1,
683+
/// or "rel1::nested" for /entity/items[0]/rel1/nested,
684+
/// or empty string if no IndexerPathSegment is found in the path ancestry.
685+
/// </returns>
686+
private static string GetRelationshipPathSuffix(HotChocolate.Path path)
687+
{
688+
List<string> pathParts = new();
689+
HotChocolate.Path? current = path;
690+
691+
// Walk up the path collecting relationship field names until we hit an IndexerPathSegment
692+
while (current is not null && !current.IsRoot)
693+
{
694+
if (current is IndexerPathSegment)
695+
{
696+
// We've reached items[n], stop here
697+
break;
698+
}
699+
700+
if (current is NamePathSegment nameSegment)
701+
{
702+
pathParts.Add(nameSegment.Name);
703+
}
704+
705+
current = current.Parent;
706+
}
707+
708+
// If we didn't find an IndexerPathSegment, return empty (this handles root-level queries)
709+
if (current is not IndexerPathSegment)
710+
{
711+
return string.Empty;
712+
}
713+
714+
// Reverse because we walked up the tree, but we want the path from root to leaf
715+
pathParts.Reverse();
716+
return string.Join("::", pathParts);
717+
}
718+
617719
/// <summary>
618720
/// Resolves the name of the root object of a selection set to
619721
/// 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?
655757
// When context.Path takes the form: "/entity/items[index]/nestedEntity" HC counts the depth as
656758
// if the path took the form: "/entity/items/items[index]/nestedEntity" -> Depth of "nestedEntity"
657759
// is 3 because depth is 0-indexed.
658-
string contextKey = GetMetadataKey(context.Path) + "::" + context.Path.Depth();
760+
StringBuilder contextKeyBuilder = new();
761+
contextKeyBuilder
762+
.Append(GetMetadataKey(context.Path))
763+
.Append("::")
764+
.Append(context.Path.Depth());
765+
766+
// For relationship fields at any depth, include the relationship path suffix to distinguish
767+
// between sibling relationships. This handles arbitrary nesting depths.
768+
// e.g., "/entity/items[0]/rel1" gets key ::3::rel1
769+
// e.g., "/entity/items[0]/rel1/nested" gets key ::4::rel1::nested
770+
string relationshipPath = GetRelationshipPathSuffix(context.Path);
771+
if (!string.IsNullOrEmpty(relationshipPath))
772+
{
773+
contextKeyBuilder
774+
.Append("::")
775+
.Append(relationshipPath);
776+
}
777+
778+
string contextKey = contextKeyBuilder.ToString();
659779

660780
// It's okay to overwrite the context when we are visiting a different item in items e.g. books/items/items[1]/publishers since
661781
// context for books/items/items[0]/publishers processing is done and that context isn't needed anymore.

0 commit comments

Comments
 (0)