From cb9b40154d625c327e0bfa72110575b65ef40c35 Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Wed, 11 Oct 2023 20:45:29 -0700 Subject: [PATCH] Throw for conflicting explicit relationships when configuring an entity type as keyless. Fixes #24722 --- .../Internal/InternalEntityTypeBuilder.cs | 53 ++++++++++++++- .../Internal/InternalForeignKeyBuilder.cs | 23 ++++++- src/EFCore/Properties/CoreStrings.Designer.cs | 12 ++-- src/EFCore/Properties/CoreStrings.resx | 12 ++-- .../ModelBuilding/OneToManyTestBase.cs | 34 +++++++++- .../ModelBuilding/OneToOneTestBase.cs | 67 +++++++++++++++++++ 6 files changed, 184 insertions(+), 17 deletions(-) diff --git a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs index 6cdef5bd72a..6854e348d6f 100644 --- a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs @@ -433,12 +433,61 @@ public static (InternalKeyBuilder, ConfigurationSource?) DetachKey(Key keyToDeta { foreach (var foreignKey in Metadata.GetReferencingForeignKeys().ToList()) { - foreignKey.DeclaringEntityType.Builder.HasNoRelationship(foreignKey, configurationSource); + if (foreignKey.GetConfigurationSource() != ConfigurationSource.Explicit + || configurationSource != ConfigurationSource.Explicit) + { + foreignKey.DeclaringEntityType.Builder.HasNoRelationship(foreignKey, configurationSource); + continue; + } + + if (foreignKey.DependentToPrincipal != null && foreignKey.GetDependentToPrincipalConfigurationSource() == ConfigurationSource.Explicit) + { + throw new InvalidOperationException( + CoreStrings.NavigationToKeylessType(foreignKey.DependentToPrincipal.Name, Metadata.DisplayName())); + } + else if ((foreignKey.IsUnique || foreignKey.GetIsUniqueConfigurationSource() != ConfigurationSource.Explicit) + && foreignKey.GetPrincipalEndConfigurationSource() != ConfigurationSource.Explicit + && foreignKey.Builder.CanSetEntityTypes( + foreignKey.DeclaringEntityType, + foreignKey.PrincipalEntityType, + configurationSource, + out _, + out var shouldResetToDependent) + && (!shouldResetToDependent || foreignKey.GetPrincipalToDependentConfigurationSource() != ConfigurationSource.Explicit)) + { + foreignKey.Builder.HasEntityTypes( + foreignKey.DeclaringEntityType, + foreignKey.PrincipalEntityType, + configurationSource); + } + else + { + throw new InvalidOperationException( + CoreStrings.PrincipalKeylessType( + Metadata.DisplayName(), + Metadata.DisplayName() + + (foreignKey.PrincipalToDependent == null + ? "" + : "." + foreignKey.PrincipalToDependent.Name), + foreignKey.DeclaringEntityType.DisplayName())); + } } foreach (var foreignKey in Metadata.GetForeignKeys()) { - foreignKey.SetPrincipalToDependent((string?)null, configurationSource); + if (foreignKey.PrincipalToDependent == null) + { + continue; + } + + if (foreignKey.GetPrincipalToDependentConfigurationSource() == ConfigurationSource.Explicit + && configurationSource == ConfigurationSource.Explicit) + { + throw new InvalidOperationException( + CoreStrings.NavigationToKeylessType(foreignKey.PrincipalToDependent.Name, Metadata.DisplayName())); + } + + foreignKey.Builder.HasNavigation((string?)null, pointsToPrincipal: false, configurationSource); } foreach (var key in Metadata.GetKeys().ToList()) diff --git a/src/EFCore/Metadata/Internal/InternalForeignKeyBuilder.cs b/src/EFCore/Metadata/Internal/InternalForeignKeyBuilder.cs index 56690729fa4..72ca557cc45 100644 --- a/src/EFCore/Metadata/Internal/InternalForeignKeyBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalForeignKeyBuilder.cs @@ -1448,6 +1448,25 @@ public virtual bool CanSetEntityTypes( EntityType principalEntityType, EntityType dependentEntityType, ConfigurationSource? configurationSource) + => CanSetEntityTypes( + principalEntityType, + dependentEntityType, + configurationSource, + out _, + out _); + + /// + /// 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 bool CanSetEntityTypes( + EntityType principalEntityType, + EntityType dependentEntityType, + ConfigurationSource? configurationSource, + out bool shouldResetToPrincipal, + out bool shouldResetToDependent) => CanSetRelatedTypes( principalEntityType, dependentEntityType, @@ -1457,8 +1476,8 @@ public virtual bool CanSetEntityTypes( configurationSource, shouldThrow: false, out _, - out _, - out _, + out shouldResetToPrincipal, + out shouldResetToDependent, out _, out _, out _); diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index ecc04f5d3f9..6a2efa2891f 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -781,7 +781,7 @@ public static string DependentEntityTypeNotInRelationship(object? dependentEntit dependentEntityType, principalEntityType, entityType); /// - /// Unable to set a base type for entity type '{entityType}' because it has been configured as keyless. + /// Unable to set a base type for entity type '{entityType}' because it has been configured as keyless. For more information on keyless entity types, see https://go.microsoft.com/fwlink/?linkid=2141943. /// public static string DerivedEntityCannotBeKeyless(object? entityType) => string.Format( @@ -805,7 +805,7 @@ public static string DerivedEntityOwnershipMismatch(object? baseEntityType, obje baseEntityType, derivedEntityType, ownedEntityType, nonOwnedEntityType); /// - /// '{derivedType}' cannot be configured as keyless because it is a derived type; the root type '{rootType}' must be configured as keyless instead. If you did not intend for '{rootType}' to be included in the model, ensure that it is not referenced by a DbSet property on your context, referenced in a configuration call to ModelBuilder in 'OnModelCreating', or referenced from a navigation on a type that is included in the model. + /// '{derivedType}' cannot be configured as keyless because it is a derived type; the root type '{rootType}' must be configured as keyless instead. If you did not intend for '{rootType}' to be included in the model, ensure that it is not referenced by a DbSet property on your context, referenced in a configuration call to ModelBuilder in 'OnModelCreating', or referenced from a navigation on a type that is included in the model. For more information on keyless entity types, see https://go.microsoft.com/fwlink/?linkid=2141943. /// public static string DerivedEntityTypeHasNoKey(object? derivedType, object? rootType) => string.Format( @@ -981,7 +981,7 @@ public static string EntityEqualityOnCompositeKeyEntitySubqueryNotSupported(obje comparisonOperator, entityType); /// - /// Cannot translate the '{comparisonOperator}' on an expression of entity type '{entityType}' because it is a keyless entity. Consider using entity properties instead. + /// Cannot translate the '{comparisonOperator}' on an expression of entity type '{entityType}' because it is a keyless entity. Consider using entity properties instead. For more information on keyless entity types, see https://go.microsoft.com/fwlink/?linkid=2141943. /// public static string EntityEqualityOnKeylessEntityNotSupported(object? comparisonOperator, object? entityType) => string.Format( @@ -1512,7 +1512,7 @@ public static string InvalidReplaceService(object? replaceService, object? useIn replaceService, useInternalServiceProvider); /// - /// The invoked method cannot be used for the entity type '{entityType}' because it does not have a primary key. + /// The invoked method cannot be used for the entity type '{entityType}' because it does not have a primary key. For more information on keyless entity types, see https://go.microsoft.com/fwlink/?linkid=2141943. /// public static string InvalidSetKeylessOperation(object? entityType) => string.Format( @@ -1920,7 +1920,7 @@ public static string NavigationSingleWrongClrType(object? navigation, object? en navigation, entityType, clrType, targetType); /// - /// The navigation '{navigation}' cannot be added because it targets the keyless entity type '{entityType}'. Navigations can only target entity types with keys. + /// The navigation '{navigation}' cannot be added because it targets the keyless entity type '{entityType}'. Navigations can only target entity types with keys. For more information on keyless entity types, see https://go.microsoft.com/fwlink/?linkid=2141943. /// public static string NavigationToKeylessType(object? navigation, object? entityType) => string.Format( @@ -2290,7 +2290,7 @@ public static string PrincipalEntityTypeNotInRelationship(object? dependentEntit dependentEntityType, principalEntityType, entityType); /// - /// The keyless entity type '{entityType}' cannot be on the principal end of the relationship between '{firstNavigationSpecification}' and '{secondNavigationSpecification}'. The principal entity type must have a key. + /// The keyless entity type '{entityType}' cannot be on the principal end of the relationship between '{firstNavigationSpecification}' and '{secondNavigationSpecification}'. The principal entity type must have a key. For more information on keyless entity types, see https://go.microsoft.com/fwlink/?linkid=2141943. /// public static string PrincipalKeylessType(object? entityType, object? firstNavigationSpecification, object? secondNavigationSpecification) => string.Format( diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index ce54c42bab7..5c073247cd8 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -406,7 +406,7 @@ You are configuring a relationship between '{dependentEntityType}' and '{principalEntityType}' but have specified a foreign key on '{entityType}'. The foreign key must be defined on a type that is part of the relationship. - Unable to set a base type for entity type '{entityType}' because it has been configured as keyless. + Unable to set a base type for entity type '{entityType}' because it has been configured as keyless. For more information on keyless entity types, see https://go.microsoft.com/fwlink/?linkid=2141943. Unable to set a base type for entity type '{entityType}' because it has one or more keys defined. Only root types can have keys. @@ -415,7 +415,7 @@ Unable to set '{baseEntityType}' as the base type for entity type '{derivedEntityType}' because '{ownedEntityType}' is configured as owned, while '{nonOwnedEntityType}' is non-owned. All entity types in a hierarchy need to have the same ownership status. See https://aka.ms/efcore-docs-owned for more information and examples. - '{derivedType}' cannot be configured as keyless because it is a derived type; the root type '{rootType}' must be configured as keyless instead. If you did not intend for '{rootType}' to be included in the model, ensure that it is not referenced by a DbSet property on your context, referenced in a configuration call to ModelBuilder in 'OnModelCreating', or referenced from a navigation on a type that is included in the model. + '{derivedType}' cannot be configured as keyless because it is a derived type; the root type '{rootType}' must be configured as keyless instead. If you did not intend for '{rootType}' to be included in the model, ensure that it is not referenced by a DbSet property on your context, referenced in a configuration call to ModelBuilder in 'OnModelCreating', or referenced from a navigation on a type that is included in the model. For more information on keyless entity types, see https://go.microsoft.com/fwlink/?linkid=2141943. A key cannot be configured on '{derivedType}' because it is a derived type. The key must be configured on the root type '{rootType}'. If you did not intend for '{rootType}' to be included in the model, ensure that it is not referenced by a DbSet property on your context, referenced in a configuration call to ModelBuilder, or referenced from a navigation on a type that is included in the model. @@ -481,7 +481,7 @@ Cannot translate '{comparisonOperator}' on a subquery expression of entity type '{entityType}' because it has a composite primary key. See https://go.microsoft.com/fwlink/?linkid=2141942 for information on how to rewrite your query. - Cannot translate the '{comparisonOperator}' on an expression of entity type '{entityType}' because it is a keyless entity. Consider using entity properties instead. + Cannot translate the '{comparisonOperator}' on an expression of entity type '{entityType}' because it is a keyless entity. Consider using entity properties instead. For more information on keyless entity types, see https://go.microsoft.com/fwlink/?linkid=2141943. The entity type '{entityType}' requires a primary key to be defined. If you intended to use a keyless entity type, call 'HasNoKey' in 'OnModelCreating'. For more information on keyless entity types, see https://go.microsoft.com/fwlink/?linkid=2141943. @@ -686,7 +686,7 @@ A call was made to '{replaceService}', but Entity Framework is not building its own internal service provider. Either allow Entity Framework to build the service provider by removing the call to '{useInternalServiceProvider}', or build replacement services into the service provider before passing it to '{useInternalServiceProvider}'. - The invoked method cannot be used for the entity type '{entityType}' because it does not have a primary key. + The invoked method cannot be used for the entity type '{entityType}' because it does not have a primary key. For more information on keyless entity types, see https://go.microsoft.com/fwlink/?linkid=2141943. Cannot create a DbSet for '{typeName}' because this type is not included in the model for the context. However the model contains an entity type with the same name in a different namespace: '{entityTypeName}'. @@ -1154,7 +1154,7 @@ The navigation '{navigation}' cannot be added to the entity type '{entityType}' because its CLR type '{clrType}' does not match the expected CLR type '{targetType}'. - The navigation '{navigation}' cannot be added because it targets the keyless entity type '{entityType}'. Navigations can only target entity types with keys. + The navigation '{navigation}' cannot be added because it targets the keyless entity type '{entityType}'. Navigations can only target entity types with keys. For more information on keyless entity types, see https://go.microsoft.com/fwlink/?linkid=2141943. No backing field was found for property '{1_entityType}.{0_property}'. Name the backing field so that it is discovered by convention, configure the backing field to use, or use a different '{propertyAccessMode}'. @@ -1298,7 +1298,7 @@ You are configuring a relationship between '{dependentEntityType}' and '{principalEntityType}', but have specified a principal key on '{entityType}'. The foreign key must target a type that is part of the relationship. - The keyless entity type '{entityType}' cannot be on the principal end of the relationship between '{firstNavigationSpecification}' and '{secondNavigationSpecification}'. The principal entity type must have a key. + The keyless entity type '{entityType}' cannot be on the principal end of the relationship between '{firstNavigationSpecification}' and '{secondNavigationSpecification}'. The principal entity type must have a key. For more information on keyless entity types, see https://go.microsoft.com/fwlink/?linkid=2141943. The relationship from '{referencingEntityTypeOrNavigation}' to '{referencedEntityTypeOrNavigation}' is not supported because the owned entity type '{ownedType}' cannot be on the principal side of a non-ownership relationship. Remove the relationship or configure the foreign key to be on '{ownedType}'. diff --git a/test/EFCore.Tests/ModelBuilding/OneToManyTestBase.cs b/test/EFCore.Tests/ModelBuilding/OneToManyTestBase.cs index 56f9f74652b..0db49908f92 100644 --- a/test/EFCore.Tests/ModelBuilding/OneToManyTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/OneToManyTestBase.cs @@ -2692,7 +2692,7 @@ public virtual void Navigation_to_shared_type_is_not_discovered_by_convention() Assert.Equal( CoreStrings.NonConfiguredNavigationToSharedType("Navigation", nameof(CollectionNavigationToSharedType)), - Assert.Throws(() => modelBuilder.FinalizeModel()).Message); + Assert.Throws(modelBuilder.FinalizeModel).Message); } [ConditionalFact] @@ -2723,6 +2723,38 @@ public virtual void WithMany_pointing_to_keyless_entity_throws() .HasOne(e => e.Reference).WithMany(e => e.Collection)).Message); } + [ConditionalFact] + public virtual void HasNoKey_call_on_principal_entity_throws() + { + var modelBuilder = CreateModelBuilder(); + modelBuilder.Entity().HasMany(e => e.Stores).WithOne(); + Assert.Equal( + CoreStrings.PrincipalKeylessType( + nameof(KeylessCollectionNavigation), + nameof(KeylessCollectionNavigation) + "." + nameof(KeylessCollectionNavigation.Stores), + nameof(Store)), + Assert.Throws( + () => modelBuilder.Entity().HasNoKey()).Message); + } + + [ConditionalFact] + public virtual void HasNoKey_call_on_principal_with_navigation_throws() + { + var modelBuilder = CreateModelBuilder(); + modelBuilder.Entity(); + modelBuilder.Entity() + .HasOne(e => e.Reference) + .WithMany(e => e.Collection); + + Assert.Equal( + CoreStrings.NavigationToKeylessType( + nameof(KeylessReferenceNavigation.Collection), + nameof(KeylessCollectionNavigation)), + Assert.Throws( + () => modelBuilder.Entity().HasNoKey()).Message); + } + + [ConditionalFact] public virtual void Reference_navigation_from_keyless_entity_type_works() { var modelBuilder = CreateModelBuilder(); diff --git a/test/EFCore.Tests/ModelBuilding/OneToOneTestBase.cs b/test/EFCore.Tests/ModelBuilding/OneToOneTestBase.cs index 932af4558e9..3a096fed7c5 100644 --- a/test/EFCore.Tests/ModelBuilding/OneToOneTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/OneToOneTestBase.cs @@ -1787,6 +1787,73 @@ public virtual void Throws_if_not_principal_or_dependent_specified() Assert.Throws(() => relationship.HasPrincipalKey(e => e.OrderId)).Message); } + [ConditionalFact] + public virtual void Configuring_principal_type_as_keyless_inverts_the_relationship() + { + var modelBuilder = CreateModelBuilder(); + modelBuilder.Entity(); + modelBuilder.Entity() + .Ignore(e => e.Order) + .HasOne().WithOne(e => e.Details); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + + var orderEntityType = (IEntityType)modelBuilder.Model.FindEntityType(typeof(Order)); + Assert.False(orderEntityType.FindNavigation(nameof(Order.Details)).IsOnDependent); + + modelBuilder.Entity().HasNoKey(); + + var model = modelBuilder.FinalizeModel(); + + orderEntityType = model.FindEntityType(typeof(Order))!; + Assert.True(orderEntityType.FindNavigation(nameof(Order.Details)).IsOnDependent); + } + + [ConditionalFact] + public virtual void Configuring_principal_type_as_keyless_throws_if_not_invertible() + { + var modelBuilder = CreateModelBuilder(); + modelBuilder.Entity(); + modelBuilder.Entity() + .Ignore(e => e.Order) + .HasOne().WithOne(e => e.Details) + .HasPrincipalKey(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + + var orderEntityType = (IEntityType)modelBuilder.Model.FindEntityType(typeof(Order)); + Assert.False(orderEntityType.FindNavigation(nameof(Order.Details)).IsOnDependent); + + Assert.Equal( + CoreStrings.PrincipalKeylessType( + nameof(Order), + nameof(Order) + "." + nameof(Order.Details), + nameof(OrderDetails)), + Assert.Throws( + () => modelBuilder.Entity().HasNoKey()).Message); + } + + [ConditionalFact] + public virtual void Configuring_principal_type_as_keyless_throws_when_there_is_an_explicit_navigation() + { + var modelBuilder = CreateModelBuilder(); + modelBuilder.Entity(); + modelBuilder.Entity() + .HasOne(e => e.Order).WithOne(e => e.Details); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + + var orderEntityType = (IEntityType)modelBuilder.Model.FindEntityType(typeof(Order)); + Assert.False(orderEntityType.FindNavigation(nameof(Order.Details)).IsOnDependent); + + Assert.Equal( + CoreStrings.NavigationToKeylessType( + nameof(OrderDetails.Order), + nameof(Order)), + Assert.Throws( + () => modelBuilder.Entity().HasNoKey()).Message); + } + [ConditionalFact] public virtual void Creates_principal_key_when_specified_on_principal_with_navigation_to_dependent() {