From f3cf42b619c5e80384f10c02d56ba7947bc29760 Mon Sep 17 00:00:00 2001 From: linlin-s Date: Wed, 18 Oct 2023 16:04:38 -0700 Subject: [PATCH 1/8] Update workflow trigger events. (#617) Co-authored-by: Tyler Gregg --- .github/workflows/ion-java-performance-regression-detector.yml | 2 +- .github/workflows/ion-test-driver.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ion-java-performance-regression-detector.yml b/.github/workflows/ion-java-performance-regression-detector.yml index bea37b9f39..ae35956dfe 100644 --- a/.github/workflows/ion-java-performance-regression-detector.yml +++ b/.github/workflows/ion-java-performance-regression-detector.yml @@ -6,7 +6,7 @@ name: Ion Java performance regression detector on: pull_request: paths: - - 'src/*' + - 'src/**' permissions: contents: read diff --git a/.github/workflows/ion-test-driver.yml b/.github/workflows/ion-test-driver.yml index d5155ad9eb..19e793306a 100644 --- a/.github/workflows/ion-test-driver.yml +++ b/.github/workflows/ion-test-driver.yml @@ -3,7 +3,7 @@ name: ion-test-driver on: pull_request: paths: - - 'src/*' + - 'src/**' permissions: contents: read From 43296c7a33ea63fd7cc1cc071f19e46958bb531b Mon Sep 17 00:00:00 2001 From: Matthew Pope <81593196+popematt@users.noreply.github.com> Date: Fri, 20 Oct 2023 11:49:16 -0700 Subject: [PATCH 2/8] Fix checkout step in regression workflow (#621) --- .../workflows/ion-java-performance-regression-detector.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ion-java-performance-regression-detector.yml b/.github/workflows/ion-java-performance-regression-detector.yml index ae35956dfe..e361fadc87 100644 --- a/.github/workflows/ion-java-performance-regression-detector.yml +++ b/.github/workflows/ion-java-performance-regression-detector.yml @@ -74,12 +74,15 @@ jobs: uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 with: ref: ${{ github.base_ref }} + submodules: recursive path: baseline - name: Checkout ion-java from the new commit. uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 with: + repository: ${{ github.event.pull_request.head.repo.full_name }} ref: ${{ github.head_ref }} + submodules: recursive path: new - name: Download test Ion Data from artifacts @@ -92,7 +95,7 @@ jobs: - name: Build ion-java from the base commit working-directory: baseline run: | - git submodule init && git submodule update && ./gradlew clean publishToMavenLocal + ./gradlew clean publishToMavenLocal - name: Benchmark ion-java from the base commit working-directory: ion-java-benchmark-cli @@ -104,7 +107,7 @@ jobs: - name: Build ion-java from the new commit working-directory: new run: | - git submodule init && git submodule update && ./gradlew clean publishToMavenLocal + ./gradlew clean publishToMavenLocal - name: Benchmark ion-java from the new commit working-directory: ion-java-benchmark-cli From 9a74c6ab100153fe02c5eb94ffeb293a85ad532c Mon Sep 17 00:00:00 2001 From: Tyler Gregg Date: Thu, 19 Oct 2023 17:10:44 -0700 Subject: [PATCH 3/8] Restores the previous binary reader implementation's local symbol table handling behavior for several edge cases. --- ...IonReaderContinuableApplicationBinary.java | 17 ++++++-- .../IonReaderContinuableTopLevelBinary.java | 5 +++ src/com/amazon/ion/impl/LocalSymbolTable.java | 20 +++------- .../impl/_Private_IonBinaryWriterBuilder.java | 4 +- .../ion/impl/_Private_LocalSymbolTable.java | 25 ++++++++++++ src/com/amazon/ion/impl/_Private_Utils.java | 4 +- test/com/amazon/ion/DatagramTest.java | 24 ++++++++++++ ...onReaderContinuableTopLevelBinaryTest.java | 31 +++++++++++++++ .../system/IonBinaryWriterBuilderTest.java | 39 +++++++++++++++++++ 9 files changed, 147 insertions(+), 22 deletions(-) create mode 100644 src/com/amazon/ion/impl/_Private_LocalSymbolTable.java diff --git a/src/com/amazon/ion/impl/IonReaderContinuableApplicationBinary.java b/src/com/amazon/ion/impl/IonReaderContinuableApplicationBinary.java index f9d2489bd0..9fe4e334b8 100644 --- a/src/com/amazon/ion/impl/IonReaderContinuableApplicationBinary.java +++ b/src/com/amazon/ion/impl/IonReaderContinuableApplicationBinary.java @@ -214,7 +214,7 @@ private SymbolTable getSystemSymbolTable() { /** * Read-only snapshot of the local symbol table at the reader's current position. */ - private class LocalSymbolTableSnapshot implements SymbolTable, SymbolTableAsStruct { + private class LocalSymbolTableSnapshot implements _Private_LocalSymbolTable, SymbolTableAsStruct { // The system symbol table. private final SymbolTable system = IonReaderContinuableApplicationBinary.this.getSystemSymbolTable(); @@ -425,6 +425,17 @@ public IonStruct getIonRepresentation(ValueFactory valueFactory) { } return structCache.getIonRepresentation(valueFactory); } + + @Override + public _Private_LocalSymbolTable makeCopy() { + // This is a mutable copy. LocalSymbolTable handles the mutability concerns. + return new LocalSymbolTable(importedTables, Arrays.asList(idToText)); + } + + @Override + public SymbolTable[] getImportedTablesNoCopy() { + return importedTables.getImportedTablesNoCopy(); + } } /** @@ -544,7 +555,7 @@ String getSymbol(int sid) { } int localSymbolOffset = sid - firstLocalSymbolId; if (localSymbolOffset > localSymbolMaxOffset) { - throw new IonException("Symbol ID exceeds the max ID of the symbol table."); + throw new UnknownSymbolException(sid); } return symbols[localSymbolOffset]; } @@ -565,7 +576,7 @@ private SymbolToken getSymbolToken(int sid) { } } if (sid >= symbolTableSize) { - throw new IonException("Symbol ID exceeds the max ID of the symbol table."); + throw new UnknownSymbolException(sid); } SymbolToken token = symbolTokensById.get(sid); if (token == null) { diff --git a/src/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java b/src/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java index dcce7d6a23..8e06fafe14 100644 --- a/src/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java +++ b/src/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java @@ -97,6 +97,11 @@ public SymbolTable pop_passed_symbol_table() { return null; } symbolTableLastTransferred = currentSymbolTable; + if (symbolTableLastTransferred.isLocalTable()) { + // This method is called when transferring the reader's symbol table to either a writer or an IonDatagram. + // Those cases require a mutable copy of the reader's symbol table. + return ((_Private_LocalSymbolTable) symbolTableLastTransferred).makeCopy(); + } return symbolTableLastTransferred; } diff --git a/src/com/amazon/ion/impl/LocalSymbolTable.java b/src/com/amazon/ion/impl/LocalSymbolTable.java index 56198844c1..cc6ca89497 100644 --- a/src/com/amazon/ion/impl/LocalSymbolTable.java +++ b/src/com/amazon/ion/impl/LocalSymbolTable.java @@ -51,7 +51,7 @@ * Instances of this class are safe for use by multiple threads. */ class LocalSymbolTable - implements SymbolTable + implements _Private_LocalSymbolTable { static class Factory implements _Private_LocalSymbolTableFactory @@ -329,7 +329,8 @@ else if (fieldType == IonType.SYMBOL && ION_SYMBOL_TABLE.equals(reader.stringVal return new LocalSymbolTableImports(importsList); } - synchronized LocalSymbolTable makeCopy() + @Override + public synchronized _Private_LocalSymbolTable makeCopy() { return new LocalSymbolTable(this, getMaxId()); } @@ -604,19 +605,8 @@ public SymbolTable[] getImportedTables() return myImportsList.getImportedTables(); } - /** - * Returns the imported symbol tables without making a copy. - *

- * Note: Callers must not modify the resulting SymbolTable array! - * This will violate the immutability property of this class. - * - * @return - * the imported symtabs, as-is; the first element is a system - * symtab, the rest are non-system shared symtabs - * - * @see #getImportedTables() - */ - SymbolTable[] getImportedTablesNoCopy() + @Override + public SymbolTable[] getImportedTablesNoCopy() { return myImportsList.getImportedTablesNoCopy(); } diff --git a/src/com/amazon/ion/impl/_Private_IonBinaryWriterBuilder.java b/src/com/amazon/ion/impl/_Private_IonBinaryWriterBuilder.java index 80f9d8d1dd..367f37a068 100644 --- a/src/com/amazon/ion/impl/_Private_IonBinaryWriterBuilder.java +++ b/src/com/amazon/ion/impl/_Private_IonBinaryWriterBuilder.java @@ -157,7 +157,7 @@ public void setInitialSymbolTable(SymbolTable symtab) if (symtab.isLocalTable()) { SymbolTable[] imports = - ((LocalSymbolTable) symtab).getImportedTablesNoCopy(); + ((_Private_LocalSymbolTable) symtab).getImportedTablesNoCopy(); for (SymbolTable imported : imports) { if (imported.isSubstitute()) @@ -353,7 +353,7 @@ SymbolTable buildContextSymbolTable() return myInitialSymbolTable; } - return ((LocalSymbolTable) myInitialSymbolTable).makeCopy(); + return ((_Private_LocalSymbolTable) myInitialSymbolTable).makeCopy(); } diff --git a/src/com/amazon/ion/impl/_Private_LocalSymbolTable.java b/src/com/amazon/ion/impl/_Private_LocalSymbolTable.java new file mode 100644 index 0000000000..f900fd0c83 --- /dev/null +++ b/src/com/amazon/ion/impl/_Private_LocalSymbolTable.java @@ -0,0 +1,25 @@ +package com.amazon.ion.impl; + +import com.amazon.ion.SymbolTable; + +interface _Private_LocalSymbolTable extends SymbolTable { + + /** + * @return a mutable copy of the symbol table. + */ + _Private_LocalSymbolTable makeCopy(); + + /** + * Returns the imported symbol tables without making a copy. + *

+ * Note: Callers must not modify the resulting SymbolTable array! + * This will violate the immutability property of this class. + * + * @return + * the imported symtabs, as-is; the first element is a system + * symtab, the rest are non-system shared symtabs + * + * @see SymbolTable#getImportedTables() + */ + SymbolTable[] getImportedTablesNoCopy(); +} diff --git a/src/com/amazon/ion/impl/_Private_Utils.java b/src/com/amazon/ion/impl/_Private_Utils.java index be806aeb72..ce68c35a19 100644 --- a/src/com/amazon/ion/impl/_Private_Utils.java +++ b/src/com/amazon/ion/impl/_Private_Utils.java @@ -800,7 +800,7 @@ public static SymbolTable copyLocalSymbolTable(SymbolTable symtab) } SymbolTable[] imports = - ((LocalSymbolTable) symtab).getImportedTablesNoCopy(); + ((_Private_LocalSymbolTable) symtab).getImportedTablesNoCopy(); // Iterate over each import, we assume that the list of imports // rarely exceeds 5. @@ -816,7 +816,7 @@ public static SymbolTable copyLocalSymbolTable(SymbolTable symtab) } } - return ((LocalSymbolTable) symtab).makeCopy(); + return ((_Private_LocalSymbolTable) symtab).makeCopy(); } /** diff --git a/test/com/amazon/ion/DatagramTest.java b/test/com/amazon/ion/DatagramTest.java index 493efb7eda..831ccfd036 100644 --- a/test/com/amazon/ion/DatagramTest.java +++ b/test/com/amazon/ion/DatagramTest.java @@ -28,8 +28,11 @@ import com.amazon.ion.impl._Private_IonValue; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.util.Collection; import java.util.Iterator; + +import com.amazon.ion.system.IonReaderBuilder; import org.junit.Before; import org.junit.Test; @@ -720,4 +723,25 @@ public void testSetWithSymbolTable() assertEquals(yID + 1, v1.symbolValue().getSid()); //now x has a mapping in the existing symbol table } + + @Test + public void modifySymbolsAfterLoadThenSerialize() + throws Exception + { + IonDatagram dg = system().getLoader().load(TestUtils.ensureBinary(system(), "{foo: bar::baz}".getBytes(StandardCharsets.UTF_8))); + IonStruct struct = (IonStruct) dg.get(0); + IonSymbol baz = (IonSymbol) struct.get("foo"); + baz.setTypeAnnotations("bar", "abc"); + byte[] serialized = dg.getBytes(); + try (IonReader reader = IonReaderBuilder.standard().build(serialized)) { + assertEquals(IonType.STRUCT, reader.next()); + reader.stepIn(); + assertEquals(IonType.SYMBOL, reader.next()); + assertEquals("foo", reader.getFieldName()); + String[] annotations = reader.getTypeAnnotations(); + assertEquals(2, annotations.length); + assertEquals("bar", annotations[0]); + assertEquals("abc", annotations[1]); + } + } } diff --git a/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java b/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java index b7422c473d..ff41c7a785 100644 --- a/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java +++ b/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java @@ -23,6 +23,7 @@ import com.amazon.ion.SystemSymbols; import com.amazon.ion.TestUtils; import com.amazon.ion.Timestamp; +import com.amazon.ion.UnknownSymbolException; import com.amazon.ion.impl.bin._Private_IonManagedBinaryWriterBuilder; import com.amazon.ion.impl.bin._Private_IonManagedWriter; import com.amazon.ion.impl.bin._Private_IonRawWriter; @@ -769,6 +770,36 @@ public void invalidVersionMarker(boolean constructFromBytes) throws Exception { }); } + @ParameterizedTest(name = "constructFromBytes={0}") + @ValueSource(booleans = {true, false}) + public void unknownSymbolInFieldName(boolean constructFromBytes) throws Exception { + reader = readerFor(constructFromBytes, 0xD3, 0x8A, 0x21, 0x01); + assertSequence(next(IonType.STRUCT), STEP_IN, next(IonType.INT)); + assertThrows(UnknownSymbolException.class, reader::getFieldNameSymbol); + assertThrows(UnknownSymbolException.class, reader::getFieldName); + reader.close(); + } + + @ParameterizedTest(name = "constructFromBytes={0}") + @ValueSource(booleans = {true, false}) + public void unknownSymbolInAnnotation(boolean constructFromBytes) throws Exception { + reader = readerFor(constructFromBytes, 0xE4, 0x81, 0x8A, 0x21, 0x01); + assertSequence(next(IonType.INT)); + assertThrows(UnknownSymbolException.class, reader::getTypeAnnotationSymbols); + assertThrows(UnknownSymbolException.class, reader::getTypeAnnotations); + reader.close(); + } + + @ParameterizedTest(name = "constructFromBytes={0}") + @ValueSource(booleans = {true, false}) + public void unknownSymbolInValue(boolean constructFromBytes) throws Exception { + reader = readerFor(constructFromBytes, 0x71, 0x0A); + assertSequence(next(IonType.SYMBOL)); + assertThrows(UnknownSymbolException.class, reader::symbolValue); + assertThrows(UnknownSymbolException.class, reader::stringValue); + reader.close(); + } + /** * Feeds all bytes from the given array into the pipe one-by-one, asserting before each byte that the reader * is not positioned on a value. diff --git a/test/com/amazon/ion/system/IonBinaryWriterBuilderTest.java b/test/com/amazon/ion/system/IonBinaryWriterBuilderTest.java index 410ac4509e..f27c6eb381 100644 --- a/test/com/amazon/ion/system/IonBinaryWriterBuilderTest.java +++ b/test/com/amazon/ion/system/IonBinaryWriterBuilderTest.java @@ -24,9 +24,12 @@ import static org.junit.Assert.fail; import com.amazon.ion.IonCatalog; +import com.amazon.ion.IonReader; import com.amazon.ion.IonSystem; +import com.amazon.ion.IonType; import com.amazon.ion.IonWriter; import com.amazon.ion.SymbolTable; +import com.amazon.ion.TestUtils; import com.amazon.ion.impl.Symtabs; import com.amazon.ion.impl._Private_IonBinaryWriterBuilder; import com.amazon.ion.impl._Private_IonWriter; @@ -34,6 +37,8 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; +import java.nio.charset.StandardCharsets; + import org.junit.Assert; import org.junit.Test; @@ -299,6 +304,40 @@ public void testImmutableInitialSymtab() assertTrue(symbolTableEquals(lst, writer.getSymbolTable())); } + @Test + public void testInitialSymtabFromReader() + throws Exception + { + SymbolTable lst; + try (IonReader reader = IonReaderBuilder.standard().build(TestUtils.ensureBinary(IonSystemBuilder.standard().build(), "foo".getBytes(StandardCharsets.UTF_8)))) { + assertEquals(IonType.SYMBOL, reader.next()); + lst = reader.getSymbolTable(); + } + + _Private_IonBinaryWriterBuilder b = + _Private_IonBinaryWriterBuilder.standard(); + b.setInitialSymbolTable(lst); + assertSame(lst, b.getInitialSymbolTable()); + + ByteArrayOutputStream out = new ByteArrayOutputStream(); + IonWriter writer = b.build(out); + assertTrue(symbolTableEquals(lst, writer.getSymbolTable())); + + writer = b.build(out); + assertTrue(symbolTableEquals(lst, writer.getSymbolTable())); + + writer.writeSymbol("bar"); + writer.writeSymbol("foo"); + writer.close(); + + try (IonReader reader = IonReaderBuilder.standard().build(out.toByteArray())) { + assertEquals(IonType.SYMBOL, reader.next()); + assertEquals("bar", reader.stringValue()); + assertEquals(IonType.SYMBOL, reader.next()); + assertEquals("foo", reader.stringValue()); + } + } + @Test(expected = UnsupportedOperationException.class) public void testInitialSymtabImmutability() { From 2963d4b34825d9f140c78e3634bfad02c1b17cbe Mon Sep 17 00:00:00 2001 From: Tyler Gregg Date: Wed, 1 Nov 2023 15:34:01 -0700 Subject: [PATCH 4/8] Fixes a bug that caused concurrent field access of cloned, read-only structs with more than 5 members to be non-deterministic. (#630) --- .../amazon/ion/impl/lite/IonStructLite.java | 8 +++++ test/com/amazon/ion/CloneTest.java | 29 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/src/com/amazon/ion/impl/lite/IonStructLite.java b/src/com/amazon/ion/impl/lite/IonStructLite.java index ed32ef6a45..c6ea1fca9a 100644 --- a/src/com/amazon/ion/impl/lite/IonStructLite.java +++ b/src/com/amazon/ion/impl/lite/IonStructLite.java @@ -101,6 +101,14 @@ private void build_field_map() _field_map.put(v._fieldName, ii); // this causes the map to have the largest index value stored } } + + @Override + public void makeReadOnly() { + // Eagerly initialize the fields map to prevent potential data races https://github.com/amazon-ion/ion-java/issues/629 + fieldMapIsActive(_child_count); + super.makeReadOnly(); + } + private void add_field(String fieldName, int newFieldIdx) { Integer idx = _field_map.get(fieldName); diff --git a/test/com/amazon/ion/CloneTest.java b/test/com/amazon/ion/CloneTest.java index 7dfe5df275..9950de5a7e 100644 --- a/test/com/amazon/ion/CloneTest.java +++ b/test/com/amazon/ion/CloneTest.java @@ -20,6 +20,10 @@ import com.amazon.ion.system.IonSystemBuilder; import org.junit.jupiter.api.Test; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; + import static com.amazon.ion.impl._Private_Utils.newSymbolToken; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.MatcherAssert.assertThat; @@ -28,6 +32,7 @@ import static org.hamcrest.Matchers.sameInstance; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; public class CloneTest @@ -261,6 +266,30 @@ public void modifyAfterCloneDoesNotChangeOriginal() { assertNotEquals(original, clone); } + @Test + public void readOnlyIonStructMultithreadedTest() { + // See: https://github.com/amazon-ion/ion-java/issues/629 + String ionStr = "{a:1,b:2,c:3,d:4,e:5,f:6}"; + + IonStruct ionValue = (IonStruct)system().singleValue(ionStr); + ionValue.makeReadOnly(); + + for (int i=0; i<100; i++) { + IonStruct clone = ionValue.clone(); + clone.makeReadOnly(); + + List> waiting = new ArrayList<>(); + for (int j = 0; j < 4; j++) { + waiting.add(CompletableFuture.runAsync(() -> { + for (int k = 0; k <= 100; k++) { + assertNotNull(clone.get("a")); + } + })); + } + waiting.forEach(CompletableFuture::join); + } + } + /** * @return the singleton IonSystem */ From ab84b08f4617f402ce74cb2727058eee74239dcb Mon Sep 17 00:00:00 2001 From: Tyler Gregg Date: Thu, 2 Nov 2023 17:15:09 -0700 Subject: [PATCH 5/8] Makes the binary reader set the encoding version when seeked to a span. (#622) --- src/com/amazon/ion/impl/IonCursorBinary.java | 12 +++++++++++- .../IonReaderContinuableTopLevelBinary.java | 2 +- .../ion/streaming/SeekableReaderTest.java | 17 +++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/com/amazon/ion/impl/IonCursorBinary.java b/src/com/amazon/ion/impl/IonCursorBinary.java index 5308dffc7e..ed1d9defe7 100644 --- a/src/com/amazon/ion/impl/IonCursorBinary.java +++ b/src/com/amazon/ion/impl/IonCursorBinary.java @@ -9,6 +9,7 @@ import com.amazon.ion.IonCursor; import com.amazon.ion.IonType; import com.amazon.ion.IvmNotificationConsumer; +import com.amazon.ion.SystemSymbols; import java.io.ByteArrayInputStream; import java.io.EOFException; @@ -1786,8 +1787,9 @@ Marker getValueMarker() { * can be used to seek the reader to a "span" of bytes that represent a value in the stream. * @param offset the offset at which the slice will begin. * @param limit the slice's limit. + * @param ionVersionId the Ion version ID for the slice, e.g. $ion_1_0 for Ion 1.0. */ - void slice(long offset, long limit) { + void slice(long offset, long limit, String ionVersionId) { peekIndex = offset; this.limit = limit; setCheckpointBeforeUnannotatedTypeId(); @@ -1795,6 +1797,14 @@ void slice(long offset, long limit) { event = Event.NEEDS_DATA; valueTid = null; containerIndex = -1; // Slices are treated as if they were at the top level. + if (SystemSymbols.ION_1_0.equals(ionVersionId)) { + typeIds = IonTypeID.TYPE_IDS_1_0; + majorVersion = 1; + minorVersion = 0; + } else { + // TODO changes are needed here to support Ion 1.1. + throw new IonException(String.format("Attempted to seek using an unsupported Ion version %s.", ionVersionId)); + } } /** diff --git a/src/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java b/src/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java index 8e06fafe14..25a9383f8e 100644 --- a/src/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java +++ b/src/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java @@ -308,8 +308,8 @@ public void hoist(Span span) { // of the value to be the end of the stream, in order to comply with the SeekableReader contract. From // an implementation perspective, this is not necessary; if we leave the buffer's limit unchanged, the // reader can continue after processing the hoisted value. - slice(binarySpan.bufferOffset, binarySpan.bufferLimit); restoreSymbolTable(binarySpan.symbolTable); + slice(binarySpan.bufferOffset, binarySpan.bufferLimit, binarySpan.symbolTable.getIonVersionId()); type = null; } } diff --git a/test/com/amazon/ion/streaming/SeekableReaderTest.java b/test/com/amazon/ion/streaming/SeekableReaderTest.java index 5510068edc..066249d386 100644 --- a/test/com/amazon/ion/streaming/SeekableReaderTest.java +++ b/test/com/amazon/ion/streaming/SeekableReaderTest.java @@ -19,6 +19,7 @@ import com.amazon.ion.IonType; import com.amazon.ion.IonWriter; import com.amazon.ion.ReaderMaker; +import com.amazon.ion.SeekableReader; import com.amazon.ion.Span; import com.amazon.ion.TestUtils; import com.amazon.ion.impl._Private_Utils; @@ -327,6 +328,22 @@ public void testHoistingAcrossSymbolTableBoundary() expectTopEof(); } + @Test + public void testHoistingFromSpanCreatedByDifferentReaderBeforeNext() + { + read("foo bar"); + in.next(); + in.next(); + Span barSpan = sr.currentSpan(); + + read("foo bar"); // Creates a new reader + initFacets(); + + hoist(barSpan); + assertSame(IonType.SYMBOL, in.next()); + assertEquals("bar", in.stringValue()); + } + //======================================================================== // Failure cases From 61d76e076c1ea4d476fd2362e83272f961e39665 Mon Sep 17 00:00:00 2001 From: Tyler Gregg Date: Thu, 2 Nov 2023 17:23:24 -0700 Subject: [PATCH 6/8] Handles the case where the binary cursor's InputStream provides fewer bytes than requested before reaching EOF. (#623) --- src/com/amazon/ion/impl/IonCursorBinary.java | 45 +++++--- ...onReaderContinuableTopLevelBinaryTest.java | 108 ++++++++++++++++++ 2 files changed, 135 insertions(+), 18 deletions(-) diff --git a/src/com/amazon/ion/impl/IonCursorBinary.java b/src/com/amazon/ion/impl/IonCursorBinary.java index ed1d9defe7..f657c634fe 100644 --- a/src/com/amazon/ion/impl/IonCursorBinary.java +++ b/src/com/amazon/ion/impl/IonCursorBinary.java @@ -524,8 +524,7 @@ private boolean fillAt(long index, long numberOfBytes) { refillableState.bytesRequested = numberOfBytes + (index - offset); if (ensureCapacity(refillableState.bytesRequested)) { // Fill all the free space, not just the shortfall; this reduces I/O. - refill(freeSpaceAt(limit)); - shortfall = refillableState.bytesRequested - availableAt(offset); + shortfall = refill(refillableState.bytesRequested); } else { // The request cannot be satisfied, but not because data was unavailable. Return normally; it is the // caller's responsibility to recover. @@ -646,24 +645,34 @@ private void shiftIndicesLeft(int shiftAmount) { } /** - * Fills the buffer with up to the requested number of additional bytes. It is the caller's responsibility to - * ensure that there is space in the buffer. - * @param numberOfBytesToFill the number of additional bytes to attempt to add to the buffer. + * Attempts to fill the buffer with up to the requested number of additional bytes. It is the caller's + * responsibility to ensure that there is space in the buffer. + * @param minimumNumberOfBytesRequired the minimum number of bytes requested to fill the current value. + * @return the shortfall between the number of bytes that were filled and the minimum number requested. If less than + * 1, then at least `minimumNumberOfBytesRequired` were filled. */ - private void refill(long numberOfBytesToFill) { + private long refill(long minimumNumberOfBytesRequired) { int numberOfBytesFilled = -1; - try { - numberOfBytesFilled = refillableState.inputStream.read(buffer, (int) limit, (int) numberOfBytesToFill); - } catch (EOFException e) { - // Certain InputStream implementations (e.g. GZIPInputStream) throw EOFException if more bytes are requested - // to read than are currently available (e.g. if a header or trailer is incomplete). - } catch (IOException e) { - throwAsIonException(e); - } - if (numberOfBytesFilled < 0) { - return; - } - limit += numberOfBytesFilled; + long shortfall; + // Sometimes an InputStream implementation will return fewer than the number of bytes requested even + // if the stream is not at EOF. If this happens and there is still a shortfall, keep requesting bytes + // until either the shortfall is filled or EOF is reached. + do { + try { + numberOfBytesFilled = refillableState.inputStream.read(buffer, (int) limit, (int) freeSpaceAt(limit)); + } catch (EOFException e) { + // Certain InputStream implementations (e.g. GZIPInputStream) throw EOFException if more bytes are requested + // to read than are currently available (e.g. if a header or trailer is incomplete). + numberOfBytesFilled = -1; + } catch (IOException e) { + throwAsIonException(e); + } + if (numberOfBytesFilled > 0) { + limit += numberOfBytesFilled; + } + shortfall = minimumNumberOfBytesRequired - availableAt(offset); + } while (shortfall > 0 && numberOfBytesFilled >= 0); + return shortfall; } /** diff --git a/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java b/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java index ff41c7a785..679286d7bf 100644 --- a/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java +++ b/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java @@ -34,10 +34,12 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.EOFException; import java.io.IOException; import java.io.InputStream; import java.math.BigDecimal; @@ -3196,6 +3198,112 @@ public void skipWithoutEnoughDataNonIncrementalFails() throws Exception { reader.close(); } + /** + * An InputStream that returns less than the number of bytes requested from bulk reads. + */ + private static class ThrottlingInputStream extends InputStream { + + private final byte[] data; + private final boolean throwFromReadOnEof; + private int offset = 0; + + /** + * @param data the data for the InputStream to provide. + * @param throwFromReadOnEof true if the stream should throw {@link java.io.EOFException} when read() is called + * at EOF. If false, simply returns -1. + */ + protected ThrottlingInputStream(byte[] data, boolean throwFromReadOnEof) { + this.data = data; + this.throwFromReadOnEof = throwFromReadOnEof; + } + + @Override + public int read() { + return data[offset++] & 0xFF; + } + + private int calculateNumberOfBytesToReturn(int numberOfBytesRequested) { + int available = data.length - offset; + int numberOfBytesToReturn; + if (available > 1 && numberOfBytesRequested > 1) { + // Return fewer bytes than requested and fewer than are available, avoiding EOF. + numberOfBytesToReturn = Math.min(available - 1, numberOfBytesRequested - 1); + } else if (available <= 0) { + return -1; // EOF + } else { + // Only 1 byte is available, so return it as long as at least 1 byte was requested. + numberOfBytesToReturn = Math.min(numberOfBytesRequested, available); + } + return numberOfBytesToReturn; + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + if (off + len > b.length) { + throw new IndexOutOfBoundsException(); + } + int numberOfBytesToReturn = calculateNumberOfBytesToReturn(len); + if (numberOfBytesToReturn < 0) { + if (throwFromReadOnEof) { + throw new EOFException(); + } + return -1; + } + System.arraycopy(data, offset, b, off, numberOfBytesToReturn); + offset += numberOfBytesToReturn; + return numberOfBytesToReturn; + } + + @Override + public long skip(long len) { + int numberOfBytesToSkip = calculateNumberOfBytesToReturn((int) len); + offset += numberOfBytesToSkip; + return numberOfBytesToSkip; + } + } + + @ParameterizedTest(name = "incrementalReadingEnabled={0},throwOnEof={1}") + @CsvSource({ + "true, true", + "true, false", + "false, true", + "false, false" + }) + public void shouldNotFailWhenAnInputStreamProvidesFewerBytesThanRequestedWithoutReachingEof(boolean incrementalReadingEnabled, boolean throwOnEof) throws Exception { + readerBuilder = readerBuilder.withIncrementalReadingEnabled(incrementalReadingEnabled) + .withBufferConfiguration(IonBufferConfiguration.Builder.standard().withInitialBufferSize(8).build()); + reader = readerFor(new ThrottlingInputStream(bytes(0xE0, 0x01, 0x00, 0xEA, 0x89, 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i'), throwOnEof)); + assertSequence( + next(IonType.STRING), stringValue("abcdefghi"), + next(null) + ); + reader.close(); + } + + @Test + public void shouldNotFailWhenAnInputStreamProvidesFewerBytesThanRequestedWithoutReachingEofAndTheReaderSkipsTheValue() throws Exception { + reader = boundedReaderFor(new ThrottlingInputStream(bytes(0xE0, 0x01, 0x00, 0xEA, 0x89, 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 0x20), false), 8, 8, byteAndOversizedValueCountingHandler); + assertSequence( + next(IonType.INT), intValue(0), + next(null) + ); + reader.close(); + assertEquals(1, oversizedCounter.get()); + } + + @Test + public void shouldNotFailWhenGZIPBoundaryIsEncounteredInStringValue() throws Exception { + ResizingPipedInputStream pipe = new ResizingPipedInputStream(128); + // The following lines create a GZIP payload boundary (trailer/header) in the middle of an Ion string value. + pipe.receive(gzippedBytes(0xE0, 0x01, 0x00, 0xEA, 0x89, 'a', 'b')); + pipe.receive(gzippedBytes('c', 'd', 'e', 'f', 'g', 'h', 'i')); + reader = readerFor(new GZIPInputStream(pipe)); + assertSequence( + next(IonType.STRING), stringValue("abcdefghi"), + next(null) + ); + } + @Test public void concatenatedAfterGZIPHeader() throws Exception { // Tests that a stream that initially contains only a GZIP header can be read successfully if more data From 8df9b7d9029e16373268e3ca42bfe858034d0d18 Mon Sep 17 00:00:00 2001 From: Tyler Gregg Date: Thu, 2 Nov 2023 17:38:39 -0700 Subject: [PATCH 7/8] Ensures that the binary reader can handle a ByteArrayInputStream that returns a negative number from available(). (#625) --- src/com/amazon/ion/impl/IonCursorBinary.java | 5 +- .../impl/IonReaderContinuableCoreBinary.java | 49 +++++++++---------- ...onReaderContinuableTopLevelBinaryTest.java | 11 +++++ .../ion/streaming/ReaderIntegerSizeTest.java | 10 ++-- 4 files changed, 41 insertions(+), 34 deletions(-) diff --git a/src/com/amazon/ion/impl/IonCursorBinary.java b/src/com/amazon/ion/impl/IonCursorBinary.java index f657c634fe..c7929e58e8 100644 --- a/src/com/amazon/ion/impl/IonCursorBinary.java +++ b/src/com/amazon/ion/impl/IonCursorBinary.java @@ -389,7 +389,10 @@ private static IonBufferConfiguration getFixedSizeConfigurationFor( ByteArrayInputStream inputStream, int alreadyReadLen ) { - int fixedBufferSize = inputStream.available(); + // Note: ByteArrayInputStream.available() can return a negative number because its constructor does + // not validate that the offset and length provided are actually within range of the provided byte array. + // Setting the result to 0 in this case avoids an error when looking up the fixed sized configuration. + int fixedBufferSize = Math.max(0, inputStream.available()); if (alreadyReadLen > 0) { fixedBufferSize += alreadyReadLen; } diff --git a/src/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java b/src/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java index 862520d14f..811281e64c 100644 --- a/src/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java +++ b/src/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java @@ -51,11 +51,13 @@ class IonReaderContinuableCoreBinary extends IonCursorBinary implements IonReade // The number of bytes occupied by a Java long. private static final int LONG_SIZE_IN_BYTES = 8; - // The smallest negative 8-byte integer that can fit in a long is -0x80_00_00_00_00_00_00_00. - private static final int MOST_SIGNIFICANT_BYTE_OF_MIN_LONG = 0x80; + // The smallest negative 8-byte integer that can fit in a long is 0x80_00_00_00_00_00_00_00 and the smallest + // negative 4-byte integer that can fit in an int is 0x80_00_00_00. + private static final int MOST_SIGNIFICANT_BYTE_OF_MIN_INTEGER = 0x80; - // The largest positive 8-byte integer that can fit in a long is 0x7F_FF_FF_FF_FF_FF_FF_FF. - private static final int MOST_SIGNIFICANT_BYTE_OF_MAX_LONG = 0x7F; + // The largest positive 8-byte integer that can fit in a long is 0x7F_FF_FF_FF_FF_FF_FF_FF and the largest positive + // 4-byte integer that can fit in an int is 0x7F_FF_FF_FF. + private static final int MOST_SIGNIFICANT_BYTE_OF_MAX_INTEGER = 0x7F; // The second-most significant bit in the most significant byte of a VarInt is the sign. private static final int VAR_INT_SIGN_BITMASK = 0x40; @@ -418,34 +420,30 @@ private boolean readBoolean_1_0() { } /** - * Determines whether the 8-byte integer starting at `valueMarker.startIndex` and ending at `valueMarker.endIndex` - * fits in a long, or requires a BigInteger. - * @return either `IntegerSize.LONG` or `IntegerSize.BIG_INTEGER`. + * Determines whether the integer starting at `valueMarker.startIndex` and ending at `valueMarker.endIndex` + * crosses a type boundary. Callers must only invoke this method when the integer's size is known to be either + * 4 or 8 bytes. + * @return true if the value fits in the Java integer type that matches its Ion serialized size; false if it + * requires the next larger size. */ - private IntegerSize classifyEightByteInt_1_0() { + private boolean classifyInteger_1_0() { if (valueTid.isNegativeInt) { - // The smallest negative 8-byte integer that can fit in a long is -0x80_00_00_00_00_00_00_00. int firstByte = buffer[(int)(valueMarker.startIndex)] & SINGLE_BYTE_MASK; - if (firstByte < MOST_SIGNIFICANT_BYTE_OF_MIN_LONG) { - return IntegerSize.LONG; - } else if (firstByte > MOST_SIGNIFICANT_BYTE_OF_MIN_LONG) { - return IntegerSize.BIG_INTEGER; + if (firstByte < MOST_SIGNIFICANT_BYTE_OF_MIN_INTEGER) { + return true; + } else if (firstByte > MOST_SIGNIFICANT_BYTE_OF_MIN_INTEGER) { + return false; } for (long i = valueMarker.startIndex + 1; i < valueMarker.endIndex; i++) { if (0x00 != buffer[(int)(i)]) { - return IntegerSize.BIG_INTEGER; + return false; } } - } else { - // The largest positive 8-byte integer that can fit in a long is 0x7F_FF_FF_FF_FF_FF_FF_FF. - if ((buffer[(int)(valueMarker.startIndex)] & SINGLE_BYTE_MASK) > MOST_SIGNIFICANT_BYTE_OF_MAX_LONG) { - return IntegerSize.BIG_INTEGER; - } + return true; } - return IntegerSize.LONG; + return (buffer[(int) (valueMarker.startIndex)] & SINGLE_BYTE_MASK) <= MOST_SIGNIFICANT_BYTE_OF_MAX_INTEGER; } - int readVarUInt_1_1() { throw new UnsupportedOperationException(); } @@ -536,16 +534,13 @@ public IntegerSize getIntegerSize() { if (valueTid.length < 0) { return IntegerSize.BIG_INTEGER; } else if (valueTid.length < INT_SIZE_IN_BYTES) { - // Note: this is conservative. Most integers of size 4 also fit in an int, but since exactly the - // same parsing code is used for ints and longs, there is no point wasting the time to determine the - // smallest possible type. return IntegerSize.INT; + } else if (valueTid.length == INT_SIZE_IN_BYTES) { + return (minorVersion != 0 || classifyInteger_1_0()) ? IntegerSize.INT : IntegerSize.LONG; } else if (valueTid.length < LONG_SIZE_IN_BYTES) { return IntegerSize.LONG; } else if (valueTid.length == LONG_SIZE_IN_BYTES) { - // Because creating BigIntegers is so expensive, it is worth it to look ahead and determine exactly - // which 8-byte integers can fit in a long. - return minorVersion == 0 ? classifyEightByteInt_1_0() : IntegerSize.LONG; + return (minorVersion != 0 || classifyInteger_1_0()) ? IntegerSize.LONG : IntegerSize.BIG_INTEGER; } return IntegerSize.BIG_INTEGER; } diff --git a/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java b/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java index 679286d7bf..5542076e2a 100644 --- a/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java +++ b/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java @@ -3368,6 +3368,17 @@ public void concatenatedAfterMissingGZIPTrailer() throws Exception { assertEquals(1, oversizedCounter.get()); } + @Test + public void shouldNotFailWhenProvidedWithAnEmptyByteArrayInputStream() throws Exception { + reader = IonReaderBuilder.standard().build(new ByteArrayInputStream(new byte[]{})); + assertSequence(next(null)); + reader.close(); + // The following ByteArrayInputStream is weird, but not disallowed. Its available() method will return -1. + reader = IonReaderBuilder.standard().build(new ByteArrayInputStream(new byte[]{}, 1, 1)); + assertSequence(next(null)); + reader.close(); + } + @ParameterizedTest(name = "constructFromBytes={0}") @ValueSource(booleans = {true, false}) public void incompleteContainerNonContinuable(boolean constructFromBytes) throws Exception { diff --git a/test/com/amazon/ion/streaming/ReaderIntegerSizeTest.java b/test/com/amazon/ion/streaming/ReaderIntegerSizeTest.java index 73fcb7193b..cc31a06cb3 100644 --- a/test/com/amazon/ion/streaming/ReaderIntegerSizeTest.java +++ b/test/com/amazon/ion/streaming/ReaderIntegerSizeTest.java @@ -142,11 +142,10 @@ public void testGetIntegerSizeNegativeIntBoundary() private void testGetIntegerSizeIntBoundary(int boundaryValue, long pastBoundary) { in.next(); - // It's fine if IntegerSize recommends a larger-than-necessary type, just not a smaller one. - assertTrue(IntegerSize.INT == in.getIntegerSize() || IntegerSize.LONG == in.getIntegerSize()); + assertEquals(IntegerSize.INT, in.getIntegerSize()); assertEquals(boundaryValue, in.intValue()); // assert nothing changes until next() - assertTrue(IntegerSize.INT == in.getIntegerSize() || IntegerSize.LONG == in.getIntegerSize()); + assertEquals(IntegerSize.INT, in.getIntegerSize()); in.next(); assertEquals(IntegerSize.LONG, in.getIntegerSize()); assertEquals(pastBoundary, in.longValue()); @@ -156,10 +155,9 @@ private void testGetIntegerSizeIntBoundary(int boundaryValue, long pastBoundary) private void testGetIntegerSizeLongBoundary(long boundaryValue, BigInteger pastBoundary) { in.next(); - // It's fine if IntegerSize recommends a larger-than-necessary type, just not a smaller one. - assertTrue(IntegerSize.LONG == in.getIntegerSize() || IntegerSize.BIG_INTEGER == in.getIntegerSize()); + assertEquals(IntegerSize.LONG, in.getIntegerSize()); assertEquals(boundaryValue, in.longValue()); - assertTrue(IntegerSize.LONG == in.getIntegerSize() || IntegerSize.BIG_INTEGER == in.getIntegerSize()); + assertEquals(IntegerSize.LONG, in.getIntegerSize()); in.next(); assertEquals(IntegerSize.BIG_INTEGER, in.getIntegerSize()); assertEquals(pastBoundary, in.bigIntegerValue()); From b0313763c84572c9e8010e0893ee9a493b41f6e0 Mon Sep 17 00:00:00 2001 From: Tyler Gregg Date: Thu, 2 Nov 2023 17:48:39 -0700 Subject: [PATCH 8/8] Fixes a bug that prevented early step-out of a container when positioned on a large scalar value in certain cases. (#628) * Makes the binary reader set the encoding version when seeked to a span. * Handles the case where the binary cursor's InputStream provides fewer bytes than requested before reaching EOF. * Ensures that the binary reader can handle a ByteArrayInputStream that returns a negative number from available(). * Accurately classifies all int values by size, rather than conservatively classifying all 4-byte ints as Java longs. * Fixes a bug that prevented early step-out of a container when positioned on a large scalar value in certain cases. * Ensures that spans are treated as if they are at the top level even when hoisted from below the top level. --- src/com/amazon/ion/impl/IonCursorBinary.java | 6 +++-- ...onReaderContinuableTopLevelBinaryTest.java | 27 +++++++++++++++++++ .../ion/streaming/SeekableReaderTest.java | 23 ++++++++++++++++ 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/com/amazon/ion/impl/IonCursorBinary.java b/src/com/amazon/ion/impl/IonCursorBinary.java index c7929e58e8..0d39112378 100644 --- a/src/com/amazon/ion/impl/IonCursorBinary.java +++ b/src/com/amazon/ion/impl/IonCursorBinary.java @@ -1470,7 +1470,7 @@ private Event slowStepOutOfContainer() { if (slowSeek(parent.endIndex - offset)) { return event; } - peekIndex = parent.endIndex; + peekIndex = offset; } setCheckpointBeforeUnannotatedTypeId(); if (--containerIndex >= 0) { @@ -1808,7 +1808,9 @@ void slice(long offset, long limit, String ionVersionId) { valueMarker.endIndex = -1; event = Event.NEEDS_DATA; valueTid = null; - containerIndex = -1; // Slices are treated as if they were at the top level. + // Slices are treated as if they were at the top level. + parent = null; + containerIndex = -1; if (SystemSymbols.ION_1_0.equals(ionVersionId)) { typeIds = IonTypeID.TYPE_IDS_1_0; majorVersion = 1; diff --git a/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java b/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java index 5542076e2a..695cd86d72 100644 --- a/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java +++ b/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java @@ -1725,6 +1725,33 @@ public void reallyLargeString(boolean constructFromBytes) throws Exception { closeAndCount(); } + @ParameterizedTest + @CsvSource({ + "true, true", + "true, false", + "false, true", + "false, false" + }) + public void skipReallyLargeStringInContainer(boolean constructFromBytes, boolean incremental) throws Exception { + StringBuilder sb = new StringBuilder(); + // 70000 is greater than twice the default buffer size of 32768. This ensures that the string, when skipped, + // will not be fully contained in the buffer. + for (int i = 0; i < 70000; i++) { + sb.append('a'); + } + String string = sb.toString(); + readerBuilder = readerBuilder.withIncrementalReadingEnabled(incremental); + reader = readerFor("{foo: \"" + string + "\"}", constructFromBytes); + assertSequence( + container(IonType.STRUCT, + next(IonType.STRING) + // This is an early-step out that causes the string value to be skipped. + ), + next(null) + ); + closeAndCount(); + } + @ParameterizedTest(name = "constructFromBytes={0}") @ValueSource(booleans = {true, false}) public void nopPadInAnnotationWrapperFails(boolean constructFromBytes) throws Exception { diff --git a/test/com/amazon/ion/streaming/SeekableReaderTest.java b/test/com/amazon/ion/streaming/SeekableReaderTest.java index 066249d386..95992ac0f9 100644 --- a/test/com/amazon/ion/streaming/SeekableReaderTest.java +++ b/test/com/amazon/ion/streaming/SeekableReaderTest.java @@ -263,6 +263,29 @@ public void testHoistingAnnotatedContainedValue() expectTopEof(); } + @Test + public void testHoistingNestedContainerWithoutSteppingOutOfParent() + { + read("{first: {foo: bar}, second: a::{baz: zar}, third: 123}"); + in.next(); + in.stepIn(); + in.next(); + in.next(); + Span span = sr.currentSpan(); + in.next(); + // Note: we do not step out of the struct. + + hoist(span); + assertSame(IonType.STRUCT, in.next()); + expectTopLevel(); + Assert.assertArrayEquals(new String[]{"a"}, in.getTypeAnnotations()); + in.stepIn(); + in.next(); + assertEquals("zar", in.stringValue()); + in.stepOut(); + expectTopEof(); + } + @Test public void testHoistingAcrossSymbolTableBoundary() throws IOException