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

Fixes Ion 1.1 symbols for opcodes E1-E3 #844

Merged
merged 3 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
end_of_line = lf
insert_final_newline = true
trim_trailing_whitespace=true

[{*.kt,*.kts}]
ij_kotlin_imports_layout = *
ij_kotlin_packages_to_use_import_on_demand=com.amazon.ion.**,java.util.*

Expand Down
63 changes: 61 additions & 2 deletions src/main/java/com/amazon/ion/impl/IonCursorBinary.java
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ private boolean ensureCapacity(long minimumNumberOfBytesRequired) {
/**
* Attempts to fill the buffer so that it contains at least `numberOfBytes` after `index`.
* @param index the index after which to fill.
* @param numberOfBytes the number of bytes after `index` that need to be present.
* @param numberOfBytes the number of bytes starting at `index` that need to be present.
* @return false if not enough bytes were available in the stream to satisfy the request; otherwise, true.
*/
private boolean fillAt(long index, long numberOfBytes) {
Expand Down Expand Up @@ -1050,6 +1050,27 @@ private long uncheckedReadFlexUInt_1_1() {
return uncheckedReadLargeFlexUInt_1_1(currentByte);
}

/**
* Reads the length of a FlexUInt (or FlexInt) at the given position.
* Does not alter the state of the peekIndex or anything else.
* @return the number of bytes used to encode the FlexUInt (or FlexInt) that starts a "position"
*/
private long uncheckedReadLengthOfFlexUInt_1_1(long position) {
int length = 1;
while (true) {
int numZeros = Integer.numberOfTrailingZeros(buffer[(int) position]);
if (numZeros < 8) {
length += numZeros;
return length;
} else {
// We don't actually know the length without looking at even more bytes,
// so look at another.
length += 8;
position++;
}
}
}

/**
* Reads a multi-byte FlexUInt into a long, ensuring enough data is available in the buffer. After this method
* returns, `peekIndex` points to the first byte after the end of the FlexUInt.
Expand Down Expand Up @@ -1089,6 +1110,31 @@ private long slowReadFlexUInt_1_1() {
return slowReadLargeFlexUInt_1_1(currentByte);
}

/**
* Reads the length of a FlexUInt (or FlexInt) at the given position.
* Does not alter the state of the peekIndex. May fill data, if needed.
* @return the number of bytes used to encode the FlexUInt (or FlexInt) that starts a "position"
* or -1 if the end of the stream has been reached
*/
private long slowReadLengthOfFlexUInt_1_1(long position) {
int length = 1;
while (true) {
if (!fillAt(position, 1)) {
return -1;
}
int numZeros = Integer.numberOfTrailingZeros(buffer[(int) position]);
if (numZeros < 8) {
length += numZeros;
return length;
} else {
// We don't actually know the length without looking at even more bytes,
// so add 8 to length, and then look at the next byte.
length += 8;
position++;
}
}
}

/**
* Reads the header of an Ion 1.1 annotation wrapper. This must only be called when it is known that the buffer
* already contains all the bytes in the header. Sets `valueMarker` with the start and end indices of the wrapped
Expand Down Expand Up @@ -1232,7 +1278,14 @@ private long calculateEndIndex_1_1(IonTypeID valueTid, boolean isAnnotated) {
event = Event.START_CONTAINER;
return DELIMITED_MARKER;
}
long endIndex = (valueTid.variableLength ? uncheckedReadFlexUInt_1_1() : valueTid.length) + peekIndex;
long length = valueTid.length;
if (valueTid.variableLength) {
length = uncheckedReadFlexUInt_1_1();
} else if (length < 0) {
// The value is a FlexInt or FlexUInt, so read the continuation bits to determine the length.
length = uncheckedReadLengthOfFlexUInt_1_1(peekIndex);
}
long endIndex = peekIndex + length;
if (valueTid.type != null && valueTid.type.ordinal() >= LIST_TYPE_ORDINAL) {
event = Event.START_CONTAINER;
} else if (valueTid.isNopPad) {
Expand Down Expand Up @@ -2008,6 +2061,12 @@ private boolean slowReadValueHeader(IonTypeID valueTid, boolean isAnnotated, Mar
if (valueLength < 0) {
return true;
}
} else if (valueTid.length < 0 && minorVersion > 0) {
// The value is itself a FlexInt or FlexUInt, so read the continuation bits to determine the length.
valueLength = slowReadLengthOfFlexUInt_1_1(peekIndex);
if (valueLength < 0) {
return true;
}
} else {
valueLength = valueTid.length;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,7 @@ private BigDecimal readTimestampFraction_1_1() {
}
value = new BigDecimal(readLargeFixedIntOrFixedUIntAsBigInteger(length), scale);
} else if (length > 0) {
value = BigDecimal.valueOf(readFixedUInt_1_1(), scale);
value = BigDecimal.valueOf(readFixedUInt_1_1(peekIndex, valueMarker.endIndex), scale);
} else {
value = BigDecimal.valueOf(0, scale);
}
Expand Down Expand Up @@ -946,13 +946,13 @@ private long readUInt(long startIndex, long endIndex) {
}

/**
* Reads a FixedUInt (little-endian), starting at `peekIndex` and ending at `valueMarker.endIndex`.
* Reads a FixedUInt (little-endian), for the range of bytes given by `startInclusive` and `endExclusive`.
* @return the value.
*/
private long readFixedUInt_1_1() {
private long readFixedUInt_1_1(long startInclusive, long endExclusive) {
long result = 0;
for (int i = (int) peekIndex; i < valueMarker.endIndex; i++) {
result |= ((long) (buffer[i] & SINGLE_BYTE_MASK) << ((i - peekIndex) * VALUE_BITS_PER_UINT_BYTE));
for (int i = (int) startInclusive; i < endExclusive; i++) {
result |= ((long) (buffer[i] & SINGLE_BYTE_MASK) << ((i - startInclusive) * VALUE_BITS_PER_UINT_BYTE));
}
return result;
}
Expand Down Expand Up @@ -1306,7 +1306,20 @@ public int symbolValueId() {
return -1;
}
prepareScalar();
return (int) readUInt(valueMarker.startIndex, valueMarker.endIndex);
if (minorVersion == 0) {
return (int) readUInt(valueMarker.startIndex, valueMarker.endIndex);
} else {
if (valueTid.length == 1){
return (int) readFixedUInt_1_1(valueMarker.startIndex, valueMarker.endIndex);
} else if (valueTid.length == 2){
return (int) readFixedUInt_1_1(valueMarker.startIndex, valueMarker.endIndex) + 256;
} else if (valueTid.length == -1) {
peekIndex = valueMarker.startIndex;
return (int) readFlexUInt_1_1() + 65792;
} else {
throw new IllegalStateException("Illegal length " + valueTid.length + " for " + valueMarker);
}
}
}

/**
Expand Down
16 changes: 15 additions & 1 deletion src/test/java/com/amazon/ion/impl/IonCursorTestUtilities.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package com.amazon.ion.impl;

import com.amazon.ion.IonBufferConfiguration;
Expand Down Expand Up @@ -209,6 +208,21 @@ static <T extends IonReaderContinuableApplicationBinary> ExpectationProvider<T>
));
}

/**
* Provides Expectations that verify that advancing the cursor to the next value positions the cursor on a scalar
* with type symbol and the given expected value.
*/
static <T extends IonReaderContinuableCoreBinary> ExpectationProvider<T> fillSymbolValue(int expectedValue) {
return consumer -> consumer.accept(new Expectation<>(
String.format("symbol($%s)", expectedValue),
reader -> {
assertEquals(VALUE_READY, reader.fillValue());
assertEquals(IonType.SYMBOL, reader.getType());
assertEquals(expectedValue, reader.symbolValueId());
}
));
}

/**
* Provides an Expectation that verifies that advancing the cursor positions it on a container value, without
* filling that container.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import static com.amazon.ion.impl.IonCursorTestUtilities.endStream;
import static com.amazon.ion.impl.IonCursorTestUtilities.fillIntValue;
import static com.amazon.ion.impl.IonCursorTestUtilities.scalar;
import static com.amazon.ion.impl.IonCursorTestUtilities.scalar;
import static com.amazon.ion.impl.IonCursorTestUtilities.fillSymbolValue;

public class IonReaderContinuableApplicationBinaryTest {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package com.amazon.ion.impl;

import com.amazon.ion.IonCursor;
import com.amazon.ion.IonException;
import com.amazon.ion.IonType;
import com.amazon.ion.TestUtils;
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;
Expand All @@ -18,29 +19,32 @@
import static com.amazon.ion.impl.IonCursorTestUtilities.ExpectationProvider;
import static com.amazon.ion.impl.IonCursorTestUtilities.assertSequence;
import static com.amazon.ion.impl.IonCursorTestUtilities.container;
import static com.amazon.ion.impl.IonCursorTestUtilities.container;
import static com.amazon.ion.impl.IonCursorTestUtilities.endContainer;
import static com.amazon.ion.impl.IonCursorTestUtilities.endStream;
import static com.amazon.ion.impl.IonCursorTestUtilities.fillContainer;
import static com.amazon.ion.impl.IonCursorTestUtilities.fillIntValue;
import static com.amazon.ion.impl.IonCursorTestUtilities.scalar;
import static com.amazon.ion.impl.IonCursorTestUtilities.fillStringValue;
import static com.amazon.ion.impl.IonCursorTestUtilities.fillSymbolValue;
import static com.amazon.ion.impl.IonCursorTestUtilities.scalar;
import static com.amazon.ion.impl.IonCursorTestUtilities.startContainer;
import static com.amazon.ion.impl.IonCursorTestUtilities.fillStringValue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;

public class IonReaderContinuableCoreBinaryTest {

private IonReaderContinuableCoreBinary initializeReader(boolean constructFromBytes, int... data) {
return initializeReader(constructFromBytes, bytes(data));
}

private IonReaderContinuableCoreBinary initializeReader(boolean constructFromBytes, byte[] data) {
IonReaderContinuableCoreBinary reader;
if (constructFromBytes) {
reader = new IonReaderContinuableCoreBinary(STANDARD_BUFFER_CONFIGURATION, bytes(data), 0, data.length);
reader = new IonReaderContinuableCoreBinary(STANDARD_BUFFER_CONFIGURATION, data, 0, data.length);
} else {
reader = new IonReaderContinuableCoreBinary(
STANDARD_BUFFER_CONFIGURATION,
new ByteArrayInputStream(bytes(data)),
new ByteArrayInputStream(data),
null,
0,
0
Expand Down Expand Up @@ -119,6 +123,40 @@ public void basicStrings(boolean constructFromBytes) {
);
}

@ParameterizedTest(name = "constructFromBytes={0}")
@CsvSource({
"0, E1 00 60",
"1, E1 01 60",
"255, E1 FF 60",
"256, E2 00 00 60",
"257, E2 01 00 60",
"512, E2 00 01 60",
"513, E2 01 01 60",
"65535, E2 FF FE 60",
"65791, E2 FF FF 60",
"65792, E3 01 60",
"65793, E3 03 60",
"65919, E3 FF 60",
"65920, E3 02 02 60",
"2147483647 , E3 F0 DF DF FF 0F 60",
})
public void sidSymbols_1_1(int sid, String bytes) {
sidSymbols_1_1_helper(sid, bytes, true);
sidSymbols_1_1_helper(sid, bytes, false);
}
void sidSymbols_1_1_helper(int sid, String bytes, boolean constructFromBytes) {
IonReaderContinuableCoreBinary reader = initializeReader(
constructFromBytes,
TestUtils.hexStringToByteArray("E0 01 01 EA " + bytes)
);
assertSequence(
reader,
scalar(), fillSymbolValue(sid),
scalar(), fillIntValue(0),
endStream()
);
}


@ParameterizedTest(name = "constructFromBytes={0}")
@ValueSource(booleans = {true, false})
Expand Down
18 changes: 2 additions & 16 deletions src/test/java/com/amazon/ion/streaming/SeekableReaderTest.java
Original file line number Diff line number Diff line change
@@ -1,25 +1,11 @@
/*
* Copyright 2007-2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

//·Copyright·Amazon.com,·Inc.·or·its·affiliates.·All·Rights·Reserved.
//·SPDX-License-Identifier:·Apache-2.0
package com.amazon.ion.streaming;

import com.amazon.ion.IonDatagram;
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;
Expand Down
Loading