Skip to content

Commit da9fe63

Browse files
AndriySvyrydAndriySvyryd
committed
Add entity splitting tests for migrations and update pipeline
Don't use identity be default for columns with FKs even when the property is mapped to other columns that should use identity Part of #620
1 parent f72888f commit da9fe63

File tree

13 files changed

+314
-39
lines changed

13 files changed

+314
-39
lines changed

‎src/EFCore.Relational/Metadata/Conventions/RelationalValueGenerationConvention.cs‎

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,23 @@ public virtual void ProcessEntityTypeAnnotationChanged(
7676
IConventionAnnotation? oldAnnotation,
7777
IConventionContext<IConventionAnnotation> context)
7878
{
79-
if (name == RelationalAnnotationNames.TableName)
79+
if (name == RelationalAnnotationNames.ViewName
80+
|| name == RelationalAnnotationNames.FunctionName
81+
|| name == RelationalAnnotationNames.SqlQuery)
82+
{
83+
if (annotation?.Value != null
84+
&& oldAnnotation?.Value == null
85+
&& entityTypeBuilder.Metadata.GetTableName() == null)
86+
{
87+
ProcessTableChanged(
88+
entityTypeBuilder,
89+
entityTypeBuilder.Metadata.GetDefaultTableName(),
90+
entityTypeBuilder.Metadata.GetDefaultSchema(),
91+
null,
92+
null);
93+
}
94+
}
95+
else if (name == RelationalAnnotationNames.TableName)
8096
{
8197
var schema = entityTypeBuilder.Metadata.GetSchema();
8298
ProcessTableChanged(
@@ -105,29 +121,43 @@ private static void ProcessTableChanged(
105121
string? newTable,
106122
string? newSchema)
107123
{
124+
if (newTable == null)
125+
{
126+
foreach (var property in entityTypeBuilder.Metadata.GetProperties())
127+
{
128+
property.Builder.ValueGenerated(null);
129+
}
130+
131+
return;
132+
}
133+
else if (oldTable == null)
134+
{
135+
foreach (var property in entityTypeBuilder.Metadata.GetProperties())
136+
{
137+
property.Builder.ValueGenerated(GetValueGenerated(property, StoreObjectIdentifier.Table(newTable, newSchema)));
138+
}
139+
140+
return;
141+
}
142+
108143
var primaryKey = entityTypeBuilder.Metadata.FindPrimaryKey();
109144
if (primaryKey == null)
110145
{
111146
return;
112147
}
113148

114-
var oldLink = oldTable != null
115-
? entityTypeBuilder.Metadata.FindRowInternalForeignKeys(StoreObjectIdentifier.Table(oldTable, oldSchema))
116-
: null;
117-
var newLink = newTable != null
118-
? entityTypeBuilder.Metadata.FindRowInternalForeignKeys(StoreObjectIdentifier.Table(newTable, newSchema))
119-
: null;
149+
var oldLink = entityTypeBuilder.Metadata.FindRowInternalForeignKeys(StoreObjectIdentifier.Table(oldTable, oldSchema));
150+
var newLink = entityTypeBuilder.Metadata.FindRowInternalForeignKeys(StoreObjectIdentifier.Table(newTable, newSchema));
120151

121-
if ((oldLink?.Any() != true
122-
&& newLink?.Any() != true)
123-
|| newLink == null)
152+
if (!oldLink.Any()
153+
&& !newLink.Any())
124154
{
125155
return;
126156
}
127157

128158
foreach (var property in primaryKey.Properties)
129159
{
130-
property.Builder.ValueGenerated(GetValueGenerated(property, StoreObjectIdentifier.Table(newTable!, newSchema)));
160+
property.Builder.ValueGenerated(GetValueGenerated(property, StoreObjectIdentifier.Table(newTable, newSchema)));
131161
}
132162
}
133163

‎src/EFCore.Relational/Metadata/Internal/ColumnMappingBaseComparer.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,19 +52,19 @@ public int Compare(IColumnMappingBase? x, IColumnMappingBase? y)
5252
return result;
5353
}
5454

55-
result = StringComparer.Ordinal.Compare(x.Property.Name, y.Property.Name);
55+
result = TableMappingBaseComparer.Instance.Compare(x.TableMapping, y.TableMapping);
5656
if (result != 0)
5757
{
5858
return result;
5959
}
6060

61-
result = StringComparer.Ordinal.Compare(x.Column.Name, y.Column.Name);
61+
result = StringComparer.Ordinal.Compare(x.Property.Name, y.Property.Name);
6262
if (result != 0)
6363
{
6464
return result;
6565
}
6666

67-
return TableMappingBaseComparer.Instance.Compare(x.TableMapping, y.TableMapping);
67+
return StringComparer.Ordinal.Compare(x.Column.Name, y.Column.Name);
6868
}
6969

7070
/// <summary>

‎src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
using System.Collections;
55
using System.Globalization;
6-
using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
76
using Microsoft.EntityFrameworkCore.Metadata.Internal;
87
using Microsoft.EntityFrameworkCore.Update.Internal;
98

‎src/EFCore.SqlServer/Extensions/SqlServerPropertyExtensions.cs‎

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -406,31 +406,53 @@ internal static SqlServerValueGenerationStrategy GetValueGenerationStrategy(
406406
ITypeMappingSource? typeMappingSource)
407407
{
408408
var annotation = property.FindAnnotation(SqlServerAnnotationNames.ValueGenerationStrategy);
409-
if (annotation != null)
409+
if (annotation?.Value != null
410+
&& StoreObjectIdentifier.Create(property.DeclaringEntityType, storeObject.StoreObjectType) == storeObject)
410411
{
411-
return (SqlServerValueGenerationStrategy?)annotation.Value ?? SqlServerValueGenerationStrategy.None;
412+
return (SqlServerValueGenerationStrategy)annotation.Value;
412413
}
413414

415+
var table = storeObject;
414416
var sharedTableRootProperty = property.FindSharedStoreObjectRootProperty(storeObject);
415417
if (sharedTableRootProperty != null)
416418
{
417-
return sharedTableRootProperty.GetValueGenerationStrategy(storeObject)
419+
return sharedTableRootProperty.GetValueGenerationStrategy(storeObject, typeMappingSource)
418420
== SqlServerValueGenerationStrategy.IdentityColumn
419-
&& property.GetContainingForeignKeys().All(fk => fk.IsBaseLinking())
421+
&& table.StoreObjectType == StoreObjectType.Table
422+
&& !property.GetContainingForeignKeys().Any(fk =>
423+
!fk.IsBaseLinking()
424+
|| (StoreObjectIdentifier.Create(fk.PrincipalEntityType, StoreObjectType.Table)
425+
is StoreObjectIdentifier principal
426+
&& fk.GetConstraintName(table, principal) != null))
420427
? SqlServerValueGenerationStrategy.IdentityColumn
421428
: SqlServerValueGenerationStrategy.None;
422429
}
423430

424431
if (property.ValueGenerated != ValueGenerated.OnAdd
425-
|| property.GetContainingForeignKeys().Any(fk => !fk.IsBaseLinking())
432+
|| table.StoreObjectType != StoreObjectType.Table
426433
|| property.TryGetDefaultValue(storeObject, out _)
427434
|| property.GetDefaultValueSql(storeObject) != null
428-
|| property.GetComputedColumnSql(storeObject) != null)
435+
|| property.GetComputedColumnSql(storeObject) != null
436+
|| property.GetContainingForeignKeys()
437+
.Any(fk =>
438+
!fk.IsBaseLinking()
439+
|| (StoreObjectIdentifier.Create(fk.PrincipalEntityType, StoreObjectType.Table)
440+
is StoreObjectIdentifier principal
441+
&& fk.GetConstraintName(table, principal) != null)))
429442
{
430443
return SqlServerValueGenerationStrategy.None;
431444
}
432445

433-
return GetDefaultValueGenerationStrategy(property, storeObject, typeMappingSource);
446+
var defaultStategy = GetDefaultValueGenerationStrategy(property, storeObject, typeMappingSource);
447+
if (defaultStategy != SqlServerValueGenerationStrategy.None)
448+
{
449+
if (annotation != null)
450+
{
451+
return (SqlServerValueGenerationStrategy?)annotation.Value ?? SqlServerValueGenerationStrategy.None;
452+
}
453+
}
454+
455+
return defaultStategy;
434456
}
435457

436458
private static SqlServerValueGenerationStrategy GetDefaultValueGenerationStrategy(IReadOnlyProperty property)
@@ -455,7 +477,6 @@ private static SqlServerValueGenerationStrategy GetDefaultValueGenerationStrateg
455477
ITypeMappingSource? typeMappingSource)
456478
{
457479
var modelStrategy = property.DeclaringEntityType.Model.GetValueGenerationStrategy();
458-
459480
if (modelStrategy == SqlServerValueGenerationStrategy.SequenceHiLo
460481
&& IsCompatibleWithValueGeneration(property, storeObject, typeMappingSource))
461482
{

‎src/EFCore.SqlServer/Metadata/Conventions/SqlServerValueGenerationConvention.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ public override void ProcessEntityTypeAnnotationChanged(
102102
return null;
103103
}
104104

105+
// If the first mapping can be value generated then we'll consider all mappings to be value generated
106+
// as this is a client-side configuration and can't be specified per-table.
105107
return GetValueGenerated(property, declaringTable, Dependencies.TypeMappingSource);
106108
}
107109

‎src/EFCore.SqlServer/Metadata/Conventions/SqlServerValueGenerationStrategyConvention.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
5-
64
// ReSharper disable once CheckNamespace
75
namespace Microsoft.EntityFrameworkCore.Metadata.Conventions;
86

‎src/EFCore.SqlServer/Metadata/Internal/SqlServerAnnotationProvider.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,8 +219,7 @@ public override IEnumerable<IAnnotation> For(IColumn column, bool designTime)
219219

220220
var table = StoreObjectIdentifier.Table(column.Table.Name, column.Table.Schema);
221221
var identityProperty = column.PropertyMappings.Where(
222-
m => (m.TableMapping.IsSharedTablePrincipal ?? true)
223-
&& m.TableMapping.EntityType == m.Property.DeclaringEntityType)
222+
m => m.TableMapping.EntityType == m.Property.DeclaringEntityType)
224223
.Select(m => m.Property)
225224
.FirstOrDefault(
226225
p => p.GetValueGenerationStrategy(table)

‎test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -574,8 +574,6 @@ public void Views_are_stored_in_the_model_snapshot()
574574
b.Property<int>(""Id"")
575575
.HasColumnType(""int"");
576576
577-
SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property<int>(""Id""), 1L, 1);
578-
579577
b.HasKey(""Id"");
580578
581579
b.ToView(""EntityWithOneProperty"", (string)null);
@@ -596,8 +594,6 @@ public void Views_with_schemas_are_stored_in_the_model_snapshot()
596594
b.Property<int>(""Id"")
597595
.HasColumnType(""int"");
598596
599-
SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property<int>(""Id""), 1L, 1);
600-
601597
b.HasKey(""Id"");
602598
603599
b.ToView(""EntityWithOneProperty"", ""ViewSchema"");
@@ -947,8 +943,6 @@ public virtual void Entity_splitting_is_stored_in_snapshot_with_views()
947943
b.Property<int>(""Id"")
948944
.HasColumnType(""int"");
949945
950-
SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property<int>(""Id""), 1L, 1);
951-
952946
b.Property<int>(""Shadow"")
953947
.HasColumnType(""int"");
954948
@@ -1040,7 +1034,6 @@ public void Unmapped_entity_types_are_stored_in_the_model_snapshot()
10401034
modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithOneProperty"", b =>
10411035
{
10421036
b.Property<int>(""Id"")
1043-
.ValueGeneratedOnAdd()
10441037
.HasColumnType(""int"");
10451038
10461039
b.HasKey(""Id"");
@@ -1122,7 +1115,6 @@ public void Entity_types_mapped_to_queries_are_stored_in_the_model_snapshot()
11221115
modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithOneProperty"", b =>
11231116
{
11241117
b.Property<int>(""Id"")
1125-
.ValueGeneratedOnAdd()
11261118
.HasColumnType(""int"");
11271119
11281120
b.HasKey(""Id"");
@@ -3329,8 +3321,6 @@ protected override void BuildModel(ModelBuilder modelBuilder)
33293321
b1.Property<int>(""Id"")
33303322
.HasColumnType(""int"");
33313323
3332-
SqlServerPropertyBuilderExtensions.UseIdentityColumn(b1.Property<int>(""Id""), 1L, 1);
3333-
33343324
b1.Property<int>(""TestEnum"")
33353325
.HasColumnType(""int"");
33363326

‎test/EFCore.Relational.Specification.Tests/EntitySplittingTestBase.cs‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
// ReSharper disable InconsistentNaming
5+
namespace Microsoft.EntityFrameworkCore;
6+
7+
public abstract class EntitySplittingTestBase : NonSharedModelTestBase
8+
{
9+
protected EntitySplittingTestBase(ITestOutputHelper testOutputHelper)
10+
{
11+
//TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
12+
}
13+
14+
[ConditionalFact(Skip = "Entity splitting query Issue #620")]
15+
public virtual async Task Can_roundtrip()
16+
{
17+
await InitializeAsync(OnModelCreating, sensitiveLogEnabled: false);
18+
19+
await using (var context = CreateContext())
20+
{
21+
var meterReading = new MeterReading { ReadingStatus = MeterReadingStatus.NotAccesible, CurrentRead = "100" };
22+
23+
context.Add(meterReading);
24+
25+
TestSqlLoggerFactory.Clear();
26+
27+
await context.SaveChangesAsync();
28+
29+
Assert.Empty(TestSqlLoggerFactory.Log.Where(l => l.Level == LogLevel.Warning));
30+
}
31+
32+
await using (var context = CreateContext())
33+
{
34+
var reading = await context.MeterReadings.SingleAsync();
35+
36+
Assert.Equal(MeterReadingStatus.NotAccesible, reading.ReadingStatus);
37+
Assert.Equal("100", reading.CurrentRead);
38+
}
39+
}
40+
41+
protected override string StoreName { get; } = "EntitySplittingTest";
42+
43+
protected TestSqlLoggerFactory TestSqlLoggerFactory
44+
=> (TestSqlLoggerFactory)ListLoggerFactory;
45+
46+
protected ContextFactory<EntitySplittingContext> ContextFactory { get; private set; }
47+
48+
protected void AssertSql(params string[] expected)
49+
=> TestSqlLoggerFactory.AssertBaseline(expected);
50+
51+
protected virtual void OnModelCreating(ModelBuilder modelBuilder)
52+
{
53+
modelBuilder.Entity<MeterReading>(
54+
ob =>
55+
{
56+
ob.ToTable("MeterReadings");
57+
ob.SplitToTable(
58+
"MeterReadingDetails", t =>
59+
{
60+
t.Property(o => o.PreviousRead);
61+
t.Property(o => o.CurrentRead);
62+
});
63+
});
64+
}
65+
66+
protected async Task InitializeAsync(Action<ModelBuilder> onModelCreating, bool sensitiveLogEnabled = true)
67+
=> ContextFactory = await InitializeAsync<EntitySplittingContext>(
68+
onModelCreating,
69+
shouldLogCategory: _ => true,
70+
onConfiguring: options =>
71+
{
72+
options.ConfigureWarnings(w => w.Log(RelationalEventId.OptionalDependentWithAllNullPropertiesWarning))
73+
.ConfigureWarnings(w => w.Log(RelationalEventId.OptionalDependentWithoutIdentifyingPropertyWarning))
74+
.EnableSensitiveDataLogging(sensitiveLogEnabled);
75+
}
76+
);
77+
78+
protected virtual EntitySplittingContext CreateContext()
79+
=> ContextFactory.CreateContext();
80+
81+
public override void Dispose()
82+
{
83+
base.Dispose();
84+
85+
ContextFactory = null;
86+
}
87+
88+
protected class EntitySplittingContext : PoolableDbContext
89+
{
90+
public EntitySplittingContext(DbContextOptions options)
91+
: base(options)
92+
{
93+
}
94+
95+
public DbSet<MeterReading> MeterReadings { get; set; }
96+
}
97+
98+
protected class MeterReading
99+
{
100+
public int Id { get; set; }
101+
public MeterReadingStatus? ReadingStatus { get; set; }
102+
public string CurrentRead { get; set; }
103+
public string PreviousRead { get; set; }
104+
}
105+
106+
protected enum MeterReadingStatus
107+
{
108+
Running = 0,
109+
NotAccesible = 2
110+
}
111+
}

0 commit comments

Comments
 (0)