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

Allow database default values for FK properties that are part of a composite key #29047

Merged
merged 2 commits into from
Sep 12, 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
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