Fix ordering detection and placement

readme.md

@@ -16,6 +16,7 @@ https://nuget.org/packages/EfOrderBy/
 
 - **Automatic ordering**: Queries without explicit `OrderBy` automatically use configured default ordering
 - **Include() support**: Nested collections in `.Include()` expressions are automatically ordered
+- **Deterministic paging**: Ordering is applied before `Skip`/`Take`/`First`, so pages and single results are stable
 - **Inheritance support**: Ordering configured on a base entity type is automatically inherited by derived types (TPH)
 - **Fluent configuration**: Configure default ordering using the familiar EF Core fluent API
 - **Multi-column ordering**: Chain multiple ordering clauses with `ThenBy` and `ThenByDescending`
@@ -82,6 +83,37 @@ var employeesByName = await context.Employees
 <!-- endSnippet -->
 
 
+## Paging and Single Results
+
+The default ordering is applied *before* any operator that chooses which rows come back, so
+`Skip`, `Take`, `First`, `FirstOrDefault`, `Last`, `LastOrDefault` and `ElementAt` all select
+from an ordered sequence:
+
+<!-- snippet: PagingAndSingleResults -->
+<a id='snippet-PagingAndSingleResults'></a>
+```cs
+// The ordering is applied before the page is taken, so the page is
+// taken from an ordered sequence rather than sorted after the fact
+var secondPage = await context.Employees
+    .Skip(20)
+    .Take(20)
+    .ToListAsync();
+
+// Ordered by HireDate, then Salary descending, so this is the
+// earliest hire rather than an arbitrary row
+var first = await context.Employees
+    .FirstAsync();
+```
+<sup><a href='/src/Tests/Snippets.cs#L89-L103' title='Snippet source file'>snippet source</a> | <a href='#snippet-PagingAndSingleResults' title='Start of snippet'>anchor</a></sup>
+<!-- endSnippet -->
+
+Without this, a page would be an arbitrary set of rows that happens to be sorted, and pages
+could both skip and repeat rows.
+
+Aggregates such as `Count` and `Any`, and `Single`, are left alone, since ordering them is
+pointless work.
+
+
 ## Include() Support
 
 Nested collections in `.Include()` expressions are automatically ordered:
@@ -95,7 +127,7 @@ var departments = await context.Departments
     .Include(_ => _.Employees)
     .ToListAsync();
 ```
-<sup><a href='/src/Tests/Snippets.cs#L89-L97' title='Snippet source file'>snippet source</a> | <a href='#snippet-IncludeSupport' title='Start of snippet'>anchor</a></sup>
+<sup><a href='/src/Tests/Snippets.cs#L110-L118' title='Snippet source file'>snippet source</a> | <a href='#snippet-IncludeSupport' title='Start of snippet'>anchor</a></sup>
 <!-- endSnippet -->
 
 
@@ -149,7 +181,7 @@ public class InheritanceDbContext : DbContext
     public DbSet<DerivedEntityB> DerivedEntitiesB => Set<DerivedEntityB>();
 }
 ```
-<sup><a href='/src/Tests/Snippets.cs#L165-L210' title='Snippet source file'>snippet source</a> | <a href='#snippet-InheritanceOrdering' title='Start of snippet'>anchor</a></sup>
+<sup><a href='/src/Tests/Snippets.cs#L186-L231' title='Snippet source file'>snippet source</a> | <a href='#snippet-InheritanceOrdering' title='Start of snippet'>anchor</a></sup>
 <!-- endSnippet -->
 
 Behavior:
@@ -172,7 +204,7 @@ builder.Entity<Product>()
     .ThenBy(_ => _.Name)
     .ThenByDescending(_ => _.Price);
 ```
-<sup><a href='/src/Tests/Snippets.cs#L102-L109' title='Snippet source file'>snippet source</a> | <a href='#snippet-MultiColumnOrdering' title='Start of snippet'>anchor</a></sup>
+<sup><a href='/src/Tests/Snippets.cs#L123-L130' title='Snippet source file'>snippet source</a> | <a href='#snippet-MultiColumnOrdering' title='Start of snippet'>anchor</a></sup>
 <!-- endSnippet -->
 
 
@@ -422,7 +454,7 @@ public class AppDbContext : DbContext
     public DbSet<Employee> Employees => Set<Employee>();
 }
 ```
-<sup><a href='/src/Tests/Snippets.cs#L113-L156' title='Snippet source file'>snippet source</a> | <a href='#snippet-CompleteExample' title='Start of snippet'>anchor</a></sup>
+<sup><a href='/src/Tests/Snippets.cs#L134-L177' title='Snippet source file'>snippet source</a> | <a href='#snippet-CompleteExample' title='Start of snippet'>anchor</a></sup>
 <!-- endSnippet -->
 
 

src/EfOrderBy/Configuration.cs

@@ -59,15 +59,22 @@ internal void AddClause(PropertyInfo propertyInfo, bool descending, bool isThenB
     /// Creates a new Configuration for a derived type by replaying the clause metadata.
     /// The derived type must have the same properties (inherited from the base).
     /// </summary>
+    // The default lookup omits non public properties, which an entity can map,
+    // so without these the ordering fails to inherit for those properties.
+    const BindingFlags propertyFlags =
+        BindingFlags.Public |
+        BindingFlags.NonPublic |
+        BindingFlags.Instance;
+
     internal Configuration CreateForDerivedType(Type derivedType)
     {
         var derived = new Configuration(derivedType) { IsInherited = true };
         foreach (var meta in ClauseMetadataList)
         {
-            var prop = derivedType.GetProperty(meta.PropertyName);
-            if (prop != null)
+            var property = derivedType.GetProperty(meta.PropertyName, propertyFlags);
+            if (property != null)
             {
-                derived.AddClause(prop, meta.Descending, meta.IsThenBy);
+                derived.AddClause(property, meta.Descending, meta.IsThenBy);
                 continue;
             }
 

src/EfOrderBy/IncludeOrderingApplicator.cs

@@ -4,7 +4,7 @@
 sealed class IncludeOrderingApplicator(IModel model, bool detectRedundantOrdering) :
     ExpressionVisitor
 {
-    static readonly ConcurrentDictionary<Type, Type?> collectionElementTypeCache = new();
+    // MakeGenericMethod is expensive enough to be worth caching, unlike the element type lookup
     static readonly ConcurrentDictionary<(MethodInfo, Type), MethodInfo> includeMethodCache = new();
 
     protected override Expression VisitMethodCall(MethodCallExpression node)
@@ -45,7 +45,7 @@ MethodCallExpression ProcessInclude(MethodCallExpression includeCall)
         }
 
         // Check if the navigation already has ordering
-        if (Interceptor.HasOrdering(lambda.Body))
+        if (QueryAnalyzer.HasOrdering(lambda.Body))
         {
             // Already has explicit ordering, don't apply default
             if (detectRedundantOrdering)
@@ -112,35 +112,36 @@ static Expression ApplyOrdering(Expression source, Configuration configuration)
         return result;
     }
 
-    static Type? GetCollectionElementType(Type type) =>
-        collectionElementTypeCache.GetOrAdd(type,
-            static type =>
+    // Not cached. A navigation is declared as one of the types below, so the fast path is a
+    // handful of type comparisons, which is cheaper than a dictionary lookup and does not pin
+    // every type it is handed for the life of the process.
+    static Type? GetCollectionElementType(Type type)
+    {
+        // Check for IEnumerable<T>
+        if (type.IsGenericType)
+        {
+            var genericDef = type.GetGenericTypeDefinition();
+            if (genericDef == typeof(IEnumerable<>) ||
+                genericDef == typeof(ICollection<>) ||
+                genericDef == typeof(IList<>) ||
+                genericDef == typeof(List<>))
             {
-                // Check for IEnumerable<T>
-                if (type.IsGenericType)
-                {
-                    var genericDef = type.GetGenericTypeDefinition();
-                    if (genericDef == typeof(IEnumerable<>) ||
-                        genericDef == typeof(ICollection<>) ||
-                        genericDef == typeof(IList<>) ||
-                        genericDef == typeof(List<>))
-                    {
-                        return type.GetGenericArguments()[0];
-                    }
-                }
-
-                // Check interfaces
-                foreach (var iface in type.GetInterfaces())
-                {
-                    if (iface.IsGenericType &&
-                        iface.GetGenericTypeDefinition() == typeof(IEnumerable<>))
-                    {
-                        return iface.GetGenericArguments()[0];
-                    }
-                }
-
-                return null;
-            });
+                return type.GetGenericArguments()[0];
+            }
+        }
+
+        // Check interfaces
+        foreach (var iface in type.GetInterfaces())
+        {
+            if (iface.IsGenericType &&
+                iface.GetGenericTypeDefinition() == typeof(IEnumerable<>))
+            {
+                return iface.GetGenericArguments()[0];
+            }
+        }
+
+        return null;
+    }
 
     Configuration? GetConfiguration(Type element)
     {