Update the strings asset file format for easier parsing. Summary: The new file format makes it so that parsing sections of the file one at a time becomes easier. Also, switch some of the `byte` and `short` to `int` to make it more flexible. Intentionally left the `byte` data type in plurals since the number of plural categories is guaranteed to be <= 6. To make it easier to maintain the unit test, I changed the file format to something easy to understand/edit. Test Plan: ant test
diff --git a/src/com/facebook/buck/android/StringResources.java b/src/com/facebook/buck/android/StringResources.java index 073cd89..77b8aba 100644 --- a/src/com/facebook/buck/android/StringResources.java +++ b/src/com/facebook/buck/android/StringResources.java
@@ -35,6 +35,14 @@ * for generating a custom format binary file for the resources. */ public class StringResources { + + /** + * Bump this whenever there's a change in the file format. The parser can decide to abort parsing + * if the version it finds in the file does not match it's own version, thereby avoiding + * potential data corruption issues. + */ + private static final int FORMAT_VERSION = 1; + public final TreeMap<Integer, String> strings; public final TreeMap<Integer, ImmutableMap<String, String>> plurals; public final TreeMultimap<Integer, String> arrays; @@ -87,116 +95,118 @@ * the following binary file format: * <p> * <pre> - * [Short: #strings][Short: #plurals][Short: #arrays] + * [Int: Version] + * [Int: # of strings] * [Int: Smallest resource id among strings] - * [Short: resource id offset][Short: length of the string] x #strings + * [Short: resource id delta][Short: length of the string] x # of strings + * [Byte array of the string value] x # of strings + * [Int: # of plurals] * [Int: Smallest resource id among plurals] - * [Short: resource id offset][Short: length of string representing the plural] x #plurals + * [[Short: resource id delta][Byte: #categories][[Byte: category][Short: length of plural + * value]] x #categories] x # of plurals + * [Byte array of plural value] x Summation of plural categories over # of plurals + * [Int: # of arrays] * [Int: Smallest resource id among arrays] - * [Short: resource id offset][Short: length of string representing the array] x #arrays - * [Byte array of the string value] x #strings - * [[Byte: #categories][[Byte: category][Short: length of plural][plural]] x #categories] x #plurals - * [[Byte: #elements][[Short: length of element][element]] x #elements] x #arrays + * [[Short: resource id delta][Int: #elements][Short: length of element] x #elements] x # of + * arrays + * [Byte array of string value] x Summation of array elements over # of arrays * </pre> * </p> */ public byte[] getBinaryFileContent() { try ( - ByteArrayOutputStream bos1 = new ByteArrayOutputStream(); - DataOutputStream mapOutStream = new DataOutputStream(bos1); - ByteArrayOutputStream bos2 = new ByteArrayOutputStream(); - DataOutputStream dataOutStream = new DataOutputStream(bos2) + ByteArrayOutputStream bytesStream = new ByteArrayOutputStream(); + DataOutputStream outputStream = new DataOutputStream(bytesStream) ) { - writeMapsSizes(mapOutStream); + outputStream.writeInt(FORMAT_VERSION); - writeStrings(mapOutStream, dataOutStream); - writePlurals(mapOutStream, dataOutStream); - writeArrays(mapOutStream, dataOutStream); + writeStrings(outputStream); + writePlurals(outputStream); + writeArrays(outputStream); - byte[] result = new byte[bos1.size() + bos2.size()]; - System.arraycopy(bos1.toByteArray(), 0, result, 0, bos1.size()); - System.arraycopy(bos2.toByteArray(), 0, result, bos1.size(), bos2.size()); - return result; + return bytesStream.toByteArray(); } catch (IOException e) { return null; } } - private void writeMapsSizes(DataOutputStream stream) throws IOException { - stream.writeShort(strings.size()); - stream.writeShort(plurals.size()); - stream.writeShort(arrays.keySet().size()); - } - - private void writeStrings(DataOutputStream mapStream, DataOutputStream dataStream) - throws IOException { + private void writeStrings(DataOutputStream outputStream) throws IOException { + outputStream.writeInt(strings.size()); if (strings.isEmpty()) { return; } - int smallestResourceId = strings.firstKey(); - mapStream.writeInt(smallestResourceId); - for (Map.Entry<Integer, String> entry : strings.entrySet()) { - byte[] resourceBytes = entry.getValue().getBytes(charset); - writeMapEntry(mapStream, entry.getKey() - smallestResourceId, resourceBytes.length); - dataStream.write(resourceBytes); + int previousResourceId = strings.firstKey(); + outputStream.writeInt(previousResourceId); + + try (ByteArrayOutputStream dataStream = new ByteArrayOutputStream()) { + for (Map.Entry<Integer, String> entry : strings.entrySet()) { + byte[] resourceBytes = entry.getValue().getBytes(charset); + writeShort(outputStream, entry.getKey() - previousResourceId); + writeShort(outputStream, resourceBytes.length); + dataStream.write(resourceBytes, 0, resourceBytes.length); + + previousResourceId = entry.getKey(); + } + outputStream.write(dataStream.toByteArray()); } } - private void writePlurals(DataOutputStream mapStream, DataOutputStream dataStream) - throws IOException { + private void writePlurals(DataOutputStream outputStream) throws IOException { + outputStream.writeInt(plurals.size()); if (plurals.isEmpty()) { return; } - int smallestResourceId = plurals.firstKey(); - mapStream.writeInt(smallestResourceId); - for (Map.Entry<Integer, ImmutableMap<String, String>> entry : plurals.entrySet()) { - ImmutableMap<String, String> categoryMap = entry.getValue(); - dataStream.writeByte(categoryMap.size()); - int resourceDataLength = 1; + int previousResourceId = plurals.firstKey(); + outputStream.writeInt(previousResourceId); - for (Map.Entry<String, String> cat : categoryMap.entrySet()) { - dataStream.writeByte(PLURAL_CATEGORY_MAP.get(cat.getKey()).byteValue()); - byte[] pluralValue = cat.getValue().getBytes(charset); - dataStream.writeShort(pluralValue.length); - dataStream.write(pluralValue); - resourceDataLength += 3 + pluralValue.length; + try (ByteArrayOutputStream dataStream = new ByteArrayOutputStream()) { + for (Map.Entry<Integer, ImmutableMap<String, String>> entry : plurals.entrySet()) { + writeShort(outputStream, entry.getKey() - previousResourceId); + ImmutableMap<String, String> categoryMap = entry.getValue(); + outputStream.writeByte(categoryMap.size()); + + for (Map.Entry<String, String> cat : categoryMap.entrySet()) { + outputStream.writeByte(PLURAL_CATEGORY_MAP.get(cat.getKey()).byteValue()); + byte[] pluralValue = cat.getValue().getBytes(charset); + writeShort(outputStream, pluralValue.length); + dataStream.write(pluralValue); + } + + previousResourceId = entry.getKey(); } - writeMapEntry(mapStream, entry.getKey() - smallestResourceId, resourceDataLength); + outputStream.write(dataStream.toByteArray()); } } - private void writeArrays(DataOutputStream mapStream, DataOutputStream dataStream) - throws IOException { + private void writeArrays(DataOutputStream outputStream) throws IOException { + outputStream.writeInt(arrays.keySet().size()); if (arrays.keySet().isEmpty()) { return; } - boolean first = true; - int smallestResourceId = 0; - for (int resourceId : arrays.keySet()) { - if (first) { - first = false; - smallestResourceId = resourceId; - mapStream.writeInt(smallestResourceId); - } - Collection<String> arrayValues = arrays.get(resourceId); - dataStream.writeByte(arrayValues.size()); - int resourceDataLength = 1; + int previousResourceId = arrays.keySet().first(); + outputStream.writeInt(previousResourceId); + try (ByteArrayOutputStream dataStream = new ByteArrayOutputStream()) { + for (int resourceId : arrays.keySet()) { + writeShort(outputStream, resourceId - previousResourceId); + Collection<String> arrayValues = arrays.get(resourceId); + outputStream.writeInt(arrayValues.size()); - for (String arrayValue : arrayValues) { - byte[] byteValue = arrayValue.getBytes(charset); - dataStream.writeShort(byteValue.length); - dataStream.write(byteValue); - resourceDataLength += 2 + byteValue.length; - } + for (String arrayValue : arrayValues) { + byte[] byteValue = arrayValue.getBytes(charset); + writeShort(outputStream, byteValue.length); + dataStream.write(byteValue); + } - writeMapEntry(mapStream, resourceId - smallestResourceId, resourceDataLength); + previousResourceId = resourceId; + } + outputStream.write(dataStream.toByteArray()); } } - private void writeMapEntry(DataOutputStream stream, int resourceOffset, int length) - throws IOException { - stream.writeShort(resourceOffset); - stream.writeShort(length); + private void writeShort(DataOutputStream stream, int number) throws IOException { + Preconditions.checkState(number <= Short.MAX_VALUE, + "Error attempting to compact a numeral to short: " + number); + stream.writeShort(number); } }
diff --git a/test/com/facebook/buck/android/CompileStringsStepTest.java b/test/com/facebook/buck/android/CompileStringsStepTest.java index 1e4c636..3aa4cfc 100644 --- a/test/com/facebook/buck/android/CompileStringsStepTest.java +++ b/test/com/facebook/buck/android/CompileStringsStepTest.java
@@ -23,6 +23,7 @@ import com.facebook.buck.step.ExecutionContext; import com.facebook.buck.util.ProjectFilesystem; import com.facebook.buck.util.XmlDomParser; +import com.google.common.base.Splitter; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultimap; @@ -35,6 +36,8 @@ import org.junit.Test; import org.w3c.dom.NodeList; +import java.io.ByteArrayOutputStream; +import java.io.DataOutputStream; import java.io.File; import java.io.IOException; import java.nio.charset.Charset; @@ -287,12 +290,44 @@ assertEquals("Incorrect number of string files written.", 3, fileContentsMap.size()); for (Map.Entry<String, byte[]> entry : fileContentsMap.entrySet()) { File expectedFile = Paths.get(TESTDATA_DIR + entry.getKey()).toFile(); - assertArrayEquals(Files.toByteArray(expectedFile), fileContentsMap.get(entry.getKey())); + assertArrayEquals(createBinaryStream(expectedFile), fileContentsMap.get(entry.getKey())); } verifyAll(); } + private byte[] createBinaryStream(File expectedFile) throws IOException { + try ( + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + DataOutputStream stream = new DataOutputStream(bos) + ) { + for (String line : Files.readLines(expectedFile, Charset.defaultCharset())) { + for (String token : Splitter.on('|').split(line)) { + char dataType = token.charAt(0); + String value = token.substring(2); + switch (dataType) { + case 'i': + stream.writeInt(Integer.parseInt(value)); + break; + case 's': + stream.writeShort(Integer.parseInt(value)); + break; + case 'b': + stream.writeByte(Integer.parseInt(value)); + break; + case 't': + stream.write(value.getBytes()); + break; + default: + throw new RuntimeException("Unexpected data type in .fbstr file: " + dataType); + } + } + } + + return bos.toByteArray(); + } + } + private static class FakeProjectFileSystem extends ProjectFilesystem {
diff --git a/test/com/facebook/buck/android/StringResourcesTest.java b/test/com/facebook/buck/android/StringResourcesTest.java index e734f51..da08b8b 100644 --- a/test/com/facebook/buck/android/StringResourcesTest.java +++ b/test/com/facebook/buck/android/StringResourcesTest.java
@@ -76,67 +76,68 @@ private void verifyBinaryStream(byte[] binaryOutput) throws IOException { DataInputStream stream = new DataInputStream(new ByteArrayInputStream(binaryOutput)); - assertEquals(3, stream.readShort()); - assertEquals(2, stream.readShort()); - assertEquals(2, stream.readShort()); + // Version + assertEquals(1, stream.readInt()); + + // Strings + assertEquals(3, stream.readInt()); assertEquals(12345678, stream.readInt()); assertEquals(0, stream.readShort()); assertEquals(5, stream.readShort()); assertEquals(1, stream.readShort()); assertEquals(5, stream.readShort()); - assertEquals(2, stream.readShort()); + assertEquals(1, stream.readShort()); assertEquals(7, stream.readShort()); - assertEquals(12345689, stream.readInt()); - assertEquals(0, stream.readShort()); - assertEquals(29, stream.readShort()); - assertEquals(3, stream.readShort()); - assertEquals(31, stream.readShort()); - - assertEquals(12345694, stream.readInt()); - assertEquals(0, stream.readShort()); - assertEquals(17, stream.readShort()); - assertEquals(5, stream.readShort()); - assertEquals(9, stream.readShort()); - // string values assertEquals("S_one", readStringOfLength(stream, 5)); assertEquals("S_two", readStringOfLength(stream, 5)); assertEquals("S_three", readStringOfLength(stream, 7)); - // plural values + // Plurals + assertEquals(2, stream.readInt()); + assertEquals(12345689, stream.readInt()); + assertEquals(0, stream.readShort()); assertEquals(3, stream.readByte()); // number of categories. assertEquals(1, stream.readByte()); assertEquals(6, stream.readShort()); - assertEquals("P1_one", readStringOfLength(stream, 6)); assertEquals(3, stream.readByte()); assertEquals(6, stream.readShort()); - assertEquals("P1_few", readStringOfLength(stream, 6)); assertEquals(4, stream.readByte()); assertEquals(7, stream.readShort()); - assertEquals("P1_many", readStringOfLength(stream, 7)); + assertEquals(3, stream.readShort()); assertEquals(3, stream.readByte()); // number of categories. assertEquals(0, stream.readByte()); assertEquals(7, stream.readShort()); - assertEquals("P2_zero", readStringOfLength(stream, 7)); assertEquals(2, stream.readByte()); assertEquals(6, stream.readShort()); - assertEquals("P2_two", readStringOfLength(stream, 6)); assertEquals(5, stream.readByte()); assertEquals(8, stream.readShort()); + + // plural strings + assertEquals("P1_one", readStringOfLength(stream, 6)); + assertEquals("P1_few", readStringOfLength(stream, 6)); + assertEquals("P1_many", readStringOfLength(stream, 7)); + assertEquals("P2_zero", readStringOfLength(stream, 7)); + assertEquals("P2_two", readStringOfLength(stream, 6)); assertEquals("P2_other", readStringOfLength(stream, 8)); - //array values - assertEquals(2, stream.readByte()); // number of array elements. + // Arrays + assertEquals(2, stream.readInt()); + assertEquals(12345694, stream.readInt()); + assertEquals(0, stream.readShort()); + assertEquals(2, stream.readInt()); // number of array elements. assertEquals(6, stream.readShort()); - assertEquals("A1_one", readStringOfLength(stream, 6)); assertEquals(6, stream.readShort()); - assertEquals("A1_two", readStringOfLength(stream, 6)); + assertEquals(5, stream.readShort()); + assertEquals(1, stream.readInt()); // number of array elements. + assertEquals(6, stream.readShort()); - assertEquals(1, stream.readByte()); // number of array elements. - assertEquals(6, stream.readShort()); + // array strings + assertEquals("A1_one", readStringOfLength(stream, 6)); + assertEquals("A1_two", readStringOfLength(stream, 6)); assertEquals("A2_one", readStringOfLength(stream, 6)); }
diff --git a/testdata/com/facebook/buck/android/R.txt b/testdata/com/facebook/buck/android/R.txt index a075c96..b189911 100644 --- a/testdata/com/facebook/buck/android/R.txt +++ b/testdata/com/facebook/buck/android/R.txt
@@ -1,8 +1,8 @@ -int string name1_1 0x7eadbeef -int string name1_2 0x7eadc0de -int string name1_3 0x7eadbabe -int string name2_1 0x7eadfa11 -int string name2_2 0x7eadba11 -int string name3_1 0x7eadbee1 -int string name3_2 0x7eadfade -int string name3_3 0x7eadf00d +int string name1_1 0x7eadbee8 +int string name1_2 0x7eadbee9 +int string name1_3 0x7eadbeea +int string name2_1 0x7eadbeeb +int string name2_2 0x7eadbeec +int string name3_1 0x7eadbeed +int string name3_2 0x7eadbeee +int string name3_3 0x7eadbeef
diff --git a/testdata/com/facebook/buck/android/es.fbstr b/testdata/com/facebook/buck/android/es.fbstr index dad3769..58e5e23 100644 --- a/testdata/com/facebook/buck/android/es.fbstr +++ b/testdata/com/facebook/buck/android/es.fbstr Binary files differ
diff --git a/testdata/com/facebook/buck/android/pt.fbstr b/testdata/com/facebook/buck/android/pt.fbstr index 495b871..7e99138 100644 --- a/testdata/com/facebook/buck/android/pt.fbstr +++ b/testdata/com/facebook/buck/android/pt.fbstr Binary files differ
diff --git a/testdata/com/facebook/buck/android/pt_BR.fbstr b/testdata/com/facebook/buck/android/pt_BR.fbstr index 7582074..30288a9 100644 --- a/testdata/com/facebook/buck/android/pt_BR.fbstr +++ b/testdata/com/facebook/buck/android/pt_BR.fbstr Binary files differ