Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

METRON-656: Make Stellar 'in' closer to functioning like python #416

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions metron-platform/metron-common/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ The query language supports the following:
* Simple equality comparison operations `==`, `!=`
* if/then/else comparisons (i.e. `if var1 < 10 then 'less than 10' else '10 or more'`)
* Determining whether a field exists (via `exists`)
* An `in` operator that works like the `in` in Python
* The ability to have parenthesis to make order of operations explicit
* User defined functions

Expand All @@ -39,11 +40,17 @@ The following keywords need to be single quote escaped in order to be used in St
| <= | \> | \>= |
| ? | \+ | \- |
| , | \* | / |
| | \* | / |

Using parens such as: "foo" : "\<ok\>" requires escaping; "foo": "\'\<ok\>\'"

## Stellar Language Inclusion Checks (`in` and `not in`)
1. `in` supports string contains. e.g. `'foo' in 'foobar' == true`
2. `in` supports collection contains. e.g. `'foo' in [ 'foo', 'bar' ] == true`
3. `in` supports map key contains. e.g. `'foo' in { 'foo' : 5} == true`
4. `not in` is the negation of the in expression. e.g. `'grok' not in 'foobar' == true`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, one last comment. I was a bit curious as to how the expression 'grok' not in 'foobar' == true would be evaluated by Stellar. I wasn't sure if it would be '(grok' not in 'foobar') == true or 'grok' not in ('foobar' == true). Unfortunately when I tried to run a test it said it is not a valid expression. I think this may be an issue in the Stellar grammar. It is probably outside the scope of this ticket, but I thought I should mention it here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's a stellar language bug. ('grok' not in 'foobar') == true should work.


## Stellar Language Comparisons ('<', '<=', '>', '>=')
## Stellar Language Comparisons (`<`, `<=`, `>`, `>=`)

1. If either side of the comparison is null then return false.
2. If both values being compared implement number then the following:
Expand All @@ -54,19 +61,19 @@ Using parens such as: "foo" : "\<ok\>" requires escaping; "foo": "\'\<ok\>\'"
3. If both sides are of the same type and are comparable then use the compareTo method to compare values.
4. If none of the above are met then an exception is thrown.

## Stellar Language Equality Check ('==', '!=')
## Stellar Language Equality Check (`==`, `!=`)

Below is how the '==' operator is expected to work:
Below is how the `==` operator is expected to work:

1. If either side of the expression is null then check equality using Java's '==' expression.
1. If either side of the expression is null then check equality using Java's `==` expression.
2. Else if both sides of the expression are of Java's type Number then:
* If either side of the expression is a double then use the double value of both sides to test equality.
* Else if either side of the expression is a float then use the float value of both sides to test equality.
* Else if either side of the expression is a long then use long value of both sides to test equality.
* Otherwise use int value of both sides to test equality
3. Otherwise use equals method compare the left side with the right side.

The '!=' operator is the negation of the above.
The `!=` operator is the negation of the above.

## Stellar Core Functions

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,22 +79,30 @@ public void enterTransformation(StellarParser.TransformationContext ctx) {
}

private boolean handleIn(Token<?> left, Token<?> right) {
Object key = null;
Object key = right.getValue();

Set<Object> set = null;
if (left.getValue() instanceof Collection) {
set = new HashSet<>((List<Object>) left.getValue());
} else if (left.getValue() != null) {
set = ImmutableSet.of(left.getValue());
} else {
set = new HashSet<>();
}

key = right.getValue();
if (key == null || set.isEmpty()) {
if (left.getValue() != null) {
if(left.getValue() instanceof String && key instanceof String) {
return ((String)left.getValue()).contains(key.toString());
}
else if(left.getValue() instanceof Collection) {
return ((Collection)left.getValue()).contains(key);
}
else if(left.getValue() instanceof Map) {
return ((Map)left.getValue()).containsKey(key);
}
else {
if(key == null) {
return key == left.getValue();
}
else {
return key.equals(left.getValue());
}
}
} else {
return false;
}
return set.contains(key);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -402,19 +402,58 @@ public void testBooleanOps() throws Exception {
}

@Test
public void testList() throws Exception {
public void testInCollection() throws Exception {
final Map<String, String> variableMap = new HashMap<String, String>() {{
put("foo", "casey");
put("empty", "");
put("spaced", "metron is great");
}};
Assert.assertTrue(runPredicate("foo in [ 'casey', 'david' ]", v -> variableMap.get(v)));
Assert.assertFalse(runPredicate("foo in [ ]", v -> variableMap.get(v)));
Assert.assertTrue(runPredicate("foo in [ foo, 'david' ]", v -> variableMap.get(v)));
Assert.assertTrue(runPredicate("foo in [ 'casey', 'david' ] and 'casey' == foo", v -> variableMap.get(v)));
Assert.assertTrue(runPredicate("foo in [ 'casey', 'david' ] and foo == 'casey'", v -> variableMap.get(v)));
Assert.assertTrue(runPredicate("foo in [ 'casey' ]", v -> variableMap.get(v)));
Assert.assertFalse(runPredicate("foo not in [ 'casey', 'david' ]", v -> variableMap.get(v)));
Assert.assertFalse(runPredicate("foo not in [ 'casey', 'david' ] and 'casey' == foo", v -> variableMap.get(v)));
Assert.assertTrue(runPredicate("null in [ null, 'something' ]", v -> variableMap.get(v)));
Assert.assertFalse(runPredicate("null not in [ null, 'something' ]", v -> variableMap.get(v)));
}

@Test
public void testInMap() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a couple test cases for String values?

runPredicate("'foo' in { 'foo' : 5 }"...
bar = 'foo'
runPredicate("'foo' in { bar : 5 }"...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing. Done, let me know what you think about them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! +1

final Map<String, String> variableMap = new HashMap<String, String>() {{
put("foo", "casey");
put("empty", "");
}};
Assert.assertTrue(runPredicate("'casey' in { foo : 5 }", v -> variableMap.get(v)));
Assert.assertFalse(runPredicate("'casey' not in { foo : 5 }", v -> variableMap.get(v)));
Assert.assertTrue(runPredicate("foo in { foo : 5 }", v -> variableMap.get(v)));
Assert.assertFalse(runPredicate("foo not in { foo : 5 }", v -> variableMap.get(v)));
Assert.assertTrue(runPredicate("'foo' in { 'foo' : 5 }", v -> variableMap.get(v)));
Assert.assertFalse(runPredicate("'foo' not in { 'foo' : 5 }", v -> variableMap.get(v)));
Assert.assertTrue(runPredicate("foo in { 'casey' : 5 }", v -> variableMap.get(v)));
Assert.assertFalse(runPredicate("foo not in { 'casey' : 5 }", v -> variableMap.get(v)));
Assert.assertFalse(runPredicate("empty in { foo : 5 }", v -> variableMap.get(v)));
Assert.assertTrue(runPredicate("empty not in { foo : 5 }", v -> variableMap.get(v)));
Assert.assertFalse(runPredicate("'foo' in { }", v -> variableMap.get(v)));
Assert.assertFalse(runPredicate("null in { 'foo' : 5 }", v -> variableMap.get(v)));
Assert.assertTrue(runPredicate("null not in { 'foo' : 5 }", v -> variableMap.get(v)));
}

@Test
public void testInString() throws Exception {
final Map<String, String> variableMap = new HashMap<String, String>() {{
put("foo", "casey");
put("empty", "");
}};
Assert.assertTrue(runPredicate("'case' in foo", v -> variableMap.get(v)));
Assert.assertFalse(runPredicate("'case' not in foo", v -> variableMap.get(v)));
Assert.assertFalse(runPredicate("'case' in empty", v -> variableMap.get(v)));
Assert.assertTrue(runPredicate("'case' not in empty", v -> variableMap.get(v)));
Assert.assertFalse(runPredicate("'case' in [ foo ]", v -> variableMap.get(v)));
Assert.assertTrue(runPredicate("'case' not in [ foo ]", v -> variableMap.get(v)));
Assert.assertFalse(runPredicate("null in foo", v -> variableMap.get(v)));
Assert.assertTrue(runPredicate("null not in foo", v -> variableMap.get(v)));
}

@Test
Expand Down