Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
bryfa opened this issue Feb 26, 2020 · 4 comments
Closed

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

bryfa opened this issue Feb 26, 2020 · 4 comments
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported

Comments

@bryfa
Copy link

bryfa commented Feb 26, 2020

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

@bryfa bryfa added the type-bug label Feb 26, 2020
@ajcvickers
Copy link
Member

@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
Copy link
Author

bryfa commented Feb 28, 2020

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
Copy link
Member

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 #18299 (comment)

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.

@ajcvickers ajcvickers added closed-no-further-action The issue is closed and no further action is planned. customer-reported and removed type-bug labels Mar 6, 2020
@RMariowski
Copy link

I've got the same problem as mentioned here #20073 (comment).
So, what exactly is solution to this issue?

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported
Projects
None yet
Development

No branches or pull requests

3 participants