pamidur / aspect-injector

AOP framework for .NET (c#, vb, etc)
Apache License 2.0
770 stars 115 forks source link

Interface injection from base class #197

Open blahlicus opened 2 years ago

blahlicus commented 2 years ago

Environment (please complete the following information):

Describe the question I don't think this is a bug per see, I think I'm just not using aspect-injector properly, please allow me to give the background info regardless:

In my program, there exists BaseClass, DerivedClass1, DerivedClass2, DerivedClass3, the derived classes have methods defined in them that are not overrides of the base class' methods. I implemented a simple interface injection with aspect-injector, I made it so that BaseClass implements my interface.

Instances of base class and dervied classes sucessfully have the AOP injection for the methods defined in the base class, but not for the methods newly defined in the derived classes, how can I make it so that the methods in the derived classes also get the code injection?

To Reproduce I have below sample code that I've ran that reproduces this issue: (ran in VS2022 preview with net6 MAUI)

To summarise, calling method1() from an instance of c triggers the injected code just fine, however, calling method3() from the same instance does not trigger the injected code, I want to know how could I make it so calling method3() will also trigger the injected code without having to add IHandleError to class c and class b as well.

public partial class MainPage : ContentPage
{
    int count = 0;

    public MainPage()
    {
        InitializeComponent();
    }

    private void OnCounterClicked(object sender, EventArgs e)
    {
        // in my UI project, I am clicking the button for this event to trigger my test
        var a = new c();
        a.method1();
        a.method2();
        a.method3();

    }

}

public class c : b {

    public string method3() {
        return "hello world";
    }
}

public class b :a {

    public string method2() {
        return "hello world";
    }
}

public class a : IHandleError
{
    public string method1() {
        return "hello world";
    }
}

[Injection(typeof(HandleError), Inherited = true)]
public interface IHandleError
{

}

[Aspect(Scope.Global)]
public class HandleError
{

    public static int counter { get; set; }

    [ThreadStatic]
    public static bool IsRootAttribute = true; // i have this in my original code but this shouldn't be related to the issue at hand

    [Advice(Kind.Around, Targets = Target.Method)]
    public object WrapperMethod([Argument(Source.Name)] string name,
        [Argument(Source.Arguments)] object[] arguments,
        [Argument(Source.Target)] Func<object[], object> method,
        [Argument(Source.ReturnType)] Type returnType) {

        // i placed breakpoint here and see that it isn't triggered when class b.method2 or class c.method3 was called
        // just FYI when an instance of class c's method1 is called, the breakpoint is hit as expected
        counter = counter + 1;

        // the below code is unimportant in the scope of this question, it is just some error handling operations that i was using AOP for
        object output = null;
        bool isRoot = IsRootAttribute;
        Exception exception = null;
        if (IsRootAttribute) {
            IsRootAttribute = false;
        }

        try {
            output = method(arguments);
        } catch (Exception ex) {
            if (isRoot == false) {
                throw;

            }
            exception = ex;
        }

        if (isRoot) {
            IsRootAttribute = true;
            if (exception != null) {

                //NavigationController.ShowSimpleAlert(exception.Message, "Error");
                if (returnType.IsValueType && returnType != typeof(void)) {
                    return Activator.CreateInstance(returnType);
                } else {
                    return null;
                }
            }

        }

        return output;
    }
}
pamidur commented 2 years ago

Hi @blahlicus , sorry for delay. Indeed today Aspect Injector does not look for interface injections in the base classes. I believe this improvement is quite easy to implement.

APEX-JOAQUIN-ROVIRA commented 1 year ago

Really interested in this feature. I accidentally opened a very similar request. Any updates on a possible implementation of this?

pamidur commented 1 year ago

Hi @APEX-JOAQUIN-ROVIRA , I've recently moved countries, didn't have much time to look into this feature unfortunately.

But looking through your closed issue I was thinking if Injection.Inherited might work for you

[Aspect(Scope.Global)]
[Injection(typeof(MyAspect), Inherited=true)]
public class MyAspect : Attribute
{
    [Advice(Kind.Before, Targets = Target.Method)]
    public void LogEnter() { Console.WriteLine($"Method call detected!"); }
}

[MyAspect]
public class A {
  void Something() { } // Calls LogEnter()
}

public class B : A {
  void Another() { } // Should log call too
}
APEX-JOAQUIN-ROVIRA commented 1 year ago

@pamidur thank you for the quick reply and I hope you are doing well. I have tested the use of Inhertited = True with the latest stable release (2.8.1), and it does not seem to work as described. New functions such as B.Another() do not call LogEnter().

Functions inherited from the base class A do call LogEnter() (i.e., B.Something() calls LogEnter()). If replaced with the override or new keywords, they do not - as one would expect.

pamidur commented 1 year ago

Oh you're right, I totally forgot, this parameter only applies to Attributes so that Injection attributes can inherit aspects. It does not apply to target classes.

BTW you can still apply Aspect to whole assembly like this

[assembly: MyAspect()]

but I believe Injection.PropagationFilter will not be enough for you since it only filters member names and not class and namespace names.

I'll look at this feature again soon, stay tuned!

APEX-JOAQUIN-ROVIRA commented 1 year ago

Thank you very much, I will actively look forward to any updates on the issue. 😃

I read about the assembly trick in the docs. However, I am weary of adding the advice to all methods of all classes unconditionally and checking if (instance is IExample) { ... } as it could have a large performance impact, specially on highly modular code bases.

mannok commented 1 year ago

Oh you're right, I totally forgot, this parameter only applies to Attributes so that Injection attributes can inherit aspects. It does not apply to target classes.

BTW you can still apply Aspect to whole assembly like this

[assembly: MyAspect()]

but I believe Injection.PropagationFilter will not be enough for you since it only filters member names and not class and namespace names.

I'll look at this feature again soon, stay tuned!

Look forward to have this feature too. Will it arrive soon? Thanks in advance