Fixes #963 by properly setting the cursor's checkpoint after reading …

…tagged values in an expression group.
amazon-ion · Oct 11, 2024 · 71f9e37 · 71f9e37
1 parent 344cf95
commit 71f9e37
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 4 deletions.
diff --git a/src/main/java/com/amazon/ion/impl/IonCursorBinary.java b/src/main/java/com/amazon/ion/impl/IonCursorBinary.java
@@ -3349,6 +3349,23 @@ private boolean fillArgumentGroupPage(ArgumentGroupMarker group) {
         return false;
     }
 
+    /**
+     * Sets the checkpoint based on whether a scalar or container header has just been read. It is up to the caller
+     * to ensure that the cursor is positioned immediately after a value header.
+     */
+    private void setCheckpointAfterValueHeader() {
+        switch (event) {
+            case START_SCALAR:
+                setCheckpoint(CheckpointLocation.AFTER_SCALAR_HEADER);
+                break;
+            case START_CONTAINER:
+                setCheckpoint(CheckpointLocation.AFTER_CONTAINER_HEADER);
+                break;
+            default:
+                throw new IllegalStateException();
+        }
+    }
+
     /**
      * Positions the cursor on the next value in the tagged group. Upon return, the value will be filled and
      * `valueMarker` set to the value's start and end indices.
@@ -3381,6 +3398,7 @@ private Event nextGroupedTaggedValue(ArgumentGroupMarker group) {
                     return event;
                 }
                 isUserValue = uncheckedReadHeader(b, false, valueMarker);
+                setCheckpointAfterValueHeader();
             }
         } else {
             if (peekIndex == group.pageEndIndex) {
@@ -3392,6 +3410,7 @@ private Event nextGroupedTaggedValue(ArgumentGroupMarker group) {
                 return event;
             }
             isUserValue = uncheckedReadHeader(buffer[(int)(peekIndex++)] & SINGLE_BYTE_MASK, false, valueMarker);
+            setCheckpointAfterValueHeader();
         }
         valueTid = valueMarker.typeId;
         if (!isUserValue) {

diff --git a/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java b/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java
@@ -5968,13 +5968,12 @@ public void invokeValuesUsingSystemMacroOpcode(String bytes) throws Exception {
     })
     public void invokeValuesWithExpressionGroupsUsingSystemMacroOpcode(String bytes) throws Exception {
         invokeValuesUsingSystemMacroOpcodeHelper(true, bytes);
-        // TODO: Determine why the checkpoint in the cursor isn't being set correctly before calling slowFillValue()
-        //       specifically while reading from a length-prefixed expression group
-        //       See https://github.com/amazon-ion/ion-java/issues/963
-        // invokeValuesUsingSystemMacroOpcodeHelper(false, bytes);
+        invokeValuesUsingSystemMacroOpcodeHelper(false, bytes);
     }
 
     private void invokeValuesUsingSystemMacroOpcodeHelper(boolean constructFromBytes, String bytes) throws Exception {
+        // Reset the byte counter between runs since this test utility method is called multiple times per test.
+        byteCounter.set(0);
         byte[] input = withIvm(1, hexStringToByteArray(cleanCommentedHexBytes(bytes)));
         readerBuilder = readerBuilder.withIncrementalReadingEnabled(false);
         reader = readerFor(readerBuilder, constructFromBytes, input);
@@ -5983,6 +5982,7 @@ private void invokeValuesUsingSystemMacroOpcodeHelper(boolean constructFromBytes
             next(IonType.INT), intValue(1),
             next(null)
         );
+        closeAndCount();
     }
 
     // TODO Ion 1.1 symbol tables with all kinds of annotation encodings (opcodes E4 - E9, inline and SID)