diff --git a/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/BooleanValuedFunctionWrapper.java b/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/BooleanValuedFunctionWrapper.java index c6e136e8a4e6..5481eb38adff 100644 --- a/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/BooleanValuedFunctionWrapper.java +++ b/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/BooleanValuedFunctionWrapper.java @@ -15,12 +15,13 @@ public BooleanValuedFunctionWrapper(Expression expr) { @Override public boolean evaluateBool(EvaluationContext context, Message message) { - return (Boolean) expr.evaluate(context, message); + final Object value = expr.evaluate(context, message); + return value != null && (Boolean) value; } @Override public boolean isConstant() { - return false; + return expr.isConstant(); } @Override diff --git a/src/main/java/org/graylog/plugins/messageprocessor/ast/functions/ParameterDescriptor.java b/src/main/java/org/graylog/plugins/messageprocessor/ast/functions/ParameterDescriptor.java index 7d81c1857460..b27feb667a9a 100644 --- a/src/main/java/org/graylog/plugins/messageprocessor/ast/functions/ParameterDescriptor.java +++ b/src/main/java/org/graylog/plugins/messageprocessor/ast/functions/ParameterDescriptor.java @@ -9,26 +9,59 @@ public abstract class ParameterDescriptor { public abstract String name(); - public static Builder builder() { - return new AutoValue_ParameterDescriptor.Builder(); + public abstract boolean optional(); + + public static Builder param() { + return new AutoValue_ParameterDescriptor.Builder().optional(false); } public static ParameterDescriptor string(String name) { - return builder().string(name).build(); + return param().string(name).build(); } public static ParameterDescriptor object(String name) { - return builder().name(name).type(Object.class).build(); + return param().object(name).build(); + } + + public static ParameterDescriptor integer(String name) { + return param().integer(name).build(); + } + + public static ParameterDescriptor floating(String name) { + return param().floating(name).build(); + } + + public static ParameterDescriptor bool(String name) { + return param().bool(name).build(); } @AutoValue.Builder public static abstract class Builder { public abstract Builder type(Class type); public abstract Builder name(String name); + public abstract Builder optional(boolean optional); public abstract ParameterDescriptor build(); + public Builder string(String name) { return type(String.class).name(name); } + public Builder object(String name) { + return type(Object.class).name(name); + } + public Builder floating(String name) { + return type(Double.class).name(name); + } + public Builder integer(String name) { + return type(Long.class).name(name); + } + public Builder bool(String name) { + return type(Boolean.class).name(name); + } + + public Builder optional() { + return optional(true); + } + } } diff --git a/src/main/java/org/graylog/plugins/messageprocessor/functions/DoubleCoercion.java b/src/main/java/org/graylog/plugins/messageprocessor/functions/DoubleCoercion.java index a99c643350b2..51beb04f45be 100644 --- a/src/main/java/org/graylog/plugins/messageprocessor/functions/DoubleCoercion.java +++ b/src/main/java/org/graylog/plugins/messageprocessor/functions/DoubleCoercion.java @@ -11,7 +11,7 @@ import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.collect.ImmutableList.of; -import static org.graylog.plugins.messageprocessor.ast.functions.ParameterDescriptor.builder; +import static org.graylog.plugins.messageprocessor.ast.functions.ParameterDescriptor.param; import static org.graylog.plugins.messageprocessor.ast.functions.ParameterDescriptor.object; public class DoubleCoercion implements Function { @@ -35,7 +35,7 @@ public FunctionDescriptor descriptor() { .returnType(Double.class) .params(of( object(VALUE), - builder().name(DEFAULT).type(Double.class).build() + param().name(DEFAULT).type(Double.class).build() )) .build(); } diff --git a/src/main/java/org/graylog/plugins/messageprocessor/functions/LongCoercion.java b/src/main/java/org/graylog/plugins/messageprocessor/functions/LongCoercion.java index b6cdc1a987ef..bc16485f4d95 100644 --- a/src/main/java/org/graylog/plugins/messageprocessor/functions/LongCoercion.java +++ b/src/main/java/org/graylog/plugins/messageprocessor/functions/LongCoercion.java @@ -11,7 +11,7 @@ import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.collect.ImmutableList.of; -import static org.graylog.plugins.messageprocessor.ast.functions.ParameterDescriptor.builder; +import static org.graylog.plugins.messageprocessor.ast.functions.ParameterDescriptor.param; import static org.graylog.plugins.messageprocessor.ast.functions.ParameterDescriptor.object; public class LongCoercion implements Function { @@ -35,7 +35,7 @@ public FunctionDescriptor descriptor() { .returnType(Long.class) .params(of( object(VALUE), - builder().name(DEFAULT).type(Long.class).build() + param().name(DEFAULT).type(Long.class).build() )) .build(); } diff --git a/src/main/java/org/graylog/plugins/messageprocessor/parser/RuleParser.java b/src/main/java/org/graylog/plugins/messageprocessor/parser/RuleParser.java index b0f57eebc413..08e8a9dcac97 100644 --- a/src/main/java/org/graylog/plugins/messageprocessor/parser/RuleParser.java +++ b/src/main/java/org/graylog/plugins/messageprocessor/parser/RuleParser.java @@ -1,5 +1,6 @@ package org.graylog.plugins.messageprocessor.parser; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -38,6 +39,8 @@ import org.graylog.plugins.messageprocessor.parser.errors.IncompatibleArgumentType; import org.graylog.plugins.messageprocessor.parser.errors.IncompatibleType; import org.graylog.plugins.messageprocessor.parser.errors.IncompatibleTypes; +import org.graylog.plugins.messageprocessor.parser.errors.MissingRequiredParam; +import org.graylog.plugins.messageprocessor.parser.errors.OptionalParametersMustBeNamed; import org.graylog.plugins.messageprocessor.parser.errors.ParseError; import org.graylog.plugins.messageprocessor.parser.errors.UndeclaredFunction; import org.graylog.plugins.messageprocessor.parser.errors.UndeclaredVariable; @@ -134,28 +137,53 @@ public void exitFunctionCall(RuleLangParser.FunctionCallContext ctx) { if (function == null) { parseContext.addError(new UndeclaredFunction(ctx)); } else { + final ImmutableList params = function.descriptor().params(); + final boolean hasOptionalParams = params.stream().anyMatch(ParameterDescriptor::optional); + // convert null key, single arg style to default named args for single arg functions if (args != null) { - if (args.containsKey(null) && function.descriptor().params().size() == 1) { + if (args.containsKey(null) && params.size() == 1) { final Expression argExpr = args.remove(null); - final ParameterDescriptor param = function.descriptor().params().get(0); + final ParameterDescriptor param = params.get(0); args.put(param.name(), argExpr); } - // check for the right number of arguments to the function - if (function.descriptor().params().size() != args.size()) { + + // check for the right number of arguments to the function if the function only has required params + if (!hasOptionalParams && params.size() != args.size()) { parseContext.addError(new WrongNumberOfArgs(ctx, function, args.size())); + } else { + // there are optional parameters, check that all required ones are present + final Map givenArguments = args; + final List missingParams = + params.stream() + .filter(p -> !p.optional()) + .map(p -> givenArguments.containsKey(p.name()) ? null : p) + .filter(p -> p != null) + .collect(Collectors.toList()); + for (ParameterDescriptor param : missingParams) { + parseContext.addError(new MissingRequiredParam(ctx, function, param)); + } + } } else if (positionalArgs != null) { // use descriptor to turn positional arguments into a map args = Maps.newHashMap(); - if (positionalArgs.size() != function.descriptor().params().size()) { + if (!hasOptionalParams && positionalArgs.size() != params.size()) { parseContext.addError(new WrongNumberOfArgs(ctx, function, positionalArgs.size())); } - int i = 0; - for (ParameterDescriptor p : function.descriptor().params()) { - final Expression argExpr = positionalArgs.get(i); - args.put(p.name(), argExpr); - i++; + if (hasOptionalParams) { + parseContext.addError(new OptionalParametersMustBeNamed(ctx, function)); + } else { + int i = 0; + for (ParameterDescriptor p : params) { + if (i >= params.size()) { + // avoid index out of bounds, we've added an error anyway + break; + } + final Expression argExpr = positionalArgs.get(i); + args.put(p.name(), argExpr); + i++; + } } } } @@ -210,7 +238,8 @@ public void exitRuleDeclaration(RuleLangParser.RuleDeclarationContext ctx) { } else if (expr.getType().equals(Boolean.class)) { condition = new BooleanValuedFunctionWrapper(expr); } else { - throw new RuntimeException("Unable to create condition, this is a bug"); + condition = new BooleanExpression(false); + log.debug("Unable to create condition, replacing with 'false'"); } ruleBuilder.when(condition); ruleBuilder.then(parseContext.statements); diff --git a/src/main/java/org/graylog/plugins/messageprocessor/parser/errors/MissingRequiredParam.java b/src/main/java/org/graylog/plugins/messageprocessor/parser/errors/MissingRequiredParam.java new file mode 100644 index 000000000000..5c30b2e48324 --- /dev/null +++ b/src/main/java/org/graylog/plugins/messageprocessor/parser/errors/MissingRequiredParam.java @@ -0,0 +1,28 @@ +package org.graylog.plugins.messageprocessor.parser.errors; + +import com.fasterxml.jackson.annotation.JsonProperty; +import org.graylog.plugins.messageprocessor.ast.functions.Function; +import org.graylog.plugins.messageprocessor.ast.functions.ParameterDescriptor; +import org.graylog.plugins.messageprocessor.parser.RuleLangParser; + +public class MissingRequiredParam extends ParseError { + private final Function function; + private final ParameterDescriptor param; + + public MissingRequiredParam(RuleLangParser.FunctionCallContext ctx, + Function function, + ParameterDescriptor param) { + super("missing_required_param", ctx); + this.function = function; + this.param = param; + } + + @JsonProperty("reason") + @Override + public String toString() { + return "Missing required parameter " + param.name() + + " of type " + param.type().getSimpleName() + + " in call to function " + function.descriptor().name() + + positionString(); + } +} diff --git a/src/main/java/org/graylog/plugins/messageprocessor/parser/errors/OptionalParametersMustBeNamed.java b/src/main/java/org/graylog/plugins/messageprocessor/parser/errors/OptionalParametersMustBeNamed.java new file mode 100644 index 000000000000..769e17a1124c --- /dev/null +++ b/src/main/java/org/graylog/plugins/messageprocessor/parser/errors/OptionalParametersMustBeNamed.java @@ -0,0 +1,20 @@ +package org.graylog.plugins.messageprocessor.parser.errors; + +import com.fasterxml.jackson.annotation.JsonProperty; +import org.graylog.plugins.messageprocessor.ast.functions.Function; +import org.graylog.plugins.messageprocessor.parser.RuleLangParser; + +public class OptionalParametersMustBeNamed extends ParseError { + private final Function function; + + public OptionalParametersMustBeNamed(RuleLangParser.FunctionCallContext ctx, Function function) { + super("must_name_optional_params", ctx); + this.function = function; + } + + @JsonProperty("reason") + @Override + public String toString() { + return "Function " + function.descriptor().name() + " has optional parameters, must use named parameters to call" + positionString(); + } +} diff --git a/src/test/java/org/graylog/plugins/messageprocessor/parser/RuleParserTest.java b/src/test/java/org/graylog/plugins/messageprocessor/parser/RuleParserTest.java index 1bf3054e53a3..2033488f4136 100644 --- a/src/test/java/org/graylog/plugins/messageprocessor/parser/RuleParserTest.java +++ b/src/test/java/org/graylog/plugins/messageprocessor/parser/RuleParserTest.java @@ -1,7 +1,6 @@ package org.graylog.plugins.messageprocessor.parser; import com.google.common.base.Charsets; -import com.google.common.collect.ImmutableList; import com.google.common.collect.Maps; import org.graylog.plugins.messageprocessor.EvaluationContext; import org.graylog.plugins.messageprocessor.ast.Rule; @@ -9,12 +8,13 @@ import org.graylog.plugins.messageprocessor.ast.functions.Function; import org.graylog.plugins.messageprocessor.ast.functions.FunctionDescriptor; import org.graylog.plugins.messageprocessor.ast.functions.ParameterDescriptor; +import org.graylog.plugins.messageprocessor.ast.statements.Statement; import org.graylog.plugins.messageprocessor.functions.HasField; import org.graylog.plugins.messageprocessor.functions.LongCoercion; import org.graylog.plugins.messageprocessor.functions.SetField; -import org.graylog.plugins.messageprocessor.ast.statements.Statement; import org.graylog.plugins.messageprocessor.functions.StringCoercion; import org.graylog.plugins.messageprocessor.parser.errors.IncompatibleArgumentType; +import org.graylog.plugins.messageprocessor.parser.errors.OptionalParametersMustBeNamed; import org.graylog.plugins.messageprocessor.parser.errors.UndeclaredFunction; import org.graylog.plugins.messageprocessor.parser.errors.UndeclaredVariable; import org.graylog2.plugin.Message; @@ -36,6 +36,8 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; +import static com.google.common.collect.ImmutableList.of; +import static org.graylog.plugins.messageprocessor.ast.functions.ParameterDescriptor.param; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -65,7 +67,7 @@ public FunctionDescriptor descriptor() { return FunctionDescriptor.builder() .name("nein") .returnType(Boolean.class) - .params(ImmutableList.of()) + .params(of()) .build(); } }); @@ -80,7 +82,7 @@ public FunctionDescriptor descriptor() { return FunctionDescriptor.builder() .name("doch") .returnType(Boolean.class) - .params(ImmutableList.of()) + .params(of()) .build(); } }); @@ -95,7 +97,7 @@ public FunctionDescriptor descriptor() { return FunctionDescriptor.builder() .name("double_valued_func") .returnType(Double.class) - .params(ImmutableList.of()) + .params(of()) .build(); } }); @@ -110,7 +112,7 @@ public FunctionDescriptor descriptor() { return FunctionDescriptor.builder() .name("one_arg") .returnType(String.class) - .params(ImmutableList.of(ParameterDescriptor.string("one"))) + .params(of(ParameterDescriptor.string("one"))) .build(); } }); @@ -128,7 +130,7 @@ public FunctionDescriptor descriptor() { return FunctionDescriptor.builder() .name("concat") .returnType(String.class) - .params(ImmutableList.of( + .params(of( ParameterDescriptor.string("one"), ParameterDescriptor.object("two"), ParameterDescriptor.object("three") @@ -148,7 +150,27 @@ public FunctionDescriptor descriptor() { return FunctionDescriptor.builder() .name("trigger_test") .returnType(Void.class) - .params(ImmutableList.of()) + .params(of()) + .build(); + } + }); + functions.put("optional", new Function() { + @Override + public Boolean evaluate(Map args, EvaluationContext context, Message message) { + return true; + } + + @Override + public FunctionDescriptor descriptor() { + return FunctionDescriptor.builder() + .name("optional") + .returnType(Boolean.class) + .params(of( + ParameterDescriptor.bool("a"), + ParameterDescriptor.string("b"), + param().floating("c").optional().build(), + ParameterDescriptor.integer("d") + )) .build(); } }); @@ -288,6 +310,26 @@ public void messageRefQuotedField() throws Exception { assertTrue(actionsTriggered.get()); } + @Test + public void optionalArguments() throws Exception { + final Rule rule = parser.parseRule(ruleForTest()); + + Message message = new Message("hello test", "source", DateTime.now()); + evaluateRule(rule, message); + assertTrue(actionsTriggered.get()); + } + + @Test + public void optionalParamsMustBeNamed() throws Exception { + try { + parser.parseRule(ruleForTest()); + } catch (ParseException e) { + assertEquals(1, e.getErrors().stream().count()); + assertTrue(e.getErrors().stream().allMatch(error -> error instanceof OptionalParametersMustBeNamed)); + } + + } + private Message evaluateRule(Rule rule, Message message) { final EvaluationContext context = new EvaluationContext(); if (rule.when().evaluateBool(context, message)) { diff --git a/src/test/resources/org/graylog/plugins/messageprocessor/parser/optionalArguments.txt b/src/test/resources/org/graylog/plugins/messageprocessor/parser/optionalArguments.txt new file mode 100644 index 000000000000..913b30f7bc0d --- /dev/null +++ b/src/test/resources/org/graylog/plugins/messageprocessor/parser/optionalArguments.txt @@ -0,0 +1,6 @@ +rule "optional function arguments" +when + optional(d: 3, a: true, b: "string") +then + trigger_test(); +end \ No newline at end of file diff --git a/src/test/resources/org/graylog/plugins/messageprocessor/parser/optionalParamsMustBeNamed.txt b/src/test/resources/org/graylog/plugins/messageprocessor/parser/optionalParamsMustBeNamed.txt new file mode 100644 index 000000000000..c0eb68be4bf7 --- /dev/null +++ b/src/test/resources/org/graylog/plugins/messageprocessor/parser/optionalParamsMustBeNamed.txt @@ -0,0 +1,5 @@ +rule "optionalParamsMustBeNamed" +when + optional(false, "string", 3) +then +end \ No newline at end of file