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

Opt-out of lazy-loading for specific navigations #10787

Closed
Tracked by #22954
ajcvickers opened this issue Jan 27, 2018 · 16 comments · Fixed by #30082
Closed
Tracked by #22954

Opt-out of lazy-loading for specific navigations #10787

ajcvickers opened this issue Jan 27, 2018 · 16 comments · Fixed by #30082
Labels
area-change-tracking area-proxies area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. ef6-parity good first issue This issue should be relatively straightforward to fix. needs-design punted-for-3.0 type-enhancement
Milestone

Comments

@ajcvickers
Copy link
Member

Currently when lazy-loading proxies are used every entity type in the model must be suitable to proxy and all navigations must be virtual. This issue is about allowing some entity types/navigations to be lazy-loaded while others are not.

It is not clear how important this is. If this is something you would use, then please comment and describe the scenario.

@mikke49
Copy link

mikke49 commented May 9, 2018

Our current scenario is using EF.CORE against SQLServer w/temporal tables AND the use of owned entity types. Without using lazy loading this works as expected, but as soon as lazy loading is enabled it all fall to pieces.

Short, enabling lazy loading forces us to declare all owned entity types' "navigation" properties as virtual, which in turn makes EF - through the use of LL proxies - end up with a lot of "self-joins" on the table with the owned entity types. Without temporal queries this works, though unescessary, but when temporal query clauses are introduced it makes a real mess and a lot of hazzle.

Allowing for some sort of conditional lazy loading, preferably by using / not using the virtual keyword, would solve the problem in an elegant way.

@ajcvickers
Copy link
Member Author

@mikke49 It's not at all clear to me how lazy-loading is causing the issues you are seeing. Can you please file a new issue with complete details and include a runnable project/solution or complete code listing that demonstrates the behavior you are seeing?

@mikke49
Copy link

mikke49 commented May 15, 2018

@ajcvickers Thanks for your reply. I'll try to be brief since we've found a solution.

The original problem we experienced was with this scenario:

  • having an entity with several owned entities (e.g. Person.Address with Address having Street, Zip etc.)
  • using FromSql to build a base query in order to use the FOR SYSTEM_TIME clause for history quering
  • having lazy loading enabled

With this scenario we got a NullreferenceException and the following stack trace:

   at lambda_method(Closure , ValueBuffer )
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalMixedEntityEntry..ctor(IStateManager stateManager, IEntityType entityType, Object entity, ValueBuffer& valueBuffer)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntryFactory.NewInternalEntityEntry(IStateManager stateManager, IEntityType entityType, Object entity, ValueBuffer& valueBuffer)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntryFactory.Create(IStateManager stateManager, IEntityType entityType, Object entity, ValueBuffer& valueBuffer)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.StartTrackingFromQuery(IEntityType baseEntityType, Object entity, ValueBuffer& valueBuffer, ISet`1 handledForeignKeys)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryBuffer.StartTracking(Object entity, IEntityType entityType)
   at lambda_method(Closure , QueryContext , Customer , Object[] )
   at Microsoft.EntityFrameworkCore.Query.Internal.IncludeCompiler._Include[TEntity](QueryContext queryContext, TEntity entity, Object[] included, Action`3 fixup)
   at lambda_method(Closure , TransparentIdentifier`2 )
   at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.<_TrackEntities>d__17`2.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.ExceptionInterceptor`1.EnumeratorExceptionInterceptor.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Parka.Core.Dal.Tests.ParkaContextTests.AccessCustomerHistory() in D:\_DATA\Oppdrag\Parka6\Parka.Core\Parka.Core.Dal.Tests\ParkaContextTests.cs:line 118

Disabling lazy loading removed the exception and it worked. We never digged into the details for the null reference exception, though.

However, what we saw was that the use of FromSql made the final query contain one Left Join to the base table for each owned entity. This is unfortunate, and we assumed that this somehow also affected the logic for the lazy loading. We have now solved this and avoid the use of FromSql completely.

What we do now is extend LINQ with new methods for history quering, and tap into the framwork and modify the SQL generation to create properly formatted history queries without using sub-queries. The firsts testes show that this also removed the "self-joins", and it all seems to work. Some more testing to do, but we believe this will be our working solution.

Thanks for your time.

@ajcvickers
Copy link
Member Author

@mikke49 Looks like you were hitting #11945

@iliveoncaffiene
Copy link

iliveoncaffiene commented Jun 25, 2018

@ajcvickers To add on this issue (and possibly clarify what I think @mikke49 was describing):
I have an entity:

    public class Product : AuditedEntity, INamedEntity
    {
        public string Name { get; set; }

        public int CompanyId { get; set; }

        public virtual Company Company { get; set; }

        /// <summary>
        /// Complex (owned) type - not a foreign key
        /// </summary>
        public ProductDimension Dimensions { get; set; }
    }

which has the setup:

    public class ProductMap : AuditedEntityMap<Product>
    {
        public override void Configure(EntityTypeBuilder<Product> builder)
        {
            base.Configure(builder);

            builder.MapName();

            builder.HasOne(x => x.Company)
                   .WithMany(x => x.Products)
                   .HasForeignKey(x => x.CompanyId)
                   .IsRequired()
                   .OnDelete(DeleteBehavior.Restrict);

            builder.OwnsOne(x => x.Dimensions);
        }
    }

and the owned type, ProductDimension is described here:

    public class ProductDimension
    {
        public decimal Decimal { get; set; }

        public ToleranceType DecimalToleranceType { get; set; }

        public decimal? DecimalTolerance { get; set; }

        public decimal Width { get; set; }

        public ToleranceType WidthToleranceType { get; set; }

        public decimal? WidthTolerance { get; set; }

        public decimal Length { get; set; }

        public ToleranceType LengthToleranceType { get; set; }

        public decimal? LengthTolerance { get; set; }

        public int MaxCoilSkidWeight { get; set; }

        public int? CoilID { get; set; }

        public int? MaxCoilOD { get; set; }
    }

(NOTE: ToleranceType is an enum)

I have lazy loading setup and am using SQL Server.
If I try to add a migration like this, I have this exception:

System.InvalidOperationException: Navigation property 'Dimensions' on entity type 'Product' is not virtual. UseLazyLoadingProxies requires all entity types to be public, unsealed, have virtual navigation properties, and have a public or protected constructor.
   at Microsoft.EntityFrameworkCore.Proxies.Internal.ProxyBindingRewriter.Apply(InternalModelBuilder modelBuilder)
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.ImmediateConventionScope.OnModelBuilt(InternalModelBuilder modelBuilder)
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.OnModelBuilt(InternalModelBuilder modelBuilder)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.Model.Validate()
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.CreateModel(DbContext context, IConventionSetBuilder conventionSetBuilder, IModelValidator validator)
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.<>c__DisplayClass5_0.<GetModel>b__1()
   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
   at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
   at System.Lazy`1.CreateValue()
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.GetModel(DbContext context, IConventionSetBuilder conventionSetBuilder, IModelValidator validator)
   at Microsoft.EntityFrameworkCore.Internal.DbContextServices.CreateModel()
   at Microsoft.EntityFrameworkCore.Internal.DbContextServices.get_Model()
   at Microsoft.EntityFrameworkCore.Infrastructure.EntityFrameworkServicesBuilder.<>c.<TryAddCoreServices>b__7_1(IServiceProvider p)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitFactory(FactoryCallSite factoryCallSite, ServiceProviderEngineScope scope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(IServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitScoped(ScopedCallSite scopedCallSite, ServiceProviderEngineScope scope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(IServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitConstructor(ConstructorCallSite constructorCallSite, ServiceProviderEngineScope scope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(IServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitScoped(ScopedCallSite scopedCallSite, ServiceProviderEngineScope scope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(IServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine.<>c__DisplayClass1_0.<RealizeService>b__0(ServiceProviderEngineScope scope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngine.GetService(Type serviceType, ServiceProviderEngineScope serviceProviderEngineScope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngineScope.GetService(Type serviceType)
   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService(IServiceProvider provider, Type serviceType)
   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService[T](IServiceProvider provider)
   at Microsoft.EntityFrameworkCore.DbContext.get_DbContextDependencies()
   at Microsoft.EntityFrameworkCore.DbContext.get_InternalServiceProvider()
   at Microsoft.EntityFrameworkCore.DbContext.Microsoft.EntityFrameworkCore.Infrastructure.IInfrastructure<System.IServiceProvider>.get_Instance()
   at Microsoft.EntityFrameworkCore.Internal.InternalAccessorExtensions.GetService[TService](IInfrastructure`1 accessor)
   at Microsoft.EntityFrameworkCore.Infrastructure.AccessorExtensions.GetService[TService](IInfrastructure`1 accessor)
   at Microsoft.EntityFrameworkCore.Design.Internal.DbContextOperations.CreateContext(Func`1 factory)
   at Microsoft.EntityFrameworkCore.Design.Internal.DbContextOperations.CreateContext(String contextType)
   at Microsoft.EntityFrameworkCore.Design.Internal.MigrationsOperations.AddMigration(String name, String outputDir, String contextType)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.AddMigrationImpl(String name, String outputDir, String contextType)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.AddMigration.<>c__DisplayClass0_1.<.ctor>b__0()
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.<>c__DisplayClass3_0`1.<Execute>b__0()
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.Execute(Action action)
Navigation property 'Dimensions' on entity type 'Product' is not virtual. UseLazyLoadingProxies requires all entity types to be public, unsealed, have virtual navigation properties, and have a public or protected constructor.

Which is unexpected (and frankly, incorrect) because ProductDimension is specified as an Owned type, and as such should not require lazy loading.
Switching the property to public virtual ProductDimension Dimensions { get; set; } does not generate the exception.
NOTE: Also after switching to a virtual property, EF still correctly maps the properties as an owned type (the properties are all on the Product table).

@ajcvickers
Copy link
Member Author

@mikke49 Thanks for the feedback. I have created #12462 for cases where we configure lazy loading even if the navigation property will always be eager loaded. However, it is not the case that an owned type will never require lazy-loading since it may contain a navigation property to some other entity type, and this should still have lazy-loading semantics.

@bugproof
Copy link

bugproof commented Aug 10, 2018

I think it's important because currently, it forces you to change all your models. IIRC EF6 worked that way it didn't require all properties to be marked as virtual. Using ILazyLoader and injecting it in your entities seems like an anti-pattern to me(is it?). If you use DDD and don't have a separate layer for your domain(that is: your EF entities are your domain models)

@cculver
Copy link

cculver commented Oct 25, 2018

Here might be a use case that I'm currently running into. I'm using zzzprojects' entity framework plus to add audit tables to my project. It's a third-party library for which I don't (but theoretically could, let's just say I don't) have access to source. If I use this third-party dll, which was not configured for lazy loading entities, when I enable lazy loading I get an exception stating that these entities must be virtual. I have no way to change that. Since there will (hopefully) be many third-party libraries providing entities and code, I'd say that it may behoove the team to come up with a way for us to enable lazy loading for our own entities while releasing other entities of the requirement to do so.

@awaitJosh
Copy link

awaitJosh commented Jan 8, 2019

I don´t know how long it needs to implement many-to-many support without join-table - but as long as this is not supported @ajcvickers great workaround (https://blog.oneunicorn.com/2017/09/25/many-to-many-relationships-in-ef-core-2-0-part-4-a-more-general-abstraction/) cannot be used as is, because of the private Navigation to the join-table.

Since it is possible to workaround this by using a public virtual ICollection<PostTags> I consider this a minor issue.

@ajcvickers ajcvickers added the good first issue This issue should be relatively straightforward to fix. label Sep 2, 2019
ajcvickers added a commit that referenced this issue Dec 29, 2022
Part of #10787

Also fixes two issues with service properties and their delegates:

- The delegate is now always created pointing to the instance of the service property stored in the entity. Previously, it could sometimes be a delegate pointing to a new service instance.
- The service property instance is always treated as an IInjectableService when attaching or materializing, so the metadata is always present on the instance.
ajcvickers added a commit that referenced this issue Dec 29, 2022
ajcvickers added a commit that referenced this issue Dec 31, 2022
Part of #10787

Also fixes two issues with service properties and their delegates:

- The delegate is now always created pointing to the instance of the service property stored in the entity. Previously, it could sometimes be a delegate pointing to a new service instance.
- The service property instance is always treated as an IInjectableService when attaching or materializing, so the metadata is always present on the instance.
@ajcvickers ajcvickers removed this from the Backlog milestone Jan 1, 2023
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jan 5, 2023
@ajcvickers ajcvickers added this to the 8.0.0 milestone Jan 5, 2023
@ajcvickers ajcvickers changed the title Lazy-loading proxies: allow entity types/navigations to be specified Opt-out of lazy-loading for specific navigations Jan 5, 2023
@sprengo
Copy link

sprengo commented Jan 17, 2023

Field only navigations will still throw an exception in ProxyBindingRewriter. Is that intentional?

@ajcvickers
Copy link
Member Author

@sprengo Even if they are configured to not lazy-load? Or if proxies are configured to ignore virtual properties?

@sprengo
Copy link

sprengo commented Jan 17, 2023

Yes, that's because this check

if (navigationBase.PropertyInfo == null)
{
    throw new InvalidOperationException(
        ProxiesStrings.FieldProperty(navigationBase.Name, entityType.DisplayName()));
}

comes before your mentioned checks.

ajcvickers added a commit that referenced this issue Jan 17, 2023
…the navigation

Also when non-virtual navigations are allowed.

Fixes #10787 (comment)
@ajcvickers ajcvickers modified the milestones: 8.0.0, 8.0.0-preview1 Jan 29, 2023
@ajcvickers ajcvickers modified the milestones: 8.0.0-preview1, 8.0.0 Nov 14, 2023
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-change-tracking area-proxies area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. ef6-parity good first issue This issue should be relatively straightforward to fix. needs-design punted-for-3.0 type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.