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

Generate values for PK properties that are also self-referencing FK properties #22585

Merged
merged 1 commit into from
Sep 18, 2020
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
27 changes: 16 additions & 11 deletions src/EFCore/ChangeTracking/Internal/KeyPropagator.cs
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
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())
ajcvickers marked this conversation as resolved.
Show resolved Hide resolved
{
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
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