castleproject / Core

Castle Core, including Castle DynamicProxy, Logging Services and DictionaryAdapter
http://www.castleproject.org/
Other
2.2k stars 467 forks source link

Not Intercepted Method is invoked on target, not on proxy. #647

Open joelweiss opened 1 year ago

joelweiss commented 1 year ago

This is a simplified version of what I am doing, but this should be enough to explain.

I want to add INotifyPropertyChanged to a POCO (Blog) class, and I want to raise PropertyChanged even if the property is changed by a method on the Target (SetBlogId()), this no longer works after 5.0.0 probably because #571 changed the SetBlogId() execution to be on the target not on the proxy.

Is there any way I can invoke the target.Method() on the Proxy?

in 4.4.1 the output is Change event was raised as expected in 5.0.0 the output is Expected a change event

using System;
using Castle.DynamicProxy;
using System.ComponentModel;
using System.Reflection;

internal class Program
{
    private static void Main(string[] args)
    {
        Blog blog = new Blog();
        ProxyGenerator generator = new ProxyGenerator();
        Blog proxy = (Blog)generator.CreateClassProxyWithTarget(
            classToProxy: typeof(Blog),
            additionalInterfacesToProxy: new[] { typeof(INotifyPropertyChanged) },
            options: new ProxyGenerationOptions(new PropertiesOnlyGenerationHook()),
            target: blog,
            interceptors: new INotifyPropertyChangedInterceptor());
        bool propertyChangedEventRaised = false;

        ((INotifyPropertyChanged)proxy).PropertyChanged += (o, e) => propertyChangedEventRaised = true;

        proxy.SetBlogId(1000);

        Console.WriteLine(propertyChangedEventRaised ? "Change event was raised as expected" : "Expected a change event");

        Console.WriteLine("Press any key to exit...");
        Console.ReadKey();
    }
}

internal class PropertiesOnlyGenerationHook : IProxyGenerationHook
{
    public void MethodsInspected() { }
    public void NonProxyableMemberNotification(Type type, MemberInfo memberInfo) { }
    // intercept propery and event setters
    public bool ShouldInterceptMethod(Type type, MethodInfo methodInfo) => methodInfo.IsSpecialName;
}

internal class INotifyPropertyChangedInterceptor : IInterceptor
{
    public void Intercept(IInvocation invocation)
    {
        if (invocation.Method.Name.StartsWith("set_", StringComparison.Ordinal))
        {
            string propertyName = invocation.Method.Name["set_".Length..];
            invocation.Proceed();
            PropertyChanged?.Invoke(invocation.Proxy, new PropertyChangedEventArgs(propertyName));
        }
        else if (invocation.Method.Name == "add_PropertyChanged")
        {
            PropertyChanged += (PropertyChangedEventHandler)invocation.Arguments[0];
        }
        else if (invocation.Method.Name == "remove_PropertyChanged")
        {
            PropertyChanged -= (PropertyChangedEventHandler)invocation.Arguments[0];
        }
        else
        {
            invocation.Proceed();
        }
    }

    private event PropertyChangedEventHandler? PropertyChanged;
}

public class Blog
{
    public virtual int BlogId { get; set; }
    public virtual void SetBlogId(int blogId) => BlogId = blogId;
}
joelweiss commented 1 year ago

In other words I want the proxy to be generated as follows

public class Blog
{
    public virtual int BlogId { get; set; }

    public virtual void SetBlogId(int blogId) => BlogId = blogId;
}

public class BlogProxy : Blog, INotifyPropertyChanged
{
    private readonly Blog _Target;

    public BlogProxy(Blog target) => _Target = target;

    public override int BlogId
    {
        get => _Target.BlogId;
        set
        {
            _Target.BlogId = value;
            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(BlogId)));
        }
    }

    // This I don't know how to achieve in > 5.0.0
    public override void SetBlogId(int blogId) => base.SetBlogId(blogId);

    public event PropertyChangedEventHandler? PropertyChanged;
}

----

blog = new Blog();
Blog blogProxy = new BlogProxy(blog);
bool blogProxyRaised = false;
((INotifyPropertyChanged)blogProxy).PropertyChanged += (o, e) => blogProxyRaised = true;
blogProxy.SetBlogId(5000);
Console.WriteLine(blogProxyRaised ? "[Proxy] Change event was raised as expected" : "[Proxy] Expected a change event");
stakx commented 1 year ago

Responding to your first post:

Is there any way I can invoke the target.Method() on the Proxy?

I don't think so. The target object, which is a plain Blog object, doesn't know about the proxy, it's just your code; so there isn't going to be any magical redirection from your code back to the proxy.

I suspect the real problem here is that object state gets split across two object instances, which happens because you've chosen to use a composition-based proxy (CreateClassProxyWithTarget). You might be more successful with an inheritance-based proxy (CreateClassProxy), where you'll end up with only a single object.

joelweiss commented 1 year ago

@stakx thanks for the response

CreateClassProxy will not work in my case, because I am proxying an existing class with data already, hence CreateClassProxyWithTarget

stakx commented 1 year ago

@joelweiss, sorry for the slow reply. Back in January, I suspected that there's nothing further we can do for you, but I wanted to sleep on it in case I could come up with something... and then I forgot.

To answer your core question, which was how to do an invocation.Proceed() that proceeds to the base class' method implementation, instead of to the target's:

// This I don't know how to achieve in > 5.0.0
public override void SetBlogId(int blogId) => base.SetBlogId(blogId);

DynamicProxy doesn't have an API for that. Also, you cannot do it using Reflection like this (inside your interceptor):

if (invocation.Method.Name == "SetBlogId")
{
    var baseMethod = invocation.Proxy.GetType().BaseType.GetMethod("SetBlogId");
    baseMethod.Invoke(invocation.Proxy, invocation.Arguments);
}

The problem here is that baseMethod.Invoke will perform a virtual dispatch, which just makes execution end up in the proxy and you get an infinite loop.

You could implement an alternate Invoke helper that does not perform virtual dispatch, so you'd really end up in the base class' method implementation. I have previously implemented a DynamicInvokeNonVirtually helper method for Moq that does just that. You could use that (in the above example interceptor code) instead of baseMethod.Invoke.

I'm not really sure whether your scenario is an edge case, or whether it might be useful to extend DynamicProxy's API with something like invocation.ProceedToBase() that would do what you asked for directly.

Any opinions, anyone?

joelweiss commented 1 year ago

@stakx thanks for the response.

yes, invocation.ProceedToBase() is exactly what I am looking for (that was the behavior before 5.0.0). In the mean time I will have a look if your suggestion to call DynamicInvokeNonVirtually will work for me.

Thanks again.