5

Edit This seems to occur for any Entity property that references another entity in one direction. In other words, for the below example, the fact that Bar overrides Equality appears to be irrelevant.

Suppose I have the following classes:

public class Foo
{
    public int? Id { get; set; }

    public virtual Bar { get; set; }

}

public class Bar : IEquatable<Bar>
{
    public int Id { get; set; }

    public override bool Equals(object obj)
    {
        var other = obj as Bar;

        return Equals(other);
    }

    public bool Equals(Bar other)
    {
        if (object.Equals(other, null))
            return false;

        return this.Id == other.Id;
    }

    public static bool operator ==(Bar left, Bar right)
    {
        return object.Equals(left, right);
    }

    public static bool operator !=(Bar left, Bar right)
    {
        return !object.Equals(left, right);
    }

    public override int GetHashCode()
    {
        return Id.GetHashCode();
    }
}

Note that here, "Bar" intentionally has "Id" equality, because it more or less represents a lookup table - so any two object references with the same Id should always be considered the same.

Here's the weird part, this all works fine when I set Foo.Bar to another Bar instance - everything updates as expected.

However, if foo has an existing Bar when it is retrieved from the DbContext and I do:

foo.Bar = null

then the property doesn't actually change!

If I do:

var throwAway = foo.Bar;
foo.Bar = null;

Then the property will actually set and save as null.

Since the Foo.Bar property is simply a virtual, auto-implemented property, I can only conclude that this has something to do with lazy-loading and Entity Framework proxies - but why this particular scenario causes a problem, I have no idea.

Why does Entity Framework behave this way, and how can I get it to actually set null reliably?

Nathan
  • 10,593
  • 10
  • 63
  • 87
  • My guess would be that lazy loading is implemented as `BarType Bar { get { if (base.Bar == null) { base.Bar = (lazily load); } return base.Bar; } set { base.Bar = value; } }`, and where `base.Bar` doesn't see the change from `null` (not yet loaded) to `null` (none). But if that is correct, it looks like a bug to me that would be worth reporting to the EF folks on their CodePlex pages. –  Feb 11 '14 at 09:00
  • @hvd I opened https://entityframework.codeplex.com/workitem/2074, so we'll see what they say. – Nathan Feb 11 '14 at 09:14

6 Answers6

2

As a workaround, the easiest way I've found to mitigate this issue is to have the setter call the getter before setting the backing field to null, e.g.

public class Foo
{
    public int? Id { get; set; }

    private Bar _bar;
    public virtual Bar 
    { 
        get { return _bar; }

        set
        {
            var entityFrameworkHack = this.Bar; //ensure the proxy has loaded
            _bar = value;
        }
    }
}

This way, the property works regardless of whether other code has actually loaded the property yet, at the cost of a potentially unneeded entity load.

Nathan
  • 10,593
  • 10
  • 63
  • 87
  • This could be potentially expensive depending on the model. It would be more efficient to only do the lazy load when the new value is null because it is only in those circumstances that the new value is ignored. I have given an example in my answer below. – Rob Kent Sep 25 '14 at 10:02
1

You are right - this happens beacause you used lazy loading in EF (virtual property). You may remove virtual (but this may be impossible for you). Other way you described in your question - call property, and set this to null.

Also you could read another topic about this problem on SO.

Community
  • 1
  • 1
Boris Parfenenkov
  • 3,219
  • 1
  • 25
  • 30
1

A way to make it work is using the property API:

var foo = context.Foos.Find(1);

context.Entry(foo).Reference(f => f.Bar).CurrentValue = null;

context.SaveChanges();

The benefit is that this works without loading the foo.Bar by lazy loading and it also works for pure POCOs that don't support lazy loading or change tracking proxies (no virtual properties). The downside is that you need a context instance available at the place where you want to set the related Bar to null.

Slauma
  • 175,098
  • 59
  • 401
  • 420
0

I am not happy with the official workaround:

    context.Entry(foo).Reference(f => f.Bar).CurrentValue = null;

because it involves too much contextual knowledge by the user of the POCO object. My fix is to trigger a load of the lazy property when setting the value to null so that we do not get a false positive comparison from EF:

    public virtual User CheckoutUser
    {
        get { return checkoutUser; }
        set
        {
            if (value != null || !LazyPropertyIsNull(CheckoutUser))
            {
                checkoutUser = value;
            }
        }
    }

and in my base DbEntity class:

    protected bool LazyPropertyIsNull<T>(T currentValue) where T : DbEntity
    {
        return (currentValue == null);
    }

Passing the property to the LazyPropertyIsNull function triggers the lazy load and the correct comparison occurs.

Please vote for this issue on the EF issues log:

Rob Kent
  • 5,183
  • 4
  • 33
  • 54
0

Personally, I think Nathan's answer (lazy-loading inside the property setter) is the most robust. However, it mushrooms your domain classes (10 lines per property) and makes it less readable.

As another workaround, I compiled two methods into a extension method:

public static void SetToNull<TEntity, TProperty>(this TEntity entity, Expression<Func<TEntity, TProperty>> navigationProperty, DbContext context = null)
    where TEntity : class
    where TProperty : class
{
    var pi = GetPropertyInfo(entity, navigationProperty);

    if (context != null)
    {
        //If DB Context is supplied, use Entry/Reference method to null out current value
        context.Entry(entity).Reference(navigationProperty).CurrentValue = null;
    }
    else
    {
        //If no DB Context, then lazy load first
        var prevValue = (TProperty)pi.GetValue(entity);
    }

    pi.SetValue(entity, null);
}

static PropertyInfo GetPropertyInfo<TSource, TProperty>(    TSource source,    Expression<Func<TSource, TProperty>> propertyLambda)
{
    Type type = typeof(TSource);

    MemberExpression member = propertyLambda.Body as MemberExpression;
    if (member == null)
        throw new ArgumentException(string.Format(
            "Expression '{0}' refers to a method, not a property.",
            propertyLambda.ToString()));

    PropertyInfo propInfo = member.Member as PropertyInfo;
    if (propInfo == null)
        throw new ArgumentException(string.Format(
            "Expression '{0}' refers to a field, not a property.",
            propertyLambda.ToString()));

    if (type != propInfo.ReflectedType &&
        !type.IsSubclassOf(propInfo.ReflectedType))
        throw new ArgumentException(string.Format(
            "Expression '{0}' refers to a property that is not from type {1}.",
            propertyLambda.ToString(),
            type));

    return propInfo;
}

This allows you to supply a DbContext if you have one, in which case it will use the most efficient method and set the CurrentValue of the Entry Reference to null.

entity.SetToNull(e => e.ReferenceProperty, dbContext);

If no DBContext is supplied, it will lazy load first.

entity.SetToNull(e => e.ReferenceProperty);
jonh
  • 233
  • 1
  • 10
0

If you want to avoid manipulating the EntityEntry, you can avoid the lazy load call to the database by including the FK property in your POCO (which you can make private if you don't want users to have access) and have the Nav property setter set that FK value to null if the setter value is null. Example:

public class InaccessibleFKDependent
{
    [Key]
    public int Id { get; set; }
    private int? PrincipalId { get; set; }
    private InaccessibleFKPrincipal _principal;
    public virtual InaccessibleFKPrincipal Principal
    {
        get => _principal;
        set
        {
            if( null == value )
            {
                PrincipalId = null;
            }

            _principal = value;
        }
    }
}

public class InaccessibleFKDependentConfiguration : IEntityTypeConfiguration<InaccessibleFKDependent>
{
    public void Configure( EntityTypeBuilder<InaccessibleFKDependent> builder )
    {
        builder.HasOne( d => d.Principal )
            .WithMany()
            .HasForeignKey( "PrincipalId" );
    }
}

Test:

    public static void TestInaccessibleFKSetToNull( DbContextOptions options )
    {
        using( var dbContext = DeleteAndRecreateDatabase( options ) )
        {
            var p = new InaccessibleFKPrincipal();

            dbContext.Add( p );
            dbContext.SaveChanges();

            var d = new InaccessibleFKDependent()
            {
                Principal = p,
            };

            dbContext.Add( d );
            dbContext.SaveChanges();
        }

        using( var dbContext = new TestContext( options ) )
        {
            var d = dbContext.InaccessibleFKDependentEntities.Single();
            d.Principal = null;
            dbContext.SaveChanges();
        }

        using( var dbContext = new TestContext( options ) )
        {
            var d = dbContext.InaccessibleFKDependentEntities
                .Include( dd => dd.Principal )
                .Single();

            System.Console.WriteLine( $"{nameof( d )}.{nameof( d.Principal )} is NULL: {null == d.Principal}" );
        }
    }
Moho
  • 15,457
  • 1
  • 30
  • 31
  • This is a nice solution but at the expense of introducing additional FK fields for each affected property plus manual configuration for each accepted property. Given that this problem only occurs when setting a property to null, is it worth it? Maybe if we could use an attribute on the FK field to automate the configuration, that would be lessen the pain. – Rob Kent Sep 21 '18 at 08:08
  • @RobKent - sure, `DataAnnotation` attributes would work fine; in any case, it would be worth it to me if it avoided a wasteful DB query, but there are other options such as eager loading the property if one knows they may be altering it or, as described in another answer, manipulating the `EntityEntry` directly (but then the user would need access to the `DbContext` which some don't want to allow - but even then they could create helper methods in a repository, for example, to set a property to null. – Moho Sep 21 '18 at 16:24
  • Given that EF Core does not allow lazy loading, and at some point we will have to try and port our EF6 apps over, my original question starts to lose its urgency for new projects. I guess the advice should be: choose EF Core or, if you are stuck on EF 6 (like me!), don't use lazy loading. – Rob Kent Nov 30 '18 at 13:17
  • EF core now supports lazy loading – Moho Dec 02 '18 at 02:03
  • That's interesting, thanks. Should make porting our application easier. – Rob Kent Dec 02 '18 at 08:09