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

Question: Best practices for providing encapsulation of collection navigation properties with lazy loading #22752

Closed
JamesMenetrey opened this issue Sep 25, 2020 · 3 comments
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported

Comments

@JamesMenetrey
Copy link

Question

Dear EF Core community,

I would like to provide encapsulation with my collection navigation properties while using lazy loading. For that purpose, I usually expose an IReadOnlyList for my public property while being backing by a backing field. EF Core does not lazy load the value of my backing field, so querying it result of an empty list. What are the best practices in EF Core 3 or 5 to gatekeep the alterations of my collections while using lazy loading? I provide a small example below to work on.

Code

    public class Foo
    {
        private readonly List<Bar> _bars; // I would like to gatekeep the changes of that collection

        public Foo(long id)
        {
            Id = id;
            _bars = new List<Bar>();
        }

        public virtual IReadOnlyList<Bar> Bars => _bars.AsReadOnly();

        public long Id { get; set; }

        public int NumberOfBars => _bars.Count; // A dummy example of extracting some info from the backing field. Note that in reality, this may alter the collection.

        public void AddBar(Bar bar) // This method gatekeeps the changes in the collection
        {
            _bars.Add(bar);
        }
    }

    public class Bar
    {
        public long Id { get; set; }

        public Bar(long id)
        {
            Id = id;
        }
    }
    private static void Main(string[] args)
    {
        using (var db = new DbContext())
        {
            var foo = new Foo(1);
            foo.AddBar(new Bar(1));
            foo.AddBar(new Bar(2));

            db.Foos.Add(foo);

            db.SaveChanges();
        }

        using (var db = new DbContext())
        {
            var f = db.Foos.First();
            Console.WriteLine(f.NumberOfBars); // Print 0, while I would like 2 to be printed
        }
    }

Output

The application prints 0, while 2 is printed when querying the navigation property instead. I understand this is due to how lazy loading is working, by subclassing Foo at runtime. I would like to know the recommended way to structure my classes to provide encapsulation while still using lazy loading. Feel free to change my structure to orient me in the right direction.

Version information

EF Core version: 3.1.8
Database provider: PostgreSQL
Target framework: .NET Core 3.1
Operating system: Windows
IDE: Visual Studio 2019 16.7.2

Thanks!

@smitpatel
Copy link
Contributor

You would need to integrate lazy loading inside your class manually rather than relying on proxies. See https://docs.microsoft.com/en-us/ef/core/querying/related-data/lazy#lazy-loading-without-proxies for more information.

@JamesMenetrey
Copy link
Author

Hey @smitpatel,

Many thanks for your answer. I try to emphasize a clean DDD implementation, where I want to avoid extra infrastructure concerns while modeling my domain layer. Injecting a lazy loader interface or delegate in every model would be too much of contamination from my perspective.

After playing around with EF Core, I came to this idea of implementation:

public class Foo
{
    public const string BarsInternName = nameof(BarsIntern);

    protected Foo() { }
    public Foo(long id) { Id = id; }

    public IReadOnlyList<Bar> Bars => BarsIntern.AsReadOnly();
    protected virtual List<Bar> BarsIntern { get; set; } = new List<Bar>();

    public long Id { get; set; }
    public int NumberOfBars => BarsIntern.Count;

    public void AddBar(Bar bar) { BarsIntern.Add(bar); }
}

public class Bar
{
    public Bar(long id) { Id = id; }

    public long Id { get; set; }
}

internal class MyDbContext : DbContext
{
    public DbSet<Foo> Foos { get; set; }

    protected override void OnModelCreating(ModelBuilder builder)
    {
        base.OnModelCreating(builder);

        builder.Entity<Foo>()
            .Ignore(x => x.Bars);

        builder.Entity<Foo>()
            .HasMany(Foo.BarsInternName)
            .WithOne();
    }
}

The idea is to rely on a protected navigation property that can then be overridden by the lazy loading proxy. The public property is no longer a navigation property, but just a way that other classes can read the collection. I'm exposing the name of the protected property using a public static constant to configure the context in a type-safe manner.

What do you think about this approach? Any drawback?

Thanks!

@ajcvickers
Copy link
Contributor

@ZenLulz This won't work if you want to use Foo.Bars in a query since it is no longer configured as a navigation property.

I try to emphasize a clean DDD implementation, where I want to avoid extra infrastructure concerns while modeling my domain layer. Injecting a lazy loader interface or delegate in every model would be too much of contamination from my perspective.

It seems to me that the code you showed above is putting in a lot of extra infrastructure in order to use lazy-loading proxies. In other words, you're doing a lot of work to make your code to fit lazy loading with the proxies. It seems much more appropriate to do what @smitpatel suggested and have the lazy-loading delegate injected. For example:

public class Foo
{
    private List<Bar> _bars;
    private readonly Action<object, string> _lazyLoader;

    public Foo(long id)
    {
        Id = id;
        _bars = new List<Bar>();
    }

    private Foo(Action<object, string> lazyLoader, long id)
        : this(id)
    {
        _lazyLoader = lazyLoader;
    }

    private List<Bar> InternalBars => _lazyLoader.Load(this, ref _bars, nameof(Bars));

    public virtual IReadOnlyList<Bar> Bars => InternalBars.AsReadOnly();

    public long Id { get; set; }

    public int NumberOfBars => InternalBars.Count;

    public void AddBar(Bar bar)
    {
        InternalBars.Add(bar);
    }
}

public class Bar
{
    public long Id { get; set; }

    public Bar(long id)
    {
        Id = id;
    }
}

public static class PocoLoadingExtensions
{
    public static TRelated Load<TRelated>(
        this Action<object, string> loader,
        object entity,
        ref TRelated navigationField,
        [CallerMemberName] string navigationName = null)
        where TRelated : class
    {
        loader?.Invoke(entity, navigationName);

        return navigationField;
    }
}

This doesn't change the public API of your entities at all or change their functionality when constructed outside of EF, but still allows lazy-loading functionality to be injected when EF creates entity instances. It also doesn't bring in any extra dependencies.

@ajcvickers ajcvickers added the closed-no-further-action The issue is closed and no further action is planned. label Oct 5, 2020
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported
Projects
None yet
Development

No branches or pull requests

3 participants