dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.63k stars 3.15k forks source link

Value object mapping and queries do not work properly in EF core 3.1 #20073

Closed bryfa closed 1 year ago

bryfa commented 4 years ago

Writing and retrieving ValueObjects not working properly

Steps to reproduce

using System;
using System.Collections.Generic;
using System.Linq;

using Microsoft.EntityFrameworkCore;

namespace EfBug
{
    internal class Bar
    {
        public int Id { get; set; }

        public Owned Owned { get; set; }

        public string Simple { get; set; }
    }

    internal class Foo
    {
        public Bar Bar { get; set; }

        public int Id { get; set; }
    }

    internal class Owned : ValueObject
    {
        public static readonly Owned Sample = new Owned("Owned property");
        public static readonly Owned Empty = new Owned(null);

        public static bool operator ==(Owned left, Owned right)
        {
            return Equals(left, right);
        }

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

        private readonly string _value;

        private Owned(string value)
        {
            _value = value;
        }

        public string Value => _value; // without this one it does not work! but it did in previous versions..

        public override string ToString()
        {
            return _value;
        }

        protected override IEnumerable<object> GetAtomicValues()
        {
            yield return _value;
        }
    }

    public abstract class ValueObject
    {
        protected static bool EqualOperator(ValueObject left, ValueObject right)
        {
            if (ReferenceEquals(left, null) ^ ReferenceEquals(right, null))
            {
                return false;
            }
            return ReferenceEquals(left, null) || left.Equals(right);
        }

        protected static bool NotEqualOperator(ValueObject left, ValueObject right)
        {
            return !(EqualOperator(left, right));
        }

        protected abstract IEnumerable<object> GetAtomicValues();

        public override bool Equals(object obj)
        {
            if (obj == null || obj.GetType() != GetType())
            {
                return false;
            }

            ValueObject other = (ValueObject)obj;
            IEnumerator<object> thisValues = GetAtomicValues().GetEnumerator();
            IEnumerator<object> otherValues = other.GetAtomicValues().GetEnumerator();
            while (thisValues.MoveNext() && otherValues.MoveNext())
            {
                if (ReferenceEquals(thisValues.Current, null) ^
                    ReferenceEquals(otherValues.Current, null))
                {
                    return false;
                }

                if (thisValues.Current != null &&
                    !thisValues.Current.Equals(otherValues.Current))
                {
                    return false;
                }
            }
            return !thisValues.MoveNext() && !otherValues.MoveNext();
        }

        public override int GetHashCode()
        {
            return GetAtomicValues()
                .Select(x => x != null ? x.GetHashCode() : 0)
                .Aggregate((x, y) => x ^ y);
        }
    }

    internal class MyContext : DbContext
    {
        public MyContext() : base(new DbContextOptionsBuilder()
             .UseSqlServer("Server=(localdb)\\mssqllocaldb;Database=EFCoreTesting;Trusted_Connection=True;MultipleActiveResultSets=true").Options)
        {
        }

        public DbSet<Bar> Bars { get; private set; }

        public DbSet<Foo> Foos { get; private set; }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Bar>().OwnsOne(e => e.Owned, p =>
            {
                p.Property(x => x.Value).HasColumnName("Owned"); //.IsRequired();
            });
        }
    }

    internal class Program
    {
        private static void Main(string[] args)
        {
            using (var context = new MyContext())
            {
                context.Database.EnsureDeleted();
                context.Database.EnsureCreated();
                context.Add(new Foo
                {
                    Bar = new Bar
                    {
                        Simple = "Simple property",
                        Owned = Owned.Sample
                    }
                });

                // This one should fail also if IsRequired()
                // is used on the owned property mapping
                context.Add(new Foo
                {
                    Bar = new Bar
                    {
                        Simple = "Simple property",
                        Owned = Owned.Empty
                    }
                });
                // but does not on my machine. After checking the DB
                // the table was created with an Owned column but null was allowed..

                context.SaveChanges();
            }

            // first selection OK
            using (var context = new MyContext())
            {
                var bar = context.Bars.First();

                Dump(bar);
            }

            // second selection OK
            using (var context = new MyContext())
            {
                var bar = context.Foos
                    .Select(e => e.Bar).First();

                Dump(bar);
            }

            // third selection OK
            using (var context = new MyContext())
            {
                var bar = context.Foos
                    .Include(f => f.Bar)
                        .Select(e => e.Bar).First();

                Dump(bar);
            }

            // fourth selection OK
            using (var context = new MyContext())
            {
                var bar = context.Foos
                    .Include(f => f.Bar)
                    .Where(x => x.Bar.Owned == null)
                    .Select(e => e.Bar)
                    .Single();

                Dump(bar);
            }

//            // fifth selection FAILS (only makes sense if NULL is allowed)
//            using (var context = new MyContext())
//            {
//                var bar = context.Foos
//                    .Include(f => f.Bar)
//                    .Where(x => x.Bar.Owned == Owned.Empty)
//                    .Select(e => e.Bar)
//                    .Single();
//
//                Dump(bar);
//            }

//            // sixth selection also (actually same as previous) FAILS
//            using (var context = new MyContext())
//            {
//                var bar = context.Foos
//                    .Include(f => f.Bar)
//                        .Where(x => x.Bar.Owned == Owned.Sample)
//                    .Select(e => e.Bar)
//                    .Single();
//            
//                Dump(bar);
//            }

            Console.ReadKey();

            void Dump(Bar bar)
            {
                Console.WriteLine("Simple property: {0}", bar.Simple);
                Console.WriteLine("Owned property : {0}", bar.Owned?.ToString() ?? "<EMPTY>");
            }
        }
    }
}

The sample fails on the 5th and 6th selection. Also note that when the Value property is removed in the Owned type class. We get different results.

Further technical details

EF Core version: 3.1.2 Database provider: Microsoft.EntityFrameworkCore.SqlServer Target framework: .NET Core 3.1 Operating system: Windows10 IDE: Visual Studio 2019 16.3

ajcvickers commented 4 years ago

@bryfa I've run your code and I do get the following exception when fifth and sixth cases. Is this what you are reporting as the issue, or is it something else about the behavior here?

Unhandled exception. System.InvalidOperationException: No backing field could be found for property 'BarId' of entity type 'Owned' and the property does not have a getter.
   at Microsoft.EntityFrameworkCore.PropertyBaseExtensions.GetMemberInfo(IPropertyBase propertyBase, Boolean forMaterialization, Boolean forSet)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.ClrPropertyGetterFactory.Create(IPropertyBase property)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.PropertyBase.<>c.<get_Getter>b__33_0(PropertyBase p)
   at Microsoft.EntityFrameworkCore.Internal.NonCapturingLazyInitializer.EnsureInitialized[TParam,TValue](TValue& target, TParam param, Func`2 valueFactory)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.PropertyBase.get_Getter()
   at Microsoft.EntityFrameworkCore.PropertyBaseExtensions.GetGetter(IPropertyBase propertyBase)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityEqualityRewritingExpressionVisitor.CreatePropertyAccessExpression(Expression target, IProperty property)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityEqualityRewritingExpressionVisitor.CreateKeyAccessExpression(Expression target, IReadOnlyList`1 properties)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityEqualityRewritingExpressionVisitor.RewriteEntityEquality(Boolean equality, IEntityType entityType, Expression left, INavigation leftNavigation, Expression right, INavigation rightNavigation, Boolean subqueryTraversed)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityEqualityRewritingExpressionVisitor.RewriteEquality(Boolean equality, Expression left, Expression right)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityEqualityRewritingExpressionVisitor.VisitBinary(BinaryExpression binaryExpression)
   at System.Linq.Expressions.BinaryExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityEqualityRewritingExpressionVisitor.RewriteAndVisitLambda(LambdaExpression lambda, EntityReferenceExpression source)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityEqualityRewritingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at System.Linq.Expressions.MethodCallExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityEqualityRewritingExpressionVisitor.VisitSelectMethodCall(MethodCallExpression methodCallExpression)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityEqualityRewritingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at System.Linq.Expressions.MethodCallExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityEqualityRewritingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at System.Linq.Expressions.MethodCallExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityEqualityRewritingExpressionVisitor.Rewrite(Expression expression)
   at Microsoft.EntityFrameworkCore.Query.QueryTranslationPreprocessor.Process(Expression query)
   at Microsoft.EntityFrameworkCore.Query.QueryCompilationContext.CreateQueryExecutor[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Storage.Database.CompileQuery[TResult](Expression query, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](IDatabase database, Expression query, IModel model, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass9_0`1.<Execute>b__0()
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQueryCore[TFunc](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.Execute[TResult](Expression expression)
   at System.Linq.Queryable.Single[TSource](IQueryable`1 source)
   at EfBug.Program.Main(String[] args) in C:\Stuff\MyRepros\AllTogetherNow\ThreeOne\ThreeOne.cs:line 210
bryfa commented 4 years ago

Yes that's the main issue here. But there is more going on.. In the previous version it was allowed to leave out the real property on the owned class:

public string Value => _value; // <-- was not needed in previous versions

with this mapping:

protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Bar>().OwnsOne(e => e.Owned, p =>
            {
                p.Property<string>("Value").HasColumnName("Owned"); //.IsRequired();
            });
        }

In the previous version(EF Core 2.2) it worked perfectly without the property, because "Value" was a "shadow property" on the entity. In this version its required.

--edit I have read the breaking changes list and noticed that this was reported. So now we need to do this:

protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Bar>().OwnsOne(e => e.Owned, p =>
            {
                p.Property("_value").HasColumnName("Owned");
            });
        }

and then we can remove the Value property from the owned type.

Another strange behavior I came across is the "IsRequired()" setting on this property. When set and after update the database does not have the "NOT NULL" constraint on the "Value object" column. Will check this again.

ajcvickers commented 4 years ago

Another strange behavior I came across is the "IsRequired()" setting on this property. When set and after update the database does not have the "NOT NULL" constraint on the "Value object" column.

This is likely a consequence of the change to to optional owned entities. See https://docs.microsoft.com/en-us/ef/core/what-is-new/ef-core-3.0/breaking-changes#de and https://github.com/dotnet/efcore/issues/18299#issuecomment-587152089

In the previous version(EF Core 2.2) it worked perfectly without the property, because "Value" was a "shadow property" on the entity.

This probably doesn't matter much, but if the _value field existed and was being used, then this wasn't technically a shadow property, but rather a property mapped directly to the field. Shadow properties don't have any storage (i.e. no explicit or generated field) for the value in the entity instance itself. Hopefully it's a bit clearer in the 3.1 version that the _value field is explicitly mapped.

RMariowski commented 4 years ago

I've got the same problem as mentioned here https://github.com/dotnet/efcore/issues/20073#issuecomment-592052242. So, what exactly is solution to this issue?