gnodet-bot commented on code in PR #26824:
URL: https://github.com/apache/camel/pull/26824#discussion_r4090780645
##########
core/camel-core-languages/src/main/docs/modules/languages/pages/simple-operators.adoc:
##########
@@ -252,8 +252,28 @@ And the following boolean operators can be used to group
expressions:
|Operator |Description
|`&&` | The `and` operator is used to group two expressions if both operands
evaluates to `true`.
|`\|\|` | The `or` operator is used to group two expressions if any operand
evaluates to `true`.
+|`!` | The `not` operator negates the function that follows it, which must
evaluate to `true` or `false`.
|====
+The `!` operator is written directly in front of the function it negates, and
only in a predicate:
+
+[source,java]
+----
+// true when the body is not empty
+simple("!${body.isEmpty()}")
+
+// the same written inside the braces
+simple("${!body.isEmpty()}")
+
+// it groups with && and || like any other predicate, and negates only its own
function
+simple("!${body.isEmpty()} && ${header.foo} == 'bar'")
+simple("${!body.isEmpty() && header.foo == 'bar'}")
+----
+
+In an expression a `!` is ordinary text, so a message such as `Hello ${body}!`
is unaffected. Note that `!` negates a
+function, not a comparison: write `${body} != 'x'` rather than `!${body} ==
'x'`, and the negated operators
+(`!contains`, `!startsWith`, `!in`, ...) stay as they are.
Review Comment:
⚠️ **CI failure — Asciidoctor attribute expansion:** `{body}` outside a
`[source,...]` block is treated as an Asciidoctor attribute reference,
producing _"skipping reference to missing attribute: body"_ warnings that fail
the doc build (run 35966397697). The examples inside the `[source,java]` block
above are fine; it's only this prose paragraph that is exposed.
Fix: escape the dollar signs (pass:c[`$`]`{...}`) or rephrase to keep the
examples inside the code block:
```suggestion
In an expression a `!` is ordinary text, so a message such as `Hello
\${body}!` is unaffected. Note that `!` negates a
function, not a comparison: write `\${body} != 'x'` rather than `!\${body}
== 'x'`, and the negated operators
(`!contains`, `!startsWith`, `!in`, ...) stay as they are.
```
##########
core/camel-core-languages/src/main/java/org/apache/camel/language/simple/ast/UnaryExpression.java:
##########
@@ -79,11 +86,33 @@ public Expression createExpression(CamelContext
camelContext, String expression)
return createIncDecExpression(camelContext, leftExp, 1);
} else if (operator == UnaryOperatorType.DEC) {
return createIncDecExpression(camelContext, leftExp, -1);
+ } else if (operator == UnaryOperatorType.NOT) {
+ return createNotExpression(camelContext, leftExp);
}
throw new SimpleParserException("Unknown unary operator " + operator,
token.getIndex());
}
+ private Expression createNotExpression(CamelContext camelContext, final
Expression exp) {
+ return new Expression() {
+ @Override
+ public <T> T evaluate(Exchange exchange, Class<T> type) {
+ Object value = exp.evaluate(exchange, Object.class);
+ Boolean bool =
camelContext.getTypeConverter().convertTo(Boolean.class, exchange, value);
+ if (bool == null) {
+ throw new SimpleParserException(
+ "Cannot negate " + left + " as it is not true or
false but: " + value, token.getIndex());
+ }
+ return camelContext.getTypeConverter().convertTo(type,
exchange, !bool);
+ }
+
+ @Override
+ public String toString() {
+ return "!" + left;
+ }
+ };
Review Comment:
⚠️ **Missing `init()` — NPE on first evaluation:** The anonymous
`Expression` returned here has no `init()` override, so `exp.init(context)` is
never called. For any OGNL-backed or type-converter–dependent function (e.g.
`body.isEmpty()`), the internal state is only set up during `init` — evaluating
without it will throw an NPE or return a stale result.
`SimpleFunctionExpression.createNegatedExpression` (the `${!...}` path)
already does this correctly; the `UnaryExpression` path (outer `!${...}`) must
follow suit:
```suggestion
private Expression createNotExpression(CamelContext camelContext, final
Expression exp) {
return new Expression() {
@Override
public void init(CamelContext context) {
exp.init(context);
}
@Override
public <T> T evaluate(Exchange exchange, Class<T> type) {
Object value = exp.evaluate(exchange, Object.class);
Boolean bool =
camelContext.getTypeConverter().convertTo(Boolean.class, exchange, value);
if (bool == null) {
throw new SimpleParserException(
"Cannot negate " + left + " as it is not true or
false but: " + value, token.getIndex());
}
return camelContext.getTypeConverter().convertTo(type,
exchange, !bool);
}
@Override
public String toString() {
return "!" + left;
}
};
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
