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

Lazy-loading of an owned entity throws #19847

Closed
xkaede opened this issue Feb 9, 2020 · 13 comments
Closed

Lazy-loading of an owned entity throws #19847

xkaede opened this issue Feb 9, 2020 · 13 comments

Comments

@xkaede
Copy link

xkaede commented Feb 9, 2020

When I Update an entity's owned type to null, then do Detect Changes. after that read the navigation property, it throws InvalidOperationException: "A tracking query projects owned entity without corresponding owner in result. Owned entities cannot be tracked without their owner. Either include the owner entity in the result or make query non-tracking using AsNoTracking().";

If I set the naviation poperty to another instance which is not null,I can read it successfully.
by the way.when when lazy loading is not used, I can read it as null well.

Steps to reproduce

public class Order
{
    public int Id { get; set; }
    public StreetAddress? ShippingAddress { get; set; }
}

[Owned]
public class StreetAddress
{
    public string Street { get; set; }
    public string City { get; set; }
}

public class TestDbContext : DbContext
{
    public DbSet<Order> Orders {get;set;}

    protected override void OnConfiguring(DbContextOptionsBuilder options)
    {
        options.UseLazyLoadingProxies();
    }
}

using (var context = new TestDbContext())
{
      // seed order with non-null ShippingAddress.
      context.Orders.Add(new Order {Id = 1, ShippingAddress = new ShippingAddress {Street ="a", City = "b"}});
     context.SaveChanges();
}

//  update ShippingAddress  to null will throw the InvalidOperationException.
using (var context = new TestDbContext())
{
    var order = context.Orders.First();
    //  set ShippingAddress to null.
    order.ShippingAddress = null;
    context.Attach(order);
    context.Update(order);
    Console.WriteLine(order.ShippingAddress);  //  prints null.
    context.ChangeTracker.DetectChanges();
    Console.WriteLine(order.ShippingAddress); // will throw an exception.
}

//  update ShippingAddress  to non-null will not throw the InvalidOperationException.
using (var context = new TestDbContext())
{
    var order = context.Orders.First();
    //  set ShippingAddress to non-null.
    order.ShippingAddress = new ShippingAddress{Street ="c", City = "d"};
    context.Attach(order);
    context.Update(order);
    Console.WriteLine(order.ShippingAddress);  //  prints updated ShippingAddress.
    context.ChangeTracker.DetectChanges();
    Console.WriteLine(order.ShippingAddress);  //  prints updated ShippingAddress without the invalid operation exception
}

Further technical details

EF Core version:3.1.1
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: netcoreapp3.1
Operating system: windows 10
IDE: Visual Studio 2019 16.3

@xkaede xkaede added the type-bug label Feb 9, 2020
@ajcvickers
Copy link
Member

@xkaede I have not been able to reproduce the exception you are seeing--my code is below. Please post a small, runnable project or a complete code listing (like below) that reproduces the behavior you are seeing.

Also, it's worth noting that in the code above the Attach and Update calls are at best unnecessary, and potentially not correct. Since the code is doing a tracking query there is no value in then explicitly tracking with Attach or Update. Also, calling Update after Attach is usually incorrect. I wrote a couple of blog posts on this that might be useful: https://blog.oneunicorn.com/2020/01/17/dontcallupdate/ and https://blog.oneunicorn.com/2020/01/18/docallupdate/.

public class Order
{
    [DatabaseGenerated(DatabaseGeneratedOption.None)]
    public int Id { get; set; }
    public StreetAddress ShippingAddress { get; set; }
}

[Owned]
public class StreetAddress
{
    public string Street { get; set; }
    public string City { get; set; }
}

public class BloggingContext : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer(My.SqlServerConnectionString);
    }

    public DbSet<Order> Orders { get; set; }
}

public class Program
{
    public static async Task Main()
    {
        using (var context = new BloggingContext())
        {
            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();

            context.Orders.Add(new Order {Id = 1, ShippingAddress = new StreetAddress {Street ="a", City = "b"}});
            context.SaveChanges();
        }

        using (var context = new BloggingContext())
        {
            var order = context.Orders.First();
            //  set ShippingAddress to null.
            order.ShippingAddress = null;
            context.Attach(order);
            context.Update(order);
            Console.WriteLine(order.ShippingAddress);  //  prints null.
            context.ChangeTracker.DetectChanges();
            Console.WriteLine(order.ShippingAddress); // will throw an exception.
        }

        using (var context = new BloggingContext())
        {
            var order = context.Orders.First();
            //  set ShippingAddress to non-null.
            order.ShippingAddress = new StreetAddress{Street ="c", City = "d"};
            context.Attach(order);
            context.Update(order);
            Console.WriteLine(order.ShippingAddress);  //  prints updated ShippingAddress.
            context.ChangeTracker.DetectChanges();
            Console.WriteLine(order.ShippingAddress);  //  pri
        }
    }
}

@xkaede
Copy link
Author

xkaede commented Feb 13, 2020

hi @ajcvickers
Thanks for your detailed explanation.

In the code above you missed the lazy loding configuration. The issues only happens when lazy loading is turned on.

public class BloggingContext : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer(My.SqlServerConnectionString);
        //  add Lazy Loading
        optionsBuilder.UseLazyLoadingProxies();
    }

    public DbSet<Order> Orders { get; set; }
}

and I readed your blog posts,It is useful for me.I didn't knwon attach and update well.

but let me explain.
I used Attach and Update in the code before in order to simplify the problem.The real scene is that I want to collect the changed entities which is Added and Modified and Deleted before SaveChanges.then send them to a message bus.below is the code.

using (var context = new BloggingContext())
{
    var order = context.Orders.First();
    //  set ShippingAddress to null.
    order.ShippingAddress = null;
    //  if I disable the AutoDetectChanges, the entity's state is Unchanged. 
    //  context.ChangeTracker.AutoDetectChangesEnabled = false;
    var entityEntries = context.ChangeTracker.Entries().ToList();
    foreach (var entity in entityEntries)
    {
        switch (entity.State)
        {
            case EntityState.Added:
                //  Trigger  an entity Added event.
            case EntityState.Modified:
                //  Trigger an entity Modified event.
            case EntityState.Deleted:
                //  Trigger an entity Deleted event.
                break;
        }
    }
    Console.WriteLine(order.ShippingAddress); // will throw an exception.
}

I tried to disable the AutoDetectChanges,but the state of the entities is all unchanged.

Here's the complete code.

internal class Program
{
    private static void Main(string[] args)
    {
        using (var context = new BloggingContext())
        {
            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();

            context.Orders.Add(new Order {Id = 1, ShippingAddress = new StreetAddress {Street = "a", City = "b"}});
            context.SaveChanges();
        }

        using (var context = new BloggingContext())
        {
            var order = context.Orders.First();
            //  set ShippingAddress to null.
            order.ShippingAddress = null;
            //  if I disable the AutoDetectChanges, the entity's state is Unchanged. 
            //  context.ChangeTracker.AutoDetectChangesEnabled = false;
            var entityEntries = context.ChangeTracker.Entries().ToList();
            foreach (var entity in entityEntries)
            {
                switch (entity.State)
                {
                    case EntityState.Added:
                    //  Trigger an entity Added event.
                    case EntityState.Modified:
                    //  Trigger an entity Modified event.
                    case EntityState.Deleted:
                        //  Trigger an entity Deleted event.
                        break;
                }
            }
            Console.WriteLine(order.ShippingAddress); // will throw an exception.
        }

        // before
        //using (var context = new BloggingContext())
        //{
        //    var order = context.Orders.First();
        //    //  set ShippingAddress to null.
        //    order.ShippingAddress = null;
        //    context.Attach(order);
        //    context.Update(order);
        //    Console.WriteLine(order.ShippingAddress); //  prints null.
        //    context.ChangeTracker.DetectChanges();
        //    Console.WriteLine(order.ShippingAddress); // will throw an exception.
        //}

        //using (var context = new BloggingContext())
        //{
        //    var order = context.Orders.First();
        //    //  set ShippingAddress to non-null.
        //    order.ShippingAddress = new StreetAddress {Street = "c", City = "d"};
        //    context.Attach(order);
        //    context.Update(order);
        //    Console.WriteLine(order.ShippingAddress); //  prints updated ShippingAddress.
        //    context.ChangeTracker.DetectChanges();
        //    Console.WriteLine(order.ShippingAddress); //  pri
        //}
    }
}

public class Order
{
    [DatabaseGenerated(DatabaseGeneratedOption.None)]
    public int Id { get; set; }

    public virtual StreetAddress ShippingAddress { get; set; }
}

[Owned]
public class StreetAddress
{
    public string Street { get; set; }
    public string City { get; set; }
}

public class BloggingContext : DbContext
{
    public DbSet<Order> Orders { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer("Server=localhost;Database=EF-Bug;Trusted_Connection=True;");
        optionsBuilder.UseLazyLoadingProxies();
    }
}

@ajcvickers
Copy link
Member

@xkaede Try disabling lazy-loading while doing your processing of the tracked entities. Something like:

 try
 {
     context.ChangeTracker.LazyLoadingEnabled = false;

    // Your code here...
 }
 finally
 {
     context.ChangeTracker.LazyLoadingEnabled = true;
 }

Team: Lazy-loading of an owned entity throws:

usr/share/dotnet/dotnet /home/ajcvickers/AllTogetherNow/ThreeOne/bin/Debug/netcoreapp3.1/ThreeOne.dll

Unhandled exception. System.InvalidOperationException: A tracking query projects owned entity without corresponding owner in result. Owned entities cannot be tracked without their owner. Either include the owner entity in the result or make query non-tracking using AsNoTracking().
   at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.EntityMaterializerInjectingExpressionVisitor.Inject(Expression expression)
   at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.InjectEntityMaterializers(Expression expression)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.VisitShapedQueryExpression(ShapedQueryExpression shapedQueryExpression)
   at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.VisitExtension(Expression extensionExpression)
   at System.Linq.Expressions.Expression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   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 Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1.GetEnumerator()
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.Load[TSource](IQueryable`1 source)
   at Microsoft.EntityFrameworkCore.Internal.EntityFinder`1.Load(INavigation navigation, InternalEntityEntry entry)
   at Microsoft.EntityFrameworkCore.ChangeTracking.NavigationEntry.Load()
   at Microsoft.EntityFrameworkCore.Internal.LazyLoader.Load(Object entity, String navigationName)
   at Microsoft.EntityFrameworkCore.Proxies.Internal.LazyLoadingInterceptor.Intercept(IInvocation invocation)
   at Castle.DynamicProxy.AbstractInvocation.Proceed()
   at Castle.Proxies.OrderProxy.get_ShippingAddress()
   at Program.Main() in /home/ajcvickers/AllTogetherNow/ThreeOne/ThreeOne.cs:line 60
   at Program.<Main>()

@ajcvickers ajcvickers changed the title Null Owned Type throws exception after Detect Changes when lazy loading is on Lazy-loading of an owned entity throws Feb 17, 2020
@xkaede
Copy link
Author

xkaede commented Feb 18, 2020

@ajcvickers Disabling lazy-loading works, but I need to lazy load entity's data when I map it to a dto, so If disable lazy-loading, I will miss the data.

@ajcvickers
Copy link
Member

Note for team: probably covered by #12462--stop lazy-loading in this case.

@ajcvickers
Copy link
Member

@AndriySvyryd Would you agree that lazy loading of any part of an aggregate should be a no-op? The reasoning being that an aggregate should always be fully loaded. This means that even if a reference to an owned entity is null after the owner has been attached, then we should still never attempt to load it?

/cc @smitpatel

@ajcvickers
Copy link
Member

@xkaede I'm not suggesting that you disable it always, but rather that you disable it while process the entities to generate the events. (This case looks a lot like auditing entity states. In those cases you pretty much always need to turn off lazy loading anyway because otherwise your whole database graph would be loaded into memory.)

@AndriySvyryd
Copy link
Member

@ajcvickers That only applies for tracked entities, for non-tracked we allow to query owned entities without the owner.

@ajcvickers
Copy link
Member

@AndriySvyryd Good point. Although lazy loading doesn't currently work for non-tracked entities anyway.

@xkaede
Copy link
Author

xkaede commented Feb 20, 2020

@ajcvickers Thank you very much.
Follow your method.I disable the lazy loading and do Explicitly Loading before map entities to generate the events like below. and it works.

using (var context = new BloggingContext())
{
    var order = context.Orders.First();
    //  set ShippingAddress to null.
    order.ShippingAddress = null;
    try
    {
        context.ChangeTracker.LazyLoadingEnabled = false;

        //  Explicitly Loading
        context.Entry(order).Collection(t=>t.OrderLines).Load();

        var entityEntries = context.ChangeTracker.Entries().ToList();
        foreach (var entity in entityEntries)
        {
            switch (entity.State)
            {
                case EntityState.Added:
                //  Trigger entity Added event.
                case EntityState.Modified:
                //  Trigger entity Modified event.
                case EntityState.Deleted:
                    //  Trigger entity Deleted event.
                    break;
            }
        }

        Console.WriteLine(order.ShippingAddress);
    }
    finally
    {
        context.ChangeTracker.LazyLoadingEnabled = true;
    }
}

@ajcvickers
Copy link
Member

Team notes: The fix for #12462 will stop this throwing because the navigation will no longer be configured to lazy-load.

That being said, if loading of owned entity is explicitly triggered through the ILazyLoader mechanism, then in this case (owner with null, non-loaded owned) then this will still throw. If many people hit this then we could consider a better exception.

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

Xriuk commented Feb 24, 2023

The reasoning being that an aggregate should always be fully loaded.

Is there a reason for this? Why can't aggregates be lazy loaded and tracked? I my entity OwnsMany aggregates and I don't want to eager load them all but only where needed why can't this happen?

For example currently I have my Products entities which are owned by the corresponding Order, which makes sense logically (at least to me). But I may need to not load the products if I'm only looking at orders, and when looking at a particular order when I need to get info about its products they get lazy loaded, only when needed.

@ajcvickers
Copy link
Member

@Xriuk An aggregate is used as a single unit, always loaded together. If you work with an order without its products, then it's not an aggregate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants