Warn for composite generated keys on SQLite (#28534)

src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs

@@ -1594,11 +1594,11 @@ protected override void ValidateInheritanceMapping(
                     throw new InvalidOperationException(
                        RelationalStrings.AbstractTpc(entityType.DisplayName(), storeObject));
                 }
-                
-                foreach (var key in entityType.GetKeys())
-                {
-                    ValidateValueGenerationForMappingStrategy(entityType, key, mappingStrategy, logger);
-                }
+            }
+
+            foreach (var key in entityType.GetKeys())
+            {
+                ValidateValueGeneration(entityType, key, logger);
             }
 
             if (entityType.BaseType != null)
@@ -1681,20 +1681,18 @@ protected override void ValidateInheritanceMapping(
     }
 
     /// <summary>
-    ///     Validates the key value generation is valid for the given inheritance mapping strategy.
+    ///     Validates the key value generation is valid.
     /// </summary>
     /// <param name="entityType">The entity type.</param>
     /// <param name="key">The key.</param>
-    /// <param name="mappingStrategy">The inheritance mapping strategy.</param>
     /// <param name="logger">The logger to use.</param>
-    protected virtual void ValidateValueGenerationForMappingStrategy(
+    protected virtual void ValidateValueGeneration(
         IEntityType entityType,
         IKey key,
-        string mappingStrategy,
         IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
     {
         if (entityType.GetTableName() != null
-            && mappingStrategy == RelationalAnnotationNames.TpcMappingStrategy)
+            && (string?)entityType[RelationalAnnotationNames.MappingStrategy] == RelationalAnnotationNames.TpcMappingStrategy)
         {
             foreach (var storeGeneratedProperty in key.Properties.Where(p => (p.ValueGenerated & ValueGenerated.OnAdd) != 0))
             {

src/EFCore.SqlServer/Infrastructure/Internal/SqlServerModelValidator.cs

@@ -141,20 +141,18 @@ protected virtual void ValidateNonKeyValueGeneration(
     }
 
     /// <summary>
-    ///     Validates the key value generation is valid for the given inheritance mapping strategy.
+    ///     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>
-    /// <param name="entityType">The entity type.</param>
-    /// <param name="key">The key.</param>
-    /// <param name="mappingStrategy">The inheritance mapping strategy.</param>
-    /// <param name="logger">The logger to use.</param>
-    protected override void ValidateValueGenerationForMappingStrategy(
+    protected override void ValidateValueGeneration(
         IEntityType entityType,
         IKey key,
-        string mappingStrategy,
         IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
     {
         if (entityType.GetTableName() != null
-            && mappingStrategy == RelationalAnnotationNames.TpcMappingStrategy)
+            && (string?)entityType[RelationalAnnotationNames.MappingStrategy] == RelationalAnnotationNames.TpcMappingStrategy)
         {
             foreach (var storeGeneratedProperty in key.Properties.Where(
                          p => (p.ValueGenerated & ValueGenerated.OnAdd) != 0

src/EFCore.Sqlite.Core/Diagnostics/Internal/SqliteLoggingDefinitions.cs

@@ -129,5 +129,5 @@ public class SqliteLoggingDefinitions : RelationalLoggingDefinitions
     ///     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>
-    public EventDefinitionBase? LogTableRebuildsPendingWarning;
+    public EventDefinitionBase? LogCompositeKeyWithValueGeneration;
 }

src/EFCore.Sqlite.Core/Diagnostics/SqliteEventId.cs

@@ -29,6 +29,7 @@ private enum Id
         // Model validation events
         SchemaConfiguredWarning = CoreEventId.ProviderBaseId,
         SequenceConfiguredWarning,
+        CompositeKeyWithValueGeneration,
 
         // Infrastructure events
         UnexpectedConnectionTypeWarning = CoreEventId.ProviderBaseId + 100,
@@ -80,6 +81,20 @@ private static EventId MakeValidationId(Id id)
     /// </remarks>
     public static readonly EventId SequenceConfiguredWarning = MakeValidationId(Id.SequenceConfiguredWarning);
 
+    /// <summary>
+    ///     An entity type has composite key which is configured to use generated values. SQLite does not support generated values
+    ///     on composite keys.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         This event is in the <see cref="DbLoggerCategory.Model.Validation" /> category.
+    ///     </para>
+    ///     <para>
+    ///         This event uses the <see cref="KeyEventData" /> payload when used with a <see cref="DiagnosticSource" />.
+    ///     </para>
+    /// </remarks>
+    public static readonly EventId CompositeKeyWithValueGeneration = MakeValidationId(Id.CompositeKeyWithValueGeneration);
+
     private static readonly string InfraPrefix = DbLoggerCategory.Infrastructure.Name + ".";
 
     private static EventId MakeInfraId(Id id)

src/EFCore.Sqlite.Core/Extensions/Internal/SqliteLoggerExtensions.cs

@@ -374,4 +374,43 @@ private static string TableRebuildPendingWarning(EventDefinitionBase definition,
         var p = (TableRebuildEventData)payload;
         return d.GenerateMessage(p.OperationType.ShortDisplayName(), p.TableName);
     }
+
+    /// <summary>
+    ///     Logs the <see cref="SqliteEventId.CompositeKeyWithValueGeneration" /> event.
+    /// </summary>
+    /// <param name="diagnostics">The diagnostics logger to use.</param>
+    /// <param name="key">The key.</param>
+    public static void CompositeKeyWithValueGeneration(
+        this IDiagnosticsLogger<DbLoggerCategory.Model.Validation> diagnostics,
+        IKey key)
+    {
+        var definition = SqliteResources.LogCompositeKeyWithValueGeneration(diagnostics);
+
+        if (diagnostics.ShouldLog(definition))
+        {
+            definition.Log(
+                diagnostics,
+                key.DeclaringEntityType.DisplayName(),
+                key.Properties.Format());
+        }
+
+        if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
+        {
+            var eventData = new KeyEventData(
+                definition,
+                CompositeKeyWithValueGeneration,
+                key);
+
+            diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);
+        }
+    }
+
+    private static string CompositeKeyWithValueGeneration(EventDefinitionBase definition, EventData payload)
+    {
+        var d = (EventDefinition<string?, string?>)definition;
+        var p = (KeyEventData)payload;
+        return d.GenerateMessage(
+            p.Key.DeclaringEntityType.DisplayName(),
+            p.Key.Properties.Format());
+    }
 }

src/EFCore.Sqlite.Core/Infrastructure/Internal/SqliteModelValidator.cs

@@ -101,4 +101,31 @@ protected override void ValidateCompatible(
                     storeObject.DisplayName()));
         }
     }
+
+    /// <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>
+    protected override void ValidateValueGeneration(
+        IEntityType entityType,
+        IKey key,
+        IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
+    {
+        base.ValidateValueGeneration(entityType, key, logger);
+
+        var keyProperties = key.Properties;
+        if (key.IsPrimaryKey()
+            && keyProperties.Count(p => p.ClrType.UnwrapNullableType().IsInteger()) > 1
+            && keyProperties.Any(
+                p => p.ValueGenerated == ValueGenerated.OnAdd
+                    && p.ClrType.UnwrapNullableType().IsInteger()
+                    && !p.TryGetDefaultValue(out _)
+                    && p.GetDefaultValueSql() == null
+                    && !p.IsForeignKey()))
+        {
+            logger.CompositeKeyWithValueGeneration(key);
+        }
+    }
 }

src/EFCore.Sqlite.Core/Properties/SqliteStrings.resx

@@ -129,6 +129,10 @@
   <data name="InvalidMigrationOperation" xml:space="preserve">
     <value>SQLite does not support this migration operation ('{operation}'). See http://go.microsoft.com/fwlink/?LinkId=723262 for more information and examples.</value>
   </data>
+  <data name="LogCompositeKeyWithValueGeneration" xml:space="preserve">
+    <value>The entity type '{entityType}' has composite key '{key}' which is configured to use generated values. SQLite does not support generated values on composite keys.</value>
+    <comment>Warning SqliteEventId.CompositeKeyWithValueGeneration string? string?</comment>
+  </data>
   <data name="LogForeignKeyScaffoldErrorPrincipalTableNotFound" xml:space="preserve">
     <value>Skipping foreign key with identity '{id}' on table '{tableName}' since principal table '{principalTableName}' was not found in the model. This usually happens when the principal table was not included in the selection set.</value>
     <comment>Warning SqliteEventId.ForeignKeyReferencesMissingTableWarning string? string? string?</comment>
@@ -194,4 +198,4 @@
   <data name="SequencesNotSupported" xml:space="preserve">
     <value>SQLite does not support sequences. See http://go.microsoft.com/fwlink/?LinkId=723262 for more information and examples.</value>
   </data>
-</root>
+</root>

test/EFCore.Relational.Specification.Tests/Query/OwnedEntityQueryRelationalTestBase.cs

@@ -55,6 +55,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
                                 {
                                     oob.ToTable(nameof(Leaf24777));
                                     oob.HasKey(x => new { ProductCommissionRulesetId = x.ModdleAId, x.UnitThreshold });
+                                    oob.Property(x => x.ModdleAId).ValueGeneratedNever();
+                                    oob.Property(x => x.UnitThreshold).ValueGeneratedNever();
                                     oob.WithOwner().HasForeignKey(e => e.ModdleAId);
                                     oob.HasData(
                                         new Leaf24777 { ModdleAId = 1, UnitThreshold = 1 },

test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs

@@ -1181,6 +1181,7 @@ public virtual void Detects_duplicate_foreignKey_names_within_hierarchy_with_dif
         modelBuilder.Entity<Cat>().HasOne<Person>().WithMany().HasForeignKey("FriendId", "Shadow").HasPrincipalKey(
             p => new { p.Id, p.Name }).HasConstraintName("FK");
         modelBuilder.Entity<Dog>().HasOne<Person>().WithMany().HasForeignKey("FriendId").HasConstraintName("FK");
+        modelBuilder.Entity<Person>().Property(e => e.Id).ValueGeneratedNever();
 
         VerifyError(
             RelationalStrings.DuplicateForeignKeyColumnMismatch(
@@ -3214,6 +3215,28 @@ public virtual void Throws_when_non_tph_entity_type_discriminator_set_to_non_str
         VerifyError(RelationalStrings.NonTphDiscriminatorValueNotString(1, "TpcDerived"), modelBuilder);
     }
 
+    [ConditionalFact]
+    public virtual void Store_generated_in_composite_key()
+    {
+        var modelBuilder = CreateConventionModelBuilder();
+        modelBuilder.Entity<CarbonComposite>(
+            b =>
+            {
+                b.HasKey(e => new { e.Id1, e.Id2 });
+                b.Property(e => e.Id2).ValueGeneratedOnAdd();
+            });
+
+        Validate(modelBuilder);
+
+        var entityType = modelBuilder.Model.FindEntityType(typeof(CarbonComposite))!;
+        var keyProperties = entityType.FindPrimaryKey()!.Properties;
+        Assert.Equal(2, keyProperties.Count);
+        Assert.Equal(nameof(CarbonComposite.Id1), keyProperties[0].Name);
+        Assert.Equal(nameof(CarbonComposite.Id2), keyProperties[1].Name);
+        Assert.Equal(ValueGenerated.Never, keyProperties[0].ValueGenerated);
+        Assert.Equal(ValueGenerated.OnAdd, keyProperties[1].ValueGenerated);
+    }
+
     private class TpcBase
     {
         public int Id { get; set; }

test/EFCore.Sqlite.FunctionalTests/CompositeKeyEndToEndSqliteTest.cs

@@ -17,6 +17,9 @@ public override Task Can_use_generated_values_in_composite_key_end_to_end()
 
     public class CompositeKeyEndToEndSqliteFixture : CompositeKeyEndToEndFixtureBase
     {
+        public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
+            => base.AddOptions(builder.ConfigureWarnings(b => b.Ignore(SqliteEventId.CompositeKeyWithValueGeneration)));
+
         protected override ITestStoreFactory TestStoreFactory
             => SqliteTestStoreFactory.Instance;
     }

test/EFCore.Sqlite.FunctionalTests/GraphUpdates/GraphUpdatesSqliteTestBase.cs

@@ -49,6 +49,9 @@ public TestSqlLoggerFactory TestSqlLoggerFactory
         protected override ITestStoreFactory TestStoreFactory
             => SqliteTestStoreFactory.Instance;
 
+        public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
+            => base.AddOptions(builder.ConfigureWarnings(b => b.Ignore(SqliteEventId.CompositeKeyWithValueGeneration)));
+
         protected virtual bool AutoDetectChanges
             => false;
 

test/EFCore.Sqlite.FunctionalTests/Query/OwnedEntityQuerySqliteTest.cs

@@ -7,4 +7,7 @@ public class OwnedEntityQuerySqliteTest : OwnedEntityQueryRelationalTestBase
 {
     protected override ITestStoreFactory TestStoreFactory
         => SqliteTestStoreFactory.Instance;
+
+    protected override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
+        => base.AddOptions(builder.ConfigureWarnings(b => b.Ignore(SqliteEventId.CompositeKeyWithValueGeneration)));
 }

test/EFCore.Sqlite.FunctionalTests/Query/OwnedQuerySqliteTest.cs

@@ -12,6 +12,9 @@ public OwnedQuerySqliteTest(OwnedQuerySqliteFixture fixture)
 
     public class OwnedQuerySqliteFixture : RelationalOwnedQueryFixture
     {
+        public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
+            => base.AddOptions(builder.ConfigureWarnings(b => b.Ignore(SqliteEventId.CompositeKeyWithValueGeneration)));
+
         protected override ITestStoreFactory TestStoreFactory
             => SqliteTestStoreFactory.Instance;
     }

test/EFCore.Sqlite.Tests/Infrastructure/SqliteModelValidatorTest.cs

@@ -72,6 +72,22 @@ public void Detects_sequences()
             modelBuilder);
     }
 
+    public override void Store_generated_in_composite_key()
+    {
+        var modelBuilder = CreateConventionModelBuilder();
+        modelBuilder.Entity<CarbonComposite>(
+            b =>
+            {
+                b.HasKey(e => new { e.Id1, e.Id2 });
+                b.Property(e => e.Id2).ValueGeneratedOnAdd();
+            });
+
+        VerifyWarning(
+            SqliteResources.LogCompositeKeyWithValueGeneration(
+                new TestLogger<SqliteLoggingDefinitions>()).GenerateMessage(nameof(CarbonComposite), "{'Id1', 'Id2'}"),
+            modelBuilder);
+    }
+
     protected override TestHelpers TestHelpers
         => SqliteTestHelpers.Instance;
 }

test/EFCore.Sqlite.Tests/SqliteEventIdTest.cs

@@ -14,12 +14,14 @@ public class SqliteEventIdTest : EventIdTestBase
     public void Every_eventId_has_a_logger_method_and_logs_when_level_enabled()
     {
         var entityType = new EntityType(typeof(object), new Model(new ConventionSet()), owned: false, ConfigurationSource.Convention);
+        var property = entityType.AddProperty("A", typeof(int), ConfigurationSource.Convention, ConfigurationSource.Convention);
         entityType.Model.FinalizeModel();
 
         var fakeFactories = new Dictionary<Type, Func<object>>
         {
             { typeof(string), () => "Fake" },
             { typeof(IEntityType), () => entityType },
+            { typeof(IKey), () => new Key(new[] { property }, ConfigurationSource.Convention) },
             { typeof(IReadOnlySequence), () => new FakeSequence() },
             { typeof(Type), () => typeof(object) }
         };

test/EFCore.Tests/Infrastructure/ModelValidatorTest.PropertyMapping.cs

@@ -361,4 +361,10 @@ protected class OwnedEntity
     {
         public string Value { get; set; }
     }
+
+    protected class CarbonComposite
+    {
+        public int Id1 { get; set; }
+        public int Id2 { get; set; }
+    }
 }