From 58c6d0efe57fb9a696be54a1328b9d0e7023d6aa Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Thu, 6 Jan 2022 16:58:19 -0800 Subject: [PATCH] Don't issue a warning when replacing an optional dependent with non-null values Fixes #26712 --- .../Update/ModificationCommand.cs | 10 +++++-- .../TableSplittingTestBase.cs | 26 ++++++++++++++++--- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/EFCore.Relational/Update/ModificationCommand.cs b/src/EFCore.Relational/Update/ModificationCommand.cs index bd034c42378..a5c7268286b 100644 --- a/src/EFCore.Relational/Update/ModificationCommand.cs +++ b/src/EFCore.Relational/Update/ModificationCommand.cs @@ -282,7 +282,7 @@ private List GenerateColumnModifications() } var optionalDependentWithAllNull = - (entry.EntityState == EntityState.Deleted + (entry.EntityState == EntityState.Modified || entry.EntityState == EntityState.Added) && tableMapping.Table.IsOptional(entry.EntityType) && tableMapping.Table.GetRowInternalForeignKeys(entry.EntityType).Any(); @@ -353,11 +353,17 @@ private List GenerateColumnModifications() if (optionalDependentWithAllNull && columnModification.IsWrite - && (entry.EntityState != EntityState.Added || columnModification.Value is not null)) + && columnModification.Value is not null) { optionalDependentWithAllNull = false; } } + else if (optionalDependentWithAllNull + && state == EntityState.Modified + && entry.GetCurrentValue(property) is not null) + { + optionalDependentWithAllNull = false; + } } if (optionalDependentWithAllNull && _logger != null) diff --git a/test/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs b/test/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs index 7bc1cbcc617..913404c7afc 100644 --- a/test/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs @@ -578,6 +578,22 @@ public virtual async Task Warn_when_save_optional_dependent_with_null_values() var log = TestSqlLoggerFactory.Log.Single(l => l.Level == LogLevel.Warning); Assert.Equal(expected, log.Message); + + TestSqlLoggerFactory.Clear(); + + meterReading.MeterReadingDetails = new MeterReadingDetail { CurrentRead = "100" }; + + context.SaveChanges(); + + Assert.Empty(TestSqlLoggerFactory.Log.Where(l => l.Level == LogLevel.Warning)); + + meterReading.MeterReadingDetails = new MeterReadingDetail(); + + context.SaveChanges(); + + log = TestSqlLoggerFactory.Log.Single(l => l.Level == LogLevel.Warning); + + Assert.Equal(expected, log.Message); } [ConditionalFact] @@ -595,9 +611,11 @@ public virtual async Task No_warn_when_save_optional_dependent_at_least_one_none context.SaveChanges(); - var log = TestSqlLoggerFactory.Log.SingleOrDefault(l => l.Level == LogLevel.Warning); + meterReading.MeterReadingDetails = new MeterReadingDetail { CurrentRead = "100" }; + + context.SaveChanges(); - Assert.Null(log.Message); + Assert.Empty(TestSqlLoggerFactory.Log.Where(l => l.Level == LogLevel.Warning)); } protected override string StoreName { get; } = "TableSplittingTest"; @@ -684,8 +702,8 @@ public SharedTableContext(DbContextOptions options) { } - protected DbSet MeterReadings { get; set; } - protected DbSet MeterReadingDetails { get; set; } + public DbSet MeterReadings { get; set; } + public DbSet MeterReadingDetails { get; set; } } protected class MeterReading