diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs index d4889526638..df8b45a3fbd 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs @@ -826,7 +826,7 @@ private bool TryRewriteEntityEquality(ExpressionType nodeType, Expression left, var rightEntityType = rightEntityReference?.EntityType; var entityType = leftEntityType ?? rightEntityType; - Debug.Assert(entityType != null, "At least either side should be entityReference so entityType should be non-null."); + Check.DebugAssert(entityType != null, "At least either side should be entityReference so entityType should be non-null."); if (leftEntityType != null && rightEntityType != null diff --git a/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs b/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs index 667604a0678..f17cf2526b0 100644 --- a/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs +++ b/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs @@ -158,7 +158,7 @@ protected virtual void GenerateEntityTypeDataAnnotations(IEntityType entityType) { attributeWriter.AddParameter(_code.UnknownLiteral(argument)); } - + _sb.AppendLine(attributeWriter.ToString()); } } @@ -318,7 +318,7 @@ protected virtual void GeneratePropertyDataAnnotations(IProperty property) { attributeWriter.AddParameter(_code.UnknownLiteral(argument)); } - + _sb.AppendLine(attributeWriter.ToString()); } } diff --git a/src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs b/src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs index 7df44c10cc3..354597b18be 100644 --- a/src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs +++ b/src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs @@ -1408,7 +1408,7 @@ private bool TryRewriteEntityEquality( var rightEntityType = rightEntityReference?.EntityType; var entityType = leftEntityType ?? rightEntityType; - Debug.Assert(entityType != null, "At least either side should be entityReference so entityType should be non-null."); + Check.DebugAssert(entityType != null, "At least either side should be entityReference so entityType should be non-null."); if (leftEntityType != null && rightEntityType != null diff --git a/src/EFCore.Relational/Extensions/RelationalEntityTypeBuilderExtensions.cs b/src/EFCore.Relational/Extensions/RelationalEntityTypeBuilderExtensions.cs index 3e724ca0651..e83a1c9d492 100644 --- a/src/EFCore.Relational/Extensions/RelationalEntityTypeBuilderExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalEntityTypeBuilderExtensions.cs @@ -2,10 +2,8 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Linq; -using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Metadata.Builders; using Microsoft.EntityFrameworkCore.Metadata.Internal; diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs index c8b7ff2084a..f20733799bc 100644 --- a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs +++ b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Metadata; @@ -733,22 +732,9 @@ protected virtual void ValidateSharedColumnsCompatibility( { foreach (var missingColumn in missingConcurrencyTokens) { - var columnFound = false; - foreach (var property in entityType.GetAllBaseTypesAscending().SelectMany(t => t.GetDeclaredProperties())) - { - if (property.GetColumnName(storeObject) == missingColumn) - { - columnFound = true; - break; - } - } - - if (!columnFound) - { - throw new InvalidOperationException( - RelationalStrings.MissingConcurrencyColumn( - entityType.DisplayName(), missingColumn, storeObject.DisplayName())); - } + throw new InvalidOperationException( + RelationalStrings.MissingConcurrencyColumn( + entityType.DisplayName(), missingColumn, storeObject.DisplayName())); } } } @@ -1477,8 +1463,8 @@ protected virtual void ValidateIndexProperties( } else if (overlappingTables.Count == 0) { - Debug.Assert(firstPropertyTables != null, nameof(firstPropertyTables)); - Debug.Assert(lastPropertyTables != null, nameof(lastPropertyTables)); + Check.DebugAssert(firstPropertyTables != null, nameof(firstPropertyTables)); + Check.DebugAssert(lastPropertyTables != null, nameof(lastPropertyTables)); logger.IndexPropertiesMappedToNonOverlappingTables( entityType, diff --git a/src/EFCore.Relational/Metadata/Conventions/TableSharingConcurrencyTokenConvention.cs b/src/EFCore.Relational/Metadata/Conventions/TableSharingConcurrencyTokenConvention.cs index fb4415cd807..cc714006bdb 100644 --- a/src/EFCore.Relational/Metadata/Conventions/TableSharingConcurrencyTokenConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/TableSharingConcurrencyTokenConvention.cs @@ -193,21 +193,22 @@ public static bool IsConcurrencyTokenMissing( IReadOnlyEntityType entityType, IReadOnlyList mappedTypes) { - if (entityType.FindPrimaryKey() == null) + if (entityType.FindPrimaryKey() == null + || propertiesMappedToConcurrencyColumn.Count == 0) { return false; } - var propertyMissing = false; + var propertyMissing = true; foreach (var mappedProperty in propertiesMappedToConcurrencyColumn) { var declaringEntityType = mappedProperty.DeclaringEntityType; if (declaringEntityType.IsAssignableFrom(entityType) - || entityType.IsAssignableFrom(declaringEntityType) || declaringEntityType.IsInOwnershipPath(entityType) || entityType.IsInOwnershipPath(declaringEntityType)) { - // The concurrency token is in the same hierarchy or in the same aggregate + // The concurrency token is on the base type or in the same aggregate + propertyMissing = false; continue; } @@ -222,11 +223,8 @@ public static bool IsConcurrencyTokenMissing( || entityType.IsAssignableFrom(fk.PrincipalEntityType))) { // The concurrency token is on a type that shares the row with a base or derived type - continue; + propertyMissing = false; } - - propertyMissing = true; - break; } return propertyMissing; diff --git a/src/EFCore.Relational/Metadata/Conventions/TableValuedDbFunctionConvention.cs b/src/EFCore.Relational/Metadata/Conventions/TableValuedDbFunctionConvention.cs index dcd421bf928..5ef1117b630 100644 --- a/src/EFCore.Relational/Metadata/Conventions/TableValuedDbFunctionConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/TableValuedDbFunctionConvention.cs @@ -70,26 +70,12 @@ private void ProcessDbFunctionAdded( var entityType = model.FindEntityType(elementType); if (entityType?.IsOwned() == true || model.IsOwned(elementType) - || (entityType == null && model.GetEntityTypes().Any(e => e.ClrType == elementType))) + || (entityType == null && model.FindEntityTypes(elementType).Any())) { return; } - IConventionEntityTypeBuilder? entityTypeBuilder; - if (entityType != null) - { - entityTypeBuilder = entityType.Builder; - } - else - { - entityTypeBuilder = dbFunctionBuilder.ModelBuilder.Entity(elementType); - if (entityTypeBuilder == null) - { - return; - } - - entityType = entityTypeBuilder.Metadata; - } + dbFunctionBuilder.ModelBuilder.Entity(elementType); } } } diff --git a/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs index 1b416ec281b..d41375d010f 100644 --- a/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs @@ -1358,7 +1358,7 @@ private bool TryRewriteEntityEquality( var rightEntityType = rightEntityReference?.EntityType; var entityType = leftEntityType ?? rightEntityType; - Debug.Assert(entityType != null, "At least one side should be entityReference so entityType should be non-null."); + Check.DebugAssert(entityType != null, "At least one side should be entityReference so entityType should be non-null."); if (leftEntityType != null && rightEntityType != null diff --git a/src/EFCore.Relational/Storage/RelationalConnection.cs b/src/EFCore.Relational/Storage/RelationalConnection.cs index dd8462f9e8b..df9caee5354 100644 --- a/src/EFCore.Relational/Storage/RelationalConnection.cs +++ b/src/EFCore.Relational/Storage/RelationalConnection.cs @@ -40,7 +40,8 @@ public abstract class RelationalConnection : IRelationalConnection, ITransaction private bool _connectionOwned; private int _openedCount; private bool _openedInternally; - private int? _commandTimeout, _defaultCommandTimeout; + private int? _commandTimeout; + private readonly int? _defaultCommandTimeout; private readonly ConcurrentStack _ambientTransactions = new(); private DbConnection? _connection; private readonly IRelationalCommandBuilder _relationalCommandBuilder; diff --git a/src/EFCore.SqlServer.NTS/Properties/SqlServerNTSStrings.Designer.cs b/src/EFCore.SqlServer.NTS/Properties/SqlServerNTSStrings.Designer.cs index 4737f29f930..411a5dee086 100644 --- a/src/EFCore.SqlServer.NTS/Properties/SqlServerNTSStrings.Designer.cs +++ b/src/EFCore.SqlServer.NTS/Properties/SqlServerNTSStrings.Designer.cs @@ -3,7 +3,6 @@ using System; using System.Reflection; using System.Resources; -using JetBrains.Annotations; #nullable enable diff --git a/src/EFCore.SqlServer/Extensions/SqlServerIndexBuilderExtensions.cs b/src/EFCore.SqlServer/Extensions/SqlServerIndexBuilderExtensions.cs index 4a1cf8e6b2c..8cb30ffd9aa 100644 --- a/src/EFCore.SqlServer/Extensions/SqlServerIndexBuilderExtensions.cs +++ b/src/EFCore.SqlServer/Extensions/SqlServerIndexBuilderExtensions.cs @@ -138,7 +138,7 @@ public static IndexBuilder IncludeProperties( /// A builder to further configure the index. public static IndexBuilder IncludeProperties( this IndexBuilder indexBuilder, - Expression> includeExpression) + Expression> includeExpression) { Check.NotNull(indexBuilder, nameof(indexBuilder)); Check.NotNull(includeExpression, nameof(includeExpression)); diff --git a/src/EFCore/ChangeTracking/EntityEntry.cs b/src/EFCore/ChangeTracking/EntityEntry.cs index 036936fdaa6..0f259d81451 100644 --- a/src/EFCore/ChangeTracking/EntityEntry.cs +++ b/src/EFCore/ChangeTracking/EntityEntry.cs @@ -325,7 +325,8 @@ public virtual PropertyValues CurrentValues /// /// /// Note that whenever real original property values are not available (e.g. entity was not yet - /// persisted to the database) this will default to the current property values of this entity. + /// persisted to the database or was retrieved in a non-tracking query) this will default to the + /// current property values of this entity. /// /// /// The original values. @@ -338,14 +339,14 @@ public virtual PropertyValues OriginalValues /// /// /// Queries the database for copies of the values of the tracked entity as they currently - /// exist in the database. If the entity is not found in the database, then null is returned. + /// exist in the database. If the entity is not found in the database, then is returned. /// /// /// Note that changing the values in the returned dictionary will not update the values /// in the database. /// /// - /// The store values, or null if the entity does not exist in the database. + /// The store values, or if the entity does not exist in the database. public virtual PropertyValues? GetDatabaseValues() { var values = Finder.GetDatabaseValues(InternalEntry); diff --git a/src/EFCore/Design/CSharpRuntimeAnnotationCodeGenerator.cs b/src/EFCore/Design/CSharpRuntimeAnnotationCodeGenerator.cs index 8edc6ce0eb7..f59fb7c3f03 100644 --- a/src/EFCore/Design/CSharpRuntimeAnnotationCodeGenerator.cs +++ b/src/EFCore/Design/CSharpRuntimeAnnotationCodeGenerator.cs @@ -43,7 +43,6 @@ public virtual void Generate(IModel model, CSharpRuntimeAnnotationCodeGeneratorP } else { - parameters.Annotations.Remove(CoreAnnotationNames.OwnedTypes); parameters.Annotations.Remove(CoreAnnotationNames.PropertyAccessMode); } diff --git a/src/EFCore/Diagnostics/CoreEventId.cs b/src/EFCore/Diagnostics/CoreEventId.cs index 845344c7760..1d80b221906 100644 --- a/src/EFCore/Diagnostics/CoreEventId.cs +++ b/src/EFCore/Diagnostics/CoreEventId.cs @@ -146,7 +146,7 @@ private static EventId MakeUpdateId(Id id) public static readonly EventId SaveChangesFailed = MakeUpdateId(Id.SaveChangesFailed); /// - /// The same entity is being tracked as a different weak entity type. + /// The same entity is being tracked as a different shared entity entity type. /// This event is in the category. /// public static readonly EventId DuplicateDependentEntityTypeInstanceWarning = diff --git a/src/EFCore/Infrastructure/IModelCacheKeyFactory.cs b/src/EFCore/Infrastructure/IModelCacheKeyFactory.cs index 0c5ad9e0a53..b6f100be0e4 100644 --- a/src/EFCore/Infrastructure/IModelCacheKeyFactory.cs +++ b/src/EFCore/Infrastructure/IModelCacheKeyFactory.cs @@ -31,7 +31,8 @@ public interface IModelCacheKeyFactory /// /// The created key. [Obsolete("Use the overload with most parameters")] - object Create(DbContext context); + object Create(DbContext context) + => Create(context, true); /// /// Gets the model cache key for a given context. diff --git a/src/EFCore/Infrastructure/ModelValidator.cs b/src/EFCore/Infrastructure/ModelValidator.cs index 8324b755e4e..c6781c0c670 100644 --- a/src/EFCore/Infrastructure/ModelValidator.cs +++ b/src/EFCore/Infrastructure/ModelValidator.cs @@ -141,7 +141,7 @@ protected virtual void ValidatePropertyMapping( { Check.NotNull(model, nameof(model)); - if (!(model is IConventionModel conventionModel)) + if (model is not IConventionModel conventionModel) { return; } @@ -167,10 +167,8 @@ protected virtual void ValidatePropertyMapping( continue; } - var clrProperties = new HashSet(StringComparer.Ordinal); - var runtimeProperties = entityType.GetRuntimeProperties(); - + var clrProperties = new HashSet(StringComparer.Ordinal); clrProperties.UnionWith( runtimeProperties.Values .Where(pi => pi.IsCandidateProperty(needsWrite: false)) @@ -181,6 +179,7 @@ protected virtual void ValidatePropertyMapping( .Concat(entityType.GetNavigations()) .Concat(entityType.GetSkipNavigations()) .Concat(entityType.GetServiceProperties()).Select(p => p.Name)); + if (entityType.IsPropertyBag) { clrProperties.ExceptWith(_dictionaryProperties); @@ -191,7 +190,6 @@ protected virtual void ValidatePropertyMapping( continue; } - var configuration = ((Model)entityType.Model).Configuration; foreach (var clrPropertyName in clrProperties) { if (entityType.FindIgnoredConfigurationSource(clrPropertyName) != null) @@ -212,31 +210,18 @@ protected virtual void ValidatePropertyMapping( continue; } - Dependencies.MemberClassifier.GetNavigationCandidates(entityType).TryGetValue(clrProperty, out var targetType); + var targetType = Dependencies.MemberClassifier.FindCandidateNavigationPropertyType( + clrProperty, conventionModel, out var targetOwned); if (targetType == null - || targetSequenceType == null) + && clrProperty.FindSetterProperty() == null) { - if (clrProperty.FindSetterProperty() == null) - { - continue; - } - - var sharedType = clrProperty.GetMemberType(); - if (conventionModel.IsShared(sharedType)) - { - targetType = sharedType; - } + continue; } - var isTargetSharedOrOwned = targetType != null - && (conventionModel.IsShared(targetType) - || conventionModel.IsOwned(targetType)); - - if (targetType?.IsValidEntityType() == true - && (isTargetSharedOrOwned - || conventionModel.FindEntityType(targetType) != null - || targetType.GetRuntimeProperties().Any(p => p.IsCandidateProperty()))) + if (targetType != null) { + var targetShared = conventionModel.IsShared(targetType); + targetOwned ??= conventionModel.IsOwned(targetType); // ReSharper disable CheckForReferenceEqualityInstead.1 // ReSharper disable CheckForReferenceEqualityInstead.3 if ((!entityType.IsKeyless @@ -244,21 +229,21 @@ protected virtual void ValidatePropertyMapping( && entityType.GetDerivedTypes().All( dt => dt.GetDeclaredNavigations().FirstOrDefault(n => n.Name == clrProperty.GetSimpleMemberName()) == null) - && (!isTargetSharedOrOwned + && (!(targetShared || targetOwned.Value) || (!targetType.Equals(entityType.ClrType) && (!entityType.IsInOwnershipPath(targetType) || (entityType.FindOwnership()!.PrincipalEntityType.ClrType.Equals(targetType) && targetSequenceType == null))))) { - if (conventionModel.IsOwned(entityType.ClrType) - && conventionModel.IsOwned(targetType)) + if (entityType.IsOwned() + && targetOwned.Value) { throw new InvalidOperationException( CoreStrings.AmbiguousOwnedNavigation( entityType.DisplayName() + "." + clrProperty.Name, targetType.ShortDisplayName())); } - if (model.IsShared(targetType)) + if (targetShared) { throw new InvalidOperationException( CoreStrings.NonConfiguredNavigationToSharedType(clrProperty.Name, entityType.DisplayName())); @@ -705,9 +690,10 @@ protected virtual void ValidateOwnership( if (ownerships.Count == 1) { + Check.DebugAssert(entityType.IsOwned(), $"Expected the entity type {entityType.DisplayName()} to be marked as owned"); + var ownership = ownerships[0]; - if (entityType.BaseType != null - && ownership.DeclaringEntityType == entityType) + if (entityType.BaseType != null) { throw new InvalidOperationException(CoreStrings.OwnedDerivedType(entityType.DisplayName())); } @@ -742,7 +728,8 @@ protected virtual void ValidateOwnership( ownership.PrincipalEntityType.DisplayName())); } } - else if (((IMutableModel)model).IsOwned(entityType.ClrType)) + else if (((IConventionModel)model).IsOwned(entityType.ClrType) + || entityType.IsOwned()) { throw new InvalidOperationException(CoreStrings.OwnerlessOwnedType(entityType.DisplayName())); } diff --git a/src/EFCore/Metadata/Builders/CollectionCollectionBuilder.cs b/src/EFCore/Metadata/Builders/CollectionCollectionBuilder.cs index db8ae49f5f5..8b127f11b07 100644 --- a/src/EFCore/Metadata/Builders/CollectionCollectionBuilder.cs +++ b/src/EFCore/Metadata/Builders/CollectionCollectionBuilder.cs @@ -138,7 +138,7 @@ public virtual EntityTypeBuilder UsingEntity( if (newJoinEntityType == null) { - newJoinEntityType = ModelBuilder.Entity(joinEntityType, ConfigurationSource.Explicit)!.Metadata; + newJoinEntityType = ModelBuilder.Entity(joinEntityType, ConfigurationSource.Explicit, shouldBeOwned: false)!.Metadata; } var entityTypeBuilder = new EntityTypeBuilder(newJoinEntityType); diff --git a/src/EFCore/Metadata/Builders/CollectionCollectionBuilder`.cs b/src/EFCore/Metadata/Builders/CollectionCollectionBuilder`.cs index efb65f336a7..a52448c712b 100644 --- a/src/EFCore/Metadata/Builders/CollectionCollectionBuilder`.cs +++ b/src/EFCore/Metadata/Builders/CollectionCollectionBuilder`.cs @@ -94,7 +94,7 @@ public virtual EntityTypeBuilder UsingEntity( if (joinEntityType == null) { - joinEntityType = ModelBuilder.Entity(typeof(TJoinEntity), ConfigurationSource.Explicit)!.Metadata; + joinEntityType = ModelBuilder.Entity(typeof(TJoinEntity), ConfigurationSource.Explicit, shouldBeOwned: false)!.Metadata; } var entityTypeBuilder = new EntityTypeBuilder(joinEntityType); diff --git a/src/EFCore/Metadata/Builders/DiscriminatorBuilder.cs b/src/EFCore/Metadata/Builders/DiscriminatorBuilder.cs index ce18ddb316e..176848ee17d 100644 --- a/src/EFCore/Metadata/Builders/DiscriminatorBuilder.cs +++ b/src/EFCore/Metadata/Builders/DiscriminatorBuilder.cs @@ -92,7 +92,7 @@ public virtual DiscriminatorBuilder HasValue(object? value) public virtual DiscriminatorBuilder HasValue(Type entityType, object? value) { var entityTypeBuilder = EntityTypeBuilder.ModelBuilder.Entity( - entityType, ConfigurationSource.Explicit, shouldBeOwned: null); + entityType, ConfigurationSource.Explicit); return HasValue(entityTypeBuilder, value, ConfigurationSource.Explicit)!; } @@ -106,7 +106,7 @@ public virtual DiscriminatorBuilder HasValue(Type entityType, object? value) public virtual DiscriminatorBuilder HasValue(string entityTypeName, object? value) { var entityTypeBuilder = EntityTypeBuilder.ModelBuilder.Entity( - entityTypeName, ConfigurationSource.Explicit, shouldBeOwned: null); + entityTypeName, ConfigurationSource.Explicit); return HasValue(entityTypeBuilder, value, ConfigurationSource.Explicit)!; } diff --git a/src/EFCore/Metadata/Builders/EntityTypeBuilder.cs b/src/EFCore/Metadata/Builders/EntityTypeBuilder.cs index 6e730e19cdd..cee5f68ca24 100644 --- a/src/EFCore/Metadata/Builders/EntityTypeBuilder.cs +++ b/src/EFCore/Metadata/Builders/EntityTypeBuilder.cs @@ -838,13 +838,13 @@ protected virtual ForeignKey HasOneBuilder( { foreignKey = Builder.HasRelationship( relatedEntityType, navigationId.MemberInfo, ConfigurationSource.Explicit, - targetIsPrincipal: Builder.Metadata == relatedEntityType ? true : (bool?)null)!.Metadata; + targetIsPrincipal: Builder.Metadata == relatedEntityType ? true : null)!.Metadata; } else { foreignKey = Builder.HasRelationship( relatedEntityType, navigationId.Name, ConfigurationSource.Explicit, - targetIsPrincipal: Builder.Metadata == relatedEntityType ? true : (bool?)null)!.Metadata; + targetIsPrincipal: Builder.Metadata == relatedEntityType ? true : null)!.Metadata; } return foreignKey; @@ -991,7 +991,7 @@ protected virtual EntityType FindRelatedEntityType(string relatedTypeName, strin => (navigationName == null ? null : Builder.ModelBuilder.Metadata.FindEntityType(relatedTypeName, navigationName, Builder.Metadata)) - ?? Builder.ModelBuilder.Entity(relatedTypeName, ConfigurationSource.Explicit)!.Metadata; + ?? Builder.ModelBuilder.Entity(relatedTypeName, ConfigurationSource.Explicit, shouldBeOwned: false)!.Metadata; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -1004,7 +1004,7 @@ protected virtual EntityType FindRelatedEntityType(Type relatedType, string? nav => (navigationName == null || !Builder.ModelBuilder.Metadata.IsShared(relatedType) ? null : Builder.ModelBuilder.Metadata.FindEntityType(relatedType, navigationName, Builder.Metadata)) - ?? Builder.ModelBuilder.Entity(relatedType, ConfigurationSource.Explicit)!.Metadata; + ?? Builder.ModelBuilder.Entity(relatedType, ConfigurationSource.Explicit, shouldBeOwned: false)!.Metadata; /// /// Configures the to be used for this entity type. diff --git a/src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs b/src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs index cf13faf79f4..836e103df44 100644 --- a/src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs +++ b/src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs @@ -550,6 +550,18 @@ IConventionEntityTypeBuilder RemoveUnusedShadowProperties( string navigationName, bool fromDataAnnotation = false); + /// + /// Configures a relationship where the target entity is owned by (or part of) this entity. + /// + /// The entity type that this relationship targets. + /// The name of the navigation property on this entity type that is part of the relationship. + /// Indicates whether the configuration was specified using a data annotation. + /// An object that can be used to configure the relationship. + IConventionForeignKeyBuilder? HasOwnership( + IConventionEntityType targetEntityType, + string navigationName, + bool fromDataAnnotation = false); + /// /// Configures a relationship where the target entity is owned by (or part of) this entity. /// @@ -565,6 +577,21 @@ IConventionEntityTypeBuilder RemoveUnusedShadowProperties( MemberInfo navigation, bool fromDataAnnotation = false); + /// + /// Configures a relationship where the target entity is owned by (or part of) this entity. + /// + /// The entity type that this relationship targets. + /// The navigation property on this entity type that is part of the relationship. + /// Indicates whether the configuration was specified using a data annotation. + /// + /// An object that can be used to configure the relationship if it exists on the entity type, + /// otherwise. + /// + IConventionForeignKeyBuilder? HasOwnership( + IConventionEntityType targetEntityType, + MemberInfo navigation, + bool fromDataAnnotation = false); + /// /// Configures a relationship where the target entity is owned by (or part of) this entity. /// @@ -585,6 +612,26 @@ IConventionEntityTypeBuilder RemoveUnusedShadowProperties( string? inverseNavigationName, bool fromDataAnnotation = false); + /// + /// Configures a relationship where the target entity is owned by (or part of) this entity. + /// + /// The entity type that this relationship targets. + /// The name of the navigation property on this entity type that is part of the relationship. + /// + /// The name of the navigation property on the target entity type that is part of the relationship. If + /// is specified, the relationship will be configured without a navigation property on the target end. + /// + /// Indicates whether the configuration was specified using a data annotation. + /// + /// An object that can be used to configure the relationship if it exists on the entity type, + /// otherwise. + /// + IConventionForeignKeyBuilder? HasOwnership( + IConventionEntityType targetEntityType, + string navigationName, + string? inverseNavigationName, + bool fromDataAnnotation = false); + /// /// Configures a relationship where the target entity is owned by (or part of) this entity. /// @@ -605,6 +652,26 @@ IConventionEntityTypeBuilder RemoveUnusedShadowProperties( MemberInfo? inverseNavigation, bool fromDataAnnotation = false); + /// + /// Configures a relationship where the target entity is owned by (or part of) this entity. + /// + /// The entity type that this relationship targets. + /// The navigation property on this entity type that is part of the relationship. + /// + /// The navigation property on the target entity type that is part of the relationship. If + /// is specified, the relationship will be configured without a navigation property on the target end. + /// + /// Indicates whether the configuration was specified using a data annotation. + /// + /// An object that can be used to configure the relationship if it exists on the entity type, + /// otherwise. + /// + IConventionForeignKeyBuilder? HasOwnership( + IConventionEntityType targetEntityType, + MemberInfo navigation, + MemberInfo? inverseNavigation, + bool fromDataAnnotation = false); + /// /// Removes a relationship from this entity type. /// @@ -654,7 +721,7 @@ IConventionEntityTypeBuilder RemoveUnusedShadowProperties( /// The same builder instance if the skip navigation was removed, /// otherwise. /// - IConventionEntityTypeBuilder? HasNoSkipNavigation(IReadOnlySkipNavigation skipNavigation, bool fromDataAnnotation = false); + IConventionEntityTypeBuilder? HasNoSkipNavigation(IConventionSkipNavigation skipNavigation, bool fromDataAnnotation = false); /// /// Returns a value indicating whether the skip navigation can be removed from this entity type. @@ -662,7 +729,7 @@ IConventionEntityTypeBuilder RemoveUnusedShadowProperties( /// The skip navigation to be removed. /// Indicates whether the configuration was specified using a data annotation. /// if the skip navigation can be removed from this entity type. - bool CanRemoveSkipNavigation(IReadOnlySkipNavigation skipNavigation, bool fromDataAnnotation = false); + bool CanRemoveSkipNavigation(IConventionSkipNavigation skipNavigation, bool fromDataAnnotation = false); /// /// Returns a value indicating whether the given navigation can be added to this entity type. diff --git a/src/EFCore/Metadata/Builders/OwnedNavigationBuilder.cs b/src/EFCore/Metadata/Builders/OwnedNavigationBuilder.cs index 6a9547099ab..f67ab7e4a3a 100644 --- a/src/EFCore/Metadata/Builders/OwnedNavigationBuilder.cs +++ b/src/EFCore/Metadata/Builders/OwnedNavigationBuilder.cs @@ -794,7 +794,7 @@ public virtual ReferenceNavigationBuilder HasOne( navigationName, DependentEntityType.Builder.HasRelationship( relatedEntityType, navigationName, ConfigurationSource.Explicit, - targetIsPrincipal: DependentEntityType == relatedEntityType ? true : (bool?)null)!.Metadata); + targetIsPrincipal: DependentEntityType == relatedEntityType ? true : null)!.Metadata); } /// @@ -868,7 +868,7 @@ public virtual ReferenceNavigationBuilder HasOne( navigationName, DependentEntityType.Builder.HasRelationship( relatedEntityType, navigationName, ConfigurationSource.Explicit, - targetIsPrincipal: DependentEntityType == relatedEntityType ? true : (bool?)null)!.Metadata); + targetIsPrincipal: DependentEntityType == relatedEntityType ? true : null)!.Metadata); } /// @@ -930,7 +930,8 @@ protected virtual EntityType FindRelatedEntityType(Type relatedType, string? nav } } - return relatedEntityType ?? DependentEntityType.Builder.ModelBuilder.Entity(relatedType, ConfigurationSource.Explicit)!.Metadata; + return relatedEntityType ?? DependentEntityType.Builder.ModelBuilder.Entity( + relatedType, ConfigurationSource.Explicit, shouldBeOwned: false)!.Metadata; } /// diff --git a/src/EFCore/Metadata/Builders/OwnedNavigationBuilder`.cs b/src/EFCore/Metadata/Builders/OwnedNavigationBuilder`.cs index 2671f652626..9923162272b 100644 --- a/src/EFCore/Metadata/Builders/OwnedNavigationBuilder`.cs +++ b/src/EFCore/Metadata/Builders/OwnedNavigationBuilder`.cs @@ -1081,7 +1081,7 @@ public virtual ReferenceNavigationBuilder H navigationName, DependentEntityType.Builder.HasRelationship( relatedEntityType, navigationName, 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 4476c6317a5..2e1e0bd328a 100644 --- a/src/EFCore/Metadata/Conventions/BaseTypeDiscoveryConvention.cs +++ b/src/EFCore/Metadata/Conventions/BaseTypeDiscoveryConvention.cs @@ -19,8 +19,7 @@ public class BaseTypeDiscoveryConvention : #pragma warning disable CS0612 // Type or member is obsolete InheritanceDiscoveryConventionBase, #pragma warning restore CS0612 // Type or member is obsolete - IEntityTypeAddedConvention, - IForeignKeyOwnershipChangedConvention + IEntityTypeAddedConvention { /// /// Creates a new instance of . @@ -37,15 +36,15 @@ public virtual void ProcessEntityTypeAdded( IConventionContext context) { var entityType = entityTypeBuilder.Metadata; - var clrType = entityType.ClrType; - if (clrType == null - || entityType.HasSharedClrType - || entityType.Model.IsOwned(clrType) - || entityType.FindDeclaredOwnership() != null) + if (entityType.HasSharedClrType + || entityType.IsOwned()) { return; } + Check.DebugAssert(entityType.GetDeclaredForeignKeys().FirstOrDefault(fk => fk.IsOwnership) == null, + "Ownerships present on non-owned entity type"); + var model = entityType.Model; var derivedTypesMap = (Dictionary>?)model[CoreAnnotationNames.DerivedTypes]; if (derivedTypesMap == null) @@ -54,12 +53,16 @@ public virtual void ProcessEntityTypeAdded( model.SetAnnotation(CoreAnnotationNames.DerivedTypes, derivedTypesMap); } + var clrType = entityType.ClrType; var baseType = clrType.BaseType!; if (derivedTypesMap.TryGetValue(clrType, out var derivedTypes)) { foreach (var derivedType in derivedTypes) { - derivedType.Builder.HasBaseType(entityType); + if (!derivedType.IsOwned()) + { + derivedType.Builder.HasBaseType(entityType); + } var otherBaseType = baseType; while (otherBaseType != typeof(object)) @@ -101,7 +104,7 @@ public virtual void ProcessEntityTypeAdded( } if (!baseEntityType.HasSharedClrType - && baseEntityType.FindOwnership() == null) + && !baseEntityType.IsOwned()) { if (entityTypeBuilder.HasBaseType(baseEntityType) is IConventionEntityTypeBuilder newEntityTypeBuilder) { @@ -109,21 +112,5 @@ public virtual void ProcessEntityTypeAdded( } } } - - /// - public virtual void ProcessForeignKeyOwnershipChanged( - IConventionForeignKeyBuilder relationshipBuilder, - IConventionContext context) - { - var foreignKey = relationshipBuilder.Metadata; - if (foreignKey.IsOwnership - && foreignKey.DeclaringEntityType.GetDirectlyDerivedTypes().Any()) - { - foreach (var derivedType in foreignKey.DeclaringEntityType.GetDirectlyDerivedTypes().ToList()) - { - derivedType.Builder.HasBaseType(null); - } - } - } } } diff --git a/src/EFCore/Metadata/Conventions/DerivedTypeDiscoveryConvention.cs b/src/EFCore/Metadata/Conventions/DerivedTypeDiscoveryConvention.cs index f21e4dbd216..585bdb39704 100644 --- a/src/EFCore/Metadata/Conventions/DerivedTypeDiscoveryConvention.cs +++ b/src/EFCore/Metadata/Conventions/DerivedTypeDiscoveryConvention.cs @@ -34,23 +34,21 @@ public virtual void ProcessEntityTypeAdded( IConventionContext context) { var entityType = entityTypeBuilder.Metadata; - var clrType = entityType.ClrType; - if (clrType == null - || entityType.HasSharedClrType + if (entityType.HasSharedClrType || entityType.HasDefiningNavigation() - || entityType.Model.IsOwned(clrType) - || entityType.FindOwnership() != null) + || entityType.IsOwned()) { return; } + var clrType = entityType.ClrType; var model = entityType.Model; foreach (var directlyDerivedType in model.GetEntityTypes()) { if (directlyDerivedType != entityType && !directlyDerivedType.HasSharedClrType && !directlyDerivedType.HasDefiningNavigation() - && !model.IsOwned(directlyDerivedType.ClrType) + && !directlyDerivedType.IsOwned() && directlyDerivedType.FindDeclaredOwnership() == null && ((directlyDerivedType.BaseType == null && clrType.IsAssignableFrom(directlyDerivedType.ClrType)) || (directlyDerivedType.BaseType == entityType.BaseType && FindClosestBaseType(directlyDerivedType) == entityType))) diff --git a/src/EFCore/Metadata/Conventions/ForeignKeyAttributeConvention.cs b/src/EFCore/Metadata/Conventions/ForeignKeyAttributeConvention.cs index 541c8258b82..bc7ded65cf3 100644 --- a/src/EFCore/Metadata/Conventions/ForeignKeyAttributeConvention.cs +++ b/src/EFCore/Metadata/Conventions/ForeignKeyAttributeConvention.cs @@ -208,7 +208,8 @@ var fkPropertiesOnDependentToPrincipal ?? new List { fkPropertyOnDependent!.GetSimpleMemberName() }; } - if (fkPropertyOnDependent != null) + if (fkPropertyOnDependent != null + || fkPropertiesOnDependentToPrincipal != null) { upgradeDependentToPrincipalNavigationSource = true; } @@ -260,7 +261,7 @@ var fkPropertiesOnDependentToPrincipal } } - return newRelationshipBuilder?.HasForeignKey(fkPropertiesToSet, fromDataAnnotation: true); + return newRelationshipBuilder.HasForeignKey(fkPropertiesToSet, fromDataAnnotation: true); } private static IConventionForeignKeyBuilder? SplitNavigationsToSeparateRelationships( diff --git a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs index 7eb7d42e967..d1ed1a7562a 100644 --- a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs +++ b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs @@ -61,7 +61,6 @@ public virtual ConventionSet CreateConventionSet() var relationshipDiscoveryConvention = new RelationshipDiscoveryConvention(Dependencies); var servicePropertyDiscoveryConvention = new ServicePropertyDiscoveryConvention(Dependencies); var indexAttributeConvention = new IndexAttributeConvention(Dependencies); - var baseTypeDiscoveryConvention = new BaseTypeDiscoveryConvention(Dependencies); conventionSet.EntityTypeAddedConventions.Add(new NotMappedEntityTypeAttributeConvention(Dependencies)); conventionSet.EntityTypeAddedConventions.Add(new OwnedEntityTypeAttributeConvention(Dependencies)); @@ -166,7 +165,6 @@ public virtual ConventionSet CreateConventionSet() conventionSet.ForeignKeyRequirednessChangedConventions.Add(foreignKeyPropertyDiscoveryConvention); conventionSet.ForeignKeyOwnershipChangedConventions.Add(new NavigationEagerLoadingConvention(Dependencies)); - conventionSet.ForeignKeyOwnershipChangedConventions.Add(baseTypeDiscoveryConvention); conventionSet.ForeignKeyOwnershipChangedConventions.Add(keyDiscoveryConvention); conventionSet.ForeignKeyOwnershipChangedConventions.Add(relationshipDiscoveryConvention); conventionSet.ForeignKeyOwnershipChangedConventions.Add(valueGeneratorConvention); diff --git a/src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs b/src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs index 78a5be4ae80..cd8e2680236 100644 --- a/src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs +++ b/src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs @@ -59,9 +59,7 @@ private void Process( return; } - var targetEntityTypeBuilder = ((InternalEntityTypeBuilder)entityTypeBuilder).GetTargetEntityTypeBuilder( - targetClrType, navigationMemberInfo, ConfigurationSource.DataAnnotation); - + var targetEntityTypeBuilder = TryGetTargetEntityTypeBuilder(entityTypeBuilder, targetClrType, navigationMemberInfo); if (targetEntityTypeBuilder == null) { return; @@ -77,13 +75,14 @@ private void Process( InversePropertyAttribute attribute) { var entityType = entityTypeBuilder.Metadata; - var targetClrType = targetEntityTypeBuilder.Metadata.ClrType; - var inverseNavigationPropertyInfo = targetEntityTypeBuilder.Metadata.GetRuntimeProperties().Values + var targetEntityType = targetEntityTypeBuilder.Metadata; + var targetClrType = targetEntityType.ClrType; + var inverseNavigationPropertyInfo = targetEntityType.GetRuntimeProperties().Values .FirstOrDefault(p => string.Equals(p.GetSimpleMemberName(), attribute.Property, StringComparison.OrdinalIgnoreCase)); if (inverseNavigationPropertyInfo == null - || !Dependencies.MemberClassifier.GetNavigationCandidates(targetEntityTypeBuilder.Metadata)[inverseNavigationPropertyInfo] - .IsAssignableFrom(entityType.ClrType)) + || !Dependencies.MemberClassifier.GetNavigationCandidates(targetEntityType)[inverseNavigationPropertyInfo] + .Type.IsAssignableFrom(entityType.ClrType)) { throw new InvalidOperationException( CoreStrings.InvalidNavigationWithInverseProperty( @@ -109,28 +108,103 @@ private void Process( navigationMemberInfo.Name, entityType.DisplayName(), inverseNavigationPropertyInfo.Name, - targetEntityTypeBuilder.Metadata.DisplayName())); + targetEntityType.DisplayName())); } } var referencingNavigationsWithAttribute = - AddInverseNavigation(entityType, navigationMemberInfo, targetEntityTypeBuilder.Metadata, inverseNavigationPropertyInfo); + AddInverseNavigation(entityType, navigationMemberInfo, targetEntityType, inverseNavigationPropertyInfo); + + if (TryRemoveIfAmbiguous( + entityType, + navigationMemberInfo, + targetEntityType, + targetEntityType.BaseType, + inverseNavigationPropertyInfo, + referencingNavigationsWithAttribute, out var conventionForeignKeyBuilder)) + { + return conventionForeignKeyBuilder; + } + + var ownership = entityType.FindOwnership(); + if (ownership != null + && ownership.PrincipalEntityType == targetEntityType + && ownership.PrincipalToDependent?.GetIdentifyingMemberInfo() != inverseNavigationPropertyInfo) + { + Dependencies.Logger.NonOwnershipInverseNavigationWarning( + entityType, navigationMemberInfo, + targetEntityType, inverseNavigationPropertyInfo, + ownership.PrincipalToDependent?.GetIdentifyingMemberInfo()!); + + return null; + } + + if (entityType.IsOwned() + && !entityType.IsInOwnershipPath(targetEntityType.ClrType)) + { + if (navigationMemberInfo.DeclaringType != entityType.ClrType + && (entityType.Model.FindEntityTypes(navigationMemberInfo.DeclaringType!).Any() + || (navigationMemberInfo.DeclaringType != entityType.ClrType.BaseType + && entityType.Model.FindEntityTypes(entityType.ClrType.BaseType!).Any()))) + { + return null; + } + + return targetEntityTypeBuilder.HasOwnership( + entityTypeBuilder.Metadata, + inverseNavigationPropertyInfo, + navigationMemberInfo, + fromDataAnnotation: true); + } + + var newForeignKeyBuilder = targetEntityTypeBuilder.HasRelationship( + entityType, + inverseNavigationPropertyInfo, + navigationMemberInfo, + fromDataAnnotation: true); + if (newForeignKeyBuilder == null + && navigationMemberInfo is PropertyInfo navigationPropertyInfo) + { + var navigationTargetType = navigationPropertyInfo.PropertyType.TryGetSequenceType(); + var inverseNavigationTargetType = inverseNavigationPropertyInfo.PropertyType.TryGetSequenceType(); + if (navigationTargetType != null + && inverseNavigationTargetType != null + && navigationTargetType.IsAssignableFrom(targetClrType) + && inverseNavigationTargetType.IsAssignableFrom(entityType.ClrType)) + { + entityTypeBuilder.HasSkipNavigation( + navigationPropertyInfo, targetEntityType, + inverseNavigationPropertyInfo, collections: true, onDependent: false, fromDataAnnotation: true); + } + } + + return newForeignKeyBuilder; + } + + private static bool TryRemoveIfAmbiguous( + IConventionEntityType entityType, + MemberInfo navigationMemberInfo, + IConventionEntityType targetEntityType, + IConventionEntityType? targetBaseType, + MemberInfo inverseNavigationMemberInfo, + List<(MemberInfo, IConventionEntityType)> referencingNavigationsWithAttribute, + out IConventionForeignKeyBuilder? configureInverseNavigation) + { var ambiguousInverse = FindAmbiguousInverse( navigationMemberInfo, entityType, referencingNavigationsWithAttribute); - var baseType = targetEntityTypeBuilder.Metadata.BaseType; while (ambiguousInverse == null - && baseType != null) + && targetBaseType != null) { - var navigationMap = GetInverseNavigations(baseType); + var navigationMap = GetInverseNavigations(targetBaseType); if (navigationMap != null - && navigationMap.TryGetValue(inverseNavigationPropertyInfo.Name, out var inverseTuple)) + && navigationMap.TryGetValue(inverseNavigationMemberInfo.Name, out var inverseTuple)) { referencingNavigationsWithAttribute = inverseTuple.References; ambiguousInverse = FindAmbiguousInverse(navigationMemberInfo, entityType, referencingNavigationsWithAttribute); } - baseType = baseType.BaseType; + targetBaseType = targetBaseType.BaseType; } if (ambiguousInverse != null) @@ -138,7 +212,7 @@ private void Process( if (entityType.FindSkipNavigation(navigationMemberInfo) is IConventionSkipNavigation existingSkipNavigation) { var existingSkipNavigationInverse = existingSkipNavigation.Inverse; - var inverseSkipNavigation = targetEntityTypeBuilder.Metadata.FindSkipNavigation(inverseNavigationPropertyInfo); + var inverseSkipNavigation = targetEntityType.FindSkipNavigation(inverseNavigationMemberInfo); var existingInverse = inverseSkipNavigation?.Inverse; var existingInverseType = existingInverse?.DeclaringEntityType; if (existingInverse != null @@ -169,11 +243,14 @@ private void Process( existingAmbiguousNavigation, fromDataAnnotation: true); } - return entityType.FindSkipNavigation(navigationMemberInfo)?.ForeignKey!.Builder; + { + configureInverseNavigation = entityType.FindSkipNavigation(navigationMemberInfo)?.ForeignKey!.Builder; + return true; + } } else { - var existingInverse = targetEntityTypeBuilder.Metadata.FindNavigation(inverseNavigationPropertyInfo)?.Inverse; + var existingInverse = targetEntityType.FindNavigation(inverseNavigationMemberInfo)?.Inverse; var existingInverseType = existingInverse?.DeclaringEntityType; if (existingInverse != null && IsAmbiguousInverse( @@ -219,70 +296,21 @@ private void Process( } } - return entityType.FindNavigation(navigationMemberInfo)?.ForeignKey.Builder; - } - } - - var ownership = entityType.FindOwnership(); - if (ownership != null - && ownership.PrincipalEntityType == targetEntityTypeBuilder.Metadata - && ownership.PrincipalToDependent?.GetIdentifyingMemberInfo() != inverseNavigationPropertyInfo) - { - Dependencies.Logger.NonOwnershipInverseNavigationWarning( - entityType, navigationMemberInfo, - targetEntityTypeBuilder.Metadata, inverseNavigationPropertyInfo, - ownership.PrincipalToDependent?.GetIdentifyingMemberInfo()!); - - return null; - } - - if (entityType.Model.IsOwned(entityType.ClrType) - && !entityType.IsInOwnershipPath(targetEntityTypeBuilder.Metadata)) - { - if (navigationMemberInfo.DeclaringType != entityType.ClrType - && (entityType.Model.FindEntityTypes(navigationMemberInfo.DeclaringType!).Any() - || (navigationMemberInfo.DeclaringType != entityType.ClrType.BaseType - && entityType.Model.FindEntityTypes(entityType.ClrType.BaseType!).Any()))) - { - return null; - } - - return targetEntityTypeBuilder.HasOwnership( - entityTypeBuilder.Metadata.ClrType!, - inverseNavigationPropertyInfo, - navigationMemberInfo, - fromDataAnnotation: true); - } - - var newForeignKeyBuilder = targetEntityTypeBuilder.HasRelationship( - entityType, - inverseNavigationPropertyInfo, - navigationMemberInfo, - fromDataAnnotation: true); - - if (newForeignKeyBuilder == null - && navigationMemberInfo is PropertyInfo navigationPropertyInfo) - { - var navigationTargetType = navigationPropertyInfo.PropertyType.TryGetSequenceType(); - var inverseNavigationTargetType = inverseNavigationPropertyInfo.PropertyType.TryGetSequenceType(); - if (navigationTargetType != null - && inverseNavigationTargetType != null - && navigationTargetType.IsAssignableFrom(targetClrType) - && inverseNavigationTargetType.IsAssignableFrom(entityType.ClrType)) - { - entityTypeBuilder.HasSkipNavigation( - navigationPropertyInfo, targetEntityTypeBuilder.Metadata, - inverseNavigationPropertyInfo, collections: true, onDependent: false, fromDataAnnotation: true); + { + configureInverseNavigation = entityType.FindNavigation(navigationMemberInfo)?.ForeignKey.Builder; + return true; + } } } - return newForeignKeyBuilder; + configureInverseNavigation = null; + return false; } /// public override void ProcessEntityTypeRemoved( IConventionModelBuilder modelBuilder, - Type type, + IConventionEntityType entityType, MemberInfo navigationMemberInfo, Type targetClrType, InversePropertyAttribute attribute, @@ -291,18 +319,20 @@ public override void ProcessEntityTypeRemoved( var targetEntityType = modelBuilder.Metadata.FindEntityType(targetClrType); if (targetEntityType != null) { - RemoveInverseNavigation(type, navigationMemberInfo, targetEntityType, attribute.Property); + RemoveInverseNavigation(entityType.ClrType, navigationMemberInfo, targetEntityType, attribute.Property); } var declaringType = navigationMemberInfo.DeclaringType; Check.DebugAssert(declaringType != null, "declaringType is null"); - if (modelBuilder.Metadata.FindEntityType(declaringType) != null) + if (modelBuilder.Metadata.FindEntityType(declaringType) != null + || entityType.HasSharedClrType) { return; } var navigationName = navigationMemberInfo.GetSimpleMemberName(); - var leastDerivedEntityTypes = modelBuilder.Metadata.FindLeastDerivedEntityTypes(declaringType); + var leastDerivedEntityTypes = modelBuilder.Metadata.FindLeastDerivedEntityTypes( + declaringType, t => !t.HasSharedClrType); foreach (var leastDerivedEntityType in leastDerivedEntityTypes) { if (leastDerivedEntityType.Builder.IsIgnored(navigationName, fromDataAnnotation: true)) @@ -377,6 +407,52 @@ public override void ProcessEntityTypeBaseTypeChanged( } } + /// + public override void ProcessEntityTypeBaseTypeChanged( + IConventionEntityTypeBuilder entityTypeBuilder, + IConventionEntityType? newBaseType, + IConventionEntityType? oldBaseType, + IConventionContext context) + { + base.ProcessEntityTypeBaseTypeChanged( + entityTypeBuilder, + newBaseType, + oldBaseType, + context); + + if (newBaseType == null) + { + return; + } + + foreach (var entityType in entityTypeBuilder.Metadata.GetDerivedTypesInclusive()) + { + var inverseNavigations = GetInverseNavigations(entityType); + if (inverseNavigations == null) + { + continue; + } + + foreach (var inverseNavigation in inverseNavigations.Values) + { + foreach (var referencingNavigationWithAttribute in inverseNavigation.References) + { + if (TryRemoveIfAmbiguous( + referencingNavigationWithAttribute.Item2, + referencingNavigationWithAttribute.Item1, + entityType, + newBaseType, + inverseNavigation.Navigation, + inverseNavigation.References, + out _)) + { + break; + } + } + } + } + } + /// public override void ProcessEntityTypeMemberIgnored( IConventionEntityTypeBuilder entityTypeBuilder, @@ -385,8 +461,8 @@ public override void ProcessEntityTypeMemberIgnored( InversePropertyAttribute attribute, IConventionContext context) { - var targetEntityType = ((InternalEntityTypeBuilder)entityTypeBuilder).GetTargetEntityTypeBuilder( - targetClrType, navigationMemberInfo, null)?.Metadata; + var targetEntityType = TryGetTargetEntityTypeBuilder(entityTypeBuilder, + targetClrType, navigationMemberInfo, shouldCreate: false)?.Metadata; if (targetEntityType == null) { return; @@ -621,6 +697,22 @@ private static void RemoveInverseNavigation( private static IConventionEntityType? FindActualEntityType(IConventionEntityType entityType) => ((Model)entityType.Model).FindActualEntityType((EntityType)entityType); + /// + /// 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 virtual IConventionEntityTypeBuilder? TryGetTargetEntityTypeBuilder( + IConventionEntityTypeBuilder entityTypeBuilder, + Type targetClrType, + MemberInfo navigationMemberInfo, + bool shouldCreate = true) + => ((InternalEntityTypeBuilder)entityTypeBuilder) + .GetTargetEntityTypeBuilder(targetClrType, navigationMemberInfo, shouldCreate ? ConfigurationSource.DataAnnotation : null); + private static Dictionary References)>? GetInverseNavigations( IConventionAnnotatable entityType) diff --git a/src/EFCore/Metadata/Conventions/ModelCleanupConvention.cs b/src/EFCore/Metadata/Conventions/ModelCleanupConvention.cs index 029575bc954..23c3e047e34 100644 --- a/src/EFCore/Metadata/Conventions/ModelCleanupConvention.cs +++ b/src/EFCore/Metadata/Conventions/ModelCleanupConvention.cs @@ -45,12 +45,10 @@ private void RemoveEntityTypesUnreachableByNavigations( { var model = modelBuilder.Metadata; var rootEntityTypes = GetRoots(model, ConfigurationSource.DataAnnotation); - using (context.DelayConventions()) + + foreach (var orphan in new GraphAdapter(model).GetUnreachableVertices(rootEntityTypes)) { - foreach (var orphan in new GraphAdapter(model).GetUnreachableVertices(rootEntityTypes)) - { - modelBuilder.HasNoEntityType(orphan, fromDataAnnotation: true); - } + modelBuilder.HasNoEntityType(orphan, fromDataAnnotation: true); } } diff --git a/src/EFCore/Metadata/Conventions/NavigationAttributeConventionBase.cs b/src/EFCore/Metadata/Conventions/NavigationAttributeConventionBase.cs index d8105873114..90fa0b8809b 100644 --- a/src/EFCore/Metadata/Conventions/NavigationAttributeConventionBase.cs +++ b/src/EFCore/Metadata/Conventions/NavigationAttributeConventionBase.cs @@ -134,7 +134,7 @@ public virtual void ProcessEntityTypeRemoved( var attributes = navigationPropertyInfo.GetCustomAttributes(inherit: true); foreach (var attribute in attributes) { - ProcessEntityTypeRemoved(modelBuilder, type, navigationPropertyInfo, targetClrType, attribute, context); + ProcessEntityTypeRemoved(modelBuilder, entityType, navigationPropertyInfo, targetClrType, attribute, context); if (((ConventionContext)context).ShouldStopProcessing()) { return; @@ -286,8 +286,7 @@ public virtual void ProcessEntityTypeMemberIgnored( private Type? FindCandidateNavigationWithAttributePropertyType(PropertyInfo propertyInfo, IConventionModel model) { - var targetClrType = Dependencies.MemberClassifier.FindCandidateNavigationPropertyType( - propertyInfo, ((Model)model).Configuration); + var targetClrType = Dependencies.MemberClassifier.FindCandidateNavigationPropertyType(propertyInfo, model, out var _); return targetClrType != null && Attribute.IsDefined(propertyInfo, typeof(TAttribute), inherit: true) ? targetClrType @@ -296,9 +295,9 @@ public virtual void ProcessEntityTypeMemberIgnored( private Type? FindCandidateNavigationWithAttributePropertyType(PropertyInfo propertyInfo, IConventionEntityType entityType) => Dependencies.MemberClassifier.GetNavigationCandidates(entityType) - .TryGetValue(propertyInfo, out var targetClrType) + .TryGetValue(propertyInfo, out var target) && Attribute.IsDefined(propertyInfo, typeof(TAttribute), inherit: true) - ? targetClrType + ? target.Type : null; /// @@ -380,14 +379,14 @@ public virtual void ProcessEntityTypeIgnored( /// Called for every navigation property that has an attribute after an entity type is removed. /// /// The builder for the model. - /// The ignored entity type. + /// The ignored entity type. /// The navigation member info. /// The CLR type of the target entity type. /// The attribute. /// Additional information associated with convention execution. public virtual void ProcessEntityTypeRemoved( IConventionModelBuilder modelBuilder, - Type type, + IConventionEntityType entityType, MemberInfo navigationMemberInfo, Type targetClrType, TAttribute attribute, diff --git a/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs b/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs index 94a531e34ab..19c1c5c488f 100644 --- a/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs +++ b/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs @@ -7,7 +7,6 @@ using System.Diagnostics; using System.Linq; using System.Reflection; -using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Infrastructure.Internal; @@ -52,52 +51,41 @@ private void DiscoverRelationships(IConventionEntityTypeBuilder entityTypeBuilde relationshipCandidates = RemoveInheritedInverseNavigations(relationshipCandidates); relationshipCandidates = RemoveSingleSidedBaseNavigations(relationshipCandidates, entityTypeBuilder); - using (context.DelayConventions()) - { - CreateRelationships(relationshipCandidates, entityTypeBuilder); - } + CreateRelationships(relationshipCandidates, entityTypeBuilder); } private IReadOnlyList FindRelationshipCandidates(IConventionEntityTypeBuilder entityTypeBuilder) { var entityType = entityTypeBuilder.Metadata; - var model = entityType.Model; var relationshipCandidates = new Dictionary(); - var ownership = entityTypeBuilder.Metadata.FindOwnership(); + var ownership = entityType.FindOwnership(); if (ownership == null - && model.IsOwned(entityTypeBuilder.Metadata.ClrType)) + && entityType.IsOwned()) { + // Handled when the ownership is actually added return relationshipCandidates.Values.ToList(); } foreach (var candidateTuple in Dependencies.MemberClassifier.GetNavigationCandidates(entityType)) { var navigationPropertyInfo = candidateTuple.Key; - var targetClrType = candidateTuple.Value; + var (targetClrType, shouldBeOwned) = candidateTuple.Value; - if (!IsCandidateNavigationProperty(entityTypeBuilder, navigationPropertyInfo.GetSimpleMemberName(), navigationPropertyInfo) - || (model.IsOwned(targetClrType) - && HasDeclaredAmbiguousNavigationsTo(entityType, targetClrType))) + if (!IsCandidateNavigationProperty(entityTypeBuilder, navigationPropertyInfo.GetSimpleMemberName(), navigationPropertyInfo)) { continue; } - IConventionEntityTypeBuilder? candidateTargetEntityTypeBuilder = ((InternalEntityTypeBuilder)entityTypeBuilder) - .GetTargetEntityTypeBuilder(targetClrType, navigationPropertyInfo, ConfigurationSource.Convention); - + var candidateTargetEntityTypeBuilder = TryGetTargetEntityTypeBuilder( + entityTypeBuilder, targetClrType, navigationPropertyInfo, shouldBeOwned); if (candidateTargetEntityTypeBuilder == null) { continue; } - var candidateTargetEntityType = candidateTargetEntityTypeBuilder.Metadata; - if (candidateTargetEntityType.IsKeyless) - { - continue; - } - 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; @@ -111,31 +99,62 @@ private IReadOnlyList FindRelationshipCandidates(IConvent return Array.Empty(); } - if (!model.IsOwned(targetClrType)) + var candidateTargetEntityType = candidateTargetEntityTypeBuilder.Metadata; + if (candidateTargetEntityType.IsKeyless + || (candidateTargetEntityType.IsOwned() + && HasDeclaredAmbiguousNavigationsTo(entityType, targetClrType))) + { + continue; + } + + Check.DebugAssert(entityType.ClrType != targetClrType + || !candidateTargetEntityType.IsOwned() + || candidateTargetEntityType.FindOwnership()?.PrincipalToDependent?.Name == navigationPropertyInfo.GetSimpleMemberName(), + "New 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())); + + if (candidateTargetEntityType.IsOwned() + && !shouldBeOwnership + && (targetOwnership?.PrincipalEntityType == entityType + || !candidateTargetEntityType.IsInOwnershipPath(entityType)) + && (ownership == null + || !entityType.IsInOwnershipPath(candidateTargetEntityType))) { - var targetOwnership = candidateTargetEntityType.FindOwnership(); - if (targetOwnership != null - && (targetOwnership.PrincipalEntityType != entityType - || targetOwnership.PrincipalToDependent?.Name != navigationPropertyInfo.GetSimpleMemberName()) - && (ownership == null - || ownership.PrincipalEntityType != candidateTargetEntityType)) - { - continue; - } + // Only the owner or nested ownees can have navigations to an owned type + // Also skip non-ownership navigations from the owner + continue; } if (relationshipCandidates.TryGetValue(candidateTargetEntityType, out var existingCandidate)) { - if (candidateTargetEntityType != entityType - || !existingCandidate.InverseProperties.Contains(navigationPropertyInfo)) + if (!existingCandidate.IsOwnership + && !shouldBeOwnership) { - if (!existingCandidate.NavigationProperties.Contains(navigationPropertyInfo)) + if (candidateTargetEntityType != entityType + || !existingCandidate.InverseProperties.Contains(navigationPropertyInfo)) { - existingCandidate.NavigationProperties.Add(navigationPropertyInfo); + if (!existingCandidate.NavigationProperties.Contains(navigationPropertyInfo)) + { + existingCandidate.NavigationProperties.Add(navigationPropertyInfo); + } } + + continue; } - continue; + var sharedTypeBuilder = entityTypeBuilder.ModelBuilder.Entity( + targetClrType, navigationPropertyInfo.GetSimpleMemberName(), entityType); + if (sharedTypeBuilder == null) + { + continue; + } + + candidateTargetEntityType = sharedTypeBuilder.Metadata; } var navigations = new List { navigationPropertyInfo }; @@ -147,25 +166,52 @@ private IReadOnlyList FindRelationshipCandidates(IConvent foreach (var inverseCandidateTuple in inverseCandidates) { var inversePropertyInfo = inverseCandidateTuple.Key; - var inverseTargetType = inverseCandidateTuple.Value; - - if ((inverseTargetType != entityType.ClrType - && (!inverseTargetType.IsAssignableFrom(entityType.ClrType) - || (!model.IsOwned(targetClrType) - && !candidateTargetEntityType.IsInOwnershipPath(entityType)))) - || navigationPropertyInfo.IsSameAs(inversePropertyInfo) - || (ownership != null - && !candidateTargetEntityType.IsInOwnershipPath(entityType) - && (candidateTargetEntityType.IsOwned() - || !model.IsOwned(targetClrType)) - && (ownership.PrincipalEntityType != candidateTargetEntityType - || ownership.PrincipalToDependent?.Name != inversePropertyInfo.GetSimpleMemberName())) + if (navigationPropertyInfo.IsSameAs(inversePropertyInfo) || !IsCandidateNavigationProperty( candidateTargetEntityTypeBuilder, inversePropertyInfo.GetSimpleMemberName(), inversePropertyInfo)) { continue; } + var inverseTargetType = inverseCandidateTuple.Value.Type; + if (inverseTargetType != entityType.ClrType + && (!inverseTargetType.IsAssignableFrom(entityType.ClrType) + || (!shouldBeOwnership + && !candidateTargetEntityType.IsInOwnershipPath(entityType)))) + { + // Only use inverse of a base type if the target is owned by the current entity type + continue; + } + + if (ownership != null + && !shouldBeOwnership + && !candidateTargetEntityType.IsInOwnershipPath(entityType) + && (ownership.PrincipalEntityType == candidateTargetEntityType + || !entityType.IsInOwnershipPath(candidateTargetEntityType)) + && (ownership.PrincipalEntityType != candidateTargetEntityType + || ownership.PrincipalToDependent?.Name != inversePropertyInfo.GetSimpleMemberName())) + { + // Only the owner or nested ownees can have navigations to an owned type + // Also skip non-ownership inverse candidates from the owner + continue; + } + + if (shouldBeOwnership + && inversePropertyInfo.PropertyType.TryGetSequenceType() != null + && navigations.Count == 1) + { + // Target type should be the principal, discover the relationship from the other side + var targetType = candidateTargetEntityType; + if (targetType.IsInModel + && IsImplicitlyCreatedUnusedSharedType(targetType)) + { + targetType.Builder.ModelBuilder.HasNoEntityType(targetType); + } + + goto Continue; + } + + if (!inverseNavigationCandidates.Contains(inversePropertyInfo)) { inverseNavigationCandidates.Add(inversePropertyInfo); @@ -174,9 +220,18 @@ private IReadOnlyList FindRelationshipCandidates(IConvent } relationshipCandidates[candidateTargetEntityType] = - new RelationshipCandidate(candidateTargetEntityTypeBuilder, navigations, inverseNavigationCandidates); + new RelationshipCandidate(candidateTargetEntityTypeBuilder, navigations, inverseNavigationCandidates, shouldBeOwnership); + + Continue:; } + return UpdateTargetEntityTypes(entityTypeBuilder, relationshipCandidates); + } + + private List UpdateTargetEntityTypes( + IConventionEntityTypeBuilder entityTypeBuilder, + Dictionary relationshipCandidates) + { var candidates = new List(); foreach (var relationshipCandidate in relationshipCandidates.Values) { @@ -191,12 +246,11 @@ private IReadOnlyList FindRelationshipCandidates(IConvent continue; } - // The entity type might have been converted to a weak entity type - var actualTargetEntityTypeBuilder = - ((InternalEntityTypeBuilder)entityTypeBuilder).GetTargetEntityTypeBuilder( - relationshipCandidate.TargetTypeBuilder.Metadata.ClrType, - relationshipCandidate.NavigationProperties.Single(), - ConfigurationSource.Convention); + // The entity type might have been converted to a shared type entity type + var actualTargetEntityTypeBuilder = TryGetTargetEntityTypeBuilder( + entityTypeBuilder, + relationshipCandidate.TargetTypeBuilder.Metadata.ClrType, + relationshipCandidate.NavigationProperties.Single()); if (actualTargetEntityTypeBuilder == null) { @@ -205,13 +259,46 @@ private IReadOnlyList FindRelationshipCandidates(IConvent candidates.Add( new RelationshipCandidate( - actualTargetEntityTypeBuilder, relationshipCandidate.NavigationProperties, - relationshipCandidate.InverseProperties)); + actualTargetEntityTypeBuilder, + relationshipCandidate.NavigationProperties, + relationshipCandidate.InverseProperties, + relationshipCandidate.IsOwnership)); } return candidates; } + /// + /// 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 the target entity type should be owned. + /// 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 virtual IConventionEntityTypeBuilder? TryGetTargetEntityTypeBuilder( + IConventionEntityTypeBuilder entityTypeBuilder, + Type targetClrType, + MemberInfo navigationMemberInfo, + bool? shouldBeOwned = null, + bool shouldCreate = true) + { + if (shouldCreate) + { + var targetEntityTypeBuilder = ((InternalEntityTypeBuilder)entityTypeBuilder) + .GetTargetEntityTypeBuilder(targetClrType, navigationMemberInfo, ConfigurationSource.Convention, + shouldBeOwned ?? ShouldBeOwned(targetClrType, entityTypeBuilder.Metadata.Model)); + if (targetEntityTypeBuilder != null) + { + return targetEntityTypeBuilder; + } + } + + return ((InternalEntityTypeBuilder)entityTypeBuilder) + .GetTargetEntityTypeBuilder(targetClrType, navigationMemberInfo, null, shouldBeOwned); + } + private static IReadOnlyList RemoveIncompatibleWithExistingRelationships( IReadOnlyList relationshipCandidates, IConventionEntityTypeBuilder entityTypeBuilder) @@ -284,7 +371,8 @@ private static IReadOnlyList RemoveIncompatibleWithExisti new RelationshipCandidate( targetEntityTypeBuilder, new List { navigationProperty }, - new List())); + new List(), + relationshipCandidate.IsOwnership)); if (relationshipCandidate.TargetTypeBuilder.Metadata == entityTypeBuilder.Metadata && relationshipCandidate.InverseProperties.Count > 0) @@ -321,7 +409,8 @@ private static IReadOnlyList RemoveIncompatibleWithExisti new RelationshipCandidate( targetEntityTypeBuilder, new List { navigationProperty }, - new List { compatibleInverse }) + new List { compatibleInverse }, + relationshipCandidate.IsOwnership) ); if (relationshipCandidate.TargetTypeBuilder.Metadata == entityTypeBuilder.Metadata @@ -521,45 +610,11 @@ private void CreateRelationships( || (targetEntityType.BaseType != null && HasAmbiguousNavigationsTo(targetEntityType.BaseType, entityType.ClrType)); - var ambiguousOwnership = relationshipCandidate.NavigationProperties.Count == 1 - && relationshipCandidate.InverseProperties.Count == 1 - && entityType.GetConfigurationSource() != ConfigurationSource.Explicit - && targetEntityType.GetConfigurationSource() != ConfigurationSource.Explicit - && targetEntityType.Model.IsOwned(entityType.ClrType) - && targetEntityType.Model.IsOwned(targetEntityType.ClrType); - - if (ambiguousOwnership) - { - var existingNavigation = - entityType.FindNavigation(relationshipCandidate.NavigationProperties.Single().GetSimpleMemberName()); - if (existingNavigation != null - && existingNavigation.ForeignKey.DeclaringEntityType == targetEntityType - && existingNavigation.ForeignKey.GetPrincipalEndConfigurationSource() - .OverridesStrictly(ConfigurationSource.Convention)) - { - ambiguousOwnership = false; - } - else - { - var existingInverse = - targetEntityType.FindNavigation(relationshipCandidate.InverseProperties.Single().GetSimpleMemberName()); - if (existingInverse != null - && existingInverse.ForeignKey.PrincipalEntityType == targetEntityType - && existingInverse.ForeignKey.GetPrincipalEndConfigurationSource() - .OverridesStrictly(ConfigurationSource.Convention)) - { - ambiguousOwnership = false; - } - } - } - if ((relationshipCandidate.NavigationProperties.Count > 1 && relationshipCandidate.InverseProperties.Count > 0 - && (!targetEntityType.Model.IsOwned(targetEntityType.ClrType) - || entityType.IsInOwnershipPath(targetEntityType))) + && !relationshipCandidate.IsOwnership) || relationshipCandidate.InverseProperties.Count > 1 || isAmbiguousOnBase - || ambiguousOwnership || HasDeclaredAmbiguousNavigationsTo(entityType, targetEntityType.ClrType) || HasDeclaredAmbiguousNavigationsTo(targetEntityType, entityType.ClrType)) { @@ -603,7 +658,7 @@ private void CreateRelationships( foreach (var navigation in relationshipCandidate.NavigationProperties) { if (!targetEntityType.IsInModel - && !targetEntityType.Model.IsOwned(targetEntityType.ClrType)) + && !targetEntityType.IsOwned()) { continue; } @@ -614,17 +669,12 @@ private void CreateRelationships( continue; } - var targetOwned = !entityType.IsInOwnershipPath(targetEntityType) - && (targetEntityType.Model.IsOwned(targetEntityType.ClrType) - || (targetEntityType.HasSharedClrType - && targetEntityType.Model.FindEntityTypes(targetEntityType.ClrType).Any(e => e.IsOwned()))); - var inverse = relationshipCandidate.InverseProperties.SingleOrDefault(); if (inverse == null) { - if (targetOwned) + if (relationshipCandidate.IsOwnership) { - entityTypeBuilder.HasOwnership(targetEntityType.ClrType, navigation); + entityTypeBuilder.HasOwnership(targetEntityType, navigation); } else { @@ -639,25 +689,9 @@ private void CreateRelationships( continue; } - if (targetOwned - && entityType.Model.IsOwned(entityType.ClrType)) + if (relationshipCandidate.IsOwnership) { - var existingInverse = targetEntityType.FindNavigation(inverse.GetSimpleMemberName()); - if (inverse.PropertyType.TryGetSequenceType() != null - || targetEntityType.GetConfigurationSource() == ConfigurationSource.Explicit - || (existingInverse != null - && existingInverse.ForeignKey.DeclaringEntityType == entityType - && existingInverse.ForeignKey.GetPrincipalEndConfigurationSource() - .OverridesStrictly(ConfigurationSource.Convention))) - { - // Target type is the principal, so the ownership should be configured from the other side - targetOwned = false; - } - } - - if (targetOwned) - { - entityTypeBuilder.HasOwnership(targetEntityType.ClrType, navigation, inverse); + entityTypeBuilder.HasOwnership(targetEntityType, navigation, inverse); } else if (entityTypeBuilder.HasRelationship(targetEntityType, navigation, inverse) == null) { @@ -676,7 +710,7 @@ private void CreateRelationships( if (relationshipCandidate.NavigationProperties.Count == 0) { if (relationshipCandidate.InverseProperties.Count == 0 - || targetEntityType.Model.IsOwned(targetEntityType.ClrType)) + || targetEntityType.IsOwned()) { unusedEntityTypes.Add(targetEntityType); } @@ -710,6 +744,15 @@ private void CreateRelationships( } } + /// + /// 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 virtual bool? ShouldBeOwned(Type targetType, IConventionModel model) + => null; + private void RemoveNavigation( PropertyInfo navigationProperty, IConventionEntityType declaringEntityType, @@ -814,7 +857,10 @@ public virtual void ProcessNavigationRemoved( { if ((targetEntityTypeBuilder.Metadata.IsInModel || !sourceEntityTypeBuilder.ModelBuilder.IsIgnored(targetEntityTypeBuilder.Metadata.Name)) - && IsCandidateNavigationProperty(sourceEntityTypeBuilder, navigationName, memberInfo)) + && memberInfo != null + && IsCandidateNavigationProperty(sourceEntityTypeBuilder, navigationName, memberInfo) + && Dependencies.MemberClassifier.FindCandidateNavigationPropertyType( + memberInfo, targetEntityTypeBuilder.Metadata.Model, out _) != null) { Process(sourceEntityTypeBuilder.Metadata, navigationName, memberInfo!, context); } @@ -839,16 +885,14 @@ private void Process( } } - [ContractAnnotation("memberInfo:null => false")] private static bool IsCandidateNavigationProperty( IConventionEntityTypeBuilder? sourceEntityTypeBuilder, string navigationName, - MemberInfo? memberInfo) - => memberInfo != null - && sourceEntityTypeBuilder?.IsIgnored(navigationName) == false + MemberInfo memberInfo) + => sourceEntityTypeBuilder?.IsIgnored(navigationName) == false && sourceEntityTypeBuilder.Metadata.FindProperty(navigationName) == null && sourceEntityTypeBuilder.Metadata.FindServiceProperty(navigationName) == null - && (!(memberInfo is PropertyInfo propertyInfo) || propertyInfo.GetIndexParameters().Length == 0) + && (memberInfo is not PropertyInfo propertyInfo || propertyInfo.GetIndexParameters().Length == 0) && (!sourceEntityTypeBuilder.Metadata.IsKeyless || (memberInfo as PropertyInfo)?.PropertyType.TryGetSequenceType() == null); @@ -917,8 +961,8 @@ private bool ProcessEntityTypeMemberIgnoredOnBase(IConventionEntityType entityTy var targetClrType = ambiguousNavigation.Value.Value; RemoveAmbiguous(entityType, targetClrType); - var targetType = ((InternalEntityTypeBuilder)entityType.Builder) - .GetTargetEntityTypeBuilder(targetClrType, ambiguousNavigation.Value.Key, null)?.Metadata; + var targetType = TryGetTargetEntityTypeBuilder( + entityType.Builder, targetClrType, ambiguousNavigation.Value.Key, shouldCreate: false)?.Metadata; if (targetType != null) { RemoveAmbiguous(targetType, entityType.ClrType); @@ -1068,16 +1112,19 @@ private sealed class RelationshipCandidate public RelationshipCandidate( IConventionEntityTypeBuilder targetTypeBuilder, List navigations, - List inverseNavigations) + List inverseNavigations, + bool ownership) { TargetTypeBuilder = targetTypeBuilder; NavigationProperties = navigations; InverseProperties = inverseNavigations; + IsOwnership = ownership; } public IConventionEntityTypeBuilder TargetTypeBuilder { [DebuggerStepThrough] get; } public List NavigationProperties { [DebuggerStepThrough] get; } public List InverseProperties { [DebuggerStepThrough] get; } + public bool IsOwnership { [DebuggerStepThrough] get; } private string DebuggerDisplay() => TargetTypeBuilder.Metadata.ToDebugString(MetadataDebugStringOptions.SingleLineDefault) @@ -1085,7 +1132,8 @@ private string DebuggerDisplay() + string.Join(", ", NavigationProperties.Select(p => p.Name)) + "] - [" + string.Join(", ", InverseProperties.Select(p => p.Name)) - + "]"; + + "]" + + (IsOwnership ? " Ownership" : ""); } } } diff --git a/src/EFCore/Metadata/Conventions/RuntimeModelConvention.cs b/src/EFCore/Metadata/Conventions/RuntimeModelConvention.cs index 47c4676a28b..52007065b1d 100644 --- a/src/EFCore/Metadata/Conventions/RuntimeModelConvention.cs +++ b/src/EFCore/Metadata/Conventions/RuntimeModelConvention.cs @@ -192,7 +192,6 @@ protected virtual void ProcessModelAnnotations( } else { - annotations.Remove(CoreAnnotationNames.OwnedTypes); annotations.Remove(CoreAnnotationNames.PropertyAccessMode); } } diff --git a/src/EFCore/Metadata/IConventionModel.cs b/src/EFCore/Metadata/IConventionModel.cs index 1314cb6e158..22df45e2884 100644 --- a/src/EFCore/Metadata/IConventionModel.cs +++ b/src/EFCore/Metadata/IConventionModel.cs @@ -79,7 +79,7 @@ public interface IConventionModel : IReadOnlyModel, IConventionAnnotatable /// /// - /// Adds a shadow state entity type to the model. + /// Adds a state entity type of default type to the model. /// /// /// Shadow entities are not currently supported in a model that is used at runtime with a . @@ -115,7 +115,7 @@ public interface IConventionModel : IReadOnlyModel, IConventionAnnotatable IConventionEntityType? AddEntityType(string name, Type clrType, bool fromDataAnnotation = false); /// - /// Adds an entity type with a defining navigation to the model. + /// Adds an owned entity type with a defining navigation to the model. /// /// The name of the entity type to be added. /// The defining navigation. @@ -129,7 +129,7 @@ public interface IConventionModel : IReadOnlyModel, IConventionAnnotatable bool fromDataAnnotation = false); /// - /// Adds an entity type with a defining navigation to the model. + /// Adds an owned entity type with a defining navigation to the model. /// /// The CLR class that is used to represent instances of this entity type. /// The defining navigation. @@ -142,6 +142,43 @@ public interface IConventionModel : IReadOnlyModel, IConventionAnnotatable IConventionEntityType definingEntityType, bool fromDataAnnotation = false); + /// + /// + /// Adds an owned entity type of default type to the model. + /// + /// + /// Shadow entities are not currently supported in a model that is used at runtime with a . + /// Therefore, shadow state entity types will only exist in migration model snapshots, etc. + /// + /// + /// The name of the entity to be added. + /// Indicates whether the configuration was specified using a data annotation. + /// The new entity type. + IConventionEntityType? AddOwnedEntityType(string name, bool fromDataAnnotation = false); + + /// + /// Adds an owned entity type to the model. + /// + /// The CLR class that is used to represent instances of the entity type. + /// Indicates whether the configuration was specified using a data annotation. + /// The new entity type. + IConventionEntityType? AddOwnedEntityType(Type type, bool fromDataAnnotation = false); + + /// + /// + /// Adds an owned shared type entity type to the model. + /// + /// + /// Shared type entity type is an entity type which can share CLR type with other types in the model but has + /// a unique name and always identified by the name. + /// + /// + /// The name of the entity to be added. + /// The CLR class that is used to represent instances of the entity type. + /// Indicates whether the configuration was specified using a data annotation. + /// The new entity type. + IConventionEntityType? AddOwnedEntityType(string name, Type clrType, bool fromDataAnnotation = false); + /// /// Gets the entity with the given name. Returns if no entity type with the given name is found /// or the given CLR type is being used by shared type entity type diff --git a/src/EFCore/Metadata/IMutableModel.cs b/src/EFCore/Metadata/IMutableModel.cs index 344f0757874..fde9f2b1c14 100644 --- a/src/EFCore/Metadata/IMutableModel.cs +++ b/src/EFCore/Metadata/IMutableModel.cs @@ -57,7 +57,7 @@ public interface IMutableModel : IReadOnlyModel, IMutableAnnotatable /// /// - /// Adds a shadow state entity type to the model. + /// Adds an entity type of default type to the model. /// /// /// Shadow entities are not currently supported in a model that is used at runtime with a . @@ -90,7 +90,7 @@ public interface IMutableModel : IReadOnlyModel, IMutableAnnotatable IMutableEntityType AddEntityType(string name, Type type); /// - /// Adds an entity type with a defining navigation to the model. + /// Adds an owned entity type with a defining navigation to the model. /// /// The name of the entity type to be added. /// The defining navigation. @@ -102,7 +102,7 @@ IMutableEntityType AddEntityType( IMutableEntityType definingEntityType); /// - /// Adds an entity type with a defining navigation to the model. + /// Adds an owned entity type with a defining navigation to the model. /// /// The CLR class that is used to represent instances of this entity type. /// The defining navigation. @@ -113,6 +113,40 @@ IMutableEntityType AddEntityType( string definingNavigationName, IMutableEntityType definingEntityType); + /// + /// + /// Adds an owned entity type of default type to the model. + /// + /// + /// Shadow entities are not currently supported in a model that is used at runtime with a . + /// Therefore, shadow state entity types will only exist in migration model snapshots, etc. + /// + /// + /// The name of the entity to be added. + /// The new entity type. + IMutableEntityType AddOwnedEntityType(string name); + + /// + /// Adds an owned entity type to the model. + /// + /// The CLR class that is used to represent instances of the entity type. + /// The new entity type. + IMutableEntityType AddOwnedEntityType(Type type); + + /// + /// + /// Adds an owned shared type entity type to the model. + /// + /// + /// Shared type entity type is an entity type which can share CLR type with other types in the model but has + /// a unique name and always identified by the name. + /// + /// + /// The name of the entity to be added. + /// The CLR class that is used to represent instances of the entity type. + /// The new entity type. + IMutableEntityType AddOwnedEntityType(string name, Type type); + /// /// Gets the entity with the given name. Returns if no entity type with the given name is found /// or the given CLR type is being used by shared type entity type diff --git a/src/EFCore/Metadata/IReadOnlyEntityType.cs b/src/EFCore/Metadata/IReadOnlyEntityType.cs index 4602ac8ce39..5e894a9b9ec 100644 --- a/src/EFCore/Metadata/IReadOnlyEntityType.cs +++ b/src/EFCore/Metadata/IReadOnlyEntityType.cs @@ -830,6 +830,11 @@ string ToDebugString(MetadataDebugStringOptions options = MetadataDebugStringOpt builder.Append(" Keyless"); } + if (IsOwned()) + { + builder.Append(" Owned"); + } + if (this is EntityType && GetChangeTrackingStrategy() != ChangeTrackingStrategy.Snapshot) { diff --git a/src/EFCore/Metadata/Internal/CoreAnnotationNames.cs b/src/EFCore/Metadata/Internal/CoreAnnotationNames.cs index 33d3874049e..4a571d1b248 100644 --- a/src/EFCore/Metadata/Internal/CoreAnnotationNames.cs +++ b/src/EFCore/Metadata/Internal/CoreAnnotationNames.cs @@ -86,14 +86,6 @@ public static class CoreAnnotationNames /// public const string NavigationAccessMode = "NavigationAccessMode"; - /// - /// 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 const string OwnedTypes = "OwnedTypes"; - /// /// 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 @@ -306,7 +298,6 @@ public static class CoreAnnotationNames ValueGeneratorFactoryType, PropertyAccessMode, NavigationAccessMode, - OwnedTypes, DiscriminatorProperty, DiscriminatorMappingComplete, DiscriminatorValue, diff --git a/src/EFCore/Metadata/Internal/EntityType.cs b/src/EFCore/Metadata/Internal/EntityType.cs index 04be8d4d8bc..a2408591b94 100644 --- a/src/EFCore/Metadata/Internal/EntityType.cs +++ b/src/EFCore/Metadata/Internal/EntityType.cs @@ -55,6 +55,7 @@ private readonly SortedDictionary _serviceProperties private List? _data; private Key? _primaryKey; private bool? _isKeyless; + private bool _isOwned; private EntityType? _baseType; private ChangeTrackingStrategy? _changeTrackingStrategy; private InternalEntityTypeBuilder? _builder; @@ -89,11 +90,12 @@ private readonly SortedDictionary _serviceProperties /// 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 EntityType(string name, Model model, ConfigurationSource configurationSource) + public EntityType(string name, Model model, bool owned, ConfigurationSource configurationSource) : base(name, Model.DefaultPropertyBagType, model, configurationSource) { _properties = new SortedDictionary(new PropertyNameComparer(this)); _builder = new InternalEntityTypeBuilder(this, model.Builder); + _isOwned = owned; } /// @@ -102,7 +104,7 @@ public EntityType(string name, Model model, ConfigurationSource configurationSou /// 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 EntityType(Type type, Model model, ConfigurationSource configurationSource) + public EntityType(Type type, Model model, bool owned, ConfigurationSource configurationSource) : base(type, model, configurationSource) { if (!type.IsValidEntityType()) @@ -119,6 +121,7 @@ public EntityType(Type type, Model model, ConfigurationSource configurationSourc _properties = new SortedDictionary(new PropertyNameComparer(this)); _builder = new InternalEntityTypeBuilder(this, model.Builder); + _isOwned = owned; } /// @@ -127,7 +130,7 @@ public EntityType(Type type, Model model, ConfigurationSource configurationSourc /// 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 EntityType(string name, Type type, Model model, ConfigurationSource configurationSource) + public EntityType(string name, Type type, Model model, bool owned, ConfigurationSource configurationSource) : base(name, type, model, configurationSource) { if (!type.IsValidEntityType()) @@ -144,6 +147,7 @@ public EntityType(string name, Type type, Model model, ConfigurationSource confi _properties = new SortedDictionary(new PropertyNameComparer(this)); _builder = new InternalEntityTypeBuilder(this, model.Builder); + _isOwned = owned; } /// @@ -196,6 +200,24 @@ public virtual bool IsKeyless set => SetIsKeyless(value, ConfigurationSource.Explicit); } + /// + /// 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 IsOwned() + => _isOwned; + + /// + /// 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 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 @@ -311,6 +333,15 @@ private void UpdateIsKeylessConfigurationSource(ConfigurationSource configuratio throw new InvalidOperationException(CoreStrings.DerivedEntityCannotBeKeyless(DisplayName())); } + if (IsOwned() != newBaseType.IsOwned()) + { + throw new InvalidOperationException(CoreStrings.DerivedEntityOwnershipMismatch( + newBaseType.DisplayName(), + DisplayName(), + IsOwned() ? DisplayName() : newBaseType.DisplayName(), + !IsOwned() ? DisplayName() : newBaseType.DisplayName())); + } + var conflictingMember = newBaseType.GetMembers() .Select(p => p.Name) .SelectMany(FindMembersInHierarchy) diff --git a/src/EFCore/Metadata/Internal/EntityTypeExtensions.cs b/src/EFCore/Metadata/Internal/EntityTypeExtensions.cs index 10b565e1ce8..d0f9b64649e 100644 --- a/src/EFCore/Metadata/Internal/EntityTypeExtensions.cs +++ b/src/EFCore/Metadata/Internal/EntityTypeExtensions.cs @@ -79,6 +79,15 @@ public static bool IsOwned(this IReadOnlyEntityType entityType) public static IReadOnlyForeignKey? FindDeclaredOwnership(this IReadOnlyEntityType entityType) => entityType.GetDeclaredForeignKeys().FirstOrDefault(fk => fk.IsOwnership); + /// + /// 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 static IConventionForeignKey? FindDeclaredOwnership(this IConventionEntityType entityType) + => entityType.GetDeclaredForeignKeys().FirstOrDefault(fk => fk.IsOwnership); + /// /// 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 @@ -118,6 +127,37 @@ public static bool IsOwned(this IReadOnlyEntityType entityType) public static bool IsInOwnershipPath(this IReadOnlyEntityType entityType, Type targetType) => entityType.FindInOwnershipPath(targetType) != 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 static bool IsInOwnershipPath(this IReadOnlyEntityType entityType, IReadOnlyEntityType targetType) + { + if (entityType == targetType) + { + return true; + } + + var owner = entityType; + while (true) + { + var ownership = owner.FindOwnership(); + if (ownership == null) + { + return false; + } + + owner = ownership.PrincipalEntityType; + if (owner.IsAssignableFrom(targetType)) + { + return true; + } + } + } + /// /// 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/ForeignKey.cs b/src/EFCore/Metadata/Internal/ForeignKey.cs index d020fdc8860..0c410a729e8 100644 --- a/src/EFCore/Metadata/Internal/ForeignKey.cs +++ b/src/EFCore/Metadata/Internal/ForeignKey.cs @@ -218,11 +218,23 @@ public virtual IReadOnlyList SetProperties( { EnsureMutable(); - Validate(properties, principalKey, DeclaringEntityType, PrincipalEntityType); - var oldProperties = Properties; var oldPrincipalKey = PrincipalKey; + if (oldProperties.SequenceEqual(properties) + && oldPrincipalKey == principalKey) + { + if (configurationSource != null) + { + UpdatePropertiesConfigurationSource(configurationSource.Value); + UpdatePrincipalKeyConfigurationSource(configurationSource.Value); + } + + return oldProperties; + } + + Validate(properties, principalKey, DeclaringEntityType, PrincipalEntityType); + DeclaringEntityType.OnForeignKeyUpdating(this); Properties = properties; @@ -452,6 +464,14 @@ public virtual void UpdatePrincipalToDependentConfigurationSource(ConfigurationS return oldNavigation!; } + if (name == null + && IsOwnership + && !pointsToPrincipal) + { + throw new InvalidOperationException(CoreStrings.OwnershipToDependent( + oldNavigation?.Name, PrincipalEntityType.DisplayName(), DeclaringEntityType.DisplayName())); + } + if (oldNavigation != null) { Check.DebugAssert(oldNavigation.Name != null, "oldNavigation.Name is null"); @@ -806,6 +826,21 @@ public virtual bool IsOwnership { EnsureMutable(); + if (ownership == true) + { + if (!DeclaringEntityType.IsOwned()) + { + throw new InvalidOperationException(CoreStrings.ClashingNonOwnedEntityType(DeclaringEntityType.DisplayName())); + } + + if (PrincipalToDependent == null) + { + throw new InvalidOperationException( + CoreStrings.NavigationlessOwnership( + PrincipalEntityType.DisplayName(), DeclaringEntityType.DisplayName())); + } + } + var oldIsOwnership = IsOwnership; _isOwnership = ownership; diff --git a/src/EFCore/Metadata/Internal/IMemberClassifier.cs b/src/EFCore/Metadata/Internal/IMemberClassifier.cs index e52acba52ef..3dcbfaf8af1 100644 --- a/src/EFCore/Metadata/Internal/IMemberClassifier.cs +++ b/src/EFCore/Metadata/Internal/IMemberClassifier.cs @@ -29,7 +29,7 @@ public interface IMemberClassifier /// 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. /// - ImmutableSortedDictionary GetNavigationCandidates(IConventionEntityType entityType); + ImmutableSortedDictionary GetNavigationCandidates(IConventionEntityType entityType); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -37,7 +37,7 @@ public interface IMemberClassifier /// 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. /// - Type? FindCandidateNavigationPropertyType(MemberInfo memberInfo, ModelConfiguration? configuration); + Type? FindCandidateNavigationPropertyType(MemberInfo memberInfo, IConventionModel model, out bool? shouldBeOwned); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs index d4fb2d720aa..6a9bff2930b 100644 --- a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs @@ -97,7 +97,7 @@ public InternalEntityTypeBuilder(EntityType metadata, InternalModelBuilder model return null; } - Metadata.SetPrimaryKey(keyBuilder.Metadata.Properties, configurationSource); + var newKey = Metadata.SetPrimaryKey(keyBuilder.Metadata.Properties, configurationSource); foreach (var key in Metadata.GetDeclaredKeys().ToList()) { if (key == keyBuilder.Metadata) @@ -112,7 +112,21 @@ public InternalEntityTypeBuilder(EntityType metadata, InternalModelBuilder model foreach (var referencingForeignKey in referencingForeignKeys) { - DetachRelationship(referencingForeignKey).Attach(); + if (referencingForeignKey.GetPropertiesConfigurationSource() != null + && !ForeignKey.AreCompatible( + newKey!.Properties, + referencingForeignKey.Properties, + referencingForeignKey.PrincipalEntityType, + Metadata, + shouldThrow: false)) + { + DetachRelationship(referencingForeignKey).Attach(); + } + else + { + referencingForeignKey.Builder.HasPrincipalKey((IReadOnlyList?)null, ConfigurationSource.Convention); + } + } } @@ -1453,7 +1467,7 @@ public virtual bool CanSetDefiningQuery(LambdaExpression? query, ConfigurationSo return HasBaseType((EntityType?)null, configurationSource); } - var baseType = ModelBuilder.Entity(baseEntityType, configurationSource); + var baseType = ModelBuilder.Entity(baseEntityType, configurationSource, shouldBeOwned: Metadata.IsOwned()); return baseType == null ? null : HasBaseType(baseType.Metadata, configurationSource); @@ -1472,7 +1486,7 @@ public virtual bool CanSetDefiningQuery(LambdaExpression? query, ConfigurationSo return HasBaseType((EntityType?)null, configurationSource); } - var baseType = ModelBuilder.Entity(baseEntityTypeName, configurationSource); + var baseType = ModelBuilder.Entity(baseEntityTypeName, configurationSource, shouldBeOwned: Metadata.IsOwned()); return baseType == null ? null : HasBaseType(baseType.Metadata, configurationSource); @@ -1609,7 +1623,7 @@ public virtual bool CanSetDefiningQuery(LambdaExpression? query, ConfigurationSo FindConflictingMembers( Metadata.GetDerivedTypesInclusive().SelectMany(et => et.GetDeclaredProperties()), baseMemberNames, - n => baseEntityType.FindProperty(n.Name) != null, + p => baseEntityType.FindProperty(p.Name) != null, p => p.DeclaringEntityType.Builder.RemoveProperty(p, ConfigurationSource.Explicit)); if (propertiesToDetach != null) @@ -1837,7 +1851,14 @@ public virtual bool CanSetDefiningQuery(LambdaExpression? query, ConfigurationSo && configurationSource == ConfigurationSource.Explicit && member.GetConfigurationSource() == ConfigurationSource.Explicit) { - continue; + throw new InvalidOperationException( + CoreStrings.DuplicatePropertiesOnBase( + Metadata.DisplayName(), + baseEntityType.DisplayName(), + ((IReadOnlyTypeBase)member.DeclaringType).DisplayName(), + member.Name, + baseEntityType.DisplayName(), + member.Name)); } if (membersToBeRemoved == null) @@ -2586,7 +2607,8 @@ private static InternalIndexBuilder DetachIndex(Index indexToDetach) Check.NotNull(principalClrType, nameof(principalClrType)); Check.NotEmpty(clrMembers, nameof(clrMembers)); - var principalTypeBuilder = ModelBuilder.Entity(principalClrType, configurationSource); + var principalTypeBuilder = ModelBuilder.Entity( + principalClrType, configurationSource, shouldBeOwned: Metadata.IsInOwnershipPath(principalClrType) ? null : false); return principalTypeBuilder == null ? null : HasForeignKey( @@ -2611,7 +2633,8 @@ private static InternalIndexBuilder DetachIndex(Index indexToDetach) Check.NotNull(principalClrType, nameof(principalClrType)); Check.NotEmpty(clrMembers, nameof(clrMembers)); - var principalTypeBuilder = ModelBuilder.Entity(principalClrType, configurationSource); + var principalTypeBuilder = ModelBuilder.Entity( + principalClrType, configurationSource, shouldBeOwned: Metadata.IsInOwnershipPath(principalClrType) ? null : false); return principalTypeBuilder == null ? null : HasForeignKey( @@ -2731,7 +2754,7 @@ private static InternalIndexBuilder DetachIndex(Index indexToDetach) Check.NotNull(targetEntityType, nameof(targetEntityType)), MemberIdentity.Create(navigationName), MemberIdentity.Create(inverseNavigationName), - setTargetAsPrincipal ? true : (bool?)null, + setTargetAsPrincipal ? true : null, configurationSource); /// @@ -2750,7 +2773,7 @@ private static InternalIndexBuilder DetachIndex(Index indexToDetach) Check.NotNull(targetEntityType, nameof(targetEntityType)), MemberIdentity.Create(navigation), MemberIdentity.Create(inverseNavigation), - setTargetAsPrincipal ? true : (bool?)null, + setTargetAsPrincipal ? true : null, configurationSource); private InternalForeignKeyBuilder? HasRelationship( @@ -2770,6 +2793,7 @@ private static InternalIndexBuilder DetachIndex(Index indexToDetach) "required should only be set if principal end is known"); var navigationProperty = navigationToTarget?.MemberInfo; + var inverseProperty = inverseNavigation?.MemberInfo; if (setTargetAsPrincipal == false || (inverseNavigation == null && navigationProperty?.GetMemberType().IsAssignableFrom( @@ -2955,9 +2979,61 @@ private static InternalIndexBuilder DetachIndex(Index indexToDetach) } else { - if (setTargetAsPrincipal == true - || (setTargetAsPrincipal == null - && !((IReadOnlyEntityType)targetEntityType).IsInOwnershipPath(Metadata))) + if (navigationToTarget?.Name != null + && navigationToTarget!.Value.MemberInfo == null + && Metadata.ClrType != Model.DefaultPropertyBagType) + { + navigationProperty = InternalForeignKeyBuilder.FindCompatibleClrMember( + navigationToTarget!.Value.Name!, Metadata, targetEntityType, shouldThrow: configurationSource == ConfigurationSource.Explicit); + if (navigationProperty != null) + { + navigationToTarget = MemberIdentity.Create(navigationProperty); + } + } + + if (inverseNavigation?.Name != null + && inverseNavigation!.Value.MemberInfo == null + && targetEntityType.ClrType != Model.DefaultPropertyBagType) + { + inverseProperty = InternalForeignKeyBuilder.FindCompatibleClrMember( + inverseNavigation!.Value.Name!, targetEntityType, Metadata, shouldThrow: configurationSource == ConfigurationSource.Explicit); + if (inverseProperty != null) + { + inverseNavigation = MemberIdentity.Create(inverseProperty); + } + } + + if (!InternalForeignKeyBuilder.AreCompatible( + navigationToTarget?.MemberInfo, + inverseNavigation?.MemberInfo, + targetEntityType, + Metadata, + shouldThrow: configurationSource == ConfigurationSource.Explicit, + out var shouldInvert, + out var shouldBeUnique)) + { + return null; + } + + if (shouldInvert == true && setTargetAsPrincipal == true) + { + throw new InvalidOperationException( + CoreStrings.PrincipalEndIncompatibleNavigations( + Metadata.DisplayName() + + (navigationToTarget == null + ? "" + : "." + navigationToTarget.Value.Name), + targetEntityType.DisplayName() + + (inverseNavigation == null + ? "" + : "." + inverseNavigation.Value.Name), + targetEntityType.DisplayName())); + } + + shouldInvert ??= setTargetAsPrincipal != true + && (setTargetAsPrincipal != null + || ((IReadOnlyEntityType)targetEntityType).IsInOwnershipPath(Metadata)); + if (!shouldInvert.Value) { newRelationship = CreateForeignKey( targetEntityType.Builder, @@ -2974,6 +3050,7 @@ private static InternalIndexBuilder DetachIndex(Index indexToDetach) inverseNavigation = navigation; navigationProperty = navigationToTarget?.MemberInfo; + inverseProperty = inverseNavigation?.MemberInfo; newRelationship = targetEntityType.Builder.CreateForeignKey( this, @@ -3003,7 +3080,6 @@ private static InternalIndexBuilder DetachIndex(Index indexToDetach) } } - var inverseProperty = inverseNavigation?.MemberInfo; if (inverseNavigation == null) { relationship = navigationProperty != null @@ -3243,13 +3319,53 @@ private static InternalIndexBuilder DetachIndex(Index indexToDetach) MemberIdentity? inverse, ConfigurationSource configurationSource) { - InternalEntityTypeBuilder? ownedEntityTypeBuilder; InternalForeignKeyBuilder? relationship; + var existingNavigation = Metadata.FindNavigation(navigation.Name!); + if (existingNavigation != null + && !existingNavigation.IsOnDependent) + { + var existingTargetType = existingNavigation.TargetEntityType; + if ((targetEntityType.Type == null + || existingTargetType.ClrType == targetEntityType.Type) + && (!targetEntityType.IsNamed + || targetEntityType.Name == existingTargetType.Name + || (targetEntityType.Type == null + && targetEntityType.Name == existingTargetType.ClrType.DisplayName()))) + { + relationship = existingNavigation.ForeignKey.Builder; + if (existingNavigation.ForeignKey.IsOwnership) + { + relationship = relationship.IsOwnership(true, configurationSource) + ?.HasNavigations(inverse, navigation, configurationSource); + + relationship?.Metadata.UpdateConfigurationSource(configurationSource); + return relationship; + } + + Check.DebugAssert(!existingTargetType.IsOwned() + || existingNavigation.DeclaringEntityType.IsInOwnershipPath(existingTargetType) + || (existingTargetType.IsInOwnershipPath(existingNavigation.DeclaringEntityType) + && existingTargetType.FindOwnership()!.PrincipalEntityType != existingNavigation.DeclaringEntityType), + $"Found '{existingNavigation.DeclaringEntityType.DisplayName()}.{existingNavigation.Name}'. " + + "Owned types should only have ownership or ownee navigations point at it"); + + relationship = relationship.IsOwnership(true, configurationSource) + ?.HasNavigations(inverse, navigation, configurationSource); + + relationship?.Metadata.UpdateConfigurationSource(configurationSource); + return relationship; + } + } + + InternalEntityTypeBuilder? ownedEntityTypeBuilder; + using (var batch = Metadata.Model.DelayConventions()) { var ownership = Metadata.FindOwnership(); - ownedEntityTypeBuilder = GetTargetEntityTypeBuilder(targetEntityType, navigation, configurationSource, targetShouldBeOwned: true); + ownedEntityTypeBuilder = GetTargetEntityTypeBuilder( + targetEntityType, navigation, configurationSource, targetShouldBeOwned: true); + // TODO: Use convention batch to get the updated builder, see #15898 var principalBuilder = Metadata.IsInModel ? Metadata.Builder : ownership?.PrincipalEntityType.FindNavigation(ownership.PrincipalToDependent!.Name)?.TargetEntityType is EntityType target @@ -3271,8 +3387,17 @@ private static InternalIndexBuilder DetachIndex(Index indexToDetach) inverseNavigation: navigation, setTargetAsPrincipal: true, configurationSource, - required: true)!; - relationship = batch.Run(relationship.IsOwnership(true, configurationSource)!); + required: true) + ?.IsOwnership(true, configurationSource); + + if (relationship == null) + { + batch.Dispose(); + } + else + { + relationship = batch.Run(relationship); + } } if (relationship is null || !relationship.Metadata.IsInModel) @@ -3289,14 +3414,213 @@ private static InternalIndexBuilder DetachIndex(Index indexToDetach) return relationship; } + private InternalForeignKeyBuilder? HasOwnership( + EntityType targetEntityType, + in MemberIdentity navigation, + MemberIdentity? inverse, + ConfigurationSource configurationSource) + { + InternalForeignKeyBuilder? relationship; + var existingNavigation = Metadata.FindNavigation(navigation.Name!); + if (existingNavigation != null + && !existingNavigation.IsOnDependent) + { + var existingTargetType = existingNavigation.TargetEntityType; + if (existingTargetType == targetEntityType) + { + relationship = existingNavigation.ForeignKey.Builder; + if (existingNavigation.ForeignKey.IsOwnership) + { + relationship = relationship.IsOwnership(true, configurationSource) + ?.HasNavigations(inverse, navigation, configurationSource); + + relationship?.Metadata.UpdateConfigurationSource(configurationSource); + return relationship; + } + + Check.DebugAssert(!existingTargetType.IsOwned() + || existingNavigation.DeclaringEntityType.IsInOwnershipPath(existingTargetType) + || (existingTargetType.IsInOwnershipPath(existingNavigation.DeclaringEntityType) + && existingTargetType.FindOwnership()!.PrincipalEntityType != existingNavigation.DeclaringEntityType), + $"Found '{existingNavigation.DeclaringEntityType.DisplayName()}.{existingNavigation.Name}'. " + + "Owned types should only have ownership or ownee navigations point at it"); + + relationship = relationship.IsOwnership(true, configurationSource) + ?.HasNavigations(inverse, navigation, configurationSource); + + relationship?.Metadata.UpdateConfigurationSource(configurationSource); + return relationship; + } + } + + using (var batch = Metadata.Model.DelayConventions()) + { + relationship = targetEntityType.Builder.HasRelationship( + targetEntityType: Metadata, + navigationToTarget: inverse, + inverseNavigation: navigation, + setTargetAsPrincipal: true, + configurationSource, + required: true) + ?.IsOwnership(true, configurationSource); + + if (relationship == null) + { + batch.Dispose(); + } + else + { + relationship = batch.Run(relationship); + } + } + + return relationship; + } + /// /// 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 RemoveNonOwnershipRelationships(ForeignKey? ownership, ConfigurationSource configurationSource) + public virtual InternalEntityTypeBuilder? IsOwned( + bool owned, + ConfigurationSource configurationSource, + ForeignKey? futureOwnership = null) { + var entityType = Metadata; + if (entityType.IsOwned() == owned) + { + entityType.UpdateConfigurationSource(configurationSource); + return this; + } + + if (!CanSetIsOwned(owned, configurationSource)) + { + return null; + } + + entityType.UpdateConfigurationSource(configurationSource); + if (owned) + { + entityType.SetIsOwned(true); + + HasBaseType((EntityType?)null, configurationSource); + + foreach (var derivedType in entityType.GetDirectlyDerivedTypes().ToList()) + { + derivedType.Builder.HasBaseType((EntityType?)null, configurationSource); + } + + if (!entityType.Builder.RemoveNonOwnershipRelationships(futureOwnership, configurationSource)) + { + return null; + } + } + else + { + entityType.SetIsOwned(false); + + var ownership = entityType.FindOwnership(); + if (ownership != null) + { + HasNoRelationship(ownership, configurationSource); + } + + foreach (var derivedType in entityType.GetDerivedTypes()) + { + derivedType.SetIsOwned(false); + var derivedOwnership = derivedType.FindDeclaredOwnership(); + if (derivedOwnership != null) + { + derivedType.Builder.HasNoRelationship(derivedOwnership, configurationSource); + } + } + } + + return this; + } + + /// + /// 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 CanSetIsOwned(bool owned, ConfigurationSource configurationSource) + { + var entityType = Metadata; + if (owned) + { + if (!entityType.IsOwned()) + { + if (!configurationSource.Overrides(entityType.GetBaseTypeConfigurationSource())) + { + return false; + } + + if (!configurationSource.OverridesStrictly(entityType.GetConfigurationSource())) + { + if (configurationSource == ConfigurationSource.Explicit) + { + throw new InvalidOperationException(CoreStrings.ClashingNonOwnedEntityType(entityType.DisplayName())); + } + + return false; + } + } + + foreach (var derivedType in entityType.GetDerivedTypes()) + { + if (!derivedType.IsOwned() + && !configurationSource.OverridesStrictly(derivedType.GetConfigurationSource())) + { + if (configurationSource == ConfigurationSource.Explicit) + { + throw new InvalidOperationException( + CoreStrings.ClashingNonOwnedDerivedEntityType(entityType.DisplayName(), derivedType.DisplayName())); + } + + return false; + } + } + } + else + { + if (entityType.IsOwned() + && !configurationSource.OverridesStrictly(entityType.GetConfigurationSource())) + { + if (configurationSource == ConfigurationSource.Explicit) + { + throw new InvalidOperationException( + CoreStrings.ClashingOwnedEntityType(entityType.DisplayName())); + } + + return false; + } + + foreach (var derivedType in entityType.GetDerivedTypes()) + { + if (derivedType.IsOwned() + && !configurationSource.OverridesStrictly(derivedType.GetConfigurationSource())) + { + if (configurationSource == ConfigurationSource.Explicit) + { + throw new InvalidOperationException( + CoreStrings.ClashingOwnedDerivedEntityType(entityType.DisplayName(), derivedType.DisplayName())); + } + + return false; + } + } + } + + return true; + } + + private bool RemoveNonOwnershipRelationships(ForeignKey? futureOwnership, ConfigurationSource configurationSource) + { + var ownership = Metadata.FindOwnership() ?? futureOwnership; var incompatibleRelationships = Metadata.GetDerivedTypesInclusive() .SelectMany(t => t.GetDeclaredForeignKeys()) .Where( @@ -3318,8 +3642,7 @@ public virtual bool RemoveNonOwnershipRelationships(ForeignKey? ownership, Confi foreach (var foreignKey in incompatibleRelationships) { - // foreignKey.Builder can be null below if calling HasNoRelationship() below - // affects the other foreign key(s) in incompatibleRelationships + // foreign keys can be removed by HasNoRelationship() for the other foreign key(s) if (foreignKey.IsInModel) { foreignKey.DeclaringEntityType.Builder.HasNoRelationship(foreignKey, configurationSource); @@ -3343,9 +3666,13 @@ private bool Contains(IReadOnlyForeignKey? inheritedFk, IReadOnlyForeignKey deri public virtual InternalEntityTypeBuilder? GetTargetEntityTypeBuilder( Type targetClrType, MemberInfo navigationInfo, - ConfigurationSource? configurationSource) + ConfigurationSource? configurationSource, + bool? targetShouldBeOwned = null) => GetTargetEntityTypeBuilder( - new TypeIdentity(targetClrType, Metadata.Model), MemberIdentity.Create(navigationInfo), configurationSource); + new TypeIdentity(targetClrType, Metadata.Model), + MemberIdentity.Create(navigationInfo), + configurationSource, + targetShouldBeOwned); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -3363,20 +3690,27 @@ private bool Contains(IReadOnlyForeignKey? inheritedFk, IReadOnlyForeignKey deri if (existingNavigation != null) { var existingTargetType = existingNavigation.TargetEntityType; - if ((!targetEntityType.IsNamed - || existingTargetType.Name == targetEntityType.Name) - && (targetEntityType.Type == null - || existingTargetType.ClrType == targetEntityType.Type)) + if ((targetEntityType.Type == null + || existingTargetType.ClrType == targetEntityType.Type) + && (!targetEntityType.IsNamed + || targetEntityType.Name == existingTargetType.Name + || (targetEntityType.Type == null + && targetEntityType.Name == existingTargetType.ClrType.DisplayName()))) { Check.DebugAssert(existingNavigation.ForeignKey.IsOwnership - || !((IReadOnlyNavigation)existingNavigation).TargetEntityType.IsOwned(), - $"Found '{existingNavigation.DeclaringEntityType.ShortName()}.{existingNavigation.Name}'. " + - "Owned types should only have ownership navigations point at it"); - - return existingTargetType.HasSharedClrType - ? ModelBuilder.SharedTypeEntity( - existingTargetType.Name, existingTargetType.ClrType, configurationSource!.Value, targetShouldBeOwned) - : ModelBuilder.Entity(existingTargetType.ClrType, configurationSource!.Value, targetShouldBeOwned); + || !((IReadOnlyNavigation)existingNavigation).TargetEntityType.IsOwned() + || existingNavigation.DeclaringEntityType.IsInOwnershipPath(existingTargetType) + || (existingTargetType.IsInOwnershipPath(existingNavigation.DeclaringEntityType) + && existingTargetType.FindOwnership()!.PrincipalEntityType != existingNavigation.DeclaringEntityType), + $"Found '{existingNavigation.DeclaringEntityType.DisplayName()}.{existingNavigation.Name}'. " + + "Owned types should only have ownership and ownee navigations point at it"); + + return configurationSource == null + ? existingNavigation.TargetEntityType.Builder + : existingTargetType.HasSharedClrType + ? ModelBuilder.SharedTypeEntity( + existingTargetType.Name, existingTargetType.ClrType, configurationSource.Value, targetShouldBeOwned) + : ModelBuilder.Entity(existingTargetType.ClrType, configurationSource.Value, targetShouldBeOwned); } if (configurationSource == null @@ -3387,7 +3721,8 @@ private bool Contains(IReadOnlyForeignKey? inheritedFk, IReadOnlyForeignKey deri } } - if (navigation.MemberInfo == null) + if (navigation.MemberInfo == null + && Metadata.ClrType != Model.DefaultPropertyBagType) { if (Metadata.GetRuntimeProperties().TryGetValue(navigation.Name!, out var propertyInfo)) { @@ -3435,46 +3770,77 @@ private bool Contains(IReadOnlyForeignKey? inheritedFk, IReadOnlyForeignKey deri return null; } - if (targetShouldBeOwned != true) + if (targetShouldBeOwned == null + && Metadata.Model.FindIsOwnedConfigurationSource(targetType) != null) { - var ownership = Metadata.FindOwnership(); - if (ownership != null) + targetShouldBeOwned = true; + } + + if (targetShouldBeOwned == true + || Metadata.IsOwned()) + { + if (targetType.Equals(Metadata.ClrType) + && configurationSource != ConfigurationSource.Explicit) { - if (targetType.Equals(Metadata.ClrType)) + // Avoid infinite recursion on self reference + return null; + } + } + + if (Metadata.IsOwned() + && (targetShouldBeOwned != true + || !configurationSource.Overrides(ConfigurationSource.Explicit))) + { + // Non-explicit relationship shouldn't create a new type if there's already + // a compatible one in the ownership path + var owner = (EntityType?)Metadata.FindInOwnershipPath(targetType); + if (owner != null) + { + if (!configurationSource.Overrides(ConfigurationSource.Explicit) + && (owner.ClrType != targetType + || (owner.HasSharedClrType + && !owner.IsOwned()))) { - // Avoid infinite recursion on self reference return null; } - - if (targetType.IsAssignableFrom(ownership.PrincipalEntityType.ClrType)) + } + else + { + var ownership = Metadata.FindOwnership(); + if (ownership != null + && targetType.IsAssignableFrom(ownership.PrincipalEntityType.ClrType)) { - if (configurationSource.HasValue) - { - ownership.PrincipalEntityType.UpdateConfigurationSource(configurationSource.Value); - } - - return ownership.PrincipalEntityType.Builder; + owner = ownership.PrincipalEntityType; } } + + if (owner != null) + { + return configurationSource == null + ? owner.Builder + : owner.HasSharedClrType + ? ModelBuilder.SharedTypeEntity( + owner.Name, owner.ClrType, configurationSource.Value, targetShouldBeOwned) + : ModelBuilder.Entity(owner.ClrType, configurationSource.Value, targetShouldBeOwned); + } } - var targetTypeName = targetEntityType.IsNamed && (targetEntityType.Type != null || targetShouldBeOwned != true) + var targetTypeName = targetEntityType.IsNamed && (targetEntityType.Type != null || targetShouldBeOwned == false) ? targetEntityType.Name : Metadata.Model.IsShared(targetType) ? Metadata.GetOwnedName(targetEntityType.IsNamed ? targetEntityType.Name : targetType.ShortDisplayName(), navigation.Name!) : Metadata.Model.GetDisplayName(targetType); - var shouldBeOwned = targetShouldBeOwned ?? Metadata.Model.IsOwned(targetType); var targetEntityTypeBuilder = ModelBuilder.Metadata.FindEntityType(targetTypeName)?.Builder; if (targetEntityTypeBuilder != null - && shouldBeOwned) + && targetEntityTypeBuilder.Metadata.IsOwned() + && targetShouldBeOwned != false) { var existingOwnership = targetEntityTypeBuilder.Metadata.FindDeclaredOwnership(); if (existingOwnership != null) { if (!configurationSource.Overrides(ConfigurationSource.Explicit) - && navigation.MemberInfo != null - && Metadata.IsInOwnershipPath(targetType)) + && targetShouldBeOwned != true) { return null; } @@ -3505,40 +3871,34 @@ private bool Contains(IReadOnlyForeignKey? inheritedFk, IReadOnlyForeignKey deri } } - if (targetEntityTypeBuilder == null) + if (configurationSource == null) { - if (configurationSource == null) + if (targetEntityTypeBuilder == null + || (targetShouldBeOwned.HasValue + && targetEntityTypeBuilder.Metadata.IsOwned() != targetShouldBeOwned.Value)) { return null; } - - if (Metadata.Model.IsShared(targetType) + } + else if (Metadata.Model.IsShared(targetType) || targetEntityType.IsNamed) - { - if (shouldBeOwned != true - || (!configurationSource.Overrides(ConfigurationSource.Explicit) - && navigation.MemberInfo != null - && Metadata.IsInOwnershipPath(targetType))) - { - return null; - } - - targetEntityTypeBuilder = ModelBuilder.SharedTypeEntity( - targetTypeName, targetType, configurationSource.Value, shouldBeOwned); - } - else - { - targetEntityTypeBuilder = targetEntityType.IsNamed - ? targetType == null - ? ModelBuilder.Entity(targetTypeName, configurationSource.Value, shouldBeOwned) - : ModelBuilder.SharedTypeEntity(targetTypeName, targetType, configurationSource.Value, shouldBeOwned) - : ModelBuilder.Entity(targetType, configurationSource.Value, shouldBeOwned); - } - - if (targetEntityTypeBuilder == null) + { + if (targetShouldBeOwned != true + && !configurationSource.Overrides(ConfigurationSource.Explicit)) { return null; } + + targetEntityTypeBuilder = ModelBuilder.SharedTypeEntity( + targetTypeName, targetType, configurationSource.Value, targetShouldBeOwned); + } + else + { + targetEntityTypeBuilder = targetEntityType.IsNamed + ? targetType == null + ? ModelBuilder.Entity(targetTypeName, configurationSource.Value, targetShouldBeOwned) + : ModelBuilder.SharedTypeEntity(targetTypeName, targetType, configurationSource.Value, targetShouldBeOwned) + : ModelBuilder.Entity(targetType, configurationSource.Value, targetShouldBeOwned); } return targetEntityTypeBuilder; @@ -5092,7 +5452,7 @@ bool IConventionEntityTypeBuilder.CanRemoveIndex(IConventionIndex index, bool fr (EntityType)targetEntityType, navigationName, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention, - setTargetAsPrincipal ? true : (bool?)null); + setTargetAsPrincipal ? true : null); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -5182,6 +5542,24 @@ bool IConventionEntityTypeBuilder.CanRemoveIndex(IConventionIndex index, bool fr targetEntityType, navigationName, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// + /// Configures a relationship where the target entity is owned by (or part of) this entity. + /// + /// The entity type that this relationship targets. + /// The name of the navigation property on this entity type that is part of the relationship. + /// Indicates whether the configuration was specified using a data annotation. + /// An object that can be used to configure the relationship. + [DebuggerStepThrough] + IConventionForeignKeyBuilder? IConventionEntityTypeBuilder.HasOwnership( + IConventionEntityType targetEntityType, + string navigationName, + bool fromDataAnnotation) + => HasOwnership( + (EntityType)targetEntityType, + MemberIdentity.Create(navigationName), + inverse: null, + fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// /// 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 @@ -5197,6 +5575,26 @@ bool IConventionEntityTypeBuilder.CanRemoveIndex(IConventionIndex index, bool fr targetEntityType, navigation, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// + /// Configures a relationship where the target entity is owned by (or part of) this entity. + /// + /// The entity type that this relationship targets. + /// The navigation property on this entity type that is part of the relationship. + /// Indicates whether the configuration was specified using a data annotation. + /// + /// An object that can be used to configure the relationship if it exists on the entity type, + /// otherwise. + /// + IConventionForeignKeyBuilder? IConventionEntityTypeBuilder.HasOwnership( + IConventionEntityType targetEntityType, + MemberInfo navigation, + bool fromDataAnnotation) + => HasOwnership( + (EntityType)targetEntityType, + MemberIdentity.Create(navigation), + inverse: null, + fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// /// 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 @@ -5213,6 +5611,31 @@ bool IConventionEntityTypeBuilder.CanRemoveIndex(IConventionIndex index, bool fr targetEntityType, navigationName, inversePropertyName, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// + /// Configures a relationship where the target entity is owned by (or part of) this entity. + /// + /// The entity type that this relationship targets. + /// The name of the navigation property on this entity type that is part of the relationship. + /// + /// The name of the navigation property on the target entity type that is part of the relationship. If + /// is specified, the relationship will be configured without a navigation property on the target end. + /// + /// Indicates whether the configuration was specified using a data annotation. + /// + /// An object that can be used to configure the relationship if it exists on the entity type, + /// otherwise. + /// + IConventionForeignKeyBuilder? IConventionEntityTypeBuilder.HasOwnership( + IConventionEntityType targetEntityType, + string navigationName, + string? inverseNavigationName, + bool fromDataAnnotation) + => HasOwnership( + (EntityType)targetEntityType, + MemberIdentity.Create(navigationName), + MemberIdentity.Create(inverseNavigationName), + fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// /// 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 @@ -5229,6 +5652,31 @@ bool IConventionEntityTypeBuilder.CanRemoveIndex(IConventionIndex index, bool fr targetEntityType, navigation, inverseProperty, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// + /// Configures a relationship where the target entity is owned by (or part of) this entity. + /// + /// The entity type that this relationship targets. + /// The navigation property on this entity type that is part of the relationship. + /// + /// The navigation property on the target entity type that is part of the relationship. If + /// is specified, the relationship will be configured without a navigation property on the target end. + /// + /// Indicates whether the configuration was specified using a data annotation. + /// + /// An object that can be used to configure the relationship if it exists on the entity type, + /// otherwise. + /// + IConventionForeignKeyBuilder? IConventionEntityTypeBuilder.HasOwnership( + IConventionEntityType targetEntityType, + MemberInfo navigation, + MemberInfo? inverseNavigation, + bool fromDataAnnotation) + => HasOwnership( + (EntityType)targetEntityType, + MemberIdentity.Create(navigation), + MemberIdentity.Create(inverseNavigation), + fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// /// 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 @@ -5363,7 +5811,7 @@ bool IConventionEntityTypeBuilder.CanHaveSkipNavigation(string skipNavigationNam /// [DebuggerStepThrough] IConventionEntityTypeBuilder? IConventionEntityTypeBuilder.HasNoSkipNavigation( - IReadOnlySkipNavigation skipNavigation, + IConventionSkipNavigation skipNavigation, bool fromDataAnnotation) => HasNoSkipNavigation( (SkipNavigation)skipNavigation, @@ -5371,7 +5819,7 @@ bool IConventionEntityTypeBuilder.CanHaveSkipNavigation(string skipNavigationNam /// [DebuggerStepThrough] - bool IConventionEntityTypeBuilder.CanRemoveSkipNavigation(IReadOnlySkipNavigation skipNavigation, bool fromDataAnnotation) + bool IConventionEntityTypeBuilder.CanRemoveSkipNavigation(IConventionSkipNavigation skipNavigation, bool fromDataAnnotation) => CanRemoveSkipNavigation( (SkipNavigation)skipNavigation, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); diff --git a/src/EFCore/Metadata/Internal/InternalForeignKeyBuilder.cs b/src/EFCore/Metadata/Internal/InternalForeignKeyBuilder.cs index 1bdd97ec1c7..3ef7297cd14 100644 --- a/src/EFCore/Metadata/Internal/InternalForeignKeyBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalForeignKeyBuilder.cs @@ -181,7 +181,7 @@ public InternalForeignKeyBuilder( Metadata.UpdateDependentToPrincipalConfigurationSource(configurationSource); if (navigationToPrincipalName != null) { - principalEntityType.RemoveIgnored(navigationToPrincipalName); + dependentEntityType.RemoveIgnored(navigationToPrincipalName); } } @@ -394,7 +394,7 @@ public InternalForeignKeyBuilder( if (navigationToPrincipal != null) { - if (navigationToDependent != null) + if (navigationToPrincipal.Value.Name == Metadata.PrincipalToDependent?.Name) { Metadata.SetPrincipalToDependent((string?)null, configurationSource); } @@ -463,11 +463,17 @@ public InternalForeignKeyBuilder( : builder; } - private static MemberInfo? FindCompatibleClrMember( + /// + /// 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 static MemberInfo? FindCompatibleClrMember( string navigationName, EntityType sourceType, EntityType targetType, - bool shouldThrow) + bool shouldThrow = false) { var navigationProperty = sourceType.GetNavigationMemberInfo(navigationName); return !Navigation.IsCompatible(navigationName, navigationProperty, sourceType, targetType, null, shouldThrow) @@ -718,69 +724,16 @@ private bool CanSetNavigations( var navigationToPrincipalProperty = navigationToPrincipal?.MemberInfo; var navigationToDependentProperty = navigationToDependent?.MemberInfo; - // ReSharper disable once InlineOutVariableDeclaration - bool? invertedShouldBeUnique = null; - if (navigationToPrincipalProperty != null - && !IsCompatible( - navigationToPrincipalProperty, - pointsToPrincipal: false, - principalEntityType, - dependentEntityType, - shouldThrow: false, - out invertedShouldBeUnique)) - { - shouldInvert = false; - } - - if (navigationToDependentProperty != null - && !IsCompatible( - navigationToDependentProperty, - pointsToPrincipal: true, - principalEntityType, - dependentEntityType, - shouldThrow: false, - out _)) - { - shouldInvert = false; - } - - if (navigationToPrincipalProperty != null - && !IsCompatible( - navigationToPrincipalProperty, - pointsToPrincipal: true, - dependentEntityType, - principalEntityType, - shouldThrow && shouldInvert != null, - out _)) - { - if (shouldInvert != null) - { - return false; - } - - shouldInvert = true; - } - - if (navigationToDependentProperty != null - && !IsCompatible( - navigationToDependentProperty, - pointsToPrincipal: false, - dependentEntityType, - principalEntityType, - shouldThrow && shouldInvert != null, - out shouldBeUnique)) - { - if (shouldInvert != null) - { - return false; - } - - shouldInvert = true; - } - - if (shouldInvert == true) + if (!AreCompatible( + navigationToPrincipalProperty, + navigationToDependentProperty, + principalEntityType, + dependentEntityType, + shouldThrow, + out shouldInvert, + out shouldBeUnique)) { - shouldBeUnique = invertedShouldBeUnique; + return false; } if (shouldBeUnique.HasValue @@ -810,7 +763,7 @@ private bool CanSetNavigations( conflictingNavigationsFound = compatibleRelationship != null || resolvableRelationships.Any( - r => (r.Resolution & (Resolution.ResetToDependent | Resolution.ResetToPrincipal)) != 0); + r => (r.Resolution & (Resolution.ResetToDependent | Resolution.ResetToPrincipal | Resolution.Remove)) != 0); if (shouldBeUnique == null && (Metadata.IsUnique || configurationSource.OverridesStrictly(Metadata.GetIsUniqueConfigurationSource())) @@ -862,9 +815,95 @@ private bool CanRemoveNavigation(bool pointsToPrincipal, ConfigurationSource? co || (configurationSource.Overrides(Metadata.GetDependentToPrincipalConfigurationSource()) && (overrideSameSource || configurationSource != Metadata.GetDependentToPrincipalConfigurationSource())) : Metadata.PrincipalToDependent == null - || (configurationSource.Overrides(Metadata.GetPrincipalToDependentConfigurationSource()) + || (!Metadata.IsOwnership + && configurationSource.Overrides(Metadata.GetPrincipalToDependentConfigurationSource()) && (overrideSameSource || configurationSource != Metadata.GetPrincipalToDependentConfigurationSource())); + /// + /// 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 static bool AreCompatible( + MemberInfo? navigationToPrincipalProperty, + MemberInfo? navigationToDependentProperty, + EntityType principalEntityType, + EntityType dependentEntityType, + bool shouldThrow, + out bool? shouldInvert, + out bool? shouldBeUnique) + { + shouldInvert = null; + shouldBeUnique = null; + + bool? invertedShouldBeUnique = null; + if (navigationToPrincipalProperty != null + && !IsCompatible( + navigationToPrincipalProperty, + pointsToPrincipal: false, + principalEntityType, + dependentEntityType, + shouldThrow: false, + out invertedShouldBeUnique)) + { + shouldInvert = false; + } + + if (navigationToDependentProperty != null + && !IsCompatible( + navigationToDependentProperty, + pointsToPrincipal: true, + principalEntityType, + dependentEntityType, + shouldThrow: false, + out _)) + { + shouldInvert = false; + } + + if (navigationToPrincipalProperty != null + && !IsCompatible( + navigationToPrincipalProperty, + pointsToPrincipal: true, + dependentEntityType, + principalEntityType, + shouldThrow && shouldInvert != null, + out _)) + { + if (shouldInvert != null) + { + return false; + } + + shouldInvert = true; + } + + if (navigationToDependentProperty != null + && !IsCompatible( + navigationToDependentProperty, + pointsToPrincipal: false, + dependentEntityType, + principalEntityType, + shouldThrow && shouldInvert != null, + out shouldBeUnique)) + { + if (shouldInvert != null) + { + return false; + } + + shouldInvert = true; + } + + if (shouldInvert == true) + { + shouldBeUnique = invertedShouldBeUnique; + } + + return true; + } + private static bool IsCompatible( MemberInfo navigationMember, bool pointsToPrincipal, @@ -1028,123 +1067,130 @@ public virtual bool CanSetIsRequiredDependent(bool? required, ConfigurationSourc return null; } + if (!ownership.Value) + { + Metadata.SetIsOwnership(ownership: false, configurationSource); + + return this; + } + + if (!Metadata.DeclaringEntityType.Builder.CanSetIsOwned(true, configurationSource)) + { + return null; + } + var declaringType = Metadata.DeclaringEntityType; - if (ownership.Value) + var newRelationshipBuilder = this; + var otherOwnership = declaringType.GetDeclaredForeignKeys().SingleOrDefault(fk => fk.IsOwnership); + var invertedOwnerships = declaringType.GetDeclaredReferencingForeignKeys() + .Where(fk => fk.IsOwnership && fk.DeclaringEntityType.ClrType == Metadata.PrincipalEntityType.ClrType).ToList(); + + if (invertedOwnerships.Any(fk => !configurationSource.Overrides(fk.GetConfigurationSource()))) { - var newRelationshipBuilder = this; - var otherOwnership = declaringType.GetDeclaredForeignKeys().SingleOrDefault(fk => fk.IsOwnership); - var invertedOwnerships = declaringType.GetDeclaredReferencingForeignKeys() - .Where(fk => fk.IsOwnership && fk.DeclaringEntityType.ClrType == Metadata.PrincipalEntityType.ClrType).ToList(); + return null; + } - if (invertedOwnerships.Any(fk => !configurationSource.Overrides(fk.GetConfigurationSource()))) + if (declaringType.HasSharedClrType) + { + if (otherOwnership != null + && !configurationSource.Overrides(otherOwnership.GetConfigurationSource())) { return null; } - if (declaringType.HasSharedClrType) + Metadata.SetIsOwnership(ownership: true, configurationSource); + newRelationshipBuilder = newRelationshipBuilder?.OnDelete(DeleteBehavior.Cascade, ConfigurationSource.Convention); + + if (newRelationshipBuilder == null) { - if (otherOwnership != null - && !configurationSource.Overrides(otherOwnership.GetConfigurationSource())) - { - return null; - } + return null; + } - Metadata.SetIsOwnership(ownership: true, configurationSource); - newRelationshipBuilder = newRelationshipBuilder?.OnDelete(DeleteBehavior.Cascade, ConfigurationSource.Convention); + if (otherOwnership?.IsInModel == true) + { + otherOwnership.DeclaringEntityType.Builder.HasNoRelationship(otherOwnership, configurationSource); + } - if (newRelationshipBuilder == null) + foreach (var invertedOwnership in invertedOwnerships) + { + if (invertedOwnership.IsInModel) { - return null; + invertedOwnership.DeclaringEntityType.Builder.HasNoRelationship(invertedOwnership, configurationSource); } + } - if (otherOwnership?.IsInModel == true) - { - otherOwnership.DeclaringEntityType.Builder.HasNoRelationship(otherOwnership, configurationSource); - } + newRelationshipBuilder.Metadata.DeclaringEntityType.Builder.IsOwned(true, configurationSource); - foreach (var invertedOwnership in invertedOwnerships) - { - if (invertedOwnership.IsInModel) - { - invertedOwnership.DeclaringEntityType.Builder.HasNoRelationship(invertedOwnership, configurationSource); - } - } + return newRelationshipBuilder; + } - return newRelationshipBuilder; - } + if (otherOwnership != null) + { + Check.DebugAssert(Metadata.DeclaringEntityType.IsOwned(), + $"Expected {Metadata.DeclaringEntityType} to be owned"); - if (otherOwnership != null) + if (!Metadata.GetConfigurationSource().Overrides(ConfigurationSource.Explicit) + && Metadata.PrincipalEntityType.IsInOwnershipPath(Metadata.DeclaringEntityType.ClrType)) { - if (!Metadata.GetConfigurationSource().Overrides(ConfigurationSource.Explicit) - && Metadata.PrincipalEntityType.IsInOwnershipPath(Metadata.DeclaringEntityType.ClrType)) - { - return null; - } - - Metadata.SetIsOwnership(ownership: true, configurationSource); + return null; + } - using var batch = ModelBuilder.Metadata.DelayConventions(); + Metadata.SetIsOwnership(ownership: true, configurationSource); - newRelationshipBuilder = newRelationshipBuilder.OnDelete(DeleteBehavior.Cascade, ConfigurationSource.Convention); - if (newRelationshipBuilder == null) - { - return null; - } + using var batch = ModelBuilder.Metadata.DelayConventions(); - foreach (var invertedOwnership in invertedOwnerships) - { - invertedOwnership.DeclaringEntityType.Builder.HasNoRelationship(invertedOwnership, configurationSource); - } + newRelationshipBuilder = newRelationshipBuilder.OnDelete(DeleteBehavior.Cascade, ConfigurationSource.Convention); + if (newRelationshipBuilder == null) + { + return null; + } - var fk = newRelationshipBuilder.Metadata; - fk.DeclaringEntityType.Builder.HasNoRelationship(fk, fk.GetConfigurationSource()); + foreach (var invertedOwnership in invertedOwnerships) + { + invertedOwnership.DeclaringEntityType.Builder.HasNoRelationship(invertedOwnership, configurationSource); + } - if (otherOwnership.Builder.MakeDeclaringTypeShared(configurationSource) == null) - { - return null; - } + var fk = newRelationshipBuilder.Metadata; + fk.DeclaringEntityType.Builder.HasNoRelationship(fk, fk.GetConfigurationSource()); - var name = Metadata.PrincipalEntityType.GetOwnedName(declaringType.ShortName(), Metadata.PrincipalToDependent!.Name); - var newEntityType = ModelBuilder.SharedTypeEntity( - name, - declaringType.ClrType, - declaringType.GetConfigurationSource(), - shouldBeOwned: true)!.Metadata; + if (otherOwnership.Builder.MakeDeclaringTypeShared(configurationSource) == null) + { + return null; + } - newRelationshipBuilder = newRelationshipBuilder.Attach(newEntityType.Builder)!; + var name = Metadata.PrincipalEntityType.GetOwnedName(declaringType.ShortName(), Metadata.PrincipalToDependent!.Name); + var newEntityType = ModelBuilder.SharedTypeEntity( + name, + declaringType.ClrType, + declaringType.GetConfigurationSource(), + shouldBeOwned: true)!.Metadata; - ModelBuilder.Metadata.ConventionDispatcher.Tracker.Update( - Metadata, newRelationshipBuilder.Metadata); + newRelationshipBuilder = newRelationshipBuilder.Attach(newEntityType.Builder)!; - return batch.Run(newRelationshipBuilder); - } + ModelBuilder.Metadata.ConventionDispatcher.Tracker.Update( + Metadata, newRelationshipBuilder.Metadata); - using (var batch = ModelBuilder.Metadata.DelayConventions()) - { - Metadata.SetIsOwnership(ownership: true, configurationSource); - newRelationshipBuilder = newRelationshipBuilder.OnDelete(DeleteBehavior.Cascade, ConfigurationSource.Convention) - ?? newRelationshipBuilder; + return batch.Run(newRelationshipBuilder); + } - foreach (var invertedOwnership in invertedOwnerships) - { - invertedOwnership.DeclaringEntityType.Builder.HasNoRelationship(invertedOwnership, configurationSource); - } + using (var batch = ModelBuilder.Metadata.DelayConventions()) + { + var declaringEntityTypeBuilder = + newRelationshipBuilder.Metadata.DeclaringEntityType.Builder.IsOwned(true, configurationSource, Metadata); - newRelationshipBuilder.Metadata.DeclaringEntityType.Builder.HasBaseType((Type?)null, configurationSource); + Check.DebugAssert(declaringEntityTypeBuilder != null, "Expected declared type to be owned"); - if (!newRelationshipBuilder.Metadata.DeclaringEntityType.Builder - .RemoveNonOwnershipRelationships(newRelationshipBuilder.Metadata, configurationSource)) - { - return null; - } + Metadata.SetIsOwnership(true, configurationSource); + newRelationshipBuilder = newRelationshipBuilder.OnDelete(DeleteBehavior.Cascade, ConfigurationSource.Convention) + ?? newRelationshipBuilder; - return batch.Run(newRelationshipBuilder); + foreach (var invertedOwnership in invertedOwnerships) + { + invertedOwnership.DeclaringEntityType.Builder.HasNoRelationship(invertedOwnership, configurationSource); } - } - Metadata.SetIsOwnership(ownership: false, configurationSource); - - return this; + return batch.Run(newRelationshipBuilder); + } } /// @@ -1154,7 +1200,10 @@ public virtual bool CanSetIsRequiredDependent(bool? required, ConfigurationSourc /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual bool CanSetIsOwnership(bool? ownership, ConfigurationSource? configurationSource) - => Metadata.IsOwnership == ownership || configurationSource.Overrides(Metadata.GetIsOwnershipConfigurationSource()); + => (Metadata.IsOwnership == ownership || configurationSource.Overrides(Metadata.GetIsOwnershipConfigurationSource())) + && (ownership != true + || Metadata.DeclaringEntityType.IsOwned() + || configurationSource.OverridesStrictly(Metadata.GetConfigurationSource())); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -1325,42 +1374,7 @@ private bool CanSetIsUnique(bool? unique, ConfigurationSource? configurationSour return true; } - // Note: These will not invert relationships, use RelatedEntityTypes for that - /// - /// 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 InternalForeignKeyBuilder? DependentEntityType( - InternalEntityTypeBuilder dependentEntityTypeBuilder, - ConfigurationSource configurationSource) - => DependentEntityType(dependentEntityTypeBuilder.Metadata, 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 InternalForeignKeyBuilder? DependentEntityType( - Type dependentType, - ConfigurationSource configurationSource) - => DependentEntityType( - ModelBuilder.Entity(dependentType, configurationSource)!.Metadata, - 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 InternalForeignKeyBuilder? DependentEntityType( - string dependentTypeName, - ConfigurationSource configurationSource) - => DependentEntityType(ModelBuilder.Entity(dependentTypeName, configurationSource)!.Metadata, configurationSource); - + // Note: This will not invert relationships, use RelatedEntityTypes for that /// /// 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 @@ -1393,44 +1407,7 @@ private bool CanSetIsUnique(bool? unique, ConfigurationSource? configurationSour : null; } - // Note: These will not invert relationships, use RelatedEntityTypes for that - /// - /// 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 InternalForeignKeyBuilder? PrincipalEntityType( - InternalEntityTypeBuilder principalEntityTypeBuilder, - ConfigurationSource configurationSource) - => PrincipalEntityType(principalEntityTypeBuilder.Metadata, 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 InternalForeignKeyBuilder? PrincipalEntityType( - Type principalType, - ConfigurationSource configurationSource) - => PrincipalEntityType( - ModelBuilder.Entity(principalType, configurationSource)!.Metadata, - 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 InternalForeignKeyBuilder? PrincipalEntityType( - string principalTypeName, - ConfigurationSource configurationSource) - => PrincipalEntityType( - ModelBuilder.Entity(principalTypeName, configurationSource)!.Metadata, - configurationSource); - + // Note: This will not invert relationships, use RelatedEntityTypes for that /// /// 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 @@ -2224,23 +2201,23 @@ private bool CanSetPrincipalKey( isUnique ??= ((Metadata.GetIsUniqueConfigurationSource()?.Overrides(configurationSource) ?? false) ? Metadata.IsUnique - : (bool?)null); + : null); isRequired ??= !oldRelationshipInverted ? ((Metadata.GetIsRequiredConfigurationSource()?.Overrides(configurationSource) ?? false) ? Metadata.IsRequired - : (bool?)null) + : null) : ((Metadata.GetIsRequiredDependentConfigurationSource()?.Overrides(ConfigurationSource.Explicit) ?? false) ? Metadata.IsRequiredDependent - : (bool?)null); + : null); isRequiredDependent ??= !oldRelationshipInverted ? ((Metadata.GetIsRequiredDependentConfigurationSource()?.Overrides(configurationSource) ?? false) ? Metadata.IsRequiredDependent - : (bool?)null) + : null) : ((Metadata.GetIsRequiredConfigurationSource()?.Overrides(ConfigurationSource.Explicit) ?? false) ? Metadata.IsRequired - : (bool?)null); + : null); isOwnership ??= !oldRelationshipInverted && (Metadata.GetIsOwnershipConfigurationSource()?.Overrides(configurationSource) ?? false) @@ -2249,7 +2226,7 @@ private bool CanSetPrincipalKey( deleteBehavior ??= ((Metadata.GetDeleteBehaviorConfigurationSource()?.Overrides(configurationSource) ?? false) ? Metadata.DeleteBehavior - : (DeleteBehavior?)null); + : null); principalEndConfigurationSource ??= (principalEntityTypeBuilder.Metadata != dependentEntityTypeBuilder.Metadata && ((principalProperties?.Count > 0) @@ -2257,7 +2234,7 @@ private bool CanSetPrincipalKey( || (navigationToDependent != null && isUnique == false) || isOwnership == true) ? configurationSource - : (ConfigurationSource?)null); + : null); principalEndConfigurationSource = principalEndConfigurationSource.Max(Metadata.GetPrincipalEndConfigurationSource()); return ReplaceForeignKey( @@ -2527,7 +2504,7 @@ private bool CanSetPrincipalKey( dependentProperties?.Count == 0 ? null : dependentProperties, principalKey, navigationToPrincipal?.Name - ?? referencingSkipNavigations?.FirstOrDefault().Navigation?.Inverse?.Name, + ?? referencingSkipNavigations?.FirstOrDefault().Navigation?.Inverse?.Name, isRequired, configurationSource: null)!; @@ -3139,7 +3116,9 @@ private InternalForeignKeyBuilder MergeFacetsFrom(Navigation newNavigation, Navi resolution |= Resolution.ResetToPrincipal; sameConfigurationSource = true; } - else + else if (!configurationSource.HasValue + || !matchingRelationship.Metadata.DeclaringEntityType.Builder + .CanRemoveForeignKey(matchingRelationship.Metadata, configurationSource.Value)) { resolvable = false; } @@ -3171,7 +3150,9 @@ private InternalForeignKeyBuilder MergeFacetsFrom(Navigation newNavigation, Navi resolution |= Resolution.ResetToDependent; sameConfigurationSource = true; } - else + else if (!configurationSource.HasValue + || !matchingRelationship.Metadata.DeclaringEntityType.Builder + .CanRemoveForeignKey(matchingRelationship.Metadata, configurationSource.Value)) { resolvable = false; } @@ -3207,7 +3188,9 @@ private InternalForeignKeyBuilder MergeFacetsFrom(Navigation newNavigation, Navi resolution |= Resolution.ResetToDependent; sameConfigurationSource = true; } - else + else if (!configurationSource.HasValue + || !matchingRelationship.Metadata.DeclaringEntityType.Builder + .CanRemoveForeignKey(matchingRelationship.Metadata, configurationSource.Value)) { resolvable = false; } @@ -3239,7 +3222,9 @@ private InternalForeignKeyBuilder MergeFacetsFrom(Navigation newNavigation, Navi resolution |= Resolution.ResetToPrincipal; sameConfigurationSource = true; } - else + else if (!configurationSource.HasValue + || !matchingRelationship.Metadata.DeclaringEntityType.Builder + .CanRemoveForeignKey(matchingRelationship.Metadata, configurationSource.Value)) { resolvable = false; } @@ -3589,15 +3574,6 @@ private static IReadOnlyList FindRelationships( } } - if (!Metadata.GetConfigurationSource().Overrides(ConfigurationSource.Explicit) - && principalEntityType.IsOwned() - && Metadata.DependentToPrincipal != null - && !Metadata.IsOwnership) - { - // An entity type can have a navigation to a principal owned type only if it's the owner - return null; - } - InternalEntityTypeBuilder dependentEntityTypeBuilder; EntityType? dependentEntityType; if (Metadata.DeclaringEntityType.IsInModel) @@ -3634,7 +3610,7 @@ private static IReadOnlyList FindRelationships( name, Metadata.DeclaringEntityType.ClrType, Metadata.DeclaringEntityType.GetConfigurationSource(), - shouldBeOwned: null)!.Metadata; + shouldBeOwned: true)!.Metadata; } else { @@ -3644,7 +3620,11 @@ private static IReadOnlyList FindRelationships( else { dependentEntityType = - ModelBuilder.Entity(Metadata.DeclaringEntityType.ClrType, configurationSource)!.Metadata; + ModelBuilder.Entity( + Metadata.DeclaringEntityType.ClrType, + configurationSource, + shouldBeOwned: Metadata.DeclaringEntityType.IsOwned()) + !.Metadata; } } } @@ -3653,12 +3633,25 @@ private static IReadOnlyList FindRelationships( } } - if (!Metadata.GetConfigurationSource().Overrides(ConfigurationSource.Explicit) - && dependentEntityType.IsOwned() - && Metadata.PrincipalToDependent != null) + if (!Metadata.IsOwnership + && !Metadata.GetConfigurationSource().Overrides(ConfigurationSource.Explicit)) { - // Only the owner can have a navigation to an owned type - return null; + if (principalEntityType.IsOwned() + && Metadata.DependentToPrincipal != null + && !dependentEntityType.IsInOwnershipPath(principalEntityType)) + { + // An entity type can have a navigation to a principal owned type only if it's in the ownership path + return null; + } + + if (dependentEntityType.IsOwned() + && Metadata.PrincipalToDependent != null + && (dependentEntityType.FindOwnership()?.PrincipalEntityType == principalEntityType + || !dependentEntityType.IsInOwnershipPath(principalEntityType))) + { + // Only a type in the ownership path can have a navigation to an owned dependent + return null; + } } if (dependentEntityType.GetForeignKeys().Contains(Metadata, LegacyReferenceEqualityComparer.Instance)) diff --git a/src/EFCore/Metadata/Internal/InternalModelBuilder.cs b/src/EFCore/Metadata/Internal/InternalModelBuilder.cs index 41775199960..4e08fb45aae 100644 --- a/src/EFCore/Metadata/Internal/InternalModelBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalModelBuilder.cs @@ -73,7 +73,7 @@ public override InternalModelBuilder ModelBuilder public virtual InternalEntityTypeBuilder? Entity( Type type, ConfigurationSource configurationSource, - bool? shouldBeOwned = false) + bool? shouldBeOwned = null) => Entity(new TypeIdentity(type, Metadata), configurationSource, shouldBeOwned); private InternalEntityTypeBuilder? Entity( @@ -86,28 +86,18 @@ public override InternalModelBuilder ModelBuilder return null; } - if (type.Type != null) + if (type.Type != null + && shouldBeOwned != null) { - if (shouldBeOwned == null) - { - if (configurationSource != ConfigurationSource.Explicit - && Metadata.Configuration?.GetConfigurationType(type.Type).IsEntityType() == false) - { - return null; - } - } - else - { - var configurationType = shouldBeOwned.Value - ? TypeConfigurationType.OwnedEntityType - : type.IsNamed - ? TypeConfigurationType.SharedTypeEntityType - : TypeConfigurationType.EntityType; + var configurationType = shouldBeOwned.Value + ? TypeConfigurationType.OwnedEntityType + : type.IsNamed + ? TypeConfigurationType.SharedTypeEntityType + : TypeConfigurationType.EntityType; - if (!CanBeConfigured(type.Type, configurationType, configurationSource)) - { - return null; - } + if (!CanBeConfigured(type.Type, configurationType, configurationSource)) + { + return null; } } @@ -122,12 +112,8 @@ public override InternalModelBuilder ModelBuilder entityType = Metadata.FindEntityType(clrType); if (entityType != null) { - if (entityType.Name == type.Name - && entityType.HasSharedClrType) - { - entityType.UpdateConfigurationSource(configurationSource); - return entityType.Builder; - } + Check.DebugAssert(entityType.Name != type.Name || !entityType.HasSharedClrType, + "Shared type entity types shouldn't be named the same as non-shared"); if (!configurationSource.OverridesStrictly(entityType.GetConfigurationSource()) && !entityType.IsOwned()) @@ -139,6 +125,7 @@ public override InternalModelBuilder ModelBuilder entityTypeSnapshot = InternalEntityTypeBuilder.DetachAllMembers(entityType); + // TODO: Use convention batch to track replaced entity type, see #15898 HasNoEntityType(entityType, ConfigurationSource.Explicit); } } @@ -159,34 +146,18 @@ public override InternalModelBuilder ModelBuilder } if (shouldBeOwned == false - && (ShouldBeOwnedType(type) - || entityType != null && entityType.IsOwned())) + && clrType != null + && (!configurationSource.OverridesStrictly(Metadata.FindIsOwnedConfigurationSource(clrType)) + || (Metadata.Configuration?.GetConfigurationType(clrType) == TypeConfigurationType.OwnedEntityType + && configurationSource != ConfigurationSource.Explicit))) { - // We always throw as configuring a type as owned always comes from user (through Explicit/DataAnnotation) - throw new InvalidOperationException( - CoreStrings.ClashingOwnedEntityType(clrType == null ? type.Name : clrType.ShortDisplayName())); - } - - if (shouldBeOwned == true - && entityType != null) - { - if (!entityType.IsOwned() - && configurationSource == ConfigurationSource.Explicit - && entityType.GetConfigurationSource() == ConfigurationSource.Explicit) + if (configurationSource == ConfigurationSource.Explicit) { - throw new InvalidOperationException(CoreStrings.ClashingNonOwnedEntityType(clrType!.ShortDisplayName())); + throw new InvalidOperationException( + CoreStrings.ClashingOwnedEntityType(clrType == null ? type.Name : clrType.ShortDisplayName())); } - foreach (var derivedType in entityType.GetDerivedTypes()) - { - if (!derivedType.IsOwned() - && configurationSource == ConfigurationSource.Explicit - && derivedType.GetConfigurationSource() == ConfigurationSource.Explicit) - { - throw new InvalidOperationException( - CoreStrings.ClashingNonOwnedDerivedEntityType(entityType.DisplayName(), derivedType.DisplayName())); - } - } + return null; } if (entityType != null) @@ -194,6 +165,12 @@ public override InternalModelBuilder ModelBuilder if (type.Type == null || entityType.ClrType == type.Type) { + if (shouldBeOwned.HasValue + && entityType.Builder.IsOwned(shouldBeOwned.Value, configurationSource) == null) + { + return null; + } + entityType.UpdateConfigurationSource(configurationSource); return entityType.Builder; } @@ -211,12 +188,50 @@ public override InternalModelBuilder ModelBuilder } } + if (type.Type != null) + { + if (shouldBeOwned == null) + { + var configurationType = Metadata.Configuration?.GetConfigurationType(type.Type); + switch (configurationType) + { + case null: + break; + case TypeConfigurationType.EntityType: + case TypeConfigurationType.SharedTypeEntityType: + { + shouldBeOwned ??= false; + break; + } + case TypeConfigurationType.OwnedEntityType: + { + shouldBeOwned ??= true; + break; + } + default: + { + if (configurationSource != ConfigurationSource.Explicit) + { + return null; + } + break; + } + } + + shouldBeOwned ??= Metadata.FindIsOwnedConfigurationSource(type.Type) != null; + } + } + else if (shouldBeOwned == null) + { + return null; + } + Metadata.RemoveIgnored(type.Name); entityType = type.IsNamed ? clrType == null - ? Metadata.AddEntityType(type.Name, configurationSource) - : Metadata.AddEntityType(type.Name, clrType, configurationSource) - : Metadata.AddEntityType(clrType!, configurationSource); + ? Metadata.AddEntityType(type.Name, shouldBeOwned.Value, configurationSource) + : Metadata.AddEntityType(type.Name, clrType, shouldBeOwned.Value, configurationSource) + : Metadata.AddEntityType(clrType!, shouldBeOwned.Value, configurationSource); if (entityType != null && entityTypeSnapshot != null) @@ -304,59 +319,31 @@ public override InternalModelBuilder ModelBuilder Metadata.RemoveIgnored(type); Metadata.AddOwned(type, ConfigurationSource.Explicit); - foreach (var entityType in Metadata.FindEntityTypes(type)) + foreach (var entityType in Metadata.FindEntityTypes(type).ToList()) { - if (entityType.IsOwned()) + if (entityType.FindOwnership() != null) { continue; } - if (!configurationSource.Overrides(entityType.GetConfigurationSource())) - { - return null; - } - - if (entityType.GetConfigurationSource() == ConfigurationSource.Explicit) - { - throw new InvalidOperationException(CoreStrings.ClashingNonOwnedEntityType(type.ShortDisplayName())); - } - - foreach (var derivedType in entityType.GetDerivedTypes()) - { - if (!derivedType.IsOwned() - && configurationSource == ConfigurationSource.Explicit - && derivedType.GetConfigurationSource() == ConfigurationSource.Explicit) - { - throw new InvalidOperationException( - CoreStrings.ClashingNonOwnedDerivedEntityType(type.ShortDisplayName(), derivedType.ShortName())); - } - } - var ownershipCandidates = entityType.GetForeignKeys().Where( fk => fk.PrincipalToDependent != null && !fk.PrincipalEntityType.IsInOwnershipPath(type)).ToList(); - if (ownershipCandidates.Count == 1) + if (ownershipCandidates.Count >= 1) { if (ownershipCandidates[0].Builder.IsOwnership(true, configurationSource) == null) { return null; } } - else if (ownershipCandidates.Count > 1) + else { - using (var batch = ModelBuilder.Metadata.DelayConventions()) + if (entityType.Builder.CanSetIsOwned(true, configurationSource)) { - var ownership = ownershipCandidates[0].Builder.IsOwnership(true, configurationSource); - if (ownership == null) - { - return null; - } - ownership.MakeDeclaringTypeShared(configurationSource); + // Discover the ownership when the type is added back + HasNoEntityType(entityType, configurationSource); } - } - else - { - if (!entityType.Builder.RemoveNonOwnershipRelationships(null, configurationSource)) + else { return null; } @@ -366,7 +353,7 @@ public override InternalModelBuilder ModelBuilder return new InternalOwnedEntityTypeBuilder(); } - private bool ShouldBeOwnedType(in TypeIdentity type) + private bool IsOwned(in TypeIdentity type) => type.Type != null && Metadata.IsOwned(type.Type); /// @@ -521,7 +508,7 @@ private bool CanIgnore(in TypeIdentity type, ConfigurationSource configurationSo return true; } - if (ShouldBeOwnedType(type) + if (IsOwned(type) && configurationSource != ConfigurationSource.Explicit) { return false; diff --git a/src/EFCore/Metadata/Internal/MemberClassifier.cs b/src/EFCore/Metadata/Internal/MemberClassifier.cs index 5f03e435d08..07d4c842e31 100644 --- a/src/EFCore/Metadata/Internal/MemberClassifier.cs +++ b/src/EFCore/Metadata/Internal/MemberClassifier.cs @@ -53,23 +53,25 @@ public MemberClassifier( /// 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 ImmutableSortedDictionary GetNavigationCandidates(IConventionEntityType entityType) + public virtual ImmutableSortedDictionary GetNavigationCandidates( + IConventionEntityType entityType) { if (entityType.FindAnnotation(CoreAnnotationNames.NavigationCandidates)?.Value - is ImmutableSortedDictionary navigationCandidates) + is ImmutableSortedDictionary navigationCandidates) { return navigationCandidates; } - var dictionaryBuilder = ImmutableSortedDictionary.CreateBuilder(MemberInfoNameComparer.Instance); + var dictionaryBuilder = ImmutableSortedDictionary.CreateBuilder( + MemberInfoNameComparer.Instance); var configuration = ((Model)entityType.Model).Configuration; foreach (var propertyInfo in entityType.GetRuntimeProperties().Values) { - var targetType = FindCandidateNavigationPropertyType(propertyInfo, configuration); + var targetType = FindCandidateNavigationPropertyType(propertyInfo, configuration, out var shouldBeOwned); if (targetType != null) { - dictionaryBuilder[propertyInfo] = targetType; + dictionaryBuilder[propertyInfo] = (targetType, shouldBeOwned); } } @@ -80,6 +82,7 @@ public virtual ImmutableSortedDictionary GetNavigationCandid { entityType.Builder.HasAnnotation(CoreAnnotationNames.NavigationCandidates, navigationCandidates); } + return navigationCandidates; } @@ -89,44 +92,50 @@ public virtual ImmutableSortedDictionary GetNavigationCandid /// 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? FindCandidateNavigationPropertyType(MemberInfo memberInfo, ModelConfiguration? configuration) + public virtual Type? FindCandidateNavigationPropertyType( + MemberInfo memberInfo, IConventionModel model, out bool? shouldBeOwned) + => FindCandidateNavigationPropertyType(memberInfo, ((Model)model).Configuration, out shouldBeOwned); + + private Type? FindCandidateNavigationPropertyType( + MemberInfo memberInfo, ModelConfiguration? configuration, out bool? shouldBeOwned) { + shouldBeOwned = null; + var propertyInfo = memberInfo as PropertyInfo; var targetType = memberInfo.GetMemberType(); var targetSequenceType = targetType.TryGetSequenceType(); - if (!(memberInfo is PropertyInfo propertyInfo) - || !propertyInfo.IsCandidateProperty(targetSequenceType == null)) - { - return null; - } - - var isConfiguredAsEntityType = configuration?.GetConfigurationType(targetType).IsEntityType(); - if (isConfiguredAsEntityType == false) - { - return null; - } + return targetSequenceType != null + && (propertyInfo == null + || propertyInfo.IsCandidateProperty(needsWrite: false)) + && IsCandidateNavigationPropertyType(targetSequenceType, memberInfo, configuration, out shouldBeOwned) + ? targetSequenceType + : (propertyInfo == null + || propertyInfo.IsCandidateProperty(needsWrite: true)) + && IsCandidateNavigationPropertyType(targetType, memberInfo, configuration, out shouldBeOwned) + ? targetType + : null; + } - if (targetSequenceType != null) + private bool IsCandidateNavigationPropertyType( + Type targetType, MemberInfo memberInfo, ModelConfiguration? configuration, out bool? shouldBeOwned) + { + shouldBeOwned = null; + var configurationType = configuration?.GetConfigurationType(targetType); + var isConfiguredAsEntityType = configurationType.IsEntityType(); + if (isConfiguredAsEntityType == false + || !targetType.IsValidEntityType()) { - isConfiguredAsEntityType ??= configuration?.GetConfigurationType(targetSequenceType).IsEntityType(); - if (isConfiguredAsEntityType == false) - { - return null; - } + return false; } - targetType = targetSequenceType ?? targetType; - if (!targetType.IsValidEntityType()) + if (configurationType != null) { - return null; + shouldBeOwned = configurationType == TypeConfigurationType.OwnedEntityType; } - targetType = targetType.UnwrapNullableType(); - return isConfiguredAsEntityType == null - && (targetType == typeof(object) - || _parameterBindingFactories.FindFactory(targetType, memberInfo.GetSimpleMemberName()) != null - || _typeMappingSource.FindMapping(targetType) != null) - ? null - : targetType; + return isConfiguredAsEntityType == true + || (targetType != typeof(object) + && _parameterBindingFactories.FindFactory(memberInfo.GetMemberType(), memberInfo.GetSimpleMemberName()) == null + && _typeMappingSource.FindMapping(targetType) == null); } /// diff --git a/src/EFCore/Metadata/Internal/Model.cs b/src/EFCore/Metadata/Internal/Model.cs index 48343d0ee9b..24ec38b34ba 100644 --- a/src/EFCore/Metadata/Internal/Model.cs +++ b/src/EFCore/Metadata/Internal/Model.cs @@ -47,6 +47,7 @@ public class Model : ConventionAnnotatable, IMutableModel, IConventionModel, IRu private readonly ConcurrentDictionary _indexerPropertyInfoMap = new(); private readonly ConcurrentDictionary _clrTypeNameMap = new(); private readonly Dictionary _ignoredTypeNames = new(StringComparer.Ordinal); + private Dictionary? _ownedTypes; private readonly Dictionary Types)> _sharedTypes = new() { { DefaultPropertyBagType, (ConfigurationSource.Convention, new SortedSet(EntityTypeFullNameComparer.Instance)) } }; @@ -153,11 +154,12 @@ public virtual IEnumerable GetEntityTypes() /// public virtual EntityType? AddEntityType( string name, + bool owned, ConfigurationSource configurationSource) { Check.NotEmpty(name, nameof(name)); - var entityType = new EntityType(name, this, configurationSource); + var entityType = new EntityType(name, this, owned, configurationSource); return AddEntityType(entityType); } @@ -170,11 +172,12 @@ public virtual IEnumerable GetEntityTypes() /// public virtual EntityType? AddEntityType( Type type, + bool owned, ConfigurationSource configurationSource) { Check.NotNull(type, nameof(type)); - var entityType = new EntityType(type, this, configurationSource); + var entityType = new EntityType(type, this, owned, configurationSource); return AddEntityType(entityType); } @@ -188,6 +191,7 @@ public virtual IEnumerable GetEntityTypes() public virtual EntityType? AddEntityType( string name, Type type, + bool owned, ConfigurationSource configurationSource) { Check.NotEmpty(name, nameof(name)); @@ -198,7 +202,7 @@ public virtual IEnumerable GetEntityTypes() throw new InvalidOperationException(CoreStrings.AmbiguousSharedTypeEntityTypeName(name)); } - var entityType = new EntityType(name, type, this, configurationSource); + var entityType = new EntityType(name, type, this, owned, configurationSource); return AddEntityType(entityType); } @@ -360,7 +364,7 @@ private static void AssertCanRemove(EntityType entityType) Check.NotEmpty(name, nameof(name)); name = definingEntityType.GetOwnedName(name, definingNavigationName); - var entityType = new EntityType(name, DefaultPropertyBagType, this, configurationSource); + var entityType = new EntityType(name, DefaultPropertyBagType, this, owned: true, configurationSource); return AddEntityType(entityType); } @@ -380,7 +384,7 @@ private static void AssertCanRemove(EntityType entityType) Check.NotNull(type, nameof(type)); var name = definingEntityType.GetOwnedName(type.ShortDisplayName(), definingNavigationName); - var entityType = new EntityType(name, type, this, configurationSource); + var entityType = new EntityType(name, type, this, owned: true, configurationSource); return AddEntityType(entityType); } @@ -508,7 +512,8 @@ 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); + => _sharedTypes.ContainsKey(type) + || Configuration?.GetConfigurationType(type) == TypeConfigurationType.SharedTypeEntityType; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -650,7 +655,8 @@ public virtual bool IsIgnoredType(Type type) /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual bool IsOwned(Type type) - => FindIsOwnedConfigurationSource(type) != null; + => FindIsOwnedConfigurationSource(type) != null + || Configuration?.GetConfigurationType(type) == TypeConfigurationType.OwnedEntityType; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -660,7 +666,7 @@ public virtual bool IsOwned(Type type) /// public virtual ConfigurationSource? FindIsOwnedConfigurationSource(Type type) { - if (this[CoreAnnotationNames.OwnedTypes] is not Dictionary ownedTypes) + if (_ownedTypes == null) { return null; } @@ -669,7 +675,7 @@ public virtual bool IsOwned(Type type) while (currentType != null) { - if (ownedTypes.TryGetValue(GetDisplayName(currentType), out var configurationSource)) + if (_ownedTypes.TryGetValue(GetDisplayName(currentType), out var configurationSource)) { return configurationSource; } @@ -690,19 +696,15 @@ public virtual void AddOwned(Type type, ConfigurationSource configurationSource) { EnsureMutable(); var name = GetDisplayName(type); - if (!(this[CoreAnnotationNames.OwnedTypes] is Dictionary ownedTypes)) - { - ownedTypes = new Dictionary(StringComparer.Ordinal); - this[CoreAnnotationNames.OwnedTypes] = ownedTypes; - } + _ownedTypes ??= new(StringComparer.Ordinal); - if (ownedTypes.TryGetValue(name, out var oldConfigurationSource)) + if (_ownedTypes.TryGetValue(name, out var oldConfigurationSource)) { - ownedTypes[name] = configurationSource.Max(oldConfigurationSource); + _ownedTypes[name] = configurationSource.Max(oldConfigurationSource); return; } - ownedTypes.Add(name, configurationSource); + _ownedTypes.Add(name, configurationSource); } /// @@ -715,13 +717,13 @@ public virtual void AddOwned(Type type, ConfigurationSource configurationSource) { EnsureMutable(); - if (this[CoreAnnotationNames.OwnedTypes] is not Dictionary ownedTypes) + if (_ownedTypes == null) { return null; } var name = GetDisplayName(type); - return ownedTypes.Remove(name) ? name : null; + return _ownedTypes.Remove(name) ? name : null; } /// @@ -1248,7 +1250,7 @@ IEnumerable IModel.FindEntityTypes(Type type) /// [DebuggerStepThrough] IMutableEntityType IMutableModel.AddEntityType(string name) - => AddEntityType(name, ConfigurationSource.Explicit)!; + => AddEntityType(name, owned: false, ConfigurationSource.Explicit)!; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -1258,7 +1260,7 @@ IMutableEntityType IMutableModel.AddEntityType(string name) /// [DebuggerStepThrough] IConventionEntityType? IConventionModel.AddEntityType(string name, bool fromDataAnnotation) - => AddEntityType(name, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + => AddEntityType(name, owned: false, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -1268,7 +1270,7 @@ IMutableEntityType IMutableModel.AddEntityType(string name) /// [DebuggerStepThrough] IMutableEntityType IMutableModel.AddEntityType(Type type) - => AddEntityType(type, ConfigurationSource.Explicit)!; + => AddEntityType(type, owned: false, ConfigurationSource.Explicit)!; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -1278,7 +1280,7 @@ IMutableEntityType IMutableModel.AddEntityType(Type type) /// [DebuggerStepThrough] IConventionEntityType? IConventionModel.AddEntityType(Type type, bool fromDataAnnotation) - => AddEntityType(type, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + => AddEntityType(type, owned: false, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -1288,7 +1290,7 @@ IMutableEntityType IMutableModel.AddEntityType(Type type) /// [DebuggerStepThrough] IMutableEntityType IMutableModel.AddEntityType(string name, Type type) - => AddEntityType(name, type, ConfigurationSource.Explicit)!; + => AddEntityType(name, type, owned: false, ConfigurationSource.Explicit)!; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -1298,7 +1300,7 @@ IMutableEntityType IMutableModel.AddEntityType(string name, Type type) /// [DebuggerStepThrough] IConventionEntityType? IConventionModel.AddEntityType(string name, Type type, bool fromDataAnnotation) - => AddEntityType(name, type, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + => AddEntityType(name, type, owned: false, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -1360,6 +1362,67 @@ IMutableEntityType IMutableModel.AddEntityType( type, definingNavigationName, (EntityType)definingEntityType, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// + /// 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. + /// + [DebuggerStepThrough] + IMutableEntityType IMutableModel.AddOwnedEntityType(string name) + => AddEntityType(name, owned: true, ConfigurationSource.Explicit)!; + + /// + /// 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. + /// + [DebuggerStepThrough] + IConventionEntityType? IConventionModel.AddOwnedEntityType(string name, bool fromDataAnnotation) + => AddEntityType(name, owned: true, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + + /// + /// 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. + /// + [DebuggerStepThrough] + IMutableEntityType IMutableModel.AddOwnedEntityType(Type type) + => AddEntityType(type, owned: true, ConfigurationSource.Explicit)!; + + /// + /// 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. + /// + [DebuggerStepThrough] + IConventionEntityType? IConventionModel.AddOwnedEntityType(Type type, bool fromDataAnnotation) + => AddEntityType(type, owned: true, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + + /// + /// 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. + /// + [DebuggerStepThrough] + IMutableEntityType IMutableModel.AddOwnedEntityType(string name, Type type) + => AddEntityType(name, type, owned: true, ConfigurationSource.Explicit)!; + + /// + /// 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. + /// + [DebuggerStepThrough] + IConventionEntityType? IConventionModel.AddOwnedEntityType(string name, Type type, bool fromDataAnnotation) + => AddEntityType(name, type, owned: true, + fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// /// 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/PropertiesSnapshot.cs b/src/EFCore/Metadata/Internal/PropertiesSnapshot.cs index 6c55f304b45..ee2a7d89f36 100644 --- a/src/EFCore/Metadata/Internal/PropertiesSnapshot.cs +++ b/src/EFCore/Metadata/Internal/PropertiesSnapshot.cs @@ -121,6 +121,7 @@ public virtual void Attach(InternalEntityTypeBuilder entityTypeBuilder) { var originalEntityType = indexBuilder.Metadata.DeclaringEntityType; var targetEntityTypeBuilder = originalEntityType.Name == entityTypeBuilder.Metadata.Name + || (!originalEntityType.HasSharedClrType && originalEntityType.ClrType == entityTypeBuilder.Metadata.ClrType) ? entityTypeBuilder : originalEntityType.Builder; indexBuilder.Attach(targetEntityTypeBuilder); diff --git a/src/EFCore/Metadata/Internal/TypeBase.cs b/src/EFCore/Metadata/Internal/TypeBase.cs index e7109d16791..3109b99fd0c 100644 --- a/src/EFCore/Metadata/Internal/TypeBase.cs +++ b/src/EFCore/Metadata/Internal/TypeBase.cs @@ -297,7 +297,7 @@ public virtual IEnumerable GetIgnoredMembers() /// public virtual ConfigurationSource? FindDeclaredIgnoredConfigurationSource(string name) => _ignoredMembers.TryGetValue(Check.NotEmpty(name, nameof(name)), out var ignoredConfigurationSource) - ? (ConfigurationSource?)ignoredConfigurationSource + ? ignoredConfigurationSource : null; /// diff --git a/src/EFCore/ModelBuilder.cs b/src/EFCore/ModelBuilder.cs index b852ff13a5c..ea50eb2d9da 100644 --- a/src/EFCore/ModelBuilder.cs +++ b/src/EFCore/ModelBuilder.cs @@ -137,7 +137,7 @@ IConventionModelBuilder IInfrastructure.Instance /// An object that can be used to configure the entity type. public virtual EntityTypeBuilder Entity() where TEntity : class - => new(Builder.Entity(typeof(TEntity), ConfigurationSource.Explicit)!.Metadata); + => new(Builder.Entity(typeof(TEntity), ConfigurationSource.Explicit, shouldBeOwned: false)!.Metadata); /// /// @@ -173,7 +173,7 @@ public virtual EntityTypeBuilder Entity(Type type) { Check.NotNull(type, nameof(type)); - return new EntityTypeBuilder(Builder.Entity(type, ConfigurationSource.Explicit)!.Metadata); + return new EntityTypeBuilder(Builder.Entity(type, ConfigurationSource.Explicit, shouldBeOwned: false)!.Metadata); } /// @@ -187,7 +187,7 @@ public virtual EntityTypeBuilder Entity(string name) { Check.NotEmpty(name, nameof(name)); - return new EntityTypeBuilder(Builder.Entity(name, ConfigurationSource.Explicit)!.Metadata); + return new EntityTypeBuilder(Builder.Entity(name, ConfigurationSource.Explicit, shouldBeOwned: false)!.Metadata); } /// @@ -211,7 +211,7 @@ public virtual EntityTypeBuilder SharedTypeEntity(string name, Type type) Check.NotEmpty(name, nameof(name)); Check.NotNull(type, nameof(type)); - return new EntityTypeBuilder(Builder.SharedTypeEntity(name, type, ConfigurationSource.Explicit)!.Metadata); + return new EntityTypeBuilder(Builder.SharedTypeEntity(name, type, ConfigurationSource.Explicit, shouldBeOwned: false)!.Metadata); } /// diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index 3d94e5ceb95..2db61657713 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -336,7 +336,7 @@ public static string ClashingNamedOwnedType(object? ownedTypeName, object? owner ownedTypeName, ownerEntityType, navigation); /// - /// The type '{entityType}' cannot be marked as owned because the derived entity type '{derivedType}' has been configured as non-owned. Either don't configure '{derivedType}' as non-owned, or call 'HasBaseType(null)' for it in 'OnModelCreating'. + /// The entity type '{entityType}' cannot be marked as owned because the derived entity type '{derivedType}' has been configured as non-owned. Either don't configure '{derivedType}' as non-owned, or call 'HasBaseType(null)' for it in 'OnModelCreating'. /// public static string ClashingNonOwnedDerivedEntityType(object? entityType, object? derivedType) => string.Format( @@ -344,7 +344,7 @@ public static string ClashingNonOwnedDerivedEntityType(object? entityType, objec entityType, derivedType); /// - /// The type '{entityType}' cannot be marked as owned because a non-owned entity type with the same name already exists. + /// The entity type '{entityType}' cannot be configured as owned because it has already been configured as a non-owned. If you want to override previous configuration first remove the entity type from the model by calling 'Ignore'. /// public static string ClashingNonOwnedEntityType(object? entityType) => string.Format( @@ -369,7 +369,15 @@ public static string ClashingNonWeakEntityType(object? entityType) entityType); /// - /// The type '{entityType}' cannot be configured as non-owned because an owned entity type with the same name already exists. + /// The entity type '{entityType}' cannot be marked as non-owned because the derived entity type '{derivedType}' has been configured as owned. Either don't configure '{derivedType}' as owned, or call 'HasBaseType(null)' for it in 'OnModelCreating'. + /// + public static string ClashingOwnedDerivedEntityType(object? entityType, object? derivedType) + => string.Format( + GetString("ClashingOwnedDerivedEntityType", nameof(entityType), nameof(derivedType)), + entityType, derivedType); + + /// + /// The entity type '{entityType}' cannot be configured as non-owned because it has already been configured as a owned. If you want to override previous configuration first remove the entity type from the model by calling 'Ignore'. /// public static string ClashingOwnedEntityType(object? entityType) => string.Format( @@ -617,6 +625,14 @@ public static string DerivedEntityCannotHaveKeys(object? entityType) GetString("DerivedEntityCannotHaveKeys", nameof(entityType)), entityType); + /// + /// 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. + /// + public static string DerivedEntityOwnershipMismatch(object? baseEntityType, object? derivedEntityType, object? ownedEntityType, object? nonOwnedEntityType) + => string.Format( + GetString("DerivedEntityOwnershipMismatch", nameof(baseEntityType), nameof(derivedEntityType), nameof(ownedEntityType), nameof(nonOwnedEntityType)), + 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. /// @@ -1664,6 +1680,14 @@ public static string NavigationIsProperty(object? property, object? entityType, GetString("NavigationIsProperty", "0_property", "1_entityType", nameof(referenceMethod), nameof(collectionMethod), nameof(propertyMethod)), property, entityType, referenceMethod, collectionMethod, propertyMethod); + /// + /// The relationship between '{principalEntityType}' and '{dependentEntityType}' cannot be configured as an ownership as there is no associated navigation to the owned type. An ownership must always have an associated navigation. + /// + public static string NavigationlessOwnership(object? principalEntityType, object? dependentEntityType) + => string.Format( + GetString("NavigationlessOwnership", nameof(principalEntityType), nameof(dependentEntityType)), + principalEntityType, dependentEntityType); + /// /// The navigation '{1_entityType}.{0_navigation}' does not have a setter and no writable backing field was found or specified. Read-only collection navigations must be initialized before use. /// @@ -2026,6 +2050,14 @@ public static string OwnerlessOwnedType(object? ownedType) GetString("OwnerlessOwnedType", nameof(ownedType)), ownedType); + /// + /// The navigation '{navigation}' cannot be changed, because the foreign key between '{principalEntityType}' and '{dependentEntityType}' is an ownership. To change the navigation to the owned entity type remove the ownership. + /// + public static string OwnershipToDependent(object? navigation, object? principalEntityType, object? dependentEntityType) + => string.Format( + GetString("OwnershipToDependent", nameof(navigation), nameof(principalEntityType), nameof(dependentEntityType)), + navigation, principalEntityType, dependentEntityType); + /// /// The DbContext of type '{contextType}' cannot be pooled because it does not have a public constructor accepting a single parameter of type DbContextOptions or has more than one constructor. /// @@ -2040,6 +2072,14 @@ public static string PoolingContextCtorError(object? contextType) public static string PoolingOptionsModified => GetString("PoolingOptionsModified"); + /// + /// When creating the relationship between '{navigationSpecification1}' and '{navigationSpecification2}' the entity type '{targetEntityType}' cannot be set as principal. + /// + public static string PrincipalEndIncompatibleNavigations(object? navigationSpecification1, object? navigationSpecification2, object? targetEntityType) + => string.Format( + GetString("PrincipalEndIncompatibleNavigations", nameof(navigationSpecification1), nameof(navigationSpecification2), nameof(targetEntityType)), + navigationSpecification1, navigationSpecification2, targetEntityType); + /// /// 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. /// diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index 150a14f0b98..7c6482d9d02 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -236,10 +236,10 @@ An entity type named '{ownedTypeName}' has already been added to the model. Use a different name when configuring the ownership '{ownerEntityType}.{navigation}' in 'OnModelCreating'. - The type '{entityType}' cannot be marked as owned because the derived entity type '{derivedType}' has been configured as non-owned. Either don't configure '{derivedType}' as non-owned, or call 'HasBaseType(null)' for it in 'OnModelCreating'. + The entity type '{entityType}' cannot be marked as owned because the derived entity type '{derivedType}' has been configured as non-owned. Either don't configure '{derivedType}' as non-owned, or call 'HasBaseType(null)' for it in 'OnModelCreating'. - The type '{entityType}' cannot be marked as owned because a non-owned entity type with the same name already exists. + The entity type '{entityType}' cannot be configured as owned because it has already been configured as a non-owned. If you want to override previous configuration first remove the entity type from the model by calling 'Ignore'. The shared-type entity type '{entityType}' with CLR type '{type}' cannot be added to the model because a non-shared entity type with the same CLR type already exists. @@ -248,8 +248,11 @@ The entity type '{entityType}' with a defining navigation cannot be added to the model because an entity type with the same name already exists. Obsolete + + The entity type '{entityType}' cannot be marked as non-owned because the derived entity type '{derivedType}' has been configured as owned. Either don't configure '{derivedType}' as owned, or call 'HasBaseType(null)' for it in 'OnModelCreating'. + - The type '{entityType}' cannot be configured as non-owned because an owned entity type with the same name already exists. + The entity type '{entityType}' cannot be configured as non-owned because it has already been configured as a owned. If you want to override previous configuration first remove the entity type from the model by calling 'Ignore'. The entity type '{entityType}' cannot be added to the model because its CLR type has been configured as a shared type. @@ -347,6 +350,9 @@ Unable to set a base type for entity type '{entityType}' because it has one or more keys defined. Only root types can have keys. + + 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. + '{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. @@ -1060,6 +1066,9 @@ The property '{1_entityType}.{0_property}' is being accessed using the '{referenceMethod}' or '{collectionMethod}' method, but is defined in the model as a non-navigation. Use the '{propertyMethod}' method to access non-navigation properties. + + The relationship between '{principalEntityType}' and '{dependentEntityType}' cannot be configured as an ownership as there is no associated navigation to the owned type. An ownership must always have an associated navigation. + The navigation '{1_entityType}.{0_navigation}' does not have a setter and no writable backing field was found or specified. Read-only collection navigations must be initialized before use. @@ -1204,12 +1213,18 @@ The entity type '{ownedType}' has been marked as owned and must be referenced from another entity type via a navigation. Add a navigation to an entity type that points at '{ownedType}' or don't configure it as owned. + + The navigation '{navigation}' cannot be changed, because the foreign key between '{principalEntityType}' and '{dependentEntityType}' is an ownership. To change the navigation to the owned entity type remove the ownership. + The DbContext of type '{contextType}' cannot be pooled because it does not have a public constructor accepting a single parameter of type DbContextOptions or has more than one constructor. 'OnConfiguring' cannot be used to modify DbContextOptions when DbContext pooling is enabled. + + When creating the relationship between '{navigationSpecification1}' and '{navigationSpecification2}' the entity type '{targetEntityType}' cannot be set as principal. + 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. diff --git a/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs b/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs index c24bfa0738f..773c1bac20f 100644 --- a/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs +++ b/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Linq.Expressions; @@ -341,7 +340,7 @@ private Expression ExpandForeignKey( var collection = !foreignKey.IsUnique && !onDependent; var targetType = onDependent ? foreignKey.PrincipalEntityType : foreignKey.DeclaringEntityType; - Debug.Assert(!targetType.IsOwned(), "Owned entity expanding foreign key."); + Check.DebugAssert(!targetType.IsOwned(), "Owned entity expanding foreign key."); var innerQueryable = new QueryRootExpression(targetType); var innerSource = (NavigationExpansionExpression)_navigationExpandingExpressionVisitor.Visit(innerQueryable); diff --git a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs index 29f735a71da..6dcb0a228e4 100644 --- a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs +++ b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs @@ -50,7 +50,6 @@ public void Test_new_annotations_handled_for_entity_types() CoreAnnotationNames.ProductVersion, CoreAnnotationNames.ValueGeneratorFactory, CoreAnnotationNames.ValueGeneratorFactoryType, - CoreAnnotationNames.OwnedTypes, CoreAnnotationNames.ValueConverter, CoreAnnotationNames.ValueConverterType, CoreAnnotationNames.ValueComparer, @@ -180,7 +179,6 @@ public void Test_new_annotations_handled_for_properties() var notForProperty = new HashSet { CoreAnnotationNames.ProductVersion, - CoreAnnotationNames.OwnedTypes, CoreAnnotationNames.NavigationAccessMode, CoreAnnotationNames.EagerLoaded, CoreAnnotationNames.QueryFilter, diff --git a/test/EFCore.Proxies.Tests/ProxyTests.cs b/test/EFCore.Proxies.Tests/ProxyTests.cs index c9ccff16dd7..87a7c79c9b9 100644 --- a/test/EFCore.Proxies.Tests/ProxyTests.cs +++ b/test/EFCore.Proxies.Tests/ProxyTests.cs @@ -269,7 +269,7 @@ public void Throws_if_attempt_to_create_EntityType_based_on_proxy_class() Assert.Equal( CoreStrings.AddingProxyTypeAsEntityType("Castle.Proxies.ClassToBeProxiedProxy"), Assert.Throws( - () => new EntityType(proxy.GetType(), model, ConfigurationSource.Explicit)).Message); + () => new EntityType(proxy.GetType(), model, owned: false, ConfigurationSource.Explicit)).Message); } // tests scenario in https://github.com/dotnet/efcore/issues/15958 diff --git a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs index 9894d169f4b..0ceafbdd2e9 100644 --- a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs +++ b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs @@ -864,7 +864,7 @@ public virtual void Passes_for_shared_columns() var modelBuilder = CreateConventionalModelBuilder(); modelBuilder.Entity().Property(a => a.Id).HasMaxLength(20).HasPrecision(15, 10).IsUnicode(); modelBuilder.Entity().OwnsOne(a => a.FavoritePerson); - modelBuilder.Entity(); + modelBuilder.Entity().Ignore(d => d.FavoritePerson); Validate(modelBuilder); } @@ -1451,7 +1451,7 @@ public virtual void Passes_for_missing_concurrency_token_on_owner() modelBuilder.Entity().OwnsOne( a => a.FavoritePerson, pb => pb.Property("Version").IsRowVersion().HasColumnName("Version")); - modelBuilder.Entity(); + modelBuilder.Entity().Ignore(d => d.FavoritePerson); Validate(modelBuilder); } @@ -1464,7 +1464,7 @@ public virtual void Passes_for_explicitly_mapped_concurrency_tokens_with_owned() modelBuilder.Entity().OwnsOne( a => a.FavoritePerson, pb => pb.Property("Version").IsRowVersion()); - modelBuilder.Entity(); + modelBuilder.Entity().Ignore(d => d.FavoritePerson); Validate(modelBuilder); } @@ -2100,56 +2100,6 @@ protected override void SetBaseType(IMutableEntityType entityType, IMutableEntit entityType.SetDiscriminatorValue(entityType.Name); } - protected class Animal - { - public int Id { get; set; } - public string Name { get; set; } - - public Person FavoritePerson { get; set; } - } - - protected class Cat : Animal - { - public string Breed { get; set; } - - [NotMapped] - public string Type { get; set; } - - public int Identity { get; set; } - } - - protected class Dog : Animal - { - public string Breed { get; set; } - - [NotMapped] - public int Type { get; set; } - - public int Identity { get; set; } - } - - protected class Person - { - public int Id { get; set; } - public string Name { get; set; } - public string FavoriteBreed { get; set; } - } - - protected class Employee : Person - { - } - - protected class Owner - { - public int Id { get; set; } - public OwnedEntity Owned { get; set; } - } - - protected class OwnedEntity - { - public string Value { get; set; } - } - public class TestDecimalToLongConverter : ValueConverter { private static readonly Expression> convertToProviderExpression = d => (long)(d * 100); diff --git a/test/EFCore.Relational.Tests/RelationalEventIdTest.cs b/test/EFCore.Relational.Tests/RelationalEventIdTest.cs index 2debbd0c950..60a22012dd5 100644 --- a/test/EFCore.Relational.Tests/RelationalEventIdTest.cs +++ b/test/EFCore.Relational.Tests/RelationalEventIdTest.cs @@ -13,7 +13,6 @@ using System.Transactions; using Microsoft.EntityFrameworkCore.ChangeTracking.Internal; using Microsoft.EntityFrameworkCore.Diagnostics; -using Microsoft.EntityFrameworkCore.Diagnostics.Internal; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Metadata.Internal; @@ -26,7 +25,6 @@ using Microsoft.EntityFrameworkCore.TestUtilities.FakeProvider; using Microsoft.EntityFrameworkCore.Update; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; using Xunit; using Index = Microsoft.EntityFrameworkCore.Metadata.Internal.Index; using IsolationLevel = System.Data.IsolationLevel; @@ -41,7 +39,7 @@ public void Every_eventId_has_a_logger_method_and_logs_when_level_enabled() { var constantExpression = Expression.Constant("A"); var model = new Model(); - var entityType = new EntityType(typeof(object), model, ConfigurationSource.Convention); + var entityType = new EntityType(typeof(object), model, owned: false, ConfigurationSource.Convention); var property = entityType.AddProperty("A", typeof(int), ConfigurationSource.Convention, ConfigurationSource.Convention); var key = entityType.AddKey(property, ConfigurationSource.Convention); var foreignKey = new ForeignKey(new List { property }, key, entityType, entityType, ConfigurationSource.Convention); diff --git a/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs b/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs index 41e5c734439..1cd51f6c167 100644 --- a/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs +++ b/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs @@ -1987,7 +1987,6 @@ private class MultipleAnswersRepeatingInverse : AnswerBaseInverse public virtual void InversePropertyAttribute_pointing_to_same_skip_nav_on_base_causes_ambiguity() { var modelBuilder = CreateModelBuilder(); - modelBuilder.Entity(); modelBuilder.Entity(); @@ -2048,9 +2047,9 @@ public virtual void ForeignKeyAttribute_creates_two_relationships_if_applied_on_ Assert.Equal(LogLevel.Warning, logEntry.Level); Assert.Equal( CoreResources.LogForeignKeyAttributesOnBothProperties(new TestLogger()).GenerateMessage( - nameof(PostDetails.Post), nameof(PostDetails), nameof(Post.PostDetails), nameof(Post), - nameof(Post.PostDetailsId), nameof(PostDetails.PostId)), + nameof(PostDetails.Post), nameof(PostDetails), + nameof(PostDetails.PostId), nameof(Post.PostDetailsId)), logEntry.Message); } diff --git a/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs b/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs index 291a059841e..f420a93f831 100644 --- a/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs +++ b/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs @@ -625,7 +625,7 @@ public virtual void Can_update_many_to_many_composite_shared_with_navs() context.Set().CreateInstance( (e, p) => { - Debug.Assert(e != null, nameof(e) + " != null"); + Assert.True(e != null, nameof(e) + " != null"); e.Id = Fixture.UseGeneratedKeys ? 0 : 7723; e.Name = "Z7723"; }), diff --git a/test/EFCore.Specification.Tests/Query/ComplexNavigationsSharedTypeQueryFixtureBase.cs b/test/EFCore.Specification.Tests/Query/ComplexNavigationsSharedTypeQueryFixtureBase.cs index b51535a638c..dc2c8550ac4 100644 --- a/test/EFCore.Specification.Tests/Query/ComplexNavigationsSharedTypeQueryFixtureBase.cs +++ b/test/EFCore.Specification.Tests/Query/ComplexNavigationsSharedTypeQueryFixtureBase.cs @@ -48,7 +48,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con var level1 = level1Builder.Metadata; ForeignKey level2Fk; - var level2 = level1.Model.AddEntityType(typeof(Level2), nameof(Level1.OneToOne_Required_PK1), level1); + var level2 = level1.Model.AddEntityType("Level1.OneToOne_Required_PK1#Level2", typeof(Level2)); using (var batch = ((Model)modelBuilder.Model).ConventionDispatcher.DelayConventions()) { level2Fk = (ForeignKey)level2.AddForeignKey(level2.FindProperty(nameof(Level2.Id)), level1.FindPrimaryKey(), level1); @@ -141,7 +141,7 @@ protected virtual void Configure(OwnedNavigationBuilder l2) .IsRequired(false); ForeignKey level3Fk; - var level3 = level2.Model.AddEntityType(typeof(Level3), nameof(Level2.OneToOne_Required_PK2), level2); + var level3 = level2.Model.AddEntityType("Level1.OneToOne_Required_PK1#Level2.OneToOne_Required_PK2#Level3", typeof(Level3)); using (var batch = ((Model)level2.Model).ConventionDispatcher.DelayConventions()) { level3Fk = (ForeignKey)level3.AddForeignKey(level3.FindProperty(nameof(Level3.Id)), level2.FindPrimaryKey(), level2); @@ -192,7 +192,9 @@ protected virtual void Configure(OwnedNavigationBuilder l3) .IsRequired(false); ForeignKey level4Fk; - var level4 = level3.Model.AddEntityType(typeof(Level4), nameof(Level3.OneToOne_Required_PK3), level3); + var level4 = level3.Model.AddEntityType( + "Level1.OneToOne_Required_PK1#Level2.OneToOne_Required_PK2#Level3.OneToOne_Required_PK3#Level4", + typeof(Level4)); using (var batch = ((Model)level3.Model).ConventionDispatcher.DelayConventions()) { level4Fk = (ForeignKey)level4.AddForeignKey(level4.FindProperty(nameof(Level4.Id)), level3.FindPrimaryKey(), level3); diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs index 16dc324c17e..5597b275c52 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs @@ -7616,7 +7616,7 @@ public virtual async Task Optional_dependent_is_null_when_sharing_required_colum Assert.NotNull(query[2].Contact.Address); AssertSql( - @"SELECT [u].[Id], [u].[RowVersion], [u].[Contact_MobileNumber], [u].[SharedProperty], [u].[RowVersion], [u].[Contact_Address_City], [u].[Contact_Address_Zip], [u].[Data_Data], [u].[Data_Exists] + @"SELECT [u].[Id], [u].[RowVersion], [u].[Contact_MobileNumber], [u].[SharedProperty], [u].[Contact_Address_City], [u].[Contact_Address_Zip], [u].[Data_Data], [u].[Data_Exists], [u].[RowVersion] FROM [User22054] AS [u] ORDER BY [u].[Id] DESC"); } diff --git a/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs b/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs index a57ac21f9de..bfd76f8da38 100644 --- a/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs +++ b/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs @@ -142,7 +142,7 @@ public virtual void Passes_for_identity_seed_and_increment_on_owner() var modelBuilder = CreateConventionalModelBuilder(); modelBuilder.Entity().Property(a => a.Id).UseIdentityColumn(2, 3); modelBuilder.Entity().OwnsOne(a => a.FavoritePerson); - modelBuilder.Entity(); + modelBuilder.Entity().Ignore(d => d.FavoritePerson); Validate(modelBuilder); } diff --git a/test/EFCore.SqlServer.Tests/SqlServerEventIdTest.cs b/test/EFCore.SqlServer.Tests/SqlServerEventIdTest.cs index b587e2d9d5e..8605c73dd2f 100644 --- a/test/EFCore.SqlServer.Tests/SqlServerEventIdTest.cs +++ b/test/EFCore.SqlServer.Tests/SqlServerEventIdTest.cs @@ -10,7 +10,6 @@ using Microsoft.EntityFrameworkCore.Metadata.Internal; using Microsoft.EntityFrameworkCore.SqlServer.Diagnostics.Internal; using Microsoft.EntityFrameworkCore.SqlServer.Extensions.Internal; -using Microsoft.EntityFrameworkCore.SqlServer.Internal; using Xunit; // ReSharper disable InconsistentNaming @@ -21,7 +20,7 @@ public class SqlServerEventIdTest : EventIdTestBase [ConditionalFact] public void Every_eventId_has_a_logger_method_and_logs_when_level_enabled() { - var entityType = new EntityType(typeof(object), new Model(new ConventionSet()), ConfigurationSource.Convention); + var entityType = new EntityType(typeof(object), new Model(new ConventionSet()), owned: false, ConfigurationSource.Convention); var property = new Property( "A", typeof(int), null, null, entityType, ConfigurationSource.Convention, ConfigurationSource.Convention); entityType.Model.FinalizeModel(); diff --git a/test/EFCore.Sqlite.Tests/SqliteEventIdTest.cs b/test/EFCore.Sqlite.Tests/SqliteEventIdTest.cs index b0b3b81314f..c628caa3c5f 100644 --- a/test/EFCore.Sqlite.Tests/SqliteEventIdTest.cs +++ b/test/EFCore.Sqlite.Tests/SqliteEventIdTest.cs @@ -20,7 +20,7 @@ public class SqliteEventIdTest : EventIdTestBase [ConditionalFact] public void Every_eventId_has_a_logger_method_and_logs_when_level_enabled() { - var entityType = new EntityType(typeof(object), new Model(new ConventionSet()), ConfigurationSource.Convention); + var entityType = new EntityType(typeof(object), new Model(new ConventionSet()), owned: false, ConfigurationSource.Convention); entityType.Model.FinalizeModel(); var fakeFactories = new Dictionary> diff --git a/test/EFCore.Tests/ExceptionTest.cs b/test/EFCore.Tests/ExceptionTest.cs index 3f30c992cd6..42fa5d26b2a 100644 --- a/test/EFCore.Tests/ExceptionTest.cs +++ b/test/EFCore.Tests/ExceptionTest.cs @@ -236,9 +236,8 @@ public bool IsConceptualNull(IProperty property) private static IEntityType CreateEntityType() { var model = new Model(); - var entityType = model.AddEntityType(typeof(object), ConfigurationSource.Convention); - model.FinalizeModel(); - return entityType; + model.AddEntityType(typeof(object), owned: false, ConfigurationSource.Convention); + return model.FinalizeModel().FindEntityType(typeof(object)); } private TException SerializeAndDeserialize(TException exception) diff --git a/test/EFCore.Tests/Infrastructure/CoreEventIdTest.cs b/test/EFCore.Tests/Infrastructure/CoreEventIdTest.cs index 11fe2ab1bc6..96d599be631 100644 --- a/test/EFCore.Tests/Infrastructure/CoreEventIdTest.cs +++ b/test/EFCore.Tests/Infrastructure/CoreEventIdTest.cs @@ -22,9 +22,10 @@ public class CoreEventIdTest : EventIdTestBase public void Every_eventId_has_a_logger_method_and_logs_when_level_enabled() { var propertyInfo = typeof(DateTime).GetTypeInfo().GetDeclaredProperty(nameof(DateTime.Now)); - var entityType = new Model().AddEntityType(typeof(object), ConfigurationSource.Convention); + var model = new Model(); + var entityType = model.AddEntityType(typeof(object), owned: false, ConfigurationSource.Convention); var property = entityType.AddProperty("A", typeof(int), ConfigurationSource.Convention, ConfigurationSource.Convention); - var otherEntityType = new EntityType(typeof(object), entityType.Model, ConfigurationSource.Convention); + var otherEntityType = new EntityType(typeof(object), entityType.Model, owned: false, ConfigurationSource.Convention); var otherProperty = otherEntityType.AddProperty( "A", typeof(int), ConfigurationSource.Convention, ConfigurationSource.Convention); var otherKey = otherEntityType.AddKey(otherProperty, ConfigurationSource.Convention); diff --git a/test/EFCore.Tests/Infrastructure/ModelValidatorTest.PropertyMapping.cs b/test/EFCore.Tests/Infrastructure/ModelValidatorTest.PropertyMapping.cs index 13dfd09d990..489749f13e0 100644 --- a/test/EFCore.Tests/Infrastructure/ModelValidatorTest.PropertyMapping.cs +++ b/test/EFCore.Tests/Infrastructure/ModelValidatorTest.PropertyMapping.cs @@ -4,10 +4,11 @@ using System; using System.Collections; using System.Collections.Generic; +using System.ComponentModel.DataAnnotations.Schema; using System.Threading; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Metadata; -using Microsoft.EntityFrameworkCore.Metadata.Internal; +using Microsoft.EntityFrameworkCore.TestModels.InheritanceModel; using Microsoft.EntityFrameworkCore.TestUtilities; using Xunit; @@ -124,6 +125,20 @@ public virtual void Throws_when_navigation_is_not_added_or_ignored() Assert.Throws(() => Validate(modelBuilder)).Message); } + [ConditionalFact] + public virtual void Throws_when_navigation_to_owned_type_is_not_added_or_ignored() + { + var modelBuilder = CreateConventionalModelBuilder(); + modelBuilder.Entity().Property(a => a.Id); + modelBuilder.Entity().OwnsOne(a => a.FavoritePerson); + modelBuilder.Entity(); + + Assert.Equal( + CoreStrings.NavigationNotAdded( + typeof(Dog).ShortDisplayName(), nameof(Dog.FavoritePerson), typeof(Person).Name), + Assert.Throws(() => Validate(modelBuilder)).Message); + } + [ConditionalFact] public virtual void Does_not_throw_when_navigation_is_added() { @@ -305,5 +320,55 @@ protected class InterfaceNavigationEntity { public IList Navigation { get; set; } } + + protected class Animal + { + public int Id { get; set; } + public string Name { get; set; } + + public Person FavoritePerson { get; set; } + } + + protected class Cat : Animal + { + public string Breed { get; set; } + + [NotMapped] + public string Type { get; set; } + + public int Identity { get; set; } + } + + protected class Dog : Animal + { + public string Breed { get; set; } + + [NotMapped] + public int Type { get; set; } + + public int Identity { get; set; } + } + + protected class Person + { + public int Id { get; set; } + public string Name { get; set; } + public string FavoriteBreed { get; set; } + } + + protected class Employee : Person + { + } + + protected class Owner + { + public int Id { get; set; } + public OwnedEntity Owned { get; set; } + } + + protected class OwnedEntity + { + public string Value { get; set; } + } } } diff --git a/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs b/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs index b76a2ddae1e..e40c38a20ca 100644 --- a/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs +++ b/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs @@ -688,6 +688,7 @@ public virtual void Passes_on_valid_owned_entity_types() entityTypeBuilder.Ignore(nameof(SampleEntity.Name), ConfigurationSource.Explicit); entityTypeBuilder.Ignore(nameof(SampleEntity.Number), ConfigurationSource.Explicit); entityTypeBuilder.Ignore(nameof(SampleEntity.OtherSamples), ConfigurationSource.Explicit); + entityTypeBuilder.Ignore(nameof(SampleEntity.AnotherReferencedEntity), ConfigurationSource.Explicit); var ownershipBuilder = entityTypeBuilder.HasOwnership( typeof(ReferencedEntity), nameof(SampleEntity.ReferencedEntity), ConfigurationSource.Convention); @@ -719,14 +720,14 @@ public virtual void Detects_entity_type_with_multiple_ownerships() ownedTypeBuilder.PrimaryKey(ownershipBuilder.Metadata.Properties.Select(p => p.Name).ToList(), ConfigurationSource.Convention); ownedTypeBuilder.HasRelationship( - entityTypeBuilder.Metadata, (string)null, null, ConfigurationSource.Convention, setTargetAsPrincipal: true) + entityTypeBuilder.Metadata, null, nameof(SampleEntity.AnotherReferencedEntity), ConfigurationSource.Convention, setTargetAsPrincipal: true) .Metadata.IsOwnership = true; ownedTypeBuilder.Ignore(nameof(ReferencedEntity.Id), ConfigurationSource.Explicit); ownedTypeBuilder.Ignore(nameof(ReferencedEntity.SampleEntityId), ConfigurationSource.Explicit); VerifyError( - CoreStrings.MultipleOwnerships(nameof(ReferencedEntity), "'SampleEntity.ReferencedEntity', 'SampleEntity.'"), + CoreStrings.MultipleOwnerships(nameof(ReferencedEntity), "'SampleEntity.ReferencedEntity', 'SampleEntity.AnotherReferencedEntity'"), builder); } @@ -741,6 +742,7 @@ public virtual void Detects_principal_owned_entity_type() entityTypeBuilder.Ignore(nameof(SampleEntity.Number), ConfigurationSource.Explicit); entityTypeBuilder.Ignore(nameof(SampleEntity.Name), ConfigurationSource.Explicit); entityTypeBuilder.Ignore(nameof(SampleEntity.OtherSamples), ConfigurationSource.Explicit); + entityTypeBuilder.Ignore(nameof(SampleEntity.AnotherReferencedEntity), ConfigurationSource.Explicit); var ownershipBuilder = entityTypeBuilder.HasOwnership( typeof(ReferencedEntity), nameof(SampleEntity.ReferencedEntity), ConfigurationSource.Convention); @@ -776,6 +778,7 @@ public virtual void Detects_non_owner_navigation_to_owned_entity_type() entityTypeBuilder.Ignore(nameof(SampleEntity.Number), ConfigurationSource.Explicit); entityTypeBuilder.Ignore(nameof(SampleEntity.Name), ConfigurationSource.Explicit); entityTypeBuilder.Ignore(nameof(SampleEntity.OtherSamples), ConfigurationSource.Explicit); + entityTypeBuilder.Ignore(nameof(SampleEntity.AnotherReferencedEntity), ConfigurationSource.Explicit); var ownershipBuilder = entityTypeBuilder.HasOwnership( typeof(ReferencedEntity), nameof(SampleEntity.ReferencedEntity), ConfigurationSource.Convention); @@ -817,14 +820,16 @@ public virtual void Detects_derived_owned_entity_type() var ownedTypeBuilder = ownershipBuilder.Metadata.DeclaringEntityType.Builder; ownedTypeBuilder.PrimaryKey(ownershipBuilder.Metadata.Properties.Select(p => p.Name).ToList(), ConfigurationSource.Convention); - var anotherEntityTypeBuilder = modelBuilder.Entity(typeof(A), ConfigurationSource.Convention); + var baseOwnershipBuilder = entityTypeBuilder.HasOwnership(typeof(A), nameof(B.A), ConfigurationSource.Convention); + var anotherEntityTypeBuilder = baseOwnershipBuilder.Metadata.DeclaringEntityType.Builder; + anotherEntityTypeBuilder = modelBuilder.Entity(typeof(A), ConfigurationSource.Convention); anotherEntityTypeBuilder.PrimaryKey(new[] { nameof(A.Id) }, ConfigurationSource.Convention); anotherEntityTypeBuilder.Property(typeof(int?), nameof(A.P0), ConfigurationSource.Explicit); anotherEntityTypeBuilder.Property(typeof(int?), nameof(A.P1), ConfigurationSource.Explicit); anotherEntityTypeBuilder.Property(typeof(int?), nameof(A.P2), ConfigurationSource.Explicit); anotherEntityTypeBuilder.Property(typeof(int?), nameof(A.P3), ConfigurationSource.Explicit); - ownedTypeBuilder.HasBaseType(typeof(A), ConfigurationSource.Convention); + Assert.NotNull(ownedTypeBuilder.HasBaseType(typeof(A), ConfigurationSource.DataAnnotation)); VerifyError(CoreStrings.OwnedDerivedType(nameof(D)), builder); } diff --git a/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs b/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs index 340cf72987d..6db42b74851 100644 --- a/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs +++ b/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs @@ -121,6 +121,8 @@ public class SampleEntity public int Number { get; set; } public string Name { get; set; } public ReferencedEntity ReferencedEntity { get; set; } + [NotMapped] + public ReferencedEntity AnotherReferencedEntity { get; set; } public ICollection OtherSamples { get; set; } } diff --git a/test/EFCore.Tests/Metadata/Conventions/ConventionDispatcherTest.cs b/test/EFCore.Tests/Metadata/Conventions/ConventionDispatcherTest.cs index 1c932f7efb2..d071896a448 100644 --- a/test/EFCore.Tests/Metadata/Conventions/ConventionDispatcherTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/ConventionDispatcherTest.cs @@ -276,7 +276,7 @@ public void OnEntityTypeAdded_calls_conventions_in_order(bool useBuilder, bool u } else { - var result = builder.Metadata.AddEntityType(typeof(Order), ConfigurationSource.Convention); + var result = builder.Metadata.AddEntityType(typeof(Order), owned: false, ConfigurationSource.Convention); Assert.Equal(!useScope, result == null); } @@ -540,7 +540,7 @@ public void OnBaseTypeChanged_calls_conventions_in_order(bool useBuilder, bool u else { builder.Metadata.SetBaseType( - builder.Metadata.Model.AddEntityType(typeof(Order), ConfigurationSource.Explicit), ConfigurationSource.Convention); + builder.Metadata.Model.AddEntityType(typeof(Order), owned: false, ConfigurationSource.Explicit), ConfigurationSource.Convention); } if (useScope) @@ -1499,8 +1499,9 @@ public void OnForeignKeyOwnershipChanged_calls_conventions_in_order(bool useBuil var builder = new InternalModelBuilder(new Model(conventions)); var principalEntityBuilder = builder.Entity(typeof(Order), ConfigurationSource.Convention); - var dependentEntityBuilder = builder.Entity(typeof(OrderDetails), ConfigurationSource.Convention); - var foreignKey = dependentEntityBuilder.HasRelationship(principalEntityBuilder.Metadata, ConfigurationSource.Convention) + var dependentEntityBuilder = builder.Entity(typeof(OrderDetails), ConfigurationSource.Convention, shouldBeOwned: true); + var foreignKey = dependentEntityBuilder.HasRelationship( + principalEntityBuilder.Metadata, null, nameof(Order.OrderDetails), ConfigurationSource.Convention) .Metadata; var scope = useScope ? builder.Metadata.ConventionDispatcher.DelayConventions() : null; diff --git a/test/EFCore.Tests/Metadata/Conventions/ForeignKeyPropertyDiscoveryConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/ForeignKeyPropertyDiscoveryConventionTest.cs index 89fe4c932ab..fc9060c0549 100644 --- a/test/EFCore.Tests/Metadata/Conventions/ForeignKeyPropertyDiscoveryConventionTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/ForeignKeyPropertyDiscoveryConventionTest.cs @@ -652,7 +652,7 @@ public void Does_not_match_composite_dependent_PK_for_unique_FK_on_derived_type( "NavProp", "InverseReferenceNav", ConfigurationSource.Convention) .IsUnique(true, ConfigurationSource.DataAnnotation) - .DependentEntityType(dependentTypeWithCompositeKey, ConfigurationSource.DataAnnotation); + .DependentEntityType(dependentTypeWithCompositeKey.Metadata, ConfigurationSource.DataAnnotation); var newRelationshipBuilder = RunConvention(relationshipBuilder); @@ -796,7 +796,7 @@ public void Does_not_match_if_a_foreign_key_on_the_best_candidate_property_alrea PrincipalType, "SomeNav", null, ConfigurationSource.Convention); Assert.Equal( - nameof(PrincipalEntity) + nameof(PrincipalEntity.PeeKay), + "SomeNav" + nameof(PrincipalEntity.PeeKay), newRelationshipBuilder.Metadata.Properties.Single().Name); newRelationshipBuilder = RunConvention(newRelationshipBuilder); diff --git a/test/EFCore.Tests/Metadata/Conventions/ModelCleanupConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/ModelCleanupConventionTest.cs index 6fc69baadf4..53928eb1813 100644 --- a/test/EFCore.Tests/Metadata/Conventions/ModelCleanupConventionTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/ModelCleanupConventionTest.cs @@ -79,12 +79,7 @@ private ProviderConventionSetBuilderDependencies CreateDependencies() => InMemoryTestHelpers.Instance.CreateContextServices().GetRequiredService(); private static InternalEntityTypeBuilder CreateInternalEntityBuilder() - { - var modelBuilder = new InternalModelBuilder(new Model()); - var entityBuilder = modelBuilder.Entity(typeof(T), ConfigurationSource.DataAnnotation); - - return entityBuilder; - } + => new InternalModelBuilder(new Model()).Entity(typeof(T), ConfigurationSource.Explicit); private class Base { diff --git a/test/EFCore.Tests/Metadata/Conventions/ServicePropertyDiscoveryConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/ServicePropertyDiscoveryConventionTest.cs index e5e6026572e..e57522d70e3 100644 --- a/test/EFCore.Tests/Metadata/Conventions/ServicePropertyDiscoveryConventionTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/ServicePropertyDiscoveryConventionTest.cs @@ -142,7 +142,7 @@ public void Finds_one_service_property() [ConditionalFact] public void Does_not_find_service_property_configured_as_property() { - var entityType = new Model().AddEntityType(typeof(BlogOneService), ConfigurationSource.Explicit); + var entityType = new Model().AddEntityType(typeof(BlogOneService), owned: false, ConfigurationSource.Explicit); entityType!.Builder.Property(typeof(ILazyLoader), nameof(BlogOneService.Loader), ConfigurationSource.Explicit) !.HasConversion(typeof(string), ConfigurationSource.Explicit); @@ -156,9 +156,9 @@ public void Does_not_find_service_property_configured_as_property() public void Does_not_find_service_property_configured_as_navigation() { var model = new Model(); - var entityType = model.AddEntityType(typeof(BlogOneService), ConfigurationSource.Explicit); + var entityType = model.AddEntityType(typeof(BlogOneService), owned: false, ConfigurationSource.Explicit); entityType!.Builder.HasRelationship( - model.AddEntityType(typeof(LazyLoader), ConfigurationSource.Explicit)!, + model.AddEntityType(typeof(LazyLoader), owned: false, ConfigurationSource.Explicit)!, nameof(BlogOneService.Loader), ConfigurationSource.Explicit); RunConvention(entityType); @@ -179,7 +179,7 @@ public void Finds_service_property_duplicate_ignored() } private EntityType RunConvention() - => RunConvention(new Model().AddEntityType(typeof(TEntity), ConfigurationSource.Explicit)!); + => RunConvention(new Model().AddEntityType(typeof(TEntity), owned: false, ConfigurationSource.Explicit)!); private EntityType RunConvention(EntityType entityType) { @@ -192,10 +192,7 @@ private EntityType RunConvention(EntityType entityType) } private ServicePropertyDiscoveryConvention CreateServicePropertyDiscoveryConvention() - { - var convention = new ServicePropertyDiscoveryConvention(CreateDependencies()); - return convention; - } + => new ServicePropertyDiscoveryConvention(CreateDependencies()); private ProviderConventionSetBuilderDependencies CreateDependencies() => InMemoryTestHelpers.Instance.CreateContextServices().GetRequiredService(); diff --git a/test/EFCore.Tests/Metadata/Internal/EntityTypeTest.cs b/test/EFCore.Tests/Metadata/Internal/EntityTypeTest.cs index f822cb3d102..dba4078d109 100644 --- a/test/EFCore.Tests/Metadata/Internal/EntityTypeTest.cs +++ b/test/EFCore.Tests/Metadata/Internal/EntityTypeTest.cs @@ -2634,11 +2634,111 @@ public void Change_tracking_can_be_set_to_snapshot_only_for_non_notifying_entiti .Message); } + [ConditionalFact] + public void Setting_base_type_for_owned_throws() + { + var model = CreateModel(); + var baseType = model.AddEntityType(typeof(BaseType)); + var entityType = model.AddOwnedEntityType(typeof(Customer)); + + Assert.Equal( + CoreStrings.DerivedEntityOwnershipMismatch( + nameof(BaseType), nameof(Customer), nameof(Customer), nameof(BaseType)), + Assert.Throws( + () => entityType.BaseType = baseType) + .Message); + } + [ConditionalFact] public void Entity_type_with_deeply_nested_owned_shared_types_builds_correctly() { - using var context = new RejectionContext(nameof(RejectionContext)); - var entityTypes = context.Model.GetEntityTypes(); + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + + modelBuilder.Entity( + entity => + { + entity.OwnsOne( + x => x.Attitude, + amb => + { + amb.OwnsOne( + x => x.FirstTest, mb => + { + mb.OwnsOne(a => a.Tester); + }); + }); + + entity.OwnsOne( + x => x.Rejection, + amb => + { + amb.OwnsOne( + x => x.FirstTest, mb => + { + mb.OwnsOne(a => a.Tester); + }); + }); + }); + + Assert.Equal( + new[] + { + "Application", + "Attitude", + "Attitude.FirstTest#FirstTest", // FirstTest is shared + "Attitude.FirstTest#FirstTest.Tester#SpecialistStaff", // SpecialistStaff is shared + "Rejection", + "Rejection.FirstTest#FirstTest", // FirstTest is shared + "Rejection.FirstTest#FirstTest.Tester#SpecialistStaff" // SpecialistStaff is shared + }, GetTypeNames()); + + modelBuilder.Entity( + entity => + { + Assert.Equal( + new[] + { + "Application", + "ApplicationVersion", + "Attitude", + "Attitude.FirstTest#FirstTest", + "Attitude.FirstTest#FirstTest.Tester#SpecialistStaff", + "Rejection", + "Rejection.FirstTest#FirstTest", + "Rejection.FirstTest#FirstTest.Tester#SpecialistStaff" + }, GetTypeNames()); + + entity.OwnsOne( + x => x.Attitude, + amb => + { + amb.OwnsOne( + x => x.FirstTest, mb => + { + mb.OwnsOne(a => a.Tester); + }); + + var typeNames = GetTypeNames(); + Assert.Equal( + new[] + { + "Application", + "Application.Attitude#Attitude", // Attitude becomes shared + "Application.Attitude#Attitude.FirstTest#FirstTest", // Attitude becomes shared + "Application.Attitude#Attitude.FirstTest#FirstTest.Tester#SpecialistStaff", // Attitude becomes shared + "ApplicationVersion", + "ApplicationVersion.Attitude#Attitude", // Attitude becomes shared + "ApplicationVersion.Attitude#Attitude.FirstTest#FirstTest", // Attitude becomes shared + "ApplicationVersion.Attitude#Attitude.FirstTest#FirstTest.Tester#SpecialistStaff", // Attitude becomes shared + "Rejection", + "Rejection.FirstTest#FirstTest", + "Rejection.FirstTest#FirstTest.Tester#SpecialistStaff" + }, typeNames); + }); + }); + + var model = modelBuilder.FinalizeModel(); + var entityTypes = model.GetEntityTypes(); Assert.Equal( new[] @@ -2655,6 +2755,9 @@ public void Entity_type_with_deeply_nested_owned_shared_types_builds_correctly() "Rejection.FirstTest#FirstTest", "Rejection.FirstTest#FirstTest.Tester#SpecialistStaff" }, entityTypes.Select(e => e.DisplayName()).ToList()); + + List GetTypeNames() + => modelBuilder.Model.GetEntityTypes().Select(e => e.DisplayName()).ToList(); } // @@ -2713,124 +2816,6 @@ private class SpecialistStaff { } - private class RejectionContext : DbContext - { - private readonly string _databaseName; - - public RejectionContext(string databaseName) - { - _databaseName = databaseName; - } - - protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) - => optionsBuilder.UseInMemoryDatabase(_databaseName); - - protected internal override void OnModelCreating(ModelBuilder modelBuilder) - { - List GetTypeNames() - => modelBuilder.Model.GetEntityTypes().Select(e => e.DisplayName()).ToList(); - - modelBuilder.Entity( - entity => - { - entity.OwnsOne( - x => x.Attitude, - amb => - { - amb.OwnsOne( - x => x.FirstTest, mb => - { - mb.OwnsOne(a => a.Tester); - }); - }); - - entity.OwnsOne( - x => x.Rejection, - amb => - { - amb.OwnsOne( - x => x.FirstTest, mb => - { - mb.OwnsOne(a => a.Tester); - }); - }); - }); - - Assert.Equal( - new[] - { - "Application", - "Attitude", - "Attitude.FirstTest#FirstTest", // FirstTest is shared - "Attitude.FirstTest#FirstTest.Tester#SpecialistStaff", // SpecialistStaff is shared - "Rejection", - "Rejection.FirstTest#FirstTest", // FirstTest is shared - "Rejection.FirstTest#FirstTest.Tester#SpecialistStaff" // SpecialistStaff is shared - }, GetTypeNames()); - - modelBuilder.Entity( - entity => - { - Assert.Equal( - new[] - { - "Application", - "ApplicationVersion", - "Attitude", - "Attitude.FirstTest#FirstTest", - "Attitude.FirstTest#FirstTest.Tester#SpecialistStaff", - "Rejection", - "Rejection.FirstTest#FirstTest", - "Rejection.FirstTest#FirstTest.Tester#SpecialistStaff" - }, GetTypeNames()); - - entity.OwnsOne( - x => x.Attitude, - amb => - { - amb.OwnsOne( - x => x.FirstTest, mb => - { - mb.OwnsOne(a => a.Tester); - }); - - var typeNames = GetTypeNames(); - Assert.Equal( - new[] - { - "Application", - "Application.Attitude#Attitude", // Attitude becomes shared - "Application.Attitude#Attitude.FirstTest#FirstTest", // Attitude becomes shared - "Application.Attitude#Attitude.FirstTest#FirstTest.Tester#SpecialistStaff", // Attitude becomes shared - "ApplicationVersion", - "ApplicationVersion.Attitude#Attitude", // Attitude becomes shared - "ApplicationVersion.Attitude#Attitude.FirstTest#FirstTest", // Attitude becomes shared - "ApplicationVersion.Attitude#Attitude.FirstTest#FirstTest.Tester#SpecialistStaff", // Attitude becomes shared - "Rejection", - "Rejection.FirstTest#FirstTest", - "Rejection.FirstTest#FirstTest.Tester#SpecialistStaff" - }, typeNames); - }); - }); - - Assert.Equal( - new[] - { - "Application", - "Application.Attitude#Attitude", - "Application.Attitude#Attitude.FirstTest#FirstTest", - "Application.Attitude#Attitude.FirstTest#FirstTest.Tester#SpecialistStaff", - "ApplicationVersion", - "ApplicationVersion.Attitude#Attitude", - "ApplicationVersion.Attitude#Attitude.FirstTest#FirstTest", - "ApplicationVersion.Attitude#Attitude.FirstTest#FirstTest.Tester#SpecialistStaff", - "Rejection", - "Rejection.FirstTest#FirstTest", - "Rejection.FirstTest#FirstTest.Tester#SpecialistStaff" - }, GetTypeNames()); - } - } - [ConditionalFact] public void All_properties_have_original_value_indexes_when_using_snapshot_change_tracking() { diff --git a/test/EFCore.Tests/Metadata/Internal/ForeignKeyTest.cs b/test/EFCore.Tests/Metadata/Internal/ForeignKeyTest.cs index 3e566dc6a5e..f749ee6b66a 100644 --- a/test/EFCore.Tests/Metadata/Internal/ForeignKeyTest.cs +++ b/test/EFCore.Tests/Metadata/Internal/ForeignKeyTest.cs @@ -575,7 +575,7 @@ public void Can_change_cascade_delete_flag() [ConditionalFact] public void Can_change_cascade_ownership() { - var entityType = CreateModel().AddEntityType("E"); + var entityType = CreateModel().AddOwnedEntityType("E"); var keyProp = entityType.AddProperty("Id", typeof(int)); var dependentProp = entityType.AddProperty("P", typeof(int)); var principalProp = entityType.AddProperty("U", typeof(int)); @@ -583,12 +583,41 @@ public void Can_change_cascade_ownership() var principalKey = entityType.AddKey(principalProp); var foreignKey = entityType.AddForeignKey(new[] { dependentProp }, principalKey, entityType); + foreignKey.SetPrincipalToDependent("S"); Assert.False(foreignKey.IsOwnership); foreignKey.IsOwnership = true; Assert.True(foreignKey.IsOwnership); + + Assert.Equal( + CoreStrings.OwnershipToDependent( + "S", + "E (Dictionary)", + "E (Dictionary)"), + Assert.Throws(() => foreignKey.SetPrincipalToDependent((string)null)).Message); + } + + [ConditionalFact] + public void IsOwnership_throws_when_no_navigation() + { + var entityType = CreateModel().AddOwnedEntityType("E"); + var keyProp = entityType.AddProperty("Id", typeof(int)); + var dependentProp = entityType.AddProperty("P", typeof(int)); + var principalProp = entityType.AddProperty("U", typeof(int)); + entityType.SetPrimaryKey(keyProp); + var principalKey = entityType.AddKey(principalProp); + + var foreignKey = entityType.AddForeignKey(new[] { dependentProp }, principalKey, entityType); + + Assert.False(foreignKey.IsOwnership); + + Assert.Equal( + CoreStrings.NavigationlessOwnership( + "E (Dictionary)", + "E (Dictionary)"), + Assert.Throws(() => foreignKey.IsOwnership = true).Message); } [ConditionalFact] diff --git a/test/EFCore.Tests/Metadata/Internal/InternalEntityTypeBuilderTest.cs b/test/EFCore.Tests/Metadata/Internal/InternalEntityTypeBuilderTest.cs index 00211990f85..7d6399f7128 100644 --- a/test/EFCore.Tests/Metadata/Internal/InternalEntityTypeBuilderTest.cs +++ b/test/EFCore.Tests/Metadata/Internal/InternalEntityTypeBuilderTest.cs @@ -2849,14 +2849,14 @@ public void Cannot_set_base_type_for_relationship_with_explicit_conflicting_inco var modelBuilder = CreateModelBuilder(); var principalEntityBuilder = modelBuilder.Entity(typeof(Customer), ConfigurationSource.Explicit); var dependentEntityBuilder = modelBuilder.Entity(typeof(Order), ConfigurationSource.Explicit); - dependentEntityBuilder.HasRelationship( - principalEntityBuilder.Metadata, Order.CustomerProperty.Name, null, ConfigurationSource.Explicit); + Assert.NotNull(dependentEntityBuilder.HasRelationship( + principalEntityBuilder.Metadata, Order.CustomerProperty.Name, null, ConfigurationSource.Explicit)); var derivedPrincipalEntityBuilder = modelBuilder.Entity(typeof(SpecialCustomer), ConfigurationSource.Explicit) .HasBaseType((string)null, ConfigurationSource.Explicit); var derivedDependentEntityBuilder = modelBuilder.Entity(typeof(SpecialOrder), ConfigurationSource.Convention); - derivedDependentEntityBuilder.HasRelationship( - derivedPrincipalEntityBuilder.Metadata, Order.CustomerProperty.Name, null, ConfigurationSource.Explicit); + Assert.NotNull(derivedDependentEntityBuilder.HasRelationship( + derivedPrincipalEntityBuilder.Metadata, Order.CustomerProperty.Name, null, ConfigurationSource.Explicit)); Assert.Null(derivedDependentEntityBuilder.HasBaseType(dependentEntityBuilder.Metadata, ConfigurationSource.Convention)); Assert.Null(derivedDependentEntityBuilder.Metadata.BaseType); diff --git a/test/EFCore.Tests/Metadata/Internal/InternalForeignKeyBuilderTest.cs b/test/EFCore.Tests/Metadata/Internal/InternalForeignKeyBuilderTest.cs index 3755cad038e..3710bbd66df 100644 --- a/test/EFCore.Tests/Metadata/Internal/InternalForeignKeyBuilderTest.cs +++ b/test/EFCore.Tests/Metadata/Internal/InternalForeignKeyBuilderTest.cs @@ -42,7 +42,7 @@ public void Facets_are_configured_with_the_specified_source() Assert.Null(fk.GetIsUniqueConfigurationSource()); Assert.Null(fk.GetDeleteBehaviorConfigurationSource()); - relationshipBuilder = relationshipBuilder.PrincipalEntityType(principalEntityBuilder, ConfigurationSource.Explicit) + relationshipBuilder = relationshipBuilder.PrincipalEntityType(principalEntityBuilder.Metadata, ConfigurationSource.Explicit) .HasPrincipalKey(key.Metadata.Properties, ConfigurationSource.Explicit).HasNavigation( Order.CustomerProperty.Name, pointsToPrincipal: true, @@ -586,9 +586,10 @@ public void Can_only_override_lower_or_equal_source_Ownership() { var modelBuilder = CreateInternalModelBuilder(); var customerEntityBuilder = modelBuilder.Entity(typeof(Customer), ConfigurationSource.Explicit); - var orderEntityBuilder = modelBuilder.Entity(typeof(Order), ConfigurationSource.Explicit); + var orderEntityBuilder = modelBuilder.Entity(typeof(Order), ConfigurationSource.Explicit, shouldBeOwned: true); - var relationshipBuilder = orderEntityBuilder.HasRelationship(customerEntityBuilder.Metadata, ConfigurationSource.Convention); + var relationshipBuilder = orderEntityBuilder.HasRelationship( + customerEntityBuilder.Metadata, null, nameof(Customer.Orders), ConfigurationSource.Convention); Assert.False(relationshipBuilder.Metadata.IsOwnership); relationshipBuilder = relationshipBuilder.IsOwnership(true, ConfigurationSource.Convention); @@ -601,6 +602,24 @@ public void Can_only_override_lower_or_equal_source_Ownership() Assert.False(relationshipBuilder.Metadata.IsOwnership); } + [ConditionalFact] + public void HasRelationship_throws_when_incompatible_navigations() + { + var modelBuilder = CreateInternalModelBuilder(); + var customerEntityBuilder = modelBuilder.Entity(typeof(Customer), ConfigurationSource.Explicit); + var orderEntityBuilder = modelBuilder.Entity(typeof(Order), ConfigurationSource.Explicit); + + Assert.Equal( + CoreStrings.PrincipalEndIncompatibleNavigations( + "Customer.Orders", + "Order.Customer", + nameof(Order)), + Assert.Throws(() => + customerEntityBuilder.HasRelationship( + orderEntityBuilder.Metadata, nameof(Customer.Orders), nameof(Order.Customer), ConfigurationSource.Convention, + setTargetAsPrincipal: true)).Message); + } + [ConditionalFact] public void Can_only_invert_lower_or_equal_source() { diff --git a/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs b/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs index e71223add08..5f66a647e13 100644 --- a/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs +++ b/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs @@ -122,7 +122,7 @@ public void Cannot_ignore_higher_source_entity_type_using_entity_type_name() public void Can_ignore_existing_entity_type_using_entity_clr_type() { var model = new Model(); - var entityType = model.AddEntityType(typeof(Customer), ConfigurationSource.Explicit); + var entityType = model.AddEntityType(typeof(Customer), owned: false, ConfigurationSource.Explicit); var modelBuilder = CreateModelBuilder(model); Assert.Same(entityType, modelBuilder.Entity(typeof(Customer), ConfigurationSource.Convention).Metadata); Assert.Null(modelBuilder.Ignore(typeof(Customer), ConfigurationSource.DataAnnotation)); @@ -137,7 +137,7 @@ public void Can_ignore_existing_entity_type_using_entity_clr_type() public void Can_ignore_existing_entity_type_using_entity_type_name() { var model = new Model(); - var entityType = model.AddEntityType(typeof(Customer).FullName, ConfigurationSource.Explicit); + var entityType = model.AddEntityType(typeof(Customer).FullName, owned: false, ConfigurationSource.Explicit); var modelBuilder = CreateModelBuilder(model); Assert.Same(entityType, modelBuilder.Entity(typeof(Customer).FullName, ConfigurationSource.Convention).Metadata); @@ -302,13 +302,20 @@ public void Can_mark_type_as_owned_type() var entityBuilder = modelBuilder.Entity(typeof(Customer), ConfigurationSource.Explicit); - Assert.NotNull(modelBuilder.Entity(typeof(Details), ConfigurationSource.Convention)); + var ownedEntityTypeBuilder = modelBuilder.Entity(typeof(Details), ConfigurationSource.Convention); + Assert.NotNull(ownedEntityTypeBuilder); Assert.False(model.IsOwned(typeof(Details))); + Assert.False(ownedEntityTypeBuilder.Metadata.IsOwned()); - Assert.NotNull(entityBuilder.HasOwnership(typeof(Details), nameof(Customer.Details), ConfigurationSource.Convention)); + Assert.Null(entityBuilder.HasOwnership(typeof(Details), nameof(Customer.Details), ConfigurationSource.Convention)); + + Assert.NotNull(entityBuilder.HasOwnership(typeof(Details), nameof(Customer.Details), ConfigurationSource.DataAnnotation)); - Assert.NotNull(modelBuilder.Ignore(typeof(Details), ConfigurationSource.Convention)); + Assert.False(model.IsOwned(typeof(Details))); + Assert.True(ownedEntityTypeBuilder.Metadata.IsOwned()); + + Assert.NotNull(modelBuilder.Ignore(typeof(Details), ConfigurationSource.DataAnnotation)); Assert.Empty(model.FindEntityTypes(typeof(Details))); @@ -316,7 +323,7 @@ public void Can_mark_type_as_owned_type() Assert.Null(modelBuilder.Owned(typeof(Details), ConfigurationSource.Convention)); - Assert.NotNull(entityBuilder.HasOwnership(typeof(Details), nameof(Customer.Details), ConfigurationSource.DataAnnotation)); + Assert.NotNull(entityBuilder.HasOwnership(typeof(Details), nameof(Customer.Details), ConfigurationSource.Explicit)); Assert.NotNull(modelBuilder.Owned(typeof(Details), ConfigurationSource.Convention)); @@ -352,7 +359,7 @@ public void Can_mark_type_as_owned_type() Assert.Null(modelBuilder.Owned(typeof(Details), ConfigurationSource.Convention)); Assert.Equal( - CoreStrings.ClashingNonOwnedEntityType(typeof(Details).Name), + CoreStrings.ClashingNonOwnedEntityType("Details (Details)"), Assert.Throws(() => modelBuilder.Owned(typeof(Details), ConfigurationSource.Explicit)).Message); } @@ -381,6 +388,7 @@ public void Can_remove_implicitly_created_join_entity_type() model.AddEntityType( "JoinEntity", typeof(Dictionary), + owned: false, ConfigurationSource.Convention).Builder; var leftFK = joinEntityTypeBuilder .HasRelationship( diff --git a/test/EFCore.Tests/Metadata/Internal/ModelTest.cs b/test/EFCore.Tests/Metadata/Internal/ModelTest.cs index dbd28513450..69a3867317a 100644 --- a/test/EFCore.Tests/Metadata/Internal/ModelTest.cs +++ b/test/EFCore.Tests/Metadata/Internal/ModelTest.cs @@ -237,7 +237,7 @@ public void Adding_duplicate_shared_type_throws() var model = (Model)CreateModel(); Assert.Null(model.RemoveEntityType(typeof(Customer).FullName)); - model.AddEntityType(typeof(Customer), ConfigurationSource.Explicit); + model.AddEntityType(typeof(Customer), owned: false, ConfigurationSource.Explicit); Assert.Equal( CoreStrings.CannotMarkShared(nameof(Customer)), diff --git a/test/EFCore.Tests/ModelBuilding/ModelBuilderNonGenericStringTest.cs b/test/EFCore.Tests/ModelBuilding/ModelBuilderNonGenericStringTest.cs index 18532073b09..1a574278383 100644 --- a/test/EFCore.Tests/ModelBuilding/ModelBuilderNonGenericStringTest.cs +++ b/test/EFCore.Tests/ModelBuilding/ModelBuilderNonGenericStringTest.cs @@ -26,20 +26,6 @@ public class NonGenericStringOwnedTypes : OwnedTypesTestBase protected override TestModelBuilder CreateTestModelBuilder(TestHelpers testHelpers, Action? configure) => new NonGenericStringTestModelBuilder(testHelpers, configure); - public override void Reconfiguring_owned_type_as_non_owned_throws() - { - var modelBuilder = CreateModelBuilder(); - - modelBuilder.Ignore(); - modelBuilder.Entity().OwnsOne(c => c.Details); - - Assert.Equal( - CoreStrings.ClashingOwnedEntityType(typeof(CustomerDetails).FullName), - Assert.Throws( - () => - modelBuilder.Entity().HasOne(c => c.Details)).Message); - } - // Shadow navigations not supported #3864 public override void Can_configure_owned_type_collection_with_one_call() { diff --git a/test/EFCore.Tests/ModelBuilding/OwnedTypesTestBase.cs b/test/EFCore.Tests/ModelBuilding/OwnedTypesTestBase.cs index d4da1f8997b..4d2559bfdca 100644 --- a/test/EFCore.Tests/ModelBuilding/OwnedTypesTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/OwnedTypesTestBase.cs @@ -275,6 +275,7 @@ public virtual void Can_configure_another_relationship_to_owner() public virtual void Changing_ownership_uniqueness_throws() { var modelBuilder = CreateModelBuilder(); + var customerBuilder = modelBuilder.Entity(); Assert.Equal( CoreStrings.UnableToSetIsUnique( @@ -282,9 +283,7 @@ public virtual void Changing_ownership_uniqueness_throws() nameof(Customer.Details), nameof(Customer)), Assert.Throws( - () => modelBuilder - .Entity() - .OwnsOne( + () => customerBuilder.OwnsOne( c => c.Details, r => { @@ -739,15 +738,14 @@ public virtual void Ambiguous_relationship_between_owned_types_throws() { var modelBuilder = CreateModelBuilder(); - modelBuilder.Owned(); - modelBuilder.Owned(); - modelBuilder.Entity(); - modelBuilder.Ignore(); + modelBuilder.Owned(); + modelBuilder.Owned(); + modelBuilder.Entity(); Assert.Equal( CoreStrings.AmbiguousOwnedNavigation( - "ToastedBun.Whoopper#Whoopper.Mustard", - nameof(Mustard)), + "Book.AlternateLabel#BookLabel.Book", + nameof(Book)), Assert.Throws(() => modelBuilder.FinalizeModel()).Message); } @@ -1345,7 +1343,9 @@ public virtual void Deriving_from_owned_type_throws() }); Assert.Equal( - CoreStrings.ClashingOwnedEntityType(nameof(BookLabel)), + modelBuilder.Model.IsShared(typeof(BookLabel)) + ? CoreStrings.ClashingSharedType(nameof(BookLabel)) + : CoreStrings.ClashingOwnedEntityType(nameof(BookLabel)), Assert.Throws( () => modelBuilder.Entity().HasBaseType()).Message); } @@ -1411,14 +1411,18 @@ public virtual void OwnedType_can_derive_from_Collection() } [ConditionalFact] - public virtual void Weak_types_with_FK_to_another_entity_works() + public virtual void Shared_type_entity_types_with_FK_to_another_entity_works() { var modelBuilder = CreateModelBuilder(); modelBuilder.Entity(); var ownerEntityTypeBuilder = modelBuilder.Entity(); ownerEntityTypeBuilder.OwnsOne( e => e.Bill1, - o => o.HasOne().WithMany().HasPrincipalKey(c => c.Name).HasForeignKey(d => d.Country)); + o => + { + o.HasOne().WithMany().HasPrincipalKey(c => c.Name).HasForeignKey(d => d.Country); + o.HasIndex(c => c.Country); + }); ownerEntityTypeBuilder.OwnsOne( e => e.Bill2, @@ -1427,6 +1431,16 @@ public virtual void Weak_types_with_FK_to_another_entity_works() var model = modelBuilder.FinalizeModel(); Assert.Equal(4, model.GetEntityTypes().Count()); + + var owner = model.FindEntityType(typeof(BillingOwner)); + + var bill1 = owner.FindNavigation(nameof(BillingOwner.Bill1)).TargetEntityType; + Assert.Equal(2, bill1.GetForeignKeys().Count()); + Assert.Single(bill1.GetIndexes()); + + var bill2 = owner.FindNavigation(nameof(BillingOwner.Bill2)).TargetEntityType; + Assert.Equal(2, bill2.GetForeignKeys().Count()); + Assert.Single(bill2.GetIndexes()); } [ConditionalFact] @@ -1598,8 +1612,7 @@ public virtual void Shared_type_used_as_owned_type_throws_for_same_name() CoreStrings.ClashingNamedOwnedType( "Shared1", nameof(OwnerOfSharedType), nameof(OwnerOfSharedType.Collection)), Assert.Throws( - () => - b.OwnsMany("Shared1", e => e.Collection)).Message); + () => b.OwnsMany("Shared1", e => e.Collection)).Message); }); } }