From 5e26c542974d307df661a74a21ec49891ff66ffc Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Wed, 14 Jul 2021 15:44:26 -0700 Subject: [PATCH] Default relationships to ownership for Cosmos Allow to reconfigure STETs as regular entity types Fixes #24803 --- .../Extensions/CosmosEntityTypeExtensions.cs | 2 +- .../CosmosServiceCollectionExtensions.cs | 1 - .../CosmosDiscriminatorConvention.cs | 80 ++-- ...osmosInversePropertyAttributeConvention.cs | 52 +++ .../CosmosRelationshipDiscoveryConvention.cs | 35 ++ .../Internal/CosmosConventionSetBuilder.cs | 49 ++- .../Conventions/StoreKeyConvention.cs | 1 - .../Internal/CosmosEntityTypeExtensions.cs | 2 +- src/EFCore.Relational/Metadata/IColumn.cs | 2 +- src/EFCore/Infrastructure/ModelValidator.cs | 19 +- .../Builders/OwnedNavigationBuilder.cs | 11 +- .../Builders/OwnedNavigationBuilder`.cs | 2 +- .../BaseTypeDiscoveryConvention.cs | 28 +- .../ProviderConventionSetBuilder.cs | 4 +- .../InversePropertyAttributeConvention.cs | 223 ++++++++--- .../NavigationAttributeConventionBase.cs | 2 - .../RelationshipDiscoveryConvention.cs | 373 +++++++++++++----- src/EFCore/Metadata/IConventionModel.cs | 27 +- src/EFCore/Metadata/IModel.cs | 2 +- src/EFCore/Metadata/IMutableModel.cs | 10 +- src/EFCore/Metadata/Internal/EntityType.cs | 9 + .../Internal/InternalEntityTypeBuilder.cs | 34 +- .../Internal/InternalForeignKeyBuilder.cs | 22 +- .../Metadata/Internal/InternalModelBuilder.cs | 48 ++- src/EFCore/Metadata/Internal/Model.cs | 48 ++- src/EFCore/Properties/CoreStrings.Designer.cs | 8 + src/EFCore/Properties/CoreStrings.resx | 3 + .../CosmosModelValidatorTest.cs | 3 + .../CosmosModelBuilderGenericTest.cs | 108 +++++ .../Internal/InternalModelBuilderTest.cs | 2 +- .../ModelBuilding/InheritanceTestBase.cs | 16 +- .../ModelBuilding/ManyToManyTestBase.cs | 6 +- .../ModelBuilding/ManyToOneTestBase.cs | 28 +- .../ModelBuilding/NonRelationshipTestBase.cs | 13 + .../ModelBuilding/OneToManyTestBase.cs | 33 +- .../ModelBuilding/OneToOneTestBase.cs | 5 + .../ModelBuilding/OwnedTypesTestBase.cs | 118 +++++- 37 files changed, 1146 insertions(+), 283 deletions(-) create mode 100644 src/EFCore.Cosmos/Metadata/Conventions/CosmosInversePropertyAttributeConvention.cs create mode 100644 src/EFCore.Cosmos/Metadata/Conventions/CosmosRelationshipDiscoveryConvention.cs diff --git a/src/EFCore.Cosmos/Extensions/CosmosEntityTypeExtensions.cs b/src/EFCore.Cosmos/Extensions/CosmosEntityTypeExtensions.cs index b06d074c5f1..4f0f86fad55 100644 --- a/src/EFCore.Cosmos/Extensions/CosmosEntityTypeExtensions.cs +++ b/src/EFCore.Cosmos/Extensions/CosmosEntityTypeExtensions.cs @@ -25,7 +25,7 @@ public static class CosmosEntityTypeExtensions ?? GetDefaultContainer(entityType); private static string? GetDefaultContainer(IReadOnlyEntityType entityType) - => entityType.IsOwned() + => entityType.FindOwnership() != null ? null : entityType.Model.GetDefaultContainer() ?? entityType.ShortName(); diff --git a/src/EFCore.Cosmos/Extensions/CosmosServiceCollectionExtensions.cs b/src/EFCore.Cosmos/Extensions/CosmosServiceCollectionExtensions.cs index d4528ba6aef..b9b866715ee 100644 --- a/src/EFCore.Cosmos/Extensions/CosmosServiceCollectionExtensions.cs +++ b/src/EFCore.Cosmos/Extensions/CosmosServiceCollectionExtensions.cs @@ -4,7 +4,6 @@ using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Cosmos.Diagnostics.Internal; using Microsoft.EntityFrameworkCore.Cosmos.Infrastructure.Internal; -using Microsoft.EntityFrameworkCore.Cosmos.Internal; using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Conventions.Internal; using Microsoft.EntityFrameworkCore.Cosmos.Query.Internal; using Microsoft.EntityFrameworkCore.Cosmos.Storage.Internal; diff --git a/src/EFCore.Cosmos/Metadata/Conventions/CosmosDiscriminatorConvention.cs b/src/EFCore.Cosmos/Metadata/Conventions/CosmosDiscriminatorConvention.cs index 471753c8058..037851d599e 100644 --- a/src/EFCore.Cosmos/Metadata/Conventions/CosmosDiscriminatorConvention.cs +++ b/src/EFCore.Cosmos/Metadata/Conventions/CosmosDiscriminatorConvention.cs @@ -1,12 +1,11 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System.Linq; using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal; using Microsoft.EntityFrameworkCore.Metadata.Builders; using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure; -using Microsoft.EntityFrameworkCore.Utilities; +// ReSharper disable once CheckNamespace namespace Microsoft.EntityFrameworkCore.Metadata.Conventions { /// @@ -16,7 +15,8 @@ public class CosmosDiscriminatorConvention : DiscriminatorConvention, IForeignKeyOwnershipChangedConvention, IForeignKeyRemovedConvention, - IEntityTypeAddedConvention + IEntityTypeAddedConvention, + IEntityTypeAnnotationChangedConvention { /// /// Creates a new instance of . @@ -36,16 +36,7 @@ public virtual void ProcessEntityTypeAdded( IConventionEntityTypeBuilder entityTypeBuilder, IConventionContext context) { - Check.NotNull(entityTypeBuilder, nameof(entityTypeBuilder)); - Check.NotNull(context, nameof(context)); - - var entityType = entityTypeBuilder.Metadata; - if (entityType.BaseType == null - && entityType.IsDocumentRoot()) - { - entityTypeBuilder.HasDiscriminator(typeof(string)) - ?.HasValue(entityType, entityType.ShortName()); - } + ProcessEntityType(entityTypeBuilder); } /// @@ -57,17 +48,9 @@ public virtual void ProcessForeignKeyOwnershipChanged( IConventionForeignKeyBuilder relationshipBuilder, IConventionContext context) { - Check.NotNull(relationshipBuilder, nameof(relationshipBuilder)); - Check.NotNull(context, nameof(context)); - var entityType = relationshipBuilder.Metadata.DeclaringEntityType; - if (relationshipBuilder.Metadata.IsOwnership - && !entityType.IsDocumentRoot() - && entityType.BaseType == null - && !entityType.GetDerivedTypes().Any()) - { - entityType.Builder.HasNoDiscriminator(); - } + + ProcessEntityType(entityType.Builder); } /// @@ -81,13 +64,52 @@ public virtual void ProcessForeignKeyRemoved( IConventionForeignKey foreignKey, IConventionContext context) { - var entityType = foreignKey.DeclaringEntityType; - if (foreignKey.IsOwnership - && !entityType.IsDocumentRoot() - && entityType.BaseType == null - && !entityType.GetDerivedTypes().Any()) + if (foreignKey.IsOwnership) { - entityType.Builder.HasNoDiscriminator(); + ProcessEntityType(entityTypeBuilder); + } + } + + /// + /// Called after an annotation is changed on an entity type. + /// + /// The builder for the entity type. + /// The annotation name. + /// The new annotation. + /// The old annotation. + /// Additional information associated with convention execution. + public void ProcessEntityTypeAnnotationChanged( + IConventionEntityTypeBuilder entityTypeBuilder, + string name, + IConventionAnnotation? annotation, + IConventionAnnotation? oldAnnotation, + IConventionContext context) + { + if (name != CosmosAnnotationNames.ContainerName + || (annotation == null) == (oldAnnotation == null)) + { + return; + } + + ProcessEntityType(entityTypeBuilder); + } + + private void ProcessEntityType(IConventionEntityTypeBuilder entityTypeBuilder) + { + var entityType = entityTypeBuilder.Metadata; + if (entityType.BaseType != null) + { + return; + } + + if (!entityType.IsDocumentRoot()) + { + entityTypeBuilder.HasNoDiscriminator(); + } + else + { + entityTypeBuilder.HasDiscriminator(typeof(string)) + ?.HasValue(entityType, entityType.ShortName()); } } diff --git a/src/EFCore.Cosmos/Metadata/Conventions/CosmosInversePropertyAttributeConvention.cs b/src/EFCore.Cosmos/Metadata/Conventions/CosmosInversePropertyAttributeConvention.cs new file mode 100644 index 00000000000..630e7c5f418 --- /dev/null +++ b/src/EFCore.Cosmos/Metadata/Conventions/CosmosInversePropertyAttributeConvention.cs @@ -0,0 +1,52 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.ComponentModel.DataAnnotations.Schema; +using System.Reflection; +using Microsoft.EntityFrameworkCore.Metadata.Builders; +using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata.Internal; + +// ReSharper disable once CheckNamespace +namespace Microsoft.EntityFrameworkCore.Metadata.Conventions +{ + /// + /// A convention that configures the inverse navigation property based on the + /// specified on the other navigation property. + /// All navigations are assumed to be targeting owned entity types for Cosmos. + /// + public class CosmosInversePropertyAttributeConvention : InversePropertyAttributeConvention + { + /// + /// Creates a new instance of . + /// + /// Parameter object containing dependencies for this convention. + public CosmosInversePropertyAttributeConvention(ProviderConventionSetBuilderDependencies dependencies) + : base(dependencies) + { + } + + /// + /// Finds or tries to create an entity type target for the given navigation member. + /// + /// The builder for the referencing entity type. + /// The CLR type of the target entity type. + /// The navigation member. + /// Whether an entity type should be created if one doesn't currently exist. + /// The builder for the target entity type or if it can't be created. + protected override IConventionEntityTypeBuilder? TryGetTargetEntityTypeBuilder( + IConventionEntityTypeBuilder entityTypeBuilder, + Type targetClrType, + MemberInfo navigationMemberInfo, + bool shouldCreate = true) + => ((InternalEntityTypeBuilder)entityTypeBuilder) +#pragma warning disable EF1001 // Internal EF Core API usage. + .GetTargetEntityTypeBuilder( + targetClrType, + navigationMemberInfo, + shouldCreate ? ConfigurationSource.DataAnnotation : null, + targetShouldBeOwned: true); +#pragma warning restore EF1001 // Internal EF Core API usage. + } +} diff --git a/src/EFCore.Cosmos/Metadata/Conventions/CosmosRelationshipDiscoveryConvention.cs b/src/EFCore.Cosmos/Metadata/Conventions/CosmosRelationshipDiscoveryConvention.cs new file mode 100644 index 00000000000..9d7932aa58d --- /dev/null +++ b/src/EFCore.Cosmos/Metadata/Conventions/CosmosRelationshipDiscoveryConvention.cs @@ -0,0 +1,35 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure; + +// ReSharper disable once CheckNamespace +namespace Microsoft.EntityFrameworkCore.Metadata.Conventions +{ + /// + /// A convention that configures relationships between entity types based on the navigation properties + /// as long as there is no ambiguity as to which is the corresponding inverse navigation. + /// All navigations are assumed to be targeting owned entity types for Cosmos. + /// + public class CosmosRelationshipDiscoveryConvention : RelationshipDiscoveryConvention + { + /// + /// Creates a new instance of . + /// + /// Parameter object containing dependencies for this convention. + public CosmosRelationshipDiscoveryConvention(ProviderConventionSetBuilderDependencies dependencies) + : base(dependencies) + { + } + + /// + /// Returns a value indicating whether the given entity type should be added as owned if it isn't currently in the model. + /// + /// Target entity type. + /// The model. + /// if the given entity type should be owned. + protected override bool? ShouldBeOwned(Type targetType, IConventionModel model) + => true; + } +} diff --git a/src/EFCore.Cosmos/Metadata/Conventions/Internal/CosmosConventionSetBuilder.cs b/src/EFCore.Cosmos/Metadata/Conventions/Internal/CosmosConventionSetBuilder.cs index c80c12f9096..f0df4f890cb 100644 --- a/src/EFCore.Cosmos/Metadata/Conventions/Internal/CosmosConventionSetBuilder.cs +++ b/src/EFCore.Cosmos/Metadata/Conventions/Internal/CosmosConventionSetBuilder.cs @@ -44,47 +44,72 @@ public override ConventionSet CreateConventionSet() var storeKeyConvention = new StoreKeyConvention(Dependencies); var discriminatorConvention = new CosmosDiscriminatorConvention(Dependencies); - var keyDiscoveryConvention = new CosmosKeyDiscoveryConvention(Dependencies); + KeyDiscoveryConvention keyDiscoveryConvention = new CosmosKeyDiscoveryConvention(Dependencies); + InversePropertyAttributeConvention inversePropertyAttributeConvention = + new CosmosInversePropertyAttributeConvention(Dependencies); + RelationshipDiscoveryConvention relationshipDiscoveryConvention = + new CosmosRelationshipDiscoveryConvention(Dependencies); conventionSet.EntityTypeAddedConventions.Add(storeKeyConvention); conventionSet.EntityTypeAddedConventions.Add(discriminatorConvention); - ReplaceConvention(conventionSet.EntityTypeAddedConventions, (KeyDiscoveryConvention)keyDiscoveryConvention); + ReplaceConvention(conventionSet.EntityTypeAddedConventions, keyDiscoveryConvention); + ReplaceConvention(conventionSet.EntityTypeAddedConventions, inversePropertyAttributeConvention); + ReplaceConvention(conventionSet.EntityTypeAddedConventions, relationshipDiscoveryConvention); + + ReplaceConvention(conventionSet.EntityTypeIgnoredConventions, relationshipDiscoveryConvention); ReplaceConvention(conventionSet.EntityTypeRemovedConventions, (DiscriminatorConvention)discriminatorConvention); + ReplaceConvention(conventionSet.EntityTypeRemovedConventions, inversePropertyAttributeConvention); conventionSet.EntityTypeBaseTypeChangedConventions.Add(storeKeyConvention); ReplaceConvention(conventionSet.EntityTypeBaseTypeChangedConventions, (DiscriminatorConvention)discriminatorConvention); - ReplaceConvention(conventionSet.EntityTypeBaseTypeChangedConventions, (KeyDiscoveryConvention)keyDiscoveryConvention); + ReplaceConvention(conventionSet.EntityTypeBaseTypeChangedConventions, keyDiscoveryConvention); + ReplaceConvention(conventionSet.EntityTypeBaseTypeChangedConventions, inversePropertyAttributeConvention); + ReplaceConvention(conventionSet.EntityTypeBaseTypeChangedConventions, relationshipDiscoveryConvention); - ReplaceConvention(conventionSet.EntityTypeMemberIgnoredConventions, (KeyDiscoveryConvention)keyDiscoveryConvention); + ReplaceConvention(conventionSet.EntityTypeMemberIgnoredConventions, keyDiscoveryConvention); + ReplaceConvention(conventionSet.EntityTypeMemberIgnoredConventions, inversePropertyAttributeConvention); + ReplaceConvention(conventionSet.EntityTypeMemberIgnoredConventions, relationshipDiscoveryConvention); conventionSet.EntityTypePrimaryKeyChangedConventions.Add(storeKeyConvention); conventionSet.KeyAddedConventions.Add(storeKeyConvention); conventionSet.KeyRemovedConventions.Add(storeKeyConvention); - ReplaceConvention(conventionSet.KeyRemovedConventions, (KeyDiscoveryConvention)keyDiscoveryConvention); + ReplaceConvention(conventionSet.KeyRemovedConventions, keyDiscoveryConvention); - ReplaceConvention(conventionSet.ForeignKeyAddedConventions, (KeyDiscoveryConvention)keyDiscoveryConvention); + ReplaceConvention(conventionSet.ForeignKeyAddedConventions, keyDiscoveryConvention); + ReplaceConvention(conventionSet.ForeignKeyRemovedConventions, relationshipDiscoveryConvention); conventionSet.ForeignKeyRemovedConventions.Add(discriminatorConvention); conventionSet.ForeignKeyRemovedConventions.Add(storeKeyConvention); - ReplaceConvention(conventionSet.ForeignKeyRemovedConventions, (KeyDiscoveryConvention)keyDiscoveryConvention); + ReplaceConvention(conventionSet.ForeignKeyRemovedConventions, keyDiscoveryConvention); - ReplaceConvention(conventionSet.ForeignKeyPropertiesChangedConventions, (KeyDiscoveryConvention)keyDiscoveryConvention); + ReplaceConvention(conventionSet.ForeignKeyPropertiesChangedConventions, keyDiscoveryConvention); - ReplaceConvention(conventionSet.ForeignKeyUniquenessChangedConventions, (KeyDiscoveryConvention)keyDiscoveryConvention); + ReplaceConvention(conventionSet.ForeignKeyUniquenessChangedConventions, keyDiscoveryConvention); conventionSet.ForeignKeyOwnershipChangedConventions.Add(discriminatorConvention); conventionSet.ForeignKeyOwnershipChangedConventions.Add(storeKeyConvention); - ReplaceConvention(conventionSet.ForeignKeyOwnershipChangedConventions, (KeyDiscoveryConvention)keyDiscoveryConvention); + ReplaceConvention(conventionSet.ForeignKeyOwnershipChangedConventions, keyDiscoveryConvention); + ReplaceConvention(conventionSet.ForeignKeyOwnershipChangedConventions, relationshipDiscoveryConvention); + + ReplaceConvention(conventionSet.ForeignKeyNullNavigationSetConventions, relationshipDiscoveryConvention); + + ReplaceConvention(conventionSet.NavigationAddedConventions, inversePropertyAttributeConvention); + ReplaceConvention(conventionSet.NavigationAddedConventions, relationshipDiscoveryConvention); + ReplaceConvention(conventionSet.NavigationRemovedConventions, relationshipDiscoveryConvention); + + conventionSet.EntityTypeAnnotationChangedConventions.Add(discriminatorConvention); conventionSet.EntityTypeAnnotationChangedConventions.Add(storeKeyConvention); - conventionSet.EntityTypeAnnotationChangedConventions.Add(keyDiscoveryConvention); + conventionSet.EntityTypeAnnotationChangedConventions.Add((CosmosKeyDiscoveryConvention)keyDiscoveryConvention); - ReplaceConvention(conventionSet.PropertyAddedConventions, (KeyDiscoveryConvention)keyDiscoveryConvention); + ReplaceConvention(conventionSet.PropertyAddedConventions, keyDiscoveryConvention); conventionSet.PropertyAnnotationChangedConventions.Add(storeKeyConvention); + ReplaceConvention(conventionSet.ModelFinalizingConventions, inversePropertyAttributeConvention); + return conventionSet; } diff --git a/src/EFCore.Cosmos/Metadata/Conventions/StoreKeyConvention.cs b/src/EFCore.Cosmos/Metadata/Conventions/StoreKeyConvention.cs index 63115c4bb0b..8120e1eebce 100644 --- a/src/EFCore.Cosmos/Metadata/Conventions/StoreKeyConvention.cs +++ b/src/EFCore.Cosmos/Metadata/Conventions/StoreKeyConvention.cs @@ -5,7 +5,6 @@ using System.Linq; using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal; using Microsoft.EntityFrameworkCore.Cosmos.ValueGeneration; -using Microsoft.EntityFrameworkCore.Cosmos.ValueGeneration.Internal; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Metadata.Builders; using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure; diff --git a/src/EFCore.Cosmos/Metadata/Internal/CosmosEntityTypeExtensions.cs b/src/EFCore.Cosmos/Metadata/Internal/CosmosEntityTypeExtensions.cs index a8af6bf82e5..154997c2b79 100644 --- a/src/EFCore.Cosmos/Metadata/Internal/CosmosEntityTypeExtensions.cs +++ b/src/EFCore.Cosmos/Metadata/Internal/CosmosEntityTypeExtensions.cs @@ -21,7 +21,7 @@ public static class CosmosEntityTypeExtensions /// public static bool IsDocumentRoot(this IReadOnlyEntityType entityType) => entityType.BaseType?.IsDocumentRoot() - ?? (!entityType.IsOwned() + ?? (entityType.FindOwnership() == null || entityType[CosmosAnnotationNames.ContainerName] != null); } } diff --git a/src/EFCore.Relational/Metadata/IColumn.cs b/src/EFCore.Relational/Metadata/IColumn.cs index ec6c3d29011..0c7dcbf6fa0 100644 --- a/src/EFCore.Relational/Metadata/IColumn.cs +++ b/src/EFCore.Relational/Metadata/IColumn.cs @@ -90,7 +90,7 @@ public virtual bool TryGetDefaultValue(out object? defaultValue) return false; } - var converter = property.GetValueConverter() ?? PropertyMappings.First().TypeMapping?.Converter; + var converter = property.GetValueConverter() ?? PropertyMappings.First().TypeMapping.Converter; if (converter != null) { diff --git a/src/EFCore/Infrastructure/ModelValidator.cs b/src/EFCore/Infrastructure/ModelValidator.cs index c6781c0c670..2f1ef8bb89c 100644 --- a/src/EFCore/Infrastructure/ModelValidator.cs +++ b/src/EFCore/Infrastructure/ModelValidator.cs @@ -221,7 +221,7 @@ protected virtual void ValidatePropertyMapping( if (targetType != null) { var targetShared = conventionModel.IsShared(targetType); - targetOwned ??= conventionModel.IsOwned(targetType); + targetOwned ??= IsOwned(targetType, conventionModel); // ReSharper disable CheckForReferenceEqualityInstead.1 // ReSharper disable CheckForReferenceEqualityInstead.3 if ((!entityType.IsKeyless @@ -230,10 +230,9 @@ protected virtual void ValidatePropertyMapping( dt => dt.GetDeclaredNavigations().FirstOrDefault(n => n.Name == clrProperty.GetSimpleMemberName()) == null) && (!(targetShared || targetOwned.Value) - || (!targetType.Equals(entityType.ClrType) - && (!entityType.IsInOwnershipPath(targetType) - || (entityType.FindOwnership()!.PrincipalEntityType.ClrType.Equals(targetType) - && targetSequenceType == null))))) + || !targetType.Equals(entityType.ClrType)) + && (!entityType.IsInOwnershipPath(targetType) + || targetSequenceType == null)) { if (entityType.IsOwned() && targetOwned.Value) @@ -274,6 +273,16 @@ protected virtual void ValidatePropertyMapping( } } + /// + /// Returns a value indicating whether that target CLR type would correspond to an owned entity type. + /// + /// The target CLR type. + /// The model. + /// if the given CLR type corresponds to an owned entity type. + protected virtual bool IsOwned(Type targetType, IConventionModel conventionModel) + => conventionModel.FindIsOwnedConfigurationSource(targetType) != null + || conventionModel.FindEntityTypes(targetType).Any(t => t.IsOwned()); + /// /// Validates that no attempt is made to ignore inherited properties. /// diff --git a/src/EFCore/Metadata/Builders/OwnedNavigationBuilder.cs b/src/EFCore/Metadata/Builders/OwnedNavigationBuilder.cs index f67ab7e4a3a..cef8c153dde 100644 --- a/src/EFCore/Metadata/Builders/OwnedNavigationBuilder.cs +++ b/src/EFCore/Metadata/Builders/OwnedNavigationBuilder.cs @@ -17,6 +17,7 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Builders public class OwnedNavigationBuilder : IInfrastructure { private InternalForeignKeyBuilder _builder; + private EntityType _dependentEntityType; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -28,7 +29,7 @@ public class OwnedNavigationBuilder : IInfrastructure [EntityFrameworkInternal] - protected virtual EntityType DependentEntityType { get; } + protected virtual EntityType DependentEntityType + => _dependentEntityType.IsInModel + ? _dependentEntityType + : _dependentEntityType = Builder.Metadata.DeclaringEntityType; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -55,7 +59,8 @@ protected virtual InternalForeignKeyBuilder Builder { get { - if (!_builder.Metadata.IsInModel) + if (!_builder.Metadata.IsInModel + && PrincipalEntityType.IsInModel) { _builder = PrincipalEntityType.FindNavigation(_builder.Metadata.PrincipalToDependent!.Name)?.ForeignKey.Builder!; } diff --git a/src/EFCore/Metadata/Builders/OwnedNavigationBuilder`.cs b/src/EFCore/Metadata/Builders/OwnedNavigationBuilder`.cs index 9923162272b..a52fb52152d 100644 --- a/src/EFCore/Metadata/Builders/OwnedNavigationBuilder`.cs +++ b/src/EFCore/Metadata/Builders/OwnedNavigationBuilder`.cs @@ -1125,7 +1125,7 @@ public virtual ReferenceNavigationBuilder H navigation, DependentEntityType.Builder.HasRelationship( relatedEntityType, navigation, ConfigurationSource.Explicit, - targetIsPrincipal: DependentEntityType == relatedEntityType ? true : (bool?)null)!.Metadata); + targetIsPrincipal: DependentEntityType == relatedEntityType ? true : null)!.Metadata); } /// diff --git a/src/EFCore/Metadata/Conventions/BaseTypeDiscoveryConvention.cs b/src/EFCore/Metadata/Conventions/BaseTypeDiscoveryConvention.cs index 2e1e0bd328a..8fcb26e00e0 100644 --- a/src/EFCore/Metadata/Conventions/BaseTypeDiscoveryConvention.cs +++ b/src/EFCore/Metadata/Conventions/BaseTypeDiscoveryConvention.cs @@ -19,7 +19,8 @@ public class BaseTypeDiscoveryConvention : #pragma warning disable CS0612 // Type or member is obsolete InheritanceDiscoveryConventionBase, #pragma warning restore CS0612 // Type or member is obsolete - IEntityTypeAddedConvention + IEntityTypeAddedConvention, + IForeignKeyRemovedConvention { /// /// Creates a new instance of . @@ -44,7 +45,14 @@ public virtual void ProcessEntityTypeAdded( Check.DebugAssert(entityType.GetDeclaredForeignKeys().FirstOrDefault(fk => fk.IsOwnership) == null, "Ownerships present on non-owned entity type"); + ProcessEntityType(entityTypeBuilder, context); + } + private static void ProcessEntityType( + IConventionEntityTypeBuilder entityTypeBuilder, + IConventionContext context) + { + var entityType = entityTypeBuilder.Metadata; var model = entityType.Model; var derivedTypesMap = (Dictionary>?)model[CoreAnnotationNames.DerivedTypes]; if (derivedTypesMap == null) @@ -106,10 +114,20 @@ public virtual void ProcessEntityTypeAdded( if (!baseEntityType.HasSharedClrType && !baseEntityType.IsOwned()) { - if (entityTypeBuilder.HasBaseType(baseEntityType) is IConventionEntityTypeBuilder newEntityTypeBuilder) - { - context.StopProcessingIfChanged(newEntityTypeBuilder); - } + entityTypeBuilder.HasBaseType(baseEntityType); + } + } + + /// + public virtual void ProcessForeignKeyRemoved( + IConventionEntityTypeBuilder entityTypeBuilder, + IConventionForeignKey foreignKey, + IConventionContext context) + { + if (foreignKey.IsOwnership + && !entityTypeBuilder.Metadata.IsOwned()) + { + ProcessEntityType(entityTypeBuilder, context); } } } diff --git a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs index ef648d3ebb6..db0b22793f0 100644 --- a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs +++ b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs @@ -149,6 +149,8 @@ public virtual ConventionSet CreateConventionSet() conventionSet.ForeignKeyAddedConventions.Add(cascadeDeleteConvention); conventionSet.ForeignKeyAddedConventions.Add(foreignKeyIndexConvention); + conventionSet.ForeignKeyRemovedConventions.Add(baseTypeDiscoveryConvention); + conventionSet.ForeignKeyRemovedConventions.Add(relationshipDiscoveryConvention); conventionSet.ForeignKeyRemovedConventions.Add(keyDiscoveryConvention); conventionSet.ForeignKeyRemovedConventions.Add(valueGeneratorConvention); conventionSet.ForeignKeyRemovedConventions.Add(foreignKeyIndexConvention); @@ -166,8 +168,8 @@ public virtual ConventionSet CreateConventionSet() conventionSet.ForeignKeyRequirednessChangedConventions.Add(foreignKeyPropertyDiscoveryConvention); conventionSet.ForeignKeyOwnershipChangedConventions.Add(new NavigationEagerLoadingConvention(Dependencies)); - conventionSet.ForeignKeyOwnershipChangedConventions.Add(keyDiscoveryConvention); conventionSet.ForeignKeyOwnershipChangedConventions.Add(relationshipDiscoveryConvention); + conventionSet.ForeignKeyOwnershipChangedConventions.Add(keyDiscoveryConvention); conventionSet.ForeignKeyOwnershipChangedConventions.Add(valueGeneratorConvention); conventionSet.ForeignKeyNullNavigationSetConventions.Add(relationshipDiscoveryConvention); diff --git a/src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs b/src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs index cd8e2680236..d692feea312 100644 --- a/src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs +++ b/src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs @@ -121,7 +121,8 @@ private void Process( targetEntityType, targetEntityType.BaseType, inverseNavigationPropertyInfo, - referencingNavigationsWithAttribute, out var conventionForeignKeyBuilder)) + referencingNavigationsWithAttribute, + out var conventionForeignKeyBuilder)) { return conventionForeignKeyBuilder; } @@ -139,13 +140,46 @@ private void Process( return null; } + var targetOwnership = targetEntityType.FindOwnership(); + if (targetOwnership != null + && targetOwnership.PrincipalEntityType == entityType + && targetOwnership.PrincipalToDependent?.GetIdentifyingMemberInfo() != navigationMemberInfo) + { + Dependencies.Logger.NonOwnershipInverseNavigationWarning( + entityType, navigationMemberInfo, + targetEntityType, inverseNavigationPropertyInfo, + targetOwnership.PrincipalToDependent?.GetIdentifyingMemberInfo()!); + + return null; + } + + if (targetEntityType.IsOwned() + && (targetOwnership == null + || targetOwnership.PrincipalEntityType == entityType)) + { + if (navigationMemberInfo.DeclaringType != entityType.ClrType + && (entityType.Model.FindEntityType(navigationMemberInfo.DeclaringType!) != null + || (navigationMemberInfo.DeclaringType != entityType.ClrType.BaseType + && entityType.Model.FindEntityType(entityType.ClrType.BaseType!) != null))) + { + return null; + } + + return entityTypeBuilder.HasOwnership( + targetEntityType, + navigationMemberInfo, + inverseNavigationPropertyInfo, + fromDataAnnotation: true); + } + if (entityType.IsOwned() - && !entityType.IsInOwnershipPath(targetEntityType.ClrType)) + && (ownership == null + || ownership.PrincipalEntityType == targetEntityType)) { if (navigationMemberInfo.DeclaringType != entityType.ClrType - && (entityType.Model.FindEntityTypes(navigationMemberInfo.DeclaringType!).Any() + && (entityType.Model.FindEntityType(navigationMemberInfo.DeclaringType!) != null || (navigationMemberInfo.DeclaringType != entityType.ClrType.BaseType - && entityType.Model.FindEntityTypes(entityType.ClrType.BaseType!).Any()))) + && entityType.Model.FindEntityType(entityType.ClrType.BaseType!) != null))) { return null; } @@ -157,6 +191,12 @@ private void Process( fromDataAnnotation: true); } + if (ownership != null + || targetOwnership != null) + { + return null; + } + var newForeignKeyBuilder = targetEntityTypeBuilder.HasRelationship( entityType, inverseNavigationPropertyInfo, @@ -189,7 +229,7 @@ private static bool TryRemoveIfAmbiguous( IConventionEntityType? targetBaseType, MemberInfo inverseNavigationMemberInfo, List<(MemberInfo, IConventionEntityType)> referencingNavigationsWithAttribute, - out IConventionForeignKeyBuilder? configureInverseNavigation) + out IConventionForeignKeyBuilder? remainingInverseNavigation) { var ambiguousInverse = FindAmbiguousInverse( navigationMemberInfo, entityType, referencingNavigationsWithAttribute); @@ -243,70 +283,73 @@ private static bool TryRemoveIfAmbiguous( existingAmbiguousNavigation, fromDataAnnotation: true); } - { - configureInverseNavigation = entityType.FindSkipNavigation(navigationMemberInfo)?.ForeignKey!.Builder; - return true; - } + remainingInverseNavigation = entityType.FindSkipNavigation(navigationMemberInfo)?.ForeignKey!.Builder; + return true; } else { var existingInverse = targetEntityType.FindNavigation(inverseNavigationMemberInfo)?.Inverse; - var existingInverseType = existingInverse?.DeclaringEntityType; if (existingInverse != null && IsAmbiguousInverse( - existingInverse.GetIdentifyingMemberInfo()!, existingInverseType!, referencingNavigationsWithAttribute)) + existingInverse.GetIdentifyingMemberInfo()!, + existingInverse.DeclaringEntityType, + referencingNavigationsWithAttribute)) { - var fk = existingInverse.ForeignKey; - if (fk.IsOwnership - || fk.DeclaringEntityType.Builder.HasNoRelationship(fk, fromDataAnnotation: true) == null) - { - fk.Builder.HasNavigation( - (string?)null, - existingInverse.IsOnDependent, - fromDataAnnotation: true); - } + Remove(existingInverse); } var existingNavigation = entityType.FindNavigation(navigationMemberInfo); if (existingNavigation != null) { - var fk = existingNavigation.ForeignKey; - if (fk.IsOwnership - || fk.DeclaringEntityType.Builder.HasNoRelationship(fk, fromDataAnnotation: true) == null) - { - fk.Builder.HasNavigation( - (string?)null, - existingNavigation.IsOnDependent, - fromDataAnnotation: true); - } + Remove(existingNavigation); } var existingAmbiguousNavigation = FindActualEntityType(ambiguousInverse.Value.Item2)! .FindNavigation(ambiguousInverse.Value.Item1); if (existingAmbiguousNavigation != null) { - var fk = existingAmbiguousNavigation.ForeignKey; - if (fk.IsOwnership - || fk.DeclaringEntityType.Builder.HasNoRelationship(fk, fromDataAnnotation: true) == null) - { - fk.Builder.HasNavigation( - (string?)null, - existingAmbiguousNavigation.IsOnDependent, - fromDataAnnotation: true); - } + Remove(existingAmbiguousNavigation); } - { - configureInverseNavigation = entityType.FindNavigation(navigationMemberInfo)?.ForeignKey.Builder; - return true; - } + remainingInverseNavigation = entityType.FindNavigation(navigationMemberInfo)?.ForeignKey.Builder; + return true; } } - configureInverseNavigation = null; + remainingInverseNavigation = null; return false; } + private static void Remove(IConventionNavigation navigation) + { + var foreignKey = navigation.ForeignKey; + if (foreignKey.IsOwnership) + { + if (navigation.IsOnDependent) + { + foreignKey.Builder.HasNavigation( + (string?)null, + navigation.IsOnDependent, + fromDataAnnotation: true); + } + else if (ConfigurationSource.DataAnnotation.Overrides(foreignKey.DeclaringEntityType.GetConfigurationSource())) + { + navigation.DeclaringEntityType.Model.Builder.HasNoEntityType(foreignKey.DeclaringEntityType, fromDataAnnotation: true); + } + else + { + foreignKey.DeclaringEntityType.Builder.HasNoRelationship(foreignKey, fromDataAnnotation: true); + } + } + else if (foreignKey.DeclaringEntityType.Builder.HasNoRelationship(foreignKey, fromDataAnnotation: true) == null) + { + foreignKey.Builder.HasNavigation( + (string?)null, + navigation.IsOnDependent, + fromDataAnnotation: true); + } + } + /// public override void ProcessEntityTypeRemoved( IConventionModelBuilder modelBuilder, @@ -319,20 +362,21 @@ public override void ProcessEntityTypeRemoved( var targetEntityType = modelBuilder.Metadata.FindEntityType(targetClrType); if (targetEntityType != null) { - RemoveInverseNavigation(entityType.ClrType, navigationMemberInfo, targetEntityType, attribute.Property); + RemoveInverseNavigation(entityType, navigationMemberInfo, targetEntityType, attribute.Property); } var declaringType = navigationMemberInfo.DeclaringType; Check.DebugAssert(declaringType != null, "declaringType is null"); if (modelBuilder.Metadata.FindEntityType(declaringType) != null - || entityType.HasSharedClrType) + || entityType.HasSharedClrType + || entityType.IsOwned()) { return; } var navigationName = navigationMemberInfo.GetSimpleMemberName(); var leastDerivedEntityTypes = modelBuilder.Metadata.FindLeastDerivedEntityTypes( - declaringType, t => !t.HasSharedClrType); + declaringType, t => !t.HasSharedClrType && !t.IsOwned()); foreach (var leastDerivedEntityType in leastDerivedEntityTypes) { if (leastDerivedEntityType.Builder.IsIgnored(navigationName, fromDataAnnotation: true)) @@ -387,8 +431,8 @@ public override void ProcessEntityTypeBaseTypeChanged( InversePropertyAttribute attribute, IConventionContext context) { - var entityClrType = entityTypeBuilder.Metadata.ClrType; - if (navigationMemberInfo.DeclaringType != entityClrType) + var entityType = entityTypeBuilder.Metadata; + if (navigationMemberInfo.DeclaringType != entityType.ClrType) { if (newBaseType == null) { @@ -396,13 +440,13 @@ public override void ProcessEntityTypeBaseTypeChanged( } else { - var targetEntityType = entityTypeBuilder.Metadata.Model.FindEntityType(targetClrType); + var targetEntityType = entityType.Model.FindEntityType(targetClrType); if (targetEntityType == null) { return; } - RemoveInverseNavigation(entityClrType, navigationMemberInfo, targetEntityType, attribute.Property); + RemoveInverseNavigation(entityType, navigationMemberInfo, targetEntityType, attribute.Property); } } } @@ -453,6 +497,24 @@ public override void ProcessEntityTypeBaseTypeChanged( } } + /// + public override void ProcessEntityTypeMemberIgnored( + IConventionEntityTypeBuilder entityTypeBuilder, + string name, + IConventionContext context) + { + base.ProcessEntityTypeMemberIgnored(entityTypeBuilder, name, context); + + var entityType = entityTypeBuilder.Metadata; + var navigationPropertyInfo = entityType.GetRuntimeProperties().Find(name); + if (navigationPropertyInfo == null) + { + return; + } + + RemoveInverseNavigation(null, null, entityType, name); + } + /// public override void ProcessEntityTypeMemberIgnored( IConventionEntityTypeBuilder entityTypeBuilder, @@ -468,7 +530,7 @@ public override void ProcessEntityTypeMemberIgnored( return; } - RemoveInverseNavigation(entityTypeBuilder.Metadata.ClrType, navigationMemberInfo, targetEntityType, attribute.Property); + RemoveInverseNavigation(entityTypeBuilder.Metadata, navigationMemberInfo, targetEntityType, attribute.Property); } /// @@ -650,11 +712,12 @@ private static (MemberInfo, IConventionEntityType)? FindAmbiguousInverse( } private static void RemoveInverseNavigation( - Type declaringType, - MemberInfo navigation, + IConventionEntityType? declaringEntityType, + MemberInfo? navigation, IConventionEntityType targetEntityType, string inverseNavigationName) { + var declaringType = declaringEntityType?.ClrType; var inverseNavigations = GetInverseNavigations(targetEntityType); if (inverseNavigations == null || !inverseNavigations.TryGetValue(inverseNavigationName, out var inverseNavigationPair)) @@ -667,8 +730,58 @@ private static void RemoveInverseNavigation( for (var index = 0; index < referencingNavigationsWithAttribute.Count; index++) { var referencingTuple = referencingNavigationsWithAttribute[index]; - if (referencingTuple.Item1.IsSameAs(navigation) - && declaringType.IsAssignableFrom(referencingTuple.Item2.ClrType)) + if (navigation == null) + { + anyRemoved = true; + referencingNavigationsWithAttribute.RemoveAt(index--); + if (referencingNavigationsWithAttribute.Count == 0) + { + inverseNavigations.Remove(inverseNavigation.Name); + } + + var otherEntityType = FindActualEntityType(referencingTuple.Item2); + if (otherEntityType != null) + { + // TODO: Trigger relationship discovery instead #25279 + + var existingInverses = targetEntityType.GetNavigations() + .Where(n => n.TargetEntityType == otherEntityType).ToList(); + + if (existingInverses.Count == 0) + { + otherEntityType.Builder.HasRelationship( + targetEntityType, + referencingTuple.Item1, + null); + } + else if (existingInverses.Count == 1) + { + var existingInverse = existingInverses[0]; + if (existingInverse.Inverse == null) + { + // TODO: Rely on layering instead of using DataAnnotation configuration source + // to override the null navigation configuration #15898 + otherEntityType.Builder.HasRelationship( + targetEntityType, + referencingTuple.Item1, + existingInverse.PropertyInfo, + fromDataAnnotation: true); + } + else + { + otherEntityType.Builder.HasRelationship( + targetEntityType, + referencingTuple.Item1, + null); + } + } + } + } + else if (referencingTuple.Item1.IsSameAs(navigation) + && ((!referencingTuple.Item2.IsInModel + && declaringType!.IsAssignableFrom(referencingTuple.Item2.ClrType)) + || (referencingTuple.Item2.IsInModel + && declaringEntityType!.IsAssignableFrom(referencingTuple.Item2)))) { anyRemoved = true; referencingNavigationsWithAttribute.RemoveAt(index--); diff --git a/src/EFCore/Metadata/Conventions/NavigationAttributeConventionBase.cs b/src/EFCore/Metadata/Conventions/NavigationAttributeConventionBase.cs index 90fa0b8809b..0429e98d8b3 100644 --- a/src/EFCore/Metadata/Conventions/NavigationAttributeConventionBase.cs +++ b/src/EFCore/Metadata/Conventions/NavigationAttributeConventionBase.cs @@ -120,8 +120,6 @@ public virtual void ProcessEntityTypeRemoved( IConventionEntityType entityType, IConventionContext context) { - var type = entityType.ClrType; - var navigations = GetNavigationsWithAttribute(entityType); if (navigations == null) { diff --git a/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs b/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs index 915bd315b41..780db735974 100644 --- a/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs +++ b/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs @@ -29,7 +29,8 @@ public class RelationshipDiscoveryConvention : INavigationRemovedConvention, INavigationAddedConvention, IForeignKeyOwnershipChangedConvention, - IForeignKeyNullNavigationSetConvention + IForeignKeyNullNavigationSetConvention, + IForeignKeyRemovedConvention { /// /// Creates a new instance of . @@ -84,40 +85,37 @@ private IReadOnlyList FindRelationshipCandidates(IConvent continue; } + var candidateTargetEntityType = candidateTargetEntityTypeBuilder.Metadata; if (!entityType.IsInModel) { // Current entity type was removed while the target entity type was being added - foreach (var relationshipCandidate in relationshipCandidates.Values) - { - var targetType = relationshipCandidate.TargetTypeBuilder.Metadata; - if (targetType.IsInModel - && IsImplicitlyCreatedUnusedSharedType(targetType)) - { - targetType.Builder.ModelBuilder.HasNoEntityType(targetType); - } - } - - return Array.Empty(); + relationshipCandidates[candidateTargetEntityType] = + new RelationshipCandidate(candidateTargetEntityTypeBuilder, new List(), new List(), false); + break; } - var candidateTargetEntityType = candidateTargetEntityTypeBuilder.Metadata; if (candidateTargetEntityType.IsKeyless || (candidateTargetEntityType.IsOwned() - && HasDeclaredAmbiguousNavigationsTo(entityType, targetClrType))) + && (HasDeclaredAmbiguousNavigationsTo(entityType, targetClrType) + || entityType.IsKeyless))) { + relationshipCandidates[candidateTargetEntityType] = + new RelationshipCandidate(candidateTargetEntityTypeBuilder, new List(), new List(), false); continue; } Check.DebugAssert(entityType.ClrType != targetClrType || !candidateTargetEntityType.IsOwned() || candidateTargetEntityType.FindOwnership()?.PrincipalToDependent?.Name == navigationPropertyInfo.GetSimpleMemberName(), - "New self-referencing ownerships shouldn't be discovered"); + "Self-referencing ownerships shouldn't be discovered"); var targetOwnership = candidateTargetEntityType.FindOwnership(); var shouldBeOwnership = candidateTargetEntityType.IsOwned() && (targetOwnership == null || (targetOwnership.PrincipalEntityType == entityType - && targetOwnership.PrincipalToDependent?.Name == navigationPropertyInfo.GetSimpleMemberName())); + && targetOwnership.PrincipalToDependent?.Name == navigationPropertyInfo.GetSimpleMemberName())) + && (ownership == null + || !entityType.IsInOwnershipPath(candidateTargetEntityType)); if (candidateTargetEntityType.IsOwned() && !shouldBeOwnership @@ -128,6 +126,18 @@ private IReadOnlyList FindRelationshipCandidates(IConvent { // Only the owner or nested ownees can have navigations to an owned type // Also skip non-ownership navigations from the owner + relationshipCandidates[candidateTargetEntityType] = + new RelationshipCandidate(candidateTargetEntityTypeBuilder, new List(), new List(), false); + continue; + } + + if (!shouldBeOwnership + && ownership != null + && navigationPropertyInfo.PropertyType != targetClrType) + { + // Don't try to configure a collection on an owned type unless it represents a sub-ownership + relationshipCandidates[candidateTargetEntityType] = + new RelationshipCandidate(candidateTargetEntityTypeBuilder, new List(), new List(), false); continue; } @@ -175,12 +185,15 @@ private IReadOnlyList FindRelationshipCandidates(IConvent } var inverseTargetType = inverseCandidateTuple.Value.Type; + var inverseIsCollection = inverseTargetType != inversePropertyInfo.PropertyType; if (inverseTargetType != entityType.ClrType && (!inverseTargetType.IsAssignableFrom(entityType.ClrType) + || inverseIsCollection || (!shouldBeOwnership && !candidateTargetEntityType.IsInOwnershipPath(entityType)))) { - // Only use inverse of a base type if the target is owned by the current entity type + // Only use inverse of a base type if the target is owned by the current entity type, + // unless it's a collection continue; } @@ -198,13 +211,13 @@ private IReadOnlyList FindRelationshipCandidates(IConvent } if (shouldBeOwnership - && inversePropertyInfo.PropertyType.TryGetSequenceType() != null + && inverseIsCollection && navigations.Count == 1) { // Target type should be the principal, discover the relationship from the other side var targetType = candidateTargetEntityType; if (targetType.IsInModel - && IsImplicitlyCreatedUnusedSharedType(targetType)) + && IsImplicitlyCreatedUnusedType(targetType)) { targetType.Builder.ModelBuilder.HasNoEntityType(targetType); } @@ -236,7 +249,18 @@ private List UpdateTargetEntityTypes( var candidates = new List(); foreach (var relationshipCandidate in relationshipCandidates.Values) { - if (relationshipCandidate.TargetTypeBuilder.Metadata.IsInModel) + var targetType = relationshipCandidate.TargetTypeBuilder.Metadata; + if (!entityTypeBuilder.Metadata.IsInModel + || relationshipCandidate.NavigationProperties.Count == 0) + { + if (IsImplicitlyCreatedUnusedType(targetType)) + { + targetType.Builder.ModelBuilder.HasNoEntityType(targetType); + } + continue; + } + + if (targetType.IsInModel) { candidates.Add(relationshipCandidate); continue; @@ -350,7 +374,7 @@ private static IReadOnlyList RemoveIncompatibleWithExisti PropertyInfo? compatibleInverse = null; foreach (var inverseProperty in relationshipCandidate.InverseProperties) { - if (IsCompatibleInverse( + if (AreCompatible( navigationProperty, inverseProperty, entityTypeBuilder, targetEntityTypeBuilder)) { if (compatibleInverse == null) @@ -387,6 +411,18 @@ private static IReadOnlyList RemoveIncompatibleWithExisti relationshipCandidate.InverseProperties.Remove(nextSelfRefCandidate); } + if (relationshipCandidate.NavigationProperties.Count == 0) + { + foreach (var inverseProperty in relationshipCandidate.InverseProperties.ToList()) + { + if (!AreCompatible( + null, inverseProperty, entityTypeBuilder, targetEntityTypeBuilder)) + { + relationshipCandidate.InverseProperties.Remove(inverseProperty); + } + } + } + continue; } @@ -394,7 +430,7 @@ private static IReadOnlyList RemoveIncompatibleWithExisti foreach (var otherNavigation in relationshipCandidate.NavigationProperties) { if (otherNavigation != navigationProperty - && IsCompatibleInverse(otherNavigation, compatibleInverse, entityTypeBuilder, targetEntityTypeBuilder)) + && AreCompatible(otherNavigation, compatibleInverse, entityTypeBuilder, targetEntityTypeBuilder)) { noOtherCompatibleNavigation = false; break; @@ -439,7 +475,7 @@ private static IReadOnlyList RemoveIncompatibleWithExisti { filteredRelationshipCandidates.Add(relationshipCandidate); } - else if (IsImplicitlyCreatedUnusedSharedType(relationshipCandidate.TargetTypeBuilder.Metadata) + else if (IsImplicitlyCreatedUnusedType(relationshipCandidate.TargetTypeBuilder.Metadata) && filteredRelationshipCandidates.All( c => c.TargetTypeBuilder.Metadata != relationshipCandidate.TargetTypeBuilder.Metadata)) { @@ -451,25 +487,39 @@ private static IReadOnlyList RemoveIncompatibleWithExisti return filteredRelationshipCandidates; } - private static bool IsCompatibleInverse( - PropertyInfo navigationProperty, - PropertyInfo inversePropertyInfo, + private static bool AreCompatible( + PropertyInfo? navigationProperty, + PropertyInfo? inversePropertyInfo, IConventionEntityTypeBuilder entityTypeBuilder, IConventionEntityTypeBuilder targetEntityTypeBuilder) { var entityType = entityTypeBuilder.Metadata; - var existingNavigation = entityType.FindNavigation(navigationProperty.GetSimpleMemberName()); - if (existingNavigation != null - && !CanMergeWith(existingNavigation, inversePropertyInfo, targetEntityTypeBuilder)) + if (navigationProperty != null) { - return false; + var existingNavigation = entityType.FindNavigation(navigationProperty.GetSimpleMemberName()); + if (existingNavigation != null + && ((inversePropertyInfo != null + && !CanSetInverse(existingNavigation, inversePropertyInfo, targetEntityTypeBuilder)) + || (!existingNavigation.TargetEntityType.IsAssignableFrom(targetEntityTypeBuilder.Metadata) + && !targetEntityTypeBuilder.Metadata.IsAssignableFrom(existingNavigation.TargetEntityType)))) + { + return false; + } + } + + if (inversePropertyInfo == null) + { + return true; } var existingInverse = targetEntityTypeBuilder.Metadata.FindNavigation(inversePropertyInfo.Name); if (existingInverse != null) { if (existingInverse.DeclaringEntityType != targetEntityTypeBuilder.Metadata - || !CanMergeWith(existingInverse, navigationProperty, entityTypeBuilder)) + || (navigationProperty != null + && !CanSetInverse(existingInverse, navigationProperty, entityTypeBuilder)) + || (!existingInverse.TargetEntityType.IsAssignableFrom(entityTypeBuilder.Metadata) + && !entityTypeBuilder.Metadata.IsAssignableFrom(existingInverse.TargetEntityType))) { return false; } @@ -484,7 +534,7 @@ private static bool IsCompatibleInverse( return true; } - private static bool CanMergeWith( + private static bool CanSetInverse( IConventionNavigation existingNavigation, MemberInfo inverse, IConventionEntityTypeBuilder inverseEntityTypeBuilder) @@ -585,7 +635,7 @@ private static IReadOnlyList RemoveSingleSidedBaseNavigat { filteredRelationshipCandidates.Add(relationshipCandidate); } - else if (IsImplicitlyCreatedUnusedSharedType(relationshipCandidate.TargetTypeBuilder.Metadata) + else if (IsImplicitlyCreatedUnusedType(relationshipCandidate.TargetTypeBuilder.Metadata) && filteredRelationshipCandidates.All( c => c.TargetTypeBuilder.Metadata != relationshipCandidate.TargetTypeBuilder.Metadata)) { @@ -605,54 +655,13 @@ private void CreateRelationships( foreach (var relationshipCandidate in relationshipCandidates) { var entityType = entityTypeBuilder.Metadata; - var targetEntityType = relationshipCandidate.TargetTypeBuilder.Metadata; - var isAmbiguousOnBase = entityType.BaseType != null - && HasAmbiguousNavigationsTo(entityType.BaseType, targetEntityType.ClrType) - || (targetEntityType.BaseType != null - && HasAmbiguousNavigationsTo(targetEntityType.BaseType, entityType.ClrType)); - - if ((relationshipCandidate.NavigationProperties.Count > 1 - && relationshipCandidate.InverseProperties.Count > 0 - && !relationshipCandidate.IsOwnership) - || relationshipCandidate.InverseProperties.Count > 1 - || isAmbiguousOnBase - || HasDeclaredAmbiguousNavigationsTo(entityType, targetEntityType.ClrType) - || HasDeclaredAmbiguousNavigationsTo(targetEntityType, entityType.ClrType)) - { - if (!isAmbiguousOnBase) - { - Dependencies.Logger.MultipleNavigationProperties( - relationshipCandidate.NavigationProperties.Count == 0 - ? new[] { new Tuple(null, targetEntityType.ClrType) } - : relationshipCandidate.NavigationProperties.Select( - n => new Tuple(n, entityType.ClrType)), - relationshipCandidate.InverseProperties.Count == 0 - ? new[] { new Tuple(null, targetEntityType.ClrType) } - : relationshipCandidate.InverseProperties.Select( - n => new Tuple(n, targetEntityType.ClrType))); - } - - foreach (var navigationProperty in relationshipCandidate.NavigationProperties.ToList()) - { - RemoveNavigation( - navigationProperty, entityType, relationshipCandidate.NavigationProperties); - } - foreach (var inverseProperty in relationshipCandidate.InverseProperties.ToList()) - { - RemoveNavigation( - inverseProperty, targetEntityType, relationshipCandidate.InverseProperties); - } - - if (!isAmbiguousOnBase) - { - AddAmbiguous(entityTypeBuilder, relationshipCandidate.NavigationProperties, targetEntityType.ClrType); - - AddAmbiguous(targetEntityType.Builder, relationshipCandidate.InverseProperties, entityType.ClrType); - } + RemoveExtraOwnershipInverse(entityType, relationshipCandidate); + var targetEntityType = relationshipCandidate.TargetTypeBuilder.Metadata; + if (RemoveIfAmbiguous(entityType, relationshipCandidate)) + { unusedEntityTypes.Add(targetEntityType); - continue; } @@ -675,11 +684,19 @@ private void CreateRelationships( { if (relationshipCandidate.IsOwnership) { - entityTypeBuilder.HasOwnership(targetEntityType, navigation); + var ownership = entityTypeBuilder.HasOwnership(targetEntityType, navigation); + if (ownership == null) + { + unusedEntityTypes.Add(targetEntityType); + } } else { - entityTypeBuilder.HasRelationship(targetEntityType, navigation); + var relationship = entityTypeBuilder.HasRelationship(targetEntityType, navigation); + if (relationship == null) + { + unusedEntityTypes.Add(targetEntityType); + } } } else @@ -692,7 +709,11 @@ private void CreateRelationships( if (relationshipCandidate.IsOwnership) { - entityTypeBuilder.HasOwnership(targetEntityType, navigation, inverse); + var ownership = entityTypeBuilder.HasOwnership(targetEntityType, navigation, inverse); + if (ownership == null) + { + unusedEntityTypes.Add(targetEntityType); + } } else if (entityTypeBuilder.HasRelationship(targetEntityType, navigation, inverse) == null) { @@ -704,6 +725,11 @@ private void CreateRelationships( entityTypeBuilder.HasSkipNavigation( navigation, targetEntityType, inverse, collections: true, onDependent: false); } + + if (entityType.FindNavigation(navigation) == null) + { + unusedEntityTypes.Add(targetEntityType); + } } } } @@ -738,13 +764,60 @@ private void CreateRelationships( foreach (var unusedEntityType in unusedEntityTypes) { - if (IsImplicitlyCreatedUnusedSharedType(unusedEntityType)) + if (IsImplicitlyCreatedUnusedType(unusedEntityType)) { entityTypeBuilder.ModelBuilder.HasNoEntityType(unusedEntityType); } } } + private void RemoveExtraOwnershipInverse(IConventionEntityType entityType, RelationshipCandidate relationshipCandidate) + { + if (relationshipCandidate.NavigationProperties.Count > 1 + && entityType.FindOwnership()?.PrincipalEntityType == relationshipCandidate.TargetTypeBuilder.Metadata) + { + Type? mostDerivedType = null; + foreach (var navigationProperty in relationshipCandidate.NavigationProperties) + { + var propertyType = navigationProperty.GetMemberType(); + if (mostDerivedType == null) + { + mostDerivedType = propertyType; + } + else if (!propertyType.IsAssignableFrom(mostDerivedType) + && mostDerivedType.IsAssignableFrom(propertyType)) + { + mostDerivedType = propertyType; + } + } + + relationshipCandidate.NavigationProperties.RemoveAll(p => + p.GetMemberType().IsAssignableFrom(mostDerivedType) && p.GetMemberType() != mostDerivedType); + } + + if (relationshipCandidate.InverseProperties.Count > 1 + && relationshipCandidate.IsOwnership) + { + Type? mostDerivedType = null; + foreach (var inverseProperty in relationshipCandidate.InverseProperties) + { + var inverseType = inverseProperty.GetMemberType(); + if (mostDerivedType == null) + { + mostDerivedType = inverseType; + } + else if (!inverseType.IsAssignableFrom(mostDerivedType) + && mostDerivedType.IsAssignableFrom(inverseType)) + { + mostDerivedType = inverseType; + } + } + + relationshipCandidate.InverseProperties.RemoveAll(p => + p.GetMemberType().IsAssignableFrom(mostDerivedType) && p.GetMemberType() != mostDerivedType); + } + } + /// /// Returns a value indicating whether the given entity type should be added as owned if it isn't currently in the model. /// @@ -754,6 +827,69 @@ private void CreateRelationships( protected virtual bool? ShouldBeOwned(Type targetType, IConventionModel model) => null; + private bool RemoveIfAmbiguous(IConventionEntityType entityType, RelationshipCandidate relationshipCandidate) + { + var targetEntityType = relationshipCandidate.TargetTypeBuilder.Metadata; + var isAmbiguousOnBase = entityType.BaseType != null + && HasAmbiguousNavigationsTo(entityType.BaseType, targetEntityType.ClrType) + || (targetEntityType.BaseType != null + && HasAmbiguousNavigationsTo(targetEntityType.BaseType, entityType.ClrType)); + if ((relationshipCandidate.NavigationProperties.Count > 1 + && relationshipCandidate.InverseProperties.Count > 0 + && !relationshipCandidate.IsOwnership) + || relationshipCandidate.InverseProperties.Count > 1 + || isAmbiguousOnBase + || HasDeclaredAmbiguousNavigationsTo(entityType, targetEntityType.ClrType) + || HasDeclaredAmbiguousNavigationsTo(targetEntityType, entityType.ClrType)) + { + if (entityType.IsOwned()) + { + var ownership = entityType.FindOwnership()!; + if (ownership.PrincipalEntityType == targetEntityType) + { + // Even if there are ambiguous navigations to the owner the ownership shouldn't be removed + relationshipCandidate.InverseProperties.Remove(ownership.PrincipalToDependent!.PropertyInfo!); + } + } + + if (!isAmbiguousOnBase) + { + Dependencies.Logger.MultipleNavigationProperties( + relationshipCandidate.NavigationProperties.Count == 0 + ? new[] { new Tuple(null, targetEntityType.ClrType) } + : relationshipCandidate.NavigationProperties.Select( + n => new Tuple(n, entityType.ClrType)), + relationshipCandidate.InverseProperties.Count == 0 + ? new[] { new Tuple(null, targetEntityType.ClrType) } + : relationshipCandidate.InverseProperties.Select( + n => new Tuple(n, targetEntityType.ClrType))); + } + + foreach (var navigationProperty in relationshipCandidate.NavigationProperties.ToList()) + { + RemoveNavigation( + navigationProperty, entityType, relationshipCandidate.NavigationProperties); + } + + foreach (var inverseProperty in relationshipCandidate.InverseProperties.ToList()) + { + RemoveNavigation( + inverseProperty, targetEntityType, relationshipCandidate.InverseProperties); + } + + if (!isAmbiguousOnBase) + { + AddAmbiguous(entityType.Builder, relationshipCandidate.NavigationProperties, targetEntityType.ClrType); + + AddAmbiguous(targetEntityType.Builder, relationshipCandidate.InverseProperties, entityType.ClrType); + } + + return true; + } + + return false; + } + private void RemoveNavigation( PropertyInfo navigationProperty, IConventionEntityType declaringEntityType, @@ -763,12 +899,33 @@ private void RemoveNavigation( var existingNavigation = declaringEntityType.FindDeclaredNavigation(navigationPropertyName); if (existingNavigation != null) { - if (existingNavigation.ForeignKey.DeclaringEntityType.Builder - .HasNoRelationship(existingNavigation.ForeignKey) - == null - && existingNavigation.ForeignKey.Builder.HasNavigation( - (string?)null, existingNavigation.IsOnDependent) - == null) + var removed = true; + if (existingNavigation.ForeignKey.IsOwnership) + { + if (existingNavigation.IsOnDependent) + { + removed = existingNavigation.ForeignKey.Builder.HasNavigation((string?)null, existingNavigation.IsOnDependent) + != null; + + } else if (IsImplicitlyCreatedUnusedType(existingNavigation.TargetEntityType)) + { + removed = declaringEntityType.Builder.ModelBuilder.HasNoEntityType(existingNavigation.TargetEntityType) + != null; + } + else + { + removed = existingNavigation.ForeignKey.DeclaringEntityType.Builder + .HasNoRelationship(existingNavigation.ForeignKey) != null; + } + } + else if (existingNavigation.ForeignKey.DeclaringEntityType.Builder + .HasNoRelationship(existingNavigation.ForeignKey) == null) + { + removed = existingNavigation.ForeignKey.Builder.HasNavigation((string?)null, existingNavigation.IsOnDependent) + != null; + } + + if (!removed) { // Navigations of higher configuration source are not ambiguous toRemoveFrom.Remove(navigationProperty); @@ -821,8 +978,10 @@ public virtual void ProcessEntityTypeBaseTypeChanged( { foreach (var ignoredMember in newBaseType.GetAllBaseTypesInclusive().SelectMany(et => et.GetIgnoredMembers())) { - entityTypeBuilder.Metadata.GetRuntimeProperties().TryGetValue(ignoredMember, out var ignoredPropertyInfo); - ProcessEntityTypeMemberIgnoredOnBase(entityType, ignoredMember, ignoredPropertyInfo); + if (entityTypeBuilder.Metadata.GetRuntimeProperties().TryGetValue(ignoredMember, out var ignoredPropertyInfo)) + { + ProcessEntityTypeMemberIgnoredOnBase(entityType, ignoredMember, ignoredPropertyInfo); + } } } @@ -849,6 +1008,19 @@ private void ApplyOnRelatedEntityTypes(IConventionEntityType entityType, IConven } } + /// + public virtual void ProcessForeignKeyRemoved( + IConventionEntityTypeBuilder entityTypeBuilder, + IConventionForeignKey foreignKey, + IConventionContext context) + { + if (foreignKey.IsOwnership + && !entityTypeBuilder.Metadata.IsOwned()) + { + DiscoverRelationships(entityTypeBuilder, context); + } + } + /// public virtual void ProcessNavigationRemoved( IConventionEntityTypeBuilder sourceEntityTypeBuilder, @@ -926,7 +1098,11 @@ public virtual void ProcessEntityTypeMemberIgnored( string name, IConventionContext context) { - entityTypeBuilder.Metadata.GetRuntimeProperties().TryGetValue(name, out var ignoredPropertyInfo); + if (!entityTypeBuilder.Metadata.GetRuntimeProperties().TryGetValue(name, out var ignoredPropertyInfo)) + { + return; + } + var anyAmbiguityRemoved = false; foreach (var derivedEntityType in entityTypeBuilder.Metadata.GetDerivedTypesInclusive()) { @@ -939,7 +1115,7 @@ public virtual void ProcessEntityTypeMemberIgnored( } } - private bool ProcessEntityTypeMemberIgnoredOnBase(IConventionEntityType entityType, string name, PropertyInfo? property) + private bool ProcessEntityTypeMemberIgnoredOnBase(IConventionEntityType entityType, string name, PropertyInfo property) { var ambiguousNavigations = GetAmbiguousNavigations(entityType); if (ambiguousNavigations == null) @@ -951,7 +1127,7 @@ private bool ProcessEntityTypeMemberIgnoredOnBase(IConventionEntityType entityTy foreach (var navigation in ambiguousNavigations) { if (navigation.Key.GetSimpleMemberName() != name - && navigation.Key.GetMemberType() != property?.PropertyType) + && navigation.Key.GetMemberType() != property.PropertyType) { continue; } @@ -1056,11 +1232,12 @@ public virtual void ProcessForeignKeyOwnershipChanged( IConventionContext context) => DiscoverRelationships(relationshipBuilder.Metadata.DeclaringEntityType.Builder, context); - private static bool IsImplicitlyCreatedUnusedSharedType(IConventionEntityType entityType) - => entityType.HasSharedClrType - && entityType.GetConfigurationSource() == ConfigurationSource.Convention - && !entityType.GetForeignKeys().Any() - && !entityType.GetReferencingForeignKeys().Any(); + // TODO: Rely on layering to remove these when no longer referenced #15898 + private static bool IsImplicitlyCreatedUnusedType(IConventionEntityType entityType) + => (entityType.IsOwned() || entityType.HasSharedClrType) + && entityType.GetConfigurationSource() == ConfigurationSource.Convention + && !entityType.GetForeignKeys().Any() + && !entityType.GetReferencingForeignKeys().Any(); private static bool IsAmbiguous(IConventionEntityType? entityType, MemberInfo navigationProperty) { diff --git a/src/EFCore/Metadata/IConventionModel.cs b/src/EFCore/Metadata/IConventionModel.cs index 22df45e2884..38f3862f2b9 100644 --- a/src/EFCore/Metadata/IConventionModel.cs +++ b/src/EFCore/Metadata/IConventionModel.cs @@ -293,7 +293,7 @@ public interface IConventionModel : IReadOnlyModel, IConventionAnnotatable new IEnumerable FindLeastDerivedEntityTypes( Type type, Func? condition = null) - => ((IReadOnlyModel)this).FindLeastDerivedEntityTypes(type, condition == null ? null : t => condition(t)) + => ((IReadOnlyModel)this).FindLeastDerivedEntityTypes(type, condition) .Cast(); /// @@ -304,6 +304,24 @@ public interface IConventionModel : IReadOnlyModel, IConventionAnnotatable /// Indicates whether the configuration was specified using a data annotation. void AddShared(Type type, bool fromDataAnnotation = false); + /// + /// Marks the given type as not shared, indicating that when discovered matching entity types + /// should not be configured as shared type entity types. + /// + /// The type of the entity type that should be shared. + /// The removed type. + Type? RemoveShared(Type type); + + /// + /// Returns the configuration source if the given type is marked as shared. + /// + /// The type that could be shared. + /// + /// The configuration source if the given type is marked as shared, + /// otherwise. + /// + ConfigurationSource? FindIsSharedConfigurationSource(Type type); + /// /// Marks the given entity type as owned, indicating that when discovered entity types using the given type /// should be configured as owned. @@ -326,18 +344,17 @@ public interface IConventionModel : IReadOnlyModel, IConventionAnnotatable /// /// The type of the entity type that could be owned. /// - /// if the given type name is marked as owned, + /// if the given type is marked as owned, /// otherwise. /// bool IsOwned(Type type) => FindIsOwnedConfigurationSource(type) != null; /// - /// Returns a value indicating whether the entity types using the given type should be configured - /// as owned types when discovered. + /// Returns the configuration source if the given type is marked as owned. /// /// The type of the entity type that could be owned. /// - /// The configuration source if the given type name is marked as owned, + /// The configuration source if the given type is marked as owned, /// otherwise. /// ConfigurationSource? FindIsOwnedConfigurationSource(Type type); diff --git a/src/EFCore/Metadata/IModel.cs b/src/EFCore/Metadata/IModel.cs index ab5eaced5d0..f89f9b63746 100644 --- a/src/EFCore/Metadata/IModel.cs +++ b/src/EFCore/Metadata/IModel.cs @@ -141,7 +141,7 @@ RuntimeModelDependencies GetModelDependencies() new IEnumerable FindLeastDerivedEntityTypes( Type type, Func? condition = null) - => ((IReadOnlyModel)this).FindLeastDerivedEntityTypes(type, condition == null ? null : t => condition(t)) + => ((IReadOnlyModel)this).FindLeastDerivedEntityTypes(type, condition) .Cast(); /// diff --git a/src/EFCore/Metadata/IMutableModel.cs b/src/EFCore/Metadata/IMutableModel.cs index fde9f2b1c14..eee4d02234f 100644 --- a/src/EFCore/Metadata/IMutableModel.cs +++ b/src/EFCore/Metadata/IMutableModel.cs @@ -263,7 +263,7 @@ IMutableEntityType AddEntityType( new IEnumerable FindLeastDerivedEntityTypes( Type type, Func? condition = null) - => ((IReadOnlyModel)this).FindLeastDerivedEntityTypes(type, condition == null ? null : t => condition(t)) + => ((IReadOnlyModel)this).FindLeastDerivedEntityTypes(type, condition) .Cast(); /// @@ -273,6 +273,14 @@ IMutableEntityType AddEntityType( /// The type of the entity type that should be shared. void AddShared(Type type); + /// + /// Marks the given type as not shared, indicating that when discovered matching entity types + /// should not be configured as shared type entity types. + /// + /// The type of the entity type that should be shared. + /// The removed type. + Type? RemoveShared(Type type); + /// /// Marks the given entity type as owned, indicating that when discovered matching entity types /// should be configured as owned. diff --git a/src/EFCore/Metadata/Internal/EntityType.cs b/src/EFCore/Metadata/Internal/EntityType.cs index a2408591b94..6f3f6b173da 100644 --- a/src/EFCore/Metadata/Internal/EntityType.cs +++ b/src/EFCore/Metadata/Internal/EntityType.cs @@ -218,6 +218,15 @@ public virtual bool IsOwned() public virtual void SetIsOwned(bool value) => _isOwned = value; + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public virtual EntityType? Owner + => FindOwnership()?.PrincipalEntityType; + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs index 3a6b38cb7e3..25a8a49d8da 100644 --- a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs @@ -1151,12 +1151,21 @@ public virtual bool IsIgnored(string name, ConfigurationSource? configurationSou Check.DebugAssert(navigation.DeclaringEntityType == Metadata, "navigation.DeclaringEntityType != Metadata"); var navigationConfigurationSource = navigation.GetConfigurationSource(); - if (foreignKey.GetConfigurationSource() != navigationConfigurationSource) + if ((navigation.IsOnDependent + && foreignKey.IsOwnership) + || (foreignKey.GetConfigurationSource() != navigationConfigurationSource) + && (navigation.IsOnDependent + || !foreignKey.IsOwnership)) { var removedNavigation = foreignKey.Builder.HasNavigation( (MemberInfo?)null, navigation.IsOnDependent, configurationSource); Check.DebugAssert(removedNavigation != null, "removedNavigation is null"); } + else if (foreignKey.IsOwnership + && configurationSource.Overrides(foreignKey.DeclaringEntityType.GetConfigurationSource())) + { + Metadata.Model.Builder.HasNoEntityType(foreignKey.DeclaringEntityType, configurationSource); + } else { var removedForeignKey = foreignKey.DeclaringEntityType.Builder.HasNoRelationship( @@ -1223,7 +1232,9 @@ public virtual bool IsIgnored(string name, ConfigurationSource? configurationSou if (derivedNavigation != null) { var foreignKey = derivedNavigation.ForeignKey; - if (foreignKey.GetConfigurationSource() != derivedNavigation.GetConfigurationSource()) + if (foreignKey.GetConfigurationSource() != derivedNavigation.GetConfigurationSource() + && (derivedNavigation.IsOnDependent + || !foreignKey.IsOwnership)) { if (derivedNavigation.GetConfigurationSource() != ConfigurationSource.Explicit) { @@ -1231,6 +1242,11 @@ public virtual bool IsIgnored(string name, ConfigurationSource? configurationSou (MemberInfo?)null, derivedNavigation.IsOnDependent, configurationSource); } } + else if (foreignKey.IsOwnership + && configurationSource.Overrides(foreignKey.DeclaringEntityType.GetConfigurationSource())) + { + Metadata.Model.Builder.HasNoEntityType(foreignKey.DeclaringEntityType, configurationSource); + } else if (foreignKey.GetConfigurationSource() != ConfigurationSource.Explicit) { foreignKey.DeclaringEntityType.Builder.HasNoRelationship( @@ -2202,6 +2218,13 @@ public static RelationshipSnapshot DetachRelationship(ForeignKey foreignKey, boo { foreach (var relationshipToBeDetached in keyToDetach.GetReferencingForeignKeys().ToList()) { + if (!relationshipToBeDetached.IsInModel + || !relationshipToBeDetached.DeclaringEntityType.IsInModel) + { + // Referencing type might have been removed while removing other foreign keys + continue; + } + if (detachedRelationships == null) { detachedRelationships = new List(); @@ -3712,13 +3735,6 @@ private bool Contains(IReadOnlyForeignKey? inheritedFk, IReadOnlyForeignKey deri existingTargetType.Name, existingTargetType.ClrType, configurationSource.Value, targetShouldBeOwned) : ModelBuilder.Entity(existingTargetType.ClrType, configurationSource.Value, targetShouldBeOwned); } - - if (configurationSource == null - || existingNavigation.ForeignKey.DeclaringEntityType.Builder - .HasNoRelationship(existingNavigation.ForeignKey, configurationSource.Value) == null) - { - return null; - } } if (navigation.MemberInfo == null diff --git a/src/EFCore/Metadata/Internal/InternalForeignKeyBuilder.cs b/src/EFCore/Metadata/Internal/InternalForeignKeyBuilder.cs index aecac93af39..0fe7d32b645 100644 --- a/src/EFCore/Metadata/Internal/InternalForeignKeyBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalForeignKeyBuilder.cs @@ -390,8 +390,6 @@ public InternalForeignKeyBuilder( using var batch = Metadata.DeclaringEntityType.Model.DelayConventions(); builder = this; - IsUnique(shouldBeUnique, shouldBeUnique.HasValue ? configurationSource : ConfigurationSource.Convention); - if (navigationToPrincipal != null) { if (navigationToPrincipal.Value.Name == Metadata.PrincipalToDependent?.Name) @@ -424,6 +422,9 @@ public InternalForeignKeyBuilder( if (navigationToDependent != null) { + // TODO: Use layering instead, issue #15898 + IsUnique(shouldBeUnique, shouldBeUnique.HasValue ? configurationSource : ConfigurationSource.Convention); + var navigationProperty = navigationToDependent.Value.MemberInfo; if (navigationToDependentName != null) { @@ -1099,7 +1100,7 @@ public virtual bool CanSetIsRequiredDependent(bool? required, ConfigurationSourc } Metadata.SetIsOwnership(ownership: true, configurationSource); - newRelationshipBuilder = newRelationshipBuilder?.OnDelete(DeleteBehavior.Cascade, ConfigurationSource.Convention); + newRelationshipBuilder = newRelationshipBuilder.OnDelete(DeleteBehavior.Cascade, ConfigurationSource.Convention); if (newRelationshipBuilder == null) { @@ -1186,7 +1187,14 @@ public virtual bool CanSetIsRequiredDependent(bool? required, ConfigurationSourc foreach (var invertedOwnership in invertedOwnerships) { - invertedOwnership.DeclaringEntityType.Builder.HasNoRelationship(invertedOwnership, configurationSource); + if (configurationSource.Overrides(invertedOwnership.DeclaringEntityType.GetConfigurationSource())) + { + ModelBuilder.HasNoEntityType(invertedOwnership.DeclaringEntityType, configurationSource); + } + else + { + invertedOwnership.DeclaringEntityType.Builder.HasNoRelationship(invertedOwnership, configurationSource); + } } return batch.Run(newRelationshipBuilder); @@ -1720,14 +1728,16 @@ public virtual bool CanInvert( { using (var batch = Metadata.DeclaringEntityType.Model.DelayConventions()) { + var useDefaultType = Metadata.GetPrincipalKeyConfigurationSource() == null + || (propertyNames != null + && Metadata.PrincipalKey.Properties.Count != propertyNames.Count); var relationship = HasForeignKey( dependentEntityType.Builder.GetOrCreateProperties( propertyNames, configurationSource, Metadata.PrincipalKey.Properties, Metadata.GetIsRequiredConfigurationSource() != null && Metadata.IsRequired, - Metadata.GetPrincipalKeyConfigurationSource() == null - && Metadata.PrincipalEntityType.FindPrimaryKey() == null), + useDefaultType), dependentEntityType, configurationSource); diff --git a/src/EFCore/Metadata/Internal/InternalModelBuilder.cs b/src/EFCore/Metadata/Internal/InternalModelBuilder.cs index 4e08fb45aae..3e1dd8152ab 100644 --- a/src/EFCore/Metadata/Internal/InternalModelBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalModelBuilder.cs @@ -135,11 +135,22 @@ public override InternalModelBuilder ModelBuilder else { clrType = type.Type!; - if (Metadata.IsShared(clrType)) + var sharedConfigurationSource = Metadata.FindIsSharedConfigurationSource(clrType); + if (sharedConfigurationSource != null) { - return configurationSource == ConfigurationSource.Explicit - ? throw new InvalidOperationException(CoreStrings.ClashingSharedType(clrType.ShortDisplayName())) - : null; + if (!configurationSource.OverridesStrictly(sharedConfigurationSource.Value)) + { + return configurationSource == ConfigurationSource.Explicit + ? throw new InvalidOperationException(CoreStrings.ClashingSharedType(clrType.ShortDisplayName())) + : null; + } + + foreach (var sharedTypeEntityType in Metadata.FindEntityTypes(clrType).ToList()) + { + HasNoEntityType(sharedTypeEntityType, configurationSource); + } + + Metadata.RemoveShared(clrType); } entityType = Metadata.FindEntityType(clrType); @@ -165,10 +176,9 @@ public override InternalModelBuilder ModelBuilder if (type.Type == null || entityType.ClrType == type.Type) { - if (shouldBeOwned.HasValue - && entityType.Builder.IsOwned(shouldBeOwned.Value, configurationSource) == null) + if (shouldBeOwned.HasValue) { - return null; + entityType.Builder.IsOwned(shouldBeOwned.Value, configurationSource); } entityType.UpdateConfigurationSource(configurationSource); @@ -362,7 +372,7 @@ private bool IsOwned(in TypeIdentity type) /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public virtual bool IsIgnored(Type type, ConfigurationSource configurationSource) + public virtual bool IsIgnored(Type type, ConfigurationSource? configurationSource) => IsIgnored(new TypeIdentity(type, Metadata), configurationSource); /// @@ -371,10 +381,10 @@ public virtual bool IsIgnored(Type type, ConfigurationSource configurationSource /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public virtual bool IsIgnored(string name, ConfigurationSource configurationSource) + public virtual bool IsIgnored(string name, ConfigurationSource? configurationSource) => IsIgnored(new TypeIdentity(name), configurationSource); - private bool IsIgnored(in TypeIdentity type, ConfigurationSource configurationSource) + private bool IsIgnored(in TypeIdentity type, ConfigurationSource? configurationSource) { if (configurationSource == ConfigurationSource.Explicit) { @@ -536,6 +546,11 @@ private bool CanIgnore(in TypeIdentity type, ConfigurationSource configurationSo /// public virtual InternalModelBuilder? HasNoEntityType(EntityType entityType, ConfigurationSource configurationSource) { + if (!entityType.IsInModel) + { + return this; + } + var entityTypeConfigurationSource = entityType.GetConfigurationSource(); if (!configurationSource.Overrides(entityTypeConfigurationSource)) { @@ -544,11 +559,18 @@ private bool CanIgnore(in TypeIdentity type, ConfigurationSource configurationSo using (Metadata.DelayConventions()) { - var entityTypeBuilder = entityType.Builder; foreach (var foreignKey in entityType.GetDeclaredReferencingForeignKeys().ToList()) { - var removed = foreignKey.DeclaringEntityType.Builder.HasNoRelationship(foreignKey, configurationSource); - Check.DebugAssert(removed != null, "removed is null"); + if (foreignKey.IsOwnership + && configurationSource.Overrides(foreignKey.DeclaringEntityType.GetConfigurationSource())) + { + HasNoEntityType(foreignKey.DeclaringEntityType, configurationSource); + } + else + { + var removed = foreignKey.DeclaringEntityType.Builder.HasNoRelationship(foreignKey, configurationSource); + Check.DebugAssert(removed != null, "removed is null"); + } } foreach (var skipNavigation in entityType.GetDeclaredReferencingSkipNavigations().ToList()) diff --git a/src/EFCore/Metadata/Internal/Model.cs b/src/EFCore/Metadata/Internal/Model.cs index 24ec38b34ba..a4779f0a297 100644 --- a/src/EFCore/Metadata/Internal/Model.cs +++ b/src/EFCore/Metadata/Internal/Model.cs @@ -429,6 +429,7 @@ public virtual string GetDisplayName(Type type) /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// + // TODO: Use layering to get the updated type #15898 public virtual EntityType? FindActualEntityType(EntityType entityType) => entityType.IsInModel ? entityType @@ -512,7 +513,7 @@ public virtual IReadOnlyCollection GetEntityTypes(string name) /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual bool IsShared(Type type) - => _sharedTypes.ContainsKey(type) + => FindIsSharedConfigurationSource(type) != null || Configuration?.GetConfigurationType(type) == TypeConfigurationType.SharedTypeEntityType; /// @@ -722,10 +723,30 @@ public virtual void AddOwned(Type type, ConfigurationSource configurationSource) return null; } - var name = GetDisplayName(type); - return _ownedTypes.Remove(name) ? name : null; + var currentType = type; + while (currentType != null) + { + var name = GetDisplayName(type); + if (_ownedTypes.Remove(name)) + { + return name; + } + + currentType = currentType.BaseType; + } + + return null; } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public virtual ConfigurationSource? FindIsSharedConfigurationSource(Type type) + => _sharedTypes.TryGetValue(type, out var existingTypes) ? existingTypes.ConfigurationSource : null; + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -736,7 +757,7 @@ public virtual void AddShared(Type type, ConfigurationSource configurationSource { EnsureMutable(); - if (_entityTypes.Any(et => !et.Value.HasSharedClrType && et.Value.ClrType == type)) + if (FindEntityType(type) != null) { throw new InvalidOperationException(CoreStrings.CannotMarkShared(type.ShortDisplayName())); } @@ -751,6 +772,25 @@ public virtual void AddShared(Type type, ConfigurationSource configurationSource } } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public virtual Type? RemoveShared(Type type) + { + EnsureMutable(); + + if (_sharedTypes.TryGetValue(type, out var existingTypes) + && existingTypes.Types.Any()) + { + throw new InvalidOperationException(CoreStrings.CannotMarkNonShared(type.ShortDisplayName())); + } + + return _sharedTypes.Remove(type) ? type : null; + } + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index 3c0b4ae9caa..8762199149f 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -271,6 +271,14 @@ public static string CannotLoadDetached(object? navigation, object? entityType) GetString("CannotLoadDetached", "0_navigation", "1_entityType"), navigation, entityType); + /// + /// The type '{type}' cannot be marked as a non-shared type since a shared type entity type with this CLR type exists in the model. + /// + public static string CannotMarkNonShared(object? type) + => string.Format( + GetString("CannotMarkNonShared", nameof(type)), + type); + /// /// The type '{type}' cannot be marked as a shared type since an entity type with the same CLR type already exists in the model. /// diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index 242ba9aacb1..aa4bbbd191f 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -211,6 +211,9 @@ The navigation '{1_entityType}.{0_navigation}' cannot be loaded because the entity is not being tracked. Navigations can only be loaded for tracked entities. + + The type '{type}' cannot be marked as a non-shared type since a shared type entity type with this CLR type exists in the model. + The type '{type}' cannot be marked as a shared type since an entity type with the same CLR type already exists in the model. diff --git a/test/EFCore.Cosmos.Tests/Infrastructure/CosmosModelValidatorTest.cs b/test/EFCore.Cosmos.Tests/Infrastructure/CosmosModelValidatorTest.cs index 790d7a7868d..810e0fb0b4e 100644 --- a/test/EFCore.Cosmos.Tests/Infrastructure/CosmosModelValidatorTest.cs +++ b/test/EFCore.Cosmos.Tests/Infrastructure/CosmosModelValidatorTest.cs @@ -137,6 +137,7 @@ public virtual void Detects_non_key_partition_key_property() public virtual void Detects_missing_partition_key_property() { var modelBuilder = CreateConventionalModelBuilder(); + modelBuilder.Entity(); modelBuilder.Entity().HasPartitionKey("PartitionKey"); VerifyError(CosmosStrings.PartitionKeyMissingProperty(typeof(Order).Name, "PartitionKey"), modelBuilder); @@ -193,6 +194,7 @@ public virtual void Detects_partition_key_of_different_type() public virtual void Detects_properties_mapped_to_same_property() { var modelBuilder = CreateConventionalModelBuilder(); + modelBuilder.Entity(); modelBuilder.Entity( ob => { @@ -209,6 +211,7 @@ public virtual void Detects_properties_mapped_to_same_property() public virtual void Detects_property_and_embedded_type_mapped_to_same_property() { var modelBuilder = CreateConventionalModelBuilder(); + modelBuilder.Entity(); modelBuilder.Entity( ob => { diff --git a/test/EFCore.Cosmos.Tests/ModelBuilding/CosmosModelBuilderGenericTest.cs b/test/EFCore.Cosmos.Tests/ModelBuilding/CosmosModelBuilderGenericTest.cs index 3cf86eb58c3..08876fa4b78 100644 --- a/test/EFCore.Cosmos.Tests/ModelBuilding/CosmosModelBuilderGenericTest.cs +++ b/test/EFCore.Cosmos.Tests/ModelBuilding/CosmosModelBuilderGenericTest.cs @@ -236,12 +236,49 @@ public override void Can_set_and_remove_base_type() // Fails due to presence of __jObject } + public override void Base_type_can_be_discovered_after_creating_foreign_keys_on_derived() + { + // Base discovered as owned + } + + public override void Base_types_are_mapped_correctly_if_discovered_last() + { + // Base discovered as owned + } + + public override void Relationships_on_derived_types_are_discovered_first_if_base_is_one_sided() + { + // Base discovered as owned + } + protected override TestModelBuilder CreateModelBuilder(Action configure = null) => CreateTestModelBuilder(CosmosTestHelpers.Instance, configure); } public class CosmosGenericOneToMany : GenericOneToMany { + [ConditionalFact(Skip = "#25279")] + public override void Keyless_type_discovered_before_referenced_entity_type_does_not_leave_temp_id() + { + base.Keyless_type_discovered_before_referenced_entity_type_does_not_leave_temp_id(); + } + + public override void Navigation_to_shared_type_is_not_discovered_by_convention() + { + var modelBuilder = CreateModelBuilder(); + + modelBuilder.Entity(); + + var model = modelBuilder.FinalizeModel(); + + var principal = model.FindEntityType(typeof(CollectionNavigationToSharedType)); + var owned = principal.FindNavigation(nameof(CollectionNavigationToSharedType.Navigation)).TargetEntityType; + Assert.True(owned.IsOwned()); + Assert.True(owned.HasSharedClrType); + Assert.Equal("CollectionNavigationToSharedType.Navigation#Dictionary", + owned.DisplayName()); + } + protected override TestModelBuilder CreateModelBuilder(Action configure = null) => CreateTestModelBuilder(CosmosTestHelpers.Instance, configure); } @@ -254,6 +291,22 @@ protected override TestModelBuilder CreateModelBuilder(Action(); + + var model = modelBuilder.FinalizeModel(); + + var principal = model.FindEntityType(typeof(ReferenceNavigationToSharedType)); + var owned = principal.FindNavigation(nameof(ReferenceNavigationToSharedType.Navigation)).TargetEntityType; + Assert.True(owned.IsOwned()); + Assert.True(owned.HasSharedClrType); + Assert.Equal("ReferenceNavigationToSharedType.Navigation#Dictionary", + owned.DisplayName()); + } + protected override TestModelBuilder CreateModelBuilder(Action configure = null) => CreateTestModelBuilder(CosmosTestHelpers.Instance, configure); } @@ -303,12 +356,67 @@ public virtual void Can_use_shared_type_as_join_entity_with_partition_keys() Assert.Equal("PartitionId", joinType.FindPrimaryKey().Properties.Last().Name); } + public override void Join_type_is_automatically_configured_by_convention() + { + // Many-to-many not configured by convention on Cosmos + } + + public override void Throws_for_ForeignKeyAttribute_on_navigation() + { + // Many-to-many not configured by convention on Cosmos + } + protected override TestModelBuilder CreateModelBuilder(Action configure = null) => CreateTestModelBuilder(CosmosTestHelpers.Instance, configure); } public class CosmosGenericOwnedTypes : GenericOwnedTypes { + [ConditionalFact(Skip = "#25279")] + public override void Can_configure_owned_type_collection_with_one_call_afterwards() + { + base.Can_configure_owned_type_collection_with_one_call_afterwards(); + } + + public override void Deriving_from_owned_type_throws() + { + // On Cosmos the base type starts as owned + } + + public override void Configuring_base_type_as_owned_throws() + { + // On Cosmos the base type starts as owned + } + + [ConditionalFact] + public virtual void Reference_type_is_discovered_as_owned() + { + var modelBuilder = CreateModelBuilder(); + + modelBuilder.Entity( + e => + { + e.Property(p => p.Id); + e.Property(p => p.AlternateKey); + e.Property(p => p.Description); + e.HasKey(p => p.Id); + }); + + var model = modelBuilder.FinalizeModel(); + + var owner = model.FindEntityType(typeof(OneToOneOwnerWithField)); + Assert.Equal(typeof(OneToOneOwnerWithField).FullName, owner.Name); + var ownership = owner.FindNavigation(nameof(OneToOneOwnerWithField.OwnedDependent)).ForeignKey; + Assert.True(ownership.IsOwnership); + Assert.Equal(nameof(OneToOneOwnerWithField.OwnedDependent), ownership.PrincipalToDependent.Name); + Assert.Equal(nameof(OneToOneOwnedWithField.OneToOneOwner), ownership.DependentToPrincipal.Name); + Assert.Equal(nameof(OneToOneOwnerWithField.Id), ownership.PrincipalKey.Properties.Single().Name); + var owned = ownership.DeclaringEntityType; + Assert.Single(owned.GetForeignKeys()); + Assert.NotNull(model.FindEntityType(typeof(OneToOneOwnedWithField))); + Assert.Equal(1, model.GetEntityTypes().Count(e => e.ClrType == typeof(OneToOneOwnedWithField))); + } + protected override TestModelBuilder CreateModelBuilder(Action configure = null) => CreateTestModelBuilder(CosmosTestHelpers.Instance, configure); } diff --git a/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs b/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs index 5f66a647e13..ecad3a479c4 100644 --- a/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs +++ b/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs @@ -333,7 +333,7 @@ public void Can_mark_type_as_owned_type() Assert.NotNull( modelBuilder.Entity(typeof(Product), ConfigurationSource.Explicit) - .HasOwnership(typeof(Details), nameof(Product.Details), ConfigurationSource.Convention)); + .HasOwnership(typeof(Details), nameof(Product.Details), ConfigurationSource.Explicit)); Assert.Null(modelBuilder.Ignore(typeof(Details), ConfigurationSource.Convention)); diff --git a/test/EFCore.Tests/ModelBuilding/InheritanceTestBase.cs b/test/EFCore.Tests/ModelBuilding/InheritanceTestBase.cs index d4e76e5b243..5cb5f9b0a7c 100644 --- a/test/EFCore.Tests/ModelBuilding/InheritanceTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/InheritanceTestBase.cs @@ -213,6 +213,7 @@ public virtual void Pulling_relationship_to_a_derived_type_creates_relationships var modelBuilder = CreateModelBuilder(); modelBuilder.Ignore(); modelBuilder.Ignore(); + modelBuilder.Ignore(); var principalEntityBuilder = modelBuilder.Entity(); principalEntityBuilder.Ignore(nameof(Customer.Orders)); @@ -278,6 +279,7 @@ public virtual void Pulling_relationship_to_a_derived_type_reverted_creates_rela var modelBuilder = CreateModelBuilder(); modelBuilder.Ignore(); modelBuilder.Ignore(); + modelBuilder.Ignore(); var principalEntityBuilder = modelBuilder.Entity(); principalEntityBuilder.Ignore(nameof(Customer.Orders)); @@ -312,6 +314,7 @@ public virtual void Pulling_relationship_to_a_derived_type_reverted_creates_rela public virtual void Can_match_navigation_to_derived_type_with_inverse_on_base() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -526,6 +529,7 @@ public virtual void Index_removed_when_covered_by_an_inherited_foreign_key() modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); + modelBuilder.Ignore(); var principalEntityBuilder = modelBuilder.Entity(); var derivedPrincipalEntityBuilder = modelBuilder.Entity(); @@ -609,6 +613,7 @@ public virtual void Index_removed_when_covered_by_an_inherited_index() modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); + modelBuilder.Ignore(); modelBuilder.Entity(); var derivedPrincipalEntityBuilder = modelBuilder.Entity(); @@ -698,10 +703,11 @@ public virtual void Can_create_relationship_between_base_type_and_derived_type() } [ConditionalFact] - public virtual void Removing_derived_type_make_sure_that_entity_type_is_removed_from_directly_derived_type() + public virtual void Removing_derived_removes_it_from_directly_derived_type() { var modelBuilder = CreateModelBuilder(); modelBuilder.Entity(); + modelBuilder.Entity(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -760,9 +766,11 @@ public virtual void Can_reconfigure_inherited_intraHierarchical_relationship() var extraSpecialBookLabelEntityBuilder = modelBuilder.Entity(); modelBuilder.Entity() .HasOne(e => (ExtraSpecialBookLabel)e.SpecialBookLabel) - .WithOne(e => (SpecialBookLabel)e.BookLabel); + .WithOne(e => (SpecialBookLabel)e.BookLabel) + .HasForeignKey(); var fk = bookLabelEntityBuilder.Metadata.FindNavigation(nameof(BookLabel.SpecialBookLabel)).ForeignKey; + Assert.Equal(new[] { fk }, extraSpecialBookLabelEntityBuilder.Metadata.GetForeignKeys()); Assert.Equal(nameof(SpecialBookLabel.BookLabel), fk.DependentToPrincipal.Name); Assert.Equal(new[] { fk }, extraSpecialBookLabelEntityBuilder.Metadata.GetForeignKeys()); } @@ -853,7 +861,7 @@ public virtual void Ordering_of_entityType_discovery_does_not_affect_key_convent } [ConditionalFact] // #7049 - public void Base_type_can_be_discovered_after_creating_foreign_keys_on_derived() + public virtual void Base_type_can_be_discovered_after_creating_foreign_keys_on_derived() { var mb = CreateModelBuilder(); mb.Entity(); @@ -863,7 +871,7 @@ public void Base_type_can_be_discovered_after_creating_foreign_keys_on_derived() } [ConditionalFact] - public void Can_get_set_discriminator_mapping_is_complete() + public virtual void Can_get_set_discriminator_mapping_is_complete() { var mb = CreateModelBuilder(); var baseTypeBuilder = mb.Entity(); diff --git a/test/EFCore.Tests/ModelBuilding/ManyToManyTestBase.cs b/test/EFCore.Tests/ModelBuilding/ManyToManyTestBase.cs index e8c0abfc28d..cebf98cd415 100644 --- a/test/EFCore.Tests/ModelBuilding/ManyToManyTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/ManyToManyTestBase.cs @@ -220,7 +220,10 @@ public virtual void Join_type_is_not_automatically_configured_when_navigations_a .Where(et => ((EntityType)et).IsImplicitlyCreatedJoinEntityType)); Assert.Empty(hob.GetSkipNavigations()); - Assert.Empty(nob.GetSkipNavigations()); + if (nob != null) + { + Assert.Empty(nob.GetSkipNavigations()); + } } [ConditionalFact] @@ -441,6 +444,7 @@ public virtual void Navigation_properties_can_set_access_mode() .Navigation(e => e.Dependents) .UsePropertyAccessMode(PropertyAccessMode.Field); + modelBuilder.Entity(); modelBuilder.Entity() .Navigation(e => e.ManyToManyPrincipals) .UsePropertyAccessMode(PropertyAccessMode.Property); diff --git a/test/EFCore.Tests/ModelBuilding/ManyToOneTestBase.cs b/test/EFCore.Tests/ModelBuilding/ManyToOneTestBase.cs index 44491c26fd3..677e02a2679 100644 --- a/test/EFCore.Tests/ModelBuilding/ManyToOneTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/ManyToOneTestBase.cs @@ -24,6 +24,7 @@ public virtual void Finds_existing_navigations_and_uses_associated_FK() .HasMany(e => e.Orders).WithOne(e => e.Customer) .HasForeignKey(e => e.CustomerId); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -120,6 +121,7 @@ public virtual void Finds_existing_navigation_to_principal_and_uses_associated_F modelBuilder .Entity().HasOne(o => o.Customer).WithMany() .HasForeignKey(c => c.CustomerId); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -153,6 +155,7 @@ public virtual void Finds_existing_navigation_to_dependent_and_uses_associated_F modelBuilder.Entity() .HasOne().WithMany(e => e.Orders) .HasForeignKey(e => e.CustomerId); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -187,6 +190,7 @@ public virtual void Creates_both_navigations_and_does_not_use_existing_FK() var model = modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity().HasOne().WithMany().HasForeignKey(e => e.CustomerId); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -221,6 +225,7 @@ public virtual void Creates_both_navigations_and_creates_new_FK() var model = modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -259,6 +264,7 @@ public virtual void Creates_relationship_with_navigation_to_principal() var model = modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -360,6 +366,7 @@ public virtual void Creates_both_navigations_and_uses_specified_FK_even_if_found var model = modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -807,6 +814,7 @@ public virtual void Can_use_explicitly_specified_PK() var model = modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -847,6 +855,7 @@ public virtual void Can_use_non_PK_principal() var model = modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -900,6 +909,7 @@ public virtual void Can_have_both_convention_properties_specified() var model = modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -941,6 +951,7 @@ public virtual void Can_have_both_convention_properties_specified_in_any_order() var model = modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -982,6 +993,7 @@ public virtual void Can_have_FK_by_convention_specified_with_explicit_principal_ var model = modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -1030,6 +1042,7 @@ public virtual void Can_have_FK_by_convention_specified_with_explicit_principal_ var model = modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -1894,8 +1907,13 @@ public virtual void Can_set_foreign_key_property_when_matching_property_added() public virtual void Creates_shadow_property_for_foreign_key_according_to_navigation_to_principal_name_when_present() { var modelBuilder = CreateModelBuilder(); - var beta = modelBuilder.Entity().Metadata; + modelBuilder.Entity(); + modelBuilder.Entity(); + modelBuilder.Ignore(); + + var model = modelBuilder.FinalizeModel(); + var beta = model.FindEntityType(typeof(Beta)); Assert.Equal("FirstNavId", beta.FindNavigation("FirstNav").ForeignKey.Properties.First().Name); Assert.Equal("SecondNavId", beta.FindNavigation("SecondNav").ForeignKey.Properties.First().Name); } @@ -1917,6 +1935,7 @@ public virtual void Creates_shadow_FK_property_with_non_shadow_PK() // For NonGenericStringTest modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Entity( b => @@ -1940,6 +1959,7 @@ public virtual void Creates_shadow_FK_property_with_shadow_PK() // For NonGenericStringTest modelBuilder.Entity(); + modelBuilder.Ignore(); var entityA = modelBuilder.Entity(); entityA.Property("ShadowPK"); @@ -1972,8 +1992,10 @@ public virtual void Handles_identity_correctly_while_removing_navigation() public virtual void One_to_many_relationship_has_no_ambiguity_explicit() { var modelBuilder = CreateModelBuilder(); - modelBuilder.Entity().Ignore(e => e.Omegas); - modelBuilder.Entity().HasMany().WithOne(e => e.Kappa); + modelBuilder.Entity(); + modelBuilder.Entity() + .Ignore(e => e.Omegas) + .HasMany().WithOne(e => e.Kappa); modelBuilder.FinalizeModel(); diff --git a/test/EFCore.Tests/ModelBuilding/NonRelationshipTestBase.cs b/test/EFCore.Tests/ModelBuilding/NonRelationshipTestBase.cs index 01c433b42e3..a43ee54d9b7 100644 --- a/test/EFCore.Tests/ModelBuilding/NonRelationshipTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/NonRelationshipTestBase.cs @@ -79,6 +79,8 @@ public virtual void Entity_key_on_shadow_property_is_discovered_by_convention() { var modelBuilder = CreateModelBuilder(); modelBuilder.Entity().Property("Id"); + modelBuilder.Entity(); + modelBuilder.Ignore(); var entity = modelBuilder.Model.FindEntityType(typeof(Order)); @@ -313,6 +315,7 @@ public virtual void Can_set_property_annotation() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); modelBuilder .Entity() .Property(c => c.Name).HasAnnotation("foo", "bar"); @@ -327,6 +330,7 @@ public virtual void Can_set_property_annotation_when_no_clr_property() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); modelBuilder .Entity() .Property(Customer.NameProperty.Name).HasAnnotation("foo", "bar"); @@ -341,6 +345,7 @@ public virtual void Can_set_property_annotation_by_type() { var modelBuilder = CreateModelBuilder(c => c.Properties().HaveAnnotation("foo", "bar")); + modelBuilder.Ignore(); var propertyBuilder = modelBuilder .Entity() .Property(c => c.Name).HasAnnotation("foo", "bar"); @@ -404,6 +409,7 @@ public virtual void Properties_can_be_ignored_by_type() { var modelBuilder = CreateModelBuilder(c => c.IgnoreAny()); + modelBuilder.Ignore(); modelBuilder.Entity(); var entityType = modelBuilder.FinalizeModel().FindEntityType(typeof(Customer)); @@ -446,6 +452,7 @@ public virtual void Can_add_shadow_properties_when_they_have_been_ignored() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); modelBuilder.Entity( b => { @@ -1521,6 +1528,7 @@ public virtual void Can_add_index() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); modelBuilder .Entity() .HasIndex(ix => ix.Name); @@ -1537,6 +1545,7 @@ public virtual void Can_add_index_when_no_clr_property() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); modelBuilder .Entity( b => @@ -1557,6 +1566,7 @@ public virtual void Can_add_multiple_indexes() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); var entityBuilder = modelBuilder.Entity(); entityBuilder.HasIndex(ix => ix.Id).IsUnique(); entityBuilder.HasIndex(ix => ix.Name).HasAnnotation("A1", "V1"); @@ -1583,6 +1593,7 @@ public virtual void Can_add_contained_indexes() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); var entityBuilder = modelBuilder.Entity(); var firstIndexBuilder = entityBuilder.HasIndex( ix => new { ix.Id, ix.AlternateKey }).IsUnique(); @@ -1798,6 +1809,7 @@ public virtual void Can_add_seed_data_objects() { var modelBuilder = CreateModelBuilder(); var model = modelBuilder.Model; + modelBuilder.Ignore(); modelBuilder.Entity( c => { @@ -1822,6 +1834,7 @@ public virtual void Can_add_seed_data_objects() public virtual void Can_add_seed_data_anonymous_objects() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); modelBuilder.Entity( c => { diff --git a/test/EFCore.Tests/ModelBuilding/OneToManyTestBase.cs b/test/EFCore.Tests/ModelBuilding/OneToManyTestBase.cs index 3ffe9ea09c5..536bba8e1b6 100644 --- a/test/EFCore.Tests/ModelBuilding/OneToManyTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/OneToManyTestBase.cs @@ -25,6 +25,7 @@ public virtual void Finds_existing_navigations_and_uses_associated_FK() modelBuilder.Entity() .HasOne(o => o.Customer).WithMany(c => c.Orders) .HasForeignKey(c => c.CustomerId); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -124,6 +125,7 @@ public virtual void Finds_existing_navigation_to_principal_and_uses_associated_F modelBuilder .Entity().HasOne(c => c.Customer).WithMany() .HasForeignKey(c => c.CustomerId); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -156,6 +158,7 @@ public virtual void Finds_existing_navigation_to_dependent_and_uses_associated_F modelBuilder.Entity().HasMany(e => e.Orders).WithOne() .HasForeignKey(e => e.CustomerId); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -191,6 +194,7 @@ public virtual void Creates_both_navigations_and_uses_existing_FK() modelBuilder .Entity().HasOne(e => e.Customer).WithMany(e => e.Orders) .HasForeignKey(c => c.CustomerId); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -226,6 +230,7 @@ public virtual void Creates_relationship_with_both_navigations() var model = modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -365,6 +370,7 @@ public virtual void Creates_both_navigations_and_uses_specified_FK_even_if_found var model = modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -823,6 +829,7 @@ public virtual void Can_use_explicitly_specified_PK() var model = modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -866,6 +873,7 @@ public virtual void Can_use_non_PK_principal() var model = modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -949,6 +957,7 @@ public virtual void Keyless_type_discovered_before_referenced_entity_type_does_n modelBuilder.Ignore(); modelBuilder.Ignore(); + modelBuilder.Ignore(); modelBuilder.Entity().HasNoKey(); modelBuilder.Entity(); @@ -970,6 +979,7 @@ public virtual void Can_have_both_convention_properties_specified() var model = modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -1013,6 +1023,7 @@ public virtual void Can_have_both_convention_properties_specified_in_any_order() var model = modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -1056,6 +1067,7 @@ public virtual void Can_have_FK_by_convention_specified_with_explicit_principal_ var model = modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -1104,6 +1116,7 @@ public virtual void Can_have_FK_by_convention_specified_with_explicit_principal_ var model = modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -1152,6 +1165,7 @@ public virtual void Can_have_FK_semi_specified_with_explicit_PK() var model = modelBuilder.Model; modelBuilder.Ignore(); modelBuilder.Ignore(); + modelBuilder.Ignore(); modelBuilder.Entity(); modelBuilder.Entity(); @@ -1199,6 +1213,7 @@ public virtual void Can_specify_requiredness_after_OnDelete() { var modelBuilder = CreateModelBuilder(); var model = modelBuilder.Model; + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Entity(); @@ -1895,6 +1910,7 @@ public virtual void Creates_overlapping_foreign_keys_with_different_nullability( var modelBuilder = CreateModelBuilder(); var model = modelBuilder.Model; modelBuilder.Ignore(); + modelBuilder.Entity(); modelBuilder.Entity( eb => { @@ -2234,8 +2250,13 @@ public virtual void Can_set_foreign_key_property_when_matching_property_added() public virtual void Creates_shadow_property_for_foreign_key_according_to_navigation_to_principal_name_when_present() { var modelBuilder = CreateModelBuilder(); - var entityB = modelBuilder.Entity().Metadata; + modelBuilder.Entity(); + modelBuilder.Entity(); + modelBuilder.Ignore(); + var model = modelBuilder.FinalizeModel(); + + var entityB = model.FindEntityType(typeof(Beta)); Assert.Equal("FirstNavId", entityB.FindNavigation("FirstNav").ForeignKey.Properties.First().Name); Assert.Equal("SecondNavId", entityB.FindNavigation("SecondNav").ForeignKey.Properties.First().Name); } @@ -2310,6 +2331,7 @@ public virtual void Handles_identity_correctly_while_removing_navigation() public virtual void Throws_when_foreign_key_references_shadow_key() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); modelBuilder.Entity().HasOne(e => e.Customer).WithMany(e => e.Orders).HasForeignKey(e => e.AnotherCustomerId); Assert.Equal( @@ -2551,6 +2573,7 @@ public virtual void One_to_many_relationship_has_no_ambiguity_explicit() var modelBuilder = CreateModelBuilder(); modelBuilder.Entity().Ignore(e => e.Omegas); modelBuilder.Entity().HasOne(e => e.Kappa).WithMany(); + modelBuilder.Entity(); modelBuilder.FinalizeModel(); @@ -2595,12 +2618,14 @@ public virtual void Do_not_match_non_unique_FK_when_overlap_with_PK() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Entity(); modelBuilder.Entity().HasKey(e => new { e.Id, e.Value }); - var fk = modelBuilder.Model.FindEntityType(typeof(CompositeChild)).GetForeignKeys().Single(); - Assert.Equal("ParentId", fk.Properties[0].Name); + var model = modelBuilder.FinalizeModel(); - modelBuilder.FinalizeModel(); + var child = model.FindEntityType(typeof(CompositeChild)); + var fk = child.GetForeignKeys().Single(); + Assert.Equal("ParentId", fk.Properties[0].Name); } [ConditionalFact] diff --git a/test/EFCore.Tests/ModelBuilding/OneToOneTestBase.cs b/test/EFCore.Tests/ModelBuilding/OneToOneTestBase.cs index 92dcafe2937..e06dd433c79 100644 --- a/test/EFCore.Tests/ModelBuilding/OneToOneTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/OneToOneTestBase.cs @@ -320,6 +320,7 @@ public virtual void Creates_both_navigations_and_new_FK_over_PK_by_convention() var model = modelBuilder.Model; modelBuilder.Ignore(); modelBuilder.Ignore(); + modelBuilder.Ignore(); modelBuilder.Entity().Ignore(d => d.Id); modelBuilder.Entity().Ignore(o => o.Details); @@ -1164,6 +1165,7 @@ public virtual void Can_have_both_keys_specified_explicitly() var model = (Model)modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); var dependentType = model.FindEntityType(typeof(OrderDetails)); @@ -1209,6 +1211,7 @@ public virtual void Can_have_both_keys_specified_explicitly_in_any_order() var model = (Model)modelBuilder.Model; modelBuilder.Entity(); modelBuilder.Entity(); + modelBuilder.Ignore(); modelBuilder.Ignore(); var dependentType = model.FindEntityType(typeof(OrderDetails)); @@ -1643,6 +1646,7 @@ public virtual void Principal_and_dependent_can_be_flipped_twice_separately() modelBuilder.Entity() .HasOne(e => e.Order).WithOne(e => e.Details) .HasPrincipalKey(e => e.Id); + modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); @@ -1739,6 +1743,7 @@ public virtual void IsRequired_throws_principal_end_is_ambiguous() var modelBuilder = CreateModelBuilder(); modelBuilder.Ignore(); modelBuilder.Ignore(); + modelBuilder.Entity(); modelBuilder.Entity().Property("OrderDetailsId"); Assert.Equal( diff --git a/test/EFCore.Tests/ModelBuilding/OwnedTypesTestBase.cs b/test/EFCore.Tests/ModelBuilding/OwnedTypesTestBase.cs index 4d2559bfdca..b040c528e90 100644 --- a/test/EFCore.Tests/ModelBuilding/OwnedTypesTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/OwnedTypesTestBase.cs @@ -20,6 +20,7 @@ public virtual void Can_configure_owned_type() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); modelBuilder.Entity() .OwnsOne( c => c.Details, db => @@ -52,6 +53,7 @@ public virtual void Can_configure_owned_type_using_nested_closure() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); modelBuilder.Entity().OwnsOne( c => c.Details, r => r.HasAnnotation("foo", "bar") @@ -72,6 +74,7 @@ public virtual void Can_configure_one_to_one_owned_type_with_fields() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); modelBuilder.Owned(); modelBuilder.Entity( e => @@ -165,6 +168,7 @@ public virtual void Can_configure_owned_type_inverse() var modelBuilder = CreateModelBuilder(); IReadOnlyModel model = modelBuilder.Model; + modelBuilder.Ignore(); modelBuilder.Entity().OwnsOne(c => c.Details); var owner = model.FindEntityType(typeof(Customer)); @@ -189,6 +193,7 @@ public virtual void Can_configure_owned_type_properties() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); modelBuilder.Entity().OwnsOne(c => c.Details) .UsePropertyAccessMode(PropertyAccessMode.FieldDuringConstruction) .HasChangeTrackingStrategy(ChangeTrackingStrategy.ChangedNotifications) @@ -210,6 +215,7 @@ public virtual void Can_configure_owned_type_key() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); modelBuilder.Entity().OwnsOne(c => c.Details) .HasKey(c => c.Id); @@ -225,6 +231,7 @@ public virtual void Can_configure_ownership_foreign_key() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); modelBuilder.Entity() .OwnsOne(c => c.Details) .WithOwner(d => d.Customer) @@ -243,6 +250,7 @@ public virtual void Can_configure_another_relationship_to_owner() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); modelBuilder.Entity().OwnsOne( c => c.Details, r => @@ -298,6 +306,7 @@ public virtual void Can_configure_multiple_ownerships() var modelBuilder = CreateModelBuilder(); modelBuilder.Ignore(); + modelBuilder.Ignore(); modelBuilder.Entity().OwnsOne(c => c.Details); modelBuilder.Entity().OwnsOne(c => c.Details); @@ -319,14 +328,13 @@ public virtual void Can_configure_one_to_one_relationship_from_an_owned_type() var modelBuilder = CreateModelBuilder(); modelBuilder.Ignore(); + modelBuilder.Ignore(); modelBuilder.Entity(); modelBuilder.Entity().OwnsOne(c => c.Details) .HasOne() .WithOne() .HasPrincipalKey(); - Assert.NotNull(modelBuilder.Model.FindEntityType(typeof(CustomerDetails))); - modelBuilder.Entity().OwnsOne(c => c.Details); var model = modelBuilder.FinalizeModel(); @@ -338,7 +346,7 @@ public virtual void Can_configure_one_to_one_relationship_from_an_owned_type() && fk.PrincipalToDependent == null); Assert.Same(ownership.DeclaringEntityType, foreignKey.DeclaringEntityType); Assert.NotEqual(ownership.Properties.Single().Name, foreignKey.Properties.Single().Name); - Assert.Equal(2, model.GetEntityTypes().Count(e => e.ClrType == typeof(CustomerDetails))); + Assert.Equal(2, model.FindEntityTypes(typeof(CustomerDetails)).Count()); Assert.Equal(2, ownership.DeclaringEntityType.GetForeignKeys().Count()); } @@ -349,6 +357,7 @@ public virtual void Can_configure_owned_type_collection_from_an_owned_type() IReadOnlyModel model = modelBuilder.Model; modelBuilder.Ignore(); + modelBuilder.Ignore(); var entityBuilder = modelBuilder.Entity().OwnsOne(o => o.Customer) .OwnsMany(c => c.Orders); @@ -387,6 +396,7 @@ public virtual void Can_configure_owned_type_collection() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); var entityBuilder = modelBuilder.Entity().OwnsMany(c => c.Orders) .UsePropertyAccessMode(PropertyAccessMode.FieldDuringConstruction) .HasChangeTrackingStrategy(ChangeTrackingStrategy.ChangedNotifications) @@ -428,6 +438,7 @@ public virtual void Can_configure_owned_type_collection_using_nested_closure() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); modelBuilder.Entity().OwnsMany( c => c.Orders, r => @@ -467,6 +478,7 @@ public virtual void Can_configure_one_to_one_relationship_from_an_owned_type_col modelBuilder.Ignore(); modelBuilder.Ignore(); + modelBuilder.Ignore(); modelBuilder.Entity().OwnsMany( c => c.Orders, ob => { @@ -476,8 +488,6 @@ public virtual void Can_configure_one_to_one_relationship_from_an_owned_type_col .HasPrincipalKey(); }); - Assert.NotNull(modelBuilder.Model.FindEntityType(typeof(Order))); - modelBuilder.Entity().OwnsMany(c => c.Orders) .HasKey(o => o.OrderId); @@ -501,7 +511,6 @@ public virtual void Can_configure_one_to_one_relationship_from_an_owned_type_col Assert.Same(ownership1.DeclaringEntityType, foreignKey.DeclaringEntityType); Assert.Null(foreignKey.PrincipalToDependent); Assert.NotEqual(ownership1.Properties.Single().Name, foreignKey.Properties.Single().Name); - Assert.Equal(5, model.GetEntityTypes().Count()); Assert.Equal(2, model.FindEntityTypes(typeof(Order)).Count()); Assert.Equal(2, ownership1.DeclaringEntityType.GetForeignKeys().Count()); @@ -543,6 +552,7 @@ public virtual void Can_configure_owned_type_from_an_owned_type_collection(HasDa var modelBuilder = CreateModelBuilder(); modelBuilder.Ignore(); + modelBuilder.Ignore(); modelBuilder.Entity().OwnsMany( c => c.Orders, ob => { @@ -754,7 +764,9 @@ public virtual void Can_configure_owned_type_collection_with_one_call() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); modelBuilder.Owned(); + modelBuilder.Entity(); modelBuilder.Entity() .OwnsMany(c => c.Orders) .HasKey(o => o.OrderId); @@ -779,7 +791,6 @@ public virtual void Can_configure_owned_type_collection_with_one_call() Assert.Equal(nameof(Order.OrderId), ownership.DeclaringEntityType.FindPrimaryKey().Properties.Single().Name); Assert.Same(ownership.DeclaringEntityType, model.FindEntityType(typeof(Order), nameof(Customer.Orders), customer)); - Assert.Equal(2, model.FindEntityTypes(typeof(Order)).Count()); Assert.True(model.IsShared(typeof(Order))); var specialOwnership = specialCustomer.FindNavigation(nameof(SpecialCustomer.SpecialOrders)).ForeignKey; @@ -793,7 +804,6 @@ public virtual void Can_configure_owned_type_collection_with_one_call() Assert.Equal(9, modelBuilder.Model.GetEntityTypes().Count()); Assert.Equal(2, modelBuilder.Model.FindEntityTypes(typeof(Order)).Count()); Assert.Equal(7, modelBuilder.Model.GetEntityTypes().Count(e => !e.HasSharedClrType)); - Assert.Equal(5, modelBuilder.Model.GetEntityTypes().Count(e => e.IsOwned())); var conventionModel = (IConventionModel)modelBuilder.Model; Assert.Null(conventionModel.FindIgnoredConfigurationSource(typeof(Order))); @@ -807,6 +817,8 @@ public virtual void Can_configure_owned_type_collection_with_one_call_afterwards { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); + modelBuilder.Entity(); modelBuilder.Owned(); modelBuilder.Entity(); @@ -838,11 +850,9 @@ public virtual void Can_configure_owned_type_collection_with_one_call_afterwards Assert.Equal( nameof(SpecialOrder.SpecialOrderId), specialOwnership.DeclaringEntityType.FindPrimaryKey().Properties.Single().Name); - Assert.Equal(9, modelBuilder.Model.GetEntityTypes().Count()); Assert.Equal(2, modelBuilder.Model.FindEntityTypes(typeof(Order)).Count()); // SpecialOrder and Address are only used once, but once they are made shared they don't revert to non-shared - Assert.Equal(5, modelBuilder.Model.GetEntityTypes().Count(e => !e.HasSharedClrType)); - Assert.Equal(5, modelBuilder.Model.GetEntityTypes().Count(e => e.IsOwned())); + Assert.Equal(7, modelBuilder.Model.GetEntityTypes().Count(e => !e.HasSharedClrType)); var conventionModel = (IConventionModel)modelBuilder.Model; Assert.Null(conventionModel.FindIgnoredConfigurationSource(typeof(Order))); @@ -856,7 +866,11 @@ public virtual void Can_configure_single_owned_type_using_attribute() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); modelBuilder.Entity(); + modelBuilder.Entity(); + modelBuilder.Entity(); var model = modelBuilder.FinalizeModel(); @@ -911,7 +925,9 @@ public virtual void Can_map_base_of_owned_type() var modelBuilder = CreateModelBuilder(); modelBuilder.Ignore(); + modelBuilder.Ignore(); modelBuilder.Entity().OwnsOne(c => c.Details); + modelBuilder.Entity(); modelBuilder.Entity(); modelBuilder.Ignore(); @@ -932,6 +948,8 @@ public virtual void Can_map_base_of_owned_type_first() var modelBuilder = CreateModelBuilder(); modelBuilder.Ignore(); + modelBuilder.Ignore(); + modelBuilder.Entity(); modelBuilder.Entity(); modelBuilder.Entity().OwnsOne(c => c.Details); modelBuilder.Ignore(); @@ -953,6 +971,8 @@ public virtual void Can_map_derived_of_owned_type() var modelBuilder = CreateModelBuilder(); modelBuilder.Ignore(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); modelBuilder.Entity().OwnsOne(c => c.Details); modelBuilder.Entity(); @@ -983,10 +1003,14 @@ public virtual void Can_map_derived_of_owned_type_first() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); modelBuilder.Entity().OwnsOne(c => c.Details); IReadOnlyModel model = modelBuilder.Model; + modelBuilder.Entity(); + var owner = model.FindEntityType(typeof(OrderCombination)); var owned = owner.FindNavigation(nameof(OrderCombination.Details)).ForeignKey.DeclaringEntityType; Assert.Empty(owned.GetDirectlyDerivedTypes()); @@ -998,7 +1022,7 @@ public virtual void Can_map_derived_of_owned_type_first() return targetType != typeof(DetailsBase) && typeof(DetailsBase).IsAssignableFrom(targetType); })); Assert.Single(owned.GetForeignKeys()); - Assert.Equal(1, model.GetEntityTypes().Count(e => e.ClrType == typeof(DetailsBase))); + Assert.Single(model.FindEntityTypes(typeof(DetailsBase))); Assert.Null(model.FindEntityType(typeof(CustomerDetails)).BaseType); modelBuilder.Entity().Ignore(c => c.Details); @@ -1012,8 +1036,11 @@ public virtual void Throws_on_FK_matching_two_relationships() { var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); modelBuilder.Ignore(); + modelBuilder.Ignore(); modelBuilder.Entity(); + modelBuilder.Entity(); Assert.Equal( CoreStrings.AmbiguousForeignKeyPropertyCandidates( @@ -1067,6 +1094,8 @@ public virtual void Can_configure_chained_ownerships() }); }); + modelBuilder.Entity(); + var model = modelBuilder.FinalizeModel(); VerifyOwnedBookLabelModel(model); @@ -1123,6 +1152,8 @@ public virtual void Can_configure_chained_ownerships_different_order() }); }); + modelBuilder.Entity(); + var model = modelBuilder.FinalizeModel(); VerifyOwnedBookLabelModel(model); @@ -1171,6 +1202,8 @@ public virtual void Can_configure_hierarchy_with_reference_navigations_as_owned( .Ignore(l => l.Book); }); + modelBuilder.Entity(); + var model = modelBuilder.FinalizeModel(); VerifyOwnedBookLabelModel(model); @@ -1218,6 +1251,8 @@ public virtual void Can_configure_hierarchy_with_reference_navigations_as_owned_ .Ignore(l => l.Book); }); + modelBuilder.Entity(); + var model = modelBuilder.FinalizeModel(); VerifyOwnedBookLabelModel(model); @@ -1277,6 +1312,60 @@ protected virtual void VerifyOwnedBookLabelModel(IReadOnlyModel model) Assert.Equal(4, model.GetEntityTypes().Count(e => e.ClrType == typeof(SpecialBookLabel))); } + [ConditionalFact] + public virtual void Removing_ambiguous_inverse_allows_navigations_to_be_discovered() + { + var modelBuilder = CreateModelBuilder(); + + modelBuilder.Owned(); + modelBuilder.Entity(); + + modelBuilder.Entity() + .OwnsOne( + b => b.AlternateLabel, al => + { + al.OwnsOne(b => b.AnotherBookLabel) + .OwnsOne(b => b.SpecialBookLabel) + .Ignore(l => l.BookLabel); + + al.OwnsOne(b => b.SpecialBookLabel) + .OwnsOne(b => b.AnotherBookLabel); + }); + + modelBuilder.Entity().Ignore(b => b.Label); + + modelBuilder.Entity(); + + var model = modelBuilder.FinalizeModel(); + + var bookOwnership = model.FindEntityType(typeof(Book)).FindNavigation(nameof(Book.AlternateLabel)).ForeignKey; + Assert.Equal(nameof(BookLabel.Book), bookOwnership.DependentToPrincipal.Name); + + var bookLabelOwnership1 = bookOwnership.DeclaringEntityType.FindNavigation( + nameof(BookLabel.AnotherBookLabel)).ForeignKey; + var bookLabelOwnership2 = bookOwnership.DeclaringEntityType.FindNavigation( + nameof(BookLabel.SpecialBookLabel)).ForeignKey; + + Assert.Null(bookLabelOwnership1.DependentToPrincipal); + Assert.Equal(nameof(SpecialBookLabel.BookLabel), bookLabelOwnership2.DependentToPrincipal.Name); + + var bookLabel2Ownership1Subownership = bookLabelOwnership1.DeclaringEntityType.FindNavigation( + nameof(BookLabel.SpecialBookLabel)).ForeignKey; + var bookLabel2Ownership2Subownership = bookLabelOwnership2.DeclaringEntityType.FindNavigation( + nameof(BookLabel.AnotherBookLabel)).ForeignKey; + + Assert.NotNull(bookLabelOwnership1.DeclaringEntityType.FindNavigation(nameof(BookLabel.Book))); + Assert.NotNull(bookLabelOwnership2.DeclaringEntityType.FindNavigation(nameof(BookLabel.Book))); + Assert.NotNull(bookLabel2Ownership1Subownership.DeclaringEntityType.FindNavigation(nameof(BookLabel.Book))); + Assert.NotNull(bookLabel2Ownership2Subownership.DeclaringEntityType.FindNavigation(nameof(BookLabel.Book))); + Assert.Equal(nameof(SpecialBookLabel.AnotherBookLabel), bookLabel2Ownership1Subownership.DependentToPrincipal.Name); + Assert.Equal(nameof(AnotherBookLabel.SpecialBookLabel), bookLabel2Ownership2Subownership.DependentToPrincipal.Name); + + Assert.Equal(1, model.GetEntityTypes().Count(e => e.ClrType == typeof(BookLabel))); + Assert.Equal(2, model.GetEntityTypes().Count(e => e.ClrType == typeof(AnotherBookLabel))); + Assert.Equal(2, model.GetEntityTypes().Count(e => e.ClrType == typeof(SpecialBookLabel))); + } + [ConditionalFact] public virtual void Can_configure_self_ownership() { @@ -1309,8 +1398,7 @@ public virtual void Reconfiguring_entity_type_as_owned_throws() Assert.Equal( CoreStrings.ClashingNonOwnedEntityType(nameof(CustomerDetails)), Assert.Throws( - () => - modelBuilder.Entity().OwnsOne(c => c.Details)).Message); + () => modelBuilder.Entity().OwnsOne(c => c.Details)).Message); } [ConditionalFact] @@ -1319,7 +1407,7 @@ public virtual void Reconfiguring_owned_type_as_non_owned_throws() var modelBuilder = CreateModelBuilder(); modelBuilder.Ignore(); - var entityType = modelBuilder.Entity().OwnsOne(c => c.Details).OwnedEntityType; + modelBuilder.Entity().OwnsOne(c => c.Details); Assert.Equal( CoreStrings.ClashingOwnedEntityType(nameof(CustomerDetails)),