Interception through dynamic proxies

[DixonDs]

My attempt to address what I asked in #364. I've added it as a separate project since a dependency on Castle.Core is introduced. The key component is DynamicProxyInterceptor, the rest are merely wrappers over Castle.Core stuff.

Please, let me know in case of any questions. I know that I should have commented all public classes at least, I may do so later. If this PR is accepted, most probably it will be followed up by another PR with some base interceptors for intercepting async methods (I need to think more about it)

[jeremydmiller]

@DixonD-git Dude, you're awesome! I'll take a harder look at this later today but I can't imagine not taking it.

If you're comfortable with dynamic proxies and Castle Core, do you think you'd be interested in revamping the Typed Factory library?

[DixonDs]

@jeremydmiller I wish I know what the Typed Factory library is :)

[jeremydmiller]

@DixonD-git

I'd definitely like to see this get in here and I think this is a good start.

I think I'd like to see some kind of reusable IInterceptorPolicy that you could use to setup the dynamic interception. You'd do it by giving an optional filter of Func<Type, bool> to know where it should apply and an array of IInterceptionBehavior objects or types. Or at least some easy way to build it out.

Before we pull this in for real, I think I want to go ahead and do #388 just to get rid of the potential conflict in Castle.Core version from RhinoMocks. That one's on me.

Bookkeeping things for later

• Nuspec file
• Use Paket for nuget dependencies and eliminate packages.config
• Use a link to CommonAssemblyInfo to embed the version number in the assembly
• Add a topic about this to the doc website

For typed factories, see what Windsor does as an example: https://github.com/castleproject/Windsor/blob/master/docs/typed-factory-facility-interface-based.md. I've never wanted this because I just don't think it's that big a deal to just use the container itself, but other folks have wanted it. The existing code is completely broken anyway, so it's almost starting over from scratch. You'd end up building an implementation on the fly that builds up either a configured instance or explicit args and then delegates to the underlying Container.

Would you be up for that?

[jeremydmiller]

And the [TestCase] usage doesn't work w/ Fixie from the command line or TD.Net. It accidentally works with the R# runner and maybe the VS runner. That's probably just a matter of configuring the Fixie conventions.

[DixonDs]

Did some more work here:

• Added DynamicProxyInterceptorPolicy
• Tried to make it work with sync and async interception behaviors on top of sync and async intercepted methods.
• Added some tests for those changes

We need to test all this stuff properly, I suspect a bunch of bugs are there:)

As for typed factories, yes, it looks interesting for me, I'll try to pick this up once I have a chance, although from the first look I've got impression that typed factories are not something that could be widely used. Anyway it is nice to have, I guess.

[DixonDs]

I had concerns about IMethodInvocationResult (I borrowed an idea for it from Unity.Interception) from the very beginning but now I understand why I had them, I guess. We are losing the stacktrace if we save an exception and throw it again. I think I'll remove IMethodInvocationResult and just propagate exceptions through all interceptors.

[khellang]

We are loosing the stacktrace if we save an exception and throw it again.

You know that .NET 4.5 has ExceptionDispatchInfo.Capture and ExceptionDispatchInfo.Throw, right?

So you can capture and rethrow an exception, preseving all its stack trace and Watson information 😄

The ExceptionDispatchInfo object stores the stack trace information and Watson information that the exception contains at the point where it is captured. The exception can be thrown at another time and possibly on another thread by calling the ExceptionDispatchInfo.Throw method. The exception is thrown as if it had flowed from the point where it was captured to the point where the Throw method is called.

[DixonDs]

@khellang Thanks, I didn't know that. Anyway, I'm not sure if we benefit from using ExceptionDispatchInfo comparing to simple exception propagation, unless I'm missing something. For instance, if we implement MethodInvocation.CreateExceptionResult so that it uses ExceptionDispatchInfo.Capture, then when implementing our own ISyncInterceptionBehavior, we would need to do something as follows if we want to have a proper stack trace:

public class ValidationInterceptor : ISyncInterceptionBehavior
{
    public IMethodInvocationResult Intercept(ISyncMethodInvocation methodInvocation)
    {
        var argument = methodInvocation.GetArgument("value");
        var argumentValue = (int)argument.Value;
        if (argumentValue < 0)
        {
            try
            {
                throw new ArgumentException();
            }
            catch (Exception e)
            {
                return methodInvocation.CreateExceptionResult(e);
            }
        }
        return methodInvocation.InvokeNext();
    }
}

i.e. we still need to throw to get a stack trace.

[DixonDs]

With the last commit, I made it work with both variants - return methodInvocation.CreateExceptionResult(exception) and throw exception in interceptors

[jeremydmiller]

Hey @DixonD-git, do you want me to pull this in now? I lost track of SM for a bit.

[DixonDs]

@jeremydmiller I would like you to review it properly, as the stuff is pretty complex. So, I guess, it's better to wait when you find time for it (whenever it is going to happen) than just merge it right away into master (unless I'm mistaken what "to pull in" means)

[jeremydmiller]

@DixonD-git I looked through it, and TBH, I don't have any hard recommendations for changing anything. What I do suggest though, is that we move this into a completely different repo so it can get its own release schedule and not make you have to use the same build tool chain for that matter. I might vote to do the same for the AutoFactory as well. I can help set up the repo's and definitely help with the CI builds.

Unfortunately, I'm about to thoroughly screw everything up in the SM codebase by making the conversion to the DNX project system and the CoreCLR stuff. I might even take the AutoFactory code out of the solution for now.

[DixonDs]

@jeremydmiller As for me, moving to another repo has cons as well. Mainly, if you have it in the same repo, you can easily branch everything if you need to make changes in several projects at once. But I'm fine with moving to separate repo as well. But is it possible to keep it under https://github.com/structuremap at least?

[qswinson]

@DixonD-git Thanks for doing this work. This is awesome! I've tried it out in my project and it was easy to integrate. My one comment is that DynamicProxyInterceptorPolicy only takes instances of IInterceptionBehavior instead of types. I think it will be more common for people to want StructureMap to resolve the IInterceptionBehavior. I created a pull request on your branch to add support for this.

[qswinson]

After using this code for a few interceptors, I have found that the IMethodInvocation interface should expose more of the DynamicProxy IInvocation properties in order to perform all of the interceptor activities I need to do.

I've created a pull request on @DixonD-git branch to add these properties.

[jeremydmiller]

Honestly, let's just get this in right now and let it build. I'll finally take a better look soon, but I'll wait for you to say when you want me to publish the Nuget. Or you could do it yourself if you want by just downloading it from the TeamCity feed