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 temp values for key properties contained in optional FKs #27579

Merged
merged 1 commit into from
Mar 8, 2022
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
6 changes: 5 additions & 1 deletion src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1459,6 +1459,9 @@ public void AcceptChanges()
}
}

private readonly static bool _useOldBehavior27455 =
AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue27455", out var enabled27455) && enabled27455;

/// <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 All @@ -1485,7 +1488,8 @@ public InternalEntityEntry PrepareToSave()

if (property.IsKey()
&& property.IsForeignKey()
&& _stateData.IsPropertyFlagged(property.GetIndex(), PropertyFlag.Unknown))
&& _stateData.IsPropertyFlagged(property.GetIndex(), PropertyFlag.Unknown)
&& (_useOldBehavior27455 || !IsStoreGenerated(property)))
{
throw new InvalidOperationException(CoreStrings.UnknownKeyValue(entityType.DisplayName(), property.Name));
}
Expand Down
10 changes: 9 additions & 1 deletion src/EFCore/Metadata/Internal/PropertyExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Utilities;

Expand Down Expand Up @@ -74,6 +76,9 @@ public static bool ForUpdate(this ValueGenerated valueGenerated)
return null;
}

private readonly static bool _useOldBehavior27455 =
AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue27455", out var enabled27455) && enabled27455;

/// <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 All @@ -83,7 +88,10 @@ public static bool ForUpdate(this ValueGenerated valueGenerated)
public static bool RequiresValueGenerator(this IReadOnlyProperty property)
=> (property.ValueGenerated.ForAdd()
&& property.IsKey()
&& (!property.IsForeignKey() || property.IsForeignKeyToSelf()))
&& (!property.IsForeignKey()
|| property.IsForeignKeyToSelf()
|| (!_useOldBehavior27455
&& property.GetContainingForeignKeys().All(fk => fk.Properties.Any(p => p != property && p.IsNullable)))))
|| property.GetValueGeneratorFactory() != null;

/// <summary>
Expand Down
67 changes: 63 additions & 4 deletions test/EFCore.Specification.Tests/StoreGeneratedTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,45 @@ public virtual void Identity_property_on_Added_entity_with_temporary_value_gets_
context => Assert.Equal("Banana Joe", context.Set<Gumball>().Single(e => e.Id == id).Identity));
}

#nullable enable
protected class CompositePrincipal
{
public int Id { get; set; }
public int? CurrentNumber { get; set; }
public CompositeDependent? Current { get; set; }
public ICollection<CompositeDependent> Periods { get; } = new HashSet<CompositeDependent>();
}

protected class CompositeDependent
{
public int PrincipalId { get; set; }
public int Number { get; set; }
public CompositePrincipal? Principal { get; set; }
}
#nullable disable

[ConditionalFact]
public virtual void Store_generated_values_are_propagated_with_composite_key_cycles()
{
var id = 0;

ExecuteWithStrategyInTransaction(
context =>
{
var period = new CompositeDependent
{
Number = 1,
Principal = new CompositePrincipal()
};

context.Add(period);
context.SaveChanges();

id = period.PrincipalId;
},
context => Assert.Equal(1, context.Set<CompositeDependent>().Single(e => e.PrincipalId == id).Number));
}

protected class NonStoreGenDependent
{
[DatabaseGenerated(DatabaseGeneratedOption.None)]
Expand Down Expand Up @@ -1947,10 +1986,30 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
modelBuilder.Entity<OptionalProduct>();
modelBuilder.Entity<StoreGenPrincipal>();

modelBuilder.Entity<NonStoreGenDependent>()
.Property(e => e.HasTemp)
.ValueGeneratedOnAddOrUpdate()
.HasValueGenerator<TemporaryIntValueGenerator>();
modelBuilder.Entity<NonStoreGenDependent>(eb =>
{
eb.Property(e => e.HasTemp)
.ValueGeneratedOnAddOrUpdate()
.HasValueGenerator<TemporaryIntValueGenerator>();
});

modelBuilder.Entity<CompositePrincipal>(entity =>
{
entity.HasKey(x => x.Id);
entity.Property(x => x.Id)
.ValueGeneratedOnAdd();
entity.HasOne(x => x.Current)
.WithOne()
.HasForeignKey<CompositePrincipal>(x => new { x.Id, x.CurrentNumber });
});

modelBuilder.Entity<CompositeDependent>(entity =>
{
entity.HasKey(x => new { x.PrincipalId, x.Number });
entity.HasOne(x => x.Principal)
.WithMany(x => x.Periods)
.HasForeignKey(x => x.PrincipalId);
});
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con

modelBuilder.Entity<NonStoreGenDependent>().Property(e => e.HasTemp).HasDefaultValue(777);

modelBuilder.Entity<CompositePrincipal>().Property(e => e.Id).UseIdentityColumn();

base.OnModelCreating(modelBuilder, context);
}
}
Expand Down