Skip to content

Dispatch better disconnect events #39

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Mar 29, 2017
Merged

Dispatch better disconnect events #39

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c607a17
Fix #38 by sending a proper close() status code
Mar 28, 2017
b6a5fd2
Add userInitiated boolean to the on-disconnected callback
Mar 28, 2017
acd04c1
Dispatch onDisconnected after a socket error
Mar 28, 2017
8481649
Fix CR comments
Mar 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ParseLiveQuery/src/main/java/com/parse/OkHttp3SocketClientFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public WebSocketClient createInstance(WebSocketClient.WebSocketClientCallback we
return new OkHttp3WebSocketClient(mClient, webSocketClientCallback, hostUrl);
}

class OkHttp3WebSocketClient implements WebSocketClient {
static class OkHttp3WebSocketClient implements WebSocketClient {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was otherwise being linked to the outer class at runtime, leading to a possible memory leak. The Factory and Client do not need to be tied together for any reason, so this should be declared static.


private static final String LOG_TAG = "OkHttpWebSocketClient";

Expand All @@ -38,7 +38,7 @@ class OkHttp3WebSocketClient implements WebSocketClient {
private State state = State.NONE;
private final OkHttpClient client;
private final String url;
private final int STATUS_CODE = 200;
private final int STATUS_CODE = 1000;
private final String CLOSING_MSG = "User invoked close";

private final WebSocketListener handler = new WebSocketListener() {
Expand Down
2 changes: 1 addition & 1 deletion ParseLiveQuery/src/main/java/com/parse/ParseLiveQueryClientCallbacks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
public interface ParseLiveQueryClientCallbacks {
void onLiveQueryClientConnected(ParseLiveQueryClient client);

void onLiveQueryClientDisconnected(ParseLiveQueryClient client);
void onLiveQueryClientDisconnected(ParseLiveQueryClient client, boolean userInitiated);

void onLiveQueryError(ParseLiveQueryClient client, LiveQueryException reason);

Expand Down
4 changes: 2 additions & 2 deletions ParseLiveQuery/src/main/java/com/parse/ParseLiveQueryClientImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ public Void then(Task<Void> task) throws Exception {
@Override
public void disconnect() {
if (webSocketClient != null) {
disconnectAsync();
userInitiatedDisconnect = true;
disconnectAsync();
}
}

Expand Down Expand Up @@ -254,7 +254,7 @@ private void dispatchConnected() {

private void dispatchDisconnected() {
for (ParseLiveQueryClientCallbacks callback : mCallbacks) {
callback.onLiveQueryClientDisconnected(this);
callback.onLiveQueryClientDisconnected(this, userInitiatedDisconnect);
}
}

Expand Down
23 changes: 20 additions & 3 deletions ParseLiveQuery/src/test/java/com/parse/TestParseLiveQueryClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,25 @@ public void testCallbackNotifiedOnDisconnect() throws Exception {
parseLiveQueryClient.registerListener(callbacks);
callbacks.transcript.assertNoEventsSoFar();

// Unexpected close from the server:
webSocketClientCallback.onClose();
callbacks.transcript.assertEventsSoFar("onLiveQueryClientDisconnected");
callbacks.transcript.assertEventsSoFar("onLiveQueryClientDisconnected: false");
}


Copy link
Contributor

Choose a reason for hiding this comment

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

extra space here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops. Will fix

@Test
public void testCallbackNotifiedOnDisconnect_expected() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be camel case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, ok - I've typically used _ to delineate a related test with different expectations or states.

Looking at the Parse-SDK-Android repo though, I see that does not appear to be the case. Will rename.

LoggingCallbacks callbacks = new LoggingCallbacks();
parseLiveQueryClient.registerListener(callbacks);
callbacks.transcript.assertNoEventsSoFar();

parseLiveQueryClient.disconnect();
verify(webSocketClient, times(1)).close();

callbacks.transcript.assertNoEventsSoFar();
// the client is a mock, so it won't actually invoke the callback automatically
webSocketClientCallback.onClose();
callbacks.transcript.assertEventsSoFar("onLiveQueryClientDisconnected: true");
}

@Test
Expand Down Expand Up @@ -548,8 +565,8 @@ public void onLiveQueryClientConnected(ParseLiveQueryClient client) {
}

@Override
public void onLiveQueryClientDisconnected(ParseLiveQueryClient client) {
transcript.add("onLiveQueryClientDisconnected");
public void onLiveQueryClientDisconnected(ParseLiveQueryClient client, boolean userInitiated) {
transcript.add("onLiveQueryClientDisconnected: " + userInitiated);
}

@Override
Expand Down