Don't compare seed values for store generated properties.

Use the default value for non-nullable properties. Mark the column as non-nullable if the converted provider type is non-nullable. Fixes #18592
dotnet · Oct 25, 2019 · a96565a · a96565a
1 parent 6148e6b
commit a96565a
Show file tree

Hide file tree

Showing 3 changed files with 104 additions and 3 deletions.
diff --git a/src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs b/src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs
@@ -429,7 +429,8 @@ public static RelationalTypeMapping FindRelationalMapping([NotNull] this IProper
  public static bool IsColumnNullable([NotNull] this IProperty property)
  => !property.IsPrimaryKey()
  && (property.DeclaringEntityType.BaseType != null
- || property.IsNullable
+ || (property.IsNullable
+ && property.GetValueConverter()?.ProviderClrType.IsNullableType() != false)
  || IsTableSplitting(property.DeclaringEntityType));
 
  private static bool IsTableSplitting(IEntityType entityType)

diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs b/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs
@@ -1798,7 +1798,8 @@ protected virtual void DiffData(
  {
  var sourceProperty = diffContext.FindSource(targetProperty);
  if (sourceProperty == null
- || !sourceEntityType.GetProperties().Contains(sourceProperty))
+ || !sourceEntityType.GetProperties().Contains(sourceProperty)
+ || targetProperty.ValueGenerated != ValueGenerated.Never)
  {
  continue;
  }
@@ -1830,7 +1831,16 @@ var modelValuesChanged
  var convertedType = sourceConverter?.ProviderClrType
  ?? targetConverter?.ProviderClrType;
 
- var storeValuesChanged = convertedSourceValue?.GetType().UnwrapNullableType() != convertedTargetValue?.GetType().UnwrapNullableType();
+ if (convertedType != null
+ && !convertedType.IsNullableType())
+ {
+ var defaultValue = convertedType.GetDefaultValue();
+ convertedSourceValue ??= defaultValue;
+ convertedTargetValue ??= defaultValue;
+ }
+
+ var storeValuesChanged = convertedSourceValue?.GetType().UnwrapNullableType()
+ != convertedTargetValue?.GetType().UnwrapNullableType();
 
  if (!storeValuesChanged
  && convertedType != null)

diff --git a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs
@@ -187,6 +187,34 @@ public void Model_differ_detects_changing_store_type_to_conversions()
  }));
  }
 
+ [ConditionalFact]
+ public void Value_conversion_to_non_nullable_type_is_not_detected()
+ {
+ Execute(
+ common => common.Entity(
+ "EntityWithOneProperty",
+ x => x.Property<int>("Id")),
+ source => source.Entity(
+ "EntityWithOneProperty",
+ x =>
+ {
+ x.Property<DateTime>("Value1")
+ .ValueGeneratedOnAddOrUpdate()
+ .IsConcurrencyToken();
+ }),
+ target => target.Entity(
+ "EntityWithOneProperty",
+ x =>
+ {
+ x.Property<byte[]>("Value1")
+ .ValueGeneratedOnAddOrUpdate()
+ .IsConcurrencyToken()
+ .HasConversion(e => new DateTime(), e => new byte[0]);
+ }),
+ Assert.Empty,
+ Assert.Empty);
+ }
+
  [ConditionalFact]
  public void Model_differ_breaks_foreign_key_cycles_in_create_table_operations()
  {
@@ -6719,6 +6747,68 @@ public void SeedData_nonkey_refactoring_value_conversion()
  Assert.Empty);
  }
 
+ [ConditionalFact]
+ public void SeedData_nonkey_refactoring_value_conversion_to_value_type()
+ {
+ Execute(
+ common => common.Entity(
+ "EntityWithOneProperty",
+ x => x.Property<int>("Id")),
+ source => source.Entity(
+ "EntityWithOneProperty",
+ x =>
+ {
+ x.Property<DateTime>("Value1");
+ x.HasData(
+ new { Id = 42 });
+ }),
+ target => target.Entity(
+ "EntityWithOneProperty",
+ x =>
+ {
+ x.Property<byte[]>("Value1")
+ .IsRequired()
+ .HasConversion(e => new DateTime(), e => new byte[0]);
+ x.HasData(
+ new { Id = 42 });
+ }),
+ Assert.Empty,
+ Assert.Empty);
+ }
+
+ [ConditionalFact]
+ public void SeedData_nonkey_refactoring_value_conversion_to_value_type_store_generated()
+ {
+ Execute(
+ common => common.Entity(
+ "EntityWithOneProperty",
+ x => x.Property<int>("Id")),
+ source => source.Entity(
+ "EntityWithOneProperty",
+ x =>
+ {
+ x.Property<DateTime>("Value1")
+ .ValueGeneratedOnAddOrUpdate()
+ .IsConcurrencyToken();
+ x.HasData(
+ new { Id = 42, Value1 = DateTime.Now });
+ }),
+ target => target.Entity(
+ "EntityWithOneProperty",
+ x =>
+ {
+ x.Property<byte[]>("Value1")
+ .IsRequired()
+ .ValueGeneratedOnAddOrUpdate()
+ .IsConcurrencyToken()
+ .HasConversion(e => new DateTime(), e => new byte[0]);
+ x.HasData(
+ new { Id = 42, Value1 = new byte[0] });
+ }),
+ Assert.Empty,
+ Assert.Empty);
+ }
+
  [ConditionalFact]
  public void SeedData_nonkey_refactoring_value_conversion_with_structural_provider_type()
  {