Skip to content

Commit 009f13c

Browse files
committed
Added wrapper EF collations, collation checks, and provider comparers.
1 parent da57bf5 commit 009f13c

File tree

10 files changed

+595
-46
lines changed

10 files changed

+595
-46
lines changed

DomainModeling.Generator/Configurators/EntityFrameworkConfigurationGenerator.cs

Lines changed: 430 additions & 27 deletions
Large diffs are not rendered by default.
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
using Microsoft.Extensions.Logging;
2+
3+
namespace Architect.DomainModeling.Tests.Common;
4+
5+
public sealed class CapturingLoggerProvider : ILoggerProvider
6+
{
7+
public IReadOnlyList<string> Logs => this._logs;
8+
private readonly List<string> _logs = [];
9+
10+
public ILogger CreateLogger(string categoryName)
11+
{
12+
return new CapturingLogger(this._logs);
13+
}
14+
15+
public void Dispose()
16+
{
17+
}
18+
19+
public sealed class CapturingLogger(
20+
List<string> logs)
21+
: ILogger
22+
{
23+
public IDisposable? BeginScope<TState>(TState state) where TState : notnull
24+
{
25+
return null;
26+
}
27+
28+
public bool IsEnabled(LogLevel logLevel)
29+
{
30+
return true;
31+
}
32+
33+
public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter)
34+
{
35+
logs.Add($"[{logLevel}] {formatter(state, exception)}");
36+
}
37+
}
38+
}

DomainModeling.Tests/EntityFramework/EntityFrameworkConfigurationGeneratorTests.cs

Lines changed: 57 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
1+
using System.Diagnostics.CodeAnalysis;
2+
using Architect.DomainModeling.Configuration;
13
using Architect.DomainModeling.Conversions;
4+
using Architect.DomainModeling.Tests.Common;
25
using Architect.DomainModeling.Tests.IdentityTestTypes;
36
using Microsoft.EntityFrameworkCore;
47
using Microsoft.EntityFrameworkCore.Metadata.Conventions;
58
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
9+
using Microsoft.Extensions.Logging;
610
using Xunit;
711

812
namespace Architect.DomainModeling.Tests.EntityFramework;
@@ -11,12 +15,20 @@ public sealed class EntityFrameworkConfigurationGeneratorTests : IDisposable
1115
{
1216
internal static bool AllowParameterizedConstructors = true;
1317

18+
private ILoggerFactory LoggerFactory { get; }
19+
private CapturingLoggerProvider CapturingLoggerProvider { get; }
20+
1421
private string UniqueName { get; } = Guid.NewGuid().ToString("N");
1522
private TestDbContext DbContext { get; }
1623

1724
public EntityFrameworkConfigurationGeneratorTests()
1825
{
19-
this.DbContext = new TestDbContext($"DataSource={this.UniqueName};Mode=Memory;Cache=Shared;");
26+
this.CapturingLoggerProvider = new CapturingLoggerProvider();
27+
this.LoggerFactory = Microsoft.Extensions.Logging.LoggerFactory.Create(options => options
28+
.SetMinimumLevel(LogLevel.Debug)
29+
.AddProvider(this.CapturingLoggerProvider));
30+
31+
this.DbContext = new TestDbContext($"DataSource={this.UniqueName};Mode=Memory;Cache=Shared;", this.LoggerFactory);
2032
this.DbContext.Database.OpenConnection();
2133
}
2234

@@ -61,7 +73,7 @@ public void ConfigureConventions_WithAllExtensionsCalled_ShouldBeAbleToWorkWithA
6173

6274
Assert.Equal(2, reloadedDomainEvent.Id);
6375

64-
Assert.Equal(2, reloadedEntity.Id.Value);
76+
Assert.Equal("A", reloadedEntity.Id.Value);
6577
Assert.Equal("One", reloadedEntity.Values.One);
6678
Assert.Equal(2m, reloadedEntity.Values.Two);
6779
Assert.Equal(3, reloadedEntity.Values.Three.Value?.Value.Value);
@@ -76,13 +88,32 @@ public void ConfigureConventions_WithAllExtensionsCalled_ShouldBeAbleToWorkWithA
7688
var providerClrTypeForStringWrapperWithCustomIntCore = mappingForStringWithCustomIntCore?.GetValueConverter()?.ProviderClrType;
7789
Assert.Equal("INTEGER", columnTypeForStringWrapperWithCustomIntCore);
7890
Assert.Equal(typeof(int), providerClrTypeForStringWrapperWithCustomIntCore);
91+
92+
// Case-sensitivity should be honored, even during key comparisons
93+
Assert.Same(reloadedEntity, this.DbContext.Set<EntityForEF>().Find(new EntityForEFId("a")));
94+
95+
// The database's collation should have been made ignore-case by our ConfigureIdentityConventions() options
96+
Assert.Same(reloadedEntity, this.DbContext.Set<EntityForEF>().SingleOrDefault(x => x.Id == "a"));
97+
98+
// The logs should warn that Wrapper1ForEF has a mismatching collation in the database
99+
var logs = this.CapturingLoggerProvider.Logs;
100+
var warning = Assert.Single(logs, log => log.StartsWith("[Warning]"));
101+
Assert.Equal("[Warning] Architect.DomainModeling.Tests.EntityFramework.ValueObjectForEF.One uses OrdinalIgnoreCase comparisons, but the default SQLite database collation acts more like Ordinal - use the options in ConfigureIdentityConventions() and ConfigureWrapperValueObjectConventions() to specify default collations, or configure property collations manually", warning);
102+
103+
// The logs should show that certain collations were set
104+
Assert.Contains(logs, log => log.Equals("[Debug] Set collation BINARY for SomeStringId properties based on the type's case-sensitivity"));
105+
Assert.Contains(logs, log => log.Equals("[Debug] Set collation NOCASE for EntityForEFId properties based on the type's case-sensitivity"));
79106
}
80107
}
81108

82109
internal sealed class TestDbContext(
83-
string connectionString)
84-
: DbContext(new DbContextOptionsBuilder<TestDbContext>().UseSqlite(connectionString).Options)
110+
string connectionString, ILoggerFactory loggerFactory)
111+
: DbContext(new DbContextOptionsBuilder<TestDbContext>()
112+
.UseLoggerFactory(loggerFactory)
113+
.UseSqlite(connectionString).Options)
85114
{
115+
[SuppressMessage("CodeQuality", "IDE0079:Remove unnecessary suppression", Justification = "Suppression is necessary.")]
116+
[SuppressMessage("Usage", "CA2263:Prefer generic overload when type is known", Justification = "We have no generic info for types received from callbacks.")]
86117
protected override void ConfigureConventions(ModelConfigurationBuilder configurationBuilder)
87118
{
88119
configurationBuilder.Conventions.Remove<ConstructorBindingConvention>();
@@ -91,17 +122,23 @@ protected override void ConfigureConventions(ModelConfigurationBuilder configura
91122

92123
configurationBuilder.ConfigureDomainModelConventions(domainModel =>
93124
{
94-
domainModel.ConfigureIdentityConventions();
125+
domainModel.ConfigureIdentityConventions(new IdentityConfigurationOptions() { CaseSensitiveCollation = "BINARY", IgnoreCaseCollation = "NOCASE", });
95126
domainModel.ConfigureWrapperValueObjectConventions();
96127
domainModel.ConfigureEntityConventions();
97128
domainModel.ConfigureDomainEventConventions();
98-
});
99129

100-
// For a wrapper whose core type EF does not support, overwriting the conventions with our own should work
101-
configurationBuilder.Properties<LazyStringWrapper>()
102-
.HaveConversion<LazyStringWrapperConverter>();
103-
configurationBuilder.DefaultTypeMapping<LazyStringWrapper>()
104-
.HasConversion<LazyStringWrapperConverter>();
130+
domainModel.CustomizeWrapperValueObjectConventions(context =>
131+
{
132+
// For a wrapper whose core type EF does not support, overwriting the conventions with our own should work
133+
if (context.CoreType == typeof(Lazy<string>))
134+
{
135+
context.ConfigurationBuilder.Properties(context.ModelType)
136+
.HaveConversion(typeof(LazyStringWrapperConverter));
137+
context.ConfigurationBuilder.DefaultTypeMapping(context.ModelType)
138+
.HasConversion(typeof(LazyStringWrapperConverter));
139+
}
140+
});
141+
});
105142
}
106143

107144
private class LazyStringWrapperConverter : ValueConverter<LazyStringWrapper, string>
@@ -168,8 +205,14 @@ public DomainEventForEF(DomainEventForEFId id, object ignored)
168205
[IdentityValueObject<decimal>]
169206
public readonly partial record struct DomainEventForEFId;
170207

208+
[IdentityValueObject<string>]
209+
public partial record struct EntityForEFId
210+
{
211+
private StringComparison StringComparison => StringComparison.OrdinalIgnoreCase;
212+
}
213+
171214
[Entity]
172-
internal sealed class EntityForEF : Entity<EntityForEFId, int>
215+
internal sealed class EntityForEF : Entity<EntityForEFId>
173216
{
174217
/// <summary>
175218
/// This lets us test if a constructor is used or not.
@@ -179,7 +222,7 @@ internal sealed class EntityForEF : Entity<EntityForEFId, int>
179222
public ValueObjectForEF Values { get; }
180223

181224
public EntityForEF(ValueObjectForEF values)
182-
: base(id: 2)
225+
: base(id: "A")
183226
{
184227
if (!EntityFrameworkConfigurationGeneratorTests.AllowParameterizedConstructors)
185228
throw new InvalidOperationException("Deserialization was not allowed to use the parameterized constructors.");
@@ -200,7 +243,7 @@ private EntityForEF()
200243
[WrapperValueObject<string>]
201244
internal sealed partial class Wrapper1ForEF
202245
{
203-
protected override StringComparison StringComparison => StringComparison.Ordinal;
246+
protected override StringComparison StringComparison => StringComparison.OrdinalIgnoreCase;
204247

205248
/// <summary>
206249
/// This lets us test if a constructor is used or not.

DomainModeling/Configuration/IIdentityConfigurator.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using System.Diagnostics.CodeAnalysis;
2-
using Architect.DomainModeling.Conversions;
32

43
namespace Architect.DomainModeling.Configuration;
54

DomainModeling/Configuration/IWrapperValueObjectConfigurator.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using System.Diagnostics.CodeAnalysis;
2-
using Architect.DomainModeling.Conversions;
32

43
namespace Architect.DomainModeling.Configuration;
54

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
namespace Architect.DomainModeling.Configuration;
2+
3+
public record class IdentityConfigurationOptions : ValueWrapperConfigurationOptions
4+
{
5+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
namespace Architect.DomainModeling.Configuration;
2+
3+
/// <summary>
4+
/// Base options for value wrappers, used by both <see cref="IdentityConfigurationOptions"/> and <see cref="WrapperValueObjectConfigurationOptions"/>.
5+
/// </summary>
6+
public abstract record class ValueWrapperConfigurationOptions
7+
{
8+
/// <summary>
9+
/// <para>
10+
/// If specified, this collation is set on configured <see langword="string"/> <see cref="IIdentity{T}"/> types that use <see cref="StringComparison.Ordinal"/>.
11+
/// </para>
12+
/// <para>
13+
/// This helps the database column match the model's behavior.
14+
/// </para>
15+
/// </summary>
16+
public string? CaseSensitiveCollation { get; init; }
17+
18+
/// <summary>
19+
/// <para>
20+
/// If specified, this collation is set on configured <see langword="string"/> <see cref="IIdentity{T}"/> types that use <see cref="StringComparison.OrdinalIgnoreCase"/>.
21+
/// </para>
22+
/// <para>
23+
/// This helps the database column match the model's behavior.
24+
/// </para>
25+
/// </summary>
26+
public string? IgnoreCaseCollation { get; init; }
27+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
namespace Architect.DomainModeling.Configuration;
2+
3+
public record class WrapperValueObjectConfigurationOptions : ValueWrapperConfigurationOptions
4+
{
5+
}

DomainModeling/DomainModeling.csproj

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,19 @@ Release notes:
3737
Platform support:
3838
- BREAKING: Platform support: Dropped support for .NET 6.0 and .NET 7.0 (EOL).
3939

40-
The base class is dead. Long live the interface!
41-
4240
Completed support for nested wrappers:
4341
- Feature: Nested WrapperValueObject/Identity types can now also wrap/unwrap and serialize/deserialize directly to/from their core (deepest) underlying type.
4442
- Feature: EF conversions for such types now automatically map to and from the core type.
4543
- Feature: A deeper underlying type can be simulated, e.g. LazyStringId : IIdentity&lt;Lazy&lt;string&gt;&gt;, ICoreValueWrapper&lt;LazyStringId, string&gt;.
4644
- BREAKING: ISerializableDomainObject is deprecated in favor of IValueWrapper, a clear and comprehensive type to represented generic value wrappers.
4745
- BREAKING: IIdentityConfigurator and IWrapperValueObjectConfigurator now receive an additional type parameter on their methods, namely the core type.
4846

47+
Correct string comparisons with EF:
48+
- Feature: ConfigureIdentityConventions()/ConfigureWrapperValueObjectConventions() now set a PROVIDER value comparer for each string wrapper property, matching the type's case-sensitivity. Since EF Core 7, EF compares keys using the provider type instead of the model type.
49+
- Feature: ConfigureIdentityConventions()/ConfigureWrapperValueObjectConventions() now warn if a string wrapper property has a collation mismatching the type's case-sensitivity, unless collation was explicitly chosen.
50+
- Feature: ConfigureIdentityConventions()/ConfigureWrapperValueObjectConventions() now take an optional "options" parameter, which allows specifying the respective collations for case-sensitive vs. ignore-case string wrappers.
51+
- Feature: ConfigureDomainModelConventions() now has convenience extension methods CustomizeIdentityConventions()/CustomizeWrapperValueObjectConventions(), for easy custom conventions, such as based on the core underlying type.
52+
4953
Performance:
5054
- Enhancement: Reduced assembly size by having source-generated WrapperValueObject/Identity types use generic JSON serializers instead of generating their own.
5155
- Enhancement: Improved source generator performance.

README.md

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,27 +371,53 @@ internal sealed class MyDbContext : DbContext
371371
{
372372
// Snip
373373
374+
[SuppressMessage("CodeQuality", "IDE0079:Remove unnecessary suppression", Justification = "Suppression is necessary.")]
375+
[SuppressMessage("Usage", "CA2263:Prefer generic overload when type is known", Justification = "We have no generic info for types received from callbacks.")]
374376
protected override void ConfigureConventions(ModelConfigurationBuilder configurationBuilder)
375377
{
376378
// Recommended to keep EF from throwing if it sees no usable constructor, if we are keeping it from using constructors anyway
377379
configurationBuilder.Conventions.Remove<ConstructorBindingConvention>();
378380

379381
configurationBuilder.ConfigureDomainModelConventions(domainModel =>
380382
{
383+
// Defaults
381384
domainModel.ConfigureIdentityConventions();
382385
domainModel.ConfigureWrapperValueObjectConventions();
383386
domainModel.ConfigureEntityConventions();
384387
domainModel.ConfigureDomainEventConventions();
388+
389+
domainModel.CustomizeIdentityConventions(context =>
390+
{
391+
// Example: Use fixed-length strings with a binary collation for all string IIdentities
392+
if (context.CoreType == typeof(string))
393+
{
394+
context.ConfigurationBuilder.Properties(context.ModelType)
395+
.HaveMaxLength(16)
396+
.AreFixedLength()
397+
.UseCollation("Latin1_General_100_BIN2");
398+
}
399+
});
400+
401+
domainModel.CustomizeWrapperValueObjectConventions(context =>
402+
{
403+
// Example: Use DECIMAL(19, 9) for all decimal wrappers
404+
if (context.CoreType == typeof(decimal))
405+
{
406+
context.ConfigurationBuilder.Properties(context.ModelType)
407+
.HavePrecision(19, 9);
408+
}
409+
});
385410
});
386411
}
387412
}
388413
```
389414

390415
`ConfigureDomainModelConventions()` itself does not have any effect other than to invoke its action, which allows the specific mapping kinds to be chosen.
391416
The inner calls, such as to `ConfigureIdentityConventions()`, configure the various conventions.
417+
The `Customize*()` methods make it easy to specify your own conventions, such as for every identity or wrapper value object with a string at its core.
392418

393419
Thanks to the provided conventions, no manual boilerplate mappings are needed, like conversions to primitives.
394-
The developer need only write meaningful mappings, such as the maximum length of a string property.
420+
Property-specific mappings are only needed where they are meaningful, such as the maximum length of a particular string property.
395421

396422
Since only conventions are registered, regular mappings can override any part of the provided behavior.
397423

0 commit comments

Comments
 (0)