diff --git a/src/GraphQL.EntityFramework/ConnectionConverter.cs b/src/GraphQL.EntityFramework/ConnectionConverter.cs index 80302220..9e331bf1 100644 --- a/src/GraphQL.EntityFramework/ConnectionConverter.cs +++ b/src/GraphQL.EntityFramework/ConnectionConverter.cs @@ -1,5 +1,27 @@ -static class ConnectionConverter +static class ConnectionConverter { + internal static bool HasOrderingInExpressionTree(Expression expression) + { + if (expression is MethodCallExpression methodCall) + { + var methodName = methodCall.Method.Name; + if (methodName is "OrderBy" or "OrderByDescending" or "ThenBy" or "ThenByDescending") + { + return true; + } + + foreach (var arg in methodCall.Arguments) + { + if (HasOrderingInExpressionTree(arg)) + { + return true; + } + } + } + + return false; + } + public static Connection ApplyConnectionContext(List list, int? first, string? afterString, int? last, string? beforeString) where T : class { @@ -98,7 +120,7 @@ public static async Task> ApplyConnectionContext) + if (queryable is not IOrderedQueryable && !HasOrderingInExpressionTree(queryable.Expression)) { throw new($"Connections require ordering. Either order the IQueryable being passed to AddQueryConnectionField, or use an orderBy in the query. Field: {context.FieldDefinition.Name}"); } @@ -229,4 +251,4 @@ static void Parse(string? afterString, string? beforeString, out int? after, out before = int.Parse(beforeString); } } -} \ No newline at end of file +} diff --git a/src/Tests/IntegrationTests/IntegrationTests.Connection_with_read_only_property_and_navigation.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Connection_with_read_only_property_and_navigation.verified.txt new file mode 100644 index 00000000..9800a93f --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests.Connection_with_read_only_property_and_navigation.verified.txt @@ -0,0 +1,60 @@ +{ + target: { + Data: { + readOnlyEntitiesConnection: { + totalCount: 3, + items: [ + { + firstName: Alice, + computedInDb: Alice A, + readOnlyParent: { + property: Parent1 + } + }, + { + firstName: Bob, + computedInDb: Bob B, + readOnlyParent: { + property: Parent1 + } + } + ] + } + } + }, + sql: [ + { + Text: +select COUNT(*) +from ReadOnlyEntities as r + }, + { + Text: +select r1.Id, + r1.Age, + r1.ComputedInDb, + r1.FirstName, + r1.LastName, + r1.ReadOnlyParentId, + r0.Id, + r0.Property +from (select r.Id, + r.Age, + r.ComputedInDb, + r.FirstName, + r.LastName, + r.ReadOnlyParentId + from ReadOnlyEntities as r + order by r.FirstName + offset @p rows fetch next @p1 rows only) as r1 + left outer join + ReadOnlyParentEntities as r0 + on r1.ReadOnlyParentId = r0.Id +order by r1.FirstName, + Parameters: { + @p: 0, + @p1: 2 + } + } + ] +} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.Connection_with_read_only_property_navigation_and_fragment.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Connection_with_read_only_property_navigation_and_fragment.verified.txt new file mode 100644 index 00000000..9800a93f --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests.Connection_with_read_only_property_navigation_and_fragment.verified.txt @@ -0,0 +1,60 @@ +{ + target: { + Data: { + readOnlyEntitiesConnection: { + totalCount: 3, + items: [ + { + firstName: Alice, + computedInDb: Alice A, + readOnlyParent: { + property: Parent1 + } + }, + { + firstName: Bob, + computedInDb: Bob B, + readOnlyParent: { + property: Parent1 + } + } + ] + } + } + }, + sql: [ + { + Text: +select COUNT(*) +from ReadOnlyEntities as r + }, + { + Text: +select r1.Id, + r1.Age, + r1.ComputedInDb, + r1.FirstName, + r1.LastName, + r1.ReadOnlyParentId, + r0.Id, + r0.Property +from (select r.Id, + r.Age, + r.ComputedInDb, + r.FirstName, + r.LastName, + r.ReadOnlyParentId + from ReadOnlyEntities as r + order by r.FirstName + offset @p rows fetch next @p1 rows only) as r1 + left outer join + ReadOnlyParentEntities as r0 + on r1.ReadOnlyParentId = r0.Id +order by r1.FirstName, + Parameters: { + @p: 0, + @p1: 2 + } + } + ] +} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.SchemaPrint.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.SchemaPrint.verified.txt index e3ae9161..46ba4147 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.SchemaPrint.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.SchemaPrint.verified.txt @@ -37,6 +37,18 @@ where: [WhereExpression!], orderBy: [OrderBy!], ids: [ID!]): ChildConnection! + readOnlyEntitiesConnection( + "Only return edges after the specified cursor." + after: String, + "Specifies the maximum number of edges to return, starting after the cursor specified by 'after', or the first number of edges if 'after' is not specified." + first: Int, + "Only return edges prior to the specified cursor." + before: String, + "Specifies the maximum number of edges to return, starting prior to the cursor specified by 'before', or the last number of edges if 'before' is not specified." + last: Int, + where: [WhereExpression!], + orderBy: [OrderBy!], + ids: [ID!]): ReadOnlyEntityConnection! parentEntitiesFiltered(id: ID, ids: [ID!], where: [WhereExpression!], orderBy: [OrderBy!], skip: Int, take: Int): [FilterParent!]! parentEntitiesConnectionFiltered( "Only return edges after the specified cursor." @@ -413,6 +425,44 @@ type ParentEdge { node: Parent! } +"A connection from an object to a list of objects of type `ReadOnlyEntity`." +type ReadOnlyEntityConnection { + "A count of the total number of objects in this connection, ignoring pagination. This allows a client to fetch the first five objects by passing \"5\" as the argument to `first`, then fetch the total count so it could display \"5 of 83\", for example. In cases where we employ infinite scrolling or don't have an exact count of entries, this field will return `null`." + totalCount: Int + "Information to aid in pagination." + pageInfo: PageInfo! + "A list of all of the edges returned in the connection." + edges: [ReadOnlyEntityEdge] + "A list of all of the objects returned in the connection. This is a convenience field provided for quickly exploring the API; rather than querying for \"{ edges { node } }\" when no edge data is needed, this field can be used instead. Note that when clients like Relay need to fetch the \"cursor\" field on the edge to enable efficient pagination, this shortcut cannot be used, and the full \"{ edges { node } } \" version should be used instead." + items: [ReadOnlyEntity!] +} + +"An edge in a connection from an object to another object of type `ReadOnlyEntity`." +type ReadOnlyEntityEdge { + "A cursor for use in pagination" + cursor: String! + "The item at the end of the edge" + node: ReadOnlyEntity! +} + +type ReadOnlyEntity { + readOnlyParent: ReadOnlyParent + age: Int! + computedInDb: String! + displayName: String! + firstName: String + id: ID! + isAdult: Boolean! + lastName: String + readOnlyParentId: ID +} + +type ReadOnlyParent { + children(id: ID, ids: [ID!], where: [WhereExpression!], orderBy: [OrderBy!], skip: Int, take: Int): [ReadOnlyEntity!]! + id: ID! + property: String +} + type FilterParent { childrenConnection( "Only return edges after the specified cursor." @@ -595,24 +645,6 @@ type Derived implements BaseEntity { status: String } -type ReadOnlyEntity { - readOnlyParent: ReadOnlyParent - age: Int! - computedInDb: String! - displayName: String! - firstName: String - id: ID! - isAdult: Boolean! - lastName: String - readOnlyParentId: ID -} - -type ReadOnlyParent { - children(id: ID, ids: [ID!], where: [WhereExpression!], orderBy: [OrderBy!], skip: Int, take: Int): [ReadOnlyEntity!]! - id: ID! - property: String -} - type ManyToManyLeft { rights(id: ID, ids: [ID!], where: [WhereExpression!], orderBy: [OrderBy!], skip: Int, take: Int): [ManyToManyRight!]! id: String! diff --git a/src/Tests/IntegrationTests/IntegrationTests_connection_ordering.cs b/src/Tests/IntegrationTests/IntegrationTests_connection_ordering.cs new file mode 100644 index 00000000..e5c6b8c8 --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests_connection_ordering.cs @@ -0,0 +1,67 @@ +public partial class IntegrationTests +{ + [Fact] + public async Task Connection_with_read_only_property_and_navigation() + { + // ReadOnlyEntity has read-only properties (DisplayName, IsAdult, ComputedInDb) + // which cause SelectExpressionBuilder to bail (returns null). + // This forces the AddIncludes path, where Include() strips IOrderedQueryable. + // The ordering check in ConnectionConverter should still pass by inspecting + // the expression tree for OrderBy/OrderByDescending. + var query = + """ + { + readOnlyEntitiesConnection(first:2) { + totalCount + items { + firstName + computedInDb + readOnlyParent { + property + } + } + } + } + """; + + var parent = new ReadOnlyParentEntity { Property = "Parent1" }; + var entity1 = new ReadOnlyEntity { FirstName = "Alice", LastName = "A", Age = 25, ReadOnlyParent = parent }; + var entity2 = new ReadOnlyEntity { FirstName = "Bob", LastName = "B", Age = 17, ReadOnlyParent = parent }; + var entity3 = new ReadOnlyEntity { FirstName = "Charlie", LastName = "C", Age = 30, ReadOnlyParent = parent }; + + await using var database = await sqlInstance.Build(); + await RunQuery(database, query, null, null, false, [parent, entity1, entity2, entity3]); + } + + [Fact] + public async Task Connection_with_read_only_property_navigation_and_fragment() + { + var query = + """ + { + readOnlyEntitiesConnection(first:2) { + totalCount + items { + ...readOnlyFields + } + } + } + + fragment readOnlyFields on ReadOnlyEntity { + firstName + computedInDb + readOnlyParent { + property + } + } + """; + + var parent = new ReadOnlyParentEntity { Property = "Parent1" }; + var entity1 = new ReadOnlyEntity { FirstName = "Alice", LastName = "A", Age = 25, ReadOnlyParent = parent }; + var entity2 = new ReadOnlyEntity { FirstName = "Bob", LastName = "B", Age = 17, ReadOnlyParent = parent }; + var entity3 = new ReadOnlyEntity { FirstName = "Charlie", LastName = "C", Age = 30, ReadOnlyParent = parent }; + + await using var database = await sqlInstance.Build(); + await RunQuery(database, query, null, null, false, [parent, entity1, entity2, entity3]); + } +} diff --git a/src/Tests/IntegrationTests/Query.cs b/src/Tests/IntegrationTests/Query.cs index 4eb07060..fd7190d5 100644 --- a/src/Tests/IntegrationTests/Query.cs +++ b/src/Tests/IntegrationTests/Query.cs @@ -80,6 +80,15 @@ public Query(IEfGraphQLService efGraphQlService) name: "childEntitiesConnection", resolve: _ => _.DbContext.ChildEntities.OrderBy(_ => _.Parent)); + // Connection with entity that has read-only properties. + // Projection bails on read-only properties, falling back to AddIncludes. + // Include() strips IOrderedQueryable, testing that the ordering check + // handles this correctly. + efGraphQlService.AddQueryConnectionField( + this, + name: "readOnlyEntitiesConnection", + resolve: _ => _.DbContext.ReadOnlyEntities.OrderBy(_ => _.FirstName)); + AddQueryField( name: "parentEntitiesFiltered", resolve: _ => _.DbContext.FilterParentEntities);