Skip to content

Commit

Permalink
Works on feedback, adds comments and improves tests.
Browse files Browse the repository at this point in the history
  • Loading branch information
@cheqianh
cheqianh committed Oct 22, 2021
1 parent 8fff77f commit 153bd9f
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 50 deletions.
8 changes: 8 additions & 0 deletions serde/src/main/java/com/amazon/ionhiveserde/IonHiveSerDe.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,14 @@ public Object deserialize(final Writable blob) throws SerDeException {

final IonSystem domFactory = ionFactory.getDomFactory();
try (final IonReader reader = ionFactory.newReader(bytes, 0, length)) {

/*
We're using an IonStruct here because:
1. We need a key-value store to carry column values
2. The top-level IonStruct as a context object carries the IonSystem which we use as a ValueFactory in
the callbacks created in PathExtractionConfig
Refer to https://github.com/amzn/ion-hive-serde/issues/61.
*/
IonStruct struct = domFactory.newEmptyStruct();
if (!serDeProperties.pathExtractorCaseSensitivity()) {
struct = new IonStructCaseInsensitiveDecorator(struct);
Expand Down
45 changes: 18 additions & 27 deletions ...com/amazon/ionhiveserde/caseinsensitivedecorator/IonSequenceCaseInsensitiveDecorator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@
import com.amazon.ion.UnknownSymbolException;
import com.amazon.ion.ValueFactory;

import java.util.ArrayList;
import java.lang.reflect.Array;
import java.util.Collection;
import java.util.List;
import java.util.ListIterator;
import java.util.stream.Collectors;

import org.jetbrains.annotations.NotNull;


Expand Down Expand Up @@ -135,48 +137,37 @@ public ListIterator<IonValue> listIterator(final int index) {
@NotNull
@Override
public List<IonValue> subList(final int fromIndex, final int toIndex) {
List<IonValue> l = new ArrayList<>();
List<IonValue> sublist = ionSequence.subList(fromIndex, toIndex);

for (IonValue ionValue : sublist) {
l.add(IonCaseInsensitiveDecorator.wrapValue(ionValue));
}
return l;
return ionSequence.subList(fromIndex, toIndex).stream()
.map(IonCaseInsensitiveDecorator::wrapValue)
.collect(Collectors.toList());
}

@NotNull
@Override
public IonValue[] toArray() {
IonValue[] rawArray = ionSequence.toArray();
int size = ionSequence.size();
for (int i = 0; i < size; i++) {
rawArray[i] = IonCaseInsensitiveDecorator.wrapValue(rawArray[i]);
}
return rawArray;
return ionSequence.stream()
.map(IonCaseInsensitiveDecorator::wrapValue)
.toArray(IonValue[]::new);
}

@NotNull
@Override
@SuppressWarnings("unchecked")
public <T> T[] toArray(final T[] a) {
T[] rawArray = ionSequence.toArray(a);
int size = rawArray.length;
for (int i = 0; i < size; i++) {
rawArray[i] = (T) IonCaseInsensitiveDecorator.wrapValue((IonValue) rawArray[i]);
}
return rawArray;
Class<?> type = a.getClass().getComponentType();
return ionSequence.stream()
.map(IonCaseInsensitiveDecorator::wrapValue)
.map(i -> (T)i)
.toArray(size -> (T[]) Array.newInstance(type, size));
}


@Override
@SuppressWarnings("unchecked")
public <T extends IonValue> T[] extract(final Class<T> type) {
T[] rawArray = ionSequence.extract(type);
int size = rawArray.length;
for (int i = 0; i < size; i++) {
rawArray[i] = (T) IonCaseInsensitiveDecorator.wrapValue((IonValue) rawArray[i]);
}
return rawArray;
return ionSequence.stream()
.map(IonCaseInsensitiveDecorator::wrapValue)
.map(i -> (T)i)
.toArray(size -> (T[]) Array.newInstance(type, size));
}

public IonSequence clone() throws UnknownSymbolException {
Expand Down
6 changes: 6 additions & 0 deletions ...a/com/amazon/ionhiveserde/caseinsensitivedecorator/IonStructCaseInsensitiveDecorator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ public ValueFactory add(final String fieldName) {
}

@Override
/*
Argument `fieldName` is case sensitive so below method removes the desired value, which means it's possible that
struct.containsKey(fieldName) returns true while struct.remove(fieldName) does nothing.
Refer to https://github.com/amzn/ion-hive-serde/issues/60.
*/
public IonValue remove(final String fieldName) {
return IonCaseInsensitiveDecorator.wrapValue(ionStruct.remove(fieldName));
}
Expand Down
51 changes: 28 additions & 23 deletions serde/src/test/kotlin/com/amazon/ionhiveserde/configuration/CaseInsensitiveDecoratorTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,27 +68,30 @@ class CaseInsensitiveDecoratorTest {
}

@Test
fun ionStructCaseInsensitiveDecoratorGetRepeatedField() {
val struct = struct_for(" { Foo: 'bar', foo: 'Bar' }")
assertEquals(struct.containsKey("FOO"), true)
assertEquals(struct.get("Foo"), ION.newSymbol("bar"))
assertEquals(struct.get("foo"), ION.newSymbol("Bar"))
fun ionStructCaseInsensitiveDecoratorGetRepeatedFieldFound() {
val struct = struct_for(" { Foo: 'Bar', foo: 'bar' }")
assertEquals(struct.get("Foo"), ION.newSymbol("Bar"))
assertEquals(struct.get("foo"), ION.newSymbol("bar"))
}

@Test
fun ionStructCaseInsensitiveDecoratorGetRepeatedFieldFoundIgnoringCase() {
val struct = struct_for(" { Foo: 'Bar', foo: 'bar' }")
assert(
struct.get("FOO") == ION.newSymbol("Bar") ||
struct.get("FOO") == ION.newSymbol("bar")
struct.get("FOO") == ION.newSymbol("Bar") || struct.get("FOO") == ION.newSymbol("bar")
)
}

@Test
fun ionStructCaseInsensitiveDecoratorGetStruct() {
val struct = struct_for(" { Foo: {} }")
assertTrue(struct.get("Foo") is IonStructCaseInsensitiveDecorator)
assertTrue(struct.get("Foo") is IonStructCaseInsensitiveDecorator, "Found ${struct.get("Foo").javaClass.simpleName} ,expected IonStructCaseInsensitiveDecorator type.")
}

@Test
fun ionStructCaseInsensitiveDecoratorGetSequence() {
val struct = struct_for(" { Foo: [] }")
assertTrue(struct.get("Foo") is IonSequenceCaseInsensitiveDecorator)
assertTrue(struct.get("Foo") is IonSequenceCaseInsensitiveDecorator, "Found ${struct.get("Foo").javaClass.simpleName} ,expected IonSequenceCaseInsensitiveDecorator type.")
}

@Test
Expand All @@ -104,14 +107,14 @@ class CaseInsensitiveDecoratorTest {
fun ionStructCaseInsensitiveDecoratorRemoveStruct() {
val struct = struct_for(" { Foo: {} }")
val s = struct.remove("Foo")
assertTrue(s is IonStructCaseInsensitiveDecorator)
assertTrue(s is IonStructCaseInsensitiveDecorator, "Found ${s.javaClass.simpleName} ,expected IonStructCaseInsensitiveDecorator type.")
}

@Test
fun ionStructCaseInsensitiveDecoratorRemoveSequence() {
val struct = struct_for(" { Foo: [] }")
val s = struct.remove("Foo")
assertTrue(s is IonSequenceCaseInsensitiveDecorator)
assertTrue(s is IonSequenceCaseInsensitiveDecorator, "Found ${s.javaClass.simpleName} ,expected IonSequenceCaseInsensitiveDecorator type.")
}

@Test
Expand All @@ -120,7 +123,7 @@ class CaseInsensitiveDecoratorTest {
val s = struct.cloneAndRemove("Foo")

assertEquals(s.size(), 0)
assertTrue(s is IonStructCaseInsensitiveDecorator)
assertTrue(s is IonStructCaseInsensitiveDecorator, "Found ${s.javaClass.simpleName} ,expected IonStructCaseInsensitiveDecorator type.")
}

@Test
Expand All @@ -130,7 +133,7 @@ class CaseInsensitiveDecoratorTest {

assertEquals(s.size(), 1)
assertEquals(s.containsKey("foo"), true)
assertTrue(s is IonStructCaseInsensitiveDecorator)
assertTrue(s is IonStructCaseInsensitiveDecorator, "Found ${s.javaClass.simpleName} ,expected IonStructCaseInsensitiveDecorator type.")
}

@Test
Expand All @@ -142,13 +145,13 @@ class CaseInsensitiveDecoratorTest {
@Test
fun ionSequenceCaseInsensitiveDecoratorGetStruct() {
val sequence = sequence_for("[{}]")
assertTrue(sequence[0] is IonStructCaseInsensitiveDecorator)
assertTrue(sequence[0] is IonStructCaseInsensitiveDecorator, "Found ${sequence[0].javaClass.simpleName} ,expected IonStructCaseInsensitiveDecorator type.")
}

@Test
fun ionSequenceCaseInsensitiveDecoratorGetSequence() {
val sequence = sequence_for("[[]]")
assertTrue(sequence[0] is IonSequenceCaseInsensitiveDecorator)
assertTrue(sequence[0] is IonSequenceCaseInsensitiveDecorator, "Found ${sequence[0].javaClass.simpleName} ,expected IonSequenceCaseInsensitiveDecorator type.")
}

@Test
Expand All @@ -164,14 +167,14 @@ class CaseInsensitiveDecoratorTest {
fun ionSequenceCaseInsensitiveDecoratorSetStruct() {
val sequence = sequence_for("[{}]")
val l = sequence.set(0, ION.newInt(2))
assertTrue(l is IonStructCaseInsensitiveDecorator)
assertTrue(l is IonStructCaseInsensitiveDecorator, "Found ${l.javaClass.simpleName} ,expected IonStructCaseInsensitiveDecorator type.")
}

@Test
fun ionSequenceCaseInsensitiveDecoratorSetSequence() {
val sequence = sequence_for("[[]]")
val l = sequence.set(0, ION.newInt(2))
assertTrue(l is IonSequenceCaseInsensitiveDecorator)
assertTrue(l is IonSequenceCaseInsensitiveDecorator, "Found ${l.javaClass.simpleName} ,expected IonSequenceCaseInsensitiveDecorator type.")
}

@Test
Expand All @@ -188,24 +191,26 @@ class CaseInsensitiveDecoratorTest {
val sequence = sequence_for("[{}]")
val l = sequence.removeAt(0)

assertTrue(l is IonStructCaseInsensitiveDecorator)
assertTrue(l is IonStructCaseInsensitiveDecorator, "Found ${l.javaClass.simpleName} ,expected IonStructCaseInsensitiveDecorator type.")
}

@Test
fun ionSequenceCaseInsensitiveDecoratorRemoveSequence() {
val sequence = sequence_for("[[]]")
val l = sequence.removeAt(0)

assertTrue(l is IonSequenceCaseInsensitiveDecorator)
assertTrue(l is IonSequenceCaseInsensitiveDecorator, "Found ${l.javaClass.simpleName} ,expected IonSequenceCaseInsensitiveDecorator type.")
}

@Test
fun ionSequenceCaseInsensitiveDecoratorListIterator() {
val sequence = sequence_for("[1, [], {}]")
val iter = sequence.listIterator()
assertEquals(iter.next(), ION.newInt(1))
assertTrue(iter.next() is IonSequenceCaseInsensitiveDecorator)
assertTrue(iter.next() is IonStructCaseInsensitiveDecorator)
val i = iter.next()
assertTrue(i is IonSequenceCaseInsensitiveDecorator, "Found ${i.javaClass.simpleName} ,expected IonSequenceCaseInsensitiveDecorator type.")
val ii = iter.next()
assertTrue(ii is IonStructCaseInsensitiveDecorator, "Found ${ii.javaClass.simpleName} ,expected IonStructCaseInsensitiveDecorator type.")
}

@Test
Expand All @@ -214,7 +219,7 @@ class CaseInsensitiveDecoratorTest {
val sublist = sequence.subList(0, 3)
// sublist: [1, [], {}]
assertEquals(sublist[0], ION.newInt(1))
assertTrue(sublist[1] is IonSequenceCaseInsensitiveDecorator)
assertTrue(sublist[2] is IonStructCaseInsensitiveDecorator)
assertTrue(sublist[1] is IonSequenceCaseInsensitiveDecorator, "Found ${sublist[1].javaClass.simpleName} ,expected IonSequenceCaseInsensitiveDecorator type.")
assertTrue(sublist[2] is IonStructCaseInsensitiveDecorator, "Found ${sublist[2].javaClass.simpleName} ,expected IonStructCaseInsensitiveDecorator type.")
}
}

0 comments on commit 153bd9f

Please sign in to comment.