Skip to content

Commit

Permalink
Detect unknown shadow primary key values when attempting to save owne…
Browse files Browse the repository at this point in the history
…d collection

Fixes #19856

The fundamental issue here is that the key for owned collections is, by default, formed from shadow properties. This means the values of these properties are lost when the entities in the collection are no longer tracked by a DbContext. When the entities are then later attached there is no way to get these values back. (Contrast this with owned non-collection entities, where the shadow key value can be synthesized from the owner key.) This in turn means that there is no way to identify these entities in the database, and there is therefore no way to update or delete them.

This PR detects this situation and throws an exception with a (hopefully) helpful error message.

The best way to deal with this is to add a CLR property for the key (which can be private) to the owned entity type. The key values are then preserved while the entities are not being tracked, and re-attaching the entities works correctly. We should document this.
  • Loading branch information
ajcvickers committed Aug 22, 2021
1 parent 213d48f commit 70227e6
Show file tree
Hide file tree
Showing 9 changed files with 590 additions and 9 deletions.
18 changes: 18 additions & 0 deletions src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1443,12 +1443,30 @@ public InternalEntityEntry PrepareToSave()
property.Name,
EntityType.DisplayName()));
}

CheckForUnknownKey(property);
}
}
else if (EntityState == EntityState.Deleted)
{
foreach (var property in entityType.GetProperties())
{
CheckForUnknownKey(property);
}
}

DiscardStoreGeneratedValues();

return this;

void CheckForUnknownKey(IProperty property)
{
if (property.IsKey()
&& _stateData.IsPropertyFlagged(property.GetIndex(), PropertyFlag.Unknown))
{
throw new InvalidOperationException(CoreStrings.UnknownShadowKeyValue(entityType.DisplayName(), property.Name));
}
}
}

/// <summary>
Expand Down
9 changes: 8 additions & 1 deletion src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore.Diagnostics;
Expand Down Expand Up @@ -108,6 +107,14 @@ public virtual void Generate(InternalEntityEntry entry, bool includePrimaryKey =
property,
generatedValue,
temporary);

if (includePrimaryKey
&& property.IsPrimaryKey()
&& property.IsShadowProperty()
&& !property.IsForeignKey())
{
entry.MarkUnknown(property);
}
}
}

Expand Down
8 changes: 8 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.

3 changes: 3 additions & 0 deletions src/EFCore/Properties/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1518,6 +1518,9 @@
<data name="UnknownKeyValue" xml:space="preserve">
<value>The value of '{entityType}.{property}' is unknown when attempting to save changes. This is because the property is also part of a foreign key for which the principal entity in the relationship is not known.</value>
</data>
<data name="UnknownShadowKeyValue" xml:space="preserve">
<value>The value of shadow key property '{entityType}.{property}' is unknown when attempting to save changes. This is because shadow property values cannot be preserved when the entity is not being tracked. Consider adding the property to the entity's .NET type. See https://aka.ms/efcore-docs-owned-collections for more information.</value>
</data>
<data name="UnnamedIndexDefinedOnIgnoredProperty" xml:space="preserve">
<value>The unnamed index specified via [Index] attribute on the entity type '{entityType}' with properties {indexProperties} is invalid. The property '{propertyName}' was marked as unmapped by [NotMapped] attribute or 'Ignore()' in 'OnModelCreating'. An index cannot use unmapped properties.</value>
</data>
Expand Down
209 changes: 209 additions & 0 deletions test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,36 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
});

modelBuilder.Entity<SharedFkDependant>();

modelBuilder.Entity<Owner>();

modelBuilder.Entity<OwnerWithKeyedCollection>(
b =>
{
b.Navigation(e => e.Owned).IsRequired();
b.Navigation(e => e.OwnedWithKey).IsRequired();

b.OwnsMany(
e => e.OwnedCollectionPrivateKey,
b => b.HasKey("OwnerWithKeyedCollectionId", "PrivateKey"));
});

modelBuilder.Entity<OwnerNoKeyGeneration>(
b =>
{
b.Property(e => e.Id).ValueGeneratedNever();

b.OwnsOne(
e => e.Owned,
b => b.Property("OwnerNoKeyGenerationId").ValueGeneratedNever());
b.OwnsMany(
e => e.OwnedCollection,
b =>
{
b.Property<int>("OwnedNoKeyGenerationId").ValueGeneratedNever();
b.Property("OwnerNoKeyGenerationId").ValueGeneratedNever();
});
});
}

protected virtual object CreateFullGraph()
Expand Down Expand Up @@ -3092,6 +3122,185 @@ public SharedFkParent Parent
}
}

protected class Owner : NotifyingEntity
{
private int _id;
private Owned _owned;
private ICollection<Owned> _ownedCollection = new ObservableHashSet<Owned>();

public int Id
{
get => _id;
set => SetWithNotify(value, ref _id);
}

public Owned Owned
{
get => _owned;
set => SetWithNotify(value, ref _owned);
}

public ICollection<Owned> OwnedCollection
{
get => _ownedCollection;
set => SetWithNotify(value, ref _ownedCollection);
}
}

[Owned]
protected class Owned : NotifyingEntity
{
private int _foo;
private string _bar;

public int Foo
{
get => _foo;
set => SetWithNotify(value, ref _foo);
}

public string Bar
{
get => _bar;
set => SetWithNotify(value, ref _bar);
}
}

protected class OwnerWithKeyedCollection : NotifyingEntity
{
private int _id;
private Owned _owned;
private OwnedWithKey _ownedWithKey;
private ICollection<OwnedWithKey> _ownedCollection = new ObservableHashSet<OwnedWithKey>();
private ICollection<OwnedWithPrivateKey> _ownedCollectionPrivateKey = new ObservableHashSet<OwnedWithPrivateKey>();

public int Id
{
get => _id;
set => SetWithNotify(value, ref _id);
}

public Owned Owned
{
get => _owned;
set => SetWithNotify(value, ref _owned);
}

public OwnedWithKey OwnedWithKey
{
get => _ownedWithKey;
set => SetWithNotify(value, ref _ownedWithKey);
}

public ICollection<OwnedWithKey> OwnedCollection
{
get => _ownedCollection;
set => SetWithNotify(value, ref _ownedCollection);
}

public ICollection<OwnedWithPrivateKey> OwnedCollectionPrivateKey
{
get => _ownedCollectionPrivateKey;
set => SetWithNotify(value, ref _ownedCollectionPrivateKey);
}
}

[Owned]
protected class OwnedWithKey : NotifyingEntity
{
private int _foo;
private string _bar;
private int _ownedWithKeyId;

public int OwnedWithKeyId
{
get => _ownedWithKeyId;
set => SetWithNotify(value, ref _ownedWithKeyId);
}

public int Foo
{
get => _foo;
set => SetWithNotify(value, ref _foo);
}

public string Bar
{
get => _bar;
set => SetWithNotify(value, ref _bar);
}
}

[Owned]
protected class OwnedWithPrivateKey : NotifyingEntity
{
private int _foo;
private string _bar;
private int _privateKey;

private int PrivateKey
{
get => _privateKey;
set => SetWithNotify(value, ref _privateKey);
}

public int Foo
{
get => _foo;
set => SetWithNotify(value, ref _foo);
}

public string Bar
{
get => _bar;
set => SetWithNotify(value, ref _bar);
}
}

protected class OwnerNoKeyGeneration : NotifyingEntity
{
private int _id;
private OwnedNoKeyGeneration _owned;
private ICollection<OwnedNoKeyGeneration> _ownedCollection = new ObservableHashSet<OwnedNoKeyGeneration>();

public int Id
{
get => _id;
set => SetWithNotify(value, ref _id);
}

public OwnedNoKeyGeneration Owned
{
get => _owned;
set => SetWithNotify(value, ref _owned);
}

public ICollection<OwnedNoKeyGeneration> OwnedCollection
{
get => _ownedCollection;
set => SetWithNotify(value, ref _ownedCollection);
}
}

[Owned]
protected class OwnedNoKeyGeneration : NotifyingEntity
{
private int _foo;
private string _bar;

public int Foo
{
get => _foo;
set => SetWithNotify(value, ref _foo);
}

public string Bar
{
get => _bar;
set => SetWithNotify(value, ref _bar);
}
}

protected class NotifyingEntity : INotifyPropertyChanging, INotifyPropertyChanged
{
protected void SetWithNotify<T>(T value, ref T field, [CallerMemberName] string propertyName = "")
Expand Down
Loading

0 comments on commit 70227e6

Please sign in to comment.