CAMEL-24582: simple language - add equals/!equals operators - #26022
Conversation
Simple's == favours numeric comparison when both sides are all-digit
strings, so "0001" == "001" is true. That is intentional and useful for
padded numbers such as the hour from ${date:now:HH}, but wrong for
identifiers like account numbers, where the leading zeros carry meaning.
The new equals and !equals operators always compare the two values as
text, alongside the existing contains, startsWith and endsWith, which
already work that way. == is unchanged.
Co-authored-by: Claude <noreply@anthropic.com>
davsclaus
left a comment
There was a problem hiding this comment.
Thanks for the well-tested PR and the detailed write-up — the tokenizer/BinaryExpression/PredicateBuilder wiring correctly mirrors the existing startsWith/!startsWith pattern (CAMEL-22868), the tokenizer ordering (!equals before equals) matches the established negation-first convention, and the doc addition to simple-operators.adoc is complete and accurate.
Requesting changes on one gap: tooling metadata not updated
SimpleLanguage.java declares @Language(value = "simple", ..., operatorsClass = SimpleOperatorConstants.class). core/camel-core-languages/src/main/java/org/apache/camel/language/simple/SimpleOperatorConstants.java is the annotated source of truth that PackageLanguageMojo.addOperator() reflects over at build time to generate the operator metadata consumed by tooling — core/camel-core-languages/src/generated/resources/META-INF/org/apache/camel/language/simple/simple.json, mirrored into catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/languages/simple.json. Every existing binary operator (contains, startsWith, endsWith, in, is, range, regex, ...) has a matching @Metadata-annotated constant there.
This PR adds EQUALS/NOT_EQUALS to BinaryOperatorType, SimpleTokenizer, BinaryExpression, and PredicateBuilder, but doesn't add the corresponding constants to SimpleOperatorConstants.java. Nothing enforces sync between the two, so this passes CI silently — the operators work at runtime but stay invisible to IDE autocomplete/hover-docs and anything else that reads the catalog metadata.
Could you add, mirroring the STARTS_WITH/NOT_STARTS_WITH pair immediately above (insert after the NOT_ENDS_WITH constant, before the // --- Unary operators --- comment):
@Metadata(description = "Tests whether the left operand string equals the right operand string, compared as text without numeric coercion.",
label = "binary",
examples = { "${header.Account1} equals ${header.Account2}" },
annotations = { "kind=binary", "syntax=LHS equals RHS", "precedence=10" })
public static final String EQUALS = "equals";
@Metadata(description = "Tests whether the left operand string does not equal the right operand string, compared as text without numeric coercion.",
label = "binary",
examples = { "${header.Account1} !equals ${header.Account2}" },
annotations = { "kind=binary", "syntax=LHS !equals RHS", "precedence=10" })
public static final String NOT_EQUALS = "!equals";After adding, please regenerate and commit simple.json in both locations via the module build (core/camel-core-languages, then the catalog module), same as any other component-metadata change.
This review does not replace specialized review tools (CodeRabbit, Sourcery) or static analysis (SonarCloud) — please still expect those to run separately.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
…adata SimpleOperatorConstants is the annotated source PackageLanguageMojo reflects over to generate simple.json, which tooling reads for autocomplete and hover docs. Nothing enforces that it stays in sync with BinaryOperatorType, so the operators worked at runtime while staying invisible to tooling. Regenerated simple.json in the module and mirrored it into the catalog. Co-authored-by: Claude <noreply@anthropic.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
✅ Generated files are up to dateAn earlier CI run reported uncommitted generated changes; the latest run no longer does. |
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 562 tested, 26 compile-only — current: 561 all testedMaveniverse Scalpel detected 588 affected modules (current approach: 561).
|
gnodet
left a comment
There was a problem hiding this comment.
Clean, well-structured enhancement that adds equals and !equals text-comparison operators to the Simple language, precisely mirroring the established startsWith/endsWith pattern across all layers (tokenizer, enum, AST, predicate builder, helper, metadata, docs, tests).
The implementation correctly handles:
- Null-handling and type-converter logic in
LanguageHelper.equalsString() - Negation-first ordering in the tokenizer (
!equalsbeforeequals) - Literal text disambiguation ("equals" as English word vs operator)
- Good test coverage: zero-padded digits, literal comparisons, case sensitivity, negated form
- Clear documentation with examples contrasting
==vsequals
📋 PR Metadata
| Aspect | Current | Suggested |
|---|---|---|
| Category | (unlabeled) | enhancement |
| Labels | core, catalog, docs |
+ enhancement |
| Milestone | (none) | 4.23.0 |
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Guillaume Nodet
Implements CAMEL-24582.
Simple's
==favours numeric comparison when both sides are all-digit strings, so"0001" == "001"istrue. That is intentional and long-standing (CAMEL-15587), and it is what you want for padded numbers such as the hour from${date:now:HH}. It is not what you want for identifiers like account numbers, where the leading zeros carry meaning. That was the case reported in CAMEL-24580.This adds
equalsand!equals, which always compare the two values as text, alongsidecontains,startsWithandendsWith, which already work that way.==is unchanged.Two choices worth a second opinion
!equalsrather thannotEquals. The issue mentions both. The existing convention is!contains,!startsWith,!regex, with the two-wordnot containsforms deprecated and logging a warning, sonotEqualswould not match any current form. Easy to change if you prefer otherwise.No
equalsIgnoreCase.==has=~andcontainshas~~, soequalswithout a case-insensitive counterpart is an asymmetry. It is outside the scope described in the issue, so I left it out rather than expanding on my own initiative. Happy to add it here or as a follow-up.Notes on the implementation
LanguageHelper.equalsStringandPredicateBuilder.equalsStringmirror the existingstartsWith/endsWithpair exactly: both sides go through the type converter toString, thenString.equals. No new comparison logic, andObjectHelper.typeCoerceEqualsis not involved.In
SimpleTokenizerthe two new entries required renumberingKNOWN_TOKENSfrom index 44 onwards and bumpingNUMBER_OF_TOKENS. TheminusValuetoken stays last, as its comment requires, so unary--keeps its priority.One thing I checked rather than assumed, since
equalsis a common English word: a binary operator only tokenises when it is surrounded by spaces (evalSurroundedBySpace), andSimpleExpressionParserdoes not acceptbinaryOperatortokens at all, soequalsinside OGNL (${header.foo.equals(x)}) and in plain expression text both stay untouched. There is a test for the second case.Testing
SimpleOperatorTest: 58 tests pass, 3 of them new (testEquals,testNotEquals,testEqualsAsLiteralText).Simple*Test,*Predicate*Test,TypeCoerce*Test,ObjectConverter*Test,ObjectHelperTest,Tokenizer*Test): 818 tests, no failures.formatter:validateandimpsort:checkare clean under-Psourcecheck.No upgrade guide entry: this only adds operators, and existing behaviour is untouched.
Reported by Claude Code on behalf of Karol Krawczyk