From 6fef0142955dfa779afd71d340d8c8f45f200640 Mon Sep 17 00:00:00 2001 From: Benoit Lemaire Date: Wed, 23 Nov 2016 08:49:18 -0800 Subject: [PATCH] Remove Jackson dependency Summary: This PR removes dependency to Jackson third-party library in Android React Native. Looking at some older PRs that got merged, it seems like some work had already been done to move away from Jackson. Anyway, there was only two classes left with a dependency on Jackson. I refactored the code to use android built-in `JsonReader` and `JsonWriter` classes instead. Prep work was done in https://github.com/facebook/react-native/pull/10516 introducing a few unit tests around serialization, to make sure that refactoring around serialization would not break things. All references to Jackson in build systems files (BUCK files & build.gradle) have also been removed now that no code depend anymore on this third-party library. Motivation behind this work is that third-party dependencies in Android React Native can prove to be a pain when trying to integrate React Native components into an already existing large Android application (I know this is not the most common use case for react-native ... yet ;P), that might a Closes https://github.com/facebook/react-native/pull/10521 Differential Revision: D4226705 Pulled By: mkonicek fbshipit-source-id: e3a7430a79dd00c871ba3c6a705b0b0c3ec3a701 --- ReactAndroid/build.gradle | 2 - .../java/com/facebook/react/cxxbridge/BUCK | 1 - .../java/com/facebook/react/devsupport/BUCK | 1 - .../devsupport/JSDebuggerWebSocketClient.java | 114 ++++++++---------- .../devsupport/JSPackagerWebSocketClient.java | 36 +++--- .../src/main/third-party/java/jackson/BUCK | 46 ------- .../test/java/com/facebook/react/bridge/BUCK | 2 - .../JSDebuggerWebSocketClientTest.java | 26 ++++ .../JSPackagerWebSocketClientTest.java | 20 +++ 9 files changed, 116 insertions(+), 132 deletions(-) delete mode 100644 ReactAndroid/src/main/third-party/java/jackson/BUCK diff --git a/ReactAndroid/build.gradle b/ReactAndroid/build.gradle index a74ca2080a38c6..d6a268335b5196 100644 --- a/ReactAndroid/build.gradle +++ b/ReactAndroid/build.gradle @@ -272,7 +272,6 @@ dependencies { compile 'com.facebook.fresco:fresco:0.11.0' compile 'com.facebook.fresco:imagepipeline-okhttp3:0.11.0' compile 'com.facebook.soloader:soloader:0.1.0' - compile 'com.fasterxml.jackson.core:jackson-core:2.2.3' compile 'com.google.code.findbugs:jsr305:3.0.0' compile 'com.squareup.okhttp3:okhttp:3.4.1' compile 'com.squareup.okhttp3:okhttp-urlconnection:3.4.1' @@ -282,7 +281,6 @@ dependencies { testCompile "junit:junit:${JUNIT_VERSION}" testCompile "org.powermock:powermock-api-mockito:${POWERMOCK_VERSION}" - testCompile 'com.fasterxml.jackson.core:jackson-databind:2.2.3' testCompile "org.powermock:powermock-module-junit4-rule:${POWERMOCK_VERSION}" testCompile "org.powermock:powermock-classloading-xstream:${POWERMOCK_VERSION}" testCompile "org.mockito:mockito-core:${MOCKITO_CORE_VERSION}" diff --git a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/BUCK b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/BUCK index 5155d6e98c2c1f..c82e783e90cb18 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/BUCK +++ b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/BUCK @@ -22,7 +22,6 @@ android_library( # dependency in the app works, too. gross. # '//native/react/jni:jni-internal', react_native_dep('third-party/java/infer-annotations:infer-annotations'), - react_native_dep('third-party/java/jackson:core'), react_native_dep('third-party/java/jsr-305:jsr-305'), react_native_target('java/com/facebook/react/bridge:bridge'), react_native_target('java/com/facebook/react/common:common'), diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/BUCK b/ReactAndroid/src/main/java/com/facebook/react/devsupport/BUCK index 791cf31ef335a2..f967b872999e88 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/BUCK +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/BUCK @@ -7,7 +7,6 @@ android_library( deps = [ react_native_dep('libraries/fbcore/src/main/java/com/facebook/common/logging:logging'), react_native_dep('third-party/java/infer-annotations:infer-annotations'), - react_native_dep('third-party/java/jackson:core'), react_native_dep('third-party/java/jsr-305:jsr-305'), react_native_dep('third-party/java/okhttp:okhttp3'), react_native_dep('third-party/java/okhttp:okhttp3-ws'), diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/JSDebuggerWebSocketClient.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/JSDebuggerWebSocketClient.java index 1541318e7a454d..21ddd022613cb0 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/JSDebuggerWebSocketClient.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/JSDebuggerWebSocketClient.java @@ -9,23 +9,23 @@ package com.facebook.react.devsupport; -import javax.annotation.Nullable; +import android.util.JsonReader; +import android.util.JsonToken; +import android.util.JsonWriter; + +import com.facebook.common.logging.FLog; +import com.facebook.infer.annotation.Assertions; +import com.facebook.react.common.JavascriptException; import java.io.IOException; import java.io.StringWriter; import java.util.HashMap; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; -import com.facebook.common.logging.FLog; -import com.facebook.infer.annotation.Assertions; -import com.facebook.react.common.JavascriptException; +import javax.annotation.Nullable; -import com.fasterxml.jackson.core.JsonFactory; -import com.fasterxml.jackson.core.JsonGenerator; -import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.core.JsonToken; import okhttp3.OkHttpClient; import okhttp3.Request; import okhttp3.RequestBody; @@ -42,7 +42,6 @@ public class JSDebuggerWebSocketClient implements WebSocketListener { private static final String TAG = "JSDebuggerWebSocketClient"; - private static final JsonFactory mJsonFactory = new JsonFactory(); public interface JSDebuggerCallback { void onSuccess(@Nullable String response); @@ -72,34 +71,19 @@ public void connect(String url, JSDebuggerCallback callback) { call.enqueue(this); } - /** - * Creates the next JSON message to send to remote JS executor, with request ID pre-filled in. - */ - private JsonGenerator startMessageObject(int requestID) throws IOException { - JsonGenerator jg = mJsonFactory.createGenerator(new StringWriter()); - jg.writeStartObject(); - jg.writeNumberField("id", requestID); - return jg; - } - - /** - * Takes in a JsonGenerator created by {@link #startMessageObject} and returns the stringified - * JSON - */ - private String endMessageObject(JsonGenerator jg) throws IOException { - jg.writeEndObject(); - jg.flush(); - return ((StringWriter) jg.getOutputTarget()).getBuffer().toString(); - } - public void prepareJSRuntime(JSDebuggerCallback callback) { int requestID = mRequestID.getAndIncrement(); mCallbacks.put(requestID, callback); try { - JsonGenerator jg = startMessageObject(requestID); - jg.writeStringField("method", "prepareJSRuntime"); - sendMessage(requestID, endMessageObject(jg)); + StringWriter sw = new StringWriter(); + JsonWriter js = new JsonWriter(sw); + js.beginObject() + .name("id").value(requestID) + .name("method").value("prepareJSRuntime") + .endObject() + .close(); + sendMessage(requestID, sw.toString()); } catch (IOException e) { triggerRequestFailure(requestID, e); } @@ -113,15 +97,18 @@ public void loadApplicationScript( mCallbacks.put(requestID, callback); try { - JsonGenerator jg = startMessageObject(requestID); - jg.writeStringField("method", "executeApplicationScript"); - jg.writeStringField("url", sourceURL); - jg.writeObjectFieldStart("inject"); + StringWriter sw = new StringWriter(); + JsonWriter js = new JsonWriter(sw) + .beginObject() + .name("id").value(requestID) + .name("method").value("executeApplicationScript") + .name("url").value(sourceURL) + .name("inject").beginObject(); for (String key : injectedObjects.keySet()) { - jg.writeObjectField(key, injectedObjects.get(key)); + js.name(key).value(injectedObjects.get(key)); } - jg.writeEndObject(); - sendMessage(requestID, endMessageObject(jg)); + js.endObject().endObject().close(); + sendMessage(requestID, sw.toString()); } catch (IOException e) { triggerRequestFailure(requestID, e); } @@ -131,16 +118,21 @@ public void executeJSCall( String methodName, String jsonArgsArray, JSDebuggerCallback callback) { - int requestID = mRequestID.getAndIncrement(); mCallbacks.put(requestID, callback); try { - JsonGenerator jg = startMessageObject(requestID); - jg.writeStringField("method", methodName); - jg.writeFieldName("arguments"); - jg.writeRawValue(jsonArgsArray); - sendMessage(requestID, endMessageObject(jg)); + StringWriter sw = new StringWriter(); + JsonWriter js = new JsonWriter(sw); + + js.beginObject() + .name("id").value(requestID) + .name("method").value(methodName); + /* JsonWriter does not offer writing raw string (without quotes), that's why + here we directly write to output string using the the underlying StringWriter */ + sw.append(",\"arguments\":").append(jsonArgsArray); + js.endObject().close(); + sendMessage(requestID, sw.toString()); } catch (IOException e) { triggerRequestFailure(requestID, e); } @@ -194,28 +186,26 @@ public void onMessage(ResponseBody response) throws IOException { return; } - String message = null; - try { - message = response.source().readUtf8(); - } finally { - response.close(); - } Integer replyID = null; try { - JsonParser parser = new JsonFactory().createParser(message); + JsonReader reader = new JsonReader(response.charStream()); String result = null; - while (parser.nextToken() != JsonToken.END_OBJECT) { - String field = parser.getCurrentName(); + reader.beginObject(); + while (reader.hasNext()) { + String field = reader.nextName(); + + if (JsonToken.NULL == reader.peek()) { + reader.skipValue(); + continue; + } + if ("replyID".equals(field)) { - parser.nextToken(); - replyID = parser.getIntValue(); + replyID = reader.nextInt(); } else if ("result".equals(field)) { - parser.nextToken(); - result = parser.getText(); + result = reader.nextString(); } else if ("error".equals(field)) { - parser.nextToken(); - String error = parser.getText(); + String error = reader.nextString(); abort(error, new JavascriptException(error)); } } @@ -228,6 +218,8 @@ public void onMessage(ResponseBody response) throws IOException { } else { abort("Parsing response message from websocket failed", e); } + } finally { + response.close(); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/JSPackagerWebSocketClient.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/JSPackagerWebSocketClient.java index 0a3f96951f0c52..2f93d0d03b847d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/JSPackagerWebSocketClient.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/JSPackagerWebSocketClient.java @@ -15,12 +15,11 @@ import android.os.Handler; import android.os.Looper; +import android.util.JsonReader; +import android.util.JsonToken; import com.facebook.common.logging.FLog; -import com.fasterxml.jackson.core.JsonFactory; -import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.core.JsonToken; import okhttp3.OkHttpClient; import okhttp3.Request; import okhttp3.Response; @@ -122,31 +121,28 @@ public void onMessage(ResponseBody response) throws IOException { return; } - String message = null; try { - message = response.source().readUtf8(); - } finally { - response.close(); - } - - try { - JsonParser parser = new JsonFactory().createParser(message); + JsonReader reader = new JsonReader(response.charStream()); Integer version = null; String target = null; String action = null; - while (parser.nextToken() != JsonToken.END_OBJECT) { - String field = parser.getCurrentName(); + reader.beginObject(); + while (reader.hasNext()) { + String field = reader.nextName(); + + if (JsonToken.NULL == reader.peek()) { + reader.skipValue(); + continue; + } + if ("version".equals(field)) { - parser.nextToken(); - version = parser.getIntValue(); + version = reader.nextInt(); } else if ("target".equals(field)) { - parser.nextToken(); - target = parser.getText(); + target = reader.nextString(); } else if ("action".equals(field)) { - parser.nextToken(); - action = parser.getText(); + action = reader.nextString(); } } if (version != 1) { @@ -159,6 +155,8 @@ public void onMessage(ResponseBody response) throws IOException { triggerMessageCallback(target, action); } catch (IOException e) { abort("Parsing response message from websocket failed", e); + } finally { + response.close(); } } diff --git a/ReactAndroid/src/main/third-party/java/jackson/BUCK b/ReactAndroid/src/main/third-party/java/jackson/BUCK deleted file mode 100644 index e6377adf4ed5ee..00000000000000 --- a/ReactAndroid/src/main/third-party/java/jackson/BUCK +++ /dev/null @@ -1,46 +0,0 @@ -include_defs('//ReactAndroid/DEFS') - -android_library( - name = 'jackson', - exported_deps = [ - ':databind', - ':annotations', - ], - visibility = ['//ReactAndroid/...',], -) - -prebuilt_jar( - name = 'core', - binary_jar = ':jackson-core-binary-jar', - visibility = ['//ReactAndroid/...',], -) - -remote_file( - name = 'jackson-core-binary-jar', - url = 'mvn:com.fasterxml.jackson.core:jackson-core:jar:2.2.3', - sha1 = '1a0113da2cab5f4c216b4e5e7c1dbfaa67087e14', -) - -prebuilt_jar( - name = 'databind', - binary_jar = ':jackson-databind-jar', - visibility = ['//ReactAndroid/...',], -) - -remote_file( - name = 'jackson-databind-jar', - url = 'mvn:com.fasterxml.jackson.core:jackson-databind:jar:2.2.3', - sha1 = '03ae380888029daefb91d3ecdca3a37d8cb92bc9', -) - -prebuilt_jar( - name = 'annotations', - binary_jar = ':jackson-annotations-jar', - visibility = ['//ReactAndroid/...',], -) - -remote_file( - name = 'jackson-annotations-jar', - url = 'mvn:com.fasterxml.jackson.core:jackson-annotations:jar:2.2.3', - sha1 = '0527fece4f23a457070a36c371a26d6c0208e1c3', -) diff --git a/ReactAndroid/src/test/java/com/facebook/react/bridge/BUCK b/ReactAndroid/src/test/java/com/facebook/react/bridge/BUCK index 882f646356e272..7b99ea4b675a72 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/bridge/BUCK +++ b/ReactAndroid/src/test/java/com/facebook/react/bridge/BUCK @@ -29,8 +29,6 @@ rn_robolectric_test( react_native_dep('libraries/fbcore/src/test/java/com/facebook/powermock:powermock'), react_native_dep('libraries/soloader/java/com/facebook/soloader:soloader'), react_native_dep('third-party/java/fest:fest'), - react_native_dep('third-party/java/jackson:core'), - react_native_dep('third-party/java/jackson:jackson'), react_native_dep('third-party/java/jsr-305:jsr-305'), react_native_dep('third-party/java/junit:junit'), react_native_dep('third-party/java/mockito:mockito'), diff --git a/ReactAndroid/src/test/java/com/facebook/react/devsupport/JSDebuggerWebSocketClientTest.java b/ReactAndroid/src/test/java/com/facebook/react/devsupport/JSDebuggerWebSocketClientTest.java index 1f25ee634822ef..1b94389a4dcb0f 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/devsupport/JSDebuggerWebSocketClientTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/devsupport/JSDebuggerWebSocketClientTest.java @@ -92,6 +92,15 @@ public void test_onMessage_WithoutReplyId_ShouldNotTriggerCallbacks() throws Exc PowerMockito.verifyPrivate(client, never()).invoke("triggerRequestFailure", anyInt(), any()); } + @Test + public void test_onMessage_With_Null_ReplyId_ShouldNotTriggerCallbacks() throws Exception { + JSDebuggerWebSocketClient client = PowerMockito.spy(new JSDebuggerWebSocketClient()); + + client.onMessage(ResponseBody.create(WebSocket.TEXT, "{\"replyID\":null, \"result\":\"OK\"}")); + PowerMockito.verifyPrivate(client, never()).invoke("triggerRequestSuccess", anyInt(), anyString()); + PowerMockito.verifyPrivate(client, never()).invoke("triggerRequestFailure", anyInt(), any()); + } + @Test public void test_onMessage_WithResult_ShouldTriggerRequestSuccess() throws Exception { JSDebuggerWebSocketClient client = PowerMockito.spy(new JSDebuggerWebSocketClient()); @@ -101,6 +110,15 @@ public void test_onMessage_WithResult_ShouldTriggerRequestSuccess() throws Excep PowerMockito.verifyPrivate(client, never()).invoke("triggerRequestFailure", anyInt(), any()); } + @Test + public void test_onMessage_With_Null_Result_ShouldTriggerRequestSuccess() throws Exception { + JSDebuggerWebSocketClient client = PowerMockito.spy(new JSDebuggerWebSocketClient()); + + client.onMessage(ResponseBody.create(WebSocket.TEXT, "{\"replyID\":0, \"result\":null}")); + PowerMockito.verifyPrivate(client).invoke("triggerRequestSuccess", 0, null); + PowerMockito.verifyPrivate(client, never()).invoke("triggerRequestFailure", anyInt(), any()); + } + @Test public void test_onMessage_WithError_ShouldCallAbort() throws Exception { JSDebuggerWebSocketClient client = PowerMockito.spy(new JSDebuggerWebSocketClient()); @@ -108,4 +126,12 @@ public void test_onMessage_WithError_ShouldCallAbort() throws Exception { client.onMessage(ResponseBody.create(WebSocket.TEXT, "{\"replyID\":0, \"error\":\"BOOM\"}")); PowerMockito.verifyPrivate(client).invoke("abort", eq("BOOM"), isA(JavascriptException.class)); } + + @Test + public void test_onMessage_With_Null_Error_ShouldTriggerRequestSuccess() throws Exception { + JSDebuggerWebSocketClient client = PowerMockito.spy(new JSDebuggerWebSocketClient()); + + client.onMessage(ResponseBody.create(WebSocket.TEXT, "{\"replyID\":0, \"error\":null}")); + PowerMockito.verifyPrivate(client).invoke("triggerRequestSuccess", anyInt(), anyString()); + } } diff --git a/ReactAndroid/src/test/java/com/facebook/react/devsupport/JSPackagerWebSocketClientTest.java b/ReactAndroid/src/test/java/com/facebook/react/devsupport/JSPackagerWebSocketClientTest.java index f137ba7c53041c..de2d3b190b2a53 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/devsupport/JSPackagerWebSocketClientTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/devsupport/JSPackagerWebSocketClientTest.java @@ -57,6 +57,16 @@ public void test_onMessage_WithoutTarget_ShouldNotTriggerCallback() throws IOExc verify(callback, never()).onMessage(anyString(), anyString()); } + @Test + public void test_onMessage_With_Null_Target_ShouldNotTriggerCallback() throws IOException { + final JSPackagerWebSocketClient.JSPackagerCallback callback = + mock(JSPackagerWebSocketClient.JSPackagerCallback.class); + final JSPackagerWebSocketClient client = new JSPackagerWebSocketClient("ws://not_needed", callback); + client.onMessage(ResponseBody.create(WebSocket.TEXT, + "{\"version\": 1, \"target\": null, \"action\": \"actionValue\"}")); + verify(callback, never()).onMessage(anyString(), anyString()); + } + @Test public void test_onMessage_WithoutAction_ShouldNotTriggerCallback() throws IOException { final JSPackagerWebSocketClient.JSPackagerCallback callback = @@ -67,6 +77,16 @@ public void test_onMessage_WithoutAction_ShouldNotTriggerCallback() throws IOExc verify(callback, never()).onMessage(anyString(), anyString()); } + @Test + public void test_onMessage_With_Null_Action_ShouldNotTriggerCallback() throws IOException { + final JSPackagerWebSocketClient.JSPackagerCallback callback = + mock(JSPackagerWebSocketClient.JSPackagerCallback.class); + final JSPackagerWebSocketClient client = new JSPackagerWebSocketClient("ws://not_needed", callback); + client.onMessage(ResponseBody.create(WebSocket.TEXT, + "{\"version\": 1, \"target\": \"targetValue\", \"action\": null}")); + verify(callback, never()).onMessage(anyString(), anyString()); + } + @Test public void test_onMessage_WrongVersion_ShouldNotTriggerCallback() throws IOException { final JSPackagerWebSocketClient.JSPackagerCallback callback =