From 307b818c03a2addfb788f14e30eb4cf9fc6a6dc4 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Mon, 12 Sep 2022 12:20:35 +0100 Subject: [PATCH] Allow database default values for FK properties that are part of a composite key (#29047) --- .../RelationalModelValidator.cs | 19 +- .../Internal/IValueGenerationManager.cs | 8 + .../Internal/InternalEntityEntry.cs | 58 ++++-- .../ChangeTracking/Internal/KeyPropagator.cs | 50 ++--- .../Internal/ValueGenerationManager.cs | 23 +++ .../Metadata/Internal/PropertyExtensions.cs | 6 +- .../KeysWithConvertersCosmosTest.cs | 4 +- .../GraphUpdates/GraphUpdatesInMemoryTest.cs | 4 + .../CustomConvertersTestBase.cs | 8 +- .../DataAnnotationTestBase.cs | 19 ++ .../GraphUpdates/GraphUpdatesTestBase.cs | 186 ++++++++++++++++++ .../GraphUpdatesTestBaseMiscellaneous.cs | 42 ++++ .../KeysWithConvertersTestBase.cs | 10 +- .../DataAnnotationSqlServerTest.cs | 53 +++++ .../GraphUpdatesSqlServerOwnedTest.cs | 4 + .../GraphUpdatesSqlServerTestBase.cs | 3 + .../GraphUpdatesSqliteTestBase.cs | 3 + .../Internal/InternalClrEntityEntryTest.cs | 2 + .../Internal/KeyPropagatorTest.cs | 107 +++++----- 19 files changed, 501 insertions(+), 108 deletions(-) diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs index 5bc046911d5..6fca7d36af0 100644 --- a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs +++ b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs @@ -475,6 +475,7 @@ private static void ValidateSproc(IStoredProcedure sproc, string mappingStrategy entityType.DisplayName(), property.GetBeforeSaveBehavior())); } + break; case StoreObjectType.UpdateStoredProcedure: @@ -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); + } } } } - + /// /// Validates the mapping/configuration of mutable in the model. /// diff --git a/src/EFCore/ChangeTracking/Internal/IValueGenerationManager.cs b/src/EFCore/ChangeTracking/Internal/IValueGenerationManager.cs index deacc268997..8a36639d79d 100644 --- a/src/EFCore/ChangeTracking/Internal/IValueGenerationManager.cs +++ b/src/EFCore/ChangeTracking/Internal/IValueGenerationManager.cs @@ -33,6 +33,14 @@ public interface IValueGenerationManager /// InternalEntityEntry? Propagate(InternalEntityEntry entry); + /// + /// 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. + /// + Task PropagateAsync(InternalEntityEntry entry, CancellationToken cancellationToken); + /// /// 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 diff --git a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs index 8fee8e0d56c..666fce4cce8 100644 --- a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs +++ b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs @@ -186,7 +186,7 @@ public async Task SetEntityStateAsync( { var oldState = _stateData.EntityState; bool adding; - Setup(); + await SetupAsync().ConfigureAwait(false); if ((adding || oldState is EntityState.Detached) && await StateManager.ValueGenerationManager @@ -194,15 +194,16 @@ public async Task SetEntityStateAsync( && 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); } } @@ -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 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 diff --git a/src/EFCore/ChangeTracking/Internal/KeyPropagator.cs b/src/EFCore/ChangeTracking/Internal/KeyPropagator.cs index 3ff94eb7c17..211b55136a8 100644 --- a/src/EFCore/ChangeTracking/Internal/KeyPropagator.cs +++ b/src/EFCore/ChangeTracking/Internal/KeyPropagator.cs @@ -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))); } } @@ -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; diff --git a/src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs b/src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs index b7684119a74..91d60fbbac4 100644 --- a/src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs +++ b/src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs @@ -57,6 +57,29 @@ public ValueGenerationManager( return chosenPrincipal; } + /// + /// 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. + /// + public virtual async Task 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; + } + /// /// 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 diff --git a/src/EFCore/Metadata/Internal/PropertyExtensions.cs b/src/EFCore/Metadata/Internal/PropertyExtensions.cs index 2d48fdecffd..f0903ede3de 100644 --- a/src/EFCore/Metadata/Internal/PropertyExtensions.cs +++ b/src/EFCore/Metadata/Internal/PropertyExtensions.cs @@ -136,13 +136,13 @@ public static bool IsKey(this Property property) /// 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) diff --git a/test/EFCore.Cosmos.FunctionalTests/KeysWithConvertersCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/KeysWithConvertersCosmosTest.cs index 0e930247018..598db5758be 100644 --- a/test/EFCore.Cosmos.FunctionalTests/KeysWithConvertersCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/KeysWithConvertersCosmosTest.cs @@ -355,8 +355,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con p => p.Value, p => new Key(p), new ValueComparer( - (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(); diff --git a/test/EFCore.InMemory.FunctionalTests/GraphUpdates/GraphUpdatesInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/GraphUpdates/GraphUpdatesInMemoryTest.cs index 30a78affcad..b23e13f5865 100644 --- a/test/EFCore.InMemory.FunctionalTests/GraphUpdates/GraphUpdatesInMemoryTest.cs +++ b/test/EFCore.InMemory.FunctionalTests/GraphUpdates/GraphUpdatesInMemoryTest.cs @@ -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; diff --git a/test/EFCore.Specification.Tests/CustomConvertersTestBase.cs b/test/EFCore.Specification.Tests/CustomConvertersTestBase.cs index 9c352ec1c36..5b12eee6f41 100644 --- a/test/EFCore.Specification.Tests/CustomConvertersTestBase.cs +++ b/test/EFCore.Specification.Tests/CustomConvertersTestBase.cs @@ -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(v => v ?? 0, v => v); }); modelBuilder.Entity( - b => b.Property(e => e.Id).ValueGeneratedNever()); + b => + { + b.Property(e => e.Id).ValueGeneratedNever(); + b.Property(e => e.PrincipalId).HasConversion(v => v, v => v); + }); modelBuilder.Entity( b => diff --git a/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs b/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs index 30e19e30d12..a11c86dfcf8 100644 --- a/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs +++ b/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs @@ -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() { diff --git a/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBase.cs b/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBase.cs index d920cd2b270..8519cfe7955 100644 --- a/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBase.cs +++ b/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBase.cs @@ -479,6 +479,64 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con }); modelBuilder.Entity(); + + modelBuilder.Entity().HasData( + new SomethingCategory + { + Id = 1, + Name = "A" + }, + new SomethingCategory + { + Id = 2, + Name = "B" + }, + new SomethingCategory + { + Id = 3, + Name = "C" + }); + + modelBuilder.Entity().HasOne(s => s.SomethingCategory) + .WithMany() + .HasForeignKey(s => s.CategoryId) + .OnDelete(DeleteBehavior.ClientSetNull); + + modelBuilder.Entity(builder => + { + builder.Property("CategoryId").IsRequired(); + + builder.HasKey(nameof(SomethingOfCategoryA.SomethingId), "CategoryId"); + + builder.HasOne(d => d.Something) + .WithOne(p => p.SomethingOfCategoryA) + .HasPrincipalKey(p => new {p.Id, p.CategoryId}) + .HasForeignKey(nameof(SomethingOfCategoryA.SomethingId), "CategoryId") + .OnDelete(DeleteBehavior.ClientSetNull); + + builder.HasOne() + .WithMany() + .HasForeignKey("CategoryId") + .OnDelete(DeleteBehavior.ClientSetNull); + }); + + modelBuilder.Entity(builder => + { + builder.Property(e => e.CategoryId).IsRequired(); + + builder.HasKey(e => new {e.SomethingId, e.CategoryId}); + + builder.HasOne(d => d.Something) + .WithOne(p => p.SomethingOfCategoryB) + .HasPrincipalKey(p => new {p.Id, p.CategoryId}) + .HasForeignKey(socb => new {socb.SomethingId, socb.CategoryId}) + .OnDelete(DeleteBehavior.ClientSetNull); + + builder.HasOne(e => e.SomethingCategory) + .WithMany() + .HasForeignKey(e => e.CategoryId) + .OnDelete(DeleteBehavior.ClientSetNull); + }); } protected virtual object CreateFullGraph() @@ -3500,6 +3558,134 @@ public virtual ICollection Users } } + protected class SomethingCategory : NotifyingEntity + { + private int _id; + private string _name; + + public int Id + { + get => _id; + set => SetWithNotify(value, ref _id); + } + + public string Name + { + get => _name; + set => SetWithNotify(value, ref _name); + } + } + + protected class Something : NotifyingEntity + { + private int _id; + private int _categoryId; + private string _name; + private SomethingCategory _somethingCategory; + private SomethingOfCategoryA _somethingOfCategoryA; + private SomethingOfCategoryB _somethingOfCategoryB; + + public int Id + { + get => _id; + set => SetWithNotify(value, ref _id); + } + + public int CategoryId + { + get => _categoryId; + set => SetWithNotify(value, ref _categoryId); + } + + public string Name + { + get => _name; + set => SetWithNotify(value, ref _name); + } + + public virtual SomethingCategory SomethingCategory + { + get => _somethingCategory; + set => SetWithNotify(value, ref _somethingCategory); + } + + public virtual SomethingOfCategoryA SomethingOfCategoryA + { + get => _somethingOfCategoryA; + set => SetWithNotify(value, ref _somethingOfCategoryA); + } + + public virtual SomethingOfCategoryB SomethingOfCategoryB + { + get => _somethingOfCategoryB; + set => SetWithNotify(value, ref _somethingOfCategoryB); + } + } + + protected class SomethingOfCategoryA : NotifyingEntity + { + private int _somethingId; + private string _name; + private Something _something; + + public int SomethingId + { + get => _somethingId; + set => SetWithNotify(value, ref _somethingId); + } + + public string Name + { + get => _name; + set => SetWithNotify(value, ref _name); + } + + public virtual Something Something + { + get => _something; + set => SetWithNotify(value, ref _something); + } + } + + protected class SomethingOfCategoryB : NotifyingEntity + { + private int _somethingId; + private int _categoryId; + private string _name; + private SomethingCategory _somethingCategory; + private Something _something; + + public int SomethingId + { + get => _somethingId; + set => SetWithNotify(value, ref _somethingId); + } + + public int CategoryId + { + get => _categoryId; + set => SetWithNotify(value, ref _categoryId); + } + + public string Name + { + get => _name; + set => SetWithNotify(value, ref _name); + } + + public virtual SomethingCategory SomethingCategory + { + get => _somethingCategory; + set => SetWithNotify(value, ref _somethingCategory); + } + + public virtual Something Something + { + get => _something; + set => SetWithNotify(value, ref _something); + } + } + protected class NotifyingEntity : INotifyPropertyChanging, INotifyPropertyChanged { protected void SetWithNotify(T value, ref T field, [CallerMemberName] string propertyName = "") diff --git a/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBaseMiscellaneous.cs b/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBaseMiscellaneous.cs index 81be5c768e3..621b7bde6d0 100644 --- a/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBaseMiscellaneous.cs +++ b/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBaseMiscellaneous.cs @@ -11,6 +11,48 @@ namespace Microsoft.EntityFrameworkCore; public abstract partial class GraphUpdatesTestBase where TFixture : GraphUpdatesTestBase.GraphUpdatesFixtureBase, new() { + [ConditionalTheory] // Issue #27299 + [InlineData(false)] + [InlineData(true)] + public virtual async Task Can_insert_when_composite_FK_has_default_value_for_one_part(bool async) + => await ExecuteWithStrategyInTransactionAsync( + async context => + { + var newSomething = new Something { CategoryId = 2, Name = "S" }; + + if (async) + { + await context.AddAsync(newSomething); + await context.SaveChangesAsync(); + } + else + { + context.Add(newSomething); + context.SaveChanges(); + } + + var somethingOfCategoryB = new SomethingOfCategoryB { SomethingId = newSomething.Id, Name = "B" }; + + if (async) + { + await context.AddAsync(somethingOfCategoryB); + await context.SaveChangesAsync(); + } + else + { + context.Add(somethingOfCategoryB); + context.SaveChanges(); + } + }, + async context => + { + var queryable = context.Set().Include(e => e.SomethingOfCategoryB); + var something = async ? (await queryable.SingleAsync()) : queryable.Single(); + + Assert.Equal("S", something.Name); + Assert.Equal("B", something.SomethingOfCategoryB.Name); + }); + [ConditionalTheory] // Issue #23974 [InlineData(false)] [InlineData(true)] diff --git a/test/EFCore.Specification.Tests/KeysWithConvertersTestBase.cs b/test/EFCore.Specification.Tests/KeysWithConvertersTestBase.cs index af732d5343c..596ac22c933 100644 --- a/test/EFCore.Specification.Tests/KeysWithConvertersTestBase.cs +++ b/test/EFCore.Specification.Tests/KeysWithConvertersTestBase.cs @@ -5588,9 +5588,9 @@ public static ValueConverter Converter public static ValueComparer Comparer = new( - (l, r) => l.Id == r.Id, - v => v.Id.GetHashCode(), - v => new BareIntClassKey(v.Id)); + (l, r) => (l == null && r == null) || (l != null && r != null && l.Id == r.Id), + v => v == null ? 0 : v.Id.GetHashCode(), + v => v == null ? null : new BareIntClassKey(v.Id)); public BareIntClassKey(int id) { @@ -7077,8 +7077,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con p => p.Value, p => new Key(p), new ValueComparer( - (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(); diff --git a/test/EFCore.SqlServer.FunctionalTests/DataAnnotationSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/DataAnnotationSqlServerTest.cs index 7c511785218..38b2141100c 100644 --- a/test/EFCore.SqlServer.FunctionalTests/DataAnnotationSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/DataAnnotationSqlServerTest.cs @@ -40,6 +40,59 @@ public virtual void Default_for_key_string_column_throws() Assert.Throws(() => Validate(modelBuilder)).Message); } + [ConditionalFact] + public virtual void Default_for_key_which_is_also_an_fk_column_does_not_throw() + { + var modelBuilder = CreateModelBuilder(); + + modelBuilder.Entity(); + modelBuilder.Entity( + b => + { + b.HasKey(e => new { e.Id, e.PrincipalId }); + b.Property(e => e.PrincipalId).HasDefaultValue(77); + }); + + Validate(modelBuilder); + } + + [ConditionalFact] + public virtual void Default_for_part_of_composite_key_does_not_throw() + { + var modelBuilder = CreateModelBuilder(); + + modelBuilder.Entity( + b => + { + b.HasKey(e => new { e.Id1, e.Id2 }); + b.Property(e => e.Id1).HasDefaultValue(77); + }); + + Validate(modelBuilder); + } + + [ConditionalFact] + public virtual void Default_for_all_parts_of_composite_key_throws() + { + var modelBuilder = CreateModelBuilder(); + + modelBuilder.Entity( + b => + { + b.HasKey(e => new { e.Id1, e.Id2 }); + b.Property(e => e.Id1).HasDefaultValue(77); + b.Property(e => e.Id2).HasDefaultValue(78); + }); + + Assert.Equal( + CoreStrings.WarningAsErrorTemplate( + RelationalEventId.ModelValidationKeyDefaultValueWarning, + RelationalResources.LogKeyHasDefaultValue(new TestLogger()) + .GenerateMessage(nameof(PrincipalB.Id1), nameof(PrincipalB)), + "RelationalEventId.ModelValidationKeyDefaultValueWarning"), + Assert.Throws(() => Validate(modelBuilder)).Message); + } + public override IModel Non_public_annotations_are_enabled() { var model = base.Non_public_annotations_are_enabled(); diff --git a/test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerOwnedTest.cs b/test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerOwnedTest.cs index d89830b5f92..e6df4ea04e3 100644 --- a/test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerOwnedTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerOwnedTest.cs @@ -23,6 +23,10 @@ public override void Required_one_to_one_with_alternate_key_are_cascade_deleted_ { } + // No owned types + public override Task Can_insert_when_composite_FK_has_default_value_for_one_part(bool async) + => Task.CompletedTask; + public override void Required_one_to_one_relationships_are_one_to_one(CascadeTiming? deleteOrphansTiming) { } diff --git a/test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerTestBase.cs b/test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerTestBase.cs index 394ca47e615..53c34f00557 100644 --- a/test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerTestBase.cs +++ b/test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerTestBase.cs @@ -42,6 +42,9 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con b.Property(e => e.IdUserState).HasDefaultValue(1); b.HasOne(e => e.UserState).WithMany(e => e.Users).HasForeignKey(e => e.IdUserState); }); + + modelBuilder.Entity().Property("CategoryId").HasDefaultValue(1); + modelBuilder.Entity().Property(e => e.CategoryId).HasDefaultValue(2);; } } } diff --git a/test/EFCore.Sqlite.FunctionalTests/GraphUpdates/GraphUpdatesSqliteTestBase.cs b/test/EFCore.Sqlite.FunctionalTests/GraphUpdates/GraphUpdatesSqliteTestBase.cs index 6d17df8ae33..0ff1fa938e4 100644 --- a/test/EFCore.Sqlite.FunctionalTests/GraphUpdates/GraphUpdatesSqliteTestBase.cs +++ b/test/EFCore.Sqlite.FunctionalTests/GraphUpdates/GraphUpdatesSqliteTestBase.cs @@ -80,6 +80,9 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con b.Property(e => e.IdUserState).HasDefaultValue(1); b.HasOne(e => e.UserState).WithMany(e => e.Users).HasForeignKey(e => e.IdUserState); }); + + modelBuilder.Entity().Property("CategoryId").HasDefaultValue(1); + modelBuilder.Entity().Property(e => e.CategoryId).HasDefaultValue(2);; } } } diff --git a/test/EFCore.Tests/ChangeTracking/Internal/InternalClrEntityEntryTest.cs b/test/EFCore.Tests/ChangeTracking/Internal/InternalClrEntityEntryTest.cs index 8763857b5ce..a672ef0a307 100644 --- a/test/EFCore.Tests/ChangeTracking/Internal/InternalClrEntityEntryTest.cs +++ b/test/EFCore.Tests/ChangeTracking/Internal/InternalClrEntityEntryTest.cs @@ -108,6 +108,8 @@ public void AcceptChanges_handles_different_entity_states_for_owned_types(Entity var ownerEntry = context.Entry( new OwnerClass { Id = 1, Owned = new OwnedClass { Value = "Kool" } }).GetInfrastructure(); + ownerEntry.SetEntityState(EntityState.Unchanged); + var entry = context.Entry(((OwnerClass)ownerEntry.Entity).Owned).GetInfrastructure(); var valueProperty = entry.EntityType.FindProperty(nameof(OwnedClass.Value)); diff --git a/test/EFCore.Tests/ChangeTracking/Internal/KeyPropagatorTest.cs b/test/EFCore.Tests/ChangeTracking/Internal/KeyPropagatorTest.cs index d9ffd383c7e..3c1a49f959b 100644 --- a/test/EFCore.Tests/ChangeTracking/Internal/KeyPropagatorTest.cs +++ b/test/EFCore.Tests/ChangeTracking/Internal/KeyPropagatorTest.cs @@ -13,7 +13,7 @@ public class KeyPropagatorTest [InlineData(false, true)] [InlineData(true, false)] [InlineData(true, true)] - public void Foreign_key_value_is_obtained_from_reference_to_principal(bool generateTemporary, bool async) + public async Task Foreign_key_value_is_obtained_from_reference_to_principal(bool generateTemporary, bool async) { var model = BuildModel(generateTemporary); @@ -26,7 +26,14 @@ public void Foreign_key_value_is_obtained_from_reference_to_principal(bool gener var property = model.FindEntityType(typeof(Product)).FindProperty("CategoryId"); var keyPropagator = contextServices.GetRequiredService(); - PropagateValue(keyPropagator, dependentEntry, property, async); + if (async) + { + await keyPropagator.PropagateValueAsync(dependentEntry, property); + } + else + { + keyPropagator.PropagateValue(dependentEntry, property); + } Assert.Equal(11, dependentEntry[property]); Assert.False(dependentEntry.HasTemporaryValue(property)); @@ -37,7 +44,7 @@ public void Foreign_key_value_is_obtained_from_reference_to_principal(bool gener [InlineData(false, true)] [InlineData(true, false)] [InlineData(true, true)] - public void Foreign_key_value_is_obtained_from_tracked_principal_with_populated_collection(bool generateTemporary, bool async) + public async Task Foreign_key_value_is_obtained_from_tracked_principal_with_populated_collection(bool generateTemporary, bool async) { var model = BuildModel(generateTemporary); var contextServices = CreateContextServices(model); @@ -50,10 +57,12 @@ public void Foreign_key_value_is_obtained_from_tracked_principal_with_populated_ manager.GetOrCreateEntry(principal).SetEntityState(EntityState.Unchanged); var dependentEntry = manager.GetOrCreateEntry(dependent); - var property = model.FindEntityType(typeof(Product)).FindProperty("CategoryId"); + var property = model.FindEntityType(typeof(Product))!.FindProperty("CategoryId")!; var keyPropagator = contextServices.GetRequiredService(); - PropagateValue(keyPropagator, dependentEntry, property, async); + _ = async + ? await keyPropagator.PropagateValueAsync(dependentEntry, property) + : keyPropagator.PropagateValue(dependentEntry, property); Assert.Equal(11, dependentEntry[property]); Assert.False(dependentEntry.HasTemporaryValue(property)); @@ -64,7 +73,7 @@ public void Foreign_key_value_is_obtained_from_tracked_principal_with_populated_ [InlineData(false, true)] [InlineData(true, false)] [InlineData(true, true)] - public void Non_identifying_foreign_key_value_is_not_generated_if_principal_key_not_set(bool generateTemporary, bool async) + public async Task Non_identifying_foreign_key_value_is_not_generated_if_principal_key_not_set(bool generateTemporary, bool async) { var model = BuildModel(generateTemporary); @@ -73,10 +82,12 @@ public void Non_identifying_foreign_key_value_is_not_generated_if_principal_key_ var contextServices = CreateContextServices(model); var dependentEntry = contextServices.GetRequiredService().GetOrCreateEntry(dependent); - var property = model.FindEntityType(typeof(Product)).FindProperty("CategoryId"); + var property = model.FindEntityType(typeof(Product))!.FindProperty("CategoryId")!; var keyPropagator = contextServices.GetRequiredService(); - PropagateValue(keyPropagator, dependentEntry, property, async); + _ = async + ? await keyPropagator.PropagateValueAsync(dependentEntry, property) + : keyPropagator.PropagateValue(dependentEntry, property); Assert.Equal(0, dependentEntry[property]); Assert.False(dependentEntry.HasTemporaryValue(property)); @@ -87,7 +98,7 @@ public void Non_identifying_foreign_key_value_is_not_generated_if_principal_key_ [InlineData(false, true)] [InlineData(true, false)] [InlineData(true, true)] - public void One_to_one_foreign_key_value_is_obtained_from_reference_to_principal(bool generateTemporary, bool async) + public async Task One_to_one_foreign_key_value_is_obtained_from_reference_to_principal(bool generateTemporary, bool async) { var model = BuildModel(generateTemporary); @@ -97,10 +108,12 @@ public void One_to_one_foreign_key_value_is_obtained_from_reference_to_principal var contextServices = CreateContextServices(model); model = contextServices.GetRequiredService(); var dependentEntry = contextServices.GetRequiredService().GetOrCreateEntry(dependent); - var property = model.FindEntityType(typeof(ProductDetail)).FindProperty("Id"); + var property = model.FindEntityType(typeof(ProductDetail))!.FindProperty("Id")!; var keyPropagator = contextServices.GetRequiredService(); - PropagateValue(keyPropagator, dependentEntry, property, async); + _ = async + ? await keyPropagator.PropagateValueAsync(dependentEntry, property) + : keyPropagator.PropagateValue(dependentEntry, property); Assert.Equal(21, dependentEntry[property]); Assert.False(dependentEntry.HasTemporaryValue(property)); @@ -111,7 +124,7 @@ public void One_to_one_foreign_key_value_is_obtained_from_reference_to_principal [InlineData(false, true)] [InlineData(true, false)] [InlineData(true, true)] - public void One_to_one_foreign_key_value_is_obtained_from_tracked_principal(bool generateTemporary, bool async) + public async Task One_to_one_foreign_key_value_is_obtained_from_tracked_principal(bool generateTemporary, bool async) { var model = BuildModel(generateTemporary); var contextServices = CreateContextServices(model); @@ -123,10 +136,12 @@ public void One_to_one_foreign_key_value_is_obtained_from_tracked_principal(bool manager.GetOrCreateEntry(principal).SetEntityState(EntityState.Unchanged); var dependentEntry = manager.GetOrCreateEntry(dependent); - var property = model.FindEntityType(typeof(ProductDetail)).FindProperty("Id"); + var property = model.FindEntityType(typeof(ProductDetail))!.FindProperty("Id")!; var keyPropagator = contextServices.GetRequiredService(); - PropagateValue(keyPropagator, dependentEntry, property, async); + _ = async + ? await keyPropagator.PropagateValueAsync(dependentEntry, property) + : keyPropagator.PropagateValue(dependentEntry, property); Assert.Equal(21, dependentEntry[property]); Assert.False(dependentEntry.HasTemporaryValue(property)); @@ -137,7 +152,7 @@ public void One_to_one_foreign_key_value_is_obtained_from_tracked_principal(bool [InlineData(false, true)] [InlineData(true, false)] [InlineData(true, true)] - public void Identifying_foreign_key_value_is_generated_if_principal_key_not_set(bool generateTemporary, bool async) + public async Task Identifying_foreign_key_value_is_generated_if_principal_key_not_set(bool generateTemporary, bool async) { var model = BuildModel(generateTemporary); @@ -146,13 +161,15 @@ public void Identifying_foreign_key_value_is_generated_if_principal_key_not_set( var contextServices = CreateContextServices(model); var dependentEntry = contextServices.GetRequiredService().GetOrCreateEntry(dependent); - var property = model.FindEntityType(typeof(ProductDetail)).FindProperty("Id"); + var property = model.FindEntityType(typeof(ProductDetail))!.FindProperty("Id")!; var keyPropagator = contextServices.GetRequiredService(); - PropagateValue(keyPropagator, dependentEntry, property, async); + _ = async + ? await keyPropagator.PropagateValueAsync(dependentEntry, property) + : keyPropagator.PropagateValue(dependentEntry, property); Assert.NotEqual(0, dependentEntry[property]); - Assert.Equal(generateTemporary, dependentEntry.HasTemporaryValue(property)); + Assert.True(dependentEntry.HasTemporaryValue(property)); } [ConditionalTheory] @@ -160,7 +177,7 @@ public void Identifying_foreign_key_value_is_generated_if_principal_key_not_set( [InlineData(false, true)] [InlineData(true, false)] [InlineData(true, true)] - public void Identifying_foreign_key_value_is_propagated_if_principal_key_is_generated(bool generateTemporary, bool async) + public async Task Identifying_foreign_key_value_is_propagated_if_principal_key_is_generated(bool generateTemporary, bool async) { var model = BuildModel(generateTemporary); @@ -172,16 +189,18 @@ public void Identifying_foreign_key_value_is_propagated_if_principal_key_is_gene var principalEntry = stateManager.GetOrCreateEntry(principal); principalEntry.SetEntityState(EntityState.Added); var dependentEntry = stateManager.GetOrCreateEntry(dependent); - var principalProperty = model.FindEntityType(typeof(Product)).FindProperty(nameof(Product.Id)); - var dependentProperty = model.FindEntityType(typeof(ProductDetail)).FindProperty(nameof(ProductDetail.Id)); + var principalProperty = model.FindEntityType(typeof(Product))!.FindProperty(nameof(Product.Id))!; + var dependentProperty = model.FindEntityType(typeof(ProductDetail))!.FindProperty(nameof(ProductDetail.Id))!; var keyPropagator = contextServices.GetRequiredService(); - PropagateValue(keyPropagator, dependentEntry, dependentProperty, async); + _ = async + ? await keyPropagator.PropagateValueAsync(dependentEntry, dependentProperty) + : keyPropagator.PropagateValue(dependentEntry, dependentProperty); Assert.NotEqual(0, principalEntry[principalProperty]); Assert.Equal(generateTemporary, principalEntry.HasTemporaryValue(principalProperty)); Assert.NotEqual(0, dependentEntry[dependentProperty]); - Assert.Equal(generateTemporary, dependentEntry.HasTemporaryValue(dependentProperty)); + Assert.True(dependentEntry.HasTemporaryValue(dependentProperty)); Assert.Equal(principalEntry[principalProperty], dependentEntry[dependentProperty]); } @@ -190,7 +209,7 @@ public void Identifying_foreign_key_value_is_propagated_if_principal_key_is_gene [InlineData(false, true)] [InlineData(true, false)] [InlineData(true, true)] - public void Composite_foreign_key_value_is_obtained_from_reference_to_principal(bool generateTemporary, bool async) + public async Task Composite_foreign_key_value_is_obtained_from_reference_to_principal(bool generateTemporary, bool async) { var model = BuildModel(generateTemporary); @@ -200,12 +219,17 @@ public void Composite_foreign_key_value_is_obtained_from_reference_to_principal( var contextServices = CreateContextServices(model); model = contextServices.GetRequiredService(); var dependentEntry = contextServices.GetRequiredService().GetOrCreateEntry(dependent); - var property1 = model.FindEntityType(typeof(OrderLineDetail)).FindProperty("OrderId"); - var property2 = model.FindEntityType(typeof(OrderLineDetail)).FindProperty("ProductId"); + var property1 = model.FindEntityType(typeof(OrderLineDetail))!.FindProperty("OrderId")!; + var property2 = model.FindEntityType(typeof(OrderLineDetail))!.FindProperty("ProductId")!; var keyPropagator = contextServices.GetRequiredService(); - PropagateValue(keyPropagator, dependentEntry, property1, async); - PropagateValue(keyPropagator, dependentEntry, property2, async); + _ = async + ? await keyPropagator.PropagateValueAsync(dependentEntry, property1) + : keyPropagator.PropagateValue(dependentEntry, property1); + + _ = async + ? await keyPropagator.PropagateValueAsync(dependentEntry, property2) + : keyPropagator.PropagateValue(dependentEntry, property2); Assert.Equal(11, dependentEntry[property1]); Assert.False(dependentEntry.HasTemporaryValue(property1)); @@ -218,7 +242,7 @@ public void Composite_foreign_key_value_is_obtained_from_reference_to_principal( [InlineData(false, true)] [InlineData(true, false)] [InlineData(true, true)] - public void Composite_foreign_key_value_is_obtained_from_tracked_principal(bool generateTemporary, bool async) + public async Task Composite_foreign_key_value_is_obtained_from_tracked_principal(bool generateTemporary, bool async) { var model = BuildModel(generateTemporary); var contextServices = CreateContextServices(model); @@ -235,12 +259,17 @@ public void Composite_foreign_key_value_is_obtained_from_tracked_principal(bool manager.GetOrCreateEntry(principal).SetEntityState(EntityState.Unchanged); var dependentEntry = manager.GetOrCreateEntry(dependent); - var property1 = model.FindEntityType(typeof(OrderLineDetail)).FindProperty("OrderId"); - var property2 = model.FindEntityType(typeof(OrderLineDetail)).FindProperty("ProductId"); + var property1 = model.FindEntityType(typeof(OrderLineDetail))!.FindProperty("OrderId")!; + var property2 = model.FindEntityType(typeof(OrderLineDetail))!.FindProperty("ProductId")!; var keyPropagator = contextServices.GetRequiredService(); - PropagateValue(keyPropagator, dependentEntry, property1, async); - PropagateValue(keyPropagator, dependentEntry, property2, async); + _ = async + ? await keyPropagator.PropagateValueAsync(dependentEntry, property1) + : keyPropagator.PropagateValue(dependentEntry, property1); + + _ = async + ? await keyPropagator.PropagateValueAsync(dependentEntry, property2) + : keyPropagator.PropagateValue(dependentEntry, property2); Assert.Equal(11, dependentEntry[property1]); Assert.False(dependentEntry.HasTemporaryValue(property1)); @@ -251,18 +280,6 @@ public void Composite_foreign_key_value_is_obtained_from_tracked_principal(bool private static IServiceProvider CreateContextServices(IModel model) => InMemoryTestHelpers.Instance.CreateContextServices(model); - private static void PropagateValue(IKeyPropagator keyPropagator, InternalEntityEntry dependentEntry, IProperty property, bool async) - { - if (async) - { - keyPropagator.PropagateValueAsync(dependentEntry, property).GetAwaiter().GetResult(); - } - else - { - keyPropagator.PropagateValue(dependentEntry, property); - } - } - private class BaseType { public int Id { get; set; }