Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support subscribe result data #54

Merged
merged 4 commits into from
Oct 11, 2021
Merged

Conversation

FZambia
Copy link
Member

@FZambia FZambia commented Oct 8, 2021

Adds data field to SubscribeSuccessEvent. This allows to put some custom data on server side to be available only to current connection. I.e. some initial subscription data. Specifically, this makes it possible to use data from Subscribe proxy: https://centrifugal.dev/docs/server/proxy#subscribe-proxy.

@FZambia FZambia temporarily deployed to test_ci October 8, 2021 12:32 Inactive
@SimonVillage
Copy link
Contributor

String toString() {
return 'ServerSubscribeEvent{channel: $channel, isResubscribed: $isResubscribed, isRecovered: $isRecovered}';
}

Should this change to something like

  String toString() {
    return 'SubscribeSuccessEvent{isResubscribed: $isResubscribed, isRecovered: $isRecovered, data: ${utf8.decode(data, allowMalformed: true)}}';
  }

@FZambia
Copy link
Member Author

FZambia commented Oct 9, 2021

Should this change to something like

I think no - this is just a string representation so no need to include all fields to it, specifically data can be non utf-8 (any binary supported here) and several kilobytes long for example.

@FZambia FZambia temporarily deployed to test_ci October 9, 2021 07:39 Inactive
@FZambia
Copy link
Member Author

FZambia commented Oct 9, 2021

Actually just noticed that for ConnectEvent we already have data included in string representation. So let's be consistent – added data to string method for SubscribeSuccessEvent too.

@FZambia FZambia merged commit c4272b2 into master Oct 11, 2021
@FZambia FZambia deleted the support_subscribe_result_data branch July 17, 2022 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants