Stop eagerly throwing when setting value generation strategy

Fixes #33413

Because value converters are now supported for generated properties, but the converter may not be configured yet.

src/EFCore.SqlServer/Extensions/SqlServerPropertyBuilderExtensions.cs

@@ -695,10 +695,8 @@ public static bool CanSetValueGenerationStrategy(
         this IConventionPropertyBuilder propertyBuilder,
         SqlServerValueGenerationStrategy? valueGenerationStrategy,
         bool fromDataAnnotation = false)
-        => (valueGenerationStrategy == null
-                || SqlServerPropertyExtensions.IsCompatibleWithValueGeneration(propertyBuilder.Metadata))
-            && propertyBuilder.CanSetAnnotation(
-                SqlServerAnnotationNames.ValueGenerationStrategy, valueGenerationStrategy, fromDataAnnotation);
+        => propertyBuilder.CanSetAnnotation(
+            SqlServerAnnotationNames.ValueGenerationStrategy, valueGenerationStrategy, fromDataAnnotation);
 
     /// <summary>
     ///     Returns a value indicating whether the given value can be set as the value generation strategy for a particular table.
@@ -718,14 +716,12 @@ public static bool CanSetValueGenerationStrategy(
         SqlServerValueGenerationStrategy? valueGenerationStrategy,
         in StoreObjectIdentifier storeObject,
         bool fromDataAnnotation = false)
-        => (valueGenerationStrategy == null
-                || SqlServerPropertyExtensions.IsCompatibleWithValueGeneration(propertyBuilder.Metadata))
-            && (propertyBuilder.Metadata.FindOverrides(storeObject)?.Builder
-                    .CanSetAnnotation(
-                        SqlServerAnnotationNames.ValueGenerationStrategy,
-                        valueGenerationStrategy,
-                        fromDataAnnotation)
-                ?? true);
+        => propertyBuilder.Metadata.FindOverrides(storeObject)?.Builder
+                .CanSetAnnotation(
+                    SqlServerAnnotationNames.ValueGenerationStrategy,
+                    valueGenerationStrategy,
+                    fromDataAnnotation)
+            ?? true;
 
     /// <summary>
     ///     Configures whether the property's column is created as sparse when targeting SQL Server.

src/EFCore.SqlServer/Extensions/SqlServerPropertyExtensions.cs

@@ -1,7 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using Microsoft.EntityFrameworkCore.SqlServer.Internal;
 using Microsoft.EntityFrameworkCore.SqlServer.Metadata.Internal;
 
 // ReSharper disable once CheckNamespace
@@ -852,9 +851,7 @@ private static SqlServerValueGenerationStrategy GetDefaultValueGenerationStrateg
     public static void SetValueGenerationStrategy(
         this IMutableProperty property,
         SqlServerValueGenerationStrategy? value)
-        => property.SetOrRemoveAnnotation(
-            SqlServerAnnotationNames.ValueGenerationStrategy,
-            CheckValueGenerationStrategy(property, value));
+        => property.SetOrRemoveAnnotation(SqlServerAnnotationNames.ValueGenerationStrategy, value);
 
     /// <summary>
     ///     Sets the <see cref="SqlServerValueGenerationStrategy" /> to use for the property.
@@ -868,9 +865,7 @@ public static void SetValueGenerationStrategy(
         SqlServerValueGenerationStrategy? value,
         bool fromDataAnnotation = false)
         => (SqlServerValueGenerationStrategy?)property.SetOrRemoveAnnotation(
-            SqlServerAnnotationNames.ValueGenerationStrategy,
-            CheckValueGenerationStrategy(property, value),
-            fromDataAnnotation)?.Value;
+            SqlServerAnnotationNames.ValueGenerationStrategy, value, fromDataAnnotation)?.Value;
 
     /// <summary>
     ///     Sets the <see cref="SqlServerValueGenerationStrategy" /> to use for the property for a particular table.
@@ -909,9 +904,7 @@ public static void SetValueGenerationStrategy(
     public static void SetValueGenerationStrategy(
         this IMutableRelationalPropertyOverrides overrides,
         SqlServerValueGenerationStrategy? value)
-        => overrides.SetOrRemoveAnnotation(
-            SqlServerAnnotationNames.ValueGenerationStrategy,
-            CheckValueGenerationStrategy(overrides.Property, value));
+        => overrides.SetOrRemoveAnnotation(SqlServerAnnotationNames.ValueGenerationStrategy, value);
 
     /// <summary>
     ///     Sets the <see cref="SqlServerValueGenerationStrategy" /> to use for the property for a particular table.
@@ -925,39 +918,8 @@ public static void SetValueGenerationStrategy(
         SqlServerValueGenerationStrategy? value,
         bool fromDataAnnotation = false)
         => (SqlServerValueGenerationStrategy?)overrides.SetOrRemoveAnnotation(
-            SqlServerAnnotationNames.ValueGenerationStrategy,
-            CheckValueGenerationStrategy(overrides.Property, value),
-            fromDataAnnotation)?.Value;
-
-    private static SqlServerValueGenerationStrategy? CheckValueGenerationStrategy(
-        IReadOnlyProperty property,
-        SqlServerValueGenerationStrategy? value)
-    {
-        if (value == null)
-        {
-            return null;
-        }
+            SqlServerAnnotationNames.ValueGenerationStrategy, value, fromDataAnnotation)?.Value;
 
-        var propertyType = property.ClrType;
-
-        if (value == SqlServerValueGenerationStrategy.IdentityColumn
-            && !IsCompatibleWithValueGeneration(property))
-        {
-            throw new ArgumentException(
-                SqlServerStrings.IdentityBadType(
-                    property.Name, property.DeclaringType.DisplayName(), propertyType.ShortDisplayName()));
-        }
-
-        if (value is SqlServerValueGenerationStrategy.SequenceHiLo or SqlServerValueGenerationStrategy.Sequence
-            && !IsCompatibleWithValueGeneration(property))
-        {
-            throw new ArgumentException(
-                SqlServerStrings.SequenceBadType(
-                    property.Name, property.DeclaringType.DisplayName(), propertyType.ShortDisplayName()));
-        }
-
-        return value;
-    }
 
     /// <summary>
     ///     Returns the <see cref="ConfigurationSource" /> for the <see cref="SqlServerValueGenerationStrategy" />.

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

@@ -135,6 +135,39 @@ protected override void ValidateValueGeneration(
         }
     }
 
+    /// <inheritdoc/>
+    protected override void ValidateTypeMappings(
+        IModel model,
+        IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
+    {
+        base.ValidateTypeMappings(model, logger);
+
+        foreach (var entityType in model.GetEntityTypes())
+        {
+            foreach (var property in entityType.GetFlattenedDeclaredProperties())
+            {
+                var strategy = property.GetValueGenerationStrategy();
+                var propertyType = property.ClrType;
+
+                if (strategy == SqlServerValueGenerationStrategy.IdentityColumn
+                    && !SqlServerPropertyExtensions.IsCompatibleWithValueGeneration(property))
+                {
+                    throw new InvalidOperationException(
+                        SqlServerStrings.IdentityBadType(
+                            property.Name, property.DeclaringType.DisplayName(), propertyType.ShortDisplayName()));
+                }
+
+                if (strategy is SqlServerValueGenerationStrategy.SequenceHiLo or SqlServerValueGenerationStrategy.Sequence
+                    && !SqlServerPropertyExtensions.IsCompatibleWithValueGeneration(property))
+                {
+                    throw new InvalidOperationException(
+                        SqlServerStrings.SequenceBadType(
+                            property.Name, property.DeclaringType.DisplayName(), propertyType.ShortDisplayName()));
+                }
+            }
+        }
+    }
+
     /// <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

test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs

@@ -72,6 +72,55 @@ public override void Detects_store_generated_PK_in_TPC()
         Assert.Equal(SqlServerValueGenerationStrategy.Sequence, keyProperty.GetValueGenerationStrategy());
     }
 
+    [ConditionalFact]
+    public virtual void Throws_for_identity_on_bad_type()
+    {
+        var modelBuilder = CreateConventionModelBuilder();
+
+        modelBuilder.Entity<Animal>(
+            b =>
+            {
+                b.Property(e => e.Name).UseIdentityColumn();
+            });
+
+        VerifyError(
+            SqlServerStrings.IdentityBadType(nameof(LivingBeing.Name), nameof(Animal), "string"),
+            modelBuilder);
+    }
+
+    [ConditionalFact]
+    public virtual void Throws_for_sequence_on_bad_type()
+    {
+        var modelBuilder = CreateConventionModelBuilder();
+
+        modelBuilder.Entity<Animal>(
+            b =>
+            {
+                b.Property(e => e.Name).UseSequence();
+            });
+
+        VerifyError(
+            SqlServerStrings.SequenceBadType(nameof(LivingBeing.Name), nameof(Animal), "string"),
+            modelBuilder);
+    }
+
+
+    [ConditionalFact]
+    public virtual void Throws_for_sequence_HiLo_on_bad_type()
+    {
+        var modelBuilder = CreateConventionModelBuilder();
+
+        modelBuilder.Entity<Animal>(
+            b =>
+            {
+                b.Property(e => e.Name).UseHiLo();
+            });
+
+        VerifyError(
+            SqlServerStrings.SequenceBadType(nameof(LivingBeing.Name), nameof(Animal), "string"),
+            modelBuilder);
+    }
+
     [ConditionalFact]
     public virtual void Passes_for_duplicate_column_names_within_hierarchy_with_identity()
     {

test/EFCore.SqlServer.Tests/Metadata/SqlServerMetadataBuilderExtensionsTest.cs

@@ -305,60 +305,6 @@ public void Can_access_property()
         Assert.Null(propertyBuilder.Metadata.GetHiLoSequenceNameConfigurationSource());
     }
 
-    [ConditionalFact]
-    public void Throws_setting_sequence_generation_for_invalid_type()
-    {
-        var propertyBuilder = CreateBuilder()
-            .Entity(typeof(Splot))
-            .Property(typeof(string), "Name");
-
-        Assert.Equal(
-            SqlServerStrings.SequenceBadType("Name", nameof(Splot), "string"),
-            Assert.Throws<ArgumentException>(
-                () => propertyBuilder.HasValueGenerationStrategy(SqlServerValueGenerationStrategy.SequenceHiLo)).Message);
-
-        Assert.Equal(
-            SqlServerStrings.SequenceBadType("Name", nameof(Splot), "string"),
-            Assert.Throws<ArgumentException>(
-                () => new PropertyBuilder((IMutableProperty)propertyBuilder.Metadata).UseHiLo()).Message);
-    }
-
-    [ConditionalFact]
-    public void Throws_setting_key_sequence_generation_for_invalid_type()
-    {
-        var propertyBuilder = CreateBuilder()
-            .Entity(typeof(Splot))
-            .Property(typeof(string), "Name");
-
-        Assert.Equal(
-            SqlServerStrings.SequenceBadType("Name", nameof(Splot), "string"),
-            Assert.Throws<ArgumentException>(
-                () => propertyBuilder.HasValueGenerationStrategy(SqlServerValueGenerationStrategy.Sequence)).Message);
-
-        Assert.Equal(
-            SqlServerStrings.SequenceBadType("Name", nameof(Splot), "string"),
-            Assert.Throws<ArgumentException>(
-                () => new PropertyBuilder((IMutableProperty)propertyBuilder.Metadata).UseSequence()).Message);
-    }
-
-    [ConditionalFact]
-    public void Throws_setting_identity_generation_for_invalid_type_only_with_explicit()
-    {
-        var propertyBuilder = CreateBuilder()
-            .Entity(typeof(Splot))
-            .Property(typeof(string), "Name");
-
-        Assert.Equal(
-            SqlServerStrings.IdentityBadType("Name", nameof(Splot), "string"),
-            Assert.Throws<ArgumentException>(
-                () => propertyBuilder.HasValueGenerationStrategy(SqlServerValueGenerationStrategy.IdentityColumn)).Message);
-
-        Assert.Equal(
-            SqlServerStrings.IdentityBadType("Name", nameof(Splot), "string"),
-            Assert.Throws<ArgumentException>(
-                () => new PropertyBuilder((IMutableProperty)propertyBuilder.Metadata).UseIdentityColumn()).Message);
-    }
-
     [ConditionalFact]
     public void Can_access_key()
     {

test/EFCore.SqlServer.Tests/Metadata/SqlServerMetadataExtensionsTest.cs

@@ -1,8 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using Microsoft.EntityFrameworkCore.SqlServer.Internal;
-
 // ReSharper disable InconsistentNaming
 namespace Microsoft.EntityFrameworkCore.Metadata;
 
@@ -281,38 +279,6 @@ public void Can_get_and_set_value_generation_on_nullable_property()
         Assert.Equal(SqlServerValueGenerationStrategy.IdentityColumn, property.GetValueGenerationStrategy());
     }
 
-    [ConditionalFact]
-    public void Throws_setting_sequence_generation_for_invalid_type()
-    {
-        var modelBuilder = GetModelBuilder();
-
-        var property = modelBuilder
-            .Entity<Customer>()
-            .Property(e => e.Name)
-            .Metadata;
-
-        Assert.Equal(
-            SqlServerStrings.SequenceBadType("Name", nameof(Customer), "string"),
-            Assert.Throws<ArgumentException>(
-                () => property.SetValueGenerationStrategy(SqlServerValueGenerationStrategy.SequenceHiLo)).Message);
-    }
-
-    [ConditionalFact]
-    public void Throws_setting_identity_generation_for_invalid_type()
-    {
-        var modelBuilder = GetModelBuilder();
-
-        var property = modelBuilder
-            .Entity<Customer>()
-            .Property(e => e.Name)
-            .Metadata;
-
-        Assert.Equal(
-            SqlServerStrings.IdentityBadType("Name", nameof(Customer), "string"),
-            Assert.Throws<ArgumentException>(
-                () => property.SetValueGenerationStrategy(SqlServerValueGenerationStrategy.IdentityColumn)).Message);
-    }
-
     [ConditionalFact]
     public void Can_get_and_set_sequence_name_on_property()
     {