pamidur / aspect-injector

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

Adding Property Value as Advice Parameter #126

Closed EyalBenari closed 2 years ago

EyalBenari commented 4 years ago

If an advice is Kind.After and the Targets=Target.Setter , then it would be really helpful to get the property value as one of the advice parameters.

My use case is collecting metrics on properties with a custom attribute (the trigger) whenever set is called (with Kind.After). So currently I need to get the property value through reflection and it's a real performance hit.

pamidur commented 4 years ago

Hi @EyalBenari , It used to be a feature of early beta, but was stripped from release because of some issues. If you help me find a consistent way to make it work, I'd happily introduce this feature back. So the issues are:

pamidur commented 4 years ago

An just in case, if you don't need previous value in setter you can use Source.ReturnValue for getter value and Source.Arguments [0] for setter value like this:

    [Aspect(Scope.Global)]
    [Injection(typeof(TestAspect))]
    public class TestAspect : Attribute
    {
        [Advice(Kind.After, Targets = Target.Setter | Target.Getter | Target.Public)]
        public void After(
            [Argument(Source.ReturnValue)] object val,
            [Argument(Source.Arguments)] object[] args,
            [Argument(Source.Metadata)] MethodBase method
            )
        {
            if (method.Name.StartsWith("set_"))
                Console.WriteLine($"New value set: {args[0]}");
            else
                Console.WriteLine($"Value returned: {val}");
        }
    }

    class Program
    {
        [TestAspect]
        public string Text { get; set; }

        static void Main(string[] args)            
        {

            var p = new Program();
            p.Text = "new text";
            Console.WriteLine(p.Text);
        }
    }
EyalBenari commented 4 years ago

The problem with arguments is that the setter function could manipulate the data before setting the value.

pamidur commented 4 years ago

Sorry, for delay, these days were too busy despite quarantine. If you want to manipulate the value you can do with Kind.Around. Basically the Kind.Around is superior to every other option because it wraps the actual method call.

    [Aspect(Scope.Global)]
    [Injection(typeof(TestAspect))]
    public class TestAspect : Attribute
    {
        [Advice(Kind.Around, Targets = Target.Setter | Target.Getter | Target.Public)]
        public object Wrap(
            [Argument(Source.Target)] Func<object[], object> target,
            [Argument(Source.Arguments)] object[] args,
            [Argument(Source.Metadata)] MethodBase method
            )
        {
            if (method.Name.StartsWith("set_"))
            {
                Console.WriteLine($"New value set: {args[0]}");
                Console.WriteLine($"But we want to make it: other");
                return target(new object[] { "other" });
            }
            else
            {
                var val = target(args);
                Console.WriteLine($"Value returned: {val}");
                return val;
            }
        }
    }

    class Program
    {
        [TestAspect]
        public string Text { get; set; }

        static void Main(string[] args)
        {

            var p = new Program();
            p.Text = "new text";
            Console.WriteLine(p.Text);
        }
    }
pamidur commented 4 years ago

Or I maybe understood you wrong,.. and you want to prevent setter advice from manipulating the value?

EyalBenari commented 4 years ago

What I'm saying is that I can't trust the setter function to not do something before setting the value. A custom set function would make this approach irrelevant:

public class TestClass { private int test; public int Test {get => test; set => test = value * 2;} }

What I need is the actual value after the set function has completed without using reflection to get it.

pamidur commented 4 years ago

I got your point,.. the thing is you can't trust it actually sets backing field at all. like

public class TestClass
{
private int test;
public int Test {get => test; set => Console.Write(value);}
}

This is the issue I described a few comments earlier. With custom properties we don't know what is the value after Setter is executed. The only (non reliable way) is to call Getter (which might be absent btw) but the value it returns might have nothing to do with value that was set before.

public class TestClass
{
public int Test {set => Console.Write(value);}
}

I honestly have no idea how to handle such variety of possibilities in predictable and consistent way. Any ideas?

EyalBenari commented 4 years ago

Maybe it's best if I only support automatic properties for updating values based on calls to setter. Then find another solution for other cases. I just hoped code injection voodoo magic could be used to get the value in another way. By the way I would love to look at the code and maybe help in a few weeks.

pamidur commented 4 years ago

That's it, if we go for automatic properties only (where setter is just setter), then current approach with arguments works pretty well.

I wish it was voodoo magic :) You are more then welcome to contribute to code or to with ideas!

pamidur commented 2 years ago

Seem answered. Feel free to reopen any time1