Add orderNavigationsByKey option

docs/configuration.md

@@ -24,9 +24,10 @@ public static void RegisterInContainer<TDbContext>(
         ResolveFilters<TDbContext>? resolveFilters = null,
         bool disableTracking = false,
         bool includeSqlInExceptions = false,
-        bool splitSideBySideCollections = false)
+        bool splitSideBySideCollections = false,
+        bool orderNavigationsByKey = true)
 ```
-<sup><a href='/src/GraphQL.EntityFramework/EfGraphQLConventions.cs#L16-L27' title='Snippet source file'>snippet source</a> | <a href='#snippet-RegisterInContainer' title='Start of snippet'>anchor</a></sup>
+<sup><a href='/src/GraphQL.EntityFramework/EfGraphQLConventions.cs#L17-L29' title='Snippet source file'>snippet source</a> | <a href='#snippet-RegisterInContainer' title='Start of snippet'>anchor</a></sup>
 <a id='snippet-RegisterInContainer-1'></a>
 ```cs
 EfGraphQLConventions.RegisterInContainer<MyDbContext>(
@@ -141,6 +142,18 @@ The resolver can not know in advance which collections a request will select, so
 It defaults to false.
 
 
+#### OrderNavigationsByKey
+
+A navigation list or connection field loaded through a projection, with no `orderBy` argument, is ordered by its key so its items come back in a deterministic order.
+
+Setting `orderNavigationsByKey` to false leaves such a collection unordered, for a model that orders the collections a projection loads by a default of its own. [EfOrderBy](https://github.com/SimonCropp/EntityFramework.OrderBy) 1.4 and later does this. EfOrderBy leaves a collection that is already ordered as it is, so the key ordering otherwise replaces the configured default, and a client that leaves out `orderBy` gets the items in key order rather than in the order the model defines.
+
+ * An `orderBy` argument is still applied, and replaces the default.
+ * A model with no default for an entity leaves that entity's collections in whatever order the database returns them, so only turn this off where every entity has one, for example with EfOrderBy's `requireOrderingForAllEntities`.
+
+It defaults to true.
+
+
 ### Usage
 
 <!-- snippet: RegisterInContainer -->
@@ -153,9 +166,10 @@ public static void RegisterInContainer<TDbContext>(
         ResolveFilters<TDbContext>? resolveFilters = null,
         bool disableTracking = false,
         bool includeSqlInExceptions = false,
-        bool splitSideBySideCollections = false)
+        bool splitSideBySideCollections = false,
+        bool orderNavigationsByKey = true)
 ```
-<sup><a href='/src/GraphQL.EntityFramework/EfGraphQLConventions.cs#L16-L27' title='Snippet source file'>snippet source</a> | <a href='#snippet-RegisterInContainer' title='Start of snippet'>anchor</a></sup>
+<sup><a href='/src/GraphQL.EntityFramework/EfGraphQLConventions.cs#L17-L29' title='Snippet source file'>snippet source</a> | <a href='#snippet-RegisterInContainer' title='Start of snippet'>anchor</a></sup>
 <a id='snippet-RegisterInContainer-1'></a>
 ```cs
 EfGraphQLConventions.RegisterInContainer<MyDbContext>(

docs/mdsource/configuration.source.md

@@ -81,6 +81,18 @@ The resolver can not know in advance which collections a request will select, so
 It defaults to false.
 
 
+#### OrderNavigationsByKey
+
+A navigation list or connection field loaded through a projection, with no `orderBy` argument, is ordered by its key so its items come back in a deterministic order.
+
+Setting `orderNavigationsByKey` to false leaves such a collection unordered, for a model that orders the collections a projection loads by a default of its own. [EfOrderBy](https://github.com/SimonCropp/EntityFramework.OrderBy) 1.4 and later does this. EfOrderBy leaves a collection that is already ordered as it is, so the key ordering otherwise replaces the configured default, and a client that leaves out `orderBy` gets the items in key order rather than in the order the model defines.
+
+ * An `orderBy` argument is still applied, and replaces the default.
+ * A model with no default for an entity leaves that entity's collections in whatever order the database returns them, so only turn this off where every entity has one, for example with EfOrderBy's `requireOrderingForAllEntities`.
+
+It defaults to true.
+
+
 ### Usage
 
 snippet: RegisterInContainer

src/Directory.Build.props

@@ -2,7 +2,7 @@
 <Project>
   <PropertyGroup>
     <NoWarn>CS1591;NU5104;CS1573;CS9107;NU1608;NU1109</NoWarn>
-    <Version>36.2.0</Version>
+    <Version>36.3.0</Version>
     <LangVersion>preview</LangVersion>
     <AssemblyVersion>1.0.0</AssemblyVersion>
     <PackageTags>EntityFrameworkCore, EntityFramework, GraphQL</PackageTags>

src/GraphQL.EntityFramework/EfGraphQLConventions.cs

@@ -12,6 +12,7 @@ public static class EfGraphQLConventions
     /// <param name="disableTracking">Use <see cref="EntityFrameworkQueryableExtensions.AsNoTracking{TEntity}"/> for all <see cref="IQueryable{T}"/> operations.</param>
     /// <param name="includeSqlInExceptions">Include the generated sql in exception messages. Off by default, since those messages can reach clients and the sql carries table and column names and, depending on the provider, parameter values.</param>
     /// <param name="splitSideBySideCollections">Use <see cref="RelationalQueryableExtensions.AsSplitQuery{TEntity}"/> for a query whose request selects collections side by side, which multiply each other's rows in a single query. A query the resolver already called AsSplitQuery or AsSingleQuery on is left as it is. Needs a relational provider.</param>
+    /// <param name="orderNavigationsByKey">Order a projected navigation list or connection field that has no orderBy argument by its key, so its items come back in a deterministic order. Pass false when the model orders the collections a projection loads by a default of its own, such as EfOrderBy 1.4 or later does, so that default is used instead of the key.</param>
 
     #region RegisterInContainer
 
@@ -22,7 +23,8 @@ public static void RegisterInContainer<TDbContext>(
             ResolveFilters<TDbContext>? resolveFilters = null,
             bool disableTracking = false,
             bool includeSqlInExceptions = false,
-            bool splitSideBySideCollections = false)
+            bool splitSideBySideCollections = false,
+            bool orderNavigationsByKey = true)
 
         #endregion
 
@@ -31,7 +33,7 @@ public static void RegisterInContainer<TDbContext>(
         RegisterScalarsAndArgs(services);
         services.AddHttpContextAccessor();
         services.AddTransient<HttpContextCapture>();
-        services.AddSingleton(provider => Build(resolveDbContext, model, resolveFilters, provider, disableTracking, includeSqlInExceptions, splitSideBySideCollections));
+        services.AddSingleton(provider => Build(resolveDbContext, model, resolveFilters, provider, disableTracking, includeSqlInExceptions, splitSideBySideCollections, orderNavigationsByKey));
         services.AddSingleton<IEfGraphQLService<TDbContext>>(provider => provider.GetRequiredService<EfGraphQLService<TDbContext>>());
         // The generated where and orderBy input types resolve every registered service, so a
         // type mapped in any of the contexts is found
@@ -45,7 +47,8 @@ static EfGraphQLService<TDbContext> Build<TDbContext>(
         IServiceProvider provider,
         bool disableTracking,
         bool includeSqlInExceptions,
-        bool splitSideBySideCollections)
+        bool splitSideBySideCollections,
+        bool orderNavigationsByKey)
         where TDbContext : DbContext
     {
         model ??= ResolveModel<TDbContext>(provider);
@@ -58,7 +61,8 @@ static EfGraphQLService<TDbContext> Build<TDbContext>(
             filters,
             disableTracking,
             includeSqlInExceptions,
-            splitSideBySideCollections);
+            splitSideBySideCollections,
+            orderNavigationsByKey);
     }
 
     static TDbContext DbContextFromProvider<TDbContext>(IServiceProvider provider, IServiceProvider? requestServices)

src/GraphQL.EntityFramework/GraphApi/EfGraphQLService.cs

@@ -13,13 +13,15 @@ public partial class EfGraphQLService<TDbContext> :
     /// <param name="disableTracking">Use <see cref="EntityFrameworkQueryableExtensions.AsNoTracking{TEntity}"/> for all <see cref="IQueryable{T}"/> operations.</param>
     /// <param name="includeSqlInExceptions">Include the generated sql in exception messages. Off by default, since those messages can reach clients and the sql carries table and column names and, depending on the provider, parameter values.</param>
     /// <param name="splitSideBySideCollections">Use <see cref="RelationalQueryableExtensions.AsSplitQuery{TEntity}"/> for a query whose request selects collections side by side, which multiply each other's rows in a single query. A query the resolver already called AsSplitQuery or AsSingleQuery on is left as it is. Needs a relational provider.</param>
+    /// <param name="orderNavigationsByKey">Order a projected navigation list or connection field that has no orderBy argument by its key, so its items come back in a deterministic order. Pass false when the model orders the collections a projection loads by a default of its own, such as EfOrderBy 1.4 or later does, so that default is used instead of the key.</param>
     public EfGraphQLService(
         IModel model,
         ResolveDbContext<TDbContext> resolveDbContext,
         ResolveFilters<TDbContext>? resolveFilters = null,
         bool disableTracking = false,
         bool includeSqlInExceptions = false,
-        bool splitSideBySideCollections = false)
+        bool splitSideBySideCollections = false,
+        bool orderNavigationsByKey = true)
     {
         this.resolveFilters = resolveFilters;
         this.disableTracking = disableTracking;
@@ -42,7 +44,7 @@ public EfGraphQLService(
                     .OrderBy(derived => Depth(derived.ClrType))
                     .Select(derived => derived.ClrType)
                     .ToList());
-        includeAppender = new(Navigations, keyNames, foreignKeys, derivedTypes, BackingFieldReader.GetBackingFields(model), splitSideBySideCollections);
+        includeAppender = new(Navigations, keyNames, foreignKeys, derivedTypes, BackingFieldReader.GetBackingFields(model), splitSideBySideCollections, orderNavigationsByKey);
     }
 
     static int Depth(Type type)

src/GraphQL.EntityFramework/IncludeAppender.cs

@@ -4,7 +4,8 @@
     IReadOnlyDictionary<Type, IReadOnlySet<string>> foreignKeys,
     IReadOnlyDictionary<Type, IReadOnlyList<Type>> derivedTypes,
     IReadOnlyDictionary<Type, IReadOnlyDictionary<string, FieldInfo>>? backingFields = null,
-    bool splitSideBySideCollections = false)
+    bool splitSideBySideCollections = false,
+    bool orderNavigationsByKey = true)
 {
     /// <summary>
     /// Narrow <paramref name="query"/> to the fields the request asked for: a select projection
@@ -52,7 +53,7 @@ public IQueryable<TItem> ApplyProjection<TDbContext, TItem>(
             query = query.AsSplitQuery();
         }
 
-        if (!SelectExpressionBuilder.TryBuild<TItem>(projection, keyNames, derivedTypes, out var expression, out var includePaths, out var argumentFields, backingFields))
+        if (!SelectExpressionBuilder.TryBuild<TItem>(projection, keyNames, derivedTypes, out var expression, out var includePaths, out var argumentFields, backingFields, orderNavigationsByKey))
         {
             return ApplyTracking(AddIncludesFromProjection(query, projection), disableTracking);
         }

src/GraphQL.EntityFramework/SelectProjection/SelectExpressionBuilder.cs

@@ -107,18 +107,25 @@ public static bool TryBuild<TEntity>(
     /// The navigation fields whose ids, where and orderBy were applied inside their collection
     /// subquery, so their resolvers know not to apply them again.
     /// </param>
+    /// <param name="orderNavigationsByKey">
+    /// Whether a projected collection without an orderBy is ordered by key. False leaves it
+    /// unordered, for a model that orders the collections a projection loads by a default of its
+    /// own, such as EfOrderBy. Ordering by key there replaced that default, since a collection
+    /// that is already ordered is left as it is.
+    /// </param>
     public static bool TryBuild<TEntity>(
         FieldProjectionInfo projection,
         IReadOnlyDictionary<Type, List<string>> keyNames,
         IReadOnlyDictionary<Type, IReadOnlyList<Type>> derivedTypes,
         [NotNullWhen(true)] out Expression<Func<TEntity, TEntity>>? expression,
         out IReadOnlyList<string> includePaths,
         out IReadOnlyList<GraphQLField> argumentFields,
-        IReadOnlyDictionary<Type, IReadOnlyDictionary<string, FieldInfo>>? backingFields = null)
+        IReadOnlyDictionary<Type, IReadOnlyDictionary<string, FieldInfo>>? backingFields = null,
+        bool orderNavigationsByKey = true)
         where TEntity : class
     {
         expression = null;
-        var state = new BuildState(keyNames, derivedTypes, backingFields);
+        var state = new BuildState(keyNames, derivedTypes, backingFields, orderNavigationsByKey);
         includePaths = state.IncludePaths;
         argumentFields = state.ArgumentFields;
         var entityType = typeof(TEntity);
@@ -547,7 +554,8 @@ static MemberAssignment BuildFullNavigationBinding(
     /// The navigation field's ids, where and orderBy, applied to the collection inside the
     /// subquery so the database evaluates them against the table rather than the resolver
     /// evaluating them in memory against the projected columns. Without an orderBy the
-    /// collection is ordered by key, where <paramref name="orderByKey"/>, for a deterministic result.
+    /// collection is ordered by key, where <paramref name="orderByKey"/> and the build orders
+    /// navigations by key, for a deterministic result.
     /// </summary>
     static Expression ApplyArguments(
         Expression source,
@@ -591,6 +599,7 @@ static Expression ApplyArguments(
         }
 
         if (orderByKey &&
+            state.OrderNavigationsByKey &&
             state.KeyNames.TryGetValue(navType, out var keys) &&
             keys.Count > 0 &&
             GetEntityMetadata(navType).Properties.TryGetValue(keys[0], out var keyMetadata))
@@ -619,10 +628,12 @@ static Expression Where(Expression source, ParameterExpression navParam, Type na
     sealed class BuildState(
         IReadOnlyDictionary<Type, List<string>> keyNames,
         IReadOnlyDictionary<Type, IReadOnlyList<Type>> derivedTypes,
-        IReadOnlyDictionary<Type, IReadOnlyDictionary<string, FieldInfo>>? backingFields)
+        IReadOnlyDictionary<Type, IReadOnlyDictionary<string, FieldInfo>>? backingFields,
+        bool orderNavigationsByKey)
     {
         public IReadOnlyDictionary<Type, List<string>> KeyNames { get; } = keyNames;
         public IReadOnlyDictionary<Type, IReadOnlyList<Type>> DerivedTypes { get; } = derivedTypes;
+        public bool OrderNavigationsByKey { get; } = orderNavigationsByKey;
         public List<string> IncludePaths { get; } = [];
         public List<GraphQLField> ArgumentFields { get; } = [];
 

src/Tests/IntegrationTests/IntegrationTests.Navigation_with_orderBy_not_ordered_by_key.verified.txt

@@ -0,0 +1,31 @@
+{
+  target: {
+    Data: {
+      parentEntities: [
+        {
+          property: Parent,
+          children: [
+            {
+              property: Child
+            }
+          ]
+        }
+      ]
+    }
+  },
+  sql: {
+    Text:
+select   p.Id,
+         c.Id,
+         c.ParentId,
+         c.Property,
+         p.Property
+from     ParentEntities as p
+         left outer join
+         ChildEntities as c
+         on p.Id = c.ParentId
+order by p.Property,
+         p.Id,
+         c.Property desc
+  }
+}

src/Tests/IntegrationTests/IntegrationTests.Navigation_without_orderBy_not_ordered_by_key.verified.txt

@@ -0,0 +1,30 @@
+{
+  target: {
+    Data: {
+      parentEntities: [
+        {
+          property: Parent,
+          children: [
+            {
+              property: Child
+            }
+          ]
+        }
+      ]
+    }
+  },
+  sql: {
+    Text:
+select   p.Id,
+         c.Id,
+         c.ParentId,
+         c.Property,
+         p.Property
+from     ParentEntities as p
+         left outer join
+         ChildEntities as c
+         on p.Id = c.ParentId
+order by p.Property,
+         p.Id
+  }
+}

src/Tests/IntegrationTests/IntegrationTests.Navigation_without_orderBy_ordered_by_key.verified.txt

@@ -0,0 +1,31 @@
+{
+  target: {
+    Data: {
+      parentEntities: [
+        {
+          property: Parent,
+          children: [
+            {
+              property: Child
+            }
+          ]
+        }
+      ]
+    }
+  },
+  sql: {
+    Text:
+select   p.Id,
+         c.Id,
+         c.ParentId,
+         c.Property,
+         p.Property
+from     ParentEntities as p
+         left outer join
+         ChildEntities as c
+         on p.Id = c.ParentId
+order by p.Property,
+         p.Id,
+         c.Id
+  }
+}

src/Tests/IntegrationTests/IntegrationTests.cs

@@ -3462,6 +3462,7 @@ static async Task RunQuery(
         bool disableTracking,
         object[] entities,
         bool includeSqlInExceptions = false,
+        bool orderNavigationsByKey = true,
         [CallerFilePath] string sourceFile = "")
     {
         var dbContext = database.Context;
@@ -3482,7 +3483,7 @@ static async Task RunQuery(
         ExecutionResult result;
         try
         {
-            result = await QueryExecutor.ExecuteQuery(query, services, context, inputs, filters, disableTracking, includeSqlInExceptions);
+            result = await QueryExecutor.ExecuteQuery(query, services, context, inputs, filters, disableTracking, includeSqlInExceptions, orderNavigationsByKey);
         }
         catch (ExecutionError executionError)
         {