Skip to content

Commit

Permalink
Generate values for PK properties that are also self-referencing FK p…
Browse files Browse the repository at this point in the history 
…roperties (#22585)

Fixes #22573
  • Loading branch information
@ajcvickers
ajcvickers committed Sep 18, 2020
1 parent 5588c48 commit 46a6e9b
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 16 deletions.
27 changes: 16 additions & 11 deletions src/EFCore/ChangeTracking/Internal/KeyPropagator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ public virtual InternalEntityEntry PropagateValue(InternalEntityEntry entry, IPr

var principalEntry = TryPropagateValue(entry, property);
if (principalEntry == null
&& property.IsKey())
&& property.IsKey()
&& !property.IsForeignKeyToSelf())
{
var valueGenerator = TryGetValueGenerator(property);

Expand Down Expand Up @@ -153,19 +154,23 @@ private static InternalEntityEntry TryPropagateValue(InternalEntityEntry entry,
if (principalEntry != null)
{
var principalProperty = foreignKey.PrincipalKey.Properties[propertyIndex];
var principalValue = principalEntry[principalProperty];
if (!principalProperty.ClrType.IsDefaultValue(principalValue))

if (principalProperty != property)
{
if (principalEntry.HasTemporaryValue(principalProperty))
var principalValue = principalEntry[principalProperty];
if (!principalProperty.ClrType.IsDefaultValue(principalValue))
{
entry.SetTemporaryValue(property, principalValue);
if (principalEntry.HasTemporaryValue(principalProperty))
{
entry.SetTemporaryValue(property, principalValue);
}
else
{
entry[property] = principalValue;
}

return principalEntry;
}
else
{
entry[property] = principalValue;
}

return principalEntry;
}
}

Expand Down
34 changes: 29 additions & 5 deletions src/EFCore/Metadata/Internal/PropertyExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
using System.Linq;
using System.Runtime.CompilerServices;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.ValueGeneration;
using Microsoft.EntityFrameworkCore.Utilities;

namespace Microsoft.EntityFrameworkCore.Metadata.Internal
{
Expand Down Expand Up @@ -95,15 +95,39 @@ public static IProperty GetGenerationProperty([NotNull] this IProperty property)
}

/// <summary>
/// Gets a value indicating whether this property requires a <see cref="ValueGenerator" /> to generate
/// values when new entities are added to the context.
/// 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 static bool RequiresValueGenerator([NotNull] this IProperty property)
=> (property.ValueGenerated.ForAdd()
&& !property.IsForeignKey()
&& property.IsKey())
&& property.IsKey()
&& (!property.IsForeignKey() || property.IsForeignKeyToSelf()))
|| property.GetValueGeneratorFactory() != null;

/// <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 static bool IsForeignKeyToSelf([NotNull] this IProperty property)
{
Check.DebugAssert(property.IsKey(), "Only call this method for properties known to be part of a key.");

foreach (var foreignKey in property.GetContainingForeignKeys())
{
var propertyIndex = foreignKey.Properties.IndexOf(property);
if (propertyIndex == foreignKey.PrincipalKey.Properties.IndexOf(property))
{
return true;
}
}

return false;
}

/// <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
Expand Down
92 changes: 92 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Xunit;

namespace Microsoft.EntityFrameworkCore
{
Expand Down Expand Up @@ -73,6 +74,97 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
}
}

public class TptIdentity : GraphUpdatesSqlServerTestBase<TptIdentity.SqlServerFixture>
{
public TptIdentity(SqlServerFixture fixture)
: base(fixture)
{
}

[ConditionalFact(Skip = "Issue #22582")]
public override void Can_add_multiple_dependents_when_multiple_possible_principal_sides()
{
}

[ConditionalFact(Skip = "Issue #22582")]
public override void Can_add_valid_first_dependent_when_multiple_possible_principal_sides()
{
}

[ConditionalFact(Skip = "Issue #22582")]
public override void Can_add_valid_second_dependent_when_multiple_possible_principal_sides()
{
}

protected override void UseTransaction(DatabaseFacade facade, IDbContextTransaction transaction)
=> facade.UseTransaction(transaction.GetDbTransaction());

public class SqlServerFixture : GraphUpdatesSqlServerFixtureBase
{
protected override string StoreName { get; } = "GraphTptIdentityUpdatesTest";

protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
{
modelBuilder.UseIdentityColumns();

base.OnModelCreating(modelBuilder, context);

modelBuilder.Entity<Root>().ToTable(nameof(Root));
modelBuilder.Entity<Required1>().ToTable(nameof(Required1));
modelBuilder.Entity<Required1Derived>().ToTable(nameof(Required1Derived));
modelBuilder.Entity<Required1MoreDerived>().ToTable(nameof(Required1MoreDerived));
modelBuilder.Entity<Required2Derived>().ToTable(nameof(Required2Derived));
modelBuilder.Entity<Required2MoreDerived>().ToTable(nameof(Required2MoreDerived));
modelBuilder.Entity<Optional1>().ToTable(nameof(Optional1));
modelBuilder.Entity<Optional1Derived>().ToTable(nameof(Optional1Derived));
modelBuilder.Entity<Optional1MoreDerived>().ToTable(nameof(Optional1MoreDerived));
modelBuilder.Entity<Optional2Derived>().ToTable(nameof(Optional2Derived));
modelBuilder.Entity<Optional2MoreDerived>().ToTable(nameof(Optional2MoreDerived));
modelBuilder.Entity<RequiredSingle1>().ToTable(nameof(RequiredSingle1));
modelBuilder.Entity<OptionalSingle1>().ToTable(nameof(OptionalSingle1));
modelBuilder.Entity<OptionalSingle2>().ToTable(nameof(OptionalSingle2));
modelBuilder.Entity<RequiredNonPkSingle1>().ToTable(nameof(RequiredNonPkSingle1));
modelBuilder.Entity<RequiredNonPkSingle2Derived>().ToTable(nameof(RequiredNonPkSingle2Derived));
modelBuilder.Entity<RequiredNonPkSingle2MoreDerived>().ToTable(nameof(RequiredNonPkSingle2MoreDerived));
modelBuilder.Entity<RequiredAk1>().ToTable(nameof(RequiredAk1));
modelBuilder.Entity<RequiredAk1Derived>().ToTable(nameof(RequiredAk1Derived));
modelBuilder.Entity<RequiredAk1MoreDerived>().ToTable(nameof(RequiredAk1MoreDerived));
modelBuilder.Entity<OptionalAk1>().ToTable(nameof(OptionalAk1));
modelBuilder.Entity<OptionalAk1Derived>().ToTable(nameof(OptionalAk1Derived));
modelBuilder.Entity<OptionalAk1MoreDerived>().ToTable(nameof(OptionalAk1MoreDerived));
modelBuilder.Entity<RequiredSingleAk1>().ToTable(nameof(RequiredSingleAk1));
modelBuilder.Entity<OptionalSingleAk1>().ToTable(nameof(OptionalSingleAk1));
modelBuilder.Entity<OptionalSingleAk2Derived>().ToTable(nameof(OptionalSingleAk2Derived));
modelBuilder.Entity<OptionalSingleAk2MoreDerived>().ToTable(nameof(OptionalSingleAk2MoreDerived));
modelBuilder.Entity<RequiredNonPkSingleAk1>().ToTable(nameof(RequiredNonPkSingleAk1));
modelBuilder.Entity<RequiredAk2>().ToTable(nameof(RequiredAk2));
modelBuilder.Entity<RequiredAk2Derived>().ToTable(nameof(RequiredAk2Derived));
modelBuilder.Entity<RequiredAk2MoreDerived>().ToTable(nameof(RequiredAk2MoreDerived));
modelBuilder.Entity<OptionalAk2>().ToTable(nameof(OptionalAk2));
modelBuilder.Entity<OptionalAk2Derived>().ToTable(nameof(OptionalAk2Derived));
modelBuilder.Entity<OptionalAk2MoreDerived>().ToTable(nameof(OptionalAk2MoreDerived));
modelBuilder.Entity<RequiredSingleAk2>().ToTable(nameof(RequiredSingleAk2));
modelBuilder.Entity<RequiredNonPkSingleAk2>().ToTable(nameof(RequiredNonPkSingleAk2));
modelBuilder.Entity<RequiredNonPkSingleAk2Derived>().ToTable(nameof(RequiredNonPkSingleAk2Derived));
modelBuilder.Entity<RequiredNonPkSingleAk2MoreDerived>().ToTable(nameof(RequiredNonPkSingleAk2MoreDerived));
modelBuilder.Entity<OptionalSingleAk2>().ToTable(nameof(OptionalSingleAk2));
modelBuilder.Entity<RequiredComposite1>().ToTable(nameof(RequiredComposite1));
modelBuilder.Entity<OptionalOverlapping2>().ToTable(nameof(OptionalOverlapping2));
modelBuilder.Entity<BadCustomer>().ToTable(nameof(BadCustomer));
modelBuilder.Entity<BadOrder>().ToTable(nameof(BadOrder));
modelBuilder.Entity<QuestTask>().ToTable(nameof(QuestTask));
modelBuilder.Entity<QuizTask>().ToTable(nameof(QuizTask));
modelBuilder.Entity<HiddenAreaTask>().ToTable(nameof(HiddenAreaTask));
modelBuilder.Entity<TaskChoice>().ToTable(nameof(TaskChoice));
modelBuilder.Entity<ParentAsAChild>().ToTable(nameof(ParentAsAChild));
modelBuilder.Entity<ChildAsAParent>().ToTable(nameof(ChildAsAParent));
modelBuilder.Entity<Poost>().ToTable(nameof(Poost));
modelBuilder.Entity<Bloog>().ToTable(nameof(Bloog));
modelBuilder.Entity<Produce>().ToTable(nameof(Produce));
}
}
}

public class Identity : GraphUpdatesSqlServerTestBase<Identity.SqlServerFixture>
{
public Identity(SqlServerFixture fixture)
Expand Down

0 comments on commit 46a6e9b

Please sign in to comment.