Skip to content

Commit

Permalink
[2.1.3] Compare structs correctly in ClrPropertyGetter delegate
Browse files Browse the repository at this point in the history
Fixes #12290

The issue here is that a ValueComparer was not being used when it should have been.
  • Loading branch information
ajcvickers committed Jun 15, 2018
1 parent 66d135f commit 081c591
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 2 deletions.
43 changes: 43 additions & 0 deletions src/EFCore.Specification.Tests/CustomConvertersTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,42 @@ protected class Email
public static implicit operator string(Email email) => email._value;
}

[Fact]
public virtual void Can_query_and_update_with_conversion_for_custom_struct()
{
int id;
using (var context = CreateContext())
{
var load = context.Set<Load>().Add(
new Load { Fuel = new Fuel(1.1) }).Entity;

Assert.Equal(1, context.SaveChanges());

id = load.LoadId;
}

using (var context = CreateContext())
{
var load = context.Set<Load>().Single(e => e.LoadId == id && e.Fuel.Equals(new Fuel(1.1)));

Assert.Equal(id, load.LoadId);
Assert.Equal(1.1, load.Fuel.Volume);
}
}

protected class Load
{
public int LoadId { get; private set; }

public Fuel Fuel { get; set; }
}

protected struct Fuel
{
public Fuel(double volume) => Volume = volume;
public double Volume { get; }
}

[Fact]
public virtual void Can_insert_and_read_back_with_case_insensitive_string_key()
{
Expand Down Expand Up @@ -164,6 +200,13 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
b.Property(e => e.Id).ValueGeneratedNever();
});

modelBuilder
.Entity<Load>(
b =>
{
b.Property(x => x.Fuel).HasConversion(f => f.Volume, v => new Fuel(v));
});

modelBuilder.Entity<BuiltInDataTypes>(
b =>
{
Expand Down
23 changes: 21 additions & 2 deletions src/EFCore/Metadata/Internal/ClrPropertyGetterFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +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
Expand Down Expand Up @@ -32,6 +33,8 @@ protected override IClrPropertyGetter CreateGeneric<TEntity, TValue, TNonNullabl
}

var entityParameter = Expression.Parameter(typeof(TEntity), "entity");
var memberType = memberInfo.GetMemberType();
var defaultExpression = Expression.Default(memberType);

Expression readExpression;
if (memberInfo.DeclaringType.GetTypeInfo().IsAssignableFrom(typeof(TEntity).GetTypeInfo()))
Expand All @@ -52,12 +55,28 @@ protected override IClrPropertyGetter CreateGeneric<TEntity, TValue, TNonNullabl
Expression.TypeAs(entityParameter, memberInfo.DeclaringType)),
Expression.Condition(
Expression.ReferenceEqual(converted, Expression.Constant(null)),
Expression.Default(memberInfo.GetMemberType()),
defaultExpression,
Expression.MakeMemberAccess(converted, memberInfo))
});
}

var hasDefaultValueExpression = Expression.Equal(readExpression, Expression.Default(memberInfo.GetMemberType()));
var useRtmBehaviour = AppContext.TryGetSwitch(
"Microsoft.EntityFrameworkCore.Issue112290",
out var isEnabled)
&& isEnabled;

var property = propertyBase as IProperty;
var comparer = memberType.IsNullableType() || useRtmBehaviour
? null
: property?.GetValueComparer()
?? property?.FindMapping()?.Comparer
?? (ValueComparer)Activator.CreateInstance(
typeof(ValueComparer<>).MakeGenericType(memberType),
new object[] { false });

var hasDefaultValueExpression = comparer == null
? Expression.Equal(readExpression, defaultExpression)
: comparer.ExtractEqualsBody(readExpression, defaultExpression);

return new ClrPropertyGetter<TEntity, TValue>(
Expression.Lambda<Func<TEntity, TValue>>(readExpression, entityParameter).Compile(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ public virtual void Columns_have_expected_data_types()
BuiltInNullableDataTypesShadow.TestNullableUnsignedInt32 ---> [nullable bigint] [Precision = 19 Scale = 0]
BuiltInNullableDataTypesShadow.TestNullableUnsignedInt64 ---> [nullable decimal] [Precision = 20 Scale = 0]
BuiltInNullableDataTypesShadow.TestString ---> [nullable nvarchar] [MaxLength = -1]
Load.Fuel ---> [float] [Precision = 53]
Load.LoadId ---> [int] [Precision = 10 Scale = 0]
MaxLengthDataTypes.ByteArray5 ---> [nullable varbinary] [MaxLength = 7]
MaxLengthDataTypes.ByteArray9000 ---> [nullable nvarchar] [MaxLength = -1]
MaxLengthDataTypes.Id ---> [int] [Precision = 10 Scale = 0]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,45 @@ public void Delegate_getter_is_returned_for_property_info()
}));
}

[Fact]
public void Delegate_getter_is_returned_for_IProperty_struct_property()
{
var entityType = new Model().AddEntityType(typeof(Customer));
var fuelProperty = entityType.AddProperty("Fuel", typeof(Fuel));

Assert.Equal(
new Fuel(1.0),
new ClrPropertyGetterFactory().Create(fuelProperty).GetClrValue(
new Customer
{
Id = 7,
Fuel = new Fuel(1.0)
}));
}

[Fact]
public void Delegate_getter_is_returned_for_struct_property_info()
{
Assert.Equal(
new Fuel(1.0),
new ClrPropertyGetterFactory().Create(typeof(Customer).GetAnyProperty("Fuel")).GetClrValue(
new Customer
{
Id = 7,
Fuel = new Fuel(1.0)
}));
}

private class Customer
{
internal int Id { get; set; }
internal Fuel Fuel { get; set; }
}

private struct Fuel
{
public Fuel(double volume) => Volume = volume;
public double Volume { get; }
}
}
}

0 comments on commit 081c591

Please sign in to comment.