Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IonRawBinaryWriter_1_1 annotations #661

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions src/main/java/com/amazon/ion/impl/bin/IonRawBinaryWriter_1_1.kt
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,18 @@ class IonRawBinaryWriter_1_1 internal constructor(
}
}

companion object {
/** Flag to indicate that annotations need to be written using FlexSyms */
private const val FLEX_SYMS_REQUIRED = -1

/**
* Annotations container always requires at least one length prefix byte. In practice, it's almost certain to
* never require more than one byte for SID annotations. We assume that it will infrequently require more than
* one byte for FlexSym annotations.
*/
private const val ANNOTATIONS_LENGTH_PREFIX_ALLOCATION_SIZE = 1
}

private val utf8StringEncoder = Utf8StringEncoderPool.getInstance().getOrCreate()

private var annotationsTextBuffer = arrayOfNulls<CharSequence>(8)
Expand All @@ -68,7 +80,6 @@ class IonRawBinaryWriter_1_1 internal constructor(
* and type of annotations required.
*/
private var annotationFlexSymFlag = 0
private val FLEX_SYMS_REQUIRED = -1

private var hasFieldName = false

Expand Down Expand Up @@ -261,20 +272,20 @@ class IonRawBinaryWriter_1_1 internal constructor(
}
if (annotationFlexSymFlag == FLEX_SYMS_REQUIRED) {
buffer.writeByte(OpCodes.ANNOTATIONS_MANY_FLEX_SYM)
buffer.reserve(lengthPrefixPreallocation)
buffer.reserve(ANNOTATIONS_LENGTH_PREFIX_ALLOCATION_SIZE)
for (i in 0 until numAnnotations) {
currentContainer.length += writeFlexSymFromAnnotationsBuffer(i)
}
} else {
buffer.writeByte(OpCodes.ANNOTATIONS_MANY_SYMBOL_ADDRESS)
buffer.reserve(lengthPrefixPreallocation)
buffer.reserve(ANNOTATIONS_LENGTH_PREFIX_ALLOCATION_SIZE)
for (i in 0 until numAnnotations) {
currentContainer.length += buffer.writeFlexUInt(annotationsIdBuffer[i])
}
}

val numAnnotationsBytes = currentContainer.length
val numLengthPrefixBytes = writeCurrentContainerLength()
val numLengthPrefixBytes = writeCurrentContainerLength(ANNOTATIONS_LENGTH_PREFIX_ALLOCATION_SIZE)

// Set the new current container
containerStack.pop()
Expand Down Expand Up @@ -432,7 +443,7 @@ class IonRawBinaryWriter_1_1 internal constructor(
buffer.shiftBytesLeft(currentContainer.length.toInt(), lengthPrefixPreallocation)
buffer.writeUInt8At(currentContainer.position, OpCodes.LIST_ZERO_LENGTH + contentLength)
} else {
thisContainerTotalLength += writeCurrentContainerLength()
thisContainerTotalLength += writeCurrentContainerLength(lengthPrefixPreallocation)
}
}
SExp -> TODO()
Expand All @@ -455,17 +466,17 @@ class IonRawBinaryWriter_1_1 internal constructor(
* Writes the length of the current container and returns the number of bytes needed to do so.
* Transparently handles PatchPoints as necessary.
*/
private fun writeCurrentContainerLength(): Int {
private fun writeCurrentContainerLength(numPreAllocatedLengthPrefixBytes: Int): Int {
val lengthPrefixBytesRequired = FlexInt.flexUIntLength(currentContainer.length)
if (lengthPrefixBytesRequired == lengthPrefixPreallocation) {
if (lengthPrefixBytesRequired == numPreAllocatedLengthPrefixBytes) {
// We have enough space, so write in the correct length.
buffer.writeFlexIntOrUIntAt(currentContainer.position + 1, currentContainer.length, lengthPrefixBytesRequired)
} else {
addPatchPointsToStack()
// All ContainerInfos are in the stack, so we know that its patchPoint is non-null.
currentContainer.patchPoint.assumeNotNull().apply {
oldPosition = currentContainer.position + 1
oldLength = lengthPrefixPreallocation
oldLength = numPreAllocatedLengthPrefixBytes
length = currentContainer.length
}
}
Expand Down
30 changes: 30 additions & 0 deletions src/main/java/com/amazon/ion/impl/bin/WriteBuffer.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

package com.amazon.ion.impl.bin;

import com.amazon.ion.impl.bin.utf8.Utf8StringEncoder;

import java.io.Closeable;
import java.io.IOException;
import java.io.OutputStream;
Expand Down Expand Up @@ -1519,6 +1521,34 @@ public int writeFixedIntOrUInt(final byte[] value) {
return value.length;
}

/**
* Writes a FlexSym with a symbol id.
*/
public int writeFlexSym(int sid) {
if (sid != 0) {
return writeFlexInt(sid);
} else {
writeByte((byte) 0x01);
writeByte((byte) 0x70);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about changing the spec so that this is 0x90 instead? That way it at least aligns with the upper nibble type (9 = symbol, whereas 7 = timestamp)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that, and I suspect that was probably the intention, but then parts of the spec were modified without fully updating other parts of the spec.

return 2;
}
}

/**
* Writes a FlexSym with inline text.
*/
public int writeFlexSym(Utf8StringEncoder.Result text) {
if (text.getEncodedLength() == 0) {
writeByte((byte) 0x01);
writeByte((byte) 0x80);
return 2;
} else {
int numLengthBytes = writeFlexInt(-text.getEncodedLength());
writeBytes(text.getBuffer(), 0, text.getEncodedLength());
return numLengthBytes + text.getEncodedLength();
}
}

/** Write the entire buffer to output stream. */
public void writeTo(final OutputStream out) throws IOException
{
Expand Down
55 changes: 37 additions & 18 deletions src/test/java/com/amazon/ion/impl/bin/IonRawBinaryWriterTest_1_1.kt
popematt marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,14 @@ class IonRawBinaryWriterTest_1_1 {
}
}

@Test
fun `write sid 0 annotation`() {
assertWriterOutputEquals("E4 01 5E") {
writeAnnotations(0)
writeBool(true)
}
}

@Test
fun `write one inline annotation`() {
val expectedBytes = "E7 FB 66 6F 6F 5F"
Expand Down Expand Up @@ -341,25 +349,34 @@ class IonRawBinaryWriterTest_1_1 {

@Test
fun `write three inline annotations`() {
popematt marked this conversation as resolved.
Show resolved Hide resolved
val expectedBytes = "E6 09 07 09 02 04 5E"
val expectedBytes = "E9 19 FB 66 6F 6F FB 62 61 72 FB 62 61 7A 5F"
assertWriterOutputEquals(expectedBytes) {
writeAnnotations(3)
writeAnnotations(4)
writeAnnotations(256)
writeBool(true)
writeAnnotations("foo")
writeAnnotations("bar")
writeAnnotations("baz")
writeBool(false)
}
assertWriterOutputEquals(expectedBytes) {
writeAnnotations(3)
writeAnnotations(4, 256)
writeBool(true)
writeAnnotations("foo")
writeAnnotations("bar", "baz")
writeBool(false)
}
assertWriterOutputEquals(expectedBytes) {
writeAnnotations(intArrayOf(3, 4))
writeAnnotations(256)
writeBool(true)
writeAnnotations(arrayOf("foo", "bar"))
writeAnnotations("baz")
writeBool(false)
}
assertWriterOutputEquals(expectedBytes) {
writeAnnotations(intArrayOf(3, 4, 256))
writeAnnotations(arrayOf("foo", "bar", "baz"))
writeBool(false)
}
}

@Test
fun `write empty text and sid 0 annotations`() {
assertWriterOutputEquals("E8 01 70 01 80 5E") {
writeAnnotations(0)
writeAnnotations("")
writeBool(true)
}
}
Expand Down Expand Up @@ -395,13 +412,15 @@ class IonRawBinaryWriterTest_1_1 {
val opCode = "E7"
val length = "C6 FD"
val text = "41 6D 61 7A 6F 6E 20 49 6F 6E 20 69 73 20 61 20 72 69 63 68 6C 79 2D 74 79 70 65 64 2C 20 73 65 " +
"6C 66 2D 64 65 73 63 72 69 62 69 6E 67 2C 20 68 69 65 72 61 72 63 68 69 63 61 6C 20 64 61 74 61 20 " +
"73 65 72 69 61 6C 69 7A 61 74 69 6F 6E 20 66 6F 72 6D 61 74 20 6F 66 66 65 72 69 6E 67 20 69 6E 74 " +
"65 72 63 68 61 6E 67 65 61 62 6C 65 20 62 69 6E 61 72 79 20 61 6E 64 20 74 65 78 74 20 72 65 70 72 " +
"65 73 65 6E 74 61 74 69 6F 6E 73 2E 5F"
"6C 66 2D 64 65 73 63 72 69 62 69 6E 67 2C 20 68 69 65 72 61 72 63 68 69 63 61 6C 20 64 61 74 61 20 " +
"73 65 72 69 61 6C 69 7A 61 74 69 6F 6E 20 66 6F 72 6D 61 74 20 6F 66 66 65 72 69 6E 67 20 69 6E 74 " +
"65 72 63 68 61 6E 67 65 61 62 6C 65 20 62 69 6E 61 72 79 20 61 6E 64 20 74 65 78 74 20 72 65 70 72 " +
"65 73 65 6E 74 61 74 69 6F 6E 73 2E 5F"
assertWriterOutputEquals("$opCode $length $text") {
writeAnnotations("Amazon Ion is a richly-typed, self-describing, hierarchical data serialization " +
"format offering interchangeable binary and text representations.")
writeAnnotations(
"Amazon Ion is a richly-typed, self-describing, hierarchical data serialization " +
"format offering interchangeable binary and text representations."
)
writeBool(false)
}
}
Expand Down
39 changes: 39 additions & 0 deletions src/test/java/com/amazon/ion/impl/bin/WriteBufferTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.math.BigInteger;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;

import com.amazon.ion.impl.bin.utf8.Utf8StringEncoder;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -1755,6 +1758,42 @@ public void testWriteFixedIntOrUIntThrowsExceptionWhenNumBytesIsOutOfBounds() {
Assertions.assertThrows(IllegalArgumentException.class, () -> buf.writeFixedIntOrUInt(0, 9));
}

@ParameterizedTest
@CsvSource({
" 0, 00000001 01110000",
" 1, 00000011",
" 2, 00000101",
"63, 01111111",
"64, 00000010 00000001",
})
public void testWriteSidFlexSym(int value, String expectedBits) {
int numBytes = buf.writeFlexSym(value);
String actualBits = byteArrayToBitString(bytes());
Assertions.assertEquals(expectedBits, actualBits);
Assertions.assertEquals((expectedBits.length() + 1)/9, numBytes);
}

@ParameterizedTest
@CsvSource({
"'', 00000001 10000000",
"a, 11111111 01100001",
"abc, 11111011 01100001 01100010 01100011",
"this is a very very very very very long symbol, " +
"10100101 01110100 01101000 01101001 01110011 00100000 01101001 01110011 00100000 01100001 00100000 " +
"01110110 01100101 01110010 01111001 00100000 01110110 01100101 01110010 01111001 00100000 01110110 " +
"01100101 01110010 01111001 00100000 01110110 01100101 01110010 01111001 00100000 01110110 01100101 " +
"01110010 01111001 00100000 01101100 01101111 01101110 01100111 00100000 01110011 01111001 01101101 " +
"01100010 01101111 01101100",
})
public void testWriteTextFlexSym(String value, String expectedBits) {
// This is a sloppy way to construct a Result, but it works for this test because we only have ascii characters.
Utf8StringEncoder.Result encoded = new Utf8StringEncoder.Result(value.length(), value.getBytes(StandardCharsets.US_ASCII));
int numBytes = buf.writeFlexSym(encoded);
String actualBits = byteArrayToBitString(bytes());
Assertions.assertEquals(expectedBits, actualBits);
Assertions.assertEquals((expectedBits.length() + 1)/9, numBytes);
}

/**
* Converts a byte array to a string of bits, such as "00110110 10001001".
* The purpose of this method is to make it easier to read and write test assertions.
Expand Down
Loading