Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow ignoring non-virtual navigations with lazy-loading proxies #29944

Merged
merged 1 commit into from
Dec 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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