Add possibility for caller proxy to return an observable

Code-Sharp / WampSharp

A C# implementation of WAMP (The Web Application Messaging Protocol)

http://wampsharp.net

Other

385 stars 83 forks source link

Add possibility for caller proxy to return an observable #330

Open Jopie64 opened 3 years ago

Jopie64 commented 3 years ago

First part of #238. Can you take a look at it for whether I'm on the good track with this feature?

Jopie64 commented 3 years ago

Also I was wondering which C# language features I could use... Is C# 7 or even 8 allowed?

darkl commented 3 years ago

C# 7.0 is okay. C# 8.0 is not supported for .NET Framework, so let's avoid it (I hope I have been doing that myself).

Jopie64 commented 3 years ago

Hi @darkl

I think I have most of it, if not everything, in now to support IObservable return values on the callee side as well as on the caller side. Can you please take another look at it and show me where I might have messed up? :) Thanks in advance!

darkl commented 3 years ago

Thanks @Jopie64 for your work! It might take me some time to review this.

darkl commented 3 years ago

I am sorry this is taking so long, I am busy with other stuff.

One thing that bothers me is that the final value will always be empty. This causes inconsistencies because if you have an old implementation using Task and IProgress and then you "forward" it using a callee that uses a callee proxy that returns a cold observable, then it will no longer return the return value of the original implementation. I'm wasn't able to find a solution that I'm happy with for this problem.

Elad

Jopie64 commented 3 years ago

Yea I feel your pain, there's always lots of important stuff to do for us devs. No hurries 😊

About that final value thing. That bothered me too for a while. But in practice it doesn't really matter at least for us. The thing to keep in mind is: It is not a 100% replacement for the IProgress API.

When you don't want to have the intermediate types be the same as the type of the final value, don't use this pattern.
When you want the final yield to contain a value, don't use this pattern.
Also when you want to use it for arbitrary API forwarding, where you don't know how the other side is using the progressive API, don't use this pattern.

In all other cases, where you just have an observable implementation on a different node than the observable subscription, this pattern can be used. For us in practice it means that we never had to make use of the extra capabilities the IProgress interface gives us. (We are currently using a workaround to be able to use this pattern.)

darkl commented 3 years ago

I think I want to change the behavior so that the CalleeProxy will omit the return value unless specified explicitly (maybe through an attribute or something). Otherwise the behavior is too confusing at least to me.

Elad

Jopie64 commented 3 years ago

I don't understand what you mean... I assume you mean the final return value? In case of observables the callee already omits its final return value right? Because OnCompleted() doesn't carry a value...

It's the caller that you might consider confusing I think. In WAMP the final return might carry a value or it might not. In case of observables you don't expect the final result to contain a value. But I currently made it so that when it does anyway, it will emit this before it completes. The reason I did it this way is that this way the caller can use observable return values in all methods, where the server can implement them as observable or task.

In practice this works well for us. The caller (even when it is mostly javascript) uses observables everywhere and the server backend implements it however it wants. And an existing API can even switch from task to observable without requirering a change in the frontend.