Metadata: Warn for optional dependents sharing table without identify…

…ing column

Resolves #23229
Resolves #23198
Resolves #21488

src/EFCore.Relational/Diagnostics/RelationalEventId.cs

@@ -87,6 +87,7 @@ private enum Id
             IndexPropertiesBothMappedAndNotMappedToTable,
             IndexPropertiesMappedToNonOverlappingTables,
             ForeignKeyPropertiesMappedToUnrelatedTables,
+            OptionalDependentWithoutIdentifyingPropertyWarning,
 
             // Update events
             BatchReadyForExecution = CoreEventId.RelationalBaseId + 700,
@@ -723,6 +724,20 @@ private static EventId MakeValidationId(Id id)
         /// </summary>
         public static readonly EventId ForeignKeyPropertiesMappedToUnrelatedTables = MakeValidationId(Id.ForeignKeyPropertiesMappedToUnrelatedTables);
 
+        /// <summary>
+        ///     <para>
+        ///         A foreign key specifies properties which don't map to the related tables.
+        ///     </para>
+        ///     <para>
+        ///         This event is in the <see cref="DbLoggerCategory.Model.Validation" /> category.
+        ///     </para>
+        ///     <para>
+        ///         This event uses the <see cref="EntityTypeEventData" /> payload when used with a <see cref="DiagnosticSource" />.
+        ///     </para>
+        /// </summary>
+        public static readonly EventId OptionalDependentWithoutIdentifyingPropertyWarning
+            = MakeValidationId(Id.OptionalDependentWithoutIdentifyingPropertyWarning);
+
         private static readonly string _updatePrefix = DbLoggerCategory.Update.Name + ".";
 
         private static EventId MakeUpdateId(Id id)

src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs

@@ -4674,7 +4674,7 @@ private static string NamedIndexPropertiesMappedToNonOverlappingTables(EventDefi
         }
 
         /// <summary>
-        ///     Logs the <see cref="RelationalEventId.IndexPropertiesMappedToNonOverlappingTables" /> event.
+        ///     Logs the <see cref="RelationalEventId.ForeignKeyPropertiesMappedToUnrelatedTables" /> event.
         /// </summary>
         /// <param name="diagnostics"> The diagnostics logger to use. </param>
         /// <param name="foreignKey"> The foreign key. </param>
@@ -4729,6 +4729,40 @@ private static string ForeignKeyPropertiesMappedToUnrelatedTables(EventDefinitio
                         p.ForeignKey.PrincipalEntityType.GetSchemaQualifiedTableName()));
         }
 
+        /// <summary>
+        ///     Logs the <see cref="RelationalEventId.OptionalDependentWithoutIdentifyingPropertyWarning" /> event.
+        /// </summary>
+        /// <param name="diagnostics"> The diagnostics logger to use. </param>
+        /// <param name="entityType"> The entity type. </param>
+        public static void OptionalDependentWithoutIdentifyingPropertyWarning(
+            this IDiagnosticsLogger<DbLoggerCategory.Model.Validation> diagnostics,
+            IEntityType entityType)
+        {
+            var definition = RelationalResources.LogOptionalDependentWithoutIdentifyingProperty(diagnostics);
+
+            if (diagnostics.ShouldLog(definition))
+            {
+                definition.Log(diagnostics, entityType.DisplayName());
+            }
+
+            if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
+            {
+                var eventData = new EntityTypeEventData(
+                    definition,
+                    OptionalDependentWithoutIdentifyingPropertyWarning,
+                    entityType);
+
+                diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);
+            }
+        }
+
+        private static string OptionalDependentWithoutIdentifyingPropertyWarning(EventDefinitionBase definition, EventData payload)
+        {
+            var d = (EventDefinition<string>)definition;
+            var p = (EntityTypeEventData)payload;
+            return d.GenerateMessage(p.EntityType.DisplayName());
+        }
+
         /// <summary>
         ///     Logs for the <see cref="RelationalEventId.BatchExecutorFailedToRollbackToSavepoint" /> event.
         /// </summary>

src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs

@@ -503,5 +503,14 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions
         /// </summary>
         [EntityFrameworkInternal]
         public EventDefinitionBase? LogBatchExecutorFailedToReleaseSavepoint;
+
+        /// <summary>
+        ///     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.
+        /// </summary>
+        [EntityFrameworkInternal]
+        public EventDefinitionBase? LogOptionalDependentWithoutIdentifyingProperty;
     }
 }

src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs

@@ -301,6 +301,89 @@ protected virtual void ValidateSharedTableCompatibility(
                 ValidateSharedKeysCompatibility(mappedTypes, table, logger);
                 ValidateSharedForeignKeysCompatibility(mappedTypes, table, logger);
                 ValidateSharedIndexesCompatibility(mappedTypes, table, logger);
+
+                // Validate optional dependents
+                if (mappedTypes.Count == 1)
+                {
+                    continue;
+                }
+
+                var principalEntityTypesMap = new Dictionary<IEntityType, (List<IEntityType> EntityTypes, bool Optional)>();
+                foreach (var entityType in mappedTypes)
+                {
+                    if (entityType.BaseType != null
+                        || entityType.FindPrimaryKey() == null)
+                    {
+                        continue;
+                    }
+
+                    var (principalEntityTypes, optional) = GetPrincipalEntityTypes(entityType);
+                    if (!optional)
+                    {
+                        continue;
+                    }
+
+                    var principalColumns = principalEntityTypes.SelectMany(e => e.GetProperties())
+                        .Select(e => e.GetColumnName(table))
+                        .Where(e => e != null)
+                        .ToList();
+                    var requiredNonSharedColumnFound = false;
+                    foreach (var property in entityType.GetProperties())
+                    {
+                        if (property.IsPrimaryKey()
+                            || property.IsNullable)
+                        {
+                            continue;
+                        }
+
+                        var columnName = property.GetColumnName(table);
+                        if (columnName != null)
+                        {
+                            if (!principalColumns.Contains(columnName))
+                            {
+                                requiredNonSharedColumnFound = true;
+                                break;
+                            }
+                        }
+                    }
+                    if (!requiredNonSharedColumnFound)
+                    {
+                        if (entityType.GetReferencingForeignKeys().Select(e => e.DeclaringEntityType).Any(t => mappedTypes.Contains(t)))
+                        {
+                            throw new InvalidOperationException(
+                                RelationalStrings.OptionalDependentWithDependentWithoutIdentifyingProperty(entityType.DisplayName()));
+                        }
+
+                        logger.OptionalDependentWithoutIdentifyingPropertyWarning(entityType);
+                    }
+                }
+
+                (List<IEntityType> EntityTypes, bool Optional) GetPrincipalEntityTypes(IEntityType entityType)
+                {
+                    if (!principalEntityTypesMap.TryGetValue(entityType, out var tuple))
+                    {
+                        var list = new List<IEntityType>();
+                        var optional = false;
+                        foreach (var foreignKey in entityType.FindForeignKeys(entityType.FindPrimaryKey()!.Properties))
+                        {
+                            var principalEntityType = foreignKey.PrincipalEntityType;
+                            if (!mappedTypes.Contains(principalEntityType))
+                            {
+                                continue;
+                            }
+                            list.Add(principalEntityType);
+                            var (entityTypes, innerOptional) = GetPrincipalEntityTypes(principalEntityType.GetRootType());
+                            list.AddRange(entityTypes);
+
+                            optional |= !foreignKey.IsRequiredDependent | innerOptional;
+                        }
+
+                        tuple = (list, optional);
+                        principalEntityTypesMap.Add(entityType, tuple);
+                    }
+
+                    return tuple;
+                }
             }
         }
 

src/EFCore.Relational/Properties/RelationalStrings.resx

@@ -550,6 +550,10 @@
     <value>Opening connection to database '{database}' on server '{server}'.</value>
     <comment>Debug RelationalEventId.ConnectionOpening string string</comment>
   </data>
+  <data name="LogOptionalDependentWithoutIdentifyingProperty" xml:space="preserve">
+    <value>Entity type '{entityType}' is an optional dependent in table sharing without any required non shared property to identify if the entity type exist. If all nullable properties contain null value in database then an object instance won't be materialized in the query.</value>
+    <comment>Error RelationalEventId.ModelValidationOptionalDependentWithoutIdentifyingPropertyWarning string</comment>
+  </data>
   <data name="LogPossibleUnintendedUseOfEquals" xml:space="preserve">
     <value>Possible unintended use of method 'Equals' for arguments '{left}' and '{right}' of different types in a query. This comparison will always return false.</value>
     <comment>Warning RelationalEventId.QueryPossibleUnintendedUseOfEqualsWarning string string</comment>
@@ -675,6 +679,9 @@
   <data name="NullTypeMappingInSqlTree" xml:space="preserve">
     <value>Expression '{sqlExpression}' in the SQL tree does not have a type mapping assigned.</value>
   </data>
+  <data name="OptionalDependentWithDependentWithoutIdentifyingProperty" xml:space="preserve">
+    <value>Entity type '{entityType}' is an optional dependent containing other dependents in table sharing without any required non shared property to identify if the entity type exist. If all nullable properties contain null value in database then an object instance won't be materialized in the query causing nested dependent's values to be lost.</value>
+  </data>
   <data name="ParameterNotObjectArray" xml:space="preserve">
     <value>Cannot use the value provided for parameter '{parameter}' because it isn't assignable to type object[].</value>
   </data>

src/EFCore/Diagnostics/EntityTypeEventData.cs

@@ -0,0 +1,36 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Diagnostics;
+using Microsoft.EntityFrameworkCore.Metadata;
+
+namespace Microsoft.EntityFrameworkCore.Diagnostics
+{
+    /// <summary>
+    ///     A <see cref="DiagnosticSource" /> event payload class for events that have
+    ///     an entity type.
+    /// </summary>
+    public class EntityTypeEventData : EventData
+    {
+        /// <summary>
+        ///     Constructs the event payload.
+        /// </summary>
+        /// <param name="eventDefinition"> The event definition. </param>
+        /// <param name="messageGenerator"> A delegate that generates a log message for this event. </param>
+        /// <param name="entityType"> The entityType. </param>
+        public EntityTypeEventData(
+            EventDefinitionBase eventDefinition,
+            Func<EventDefinitionBase, EventData, string> messageGenerator,
+            IReadOnlyEntityType entityType)
+            : base(eventDefinition, messageGenerator)
+        {
+            EntityType = entityType;
+        }
+
+        /// <summary>
+        ///     The entity type.
+        /// </summary>
+        public virtual IReadOnlyEntityType EntityType { get; }
+    }
+}

test/EFCore.Cosmos.FunctionalTests/Query/OwnedQueryCosmosTest.cs

@@ -674,7 +674,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                         eb.OwnsOne(
                             p => p.BranchAddress, ab =>
                             {
-                                ab.IndexerProperty<string>("BranchName");
+                                ab.IndexerProperty<string>("BranchName").IsRequired();
                                 ab.HasData(
                                     new
                                     {
@@ -760,7 +760,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                         eb.OwnsOne(
                             p => p.LeafBAddress, ab =>
                             {
-                                ab.IndexerProperty<string>("LeafBType");
+                                ab.IndexerProperty<string>("LeafBType").IsRequired();
                                 ab.HasData(
                                     new
                                     {