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

Custom proxy generation for entities #14554

Closed
mortenbock opened this issue Jan 30, 2019 · 6 comments · Fixed by #15630 or #28127
Closed

Custom proxy generation for entities #14554

mortenbock opened this issue Jan 30, 2019 · 6 comments · Fixed by #15630 or #28127
Labels
area-proxies closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-3.0 type-enhancement
Milestone

Comments

@mortenbock
Copy link

I would like to generate my own proxies for the entities returned by EF.

The particular feature I would like to be able to implement, is similar to the LazyLoading proxies. But instead of lazy loading, I want to throw exceptions when un-loaded navigational properties are accessed.

And example navigational path could be Blog => Post => Comment.

A method that takes a Post as an argument might not know if the Comments list was loaded by the query. So if the Comments property is accessed, I would want my proxy to throw an exception, if there was no Include for that relation in the query.

Currently, the issue is that the property would just be null, which could be interpreted wrongly be the runtime code.

I looked at the LazyLoading implementation, but it seems most of the code required to do this is marked as "Internal, do not use", so I would suggest there was a supported way of doing this. (If there is, I could not find it, but would love to know)

@ajcvickers ajcvickers added this to the 3.0.0 milestone Jan 30, 2019
@ajcvickers ajcvickers self-assigned this Jan 30, 2019
@ajcvickers
Copy link
Contributor

Putting this in 3.0 to review the internal code used and ensure that proxies can be created without depending on internal code.

ajcvickers added a commit that referenced this issue May 5, 2019
This allows others to build proxies without using internal code.

Fixes #14554 #15252

Internal code is still being used for conventions.
@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 May 5, 2019
ajcvickers added a commit that referenced this issue May 8, 2019
This allows others to build proxies without using internal code.

Fixes #14554 #15252

Internal code is still being used for conventions.
ajcvickers added a commit that referenced this issue May 8, 2019
This allows others to build proxies without using internal code.

Fixes #14554 #15252

Internal code is still being used for conventions.
@ajcvickers ajcvickers modified the milestones: 3.0.0, 3.0.0-preview6 Jun 5, 2019
@ajcvickers ajcvickers removed the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 16, 2019
@ajcvickers ajcvickers reopened this Aug 16, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0-preview6, Backlog Aug 16, 2019
@ajcvickers
Copy link
Contributor

Re-opening this since we decided in #17159 to make the annotations needed internal again.

@davidkraus
Copy link

Hey, i'm looking for the same functionality - Is this now possible? If yes can someone give me a hint where to start.

@ajcvickers
Copy link
Contributor

@davidkraus The only issue here is that you may need to make use of some EF internal APIs, which could mean your proxy implementation breaks when we change those APIs. If that's something you can live with, then the best place to start is with the code for proxies in the this repo. I'd be interested to hear how it goes.

@davidkraus
Copy link

@ajcvickers using internal APIs is fine since it is 'just' a feature to catch errors earlier.

I gave it a shot and it seems to work - I copied the whole Proxy package (the 3.1.8 version, not the current one) into our Project and startet from there.

The important changes i made are in the LazyLoadingInterceptor (renamed it to RestrictedLoadingInterceptor), the rest of the Proxy code is nearly unchanged.

Code

  public class RestrictedLoadingInterceptor : IInterceptor
    {
        private static readonly PropertyInfo _lazyLoaderProperty
            = typeof(IProxyLazyLoader).GetProperty(nameof(IProxyLazyLoader.LazyLoader));

        private static readonly MethodInfo _lazyLoaderGetter = _lazyLoaderProperty.GetMethod;
        private static readonly MethodInfo _lazyLoaderSetter = _lazyLoaderProperty.SetMethod;

        private readonly IEntityType _entityType;
        private ILazyLoader _loader;
        private ICurrentDbContext _currentDbContext;

        public RestrictedLoadingInterceptor(
            [NotNull] IEntityType entityType,
            [NotNull] ILazyLoader loader,
            [NotNull] ICurrentDbContext currentDbContext)
        {
            _entityType = entityType;
            _loader = loader;
            _currentDbContext = currentDbContext;
        }

        public virtual void Intercept(IInvocation invocation)
        {
            var methodName = invocation.Method.Name;

            if (_lazyLoaderGetter.Equals(invocation.Method))
            {
                invocation.ReturnValue = _loader;
            }
            else if (_lazyLoaderSetter.Equals(invocation.Method))
            {
                _loader = (ILazyLoader)invocation.Arguments[0];
            } else {
                if (_loader != null
                    && methodName.StartsWith("get_", StringComparison.Ordinal)) {

                    var navigationName = methodName.Substring(4);
                    var navigation = _entityType.FindNavigation(navigationName);

                    if (navigation != null) {
                        if (!_currentDbContext.Context.Entry(invocation.InvocationTarget).Reference(navigationName)
                            .IsLoaded) {

                            throw new RestrictedLoadingException(
                                $"Could not access {navigation.Name} on Entity {_entityType.Name} because it was not previously loaded. Use .Include to make sure it is loaded."); 
                        }
                    }
                }

                invocation.Proceed();
            }
        }
    }

To get it working i added an injection for ICurrentDbContext and used _currentDbContext.Context.Entry(invocation.InvocationTarget).Reference(navigationName).IsLoaded to find out if that property is already loaded. If not i just throw a custom Exception. As i am pretty new to C# and EF Core i can't really say if that is a bullet proof solution. The thing i'm worried about (and feels hacky) is the usage of invocation.InvocationTarget. It works in a trivial unit test but it would be great if a more advanced developer could give me some input on that code.

@ajcvickers
Copy link
Contributor

@davidkraus I've been pondering what to say here. My experience of proxies has not been all that great, primarily for two reasons:

  • It's easy to not create a proxy without realizing you're doing anything wrong. (Or alternately, you force everything to always be a proxy, which becomes painful when you want to create an entity instance without direct access to the context.)
  • The fact that the CLR type of the instance is now a proxy type can cause issues with comparisons, serialization. etc.

Now I'm not sayin that these are insurmountable issues, but it feels to me like introducing proxies in order to override and throw exceptions is probably not the correct trade-off.

One thing that would be easier and not have these issues would be to inject the DbContext into your entities like this:

public class Blog
{
    private readonly DbContext _context;
    private ICollection<Post> _posts;

    public Blog()
    {
    }

    private Blog(DbContext context)
    {
        _context = context;
    }
    
    public int Id { get; set; }

    public ICollection<Post> Posts
    {
        get
        {
            if (_context != null && !_context.Entry(this).Navigation(nameof(Posts)).IsLoaded)
            {
                throw new InvalidOperationException("Posts is not loaded.");
            }
            return _posts;
        }
        set => _posts = value;
    }
}

public class Post
{
    private readonly DbContext _context;
    private Blog _blog;

    public Post()
    {
    }

    private Post(DbContext context)
    {
        _context = context;
    }
    
    public int Id { get; set; }

    public Blog Blog
    {
        get
        {
            if (_context != null && !_context.Entry(this).Navigation(nameof(Blog)).IsLoaded)
            {
                throw new InvalidOperationException("Posts is not loaded.");
            }
            return _blog;
        }
        set => _blog = value;
    }
}

Note however, that this does put an EF dependency directly into your entity type.

@ajcvickers ajcvickers modified the milestones: Backlog, 7.0.0 May 30, 2022
@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 May 30, 2022
ajcvickers added a commit that referenced this issue May 30, 2022
Part of #626
Part of #15911
Fixes #20135
Fixes #14554
Fixes #24902

This is the lowest level of materialization interception--it allows the actual constructor/factory binding to be changed, such that the expression tree for the compiled delegate is altered.

Introduces singleton interceptors, which cannot be changed per context instance without re-building the internal service provider. (Note that we throw by default if this is abused and results in many service providers being created.)

The use of this for proxies has two big advantages:
- Proxy types are created lazily, which vastly improves model building time for big models with proxies. See #20135.
- Proxies can now be used with the compiled model, since the proxy types are not compiled into the model. See
ajcvickers added a commit that referenced this issue Jun 1, 2022
Part of #626
Part of #15911
Fixes #20135
Fixes #14554
Fixes #24902

This is the lowest level of materialization interception--it allows the actual constructor/factory binding to be changed, such that the expression tree for the compiled delegate is altered.

Introduces singleton interceptors, which cannot be changed per context instance without re-building the internal service provider. (Note that we throw by default if this is abused and results in many service providers being created.)

The use of this for proxies has two big advantages:
- Proxy types are created lazily, which vastly improves model building time for big models with proxies. See #20135.
- Proxies can now be used with the compiled model, since the proxy types are not compiled into the model. See
@ajcvickers ajcvickers modified the milestones: 7.0.0, 7.0.0-preview6 Jun 20, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0-preview6, 7.0.0 Nov 5, 2022
@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-proxies closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-3.0 type-enhancement
Projects
None yet
3 participants