Implements Type in SignalRValueProvider

[danielwertheim]

Implements the Type property in the SignalRValueProvider as it currently was left unassigned, or was this by design? Also introduces a specific SignalRNullValueProvider and unifies the creation of it using a simple factory method.

Didn't add the type checks that e.g. the ObjectValueProvider does as the factory method ensures the type is correct.

[wanlwanl]

Thanks for contributing! The PR look good! I found NullObjectTask and SignalRNullValueProvider aren't frequently used, how about merge them into SignalRValueProvider so that we only keep one class?

[danielwertheim]

Do you mean as nested classes or to move all logic into SignalRValueProvider?

[danielwertheim]

@wanlwanl is this a "hot" path? Would it be better to create a field holding a Task.FromResult(value) and set in in the constructor?

[danielwertheim]

Did the change I meant. If no good, I can revert.

[danielwertheim]

@wanlwanl Merged the code into the SignalRValueProvider class so that the NullProvider isn't exposed anywhere else. I hope that was what you meant.

[wanlwanl]

Hi @danielwertheim , I create a PR based on this PR, please take a look.

[danielwertheim]

@wanlwanl I've changed so that the optimized null value provider etc. is enclosed in the SignalRValueProvider. I've also fixed that the actual type of the value is used. I've looked at other Azure projects and according to the purpose of ToInvokeString it seem to be to provide a string representation of the enclosed value. I'm a bit unsure about the generation of the string used in your PR suggestion. Wouldn't that suggestion be a breaking change as previously it was the ToString value of the contained value? Which seem to be the behavior of e.g. ObjectValueProvider in WebJobs project and in e.g. MicrosoftGraph extension project. So should that changes really be in this PR? I noticed it seemed not implemented in full as well as there were a todo added.

[wanlwanl]

@danielwertheim Thanks for update!

SignalR extension used to add binding via BindToInput which uses BindToInputBindingProvider. I think we can refer the implementation of BindToInputBindingProvider.

As for ToInvoke, I also found some repos/blogs make it as a ToString, but in BindingBase, it mentions about "Called when we invoke from dashboard. ", I've asked an engineer in Function team, he told me "This is the WebJobs dashboard available in Kudu. This is primarily for WebJobs tests/direct invoke scenarios and doesn't apply to Functions much", so we don't need to worry about this method. I think keep it the same to BindingBase is ok.