search

centrifugal / centrifuge-swift

Swift client SDK for bidirectional real-time communication with Centrifugo and Centrifuge-based server over WebSocket
MIT License
48 stars 44 forks source link

Fix access levels for importing as CocoaPod #1

Closed fedulvtubudul closed 5 years ago

fedulvtubudul commented 5 years ago

Update the access level for some methods and properties implicitly marked as internal when Centrifugo is imported as a module (via CocoaPods)

FZambia commented 5 years ago

@fedulvtubudul hello! Thank you very much. Really appreciate your help. This is my first Swift library so I can miss obvious things.

There are also some problems with synchronization between threads I know about that result into crash so please do not use this lib in your production apps. I am looking in a way to fix those problems at moment. If you are interested - I can provide more details about remaining work here.

Regarding to your pull request: why do you need client property of CentrifugeClient class to be public btw? As far as I understand all required actions can be made without access to client property. This property can change during client lifetime - after reconnects for example - so my intention was to hide client ID from users.

fedulvtubudul commented 5 years ago

@fedulvtubudul hello! Thank you very much. Really appreciate your help. This is my first Swift library so I can miss obvious things.

We are happy as well just to see this native implementation of the Centrifuge client, as we were almost ready to start implementing it ourselves, while noticed this new repo few days ago :)

There are also some problems with synchronization between threads I know about that result into crash so please do not use this lib in your production apps. I am looking in a way to fix those problems at moment. If you are interested - I can provide more details about remaining work here.

I would really appreciate any details, as we are planning to use this lib in production-ready app in maybe a month I hope. Anything we will manage to fix or improve, will be published in our fork of the lib.

Regarding to your pull request: why do you need client property of CentrifugeClient class to be public btw? As far as I understand all required actions can be made without access to client property. This property can change during client lifetime - after reconnects for example - so my intention was to hide client ID from users.

OK, I see. Will return that one to internal.

FZambia commented 5 years ago

Ok, cool. So in general idea I want to achieve in this lib is making everything asynchronously. I'd also want users of library could write code sequentially without relying on callbacks. Like this:

client.connect()
client.publish(channel, data, completion: {....})

I.e. users don't need to wait for successful client connection callback before calling publish. Client internally waits for connection establishement and then proceeds with publish.

The same for subscribe:

let sub = client.newSubscription(channel)
sub.presence(completion: {...})

I.e. library will internally wait for subscription success or error and then will resolve presence call.

This all involves pretty complex logic of having some sort of futures that wait for event (successful connection to server or successful subscription/error in subscription) and then proceed sending required command over established connection or ready to use subscription. I have not come up to proper synchronization of all these things - there is a bit messy and dirty sync in some places at moment. And some races that result into crashes.