reactiveui / ReactiveUI

An advanced, composable, functional reactive model-view-viewmodel framework for all .NET platforms that is inspired by functional reactive programming. ReactiveUI allows you to abstract mutable state away from your user interfaces, express the idea around a feature in one readable place and improve the testability of your application.
https://www.reactiveui.net
MIT License
8.08k stars 1.12k forks source link

[Bug]: No notifications from nested object if it was generated using Castle.DynamicProxy #3418

Closed jinek closed 1 year ago

jinek commented 1 year ago

Describe the bug šŸž

I'm trying to subscribe my parent object to notifications from nested object while that nested object (represented by an interface) has been generated with Castle.

class ParentObject : ReactiveObject
{
    [Reactive] public INestedObject NestedObject { get; set; }

    public ParentObject()
    {
        this.WhenAnyValue(parent => parent.NestedObject,
                parent => parent.NestedObject.Input,
                (parent, _) => parent?.Input)
            .Subscribe(str =>
            {
                Console.WriteLine(str);
            });
    }
}

entire file is here https://github.com/jinek/ReactiveUIBugRepro1/blob/main/ConsoleApp1/Program.cs

INestedObject is generated using Castle.DynamicProxy. If i change property Input of INestedObject then subscription of parent object is not fired.

Step to reproduce

  1. Clone https://github.com/jinek/ReactiveUIBugRepro1 and run the project.
  2. Observe, that output does not contain values '2' and '3'.
  3. Replace (INestedObject)Activator.CreateInstance(interfaceProxyTypeWithTarget, Array.Empty<IInterceptor>(), new NestedObject()); with just new NestedObject();
  4. Run the project again
  5. Observe all values in the output: 1, 2, 3, 4.

Reproduction repository

https://github.com/jinek/ReactiveUIBugRepro1

Expected behavior

Notifications from generated nested objects must propagate to subscribers.

Screenshots šŸ–¼ļø

image

IDE

Rider Windows, Rider macOS

Operating system

Linux Mint

Version

Castle.Core 5.1.0

Device

No response

ReactiveUI Version

18.3.1

Additional information ā„¹ļø

I don't think problem could be in Castle.Core because notification from generated nested object come correctly:

((INotifyPropertyChanged)nestedObject).PropertyChanged += (sender, eventArgs) =>
{
    Console.WriteLine($"{eventArgs.PropertyName}:{nestedObject.Input}");
};

outputs this:

Input:1
1
Input:2
Input:3
Input:4
anaisbetts commented 1 year ago

This is expected afaik, you have created a ReactiveObject that doesn't actually like a ReactiveObject (i.e. fire .Changing and .Changed) when you create a Castle knockoff. Because Castle has created a very convincing replica, ReactiveUI will treat Castle objects as what they say they are, and ReactiveObject's handler has a higher affinity (priority) than INotifyPropertyChanged.

You might be able to work around this by registering a change notifier that can detect Castle objects and implement change notifications on them directly, then return a higher affinity than the ReactiveObject handler

jinek commented 1 year ago

@anaisbetts Thank you for hints. I have tried to subsribe to IReactiveObject events:

nestedObject.PropertyChanging += (sender, eventArgs) =>
{
    Console.WriteLine($"Changing: {eventArgs.PropertyName}:{nestedObject.Input}");
};
nestedObject.PropertyChanged += (sender, eventArgs) =>
{
    Console.WriteLine($"Changed: {eventArgs.PropertyName}:{nestedObject.Input}");
};

And I can see they are fired:

Changing: Input:
Changed: Input:1
1
Changing: Input:1
Changed: Input:2
Changing: Input:2
Changed: Input:3
Changing: Input:3
Changed: Input:4

Update: But I see now, it seems ReactiveUI determines notifier by sender argument, not by the object which was subscribed on, which absolutely makes sense because of the design of

public delegate void PropertyChangedEventHandler(object? sender, PropertyChangedEventArgs e);
jinek commented 1 year ago

In case one is seeking for a solution, here is workaround: INestedObject must implement only INotifyPropertyChanged and INotifyPropertyChanging (not IReactiveObject as ReactiveUI handles them differently). However, the target object can implement IReactiveObject or inherit from ReactiveObject.

Each generated proxy needs to subscribe to own events and re-raise them providing the proxy as a sender. For this, we can create base class of all proxies:

public class ProxyBase
{
    private readonly FieldInfo _propertyChangedHandlerFieldInfo = typeof(ReactiveObject)
        .GetField("PropertyChangedHandler", BindingFlags.Instance | BindingFlags.NonPublic);

    private readonly FieldInfo _propertyChangingHandlerFieldInfo = typeof(ReactiveObject)
        .GetField("PropertyChangingHandler", BindingFlags.Instance | BindingFlags.NonPublic);

    public ProxyBase()
    {
        ((INotifyPropertyChanged)this).PropertyChanged += (sender, args) =>
        {
            if (Equals(sender) || sender is not ReactiveObject) return;

            var propertyChangedEventHandler = (PropertyChangedEventHandler?)_propertyChangedHandlerFieldInfo.GetValue(sender);
            propertyChangedEventHandler?.Invoke(this, new PropertyChangedEventArgs(args.PropertyName));
        };
        ((INotifyPropertyChanging)this).PropertyChanging += (sender, args) =>
        {
            if (Equals(sender) || sender is not ReactiveObject) return;

            var propertyChangingEventHandler = (PropertyChangingEventHandler?)_propertyChangingHandlerFieldInfo.GetValue(sender);
            propertyChangingEventHandler?.Invoke(this, new PropertyChangingEventArgs(args.PropertyName));
        };
    }
}

In case of Castle the proxy base type can be provided at the moment of generation:

var defaultProxyBuilder = new DefaultProxyBuilder();
var proxyGenerationOptions = ProxyGenerationOptions.Default;
proxyGenerationOptions.BaseTypeForInterfaceProxy = typeof(ProxyBase); // <--- here

Type interfaceProxyTypeWithTarget = defaultProxyBuilder.CreateInterfaceProxyTypeWithTarget(typeof(INestedObject),
    Type.EmptyTypes, typeof(NestedObject), proxyGenerationOptions);

Now parent object is correctly notified about changes in nested objects

jinek commented 1 year ago

I made several more checks and it looks like DynamicData, System.Reactive and Avalonia Binding works fine in this case.

glennawatson commented 1 year ago

Thanks for the workaround. Since this is outside the scope of rxui I am closing the issue but I'm sure if anyone else is using castle they'll find it useful.

jinek commented 1 year ago

Thanks for the workaround. Since this is outside the scope of rxui I am closing the issue but I'm sure if anyone else is using castle they'll find it useful.

Up to you. What I'm saying is that this issue is reproducible only with RXUI.

github-actions[bot] commented 1 year ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.