Safely call close on the websocket client with synchronized calls (#83)

* Safely call close on the websocket client with synchronized calls

* Added a test and more synchronized logic

Author: mmimeault

ParseLiveQuery/src/main/java/com/parse/OkHttp3SocketClientFactory.java

@@ -35,7 +35,7 @@ static class OkHttp3WebSocketClient implements WebSocketClient {
 
         private final WebSocketClientCallback webSocketClientCallback;
         private WebSocket webSocket;
-        private State state = State.NONE;
+        private volatile State state = State.NONE;
         private final OkHttpClient client;
         private final String url;
         private final int STATUS_CODE = 1000;
@@ -73,7 +73,7 @@ public void onFailure(okhttp3.WebSocket webSocket, Throwable t, Response respons
         };
 
         private OkHttp3WebSocketClient(OkHttpClient okHttpClient,
-                WebSocketClientCallback webSocketClientCallback, URI hostUrl) {
+                                       WebSocketClientCallback webSocketClientCallback, URI hostUrl) {
             client = okHttpClient;
             this.webSocketClientCallback = webSocketClientCallback;
             url = hostUrl.toString();
@@ -94,12 +94,14 @@ public synchronized void open() {
 
         @Override
         public synchronized void close() {
-            setState(State.DISCONNECTING);
-            webSocket.close(STATUS_CODE, CLOSING_MSG);
+            if (State.NONE != state) {
+                setState(State.DISCONNECTING);
+                webSocket.close(STATUS_CODE, CLOSING_MSG);
+            }
         }
 
         @Override
-        public void send(String message) {
+        public synchronized void send(String message) {
             if (state == State.CONNECTED) {
                 webSocket.send(message);
             }
@@ -110,10 +112,12 @@ public State getState() {
             return state;
         }
 
-        private synchronized void setState(State newState) {
-            this.state = newState;
+        private void setState(State newState) {
+            synchronized (this) {
+                this.state = newState;
+            }
             this.webSocketClientCallback.stateChanged();
         }
     }
 
-}
+}

ParseLiveQuery/src/main/java/com/parse/ParseLiveQueryClientImpl.java

@@ -1,9 +1,7 @@
 package com.parse;
 
 import android.util.Log;
-import bolts.Continuation;
-import bolts.Task;
-import okhttp3.OkHttpClient;
+
 import org.json.JSONException;
 import org.json.JSONObject;
 
@@ -17,6 +15,10 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.Executor;
 
+import bolts.Continuation;
+import bolts.Task;
+import okhttp3.OkHttpClient;
+
 import static com.parse.Parse.checkInit;
 
 /* package */ class ParseLiveQueryClientImpl implements ParseLiveQueryClient {
@@ -142,7 +144,7 @@ public <T extends ParseObject> void unsubscribe(final ParseQuery<T> query, final
     }
 
     @Override
-    public void reconnect() {
+    public synchronized void reconnect() {
         if (webSocketClient != null) {
             webSocketClient.close();
         }
@@ -154,7 +156,7 @@ public void reconnect() {
     }
 
     @Override
-    public void disconnect() {
+    public synchronized void disconnect() {
         if (webSocketClient != null) {
             webSocketClient.close();
             webSocketClient = null;

ParseLiveQuery/src/test/java/com/parse/TestOkHttpClientFactory.java

@@ -0,0 +1,49 @@
+package com.parse;
+
+import junit.framework.Assert;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import java.net.URI;
+
+import okhttp3.OkHttpClient;
+
+import static com.parse.WebSocketClient.State.NONE;
+
+@RunWith(MockitoJUnitRunner.class)
+public class TestOkHttpClientFactory {
+
+    @Mock
+    private OkHttpClient okHttpClientMock;
+
+    @Mock
+    private WebSocketClient.WebSocketClientCallback webSocketClientCallbackMock;
+
+    private OkHttp3SocketClientFactory okHttp3SocketClientFactory;
+    private WebSocketClient webSocketClient;
+
+    @Before
+    public void setUp() throws Exception {
+        okHttp3SocketClientFactory = new OkHttp3SocketClientFactory(okHttpClientMock);
+        webSocketClient = okHttp3SocketClientFactory.createInstance(webSocketClientCallbackMock, new URI("http://www.test.com"));
+    }
+
+    @After
+    public void tearDown() {
+        webSocketClient.close();
+    }
+
+    @Test
+    public void testClientCloseWithoutOpenShouldBeNoOp()  {
+        Assert.assertEquals(NONE, webSocketClient.getState());
+        webSocketClient.close();
+        webSocketClient.send("test");
+        Assert.assertEquals(NONE, webSocketClient.getState());
+    }
+
+}