Fix temporary value snapshot

Fixes #17997

The problem here was that the temporary values snapshot was copying current values rather than starting with empty values. This then meant that when the current value was set to null, the temp value took over, which was then incorrect. Related to this, when setting a current value to null, the temp value (if it exists) must also be set to null otherwise it will start being used.

src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs

@@ -911,7 +911,7 @@ public virtual void EnsureTemporaryValues()
         {
             if (_temporaryValues.IsEmpty)
             {
-                _temporaryValues = new SidecarValues(this);
+                _temporaryValues = new SidecarValues(((EntityType)EntityType).TemporaryValuesFactory(this));
             }
         }
 
@@ -925,7 +925,7 @@ public virtual void EnsureStoreGeneratedValues()
         {
             if (_storeGeneratedValues.IsEmpty)
             {
-                _storeGeneratedValues = new SidecarValues(this);
+                _storeGeneratedValues = new SidecarValues(((EntityType)EntityType).StoreGeneratedValuesFactory(this));
             }
         }
 
@@ -1141,6 +1141,14 @@ private void SetProperty(
                     if (valueType == CurrentValueType.Normal)
                     {
                         WritePropertyValue(propertyBase, value, isMaterialization);
+
+                        if (currentValueType != CurrentValueType.Normal
+                            && !_temporaryValues.IsEmpty
+                            && equals(value, asProperty.ClrType.GetDefaultValue()))
+                        {
+                            var storeGeneratedIndex = asProperty.GetStoreGeneratedIndex();
+                            _temporaryValues.SetValue(asProperty, value, storeGeneratedIndex);
+                        }
                     }
                     else
                     {

src/EFCore/ChangeTracking/Internal/SidecarValues.cs

@@ -16,9 +16,9 @@ private readonly struct SidecarValues
         {
             private readonly ISnapshot _values;
 
-            public SidecarValues(InternalEntityEntry entry)
+            public SidecarValues(ISnapshot valuesFactory)
             {
-                _values = ((EntityType)entry.EntityType).SidecarValuesFactory(entry);
+                _values = valuesFactory;
             }
 
             public bool TryGetValue(int index, out object value)

src/EFCore/ChangeTracking/Internal/SnapshotFactoryFactory.cs

@@ -92,7 +92,13 @@ protected virtual Expression CreateConstructorExpression(
             return constructorExpression;
         }
 
-        private Expression CreateSnapshotExpression(
+        /// <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>
+        protected virtual Expression CreateSnapshotExpression(
             Type entityType,
             ParameterExpression parameter,
             Type[] types,

src/EFCore/ChangeTracking/Internal/TemporaryValuesFactoryFactory.cs

@@ -0,0 +1,39 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using System.Linq.Expressions;
+using Microsoft.EntityFrameworkCore.Metadata;
+
+namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal
+{
+    /// <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 class TemporaryValuesFactoryFactory : SidecarValuesFactoryFactory
+    {
+        /// <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>
+        protected override Expression CreateSnapshotExpression(
+            Type entityType,
+            ParameterExpression parameter,
+            Type[] types,
+            IList<IPropertyBase> propertyBases)
+        {
+            var constructorExpression = Expression.Convert(
+                Expression.New(
+                    Snapshot.CreateSnapshotType(types).GetDeclaredConstructor(new Type[0])),
+                typeof(ISnapshot));
+
+            return constructorExpression;
+        }
+    }
+}

src/EFCore/Metadata/Internal/EntityType.cs

@@ -59,7 +59,8 @@ private readonly SortedDictionary<string, ServiceProperty> _serviceProperties
 
         private Func<InternalEntityEntry, ISnapshot> _relationshipSnapshotFactory;
         private Func<InternalEntityEntry, ISnapshot> _originalValuesFactory;
-        private Func<InternalEntityEntry, ISnapshot> _sidecarValuesFactory;
+        private Func<InternalEntityEntry, ISnapshot> _temporaryValuesFactory;
+        private Func<InternalEntityEntry, ISnapshot> _storeGeneratedValuesFactory;
         private Func<ValueBuffer, ISnapshot> _shadowValuesFactory;
         private Func<ISnapshot> _emptyShadowValuesFactory;
 
@@ -2042,11 +2043,22 @@ public virtual Func<InternalEntityEntry, ISnapshot> OriginalValuesFactory
         ///     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 Func<InternalEntityEntry, ISnapshot> SidecarValuesFactory
+        public virtual Func<InternalEntityEntry, ISnapshot> StoreGeneratedValuesFactory
             => NonCapturingLazyInitializer.EnsureInitialized(
-                ref _sidecarValuesFactory, this,
+                ref _storeGeneratedValuesFactory, this,
                 entityType => new SidecarValuesFactoryFactory().Create(entityType));
 
+        /// <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 Func<InternalEntityEntry, ISnapshot> TemporaryValuesFactory
+            => NonCapturingLazyInitializer.EnsureInitialized(
+                ref _temporaryValuesFactory, this,
+                entityType => new TemporaryValuesFactoryFactory().Create(entityType));
+
         /// <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

test/EFCore.Specification.Tests/GraphUpdatesTestBase.cs

@@ -23,6 +23,58 @@ public abstract partial class GraphUpdatesTestBase<TFixture> : IClassFixture<TFi
 
         protected TFixture Fixture { get; }
 
+        [ConditionalTheory]
+        [InlineData((int)ChangeMechanism.Fk)]
+        [InlineData((int)ChangeMechanism.Dependent)]
+        [InlineData((int)(ChangeMechanism.Dependent | ChangeMechanism.Fk))]
+        public virtual void Changes_to_Added_relationships_are_picked_up(ChangeMechanism changeMechanism)
+        {
+            var id = 0;
+
+            ExecuteWithStrategyInTransaction(
+                context =>
+                {
+                    var entity = new OptionalSingle1();
+
+                    if ((changeMechanism & ChangeMechanism.Fk) != 0)
+                    {
+                        entity.RootId = 5545;
+                    }
+
+                    if ((changeMechanism & ChangeMechanism.Dependent) != 0)
+                    {
+                        entity.Root = new Root();
+                    }
+
+                    context.Add(entity);
+
+                    if ((changeMechanism & ChangeMechanism.Fk) != 0)
+                    {
+                        entity.RootId = null;
+                    }
+
+                    if ((changeMechanism & ChangeMechanism.Dependent) != 0)
+                    {
+                        entity.Root = null;
+                    }
+
+                    context.ChangeTracker.DetectChanges();
+
+                    Assert.Null(entity.RootId);
+                    Assert.Null(entity.Root);
+
+                    context.SaveChanges();
+
+                    id = entity.Id;
+                },
+                context =>
+                {
+                    var entity = context.Set<OptionalSingle1>().Include(e => e.Root).Single(e => e.Id == id);
+                    Assert.Null(entity.Root);
+                    Assert.Null(entity.RootId);
+                });
+        }
+
         [ConditionalTheory]
         [InlineData(false, CascadeTiming.OnSaveChanges)]
         [InlineData(false, CascadeTiming.Immediate)]