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

Cosmos DB: NullReferenceException on SaveChanges when saving modified owned collection #13640

Closed
divega opened this issue Oct 16, 2018 · 11 comments
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@divega
Copy link
Contributor

divega commented Oct 16, 2018

Found this using nightly builds of Microsoft.EntityFrameworkCore.Cosmos.

System.NullReferenceException
  HResult=0x80004003
  Message=Object reference not set to an instance of an object.
  Source=Microsoft.EntityFrameworkCore.Cosmos
  StackTrace:
   at Microsoft.EntityFrameworkCore.Cosmos.Update.Internal.DocumentSource.UpdateDocument(JObject document, IUpdateEntry entry)
   at Microsoft.EntityFrameworkCore.Cosmos.Storage.Internal.CosmosDatabase.Save(IUpdateEntry entry)
   at Microsoft.EntityFrameworkCore.Cosmos.Storage.Internal.CosmosDatabase.SaveChanges(IReadOnlyList`1 entries)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(IReadOnlyList`1 entriesToSave)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Demos.Program.Main(String[] args) in C:\Users\divega\source\repos\CosmosDBNullRef\CosmosDBNullRef\Program.cs:line 64
 

Including the repro below. The error does not occur if I only query for the post and then modify the collection and save.

using System.Collections.Generic;
using System.Linq;
using Microsoft.EntityFrameworkCore;

namespace Demos
{
    public class Program
    {
        public static void Main(string[] args)
        {

            using (var context = new BloggingContext())
            {
                context.Database.EnsureDeleted();
                context.Database.EnsureCreated();

                context.Blogs.AddRange(
                    new Blog
                    {
                        BlogId = 1,
                        Name = "ADO.NET",
                        Url = "http://blogs.msdn.com/adonet",
                        Posts = new List<Post>
                        {
                            new Post
                            {
                                PostId = 1,
                                Title = "Welcome to this blog!",
                                Tags = new List<Tag>
                                {
                                    new Tag
                                    {
                                        Name = "Meta"
                                    }
                                }

                            },
                            new Post
                            {
                                PostId = 2,
                                Title = "Getting Started with ADO.NET",
                                Tags = new List<Tag>
                                {
                                    new Tag
                                    {
                                        Name = "Entity Framework Core"
                                    },
                                    new Tag
                                    {
                                        Name = "ADO.NET"
                                    }
                                }
                            }
                        }
                    });

                context.SaveChanges();
            }

            using (var context = new BloggingContext())
            {
                var blog = context.Blogs.Include(b=>b.Posts).Single(b => b.Name == "ADO.NET");
                // adding element to owned collection
                blog.Posts[1].Tags.Add(new Tag { Name = ".NET Core" }); 
                // Exception is thrown here:
                context.SaveChanges();
            }
        }
    }

    public class BloggingContext : DbContext
    {
        public DbSet<Blog> Blogs { get; set; }
        public DbSet<Post> Posts { get; set; }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder
                .UseCosmos(
                    "https://localhost:8081",
                    "C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==",
                    "EFCoreDemo");
        }
        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Post>().OwnsMany(p => p.Tags).HasKey(t => t.Name);
        }
    }

    public class Blog
    {
        public int BlogId { get; set; }
        public string Name { get; set; }
        public string Url { get; set; }
        public List<Post> Posts { get; set; }
    }

    public class Post
    {
        public int PostId { get; set; }
        public string Title { get; set; }
        public string Content { get; set; }
        public List<Tag> Tags { get; set; }
    }

    public class Tag
    {
        public string Name { get; set; }
    }
}
@AndriySvyryd
Copy link
Member

@divega I can fix the exception, but the Tag wouldn't be persistent anyway. The underlying issue is that we don't mark it as Added when detecting changes because the PK is already set. I don't remember exactly what was the reasoning for this behavior and whether we were already planning to change it. @ajcvickers thoughts?

@ajcvickers
Copy link
Contributor

@AndriySvyryd Is this a behavior specifically for owned types? Normally, DetectChanges always marks things as Added, regardless of key value. But I vaguely remember we tried to make some scenarios for owned entities work "better" by special casing what we do in DetectChanges. Is this something we should revisit?

@AndriySvyryd
Copy link
Member

I'm talking about https://github.com/aspnet/EntityFrameworkCore/blob/master/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs#L73
It's not owned-specific, though note that the Async versions got out-of-sync (pun intended) and does check ownership.

@AndriySvyryd
Copy link
Member

@ajcvickers Nevermind, that wasn't the real root issue. The existing owned entity wasn't tracked after querying due to #13579 and that confused the model differ as it was still part of the relationship snapshot.

@ekjuanrejon
Copy link

@AndriySvyryd whats the progress on this. I am having the same issue with preview 4

@AndriySvyryd
Copy link
Member

@ekjuanrejon The fix depends on #14455

@TomasHubelbauer
Copy link

#14455 has been merged, does that mean this should be fixed now? Because I am still running into this issue. Is there a workaround I could use meanwhile?

@ajcvickers
Copy link
Contributor

@TomasHubelbauer That comment indicates that #14455 was required before this could be fixed, but not that this is fixed itself.

@ajcvickers
Copy link
Contributor

@TomasHubelbauer It's also worth noting that if you're using Cosmos in the 3.0 previews, then there is still a lot of work left to do to get the query overhaul integrated with Cosmos. This won't happen for preview6, so if you are doing this you're probably better off staying on preview5 for now.

@AndriySvyryd
Copy link
Member

Fixed in 7d4b425

@AndriySvyryd
Copy link
Member

Currently Include doesn't work, so the repro wouldn't work as is, but the tracking issue is fixed.

@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 9, 2019
@AndriySvyryd AndriySvyryd removed their assignment Aug 13, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0, 3.0.0-preview9 Aug 21, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0-preview9, 3.0.0 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Projects
None yet
Development

No branches or pull requests

5 participants