From e28a12c61394055f0481d8ccad8f4c68339bd58b Mon Sep 17 00:00:00 2001 From: Lukas Piatkowski Date: Tue, 14 Feb 2017 09:37:33 -0800 Subject: [PATCH] Introducing Responder and JSONObject to JSPackagerClient Reviewed By: cwdick Differential Revision: D4537115 fbshipit-source-id: 61ee03d3e700bbee8b1cdb02bbf31a8491ca7891 --- .../react/devsupport/DevServerHelper.java | 22 ++-- .../devsupport/DevSupportManagerImpl.java | 26 ++-- .../packagerconnection/JSPackagerClient.java | 111 ++++++++++++------ .../ReconnectingWebSocket.java | 28 ++--- .../JSPackagerClientTest.java | 28 ++--- 5 files changed, 112 insertions(+), 103 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java index 24876dfc099..4f4358ac121 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java @@ -31,7 +31,6 @@ import com.facebook.react.common.network.OkHttpCallUtil; import com.facebook.react.devsupport.interfaces.PackagerStatusCallback; import com.facebook.react.modules.systeminfo.AndroidInfoHelpers; import com.facebook.react.packagerconnection.JSPackagerClient; -import com.facebook.react.packagerconnection.ReconnectingWebSocket; import okhttp3.Call; import okhttp3.Callback; @@ -89,7 +88,7 @@ public class DevServerHelper { public interface PackagerCommandListener { void onPackagerReloadCommand(); void onCaptureHeapCommand(); - void onPokeSamplingProfilerCommand(@Nullable final ReconnectingWebSocket.WebSocketSender webSocket); + void onPokeSamplingProfilerCommand(@Nullable final JSPackagerClient.Responder responder); } private final DevInternalSettings mSettings; @@ -121,28 +120,25 @@ public class DevServerHelper { } new AsyncTask() { @Override - protected Void doInBackground(Void... params) { + protected Void doInBackground(Void... backgroundParams) { Map handlers = new HashMap(); - handlers.put("reload", new JSPackagerClient.RequestHandler() { + handlers.put("reload", new JSPackagerClient.NotificationOnlyHandler() { @Override - public void onNotification( - @Nullable ReconnectingWebSocket.WebSocketSender webSocket) { + public void onNotification(@Nullable Object params) { commandListener.onPackagerReloadCommand(); } }); - handlers.put("captureHeap", new JSPackagerClient.RequestHandler() { + handlers.put("captureHeap", new JSPackagerClient.NotificationOnlyHandler() { @Override - public void onNotification( - @Nullable ReconnectingWebSocket.WebSocketSender webSocket) { + public void onNotification(@Nullable Object params) { commandListener.onCaptureHeapCommand(); } }); - handlers.put("pokeSamplingProfiler", new JSPackagerClient.RequestHandler() { + handlers.put("pokeSamplingProfiler", new JSPackagerClient.RequestOnlyHandler() { @Override - public void onNotification( - @Nullable ReconnectingWebSocket.WebSocketSender webSocket) { - commandListener.onPokeSamplingProfilerCommand(webSocket); + public void onRequest(@Nullable Object params, JSPackagerClient.Responder responder) { + commandListener.onPokeSamplingProfilerCommand(responder); } }); diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java index bd34a37f9ad..e17b7b17b0d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java @@ -56,13 +56,12 @@ import com.facebook.react.devsupport.interfaces.DevSupportManager; import com.facebook.react.devsupport.interfaces.PackagerStatusCallback; import com.facebook.react.devsupport.interfaces.StackFrame; import com.facebook.react.modules.debug.interfaces.DeveloperSettings; -import com.facebook.react.packagerconnection.ReconnectingWebSocket; +import com.facebook.react.packagerconnection.JSPackagerClient; import okhttp3.MediaType; import okhttp3.OkHttpClient; import okhttp3.Request; import okhttp3.RequestBody; -import okhttp3.ws.WebSocket; /** * Interface for accessing and interacting with development features. Following features @@ -693,12 +692,11 @@ public class DevSupportManagerImpl implements } @Override - public void onPokeSamplingProfilerCommand( - @Nullable final ReconnectingWebSocket.WebSocketSender webSocket) { + public void onPokeSamplingProfilerCommand(@Nullable final JSPackagerClient.Responder responder) { UiThreadUtil.runOnUiThread(new Runnable() { @Override public void run() { - handlePokeSamplingProfiler(webSocket); + handlePokeSamplingProfiler(responder); } }); } @@ -709,8 +707,7 @@ public class DevSupportManagerImpl implements JSCHeapUpload.captureCallback(mDevServerHelper.getHeapCaptureUploadUrl())); } - private void handlePokeSamplingProfiler( - @Nullable ReconnectingWebSocket.WebSocketSender webSocket) { + private void handlePokeSamplingProfiler(@Nullable final JSPackagerClient.Responder responder) { try { List pokeResults = JSCSamplingProfiler.poke(60000); for (String result : pokeResults) { @@ -720,14 +717,11 @@ public class DevSupportManagerImpl implements ? "Started JSC Sampling Profiler" : "Stopped JSC Sampling Profiler", Toast.LENGTH_LONG).show(); - if (webSocket != null) { - // WebSocket is provided, so there is a client waiting our response - webSocket.sendMessage( - RequestBody.create( - WebSocket.TEXT, - result == null - ? "{\"target\":\"profiler\", \"action\":\"started\"}" - : result)); + if (responder != null) { + // Responder is provided, so there is a client waiting our response + responder.respond(result == null + ? "{\"target\":\"profiler\", \"action\":\"started\"}" + : result); } else if (result != null) { // The profile was not initiated by external client, so process the // profile if there is one in the result @@ -738,8 +732,6 @@ public class DevSupportManagerImpl implements } } catch (JSCSamplingProfiler.ProfilerException e) { showNewJavaError(e.getMessage(), e); - } catch (IOException e) { - showNewJavaError(e.getMessage(), e); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/packagerconnection/JSPackagerClient.java b/ReactAndroid/src/main/java/com/facebook/react/packagerconnection/JSPackagerClient.java index 5e08ee9ac54..9482b2753d6 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/packagerconnection/JSPackagerClient.java +++ b/ReactAndroid/src/main/java/com/facebook/react/packagerconnection/JSPackagerClient.java @@ -10,25 +10,68 @@ package com.facebook.react.packagerconnection; import javax.annotation.Nullable; -import java.io.IOException; import java.util.Map; -import android.util.JsonReader; -import android.util.JsonToken; - import com.facebook.common.logging.FLog; +import okhttp3.RequestBody; import okhttp3.ResponseBody; import okhttp3.ws.WebSocket; +import org.json.JSONObject; + /** * A client for packager that uses WebSocket connection. */ final public class JSPackagerClient implements ReconnectingWebSocket.MessageCallback { private static final String TAG = JSPackagerClient.class.getSimpleName(); + private static final int PROTOCOL_VERSION = 1; + + public class Responder { + private Object mId; + + public Responder(Object id) { + mId = id; + } + + public void respond(String result) { + try { + mWebSocket.sendMessage(RequestBody.create(WebSocket.TEXT, result)); + } catch (Exception e) { + FLog.e(TAG, "Responding failed", e); + } + } + + public void error(Object error) { + try { + JSONObject message = new JSONObject(); + message.put("id", mId); + message.put("error", error); + mWebSocket.sendMessage(RequestBody.create(WebSocket.TEXT, message.toString())); + } catch (Exception e) { + FLog.e(TAG, "Responding with error failed", e); + } + } + } public interface RequestHandler { - public void onNotification(@Nullable ReconnectingWebSocket.WebSocketSender webSocket); + public void onRequest(@Nullable Object params, Responder responder); + public void onNotification(@Nullable Object params); + } + + public static abstract class NotificationOnlyHandler implements RequestHandler { + final public void onRequest(@Nullable Object params, Responder responder) { + responder.error("Request is not supported"); + FLog.e(TAG, "Request is not supported"); + } + abstract public void onNotification(@Nullable Object params); + } + + public static abstract class RequestOnlyHandler implements RequestHandler { + abstract public void onRequest(@Nullable Object params, Responder responder); + final public void onNotification(@Nullable Object params) { + FLog.e(TAG, "Notification is not supported"); + } } private ReconnectingWebSocket mWebSocket; @@ -49,47 +92,45 @@ final public class JSPackagerClient implements ReconnectingWebSocket.MessageCall } @Override - public void onMessage(@Nullable ReconnectingWebSocket.WebSocketSender webSocket, ResponseBody response) { + public void onMessage(ResponseBody response) { if (response.contentType() != WebSocket.TEXT) { - FLog.w(TAG, "Websocket received unexpected message with payload of type " + response.contentType()); + FLog.w( + TAG, + "Websocket received message with payload of unexpected type " + response.contentType()); return; } try { - JsonReader reader = new JsonReader(response.charStream()); + JSONObject message = new JSONObject(response.string()); - Integer version = null; - String target = null; - String action = null; + int version = message.optInt("version"); + String target = message.optString("target"); + String action = message.optString("action"); - reader.beginObject(); - while (reader.hasNext()) { - String field = reader.nextName(); - - if (JsonToken.NULL == reader.peek()) { - reader.skipValue(); - continue; - } - - if ("version".equals(field)) { - version = reader.nextInt(); - } else if ("target".equals(field)) { - target = reader.nextString(); - } else if ("action".equals(field)) { - action = reader.nextString(); - } - } - reader.close(); - - if (version == null || target == null || action == null || version != 1) { + if (version != PROTOCOL_VERSION) { + FLog.e( + TAG, + "Message with incompatible or missing version of protocol received: " + version); return; } - if ("bridge".equals(target) && mRequestHandlers.containsKey(action)) { - mRequestHandlers.get(action).onNotification(webSocket); + if (!"bridge".equals(target)) { + return; } - } catch (IOException e) { - FLog.e(TAG, "Parsing response message from websocket failed", e); + + RequestHandler handler = mRequestHandlers.get(action); + if (handler == null) { + FLog.e(TAG, "No request handler for action: " + action); + return; + } + + if (!"pokeSamplingProfiler".equals(action)) { + handler.onNotification(null); + } else { + handler.onRequest(null, new Responder("profiler")); + } + } catch (Exception e) { + FLog.e(TAG, "Handling the message failed", e); } finally { response.close(); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/packagerconnection/ReconnectingWebSocket.java b/ReactAndroid/src/main/java/com/facebook/react/packagerconnection/ReconnectingWebSocket.java index 40ec85951fd..f774c1d1077 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/packagerconnection/ReconnectingWebSocket.java +++ b/ReactAndroid/src/main/java/com/facebook/react/packagerconnection/ReconnectingWebSocket.java @@ -11,6 +11,7 @@ package com.facebook.react.packagerconnection; import javax.annotation.Nullable; import java.io.IOException; +import java.nio.channels.ClosedChannelException; import java.util.concurrent.TimeUnit; import android.os.Handler; @@ -36,20 +37,8 @@ final public class ReconnectingWebSocket implements WebSocketListener { private static final int RECONNECT_DELAY_MS = 2000; - static public class WebSocketSender { - private WebSocket mWebSocket; - - public WebSocketSender(WebSocket webSocket) { - mWebSocket = webSocket; - } - - public void sendMessage(RequestBody requestBody) throws IOException { - mWebSocket.sendMessage(requestBody); - } - } - public interface MessageCallback { - void onMessage(@Nullable WebSocketSender webSocket, ResponseBody message); + void onMessage(ResponseBody message); } private final String mUrl; @@ -150,10 +139,7 @@ final public class ReconnectingWebSocket implements WebSocketListener { @Override public synchronized void onMessage(ResponseBody message) { if (mCallback != null) { - WebSocketSender webSocketSender = mWebSocket == null - ? null - : new WebSocketSender(mWebSocket); - mCallback.onMessage(webSocketSender, message); + mCallback.onMessage(message); } } @@ -167,4 +153,12 @@ final public class ReconnectingWebSocket implements WebSocketListener { reconnect(); } } + + public synchronized void sendMessage(RequestBody message) throws IOException { + if (mWebSocket != null) { + mWebSocket.sendMessage(message); + } else { + throw new ClosedChannelException(); + } + } } diff --git a/ReactAndroid/src/test/java/com/facebook/react/packagerconnection/JSPackagerClientTest.java b/ReactAndroid/src/test/java/com/facebook/react/packagerconnection/JSPackagerClientTest.java index dcca178ae0d..41286165510 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/packagerconnection/JSPackagerClientTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/packagerconnection/JSPackagerClientTest.java @@ -37,14 +37,12 @@ public class JSPackagerClientTest { JSPackagerClient.RequestHandler handler = mock(JSPackagerClient.RequestHandler.class); final JSPackagerClient client = new JSPackagerClient("ws://not_needed", createRH("actionValue", handler)); WebSocket webSocket = mock(WebSocket.class); - ReconnectingWebSocket.WebSocketSender wbs = new ReconnectingWebSocket.WebSocketSender(webSocket); client.onMessage( - wbs, ResponseBody.create( WebSocket.TEXT, "{\"version\": 1, \"target\": \"bridge\", \"action\": \"actionValue\"}")); - verify(handler).onNotification(wbs); + verify(handler).onNotification(any()); } @Test @@ -52,14 +50,12 @@ public class JSPackagerClientTest { JSPackagerClient.RequestHandler handler = mock(JSPackagerClient.RequestHandler.class); final JSPackagerClient client = new JSPackagerClient("ws://not_needed", createRH("actionValue", handler)); WebSocket webSocket = mock(WebSocket.class); - ReconnectingWebSocket.WebSocketSender wbs = new ReconnectingWebSocket.WebSocketSender(webSocket); client.onMessage( - wbs, ResponseBody.create( WebSocket.BINARY, "{\"version\": 1, \"target\": \"bridge\", \"action\": \"actionValue\"}")); - verify(handler, never()).onNotification(wbs); + verify(handler, never()).onNotification(any()); } @Test @@ -67,14 +63,12 @@ public class JSPackagerClientTest { JSPackagerClient.RequestHandler handler = mock(JSPackagerClient.RequestHandler.class); final JSPackagerClient client = new JSPackagerClient("ws://not_needed", createRH("actionValue", handler)); WebSocket webSocket = mock(WebSocket.class); - ReconnectingWebSocket.WebSocketSender wbs = new ReconnectingWebSocket.WebSocketSender(webSocket); client.onMessage( - wbs, ResponseBody.create( WebSocket.TEXT, "{\"version\": 1, \"action\": \"actionValue\"}")); - verify(handler, never()).onNotification(wbs); + verify(handler, never()).onNotification(any()); } @Test @@ -82,14 +76,12 @@ public class JSPackagerClientTest { JSPackagerClient.RequestHandler handler = mock(JSPackagerClient.RequestHandler.class); final JSPackagerClient client = new JSPackagerClient("ws://not_needed", createRH("actionValue", handler)); WebSocket webSocket = mock(WebSocket.class); - ReconnectingWebSocket.WebSocketSender wbs = new ReconnectingWebSocket.WebSocketSender(webSocket); client.onMessage( - wbs, ResponseBody.create( WebSocket.TEXT, "{\"version\": 1, \"target\": null, \"action\": \"actionValue\"}")); - verify(handler, never()).onNotification(wbs); + verify(handler, never()).onNotification(any()); } @Test @@ -97,14 +89,12 @@ public class JSPackagerClientTest { JSPackagerClient.RequestHandler handler = mock(JSPackagerClient.RequestHandler.class); final JSPackagerClient client = new JSPackagerClient("ws://not_needed", createRH("actionValue", handler)); WebSocket webSocket = mock(WebSocket.class); - ReconnectingWebSocket.WebSocketSender wbs = new ReconnectingWebSocket.WebSocketSender(webSocket); client.onMessage( - wbs, ResponseBody.create( WebSocket.TEXT, "{\"version\": 1, \"target\": \"bridge\"}")); - verify(handler, never()).onNotification(wbs); + verify(handler, never()).onNotification(any()); } @Test @@ -112,14 +102,12 @@ public class JSPackagerClientTest { JSPackagerClient.RequestHandler handler = mock(JSPackagerClient.RequestHandler.class); final JSPackagerClient client = new JSPackagerClient("ws://not_needed", createRH("actionValue", handler)); WebSocket webSocket = mock(WebSocket.class); - ReconnectingWebSocket.WebSocketSender wbs = new ReconnectingWebSocket.WebSocketSender(webSocket); client.onMessage( - wbs, ResponseBody.create( WebSocket.TEXT, "{\"version\": 1, \"target\": \"bridge\", \"action\": null}")); - verify(handler, never()).onNotification(wbs); + verify(handler, never()).onNotification(any()); } @Test @@ -127,13 +115,11 @@ public class JSPackagerClientTest { JSPackagerClient.RequestHandler handler = mock(JSPackagerClient.RequestHandler.class); final JSPackagerClient client = new JSPackagerClient("ws://not_needed", createRH("actionValue", handler)); WebSocket webSocket = mock(WebSocket.class); - ReconnectingWebSocket.WebSocketSender wbs = new ReconnectingWebSocket.WebSocketSender(webSocket); client.onMessage( - wbs, ResponseBody.create( WebSocket.TEXT, "{\"version\": 2, \"target\": \"bridge\", \"action\": \"actionValue\"}")); - verify(handler, never()).onNotification(wbs); + verify(handler, never()).onNotification(any()); } }