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

Lazy-loading proxies: allow entity types/navigations to be specified #10787

Open
ajcvickers opened this Issue Jan 27, 2018 · 8 comments

Comments

Projects
None yet
6 participants
@ajcvickers
Member

ajcvickers commented Jan 27, 2018

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

This comment has been minimized.

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

This comment has been minimized.

Member

ajcvickers commented May 9, 2018

@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

This comment has been minimized.

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

This comment has been minimized.

Member

ajcvickers commented May 15, 2018

@mikke49 Looks like you were hitting #11945

@iliveoncaffiene

This comment has been minimized.

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

This comment has been minimized.

Member

ajcvickers commented Jun 25, 2018

@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.

@ajcvickers ajcvickers modified the milestones: 3.0.0, Backlog Aug 3, 2018

@ajcvickers ajcvickers removed the propose-punt label Aug 3, 2018

@Sigvaard

This comment has been minimized.

Sigvaard 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

This comment has been minimized.

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.

@ajcvickers ajcvickers removed the size~2-weeks label Nov 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment