Question: Intercepting async and non-async methods

[eugene-chehovskoy]

Hi,

For example we have a class with async and non-async methods and I want to use interceptor for both method types. AsyncInterceptor class already have the object Invoke(IInvocationInfo invocationInfo) method, but it is called even for async methods and if I create interception behavior (AOP advice) for async and non-async interceptor methods (e.g. object Invoke(IInvocationInfo invocationInfo) and async Task InvokeAsync(IInvocationInfo invocationInfo)), it will be called twice. It would be cool if object Invoke(IInvocationInfo invocationInfo) is called only for non async methods.

[seesharper]

I see what you mean. In order to support this, we would have to introduce another method, InvokeSync that gets invoked only for synchronous methods.That way we can split a method call into three categories (Sync, Task and TaskOfT) where each of there gets a dedicated virtual method that gets invoked only once per call. How does that sound?

[eugene-chehovskoy]

I think it sounds much better than my suggestion.

[eugene-chehovskoy]

Only one bad thing is that current class name "AsyncInterceptor" will cause some misunderstanding because it will handle not just async methods.

[seesharper]

Naming is difficult I can see why it might be confusing to intercept synchronous methods through this class. It could also have been done with the decorator pattern, but that would make the API slightly more cumbersome to use. In that case we could create a new AsyncInterceptor passing in the underlying interceptor in the constructor. The AsyncInterceptor would then only directly expose the two methods that handles the async methods.

Something like this

var interceptor = new AsyncInterceptor(new MyInterceptor());

[eugene-chehovskoy]

It should be easy to create an instance of interceptor because we might pass logger, cacher, .etc into the constructor and we need to create that instance via GetInstance for not to passing parameters manually. What if create 3rd type of interceptor which handles everything. IInterceptor for non-async, AsyncInterceptor for async and new multipurpose for both. It might be cumbersome as well, but it won't violate single responsibility, every approach has it's own role, but I'm not sure either it's good or not.

[seesharper]

You can still retrieve the interceptors using GetInstance, but you would have to write two interceptors. One for the async methods which will be the decorator for the interceptor that handles sync methods.

container.Register<IInterceptor, MyInterceptor>();
container.Decorate<IInterceptor, MyAsyncInterceptor>();

While this might be the most "correct" way in respect to single responsibility, I just don't know if this makes more sense for most people.

A side effect of this is that you would have to inject "ICache" into both interceptors to handle both sync and async methods.

Another alternative is to simply call the class Interceptor and make it clear in the documentation that this is a base class from which you can inherit to handle all types of methods.

[eugene-chehovskoy]

Is MyInterceptor implementation of IInterceptor not AsyncInterceptor? I've already thought about creating two interceptors, but the problem is that interceptors which implement IInterceptor are also called even for async methods hence code will be called twice or implementation using decorator won't have such problem?

[seesharper]

MyInterceptor is just a plain IInterceptor implementation. The MyAsyncInterceptor inherits AsyncInterceptor and only forwards to MyInterceptor for non-async methods

[eugene-chehovskoy]

Sounds good. Does this approach work right now or it has to be implemented? Asking because I've never tried LightInject's decoration feature before.

[seesharper]

It is not implemented yet, although that is the easy part. The hard part is to decide which option to choose.

Extend AsyncInterceptor with another method (InvokeSync) that gets executed only for synchronous methods.

The advantage here is that you only need one interceptor and hence no need to inject say a logger into two interceptors to support logging of both synchronous and asynchronous methods.

The disadvantage as I see it is mostly from an architectural standpoint where one could argue that it violates the single responsibility principle.

Create the AsyncInterceptor as a decorator base class.

The advantage is that we don't need a method called InvokeSync (not too happy with that name), since synchronous methods are just passed down to the interceptor it decorates and hence no need for an extra method.

The disadvantage is that it might be a slightly steeper learning curve for novice developers as they might not be familiar with the decorator pattern.

Personally I prefer the decorator approach.

[eugene-chehovskoy]

I don't think that novice developers even know about AOP approach. Decorator pattern won't be the most difficult thing for them. If you think that the decorator approach would be better (and so do I) maybe let's choose it. Only one thing is that decorator approach should be mentioned in the interceptor documentation and everybody will be able to understand how to use it.

[seesharper]

I agree.

[eugene-chehovskoy]

It looks like you finished implementing this feature, are you going to push it to the NuGet?

[Sky4CE]

Hi seesharper, could you please push this feature into Nuget ? We have been waiting for a couple of months for this. Now is a point for us to decide either move to some other IOC and refactor a good part of the project or still wait for this feature to be done. But as I understand this already finished, so could I kindly ask you to push this? Thanks.

[seesharper]

I am sorry that this takes a little more time than first planned. This is going as part of LightInject.Interception 2.0.0 that in turn aligns with LightInject 5.0 which is a rather big release. In the meantime you should be able to just copy the AsyncInterceptor class and use that as a base class for your interceptors. It should work just fine with the current version of LightInject.Interception.

…

On Wed, Dec 28, 2016 at 1:21 PM, Vitaliy ***@***.***> wrote: Hi seesharper, could you please push this feature into Nuget ? We have been waiting for a couple of months for this. Now is a point for us to decide either move to some other IOC and refactor a good part of the project or still wait for this feature to be done. But as I understand this already finished, so could I kindly ask you to push this? Thanks. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#9 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA_HWTXv3UjOBLhAlvZhVVCYNc8PXbQ1ks5rMlRJgaJpZM4KGAOE> .