diff --git a/src/EFCore/Infrastructure/Internal/LazyLoader.cs b/src/EFCore/Infrastructure/Internal/LazyLoader.cs index ddbab77f791..ad09a680b38 100644 --- a/src/EFCore/Infrastructure/Internal/LazyLoader.cs +++ b/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,7 +116,7 @@ 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. /// - public virtual Task LoadAsync( + public virtual async Task LoadAsync( object entity, CancellationToken cancellationToken = default, [CallerMemberName] string navigationName = "") @@ -116,9 +124,18 @@ public virtual Task LoadAsync( 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) diff --git a/test/EFCore.Specification.Tests/LazyLoadProxyTestBase.cs b/test/EFCore.Specification.Tests/LazyLoadProxyTestBase.cs index 49c4a815efd..e5dcaaad574 100644 --- a/test/EFCore.Specification.Tests/LazyLoadProxyTestBase.cs +++ b/test/EFCore.Specification.Tests/LazyLoadProxyTestBase.cs @@ -2513,6 +2513,7 @@ public abstract class Parent public virtual IEnumerable ChildrenCompositeKey { get; set; } public virtual SingleCompositeKey SingleCompositeKey { get; set; } public virtual WithRecursiveProperty WithRecursiveProperty { get; set; } + public virtual IEnumerable 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 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( e => new { e.ParentAlternateId, e.ParentId }); + + b.HasMany(e => e.ManyChildren).WithMany(e => e.ManyParents); }); modelBuilder.Entity(); @@ -3002,7 +3007,8 @@ protected override void Seed(DbContext context) ChildrenCompositeKey = new List { new() { Id = 51 }, new() { Id = 52 } }, SingleCompositeKey = new SingleCompositeKey { Id = 62 }, - WithRecursiveProperty = new WithRecursiveProperty { Id = 8086 } + WithRecursiveProperty = new WithRecursiveProperty { Id = 8086 }, + ManyChildren = new List { new() { Id = 999 } } }); context.Add( diff --git a/test/EFCore.Sqlite.FunctionalTests/LazyLoadProxySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/LazyLoadProxySqliteTest.cs index c85dc4516dd..ee735957d09 100644 --- a/test/EFCore.Sqlite.FunctionalTests/LazyLoadProxySqliteTest.cs +++ b/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().Single(); + Assert.False(context.Entry(principal).Collection(e => e.Children).IsLoaded); + + Fixture.Interceptor.Throw = true; + + Assert.Equal("Bang!", Assert.Throws(() => 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().Single(); + Assert.False(context.Entry(principal).Reference(e => e.Single).IsLoaded); + + Fixture.Interceptor.Throw = true; + + Assert.Equal("Bang!", Assert.Throws(() => 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().Single(); + Assert.False(context.Entry(principal).Collection(e => e.ManyChildren).IsLoaded); + + Fixture.Interceptor.Throw = true; + + Assert.Equal("Bang!", Assert.Throws(() => 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().OrderBy(e => e.Id).First(); + Assert.False(context.Entry(dependent).Reference(e => e.Parent).IsLoaded); + + Fixture.Interceptor.Throw = true; + + Assert.Equal("Bang!", Assert.Throws(() => 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().OrderBy(e => e.Id).First(); + Assert.False(context.Entry(dependent).Reference(e => e.Parent).IsLoaded); + + Fixture.Interceptor.Throw = true; + + Assert.Equal("Bang!", Assert.Throws(() => 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 ReaderExecuting( + DbCommand command, + CommandEventData eventData, + InterceptionResult 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; }