Addressed review comments

Signed-off-by: Krishna Kondaka <[email protected]>
opensearch-project · Aug 14, 2024 · 3b3603a · 3b3603a
1 parent b1561dd
commit 3b3603a
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 2 deletions.
diff --git a/data-prepper-expression/src/main/antlr/DataPrepperExpression.g4 b/data-prepper-expression/src/main/antlr/DataPrepperExpression.g4
@@ -122,7 +122,11 @@ regexPattern
     ;
 
 setInitializer
-    : LBRACE primary (SET_DELIMITER primary)* RBRACE
+    : LBRACE setMembers RBRACE
+    ;
+
+setMembers
+    : literal (SPACE* SET_DELIMITER SPACE* literal)*
     ;
 
 unaryOperator

diff --git a/...ression/src/main/java/org/opensearch/dataprepper/expression/ParseTreeCoercionService.java b/...ression/src/main/java/org/opensearch/dataprepper/expression/ParseTreeCoercionService.java
@@ -12,6 +12,9 @@
 import javax.inject.Inject;
 import javax.inject.Named;
 import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
 import java.util.Map;
 import java.util.List;
 import java.util.ArrayList;
@@ -80,6 +83,9 @@ public Object coercePrimaryTerminalNode(final TerminalNode node, final Event eve
                 return Float.valueOf(nodeStringValue);
             case DataPrepperExpressionParser.Boolean:
                 return Boolean.valueOf(nodeStringValue);
+            case DataPrepperExpressionParser.COMMA:
+            case DataPrepperExpressionParser.SET_DELIMITER:
+                return nodeType;
             case DataPrepperExpressionParser.Null:
                 return null;
             case DataPrepperExpressionParser.DataTypes:

diff --git a/...ssion/src/main/java/org/opensearch/dataprepper/expression/ParseTreeEvaluatorListener.java b/...ssion/src/main/java/org/opensearch/dataprepper/expression/ParseTreeEvaluatorListener.java
@@ -14,6 +14,8 @@
 import org.opensearch.dataprepper.expression.antlr.DataPrepperExpressionListener;
 import org.opensearch.dataprepper.expression.antlr.DataPrepperExpressionParser;
 
+import java.util.HashSet;
+import java.util.Set;
 import java.util.Stack;
 
 /**
@@ -35,13 +37,16 @@ class ParseTreeEvaluatorListener extends DataPrepperExpressionBaseListener {
     private final Stack<Integer> operatorSymbolStack;
     private final Stack<Object> operandStack;
     private final Event event;
+    private boolean listStart;
+    private Set<Object> setMembers;
 
     public ParseTreeEvaluatorListener(final OperatorProvider operatorProvider,
                                       final ParseTreeCoercionService coercionService,
                                       final Event event) {
         this.coercionService = coercionService;
         this.operatorProvider = operatorProvider;
         this.event = event;
+        this.listStart = false;
         operatorSymbolStack = new Stack<>();
         operandStack = new Stack<>();
     }
@@ -54,6 +59,24 @@ public Object getResult() {
         return operandStack.peek();
     }
 
+    private void validateSetMembers(Set<Object> setMembers) {
+        int numbers = 0;
+        int strings = 0;
+        int booleans = 0;
+        for (Object member: setMembers) {
+            if (member instanceof Number) {
+                numbers++;
+            } else if (member instanceof String) {
+                strings++;
+            } else if (member instanceof Boolean) {
+                booleans++;
+            }
+        }
+        if (numbers != setMembers.size() && strings != setMembers.size() && booleans != setMembers.size()) {
+            throw new RuntimeException("All set members should be of same type");
+        }
+    }
+
     @Override
     public void visitTerminal(TerminalNode node) {
         final int nodeType = node.getSymbol().getType();
@@ -62,12 +85,25 @@ public void visitTerminal(TerminalNode node) {
         }
         if (operatorProvider.containsOperator(nodeType) || nodeType == DataPrepperExpressionParser.LPAREN) {
             operatorSymbolStack.push(nodeType);
+        } else if (nodeType == DataPrepperExpressionParser.LBRACE) {
+            listStart = true;
+            setMembers = new HashSet<>();
+        } else if (nodeType == DataPrepperExpressionParser.RBRACE) {
+            listStart = false;
+            validateSetMembers(setMembers);
+            operandStack.push(setMembers);
         } else if (nodeType == DataPrepperExpressionParser.RPAREN) {
             // pop LPAREN at operatorSymbolStack top
             operatorSymbolStack.pop();
         } else {
             final Object arg = coercionService.coercePrimaryTerminalNode(node, event);
-            operandStack.push(arg);
+            if (listStart) {
+                if (!(arg instanceof Integer) || (((int)arg) != DataPrepperExpressionParser.COMMA && ((int)arg) != DataPrepperExpressionParser.SET_DELIMITER)) {
+                    setMembers.add(arg);
+                }
+            } else {
+                operandStack.push(arg);
+            }
         }
     }
 

diff --git a/.../java/org/opensearch/dataprepper/expression/GenericExpressionEvaluator_ConditionalIT.java b/.../java/org/opensearch/dataprepper/expression/GenericExpressionEvaluator_ConditionalIT.java
@@ -216,6 +216,21 @@ private static Stream<Arguments> validExpressionArguments() {
                 arguments("/sourceIp == null", event("{\"sourceIp\": [\"test\", \"test_two\"]}"), false),
                 arguments("/sourceIp == null", event("{\"sourceIp\": {\"test\": \"test_two\"}}"), false),
                 arguments("/sourceIp != null", event("{\"sourceIp\": {\"test\": \"test_two\"}}"), true),
+                arguments("/value in {200.222, 300.333, 400}", event("{\"value\": 400}"), true),
+                arguments("/value in {200.222, 300.333, 400}", event("{\"value\": 400.222}"), false),
+                arguments("/value not in {200.222, 300.333, 400}", event("{\"value\": 400}"), false),
+                arguments("/value not in {200.222, 300.333, 400}", event("{\"value\": 800.222}"), true),
+                arguments("/color in {\"blue\", \"red\", \"yellow\", \"green\"}", event("{\"color\": \"yellow\"}"), true),
+                arguments("/color in {\"blue\", \"red\", \"yellow\", \"green\"}", event("{\"color\": \"gray\"}"), false),
+                arguments("/color not in {\"blue\", \"red\", \"yellow\", \"green\"}", event("{\"color\": \"gray\"}"), true),
+                arguments("/color not in {\"blue\", \"red\", \"yellow\", \"green\"}", event("{\"color\": \"blue\"}"), false),
+                arguments("/color in {\"blue\", \"\", \"red\", \"yellow\", \"green\"}", event("{\"color\": \"\"}"), true),
+                arguments("/status_code in {200 , 300}", event("{\"status_code\": 200}"), true),
+                arguments("/status_code in {2 , 3}", event("{\"status_code\": 2}"), true),
+                arguments("/status_code not in {200 , 300}", event("{\"status_code\": 400}"), true),
+                arguments("/status_code in {200 , 300}", event("{\"status_code\": 500}"), false),
+                arguments("/flag in {true , false}", event("{\"flag\": false}"), true),
+                arguments("/flag in {true , false}", event("{\"flag\": true}"), true),
                 arguments("/name =~ \".*dataprepper-[0-9]+\"", event("{\"name\": \"dataprepper-0\"}"), true),
                 arguments("/name =~ \".*dataprepper-[0-9]+\"", event("{\"name\": \"dataprepper-212\"}"), true),
                 arguments("/name =~ \".*dataprepper-[0-9]+\"", event("{\"name\": \"dataprepper-abc\"}"), false),
@@ -273,6 +288,18 @@ private static Stream<Arguments> invalidExpressionArguments() {
                 arguments("contains(1234, /strField)", event("{\"intField\":1234,\"strField\":\"string\"}")),
                 arguments("contains(str, /strField)", event("{\"intField\":1234,\"strField\":\"string\"}")),
                 arguments("contains(/strField, 1234)", event("{\"intField\":1234,\"strField\":\"string\"}")),
+                arguments("/color in {\"blue\", 222.0, \"yellow\", \"green\"}", event("{\"color\": \"yellow\"}")),
+                arguments("/color in {\"blue, \"yellow\", \"green\"}", event("{\"color\": \"yellow\"}")),
+                arguments("/color in {\"blue\", yellow\", \"green\"}", event("{\"color\": \"yellow\"}")),
+                arguments("/color in {\", \"yellow\", \"green\"}", event("{\"color\": \"yellow\"}")),
+                arguments("/color in { \", \"yellow\", \"green\"}", event("{\"color\": \"yellow\"}")),
+                arguments("/color in {, \"yellow\", \"green\"}", event("{\"color\": \"yellow\"}")),
+                arguments("/color in { , \"yellow\", \"green\"}", event("{\"color\": \"yellow\"}")),
+                arguments("/color in {blue, \"yellow\", \"green\"}", event("{\"color\": \"yellow\"}")),
+                arguments("/color in { blue, \"yellow\", \"green\"}", event("{\"color\": \"yellow\"}")),
+                arguments("/color in {\"\",blue, \"yellow\", \"green\"}", event("{\"color\": \"yellow\"}")),
+                arguments("/value in {22a2.0, 100}", event("{\"value\": 100}")),
+                arguments("/value in {222, 10a0}", event("{\"value\": 100}")),
                 arguments("getMetadata(10)", tagEvent),
                 arguments("getMetadata("+ testMetadataKey+ ")", tagEvent),
                 arguments("getMetadata(\""+ testMetadataKey+")", tagEvent),