From ba76b5b95678c50c26a886d651ac72c3c898e15e Mon Sep 17 00:00:00 2001 From: Tyler Gregg Date: Wed, 30 Oct 2024 13:23:09 -0700 Subject: [PATCH] Ensures that macro arguments are present in the binary reader's buffer before attempting to materialize them. --- .../impl/IonReaderContinuableCoreBinary.java | 9 +++ .../impl/macro/ReaderAdapterContinuable.kt | 77 ++++++++++++++++--- ...onReaderContinuableTopLevelBinaryTest.java | 25 ++++++ 3 files changed, 99 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java b/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java index d4d101f26..b2394ed2a 100644 --- a/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java +++ b/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java @@ -1789,6 +1789,15 @@ public Event nextValue() { return event; } + @Override + public Event fillValue() { + if (isEvaluatingEExpression) { + event = Event.VALUE_READY; + return event; + } + return super.fillValue(); + } + @Override public Event stepIntoContainer() { if (isEvaluatingEExpression) { diff --git a/src/main/java/com/amazon/ion/impl/macro/ReaderAdapterContinuable.kt b/src/main/java/com/amazon/ion/impl/macro/ReaderAdapterContinuable.kt index 9bf72f9d9..61aabc47f 100644 --- a/src/main/java/com/amazon/ion/impl/macro/ReaderAdapterContinuable.kt +++ b/src/main/java/com/amazon/ion/impl/macro/ReaderAdapterContinuable.kt @@ -25,15 +25,44 @@ internal class ReaderAdapterContinuable(val reader: IonReaderContinuableCore) : return event != IonCursor.Event.NEEDS_DATA && event != IonCursor.Event.END_CONTAINER } - override fun stringValue(): String = reader.stringValue() + /** + * Ensures that the value on which the reader is positioned is fully buffered. + */ + private fun prepareValue() { + // TODO performance: fill entire expression groups up-front so that the reader will usually not be in slow + // mode when this is called. + if (reader.fillValue() != IonCursor.Event.VALUE_READY) { + throw IonException("TODO: support continuable reading and oversize value handling via this adapter.") + } + } + + override fun stringValue(): String { + prepareValue() + return reader.stringValue() + } - override fun intValue(): Int = reader.intValue() + override fun intValue(): Int { + prepareValue() + return reader.intValue() + } - override fun decimalValue(): BigDecimal = reader.decimalValue() + override fun decimalValue(): BigDecimal { + prepareValue() + return reader.decimalValue() + } - override fun doubleValue(): Double = reader.doubleValue() + override fun doubleValue(): Double { + prepareValue() + return reader.doubleValue() + } override fun stepIntoContainer() { + // Note: the following line ensures the entire container is buffered. This improves performance when reading the + // container's elements because there is less work to do per element. However, very large containers would + // increase memory usage. The current implementation already assumes this risk by eagerly materializing + // macro invocation arguments. However, if that is changed, then removing the following line should also be + // considered. + prepareValue() reader.stepIntoContainer() } @@ -50,25 +79,49 @@ internal class ReaderAdapterContinuable(val reader: IonReaderContinuableCore) : return annotations } - override fun symbolValue(): SymbolToken = reader.symbolValue() + override fun symbolValue(): SymbolToken { + prepareValue() + return reader.symbolValue() + } - override fun getIntegerSize(): IntegerSize = reader.integerSize + override fun getIntegerSize(): IntegerSize { + prepareValue() + return reader.integerSize + } override fun getFieldNameSymbol(): SymbolToken = reader.fieldNameSymbol override fun isInStruct(): Boolean = reader.isInStruct - override fun newBytes(): ByteArray = reader.newBytes() + override fun newBytes(): ByteArray { + prepareValue() + return reader.newBytes() + } - override fun timestampValue(): Timestamp = reader.timestampValue() + override fun timestampValue(): Timestamp { + prepareValue() + return reader.timestampValue() + } - override fun bigIntegerValue(): BigInteger = reader.bigIntegerValue() + override fun bigIntegerValue(): BigInteger { + prepareValue() + return reader.bigIntegerValue() + } - override fun longValue(): Long = reader.longValue() + override fun longValue(): Long { + prepareValue() + return reader.longValue() + } override fun isNullValue(): Boolean = reader.isNullValue - override fun booleanValue(): Boolean = reader.booleanValue() + override fun booleanValue(): Boolean { + prepareValue() + return reader.booleanValue() + } - override fun integerSize(): IntegerSize? = reader.integerSize + override fun integerSize(): IntegerSize? { + prepareValue() + return reader.integerSize + } } diff --git a/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java b/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java index dca82e1d4..2be25c0e9 100644 --- a/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java +++ b/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java @@ -6004,5 +6004,30 @@ public void addSymbolsSystemMacro(boolean constructFromBytes) throws Exception { } } + @Test + public void addSymbolsSystemMacroWhenNotEntirelyBuffered() throws Exception { + int[] data = new int[] { + /* 0 - 3 */ // 0xEA, 0x01, 0x01, 0xE0, // implicitly provided by BinaryIonAppender + /* 4 - 5 */ 0xEF, 0x0C, // system macro add_symbols + /* 6 - 7 */ 0x02, 0x01, // AEB: 0b------aa; a=10, FlexInt 0, a delimited expression group + /* 8 - 12 */ 0x93, 0x66, 0x6F, 0x72, // "for" + /* 13 - 16 */ 0x93, 0x66, 0x75, 0x72, // "fur" + /* 17 - 21 */ 0x94, 0x66, 0x6F, 0x75, 0x72, // "four" + /* 22 - 22 */ 0xF0, // delimited end... of expression group + /* 23 - 24 */ 0xE1, 0x41 + 3, // SID single byte ${usid 1} => "four" + }; + byte[] bytes = new TestUtils.BinaryIonAppender(1).append(data).toByteArray(); + totalBytesInStream = bytes.length; + readerBuilder = IonReaderBuilder.standard().withIncrementalReadingEnabled(true); + ByteArrayInputStream inputStream = new ByteArrayInputStream(bytes); + reader = boundedReaderFor(inputStream, 16, Integer.MAX_VALUE, byteCountingHandler); + assertSequence( + next(IonType.SYMBOL), + stringValue("four"), + next(null) + ); + closeAndCount(); + } + // TODO Ion 1.1 symbol tables with all kinds of annotation encodings (opcodes E4 - E9, inline and SID) }