Skip to content

Add callbacks to be notified of connect/disconnect/error events #34

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 1 commit into from
Mar 27, 2017
Merged

Add callbacks to be notified of connect/disconnect/error events #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion ParseLiveQuery/src/main/java/com/parse/ParseLiveQueryClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ public interface ParseLiveQueryClient {

void disconnect();

void registerListener(ParseLiveQueryClientCallbacks listener);

void unregisterListener(ParseLiveQueryClientCallbacks listener);

class Factory {

public static ParseLiveQueryClient getClient() {
Expand All @@ -38,5 +42,4 @@ static ParseLiveQueryClient getClient(URI uri, WebSocketClientFactory webSocketC
}

}

}
11 changes: 11 additions & 0 deletions ParseLiveQuery/src/main/java/com/parse/ParseLiveQueryClientCallbacks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.parse;

public interface ParseLiveQueryClientCallbacks {
void onLiveQueryClientConnected(ParseLiveQueryClient client);

void onLiveQueryClientDisconnected(ParseLiveQueryClient client);

void onLiveQueryError(ParseLiveQueryClient client, LiveQueryException reason);

void onSocketError(ParseLiveQueryClient client, Throwable reason);
}
48 changes: 47 additions & 1 deletion 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 @@ -9,6 +9,8 @@
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.Executor;

Expand All @@ -30,6 +32,8 @@
private final WebSocketClientFactory webSocketClientFactory;
private final WebSocketClient.WebSocketClientCallback webSocketClientCallback;

private final List<ParseLiveQueryClientCallbacks> mCallbacks = new ArrayList<>();

private WebSocketClient webSocketClient;
private int requestIdCount = 1;
private boolean userInitiatedDisconnect = false;
Expand Down Expand Up @@ -139,6 +143,16 @@ public void disconnect() {
}
}

@Override
public void registerListener(ParseLiveQueryClientCallbacks listener) {
mCallbacks.add(listener);
}

@Override
public void unregisterListener(ParseLiveQueryClientCallbacks listener) {
mCallbacks.remove(listener);
}

// Private methods

private synchronized int requestIdGenerator() {
Expand Down Expand Up @@ -189,6 +203,7 @@ private void parseMessage(String message) throws LiveQueryException {

switch (rawOperation) {
case "connected":
dispatchConnected();
Log.v(LOG_TAG, "Connected, sending pending subscription");
for (int i = 0; i < subscriptions.size(); i++) {
sendSubscription(subscriptions.valueAt(i));
Expand Down Expand Up @@ -231,6 +246,31 @@ private void parseMessage(String message) throws LiveQueryException {
}
}

private void dispatchConnected() {
for (ParseLiveQueryClientCallbacks callback : mCallbacks) {
callback.onLiveQueryClientConnected(this);
}
}

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


private void dispatchServerError(LiveQueryException exc) {
for (ParseLiveQueryClientCallbacks callback : mCallbacks) {
callback.onLiveQueryError(this, exc);
}
}

private void dispatchSocketError(Throwable reason) {
for (ParseLiveQueryClientCallbacks callback : mCallbacks) {
callback.onSocketError(this, reason);
}
}

private <T extends ParseObject> void handleSubscribedEvent(JSONObject jsonObject) throws JSONException {
final int requestId = jsonObject.getInt("requestId");
final Subscription<T> subscription = subscriptionForRequestId(requestId);
Expand Down Expand Up @@ -263,9 +303,13 @@ private <T extends ParseObject> void handleErrorEvent(JSONObject jsonObject) thr
String error = jsonObject.getString("error");
Boolean reconnect = jsonObject.getBoolean("reconnect");
final Subscription<T> subscription = subscriptionForRequestId(requestId);
LiveQueryException exc = new LiveQueryException.ServerReportedException(code, error, reconnect);

if (subscription != null) {
subscription.didEncounter(new LiveQueryException.ServerReportedException(code, error, reconnect), subscription.getQuery());
subscription.didEncounter(exc, subscription.getQuery());
}

dispatchServerError(exc);
}

private <T extends ParseObject> Subscription<T> subscriptionForRequestId(int requestId) {
Expand Down Expand Up @@ -341,11 +385,13 @@ public Void then(Task<Void> task) {
@Override
public void onClose() {
Log.v(LOG_TAG, "Socket onClose");
dispatchDisconnected();
Copy link
Contributor

@rogerhu rogerhu Mar 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Do you need to put dispatchConnected() on onOpen()}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dispatchConnected() happens only when the server responds with the op=connected message

Copy link
Contributor

Choose a reason for hiding this comment

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

gotcha

}

@Override
public void onError(Throwable exception) {
Log.e(LOG_TAG, "Socket onError", exception);
dispatchSocketError(exception);
}

@Override
Expand Down
67 changes: 66 additions & 1 deletion 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 @@ -7,12 +7,13 @@
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.annotation.Config;
import org.robolectric.util.Transcript;

import java.io.IOException;
import java.net.URI;
import java.util.LinkedList;
import java.util.Queue;
Expand Down Expand Up @@ -398,6 +399,46 @@ public void testDisconnectOnBackgroundThread() throws Exception {
verify(webSocketClient, times(1)).close();
}

@Test
public void testCallbackNotifiedOnDisconnect() throws Exception {
LoggingCallbacks callbacks = new LoggingCallbacks();
parseLiveQueryClient.registerListener(callbacks);
callbacks.transcript.assertNoEventsSoFar();

webSocketClientCallback.onClose();
callbacks.transcript.assertEventsSoFar("onLiveQueryClientDisconnected");
}

@Test
public void testCallbackNotifiedOnConnect() throws Exception {
LoggingCallbacks callbacks = new LoggingCallbacks();
parseLiveQueryClient.registerListener(callbacks);
callbacks.transcript.assertNoEventsSoFar();

reconnect();
callbacks.transcript.assertEventsSoFar("onLiveQueryClientConnected");
}

@Test
public void testCallbackNotifiedOnSocketError() throws Exception {
LoggingCallbacks callbacks = new LoggingCallbacks();
parseLiveQueryClient.registerListener(callbacks);
callbacks.transcript.assertNoEventsSoFar();

webSocketClientCallback.onError(new IOException("bad things happened"));
callbacks.transcript.assertEventsSoFar("onSocketError: java.io.IOException: bad things happened");
}

@Test
public void testCallbackNotifiedOnServerError() throws Exception {
LoggingCallbacks callbacks = new LoggingCallbacks();
parseLiveQueryClient.registerListener(callbacks);
callbacks.transcript.assertNoEventsSoFar();

webSocketClientCallback.onMessage(createErrorMessage(1).toString());
callbacks.transcript.assertEventsSoFar("onLiveQueryError: com.parse.LiveQueryException$ServerReportedException: Server reported error; code: 1, error: testError, reconnect: true");
}

private SubscriptionHandling<ParseObject> createSubscription(ParseQuery<ParseObject> parseQuery,
SubscriptionHandling.HandleSubscribeCallback<ParseObject> subscribeMockCallback) throws Exception {
SubscriptionHandling<ParseObject> subscriptionHandling = parseLiveQueryClient.subscribe(parseQuery).handleSubscribe(subscribeMockCallback);
Expand Down Expand Up @@ -498,6 +539,30 @@ private static JSONObject createObjectDeleteMessage(int requestId, ParseObject p
return jsonObject;
}

private static class LoggingCallbacks implements ParseLiveQueryClientCallbacks {
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be non-static given how you instantiate it in each test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think static vs non-static is relevant based on how it's instantiated here. It's declared static because it does not rely on anything from the outer/containing class (so it doesn't require an instance of the containing class).

It's probably not relevant in a unit test, but in general I prefer static inner class, unless it is explicitly required that the inner class instance must be associated with an instance of the outer class (such as a callback method that has to invoke something from the containing class). Not declaring the class static, even if you don't need make any calls to the outer class, can result in memory leaks since that instance of the inner class necessarily holds a reference to the instance of the outer class that created it.

Copy link
Contributor

@rogerhu rogerhu Mar 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I was more worried that the use of transcript could be misused if the class isn't instantiated but it looks to be declared private so can't be accessed directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah. Yeah the class still needs to be instantiated in order to use Transcript. It's declared final, but not static. So it is still a new Transcript instance for every instance of LoggingCallbacks (as it should be, in order to properly test that the callbacks are invoked when they're supposed to be).

Just want to make sure we're on the same page about the class and field declarations. The class being declared static does not mean that the transcript is a static field. It's still an instance field.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha, thanks for the clarification.

final Transcript transcript = new Transcript();

@Override
public void onLiveQueryClientConnected(ParseLiveQueryClient client) {
transcript.add("onLiveQueryClientConnected");
}

@Override
public void onLiveQueryClientDisconnected(ParseLiveQueryClient client) {
transcript.add("onLiveQueryClientDisconnected");
}

@Override
public void onLiveQueryError(ParseLiveQueryClient client, LiveQueryException reason) {
transcript.add("onLiveQueryError: " + reason);
}

@Override
public void onSocketError(ParseLiveQueryClient client, Throwable reason) {
transcript.add("onSocketError: " + reason);
}
}

private static class PauseableExecutor implements Executor {
private boolean isPaused = false;
private final Queue<Runnable> queue = new LinkedList<>();
Expand Down