diff --git a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs index 05c7f42444d..f67de4c9578 100644 --- a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs +++ b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs @@ -563,6 +563,11 @@ private CurrentValueType GetValueType( equals = ValuesEqualFunc(property); } + if (!PropertyHasDefaultValue(property)) + { + return CurrentValueType.Normal; + } + var defaultValue = property.ClrType.GetDefaultValue(); var value = ReadPropertyValue(property); if (!equals(value, defaultValue)) diff --git a/src/EFCore/Extensions/Internal/ExpressionExtensions.cs b/src/EFCore/Extensions/Internal/ExpressionExtensions.cs index 59d88e89fc3..87b87949e67 100644 --- a/src/EFCore/Extensions/Internal/ExpressionExtensions.cs +++ b/src/EFCore/Extensions/Internal/ExpressionExtensions.cs @@ -3,11 +3,12 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using System.Linq.Expressions; using System.Reflection; using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.ChangeTracking; +using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Utilities; // ReSharper disable once CheckNamespace @@ -21,6 +22,42 @@ namespace Microsoft.EntityFrameworkCore.Internal /// public static class ExpressionExtensions { + /// + /// 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. + /// + public static Expression MakeHasDefaultValue( + [NotNull] this Expression currentValueExpression, [NotNull] IPropertyBase propertyBase) + { + if (!currentValueExpression.Type.IsValueType) + { + return Expression.ReferenceEqual( + currentValueExpression, + Expression.Constant(null, currentValueExpression.Type)); + } + + if (currentValueExpression.Type.IsGenericType + && currentValueExpression.Type.GetGenericTypeDefinition() == typeof(Nullable<>)) + { + return Expression.Not( + Expression.Call( + currentValueExpression, + currentValueExpression.Type.GetMethod("get_HasValue"))); + } + + var property = propertyBase as IProperty; + var comparer = property?.GetValueComparer() + ?? ValueComparer.CreateDefault(typeof(TProperty), favorStructuralComparisons: false); + + return comparer.ExtractEqualsBody( + comparer.Type != typeof(TProperty) + ? Expression.Convert(currentValueExpression, comparer.Type) + : currentValueExpression, + Expression.Default(comparer.Type)); + } + /// /// 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 diff --git a/src/EFCore/Metadata/Internal/ClrPropertyGetterFactory.cs b/src/EFCore/Metadata/Internal/ClrPropertyGetterFactory.cs index 15aab4c1854..db32267741c 100644 --- a/src/EFCore/Metadata/Internal/ClrPropertyGetterFactory.cs +++ b/src/EFCore/Metadata/Internal/ClrPropertyGetterFactory.cs @@ -5,7 +5,7 @@ using System.Collections.Generic; using System.Linq.Expressions; using System.Reflection; -using Microsoft.EntityFrameworkCore.ChangeTracking; +using Microsoft.EntityFrameworkCore.Internal; namespace Microsoft.EntityFrameworkCore.Metadata.Internal { @@ -61,40 +61,14 @@ protected override IClrPropertyGetter CreateGeneric(propertyBase); - Expression hasDefaultValueExpression; - - if (!readExpression.Type.IsValueType) - { - hasDefaultValueExpression - = Expression.ReferenceEqual( - readExpression, - Expression.Constant(null, readExpression.Type)); - } - else if (readExpression.Type.IsGenericType - && readExpression.Type.GetGenericTypeDefinition() == typeof(Nullable<>)) - { - hasDefaultValueExpression - = Expression.Not( - Expression.Call( - readExpression, - readExpression.Type.GetMethod("get_HasValue"))); - } - else + if (readExpression.Type != typeof(TValue)) { - var property = propertyBase as IProperty; - var comparer = property?.GetValueComparer() - ?? ValueComparer.CreateDefault(typeof(TValue), favorStructuralComparisons: false); - - hasDefaultValueExpression = comparer.ExtractEqualsBody( - comparer.Type != typeof(TValue) - ? Expression.Convert(readExpression, comparer.Type) - : readExpression, - Expression.Default(comparer.Type)); + readExpression = Expression.Condition( + hasDefaultValueExpression, + Expression.Constant(default(TValue), typeof(TValue)), + Expression.Convert(readExpression, typeof(TValue))); } return new ClrPropertyGetter( diff --git a/src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs b/src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs index f489b517d11..0017030019e 100644 --- a/src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs +++ b/src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs @@ -8,6 +8,7 @@ using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.ChangeTracking.Internal; using Microsoft.EntityFrameworkCore.Diagnostics; +using Microsoft.EntityFrameworkCore.Internal; using Microsoft.EntityFrameworkCore.Storage; using Microsoft.EntityFrameworkCore.Update; @@ -76,7 +77,10 @@ private static Func CreateCurrentValueGetter if (currentValueExpression.Type != typeof(TProperty)) { - currentValueExpression = Expression.Convert(currentValueExpression, typeof(TProperty)); + currentValueExpression = Expression.Condition( + currentValueExpression.MakeHasDefaultValue(propertyBase), + Expression.Constant(default(TProperty), typeof(TProperty)), + Expression.Convert(currentValueExpression, typeof(TProperty))); } } diff --git a/test/EFCore.Specification.Tests/FieldMappingTestBase.cs b/test/EFCore.Specification.Tests/FieldMappingTestBase.cs index e6550307a43..eb32271461d 100644 --- a/test/EFCore.Specification.Tests/FieldMappingTestBase.cs +++ b/test/EFCore.Specification.Tests/FieldMappingTestBase.cs @@ -39,7 +39,7 @@ protected class User2 : IUser2 protected class LoginSession { - private object _id = 0; + private object _id; private IUser2 _user; private object _users; diff --git a/test/EFCore.Specification.Tests/StoreGeneratedTestBase.cs b/test/EFCore.Specification.Tests/StoreGeneratedTestBase.cs index 43407dd4d8a..df3fc52893e 100644 --- a/test/EFCore.Specification.Tests/StoreGeneratedTestBase.cs +++ b/test/EFCore.Specification.Tests/StoreGeneratedTestBase.cs @@ -1201,7 +1201,7 @@ public virtual void Fields_used_correctly_for_store_generated_values() }); } - [ConditionalFact(Skip = "Issue #15182")] + [ConditionalFact] public virtual void Nullable_fields_get_defaults_when_not_set() { ExecuteWithStrategyInTransaction( @@ -1212,64 +1212,189 @@ public virtual void Nullable_fields_get_defaults_when_not_set() context.SaveChanges(); Assert.NotEqual(0, entity.Id); - Assert.True(entity.NullableBackedBool); - Assert.Equal(-1, entity.NullableBackedInt); + Assert.True(entity.NullableBackedBoolTrueDefault); + Assert.Equal(-1, entity.NullableBackedIntNonZeroDefault); + Assert.False(entity.NullableBackedBoolFalseDefault); + Assert.Equal(0, entity.NullableBackedIntZeroDefault); }, context => { var entity = context.Set().Single(); - Assert.True(entity.NullableBackedBool); - Assert.Equal(-1, entity.NullableBackedInt); + Assert.True(entity.NullableBackedBoolTrueDefault); + Assert.Equal(-1, entity.NullableBackedIntNonZeroDefault); + Assert.False(entity.NullableBackedBoolFalseDefault); + Assert.Equal(0, entity.NullableBackedIntZeroDefault); }); } - [ConditionalFact(Skip = "Issue #15182")] + [ConditionalFact] public virtual void Nullable_fields_store_non_defaults_when_set() { ExecuteWithStrategyInTransaction( context => { - var entity = context.Add(new WithNullableBackingFields { NullableBackedBool = false, NullableBackedInt = 0 }).Entity; + var entity = context.Add( + new WithNullableBackingFields + { + NullableBackedBoolTrueDefault = false, + NullableBackedIntNonZeroDefault = 0, + NullableBackedBoolFalseDefault = true, + NullableBackedIntZeroDefault = -1 + }).Entity; context.SaveChanges(); Assert.NotEqual(0, entity.Id); - Assert.False(entity.NullableBackedBool); - Assert.Equal(0, entity.NullableBackedInt); + Assert.False(entity.NullableBackedBoolTrueDefault); + Assert.Equal(0, entity.NullableBackedIntNonZeroDefault); + Assert.True(entity.NullableBackedBoolFalseDefault); + Assert.Equal(-1, entity.NullableBackedIntZeroDefault); }, context => { var entity = context.Set().Single(); - Assert.False(entity.NullableBackedBool); - Assert.Equal(0, entity.NullableBackedInt); + Assert.False(entity.NullableBackedBoolTrueDefault); + Assert.Equal(0, entity.NullableBackedIntNonZeroDefault); + Assert.True(entity.NullableBackedBoolFalseDefault); + Assert.Equal(-1, entity.NullableBackedIntZeroDefault); }); } - [ConditionalFact(Skip = "Issue #15182")] + [ConditionalFact] public virtual void Nullable_fields_store_any_value_when_set() { ExecuteWithStrategyInTransaction( context => { - var entity = context.Add(new WithNullableBackingFields { NullableBackedBool = true, NullableBackedInt = 3 }).Entity; + var entity = context.Add( + new WithNullableBackingFields + { + NullableBackedBoolTrueDefault = true, + NullableBackedIntNonZeroDefault = 3, + NullableBackedBoolFalseDefault = true, + NullableBackedIntZeroDefault = 5 + }).Entity; context.SaveChanges(); Assert.NotEqual(0, entity.Id); - Assert.True(entity.NullableBackedBool); - Assert.Equal(3, entity.NullableBackedInt); + Assert.True(entity.NullableBackedBoolTrueDefault); + Assert.Equal(3, entity.NullableBackedIntNonZeroDefault); + Assert.True(entity.NullableBackedBoolFalseDefault); + Assert.Equal(5, entity.NullableBackedIntZeroDefault); }, context => { var entity = context.Set().Single(); - Assert.True(entity.NullableBackedBool); - Assert.Equal(3, entity.NullableBackedInt); + Assert.True(entity.NullableBackedBoolTrueDefault); + Assert.Equal(3, entity.NullableBackedIntNonZeroDefault); + Assert.True(entity.NullableBackedBoolFalseDefault); + Assert.Equal(5, entity.NullableBackedIntZeroDefault); + }); + } + + [ConditionalFact] + public virtual void Object_fields_get_defaults_when_not_set() + { + ExecuteWithStrategyInTransaction( + context => + { + var entity = context.Add(new WithObjectBackingFields()).Entity; + + context.SaveChanges(); + + Assert.NotEqual(0, entity.Id); + Assert.True(entity.NullableBackedBoolTrueDefault); + Assert.Equal(-1, entity.NullableBackedIntNonZeroDefault); + Assert.False(entity.NullableBackedBoolFalseDefault); + Assert.Equal(0, entity.NullableBackedIntZeroDefault); + }, + context => + { + var entity = context.Set().Single(); + Assert.True(entity.NullableBackedBoolTrueDefault); + Assert.Equal(-1, entity.NullableBackedIntNonZeroDefault); + Assert.False(entity.NullableBackedBoolFalseDefault); + Assert.Equal(0, entity.NullableBackedIntZeroDefault); + }); + } + + [ConditionalFact] + public virtual void Object_fields_store_non_defaults_when_set() + { + ExecuteWithStrategyInTransaction( + context => + { + var entity = context.Add( + new WithObjectBackingFields + { + NullableBackedBoolTrueDefault = false, + NullableBackedIntNonZeroDefault = 0, + NullableBackedBoolFalseDefault = true, + NullableBackedIntZeroDefault = -1 + }).Entity; + + context.SaveChanges(); + + Assert.NotEqual(0, entity.Id); + Assert.False(entity.NullableBackedBoolTrueDefault); + Assert.Equal(0, entity.NullableBackedIntNonZeroDefault); + Assert.True(entity.NullableBackedBoolFalseDefault); + Assert.Equal(-1, entity.NullableBackedIntZeroDefault); + }, + context => + { + var entity = context.Set().Single(); + Assert.False(entity.NullableBackedBoolTrueDefault); + Assert.Equal(0, entity.NullableBackedIntNonZeroDefault); + Assert.True(entity.NullableBackedBoolFalseDefault); + Assert.Equal(-1, entity.NullableBackedIntZeroDefault); + }); + } + + [ConditionalFact] + public virtual void Object_fields_store_any_value_when_set() + { + ExecuteWithStrategyInTransaction( + context => + { + var entity = context.Add( + new WithObjectBackingFields + { + NullableBackedBoolTrueDefault = true, + NullableBackedIntNonZeroDefault = 3, + NullableBackedBoolFalseDefault = true, + NullableBackedIntZeroDefault = 5 + }).Entity; + + context.SaveChanges(); + + Assert.NotEqual(0, entity.Id); + Assert.True(entity.NullableBackedBoolTrueDefault); + Assert.Equal(3, entity.NullableBackedIntNonZeroDefault); + Assert.True(entity.NullableBackedBoolFalseDefault); + Assert.Equal(5, entity.NullableBackedIntZeroDefault); + }, + context => + { + var entity = context.Set().Single(); + Assert.True(entity.NullableBackedBoolTrueDefault); + Assert.Equal(3, entity.NullableBackedIntNonZeroDefault); + Assert.True(entity.NullableBackedBoolFalseDefault); + Assert.Equal(5, entity.NullableBackedIntZeroDefault); }); } protected class Darwin { - public int Id { get; set; } + public int? _id; + + public int Id + { + get => _id ?? 0; + set => _id = value; + } + public string Name { get; set; } } @@ -1384,20 +1509,79 @@ public int Id set => _id = value; } - private bool? _nullableBackedBool; + private bool? _nullableBackedBoolTrueDefault; + + public bool NullableBackedBoolTrueDefault + { + get => _nullableBackedBoolTrueDefault ?? true; + set => _nullableBackedBoolTrueDefault = value; + } + + private int? _nullableBackedIntNonZeroDefault; + + public int NullableBackedIntNonZeroDefault + { + get => _nullableBackedIntNonZeroDefault ?? -1; + set => _nullableBackedIntNonZeroDefault = value; + } + + private bool? _nullableBackedBoolFalseDefault; + + public bool NullableBackedBoolFalseDefault + { + get => _nullableBackedBoolFalseDefault ?? false; + set => _nullableBackedBoolFalseDefault = value; + } + + private int? _nullableBackedIntZeroDefault; + + public int NullableBackedIntZeroDefault + { + get => _nullableBackedIntZeroDefault ?? 0; + set => _nullableBackedIntZeroDefault = value; + } + } + + protected class WithObjectBackingFields + { + private object _id; + + public int Id + { + get => (int)(_id ?? 0); + set => _id = value; + } + + private object _nullableBackedBoolTrueDefault; + + public bool NullableBackedBoolTrueDefault + { + get => (bool)(_nullableBackedBoolTrueDefault ?? false); + set => _nullableBackedBoolTrueDefault = value; + } + + private object _nullableBackedIntNonZeroDefault; + + public int NullableBackedIntNonZeroDefault + { + get => (int)(_nullableBackedIntNonZeroDefault ?? -1); + set => _nullableBackedIntNonZeroDefault = value; + } + + private object _nullableBackedBoolFalseDefault; - public bool NullableBackedBool + public bool NullableBackedBoolFalseDefault { - get => _nullableBackedBool ?? false; - set => _nullableBackedBool = value; + get => (bool)(_nullableBackedBoolFalseDefault ?? false); + set => _nullableBackedBoolFalseDefault = value; } - private int? _nullableBackedInt; + private object _nullableBackedIntZeroDefault; - public int NullableBackedInt + public int NullableBackedIntZeroDefault { - get => _nullableBackedInt ?? 0; - set => _nullableBackedInt = value; + get => (int)(_nullableBackedIntZeroDefault ?? 0); + set => _nullableBackedIntZeroDefault = value; } } diff --git a/test/EFCore.Specification.Tests/TestModels/Northwind/Employee.cs b/test/EFCore.Specification.Tests/TestModels/Northwind/Employee.cs index 3b987b16a55..09104f26aa9 100644 --- a/test/EFCore.Specification.Tests/TestModels/Northwind/Employee.cs +++ b/test/EFCore.Specification.Tests/TestModels/Northwind/Employee.cs @@ -7,7 +7,14 @@ namespace Microsoft.EntityFrameworkCore.TestModels.Northwind { public class Employee { - public uint EmployeeID { get; set; } + private uint? _employeeId; + + public uint EmployeeID + { + get => _employeeId ?? (uint)0; + set => _employeeId = value; + } + public string LastName { get; set; } public string FirstName { get; set; } public string Title { get; set; } diff --git a/test/EFCore.Specification.Tests/TestModels/Northwind/Order.cs b/test/EFCore.Specification.Tests/TestModels/Northwind/Order.cs index 6cf66c74cff..28510a41ad4 100644 --- a/test/EFCore.Specification.Tests/TestModels/Northwind/Order.cs +++ b/test/EFCore.Specification.Tests/TestModels/Northwind/Order.cs @@ -8,7 +8,14 @@ namespace Microsoft.EntityFrameworkCore.TestModels.Northwind { public class Order { - public int OrderID { get; set; } + private int? _orderId; + + public int OrderID + { + get => _orderId ?? 0; + set => _orderId = value; + } + public string CustomerID { get; set; } public uint? EmployeeID { get; set; } public DateTime? OrderDate { get; set; } diff --git a/test/EFCore.Specification.Tests/TestModels/Northwind/OrderDetail.cs b/test/EFCore.Specification.Tests/TestModels/Northwind/OrderDetail.cs index e81837cbe9f..a3242e2252b 100644 --- a/test/EFCore.Specification.Tests/TestModels/Northwind/OrderDetail.cs +++ b/test/EFCore.Specification.Tests/TestModels/Northwind/OrderDetail.cs @@ -7,8 +7,21 @@ namespace Microsoft.EntityFrameworkCore.TestModels.Northwind { public class OrderDetail : IComparable { - public int OrderID { get; set; } - public int ProductID { get; set; } + private int? _orderId; + private int? _productId; + + public int OrderID + { + get => _orderId ?? 0; + set => _orderId = value; + } + + public int ProductID + { + get => _productId ?? 0; + set => _productId = value; + } + public decimal UnitPrice { get; set; } public short Quantity { get; set; } public float Discount { get; set; } diff --git a/test/EFCore.Specification.Tests/TestModels/Northwind/Product.cs b/test/EFCore.Specification.Tests/TestModels/Northwind/Product.cs index c7d664a4d13..dabe2111afa 100644 --- a/test/EFCore.Specification.Tests/TestModels/Northwind/Product.cs +++ b/test/EFCore.Specification.Tests/TestModels/Northwind/Product.cs @@ -7,12 +7,19 @@ namespace Microsoft.EntityFrameworkCore.TestModels.Northwind { public class Product { + private int? _productId; + public Product() { OrderDetails = new List(); } - public int ProductID { get; set; } + public int ProductID + { + get => _productId ?? 0; + set => _productId = value; + } + public string ProductName { get; set; } public int? SupplierID { get; set; } public int? CategoryID { get; set; } diff --git a/test/EFCore.SqlServer.FunctionalTests/StoreGeneratedSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/StoreGeneratedSqlServerTest.cs index 6436dc4027c..0e90b6ede6d 100644 --- a/test/EFCore.SqlServer.FunctionalTests/StoreGeneratedSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/StoreGeneratedSqlServerTest.cs @@ -45,6 +45,7 @@ public virtual void Exception_in_SaveChanges_causes_store_values_to_be_reverted( foreach (var entity in entities.Take(100)) { Assert.Equal(0, entity.Id); + Assert.Null(entity._id); } Assert.Equal(1777, entities[100].Id); @@ -70,6 +71,7 @@ public virtual void Exception_in_SaveChanges_causes_store_values_to_be_reverted( foreach (var entity in entities.Take(100)) { Assert.Equal(0, entity.Id); + Assert.Null(entity._id); } Assert.Equal(1777, entities[100].Id); @@ -172,8 +174,19 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con modelBuilder.Entity( b => { - b.Property(e => e.NullableBackedBool).HasDefaultValue(true); - b.Property(e => e.NullableBackedInt).HasDefaultValue(-1); + b.Property(e => e.NullableBackedBoolTrueDefault).HasDefaultValue(true); + b.Property(e => e.NullableBackedIntNonZeroDefault).HasDefaultValue(-1); + b.Property(e => e.NullableBackedBoolFalseDefault).HasDefaultValue(false); + b.Property(e => e.NullableBackedIntZeroDefault).HasDefaultValue(0); + }); + + modelBuilder.Entity( + b => + { + b.Property(e => e.NullableBackedBoolTrueDefault).HasDefaultValue(true); + b.Property(e => e.NullableBackedIntNonZeroDefault).HasDefaultValue(-1); + b.Property(e => e.NullableBackedBoolFalseDefault).HasDefaultValue(false); + b.Property(e => e.NullableBackedIntZeroDefault).HasDefaultValue(0); }); base.OnModelCreating(modelBuilder, context); diff --git a/test/EFCore.Sqlite.FunctionalTests/StoreGeneratedSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/StoreGeneratedSqliteTest.cs index d310646000f..12dc3ddb954 100644 --- a/test/EFCore.Sqlite.FunctionalTests/StoreGeneratedSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/StoreGeneratedSqliteTest.cs @@ -110,8 +110,19 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con modelBuilder.Entity( b => { - b.Property(e => e.NullableBackedBool).HasDefaultValue(true); - b.Property(e => e.NullableBackedInt).HasDefaultValue(-1); + b.Property(e => e.NullableBackedBoolTrueDefault).HasDefaultValue(true); + b.Property(e => e.NullableBackedIntNonZeroDefault).HasDefaultValue(-1); + b.Property(e => e.NullableBackedBoolFalseDefault).HasDefaultValue(false); + b.Property(e => e.NullableBackedIntZeroDefault).HasDefaultValue(0); + }); + + modelBuilder.Entity( + b => + { + b.Property(e => e.NullableBackedBoolTrueDefault).HasDefaultValue(true); + b.Property(e => e.NullableBackedIntNonZeroDefault).HasDefaultValue(-1); + b.Property(e => e.NullableBackedBoolFalseDefault).HasDefaultValue(false); + b.Property(e => e.NullableBackedIntZeroDefault).HasDefaultValue(0); }); modelBuilder.Entity().Property(e => e.Id)