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

How to update an entity with collection property that has either new, updated or deleted entities? #26830

Closed
LeonarddeR opened this issue Nov 25, 2021 · 11 comments
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported

Comments

@LeonarddeR
Copy link

LeonarddeR commented Nov 25, 2021

I have the following models:

public class SubscriptionLine
{
	public Guid Id { get; set; }
}

public class Subscription 
{
	public Guid Id { get; set; }
	
	public IEnumerable<SubscriptionLine> SubscriptionLines { get; set; }
}

I'm syncing these models to a database from an external source where a subscription comes in with a bunch of subscription lines. All the Ids are generated by the external source. Therefore:

  1. I know whether the subscription is in the database or not. If it is not, I add it to the database using dbContext.Add, which will also add all the subscription lines. That's just fine.
  2. If the subscription is already known to the database (i.e. it has been changed in the external source), I will have to update it, including the subscription lines. Every subscription line either
    • Is new, in which it has to be inserted
    • Is changed, in which case it has to be updated
    • Is no longer part of the SubscriptionLines IEnumerable, in which case it has to be removed from the database.

I'm not sure how to accomplish this, as I read in the documentation that a call to Update sets the state of the Subscription entity and all the SubscriptionLines to Modified. However, this means that entities are being updated that are not yet part of the database, which will fail of course.

When the subscription entity is known to the database, I can of course remove it, which will remove all the subscription lines, and then reinsert it. However, this looks like an overkill strategy. I read that updating a collection i.e. clearing it and than adding the new items does not work.

I'm asking this here because I think this is a use case that makes sense to be documented in the official documentation.

@ajcvickers
Copy link
Member

@LeonarddeR There is no easy answer to this. EF needs to know whether each entity is new or not, but if the entities come to EF with pre-generated keys, then there is no way for EF to automatically distinguish new entities from existing ones. Further, there is no way for EF to know if an entity has been removed from a collection as opposed to never being there in the first place. This is discussed in Explicitly Tracking Entities in the documentation.

In general, the ways to deal with this are:

  • Query for the existing graph and use it as the source of truth for what is currently in the databases, adding/attaching/removing as needed.
  • Keep track of new/deleted/modified entities in the external source, and send this information back to EF to be used to add/attach/remove as appropriate.

Also, consider voting to Turn-key Disconnected Entities Solution.

@LeonarddeR
Copy link
Author

LeonarddeR commented Nov 26, 2021

Thanks for your reply!

  • Query for the existing graph and use it as the source of truth for what is currently in the databases, adding/attaching/removing as needed.

This makes sense. I have no problem with querying the database for an existing entity, it's just the question how to update it properly. I assume that:

var currentSubscription = context.Subscriptions.Include(s => s.SubscriptionLines).FirstAsync();

Gets the current subscription with current lines from the database. Then, it should be relatively easy to find out whether the lines on a new subscription are tracked by EF or not. Is that correct? E.g. does context.Entity(newSubscription.SubscriptionLines.First()) respect the fact that an entry with the same primary key was tracked by the FirstAsync call above? Or is entity tracking information bound to the entity object, and not to the entity primary key?
If the former, I assume I can first fetch the currently saved subscription, then call the TrackGraph<TState> overload on a new subscription, where the provided callback should set tracked enttities to modified and untracked entities to added? I understand that this scenario doesn't deal with deleted objects, but that case is less important to me in this case.

@ajcvickers
Copy link
Member

@LeonarddeR Typically, this kind of thing is done through the EntityEntry and/or PropertyEntry APIs. For example, if you have a loaded entity, then you can apply current or original values to that entity from another instance or, from an instance of a DTO.

@LeonarddeR
Copy link
Author

LeonarddeR commented Nov 29, 2021

Thanks for pointing me at these docs, I must have missed them.

Code is now as follows:

    public async Task<Int32> CreateOrUpdateEntitiesAsync(List<TModel> entities, CancellationToken cancellationToken)
    {
        TId[]? existingIds = await this.GetExistingIdsAsync(entities, cancellationToken);
        foreach (TModel? entity in entities)
        {
            TId? idValue = this.ModelInfo.IdentifierValue<TModel, TId>(entity);
            if (existingIds.Contains(idValue))
            {
                EntityEntry<TModel> entry = this._dbContext.Update(entity);
                foreach (var collection in entry.Collections)
                {
                    if (collection.CurrentValue is null)
                    {
                        continue;
                    }
                    await collection.LoadAsync(cancellationToken);
                    IEnumerable<EntityEntry> subEntries = collection.CurrentValue.Cast<Object>().Select(e => this._dbContext.Entry(e));
                    foreach (var subEntry in subEntries)
                    {
                        if (subEntry.State == EntityState.Unchanged)
                        {
                            // This is a deleted sub entry.
                            subEntry.State = EntityState.Deleted;
                        }
                        else if ((await subEntry.GetDatabaseValuesAsync(cancellationToken)) is null)
                        {
                            subEntry.State = EntityState.Added;
                        }
                    }
                }
            }
            else
            {
                this._dbContext.Add(entity);
            }
        }

LoadAsync turned out to be very helpful to filter out the entities to delete. There's one major disadvantage to the current code, namely that I need to call subEntry.GetDatabaseValuesAsync to find out whether the entity has to be added or modified, even though I should already have this info in memory somehow by calling LoadAsync on the collection. @ajcvickers May be you have a suggestion to work around that?

@ajcvickers
Copy link
Member

@LeonarddeR It's not clear to me exactly what queries that method is executing. If the existing entity is queried with a tracking query, then EF keeps track of the "original values" of that entity, and can therefore determine automatically if any value has changed. There should be no need for GetDatabaseValuesAsync in this kind of scenario. I would suggest having a good read through the change tracking docs to understand how EF Core change tracking works and how it is used to generate database updates.

@LeonarddeR
Copy link
Author

@LeonarddeR It's not clear to me exactly what queries that method is executing. If the existing entity is queried with a tracking query, then EF keeps track of the "original values" of that entity, and can therefore determine automatically if any value has changed.

The code calling the CreateOrUpdateEntitiesAsync as shared above tries to add or update entities that are coming from an external source, and therefore not known to the DB Context, i.e. they are not yet tracked when they're entering the method. That's why I'm calling a GetExistingIdsAsync method that returns what entities are known to the database by id. However, that's on the parent level, i.e. the Subscription class as shared in my initial comment in this thread.
It's getting more difficult when I'm traversing the collection navigation properties to find out whether entities are already known in the db or not. That's why I'm calling Update on the subscription. That should set both the subscription as well as all subscriptionLines in the modified state. Calling collection.LoadAsync ensures that the subscription lines that are part of the database but no longer part of the subscription object's collection will be added to the collection in the unchanged state. SO that's why I know that I can safely delete the entities that are in the unchanged state. However, I can no longer distinguish between added and modified entities, since even the entities that are new are in the modified state. Therefore I call GetDatabaseValuesAsync to compensate for this.

@ajcvickers
Copy link
Member

@LeonarddeR Is it not possible to query for the graph of entities that might have changed and all their children, rather than trying to lookup each one individually? Once you have the graph loaded with all entities in the Unchanged state, then apply changes to these entities using the values from the client data. When SaveChanges is called, only the entities that actually changed will be saved.

@LeonarddeR
Copy link
Author

Would you be able to provide a small code example to get started?

A problem I might not have mentioned before is that I know neither the collection properties nor their types when querying the graph. So the base type might either be Subscription with a SubscriptionLines property of type IEnumerable it could also be an Invoice type with the InvoiceLines collection of type IEnumerable.

@ajcvickers
Copy link
Member

@LeonarddeR I have filed More samples for disconnected entities in the docs repo to track creating samples for the common ways to do disconnected entities.

@LeonarddeR
Copy link
Author

Thanks for this!

@ajcvickers
Copy link
Member

Closing this; adding sample code tracked by dotnet/EntityFramework.Docs#3601

@ajcvickers ajcvickers added the closed-no-further-action The issue is closed and no further action is planned. label Dec 10, 2021
@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

2 participants