Skip to content

Commit

Permalink
Fixup deleted join entities
Browse files Browse the repository at this point in the history 
Perform navigation fixup on many-to-many skip navigations when just the join entity is removed
Don't perform navigation fixup using a deleted entity when there is also an added entity with the same key value.

Fixes #24123
  • Loading branch information
@AndriySvyryd
AndriySvyryd committed Sep 17, 2021
1 parent c785e21 commit 76a18e5
Show file tree
Hide file tree
Showing 12 changed files with 323 additions and 134 deletions.
68 changes: 47 additions & 21 deletions src/EFCore/ChangeTracking/Internal/ChangeDetector.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,21 @@ public virtual void DetectChanges(IStateManager stateManager)

foreach (var entry in stateManager.ToList()) // Might be too big, but usually _all_ entities are using Snapshot tracking
{
if (entry.EntityState != EntityState.Detached)
switch (entry.EntityState)
{
LocalDetectChanges(entry);
case EntityState.Detached:
break;
case EntityState.Deleted:
if (entry.SharedIdentityEntry != null)
{
continue;
}

LocalDetectChanges(entry);
break;
default:
LocalDetectChanges(entry);
break;
}
}

Expand Down Expand Up @@ -205,26 +217,9 @@ private void LocalDetectChanges(InternalEntityEntry entry)
&& !entry.IsModified(property)
&& !entry.IsConceptualNull(property))
{
var current = entry[property];
var original = entry.GetOriginalValue(property);

if (!property.GetValueComparer().Equals(current, original))
{
if (entry.EntityState == EntityState.Deleted)
{
ThrowIfKeyChanged(entry, property);
}
else
{
LogChangeDetected(entry, property, original, current);
entry.SetPropertyModified(property);
}
}
DetectValueChange(entry, property);
}
}

foreach (var property in entityType.GetProperties())
{
DetectKeyChange(entry, property);
}

Expand All @@ -242,6 +237,31 @@ private void LocalDetectChanges(InternalEntityEntry entry)
}
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public void DetectValueChange(InternalEntityEntry entry, IProperty property)
{
var current = entry[property];
var original = entry.GetOriginalValue(property);

if (!property.GetValueComparer().Equals(current, original))
{
if (entry.EntityState == EntityState.Deleted)
{
ThrowIfKeyChanged(entry, property);
}
else
{
LogChangeDetected(entry, property, original, current);
entry.SetPropertyModified(property);
}
}
}

private void LogChangeDetected(InternalEntityEntry entry, IProperty property, object? original, object? current)
{
if (_loggingOptions.IsSensitiveDataLoggingEnabled)
Expand Down Expand Up @@ -289,7 +309,13 @@ private void DetectKeyChange(InternalEntityEntry entry, IProperty property)
}
}

private void DetectNavigationChange(InternalEntityEntry entry, INavigationBase navigationBase)
/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public void DetectNavigationChange(InternalEntityEntry entry, INavigationBase navigationBase)
{
var snapshotValue = entry.GetRelationshipSnapshotValue(navigationBase);
var currentValue = entry[navigationBase];
Expand Down
99 changes: 56 additions & 43 deletions src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.ComponentModel;
Expand Down Expand Up @@ -260,7 +261,7 @@ private void SetEntityState(EntityState oldState, EntityState newState, bool acc
throw new InvalidOperationException(
CoreStrings.TempValuePersists(
property.Name,
EntityType.DisplayName(), newState));
entityType.DisplayName(), newState));
}
}
}
Expand Down Expand Up @@ -290,7 +291,7 @@ private void SetEntityState(EntityState oldState, EntityState newState, bool acc
if (newState == EntityState.Unchanged)
{
_stateData.FlagAllProperties(
EntityType.PropertyCount(), PropertyFlag.Modified,
entityType.PropertyCount(), PropertyFlag.Modified,
flagged: false);
}

Expand Down Expand Up @@ -318,6 +319,8 @@ private void SetEntityState(EntityState oldState, EntityState newState, bool acc

_stateData.EntityState = newState;

// Save shared identity entity before it's detached
var sharedIdentityEntry = SharedIdentityEntry;
if (oldState == EntityState.Detached)
{
StateManager.StartTracking(this);
Expand All @@ -331,7 +334,7 @@ private void SetEntityState(EntityState oldState, EntityState newState, bool acc
|| newState == EntityState.Detached)
&& HasConceptualNull)
{
_stateData.FlagAllProperties(EntityType.PropertyCount(), PropertyFlag.Null, flagged: false);
_stateData.FlagAllProperties(entityType.PropertyCount(), PropertyFlag.Null, flagged: false);
}

if (oldState == EntityState.Detached
Expand All @@ -352,38 +355,53 @@ private void SetEntityState(EntityState oldState, EntityState newState, bool acc

FireStateChanged(oldState);

if (SharedIdentityEntry != null)
{
if (newState == EntityState.Unchanged)
{
SharedIdentityEntry.SetEntityState(EntityState.Detached);
}
else if (newState == EntityState.Modified
&& SharedIdentityEntry.EntityState == EntityState.Modified)
{
if (StateManager.SensitiveLoggingEnabled)
{
throw new InvalidOperationException(
CoreStrings.IdentityConflictSensitive(
EntityType.DisplayName(),
this.BuildCurrentValuesString(EntityType.FindPrimaryKey()!.Properties)));
}

throw new InvalidOperationException(
CoreStrings.IdentityConflict(
EntityType.DisplayName(),
EntityType.FindPrimaryKey()!.Properties.Format()));
}
}
HandleSharedIdentityEntry(oldState, newState, entityType);

if ((newState == EntityState.Deleted
|| newState == EntityState.Detached)
&& sharedIdentityEntry == null
&& StateManager.CascadeDeleteTiming == CascadeTiming.Immediate)
{
StateManager.CascadeDelete(this, force: false);
}
}

private void HandleSharedIdentityEntry(EntityState oldState, EntityState newState, IEntityType entityType)
{
var sharedIdentityEntry = SharedIdentityEntry;
if (sharedIdentityEntry == null)
{
return;
}

switch (newState)
{
case EntityState.Unchanged:
sharedIdentityEntry.SetEntityState(EntityState.Detached);
break;
case EntityState.Added:
case EntityState.Modified:
if (sharedIdentityEntry.EntityState == EntityState.Added
|| sharedIdentityEntry.EntityState == EntityState.Modified)
{
if (StateManager.SensitiveLoggingEnabled)
{
throw new InvalidOperationException(
CoreStrings.IdentityConflictSensitive(
EntityType.DisplayName(),
this.BuildCurrentValuesString(EntityType.FindPrimaryKey()!.Properties)));
}

throw new InvalidOperationException(
CoreStrings.IdentityConflict(
EntityType.DisplayName(),
EntityType.FindPrimaryKey()!.Properties.Format()));
}

break;
}
}

private void FireStateChanged(EntityState oldState)
{
StateManager.InternalEntityEntryNotifier.StateChanged(this, oldState, fromQuery: false);
Expand Down Expand Up @@ -1011,17 +1029,11 @@ public void SetOriginalValue(

// If setting the original value results in the current value being different from the
// original value, then mark the property as modified.
if (EntityState == EntityState.Unchanged
|| (EntityState == EntityState.Modified
&& !IsModified(property)))
{
var currentValue = this[propertyBase];
var propertyIndex = property.GetIndex();
if (!ValuesEqualFunc(property)(currentValue, value)
&& !_stateData.IsPropertyFlagged(propertyIndex, PropertyFlag.Unknown))
{
SetPropertyModified(property);
}
if ((EntityState == EntityState.Unchanged
|| (EntityState == EntityState.Modified && !IsModified(property)))
&& !_stateData.IsPropertyFlagged(property.GetIndex(), PropertyFlag.Unknown))
{
((StateManager as StateManager)?.ChangeDetector as ChangeDetector)?.DetectValueChange(this, property);
}
}

Expand All @@ -1034,6 +1046,7 @@ public void SetOriginalValue(
public void SetRelationshipSnapshotValue(IPropertyBase propertyBase, object? value)
{
EnsureRelationshipSnapshot();

_relationshipsSnapshot.SetValue(propertyBase, value);
}

Expand Down Expand Up @@ -1118,11 +1131,11 @@ public bool HasRelationshipSnapshot
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public void RemoveFromCollectionSnapshot(
IPropertyBase propertyBase,
INavigationBase navigation,
object removedEntity)
{
EnsureRelationshipSnapshot();
_relationshipsSnapshot.RemoveFromCollection(propertyBase, removedEntity);
_relationshipsSnapshot.RemoveFromCollection(navigation, removedEntity);
}

/// <summary>
Expand All @@ -1131,10 +1144,10 @@ public void RemoveFromCollectionSnapshot(
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public void AddToCollectionSnapshot(IPropertyBase propertyBase, object addedEntity)
public void AddToCollectionSnapshot(INavigationBase navigation, object addedEntity)
{
EnsureRelationshipSnapshot();
_relationshipsSnapshot.AddToCollection(propertyBase, addedEntity);
_relationshipsSnapshot.AddToCollection(navigation, addedEntity);
}

/// <summary>
Expand All @@ -1144,11 +1157,11 @@ public void AddToCollectionSnapshot(IPropertyBase propertyBase, object addedEnti
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public void AddRangeToCollectionSnapshot(
IPropertyBase propertyBase,
INavigationBase navigation,
IEnumerable<object> addedEntities)
{
EnsureRelationshipSnapshot();
_relationshipsSnapshot.AddRangeToCollection(propertyBase, addedEntities);
_relationshipsSnapshot.AddRangeToCollection(navigation, addedEntities);
}

/// <summary>
Expand Down
Loading

0 comments on commit 76a18e5

Please sign in to comment.