LazyLoad no Virtual

[ralmsdeveloper]

@ajcvickers would like to know what impact this would cause.
I've been working on something where not necessarily I would have "Virtual" type navigators, where today we are forced to declare everything as "Virtual" by activating LazyLoad.
I made a little adjustment where everything works perfectly for me.

Here is an example:

public virtual InternalModelBuilder Apply(InternalModelBuilder modelBuilder)
{
    if (_options?.UseLazyLoadingProxies == true)
    {
        foreach (var entityType in modelBuilder.Metadata.GetEntityTypes())
        {
            if (entityType.ClrType != null
                && !entityType.ClrType.IsAbstract
                && entityType.GetNavigations().Any(p => p.PropertyInfo.GetMethod.IsVirtual))
            {
                if (entityType.ClrType.IsSealed)
                {
                    throw new InvalidOperationException(ProxiesStrings.ItsASeal(entityType.DisplayName()));
                }

                var proxyType = _proxyFactory.CreateLazyLoadingProxyType(entityType);

                var serviceProperty = entityType.GetServiceProperties().FirstOrDefault(e => e.ClrType == typeof(ILazyLoader));
                if (serviceProperty == null)
                {
                    serviceProperty = entityType.AddServiceProperty(_lazyLoaderProperty, ConfigurationSource.Convention);
                    serviceProperty.SetParameterBinding(
                        (ServiceParameterBinding)new LazyLoaderParameterBindingFactory().Bind(
                            entityType,
                            typeof(ILazyLoader),
                            nameof(IProxyLazyLoader.LazyLoader)));
                }

                var binding = (ConstructorBinding)entityType[CoreAnnotationNames.ConstructorBinding];
                if (binding == null)
                {
                    _directBindingConvention.Apply(modelBuilder);
                }

                binding = (ConstructorBinding)entityType[CoreAnnotationNames.ConstructorBinding];

                entityType[CoreAnnotationNames.ConstructorBinding]
                    = new FactoryMethodConstructorBinding(
                        _proxyFactory,
                        _createLazyLoadingProxyMethod,
                        new List<ParameterBinding>
                        {
                            new EntityTypeParameterBinding(),
                            new DefaultServiceParameterBinding(typeof(ILazyLoader), typeof(ILazyLoader), serviceProperty),
                            new ObjectArrayParameterBinding(binding.ParameterBindings)
                        },
                        proxyType);

                foreach (var navigation in entityType.GetNavigations())
                {
                    if (navigation.PropertyInfo == null)
                    {
                        throw new InvalidOperationException(
                            ProxiesStrings.FieldNavigation(navigation.Name, entityType.DisplayName()));
                    }

                    if (navigation.PropertyInfo.GetMethod.IsVirtual)
                    {
                        navigation.SetPropertyAccessMode(PropertyAccessMode.Field);
                    }
                    
                }
            }
        }
    }

    return modelBuilder;
}

I added just this:
entityType.GetNavigations().Any(p => p.PropertyInfo.GetMethod.IsVirtual))
And:

 if (navigation.PropertyInfo.GetMethod.IsVirtual)
 {
    navigation.SetPropertyAccessMode(PropertyAccessMode.Field);
 }

I believe that with this it would not be necessary to force the user to define a virtual type navigation property.

Thank you in advance.

[divega]

@ralmsdeveloper the requirement that all navigation properties are virtual if you enable lazy loading proxies is by design in EF Core 2.1. Our experience with the same feature in EF6 has made prefer a stricter approach: Unintentionally forgetting to make your navigation properties virtual and not realizing that lazy loading wasn't going to kick in until much later seemed to be much more common than wanting to do lazy loading only on some entities.

We are open to provide more granular control in the future, but we would like to get more customer feedback before we decide in what direction we move. E.g.:

• we could add a way to explicitly override proxy creation requirements on specific entity types or
• a way to enable lazy loading proxy creation on a best-effort basis only (which is what I think you have implemented)

It would be useful if you could explain what option would work best for you and why.

In the meanwhile, you are free to use a modified version of our proxy logic, or to use the lower level building blocks (e.g. ILazyLoader or delegate injection) in your application.

[ralmsdeveloper]

@divega Thank you so much!

You believe there might be the possibility of implementing something like:

builder.Property(p => p.Navegator).NoLazyLoad()

So it would be more flexible, the user would have the possibility to configure which properties would have LazyLoad.

I know and I think all your efforts are very important, I realize that the team has given the best.

[ralmsdeveloper]

I know that we currently have the possibility of: ILazyLoader or delegate injection
But this is more complex and dirty the code somehow, note that I am not criticizing, I am just making a comment so that there is the possibility of leaving a code clean, without the need to implement the ILazyLoader manually.

[divega]

@ralmsdeveloper sure we could provide granular control at the navigation property level, assuming that is what you meant.

No offense taken 😄 Service and delegate injection give you complete control at the cost of more complexity in your entity types.

[ralmsdeveloper]

Yes, you're right.
Should I keep this conversation open? So we can track all the talk about this possibility here, or if you wish I can close it.

[ajcvickers]

Cover by #10787?

[ralmsdeveloper]

All right then, I'll follow up on this issue #10787
Thanks!