Fix for 20014. KeyAttribute does not override Keyless. Log warning.

src/EFCore/Diagnostics/CoreEventId.cs

@@ -102,6 +102,7 @@ private enum Id
             RequiredAttributeInverted,
             RequiredAttributeOnCollection,
             CollectionWithoutComparer,
+            ConflictingKeylessAndKeyConfigurationWarning,
 
             // ChangeTracking events
             DetectChangesStarting = CoreBaseId + 800,
@@ -615,6 +616,21 @@ public static readonly EventId PossibleUnintendedReferenceComparisonWarning
         /// </summary>
         public static readonly EventId RedundantForeignKeyWarning = MakeModelValidationId(Id.RedundantForeignKeyWarning);
 
+        /// <summary>
+        ///     <para>
+        ///         The IsKeyless setting on the entity type is conflicting with
+        ///         an attempt to set a key on at least one of its properties.
+        ///     </para>
+        ///     <para>
+        ///         This event is in the <see cref="DbLoggerCategory.Model.Validation" /> category.
+        ///     </para>
+        ///     <para>
+        ///         This event uses the <see cref="PropertyEventData" /> payload when used with a
+        ///         <see cref="DiagnosticSource" />.
+        ///     </para>
+        /// </summary>
+        public static readonly EventId ConflictingKeylessAndKeyConfigurationWarning = MakeModelValidationId(Id.ConflictingKeylessAndKeyConfigurationWarning);
+
         private static readonly string _changeTrackingPrefix = DbLoggerCategory.ChangeTracking.Name + ".";
         private static EventId MakeChangeTrackingId(Id id) => new EventId((int)id, _changeTrackingPrefix + id);
 

src/EFCore/Diagnostics/CoreLoggerExtensions.cs

@@ -2912,5 +2912,41 @@ private static string ContextDisposed(EventDefinitionBase definition, EventData
             var p = (DbContextEventData)payload;
             return d.GenerateMessage(p.Context.GetType().ShortDisplayName());
         }
+
+        /// <summary>
+        ///     Logs for the <see cref="CoreEventId.ConflictingKeylessAndKeyConfigurationWarning" /> event.
+        /// </summary>
+        /// <param name="diagnostics"> The diagnostics logger to use. </param>
+        /// <param name="property"> The property which is being defined as part of a key. </param>
+        public static void ConflictingKeylessAndKeyConfigurationWarning(
+            [NotNull] this IDiagnosticsLogger<DbLoggerCategory.Model.Validation> diagnostics,
+            [NotNull] IProperty property)
+        {
+            var definition = CoreResources.LogConflictingKeylessAndKeyConfiguration(diagnostics);
+
+            if (diagnostics.ShouldLog(definition))
+            {
+                definition.Log(diagnostics, property.Name, property.DeclaringEntityType.DisplayName());
+            }
+
+            if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
+            {
+                var eventData = new PropertyEventData(
+                    definition,
+                    ConflictingKeylessAndKeyConfigurationWarning,
+                    property);
+
+                diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);
+            }
+        }
+
+        private static string ConflictingKeylessAndKeyConfigurationWarning(EventDefinitionBase definition, EventData payload)
+        {
+            var d = (EventDefinition<string, string>)definition;
+            var p = (PropertyEventData)payload;
+            return d.GenerateMessage(
+                p.Property.Name,
+                p.Property.DeclaringEntityType.DisplayName());
+        }
     }
 }

src/EFCore/Diagnostics/LoggingDefinitions.cs

@@ -636,5 +636,14 @@ public abstract class LoggingDefinitions
         /// </summary>
         [EntityFrameworkInternal]
         public EventDefinitionBase LogRedundantForeignKey;
+
+        /// <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 LogConflictingKeylessAndKeyConfiguration;
     }
 }

src/EFCore/Metadata/Conventions/KeyAttributeConvention.cs

@@ -42,6 +42,15 @@ protected override void ProcessPropertyAdded(
             IConventionContext context)
         {
             var entityType = propertyBuilder.Metadata.DeclaringEntityType;
+            if (entityType.IsKeyless
+                && entityType.GetIsKeylessConfigurationSource()
+                    .Overrides(ConfigurationSource.DataAnnotation))
+            {
+                Dependencies.ValidationLogger
+                    .ConflictingKeylessAndKeyConfigurationWarning(propertyBuilder.Metadata);
+                return;
+            }
+
             if (entityType.BaseType != null)
             {
                 return;

src/EFCore/Metadata/IConventionEntityType.cs

@@ -69,6 +69,12 @@ public interface IConventionEntityType : IEntityType, IConventionTypeBase
         /// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
         void HasNoKey(bool? keyless, bool fromDataAnnotation = false);
 
+        /// <summary>
+        ///     Returns the configuration source for the IsKeyless property.
+        /// </summary>
+        /// <returns> The configuration source for the IsKeyless property. </returns>
+        ConfigurationSource? GetIsKeylessConfigurationSource();
+
         /// <summary>
         ///     Sets the primary key for this entity type.
         /// </summary>

src/EFCore/Properties/CoreStrings.resx

@@ -1269,4 +1269,8 @@
   <data name="LastUsedWithoutOrderBy" xml:space="preserve">
     <value>Queries performing '{method}' operation must have a deterministic sort order. Rewrite the query to apply an OrderBy clause on the sequence before calling '{method}'.</value>
   </data>
+  <data name="LogConflictingKeylessAndKeyConfiguration" xml:space="preserve">
+    <value>The property '{property}' is being defined as being part of a key, but its entity '{entity}' is defined as being keyless.</value>
+    <comment>Warning CoreEventId.ConflictingKeylessAndKeyConfigurationWarning string string</comment>
+  </data>
 </root>

test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs

@@ -93,6 +93,23 @@ public virtual void Detects_custom_converter_for_collection_type_without_compare
                  convertedProperty.DeclaringEntityType.Model);
         }
 
+        [ConditionalFact]
+        public virtual void Warns_on_attempt_to_set_key_when_entity_is_keyless()
+        {
+            var model = CreateConventionalModelBuilder().Model;
+
+            var entity = model.AddEntityType(typeof(KeylessAndKeyConflict));
+
+            VerifyWarning(
+                CoreResources.LogConflictingKeylessAndKeyConfiguration(
+                    new TestLogger<TestLoggingDefinitions>())
+                    .GenerateMessage("NotAKey", nameof(KeylessAndKeyConflict)),
+                model);
+
+            Assert.True(entity.IsKeyless);
+            Assert.Null(entity.FindPrimaryKey());
+        }
+
         [ConditionalFact]
         public virtual void Ignores_custom_converter_for_collection_type_with_comparer()
         {

test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.ComponentModel.DataAnnotations;
 using System.ComponentModel.DataAnnotations.Schema;
 using System.Diagnostics;
 using System.Linq;
@@ -107,6 +108,13 @@ protected class F : D
         {
         }
 
+        [Keyless]
+        protected class KeylessAndKeyConflict
+        {
+            [Key] // should be ignored because of Keyless above
+            public int NotAKey { get; set; }
+        }
+
         protected abstract class Abstract : A
         {
         }

test/EFCore.Tests/Metadata/Conventions/EntityTypeAttributeConventionTest.cs

@@ -120,14 +120,14 @@ public void KeylessAttribute_can_be_overriden_using_explicit_configuration()
         }
 
         [ConditionalFact]
-        public void KeyAttribute_overrides_keyless_attribute()
+        public void KeyAttribute_does_not_override_keyless_attribute()
         {
             var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder();
 
             var entityBuilder = modelBuilder.Entity<KeyClash>();
 
-            Assert.False(entityBuilder.Metadata.IsKeyless);
-            Assert.NotNull(entityBuilder.Metadata.FindPrimaryKey());
+            Assert.True(entityBuilder.Metadata.IsKeyless);
+            Assert.Null(entityBuilder.Metadata.FindPrimaryKey());
         }
 
         #endregion