Skip to content

Commit

Permalink
Android: Sort modules by ID when serializing delta bundle
Browse files Browse the repository at this point in the history
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: react-native-community/cli#279

Reviewed By: dcaspi

Differential Revision: D15301927

fbshipit-source-id: 27bdecfb3d6963aa358e4d542c8b7663fd9eb437
  • Loading branch information
motiz88 authored and facebook-github-bot committed May 14, 2019
1 parent 4105450 commit a05e9f8
Show file tree
Hide file tree
Showing 2 changed files with 145 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -75,7 +75,7 @@ private static class BundleDeltaJavaClient extends BundleDeltaClient {

byte[] mPreCode;
byte[] mPostCode;
final LinkedHashMap<Number, byte[]> mModules = new LinkedHashMap<Number, byte[]>();
final TreeMap<Number, byte[]> mModules = new TreeMap<Number, byte[]>();

@Override
public boolean canHandle(ClientType type) {
Expand Down Expand Up @@ -146,7 +146,7 @@ public synchronized Pair<Boolean, NativeDeltaClient> processDelta(
return Pair.create(Boolean.TRUE, null);
}

private static int setModules(JsonReader jsonReader, LinkedHashMap<Number, byte[]> map)
private static int setModules(JsonReader jsonReader, TreeMap<Number, byte[]> map)
throws IOException {
jsonReader.beginArray();

Expand All @@ -167,7 +167,7 @@ private static int setModules(JsonReader jsonReader, LinkedHashMap<Number, byte[
return numModules;
}

private static int removeModules(JsonReader jsonReader, LinkedHashMap<Number, byte[]> map)
private static int removeModules(JsonReader jsonReader, TreeMap<Number, byte[]> map)
throws IOException {
jsonReader.beginArray();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* <p>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);
}
}

0 comments on commit a05e9f8

Please sign in to comment.