Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove old principal key after a value converter is applied #29073

Merged
merged 3 commits into from
Sep 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ private void Create(
property.DeclaringEntityType.ShortName(), property.Name, nameof(PropertyBuilder.HasConversion)));
}

var valueConverterType = (Type?)property[CoreAnnotationNames.ValueConverterType];
var valueConverterType = GetValueConverterType(property);
if (valueConverterType == null
&& property.GetValueConverter() != null)
{
Expand Down Expand Up @@ -847,6 +847,45 @@ private void Create(
mainBuilder.AppendLine();
}

private static Type? GetValueConverterType(IProperty property)
{
var type = (Type?)property[CoreAnnotationNames.ValueConverterType];
if (type != null)
{
return type;
}

var principalProperty = property;
for (var i = 0; i < 10000; i++)
{
foreach (var foreignKey in principalProperty.GetContainingForeignKeys())
{
for (var propertyIndex = 0; propertyIndex < foreignKey.Properties.Count; propertyIndex++)
{
if (principalProperty == foreignKey.Properties[propertyIndex])
{
var newPrincipalProperty = foreignKey.PrincipalKey.Properties[propertyIndex];
if (property == principalProperty
|| newPrincipalProperty == principalProperty)
{
break;
}

principalProperty = newPrincipalProperty;

type = (Type?)principalProperty[CoreAnnotationNames.ValueConverterType];
if (type != null)
{
return type;
}
}
}
}
}

return null;
}

private void PropertyBaseParameters(
IPropertyBase property,
CSharpRuntimeAnnotationCodeGeneratorParameters parameters,
Expand Down
3 changes: 2 additions & 1 deletion src/EFCore.Relational/Metadata/Internal/CheckConstraint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,8 @@ public virtual InternalCheckConstraintBuilder Builder
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual bool IsInModel
=> _builder is not null;
=> _builder is not null
&& ((IConventionAnnotatable)EntityType).IsInModel;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ public virtual InternalEntityTypeMappingFragmentBuilder Builder
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual bool IsInModel
=> _builder is not null;
=> _builder is not null
&& ((IConventionAnnotatable)EntityType).IsInModel;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ public virtual InternalRelationalPropertyOverridesBuilder Builder
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual bool IsInModel
=> _builder is not null;
=> _builder is not null
&& ((IConventionAnnotatable)Property).IsInModel;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ public virtual InternalStoredProcedureParameterBuilder Builder
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual bool IsInModel
=> _builder is not null;
=> _builder is not null
&& StoredProcedure.IsInModel;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ public virtual InternalStoredProcedureResultColumnBuilder Builder
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual bool IsInModel
=> _builder is not null;
=> _builder is not null
&& StoredProcedure.IsInModel;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
1 change: 1 addition & 0 deletions src/EFCore/Infrastructure/ModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,7 @@ protected virtual void ValidateTypeMappings(
var type = converter.ModelClrType;
if (type != typeof(string)
&& !(type == typeof(byte[]) && property.IsKey()) // Already special-cased elsewhere
&& !property.IsForeignKey()
&& type.TryGetSequenceType() != null)
{
logger.CollectionWithoutComparer(property);
Expand Down
2 changes: 0 additions & 2 deletions src/EFCore/Metadata/Conventions/KeyDiscoveryConvention.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.EntityFrameworkCore.Metadata.Builders;

namespace Microsoft.EntityFrameworkCore.Metadata.Conventions;

/// <summary>
Expand Down
19 changes: 18 additions & 1 deletion src/EFCore/Metadata/Conventions/ModelCleanupConvention.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions;
/// <remarks>
/// See <see href="https://aka.ms/efcore-docs-conventions">Model building conventions</see> for more information and examples.
/// </remarks>
public class ModelCleanupConvention : IModelFinalizingConvention
public class ModelCleanupConvention :
IForeignKeyRemovedConvention,
IModelFinalizingConvention
{
/// <summary>
/// Creates a new instance of <see cref="ModelCleanupConvention" />.
Expand All @@ -25,6 +27,21 @@ public ModelCleanupConvention(ProviderConventionSetBuilderDependencies dependenc
/// </summary>
protected virtual ProviderConventionSetBuilderDependencies Dependencies { get; }

/// <inheritdoc />
public virtual void ProcessForeignKeyRemoved(
IConventionEntityTypeBuilder entityTypeBuilder,
IConventionForeignKey foreignKey,
IConventionContext<IConventionForeignKey> context)
{
var principalKey = foreignKey.PrincipalKey;
if (principalKey.IsInModel
&& !principalKey.IsPrimaryKey()
&& !principalKey.GetReferencingForeignKeys().Any())
{
principalKey.DeclaringEntityType.Builder.HasNoKey(principalKey);
}
}

/// <inheritdoc />
public virtual void ProcessModelFinalizing(
IConventionModelBuilder modelBuilder,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,10 @@ private void DiscoverUnidirectionalInverses(
{
foreach (var inverseCandidateEntityType in model.FindEntityTypes(inverseCandidateType).ToList())
{
DiscoverRelationships(inverseCandidateEntityType.Builder, context);
if (inverseCandidateEntityType.IsInModel)
{
DiscoverRelationships(inverseCandidateEntityType.Builder, context);
}
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/EFCore/Metadata/Internal/ForeignKey.cs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ public virtual InternalForeignKeyBuilder Builder
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual bool IsInModel
=> _builder is not null;
=> _builder is not null
&& DeclaringEntityType.IsInModel;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
3 changes: 2 additions & 1 deletion src/EFCore/Metadata/Internal/Index.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ public virtual InternalIndexBuilder Builder
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual bool IsInModel
=> _builder is not null;
=> _builder is not null
&& DeclaringEntityType.IsInModel;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
129 changes: 66 additions & 63 deletions src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ public InternalEntityTypeBuilder(EntityType metadata, InternalModelBuilder model
var newKey = Metadata.SetPrimaryKey(keyBuilder.Metadata.Properties, configurationSource);
foreach (var key in Metadata.GetDeclaredKeys().ToList())
{
if (key == keyBuilder.Metadata)
if (key == keyBuilder.Metadata
|| !key.IsInModel)
{
continue;
}
Expand Down Expand Up @@ -3956,6 +3957,12 @@ private static bool Contains(IReadOnlyForeignKey? inheritedFk, IReadOnlyForeignK
}
else
{
if (targetEntityTypeBuilder != null
&& targetEntityTypeBuilder.Metadata.GetConfigurationSource().OverridesStrictly(configurationSource))
{
return targetEntityTypeBuilder;
}

targetEntityTypeBuilder = targetEntityType.IsNamed
? ModelBuilder.SharedTypeEntity(targetTypeName, targetType, configurationSource.Value, targetShouldBeOwned)
: ModelBuilder.Entity(targetType, configurationSource.Value, targetShouldBeOwned);
Expand Down Expand Up @@ -4490,81 +4497,77 @@ public virtual bool ShouldReuniquifyTemporaryProperties(ForeignKey foreignKey)
var clrProperties = Metadata.GetRuntimeProperties();
var clrFields = Metadata.GetRuntimeFields();
var canReuniquify = false;
using (var principalPropertyNamesEnumerator = principalPropertyNames.GetEnumerator())
{
using var principalPropertyTypesEnumerator = principalPropertyTypes.GetEnumerator();
for (var i = 0;
i < propertyCount
&& principalPropertyNamesEnumerator.MoveNext()
&& principalPropertyTypesEnumerator.MoveNext();
i++)
{
var keyPropertyName = principalPropertyNamesEnumerator.Current;
var keyPropertyType = principalPropertyTypesEnumerator.Current;
var keyModifiedBaseName = keyPropertyName.StartsWith(baseName, StringComparison.OrdinalIgnoreCase)
? keyPropertyName
: baseName + keyPropertyName;
string propertyName;
var clrType = keyPropertyType.MakeNullable(!isRequired);
var index = -1;
while (true)
using var principalPropertyNamesEnumerator = principalPropertyNames.GetEnumerator();
using var principalPropertyTypesEnumerator = principalPropertyTypes.GetEnumerator();
for (var i = 0;
i < propertyCount
&& principalPropertyNamesEnumerator.MoveNext()
&& principalPropertyTypesEnumerator.MoveNext();
i++)
{
var keyPropertyName = principalPropertyNamesEnumerator.Current;
var keyPropertyType = principalPropertyTypesEnumerator.Current;

var keyModifiedBaseName = keyPropertyName.StartsWith(baseName, StringComparison.OrdinalIgnoreCase)
? keyPropertyName
: baseName + keyPropertyName;
string propertyName;
var clrType = keyPropertyType.MakeNullable(!isRequired);
var index = -1;
while (true)
{
propertyName = keyModifiedBaseName + (++index > 0 ? index.ToString(CultureInfo.InvariantCulture) : "");
if (!Metadata.FindPropertiesInHierarchy(propertyName).Any()
&& !clrProperties.ContainsKey(propertyName)
&& !clrFields.ContainsKey(propertyName)
&& !IsIgnored(propertyName, ConfigurationSource.Convention))
{
propertyName = keyModifiedBaseName + (++index > 0 ? index.ToString(CultureInfo.InvariantCulture) : "");
if (!Metadata.FindPropertiesInHierarchy(propertyName).Any()
&& !clrProperties.ContainsKey(propertyName)
&& !clrFields.ContainsKey(propertyName)
&& !IsIgnored(propertyName, ConfigurationSource.Convention))
if (currentProperties == null)
{
if (currentProperties == null)
{
var propertyBuilder = Property(
clrType, propertyName, typeConfigurationSource: null,
configurationSource: ConfigurationSource.Convention);

if (propertyBuilder == null)
{
return (false, null);
}

if (index > 0)
{
propertyBuilder.HasAnnotation(
CoreAnnotationNames.PreUniquificationName,
keyModifiedBaseName,
ConfigurationSource.Convention);
}

if (clrType.IsNullableType())
{
propertyBuilder.IsRequired(isRequired, ConfigurationSource.Convention);
}
var propertyBuilder = Property(
clrType, propertyName, typeConfigurationSource: null,
configurationSource: ConfigurationSource.Convention);

newProperties![i] = propertyBuilder.Metadata;
if (propertyBuilder == null)
{
return (false, null);
}
else if (!Metadata.Model.Builder.CanBeConfigured(
clrType, TypeConfigurationType.Property, ConfigurationSource.Convention))

if (index > 0)
{
propertyBuilder.HasAnnotation(
CoreAnnotationNames.PreUniquificationName,
keyModifiedBaseName,
ConfigurationSource.Convention);
}
else

if (clrType.IsNullableType())
{
canReuniquify = true;
propertyBuilder.IsRequired(isRequired, ConfigurationSource.Convention);
}

break;
newProperties![i] = propertyBuilder.Metadata;
}

var currentProperty = currentProperties?.SingleOrDefault(p => p.Name == propertyName);
if (currentProperty != null)
else if (Metadata.Model.Builder.CanBeConfigured(
clrType, TypeConfigurationType.Property, ConfigurationSource.Convention))
{
if (((IConventionProperty)currentProperty).IsImplicitlyCreated()
&& currentProperty.ClrType != clrType
&& isRequired)
{
canReuniquify = true;
}
canReuniquify = true;
}

break;
}

break;
var currentProperty = currentProperties?.SingleOrDefault(p => p.Name == propertyName);
if (currentProperty != null)
{
if (((IConventionProperty)currentProperty).IsImplicitlyCreated()
&& currentProperty.ClrType != clrType
&& isRequired)
{
canReuniquify = true;
}

break;
}
}
}
Expand Down
24 changes: 12 additions & 12 deletions src/EFCore/Metadata/Internal/InternalModelBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,14 @@ public override InternalModelBuilder ModelBuilder
return null;
}

foreach (var existingEntityType in Metadata.FindEntityTypes(type))
{
if (!existingEntityType.Builder.CanSetIsOwned(true, configurationSource))
{
return null;
}
}

Metadata.RemoveIgnored(type);
Metadata.AddOwned(type, ConfigurationSource.Explicit);

Expand All @@ -337,15 +345,8 @@ public override InternalModelBuilder ModelBuilder
}
else
{
if (entityType.Builder.CanSetIsOwned(true, configurationSource))
{
// Discover the ownership when the type is added back
HasNoEntityType(entityType, configurationSource);
}
else
{
return null;
}
// Discover the ownership when the type is added back
HasNoEntityType(entityType, configurationSource);
}
}

Expand Down Expand Up @@ -405,9 +406,8 @@ public virtual bool CanBeConfigured(Type type, TypeConfigurationType configurati
}

if (!configurationType.IsEntityType()
&& (!configurationSource.Overrides(Metadata.FindEntityType(type)?.GetConfigurationSource())
|| !configurationSource.Overrides(Metadata.FindIsOwnedConfigurationSource(type))
|| Metadata.IsShared(type)))
&& (!configurationSource.Overrides(Metadata.FindIsOwnedConfigurationSource(type))
|| Metadata.FindEntityTypes(type).Any(e => !configurationSource.Overrides(e.GetConfigurationSource()))))
{
return false;
}
Expand Down
Loading