Add Type Check to Strict Extractor (#34)

* Add Type Check to Strict Extractor Search paths on field names, indexed elements or wildcards imply an expectation for container types. Before this change, a search path looking for a field would simply not be invoked if the data was not a struct. With this change, users can elect to have an Exception thrown when data does not meet the implied type expectations of their paths. The type check is performed after any callback for the parent itself: that is not known to be mistyped by the paths. It is only implemented for the "strict" matcher and the change to the API reflects that. * Add Test for null.struct
amazon-ion · Jan 8, 2025 · df9a213 · df9a213
1 parent f78144f
commit df9a213
Show file tree

Hide file tree

Showing 7 changed files with 111 additions and 10 deletions.
diff --git a/src/main/java/com/amazon/ionpathextraction/FsmMatcher.java b/src/main/java/com/amazon/ionpathextraction/FsmMatcher.java
@@ -27,11 +27,32 @@ abstract class FsmMatcher<T> {
      */
     BiFunction<IonReader, T, Integer> callback;
 
+    enum Transitionable {
+        TERMINAL(false, false),
+        POSSIBLE(false, true),
+        MISTYPED(true, false);
+
+        final boolean invalid;
+        final boolean possible;
+
+        Transitionable(final boolean invalid, final boolean possible) {
+            this.invalid = invalid;
+            this.possible = possible;
+        }
+    }
+
     /**
-     * Indicates if there _may_ be transitions to child matchers from the given IonType.
+     * Indicates if there _may_ be transitions to child matchers from the given IonType,
+     * or if the given IonType is mistyped for the expected transitions.
      */
-    boolean transitionsFrom(final IonType ionType) {
-        return IonType.isContainer(ionType);
+    Transitionable transitionsFrom(final IonType ionType) {
+        if (IonType.isContainer(ionType)) {
+            return Transitionable.POSSIBLE;
+        }
+        if (ionType == IonType.NULL) {
+            return Transitionable.TERMINAL;
+        }
+        return Transitionable.MISTYPED;
     }
 
     /**

diff --git a/src/main/java/com/amazon/ionpathextraction/FsmMatcherBuilder.java b/src/main/java/com/amazon/ionpathextraction/FsmMatcherBuilder.java
@@ -214,8 +214,14 @@ private static class FieldMatcher<T> extends FsmMatcher<T> {
         }
 
         @Override
-        boolean transitionsFrom(final IonType ionType) {
-            return ionType == IonType.STRUCT;
+        Transitionable transitionsFrom(final IonType ionType) {
+            if (ionType == IonType.STRUCT) {
+                return Transitionable.POSSIBLE;
+            }
+            if (ionType == IonType.NULL) {
+                return Transitionable.TERMINAL;
+            }
+            return Transitionable.MISTYPED;
         }
 
         @Override
@@ -259,8 +265,8 @@ private static class TerminalMatcher<T> extends FsmMatcher<T> {
         }
 
         @Override
-        boolean transitionsFrom(final IonType ionType) {
-            return false;
+        Transitionable transitionsFrom(final IonType ionType) {
+            return Transitionable.TERMINAL;
         }
 
         @Override

diff --git a/src/main/java/com/amazon/ionpathextraction/FsmPathExtractor.java b/src/main/java/com/amazon/ionpathextraction/FsmPathExtractor.java
@@ -18,6 +18,7 @@
 
 import com.amazon.ion.IonReader;
 import com.amazon.ion.IonType;
+import com.amazon.ionpathextraction.exceptions.PathExtractionException;
 import com.amazon.ionpathextraction.internal.PathExtractorConfig;
 import java.util.List;
 import java.util.function.BiFunction;
@@ -35,17 +36,21 @@
  */
 class FsmPathExtractor<T> implements PathExtractor<T> {
     private final FsmMatcher<T> rootMatcher;
+    private final boolean strictTyping;
     private final PathExtractorConfig config;
 
     private FsmPathExtractor(
             final FsmMatcher<T> rootMatcher,
+            final boolean strictTyping,
             final PathExtractorConfig config) {
         this.rootMatcher = rootMatcher;
+        this.strictTyping = strictTyping;
         this.config = config;
     }
 
     static <U> FsmPathExtractor<U> create(
             final List<SearchPath<U>> searchPaths,
+            final boolean strictTyping,
             final PathExtractorConfig config) {
         FsmMatcherBuilder<U> builder = new FsmMatcherBuilder<>(
                 config.isMatchCaseInsensitive(),
@@ -54,7 +59,7 @@ static <U> FsmPathExtractor<U> create(
             builder.accept(path);
         }
 
-        return new FsmPathExtractor<>(builder.build(), config);
+        return new FsmPathExtractor<>(builder.build(), strictTyping, config);
     }
 
     @Override
@@ -105,7 +110,8 @@ private int matchRecursive(
             }
         }
 
-        if (child.transitionsFrom(reader.getType())) {
+        FsmMatcher.Transitionable transitionable = child.transitionsFrom(reader.getType());
+        if (transitionable.possible) {
             reader.stepIn();
             int childPos = 0;
             while (reader.next() != null) {
@@ -116,6 +122,12 @@ private int matchRecursive(
                 }
             }
             reader.stepOut();
+            return 0;
+        }
+
+        if (transitionable.invalid && strictTyping) {
+            throw new PathExtractionException(
+                    String.format("IonType %s is not valid for transitions from %s", reader.getType(), child));
         }
 
         return 0;

diff --git a/src/main/java/com/amazon/ionpathextraction/PathExtractorBuilder.java b/src/main/java/com/amazon/ionpathextraction/PathExtractorBuilder.java
@@ -99,7 +99,22 @@ public PathExtractor<T> build() {
      * @throws UnsupportedPathExpression if any search path or the paths combined, are not supported.
      */
     public PathExtractor<T> buildStrict() {
+        return buildStrict(false);
+    }
+
+    /**
+     * Instantiate a "strict" path extractor, which also enforces type expectations.
+     * <br>
+     * Paths that attempt to find named children are only valid on Structs or untyped null.
+     * Paths that attempt to find indexed (or wildcard) children are only valid on containers or untyped null.
+     * For backwards compatibility that includes Structs, though they are defined as unordered per the Ion Datamodel.
+     * <br>
+     * The type check is performed _after_ any callbacks registered for the current path and
+     * _before_ any child matches are attempted.
+     */
+    public PathExtractor<T> buildStrict(final boolean strictTyping) {
         return FsmPathExtractor.create(searchPaths,
+                strictTyping,
                 new PathExtractorConfig(matchRelativePaths, matchCaseInsensitive, matchFieldsCaseInsensitive));
     }
 

diff --git a/src/test/kotlin/com/amazon/ionpathextraction/FsmPathExtractorTest.kt b/src/test/kotlin/com/amazon/ionpathextraction/FsmPathExtractorTest.kt
@@ -1,5 +1,9 @@
 package com.amazon.ionpathextraction
 
+import com.amazon.ion.IonReader
+import com.amazon.ionpathextraction.exceptions.PathExtractionException
+import org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.assertThrows
 import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.MethodSource
@@ -30,4 +34,41 @@ class FsmPathExtractorTest : PathExtractorTest() {
             super.testSearchPaths(testCase)
         }
     }
+
+    data class TypingTestCase(val searchPath: String, val validity: List<Boolean>, val matchCount: Int)
+
+    @Test
+    fun testStrictTyping() {
+        val inputs = listOf("17", "[31]", "(53)", "null", "{ foo: 67 }")
+        val testCases = listOf(             //          17,  [31],  (53),  null, { foo: 67 }
+            TypingTestCase("()",     listOf(true,  true,  true,  true, true), 5),
+            TypingTestCase("A::()",  listOf(true,  true,  true,  true, true), 0),
+            TypingTestCase("(*)",    listOf(false, true,  true,  true, true), 3),
+            TypingTestCase("(A::*)", listOf(false, true,  true,  true, true), 0),
+            TypingTestCase("(0)",    listOf(false, true,  true,  true, true), 3),
+            TypingTestCase("(foo)",  listOf(false, false, false, true, true), 1))
+
+        testCases.forEach { testCase ->
+            var count = 0;
+            val counter = { _: IonReader ->
+                count += 1
+                0
+            }
+            val extractor = PathExtractorBuilder.standard<Any>()
+                .withSearchPath(testCase.searchPath, counter)
+                .buildStrict(true)
+
+            for (j in inputs.indices) {
+                val ionReader = ION.newReader(inputs[j])
+                if (testCase.validity[j]) {
+                    extractor.match(ionReader)
+                } else {
+                    assertThrows<PathExtractionException> {
+                        extractor.match(ionReader)
+                    }
+                }
+            }
+            assertEquals(testCase.matchCount, count)
+        }
+    }
 }
diff --git a/src/test/kotlin/com/amazon/ionpathextraction/PathExtractorTest.kt b/src/test/kotlin/com/amazon/ionpathextraction/PathExtractorTest.kt
@@ -33,7 +33,7 @@ import kotlin.test.assertTrue
 
 abstract class PathExtractorTest {
     companion object {
-        private val ION = IonSystemBuilder.standard().build()
+        val ION: IonSystem = IonSystemBuilder.standard().build()
 
         data class TestCase(val searchPaths: List<String>,
                             val data: String,

diff --git a/src/test/resources/test-cases.ion b/src/test/resources/test-cases.ion
@@ -319,4 +319,10 @@ legacy::{
   data: $datagram::[[17]],
   expected: [],
   caseInsensitive: Fields
+}
+{
+  searchPath: (foo),
+  data: $datagram::[null.struct],
+  expected: [],
+  caseInsensitive: Fields
 }