Add data getter protocole for update token with data #105
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
FZambia
reviewed
Oct 28, 2024
| } | ||
|
|
||
| public protocol CentrifugeConnectionDataGetter: NSObject { | ||
| func getConnectionData(_ event: CentrifugeConnectionTokenEvent, completion: @escaping (Result<Data, Error>) -> ()) |
Member
There was a problem hiding this comment.
It should not accept CentrifugeConnectionTokenEvent, must be separate type.
FZambia
reviewed
Oct 28, 2024
| (!token.isEmpty && config.tokenGetter != nil) { | ||
| let updateData = config.dataGetter != nil | ||
| let updateToken = config.tokenGetter != nil | ||
| if updateData { |
Member
There was a problem hiding this comment.
I do not see where token is updated in this case. Generally, I think a bit more code refactoring required for this feature to make the code easier to follow. It already was a bit hard to follow.
Member
|
During last days I was thinking that getter protocols for data is not the best approach to use in our SDKs 🤔 It makes sense for tokens, but for data/headers it's much simpler to just provide Client.setData and Client.setHeader methods. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since in other projects for the client (JS and Java where I noticed this) there is a token update via a dataGetter and on the project I am working on this was also necessary, I added a new protocol CentrifugeConnectionDataGetter which in its completion can accept an object of the Date type and there you can return as our back expects from us
I hope this will help others too