Support making a sente websocket client connection from clj

[kaosko]

• add dependency to pure java ws implementation org.java-websocket/Java-WebSocket
• make chsk-**connect! functions available to clj
• allow setting custom headers to the (java) webclient
• add an "Authorization" header based authorization mechanism for incoming get-or-ws request
• ignore csrf and origin checks if Authorization header exists
• pass uid to authorized-header-fn
• use CloseFrame.normal to test if java ws close was clean
• Make send-buffers public similar to connected-uids

[kaosko]

I'm using Sente to send data from on-premise edge nodes to the cloud. Cleaned up the fork I made, hoping you could merge this upstream. It's rebased on top of latest Sente. Also implemented token based authorization via Authorization header (for clj clients). send-buffers are public so I could monitor possible congestions.

[kaosko]

Oops, I hadn't even noticed that there's another, still open pull request #275 for the same functionality. Anyway, this doesn't use aleph but Java-WebSocket, which is an excellent match as it's modeled after the browser websocket api.

[ptaoussanis]

@kaosko Hi Kalle!

Definitely interested in this, and Java-WebSocket looks like a good fit.

This is currently a very large PR. Would it be feasible to try breaking this up into smaller, logically-contained commits?

For example:

• Purely aesthetic formatting/indentation changes/fixes.
• Make send-buffers public
• Add new authorization header stuff
• Make some extra fns available to clj
• etc.

This'll make your reviewer's job much easier - since it'll enable them to step through the logical changes 1 by 1.

Intending to try batch some open-source work this week/end for the first time in a while - so if you're able to get this updated soon, I'll try to review+merge ASAP for possible inclusion in an upcoming release.

Cheers!

[kaosko]

Sure, I'm open to that and also happen to have some time in the next few days. However, if you just want to see the commit history of how I worked towards it, you can review https://github.com/kaosko/sente/tree/serverws. There really isn't that much new code, but the biggest issue making the diff so large is caused by the removal of #?(:cljs in several places. If I reworked the history so that I first remove those, add the changes for handling ws clj and then add the rest and keep indentation as is throughout, would that be ok? Or, would you rather like me to rebase the whole serverws on top of yours and take all the commits in?

[ptaoussanis]

If I reworked the history so that I first remove those, add the changes for handling ws clj and then add the rest and keep indentation as is throughout, would that be ok? Or, would you rather like me to rebase the whole serverws on top of yours and take all the commits in?

Not sure I understand the two options, sorry.

In case it helps to clarify: the motivation for separate commits here isn't purely for the convenience of review, but also for clarity in Sente's own commit history. (I.e. the intention will be to retain relevant commit structure in Sente master after merge).

Re: noise from conditionals / indenting / etc. - feel free to make any changes/fixes you like, would just request that those changes be separate from changes introducing new code or significant code modifications.

Or put another way: to try separate significant changes (requiring review) from incidental changes.

Hope that makes sense?

[kaosko]

Ok, well just taking the original commits from the branch is the easiest for me, but I'll condense it to some suitable number of commits for clean history.

[kaosko]

Now rebased on top of your latest changes and reverted indentation to highlight the functional changes. However, I did keep the Authorization header related changes together with generic clj ws client changes because to me, they kind of go together - it'd be odd not to support any authentication mechanism for clj clients. Making the send-buffers public are in a separate commits. Hope this is more reviewable. Let me know if you want me to commit the indentation as well or you do it.

[kaosko]

Rebased once more on top of your renames and authorization fn inclusion.

[ptaoussanis]

Thanks a lot @kaosko - no need for further rebases. I have what I need for the moment, just blocking on time to properly go through the changes. Will try this weekend.

[kaosko]

Great thanks. I'm currently using my latest rebase in a real world app but for maintenance purposes, happy to get this to upstream one way or another.

[ptaoussanis]

@kaosko Have a branch here with some minor tweaks to this PR. Haven't tested the Java client.

If you get an opportunity, would you please check it out and let me know if it's working for you? Also on Clojars as [com.taoensso/sente "1.16.0-SNAPSHOT"].

Thanks!

[kaosko]

I'll test the branch during the week thanks.

[kaosko]

Confirming that [com.taoensso/sente "1.16.0-SNAPSHOT"] works.

[ptaoussanis]

Merging manually now, thanks!