Make ownership handling during model building more consistent

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;
@@ -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,

src/EFCore.Relational/Metadata/Conventions/TableSharingConcurrencyTokenConvention.cs

@@ -193,21 +193,22 @@ public static bool IsConcurrencyTokenMissing(
  IReadOnlyEntityType entityType,
  IReadOnlyList<IReadOnlyEntityType> 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;

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

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