Skip to content

Commit 2cec5f7

Browse files
authored
[5.0.x] Don't leave unknown FK values when principal is known but has not-set, non-generated, value (#24178)
* Don't leave unknown FK values when principal is known but has not-set, non-generated, value (#23875) Fixes #23730 * [5.0.x] Don't leave unknown FK values when principal is known but has not-set, non-generated, value Port of 6.0 fix #23875 to 5.0 release. Fixes #23730 **Description** An exception is thrown when no key value is set for a non-generated key of an owned type. Normally this is a negative case since non-generated key values must be explicitly set. However, this can works when the non-generated value is part of a composite key for which other parts of the key are generated. In this case, the non-generated part can have the same default value for multiple inserts without violating the primary key constraint. **Customer Impact** This is a regression for the case described above. There is no reasonable workaround. (We already fixed this for EF Core 6.0, but decided not to patch since it seemed to be a regression only in a negative case. Since then other customers have reported the issue and one customer outlined the scenario above where it is a regression in working code.) **How found** Reported by multiple customers. **Test coverage** Test coverage for this case has been added in this PR. **Regression?** Yes, from EF Core 3.1. **Risk** Low. The fix is already in EF Core 6.0 and is targetted to this case. Also quirked.
1 parent 09ec021 commit 2cec5f7

File tree

2 files changed

+91
-12
lines changed

2 files changed

+91
-12
lines changed

src/EFCore/ChangeTracking/Internal/KeyPropagator.cs

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4+
using System;
45
using System.Threading;
56
using System.Threading.Tasks;
67
using JetBrains.Annotations;
@@ -30,6 +31,8 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal
3031
public class KeyPropagator : IKeyPropagator
3132
{
3233
private readonly IValueGeneratorSelector _valueGeneratorSelector;
34+
private readonly bool _useOldBehavior
35+
= AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue23730", out var enabled) && enabled;
3336

3437
/// <summary>
3538
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -53,12 +56,19 @@ public virtual InternalEntityEntry PropagateValue(InternalEntityEntry entry, IPr
5356
{
5457
Check.DebugAssert(property.IsForeignKey(), $"property {property} is not part of an FK");
5558

56-
var principalEntry = TryPropagateValue(entry, property);
59+
var generationProperty = _useOldBehavior
60+
? null
61+
: property.GetGenerationProperty();
62+
63+
var principalEntry = TryPropagateValue(entry, property, generationProperty);
64+
5765
if (principalEntry == null
5866
&& property.IsKey()
5967
&& !property.IsForeignKeyToSelf())
6068
{
61-
var valueGenerator = TryGetValueGenerator(property);
69+
var valueGenerator = _useOldBehavior
70+
? TryGetValueGenerator(property.GetGenerationProperty())
71+
: TryGetValueGenerator(generationProperty);
6272

6373
if (valueGenerator != null)
6474
{
@@ -93,11 +103,15 @@ public virtual async Task<InternalEntityEntry> PropagateValueAsync(
93103
{
94104
Check.DebugAssert(property.IsForeignKey(), $"property {property} is not part of an FK");
95105

96-
var principalEntry = TryPropagateValue(entry, property);
106+
var generationProperty = property.GetGenerationProperty();
107+
var principalEntry = TryPropagateValue(entry, property, generationProperty);
108+
97109
if (principalEntry == null
98110
&& property.IsKey())
99111
{
100-
var valueGenerator = TryGetValueGenerator(property);
112+
var valueGenerator = _useOldBehavior
113+
? TryGetValueGenerator(property.GetGenerationProperty())
114+
: TryGetValueGenerator(generationProperty);
101115

102116
if (valueGenerator != null)
103117
{
@@ -120,7 +134,7 @@ public virtual async Task<InternalEntityEntry> PropagateValueAsync(
120134
return principalEntry;
121135
}
122136

123-
private static InternalEntityEntry TryPropagateValue(InternalEntityEntry entry, IProperty property)
137+
private InternalEntityEntry TryPropagateValue(InternalEntityEntry entry, IProperty property, IProperty generationProperty)
124138
{
125139
var entityType = entry.EntityType;
126140
var stateManager = entry.StateManager;
@@ -158,7 +172,9 @@ private static InternalEntityEntry TryPropagateValue(InternalEntityEntry entry,
158172
if (principalProperty != property)
159173
{
160174
var principalValue = principalEntry[principalProperty];
161-
if (!principalProperty.ClrType.IsDefaultValue(principalValue))
175+
176+
if ((generationProperty == null && !_useOldBehavior)
177+
|| !principalProperty.ClrType.IsDefaultValue(principalValue))
162178
{
163179
if (principalEntry.HasTemporaryValue(principalProperty))
164180
{
@@ -182,13 +198,9 @@ private static InternalEntityEntry TryPropagateValue(InternalEntityEntry entry,
182198
return null;
183199
}
184200

185-
private ValueGenerator TryGetValueGenerator(IProperty property)
186-
{
187-
var generationProperty = property.GetGenerationProperty();
188-
189-
return generationProperty != null
201+
private ValueGenerator TryGetValueGenerator(IProperty generationProperty)
202+
=> generationProperty != null
190203
? _valueGeneratorSelector.Select(generationProperty, generationProperty.DeclaringEntityType)
191204
: null;
192-
}
193205
}
194206
}

test/EFCore.Tests/ChangeTracking/Internal/OwnedFixupTest.cs

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Collections.Generic;
66
using System.Collections.ObjectModel;
7+
using System.ComponentModel.DataAnnotations.Schema;
78
using System.Linq;
89
using System.Threading.Tasks;
910
using Microsoft.EntityFrameworkCore.Diagnostics;
@@ -4211,6 +4212,72 @@ EntityState GetEntryState<TEntity>(EquatableEntitiesContext context, string role
42114212
}
42124213
}
42134214

4215+
[ConditionalTheory]
4216+
[InlineData(false)]
4217+
[InlineData(true)]
4218+
public async Task SaveChanges_when_owner_has_PK_with_default_values(bool async)
4219+
{
4220+
using (var context = new OneRowContext(async))
4221+
{
4222+
var blog = new Blog { Type = new OwnedType { Value = "A" } };
4223+
4224+
_ = async
4225+
? await context.AddAsync(blog)
4226+
: context.Add(blog);
4227+
4228+
Assert.Equal(EntityState.Added, context.Entry(blog).State);
4229+
Assert.Equal(EntityState.Added, context.Entry(blog.Type).State);
4230+
Assert.Equal(0, blog.Id);
4231+
Assert.Equal(0, context.Entry(blog.Type).Property<int>("BlogId").CurrentValue);
4232+
4233+
_ = async
4234+
? await context.SaveChangesAsync()
4235+
: context.SaveChanges();
4236+
4237+
Assert.Equal(EntityState.Unchanged, context.Entry(blog).State);
4238+
Assert.Equal(EntityState.Unchanged, context.Entry(blog.Type).State);
4239+
Assert.Equal(0, blog.Id);
4240+
Assert.Equal(0, context.Entry(blog.Type).Property<int>("BlogId").CurrentValue);
4241+
}
4242+
4243+
using (var context = new OneRowContext(async))
4244+
{
4245+
// Trying to do the same thing again will throw since only one row can have ID zero.
4246+
4247+
context.Add(new Blog { Type = new OwnedType { Value = "A" } });
4248+
Assert.Throws<ArgumentException>(() => context.SaveChanges());
4249+
}
4250+
}
4251+
4252+
private class OneRowContext : DbContext
4253+
{
4254+
private readonly bool _async;
4255+
4256+
public OneRowContext(bool async)
4257+
{
4258+
_async = async;
4259+
}
4260+
4261+
protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
4262+
=> optionsBuilder.UseInMemoryDatabase(nameof(OneRowContext) + _async);
4263+
4264+
public DbSet<Blog> Blogs { get; set; }
4265+
}
4266+
4267+
public class Blog
4268+
{
4269+
[DatabaseGenerated(DatabaseGeneratedOption.None)]
4270+
public int Id { get; set; }
4271+
4272+
public OwnedType Type { get; set; }
4273+
}
4274+
4275+
[Owned]
4276+
public class OwnedType
4277+
{
4278+
public string Value { get; set; }
4279+
}
4280+
42144281
private class User
42154282
{
42164283
public Guid UserId { get; set; }

0 commit comments

Comments
 (0)