Skip to content
This repository was archived by the owner on Sep 28, 2022. It is now read-only.

RealtimeController:subscribe#58

Merged
scottinet merged 12 commits into
3-devfrom
KZL-1352-realtime-controller-subscribe
Feb 25, 2020
Merged

RealtimeController:subscribe#58
scottinet merged 12 commits into
3-devfrom
KZL-1352-realtime-controller-subscribe

Conversation

@jenow
Copy link
Copy Markdown
Contributor

@jenow jenow commented Feb 18, 2020

What does this PR do ?

Add a RealtimeController with the subscribe method.

How should this be manually tested?

./gradlew test

@jenow jenow self-assigned this Feb 18, 2020
@jenow jenow added the wip label Feb 18, 2020
@jenow jenow removed the wip label Feb 19, 2020
Comment thread .travis.yml Outdated
Comment thread doc/3/controllers/realtime/subscribe/index.md Outdated
Comment thread doc/3/controllers/realtime/subscribe/index.md Outdated
Comment thread doc/3/controllers/realtime/index.md Outdated
Co-Authored-By: Adrien Maret <amaret93@gmail.com>
@jenow jenow requested a review from Aschen February 19, 2020 16:27
Copy link
Copy Markdown
Contributor

@scottinet scottinet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor issues here and there, nothing important.

But something bothers me: how are realtime subscriptions handled whenever the SDK reconnects after a connection loss?

Comment thread doc/3/controllers/realtime/subscribe/index.md Outdated
Comment thread doc/3/controllers/realtime/subscribe/index.md Outdated
Comment thread doc/3/core-classes/subscribe-options/index.md Outdated
Comment thread doc/3/core-classes/subscribe-options/index.md Outdated
Comment thread doc/3/essentials/realtime-notifications/index.md Outdated
Comment thread src/main/java/io/kuzzle/sdk/Options/SubscribeOptions.java
(response) -> {
String channel = ((ConcurrentHashMap<String, Object>) response.result).get("channel").toString();
Subscription subscription = new Subscription(
options == null ? new SubscribeOptions().isSubscribeToSelf() : options.isSubscribeToSelf(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the SubscribeOptions object is mutable, you should probably duplicate it at the beginning of this function and, from there, only use the copy, to prevent potential race conditions.

@jenow
Copy link
Copy Markdown
Contributor Author

jenow commented Feb 20, 2020

A few minor issues here and there, nothing important.

But something bothers me: how are realtime subscriptions handled whenever the SDK reconnects after a connection loss?

I didn't know the SDK should re subscribe after a connection loss. I took example on the C# SDK and it simply close all the subscriptions when it looses connection.
Should we resubscribe? It's not a lot of work.

jenow and others added 4 commits February 24, 2020 13:30
Co-Authored-By: Sébastien Cottinet <scottinet@protonmail.com>
…uzzleio/sdk-java into KZL-1352-realtime-controller-subscribe
@jenow jenow requested a review from scottinet February 24, 2020 16:49
@scottinet scottinet merged commit f65f9ae into 3-dev Feb 25, 2020
@scottinet scottinet deleted the KZL-1352-realtime-controller-subscribe branch February 25, 2020 09:38
@jenow jenow mentioned this pull request May 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants