Skip to content

Commit 2f95488

Browse files
committed
Only discover complex types when configured through pre-convention model configuration or annotation
Throw for complex properties of value types Throw for optional complex properties Part of #13947 Fixes #31344
1 parent 2685f70 commit 2f95488

File tree

13 files changed

+123
-66
lines changed

13 files changed

+123
-66
lines changed

src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs

+15-10
Original file line numberDiff line numberDiff line change
@@ -211,22 +211,22 @@ private static void TryUniquifyTableNames(
211211
}
212212

213213
private static void TryUniquifyColumnNames(
214-
IConventionEntityType entityType,
215-
Dictionary<string, IConventionProperty> properties,
214+
IConventionTypeBase type,
215+
Dictionary<string, IConventionProperty> columns,
216216
in StoreObjectIdentifier storeObject,
217217
int maxLength)
218218
{
219-
foreach (var property in entityType.GetDeclaredProperties())
219+
foreach (var property in type.GetDeclaredProperties())
220220
{
221221
var columnName = property.GetColumnName(storeObject);
222222
if (columnName == null)
223223
{
224224
continue;
225225
}
226226

227-
if (!properties.TryGetValue(columnName, out var otherProperty))
227+
if (!columns.TryGetValue(columnName, out var otherProperty))
228228
{
229-
properties[columnName] = property;
229+
columns[columnName] = property;
230230
continue;
231231
}
232232

@@ -256,10 +256,10 @@ private static void TryUniquifyColumnNames(
256256
&& !otherProperty.DeclaringType.IsStrictlyDerivedFrom(property.DeclaringType))
257257
|| (property.DeclaringType as IConventionEntityType)?.FindRowInternalForeignKeys(storeObject).Any() == true)
258258
{
259-
var newColumnName = TryUniquify(property, columnName, properties, storeObject, usePrefix, maxLength);
259+
var newColumnName = TryUniquify(property, columnName, columns, storeObject, usePrefix, maxLength);
260260
if (newColumnName != null)
261261
{
262-
properties[newColumnName] = property;
262+
columns[newColumnName] = property;
263263
continue;
264264
}
265265
}
@@ -269,14 +269,19 @@ private static void TryUniquifyColumnNames(
269269
&& !otherProperty.DeclaringType.IsStrictlyDerivedFrom(property.DeclaringType))
270270
|| (otherProperty.DeclaringType as IConventionEntityType)?.FindRowInternalForeignKeys(storeObject).Any() == true)
271271
{
272-
var newOtherColumnName = TryUniquify(otherProperty, columnName, properties, storeObject, usePrefix, maxLength);
272+
var newOtherColumnName = TryUniquify(otherProperty, columnName, columns, storeObject, usePrefix, maxLength);
273273
if (newOtherColumnName != null)
274274
{
275-
properties[columnName] = property;
276-
properties[newOtherColumnName] = otherProperty;
275+
columns[columnName] = property;
276+
columns[newOtherColumnName] = otherProperty;
277277
}
278278
}
279279
}
280+
281+
foreach (var complexPropery in type.GetDeclaredComplexProperties())
282+
{
283+
TryUniquifyColumnNames(complexPropery.ComplexType, columns, storeObject, maxLength);
284+
}
280285
}
281286

282287
private static string? TryUniquify(

src/EFCore/Infrastructure/ModelValidator.cs

+13
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,19 @@ void Validate(IConventionTypeBase typeBase)
176176
CoreStrings.ComplexPropertyCollection(typeBase.DisplayName(), complexProperty.Name));
177177
}
178178

179+
if (complexProperty.IsNullable)
180+
{
181+
throw new InvalidOperationException(
182+
CoreStrings.ComplexPropertyOptional(typeBase.DisplayName(), complexProperty.Name));
183+
}
184+
185+
if (complexProperty.ComplexType.ClrType.IsValueType)
186+
{
187+
throw new InvalidOperationException(
188+
CoreStrings.ValueComplexType(
189+
typeBase.DisplayName(), complexProperty.Name, complexProperty.ComplexType.ClrType.ShortDisplayName()));
190+
}
191+
179192
if (complexProperty.ComplexType.GetMembers().Count() == 0)
180193
{
181194
throw new InvalidOperationException(

src/EFCore/Metadata/Builders/IConventionModelBuilder.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ public interface IConventionModelBuilder : IConventionAnnotatableBuilder
189189
/// <returns>
190190
/// An <see cref="IConventionModelBuilder" /> to continue configuration if the annotation was set, <see langword="null" /> otherwise.
191191
/// </returns>
192-
IConventionModelBuilder? Complex(
192+
IConventionModelBuilder? ComplexType(
193193
[DynamicallyAccessedMembers(IEntityType.DynamicallyAccessedMemberTypes)] Type type,
194194
bool fromDataAnnotation = false);
195195

src/EFCore/Metadata/Conventions/ComplexTypeAttributeConvention.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ protected override void ProcessEntityTypeAdded(
3333
ComplexTypeAttribute attribute,
3434
IConventionContext<IConventionEntityTypeBuilder> context)
3535
{
36-
entityTypeBuilder.Metadata.Model.Builder.Complex(entityTypeBuilder.Metadata.ClrType);
36+
entityTypeBuilder.Metadata.Model.Builder.ComplexType(entityTypeBuilder.Metadata.ClrType);
3737

3838
if (!entityTypeBuilder.Metadata.IsInModel)
3939
{

src/EFCore/Metadata/Internal/InternalModelBuilder.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -937,7 +937,7 @@ IConventionModel IConventionModelBuilder.Metadata
937937
/// doing so can result in application failures when updating to a new Entity Framework Core release.
938938
/// </summary>
939939
[DebuggerStepThrough]
940-
IConventionModelBuilder? IConventionModelBuilder.Complex(Type type, bool fromDataAnnotation)
940+
IConventionModelBuilder? IConventionModelBuilder.ComplexType(Type type, bool fromDataAnnotation)
941941
=> Complex(type, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);
942942

943943
/// <summary>

src/EFCore/Metadata/Internal/InternalTypeBaseBuilder.cs

-2
Original file line numberDiff line numberDiff line change
@@ -991,8 +991,6 @@ protected virtual void RemovePropertyIfUnused(Property property, ConfigurationSo
991991
RemoveMembersInHierarchy(propertyName, configurationSource.Value);
992992
}
993993

994-
model.Builder.Complex(complexType!, configurationSource.Value);
995-
996994
complexProperty = typeBase.AddComplexProperty(
997995
propertyName, propertyType, memberInfo, complexTypeName, complexType!, collection.Value, configurationSource.Value)!;
998996

src/EFCore/Properties/CoreStrings.Designer.cs

+16
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/EFCore/Properties/CoreStrings.resx

+6
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,9 @@
288288
<data name="ComplexPropertyIndexer" xml:space="preserve">
289289
<value>Adding the complex property '{type}.{property}' as an indexer property isn't supported.</value>
290290
</data>
291+
<data name="ComplexPropertyOptional" xml:space="preserve">
292+
<value>Configuring the complex property '{type}.{property}' as optional is not supported, call 'IsRequired()'.</value>
293+
</data>
291294
<data name="ComplexPropertyShadow" xml:space="preserve">
292295
<value>Configuring the complex property '{type}.{property}' in shadow state isn't supported.</value>
293296
</data>
@@ -1560,6 +1563,9 @@
15601563
<data name="ValueCannotBeNull" xml:space="preserve">
15611564
<value>The value for property '{1_entityType}.{0_property}' cannot be set to null because its type is '{propertyType}' which is not a nullable type.</value>
15621565
</data>
1566+
<data name="ValueComplexType" xml:space="preserve">
1567+
<value>Adding the complex property '{type}.{property}' isn't supported because it's of a value type '{propertyType}'.</value>
1568+
</data>
15631569
<data name="VisitIsNotAllowed" xml:space="preserve">
15641570
<value>Calling '{visitMethodName}' is not allowed. Visit the expression manually for the relevant part in the visitor.</value>
15651571
</data>

test/EFCore.Cosmos.FunctionalTests/ConfigPatternsCosmosTest.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ public async Task Should_throw_if_specified_region_is_wrong()
8888
exception.Message);
8989
}
9090

91-
[ConditionalFact(Skip = "Issue Azure/azure-cosmos-dotnet-v3#3990")]
91+
[ConditionalFact(Skip = "Issue #runtime/issues/89118")]
9292
public async Task Should_not_throw_if_specified_connection_mode_is_right()
9393
{
9494
var connectionMode = ConnectionMode.Direct;

test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs

+2-4
Original file line numberDiff line numberDiff line change
@@ -5322,8 +5322,7 @@ public static RuntimeComplexProperty Create(RuntimeComplexType declaringType)
53225322
"Microsoft.EntityFrameworkCore.Scaffolding.Internal.CSharpRuntimeModelCodeGeneratorTest+PrincipalBase.Owned#OwnedType.Principal#PrincipalBase",
53235323
typeof(CSharpRuntimeModelCodeGeneratorTest.PrincipalBase),
53245324
propertyInfo: typeof(CSharpRuntimeModelCodeGeneratorTest.OwnedType).GetProperty("Principal", BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly),
5325-
fieldInfo: typeof(CSharpRuntimeModelCodeGeneratorTest.OwnedType).GetField("<Principal>k__BackingField", BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly),
5326-
nullable: true);
5325+
fieldInfo: typeof(CSharpRuntimeModelCodeGeneratorTest.OwnedType).GetField("<Principal>k__BackingField", BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly));
53275326

53285327
var complexType = complexProperty.ComplexType;
53295328
var alternateId = complexType.AddProperty(
@@ -5882,8 +5881,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
58825881
.IsRowVersion()
58835882
.HasAnnotation("foo", "bar");
58845883
eb.Ignore(e => e.Context);
5885-
eb.ComplexProperty(o => o.Principal)
5886-
;
5884+
eb.ComplexProperty(o => o.Principal).IsRequired();
58875885
});
58885886

58895887
eb.ToTable("PrincipalBase");

test/EFCore.Tests/ApiConsistencyTest.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ protected override void Initialize()
158158
typeof(IConventionAnnotatable).GetMethod(nameof(IConventionAnnotatable.SetAnnotation)),
159159
typeof(IConventionAnnotatable).GetMethod(nameof(IConventionAnnotatable.SetOrRemoveAnnotation)),
160160
typeof(IConventionModelBuilder).GetMethod(nameof(IConventionModelBuilder.HasNoEntityType)),
161-
typeof(IConventionModelBuilder).GetMethod(nameof(IConventionModelBuilder.Complex)),
161+
typeof(IConventionModelBuilder).GetMethod(nameof(IConventionModelBuilder.ComplexType)),
162162
typeof(IReadOnlyEntityType).GetMethod(nameof(IReadOnlyEntityType.GetConcreteDerivedTypesInclusive)),
163163
typeof(IMutableEntityType).GetMethod(nameof(IMutableEntityType.AddData)),
164164
typeof(IReadOnlyNavigationBase).GetMethod("get_DeclaringEntityType"),

test/EFCore.Tests/ModelBuilding/ComplexTypeTestBase.cs

+59-38
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public virtual void Can_set_complex_property_annotation()
3232
Assert.Equal("bar2", complexProperty["foo2"]);
3333
Assert.Equal(typeof(Customer).Name, complexProperty.Name);
3434
Assert.Equal("""
35-
Customer (Customer)
35+
Customer (Customer) Required
3636
ComplexType: ComplexProperties.Customer#Customer
3737
Properties:
3838
AlternateKey (Guid) Required
@@ -1507,7 +1507,7 @@ public virtual void Can_ignore_a_field()
15071507
}
15081508

15091509
[ConditionalFact]
1510-
public virtual void Complex_properties_discovered_by_convention()
1510+
public virtual void Complex_properties_not_discovered_by_convention()
15111511
{
15121512
var modelBuilder = CreateModelBuilder();
15131513

@@ -1518,10 +1518,10 @@ public virtual void Complex_properties_discovered_by_convention()
15181518
.Entity<ComplexProperties>()
15191519
.ComplexProperty(e => e.Customer);
15201520

1521-
modelBuilder
1522-
.Entity<ValueComplexProperties>()
1523-
.Ignore(e => e.Tuple)
1524-
.ComplexProperty(e => e.Label);
1521+
//modelBuilder
1522+
// .Entity<ValueComplexProperties>()
1523+
// .Ignore(e => e.Tuple)
1524+
// .ComplexProperty(e => e.Label);
15251525

15261526
var model = modelBuilder.FinalizeModel();
15271527

@@ -1530,26 +1530,26 @@ public virtual void Complex_properties_discovered_by_convention()
15301530
var indexedType = model.FindEntityType(typeof(ComplexProperties))
15311531
.FindComplexProperty(nameof(ComplexProperties.IndexedClass)).ComplexType;
15321532

1533-
var valueType = model.FindEntityType(typeof(ValueComplexProperties));
1534-
var labelProperty = valueType.FindComplexProperty(nameof(ValueComplexProperties.Label));
1535-
Assert.False(labelProperty.IsNullable);
1536-
Assert.Equal(typeof(ProductLabel), labelProperty.ClrType);
1537-
var labelType = labelProperty.ComplexType;
1538-
Assert.Equal(typeof(ProductLabel), labelType.ClrType);
1539-
1540-
var labelCustomerProperty = labelType.FindComplexProperty(nameof(ProductLabel.Customer));
1541-
Assert.True(labelCustomerProperty.IsNullable);
1542-
Assert.Equal(typeof(Customer), labelCustomerProperty.ClrType);
1543-
1544-
var oldLabelProperty = valueType.FindComplexProperty(nameof(ValueComplexProperties.OldLabel));
1545-
Assert.True(oldLabelProperty.IsNullable);
1546-
Assert.Equal(typeof(ProductLabel?), oldLabelProperty.ClrType);
1547-
var oldLabelType = oldLabelProperty.ComplexType;
1548-
Assert.Equal(typeof(ProductLabel), oldLabelType.ClrType);
1549-
1550-
var oldLabelCustomerProperty = labelType.FindComplexProperty(nameof(ProductLabel.Customer));
1551-
Assert.True(oldLabelCustomerProperty.IsNullable);
1552-
Assert.Equal(typeof(Customer), oldLabelCustomerProperty.ClrType);
1533+
//var valueType = model.FindEntityType(typeof(ValueComplexProperties));
1534+
//var labelProperty = valueType.FindComplexProperty(nameof(ValueComplexProperties.Label));
1535+
//Assert.False(labelProperty.IsNullable);
1536+
//Assert.Equal(typeof(ProductLabel), labelProperty.ClrType);
1537+
//var labelType = labelProperty.ComplexType;
1538+
//Assert.Equal(typeof(ProductLabel), labelType.ClrType);
1539+
1540+
//var labelCustomerProperty = labelType.FindComplexProperty(nameof(ProductLabel.Customer));
1541+
//Assert.True(labelCustomerProperty.IsNullable);
1542+
//Assert.Equal(typeof(Customer), labelCustomerProperty.ClrType);
1543+
1544+
//var oldLabelProperty = valueType.FindComplexProperty(nameof(ValueComplexProperties.OldLabel));
1545+
//Assert.True(oldLabelProperty.IsNullable);
1546+
//Assert.Equal(typeof(ProductLabel?), oldLabelProperty.ClrType);
1547+
//var oldLabelType = oldLabelProperty.ComplexType;
1548+
//Assert.Equal(typeof(ProductLabel), oldLabelType.ClrType);
1549+
1550+
//var oldLabelCustomerProperty = labelType.FindComplexProperty(nameof(ProductLabel.Customer));
1551+
//Assert.True(oldLabelCustomerProperty.IsNullable);
1552+
//Assert.Equal(typeof(Customer), oldLabelCustomerProperty.ClrType);
15531553
}
15541554

15551555
[ConditionalFact]
@@ -1571,7 +1571,22 @@ public virtual void Complex_properties_can_be_configured_by_type()
15711571
}
15721572

15731573
[ConditionalFact]
1574-
public virtual void Can_map_tuple()
1574+
public virtual void Throws_for_optional_complex_property()
1575+
{
1576+
var modelBuilder = CreateModelBuilder();
1577+
1578+
modelBuilder
1579+
.Entity<ComplexProperties>()
1580+
.ComplexProperty(e => e.Customer).IsRequired(false);
1581+
1582+
Assert.Equal(
1583+
CoreStrings.ComplexPropertyOptional(
1584+
nameof(ComplexProperties), nameof(ComplexProperties.Customer)),
1585+
Assert.Throws<InvalidOperationException>(modelBuilder.FinalizeModel).Message);
1586+
}
1587+
1588+
[ConditionalFact]
1589+
public virtual void Throws_for_tuple()
15751590
{
15761591
var modelBuilder = CreateModelBuilder();
15771592

@@ -1581,17 +1596,23 @@ public virtual void Can_map_tuple()
15811596
.Ignore(e => e.OldLabel)
15821597
.ComplexProperty(e => e.Tuple);
15831598

1584-
var model = modelBuilder.FinalizeModel();
1585-
1586-
var valueType = model.FindEntityType(typeof(ValueComplexProperties));
1587-
var tupleProperty = valueType.FindComplexProperty(nameof(ValueComplexProperties.Tuple));
1588-
Assert.False(tupleProperty.IsNullable);
1589-
Assert.Equal(typeof((string, int)), tupleProperty.ClrType);
1590-
var tupleType = tupleProperty.ComplexType;
1591-
Assert.Equal(typeof((string, int)), tupleType.ClrType);
1592-
Assert.Equal("ValueComplexProperties.Tuple#ValueTuple<string, int>", tupleType.DisplayName());
1593-
1594-
Assert.Equal(2, tupleType.GetProperties().Count());
1599+
Assert.Equal(
1600+
CoreStrings.ValueComplexType(
1601+
nameof(ValueComplexProperties), nameof(ValueComplexProperties.Tuple), typeof((string, int)).ShortDisplayName()),
1602+
Assert.Throws<InvalidOperationException>(modelBuilder.FinalizeModel).Message);
1603+
1604+
// Uncomment when value types are supported.
1605+
//var model = modelBuilder.FinalizeModel();
1606+
1607+
//var valueType = model.FindEntityType(typeof(ValueComplexProperties));
1608+
//var tupleProperty = valueType.FindComplexProperty(nameof(ValueComplexProperties.Tuple));
1609+
//Assert.False(tupleProperty.IsNullable);
1610+
//Assert.Equal(typeof((string, int)), tupleProperty.ClrType);
1611+
//var tupleType = tupleProperty.ComplexType;
1612+
//Assert.Equal(typeof((string, int)), tupleType.ClrType);
1613+
//Assert.Equal("ValueComplexProperties.Tuple#ValueTuple<string, int>", tupleType.DisplayName());
1614+
1615+
//Assert.Equal(2, tupleType.GetProperties().Count());
15951616
}
15961617

15971618
[ConditionalFact]

test/EFCore.Tests/ModelBuilding/TestModel.cs

+7-7
Original file line numberDiff line numberDiff line change
@@ -855,19 +855,19 @@ protected interface IReplaceable
855855
protected class ComplexProperties
856856
{
857857
public int Id { get; set; }
858-
public Customer? Customer { get; set; }
859-
public DoubleProperty? DoubleProperty { get; set; }
860-
public IndexedClass? IndexedClass { get; set; }
861-
public Quarks? Quarks { get; set; }
858+
public Customer Customer { get; set; } = null!;
859+
public DoubleProperty DoubleProperty { get; set; } = null!;
860+
public IndexedClass IndexedClass { get; set; } = null!;
861+
public Quarks Quarks { get; set; } = null!;
862862

863863
[NotMapped]
864-
public DynamicProperty? DynamicProperty { get; set; }
864+
public DynamicProperty DynamicProperty { get; set; } = null!;
865865

866866
[NotMapped]
867-
public EntityWithFields? EntityWithFields { get; set; }
867+
public EntityWithFields EntityWithFields { get; set; } = null!;
868868

869869
[NotMapped]
870-
public WrappedStringEntity? WrappedStringEntity { get; set; }
870+
public WrappedStringEntity WrappedStringEntity { get; set; } = null!;
871871
}
872872

873873
protected class ValueComplexProperties

0 commit comments

Comments
 (0)