From a05e9f8e094b25cc86ee297477cccafc3be5ef52 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Tue, 14 May 2019 10:19:05 -0700 Subject: [PATCH] Android: Sort modules by ID when serializing delta bundle Summary: Fixes redbox/yellowbox symbolication when the Java delta client is enabled. Previously the modules would get concatenated in a nondeterministic order (owing to Metro's parallelism) which differed from their order in the source map, where they're explicitly sorted by module ID. This diff changes the data structure holding modules in memory from a `LinkedHashMap` (which iterates in insertion order) to a `TreeMap` (which iterates in key order). NOTE: Similar to this change in the Chrome debugger's delta client: https://github.com/react-native-community/cli/pull/279 Reviewed By: dcaspi Differential Revision: D15301927 fbshipit-source-id: 27bdecfb3d6963aa358e4d542c8b7663fd9eb437 --- .../react/devsupport/BundleDeltaClient.java | 8 +- .../devsupport/BundleDeltaClientTest.java | 141 ++++++++++++++++++ 2 files changed, 145 insertions(+), 4 deletions(-) create mode 100644 ReactAndroid/src/test/java/com/facebook/react/devsupport/BundleDeltaClientTest.java diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/BundleDeltaClient.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/BundleDeltaClient.java index e035c976a52bc2..dcac48b17258d7 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/BundleDeltaClient.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/BundleDeltaClient.java @@ -11,7 +11,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStreamReader; -import java.util.LinkedHashMap; +import java.util.TreeMap; import javax.annotation.Nullable; import android.util.JsonReader; @@ -75,7 +75,7 @@ private static class BundleDeltaJavaClient extends BundleDeltaClient { byte[] mPreCode; byte[] mPostCode; - final LinkedHashMap mModules = new LinkedHashMap(); + final TreeMap mModules = new TreeMap(); @Override public boolean canHandle(ClientType type) { @@ -146,7 +146,7 @@ public synchronized Pair processDelta( return Pair.create(Boolean.TRUE, null); } - private static int setModules(JsonReader jsonReader, LinkedHashMap map) + private static int setModules(JsonReader jsonReader, TreeMap map) throws IOException { jsonReader.beginArray(); @@ -167,7 +167,7 @@ private static int setModules(JsonReader jsonReader, LinkedHashMap map) + private static int removeModules(JsonReader jsonReader, TreeMap map) throws IOException { jsonReader.beginArray(); diff --git a/ReactAndroid/src/test/java/com/facebook/react/devsupport/BundleDeltaClientTest.java b/ReactAndroid/src/test/java/com/facebook/react/devsupport/BundleDeltaClientTest.java new file mode 100644 index 00000000000000..ef36e6a0b8cbd3 --- /dev/null +++ b/ReactAndroid/src/test/java/com/facebook/react/devsupport/BundleDeltaClientTest.java @@ -0,0 +1,141 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + *

This source code is licensed under the MIT license found in the LICENSE file in the root + * directory of this source tree. + */ +package com.facebook.react.devsupport; + +import static org.fest.assertions.api.Assertions.assertThat; + +import com.facebook.react.common.StandardCharsets; +import com.facebook.react.devsupport.BundleDeltaClient; +import org.junit.Test; +import org.junit.Before; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import okio.BufferedSource; +import org.junit.Rule; +import org.junit.rules.TemporaryFolder; +import okio.Okio; +import java.io.ByteArrayInputStream; +import java.nio.file.Files; +import java.io.File; +import java.io.IOException; + +@RunWith(RobolectricTestRunner.class) +public class BundleDeltaClientTest { + private BundleDeltaClient mClient; + + @Rule public TemporaryFolder mFolder = new TemporaryFolder(); + + @Before + public void setUp() { + mClient = BundleDeltaClient.create(BundleDeltaClient.ClientType.DEV_SUPPORT); + } + + @Test + public void testAcceptsSimpleInitialBundle() throws IOException { + File file = mFolder.newFile(); + mClient.processDelta( + bufferedSource( + "{" + + "\"pre\": \"console.log('Hello World!');\"," + + "\"post\": \"console.log('That is all folks!');\"," + + "\"modules\": [[0, \"console.log('Best module.');\"]]" + + "}"), + file); + assertThat(contentOf(file)) + .isEqualTo( + "console.log('Hello World!');\n" + + "console.log('Best module.');\n" + + "console.log('That is all folks!');\n"); + } + + @Test + public void testPatchesInitialBundleWithDeltaBundle() throws IOException { + File file = mFolder.newFile(); + mClient.processDelta( + bufferedSource( + "{" + + "\"pre\": \"pre\"," + + "\"post\": \"post\"," + + "\"modules\": [[0, \"0\"], [1, \"1\"]]" + + "}"), + file); + file = mFolder.newFile(); + mClient.processDelta( + bufferedSource( + "{" + + "\"added\": [[2, \"2\"]]," + + "\"modified\": [[0, \"0.1\"]]," + + "\"deleted\": [1]" + + "}"), + file); + assertThat(contentOf(file)) + .isEqualTo( + "pre\n" + + "0.1\n" + + "2\n" + + "post\n"); + } + + @Test + public void testSortsModulesByIdInInitialBundle() throws IOException { + File file = mFolder.newFile(); + mClient.processDelta( + bufferedSource( + "{" + + "\"pre\": \"console.log('Hello World!');\"," + + "\"post\": \"console.log('That is all folks!');\"," + + "\"modules\": [[3, \"3\"], [0, \"0\"], [2, \"2\"], [1, \"1\"]]" + + "}"), + file); + assertThat(contentOf(file)) + .isEqualTo( + "console.log('Hello World!');\n" + + "0\n" + + "1\n" + + "2\n" + + "3\n" + + "console.log('That is all folks!');\n"); + } + + @Test + public void testSortsModulesByIdInPatchedBundle() throws IOException { + File file = mFolder.newFile(); + mClient.processDelta( + bufferedSource( + "{" + + "\"pre\": \"console.log('Hello World!');\"," + + "\"post\": \"console.log('That is all folks!');\"," + + "\"modules\": [[3, \"3\"], [0, \"0\"], [1, \"1\"]]" + + "}"), + file); + file = mFolder.newFile(); + mClient.processDelta( + bufferedSource( + "{" + + "\"added\": [[2, \"2\"]]," + + "\"modified\": [[0, \"0.1\"]]," + + "\"deleted\": [1]" + + "}"), + file); + assertThat(contentOf(file)) + .isEqualTo( + "console.log('Hello World!');\n" + + "0.1\n" + + "2\n" + + "3\n" + + "console.log('That is all folks!');\n"); + } + + private static BufferedSource bufferedSource(String string) { + return Okio.buffer( + Okio.source(new ByteArrayInputStream(string.getBytes(StandardCharsets.UTF_8)))); + } + + private static String contentOf(File file) throws IOException { + return new String(Files.readAllBytes(file.toPath()), StandardCharsets.UTF_8); + } +}