Skip to content
Permalink
Browse files

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: 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 a05e9f8e094b25cc86ee297477cccafc3be5ef52
@@ -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 @@ public synchronized void reset() {

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) {
@@ -146,7 +146,7 @@ public synchronized void reset() {
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();

@@ -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();

@@ -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.
You can’t perform that action at this time.