Skip to content

Commit

Permalink
Make state change to Deleted always result in Deleted state
Browse files Browse the repository at this point in the history 
Issue #1247. Calling Remove for and Added entity still resulted in the Detached (Unknown) state, but now setting the state to Deleted does result in the Deleted state.

The only place where setting a state may result in a different state is when attempting to set to Unchanged with an FK that is inconsistent. See #1246.
  • Loading branch information
@ajcvickers
ajcvickers committed Dec 10, 2014
1 parent bfbe3c2 commit f976d30
Show file tree
Hide file tree
Showing 7 changed files with 981 additions and 51 deletions.
45 changes: 28 additions & 17 deletions src/EntityFramework.Core/ChangeTracking/NavigationFixer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ private void InitialFixup(StateEntry entry, EntityState oldState)
NavigationCollectionChangedAction(
entry,
navigation,
((IEnumerable)navigationValue).Cast<object>(),
((IEnumerable)navigationValue).Cast<object>().ToList(),
Enumerable.Empty<object>());
}
else
Expand Down Expand Up @@ -393,26 +393,31 @@ private void Unfixup(IEnumerable<INavigation> navigations, StateEntry oldPrincip
{
foreach (var navigation in navigations)
{
if (navigation.PointsToPrincipal)
Unfixup(navigation, oldPrincipalEntry, dependentEntry);
oldPrincipalEntry.RelationshipsSnapshot.TakeSnapshot(navigation);
}
}

private void Unfixup(INavigation navigation, StateEntry oldPrincipalEntry, StateEntry dependentEntry)
{
if (navigation.PointsToPrincipal)
{
_setterSource.GetAccessor(navigation).SetClrValue(dependentEntry.Entity, null);
dependentEntry.RelationshipsSnapshot.TakeSnapshot(navigation);
}
else
{
if (navigation.IsCollection())
{
_setterSource.GetAccessor(navigation).SetClrValue(dependentEntry.Entity, null);
dependentEntry.RelationshipsSnapshot.TakeSnapshot(navigation);
var collectionAccessor = _collectionAccessorSource.GetAccessor(navigation);
if (collectionAccessor.Contains(oldPrincipalEntry.Entity, dependentEntry.Entity))
{
collectionAccessor.Remove(oldPrincipalEntry.Entity, dependentEntry.Entity);
}
}
else
{
if (navigation.IsCollection())
{
var collectionAccessor = _collectionAccessorSource.GetAccessor(navigation);
if (collectionAccessor.Contains(oldPrincipalEntry.Entity, dependentEntry.Entity))
{
collectionAccessor.Remove(oldPrincipalEntry.Entity, dependentEntry.Entity);
}
}
else
{
_setterSource.GetAccessor(navigation).SetClrValue(oldPrincipalEntry.Entity, null);
}
oldPrincipalEntry.RelationshipsSnapshot.TakeSnapshot(navigation);
_setterSource.GetAccessor(navigation).SetClrValue(oldPrincipalEntry.Entity, null);
}
}
}
Expand Down Expand Up @@ -505,6 +510,12 @@ private void SetInverse(StateEntry entry, INavigation navigation, object entity)
}
else
{
var oldEntity = _getterSource.GetAccessor(inverse).GetClrValue(entity);
if (oldEntity != null && oldEntity != entry.Entity)
{
Unfixup(navigation, entry.StateManager.GetOrCreateEntry(oldEntity), entry.StateManager.GetOrCreateEntry(entity));
}

_setterSource.GetAccessor(inverse).SetClrValue(entity, entry.Entity);
}

Expand Down
8 changes: 0 additions & 8 deletions src/EntityFramework.Core/ChangeTracking/StateEntry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,14 +176,6 @@ private void SetEntityState(EntityState oldState, EntityState newState)
return;
}

// An Added entity does not yet exist in the database. If it is then marked as deleted there is
// nothing to delete because it was not yet inserted, so just make sure it doesn't get inserted.
if (oldState == EntityState.Added
&& newState == EntityState.Deleted)
{
newState = EntityState.Unknown;
}

if (newState == EntityState.Unchanged)
{
_stateData.FlagAllProperties(EntityType.Properties.Count(), isFlagged: false);
Expand Down
42 changes: 38 additions & 4 deletions src/EntityFramework.Core/DbContext.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,12 @@ public virtual EntityEntry<TEntity> Remove<TEntity>([NotNull] TEntity entity)
{
Check.NotNull(entity, "entity");

return SetEntityState(entity, EntityState.Deleted);
// An Added entity does not yet exist in the database. If it is then marked as deleted there is
// nothing to delete because it was not yet inserted, so just make sure it doesn't get inserted.
return SetEntityState(
entity, Entry(entity).State == EntityState.Added
? EntityState.Unknown
: EntityState.Deleted);
}

private EntityEntry<TEntity> SetEntityState<TEntity>(TEntity entity, EntityState entityState)
Expand Down Expand Up @@ -354,7 +359,12 @@ public virtual EntityEntry Remove([NotNull] object entity)
{
Check.NotNull(entity, "entity");

return SetEntityState(entity, EntityState.Deleted);
// An Added entity does not yet exist in the database. If it is then marked as deleted there is
// nothing to delete because it was not yet inserted, so just make sure it doesn't get inserted.
return SetEntityState(
entity, Entry(entity).State == EntityState.Added
? EntityState.Unknown
: EntityState.Deleted);
}

private EntityEntry SetEntityState(object entity, EntityState entityState)
Expand Down Expand Up @@ -416,7 +426,19 @@ public virtual IReadOnlyList<EntityEntry<TEntity>> Remove<TEntity>([NotNull] par
{
Check.NotNull(entities, "entities");

return SetEntityStates(entities, EntityState.Deleted);
var entries = GetOrCreateEntries(entities);

// An Added entity does not yet exist in the database. If it is then marked as deleted there is
// nothing to delete because it was not yet inserted, so just make sure it doesn't get inserted.
foreach (var entry in entries)
{
entry.SetState(
entry.State == EntityState.Added
? EntityState.Unknown
: EntityState.Deleted);
}

return entries;
}

private List<EntityEntry<TEntity>> SetEntityStates<TEntity>(TEntity[] entities, EntityState entityState)
Expand Down Expand Up @@ -488,7 +510,19 @@ public virtual IReadOnlyList<EntityEntry> Remove([NotNull] params object[] entit
{
Check.NotNull(entities, "entities");

return SetEntityStates(entities, EntityState.Deleted);
var entries = GetOrCreateEntries(entities);

// An Added entity does not yet exist in the database. If it is then marked as deleted there is
// nothing to delete because it was not yet inserted, so just make sure it doesn't get inserted.
foreach (var entry in entries)
{
entry.SetState(
entry.State == EntityState.Added
? EntityState.Unknown
: EntityState.Deleted);
}

return entries;
}

private List<EntityEntry> SetEntityStates(object[] entities, EntityState entityState)
Expand Down
63 changes: 63 additions & 0 deletions test/EntityFramework.Core.Tests/ChangeTracking/EntityEntryTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,69 @@ public async Task Can_get_and_change_state_async()
}
}

[Fact]
public void Can_use_entry_to_change_state_to_Added()
{
ChangeStateOnEntry(EntityState.Unknown, EntityState.Added);
ChangeStateOnEntry(EntityState.Unchanged, EntityState.Added);
ChangeStateOnEntry(EntityState.Deleted, EntityState.Added);
ChangeStateOnEntry(EntityState.Modified, EntityState.Added);
ChangeStateOnEntry(EntityState.Added, EntityState.Added);
}

[Fact]
public void Can_use_entry_to_change_state_to_Unchanged()
{
ChangeStateOnEntry(EntityState.Unknown, EntityState.Unchanged);
ChangeStateOnEntry(EntityState.Unchanged, EntityState.Unchanged);
ChangeStateOnEntry(EntityState.Deleted, EntityState.Unchanged);
ChangeStateOnEntry(EntityState.Modified, EntityState.Unchanged);
ChangeStateOnEntry(EntityState.Added, EntityState.Unchanged);
}

[Fact]
public void Can_use_entry_to_change_state_to_Modified()
{
ChangeStateOnEntry(EntityState.Unknown, EntityState.Modified);
ChangeStateOnEntry(EntityState.Unchanged, EntityState.Modified);
ChangeStateOnEntry(EntityState.Deleted, EntityState.Modified);
ChangeStateOnEntry(EntityState.Modified, EntityState.Modified);
ChangeStateOnEntry(EntityState.Added, EntityState.Modified);
}

[Fact]
public void Can_use_entry_to_change_state_to_Deleted()
{
ChangeStateOnEntry(EntityState.Unknown, EntityState.Deleted);
ChangeStateOnEntry(EntityState.Unchanged, EntityState.Deleted);
ChangeStateOnEntry(EntityState.Deleted, EntityState.Deleted);
ChangeStateOnEntry(EntityState.Modified, EntityState.Deleted);
ChangeStateOnEntry(EntityState.Added, EntityState.Deleted);
}

[Fact]
public void Can_use_entry_to_change_state_to_Unknown()
{
ChangeStateOnEntry(EntityState.Unknown, EntityState.Unknown);
ChangeStateOnEntry(EntityState.Unchanged, EntityState.Unknown);
ChangeStateOnEntry(EntityState.Deleted, EntityState.Unknown);
ChangeStateOnEntry(EntityState.Modified, EntityState.Unknown);
ChangeStateOnEntry(EntityState.Added, EntityState.Unknown);
}

private void ChangeStateOnEntry(EntityState initialState, EntityState expectedState)
{
using (var context = new FreezerContext())
{
var entry = context.Add(new Chunky());

entry.SetState(initialState);
entry.SetState(expectedState);

Assert.Equal(expectedState, entry.State);
}
}

[Fact]
public void Can_get_property_entry_by_name()
{
Expand Down
8 changes: 4 additions & 4 deletions test/EntityFramework.Core.Tests/ChangeTracking/StateEntryTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ public void Changing_state_to_Unknown_causes_entity_to_stop_tracking()
Assert.DoesNotContain(entry, contextServices.GetRequiredService<StateManager>().StateEntries);
}

[Fact] // GitHub #251
public void Changing_state_from_Added_to_Deleted_causes_entity_to_stop_tracking()
[Fact] // GitHub #251, #1247
public void Changing_state_from_Added_to_Deleted_does_what_you_ask()
{
var model = BuildModel();
var entityType = model.GetEntityType(typeof(SomeEntity).FullName);
Expand All @@ -69,8 +69,8 @@ public void Changing_state_from_Added_to_Deleted_causes_entity_to_stop_tracking(
entry.SetEntityState(EntityState.Added);
entry.SetEntityState(EntityState.Deleted);

Assert.Equal(EntityState.Unknown, entry.EntityState);
Assert.DoesNotContain(entry, contextServices.GetRequiredService<StateManager>().StateEntries);
Assert.Equal(EntityState.Deleted, entry.EntityState);
Assert.Contains(entry, contextServices.GetRequiredService<StateManager>().StateEntries);
}

[Fact]
Expand Down
Loading

0 comments on commit f976d30

Please sign in to comment.