Add custom query params to make-channel-socket!

[danielcompton]

This patch allows the user to pass additional query parameters in the additional web socket connection. These can be used for authentication. Websockets don't have an API to pass any headers, so any out of band information has to go over the query params.

Because :chsk-url-fn has been deprecated, I think we need to have another parameter for this. A slightly different approach would be to adjust the internal API so client-id isn't passed separately to the map->* functions, but instead we just pass a map of params.

Fixes #135

[danielcompton]

client-id and params could be combined.

[ptaoussanis]

Hi Daniel,

Thanks for the clear PR, much appreciated! 👍

Am I correct in understanding that this change is just to make it easier to include query-params in the chsk URL? You're aware that the URL/path can already contain query params, right?

[ptaoussanis]

This looks good to me btw, will merge + cut a new release if you give me the go-ahead.

client-id and params could be combined.

Think probably better to keep them separated as you have. They're semantically different (params are user-space, client-id required for the implementation).

[danielcompton]

Am I correct in understanding that this change is just to make it easier to include query-params in the chsk URL? You're aware that the URL/path can already contain query params, right?

Rereading #135 (comment) earlier today when working on this PR I finally clicked on what you were meaning last month. However now that chsk-url-fn has been deprecated I don't see any other way to influence the URL that is used for the connection?

if you give me the go-ahead.

They're semantically different

That's how I felt too.

Thanks!

[ptaoussanis]

However now that chsk-url-fn has been deprecated I don't see any other way to influence the URL that is used for the connection?

Oh, just with the path argument. So if your path is usually /chsk, you could change it to something like "/chsk/?email=foo%40example.com", etc.

Does that make sense?

Sorry if I wasn't too clear before. In any case, happy with this PR since it definitely makes things easier.

Is that... a gopher? :-)

[ptaoussanis]

Oh btw see you adjusted this to target the dev branch, thanks!

[danielcompton]

That's the shipit emoji :)

[ptaoussanis]

That's the shipit emoji :)

Perhaps telling that I've never seen it before ;-)

Okay, [com.taoensso/sente "1.7.0-alpha4"] is up on Clojars. Does this fully resolve #135 for you? Okay to close that?

[danielcompton]

Yep, closed it now. Thanks!

[ptaoussanis]

Quick update: just pushed [com.taoensso/sente "1.7.0-alpha5"] since we missed a case: the params weren't being included in Ajax-based callback requests. Should be okay now.

[danielcompton]

Thanks, and sorry about that.

[ptaoussanis]

Oh, no problem - easy to miss. PR was very helpful, wouldn't have gotten to this otherwise.