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

Child entities not persisting when using AddRangeAsync and SaveChangesAsync #7298

Closed
cdarrigo opened this issue Dec 22, 2016 · 9 comments
Closed
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

@cdarrigo
Copy link

Attempting to add a graph of entities where there is a 1:m relationship (parent: children) using the AddRangeAsync and Savechanges Async method will prevent the children from being persisted to the DB.

In the code below, the a collection of StoreProductEntity (Parent) are being persisted in the DBContext . Each StoreProductEntity has an IList (child). After calling AddRangeAsync() and SaveChangesAsync(), only the parent entities are persisted, the children are not.

This is different behavior than the corresponding synchronous methods, AddRange() and SaveChanges(), where in the entire graph (parent and children) are persisted to the DB.

[Table("StoreProduct")]
    public class StoreProductEntity
    {
        public int Id { get; set; }
        public string Name { get; set; }
        
        public virtual IList<StoreProductPriceEntity> Prices { get; set; }

    }
    [Table("StoreProductPrice")]
    public class StoreProductPriceEntity
    {
        public int Id { get; set; }
        public decimal SalesPrice { get; set; }
        public virtual StoreProductEntity StoreProduct { get; set; }
        public int StoreProductId { get; set; }
    }

    public class Provider
    {
        private readonly DataContext dataContext;

        public Provider(DataContext dataContext)
        {
            this.dataContext = dataContext;
        }
        public async Task<int> AddAndSaveAsync(IEnumerable<StoreProductEntity> entities)
        {
            await this.dataContext.StoreProducts.AddRangeAsync(entities);
            var numAdded = await this.dataContext.SaveChangesAsync();
            // StoreProductEntities are saved, the children StoreProductPrice entities are not saving.
            return numAdded;
        }
    }

    public class Sample
    {
        private readonly Provider provider;

        public Sample(Provider provider)
        {
            this.provider = provider;
        }
        public async Task<int> SaveEntitiesSample()
        {
            var entities = new List<StoreProductEntity>
            {
                new StoreProductEntity
                {
                    Name = "A",
                    Prices = new List<StoreProductPriceEntity>
                    {
                        new StoreProductPriceEntity {SalesPrice = 1m}
                    }
                },
                new StoreProductEntity
                {
                    Name = "B",
                    Prices = new List<StoreProductPriceEntity>
                    {
                        new StoreProductPriceEntity {SalesPrice = 2m}
                    }
                }
            };

            // This call will persist the StoreProducts, but not the nested storeproductPrices.
            return await this.provider.AddAndSaveAsync(entities);

            //Also Tried. 
            foreach (var entity in entities)
            {
                foreach (var price in entity.Prices)
                {
                    price.StoreProduct = entity;
                }
            }
            // same result.  Store Products are persisted, but not the nested/child storeproductprices.
            return await this.provider.AddAndSaveAsync(entities);


        }
    }

Further technical details

EF Core version: 1.1
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Win 10
IDE: Visual Studio 2015

@mkprajeen
Copy link

I too face same issue while adding enity with child collection using AddRangeAsync with SaveChangesAsync

@ajcvickers
Copy link
Member

Note for triage: looks like async Add methods are, at least sometimes, only acting on a single object.

@cdarrigo @mkprajeen You almost certainly don't need to use the async methods for Add/AddRange. They provide no benefit over the sync versions except when using the specialized sequence value generator. (It would be great to know if you are actually using that value generator.)

@mkprajeen
Copy link

It works fine when I changed to AddRange instead of AddRangeAsync. thanks ajcvickers

@divega divega added the type-bug label Jan 6, 2017
@divega divega added this to the 2.0.0 milestone Jan 6, 2017
@ajcvickers
Copy link
Member

@divega @rowanmiller Should this also go in 1.1.1? AddAsync first shipped in 1.1.

@rowanmiller
Copy link
Contributor

To me it seems like a serious enough issue to warrant a patch.

@ajcvickers ajcvickers modified the milestones: 1.1.1, 2.0.0 Jan 13, 2017
@ajcvickers
Copy link
Member

Talked to @divega and moving.

@ajcvickers
Copy link
Member

Justification: The behavior of Add and AddAsync should be the same. The async versions not doing graph behavior is a copy/paste error.
Risk: Low-risk; copy/paste error.

ajcvickers added a commit that referenced this issue Jan 17, 2017
Issue #7298

This requires quite a lot of new async code because all the graph traversal and processing code now needs async paths.
ajcvickers added a commit that referenced this issue Jan 18, 2017
Issue #7298

This is a lower-risk fix replacing #7430. The fix is to call the non-async methods. There is an extremely low probability that any real-world code is making database calls in the async methods combined with an even lower probability that making those calls synchronously would cause issues, so this fix is low-risk and does not have binary-compat issues.
@Eilon
Copy link
Member

Eilon commented Jan 19, 2017

This patch bug is approved. Please use the normal code review process w/ a PR and make sure the fix is in the correct branch, then close the bug and mark it as done.

ajcvickers added a commit that referenced this issue Jan 19, 2017
Issue #7298

This is a lower-risk fix replacing #7430. The fix is to call the non-async methods. There is an extremely low probability that any real-world code is making database calls in the async methods combined with an even lower probability that making those calls synchronously would cause issues, so this fix is low-risk and does not have binary-compat issues.
ajcvickers added a commit that referenced this issue Jan 19, 2017
Issue #7298

This is a lower-risk fix replacing #7430. The fix is to call the non-async methods. There is an extremely low probability that any real-world code is making database calls in the async methods combined with an even lower probability that making those calls synchronously would cause issues, so this fix is low-risk and does not have binary-compat issues.
@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 Jan 19, 2017
@anpete
Copy link
Contributor

anpete commented Feb 24, 2017

✅ Verified

@ajcvickers ajcvickers removed their assignment Sep 1, 2024
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

7 participants