Skip to content

Commit

Permalink
Only discover complex types when configured through pre-convention mo…
Browse files Browse the repository at this point in the history
…del configuration or annotation

Throw for complex properties of value types.
Throw for optional complex properties.

Part of #13947
Fixes #31344
  • Loading branch information
AndriySvyryd committed Aug 7, 2023
1 parent f71c5ad commit 7532000
Show file tree
Hide file tree
Showing 15 changed files with 247 additions and 116 deletions.
25 changes: 15 additions & 10 deletions src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs
Original file line number Diff line number Diff line change
Expand Up @@ -211,22 +211,22 @@ protected virtual bool TriggersUniqueAcrossTables
}

private static void TryUniquifyColumnNames(
IConventionEntityType entityType,
Dictionary<string, IConventionProperty> properties,
IConventionTypeBase type,
Dictionary<string, IConventionProperty> columns,
in StoreObjectIdentifier storeObject,
int maxLength)
{
foreach (var property in entityType.GetDeclaredProperties())
foreach (var property in type.GetDeclaredProperties())
{
var columnName = property.GetColumnName(storeObject);
if (columnName == null)
{
continue;
}

if (!properties.TryGetValue(columnName, out var otherProperty))
if (!columns.TryGetValue(columnName, out var otherProperty))
{
properties[columnName] = property;
columns[columnName] = property;
continue;
}

Expand Down Expand Up @@ -256,10 +256,10 @@ protected virtual bool TriggersUniqueAcrossTables
&& !otherProperty.DeclaringType.IsStrictlyDerivedFrom(property.DeclaringType))
|| (property.DeclaringType as IConventionEntityType)?.FindRowInternalForeignKeys(storeObject).Any() == true)
{
var newColumnName = TryUniquify(property, columnName, properties, storeObject, usePrefix, maxLength);
var newColumnName = TryUniquify(property, columnName, columns, storeObject, usePrefix, maxLength);
if (newColumnName != null)
{
properties[newColumnName] = property;
columns[newColumnName] = property;
continue;
}
}
Expand All @@ -269,14 +269,19 @@ protected virtual bool TriggersUniqueAcrossTables
&& !otherProperty.DeclaringType.IsStrictlyDerivedFrom(property.DeclaringType))
|| (otherProperty.DeclaringType as IConventionEntityType)?.FindRowInternalForeignKeys(storeObject).Any() == true)
{
var newOtherColumnName = TryUniquify(otherProperty, columnName, properties, storeObject, usePrefix, maxLength);
var newOtherColumnName = TryUniquify(otherProperty, columnName, columns, storeObject, usePrefix, maxLength);
if (newOtherColumnName != null)
{
properties[columnName] = property;
properties[newOtherColumnName] = otherProperty;
columns[columnName] = property;
columns[newOtherColumnName] = otherProperty;
}
}
}

foreach (var complexProperty in type.GetDeclaredComplexProperties())
{
TryUniquifyColumnNames(complexProperty.ComplexType, columns, storeObject, maxLength);
}
}

private static string? TryUniquify(
Expand Down
35 changes: 35 additions & 0 deletions src/EFCore/Infrastructure/ModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,19 @@ void Validate(IConventionTypeBase typeBase)
CoreStrings.ComplexPropertyCollection(typeBase.DisplayName(), complexProperty.Name));
}

if (complexProperty.IsNullable)
{
throw new InvalidOperationException(
CoreStrings.ComplexPropertyOptional(typeBase.DisplayName(), complexProperty.Name));
}

if (complexProperty.ComplexType.ClrType.IsValueType)
{
throw new InvalidOperationException(
CoreStrings.ValueComplexType(
typeBase.DisplayName(), complexProperty.Name, complexProperty.ComplexType.ClrType.ShortDisplayName()));
}

if (complexProperty.ComplexType.GetMembers().Count() == 0)
{
throw new InvalidOperationException(
Expand Down Expand Up @@ -1151,6 +1164,28 @@ protected virtual void ValidateData(IModel model, IDiagnosticsLogger<DbLoggerCat
}
}

foreach (var complexProperty in entityType.GetComplexProperties())
{
if (seedDatum.TryGetValue(complexProperty.Name, out var value)
&& ((complexProperty.IsCollection && value is IEnumerable collection && collection.Any())
|| (!complexProperty.IsCollection && value != complexProperty.Sentinel)))
{
if (sensitiveDataLogged)
{
throw new InvalidOperationException(
CoreStrings.SeedDatumComplexPropertySensitive(
entityType.DisplayName(),
string.Join(", ", key.Properties.Select((p, i) => p.Name + ":" + keyValues[i])),
complexProperty.Name));
}

throw new InvalidOperationException(
CoreStrings.SeedDatumComplexProperty(
entityType.DisplayName(),
complexProperty.Name));
}
}

if (identityMap == null)
{
if (!identityMaps.TryGetValue(key, out identityMap))
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/Builders/IConventionModelBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ public interface IConventionModelBuilder : IConventionAnnotatableBuilder
/// <returns>
/// An <see cref="IConventionModelBuilder" /> to continue configuration if the annotation was set, <see langword="null" /> otherwise.
/// </returns>
IConventionModelBuilder? Complex(
IConventionModelBuilder? ComplexType(
[DynamicallyAccessedMembers(IEntityType.DynamicallyAccessedMemberTypes)] Type type,
bool fromDataAnnotation = false);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public ComplexTypeAttributeConvention(ProviderConventionSetBuilderDependencies d
ComplexTypeAttribute attribute,
IConventionContext<IConventionEntityTypeBuilder> context)
{
entityTypeBuilder.Metadata.Model.Builder.Complex(entityTypeBuilder.Metadata.ClrType);
entityTypeBuilder.Metadata.Model.Builder.ComplexType(entityTypeBuilder.Metadata.ClrType);

if (!entityTypeBuilder.Metadata.IsInModel)
{
Expand Down
1 change: 1 addition & 0 deletions src/EFCore/Metadata/Internal/EntityType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2701,6 +2701,7 @@ public virtual IEnumerable<Trigger> GetDeclaredTriggers()
propertiesList ??= GetProperties()
.Concat<IPropertyBase>(GetNavigations())
.Concat(GetSkipNavigations())
.Concat(GetComplexProperties())
.ToList();
if (ClrType.IsAssignableFrom(type))
{
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/Internal/InternalModelBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -937,7 +937,7 @@ IConventionModel IConventionModelBuilder.Metadata
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
[DebuggerStepThrough]
IConventionModelBuilder? IConventionModelBuilder.Complex(Type type, bool fromDataAnnotation)
IConventionModelBuilder? IConventionModelBuilder.ComplexType(Type type, bool fromDataAnnotation)
=> Complex(type, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);

/// <summary>
Expand Down
2 changes: 0 additions & 2 deletions src/EFCore/Metadata/Internal/InternalTypeBaseBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -991,8 +991,6 @@ protected virtual void RemovePropertyIfUnused(Property property, ConfigurationSo
RemoveMembersInHierarchy(propertyName, configurationSource.Value);
}

model.Builder.Complex(complexType!, configurationSource.Value);

complexProperty = typeBase.AddComplexProperty(
propertyName, propertyType, memberInfo, complexTypeName, complexType!, collection.Value, configurationSource.Value)!;

Expand Down
38 changes: 35 additions & 3 deletions src/EFCore/Properties/CoreStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 15 additions & 3 deletions src/EFCore/Properties/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -283,13 +283,16 @@
<value>The collection complex property '{property}' cannot be added to the type '{type}' because its CLR type '{clrType}' does not implement 'IEnumerable&lt;{targetType}&gt;'. Collection complex property must implement IEnumerable&lt;&gt; of the complex type.</value>
</data>
<data name="ComplexPropertyCollection" xml:space="preserve">
<value>Adding the collection complex property '{type}.{property}' isn't supported.</value>
<value>Adding the collection complex property '{type}.{property}' isn't supported. See https://github.com/dotnet/efcore/issues/31237 for more information.</value>
</data>
<data name="ComplexPropertyIndexer" xml:space="preserve">
<value>Adding the complex property '{type}.{property}' as an indexer property isn't supported.</value>
<value>Adding the complex property '{type}.{property}' as an indexer property isn't supported. See https://github.com/dotnet/efcore/issues/31244 for more information.</value>
</data>
<data name="ComplexPropertyOptional" xml:space="preserve">
<value>Configuring the complex property '{type}.{property}' as optional is not supported, call 'IsRequired()'. See https://github.com/dotnet/efcore/issues/31376 for more information.</value>
</data>
<data name="ComplexPropertyShadow" xml:space="preserve">
<value>Configuring the complex property '{type}.{property}' in shadow state isn't supported.</value>
<value>Configuring the complex property '{type}.{property}' in shadow state isn't supported. See https://github.com/dotnet/efcore/issues/31243 for more information.</value>
</data>
<data name="ComplexPropertyWrongClrType" xml:space="preserve">
<value>The complex property '{property}' cannot be added to the type '{type}' because its CLR type '{clrType}' does not match the expected CLR type '{targetType}'.</value>
Expand Down Expand Up @@ -1395,6 +1398,12 @@
<data name="SavepointsNotSupported" xml:space="preserve">
<value>Savepoints are not supported by the database provider in use.</value>
</data>
<data name="SeedDatumComplexProperty" xml:space="preserve">
<value>The seed entity for entity type '{entityType}' cannot be added because it has the complex property '{property}' set. Complex properties are currently not supported in seeding. See https://github.com/dotnet/efcore/issues/31254 for more information. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the involved property values.</value>
</data>
<data name="SeedDatumComplexPropertySensitive" xml:space="preserve">
<value>The seed entity for entity type '{entityType}' with the key value '{keyValue}' cannot be added because it has the complex property '{property}' set. Complex properties are currently not supported in seeding. See https://github.com/dotnet/efcore/issues/31254 for more information.</value>
</data>
<data name="SeedDatumDefaultValue" xml:space="preserve">
<value>The seed entity for entity type '{entityType}' cannot be added because a default value was provided for the required property '{property}'. Please provide a value different from '{defaultValue}'.</value>
</data>
Expand Down Expand Up @@ -1560,6 +1569,9 @@
<data name="ValueCannotBeNull" xml:space="preserve">
<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>
</data>
<data name="ValueComplexType" xml:space="preserve">
<value>Adding the complex property '{type}.{property}' isn't supported because it's of a value type '{propertyType}'. See https://github.com/dotnet/efcore/issues/9906 for more information.</value>
</data>
<data name="VisitIsNotAllowed" xml:space="preserve">
<value>Calling '{visitMethodName}' is not allowed. Visit the expression manually for the relevant part in the visitor.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public async Task Should_throw_if_specified_region_is_wrong()
exception.Message);
}

[ConditionalFact(Skip = "Issue Azure/azure-cosmos-dotnet-v3#3990")]
[ConditionalFact(Skip = "Issue #runtime/issues/89118")]
public async Task Should_not_throw_if_specified_connection_mode_is_right()
{
var connectionMode = ConnectionMode.Direct;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5322,8 +5322,7 @@ public static RuntimeComplexProperty Create(RuntimeComplexType declaringType)
"Microsoft.EntityFrameworkCore.Scaffolding.Internal.CSharpRuntimeModelCodeGeneratorTest+PrincipalBase.Owned#OwnedType.Principal#PrincipalBase",
typeof(CSharpRuntimeModelCodeGeneratorTest.PrincipalBase),
propertyInfo: typeof(CSharpRuntimeModelCodeGeneratorTest.OwnedType).GetProperty("Principal", BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly),
fieldInfo: typeof(CSharpRuntimeModelCodeGeneratorTest.OwnedType).GetField("<Principal>k__BackingField", BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly),
nullable: true);
fieldInfo: typeof(CSharpRuntimeModelCodeGeneratorTest.OwnedType).GetField("<Principal>k__BackingField", BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly));

var complexType = complexProperty.ComplexType;
var alternateId = complexType.AddProperty(
Expand Down Expand Up @@ -5882,8 +5881,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
.IsRowVersion()
.HasAnnotation("foo", "bar");
eb.Ignore(e => e.Context);
eb.ComplexProperty(o => o.Principal)
;
eb.ComplexProperty(o => o.Principal).IsRequired();
});

eb.ToTable("PrincipalBase");
Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.Tests/ApiConsistencyTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ protected override void Initialize()
typeof(IConventionAnnotatable).GetMethod(nameof(IConventionAnnotatable.SetAnnotation)),
typeof(IConventionAnnotatable).GetMethod(nameof(IConventionAnnotatable.SetOrRemoveAnnotation)),
typeof(IConventionModelBuilder).GetMethod(nameof(IConventionModelBuilder.HasNoEntityType)),
typeof(IConventionModelBuilder).GetMethod(nameof(IConventionModelBuilder.Complex)),
typeof(IConventionModelBuilder).GetMethod(nameof(IConventionModelBuilder.ComplexType)),
typeof(IReadOnlyEntityType).GetMethod(nameof(IReadOnlyEntityType.GetConcreteDerivedTypesInclusive)),
typeof(IMutableEntityType).GetMethod(nameof(IMutableEntityType.AddData)),
typeof(IReadOnlyNavigationBase).GetMethod("get_DeclaringEntityType"),
Expand Down
Loading

0 comments on commit 7532000

Please sign in to comment.