Skip to content

Commit

Permalink
allow optional paramters for functions
Browse files Browse the repository at this point in the history 
 - guard against null in BooleanValueFunctionWrapper
 - refactor ParamDescription builder to support more common types
 - new errors for parameter handling
 - test cases
  • Loading branch information
@kroepke
kroepke committed Jan 10, 2016
1 parent 17bc974 commit f4e2eca
Show file tree
Hide file tree
Showing 10 changed files with 193 additions and 29 deletions.
5 changes: 3 additions & 2 deletions ...va/org/graylog/plugins/messageprocessor/ast/expressions/BooleanValuedFunctionWrapper.java
View file
Expand Up @@ -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
Expand Down
41 changes: 37 additions & 4 deletions src/main/java/org/graylog/plugins/messageprocessor/ast/functions/ParameterDescriptor.java
View file
Expand Up @@ -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);
}

}
}
4 changes: 2 additions & 2 deletions src/main/java/org/graylog/plugins/messageprocessor/functions/DoubleCoercion.java
View file
Expand Up @@ -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<Double> {
Expand All @@ -35,7 +35,7 @@ public FunctionDescriptor<Double> descriptor() {
.returnType(Double.class)
.params(of(
object(VALUE),
builder().name(DEFAULT).type(Double.class).build()
param().name(DEFAULT).type(Double.class).build()
))
.build();
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/graylog/plugins/messageprocessor/functions/LongCoercion.java
View file
Expand Up @@ -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<Long> {
Expand All @@ -35,7 +35,7 @@ public FunctionDescriptor<Long> descriptor() {
.returnType(Long.class)
.params(of(
object(VALUE),
builder().name(DEFAULT).type(Long.class).build()
param().name(DEFAULT).type(Long.class).build()
))
.build();
}
Expand Down
51 changes: 40 additions & 11 deletions src/main/java/org/graylog/plugins/messageprocessor/parser/RuleParser.java
View file
@@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -134,28 +137,53 @@ public void exitFunctionCall(RuleLangParser.FunctionCallContext ctx) {
if (function == null) {
parseContext.addError(new UndeclaredFunction(ctx));
} else {
final ImmutableList<ParameterDescriptor> 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<String, Expression> givenArguments = args;
final List<ParameterDescriptor> 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++;
}
}
}
}
Expand Down Expand Up @@ -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);
Expand Down
28 changes: 28 additions & 0 deletions src/main/java/org/graylog/plugins/messageprocessor/parser/errors/MissingRequiredParam.java
View file
@@ -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();
}
}
20 changes: 20 additions & 0 deletions ...ava/org/graylog/plugins/messageprocessor/parser/errors/OptionalParametersMustBeNamed.java
View file
@@ -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();
}
}
58 changes: 50 additions & 8 deletions src/test/java/org/graylog/plugins/messageprocessor/parser/RuleParserTest.java
View file
@@ -1,20 +1,20 @@
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;
import org.graylog.plugins.messageprocessor.ast.expressions.Expression;
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;
Expand All @@ -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;
Expand Down Expand Up @@ -65,7 +67,7 @@ public FunctionDescriptor<Boolean> descriptor() {
return FunctionDescriptor.<Boolean>builder()
.name("nein")
.returnType(Boolean.class)
.params(ImmutableList.of())
.params(of())
.build();
}
});
Expand All @@ -80,7 +82,7 @@ public FunctionDescriptor<Boolean> descriptor() {
return FunctionDescriptor.<Boolean>builder()
.name("doch")
.returnType(Boolean.class)
.params(ImmutableList.of())
.params(of())
.build();
}
});
Expand All @@ -95,7 +97,7 @@ public FunctionDescriptor<Double> descriptor() {
return FunctionDescriptor.<Double>builder()
.name("double_valued_func")
.returnType(Double.class)
.params(ImmutableList.of())
.params(of())
.build();
}
});
Expand All @@ -110,7 +112,7 @@ public FunctionDescriptor<String> descriptor() {
return FunctionDescriptor.<String>builder()
.name("one_arg")
.returnType(String.class)
.params(ImmutableList.of(ParameterDescriptor.string("one")))
.params(of(ParameterDescriptor.string("one")))
.build();
}
});
Expand All @@ -128,7 +130,7 @@ public FunctionDescriptor<String> descriptor() {
return FunctionDescriptor.<String>builder()
.name("concat")
.returnType(String.class)
.params(ImmutableList.of(
.params(of(
ParameterDescriptor.string("one"),
ParameterDescriptor.object("two"),
ParameterDescriptor.object("three")
Expand All @@ -148,7 +150,27 @@ public FunctionDescriptor<Void> descriptor() {
return FunctionDescriptor.<Void>builder()
.name("trigger_test")
.returnType(Void.class)
.params(ImmutableList.of())
.params(of())
.build();
}
});
functions.put("optional", new Function<Boolean>() {
@Override
public Boolean evaluate(Map<String, Expression> args, EvaluationContext context, Message message) {
return true;
}

@Override
public FunctionDescriptor<Boolean> descriptor() {
return FunctionDescriptor.<Boolean>builder()
.name("optional")
.returnType(Boolean.class)
.params(of(
ParameterDescriptor.bool("a"),
ParameterDescriptor.string("b"),
param().floating("c").optional().build(),
ParameterDescriptor.integer("d")
))
.build();
}
});
Expand Down Expand Up @@ -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)) {
Expand Down
6 changes: 6 additions & 0 deletions src/test/resources/org/graylog/plugins/messageprocessor/parser/optionalArguments.txt
View file
@@ -0,0 +1,6 @@
rule "optional function arguments"
when
optional(d: 3, a: true, b: "string")
then
trigger_test();
end

0 comments on commit f4e2eca

Please sign in to comment.