Make ownership handling during model building more consistent

Store Owned configuration on EntityType
Add methods to IMutableModel and IConventionModel for creating owned entity types
Mark entity types discovered from DbSet properties on DbContext as configured explicitly
Make sure ownerships have an associated principal to dependent navigation at all times
Store owned types in a field instead of an annotation

Fixes #25138

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

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());
             }
         }

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

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;

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;
@@ -1477,8 +1476,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,

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);
         }
     }
 }

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

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<Transaction> _ambientTransactions = new();
         private DbConnection? _connection;
         private readonly IRelationalCommandBuilder _relationalCommandBuilder;

src/EFCore.SqlServer/Extensions/SqlServerIndexBuilderExtensions.cs

@@ -138,7 +138,7 @@ public static IndexBuilder<TEntity> IncludeProperties<TEntity>(
         /// <returns> A builder to further configure the index. </returns>
         public static IndexBuilder<TEntity> IncludeProperties<TEntity>(
             this IndexBuilder<TEntity> indexBuilder,
-            Expression<Func<TEntity, object>> includeExpression)
+            Expression<Func<TEntity, object?>> includeExpression)
         {
             Check.NotNull(indexBuilder, nameof(indexBuilder));
             Check.NotNull(includeExpression, nameof(includeExpression));

src/EFCore/ChangeTracking/EntityEntry.cs

@@ -325,7 +325,8 @@ public virtual PropertyValues CurrentValues
         ///     </para>
         ///     <para>
         ///         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.
         ///     </para>
         /// </summary>
         /// <value> The original values. </value>
@@ -338,14 +339,14 @@ public virtual PropertyValues OriginalValues
         /// <summary>
         ///     <para>
         ///         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 <see langword="null"/> is returned.
         ///     </para>
         ///     <para>
         ///         Note that changing the values in the returned dictionary will not update the values
         ///         in the database.
         ///     </para>
         /// </summary>
-        /// <returns> The store values, or null if the entity does not exist in the database. </returns>
+        /// <returns> The store values, or <see langword="null"/> if the entity does not exist in the database. </returns>
         public virtual PropertyValues? GetDatabaseValues()
         {
             var values = Finder.GetDatabaseValues(InternalEntry);

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);
             }
 

src/EFCore/Diagnostics/CoreEventId.cs

@@ -146,7 +146,7 @@ private static EventId MakeUpdateId(Id id)
         public static readonly EventId SaveChangesFailed = MakeUpdateId(Id.SaveChangesFailed);
 
         /// <summary>
-        ///     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 <see cref="DbLoggerCategory.Update" /> category.
         /// </summary>
         public static readonly EventId DuplicateDependentEntityTypeInstanceWarning =

src/EFCore/Infrastructure/IModelCacheKeyFactory.cs

@@ -31,7 +31,8 @@ public interface IModelCacheKeyFactory
         /// </param>
         /// <returns> The created key. </returns>
         [Obsolete("Use the overload with most parameters")]
-        object Create(DbContext context);
+        object Create(DbContext context)
+            => Create(context, true);
 
         /// <summary>
         ///     Gets the model cache key for a given context.

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<string>(StringComparer.Ordinal);
-
                 var runtimeProperties = entityType.GetRuntimeProperties();
-
+                var clrProperties = new HashSet<string>(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,53 +210,40 @@ 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
                                 || targetSequenceType == null)
                             && 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,13 +690,20 @@ 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()));
                     }
 
+                    if (ownership.PrincipalToDependent == null)
+                    {
+                        throw new InvalidOperationException(CoreStrings.NavigationlessOwnership(
+                            ownership.PrincipalEntityType.DisplayName(), entityType.DisplayName()));
+                    }
+
                     foreach (var referencingFk in entityType.GetReferencingForeignKeys().Where(
                         fk => !fk.IsOwnership
                             && !Contains(fk.DeclaringEntityType.FindOwnership(), fk)))
@@ -742,7 +734,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()));
                 }

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);

src/EFCore/Metadata/Builders/CollectionCollectionBuilder`.cs

@@ -94,7 +94,7 @@ public virtual EntityTypeBuilder<TJoinEntity> UsingEntity<TJoinEntity>(
 
             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<TJoinEntity>(joinEntityType);

src/EFCore/Metadata/Builders/DiscriminatorBuilder.cs

@@ -92,7 +92,7 @@ public virtual DiscriminatorBuilder HasValue<TEntity>(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)!;
         }

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;
 
         /// <summary>
         ///     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;
 
         /// <summary>
         ///     Configures the <see cref="ChangeTrackingStrategy" /> to be used for this entity type.