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.

Sign up for GitHub

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

Jump to bottom

Stop setting JSON array entities' state to Modified in SaveChanges #28926

Merged
merged 1 commit into from
Aug 31, 2022
Merged

Stop setting JSON array entities' state to Modified in SaveChanges #28926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 2 additions & 22 deletions src/EFCore.Relational/Update/ModificationCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,25 +259,7 @@ private sealed class JsonPartialUpdateInfo
public object? PropertyValue { get; set; }
}

private record struct JsonPartialUpdatePathEntry
{
public JsonPartialUpdatePathEntry(
string propertyName,
int? ordinal,
IUpdateEntry parentEntry,
INavigation navigation)
{
PropertyName = propertyName;
Ordinal = ordinal;
ParentEntry = parentEntry;
Navigation = navigation;
}

public string PropertyName { get; }
public int? Ordinal { get; }
public IUpdateEntry ParentEntry { get; }
public INavigation Navigation { get; }
}
private record struct JsonPartialUpdatePathEntry(string PropertyName, int? Ordinal, IUpdateEntry ParentEntry, INavigation Navigation);

private List<IColumnModification> GenerateColumnModifications()
{
Expand Down Expand Up @@ -334,7 +316,6 @@ private List<IColumnModification> GenerateColumnModifications()
var processedEntries = new List<IUpdateEntry>();
foreach (var entry in _entries.Where(e => e.EntityType.IsMappedToJson()))
{
var modifiedMembers = entry.ToEntityEntry().Properties.Where(m => m.IsModified).ToList();
var jsonColumn = entry.EntityType.GetContainerColumnName()!;
var jsonPartialUpdateInfo = FindJsonPartialUpdateInfo(entry, processedEntries);

Expand Down Expand Up @@ -423,7 +404,6 @@ private List<IColumnModification> GenerateColumnModifications()
}
}

var processedJsonNavigations = new List<INavigation>();
foreach (var entry in _entries.Where(x => !x.EntityType.IsMappedToJson()))
{
var nonMainEntry = !_mainEntryAdded || entry != _entries[0];
Expand Down Expand Up @@ -757,7 +737,7 @@ static JsonPartialUpdateInfo FindCommonJsonPartialUpdateInfo(
{
if (property.IsOrdinalKeyProperty() && ordinal != null)
{
entry.SetStoreGeneratedValue(property, ordinal.Value);
entry.SetStoreGeneratedValue(property, ordinal.Value, setModified: false);
}

continue;
Expand Down
28 changes: 5 additions & 23 deletions src/EFCore/ChangeTracking/Internal/EntityReferenceMap.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,29 +228,11 @@ public virtual IEnumerable<InternalEntityEntry> GetEntriesForState(
{
// Perf sensitive

var returnAdded
= added
&& _addedReferenceMap != null
&& _addedReferenceMap.Count > 0;

var returnModified
= modified
&& _modifiedReferenceMap != null
&& _modifiedReferenceMap.Count > 0;

var returnDeleted
= deleted
&& _deletedReferenceMap != null
&& _deletedReferenceMap.Count > 0;

var returnUnchanged
= unchanged
&& _unchangedReferenceMap != null
&& _unchangedReferenceMap.Count > 0;

var hasSharedTypes
= _sharedTypeReferenceMap != null
&& _sharedTypeReferenceMap.Count > 0;
var returnAdded = added && _addedReferenceMap is { Count: > 0 };
var returnModified = modified && _modifiedReferenceMap is { Count: > 0 };
var returnDeleted = deleted && _deletedReferenceMap is { Count: > 0 };
var returnUnchanged = unchanged && _unchangedReferenceMap is { Count: > 0 };
var hasSharedTypes = _sharedTypeReferenceMap is { Count: > 0 };

if (!hasSharedTypes)
{
Expand Down
10 changes: 4 additions & 6 deletions src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,8 +523,7 @@ public void SetPropertyModified(

var currentState = _stateData.EntityState;

if (currentState == EntityState.Added
|| currentState == EntityState.Detached
if (currentState is EntityState.Added or EntityState.Detached
|| !changeState)
{
var index = property.GetOriginalValueIndex();
Expand Down Expand Up @@ -573,8 +572,7 @@ public void SetPropertyModified(
}

if (isModified
&& (currentState == EntityState.Unchanged
|| currentState == EntityState.Detached))
&& currentState is EntityState.Unchanged or EntityState.Detached)
{
if (changeState)
{
Expand Down Expand Up @@ -746,7 +744,7 @@ public void MarkAsTemporary(IProperty property, bool temporary)
/// 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 SetStoreGeneratedValue(IProperty property, object? value)
public void SetStoreGeneratedValue(IProperty property, object? value, bool setModified = true)
{
if (property.GetStoreGeneratedIndex() == -1)
{
Expand All @@ -758,7 +756,7 @@ public void SetStoreGeneratedValue(IProperty property, object? value)
property,
value,
isMaterialization: false,
setModified: true,
setModified,
isCascadeDelete: false,
CurrentValueType.StoreGenerated);
}
Expand Down
3 changes: 2 additions & 1 deletion src/EFCore/Update/IUpdateEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ public interface IUpdateEntry
/// </summary>
/// <param name="property">The property to set the value for.</param>
/// <param name="value">The value to set.</param>
void SetStoreGeneratedValue(IProperty property, object? value);
/// <param name="setModified">Whether to set the store-generated property's state to Modified.</param>
void SetStoreGeneratedValue(IProperty property, object? value, bool setModified = true);

/// <summary>
/// Gets an <see cref="EntityEntry" /> for the entity being saved. <see cref="EntityEntry" /> is an API optimized for
Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ public void SetOriginalValue(IProperty property, object value)
public void SetPropertyModified(IProperty property)
=> throw new NotImplementedException();

public void SetStoreGeneratedValue(IProperty property, object value)
public void SetStoreGeneratedValue(IProperty property, object value, bool setModified = true)
=> throw new NotImplementedException();

public EntityEntry ToEntityEntry()
Expand Down
3 changes: 3 additions & 0 deletions test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,9 @@ public virtual Task Add_element_to_json_collection_leaf()
entity.OwnedReferenceRoot.OwnedReferenceBranch.OwnedCollectionLeaf.Add(newLeaf);
ClearLog();
await context.SaveChangesAsync();

// Do SaveChanges again, see #28813
await context.SaveChangesAsync();
},
async context =>
{
Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.Tests/ExceptionTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ public TProperty GetCurrentValue<TProperty>(IPropertyBase propertyBase)
public TProperty GetOriginalValue<TProperty>(IProperty property)
=> throw new NotImplementedException();

public void SetStoreGeneratedValue(IProperty property, object value)
public void SetStoreGeneratedValue(IProperty property, object value, bool setModified = true)
=> throw new NotImplementedException();

public EntityEntry ToEntityEntry()
Expand Down