From 4df8e7bc9483250ee9fdb64ef3bccbbdd1229187 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Thu, 1 Feb 2018 15:20:17 -0800 Subject: [PATCH] Throw when attempting to lazy-load after no-tracking query Part of #10042, #10509, #3797 It would be good to make this work in a future release, but this involves running a no-tracking query with fixup where the root entity is already materialized. For now, this will throw so we can make it work later without it being a breaking change. Lazy-loading behaviors for non-tracked entities: * Proxies: * No-op if entity was explicitly detached * Throw if it was queried as no-tracking * Lazy-loading entities with loader service property: * No-op if entity was explicitly detached * Throw if it was queried as no-tracking * Lazy-loading entities without service property: * Throw even if entity was explicitly detached, but entity can set loader to null, or a service property can be defined * Throw if it was queried as no-tracking --- .../LazyLoadProxyTestBase.cs | 42 +++ .../LoadTestBase.cs | 312 ++++++++++++------ src/EFCore/Internal/LazyLoader.cs | 12 +- 3 files changed, 261 insertions(+), 105 deletions(-) diff --git a/src/EFCore.Specification.Tests/LazyLoadProxyTestBase.cs b/src/EFCore.Specification.Tests/LazyLoadProxyTestBase.cs index af9d391c322..8584a323ad6 100644 --- a/src/EFCore.Specification.Tests/LazyLoadProxyTestBase.cs +++ b/src/EFCore.Specification.Tests/LazyLoadProxyTestBase.cs @@ -1380,6 +1380,48 @@ public virtual void Lazy_load_reference_to_dependent_for_detached_is_no_op() Assert.Null(parent.Single); } } + + [Fact] + public virtual void Lazy_load_collection_for_no_tracking_throws() + { + using (var context = CreateContext(lazyLoadingEnabled: true)) + { + var parent = context.Set().AsNoTracking().Single(); + + Assert.Equal( + CoreStrings.CannotLoadDetached(nameof(Parent.Children), nameof(Parent)), + Assert.Throws( + () => parent.Children).Message); + } + } + + [Fact] + public virtual void Lazy_load_reference_to_principal_for_no_tracking_throws() + { + using (var context = CreateContext(lazyLoadingEnabled: true)) + { + var child = context.Set().AsNoTracking().Single(e => e.Id == 12); + + Assert.Equal( + CoreStrings.CannotLoadDetached(nameof(Child.Parent), nameof(Child)), + Assert.Throws( + () => child.Parent).Message); + } + } + + [Fact] + public virtual void Lazy_load_reference_to_dependent_for_no_tracking_throws() + { + using (var context = CreateContext(lazyLoadingEnabled: true)) + { + var parent = context.Set().AsNoTracking().Single(); + + Assert.Equal( + CoreStrings.CannotLoadDetached(nameof(Parent.Single), nameof(Parent)), + Assert.Throws( + () => parent.Single).Message); + } + } [Theory] [InlineData(EntityState.Unchanged, true)] diff --git a/src/EFCore.Specification.Tests/LoadTestBase.cs b/src/EFCore.Specification.Tests/LoadTestBase.cs index 7390f43da08..46357f9d0fc 100644 --- a/src/EFCore.Specification.Tests/LoadTestBase.cs +++ b/src/EFCore.Specification.Tests/LoadTestBase.cs @@ -1216,42 +1216,66 @@ public virtual void Lazy_load_one_to_one_reference_to_principal_null_FK_composit } } - [Fact] - public virtual void Lazy_load_collection_for_detached_is_no_op() + [Theory] + [InlineData(true)] + [InlineData(false)] + public virtual void Lazy_load_collection_for_detached_throws(bool noTracking) { - using (var context = CreateContext(lazyLoadingEnabled: true)) + using (var context = CreateContext(lazyLoadingEnabled: true, noTracking: noTracking)) { var parent = context.Set().Single(); - context.Entry(parent).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(parent).State = EntityState.Detached; + } - Assert.Null(parent.Children); + Assert.Equal( + CoreStrings.CannotLoadDetached(nameof(Parent.Children), nameof(Parent)), + Assert.Throws( + () => parent.Children).Message); } } - [Fact] - public virtual void Lazy_load_reference_to_principal_for_detached_is_no_op() + [Theory] + [InlineData(true)] + [InlineData(false)] + public virtual void Lazy_load_reference_to_principal_for_detached_throws(bool noTracking) { - using (var context = CreateContext(lazyLoadingEnabled: true)) + using (var context = CreateContext(lazyLoadingEnabled: true, noTracking: noTracking)) { var child = context.Set().Single(e => e.Id == 12); - context.Entry(child).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(child).State = EntityState.Detached; + } - Assert.Null(child.Parent); + Assert.Equal( + CoreStrings.CannotLoadDetached(nameof(Child.Parent), nameof(Child)), + Assert.Throws( + () => child.Parent).Message); } } - [Fact] - public virtual void Lazy_load_reference_to_dependent_for_detached_is_no_op() + [Theory] + [InlineData(true)] + [InlineData(false)] + public virtual void Lazy_load_reference_to_dependent_for_detached_throws(bool noTracking) { - using (var context = CreateContext(lazyLoadingEnabled: true)) + using (var context = CreateContext(lazyLoadingEnabled: true, noTracking: noTracking)) { var parent = context.Set().Single(); - context.Entry(parent).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(parent).State = EntityState.Detached; + } - Assert.Null(parent.Single); + Assert.Equal( + CoreStrings.CannotLoadDetached(nameof(Parent.Single), nameof(Parent)), + Assert.Throws( + () => parent.Single).Message); } } @@ -5091,17 +5115,22 @@ public virtual void Can_change_IsLoaded_flag_for_reference_only_if_null() } [Theory] - [InlineData(true)] - [InlineData(false)] - public virtual async Task Load_collection_for_detached_throws(bool async) + [InlineData(true, false)] + [InlineData(false, false)] + [InlineData(true, true)] + [InlineData(false, true)] + public virtual async Task Load_collection_for_detached_throws(bool async, bool noTracking) { - using (var context = CreateContext()) + using (var context = CreateContext(noTracking: noTracking)) { var parent = context.Set().Single(); var collectionEntry = context.Entry(parent).Collection(e => e.Children); - context.Entry(parent).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(parent).State = EntityState.Detached; + } Assert.Equal( CoreStrings.CannotLoadDetached(nameof(Parent.Children), nameof(Parent)), @@ -5121,17 +5150,22 @@ public virtual async Task Load_collection_for_detached_throws(bool async) } [Theory] - [InlineData(true)] - [InlineData(false)] - public virtual async Task Load_collection_using_string_for_detached_throws(bool async) + [InlineData(true, false)] + [InlineData(false, false)] + [InlineData(true, true)] + [InlineData(false, true)] + public virtual async Task Load_collection_using_string_for_detached_throws(bool async, bool noTracking) { - using (var context = CreateContext()) + using (var context = CreateContext(noTracking: noTracking)) { var parent = context.Set().Single(); var collectionEntry = context.Entry(parent).Collection(nameof(Parent.Children)); - context.Entry(parent).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(parent).State = EntityState.Detached; + } Assert.Equal( CoreStrings.CannotLoadDetached(nameof(Parent.Children), nameof(Parent)), @@ -5151,17 +5185,22 @@ public virtual async Task Load_collection_using_string_for_detached_throws(bool } [Theory] - [InlineData(true)] - [InlineData(false)] - public virtual async Task Load_collection_with_navigation_for_detached_throws(bool async) + [InlineData(true, false)] + [InlineData(false, false)] + [InlineData(true, true)] + [InlineData(false, true)] + public virtual async Task Load_collection_with_navigation_for_detached_throws(bool async, bool noTracking) { - using (var context = CreateContext()) + using (var context = CreateContext(noTracking: noTracking)) { var parent = context.Set().Single(); var collectionEntry = context.Entry(parent).Navigation(nameof(Parent.Children)); - context.Entry(parent).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(parent).State = EntityState.Detached; + } Assert.Equal( CoreStrings.CannotLoadDetached(nameof(Parent.Children), nameof(Parent)), @@ -5181,17 +5220,22 @@ public virtual async Task Load_collection_with_navigation_for_detached_throws(bo } [Theory] - [InlineData(true)] - [InlineData(false)] - public virtual async Task Load_reference_to_principal_for_detached_throws(bool async) + [InlineData(true, false)] + [InlineData(false, false)] + [InlineData(true, true)] + [InlineData(false, true)] + public virtual async Task Load_reference_to_principal_for_detached_throws(bool async, bool noTracking) { - using (var context = CreateContext()) + using (var context = CreateContext(noTracking: noTracking)) { var child = context.Set().Single(e => e.Id == 12); var referenceEntry = context.Entry(child).Reference(e => e.Parent); - context.Entry(child).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(child).State = EntityState.Detached; + } Assert.Equal( CoreStrings.CannotLoadDetached(nameof(Child.Parent), nameof(Child)), @@ -5211,17 +5255,22 @@ public virtual async Task Load_reference_to_principal_for_detached_throws(bool a } [Theory] - [InlineData(true)] - [InlineData(false)] - public virtual async Task Load_reference_with_navigation_to_principal_for_detached_throws(bool async) + [InlineData(true, false)] + [InlineData(false, false)] + [InlineData(true, true)] + [InlineData(false, true)] + public virtual async Task Load_reference_with_navigation_to_principal_for_detached_throws(bool async, bool noTracking) { - using (var context = CreateContext()) + using (var context = CreateContext(noTracking: noTracking)) { var child = context.Set().Single(e => e.Id == 12); var referenceEntry = context.Entry(child).Navigation(nameof(Child.Parent)); - context.Entry(child).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(child).State = EntityState.Detached; + } Assert.Equal( CoreStrings.CannotLoadDetached(nameof(Child.Parent), nameof(Child)), @@ -5241,17 +5290,22 @@ public virtual async Task Load_reference_with_navigation_to_principal_for_detach } [Theory] - [InlineData(true)] - [InlineData(false)] - public virtual async Task Load_reference_using_string_to_principal_for_detached_throws(bool async) + [InlineData(true, false)] + [InlineData(false, false)] + [InlineData(true, true)] + [InlineData(false, true)] + public virtual async Task Load_reference_using_string_to_principal_for_detached_throws(bool async, bool noTracking) { - using (var context = CreateContext()) + using (var context = CreateContext(noTracking: noTracking)) { var child = context.Set().Single(e => e.Id == 12); var referenceEntry = context.Entry(child).Reference(nameof(Child.Parent)); - context.Entry(child).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(child).State = EntityState.Detached; + } Assert.Equal( CoreStrings.CannotLoadDetached(nameof(Child.Parent), nameof(Child)), @@ -5271,17 +5325,22 @@ public virtual async Task Load_reference_using_string_to_principal_for_detached_ } [Theory] - [InlineData(true)] - [InlineData(false)] - public virtual async Task Load_reference_to_dependent_for_detached_throws(bool async) + [InlineData(true, false)] + [InlineData(false, false)] + [InlineData(true, true)] + [InlineData(false, true)] + public virtual async Task Load_reference_to_dependent_for_detached_throws(bool async, bool noTracking) { - using (var context = CreateContext()) + using (var context = CreateContext(noTracking: noTracking)) { var parent = context.Set().Single(); var referenceEntry = context.Entry(parent).Reference(e => e.Single); - context.Entry(parent).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(parent).State = EntityState.Detached; + } Assert.Equal( CoreStrings.CannotLoadDetached(nameof(Parent.Single), nameof(Parent)), @@ -5301,17 +5360,22 @@ public virtual async Task Load_reference_to_dependent_for_detached_throws(bool a } [Theory] - [InlineData(true)] - [InlineData(false)] - public virtual async Task Load_reference_to_dependent_with_navigation_for_detached_throws(bool async) + [InlineData(true, false)] + [InlineData(false, false)] + [InlineData(true, true)] + [InlineData(false, true)] + public virtual async Task Load_reference_to_dependent_with_navigation_for_detached_throws(bool async, bool noTracking) { - using (var context = CreateContext()) + using (var context = CreateContext(noTracking: noTracking)) { var parent = context.Set().Single(); var referenceEntry = context.Entry(parent).Navigation(nameof(Parent.Single)); - context.Entry(parent).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(parent).State = EntityState.Detached; + } Assert.Equal( CoreStrings.CannotLoadDetached(nameof(Parent.Single), nameof(Parent)), @@ -5331,17 +5395,22 @@ public virtual async Task Load_reference_to_dependent_with_navigation_for_detach } [Theory] - [InlineData(true)] - [InlineData(false)] - public virtual async Task Load_reference_to_dependent_using_string_for_detached_throws(bool async) + [InlineData(true, false)] + [InlineData(false, false)] + [InlineData(true, true)] + [InlineData(false, true)] + public virtual async Task Load_reference_to_dependent_using_string_for_detached_throws(bool async, bool noTracking) { - using (var context = CreateContext()) + using (var context = CreateContext(noTracking: noTracking)) { var parent = context.Set().Single(); var referenceEntry = context.Entry(parent).Reference(nameof(Parent.Single)); - context.Entry(parent).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(parent).State = EntityState.Detached; + } Assert.Equal( CoreStrings.CannotLoadDetached(nameof(Parent.Single), nameof(Parent)), @@ -5360,16 +5429,21 @@ public virtual async Task Load_reference_to_dependent_using_string_for_detached_ } } - [Fact] - public virtual void Query_collection_for_detached_throws() + [Theory] + [InlineData(true)] + [InlineData(false)] + public virtual void Query_collection_for_detached_throws(bool noTracking) { - using (var context = CreateContext()) + using (var context = CreateContext(noTracking: noTracking)) { var parent = context.Set().Single(); var collectionEntry = context.Entry(parent).Collection(e => e.Children); - context.Entry(parent).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(parent).State = EntityState.Detached; + } Assert.Equal( CoreStrings.CannotLoadDetached(nameof(Parent.Children), nameof(Parent)), @@ -5377,16 +5451,21 @@ public virtual void Query_collection_for_detached_throws() } } - [Fact] - public virtual void Query_collection_using_string_for_detached_throws() + [Theory] + [InlineData(true)] + [InlineData(false)] + public virtual void Query_collection_using_string_for_detached_throws(bool noTracking) { - using (var context = CreateContext()) + using (var context = CreateContext(noTracking: noTracking)) { var parent = context.Set().Single(); var collectionEntry = context.Entry(parent).Collection(nameof(Parent.Children)); - context.Entry(parent).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(parent).State = EntityState.Detached; + } Assert.Equal( CoreStrings.CannotLoadDetached(nameof(Parent.Children), nameof(Parent)), @@ -5394,16 +5473,21 @@ public virtual void Query_collection_using_string_for_detached_throws() } } - [Fact] - public virtual void Query_collection_with_navigation_for_detached_throws() + [Theory] + [InlineData(true)] + [InlineData(false)] + public virtual void Query_collection_with_navigation_for_detached_throws(bool noTracking) { - using (var context = CreateContext()) + using (var context = CreateContext(noTracking: noTracking)) { var parent = context.Set().Single(); var collectionEntry = context.Entry(parent).Navigation(nameof(Parent.Children)); - context.Entry(parent).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(parent).State = EntityState.Detached; + } Assert.Equal( CoreStrings.CannotLoadDetached(nameof(Parent.Children), nameof(Parent)), @@ -5411,16 +5495,21 @@ public virtual void Query_collection_with_navigation_for_detached_throws() } } - [Fact] - public virtual void Query_reference_to_principal_for_detached_throws() + [Theory] + [InlineData(true)] + [InlineData(false)] + public virtual void Query_reference_to_principal_for_detached_throws(bool noTracking) { - using (var context = CreateContext()) + using (var context = CreateContext(noTracking: noTracking)) { var child = context.Set().Single(e => e.Id == 12); var referenceEntry = context.Entry(child).Reference(e => e.Parent); - context.Entry(child).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(child).State = EntityState.Detached; + } Assert.Equal( CoreStrings.CannotLoadDetached(nameof(Child.Parent), nameof(Child)), @@ -5428,16 +5517,21 @@ public virtual void Query_reference_to_principal_for_detached_throws() } } - [Fact] - public virtual void Query_reference_with_navigation_to_principal_for_detached_throws() + [Theory] + [InlineData(true)] + [InlineData(false)] + public virtual void Query_reference_with_navigation_to_principal_for_detached_throws(bool noTracking) { - using (var context = CreateContext()) + using (var context = CreateContext(noTracking: noTracking)) { var child = context.Set().Single(e => e.Id == 12); var referenceEntry = context.Entry(child).Navigation(nameof(Child.Parent)); - context.Entry(child).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(child).State = EntityState.Detached; + } Assert.Equal( CoreStrings.CannotLoadDetached(nameof(Child.Parent), nameof(Child)), @@ -5445,16 +5539,21 @@ public virtual void Query_reference_with_navigation_to_principal_for_detached_th } } - [Fact] - public virtual void Query_reference_using_string_to_principal_for_detached_throws() + [Theory] + [InlineData(true)] + [InlineData(false)] + public virtual void Query_reference_using_string_to_principal_for_detached_throws(bool noTracking) { - using (var context = CreateContext()) + using (var context = CreateContext(noTracking: noTracking)) { var child = context.Set().Single(e => e.Id == 12); var referenceEntry = context.Entry(child).Reference(nameof(Child.Parent)); - context.Entry(child).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(child).State = EntityState.Detached; + } Assert.Equal( CoreStrings.CannotLoadDetached(nameof(Child.Parent), nameof(Child)), @@ -5462,16 +5561,21 @@ public virtual void Query_reference_using_string_to_principal_for_detached_throw } } - [Fact] - public virtual void Query_reference_to_dependent_for_detached_throws() + [Theory] + [InlineData(true)] + [InlineData(false)] + public virtual void Query_reference_to_dependent_for_detached_throws(bool noTracking) { - using (var context = CreateContext()) + using (var context = CreateContext(noTracking: noTracking)) { var parent = context.Set().Single(); var referenceEntry = context.Entry(parent).Reference(e => e.Single); - context.Entry(parent).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(parent).State = EntityState.Detached; + } Assert.Equal( CoreStrings.CannotLoadDetached(nameof(Parent.Single), nameof(Parent)), @@ -5479,16 +5583,21 @@ public virtual void Query_reference_to_dependent_for_detached_throws() } } - [Fact] - public virtual void Query_reference_to_dependent_with_navigation_for_detached_throws() + [Theory] + [InlineData(true)] + [InlineData(false)] + public virtual void Query_reference_to_dependent_with_navigation_for_detached_throws(bool noTracking) { - using (var context = CreateContext()) + using (var context = CreateContext(noTracking: noTracking)) { var parent = context.Set().Single(); var referenceEntry = context.Entry(parent).Navigation(nameof(Parent.Single)); - context.Entry(parent).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(parent).State = EntityState.Detached; + } Assert.Equal( CoreStrings.CannotLoadDetached(nameof(Parent.Single), nameof(Parent)), @@ -5496,16 +5605,21 @@ public virtual void Query_reference_to_dependent_with_navigation_for_detached_th } } - [Fact] - public virtual void Query_reference_to_dependent_using_string_for_detached_throws() + [Theory] + [InlineData(true)] + [InlineData(false)] + public virtual void Query_reference_to_dependent_using_string_for_detached_throws(bool noTracking) { - using (var context = CreateContext()) + using (var context = CreateContext(noTracking: noTracking)) { var parent = context.Set().Single(); var referenceEntry = context.Entry(parent).Reference(nameof(Parent.Single)); - context.Entry(parent).State = EntityState.Detached; + if (!noTracking) + { + context.Entry(parent).State = EntityState.Detached; + } Assert.Equal( CoreStrings.CannotLoadDetached(nameof(Parent.Single), nameof(Parent)), @@ -5825,11 +5939,15 @@ public Parent Parent } } - protected DbContext CreateContext(bool lazyLoadingEnabled = false) + protected DbContext CreateContext(bool lazyLoadingEnabled = false, bool noTracking = false) { var context = Fixture.CreateContext(); context.ChangeTracker.LazyLoadingEnabled = lazyLoadingEnabled; + context.ChangeTracker.QueryTrackingBehavior = noTracking + ? QueryTrackingBehavior.NoTracking + : QueryTrackingBehavior.TrackAll; + return context; } diff --git a/src/EFCore/Internal/LazyLoader.cs b/src/EFCore/Internal/LazyLoader.cs index f21b24309bf..603593b176f 100644 --- a/src/EFCore/Internal/LazyLoader.cs +++ b/src/EFCore/Internal/LazyLoader.cs @@ -60,16 +60,12 @@ public virtual void Load(object entity, [CallerMemberName] string navigationName } else if (Context.ChangeTracker.LazyLoadingEnabled) { - var entityEntry = Context.Entry(entity); - if (entityEntry.State != EntityState.Detached) + var entry = Context.Entry(entity).Navigation(navigationName); + if (!entry.IsLoaded) { - var entry = entityEntry.Navigation(navigationName); - if (!entry.IsLoaded) - { - Logger.NavigationLazyLoading(Context, entity, navigationName); + Logger.NavigationLazyLoading(Context, entity, navigationName); - entry.Load(); - } + entry.Load(); } } }