Don't mark property as loaded in lazy-loader if lazy-loading fails (#…

…25898)

src/EFCore/Infrastructure/Internal/LazyLoader.cs

@@ -98,7 +98,15 @@ public virtual void Load(object entity, [CallerMemberName] string navigationName
 
             if (ShouldLoad(entity, navigationName, out var entry))
             {
-                entry.Load();
+                try
+                {
+                    entry.Load();
+                }
+                catch
+                {
+                    SetLoaded(entity, navigationName, false);
+                    throw;
+                }
             }
         }
 
@@ -108,17 +116,26 @@ public virtual void Load(object entity, [CallerMemberName] string navigationName
         ///     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 Task LoadAsync(
+        public virtual async Task LoadAsync(
             object entity,
             CancellationToken cancellationToken = default,
             [CallerMemberName] string navigationName = "")
         {
             Check.NotNull(entity, nameof(entity));
             Check.NotEmpty(navigationName, nameof(navigationName));
 
-            return ShouldLoad(entity, navigationName, out var entry)
-                ? entry.LoadAsync(cancellationToken)
-                : Task.CompletedTask;
+            if (ShouldLoad(entity, navigationName, out var entry))
+            {
+                try
+                {
+                    await entry.LoadAsync(cancellationToken);
+                }
+                catch
+                {
+                    SetLoaded(entity, navigationName, false);
+                    throw;
+                }
+            }
         }
 
         private bool ShouldLoad(object entity, string navigationName, [NotNullWhen(true)] out NavigationEntry? navigationEntry)

test/EFCore.Specification.Tests/LazyLoadProxyTestBase.cs

@@ -2513,6 +2513,7 @@ public abstract class Parent
             public virtual IEnumerable<ChildCompositeKey> ChildrenCompositeKey { get; set; }
             public virtual SingleCompositeKey SingleCompositeKey { get; set; }
             public virtual WithRecursiveProperty WithRecursiveProperty { get; set; }
+            public virtual IEnumerable<Child> ManyChildren { get; set; }
         }
 
         public class Mother : Parent
@@ -2556,6 +2557,8 @@ public class Child
             public int? ParentId { get; set; }
 
             public virtual Parent Parent { get; set; }
+
+            public virtual IEnumerable<Parent> ManyParents { get; set; }
         }
 
         public class SinglePkToPk
@@ -2885,6 +2888,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                                 e => new { e.AlternateId, e.Id })
                             .HasForeignKey<SingleCompositeKey>(
                                 e => new { e.ParentAlternateId, e.ParentId });
+
+                        b.HasMany(e => e.ManyChildren).WithMany(e => e.ManyParents);
                     });
 
                 modelBuilder.Entity<Mother>();
@@ -3002,7 +3007,8 @@ protected override void Seed(DbContext context)
                         ChildrenCompositeKey =
                             new List<ChildCompositeKey> { new() { Id = 51 }, new() { Id = 52 } },
                         SingleCompositeKey = new SingleCompositeKey { Id = 62 },
-                        WithRecursiveProperty = new WithRecursiveProperty { Id = 8086 }
+                        WithRecursiveProperty = new WithRecursiveProperty { Id = 8086 },
+                        ManyChildren = new List<Child> { new() { Id = 999 } }
                     });
 
                 context.Add(

test/EFCore.Sqlite.FunctionalTests/LazyLoadProxySqliteTest.cs

@@ -1,7 +1,12 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System;
+using System.Data.Common;
+using System.Linq;
+using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.EntityFrameworkCore.TestUtilities;
+using Xunit;
 
 namespace Microsoft.EntityFrameworkCore
 {
@@ -12,8 +17,126 @@ public LazyLoadProxySqliteTest(LoadSqliteFixture fixture)
         {
         }
 
+        [ConditionalFact]
+        public void IsLoaded_is_not_set_if_loading_principal_collection_fails()
+        {
+            using var context = Fixture.CreateContext();
+
+            var principal = context.Set<Parent>().Single();
+            Assert.False(context.Entry(principal).Collection(e => e.Children).IsLoaded);
+
+            Fixture.Interceptor.Throw = true;
+
+            Assert.Equal("Bang!", Assert.Throws<Exception>(() => principal.Children).Message);
+            Assert.False(context.Entry(principal).Collection(e => e.Children).IsLoaded);
+
+            Fixture.Interceptor.Throw = false;
+
+            Assert.NotEmpty(principal.Children);
+            Assert.True(context.Entry(principal).Collection(e => e.Children).IsLoaded);
+        }
+
+        [ConditionalFact]
+        public void IsLoaded_is_not_set_if_loading_principal_single_reference_fails()
+        {
+            using var context = Fixture.CreateContext();
+
+            var principal = context.Set<Parent>().Single();
+            Assert.False(context.Entry(principal).Reference(e => e.Single).IsLoaded);
+
+            Fixture.Interceptor.Throw = true;
+
+            Assert.Equal("Bang!", Assert.Throws<Exception>(() => principal.Single).Message);
+            Assert.False(context.Entry(principal).Reference(e => e.Single).IsLoaded);
+
+            Fixture.Interceptor.Throw = false;
+
+            Assert.NotNull(principal.Single);
+            Assert.True(context.Entry(principal).Reference(e => e.Single).IsLoaded);
+        }
+
+        [ConditionalFact]
+        public void IsLoaded_is_not_set_if_loading_many_to_many_collection_fails()
+        {
+            using var context = Fixture.CreateContext();
+
+            var principal = context.Set<Parent>().Single();
+            Assert.False(context.Entry(principal).Collection(e => e.ManyChildren).IsLoaded);
+
+            Fixture.Interceptor.Throw = true;
+
+            Assert.Equal("Bang!", Assert.Throws<Exception>(() => principal.ManyChildren).Message);
+            Assert.False(context.Entry(principal).Collection(e => e.ManyChildren).IsLoaded);
+
+            Fixture.Interceptor.Throw = false;
+
+            Assert.NotEmpty(principal.ManyChildren);
+            Assert.True(context.Entry(principal).Collection(e => e.ManyChildren).IsLoaded);
+        }
+
+        [ConditionalFact]
+        public void IsLoaded_is_not_set_if_loading_dependent_single_reference_fails()
+        {
+            using var context = Fixture.CreateContext();
+
+            var dependent = context.Set<Single>().OrderBy(e => e.Id).First();
+            Assert.False(context.Entry(dependent).Reference(e => e.Parent).IsLoaded);
+
+            Fixture.Interceptor.Throw = true;
+
+            Assert.Equal("Bang!", Assert.Throws<Exception>(() => dependent.Parent).Message);
+            Assert.False(context.Entry(dependent).Reference(e => e.Parent).IsLoaded);
+
+            Fixture.Interceptor.Throw = false;
+
+            Assert.NotNull(dependent.Parent);
+            Assert.True(context.Entry(dependent).Reference(e => e.Parent).IsLoaded);
+        }
+
+        [ConditionalFact]
+        public void IsLoaded_is_not_set_if_loading_dependent_collection_reference_fails()
+        {
+            using var context = Fixture.CreateContext();
+
+            var dependent = context.Set<Child>().OrderBy(e => e.Id).First();
+            Assert.False(context.Entry(dependent).Reference(e => e.Parent).IsLoaded);
+
+            Fixture.Interceptor.Throw = true;
+
+            Assert.Equal("Bang!", Assert.Throws<Exception>(() => dependent.Parent).Message);
+            Assert.False(context.Entry(dependent).Reference(e => e.Parent).IsLoaded);
+
+            Fixture.Interceptor.Throw = false;
+
+            Assert.NotNull(dependent.Parent);
+            Assert.True(context.Entry(dependent).Reference(e => e.Parent).IsLoaded);
+        }
+
+        public class ThrowingInterceptor : DbCommandInterceptor
+        {
+            public bool Throw { get; set; }
+
+            public override InterceptionResult<DbDataReader> ReaderExecuting(
+                DbCommand command,
+                CommandEventData eventData,
+                InterceptionResult<DbDataReader> result)
+            {
+                if (Throw)
+                {
+                    throw new Exception("Bang!");
+                }
+
+                return base.ReaderExecuting(command, eventData, result);
+            }
+        }
+
         public class LoadSqliteFixture : LoadFixtureBase
         {
+            public ThrowingInterceptor Interceptor { get; } = new();
+
+            public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
+                => base.AddOptions(builder.UseLazyLoadingProxies().AddInterceptors(Interceptor));
+
             protected override ITestStoreFactory TestStoreFactory
                 => SqliteTestStoreFactory.Instance;
         }