Repository: amzn/ion-java
Pull request #181 (2018-12-10 19:00:44)

IonSequenceLite#retainAll javadoc fix and tests
*Issue #, if available:*
https://github.com/amzn/ion-java/issues/179
https://github.com/amzn/ion-java/issues/49

*Description of changes:*
Adds unit tests for all `IonSequenceLite` implementations and fixes `IonDatagram` javadoc. The method remove from `IonDatagram` interface is inherited from `IonSequence` and works as described in its javadoc so no need to have it in `IonDatagram` as well 

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/src/software/amazon/ion/IonDatagram.java b/src/software/amazon/ion/IonDatagram.java
index 747390f4..fad4c6ce 100644
--- a/src/software/amazon/ion/IonDatagram.java
+++ b/src/software/amazon/ion/IonDatagram.java
@@ -238,19 +238,6 @@ public interface IonDatagram
      */
     public void makeNull();
 
-
-    /**
-     * This inherited method is not yet supported by datagrams.
-     * <p>
-     * Vote for SIM issue amzn/ion-java#49 if you need this.
-     *
-     * @throws UnsupportedOperationException at every call.
-     *
-     * @see <a href="https://github.com/amzn/ion-java/issues/49">amzn/ion-java#49</a>
-     */
-    public boolean retainAll(Collection<?> c);
-
-
     public IonDatagram clone()
         throws UnknownSymbolException;
 }
diff --git a/src/software/amazon/ion/impl/IonReaderBinarySystemX.java b/src/software/amazon/ion/impl/IonReaderBinarySystemX.java
index 27b9900d..f28361e7 100644
--- a/src/software/amazon/ion/impl/IonReaderBinarySystemX.java
+++ b/src/software/amazon/ion/impl/IonReaderBinarySystemX.java
@@ -180,14 +180,19 @@ class IonReaderBinarySystemX
             _v.setAuthoritativeType(AS_TYPE.boolean_value);
             break;
         case INT:
+            boolean is_negative = _value_tid == PrivateIonConstants.tidNegInt;
+
             if (_value_len == 0) {
+                if (is_negative) {
+                    throwIllegalNegativeZeroException();
+                }
+
                 int v = 0;
                 _v.setValue(v);
                 _v.setAuthoritativeType(AS_TYPE.int_value);
             }
             else if (_value_len <= Long.BYTES) {
                 long v = readULong(_value_len);
-                boolean is_negative = _value_tid == PrivateIonConstants.tidNegInt;
 
                 if (v < 0) {
                     // we probably can't fit this magnitude properly into a Java long
@@ -205,6 +210,9 @@ class IonReaderBinarySystemX
                 }
                 else {
                     if (is_negative) {
+                        if(v == 0) {
+                            throwIllegalNegativeZeroException();
+                        }
                         v = -v;
                     }
                     if (v < Integer.MIN_VALUE || v > Integer.MAX_VALUE) {
@@ -217,7 +225,6 @@ class IonReaderBinarySystemX
                 }
             }
             else {
-                boolean is_negative = (_value_tid == PrivateIonConstants.tidNegInt);
                 BigInteger v = readBigInteger(_value_len, is_negative);
                 _v.setValue(v);
                 _v.setAuthoritativeType(AS_TYPE.bigInteger_value);
@@ -482,4 +489,8 @@ class IonReaderBinarySystemX
     {
         return null;
     }
+
+    private void throwIllegalNegativeZeroException() {
+        throw newErrorAt("negative zero is illegal in the binary format");
+    }
 }


Test patch:
diff --git a/test/software/amazon/ion/TestUtils.java b/test/software/amazon/ion/TestUtils.java
index ba7cad75..f76b96d7 100644
--- a/test/software/amazon/ion/TestUtils.java
+++ b/test/software/amazon/ion/TestUtils.java
@@ -144,8 +144,6 @@ public class TestUtils
             , "good/utf16.ion"                              // TODO amzn/ion-java#61
             , "good/utf32.ion"                              // TODO amzn/ion-java#61
             , "good/whitespace.ion"                         // TODO amzn/ion-java#104
-            , "bad/negativeIntZero.10n"                     // TODO amzn/ion-java#138
-            , "bad/negativeIntZeroLn.10n"                   // TODO amzn/ion-java#138
             , "good/item1.10n"                              // TODO amzn/ion-java#126 (roundtrip symbols with unknown text)
         );
 
diff --git a/test/software/amazon/ion/impl/lite/BaseIonSequenceLiteTest.java b/test/software/amazon/ion/impl/lite/BaseIonSequenceLiteTest.java
new file mode 100644
index 00000000..e0614b3e
--- /dev/null
+++ b/test/software/amazon/ion/impl/lite/BaseIonSequenceLiteTest.java
@@ -0,0 +1,67 @@
+package software.amazon.ion.impl.lite;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertTrue;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import org.junit.Test;
+import software.amazon.ion.IonSequence;
+import software.amazon.ion.IonSystem;
+import software.amazon.ion.IonValue;
+import software.amazon.ion.ReadOnlyValueException;
+import software.amazon.ion.system.IonSystemBuilder;
+
+public abstract class BaseIonSequenceLiteTest {
+
+    protected static final IonSystem SYSTEM = IonSystemBuilder.standard().build();
+
+    protected abstract IonSequence newEmptySequence();
+
+    @Test
+    public void retainAll() {
+        final IonSequence sequence = newEmptySequence();
+        final ArrayList<IonValue> toRetain = new ArrayList<IonValue>();
+
+        final IonValue retainedValue = SYSTEM.newInt(1);
+        sequence.add(retainedValue);
+        toRetain.add(retainedValue);
+
+        final IonValue toRemoveValue = SYSTEM.newInt(2);
+        sequence.add(toRemoveValue);
+
+        assertTrue(sequence.retainAll(toRetain));
+
+        assertEquals(1, sequence.size());
+        assertTrue(sequence.contains(retainedValue));
+        assertFalse(sequence.contains(toRemoveValue));
+    }
+
+    @Test
+    public void retainAllUsesReferenceEquality() {
+        final IonSequence sequence = newEmptySequence();
+        final ArrayList<IonValue> toRetain = new ArrayList<IonValue>();
+
+        final IonValue value = SYSTEM.newInt(1);
+        sequence.add(value);
+
+        final IonValue equalValue = SYSTEM.newInt(1);
+        toRetain.add(equalValue);
+
+        assertEquals(equalValue, value);
+        assertNotSame(equalValue, value);
+
+        assertTrue(sequence.retainAll(toRetain));
+        assertEquals(0, sequence.size());
+    }
+
+    @Test(expected = ReadOnlyValueException.class)
+    public void retainAllReadOnly() {
+        final IonSequence sequence = newEmptySequence();
+        sequence.makeReadOnly();
+
+        sequence.retainAll(Collections.emptyList());
+    }
+}
\ No newline at end of file
diff --git a/test/software/amazon/ion/impl/lite/IonDatagramLiteTest.java b/test/software/amazon/ion/impl/lite/IonDatagramLiteTest.java
new file mode 100644
index 00000000..45af7a15
--- /dev/null
+++ b/test/software/amazon/ion/impl/lite/IonDatagramLiteTest.java
@@ -0,0 +1,10 @@
+package software.amazon.ion.impl.lite;
+
+import software.amazon.ion.IonSequence;
+
+public class IonDatagramLiteTest extends BaseIonSequenceLiteTest {
+    @Override
+    protected IonSequence newEmptySequence() {
+        return SYSTEM.newDatagram();
+    }
+}
diff --git a/test/software/amazon/ion/impl/lite/IonListLiteTest.java b/test/software/amazon/ion/impl/lite/IonListLiteTest.java
new file mode 100644
index 00000000..ee5b48c9
--- /dev/null
+++ b/test/software/amazon/ion/impl/lite/IonListLiteTest.java
@@ -0,0 +1,10 @@
+package software.amazon.ion.impl.lite;
+
+import software.amazon.ion.IonSequence;
+
+public class IonListLiteTest extends BaseIonSequenceLiteTest {
+    @Override
+    protected IonSequence newEmptySequence() {
+        return SYSTEM.newEmptyList();
+    }
+}
diff --git a/test/software/amazon/ion/impl/lite/IonSexpLiteTest.java b/test/software/amazon/ion/impl/lite/IonSexpLiteTest.java
new file mode 100644
index 00000000..56528bcd
--- /dev/null
+++ b/test/software/amazon/ion/impl/lite/IonSexpLiteTest.java
@@ -0,0 +1,10 @@
+package software.amazon.ion.impl.lite;
+
+import software.amazon.ion.IonSequence;
+
+public class IonSexpLiteTest extends BaseIonSequenceLiteTest {
+    @Override
+    protected IonSequence newEmptySequence() {
+        return SYSTEM.newEmptySexp();
+    }
+}