From a96565a2c3b991cf0919775e2c5f423383eeb51a Mon Sep 17 00:00:00 2001 From: AndriySvyryd Date: Fri, 25 Oct 2019 12:54:44 -0700 Subject: [PATCH] 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 --- .../RelationalPropertyExtensions.cs | 3 +- .../Internal/MigrationsModelDiffer.cs | 14 ++- .../Internal/MigrationsModelDifferTest.cs | 90 +++++++++++++++++++ 3 files changed, 104 insertions(+), 3 deletions(-) diff --git a/src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs b/src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs index 18a2192105d..542f7c66759 100644 --- 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 index c30bd156efd..4b89f0eb690 100644 --- 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 index ca81a6601ad..d437e4e4a0c 100644 --- 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("Id")), + source => source.Entity( + "EntityWithOneProperty", + x => + { + x.Property("Value1") + .ValueGeneratedOnAddOrUpdate() + .IsConcurrencyToken(); + }), + target => target.Entity( + "EntityWithOneProperty", + x => + { + x.Property("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("Id")), + source => source.Entity( + "EntityWithOneProperty", + x => + { + x.Property("Value1"); + x.HasData( + new { Id = 42 }); + }), + target => target.Entity( + "EntityWithOneProperty", + x => + { + x.Property("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("Id")), + source => source.Entity( + "EntityWithOneProperty", + x => + { + x.Property("Value1") + .ValueGeneratedOnAddOrUpdate() + .IsConcurrencyToken(); + x.HasData( + new { Id = 42, Value1 = DateTime.Now }); + }), + target => target.Entity( + "EntityWithOneProperty", + x => + { + x.Property("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() {