Skip to content

Commit

Permalink
Allow database default values for FK properties that are part of a co…
Browse files Browse the repository at this point in the history
…mposite key (#29047)
  • Loading branch information
ajcvickers authored Sep 12, 2022
1 parent 1362d96 commit 307b818
Show file tree
Hide file tree
Showing 19 changed files with 501 additions and 108 deletions.
19 changes: 16 additions & 3 deletions src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,7 @@ private static void ValidateSproc(IStoredProcedure sproc, string mappingStrategy
entityType.DisplayName(),
property.GetBeforeSaveBehavior()));
}

break;

case StoreObjectType.UpdateStoredProcedure:
Expand Down Expand Up @@ -708,19 +709,31 @@ protected virtual void ValidateDefaultValuesOnKeys(
{
foreach (var key in entityType.GetDeclaredKeys())
{
IProperty? propertyWithDefault = null;
foreach (var property in key.Properties)
{
var defaultValue = (IConventionAnnotation?)property.FindAnnotation(RelationalAnnotationNames.DefaultValue);
if (defaultValue?.Value != null
if (!property.IsForeignKey()
&& defaultValue?.Value != null
&& defaultValue.GetConfigurationSource().Overrides(ConfigurationSource.DataAnnotation))
{
logger.ModelValidationKeyDefaultValueWarning(property);
propertyWithDefault ??= property;
}
else
{
propertyWithDefault = null;
break;
}
}

if (propertyWithDefault != null)
{
logger.ModelValidationKeyDefaultValueWarning(propertyWithDefault);
}
}
}
}

/// <summary>
/// Validates the mapping/configuration of mutable in the model.
/// </summary>
Expand Down
8 changes: 8 additions & 0 deletions src/EFCore/ChangeTracking/Internal/IValueGenerationManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ public interface IValueGenerationManager
/// </summary>
InternalEntityEntry? Propagate(InternalEntityEntry entry);

/// <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>
Task<InternalEntityEntry?> PropagateAsync(InternalEntityEntry entry, CancellationToken cancellationToken);

/// <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
58 changes: 41 additions & 17 deletions src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -186,23 +186,24 @@ public async Task SetEntityStateAsync(
{
var oldState = _stateData.EntityState;
bool adding;
Setup();
await SetupAsync().ConfigureAwait(false);

if ((adding || oldState is EntityState.Detached)
&& await StateManager.ValueGenerationManager
.GenerateAsync(this, includePrimaryKey: adding, cancellationToken).ConfigureAwait(false)
&& fallbackState.HasValue)
{
entityState = fallbackState.Value;
Setup();
await SetupAsync().ConfigureAwait(false);
}

SetEntityState(oldState, entityState, acceptChanges, modifyProperties);

void Setup()
async Task SetupAsync()
{
adding = PrepareForAdd(entityState);
entityState = PropagateToUnknownKey(oldState, entityState, adding, forceStateWhenUnknownKey);
entityState = await PropagateToUnknownKeyAsync(
oldState, entityState, adding, forceStateWhenUnknownKey, cancellationToken).ConfigureAwait(false);
}
}

Expand All @@ -214,27 +215,50 @@ private EntityState PropagateToUnknownKey(
{
var keyUnknown = IsKeyUnknown;

if (adding
|| (oldState == EntityState.Detached
&& keyUnknown))
if (adding || (oldState == EntityState.Detached && keyUnknown))
{
var principalEntry = StateManager.ValueGenerationManager.Propagate(this);

if (forceStateWhenUnknownKey.HasValue
&& keyUnknown
&& principalEntry != null
&& principalEntry.EntityState != EntityState.Detached
&& principalEntry.EntityState != EntityState.Deleted)
{
entityState = principalEntry.EntityState == EntityState.Added
? EntityState.Added
: forceStateWhenUnknownKey.Value;
}
entityState = ForceState(entityState, forceStateWhenUnknownKey, keyUnknown, principalEntry);
}

return entityState;
}

private async Task<EntityState> PropagateToUnknownKeyAsync(
EntityState oldState,
EntityState entityState,
bool adding,
EntityState? forceStateWhenUnknownKey,
CancellationToken cancellationToken)
{
var keyUnknown = IsKeyUnknown;

if (adding || (oldState == EntityState.Detached && keyUnknown))
{
var principalEntry = await StateManager.ValueGenerationManager.PropagateAsync(this, cancellationToken).ConfigureAwait(false);

entityState = ForceState(entityState, forceStateWhenUnknownKey, keyUnknown, principalEntry);
}

return entityState;
}

private static EntityState ForceState(
EntityState entityState,
EntityState? forceStateWhenUnknownKey,
bool keyUnknown,
InternalEntityEntry? principalEntry)
=> forceStateWhenUnknownKey.HasValue
&& keyUnknown
&& principalEntry != null
&& principalEntry.EntityState != EntityState.Detached
&& principalEntry.EntityState != EntityState.Deleted
? principalEntry.EntityState == EntityState.Added
? EntityState.Added
: forceStateWhenUnknownKey.Value
: entityState;

private bool PrepareForAdd(EntityState newState)
{
if (newState != EntityState.Added
Expand Down
50 changes: 19 additions & 31 deletions src/EFCore/ChangeTracking/Internal/KeyPropagator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,7 @@ public KeyPropagator(

if (valueGenerator != null)
{
var value = valueGenerator.Next(new EntityEntry(entry));

if (valueGenerator.GeneratesTemporaryValues)
{
entry.SetTemporaryValue(property, value);
}
else
{
entry[property] = value;
}

if (!valueGenerator.GeneratesStableValues)
{
entry.MarkUnknown(property);
}
SetValue(entry, property, valueGenerator, valueGenerator.Next(new EntityEntry(entry)));
}
}

Expand Down Expand Up @@ -101,28 +87,30 @@ public KeyPropagator(

if (valueGenerator != null)
{
var value = await valueGenerator.NextAsync(new EntityEntry(entry), cancellationToken)
.ConfigureAwait(false);

if (valueGenerator.GeneratesTemporaryValues)
{
entry.SetTemporaryValue(property, value);
}
else
{
entry[property] = value;
}

if (!valueGenerator.GeneratesStableValues)
{
entry.MarkUnknown(property);
}
SetValue(
entry,
property,
valueGenerator,
await valueGenerator.NextAsync(new EntityEntry(entry), cancellationToken).ConfigureAwait(false));
}
}

return principalEntry;
}

private static void SetValue(InternalEntityEntry entry, IProperty property, ValueGenerator valueGenerator, object? value)
{
if (valueGenerator.GeneratesStableValues)
{
entry[property] = value;
}
else
{
entry.SetTemporaryValue(property, value);
entry.MarkUnknown(property);
}
}

private static InternalEntityEntry? TryPropagateValue(InternalEntityEntry entry, IProperty property, IProperty? generationProperty)
{
var entityType = entry.EntityType;
Expand Down
23 changes: 23 additions & 0 deletions src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,29 @@ public ValueGenerationManager(
return chosenPrincipal;
}

/// <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 async Task<InternalEntityEntry?> PropagateAsync(InternalEntityEntry entry, CancellationToken cancellationToken)
{
InternalEntityEntry? chosenPrincipal = null;
foreach (var property in entry.EntityType.GetForeignKeyProperties())
{
if (!entry.HasDefaultValue(property))
{
continue;
}

var principalEntry = await _keyPropagator.PropagateValueAsync(entry, property, cancellationToken).ConfigureAwait(false);
chosenPrincipal ??= principalEntry;
}

return chosenPrincipal;
}

/// <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
6 changes: 3 additions & 3 deletions src/EFCore/Metadata/Internal/PropertyExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,13 @@ public static bool IsKey(this Property property)
/// </summary>
public static bool MayBeStoreGenerated(this IProperty property)
{
if (property.ValueGenerated != ValueGenerated.Never)
if (property.ValueGenerated != ValueGenerated.Never
|| property.IsForeignKey())
{
return true;
}

if (property.IsKey()
|| property.IsForeignKey())
if (property.IsKey())
{
var generationProperty = property.FindGenerationProperty();
return (generationProperty != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
p => p.Value,
p => new Key(p),
new ValueComparer<Key>(
(l, r) => l.Value == r.Value,
v => v.Value.GetHashCode()));
(l, r) => (l == null && r == null) || (l != null && r != null && l.Value == r.Value),
v => v == null ? 0 : v.Value.GetHashCode()));
entity.OwnsOne(p => p.Text);
entity.Navigation(p => p.Text).IsRequired();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ public GraphUpdatesInMemoryTest(InMemoryFixture fixture)
{
}

// In-memory database does not have database default values
public override Task Can_insert_when_composite_FK_has_default_value_for_one_part(bool async)
=> Task.CompletedTask;

// In-memory database does not have database default values
public override Task Can_insert_when_FK_has_default_value(bool async)
=> Task.CompletedTask;
Expand Down
8 changes: 6 additions & 2 deletions test/EFCore.Specification.Tests/CustomConvertersTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -845,11 +845,15 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
{
b.HasMany(e => e.Dependents).WithOne(e => e.Principal).HasForeignKey(e => e.PrincipalId);
b.Property(e => e.Id).ValueGeneratedNever();
b.Property(e => e.Id).HasConversion(v => v, v => (int)v);
b.Property(e => e.Id).HasConversion<int>(v => v ?? 0, v => v);
});

modelBuilder.Entity<NonNullableDependent>(
b => b.Property(e => e.Id).ValueGeneratedNever());
b =>
{
b.Property(e => e.Id).ValueGeneratedNever();
b.Property(e => e.PrincipalId).HasConversion<int>(v => v, v => v);
});

modelBuilder.Entity<User>(
b =>
Expand Down
19 changes: 19 additions & 0 deletions test/EFCore.Specification.Tests/DataAnnotationTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,25 @@ protected class Profile1
public virtual Login1 User { get; set; }
}

protected class PrincipalA
{
public int Id { get; set; }
public DependantA Dependant { get; set; }
}

protected class DependantA
{
public int Id { get; set; }
public int PrincipalId { get; set; }
public PrincipalA Principal { get; set; }
}

protected class PrincipalB
{
public int Id1 { get; set; }
public int Id2 { get; set; }
}

[ConditionalFact]
public virtual IModel Key_and_column_work_together()
{
Expand Down
Loading

0 comments on commit 307b818

Please sign in to comment.