diff --git a/readme.md b/readme.md index a06a96b..2a0095b 100644 --- a/readme.md +++ b/readme.md @@ -22,7 +22,7 @@ https://nuget.org/packages/EfOrderBy/ - **Multi-column ordering**: Chain multiple ordering clauses with `ThenBy` and `ThenByDescending` - **Automatic indexes**: Database indexes are automatically created for ordering columns - **Validation mode**: Optionally require all entities to have default ordering configured -- **Redundant ordering detection**: Optionally throw when a query explicitly applies the same ordering as the configured default +- **Redundant ordering detection**: Optionally throw when a query explicitly applies the same ordering as the configured default, per context or process wide ## Usage @@ -79,7 +79,7 @@ var employeesByName = await context.Employees .OrderBy(_ => _.Name) .ToListAsync(); ``` -snippet source | anchor +snippet source | anchor @@ -104,7 +104,7 @@ var secondPage = await context.Employees var first = await context.Employees .FirstAsync(); ``` -snippet source | anchor +snippet source | anchor Without this, a page would be an arbitrary set of rows that happens to be sorted, and pages @@ -127,7 +127,7 @@ var departments = await context.Departments .Include(_ => _.Employees) .ToListAsync(); ``` -snippet source | anchor +snippet source | anchor @@ -181,7 +181,7 @@ public class InheritanceDbContext : DbContext public DbSet DerivedEntitiesB => Set(); } ``` -snippet source | anchor +snippet source | anchor Behavior: @@ -204,7 +204,7 @@ builder.Entity() .ThenBy(_ => _.Name) .ThenByDescending(_ => _.Price); ``` -snippet source | anchor +snippet source | anchor @@ -287,7 +287,7 @@ protected override void OnConfiguring(DbContextOptionsBuilder builder) => builder.UseDefaultOrderBy( createIndexes: false); ``` -snippet source | anchor +snippet source | anchor When index creation is disabled, calling `WithIndexName()` throws an `Exception`. @@ -388,6 +388,36 @@ var departments = await context.Departments Detection happens during query compilation, so it applies to every distinct query the application executes, whether or not the query hits the database. +### Enabling for a whole test project + +Setting it on every `DbContext` gets tedious in a test project that has several. `OrderBySettings.ThrowOnRedundantOrderBy` is a process wide default, so a module initializer turns it on for all of them: + + + +```cs +[ModuleInitializer] +public static void Initialize() => + OrderBySettings.ThrowOnRedundantOrderBy = true; +``` +snippet source | anchor + + +`UseDefaultOrderBy` reads this setting only when its `throwOnRedundantOrderBy` parameter is left unset. Passing a value always wins, so an individual context can opt out: + + + +```cs +// Passing false explicitly overrides OrderBySettings.ThrowOnRedundantOrderBy +protected override void OnConfiguring(DbContextOptionsBuilder builder) => + builder.UseDefaultOrderBy( + throwOnRedundantOrderBy: false); +``` +snippet source | anchor + + +The setting is read during query compilation rather than when the options are built, so it applies to contexts whose options were built before the initializer ran. + + ## Configuration Errors Calling `OrderBy` or `OrderByDescending` multiple times for the same entity type throws an `Exception`: @@ -454,7 +484,7 @@ public class AppDbContext : DbContext public DbSet Employees => Set(); } ``` -snippet source | anchor +snippet source | anchor diff --git a/src/EfOrderBy/Interceptor.cs b/src/EfOrderBy/Interceptor.cs index 63ab96a..fee34d9 100644 --- a/src/EfOrderBy/Interceptor.cs +++ b/src/EfOrderBy/Interceptor.cs @@ -19,7 +19,10 @@ public Expression QueryCompilationStarting(Expression query, QueryExpressionEven RequiredOrder.Validate(context, extension); - var detectRedundantOrdering = extension?.ThrowOnRedundantOrderBy ?? false; + // Read per query rather than when the options are built, so a module initializer that + // runs after some options were already built still takes effect + var detectRedundantOrdering = extension?.ThrowOnRedundantOrderBy ?? + OrderBySettings.ThrowOnRedundantOrderBy; // Analyze the query for ordering and includes in a single pass var (hasOrdering, hasInclude) = QueryAnalyzer.Analyze(query); diff --git a/src/EfOrderBy/OrderByExtensions.cs b/src/EfOrderBy/OrderByExtensions.cs index 06c43e2..fee4bfd 100644 --- a/src/EfOrderBy/OrderByExtensions.cs +++ b/src/EfOrderBy/OrderByExtensions.cs @@ -25,12 +25,14 @@ public static class OrderByExtensions /// When true, throws an exception when a query explicitly orders by exactly the same /// properties and directions as the configured default ordering. Useful in tests to /// find query ordering that is redundant because the default ordering already applies it. + /// When null (default), is used, + /// so passing false here opts this context out of that process wide setting. /// public static DbContextOptionsBuilder UseDefaultOrderBy( this DbContextOptionsBuilder builder, bool requireOrderingForAllEntities = false, bool? createIndexes = true, - bool throwOnRedundantOrderBy = false) + bool? throwOnRedundantOrderBy = null) { builder.AddInterceptors(interceptor); diff --git a/src/EfOrderBy/OrderBySettings.cs b/src/EfOrderBy/OrderBySettings.cs new file mode 100644 index 0000000..db8e121 --- /dev/null +++ b/src/EfOrderBy/OrderBySettings.cs @@ -0,0 +1,18 @@ +namespace EfOrderBy; + +/// +/// Process wide defaults for . +/// +/// +/// Intended to be set from a module initializer in a test project, so every +/// in that project opts in without each call +/// having to be changed. A value passed to that method always wins, whether true or false. +/// +public static class OrderBySettings +{ + /// + /// The default used when the throwOnRedundantOrderBy parameter of + /// is left null. Defaults to false. + /// + public static bool ThrowOnRedundantOrderBy { get; set; } +} diff --git a/src/EfOrderBy/OrderRequiredExtension.cs b/src/EfOrderBy/OrderRequiredExtension.cs index 8d5d61d..0cdfcf7 100644 --- a/src/EfOrderBy/OrderRequiredExtension.cs +++ b/src/EfOrderBy/OrderRequiredExtension.cs @@ -1,8 +1,12 @@ -sealed class OrderRequiredExtension(bool requireOrderingForAllEntities, bool createIndexes, bool throwOnRedundantOrderBy) : +sealed class OrderRequiredExtension(bool requireOrderingForAllEntities, bool createIndexes, bool? throwOnRedundantOrderBy) : IDbContextOptionsExtension { public bool RequireOrderingForAllEntities { get; } = requireOrderingForAllEntities; - public bool ThrowOnRedundantOrderBy { get; } = throwOnRedundantOrderBy; + + // Null means the caller did not set it, so OrderBySettings decides. An explicit false is + // not the same thing: it opts this context out of a process wide setting of true. + public bool? ThrowOnRedundantOrderBy { get; } = throwOnRedundantOrderBy; + bool CreateIndexes { get; } = createIndexes; public DbContextOptionsExtensionInfo Info => new ExtensionInfo(this); @@ -34,7 +38,13 @@ class ExtensionInfo(IDbContextOptionsExtension extension) : public override string LogFragment => $"{(Extension.RequireOrderingForAllEntities ? "RequireOrderingForAllEntities " : "")}" + $"{(Extension.CreateIndexes ? "" : "CreateIndexes=false ")}" + - $"{(Extension.ThrowOnRedundantOrderBy ? "ThrowOnRedundantOrderBy " : "")}"; + // An explicit false is worth logging, since it overrides OrderBySettings + Extension.ThrowOnRedundantOrderBy switch + { + true => "ThrowOnRedundantOrderBy ", + false => "ThrowOnRedundantOrderBy=false ", + null => "" + }; public override int GetServiceProviderHashCode() => HashCode.Combine(Extension.RequireOrderingForAllEntities, Extension.CreateIndexes, Extension.ThrowOnRedundantOrderBy); @@ -49,7 +59,7 @@ public override void PopulateDebugInfo(IDictionary debugInfo) { debugInfo["DefaultOrderBy:RequireOrderingForAllEntities"] = Extension.RequireOrderingForAllEntities.ToString(); debugInfo["DefaultOrderBy:CreateIndexes"] = Extension.CreateIndexes.ToString(); - debugInfo["DefaultOrderBy:ThrowOnRedundantOrderBy"] = Extension.ThrowOnRedundantOrderBy.ToString(); + debugInfo["DefaultOrderBy:ThrowOnRedundantOrderBy"] = Extension.ThrowOnRedundantOrderBy?.ToString() ?? "null"; } } } diff --git a/src/EntityFramework.OrderBy.slnx b/src/EntityFramework.OrderBy.slnx index 1d9b83c..c20f208 100644 --- a/src/EntityFramework.OrderBy.slnx +++ b/src/EntityFramework.OrderBy.slnx @@ -8,5 +8,6 @@ + diff --git a/src/Settings.Tests/GlobalUsings.cs b/src/Settings.Tests/GlobalUsings.cs new file mode 100644 index 0000000..bc303a0 --- /dev/null +++ b/src/Settings.Tests/GlobalUsings.cs @@ -0,0 +1,4 @@ +global using System.Runtime.CompilerServices; +global using EfOrderBy; +global using Microsoft.EntityFrameworkCore; +global using NUnit.Framework; diff --git a/src/Settings.Tests/ModuleInitializer.cs b/src/Settings.Tests/ModuleInitializer.cs new file mode 100644 index 0000000..9631cb4 --- /dev/null +++ b/src/Settings.Tests/ModuleInitializer.cs @@ -0,0 +1,13 @@ +public static class ModuleInitializer +{ + // The whole point of this project. OrderBySettings is process wide, so it cannot be + // exercised alongside tests that expect redundant ordering to be allowed, which is why + // these tests live in their own assembly. + #region ThrowOnRedundantOrderBySetting + + [ModuleInitializer] + public static void Initialize() => + OrderBySettings.ThrowOnRedundantOrderBy = true; + + #endregion +} diff --git a/src/Settings.Tests/Settings.Tests.csproj b/src/Settings.Tests/Settings.Tests.csproj new file mode 100644 index 0000000..3bb5837 --- /dev/null +++ b/src/Settings.Tests/Settings.Tests.csproj @@ -0,0 +1,17 @@ + + + + false + true + + + + + + + + + + + + diff --git a/src/Settings.Tests/ThrowOnRedundantOrderBySettingTests.cs b/src/Settings.Tests/ThrowOnRedundantOrderBySettingTests.cs new file mode 100644 index 0000000..00edbd7 --- /dev/null +++ b/src/Settings.Tests/ThrowOnRedundantOrderBySettingTests.cs @@ -0,0 +1,163 @@ +[TestFixture] +public class ThrowOnRedundantOrderBySettingTests +{ + static DbContextOptions unset = + new DbContextOptionsBuilder() + .UseSqlServer("Server=.;Database=Test;") + .UseDefaultOrderBy() + .Options; + + static DbContextOptions optedOut = + new DbContextOptionsBuilder() + .UseSqlServer("Server=.;Database=Test;") + .UseDefaultOrderBy(throwOnRedundantOrderBy: false) + .Options; + + static DbContextOptions optedIn = + new DbContextOptionsBuilder() + .UseSqlServer("Server=.;Database=Test;") + .UseDefaultOrderBy(throwOnRedundantOrderBy: true) + .Options; + + [Test] + public void ModuleInitializerAppliesTheSetting() => + Assert.That(OrderBySettings.ThrowOnRedundantOrderBy, Is.True); + + [Test] + public void NotSetOnTheContext_UsesTheSetting() + { + using var context = new UnsetContext(unset); + + var exception = Assert.Throws( + () => context.Entities + .OrderBy(_ => _.Name) + .ThenByDescending(_ => _.Priority) + .ToQueryString()); + + Assert.That(exception!.Message, Does.Contain("SettingsEntity")); + Assert.That(exception.Message, Does.Contain("OrderBy(Name).ThenByDescending(Priority)")); + } + + [Test] + public void FalseOnTheContext_OverridesTheSetting() + { + using var context = new OptedOutContext(optedOut); + + Assert.DoesNotThrow( + () => context.Entities + .OrderBy(_ => _.Name) + .ThenByDescending(_ => _.Priority) + .ToQueryString()); + } + + [Test] + public void TrueOnTheContext_AgreesWithTheSetting() + { + using var context = new OptedInContext(optedIn); + + Assert.Throws( + () => context.Entities + .OrderBy(_ => _.Name) + .ThenByDescending(_ => _.Priority) + .ToQueryString()); + } + + [Test] + public void NotSetOnTheContext_UsesTheSettingForIncludes() + { + using var context = new UnsetContext(unset); + + Assert.Throws( + () => context.Entities + .Include(_ => _.Children.OrderBy(child => child.SortOrder)) + .ToQueryString()); + } + + [Test] + public void FalseOnTheContext_OverridesTheSettingForIncludes() + { + using var context = new OptedOutContext(optedOut); + + Assert.DoesNotThrow( + () => context.Entities + .Include(_ => _.Children.OrderBy(child => child.SortOrder)) + .ToQueryString()); + } + + [Test] + public void NonRedundantOrdering_DoesNotThrow() + { + using var context = new UnsetContext(unset); + + Assert.DoesNotThrow( + () => context.Entities + .OrderByDescending(_ => _.Name) + .ToQueryString()); + } +} + +public class SettingsEntity +{ + public int Id { get; set; } + public string Name { get; set; } = ""; + public int Priority { get; set; } + public List Children { get; set; } = []; +} + +public class SettingsChild +{ + public int Id { get; set; } + public int SettingsEntityId { get; set; } + public SettingsEntity Parent { get; set; } = null!; + public int SortOrder { get; set; } +} + +public class UnsetContext(DbContextOptions options) : + DbContext(options) +{ + public DbSet Entities => Set(); + + protected override void OnModelCreating(ModelBuilder builder) => + builder.ConfigureSettingsEntities(); +} + +public class OptedOutContext(DbContextOptions options) : + DbContext(options) +{ + public DbSet Entities => Set(); + + protected override void OnModelCreating(ModelBuilder builder) => + builder.ConfigureSettingsEntities(); +} + +public class OptedInContext(DbContextOptions options) : + DbContext(options) +{ + public DbSet Entities => Set(); + + protected override void OnModelCreating(ModelBuilder builder) => + builder.ConfigureSettingsEntities(); +} + +static class SettingsModelBuilder +{ + // All three contexts share these entities, so the configuration must match + public static void ConfigureSettingsEntities(this ModelBuilder builder) + { + builder.Entity() + .HasMany(_ => _.Children) + .WithOne(_ => _.Parent) + .HasForeignKey(_ => _.SettingsEntityId) + .IsRequired(); + + builder.Entity() + .Property(_ => _.Name).HasMaxLength(450); + + builder.Entity() + .OrderBy(_ => _.Name) + .ThenByDescending(_ => _.Priority); + + builder.Entity() + .OrderBy(_ => _.SortOrder); + } +} diff --git a/src/Tests/Snippets.cs b/src/Tests/Snippets.cs index 149aafd..355d294 100644 --- a/src/Tests/Snippets.cs +++ b/src/Tests/Snippets.cs @@ -51,6 +51,18 @@ protected override void OnConfiguring(DbContextOptionsBuilder builder) => #endregion } +public class OptOutOfThrowOnRedundantOrderByExample : DbContext +{ + #region OptOutOfThrowOnRedundantOrderBy + + // Passing false explicitly overrides OrderBySettings.ThrowOnRedundantOrderBy + protected override void OnConfiguring(DbContextOptionsBuilder builder) => + builder.UseDefaultOrderBy( + throwOnRedundantOrderBy: false); + + #endregion +} + public class DisableIndexCreationExample : DbContext { #region DisableIndexCreation