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

Throw better exception when replacing an owned entity #9833

Merged
merged 1 commit into from
Sep 16, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
38 changes: 28 additions & 10 deletions src/EFCore/ChangeTracking/Internal/IdentityMap.cs
Original file line number Diff line number Diff line change
Expand Up @@ -196,24 +196,42 @@ public virtual void Add(InternalEntityEntry entry)
/// </summary>
protected virtual void Add([NotNull] TKey key, [NotNull] InternalEntityEntry entry)
{
InternalEntityEntry existingEntry;
if (_identityMap.TryGetValue(key, out existingEntry))
if (_identityMap.TryGetValue(key, out var existingEntry))
{
if (existingEntry != entry)
{
if (_sensitiveLoggingEnabled)
if (entry.EntityType.IsOwned())
{
if (_sensitiveLoggingEnabled)
{
throw new InvalidOperationException(
CoreStrings.IdentityConflictOwnedSensitive(
entry.EntityType.DisplayName(),
entry.BuildCurrentValuesString(Key.Properties)));

}

throw new InvalidOperationException(
CoreStrings.IdentityConflictSensitive(
CoreStrings.IdentityConflictOwned(
entry.EntityType.DisplayName(),
entry.BuildCurrentValuesString(Key.Properties)));

Property.Format(Key.Properties)));
}
else
{
if (_sensitiveLoggingEnabled)
{
throw new InvalidOperationException(
CoreStrings.IdentityConflictSensitive(
entry.EntityType.DisplayName(),
entry.BuildCurrentValuesString(Key.Properties)));

throw new InvalidOperationException(
CoreStrings.IdentityConflict(
entry.EntityType.DisplayName(),
Property.Format(Key.Properties)));
}

throw new InvalidOperationException(
CoreStrings.IdentityConflict(
entry.EntityType.DisplayName(),
Property.Format(Key.Properties)));
}
}
}
else
Expand Down
16 changes: 16 additions & 0 deletions src/EFCore/Properties/CoreStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

60 changes: 33 additions & 27 deletions src/EFCore/Properties/CoreStrings.resx
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>
<root>
<!--
Microsoft ResX Schema

<!--
Microsoft ResX Schema
Version 2.0

The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
associated with the data types.

Example:

... ado.net/XML headers & schema ...
<resheader name="resmimetype">text/microsoft-resx</resheader>
<resheader name="version">2.0</resheader>
Expand All @@ -26,36 +26,36 @@
<value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
<comment>This is a comment</comment>
</data>

There are any number of "resheader" rows that contain simple
There are any number of "resheader" rows that contain simple
name/value pairs.

Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
mimetype set.

The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
extensible. For a given mimetype the value must be set accordingly:

Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
read any of the formats listed below.

mimetype: application/x-microsoft.net.object.binary.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
: and then encoded with base64 encoding.

mimetype: application/x-microsoft.net.object.soap.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Soap.SoapFormatter
: and then encoded with base64 encoding.

mimetype: application/x-microsoft.net.object.bytearray.base64
value : The object must be serialized into a byte array
value : The object must be serialized into a byte array
: using a System.ComponentModel.TypeConverter
: and then encoded with base64 encoding.
-->
Expand Down Expand Up @@ -790,4 +790,10 @@
<value>The same entity is being tracked as different dependent entity types '{dependent1}' and '{dependent2}'. If a property value changes it will result in two store changes, which might not be the desired outcome.</value>
<comment>Warning CoreEventId.DuplicateDependentEntityTypeInstanceWarning string string</comment>
</data>
<data name="IdentityConflictOwned" xml:space="preserve">
<value>The instance of entity type '{entityType}' cannot be tracked because another instance with the same key value for {keyProperties} is already being tracked. When replacing owned entities modify the properties without changing the instance or detach the previous owned entity entry first. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the conflicting key values.</value>
</data>
<data name="IdentityConflictOwnedSensitive" xml:space="preserve">
<value>The instance of entity type '{entityType}' cannot be tracked because another instance with the key value '{keyValue}' is already being tracked. When replacing owned entities modify the properties without changing the instance or detach the previous owned entity entry first.</value>
</data>
</root>
56 changes: 56 additions & 0 deletions test/EFCore.Tests/ChangeTracking/Internal/OwnedFixupTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,52 @@ public void Identity_changed_bidirectional(EntityState entityState)
}
}

[Theory] // Remove when #7340 is fixed
[InlineData(EntityState.Modified)]
[InlineData(EntityState.Unchanged)]
public void Replacing_owned_entity_throws(EntityState entityState)
{
using (var context = new FixupContext())
{
var principal = new ParentPN { Id = 77 };
var dependent1 = new ChildPN { Name = "1" };
principal.Child1 = dependent1;
var dependent2 = new ChildPN { Name = "2" };

context.ChangeTracker.TrackGraph(principal, e => e.Entry.State = entityState);

principal.Child1 = dependent2;

Assert.Equal(
CoreStrings.IdentityConflictOwned("ParentPN.Child1#ChildPN", "{'ParentId'}"),
Assert.Throws<InvalidOperationException>(
() => context.ChangeTracker.DetectChanges()).Message);
}
}

[Theory] // Remove when #7340 is fixed
[InlineData(EntityState.Modified)]
[InlineData(EntityState.Unchanged)]
public void Replacing_owned_entity_throws_sensitive(EntityState entityState)
{
using (var context = new SensitiveFixupContext())
{
var principal = new ParentPN { Id = 77 };
var dependent1 = new ChildPN { Name = "1" };
principal.Child1 = dependent1;
var dependent2 = new ChildPN { Name = "2" };

context.ChangeTracker.TrackGraph(principal, e => e.Entry.State = entityState);

principal.Child1 = dependent2;

Assert.Equal(
CoreStrings.IdentityConflictOwnedSensitive("ParentPN.Child1#ChildPN", "ParentId:77"),
Assert.Throws<InvalidOperationException>(
() => context.ChangeTracker.DetectChanges()).Message);
}
}

[Theory(Skip = "See issue#7340")]
[InlineData(EntityState.Added)]
[InlineData(EntityState.Modified)]
Expand Down Expand Up @@ -914,6 +960,16 @@ protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBu
}
}

private class SensitiveFixupContext : FixupContext
{
protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
{
optionsBuilder.EnableSensitiveDataLogging();

base.OnConfiguring(optionsBuilder);
}
}

private void AssertFixup(DbContext context, Action asserts)
{
asserts();
Expand Down