Skip to content

Commit

Permalink
Allow ignoring non-virtual navigations with lazy-loading proxies
Browse files Browse the repository at this point in the history
Part of #10787

Like the EF6 behavior, but opt-in.
  • Loading branch information
ajcvickers committed Dec 29, 2022
1 parent 0993a63 commit b0e47a8
Show file tree
Hide file tree
Showing 6 changed files with 165 additions and 15 deletions.
18 changes: 16 additions & 2 deletions src/EFCore.Proxies/Proxies/Internal/ProxiesOptionsExtension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public class ProxiesOptionsExtension : IDbContextOptionsExtension
{
private DbContextOptionsExtensionInfo? _info;
private bool _useLazyLoadingProxies;
private bool _ignoreNonVirtualNavigations;
private bool _useChangeTrackingProxies;
private bool _checkEquality;

Expand All @@ -38,6 +39,7 @@ public ProxiesOptionsExtension()
protected ProxiesOptionsExtension(ProxiesOptionsExtension copyFrom)
{
_useLazyLoadingProxies = copyFrom._useLazyLoadingProxies;
_ignoreNonVirtualNavigations = copyFrom._ignoreNonVirtualNavigations;
_useChangeTrackingProxies = copyFrom._useChangeTrackingProxies;
_checkEquality = copyFrom._checkEquality;
}
Expand Down Expand Up @@ -69,6 +71,15 @@ protected virtual ProxiesOptionsExtension Clone()
public virtual bool UseLazyLoadingProxies
=> _useLazyLoadingProxies;

/// <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 bool IgnoreNonVirtualNavigations
=> _ignoreNonVirtualNavigations;

/// <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
Expand Down Expand Up @@ -102,11 +113,12 @@ public virtual bool UseProxies
/// 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 ProxiesOptionsExtension WithLazyLoading(bool useLazyLoadingProxies = true)
public virtual ProxiesOptionsExtension WithLazyLoading(bool useLazyLoadingProxies, bool ignoreNonVirtualNavigations)
{
var clone = Clone();

clone._useLazyLoadingProxies = useLazyLoadingProxies;
clone._ignoreNonVirtualNavigations = ignoreNonVirtualNavigations;

return clone;
}
Expand All @@ -117,7 +129,7 @@ public virtual ProxiesOptionsExtension WithLazyLoading(bool useLazyLoadingProxie
/// 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 ProxiesOptionsExtension WithChangeTracking(bool useChangeTrackingProxies = true, bool checkEquality = true)
public virtual ProxiesOptionsExtension WithChangeTracking(bool useChangeTrackingProxies, bool checkEquality)
{
var clone = Clone();

Expand Down Expand Up @@ -187,6 +199,7 @@ public override int GetServiceProviderHashCode()
{
var hashCode = new HashCode();
hashCode.Add(Extension.UseLazyLoadingProxies);
hashCode.Add(Extension.IgnoreNonVirtualNavigations);
hashCode.Add(Extension.UseChangeTrackingProxies);
hashCode.Add(Extension.CheckEquality);
return hashCode.ToHashCode();
Expand All @@ -195,6 +208,7 @@ public override int GetServiceProviderHashCode()
public override bool ShouldUseSameServiceProvider(DbContextOptionsExtensionInfo other)
=> other is ExtensionInfo otherInfo
&& Extension.UseLazyLoadingProxies == otherInfo.Extension.UseLazyLoadingProxies
&& Extension.IgnoreNonVirtualNavigations == otherInfo.Extension.IgnoreNonVirtualNavigations
&& Extension.UseChangeTrackingProxies == otherInfo.Extension.UseChangeTrackingProxies
&& Extension.CheckEquality == otherInfo.Extension.CheckEquality;

Expand Down
19 changes: 12 additions & 7 deletions src/EFCore.Proxies/Proxies/Internal/ProxyBindingRewriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,20 @@ public virtual void ProcessModelFinalizing(

if (_options.UseLazyLoadingProxies)
{
if (!navigationBase.PropertyInfo.GetMethod!.IsReallyVirtual()
&& (!(navigationBase is INavigation navigation
&& navigation.ForeignKey.IsOwnership)))
if (!navigationBase.PropertyInfo.GetMethod!.IsReallyVirtual())
{
throw new InvalidOperationException(
ProxiesStrings.NonVirtualProperty(navigationBase.Name, entityType.DisplayName()));
if (!_options.IgnoreNonVirtualNavigations
&& !(navigationBase is INavigation navigation
&& navigation.ForeignKey.IsOwnership))
{
throw new InvalidOperationException(
ProxiesStrings.NonVirtualProperty(navigationBase.Name, entityType.DisplayName()));
}
}
else
{
navigationBase.SetPropertyAccessMode(PropertyAccessMode.Field);
}

navigationBase.SetPropertyAccessMode(PropertyAccessMode.Field);
}
}
}
Expand Down
16 changes: 13 additions & 3 deletions src/EFCore.Proxies/ProxiesExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,20 @@ public static DbContextOptionsBuilder<TContext> UseChangeTrackingProxies<TContex
/// or exposed AddDbContext.
/// </param>
/// <param name="useLazyLoadingProxies"><see langword="true" /> to use lazy loading proxies; <see langword="false" /> to prevent their use.</param>
/// <param name="ignoreNonVirtualNavigations">
/// <see langword="true" /> to ignore navigations that are not virtual. The default value is
/// <see langword="false" />, meaning an exception will be thrown if a non-virtual navigation is found.
/// </param>
/// <returns>The same builder to allow method calls to be chained.</returns>
public static DbContextOptionsBuilder UseLazyLoadingProxies(
this DbContextOptionsBuilder optionsBuilder,
bool useLazyLoadingProxies = true)
bool useLazyLoadingProxies = true,
bool ignoreNonVirtualNavigations = false)
{
var extension = optionsBuilder.Options.FindExtension<ProxiesOptionsExtension>()
?? new ProxiesOptionsExtension();

extension = extension.WithLazyLoading(useLazyLoadingProxies);
extension = extension.WithLazyLoading(useLazyLoadingProxies, ignoreNonVirtualNavigations);

((IDbContextOptionsBuilderInfrastructure)optionsBuilder).AddOrUpdateExtension(extension);

Expand All @@ -139,10 +144,15 @@ public static DbContextOptionsBuilder UseLazyLoadingProxies(
/// or exposed AddDbContext.
/// </param>
/// <param name="useLazyLoadingProxies"><see langword="true" /> to use lazy loading proxies; <see langword="false" /> to prevent their use.</param>
/// <param name="ignoreNonVirtualNavigations">
/// <see langword="true" /> to ignore navigations that are not virtual. The default value is
/// <see langword="false" />, meaning an exception will be thrown if a non-virtual navigation is found.
/// </param>
/// <returns>The same builder to allow method calls to be chained.</returns>
public static DbContextOptionsBuilder<TContext> UseLazyLoadingProxies<TContext>(
this DbContextOptionsBuilder<TContext> optionsBuilder,
bool useLazyLoadingProxies = true)
bool useLazyLoadingProxies = true,
bool ignoreNonVirtualNavigations = false)
where TContext : DbContext
=> (DbContextOptionsBuilder<TContext>)UseLazyLoadingProxies((DbContextOptionsBuilder)optionsBuilder, useLazyLoadingProxies);

Expand Down
17 changes: 17 additions & 0 deletions test/EFCore.Proxies.Tests/LazyLoadingProxyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ public void Throws_if_non_virtual_navigation_to_non_owned_type()
() => context.Model).Message);
}

[ConditionalFact]
public void Does_not_throw_if_non_virtual_navigation_to_non_owned_type_is_allowed()
{
using var context = new LazyContextIgnoreVirtuals<LazyNonVirtualNavEntity>();
Assert.NotNull(
context.Model.FindEntityType(typeof(LazyNonVirtualNavEntity))!.FindNavigation(nameof(LazyNonVirtualNavEntity.SelfRef)));
}

[ConditionalFact]
public void Does_not_throw_if_non_virtual_navigation_to_owned_type()
{
Expand Down Expand Up @@ -82,6 +90,15 @@ public void Throws_when_context_is_disposed()
() => phone.Texts).Message);
}

private class LazyContextIgnoreVirtuals<TEntity> : TestContext<TEntity>
where TEntity : class
{
public LazyContextIgnoreVirtuals()
: base(dbName: "LazyLoadingContext", useLazyLoading: true, useChangeDetection: false, ignoreNonVirtualNavigations: true)
{
}
}

private class LazyContext<TEntity> : TestContext<TEntity>
where TEntity : class
{
Expand Down
7 changes: 5 additions & 2 deletions test/EFCore.Proxies.Tests/TestUtilities/TestContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ internal abstract class TestContext<TEntity> : DbContext
private readonly IServiceProvider _internalServiceProvider;
private readonly string _dbName;
private readonly bool _useLazyLoadingProxies;
private readonly bool _ignoreNonVirtualNavigations;
private readonly bool _useChangeDetectionProxies;
private readonly bool _checkEquality;
private readonly ChangeTrackingStrategy? _changeTrackingStrategy;
Expand All @@ -20,7 +21,8 @@ protected TestContext(
bool useLazyLoading = false,
bool useChangeDetection = false,
bool checkEquality = true,
ChangeTrackingStrategy? changeTrackingStrategy = null)
ChangeTrackingStrategy? changeTrackingStrategy = null,
bool ignoreNonVirtualNavigations = false)
{
_internalServiceProvider
= new ServiceCollection()
Expand All @@ -33,13 +35,14 @@ protected TestContext(
_useChangeDetectionProxies = useChangeDetection;
_checkEquality = checkEquality;
_changeTrackingStrategy = changeTrackingStrategy;
_ignoreNonVirtualNavigations = ignoreNonVirtualNavigations;
}

protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
{
if (_useLazyLoadingProxies)
{
optionsBuilder.UseLazyLoadingProxies();
optionsBuilder.UseLazyLoadingProxies(ignoreNonVirtualNavigations: _ignoreNonVirtualNavigations);
}

if (_useChangeDetectionProxies)
Expand Down
103 changes: 102 additions & 1 deletion test/EFCore.Specification.Tests/LazyLoadProxyTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,74 @@ public virtual void Setting_reference_to_owned_type_to_null_is_allowed_on_virtua
Assert.Null(owner.Address);
}

[ConditionalFact]
public virtual void Non_virtual_one_to_one_reference_to_principal_is_not_lazy_loaded()
{
using var context = CreateContext(lazyLoadingEnabled: true);

var child = context.Set<NonVirtualChild>().Single(e => e.SingleParent != null);

Assert.Null(child.SingleParent);
context.Entry(child).Reference(e => e.SingleParent).Load();
Assert.NotNull(child.SingleParent);

child.SingleParent = null;
Assert.Null(child.SingleParent);
context.ChangeTracker.DetectChanges();
Assert.Null(child.SingleParent);
}

[ConditionalFact]
public virtual void Non_virtual_one_to_many_reference_to_principal_is_not_lazy_loaded()
{
using var context = CreateContext(lazyLoadingEnabled: true);

var child = context.Set<NonVirtualChild>().Single(e => e.CollectionParent != null);

Assert.Null(child.CollectionParent);
context.Entry(child).Reference(e => e.CollectionParent).Load();
Assert.NotNull(child.CollectionParent);

child.CollectionParent = null;
Assert.Null(child.CollectionParent);
context.ChangeTracker.DetectChanges();
Assert.Null(child.CollectionParent);
}

[ConditionalFact]
public virtual void Non_virtual_reference_to_dependent_is_not_lazy_loaded()
{
using var context = CreateContext(lazyLoadingEnabled: true);

var parent = context.Set<NonVirtualParent>().Single();

Assert.Null(parent.Child);
context.Entry(parent).Reference(e => e.Child).Load();
Assert.NotNull(parent.Child);

parent.Child = null;
Assert.Null(parent.Child);
context.ChangeTracker.DetectChanges();
Assert.Null(parent.Child);
}

[ConditionalFact]
public virtual void Non_virtual_collection_is_not_lazy_loaded()
{
using var context = CreateContext(lazyLoadingEnabled: true);

var parent = context.Set<NonVirtualParent>().Single();

Assert.Null(parent.Children);
context.Entry(parent).Collection(e => e.Children).Load();
Assert.Single(parent.Children!);

parent.Children.Clear();
Assert.Empty(parent.Children);
context.ChangeTracker.DetectChanges();
Assert.Empty(parent.Children);
}

[ConditionalTheory]
[InlineData(EntityState.Unchanged)]
[InlineData(EntityState.Added)]
Expand Down Expand Up @@ -3317,6 +3385,24 @@ public class Quest : Tribe
public virtual Called Called { set; get; }
}

public class NonVirtualParent
{
[DatabaseGenerated(DatabaseGeneratedOption.None)]
public int Id { get; set; }

public NonVirtualChild Child { get; set; }
public List<NonVirtualChild> Children { get; set; }
}

public class NonVirtualChild
{
[DatabaseGenerated(DatabaseGeneratedOption.None)]
public int Id { get; set; }

public NonVirtualParent SingleParent { get; set; }
public NonVirtualParent CollectionParent { get; set; }
}

public class NonVirtualOneToOneOwner
{
[DatabaseGenerated(DatabaseGeneratedOption.None)]
Expand Down Expand Up @@ -3444,7 +3530,7 @@ protected override string StoreName
=> "LazyLoadProxyTest";

public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
=> base.AddOptions(builder.UseLazyLoadingProxies());
=> base.AddOptions(builder.UseLazyLoadingProxies(ignoreNonVirtualNavigations: true));

protected override IServiceCollection AddServices(IServiceCollection serviceCollection)
=> base.AddServices(
Expand Down Expand Up @@ -3618,6 +3704,13 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
.OwnsMany(o => o.Addresses, a => a.HasKey("Id"));
modelBuilder.Entity<ExplicitLazyLoadVirtualOneToManyOwner>()
.OwnsMany(o => o.Addresses, a => a.HasKey("Id"));

modelBuilder.Entity<NonVirtualParent>(
b =>
{
b.HasOne(e => e.Child).WithOne(e => e.SingleParent).HasPrincipalKey<NonVirtualChild>();
b.HasMany(e => e.Children).WithOne(e => e.CollectionParent);
});
}

protected override void Seed(DbContext context)
Expand Down Expand Up @@ -3736,6 +3829,14 @@ protected override void Seed(DbContext context)
Id = 600, Addresses = new List<OwnedAddress> { new() { Street = "12 Grimmauld Place", PostalCode = "L0N D0N" } }
});

context.Add(
new NonVirtualParent
{
Id = 100,
Child = new() { Id = 100 },
Children = new() { new() { Id = 101 } }
});

context.SaveChanges();
}
}
Expand Down

0 comments on commit b0e47a8

Please sign in to comment.