Drop the per row context copy in navigation resolvers, and page connections with one query

docs/defining-graphs.md

@@ -351,6 +351,8 @@ Creating a page-able field is supported through [GraphQL Connections](https://gr
 
 Cursors are the zero based index of the edge in the ordered set. `after` and `before` are exclusive, and bound the window of items a page is taken from. `first` keeps that many from the start of the window, then `last` keeps that many from the end, in the order the [Relay specification](https://relay.dev/graphql/connections.htm#sec-Pagination-algorithm) applies them. Edges are always in the set's order, whichever end the page was taken from. `hasPreviousPage` is true when the page starts after the first item, `hasNextPage` when items exist after the page. An empty page has null `startCursor` and `endCursor`.
 
+A root connection reads its items with one query and the total with another. The count query only runs when the selection needs it: when `totalCount` is selected, or when paging with `last` or `before`, which place the window from the end. Paged with `first` and `after`, a selection of `edges`, `items` and `pageInfo` is a single query: it reads one row past the page, and that row answers `hasNextPage`. Without the count, an empty page past the end reports `hasPreviousPage` false, which the Relay specification allows when paging forward.
+
 
 ### Root Query
 

docs/mdsource/defining-graphs.source.md

@@ -247,6 +247,8 @@ Creating a page-able field is supported through [GraphQL Connections](https://gr
 
 Cursors are the zero based index of the edge in the ordered set. `after` and `before` are exclusive, and bound the window of items a page is taken from. `first` keeps that many from the start of the window, then `last` keeps that many from the end, in the order the [Relay specification](https://relay.dev/graphql/connections.htm#sec-Pagination-algorithm) applies them. Edges are always in the set's order, whichever end the page was taken from. `hasPreviousPage` is true when the page starts after the first item, `hasNextPage` when items exist after the page. An empty page has null `startCursor` and `endCursor`.
 
+A root connection reads its items with one query and the total with another. The count query only runs when the selection needs it: when `totalCount` is selected, or when paging with `last` or `before`, which place the window from the end. Paged with `first` and `after`, a selection of `edges`, `items` and `pageInfo` is a single query: it reads one row past the page, and that row answers `hasNextPage`. Without the count, an empty page past the end reports `hasPreviousPage` false, which the Relay specification allows when paging forward.
+
 
 ### Root Query
 

src/Benchmarks/RequestSplitBenchmark.cs

@@ -39,6 +39,19 @@ public class RequestSplitBenchmark
         }
         """;
 
+    const string inMemoryArgumentsQuery = """
+        {
+          parents {
+            id
+            property
+            childrenReversed(where: {property: {startsWith: "Child"}}, orderBy: {property: descending}) {
+              id
+              property
+            }
+          }
+        }
+        """;
+
     const string fragmentsQuery = """
         {
           parents {
@@ -111,6 +124,7 @@ public async Task Setup()
         await Execute(librarySchema, argumentsQuery);
         capturedArguments = CapturingQuery.Captured!;
         await Execute(librarySchema, fragmentsQuery);
+        await Execute(librarySchema, inMemoryArgumentsQuery);
         await Execute(plainSchema, simpleQuery);
 
         projected = includeAppender.ApplyProjection<BenchmarkDbContext, ParentEntity>(capturedSimple, null, Parents);
@@ -153,6 +167,14 @@ public Task<int> FullWithArguments() =>
     public Task<int> FullWithFragments() =>
         Execute(librarySchema, fragmentsQuery);
 
+    /// <summary>
+    /// The where and orderBy of a navigation whose resolver returns a collection other than the
+    /// projected one, so they are evaluated in memory in the resolver, once per parent row.
+    /// </summary>
+    [Benchmark]
+    public Task<int> FullWithInMemoryArguments() =>
+        Execute(librarySchema, inMemoryArgumentsQuery);
+
     [Benchmark]
     public System.Linq.Expressions.Expression ApplyArguments() =>
         Parents.ApplyGraphQlArguments(capturedArguments, keyNames, true, false).Expression;

src/Benchmarks/SimpleQueryBenchmark.cs

@@ -40,6 +40,12 @@ public ParentGraphType(IEfGraphQLService<BenchmarkDbContext> graphQlService) :
             name: "children",
             projection: _ => _.Children,
             resolve: _ => _.Projection);
+        // Returns a collection other than the projected one, so its arguments are applied in
+        // memory, once per parent row
+        AddNavigationListField(
+            name: "childrenReversed",
+            projection: _ => _.Children,
+            resolve: _ => _.Projection.Reverse());
         AutoMap();
     }
 }

src/GraphQL.EntityFramework/ConnectionConverter.cs

@@ -39,7 +39,8 @@ public static Connection<T> ApplyConnectionContext<T>(List<T> list, int? first,
         var count = list.Count;
         var (skip, take) = Window(first, after, last, before, count);
         var page = list.Skip(skip).Take(take);
-        return Build(skip, take, count, page);
+        // long, since a large `first` makes take + skip overflow and wrap negative
+        return Build(skip, count, skip > 0, count > (long) take + skip, page);
     }
 
     /// <summary>
@@ -112,22 +113,100 @@ public static async Task<Connection<TItem>> ApplyConnectionContext<TDbContext, T
         {
             throw new($"Connections require ordering. Either order the IQueryable being passed to AddQueryConnectionField, or use an orderBy in the query. Field: {context.FieldDefinition.Name}");
         }
-        var count = await queryable.CountAsync(cancel);
-        cancel.ThrowIfCancellationRequested();
-        var (skip, take) = Window(first, after, last, before, count);
-        var page = queryable.Skip(skip).Take(take);
-        QueryLogger.Write(page);
-        IEnumerable<TItem> result = await page.ToListAsync(cancel);
+
+        int skip;
+        int? count = null;
+        bool hasPreviousPage;
+        bool hasNextPage;
+        List<TItem> rows;
+        if (NeedsCount(context, last, before))
+        {
+            count = await queryable.CountAsync(cancel);
+            cancel.ThrowIfCancellationRequested();
+            int take;
+            (skip, take) = Window(first, after, last, before, count.Value);
+            var page = queryable.Skip(skip).Take(take);
+            QueryLogger.Write(page);
+            rows = await page.ToListAsync(cancel);
+            hasPreviousPage = skip > 0;
+            // long, since a large `first` makes take + skip overflow and wrap negative
+            hasNextPage = count > (long) take + skip;
+        }
+        else
+        {
+            // The window is bounded from the start only, so it needs no count to place it. The
+            // count clamped the offset to the end; past it the page query reads an empty page.
+            skip = after + 1 ?? 0;
+            var page = queryable.Skip(skip);
+            if (first is not null)
+            {
+                // One row past the page says whether a next page exists
+                page = page.Take(Peek(first.Value));
+            }
+
+            QueryLogger.Write(page);
+            rows = await page.ToListAsync(cancel);
+            hasNextPage = first is not null && rows.Count > first.Value;
+            // Rows on the page prove rows before it. An empty page proves nothing, and paging
+            // forward the spec allows false when that is unknown.
+            hasPreviousPage = skip > 0 && rows.Count > 0;
+            if (hasNextPage)
+            {
+                rows.RemoveAt(rows.Count - 1);
+            }
+        }
+
+        IEnumerable<TItem> result = rows;
         if (filters != null)
         {
             result = await filters.ApplyFilter(result, context.UserContext, data, context.User);
         }
 
         cancel.ThrowIfCancellationRequested();
-        return Build(skip, take, count, result);
+        return Build(skip, count, hasPreviousPage, hasNextPage, result);
+    }
+
+    /// <summary>
+    /// The page size plus the one row read past it, capped so the largest page size does not wrap.
+    /// </summary>
+    static int Peek(int first) =>
+        first == int.MaxValue ? first : first + 1;
+
+    /// <summary>
+    /// Whether the count query has to run: when totalCount is selected, or when the window is
+    /// bounded from the end, by last or before, so the count is needed to place it. The page
+    /// info alone does not need it, since hasNextPage is answered by the row read past the page.
+    /// A connection selecting edges, items and page info otherwise paid a second round trip, a
+    /// COUNT over the whole filtered set, for a number nothing read. The selection is unknown
+    /// for a context built outside an execution, which counts.
+    /// </summary>
+    static bool NeedsCount(IResolveFieldContext context, int? last, int? before)
+    {
+        if (last is not null ||
+            before is not null)
+        {
+            return true;
+        }
+
+        var subFields = context.SubFields;
+        if (subFields is null)
+        {
+            return true;
+        }
+
+        foreach (var (field, _) in subFields.Values)
+        {
+            if (field.Name.Value.Equals("totalCount"))
+            {
+                return true;
+            }
+        }
+
+        return false;
     }
 
-    static Connection<T> Build<T>(int skip, int take, int count, IEnumerable<T> result)
+    /// <param name="count">Null when the count query was skipped, which only happens when the total count was not selected.</param>
+    static Connection<T> Build<T>(int skip, int? count, bool hasPreviousPage, bool hasNextPage, IEnumerable<T> result)
     {
         var edges = result
             .Select((item, index) =>
@@ -144,9 +223,8 @@ static Connection<T> Build<T>(int skip, int take, int count, IEnumerable<T> resu
             Edges = edges,
             PageInfo = new()
             {
-                // long, since a large `first` makes take + skip overflow and wrap negative
-                HasNextPage = count > (long) take + skip,
-                HasPreviousPage = skip > 0,
+                HasNextPage = hasNextPage,
+                HasPreviousPage = hasPreviousPage,
                 // Null when there are no edges, as the spec has it. The edges are used rather
                 // than the window since filters can remove items after the query.
                 StartCursor = edges.FirstOrDefault()?.Cursor,

src/GraphQL.EntityFramework/Filters/Filters.cs

@@ -93,6 +93,7 @@ void AddEntry<TEntity>(IFilterEntry<TDbContext> entry)
 
         forType.Add(entry);
         filtersByType.Clear();
+        filtersForHierarchy.Clear();
     }
 
     /// <summary>
@@ -116,10 +117,17 @@ List<IFilterEntry<TDbContext>> GetFilters(Type entityType) =>
     /// load: those on the type and its base types, which apply to every item, and those on derived
     /// types, which apply to the derived items the query can return.
     /// </summary>
-    internal IEnumerable<IFilterEntry<TDbContext>> GetFiltersForHierarchy(Type entityType) =>
-        entries
-            .Where(_ => _.Key.IsAssignableFrom(entityType) || entityType.IsAssignableFrom(_.Key))
-            .SelectMany(_ => _.Value);
+    internal IReadOnlyList<IFilterEntry<TDbContext>> GetFiltersForHierarchy(Type entityType) =>
+        filtersForHierarchy.GetOrAdd(
+            entityType,
+            type => entries
+                .Where(_ => _.Key.IsAssignableFrom(type) || type.IsAssignableFrom(_.Key))
+                .SelectMany(_ => _.Value)
+                .ToList());
+
+    // Looked up for every entity type in a projection on every request, so cached per type the
+    // same way as GetFilters, and reset when a filter is added
+    ConcurrentDictionary<Type, List<IFilterEntry<TDbContext>>> filtersForHierarchy = new();
 
     /// <summary>
     /// Returns true if there are any filters registered.

src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Navigation.cs

@@ -28,15 +28,20 @@ public FieldBuilder<TSource, TReturn> AddNavigationField<TSource, TReturn, TProj
         field.Resolver = new FuncFieldResolver<TSource, TReturn?>(
             async context =>
             {
-                var fieldContext = BuildContext(context);
+                // Runs once per parent row. Building a ResolveEfFieldContext here copied every property
+                // of the GraphQL.NET context, which forced the lazily computed ones, SubFields, Path,
+                // ResponsePath, Parent and Arguments, to be computed and allocated per row, when all
+                // this needs is the DbContext and the filters.
+                var dbContext = ResolveDbContext(context);
+                var filters = ResolveFilters(context);
                 var projected = compiledProjection(context.Source);
 
                 var projectionContext = new ResolveProjectionContext<TDbContext, TProjection>
                 {
                     Projection = projected,
-                    DbContext = fieldContext.DbContext,
+                    DbContext = dbContext,
                     User = context.User,
-                    Filters = fieldContext.Filters,
+                    Filters = filters,
                     FieldContext = context
                 };
 
@@ -57,12 +62,12 @@ public FieldBuilder<TSource, TReturn> AddNavigationField<TSource, TReturn, TProj
                         exception);
                 }
 
-                if (fieldContext.Filters == null)
+                if (filters == null)
                 {
                     return result;
                 }
 
-                if (await fieldContext.Filters.ShouldInclude(context.UserContext, fieldContext.DbContext, context.User, result))
+                if (await filters.ShouldInclude(context.UserContext, dbContext, context.User, result))
                 {
                     return result;
                 }

src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_NavigationConnection.cs

@@ -26,15 +26,20 @@ public ConnectionBuilder<TSource> AddNavigationConnectionField<TSource, TReturn,
         var names = GetKeyNames<TReturn>();
         builder.ResolveAsync(async context =>
         {
-            var efFieldContext = BuildContext(context);
+            // Runs once per parent row. Building a ResolveEfFieldContext here copied every property
+            // of the GraphQL.NET context, which forced the lazily computed ones, SubFields, Path,
+            // ResponsePath, Parent and Arguments, to be computed and allocated per row, when all
+            // this needs is the DbContext and the filters.
+            var dbContext = ResolveDbContext(context);
+            var filters = ResolveFilters(context);
             var projected = compiledProjection(context.Source);
 
             var projectionContext = new ResolveProjectionContext<TDbContext, TProjection>
             {
                 Projection = projected,
-                DbContext = efFieldContext.DbContext,
+                DbContext = dbContext,
                 User = context.User,
-                Filters = efFieldContext.Filters,
+                Filters = filters,
                 FieldContext = context
             };
 
@@ -59,11 +64,17 @@ public ConnectionBuilder<TSource> AddNavigationConnectionField<TSource, TReturn,
                 throw new("This API expects the resolver to return a IEnumerable, not an IQueryable. Instead use AddQueryConnectionField.");
             }
 
-            var applied = ReferenceEquals(enumerable, projected) && PushDown.IsApplied(context);
-            enumerable = enumerable.ApplyGraphQlArguments(names, context, omitQueryArguments, applied);
-            if (efFieldContext.Filters != null)
+            // A field selected without arguments has nothing to apply, so the argument reads
+            // and the push down lookup are skipped
+            if (ArgumentReader.HasArguments(context))
             {
-                enumerable = await efFieldContext.Filters.ApplyFilter(enumerable, context.UserContext, efFieldContext.DbContext, context.User);
+                var applied = ReferenceEquals(enumerable, projected) && PushDown.IsApplied(context);
+                enumerable = enumerable.ApplyGraphQlArguments(names, context, omitQueryArguments, applied);
+            }
+
+            if (filters != null)
+            {
+                enumerable = await filters.ApplyFilter(enumerable, context.UserContext, dbContext, context.User);
             }
 
             var page = enumerable.ToList();

src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_NavigationList.cs

@@ -29,15 +29,20 @@ public FieldBuilder<TSource, TReturn> AddNavigationListField<TSource, TReturn, T
 
         field.Resolver = new FuncFieldResolver<TSource, IEnumerable<TReturn>>(async context =>
         {
-            var fieldContext = BuildContext(context);
+            // Runs once per parent row. Building a ResolveEfFieldContext here copied every property
+            // of the GraphQL.NET context, which forced the lazily computed ones, SubFields, Path,
+            // ResponsePath, Parent and Arguments, to be computed and allocated per row, when all
+            // this needs is the DbContext and the filters.
+            var dbContext = ResolveDbContext(context);
+            var filters = ResolveFilters(context);
             var projected = compiledProjection(context.Source);
 
             var projectionContext = new ResolveProjectionContext<TDbContext, TProjection>
             {
                 Projection = projected,
-                DbContext = fieldContext.DbContext,
+                DbContext = dbContext,
                 User = context.User,
-                Filters = fieldContext.Filters,
+                Filters = filters,
                 FieldContext = context
             };
 
@@ -48,16 +53,22 @@ public FieldBuilder<TSource, TReturn> AddNavigationListField<TSource, TReturn, T
                 throw new("This API expects the resolver to return a IEnumerable, not an IQueryable. Instead use AddQueryField.");
             }
 
-            // The collection arrives with ids, where and orderBy already applied when the parent
-            // was loaded through a projection and the resolver returned that collection as is
-            var applied = ReferenceEquals(result, projected) && PushDown.IsApplied(context);
-            result = result.ApplyGraphQlArguments(names, context, omitQueryArguments, applied);
-            if (fieldContext.Filters == null)
+            // A field selected without arguments has nothing to apply, and that is the common
+            // case for a navigation, so the argument reads and the push down lookup are skipped
+            if (ArgumentReader.HasArguments(context))
+            {
+                // The collection arrives with ids, where and orderBy already applied when the parent
+                // was loaded through a projection and the resolver returned that collection as is
+                var applied = ReferenceEquals(result, projected) && PushDown.IsApplied(context);
+                result = result.ApplyGraphQlArguments(names, context, omitQueryArguments, applied);
+            }
+
+            if (filters == null)
             {
                 return result;
             }
 
-            return await fieldContext.Filters.ApplyFilter(result, context.UserContext, fieldContext.DbContext, context.User);
+            return await filters.ApplyFilter(result, context.UserContext, dbContext, context.User);
         });
 
         graph.AddField(field);