Skip to content

Commit

Permalink
Throw if identifying foreign key is still unknown when saving changes
Browse files Browse the repository at this point in the history
Fixes #13666

Investigated this in more detail. Two `Object` (this is a custom `Object` class, not the normal `System.Object` type) entities are created. The first one is directly associated with a Container. The second one has no Container reference or FK set. However, because the FK is also part of the PK, and the PK uses generated values, it means that the FK gets a value set.

We do this to handle dependents encountered before principals when attaching the graph. But the intention here is that the dummy key value generated will be replaced once the principal is attached. If this doesn't happen, then SaveChanges will throw due to the FK constraint violation. This change provides clarity by failing in SaveChanges so we avoid ever sending the dummy value to the database.
  • Loading branch information
ajcvickers committed Dec 30, 2019
1 parent d5ead04 commit b5efa4b
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 0 deletions.
16 changes: 16 additions & 0 deletions src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,15 @@ protected virtual void MarkShadowPropertiesNotSet([NotNull] IEntityType entityTy
}
}

/// <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 virtual void MarkUnknown([NotNull] IProperty property)
=> _stateData.FlagProperty(property.GetIndex(), PropertyFlag.Unknown, true);

internal static readonly MethodInfo ReadShadowValueMethod
= typeof(InternalEntityEntry).GetTypeInfo().GetDeclaredMethod(nameof(ReadShadowValue));

Expand Down Expand Up @@ -1286,6 +1295,13 @@ public virtual InternalEntityEntry PrepareToSave()
property.Name,
EntityType.DisplayName()));
}

if (property.IsKey()
&& property.IsForeignKey()
&& _stateData.IsPropertyFlagged(property.GetIndex(), PropertyFlag.Unknown))
{
throw new InvalidOperationException(CoreStrings.UnknownKeyValue(entityType.DisplayName(), property.Name));
}
}
}
else if (EntityState == EntityState.Modified)
Expand Down
4 changes: 4 additions & 0 deletions src/EFCore/ChangeTracking/Internal/KeyPropagator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ public virtual InternalEntityEntry PropagateValue(InternalEntityEntry entry, IPr
{
entry[property] = value;
}

entry.MarkUnknown(property);
}
}

Expand Down Expand Up @@ -108,6 +110,8 @@ public virtual async Task<InternalEntityEntry> PropagateValueAsync(
{
entry[property] = value;
}

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 @@ -322,6 +322,9 @@
<data name="TempValuePersists" xml:space="preserve">
<value>The property '{property}' on entity type '{entityType}' has a temporary value while attempting to change the entity's state to '{state}'. Either set a permanent value explicitly or ensure that the database is configured to generate values for this property.</value>
</data>
<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="LogExceptionDuringQueryIteration" xml:space="preserve">
<value>An exception occurred while iterating over the results of a query for context type '{contextType}'.{newline}{error}</value>
<comment>Error CoreEventId.QueryIterationFailed Type string Exception</comment>
Expand Down
51 changes: 51 additions & 0 deletions test/EFCore.Tests/ChangeTracking/ChangeTrackerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,57 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking
{
public class ChangeTrackerTest
{
[ConditionalTheory]
[InlineData(false)]
[InlineData(true)]
public async Task Keys_generated_on_behalf_of_a_principal_are_not_saved(bool async)
{
using var context = new WeakHerosContext();

var entity = new Weak { Id = Guid.NewGuid() };

if (async)
{
await context.AddAsync(entity);
}
else
{
context.Add(entity);
}

Assert.Equal(
CoreStrings.UnknownKeyValue(nameof(Weak), nameof(Weak.HeroId)),
Assert.Throws<InvalidOperationException>(() => context.SaveChanges()).Message);
}

public class Hero
{
public Guid Id { get; set; }
public ICollection<Weak> Weaks { get; set; }
}

public class Weak
{
public Guid Id { get; set; }
public Guid HeroId { get; set; }

public Hero Hero { get; set; }
}

public class WeakHerosContext : DbContext
{
protected internal override void OnModelCreating(ModelBuilder modelBuilder)
=> modelBuilder.Entity<Weak>(
b =>
{
b.HasKey(e => new { e.Id, e.HeroId });
b.HasOne(e => e.Hero).WithMany(e => e.Weaks).HasForeignKey(e => e.HeroId);
});

protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
=> optionsBuilder.UseInMemoryDatabase(nameof(WeakHerosContext));
}

[ConditionalFact]
public void DetectChanges_is_logged()
{
Expand Down

0 comments on commit b5efa4b

Please sign in to comment.