Skip to content

Commit

Permalink
Update DeleteBehavior.Restrict to not apply when navigations are changed
Browse files Browse the repository at this point in the history
Issue #9703

Restrict now means that EF will never either set an FK to null or perform a cascade delete if an entity is deleted. This is the same as before, although the exception type may change--it's a negative case and we triaged this break as okay. However, if a navigation property is explicitly changed, then the FK associated with that navigation property will still be set to null. This is the change from the previous behavior. This changes some negative cases into positive cases, which was also triaged as okay.
  • Loading branch information
ajcvickers committed Feb 12, 2018
1 parent d1159c1 commit 5a02b32
Show file tree
Hide file tree
Showing 6 changed files with 958 additions and 541 deletions.
1,020 changes: 517 additions & 503 deletions src/EFCore.Specification.Tests/GraphUpdatesTestBase.cs

Large diffs are not rendered by default.

8 changes: 3 additions & 5 deletions src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ public virtual void MarkUnchangedFromQuery([CanBeNull] ISet<IForeignKey> handled
StateManager.InternalEntityEntryNotifier.StateChanged(this, EntityState.Detached, fromQuery: true);

StateManager.OnTracked(this, fromQuery: true);

var trackingQueryMode = StateManager.GetTrackingQueryMode(EntityType);
if (trackingQueryMode != TrackingQueryMode.Simple)
{
Expand Down Expand Up @@ -809,8 +809,7 @@ public virtual void SetProperty([NotNull] IPropertyBase propertyBase, [CanBeNull
if (asProperty != null
&& (!asProperty.ClrType.IsNullableType()
|| asProperty.GetContainingForeignKeys().Any(
fk => (fk.DeleteBehavior == DeleteBehavior.Cascade
|| fk.DeleteBehavior == DeleteBehavior.Restrict)
fk => (fk.DeleteBehavior == DeleteBehavior.Cascade)
&& fk.DeclaringEntityType.IsAssignableFrom(EntityType))))
{
if (value == null)
Expand Down Expand Up @@ -954,8 +953,7 @@ public virtual void HandleConceptualNulls(bool sensitiveLoggingEnabled)
if (_stateData.IsPropertyFlagged(property.GetIndex(), PropertyFlag.Null))
{
if (properties.Any(p => p.IsNullable)
&& foreignKey.DeleteBehavior != DeleteBehavior.Cascade
&& foreignKey.DeleteBehavior != DeleteBehavior.Restrict)
&& foreignKey.DeleteBehavior != DeleteBehavior.Cascade)
{
foreach (var toNull in properties)
{
Expand Down
30 changes: 18 additions & 12 deletions src/EFCore/ChangeTracking/Internal/NavigationFixer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -487,29 +487,35 @@ private void DeleteFixup(InternalEntityEntry entry)

foreach (var foreignKey in entityType.GetForeignKeys())
{
var principalToDependent = foreignKey.PrincipalToDependent;
if (principalToDependent != null)
if (foreignKey.DeleteBehavior != DeleteBehavior.Restrict)
{
var principalEntry = stateManager.GetPrincipal(entry, foreignKey);
if (principalEntry != null
&& principalEntry.EntityState != EntityState.Deleted)
var principalToDependent = foreignKey.PrincipalToDependent;
if (principalToDependent != null)
{
ResetReferenceOrRemoveCollection(principalEntry, principalToDependent, entry);
var principalEntry = stateManager.GetPrincipal(entry, foreignKey);
if (principalEntry != null
&& principalEntry.EntityState != EntityState.Deleted)
{
ResetReferenceOrRemoveCollection(principalEntry, principalToDependent, entry);
}
}
}
}

foreach (var foreignKey in entityType.GetReferencingForeignKeys())
{
var dependentToPrincipal = foreignKey.DependentToPrincipal;
if (dependentToPrincipal != null)
if (foreignKey.DeleteBehavior != DeleteBehavior.Restrict)
{
var dependentEntries = stateManager.GetDependents(entry, foreignKey);
foreach (var dependentEntry in dependentEntries)
var dependentToPrincipal = foreignKey.DependentToPrincipal;
if (dependentToPrincipal != null)
{
if (dependentEntry[dependentToPrincipal] == entry.Entity)
var dependentEntries = stateManager.GetDependents(entry, foreignKey);
foreach (var dependentEntry in dependentEntries)
{
SetNavigation(dependentEntry, dependentToPrincipal, null);
if (dependentEntry[dependentToPrincipal] == entry.Entity)
{
SetNavigation(dependentEntry, dependentToPrincipal, null);
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/ChangeTracking/Internal/StateManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,7 @@ private void CascadeDelete(InternalEntityEntry entry)

CascadeDelete(dependent);
}
else
else if (fk.DeleteBehavior != DeleteBehavior.Restrict)
{
foreach (var dependentProperty in fk.Properties)
{
Expand Down
60 changes: 40 additions & 20 deletions test/EFCore.InMemory.FunctionalTests/GraphUpdatesInMemoryTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,28 @@ public GraphUpdatesInMemoryTest(GraphUpdatesInMemoryFixture fixture)
{
}

public override void Optional_One_to_one_relationships_are_one_to_one()
public override DbUpdateException Optional_One_to_one_relationships_are_one_to_one()
{
// FK uniqueness not enforced in in-memory database
return null;
}

public override void Required_One_to_one_relationships_are_one_to_one()
public override DbUpdateException Required_One_to_one_relationships_are_one_to_one()
{
// FK uniqueness not enforced in in-memory database
return null;
}

public override void Optional_One_to_one_with_AK_relationships_are_one_to_one()
public override DbUpdateException Optional_One_to_one_with_AK_relationships_are_one_to_one()
{
// FK uniqueness not enforced in in-memory database
return null;
}

public override void Required_One_to_one_with_AK_relationships_are_one_to_one()
public override DbUpdateException Required_One_to_one_with_AK_relationships_are_one_to_one()
{
// FK uniqueness not enforced in in-memory database
return null;
}

public override void Save_required_one_to_one_changed_by_reference_with_alternate_key(ChangeMechanism changeMechanism, bool useExistingEntities)
Expand All @@ -45,9 +49,10 @@ public override void Save_required_non_PK_one_to_one_changed_by_reference_with_a
// Cascade delete not supported by in-memory database
}

public override void Save_required_one_to_one_changed_by_reference(ChangeMechanism changeMechanism)
public override DbUpdateException Save_required_one_to_one_changed_by_reference(ChangeMechanism changeMechanism)
{
// Cascade delete not supported by in-memory database
return null;
}

public override void Save_removed_required_many_to_one_dependents(ChangeMechanism changeMechanism)
Expand All @@ -65,9 +70,10 @@ public override void Sever_required_one_to_one_with_alternate_key(ChangeMechanis
// Cascade delete not supported by in-memory database
}

public override void Sever_required_one_to_one(ChangeMechanism changeMechanism)
public override DbUpdateException Sever_required_one_to_one(ChangeMechanism changeMechanism)
{
// Cascade delete not supported by in-memory database
return null;
}

public override void Sever_required_non_PK_one_to_one(ChangeMechanism changeMechanism)
Expand All @@ -80,74 +86,88 @@ public override void Sever_required_non_PK_one_to_one_with_alternate_key(ChangeM
// Cascade delete not supported by in-memory database
}

public override void Required_many_to_one_dependents_are_cascade_deleted_in_store()
public override DbUpdateException Required_many_to_one_dependents_are_cascade_deleted_in_store()
{
// Cascade delete not supported by in-memory database
return null;
}

public override void Required_one_to_one_are_cascade_deleted_in_store()
public override DbUpdateException Required_one_to_one_are_cascade_deleted_in_store()
{
// Cascade delete not supported by in-memory database
return null;
}

public override void Required_non_PK_one_to_one_are_cascade_deleted_in_store()
public override DbUpdateException Required_non_PK_one_to_one_are_cascade_deleted_in_store()
{
// Cascade delete not supported by in-memory database
return null;
}

public override void Required_many_to_one_dependents_with_alternate_key_are_cascade_deleted_in_store()
public override DbUpdateException Required_many_to_one_dependents_with_alternate_key_are_cascade_deleted_in_store()
{
// Cascade delete not supported by in-memory database
return null;
}

public override void Required_one_to_one_with_alternate_key_are_cascade_deleted_in_store()
public override DbUpdateException Required_one_to_one_with_alternate_key_are_cascade_deleted_in_store()
{
// Cascade delete not supported by in-memory database
return null;
}

public override void Required_non_PK_one_to_one_with_alternate_key_are_cascade_deleted_in_store()
public override DbUpdateException Required_non_PK_one_to_one_with_alternate_key_are_cascade_deleted_in_store()
{
// Cascade delete not supported by in-memory database
return null;
}

public override void Optional_many_to_one_dependents_are_orphaned_in_store()
public override DbUpdateException Optional_many_to_one_dependents_are_orphaned_in_store()
{
// Cascade nulls not supported by in-memory database
return null;
}

public override void Optional_one_to_one_are_orphaned_in_store()
public override DbUpdateException Optional_one_to_one_are_orphaned_in_store()
{
// Cascade nulls not supported by in-memory database
return null;
}

public override void Optional_many_to_one_dependents_with_alternate_key_are_orphaned_in_store()
public override DbUpdateException Optional_many_to_one_dependents_with_alternate_key_are_orphaned_in_store()
{
// Cascade nulls not supported by in-memory database
return null;
}

public override void Optional_one_to_one_with_alternate_key_are_orphaned_in_store()
public override DbUpdateException Optional_one_to_one_with_alternate_key_are_orphaned_in_store()
{
// Cascade nulls not supported by in-memory database
return null;
}

public override void Required_non_PK_one_to_one_with_alternate_key_are_cascade_detached_when_Added()
public override DbUpdateException Required_non_PK_one_to_one_with_alternate_key_are_cascade_detached_when_Added()
{
// Cascade nulls not supported by in-memory database
return null;
}

public override void Required_one_to_one_are_cascade_detached_when_Added()
public override DbUpdateException Required_one_to_one_are_cascade_detached_when_Added()
{
// Cascade nulls not supported by in-memory database
return null;
}

public override void Required_one_to_one_with_alternate_key_are_cascade_detached_when_Added()
public override DbUpdateException Required_one_to_one_with_alternate_key_are_cascade_detached_when_Added()
{
// Cascade nulls not supported by in-memory database
return null;
}

public override void Required_non_PK_one_to_one_are_cascade_detached_when_Added()
public override DbUpdateException Required_non_PK_one_to_one_are_cascade_detached_when_Added()
{
// Cascade nulls not supported by in-memory database
return null;
}

protected override void ExecuteWithStrategyInTransaction(
Expand Down

0 comments on commit 5a02b32

Please sign in to comment.