fix subscription callback invoke and pubsub tests

[tanmaykm]

The subscription look callbacks were getting invoked with the SubscriptionMessage struct. But the docs do not mention that, and seem to suggest that the callback would receive the actual message, which seems natural. The callbacks in tests seem to be expecting the actual message too.

Updated the callback invocation to pass the actual message instead of SubscriptionMessage struct.

[jkaye2012]

Hey there, thanks for the PR.

I think this would be a reasonable addition to the library, but should be added as a new method. Otherwise, this would be a major breaking change to the library that would require a new major version and new releases.

[tanmaykm]

Fair enough.
Maybe we could have subscribe take an optional kwarg that indicates the nature of the function it is registering, and it can then wrap the supplied function accordingly?

How about:

@enum CallbackType  CallbackMsgStruct = 1 CallbackMsgData = 2

function subscribe(conn::SubscriptionConnection, channel::AbstractString, callback::Function; callback_type::CallbackType=CallbackMsgStruct)
...
end

function subscribe(conn::SubscriptionConnection, subs::Dict{AbstractString, Union{Function, Pair{Function,CallbackType}})
...
end

[jkaye2012]

Generally speaking, I prefer to follow "each method does one thing" rather than giving methods dynamic behavior based on their arguments.

I think the enum is reasonable and should be stored "behind the scenes" (probably along with the callback) to denote which behavior the user wants. With the current storage, this information would be completely erased since only the raw functions are stored.

If we leave the existing methods unchanged and add new ones:

function subscribe_data(conn::SubscriptionConnection, channel::AbstractString, callback::Function)
function subscribe_data(conn::SubscriptionConnection, subs::Dict{AbstractString, Function})

Then, with corresponding documentation I think things will be really easy for users to follow and the code within the library should be clearer as well!

[tanmaykm]

Yes, that sounds good. I have made some changes and added the subscribe_data method.

[jkaye2012]

This looks good to me. We can merge it and then bring in your CI changes before cutting a new version!