From 3f2cc23bbb1f8f3e0088bd32223ac9f833b7c3e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Contreras=20Guill=C3=A9n?= Date: Sun, 24 May 2026 19:19:01 +0200 Subject: [PATCH 1/3] refactor(dsl): harden DSL with fail-loud contract, custom function registry, end-to-end tests; bump to 26.05.08 Removes silent-failure pockets across the parser, evaluator, and action executor; adds a pluggable function-registry extension point and three new DSL primitives; brings docs in line with the actual codebase; and bumps the project version to 26.05.08. Core correctness fixes - Parser: complex map-shaped action handling now throws ASTException with the original-map + reconstructed-syntax context instead of catch-and- return-null. Sub-rule action lists now route through the same parseActionList as the top level, so YAML-collapsed forEach / while / do actions parse the same way in either context. - Lexer/parser: ExpressionParser now attaches the parsed array index to VariableExpression (was parsed then discarded). - Numeric coercion: toNumberSafe and toBigDecimal converge on one contract: null treated as ZERO (financial-aggregation convention), non-numeric strings raise IllegalArgumentException with operand type info. - Conditions/actions: evaluateConditions, evaluateConditionalBlock, and executeActions all propagate RuntimeExceptions wrapped in RuleEvaluationException; the outer evaluateRules catch converts these to success=false with the action/condition index, debug string, and cause. - Action executor: unknown function names in `call` actions and unknown ArithmeticOperationType branches throw IllegalArgumentException with the registry-aware diagnostic. Arithmetic actions on non-numeric operands throw rather than silently no-op'ing. - Expression evaluator: matches() raises on bad regex pattern; getPropertyValue throws on missing bean accessor (maps still get null on missing key, matching json_get semantics); is_valid rejects unknown validation types with a list of supported types. - 16+ financial / formatting / utility functions (calculate_loan_payment, compound_interest, amortization, debt_to_income_ratio, credit_utilization, loan_to_value, calculate_apr, calculate_credit_score, calculate_risk_score, payment_history_score, format_currency, format_percentage, distance_between, time_hour, in_range, calculate_debt_ratio, calculate_ltv, calculate_payment_schedule): catch-and-return-null replaced with throws via a shared wrapFunctionError helper that prefixes the message with the function name and preserves already-good diagnostics. - dateadd / datediff / toLong: bad inputs and unknown units now throw with the list of supported units. Reactive correctness - ASTRulesEvaluationEngine.evaluateRulesReactive wraps the synchronous visitor in Mono.fromCallable(...).subscribeOn(Schedulers.boundedElastic()) so REST/JSON built-ins can block safely without stalling the Netty event loop. - CacheServiceImpl fire-and-forget writes now end with .onErrorComplete() before .subscribe(); errors are still logged via the existing doOnError. Variable-store safety - EvaluationContext switched from ConcurrentHashMap (rejects null values with NPE) to Collections.synchronizedMap(LinkedHashMap). json_get returning null on a missing path no longer NPEs when stored; iteration order is now insertion-stable. - @Data replaced with @Getter + selective @Setter; the three variable maps are final, so the auto-generated bulk setters that would bypass the typed setters' validateVariableName guard rails no longer exist. Extension point (new) - org.fireflyframework.rules.core.dsl.function.RuleFunction: functional interface, Object apply(Object[] args). - org.fireflyframework.rules.core.dsl.function.CustomFunctionRegistry: Spring @Component holding registered functions. Case-insensitive lookup. Checked before the built-in catalog, so custom functions may shadow built-ins. Wired through ASTRulesEvaluationEngine -> ActionExecutor -> ExpressionEvaluator via optional constructor parameters; existing callers keep working without changes. - ActionExecutor's default branch for `call ` delegates to the expression evaluator, so the same registered function is reachable from both expression (run / calculate / condition) and action (call) contexts. New built-in functions - coalesce(a, b, c, ...): returns the first non-null argument. - if_else(condition, thenValue, elseValue): inline ternary expression for use inside run / calculate / output contexts. - is_in_range(value, low, high): function form of the between operator. - calculate_age(birthDate[, asOfDate]), format_date(date[, pattern]), validate_email(value), validate_phone(value): function forms that complement the existing operator equivalents. Dead-code / stub removal - Deleted orphan AST classes never produced by the parser: AssignmentAction, AssignmentOperator, ArithmeticExpression, ArithmeticOperation. Removed visitor methods across ASTVisitor, ActionExecutor, ExpressionEvaluator, ValidationVisitor, PythonCodeGenerator, YamlDslValidator, and ASTRulesEvaluationEngine. - Deleted DSLParser.validateAST() (was a stub with no callers). - Deleted the commented-out HealthIndicator TODO block in DatabaseConfig (actuator wired in via the web module; a proper indicator belongs there). - Deleted the permanently @Disabled AuditTrailIntegrationTest (no testcontainers infrastructure was present; logic is exercised by the unit-level AuditHelperTest and AuditTrailServiceTest). - Cleaned two stale "this was the TODO that's now implemented" breadcrumbs. Test coverage - 345 tests, 0 failures, 0 errors, 0 skipped (was 323/0/5 before this work). - New: CustomFunctionRegistryTest (7), DslPrimitivesTest (9), DoWhileAnd- ConditionFunctionTest (3), EndToEndScenarioTest (5). - EndToEndScenarioTest exercises the full pipeline in one realistic loan- eligibility rule across approval / decline / tier-cutoff / empty-debt / circuit-breaker scenarios. - testCallAction split into a happy-path test against the `log` built-in and a typed-error test for unknown function names. - testDateFunctionsErrorHandling split into a happy path + two fail-loud assertions, matching the new contract. Documentation (updated to match the codebase) - README.md: features list rewritten; quick-start YAML uses canonical when/then/else syntax that actually parses; new Custom Functions and Error Contract sections. - docs/yaml-dsl-reference.md: new functions documented in their right sections; examples that used `calculate` for function calls corrected to `run` (calculate is pure-math-only); new Custom Functions extension-point section; Error Behavior Reference table contrasting old vs new contract for 11 situations; REST chain-friendly contract called out explicitly as the deliberate exception to fail-loud. - docs/developer-guide.md: all references to the four removed AST classes removed from the file-tree diagram, AST hierarchy diagram, visitor interface example, and visitor-implementation walkthrough; replaced with the actual current AST plus the new function/ package. - docs/architecture.md: Error Handling section expanded into a 12-row reference table. Version - 26.05.07 -> 26.05.08 across all five module poms and the parent. --- README.md | 171 ++++-- docs/architecture.md | 41 +- docs/developer-guide.md | 216 +++---- docs/yaml-dsl-reference.md | 131 ++++- fireflyframework-rule-engine-core/pom.xml | 2 +- .../rules/core/config/DatabaseConfig.java | 23 - .../rules/core/dsl/ASTVisitor.java | 6 +- .../core/dsl/action/AssignmentAction.java | 79 --- .../core/dsl/action/AssignmentOperator.java | 62 -- .../dsl/compiler/PythonCodeGenerator.java | 63 -- .../evaluation/ASTRulesEvaluationEngine.java | 149 ++--- .../evaluation/RuleEvaluationException.java | 36 ++ .../core/dsl/exception/ASTException.java | 4 + .../dsl/expression/ArithmeticExpression.java | 103 ---- .../dsl/expression/ArithmeticOperation.java | 89 --- .../dsl/function/CustomFunctionRegistry.java | 102 ++++ .../rules/core/dsl/function/RuleFunction.java | 48 ++ .../core/dsl/parser/ASTRulesDSLParser.java | 269 +++------ .../rules/core/dsl/parser/DSLParser.java | 18 - .../core/dsl/parser/ExpressionParser.java | 11 +- .../core/dsl/visitor/ActionExecutor.java | 147 ++--- .../core/dsl/visitor/EvaluationContext.java | 78 ++- .../core/dsl/visitor/ExpressionEvaluator.java | 554 ++++++++++-------- .../core/dsl/visitor/ValidationVisitor.java | 74 --- .../core/services/impl/CacheServiceImpl.java | 16 +- .../core/validation/YamlDslValidator.java | 14 - .../core/audit/AuditTrailIntegrationTest.java | 307 ---------- .../core/dsl/ASTParserIntegrationTest.java | 19 +- .../core/dsl/AdvancedDSLFeaturesTest.java | 75 ++- .../dsl/DoWhileAndConditionFunctionTest.java | 132 +++++ .../rules/core/dsl/DslPrimitivesTest.java | 223 +++++++ .../rules/core/dsl/EndToEndScenarioTest.java | 249 ++++++++ .../function/CustomFunctionRegistryTest.java | 156 +++++ .../pom.xml | 2 +- fireflyframework-rule-engine-models/pom.xml | 2 +- fireflyframework-rule-engine-sdk/pom.xml | 2 +- fireflyframework-rule-engine-web/pom.xml | 2 +- pom.xml | 2 +- 38 files changed, 1953 insertions(+), 1724 deletions(-) delete mode 100644 fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/action/AssignmentAction.java delete mode 100644 fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/action/AssignmentOperator.java create mode 100644 fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/evaluation/RuleEvaluationException.java delete mode 100644 fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/expression/ArithmeticExpression.java delete mode 100644 fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/expression/ArithmeticOperation.java create mode 100644 fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/function/CustomFunctionRegistry.java create mode 100644 fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/function/RuleFunction.java delete mode 100644 fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/audit/AuditTrailIntegrationTest.java create mode 100644 fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/DoWhileAndConditionFunctionTest.java create mode 100644 fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/DslPrimitivesTest.java create mode 100644 fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/EndToEndScenarioTest.java create mode 100644 fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/function/CustomFunctionRegistryTest.java diff --git a/README.md b/README.md index be24aa0..ef06cf3 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,8 @@ - [Requirements](#requirements) - [Installation](#installation) - [Quick Start](#quick-start) +- [Custom Functions](#custom-functions) +- [Error Contract](#error-contract) - [Configuration](#configuration) - [Documentation](#documentation) - [Contributing](#contributing) @@ -23,27 +25,30 @@ ## Overview -Firefly Framework Rule Engine provides a powerful business rule evaluation system based on a custom YAML DSL. Rules are defined in YAML format and parsed into an Abstract Syntax Tree (AST) for efficient evaluation. The engine supports complex conditions, arithmetic operations, loop constructs, function calls, REST API calls, and JsonPath expressions. +Firefly Framework Rule Engine provides a business rule evaluation system based on a custom YAML DSL. Rules are defined in YAML and parsed into an Abstract Syntax Tree (AST) for efficient evaluation. The engine supports rich conditions, arithmetic, loop constructs, function calls, REST API calls, JsonPath expressions, and a pluggable function-registry extension point. -The project is structured as a multi-module build with five sub-modules: interfaces (DTOs and validation), models (database entities and repositories), core (DSL parser, evaluator, and services), SDK (client library), and web (REST controllers). It features Python code generation for rule compilation, batch evaluation, audit trail tracking, and caching for rule definitions. +The project is structured as a multi-module Maven build with five sub-modules: `interfaces` (DTOs and validation), `models` (R2DBC entities and repositories), `core` (DSL parser, evaluator, services, function registry), `web` (Spring WebFlux REST controllers), and `sdk` (generated client). The engine ships with Python code generation for offline rule execution, batch evaluation, audit-trail tracking, and a dedicated cache layer. -The YAML DSL supports variables, conditionals, loops (while, do-while, for-each), list operations, circuit breaker actions, and nested rule invocations, making it suitable for complex business rule scenarios such as credit scoring, eligibility checks, and pricing calculations. +The YAML DSL supports input/computed/constant variable tiers, 30+ comparison operators, logical composition (and/or/not), loops (`forEach`, `while`, `do-while`), inline conditionals (`if/then/else`), 70+ built-in functions (financial, date, string, list, validation, REST, JSON, type-conversion), and circuit-breaker actions for early termination. ## Features -- Custom YAML DSL with lexer, parser, and AST-based evaluation -- Condition types: comparison, logical (AND/OR), expression-based -- Action types: set, calculate, conditional, loops (while, do-while, for-each), function calls -- Expression types: arithmetic, binary, unary, literals, variables, JsonPath, REST calls -- Python code generation and compilation for rule optimization -- Batch rule evaluation for processing multiple inputs -- Rule definition CRUD with database persistence via R2DBC -- Constants management for shared rule variables -- Audit trail tracking for all rule evaluations -- YAML DSL validation with syntax and naming convention checks -- Caching for rule definitions and evaluation results -- REST API controllers for evaluation, definitions, constants, audit, and validation -- Reactive APIs using Project Reactor +- Custom YAML DSL with dedicated lexer + recursive-descent parser + visitor-based evaluator +- 30+ comparison operators including `between`, `in_list`, `matches`, `is_email`, `is_credit_score`, etc. +- Logical composition (`and`, `or`, `not`) with short-circuit evaluation +- Action types: `set`, `calculate`, `run`, `call`, arithmetic (`add`/`subtract`/`multiply`/`divide`), list ops (`append`/`prepend`/`remove`), `forEach`, `while`, `do-while`, `if/then/else`, `circuit_breaker` +- 70+ built-in functions covering math, string, date, list, financial, validation, REST, JSON path, and type conversion +- Pluggable function registry (`CustomFunctionRegistry`) — register your own `RuleFunction` beans and call them from rules +- Constants tier loaded from the database with TTL caching; auto-detection of `UPPER_CASE` references in the AST +- Reactive evaluation API on Project Reactor; synchronous visitor scheduled on `Schedulers.boundedElastic()` so it never blocks the Netty event loop +- Python code generation for offline rule execution +- Batch evaluation with bounded concurrency and per-request timeouts +- Rule-definition CRUD with R2DBC persistence and a cached AST +- Audit-trail tracking for every evaluation (correlated, PII-masked) +- YAML DSL validation: syntax, naming-convention, dependency, function-existence +- RFC 7807 problem-detail error responses; correlation IDs propagated across the chain +- Fail-fast error contract: malformed rules, unknown functions, type-coercion errors, and bad regexes surface as `success=false` with precise diagnostics rather than silently flipping to the else branch +- Spring WebFlux controllers; OpenAPI 3 / Swagger UI ## Requirements @@ -61,70 +66,138 @@ The rule engine is a multi-module project. Include the modules you need: org.fireflyframework fireflyframework-rule-engine-core - 26.02.07 + 26.05.07 org.fireflyframework fireflyframework-rule-engine-interfaces - 26.02.07 + 26.05.07 org.fireflyframework fireflyframework-rule-engine-sdk - 26.02.07 + 26.05.07 ``` ## Quick Start +### Naming conventions +The DSL is strict about variable naming so the engine can resolve names without ambiguity: + +| Tier | Convention | Example | +| ---------------- | ------------ | ------------------------------------ | +| Input variables | `camelCase` | `creditScore`, `annualIncome` | +| Computed values | `snake_case` | `debt_to_income`, `risk_tier` | +| Database constants | `UPPER_CASE` | `MIN_CREDIT_SCORE`, `MAX_DTI` | + +### Example rule (YAML DSL) + ```yaml -# Example rule definition (YAML DSL) -name: credit-score-check -version: 1 +name: "Credit Eligibility" +description: "Two-stage credit and income gate" +version: "1.0.0" + inputs: - - name: creditScore - type: number - - name: income - type: number - -rules: - - name: evaluate-eligibility - conditions: - - field: creditScore - operator: ">=" - value: 700 - - field: income - operator: ">=" - value: 50000 - actions: - - set: - eligible: true - tier: "premium" - else: - - set: - eligible: false - tier: "standard" + creditScore: "number" + annualIncome: "number" + +constants: + - code: MIN_CREDIT_SCORE + defaultValue: 700 + - code: MIN_INCOME + defaultValue: 50000 + +when: + - creditScore at_least MIN_CREDIT_SCORE + - annualIncome at_least MIN_INCOME + +then: + - calculate debt_to_income as 0 # placeholder; real rules would compute this + - set tier to if_else(creditScore at_least 800, "PRIME", "PREFERRED") + - set eligible to true + +else: + - set tier to "STANDARD" + - set eligible to false + +output: + eligible: eligible + tier: tier ``` +### Calling the engine from Java + ```java @Service public class CreditCheckService { private final RulesEvaluationService evaluationService; - public Mono evaluate(Map inputs) { - RulesEvaluationRequestDTO request = new RulesEvaluationRequestDTO(); - request.setRuleCode("credit-score-check"); - request.setInputs(inputs); - return evaluationService.evaluate(request); + public Mono evaluate(Map inputData) { + RuleEvaluationByCodeRequestDTO request = RuleEvaluationByCodeRequestDTO.builder() + .ruleDefinitionCode("credit_eligibility_v1") + .inputData(inputData) + .build(); + return evaluationService.evaluateRuleByCode(request, exchange); + } +} +``` + +The same evaluation is also reachable via REST: `POST /api/v1/rules/evaluate/direct` (Base64-encoded YAML), `/evaluate/plain` (raw YAML), or `/evaluate/by-code` (stored rule code). + +## Custom Functions + +You can extend the DSL with your own functions without modifying the engine: + +```java +@Configuration +class MyRulesConfig { + + @Bean + CommandLineRunner registerCustomFunctions(CustomFunctionRegistry registry) { + return args -> { + registry.register("regional_risk", a -> + Set.of("CA", "NY").contains(a[0]) ? 10 : 0); + registry.register("fraud_score", a -> + fraudService.score(String.valueOf(a[0]))); + }; } } ``` +Then in a rule: + +```yaml +then: + - run risk_bump as regional_risk(region) + - run fraud as fraud_score(applicantId) +``` + +Custom functions are checked **before** the built-in catalog (so they can shadow a built-in if you choose). Names are matched case-insensitively. The same function is callable from both expression contexts (`run`/`calculate`) and `call`-action contexts. + +## Error Contract + +The engine fails loud by design — errors are never silently swallowed: + +| Situation | Result | +| -------------------------------------------- | --------------------------------------------------------------------------- | +| Unknown function name | `IllegalArgumentException` -> rule reports `success=false` with the name | +| Non-numeric string in arithmetic | `IllegalArgumentException` naming the operand | +| Bad regex pattern in `matches` | `IllegalArgumentException` naming the pattern | +| Missing bean property | `IllegalArgumentException` naming the class + property | +| Unknown `is_valid` validation type | `IllegalArgumentException` listing supported types | +| Action throws during execution | Rule reports `success=false` with action index + debug string + cause | +| Condition throws during evaluation | Rule reports `success=false` (does not silently flip to the else branch) | +| `circuit_breaker` action triggered | Rule reports `success=true` with `circuitBreakerTriggered=true` + message | +| REST function HTTP failure | Returns a structured `{success:false, error:true, message}` map (chain-friendly) | +| `getCachedAST` / cache read failure | Treated as cache miss (parse path); logged via `doOnError` | +| `set computed_var to null` (e.g., `json_get` missing path) | Variable is stored as null; the rule succeeds | + ## Configuration ```yaml diff --git a/docs/architecture.md b/docs/architecture.md index 6adaa22..74806c9 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -1158,16 +1158,37 @@ The `RestCallServiceImpl` includes comprehensive URL validation before executing - **Timing-safe comparison**: Hash verification uses `hmac.compare_digest()` to prevent timing attacks ### 4. Safe Code Evaluation -- **No unsafe reflection**: `ExpressionEvaluator.getPropertyValue()` uses public getter methods only; `field.setAccessible(true)` is not used -- **Division/modulo by zero**: `ExpressionEvaluator` and `ActionExecutor` throw `ArithmeticException` instead of returning null or silently failing -- **Short-circuit evaluation**: AND/OR operators use lazy evaluation to prevent unnecessary side effects -- **Thread-safe code generation**: `PythonCodeGenerator` uses `ThreadLocal` for mutable state to ensure safe concurrent compilation - -### 5. Error Handling -- Graceful degradation for missing constants -- Circuit breaker pattern for external dependencies -- Comprehensive logging for audit trails -- Null-safe audit context extraction (defaults to "system" when no web exchange is available) +- **No unsafe reflection**: `ExpressionEvaluator.getPropertyValue()` uses public getter methods only (`getX` / `isX`); `field.setAccessible(true)` is never called. A missing getter throws `IllegalArgumentException` with the class + property name rather than silently returning null. +- **Division/modulo by zero**: `ExpressionEvaluator` and `ActionExecutor` throw `ArithmeticException` rather than returning null or silently failing. +- **Short-circuit evaluation**: AND/OR operators use lazy evaluation to prevent unnecessary side effects. +- **Thread-safe code generation**: `PythonCodeGenerator` uses `ThreadLocal` for mutable state to ensure safe concurrent compilation. + +### 5. Error Handling (Fail-Loud Contract) + +The engine is intentionally non-silent. Errors propagate to the rule's `success=false` +result with a precise diagnostic message rather than being swallowed and producing a +plausible-but-wrong output. + +| Source of failure | Behaviour | +| ------------------------------------------ | ------------------------------------------------------------------------ | +| Unknown function name | `IllegalArgumentException` -> `success=false` | +| Non-numeric string in arithmetic | `IllegalArgumentException` naming the operand | +| Bad regex pattern in `matches` | `IllegalArgumentException` naming the pattern | +| Missing bean property | `IllegalArgumentException` naming class + property | +| Unknown `is_valid` validation type | `IllegalArgumentException` listing supported types | +| Unknown `dateadd`/`datediff` unit | `IllegalArgumentException` listing supported units | +| Action throws mid-execution | Rule reports `success=false` with action index + debug string + cause | +| Condition throws mid-evaluation | Rule reports `success=false` (does not silently flip to the else branch) | +| `circuit_breaker` action triggered | Rule reports `success=true` with `circuitBreakerTriggered=true` | +| Required constant missing in database | `success=false` listing the missing codes | +| REST function HTTP failure | Structured error map `{success:false, error:true, message:...}` (intentional chain-friendly contract; rules can branch on `response.success`) | +| Cache read failure | Treated as cache miss; logged via `doOnError` | + +Surrounding mechanisms: +- Graceful degradation for missing **optional** constants (with explicit `defaultValue`). +- Circuit breaker pattern for external dependencies. +- Comprehensive audit trail (every evaluation logged with correlation ID). +- Null-safe audit context extraction (defaults to "system" when no web exchange is available). ### 6. Cache Integrity - Cache invalidation on all CRUD operations (create, update, delete) for rule definitions diff --git a/docs/developer-guide.md b/docs/developer-guide.md index 8e82c5f..64ceaa1 100644 --- a/docs/developer-guide.md +++ b/docs/developer-guide.md @@ -207,34 +207,38 @@ org.fireflyframework.rules.core.dsl.ast/ │ └── BaseParser.java # Common parsing utilities ├── expression/ # Expression AST nodes │ ├── Expression.java # Base expression class -│ ├── BinaryExpression.java # Binary operations (+, -, *, /, etc.) +│ ├── BinaryExpression.java # Binary operations (+, -, *, /, %, **, comparisons) │ ├── UnaryExpression.java # Unary operations (-, !, validation ops) -│ ├── LiteralExpression.java # Literal values (numbers, strings, booleans) -│ ├── VariableExpression.java # Variable references +│ ├── LiteralExpression.java # Literal values (numbers, strings, booleans, arrays) +│ ├── VariableExpression.java # Variable references with property/index access │ ├── FunctionCallExpression.java # Function calls with parameters -│ ├── ArithmeticExpression.java # Complex arithmetic expressions -│ ├── JsonPathExpression.java # JSON path queries -│ ├── RestCallExpression.java # REST API calls +│ ├── JsonPathExpression.java # JSON path queries (visitor support; emitted by builders, not the parser) +│ ├── RestCallExpression.java # REST API calls (visitor support; emitted by builders, not the parser) │ ├── BinaryOperator.java # Binary operator enumeration │ ├── UnaryOperator.java # Unary operator enumeration │ └── ExpressionType.java # Expression type enumeration ├── condition/ # Condition AST nodes │ ├── Condition.java # Base condition class -│ ├── ComparisonCondition.java # Comparison operations (>, <, ==, etc.) +│ ├── ComparisonCondition.java # Comparison operations (>, <, ==, between, in_list, etc.) │ ├── LogicalCondition.java # Logical operations (AND, OR, NOT) │ ├── ExpressionCondition.java # Expression-based conditions │ └── ComparisonOperator.java # Comparison operator enumeration ├── action/ # Action AST nodes │ ├── Action.java # Base action class -│ ├── SetAction.java # Variable assignment (set var to value) -│ ├── CalculateAction.java # Arithmetic calculations -│ ├── AssignmentAction.java # Assignment operations (=, +=, etc.) -│ ├── FunctionCallAction.java # Function execution -│ ├── ConditionalAction.java # If-then-else actions -│ ├── ArithmeticAction.java # Arithmetic actions -│ ├── ListAction.java # List operations (append, prepend, etc.) -│ ├── CircuitBreakerAction.java # Execution control -│ └── AssignmentOperator.java # Assignment operator enumeration +│ ├── SetAction.java # Variable assignment (`set var to value`) +│ ├── CalculateAction.java # Pure-math arithmetic (`calculate var as expr`) +│ ├── RunAction.java # Function-call assignment (`run var as fn(...)`) +│ ├── FunctionCallAction.java # Standalone function execution (`call fn with [...]`) +│ ├── ConditionalAction.java # Inline if-then-else as an action +│ ├── ArithmeticAction.java # `add`/`subtract`/`multiply`/`divide` X to/from/by var +│ ├── ListAction.java # List operations (append, prepend, remove) +│ ├── CircuitBreakerAction.java # Early termination with a structured message +│ ├── ForEachAction.java # `forEach item in items: ...` +│ ├── WhileAction.java # `while condition: ...` +│ └── DoWhileAction.java # `do: ... while condition` +├── function/ # Extension point for user-defined functions +│ ├── RuleFunction.java # Functional interface: `Object apply(Object[] args)` +│ └── CustomFunctionRegistry.java # @Component holding registered functions ├── visitor/ # Visitor pattern implementations │ ├── EvaluationContext.java # Execution context and state │ ├── ExpressionEvaluator.java # Expression evaluation visitor @@ -663,64 +667,33 @@ public abstract class ASTNode { ``` ASTNode (abstract base) ├── Expression (abstract) -│ ├── LiteralExpression -│ │ ├── NumberLiteral (integers, decimals) -│ │ ├── StringLiteral (quoted strings) -│ │ ├── BooleanLiteral (true/false) -│ │ └── NullLiteral (null values) -│ ├── VariableExpression (variable references) -│ ├── BinaryExpression -│ │ ├── ArithmeticExpression (+, -, *, /, %, **) -│ │ ├── ComparisonExpression (>, <, ==, !=, >=, <=) -│ │ ├── LogicalExpression (AND, OR) -│ │ ├── StringExpression (contains, starts_with, ends_with) -│ │ └── ListExpression (in, not_in) -│ ├── UnaryExpression -│ │ ├── ArithmeticUnary (-, +) -│ │ ├── LogicalUnary (NOT, !) -│ │ └── ValidationUnary (is_positive, is_email, is_phone, etc.) -│ ├── FunctionCallExpression -│ │ ├── MathFunctions (abs, round, ceil, floor, etc.) -│ │ ├── StringFunctions (length, substring, upper, lower, etc.) -│ │ ├── DateFunctions (now, date_add, date_diff, etc.) -│ │ └── CustomFunctions (user-defined functions) -│ ├── ArithmeticExpression (complex multi-operand arithmetic) -│ ├── JsonPathExpression (JSON path queries) -│ └── RestCallExpression (REST API calls) +│ ├── LiteralExpression # numbers, strings, booleans, null, array literals +│ ├── VariableExpression # variable refs with optional property path / index access +│ ├── BinaryExpression # +, -, *, /, %, **, comparisons, and/or, contains, starts_with, etc. +│ ├── UnaryExpression # -, +, NOT, EXISTS, IS_NULL, IS_EMAIL, IS_POSITIVE, etc. +│ ├── FunctionCallExpression # math, string, date, list, financial, validation, REST, JSON funcs +│ ├── JsonPathExpression # visitor-supported; emitted by builders, not the lexer/parser +│ └── RestCallExpression # visitor-supported; emitted by builders, not the lexer/parser ├── Condition (abstract) -│ ├── ComparisonCondition -│ │ ├── SimpleComparison (var > value) -│ │ ├── BetweenCondition (var between min and max) -│ │ ├── InCondition (var in [list]) -│ │ └── ValidationCondition (var is_positive) -│ ├── LogicalCondition -│ │ ├── AndCondition (condition1 AND condition2) -│ │ ├── OrCondition (condition1 OR condition2) -│ │ └── NotCondition (NOT condition) -│ └── ExpressionCondition (expression-based conditions) +│ ├── ComparisonCondition # `>=`, `at_least`, `between ... and ...`, `in_list [...]`, `is_email`, etc. +│ ├── LogicalCondition # AND / OR / NOT composition +│ └── ExpressionCondition # wraps any boolean-valued Expression └── Action (abstract) - ├── SetAction (set variable to value) - ├── CalculateAction (calculate variable as expression) - ├── AssignmentAction - │ ├── SimpleAssignment (var = value) - │ ├── AddAssignment (var += value) - │ ├── SubtractAssignment (var -= value) - │ ├── MultiplyAssignment (var *= value) - │ └── DivideAssignment (var /= value) - ├── FunctionCallAction (call function with parameters) - ├── ConditionalAction - │ ├── IfAction (if condition then actions) - │ ├── IfElseAction (if condition then actions else actions) - │ └── SwitchAction (switch-case logic) - ├── ArithmeticAction (arithmetic operations as actions) - ├── ListAction - │ ├── AppendAction (append to list) - │ ├── PrependAction (prepend to list) - │ ├── RemoveAction (remove from list) - │ └── ClearAction (clear list) - └── CircuitBreakerAction (execution control and error handling) + ├── SetAction # `set var to value` + ├── CalculateAction # `calculate var as ` + ├── RunAction # `run var as ` + ├── FunctionCallAction # `call fn with [args]` + ├── ConditionalAction # `if cond then actions [else actions]` + ├── ArithmeticAction # `add X to var`, `subtract X from var`, `multiply var by X`, `divide var by X` + ├── ListAction # `append X to list`, `prepend X to list`, `remove X from list` + ├── ForEachAction # `forEach item[, index] in items: actions` + ├── WhileAction # `while cond: actions` + ├── DoWhileAction # `do: actions while cond` + └── CircuitBreakerAction # `circuit_breaker "MESSAGE"` -- early termination ``` +> **Note:** The DSL was simplified in 2026-05 to remove orphan AST classes that were never produced by the parser. The compound-assignment family (`+=`, `-=`, etc.) and the multi-operand `ArithmeticExpression` n-ary form are no longer present. Use `add`/`subtract`/`multiply`/`divide` arithmetic actions or `calculate`/`run` with `+`/`-`/`*`/`/` for the same outcomes. + ### 🔢 **Expression Nodes: Representing Computable Values** **What is an Expression?** @@ -1316,11 +1289,9 @@ public class ActionParser extends BaseParser { return parseConditionalAction(); } - // Handle arithmetic actions (var += value) - if (check(TokenType.IDENTIFIER) && checkNext(TokenType.ASSIGN, TokenType.PLUS_ASSIGN, - TokenType.MINUS_ASSIGN, TokenType.MULTIPLY_ASSIGN)) { - return parseAssignmentAction(); - } + // Arithmetic actions are emitted by `add`/`subtract`/`multiply`/`divide` keywords + // (parseArithmeticAction), not by `=` / `+=` operators -- the latter are not part + // of the action DSL. throw error("Expected action statement"); } @@ -1400,29 +1371,31 @@ The `ASTVisitor` interface is the heart of the visitor pattern implementation: public interface ASTVisitor { // Expression visitors - handle value computation - T visitBinaryExpression(BinaryExpression node); // a + b, a > b, etc. - T visitUnaryExpression(UnaryExpression node); // -a, !a, is_positive(a) - T visitVariableExpression(VariableExpression node); // creditScore, income - T visitLiteralExpression(LiteralExpression node); // 42, "hello", true - T visitFunctionCallExpression(FunctionCallExpression node); // max(a, b) - T visitArithmeticExpression(ArithmeticExpression node); // Complex arithmetic - T visitJsonPathExpression(JsonPathExpression node); // $.user.name - T visitRestCallExpression(RestCallExpression node); // REST API calls + T visitBinaryExpression(BinaryExpression node); // a + b, a > b, a and b, etc. + T visitUnaryExpression(UnaryExpression node); // -a, !a, is_positive(a) + T visitVariableExpression(VariableExpression node); // creditScore, user.profile.name + T visitLiteralExpression(LiteralExpression node); // 42, "hello", true, [1,2,3] + T visitFunctionCallExpression(FunctionCallExpression node); // max(a, b), if_else(...), coalesce(...) + T visitJsonPathExpression(JsonPathExpression node); // structural node; emitted by builders + T visitRestCallExpression(RestCallExpression node); // structural node; emitted by builders // Condition visitors - handle boolean logic - T visitComparisonCondition(ComparisonCondition node); // a > b - T visitLogicalCondition(LogicalCondition node); // a AND b - T visitExpressionCondition(ExpressionCondition node); // Expression as condition + T visitComparisonCondition(ComparisonCondition node); // a > b, a between x and y, etc. + T visitLogicalCondition(LogicalCondition node); // a AND b, a OR b, NOT a + T visitExpressionCondition(ExpressionCondition node); // any Expression as a condition // Action visitors - handle state changes T visitSetAction(SetAction node); // set var to value - T visitCalculateAction(CalculateAction node); // calculate var as expr - T visitAssignmentAction(AssignmentAction node); // var = value, var += value - T visitFunctionCallAction(FunctionCallAction node); // call function() - T visitConditionalAction(ConditionalAction node); // if-then-else - T visitArithmeticAction(ArithmeticAction node); // Arithmetic as action - T visitListAction(ListAction node); // List operations - T visitCircuitBreakerAction(CircuitBreakerAction node); // Error handling + T visitCalculateAction(CalculateAction node); // calculate var as expr (pure-math only) + T visitRunAction(RunAction node); // run var as fn(...) / json_get(...) / rest_*(...) + T visitFunctionCallAction(FunctionCallAction node); // call fn with [args] + T visitConditionalAction(ConditionalAction node); // if cond then ... else ... + T visitArithmeticAction(ArithmeticAction node); // add/subtract/multiply/divide ... to/from/by var + T visitListAction(ListAction node); // append/prepend/remove ... to/from list + T visitCircuitBreakerAction(CircuitBreakerAction node); // circuit_breaker "MESSAGE" + T visitForEachAction(ForEachAction node); // forEach item[, index] in items: actions + T visitWhileAction(WhileAction node); // while cond: actions + T visitDoWhileAction(DoWhileAction node); // do: actions while cond } ``` @@ -1578,44 +1551,33 @@ public class ActionExecutor implements ASTVisitor { } @Override - public Void visitAssignmentAction(AssignmentAction node) { - // Step 1: Evaluate the new value + public Void visitArithmeticAction(ArithmeticAction node) { + // Equivalent of an "in-place" compound assignment, expressed via dedicated keywords + // in the DSL (`add X to var`, `subtract X from var`, etc.). Both operands must be + // numeric; non-numeric operands raise IllegalArgumentException so authoring bugs + // surface immediately rather than silently no-op'ing. Object value = node.getValue().accept(expressionEvaluator); + Object current = context.getVariable(node.getVariableName()); - // Step 2: Apply the assignment operator - switch (node.getOperator()) { - case ASSIGN -> { - // Simple assignment: var = value - context.setComputedVariable(node.getVariableName(), value); - } - case ADD_ASSIGN -> { - // Addition assignment: var += value - Object currentValue = context.getVariable(node.getVariableName()); - if (currentValue instanceof Number && value instanceof Number) { - // Numeric addition - BigDecimal current = toBigDecimal(currentValue); - BigDecimal addValue = toBigDecimal(value); - context.setComputedVariable(node.getVariableName(), current.add(addValue)); - } else { - // String concatenation - context.setComputedVariable(node.getVariableName(), - currentValue.toString() + value.toString()); - } - } - case SUBTRACT_ASSIGN -> { - // Subtraction assignment: var -= value - Object currentValue = context.getVariable(node.getVariableName()); - if (currentValue instanceof Number && value instanceof Number) { - BigDecimal current = toBigDecimal(currentValue); - BigDecimal subValue = toBigDecimal(value); - context.setComputedVariable(node.getVariableName(), current.subtract(subValue)); - } else { - throw new ASTException("Cannot subtract non-numeric values"); - } - } - // ... other assignment operators + if (!(current instanceof Number) || !(value instanceof Number)) { + throw new IllegalArgumentException( + "Arithmetic action requires numeric operands"); } + BigDecimal currentNum = toBigDecimal(current); + BigDecimal valueNum = toBigDecimal(value); + BigDecimal result = switch (node.getOperation()) { + case ADD -> currentNum.add(valueNum); + case SUBTRACT -> currentNum.subtract(valueNum); + case MULTIPLY -> currentNum.multiply(valueNum); + case DIVIDE -> { + if (valueNum.signum() == 0) { + throw new ArithmeticException("Division by zero in arithmetic action"); + } + yield currentNum.divide(valueNum, 10, RoundingMode.HALF_UP); + } + }; + context.setComputedVariable(node.getVariableName(), result); return null; } diff --git a/docs/yaml-dsl-reference.md b/docs/yaml-dsl-reference.md index e7da86b..8a75646 100644 --- a/docs/yaml-dsl-reference.md +++ b/docs/yaml-dsl-reference.md @@ -1017,19 +1017,35 @@ conditions: - run uppercase as upper(name) - run lowercase as lower(email) - run trimmed as trim(input_text) -- calculate length as length(description) +- run name_length as length(description) # Date/time functions -- calculate current_date as now() -- calculate formatted_date as format_date(date_value, "yyyy-MM-dd") -- calculate age_years as calculate_age(birth_date) - -# Validation functions -- calculate is_valid_email as validate_email(email_address) -- calculate is_valid_phone as validate_phone(phone_number) -- calculate is_business_day as is_business_day(date_value) +- run current_date as now() +- run today_date as today() +- run formatted_date as format_date(date_value, "yyyy-MM-dd") +- run pretty_date as format_date(date_value, "dd MMM yyyy") +- run age_years as calculate_age(birth_date) # from today +- run age_at_event as calculate_age(birth_date, event_date) +- run plus_thirty as dateadd(today_date, 30, "days") +- run days_between as datediff(start_date, end_date, "days") + +# Validation functions (function-call form complements the `is_email`/`is_phone` operators) +- run email_ok as validate_email(email_address) +- run phone_ok as validate_phone(phone_number) +- run is_business_day_today as is_business_day(today_date) +- run any_check as is_valid(value, "email") # 12 known types; unknown -> error + +# Null-handling & conditional helpers (DSL primitives) +- run preferred_name as coalesce(nickname, full_name, "Anonymous") # first non-null wins +- run tier as if_else(creditScore at_least 750, "PRIME", "STANDARD") # inline ternary +- run within_window as is_in_range(score, 600, 850) # function form of `between` ``` +> **Important:** Function calls and REST/JSON expressions are only legal in `run` actions and in +> expression contexts (function arguments, conditions, output mappings). The `calculate` action +> is restricted to pure mathematical expressions (`+ - * / % **`) on numeric inputs -- +> attempting to use a function call inside `calculate` raises a clean validation error. + ### REST Call Expressions ```yaml @@ -1042,12 +1058,18 @@ conditions: - run validation_result as rest_post("https://validator.com/check", {"email": email, "phone": phone}) # PUT requests with headers -- calculate update_result as rest_put("https://api.example.com/users/123", user_data, {"Authorization": "Bearer " + token}) +- run update_result as rest_put("https://api.example.com/users/123", user_data, {"Authorization": "Bearer " + token}) # DELETE requests -- calculate delete_result as rest_delete("https://api.example.com/records/" + record_id) +- run delete_result as rest_delete("https://api.example.com/records/" + record_id) ``` +> **REST error contract:** On HTTP failure (non-2xx, network error, DNS, timeout), the REST +> functions return a structured map: `{success: false, error: true, message: "
"}`. +> Rules can branch on `response.success` to handle errors gracefully. This is intentional +> "chain-friendly" behaviour and is the only place in the engine where a failure does not +> raise an exception; everywhere else, errors propagate as `success=false` on the rule result. + ### JSON Path Expressions ```yaml @@ -1219,15 +1241,29 @@ conditions: ```yaml # Distance and location -- calculate distance as distance_between(lat1, lon1, lat2, lon2) +- run distance as distance_between(lat1, lon1, lat2, lon2) # Data security -- calculate encrypted as encrypt(data, key) -- calculate decrypted as decrypt(encrypted_data, key) -- calculate masked as mask_data(sensitive_data, mask_pattern) +- run encrypted as encrypt(data, key) +- run decrypted as decrypt(encrypted_data, key) +- run masked as mask_data(sensitive_data, mask_pattern) # Advanced financial calculations -- calculate payment_schedule as calculate_payment_schedule(principal, rate, term) +- run payment_schedule as calculate_payment_schedule(principal, rate, term) +``` + +### Null-handling, Conditional, and Range Helpers + +```yaml +# coalesce: first non-null wins (NULL-coalescing default) +- run preferred_name as coalesce(nickname, full_name, "Anonymous") + +# if_else: inline ternary expression (avoids a full `if/then/else` action block) +- run tier as if_else(creditScore at_least 750, "PRIME", "STANDARD") +- run discount as if_else(membership equals "GOLD", 0.20, 0.05) + +# is_in_range: function form of the `between` operator (inclusive both ends) +- run score_band_ok as is_in_range(score, 600, 850) ``` ### Logging and Audit Functions @@ -1239,6 +1275,39 @@ conditions: - call send_notification with ["recipient", "message"] ``` +### Custom Functions (Extension Point) + +Register your own functions in a Spring `@Bean` and call them from any rule: + +```java +@Configuration +class MyRulesConfig { + @Bean + CommandLineRunner registerCustomFunctions(CustomFunctionRegistry registry) { + return args -> { + registry.register("regional_risk", a -> + Set.of("CA", "NY").contains(a[0]) ? 10 : 0); + registry.register("fraud_score", a -> + fraudService.score(String.valueOf(a[0]))); + }; + } +} +``` + +```yaml +# Then use them like any built-in function: +when: + - fraud_score(applicantId) at_most MAX_FRAUD_SCORE +then: + - run risk_bump as regional_risk(region) + - run is_clean as fraud_score(applicantId) less_than 50 +``` + +**Resolution order:** Custom functions are checked **before** the built-in catalog -- if you +register a function with the same name as a built-in (e.g., `max`), your function wins. +Names are matched case-insensitively. The same registered function is reachable from both +expression contexts (`run` / `calculate` arg / condition) and action contexts (`call`). + --- ## Advanced Features @@ -1432,12 +1501,12 @@ then: - if has_address equals true then run zip_code as json_get(customer_data, "addressInfo.zipCode") # Validation if required - - if requiresValidation equals true then calculate email_valid as validate_email(customer_email) - - if requiresValidation equals true then calculate phone_valid as validate_phone(customer_phone) + - if requiresValidation equals true then run email_valid as validate_email(customer_email) + - if requiresValidation equals true then run phone_valid as validate_phone(customer_phone) # Set processing status - set data_enrichment_complete to true - - calculate processing_timestamp as now() + - run processing_timestamp as now() else: - set data_enrichment_complete to false @@ -1682,6 +1751,30 @@ output: - Validation operators in conditional expressions - Mixed validation and comparison operators in complex conditions +### Version 26.05 - Fail-Loud Error Contract & New Primitives + +- **New primitives:** `coalesce(...)`, `if_else(cond, then, else)`, `is_in_range(value, low, high)`. +- **New built-ins:** `calculate_age(birth[, asOf])`, `format_date(date[, pattern])`, `validate_email(value)`, `validate_phone(value)` (function-call form complements existing `is_email`/`is_phone` operators). +- **Extension point:** `CustomFunctionRegistry` Spring bean lets applications register their own `RuleFunction` implementations callable from rules. +- **Error contract:** the engine now fails loud by design rather than silently swallowing errors. See the table below. +- **Sub-rule action parity:** sub-rules under `rules:` now accept the same map-shaped actions as the top level (e.g., YAML-collapsed `- forEach x in xs: ...`). + +#### Error Behavior Reference + +| Situation | Old (pre-26.05) | New | +| -------------------------------------------- | ------------------------------ | -------------------------------------------------------------------- | +| Unknown function name | `log.warn` + null | `IllegalArgumentException` -> rule reports `success=false` | +| Non-numeric string in arithmetic | silently coerced to ZERO | `IllegalArgumentException` naming the operand | +| Bad regex pattern in `matches` | `false` | `IllegalArgumentException` naming the pattern | +| Missing bean property | `log.warn` + null | `IllegalArgumentException` naming class + property (maps still get null on missing key) | +| Unknown `is_valid(value, type)` type | `false` | `IllegalArgumentException` listing supported types | +| Unknown `dateadd`/`datediff` unit | `log.warn` + null | `IllegalArgumentException` listing supported units | +| Action throws during execution | logged + next action runs | Rule reports `success=false` with action index + cause | +| Condition throws during evaluation | silently flips to else branch | Rule reports `success=false` with the real cause | +| `circuit_breaker` action triggered | (unchanged) | Rule reports `success=true` with `circuitBreakerTriggered=true` | +| REST function HTTP failure | structured error map (unchanged) | Structured error map (unchanged -- intentional chain-friendly form) | +| `set var to null` (e.g., `json_get` missing) | NPE caught silently | Variable stored as null; rule succeeds | + --- --- diff --git a/fireflyframework-rule-engine-core/pom.xml b/fireflyframework-rule-engine-core/pom.xml index 190664d..39dac01 100644 --- a/fireflyframework-rule-engine-core/pom.xml +++ b/fireflyframework-rule-engine-core/pom.xml @@ -6,7 +6,7 @@ org.fireflyframework fireflyframework-rule-engine - 26.05.07 + 26.05.08 fireflyframework-rule-engine-core diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/config/DatabaseConfig.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/config/DatabaseConfig.java index 09abda5..4704468 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/config/DatabaseConfig.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/config/DatabaseConfig.java @@ -136,29 +136,6 @@ public ConnectionFactory connectionFactory() { return connectionPool; } - // Health indicator removed - requires actuator dependency - // TODO: Add back when actuator is available - /* - @Bean - public HealthIndicator connectionPoolHealthIndicator() { - return () -> { - try { - return Health.up() - .withDetail("pool.initialSize", initialSize) - .withDetail("pool.maxSize", maxSize) - .withDetail("pool.minIdle", minIdle) - .withDetail("pool.maxIdleTime", maxIdleTime.toString()) - .withDetail("pool.maxAcquireTime", maxAcquireTime.toString()) - .build(); - } catch (Exception e) { - return Health.down() - .withDetail("error", e.getMessage()) - .build(); - } - }; - } - */ - /** * Scheduled task to log connection pool statistics. * Helps monitor pool performance and identify potential issues. diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/ASTVisitor.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/ASTVisitor.java index 9a0a2fc..167afe4 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/ASTVisitor.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/ASTVisitor.java @@ -39,17 +39,15 @@ public interface ASTVisitor { T visitVariableExpression(VariableExpression node); T visitLiteralExpression(LiteralExpression node); T visitFunctionCallExpression(FunctionCallExpression node); - T visitArithmeticExpression(ArithmeticExpression node); T visitJsonPathExpression(JsonPathExpression node); T visitRestCallExpression(RestCallExpression node); - + // Condition visitors T visitComparisonCondition(ComparisonCondition node); T visitLogicalCondition(LogicalCondition node); T visitExpressionCondition(ExpressionCondition node); - + // Action visitors - T visitAssignmentAction(AssignmentAction node); T visitFunctionCallAction(FunctionCallAction node); T visitConditionalAction(ConditionalAction node); T visitCalculateAction(CalculateAction node); diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/action/AssignmentAction.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/action/AssignmentAction.java deleted file mode 100644 index 502688a..0000000 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/action/AssignmentAction.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * Copyright 2024-2026 Firefly Software Foundation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.fireflyframework.rules.core.dsl.action; - -import org.fireflyframework.rules.core.dsl.ASTVisitor; -import org.fireflyframework.rules.core.dsl.SourceLocation; -import org.fireflyframework.rules.core.dsl.expression.Expression; -import lombok.Data; -import lombok.EqualsAndHashCode; - -/** - * Represents a variable assignment action. - * Examples: set result to "approved", assign score to calculateScore(customer) - */ -@Data -@EqualsAndHashCode(callSuper = true) -public class AssignmentAction extends Action { - - /** - * Name of the variable to assign to - */ - private String variableName; - - /** - * Expression to evaluate and assign to the variable - */ - private Expression value; - - /** - * Assignment operator type - */ - private AssignmentOperator operator; - - public AssignmentAction(SourceLocation location, String variableName, Expression value) { - super(location); - this.variableName = variableName; - this.value = value; - this.operator = AssignmentOperator.ASSIGN; - } - - public AssignmentAction(SourceLocation location, String variableName, Expression value, AssignmentOperator operator) { - super(location); - this.variableName = variableName; - this.value = value; - this.operator = operator; - } - - @Override - public T accept(ASTVisitor visitor) { - return visitor.visitAssignmentAction(this); - } - - @Override - public boolean hasVariableReferences() { - return value.hasVariableReferences(); - } - - @Override - public String toDebugString() { - return String.format("%s %s %s", variableName, operator.getSymbol(), value.toDebugString()); - } -} - - - diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/action/AssignmentOperator.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/action/AssignmentOperator.java deleted file mode 100644 index 17ca124..0000000 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/action/AssignmentOperator.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright 2024-2026 Firefly Software Foundation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.fireflyframework.rules.core.dsl.action; - -/** - * Enumeration of assignment operators. - */ -public enum AssignmentOperator { - ASSIGN("to", "="), - ADD_ASSIGN("add", "+="), - SUBTRACT_ASSIGN("subtract", "-="), - MULTIPLY_ASSIGN("multiply", "*="), - DIVIDE_ASSIGN("divide", "/="); - - private final String keyword; - private final String symbol; - - AssignmentOperator(String keyword, String symbol) { - this.keyword = keyword; - this.symbol = symbol; - } - - public String getKeyword() { - return keyword; - } - - public String getSymbol() { - return symbol; - } - - public static AssignmentOperator fromKeyword(String keyword) { - for (AssignmentOperator op : values()) { - if (op.keyword.equals(keyword)) { - return op; - } - } - throw new IllegalArgumentException("Unknown assignment operator: " + keyword); - } - - public static AssignmentOperator fromSymbol(String symbol) { - for (AssignmentOperator op : values()) { - if (symbol.equals(op.symbol)) { - return op; - } - } - throw new IllegalArgumentException("Unknown assignment operator symbol: " + symbol); - } -} diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/compiler/PythonCodeGenerator.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/compiler/PythonCodeGenerator.java index b801255..b31d318 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/compiler/PythonCodeGenerator.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/compiler/PythonCodeGenerator.java @@ -492,23 +492,6 @@ public String visitFunctionCallExpression(FunctionCallExpression node) { } } - @Override - public String visitArithmeticExpression(ArithmeticExpression node) { - // ArithmeticExpression is a complex expression with multiple operands - String operation = node.getOperation().getName(); - List operands = node.getOperands().stream() - .map(operand -> operand.accept(this)) - .collect(Collectors.toList()); - - return switch (operation.toLowerCase()) { - case "sum" -> String.format("sum([%s])", String.join(", ", operands)); - case "average", "avg" -> String.format("statistics.mean([%s])", String.join(", ", operands)); - case "max" -> String.format("max(%s)", String.join(", ", operands)); - case "min" -> String.format("min(%s)", String.join(", ", operands)); - default -> String.format("firefly_%s(%s)", operation, String.join(", ", operands)); - }; - } - @Override public String visitJsonPathExpression(JsonPathExpression node) { String sourceExpression = node.getSourceExpression().accept(this); @@ -614,24 +597,6 @@ public String visitExpressionCondition(ExpressionCondition node) { } // Action visitors - @Override - public String visitAssignmentAction(AssignmentAction node) { - String value = node.getValue().accept(this); - String varName = sanitizeVariableName(node.getVariableName()); - - return switch (node.getOperator()) { - case ASSIGN -> String.format("%s['%s'] = %s", CONTEXT_VAR, varName, value); - case ADD_ASSIGN -> String.format("%s['%s'] = %s.get('%s', 0) + %s", - CONTEXT_VAR, varName, CONTEXT_VAR, varName, value); - case SUBTRACT_ASSIGN -> String.format("%s['%s'] = %s.get('%s', 0) - %s", - CONTEXT_VAR, varName, CONTEXT_VAR, varName, value); - case MULTIPLY_ASSIGN -> String.format("%s['%s'] = %s.get('%s', 1) * %s", - CONTEXT_VAR, varName, CONTEXT_VAR, varName, value); - case DIVIDE_ASSIGN -> String.format("%s['%s'] = %s.get('%s', 1) / %s", - CONTEXT_VAR, varName, CONTEXT_VAR, varName, value); - }; - } - @Override public String visitFunctionCallAction(FunctionCallAction node) { String functionName = mapFunctionToPython(node.getFunctionName()); @@ -942,9 +907,6 @@ private Set extractSetVariablesFromActions(List actions) { } else if (action instanceof CalculateAction) { CalculateAction calcAction = (CalculateAction) action; setVariables.add(calcAction.getResultVariable()); - } else if (action instanceof AssignmentAction) { - AssignmentAction assignAction = (AssignmentAction) action; - setVariables.add(assignAction.getVariableName()); } } @@ -1111,31 +1073,6 @@ private String mapComparisonOperatorToPython(ComparisonOperator operator) { }; } - private String mapArithmeticOperationToPython(ArithmeticOperation operation) { - return switch (operation) { - // Basic arithmetic - case ADD -> "+"; - case SUBTRACT -> "-"; - case MULTIPLY -> "*"; - case DIVIDE -> "/"; - case MODULO -> "%"; - case POWER -> "**"; - - // Mathematical functions - case ABS -> "abs"; - case MIN -> "min"; - case MAX -> "max"; - case ROUND -> "round"; - case FLOOR -> "math.floor"; - case CEIL -> "math.ceil"; - case SQRT -> "math.sqrt"; - case SUM -> "sum"; - case AVERAGE -> "firefly_average"; - - default -> throw new UnsupportedOperationException("Unsupported arithmetic operation: " + operation); - }; - } - private String mapFunctionToPython(String functionName) { return switch (functionName.toLowerCase()) { // Built-in Python functions diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/evaluation/ASTRulesEvaluationEngine.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/evaluation/ASTRulesEvaluationEngine.java index 43be71b..9808189 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/evaluation/ASTRulesEvaluationEngine.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/evaluation/ASTRulesEvaluationEngine.java @@ -23,6 +23,7 @@ import org.fireflyframework.rules.core.dsl.condition.ExpressionCondition; import org.fireflyframework.rules.core.dsl.condition.LogicalCondition; import org.fireflyframework.rules.core.dsl.expression.*; +import org.fireflyframework.rules.core.dsl.function.CustomFunctionRegistry; import org.fireflyframework.rules.core.dsl.model.ASTRulesDSL; import org.fireflyframework.rules.core.dsl.parser.ASTRulesDSLParser; import org.fireflyframework.rules.core.dsl.visitor.ActionExecutor; @@ -39,6 +40,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; import reactor.core.publisher.Mono; +import reactor.core.scheduler.Schedulers; import java.util.*; import java.util.stream.Collectors; @@ -55,40 +57,64 @@ public class ASTRulesEvaluationEngine { private final ConstantService constantService; private final RestCallService restCallService; private final JsonPathService jsonPathService; + private final CustomFunctionRegistry customFunctions; /** - * Primary constructor for Spring dependency injection - * RestCallService and JsonPathService are optional and will use defaults if not provided + * Primary constructor for Spring dependency injection. + *

+ * {@code restCallService}, {@code jsonPathService}, and {@code customFunctions} are + * optional. When absent, REST/JSON built-ins fall back to internal default implementations + * and no user-registered functions are available. */ @Autowired public ASTRulesEvaluationEngine(ASTRulesDSLParser parser, ConstantService constantService, @Autowired(required = false) RestCallService restCallService, - @Autowired(required = false) JsonPathService jsonPathService) { + @Autowired(required = false) JsonPathService jsonPathService, + @Autowired(required = false) CustomFunctionRegistry customFunctions) { this.parser = Objects.requireNonNull(parser, "ASTRulesDSLParser cannot be null"); this.constantService = Objects.requireNonNull(constantService, "ConstantService cannot be null"); this.restCallService = restCallService != null ? restCallService : new RestCallServiceImpl(); this.jsonPathService = jsonPathService != null ? jsonPathService : new JsonPathServiceImpl(); + this.customFunctions = customFunctions; } /** - * Constructor with default REST and JSON services (for testing) + * Test-friendly constructor with default REST/JSON services and no custom function registry. */ public ASTRulesEvaluationEngine(ASTRulesDSLParser parser, ConstantService constantService) { this.parser = Objects.requireNonNull(parser, "ASTRulesDSLParser cannot be null"); this.constantService = Objects.requireNonNull(constantService, "ConstantService cannot be null"); this.restCallService = new RestCallServiceImpl(); this.jsonPathService = new JsonPathServiceImpl(); + this.customFunctions = null; + } + + /** + * Test-friendly 4-arg constructor (parser, constantService, restCallService, jsonPathService) + * preserved for backward compatibility with existing tests. Delegates to the 5-arg form with + * a {@code null} custom function registry. + */ + public ASTRulesEvaluationEngine(ASTRulesDSLParser parser, + ConstantService constantService, + RestCallService restCallService, + JsonPathService jsonPathService) { + this(parser, constantService, restCallService, jsonPathService, null); } /** - * Evaluate rules against the provided input data + * Evaluate rules against the provided input data. + *

+ * The visitor-based evaluator is synchronous and may transitively block (e.g., built-in + * REST/JSON functions). To avoid stalling the Netty event loop, the evaluation step is + * scheduled on {@code Schedulers.boundedElastic()} which is designed for blocking work. */ public Mono evaluateRulesReactive(String rulesDefinition, Map inputData) { long startTime = System.currentTimeMillis(); return parser.parseRulesReactive(rulesDefinition) .flatMap(rulesDSL -> createEvaluationContextReactive(rulesDSL, inputData) - .map(context -> evaluateRules(rulesDSL, context))) + .flatMap(context -> Mono.fromCallable(() -> evaluateRules(rulesDSL, context)) + .subscribeOn(Schedulers.boundedElastic()))) .onErrorResume(error -> { long executionTime = System.currentTimeMillis() - startTime; JsonLogger.error(log, "Rules evaluation failed", error); @@ -113,12 +139,14 @@ public ASTRulesEvaluationResult evaluateRules(String rulesDefinition, Map evaluateRulesReactive(ASTRulesDSL rulesDSL, Map inputData) { long startTime = System.currentTimeMillis(); return createEvaluationContextReactive(rulesDSL, inputData) - .map(context -> evaluateRules(rulesDSL, context)) + .flatMap(context -> Mono.fromCallable(() -> evaluateRules(rulesDSL, context)) + .subscribeOn(Schedulers.boundedElastic())) .onErrorResume(error -> { long executionTime = System.currentTimeMillis() - startTime; JsonLogger.error(log, "Rules evaluation failed", error); @@ -239,21 +267,25 @@ private boolean evaluateConditions(List conditions, EvaluationContext for (int i = 0; i < conditions.size(); i++) { Condition condition = conditions.get(i); + ExpressionEvaluator evaluator = new ExpressionEvaluator(context, restCallService, jsonPathService, customFunctions); + Object result; try { - ExpressionEvaluator evaluator = new ExpressionEvaluator(context, restCallService, jsonPathService); - Object result = condition.accept(evaluator); - boolean boolResult = toBoolean(result); - - JsonLogger.info(log, context.getOperationId(), - String.format("Condition %d evaluation: %s = %s", i + 1, condition.toDebugString(), boolResult)); - - if (!boolResult) { - JsonLogger.info(log, context.getOperationId(), "Condition failed - short-circuiting evaluation"); - return false; - } - } catch (Exception e) { - String operationId = context.getOperationId(); - JsonLogger.error(log, operationId, "Error evaluating condition: " + condition.toDebugString(), e); + result = condition.accept(evaluator); + } catch (RuntimeException e) { + // Propagate so the outer evaluateRules() handler reports success=false + // with the real cause; swallowing here would silently flip rules to the + // else branch and mask authoring or data bugs. + throw new RuleEvaluationException( + "Failed to evaluate condition " + (i + 1) + " (" + condition.toDebugString() + "): " + + e.getMessage(), e); + } + + boolean boolResult = toBoolean(result); + JsonLogger.info(log, context.getOperationId(), + String.format("Condition %d evaluation: %s = %s", i + 1, condition.toDebugString(), boolResult)); + + if (!boolResult) { + JsonLogger.info(log, context.getOperationId(), "Condition failed - short-circuiting evaluation"); return false; } } @@ -278,20 +310,23 @@ private void executeActions(List actions, EvaluationContext context) { JsonLogger.info(log, context.getOperationId(), String.format("Executing action %d: %s", i + 1, action.toDebugString())); - ActionExecutor executor = new ActionExecutor(context, restCallService, jsonPathService); + ActionExecutor executor = new ActionExecutor(context, restCallService, jsonPathService, customFunctions); action.accept(executor); JsonLogger.info(log, context.getOperationId(), String.format("Action %d completed successfully", i + 1)); } catch (org.fireflyframework.rules.core.dsl.exception.CircuitBreakerException e) { - // Circuit breaker is a controlled stop, not an error JsonLogger.info(log, context.getOperationId(), "Circuit breaker triggered: " + e.getCircuitBreakerMessage() + " - stopping execution"); - // Re-throw to stop execution immediately throw e; - } catch (Exception e) { - String operationId = context.getOperationId(); - JsonLogger.error(log, operationId, "Error executing action: " + action.toDebugString(), e); + } catch (RuntimeException e) { + // Fail-fast: the previous swallow-and-continue policy let later actions + // read variables that the failing action never set, masking the real cause. + // The outer evaluateRules() catch converts this into success=false with the + // original message preserved. + throw new RuleEvaluationException( + "Failed to execute action " + (i + 1) + " (" + action.toDebugString() + "): " + + e.getMessage(), e); } } JsonLogger.info(log, context.getOperationId(), "All actions completed"); @@ -354,29 +389,28 @@ private boolean evaluateConditionalBlock(ASTRulesDSL.ASTConditionalBlock conditi if (conditionalBlock == null || conditionalBlock.getIfCondition() == null) { return false; } - + + ExpressionEvaluator evaluator = new ExpressionEvaluator(context, restCallService, jsonPathService, customFunctions); + Object result; try { - // Evaluate the condition using AST - ExpressionEvaluator evaluator = new ExpressionEvaluator(context, restCallService, jsonPathService); - Object result = conditionalBlock.getIfCondition().accept(evaluator); - boolean conditionResult = toBoolean(result); - - // Execute appropriate action block - ASTRulesDSL.ASTActionBlock actionBlock = conditionResult ? - conditionalBlock.getThenBlock() : - conditionalBlock.getElseBlock(); - - if (actionBlock != null) { - executeActionBlock(actionBlock, context); - } - - return conditionResult; - - } catch (Exception e) { - String operationId = context.getOperationId(); - JsonLogger.error(log, operationId, "Error evaluating conditional block", e); - return false; + result = conditionalBlock.getIfCondition().accept(evaluator); + } catch (RuntimeException e) { + // Propagate so the rule reports the real cause instead of silently falling + // through to the else branch. + throw new RuleEvaluationException( + "Failed to evaluate conditional block 'if' clause: " + e.getMessage(), e); } + boolean conditionResult = toBoolean(result); + + ASTRulesDSL.ASTActionBlock actionBlock = conditionResult ? + conditionalBlock.getThenBlock() : + conditionalBlock.getElseBlock(); + + if (actionBlock != null) { + executeActionBlock(actionBlock, context); + } + + return conditionResult; } /** @@ -395,7 +429,6 @@ private void executeActionBlock(ASTRulesDSL.ASTActionBlock actionBlock, Evaluati executeActions(actionBlock.getActions(), context); } - // Handle nested conditional blocks - this was the TODO that's now implemented! if (actionBlock.getNestedConditions() != null) { evaluateConditionalBlock(actionBlock.getNestedConditions(), context); } @@ -666,12 +699,6 @@ public Void visitExpressionCondition(ExpressionCondition node) { return null; } - @Override - public Void visitAssignmentAction(AssignmentAction node) { - node.getValue().accept(this); - return null; - } - @Override public Void visitCalculateAction(CalculateAction node) { node.getExpression().accept(this); @@ -737,16 +764,6 @@ public Void visitFunctionCallExpression(FunctionCallExpression node) { return null; } - @Override - public Void visitArithmeticExpression(ArithmeticExpression node) { - if (node.getOperands() != null) { - node.getOperands().forEach(operand -> operand.accept(this)); - } - return null; - } - - - @Override public Void visitArithmeticAction(ArithmeticAction node) { if (node.getValue() != null) { diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/evaluation/RuleEvaluationException.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/evaluation/RuleEvaluationException.java new file mode 100644 index 0000000..dd6310a --- /dev/null +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/evaluation/RuleEvaluationException.java @@ -0,0 +1,36 @@ +/* + * Copyright 2024-2026 Firefly Software Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.fireflyframework.rules.core.dsl.evaluation; + +/** + * Raised when an action or condition fails during rule evaluation. + *

+ * The engine wraps the underlying failure (e.g. unknown function, type-coercion error, + * division by zero, missing variable) with the index of the offending action or condition + * and its source debug string so the outer evaluator can report a precise + * {@code success=false} result instead of silently flipping to the else branch. + */ +public class RuleEvaluationException extends RuntimeException { + + public RuleEvaluationException(String message) { + super(message); + } + + public RuleEvaluationException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/exception/ASTException.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/exception/ASTException.java index 38f38ec..55bc374 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/exception/ASTException.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/exception/ASTException.java @@ -55,6 +55,10 @@ public ASTException(String message, SourceLocation location) { public ASTException(String message) { this(message, null, "AST_GENERIC", List.of(), null); } + + public ASTException(String message, Throwable cause) { + this(message, null, "AST_GENERIC", List.of(), cause); + } /** * Get a detailed error message with location and suggestions diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/expression/ArithmeticExpression.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/expression/ArithmeticExpression.java deleted file mode 100644 index 591b3b0..0000000 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/expression/ArithmeticExpression.java +++ /dev/null @@ -1,103 +0,0 @@ -/* - * Copyright 2024-2026 Firefly Software Foundation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.fireflyframework.rules.core.dsl.expression; - -import org.fireflyframework.rules.core.dsl.ASTVisitor; -import org.fireflyframework.rules.core.dsl.SourceLocation; -import lombok.Data; -import lombok.EqualsAndHashCode; - -import java.util.List; -import java.util.stream.Collectors; - -/** - * Represents an arithmetic expression with multiple operands. - * Examples: add(a, b, c), multiply(x, y), max(values...) - */ -@Data -@EqualsAndHashCode(callSuper = true) -public class ArithmeticExpression extends Expression { - - /** - * The arithmetic operation to perform - */ - private ArithmeticOperation operation; - - /** - * Operands for the arithmetic operation - */ - private List operands; - - public ArithmeticExpression(SourceLocation location, ArithmeticOperation operation, List operands) { - super(location); - this.operation = operation; - this.operands = operands; - } - - @Override - public T accept(ASTVisitor visitor) { - return visitor.visitArithmeticExpression(this); - } - - @Override - public ExpressionType getExpressionType() { - return ExpressionType.NUMBER; - } - - @Override - public boolean isConstant() { - return operands != null && operands.stream().allMatch(Expression::isConstant); - } - - @Override - public boolean hasVariableReferences() { - return operands != null && operands.stream().anyMatch(Expression::hasVariableReferences); - } - - @Override - public String toDebugString() { - if (operands == null || operands.isEmpty()) { - return operation.getSymbol() + "()"; - } - - String args = operands.stream() - .map(Expression::toDebugString) - .collect(Collectors.joining(", ")); - - return operation.getSymbol() + "(" + args + ")"; - } - - /** - * Get the number of operands - */ - public int getOperandCount() { - return operands != null ? operands.size() : 0; - } - - /** - * Get a specific operand by index - */ - public Expression getOperand(int index) { - if (operands == null || index < 0 || index >= operands.size()) { - throw new IndexOutOfBoundsException("Operand index out of bounds: " + index); - } - return operands.get(index); - } -} - - - diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/expression/ArithmeticOperation.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/expression/ArithmeticOperation.java deleted file mode 100644 index ec1ecb9..0000000 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/expression/ArithmeticOperation.java +++ /dev/null @@ -1,89 +0,0 @@ -/* - * Copyright 2024-2026 Firefly Software Foundation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.fireflyframework.rules.core.dsl.expression; - -/** - * Enumeration of arithmetic operations. - */ -public enum ArithmeticOperation { - // Basic arithmetic - ADD("add", "+", 6), - SUBTRACT("subtract", "-", 6), - MULTIPLY("multiply", "*", 7), - DIVIDE("divide", "/", 7), - MODULO("modulo", "%", 7), - POWER("power", "^", 8), - - // Mathematical functions - ABS("abs", "abs", 9), - MIN("min", "min", 9), - MAX("max", "max", 9), - ROUND("round", "round", 9), - FLOOR("floor", "floor", 9), - CEIL("ceil", "ceil", 9), - SQRT("sqrt", "sqrt", 9), - SUM("sum", "sum", 9), - AVERAGE("average", "avg", 9); - - private final String name; - private final String symbol; - private final int precedence; - - ArithmeticOperation(String name, String symbol, int precedence) { - this.name = name; - this.symbol = symbol; - this.precedence = precedence; - } - - public String getName() { - return name; - } - - public String getSymbol() { - return symbol; - } - - public int getPrecedence() { - return precedence; - } - - public int getMinOperands() { - return switch (this) { - case ABS, ROUND, FLOOR, CEIL, SQRT -> 1; - case MIN, MAX -> 2; - case SUM, AVERAGE -> 1; // Can take 1 or more - default -> 2; // Binary operations - }; - } - - public int getMaxOperands() { - return switch (this) { - case ABS, ROUND, FLOOR, CEIL, SQRT -> 1; - case SUM, AVERAGE -> Integer.MAX_VALUE; // Can take any number - default -> 2; // Binary operations - }; - } - - public static ArithmeticOperation fromSymbol(String symbol) { - for (ArithmeticOperation op : values()) { - if (op.symbol.equals(symbol)) { - return op; - } - } - throw new IllegalArgumentException("Unknown arithmetic operation: " + symbol); - } -} diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/function/CustomFunctionRegistry.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/function/CustomFunctionRegistry.java new file mode 100644 index 0000000..337fd1d --- /dev/null +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/function/CustomFunctionRegistry.java @@ -0,0 +1,102 @@ +/* + * Copyright 2024-2026 Firefly Software Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.fireflyframework.rules.core.dsl.function; + +import org.springframework.stereotype.Component; + +import java.util.Collections; +import java.util.Locale; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +/** + * Registry of user-defined {@link RuleFunction} implementations available to the rule DSL. + *

+ * The registry is consulted before the built-in function catalog, allowing applications to + * extend or override the engine without modifying its source. + * + *

Registration

+ * 
{@code
+ * @Configuration
+ * class MyRulesConfig {
+ *     @Bean
+ *     CommandLineRunner registerCustomFunctions(CustomFunctionRegistry registry) {
+ *         return args -> registry.register("my_score", a -> myScoring((Number) a[0]));
+ *     }
+ * }
+ * }
+ * + *

Lookups are case-insensitive. The registry is thread-safe. + */ +@Component +public class CustomFunctionRegistry { + + private final Map functions = new ConcurrentHashMap<>(); + + /** + * Register a function under the given name. If a function with the same name (ignoring case) + * is already registered, it is replaced. + * + * @param name the function name as referenced from the DSL; must not be {@code null} or blank + * @param function the function implementation; must not be {@code null} + * @throws IllegalArgumentException if {@code name} is blank or {@code function} is {@code null} + */ + public void register(String name, RuleFunction function) { + if (name == null || name.isBlank()) { + throw new IllegalArgumentException("Function name must not be blank"); + } + if (function == null) { + throw new IllegalArgumentException("Function must not be null"); + } + functions.put(name.toLowerCase(Locale.ROOT), function); + } + + /** + * Remove a registered function by name. No-op if the name is not registered. + * + * @return {@code true} if the function existed and was removed + */ + public boolean unregister(String name) { + if (name == null) return false; + return functions.remove(name.toLowerCase(Locale.ROOT)) != null; + } + + /** + * Look up a registered function by name (case-insensitive). + */ + public Optional lookup(String name) { + if (name == null) return Optional.empty(); + return Optional.ofNullable(functions.get(name.toLowerCase(Locale.ROOT))); + } + + /** + * Return the names of all currently registered functions (case folded to lower-case). + * The returned set is an immutable snapshot. + */ + public Set registeredNames() { + return Collections.unmodifiableSet(Set.copyOf(functions.keySet())); + } + + /** + * Whether a function with the given name (case-insensitive) is registered. + */ + public boolean contains(String name) { + return lookup(name).isPresent(); + } +} diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/function/RuleFunction.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/function/RuleFunction.java new file mode 100644 index 0000000..6a0f99d --- /dev/null +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/function/RuleFunction.java @@ -0,0 +1,48 @@ +/* + * Copyright 2024-2026 Firefly Software Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.fireflyframework.rules.core.dsl.function; + +/** + * Pluggable function callable from the rule DSL via the {@code call} action or as part of an + * expression (e.g., {@code calculate result as my_function(amount, rate)}). + *

+ * Implementations are registered with {@link CustomFunctionRegistry}. Custom functions are + * looked up before the built-in catalog, so a registration with the same name as a + * built-in (e.g., {@code "max"}) deliberately shadows the built-in. Names are matched + * case-insensitively. + * + *

Contract

+ *
    + *
  • Arguments arrive as evaluated values (literals, resolved variables, or nested + * function results) -- not as AST nodes.
    • + *
  • Implementations should throw {@link IllegalArgumentException} for invalid argument + * counts or types; the evaluator surfaces these to the caller.
    • + *
  • Implementations must be thread-safe; the same instance can be invoked concurrently + * across rule evaluations.
    • + *
+ */ +@FunctionalInterface +public interface RuleFunction { + + /** + * Apply this function to the evaluated argument list. + * + * @param args evaluated argument values (may be empty, never {@code null}) + * @return the function result; may be {@code null} if the function legitimately returns a null value + */ + Object apply(Object[] args); +} diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/ASTRulesDSLParser.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/ASTRulesDSLParser.java index 5105dcd..676996e 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/ASTRulesDSLParser.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/ASTRulesDSLParser.java @@ -33,8 +33,6 @@ import java.util.List; import java.util.Map; -import java.util.Objects; -import java.util.Optional; import java.util.stream.Collectors; /** @@ -300,22 +298,18 @@ private ASTRulesDSL.ASTSubRule convertToSubRule(Map ruleMap) { builder.whenConditions(whenConditions); } + // Sub-rules use the same action-parsing path as the top-level when/then/else so + // YAML-collapsed map-syntax actions (e.g. `- forEach x in xs: action`) work in + // both contexts. Routing through parseActionList preserves Map entries instead of + // calling toString() on them. if (ruleMap.containsKey("then")) { - List thenStrings = convertToStringList(ruleMap.get("then")); - List thenActions = thenStrings.stream() - .map(dslParser::parseAction) - .collect(Collectors.toList()); - builder.thenActions(thenActions); + builder.thenActions(parseActionList(ruleMap.get("then"))); } - + if (ruleMap.containsKey("else")) { - List elseStrings = convertToStringList(ruleMap.get("else")); - List elseActions = elseStrings.stream() - .map(dslParser::parseAction) - .collect(Collectors.toList()); - builder.elseActions(elseActions); + builder.elseActions(parseActionList(ruleMap.get("else"))); } - + // Complex syntax if (ruleMap.containsKey("conditions")) { Map conditionsMap = (Map) ruleMap.get("conditions"); @@ -373,7 +367,6 @@ private ASTRulesDSL.ASTActionBlock convertToActionBlock(Map acti builder.actions(actions); } - // Parse nested conditions - this was the TODO that's now implemented! if (actionMap.containsKey("conditions")) { Map nestedConditionsMap = (Map) actionMap.get("conditions"); ASTRulesDSL.ASTConditionalBlock nestedConditions = convertToConditionalBlock(nestedConditionsMap); @@ -384,7 +377,8 @@ private ASTRulesDSL.ASTActionBlock convertToActionBlock(Map acti } /** - * Parse actions list that can contain either simple syntax strings or complex syntax maps + * Parse actions list that can contain either simple syntax strings or complex syntax maps. + * Throws ASTException on any parse failure rather than silently dropping malformed actions. */ @SuppressWarnings("unchecked") private List parseActionsList(Object actionsObj) { @@ -392,7 +386,6 @@ private List parseActionsList(Object actionsObj) { List actionsList = (List) actionsObj; return actionsList.stream() .map(this::parseActionItem) - .filter(Objects::nonNull) .collect(Collectors.toList()); } else if (actionsObj instanceof String) { return List.of(dslParser.parseAction((String) actionsObj)); @@ -402,43 +395,42 @@ private List parseActionsList(Object actionsObj) { } /** - * Parse a single action item (either string or map) + * Parse a single action item (either string or map). + * Throws ASTException on unknown types or malformed action structures so that + * malformed rules surface immediately instead of silently losing actions. */ @SuppressWarnings("unchecked") private Action parseActionItem(Object actionObj) { + if (actionObj == null) { + throw new ASTException("Action entry cannot be null"); + } if (actionObj instanceof String) { return dslParser.parseAction((String) actionObj); } else if (actionObj instanceof Map) { Map actionMap = (Map) actionObj; - // Check if this is a single-entry map that might be a forEach/while/do-while action - // YAML interprets "forEach item in items: action" as {"forEach item in items": "action"} - // Same for "while condition: action" as {"while condition": "action"} - // Same for "do: action while condition" as {"do": "action while condition"} + // Single-entry maps like {"forEach item in items": "action"} originate from YAML + // collapsing simple loop syntax. Try the literal reconstruction first; if that + // does not match the lexer expectations, fall through to the structured-map path. if (actionMap.size() == 1) { Map.Entry entry = actionMap.entrySet().iterator().next(); String key = entry.getKey(); Object value = entry.getValue(); - // Check if key starts with forEach, while, or do if (key.startsWith("forEach ") || key.startsWith("while ") || key.equals("do")) { - // Reconstruct the action string String actionString = key + ": " + formatActionValue(value); try { - Action action = dslParser.parseAction(actionString); - return action; + return dslParser.parseAction(actionString); } catch (Exception e) { - log.warn("Failed to parse reconstructed loop action: {}", actionString, e); - // Fall through to complex action parsing + log.debug("Reconstructed loop parse failed for '{}', retrying as structured map", actionString); + // Fall through to complex action parsing. } } } - // Complex syntax: { set: { variable: "name", value: "value" } } return parseComplexAction(actionMap); } else { - log.warn("Unexpected action object type: {}", actionObj.getClass()); - return null; + throw new ASTException("Unsupported action entry type: " + actionObj.getClass().getName()); } } @@ -460,191 +452,115 @@ private String formatActionValue(Object value) { } /** - * Parse complex syntax action from map + * Parse complex map-shaped action and re-emit as the simple-syntax string the lexer expects. + * Throws ASTException on any parse failure so malformed entries never silently disappear. */ @SuppressWarnings("unchecked") private Action parseComplexAction(Map actionMap) { - // Handle different action types if (actionMap.containsKey("set")) { Map setMap = (Map) actionMap.get("set"); String variable = (String) setMap.get("variable"); Object value = setMap.get("value"); - - // Convert to simple syntax and parse - String simpleSyntax = "set " + variable + " to " + formatValue(value); - try { - return dslParser.parseAction(simpleSyntax); - } catch (Exception e) { - log.warn("Failed to parse complex set action: {}", actionMap, e); - return null; - } - } else if (actionMap.containsKey("calculate")) { + return parseReconstructed("set " + variable + " to " + formatValue(value), "set", actionMap); + } + if (actionMap.containsKey("calculate")) { Map calcMap = (Map) actionMap.get("calculate"); String variable = (String) calcMap.get("variable"); String expression = (String) calcMap.get("expression"); - - // Convert to simple syntax and parse - String simpleSyntax = "calculate " + variable + " as " + expression; - try { - return dslParser.parseAction(simpleSyntax); - } catch (Exception e) { - log.warn("Failed to parse complex calculate action: {}", actionMap, e); - return null; - } - } else if (actionMap.containsKey("run")) { + return parseReconstructed("calculate " + variable + " as " + expression, "calculate", actionMap); + } + if (actionMap.containsKey("run")) { Map runMap = (Map) actionMap.get("run"); String variable = (String) runMap.get("variable"); String expression = (String) runMap.get("expression"); - - // Convert to simple syntax and parse - String simpleSyntax = "run " + variable + " as " + expression; - try { - return dslParser.parseAction(simpleSyntax); - } catch (Exception e) { - log.warn("Failed to parse complex run action: {}", actionMap, e); - return null; - } - } else if (actionMap.containsKey("call")) { + return parseReconstructed("run " + variable + " as " + expression, "run", actionMap); + } + if (actionMap.containsKey("call")) { Map callMap = (Map) actionMap.get("call"); String function = (String) callMap.get("function"); List parameters = (List) callMap.get("parameters"); - - // Convert to simple syntax and parse String paramStr = parameters.stream() .map(this::formatValue) .collect(Collectors.joining(", ")); - String simpleSyntax = "call " + function + " with [" + paramStr + "]"; - try { - return dslParser.parseAction(simpleSyntax); - } catch (Exception e) { - log.warn("Failed to parse complex call action: {}", actionMap, e); - return null; - } - } else if (actionMap.containsKey("forEach")) { + return parseReconstructed("call " + function + " with [" + paramStr + "]", "call", actionMap); + } + if (actionMap.containsKey("forEach")) { Map forEachMap = (Map) actionMap.get("forEach"); String variable = (String) forEachMap.get("variable"); - String index = (String) forEachMap.get("index"); // optional + String index = (String) forEachMap.get("index"); Object inValue = forEachMap.get("in"); Object doValue = forEachMap.get("do"); - // Build simple syntax - StringBuilder simpleSyntax = new StringBuilder("forEach "); - simpleSyntax.append(variable); - + StringBuilder simpleSyntax = new StringBuilder("forEach ").append(variable); if (index != null && !index.trim().isEmpty()) { simpleSyntax.append(", ").append(index); } - - simpleSyntax.append(" in ").append(formatValue(inValue)); - simpleSyntax.append(": "); - - // Parse body actions - if (doValue instanceof List) { - List doList = (List) doValue; - List bodyActionStrings = new ArrayList<>(); - - for (Object actionObj : doList) { - if (actionObj instanceof String) { - bodyActionStrings.add((String) actionObj); - } else if (actionObj instanceof Map) { - // Recursively parse complex action and convert to string - Action bodyAction = parseComplexAction((Map) actionObj); - if (bodyAction != null) { - bodyActionStrings.add(bodyAction.toDebugString()); - } - } - } - - simpleSyntax.append(String.join("; ", bodyActionStrings)); - } else if (doValue instanceof String) { - simpleSyntax.append((String) doValue); - } - - try { - return dslParser.parseAction(simpleSyntax.toString()); - } catch (Exception e) { - log.warn("Failed to parse complex forEach action: {}", actionMap, e); - return null; - } - } else if (actionMap.containsKey("while")) { + simpleSyntax.append(" in ").append(formatValue(inValue)).append(": "); + appendLoopBody(doValue, simpleSyntax); + return parseReconstructed(simpleSyntax.toString(), "forEach", actionMap); + } + if (actionMap.containsKey("while")) { Map whileMap = (Map) actionMap.get("while"); String condition = (String) whileMap.get("condition"); Object doValue = whileMap.get("do"); - // Build simple syntax - StringBuilder simpleSyntax = new StringBuilder("while "); - simpleSyntax.append(condition); - simpleSyntax.append(": "); - - // Parse body actions - if (doValue instanceof List) { - List doList = (List) doValue; - List bodyActionStrings = new ArrayList<>(); - - for (Object actionObj : doList) { - if (actionObj instanceof String) { - bodyActionStrings.add((String) actionObj); - } else if (actionObj instanceof Map) { - // Recursively parse complex action and convert to string - Action bodyAction = parseComplexAction((Map) actionObj); - if (bodyAction != null) { - bodyActionStrings.add(bodyAction.toDebugString()); - } - } - } - - simpleSyntax.append(String.join("; ", bodyActionStrings)); - } else if (doValue instanceof String) { - simpleSyntax.append((String) doValue); - } - - try { - return dslParser.parseAction(simpleSyntax.toString()); - } catch (Exception e) { - log.warn("Failed to parse complex while action: {}", actionMap, e); - return null; - } - } else if (actionMap.containsKey("do")) { + StringBuilder simpleSyntax = new StringBuilder("while ").append(condition).append(": "); + appendLoopBody(doValue, simpleSyntax); + return parseReconstructed(simpleSyntax.toString(), "while", actionMap); + } + if (actionMap.containsKey("do")) { Map doWhileMap = (Map) actionMap.get("do"); Object doValue = doWhileMap.get("actions"); String condition = (String) doWhileMap.get("while"); - // Build simple syntax StringBuilder simpleSyntax = new StringBuilder("do: "); + appendLoopBody(doValue, simpleSyntax); + simpleSyntax.append(" while ").append(condition); + return parseReconstructed(simpleSyntax.toString(), "do-while", actionMap); + } + throw new ASTException("Unknown complex action type. Keys present: " + actionMap.keySet()); + } - // Parse body actions - if (doValue instanceof List) { - List doList = (List) doValue; - List bodyActionStrings = new ArrayList<>(); - - for (Object actionObj : doList) { - if (actionObj instanceof String) { - bodyActionStrings.add((String) actionObj); - } else if (actionObj instanceof Map) { - // Recursively parse complex action and convert to string - Action bodyAction = parseComplexAction((Map) actionObj); - if (bodyAction != null) { - bodyActionStrings.add(bodyAction.toDebugString()); - } - } + /** + * Reconstruct a loop body from a List of action entries or a single String into the + * semicolon-joined form the ActionParser consumes. + */ + @SuppressWarnings("unchecked") + private void appendLoopBody(Object doValue, StringBuilder out) { + if (doValue instanceof List) { + List doList = (List) doValue; + List bodyActionStrings = new ArrayList<>(); + for (Object actionObj : doList) { + if (actionObj instanceof String) { + bodyActionStrings.add((String) actionObj); + } else if (actionObj instanceof Map) { + Action bodyAction = parseComplexAction((Map) actionObj); + bodyActionStrings.add(bodyAction.toDebugString()); + } else { + throw new ASTException("Unsupported loop body entry type: " + + (actionObj == null ? "null" : actionObj.getClass().getName())); } - - simpleSyntax.append(String.join("; ", bodyActionStrings)); - } else if (doValue instanceof String) { - simpleSyntax.append((String) doValue); } + out.append(String.join("; ", bodyActionStrings)); + } else if (doValue instanceof String) { + out.append((String) doValue); + } else if (doValue != null) { + throw new ASTException("Unsupported loop body type: " + doValue.getClass().getName()); + } + } - simpleSyntax.append(" while ").append(condition); - - try { - return dslParser.parseAction(simpleSyntax.toString()); - } catch (Exception e) { - log.warn("Failed to parse complex do-while action: {}", actionMap, e); - return null; - } - } else { - log.warn("Unknown complex action type: {}", actionMap.keySet()); - return null; + /** + * Parse a reconstructed simple-syntax string, wrapping any lexer/parser failure in an + * ASTException tagged with the original complex-form context for easier diagnostics. + */ + private Action parseReconstructed(String simpleSyntax, String actionKind, Map originalMap) { + try { + return dslParser.parseAction(simpleSyntax); + } catch (Exception e) { + throw new ASTException( + "Failed to parse complex " + actionKind + " action " + originalMap + + " (reconstructed: " + simpleSyntax + "): " + e.getMessage(), + e); } } @@ -703,7 +619,6 @@ private List parseActionList(Object obj) { throw e; } }) - .filter(action -> action != null) .collect(Collectors.toList()); } else if (obj instanceof String) { return List.of(dslParser.parseAction((String) obj)); diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/DSLParser.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/DSLParser.java index b66e0c2..9b5ee90 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/DSLParser.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/DSLParser.java @@ -187,22 +187,4 @@ private List tokenize(String source) { return lexer.tokenize(); } - /** - * Validate that a parsed AST node is well-formed - */ - public void validateAST(Object astNode) { - if (astNode == null) { - throw new ParseException( - "AST node cannot be null", - null, - "PARSE_VALIDATION_001" - ); - } - - // Additional validation logic can be added here - // For example, checking that all required fields are present, - // that variable references are valid, etc. - - log.debug("AST validation passed for node type: {}", astNode.getClass().getSimpleName()); - } } diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/ExpressionParser.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/ExpressionParser.java index e776e0f..5aaf92a 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/ExpressionParser.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/ExpressionParser.java @@ -291,8 +291,15 @@ private Expression primary() { indexExpression = parseExpression(); consume(TokenType.RBRACKET, "Expected ']' after array index"); } - - return new VariableExpression(identifier.getLocation(), identifier.getLexeme(), propertyPath.isEmpty() ? null : propertyPath); + + VariableExpression variable = new VariableExpression( + identifier.getLocation(), + identifier.getLexeme(), + propertyPath.isEmpty() ? null : propertyPath); + if (indexExpression != null) { + variable.setIndexExpression(indexExpression); + } + return variable; } if (match(TokenType.LPAREN)) { diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ActionExecutor.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ActionExecutor.java index 16a393d..f50ba2a 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ActionExecutor.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ActionExecutor.java @@ -22,6 +22,7 @@ import org.fireflyframework.rules.core.dsl.condition.ExpressionCondition; import org.fireflyframework.rules.core.dsl.condition.LogicalCondition; import org.fireflyframework.rules.core.dsl.expression.*; +import org.fireflyframework.rules.core.dsl.function.CustomFunctionRegistry; import org.fireflyframework.rules.core.services.JsonPathService; import org.fireflyframework.rules.core.services.RestCallService; import lombok.extern.slf4j.Slf4j; @@ -37,70 +38,23 @@ public class ActionExecutor implements ASTVisitor { private final ExpressionEvaluator expressionEvaluator; public ActionExecutor(EvaluationContext context) { - this.context = context; - this.expressionEvaluator = new ExpressionEvaluator(context); + this(context, null, null, null); } public ActionExecutor(EvaluationContext context, RestCallService restCallService, JsonPathService jsonPathService) { + this(context, restCallService, jsonPathService, null); + } + + public ActionExecutor(EvaluationContext context, + RestCallService restCallService, + JsonPathService jsonPathService, + CustomFunctionRegistry customFunctions) { this.context = context; - this.expressionEvaluator = new ExpressionEvaluator(context, restCallService, jsonPathService); + this.expressionEvaluator = new ExpressionEvaluator(context, restCallService, jsonPathService, customFunctions); } // Action visitors - @Override - public Void visitAssignmentAction(AssignmentAction node) { - Object value = node.getValue().accept(expressionEvaluator); - - switch (node.getOperator()) { - case ASSIGN -> context.setComputedVariable(node.getVariableName(), value); - case ADD_ASSIGN -> { - Object currentValue = context.getVariable(node.getVariableName()); - if (currentValue instanceof Number && value instanceof Number) { - java.math.BigDecimal current = toBigDecimal(currentValue); - java.math.BigDecimal addValue = toBigDecimal(value); - context.setComputedVariable(node.getVariableName(), current.add(addValue)); - } else { - String currentStr = currentValue != null ? currentValue.toString() : ""; - String valueStr = value != null ? value.toString() : ""; - context.setComputedVariable(node.getVariableName(), currentStr + valueStr); - } - } - case SUBTRACT_ASSIGN -> { - Object currentValue = context.getVariable(node.getVariableName()); - if (currentValue instanceof Number && value instanceof Number) { - java.math.BigDecimal current = toBigDecimal(currentValue); - java.math.BigDecimal subtractValue = toBigDecimal(value); - context.setComputedVariable(node.getVariableName(), current.subtract(subtractValue)); - } - } - case MULTIPLY_ASSIGN -> { - Object currentValue = context.getVariable(node.getVariableName()); - if (currentValue instanceof Number && value instanceof Number) { - java.math.BigDecimal current = toBigDecimal(currentValue); - java.math.BigDecimal multiplyValue = toBigDecimal(value); - context.setComputedVariable(node.getVariableName(), current.multiply(multiplyValue)); - } - } - case DIVIDE_ASSIGN -> { - Object currentValue = context.getVariable(node.getVariableName()); - if (currentValue instanceof Number && value instanceof Number) { - java.math.BigDecimal current = toBigDecimal(currentValue); - java.math.BigDecimal divisor = toBigDecimal(value); - if (divisor.compareTo(java.math.BigDecimal.ZERO) == 0) { - throw new ArithmeticException("Division by zero in /= assignment"); - } - context.setComputedVariable(node.getVariableName(), - current.divide(divisor, 10, java.math.RoundingMode.HALF_UP)); - } - } - } - - log.debug("Executed assignment: {} {} {}", - node.getVariableName(), node.getOperator().getSymbol(), value); - return null; - } - @Override public Void visitFunctionCallAction(FunctionCallAction node) { // Evaluate arguments @@ -186,17 +140,6 @@ private boolean containsNonMathematicalOperation(Expression expression) { if (expression instanceof UnaryExpression unaryExpr) { return containsNonMathematicalOperation(unaryExpr.getOperand()); } - if (expression instanceof ArithmeticExpression arithmeticExpr) { - // Check all operands in the arithmetic expression - if (arithmeticExpr.getOperands() != null) { - for (Expression operand : arithmeticExpr.getOperands()) { - if (containsNonMathematicalOperation(operand)) { - return true; - } - } - } - return false; - } // LiteralExpression and VariableExpression are allowed return false; } @@ -246,11 +189,6 @@ public Void visitFunctionCallExpression(FunctionCallExpression node) { throw new UnsupportedOperationException("Expressions cannot be executed as actions"); } - @Override - public Void visitArithmeticExpression(ArithmeticExpression node) { - throw new UnsupportedOperationException("Expressions cannot be executed as actions"); - } - // Condition visitors (not used in action execution) @Override @@ -350,10 +288,16 @@ private Object executeFunction(String functionName, Object[] args) { ) ); } - default -> { - log.warn("Unknown function: {}", functionName); - yield null; - } + // Any other name -- custom-registered or any expression-tier built-in -- delegates + // to the expression evaluator, which checks the custom registry first and throws + // IllegalArgumentException with the registry-aware diagnostic if still unknown. + default -> expressionEvaluator.visitFunctionCallExpression( + new FunctionCallExpression( + null, functionName, + java.util.Arrays.stream(args) + .map(arg -> new LiteralExpression(null, arg)) + .collect(java.util.stream.Collectors.toList()) + )); }; } @@ -366,37 +310,36 @@ public Void visitArithmeticAction(ArithmeticAction node) { Object value = node.getValue().accept(expressionEvaluator); Object current = context.getVariable(node.getVariableName()); - if (current instanceof Number && value instanceof Number) { - java.math.BigDecimal currentNum = toBigDecimal(current); - java.math.BigDecimal valueNum = toBigDecimal(value); - java.math.BigDecimal result; - - switch (node.getOperation()) { - case ADD -> result = currentNum.add(valueNum); - case SUBTRACT -> result = currentNum.subtract(valueNum); - case MULTIPLY -> result = currentNum.multiply(valueNum); - case DIVIDE -> { - if (valueNum.compareTo(java.math.BigDecimal.ZERO) == 0) { - throw new ArithmeticException("Division by zero in arithmetic action"); - } - result = currentNum.divide(valueNum, 10, java.math.RoundingMode.HALF_UP); - } - default -> { - log.warn("Unknown arithmetic operation: {}", node.getOperation()); - result = current instanceof java.math.BigDecimal ? (java.math.BigDecimal) current : toBigDecimal(current); - } - } - - context.setComputedVariable(node.getVariableName(), result); - } else { - log.warn("Arithmetic action requires numeric operands: {} ({}), {} ({})", - current, current != null ? current.getClass().getSimpleName() : "null", - value, value != null ? value.getClass().getSimpleName() : "null"); + if (!(current instanceof Number) || !(value instanceof Number)) { + throw new IllegalArgumentException( + "Arithmetic action '" + node.getOperation().getKeyword() + "' on '" + + node.getVariableName() + "' requires numeric operands. Got current=" + + describeType(current) + ", value=" + describeType(value)); } + java.math.BigDecimal currentNum = toBigDecimal(current); + java.math.BigDecimal valueNum = toBigDecimal(value); + java.math.BigDecimal result = switch (node.getOperation()) { + case ADD -> currentNum.add(valueNum); + case SUBTRACT -> currentNum.subtract(valueNum); + case MULTIPLY -> currentNum.multiply(valueNum); + case DIVIDE -> { + if (valueNum.compareTo(java.math.BigDecimal.ZERO) == 0) { + throw new ArithmeticException("Division by zero in arithmetic action on '" + + node.getVariableName() + "'"); + } + yield currentNum.divide(valueNum, 10, java.math.RoundingMode.HALF_UP); + } + }; + context.setComputedVariable(node.getVariableName(), result); return null; } + private static String describeType(Object o) { + if (o == null) return "null"; + return o + " (" + o.getClass().getSimpleName() + ")"; + } + @Override public Void visitListAction(ListAction node) { log.debug("Executing list action: {} {} {} {}", diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/EvaluationContext.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/EvaluationContext.java index 5737204..328d7f8 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/EvaluationContext.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/EvaluationContext.java @@ -16,61 +16,57 @@ package org.fireflyframework.rules.core.dsl.visitor; -import lombok.Data; +import lombok.Getter; import lombok.NoArgsConstructor; +import lombok.Setter; +import java.util.Collections; +import java.util.LinkedHashMap; import java.util.Map; import java.util.UUID; -import java.util.concurrent.ConcurrentHashMap; /** * Consolidated context object that holds the state during AST-based rule evaluation. - * Contains variables, constants, and execution state with full feature support. - * This replaces both the legacy and AST-specific EvaluationContext classes. + *

+ * The three variable maps ({@code inputVariables}, {@code systemConstants}, + * {@code computedVariables}) are intentionally exposed only through the typed setter + * methods on this class -- which validate names -- and getters that return defensive + * copies. There are no bulk setters; that prevents callers from substituting an + * unvalidated map and silently bypassing the naming-convention guard rails. + *

+ * The maps use {@link Collections#synchronizedMap synchronized {@link LinkedHashMap}s} + * rather than {@link java.util.concurrent.ConcurrentHashMap} because rule outputs + * legitimately include nulls (e.g., {@code json_get} on a missing path); ConcurrentHashMap + * rejects null values with an NPE and would mask the assignment as an unrelated runtime + * error. Each evaluation owns its own context so the sync overhead is negligible. */ -@Data +@Getter @NoArgsConstructor public class EvaluationContext { - /** - * Name of the rule being evaluated - */ - private String ruleName; + /** Name of the rule being evaluated. */ + @Setter private String ruleName; - /** - * Operation ID for tracing and logging - */ - private String operationId; + /** Operation ID for tracing and logging. */ + @Setter private String operationId; - /** - * Start time of evaluation (for performance tracking) - */ - private long startTime; + /** Start time of evaluation (for performance tracking). */ + @Setter private long startTime; - /** - * Input variables provided by the controller (runtime values like annualIncome, creditScore) - */ - private Map inputVariables = new ConcurrentHashMap<>(); + /** Input variables provided by the controller (e.g., {@code annualIncome}, {@code creditScore}). */ + private final Map inputVariables = Collections.synchronizedMap(new LinkedHashMap<>()); - /** - * System constants loaded from database (system-wide values like MINIMUM_CREDIT_SCORE, MAX_LOAN_AMOUNT) - */ - private Map systemConstants = new ConcurrentHashMap<>(); + /** System constants loaded from database (e.g., {@code MIN_CREDIT_SCORE}, {@code MAX_LOAN_AMOUNT}). */ + private final Map systemConstants = Collections.synchronizedMap(new LinkedHashMap<>()); - /** - * Computed variables created during rule evaluation (like loan_to_income_ratio, final_score) - */ - private Map computedVariables = new ConcurrentHashMap<>(); + /** Computed variables created during rule evaluation (e.g., {@code debt_to_income}, {@code final_score}). */ + private final Map computedVariables = Collections.synchronizedMap(new LinkedHashMap<>()); - /** - * Whether the circuit breaker has been triggered - */ - private boolean circuitBreakerTriggered = false; + /** Whether the circuit breaker has been triggered. */ + @Setter private boolean circuitBreakerTriggered = false; - /** - * Message associated with circuit breaker trigger - */ - private String circuitBreakerMessage; + /** Message associated with circuit breaker trigger. */ + @Setter private String circuitBreakerMessage; /** * Constructor with operation ID @@ -205,7 +201,7 @@ public boolean hasVariable(String name) { * @return map of input variables */ public Map getInputVariables() { - return new ConcurrentHashMap<>(inputVariables); + synchronized (inputVariables) { return new LinkedHashMap<>(inputVariables); } } /** @@ -214,7 +210,7 @@ public Map getInputVariables() { * @return map of computed variables */ public Map getComputedVariables() { - return new ConcurrentHashMap<>(computedVariables); + synchronized (computedVariables) { return new LinkedHashMap<>(computedVariables); } } /** @@ -223,7 +219,7 @@ public Map getComputedVariables() { * @return map of system constants */ public Map getSystemConstants() { - return new ConcurrentHashMap<>(systemConstants); + synchronized (systemConstants) { return new LinkedHashMap<>(systemConstants); } } /** @@ -239,7 +235,7 @@ public Map getConstants() { * Get all variables (including computed ones) for AST compatibility */ public Map getAllVariables() { - Map all = new ConcurrentHashMap<>(); + Map all = new LinkedHashMap<>(); all.putAll(systemConstants); all.putAll(inputVariables); all.putAll(computedVariables); diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ExpressionEvaluator.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ExpressionEvaluator.java index d6695da..fc2f185 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ExpressionEvaluator.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ExpressionEvaluator.java @@ -22,6 +22,8 @@ import org.fireflyframework.rules.core.dsl.condition.ExpressionCondition; import org.fireflyframework.rules.core.dsl.condition.LogicalCondition; import org.fireflyframework.rules.core.dsl.expression.*; +import org.fireflyframework.rules.core.dsl.function.CustomFunctionRegistry; +import org.fireflyframework.rules.core.dsl.function.RuleFunction; import org.fireflyframework.rules.core.services.JsonPathService; import org.fireflyframework.rules.core.services.RestCallService; import org.fireflyframework.rules.core.utils.JsonLogger; @@ -44,6 +46,7 @@ public class ExpressionEvaluator implements ASTVisitor { private final EvaluationContext context; private final RestCallService restCallService; private final JsonPathService jsonPathService; + private final CustomFunctionRegistry customFunctions; private static final int REGEX_CACHE_SIZE = 64; @SuppressWarnings("serial") @@ -55,15 +58,21 @@ protected boolean removeEldestEntry(Map.Entry eldest) { }; public ExpressionEvaluator(EvaluationContext context) { - this.context = context; - this.restCallService = null; // Will be injected when needed - this.jsonPathService = null; // Will be injected when needed + this(context, null, null, null); } public ExpressionEvaluator(EvaluationContext context, RestCallService restCallService, JsonPathService jsonPathService) { + this(context, restCallService, jsonPathService, null); + } + + public ExpressionEvaluator(EvaluationContext context, + RestCallService restCallService, + JsonPathService jsonPathService, + CustomFunctionRegistry customFunctions) { this.context = context; this.restCallService = restCallService; this.jsonPathService = jsonPathService; + this.customFunctions = customFunctions; } // Expression visitors @@ -197,7 +206,16 @@ public Object visitFunctionCallExpression(FunctionCallExpression node) { node.getArguments().stream().map(arg -> arg.accept(this)).toArray() : new Object[0]; - // Built-in mathematical functions + // User-registered functions are checked first so they may extend or override built-ins. + if (customFunctions != null) { + RuleFunction custom = customFunctions.lookup(functionName).orElse(null); + if (custom != null) { + return custom.apply(args); + } + } + + // Built-in catalog. Unknown names throw rather than silently returning null -- + // the user almost certainly typo'd a function name and silent null masks that. return switch (functionName.toLowerCase()) { case "max" -> evaluateMax(args); case "min" -> evaluateMin(args); @@ -224,6 +242,12 @@ public Object visitFunctionCallExpression(FunctionCallExpression node) { case "today" -> evaluateToday(args); case "dateadd" -> evaluateDateAdd(args); case "datediff" -> evaluateDateDiff(args); + case "calculate_age" -> evaluateCalculateAge(args); + case "format_date" -> evaluateFormatDate(args); + + // Validation functions (function-call form complements the `is_email`/`is_phone` operators) + case "validate_email" -> isEmail(args.length > 0 ? args[0] : null); + case "validate_phone" -> isPhone(args.length > 0 ? args[0] : null); // List functions case "size", "count" -> evaluateSize(args); @@ -237,6 +261,11 @@ public Object visitFunctionCallExpression(FunctionCallExpression node) { case "tostring", "string" -> evaluateToString(args); case "toboolean", "boolean" -> evaluateToBoolean(args); + // Null-handling / conditional helpers + case "coalesce" -> evaluateCoalesce(args); + case "if_else", "ifelse" -> evaluateIfElse(args); + case "is_in_range" -> inRange(args); + // Financial calculation functions case "calculate_loan_payment" -> calculateLoanPayment(args); case "calculate_compound_interest" -> calculateCompoundInterest(args); @@ -296,81 +325,12 @@ public Object visitFunctionCallExpression(FunctionCallExpression node) { case "json_size" -> jsonSize(args); case "json_type" -> jsonType(args); - default -> { - log.warn("Unknown function: {}", functionName); - yield null; - } + default -> throw new IllegalArgumentException( + "Unknown function '" + functionName + "'. Register it via CustomFunctionRegistry or " + + "check spelling against the built-in catalog."); }; } - - @Override - public Object visitArithmeticExpression(ArithmeticExpression node) { - List operandValues = node.getOperands().stream() - .map(operand -> operand.accept(this)) - .toList(); - - return switch (node.getOperation()) { - case ADD -> operandValues.stream() - .map(this::toNumber) - .reduce(BigDecimal.ZERO, BigDecimal::add); - case MULTIPLY -> operandValues.stream() - .map(this::toNumber) - .reduce(BigDecimal.ONE, BigDecimal::multiply); - case MIN -> operandValues.stream() - .map(this::toNumber) - .min(BigDecimal::compareTo) - .orElse(BigDecimal.ZERO); - case MAX -> operandValues.stream() - .map(this::toNumber) - .max(BigDecimal::compareTo) - .orElse(BigDecimal.ZERO); - case SUM -> operandValues.stream() - .map(this::toNumber) - .reduce(BigDecimal.ZERO, BigDecimal::add); - case AVERAGE -> { - BigDecimal sum = operandValues.stream() - .map(this::toNumber) - .reduce(BigDecimal.ZERO, BigDecimal::add); - yield sum.divide(BigDecimal.valueOf(operandValues.size()), 10, RoundingMode.HALF_UP); - } - default -> { - if (operandValues.size() >= 2) { - BigDecimal first = toNumber(operandValues.get(0)); - BigDecimal second = toNumber(operandValues.get(1)); - yield switch (node.getOperation()) { - case SUBTRACT -> first.subtract(second); - case DIVIDE -> { - if (second.compareTo(BigDecimal.ZERO) == 0) { - throw new ArithmeticException("Division by zero"); - } - yield first.divide(second, 10, RoundingMode.HALF_UP); - } - case MODULO -> { - if (second.compareTo(BigDecimal.ZERO) == 0) { - throw new ArithmeticException("Modulo by zero"); - } - yield first.remainder(second); - } - case POWER -> BigDecimal.valueOf(Math.pow(first.doubleValue(), second.doubleValue())); - default -> first; - }; - } else if (operandValues.size() == 1) { - BigDecimal value = toNumber(operandValues.get(0)); - yield switch (node.getOperation()) { - case ABS -> value.abs(); - case ROUND -> BigDecimal.valueOf(Math.round(value.doubleValue())); - case FLOOR -> BigDecimal.valueOf(Math.floor(value.doubleValue())); - case CEIL -> BigDecimal.valueOf(Math.ceil(value.doubleValue())); - case SQRT -> BigDecimal.valueOf(Math.sqrt(value.doubleValue())); - default -> value; - }; - } else { - yield BigDecimal.ZERO; - } - } - }; - } - + // Condition visitors (return Boolean) @Override @@ -460,12 +420,7 @@ public Object visitExpressionCondition(ExpressionCondition node) { } // Action visitors (not used in expression evaluation) - - @Override - public Object visitAssignmentAction(AssignmentAction node) { - throw new UnsupportedOperationException("Actions cannot be evaluated as expressions"); - } - + @Override public Object visitFunctionCallAction(FunctionCallAction node) { throw new UnsupportedOperationException("Actions cannot be evaluated as expressions"); @@ -614,15 +569,16 @@ private boolean endsWith(Object left, Object right) { } private boolean matches(Object left, Object right) { + String leftStr = toString(left); + String rightStr = toString(right); try { - String leftStr = toString(left); - String rightStr = toString(right); - Pattern pattern = REGEX_CACHE.computeIfAbsent(rightStr, Pattern::compile); return pattern.matcher(leftStr).find(); - } catch (Exception e) { - log.warn("Invalid regex pattern: {}", right); - return false; + } catch (java.util.regex.PatternSyntaxException e) { + // Treat a broken pattern as an authoring bug rather than "no match" -- the + // latter silently flips rules to the wrong branch. + throw new IllegalArgumentException( + "Invalid regex pattern '" + rightStr + "': " + e.getDescription(), e); } } @@ -655,7 +611,27 @@ private BigDecimal toNumber(Object value) { } /** - * Safe number conversion that treats null as zero (for arithmetic operations) + * Wrap an exception thrown from a built-in function in an {@link IllegalArgumentException} + * tagged with the function name -- unless the exception already carries good diagnostic + * context, in which case it is rethrown unchanged. Used by the financial / formatting / + * date built-ins to give consistent error messages and avoid the previous catch-and-null + * silent-failure pattern. + */ + private static IllegalArgumentException wrapFunctionError(String functionName, RuntimeException cause) { + if (cause instanceof IllegalArgumentException && cause.getMessage() != null + && cause.getMessage().startsWith(functionName)) { + return (IllegalArgumentException) cause; + } + return new IllegalArgumentException( + functionName + ": " + (cause.getMessage() != null ? cause.getMessage() + : cause.getClass().getSimpleName()), + cause); + } + + /** + * Number conversion that treats null as zero (matching null-as-no-value semantics + * common in financial and SQL contexts), but raises IllegalArgumentException for + * non-numeric strings so type bugs surface instead of silently producing zero. */ private BigDecimal toNumberSafe(Object value) { if (value == null) return BigDecimal.ZERO; @@ -664,7 +640,9 @@ private BigDecimal toNumberSafe(Object value) { try { return new BigDecimal(value.toString()); } catch (NumberFormatException e) { - return BigDecimal.ZERO; + throw new IllegalArgumentException( + "Cannot convert '" + value + "' (type " + value.getClass().getSimpleName() + + ") to a number for arithmetic; check that the operand is numeric"); } } @@ -682,15 +660,17 @@ private String toString(Object value) { return value != null ? value.toString() : ""; } + /** + * Numeric coercion shared by all built-in functions (sum, max, financial calcs, etc). + *

+ * Delegates to {@link #toNumberSafe(Object)} so the entire engine has one consistent + * coercion contract: null treated as zero (matches SQL/financial semantics for missing + * values in aggregations), but any non-numeric string raises + * {@link IllegalArgumentException} so type bugs in rule authoring surface immediately + * rather than being silently zeroed. + */ private BigDecimal toBigDecimal(Object value) { - if (value == null) return BigDecimal.ZERO; - if (value instanceof BigDecimal) return (BigDecimal) value; - if (value instanceof Number) return BigDecimal.valueOf(((Number) value).doubleValue()); - try { - return new BigDecimal(value.toString()); - } catch (NumberFormatException e) { - return BigDecimal.ZERO; - } + return toNumberSafe(value); } // Built-in function implementations @@ -838,79 +818,92 @@ private Object evaluateToday(Object[] args) { private Object evaluateDateAdd(Object[] args) { if (args.length != 3) { - log.warn("DateAdd function requires 3 arguments: date, amount, unit"); - return null; - } - - try { - // Parse the date - java.time.LocalDate date = parseDate(args[0]); - if (date == null) { - log.warn("Invalid date format in dateadd: {}", args[0]); - return null; - } - - // Parse the amount - int amount = toNumber(args[1]).intValue(); - - // Parse the unit - String unit = toString(args[2]).toLowerCase(); - - java.time.LocalDate result = switch (unit) { - case "days", "day", "d" -> date.plusDays(amount); - case "weeks", "week", "w" -> date.plusWeeks(amount); - case "months", "month", "m" -> date.plusMonths(amount); - case "years", "year", "y" -> date.plusYears(amount); - default -> { - log.warn("Unsupported date unit in dateadd: {}. Supported units: days, weeks, months, years", unit); - yield null; - } - }; - - return result != null ? result.toString() : null; - - } catch (Exception e) { - log.warn("Error in dateadd function: {}", e.getMessage()); - return null; - } + throw new IllegalArgumentException( + "dateadd(date, amount, unit) requires exactly 3 arguments; got " + args.length); + } + java.time.LocalDate date = parseDate(args[0]); + if (date == null) { + throw new IllegalArgumentException("dateadd: unparseable date '" + args[0] + "'"); + } + int amount = toNumber(args[1]).intValue(); + String unit = toString(args[2]).toLowerCase(); + java.time.LocalDate result = switch (unit) { + case "days", "day", "d" -> date.plusDays(amount); + case "weeks", "week", "w" -> date.plusWeeks(amount); + case "months", "month", "m" -> date.plusMonths(amount); + case "years", "year", "y" -> date.plusYears(amount); + default -> throw new IllegalArgumentException( + "dateadd: unsupported unit '" + unit + "'. Supported: days, weeks, months, years (and short forms d/w/m/y)."); + }; + return result.toString(); } private Object evaluateDateDiff(Object[] args) { if (args.length < 2 || args.length > 3) { - log.warn("DateDiff function requires 2 or 3 arguments: date1, date2, [unit]"); - return null; + throw new IllegalArgumentException( + "datediff(date1, date2[, unit]) requires 2 or 3 arguments; got " + args.length); + } + java.time.LocalDate date1 = parseDate(args[0]); + if (date1 == null) { + throw new IllegalArgumentException("datediff: unparseable date1 '" + args[0] + "'"); + } + java.time.LocalDate date2 = parseDate(args[1]); + if (date2 == null) { + throw new IllegalArgumentException("datediff: unparseable date2 '" + args[1] + "'"); + } + String unit = args.length > 2 ? toString(args[2]).toLowerCase() : "days"; + java.time.Period period = java.time.Period.between(date1, date2); + long daysDiff = java.time.temporal.ChronoUnit.DAYS.between(date1, date2); + return switch (unit) { + case "days", "day", "d" -> BigDecimal.valueOf(daysDiff); + case "weeks", "week", "w" -> BigDecimal.valueOf(daysDiff / 7); + case "months", "month", "m" -> BigDecimal.valueOf(period.toTotalMonths()); + case "years", "year", "y" -> BigDecimal.valueOf(period.getYears()); + default -> throw new IllegalArgumentException( + "datediff: unsupported unit '" + unit + "'. Supported: days, weeks, months, years (and short forms d/w/m/y)."); + }; + } + + /** + * Compute age in years from a birth date. Accepts ISO {@code yyyy-MM-dd} or any string the + * shared {@link #parseDate(Object)} helper recognises. With one argument, the reference date + * is "today"; with two, the caller supplies the reference date (useful for "age at policy + * inception"). + */ + private Object evaluateCalculateAge(Object[] args) { + if (args.length < 1 || args.length > 2) { + throw new IllegalArgumentException( + "calculate_age(birthDate[, asOfDate]) takes 1 or 2 arguments, got " + args.length); + } + java.time.LocalDate birth = parseDate(args[0]); + if (birth == null) { + throw new IllegalArgumentException("calculate_age: unparseable birth date '" + args[0] + "'"); } + java.time.LocalDate asOf = args.length == 2 ? parseDate(args[1]) : java.time.LocalDate.now(); + if (asOf == null) { + throw new IllegalArgumentException("calculate_age: unparseable reference date '" + args[1] + "'"); + } + return BigDecimal.valueOf(java.time.Period.between(birth, asOf).getYears()); + } + /** + * Format a date using a {@link java.time.format.DateTimeFormatter} pattern. + * Default pattern is ISO {@code yyyy-MM-dd}. + */ + private Object evaluateFormatDate(Object[] args) { + if (args.length < 1 || args.length > 2) { + throw new IllegalArgumentException( + "format_date(date[, pattern]) takes 1 or 2 arguments, got " + args.length); + } + java.time.LocalDate date = parseDate(args[0]); + if (date == null) { + throw new IllegalArgumentException("format_date: unparseable date '" + args[0] + "'"); + } + String pattern = args.length == 2 ? toString(args[1]) : "yyyy-MM-dd"; try { - // Parse the dates - java.time.LocalDate date1 = parseDate(args[0]); - java.time.LocalDate date2 = parseDate(args[1]); - - if (date1 == null || date2 == null) { - log.warn("Invalid date format in datediff: {} or {}", args[0], args[1]); - return null; - } - - // Parse the unit (default to days) - String unit = args.length > 2 ? toString(args[2]).toLowerCase() : "days"; - - java.time.Period period = java.time.Period.between(date1, date2); - long daysDiff = java.time.temporal.ChronoUnit.DAYS.between(date1, date2); - - return switch (unit) { - case "days", "day", "d" -> BigDecimal.valueOf(daysDiff); - case "weeks", "week", "w" -> BigDecimal.valueOf(daysDiff / 7); - case "months", "month", "m" -> BigDecimal.valueOf(period.toTotalMonths()); - case "years", "year", "y" -> BigDecimal.valueOf(period.getYears()); - default -> { - log.warn("Unsupported date unit in datediff: {}. Supported units: days, weeks, months, years", unit); - yield null; - } - }; - - } catch (Exception e) { - log.warn("Error in datediff function: {}", e.getMessage()); - return null; + return date.format(java.time.format.DateTimeFormatter.ofPattern(pattern)); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException("format_date: invalid pattern '" + pattern + "': " + e.getMessage(), e); } } @@ -1003,6 +996,44 @@ private Object evaluateToBoolean(Object[] args) { return toBoolean(args[0]); } + /** + * Return the first non-null argument. Common pattern for default values: + * {@code coalesce(user.preferred_name, user.legal_name, "Unknown")}. + *

+ * Note: arguments are evaluated eagerly (the parser already resolved them before + * the function was called); this differs from short-circuit semantics in some other + * DSLs but matches the rest of this engine's function-call evaluation model. + */ + private Object evaluateCoalesce(Object[] args) { + if (args.length == 0) { + throw new IllegalArgumentException("coalesce(...) requires at least one argument"); + } + for (Object arg : args) { + if (arg != null) { + return arg; + } + } + return null; + } + + /** + * Inline conditional expression: {@code if_else(condition, thenValue, elseValue)}. + *

+ * Equivalent to a ternary operator inside an expression context (such as a + * {@code calculate} or {@code run} action), avoiding a full {@code if/then/else} + * action block when you only need to pick between two values. + *

+ * Note: both branches are evaluated up-front, matching the eager-argument contract + * of the rest of the function-call layer. + */ + private Object evaluateIfElse(Object[] args) { + if (args.length != 3) { + throw new IllegalArgumentException( + "if_else(condition, then, else) requires exactly 3 arguments; got " + args.length); + } + return toBoolean(args[0]) ? args[1] : args[2]; + } + // Property access implementation private Object evaluatePropertyAccess(Object object, String propertyPath) { @@ -1034,21 +1065,41 @@ else if (current instanceof List && part.matches("\\d+")) { } private Object getPropertyValue(Object object, String propertyName) { + // Map-valued objects: property access is a key lookup. Missing key -> null + // (legitimate "missing value" semantics, mirrored by json_get). + if (object instanceof java.util.Map) { + return ((java.util.Map) object).get(propertyName); + } + // Bean access via reflection: try get then is; throw if neither exists. + // A missing getter is almost always a typo in the rule, not a legitimate runtime + // nullable, so surface it rather than masking it as a null value. + String suffix = Character.toUpperCase(propertyName.charAt(0)) + propertyName.substring(1); + Class cls = object.getClass(); + java.lang.reflect.Method accessor = findAccessor(cls, "get" + suffix); + if (accessor == null) { + accessor = findAccessor(cls, "is" + suffix); + } + if (accessor == null) { + throw new IllegalArgumentException( + "No accessor 'get" + suffix + "' or 'is" + suffix + "' on " + cls.getName() + + "; check that property '" + propertyName + "' exists on the bean"); + } try { - // Try getter method first - String getterName = "get" + Character.toUpperCase(propertyName.charAt(0)) + propertyName.substring(1); - java.lang.reflect.Method getter = object.getClass().getMethod(getterName); - return getter.invoke(object); - } catch (Exception e) { - try { - // Try boolean getter - String booleanGetterName = "is" + Character.toUpperCase(propertyName.charAt(0)) + propertyName.substring(1); - java.lang.reflect.Method booleanGetter = object.getClass().getMethod(booleanGetterName); - return booleanGetter.invoke(object); - } catch (Exception e2) { - log.warn("Could not access property '{}' on object of type {}", propertyName, object.getClass().getSimpleName()); - return null; - } + return accessor.invoke(object); + } catch (ReflectiveOperationException e) { + Throwable cause = e.getCause() != null ? e.getCause() : e; + throw new IllegalArgumentException( + "Failed to read property '" + propertyName + "' on " + cls.getName() + + ": " + cause.getMessage(), cause); + } + } + + private static java.lang.reflect.Method findAccessor(Class cls, String name) { + try { + java.lang.reflect.Method m = cls.getMethod(name); + return m.getParameterCount() == 0 ? m : null; + } catch (NoSuchMethodException e) { + return null; } } @@ -1377,9 +1428,8 @@ private Object calculateLoanPayment(Object[] args) { } return result; - } catch (Exception e) { - log.warn("Error calculating loan payment: {}", e.getMessage()); - return null; + } catch (RuntimeException e) { + throw wrapFunctionError("calculate_loan_payment", e); } } @@ -1398,9 +1448,8 @@ private Object calculateCompoundInterest(Object[] args) { BigDecimal compoundFactor = onePlusRate.pow(totalPeriods); return principal.multiply(compoundFactor); - } catch (Exception e) { - log.warn("Error calculating compound interest: {}", e.getMessage()); - return null; + } catch (RuntimeException e) { + throw wrapFunctionError("calculate_compound_interest", e); } } @@ -1424,9 +1473,8 @@ private Object calculateAmortization(Object[] args) { result.put("principal", principal); return result; - } catch (Exception e) { - log.warn("Error calculating amortization: {}", e.getMessage()); - return null; + } catch (RuntimeException e) { + throw wrapFunctionError("calculate_amortization", e); } } @@ -1440,9 +1488,8 @@ private Object debtToIncomeRatio(Object[] args) { // Return as decimal ratio (0.333), not percentage (33.33) return monthlyDebt.divide(monthlyIncome, 4, RoundingMode.HALF_UP); - } catch (Exception e) { - log.warn("Error calculating debt-to-income ratio: {}", e.getMessage()); - return null; + } catch (RuntimeException e) { + throw wrapFunctionError("debt_to_income_ratio", e); } } @@ -1456,9 +1503,8 @@ private Object creditUtilization(Object[] args) { return currentBalance.divide(creditLimit, 4, RoundingMode.HALF_UP) .multiply(BigDecimal.valueOf(100)); - } catch (Exception e) { - log.warn("Error calculating credit utilization: {}", e.getMessage()); - return null; + } catch (RuntimeException e) { + throw wrapFunctionError("credit_utilization", e); } } @@ -1472,9 +1518,8 @@ private Object loanToValue(Object[] args) { return loanAmount.divide(propertyValue, 4, RoundingMode.HALF_UP) .multiply(BigDecimal.valueOf(100)); - } catch (Exception e) { - log.warn("Error calculating loan-to-value ratio: {}", e.getMessage()); - return null; + } catch (RuntimeException e) { + throw wrapFunctionError("loan_to_value", e); } } @@ -1496,9 +1541,8 @@ private Object calculateAPR(Object[] args) { return annualInterest.divide(avgBalance, 4, RoundingMode.HALF_UP) .multiply(BigDecimal.valueOf(100)); - } catch (Exception e) { - log.warn("Error calculating APR: {}", e.getMessage()); - return null; + } catch (RuntimeException e) { + throw wrapFunctionError("calculate_apr", e); } } @@ -1528,9 +1572,8 @@ private Object calculateCreditScore(Object[] args) { if (scaledScore.compareTo(BigDecimal.valueOf(850)) > 0) return 850; return scaledScore.intValue(); - } catch (Exception e) { - log.warn("Error calculating credit score: {}", e.getMessage()); - return null; + } catch (RuntimeException e) { + throw wrapFunctionError("calculate_credit_score", e); } } @@ -1570,9 +1613,8 @@ private Object calculateRiskScore(Object[] args) { } return riskScore; - } catch (Exception e) { - log.warn("Error calculating risk score: {}", e.getMessage()); - return null; + } catch (RuntimeException e) { + throw wrapFunctionError("calculate_risk_score", e); } } @@ -1601,9 +1643,8 @@ private Object paymentHistoryScore(Object[] args) { if (score.compareTo(BigDecimal.valueOf(100)) > 0) return 100; return score; - } catch (Exception e) { - log.warn("Error calculating payment history score: {}", e.getMessage()); - return null; + } catch (RuntimeException e) { + throw wrapFunctionError("payment_history_score", e); } } @@ -1653,9 +1694,8 @@ private Object formatCurrency(Object[] args) { } return formatter.format(amount); - } catch (Exception e) { - log.warn("Error formatting currency: {}", e.getMessage()); - return null; + } catch (RuntimeException e) { + throw wrapFunctionError("format_currency", e); } } @@ -1670,9 +1710,8 @@ private Object formatPercentage(Object[] args) { formatter.setMinimumFractionDigits(decimals); return formatter.format(value.divide(BigDecimal.valueOf(100), 10, RoundingMode.HALF_UP)); - } catch (Exception e) { - log.warn("Error formatting percentage: {}", e.getMessage()); - return null; + } catch (RuntimeException e) { + throw wrapFunctionError("format_percentage", e); } } @@ -1715,9 +1754,8 @@ private Object distanceBetween(Object[] args) { double distance = R * c; return BigDecimal.valueOf(distance); - } catch (Exception e) { - log.warn("Error calculating distance: {}", e.getMessage()); - return null; + } catch (RuntimeException e) { + throw wrapFunctionError("distance_between", e); } } @@ -1739,50 +1777,50 @@ private Object timeHour(Object[] args) { } return null; - } catch (Exception e) { - log.warn("Error extracting hour from timestamp: {}", e.getMessage()); - return null; + } catch (RuntimeException e) { + throw wrapFunctionError("time_hour", e); } } private Object isValid(Object[] args) { - if (args.length < 2) return false; - try { - Object value = args[0]; - String validationType = toString(args[1]); - - return switch (validationType.toLowerCase()) { - case "email", "email_format" -> isEmail(value); - case "phone", "phone_format" -> isPhone(value); - case "ssn", "ssn_format" -> isSSN(value); - case "credit_score" -> isCreditScore(value); - case "account_number" -> isAccountNumber(value); - case "routing_number" -> isRoutingNumber(value); - case "numeric" -> isNumeric(value); - case "date" -> isDate(value); - case "positive" -> isPositive(value); - case "negative" -> isNegative(value); - case "currency" -> isCurrency(value); - case "percentage" -> isPercentage(value); - default -> false; - }; - } catch (Exception e) { - log.warn("Error in validation: {}", e.getMessage()); - return false; + if (args.length < 2) { + throw new IllegalArgumentException( + "is_valid(value, type) requires 2 arguments (value, type); got " + args.length); } + Object value = args[0]; + String validationType = toString(args[1]); + return switch (validationType.toLowerCase()) { + case "email", "email_format" -> isEmail(value); + case "phone", "phone_format" -> isPhone(value); + case "ssn", "ssn_format" -> isSSN(value); + case "credit_score" -> isCreditScore(value); + case "account_number" -> isAccountNumber(value); + case "routing_number" -> isRoutingNumber(value); + case "numeric" -> isNumeric(value); + case "date" -> isDate(value); + case "positive" -> isPositive(value); + case "negative" -> isNegative(value); + case "currency" -> isCurrency(value); + case "percentage" -> isPercentage(value); + default -> throw new IllegalArgumentException( + "is_valid: unknown validation type '" + validationType + "'. " + + "Known types: email, phone, ssn, credit_score, account_number, " + + "routing_number, numeric, date, positive, negative, currency, percentage."); + }; } private Object inRange(Object[] args) { - if (args.length < 3) return false; + if (args.length < 3) { + throw new IllegalArgumentException( + "is_in_range(value, low, high) / in_range(...) requires 3 arguments; got " + args.length); + } try { BigDecimal value = toNumber(args[0]); BigDecimal min = toNumber(args[1]); BigDecimal max = toNumber(args[2]); - return value.compareTo(min) >= 0 && value.compareTo(max) <= 0; - } catch (Exception e) { - log.warn("Error checking range: {}", e.getMessage()); - return false; + } catch (RuntimeException e) { + throw wrapFunctionError("in_range", e); } } @@ -1896,9 +1934,8 @@ private Object calculateDebtRatio(Object[] args) { BigDecimal ratio = totalDebt.divide(totalIncome, 4, RoundingMode.HALF_UP); return ratio.multiply(BigDecimal.valueOf(100)); // Return as percentage - } catch (Exception e) { - log.warn("Error calculating debt ratio: {}", e.getMessage()); - return null; + } catch (RuntimeException e) { + throw wrapFunctionError("calculate_debt_ratio", e); } } @@ -1915,9 +1952,8 @@ private Object calculateLTV(Object[] args) { BigDecimal ltv = loanAmount.divide(propertyValue, 4, RoundingMode.HALF_UP); return ltv.multiply(BigDecimal.valueOf(100)); // Return as percentage - } catch (Exception e) { - log.warn("Error calculating LTV: {}", e.getMessage()); - return null; + } catch (RuntimeException e) { + throw wrapFunctionError("calculate_ltv", e); } } @@ -1956,9 +1992,8 @@ private Object calculatePaymentSchedule(Object[] args) { } return schedule; - } catch (Exception e) { - log.warn("Error calculating payment schedule: {}", e.getMessage()); - return null; + } catch (RuntimeException e) { + throw wrapFunctionError("calculate_payment_schedule", e); } } @@ -2291,7 +2326,8 @@ private Long toLong(Object value) { try { return Long.parseLong(value.toString()); } catch (NumberFormatException e) { - return null; + throw new IllegalArgumentException( + "Cannot convert '" + value + "' to a long; expected integer-like input", e); } } } diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ValidationVisitor.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ValidationVisitor.java index 04a4ac8..91cd2da 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ValidationVisitor.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ValidationVisitor.java @@ -188,50 +188,6 @@ public List visitFunctionCallExpression(FunctionCallExpression return errors; } - @Override - public List visitArithmeticExpression(ArithmeticExpression node) { - List errors = new ArrayList<>(); - - // Validate operands - for (Expression operand : node.getOperands()) { - errors.addAll(operand.accept(this)); - - // Only flag as error if we have definite non-numeric types - if (isDefinitelyNonNumeric(operand.getExpressionType())) { - errors.add(new ValidationError( - "Arithmetic operations require numeric operands", - node.getLocation(), - "VAL_009" - )); - } - } - - // Validate operand count - int operandCount = node.getOperandCount(); - int minOperands = node.getOperation().getMinOperands(); - int maxOperands = node.getOperation().getMaxOperands(); - - if (operandCount < minOperands) { - errors.add(new ValidationError( - String.format("Operation %s requires at least %d operands, got %d", - node.getOperation().getSymbol(), minOperands, operandCount), - node.getLocation(), - "VAL_010" - )); - } - - if (operandCount > maxOperands) { - errors.add(new ValidationError( - String.format("Operation %s allows at most %d operands, got %d", - node.getOperation().getSymbol(), maxOperands, operandCount), - node.getLocation(), - "VAL_011" - )); - } - - return errors; - } - // Condition visitors @Override @@ -303,25 +259,6 @@ public List visitExpressionCondition(ExpressionCondition node) // Action visitors - @Override - public List visitAssignmentAction(AssignmentAction node) { - List errors = new ArrayList<>(); - - // Validate value expression - errors.addAll(node.getValue().accept(this)); - - // Variable name validation (basic check) - if (node.getVariableName() == null || node.getVariableName().trim().isEmpty()) { - errors.add(new ValidationError( - "Assignment action requires a variable name", - node.getLocation(), - "VAL_015" - )); - } - - return errors; - } - @Override public List visitFunctionCallAction(FunctionCallAction node) { List errors = new ArrayList<>(); @@ -418,17 +355,6 @@ private boolean containsNonMathematicalOperation(Expression expression) { if (expression instanceof UnaryExpression unaryExpr) { return containsNonMathematicalOperation(unaryExpr.getOperand()); } - if (expression instanceof ArithmeticExpression arithmeticExpr) { - // Check all operands in the arithmetic expression - if (arithmeticExpr.getOperands() != null) { - for (Expression operand : arithmeticExpr.getOperands()) { - if (containsNonMathematicalOperation(operand)) { - return true; - } - } - } - return false; - } // LiteralExpression and VariableExpression are allowed return false; } diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/services/impl/CacheServiceImpl.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/services/impl/CacheServiceImpl.java index bdcfd58..4ea548f 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/services/impl/CacheServiceImpl.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/services/impl/CacheServiceImpl.java @@ -150,7 +150,8 @@ public void cacheConstant(String code, ConstantDTO constant) { String fullKey = CONSTANT_PREFIX + code; cacheManager.put(fullKey, constant) .doOnError(e -> log.warn("Cache write failed for constant {}: {}", code, e.getMessage())) - .subscribe(); + .onErrorComplete() + .subscribe(); log.debug("Cached constant with code: {}", code); } catch (Exception e) { log.warn("Error caching constant for code: {}", code, e); @@ -178,6 +179,7 @@ public void invalidateConstant(String code) { String fullKey = CONSTANT_PREFIX + code; cacheManager.evict(fullKey) .doOnError(e -> log.warn("Cache evict failed for constant {}: {}", code, e.getMessage())) + .onErrorComplete() .subscribe(); log.debug("Invalidated constants cache entry for code: {}", code); } @@ -186,6 +188,7 @@ public void invalidateConstant(String code) { public void clearConstantsCache() { cacheManager.clear() .doOnError(e -> log.warn("Cache clear failed for constants: {}", e.getMessage())) + .onErrorComplete() .subscribe(); log.info("Cleared all constants cache entries"); } @@ -212,7 +215,8 @@ public void cacheRuleDefinition(String code, RuleDefinitionDTO ruleDefinition) { String fullKey = RULE_DEF_PREFIX + code; cacheManager.put(fullKey, ruleDefinition) .doOnError(e -> log.warn("Cache write failed for rule definition {}: {}", code, e.getMessage())) - .subscribe(); + .onErrorComplete() + .subscribe(); log.debug("Cached rule definition with code: {}", code); } catch (Exception e) { log.warn("Error caching rule definition for code: {}", code, e); @@ -224,6 +228,7 @@ public void invalidateRuleDefinition(String code) { String fullKey = RULE_DEF_PREFIX + code; cacheManager.evict(fullKey) .doOnError(e -> log.warn("Cache evict failed for rule definition {}: {}", code, e.getMessage())) + .onErrorComplete() .subscribe(); log.debug("Invalidated rule definitions cache entry for code: {}", code); } @@ -232,6 +237,7 @@ public void invalidateRuleDefinition(String code) { public void clearRuleDefinitionsCache() { cacheManager.clear() .doOnError(e -> log.warn("Cache clear failed for rule definitions: {}", e.getMessage())) + .onErrorComplete() .subscribe(); log.info("Cleared all rule definitions cache entries"); } @@ -258,7 +264,8 @@ public void cacheValidationResult(String cacheKey, ValidationResult validationRe String fullKey = VALIDATION_PREFIX + cacheKey; cacheManager.put(fullKey, validationResult) .doOnError(e -> log.warn("Cache write failed for validation result {}: {}", cacheKey, e.getMessage())) - .subscribe(); + .onErrorComplete() + .subscribe(); log.debug("Cached validation result with key: {}", cacheKey); } catch (Exception e) { log.warn("Error caching validation result for key: {}", cacheKey, e); @@ -270,6 +277,7 @@ public void invalidateValidationResult(String cacheKey) { String fullKey = VALIDATION_PREFIX + cacheKey; cacheManager.evict(fullKey) .doOnError(e -> log.warn("Cache evict failed for validation result {}: {}", cacheKey, e.getMessage())) + .onErrorComplete() .subscribe(); log.debug("Invalidated validation cache entry for key: {}", cacheKey); } @@ -278,6 +286,7 @@ public void invalidateValidationResult(String cacheKey) { public void clearValidationCache() { cacheManager.clear() .doOnError(e -> log.warn("Cache clear failed for validation cache: {}", e.getMessage())) + .onErrorComplete() .subscribe(); log.info("Cleared all validation cache entries"); } @@ -324,6 +333,7 @@ public CacheProviderInfo getCacheProviderInfo() { public void clearAllCaches() { cacheManager.clear() .doOnError(e -> log.warn("Cache clear failed: {}", e.getMessage())) + .onErrorComplete() .subscribe(); log.info("Cleared all caches"); } diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/validation/YamlDslValidator.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/validation/YamlDslValidator.java index c13a787..e3accd8 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/validation/YamlDslValidator.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/validation/YamlDslValidator.java @@ -658,14 +658,6 @@ public Void visitFunctionCallExpression(FunctionCallExpression node) { return null; } - @Override - public Void visitArithmeticExpression(ArithmeticExpression node) { - if (node.getOperands() != null) { - node.getOperands().forEach(operand -> operand.accept(this)); - } - return null; - } - @Override public Void visitComparisonCondition(ComparisonCondition node) { if (node.getLeft() != null) node.getLeft().accept(this); @@ -687,12 +679,6 @@ public Void visitExpressionCondition(ExpressionCondition node) { return null; } - @Override - public Void visitAssignmentAction(AssignmentAction node) { - if (node.getValue() != null) node.getValue().accept(this); - return null; - } - @Override public Void visitFunctionCallAction(FunctionCallAction node) { if (node.getArguments() != null) { diff --git a/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/audit/AuditTrailIntegrationTest.java b/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/audit/AuditTrailIntegrationTest.java deleted file mode 100644 index 32f97ae..0000000 --- a/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/audit/AuditTrailIntegrationTest.java +++ /dev/null @@ -1,307 +0,0 @@ -/* - * Copyright 2024-2026 Firefly Software Foundation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.fireflyframework.rules.core.audit; - -import org.fireflyframework.rules.core.services.AuditTrailService; -import org.fireflyframework.rules.core.services.impl.AuditTrailServiceImpl; -import org.fireflyframework.rules.interfaces.dtos.audit.AuditEventType; -import org.fireflyframework.rules.interfaces.dtos.audit.AuditTrailDTO; -import org.fireflyframework.rules.interfaces.dtos.audit.AuditTrailFilterDTO; -import org.junit.jupiter.api.Disabled; -import org.junit.jupiter.api.Test; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.test.context.SpringBootTest; -import org.springframework.test.context.ActiveProfiles; -import reactor.test.StepVerifier; - -import java.time.OffsetDateTime; -import java.util.HashMap; -import java.util.Map; -import java.util.UUID; - -import static org.assertj.core.api.Assertions.assertThat; - -/** - * Integration tests for the audit trail system. - * Tests the complete audit trail functionality including service and repository layers. - */ -@Disabled("Requires full Spring Boot context - enable for integration testing") -@SpringBootTest(classes = { - AuditTrailService.class, - AuditTrailServiceImpl.class, - AuditHelper.class -}) -@ActiveProfiles("test") -class AuditTrailIntegrationTest { - - @Autowired - private AuditTrailService auditTrailService; - - @Test - void auditTrailLifecycle_ShouldWorkEndToEnd() { - // Given - UUID entityId = UUID.randomUUID(); - String ruleCode = "integration_test_rule_v1"; - String userId = "integration.test@company.com"; - - // When - Record audit event - StepVerifier.create(auditTrailService.recordAuditEvent( - AuditEventType.RULE_DEFINITION_CREATE, - entityId, - ruleCode, - userId, - "POST", - "/api/v1/rules/definitions", - "{\"code\":\"integration_test_rule\"}", - "{\"id\":\"" + entityId + "\"}", - 201, - true, - 250L - )) - .assertNext(auditTrailDTO -> { - assertThat(auditTrailDTO).isNotNull(); - assertThat(auditTrailDTO.getOperationType()).isEqualTo("RULE_DEFINITION_CREATE"); - assertThat(auditTrailDTO.getEntityType()).isEqualTo("RULE_DEFINITION"); - assertThat(auditTrailDTO.getEntityId()).isEqualTo(entityId); - assertThat(auditTrailDTO.getRuleCode()).isEqualTo(ruleCode); - assertThat(auditTrailDTO.getUserId()).isEqualTo(userId); - assertThat(auditTrailDTO.getSuccess()).isTrue(); - assertThat(auditTrailDTO.getExecutionTimeMs()).isEqualTo(250L); - }) - .verifyComplete(); - - // Then - Retrieve audit trails with filter - AuditTrailFilterDTO filterDTO = AuditTrailFilterDTO.builder() - .operationType("RULE_DEFINITION_CREATE") - .userId(userId) - .page(0) - .size(10) - .build(); - - StepVerifier.create(auditTrailService.getAuditTrails(filterDTO)) - .assertNext(response -> { - assertThat(response).isNotNull(); - assertThat(response.getContent()).isNotEmpty(); - assertThat(response.getContent().get(0).getRuleCode()).isEqualTo(ruleCode); - assertThat(response.getContent().get(0).getUserId()).isEqualTo(userId); - }) - .verifyComplete(); - - // And - Get recent audit trails for entity - StepVerifier.create(auditTrailService.getRecentAuditTrailsForEntity(entityId, 5)) - .assertNext(auditTrails -> { - assertThat(auditTrails).isNotEmpty(); - assertThat(auditTrails.get(0).getEntityId()).isEqualTo(entityId); - assertThat(auditTrails.get(0).getRuleCode()).isEqualTo(ruleCode); - }) - .verifyComplete(); - } - - @Test - void recordAuditEventWithMetadata_ShouldStoreMetadata() { - // Given - Map metadata = new HashMap<>(); - metadata.put("evaluationType", "DIRECT"); - metadata.put("inputDataSize", 5); - metadata.put("conditionResult", true); - metadata.put("circuitBreakerTriggered", false); - - // When - StepVerifier.create(auditTrailService.recordAuditEventWithMetadata( - AuditEventType.RULE_EVALUATION_DIRECT, - null, // entityId - null, // ruleCode - "test.user@company.com", - "POST", - "/api/v1/rules/evaluate/direct", - "{\"inputData\":{\"creditScore\":750}}", - "{\"success\":true,\"conditionResult\":true}", - 200, - true, - 180L, - metadata - )) - .assertNext(auditTrailDTO -> { - assertThat(auditTrailDTO).isNotNull(); - assertThat(auditTrailDTO.getOperationType()).isEqualTo("RULE_EVALUATION_DIRECT"); - assertThat(auditTrailDTO.getEntityType()).isEqualTo("RULE_EVALUATION"); - assertThat(auditTrailDTO.getMetadata()).isNotNull(); - assertThat(auditTrailDTO.getMetadata()).contains("evaluationType"); - assertThat(auditTrailDTO.getMetadata()).contains("DIRECT"); - }) - .verifyComplete(); - } - - @Test - void getAuditTrailStatistics_ShouldReturnCorrectStatistics() { - // Given - Record some audit events first - String userId = "stats.test@company.com"; - - // Record successful operation - auditTrailService.recordAuditEvent( - AuditEventType.RULE_DEFINITION_CREATE, - UUID.randomUUID(), - "stats_test_rule_1", - userId, - "POST", - "/api/v1/rules/definitions", - "{}", - "{}", - 201, - true, - 100L - ).block(); - - // Record failed operation - auditTrailService.recordAuditEvent( - AuditEventType.RULE_DEFINITION_UPDATE, - UUID.randomUUID(), - "stats_test_rule_2", - userId, - "PUT", - "/api/v1/rules/definitions/123", - "{}", - null, - 400, - false, - 50L - ).block(); - - // When - StepVerifier.create(auditTrailService.getAuditTrailStatistics()) - .assertNext(stats -> { - assertThat(stats).isNotNull(); - assertThat(stats.get("totalCount")).isNotNull(); - assertThat(stats.get("successCount")).isNotNull(); - assertThat(stats.get("failureCount")).isNotNull(); - assertThat(stats.get("successRate")).isNotNull(); - assertThat(stats.get("operationCounts")).isNotNull(); - - // Verify that our test data is included - Long totalCount = (Long) stats.get("totalCount"); - assertThat(totalCount).isGreaterThanOrEqualTo(2L); - }) - .verifyComplete(); - } - - @Test - void filterAuditTrailsByDateRange_ShouldReturnCorrectResults() { - // Given - String userId = "daterange.test@company.com"; - OffsetDateTime now = OffsetDateTime.now(); - OffsetDateTime startDate = now.minusHours(1); - OffsetDateTime endDate = now.plusHours(1); - - // Record an audit event - auditTrailService.recordAuditEvent( - AuditEventType.RULE_EVALUATION_BY_CODE, - null, - "daterange_test_rule", - userId, - "POST", - "/api/v1/rules/evaluate/by-code", - "{}", - "{}", - 200, - true, - 120L - ).block(); - - // When - AuditTrailFilterDTO filterDTO = AuditTrailFilterDTO.builder() - .userId(userId) - .startDate(startDate) - .endDate(endDate) - .page(0) - .size(10) - .build(); - - StepVerifier.create(auditTrailService.getAuditTrails(filterDTO)) - .assertNext(response -> { - assertThat(response).isNotNull(); - assertThat(response.getContent()).isNotEmpty(); - - // Verify the audit trail is within the date range - AuditTrailDTO auditTrail = response.getContent().get(0); - assertThat(auditTrail.getUserId()).isEqualTo(userId); - assertThat(auditTrail.getCreatedAt()).isAfter(startDate); - assertThat(auditTrail.getCreatedAt()).isBefore(endDate); - }) - .verifyComplete(); - } - - @Test - void filterAuditTrailsByMultipleCriteria_ShouldReturnFilteredResults() { - // Given - String userId = "multicriteria.test@company.com"; - String ruleCode = "multicriteria_test_rule"; - - // Record audit events with different criteria - auditTrailService.recordAuditEvent( - AuditEventType.RULE_DEFINITION_CREATE, - UUID.randomUUID(), - ruleCode, - userId, - "POST", - "/api/v1/rules/definitions", - "{}", - "{}", - 201, - true, - 150L - ).block(); - - auditTrailService.recordAuditEvent( - AuditEventType.RULE_DEFINITION_UPDATE, - UUID.randomUUID(), - "different_rule", - userId, - "PUT", - "/api/v1/rules/definitions/123", - "{}", - "{}", - 200, - true, - 100L - ).block(); - - // When - Filter by operation type and rule code - AuditTrailFilterDTO filterDTO = AuditTrailFilterDTO.builder() - .operationType("RULE_DEFINITION_CREATE") - .ruleCode(ruleCode) - .userId(userId) - .success(true) - .page(0) - .size(10) - .build(); - - StepVerifier.create(auditTrailService.getAuditTrails(filterDTO)) - .assertNext(response -> { - assertThat(response).isNotNull(); - assertThat(response.getContent()).isNotEmpty(); - - // Verify all results match the filter criteria - response.getContent().forEach(auditTrail -> { - assertThat(auditTrail.getOperationType()).isEqualTo("RULE_DEFINITION_CREATE"); - assertThat(auditTrail.getRuleCode()).isEqualTo(ruleCode); - assertThat(auditTrail.getUserId()).isEqualTo(userId); - assertThat(auditTrail.getSuccess()).isTrue(); - }); - }) - .verifyComplete(); - } -} diff --git a/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/ASTParserIntegrationTest.java b/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/ASTParserIntegrationTest.java index 63dd5c1..049b49c 100644 --- a/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/ASTParserIntegrationTest.java +++ b/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/ASTParserIntegrationTest.java @@ -238,14 +238,27 @@ void testConditionalAction() { } @Test - @DisplayName("Should parse and execute CALL actions") - void testCallAction() { + @DisplayName("Should parse and execute CALL actions for known built-in functions") + void testCallActionWithBuiltin() { + // `log` is a built-in action function; `validateCredit` (the previous test's name) + // does not exist, and after the silent-null-on-unknown-function fix the executor + // now throws -- intentional behaviour to surface typos. assertThatCode(() -> { - Action action = dslParser.parseAction("call validateCredit with [score, income]"); + Action action = dslParser.parseAction("call log with [\"score check\", \"INFO\"]"); ActionExecutor executor = new ActionExecutor(context); action.accept(executor); }).doesNotThrowAnyException(); } + + @Test + @DisplayName("CALL action with unknown function name surfaces a typed error") + void testCallActionUnknownFunctionThrows() { + Action action = dslParser.parseAction("call validateCredit with [score, income]"); + ActionExecutor executor = new ActionExecutor(context); + assertThatThrownBy(() -> action.accept(executor)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("validateCredit"); + } } @Nested diff --git a/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/AdvancedDSLFeaturesTest.java b/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/AdvancedDSLFeaturesTest.java index 235f9ab..b36ec42 100644 --- a/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/AdvancedDSLFeaturesTest.java +++ b/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/AdvancedDSLFeaturesTest.java @@ -30,6 +30,7 @@ import java.util.Map; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.*; /** @@ -499,49 +500,75 @@ void testDateFunctionsWithDifferentFormats() { } @Test - @DisplayName("Test date functions error handling") - void testDateFunctionsErrorHandling() { + @DisplayName("dateadd succeeds for valid dates") + void testDateAddValid() { String yamlRule = """ - name: "Date Error Handling Test" - description: "Test date functions with invalid inputs" - + name: "Date Add Happy Path" inputs: - validDate - - invalidDate - - when: - - validDate is_not_null then: - run validResult as dateadd(validDate, 1, "days") - - run invalidResult as dateadd(invalidDate, 1, "days") - - run invalidUnit as dateadd(validDate, 1, "invalid_unit") - - run diffResult as datediff(validDate, invalidDate, "days") - set result to "COMPLETED" output: validResult: text - invalidResult: text - invalidUnit: text - diffResult: number result: text """; - Map inputData = Map.of( - "validDate", "2024-01-01", - "invalidDate", "not-a-date" - ); - - ASTRulesEvaluationResult result = evaluationEngine.evaluateRules(yamlRule, inputData); + ASTRulesEvaluationResult result = evaluationEngine.evaluateRules( + yamlRule, Map.of("validDate", "2024-01-01")); assertTrue(result.isSuccess()); assertEquals("2024-01-02", result.getOutputData().get("validResult")); - assertNull(result.getOutputData().get("invalidResult")); - assertNull(result.getOutputData().get("invalidUnit")); - assertNull(result.getOutputData().get("diffResult")); assertEquals("COMPLETED", result.getOutputData().get("result")); } + @Test + @DisplayName("dateadd surfaces unparseable dates as a clean rule failure") + void testDateAddInvalidDateFailsLoud() { + String yamlRule = """ + name: "Date Add Invalid" + inputs: + - badDate + + then: + - run result as dateadd(badDate, 1, "days") + + output: + result: text + """; + + ASTRulesEvaluationResult result = evaluationEngine.evaluateRules( + yamlRule, Map.of("badDate", "not-a-date")); + + assertFalse(result.isSuccess()); + assertThat(result.getError()).contains("dateadd"); + } + + @Test + @DisplayName("dateadd surfaces unknown units as a clean rule failure") + void testDateAddInvalidUnitFailsLoud() { + String yamlRule = """ + name: "Date Add Invalid Unit" + inputs: + - validDate + + then: + - run result as dateadd(validDate, 1, "invalid_unit") + + output: + result: text + """; + + ASTRulesEvaluationResult result = evaluationEngine.evaluateRules( + yamlRule, Map.of("validDate", "2024-01-01")); + + assertFalse(result.isSuccess()); + assertThat(result.getError()).contains("dateadd"); + assertThat(result.getError()).contains("invalid_unit"); + } + @Test @DisplayName("Test operator aliases recognition") void testOperatorAliases() { diff --git a/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/DoWhileAndConditionFunctionTest.java b/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/DoWhileAndConditionFunctionTest.java new file mode 100644 index 0000000..f08a124 --- /dev/null +++ b/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/DoWhileAndConditionFunctionTest.java @@ -0,0 +1,132 @@ +/* + * Copyright 2024-2026 Firefly Software Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.fireflyframework.rules.core.dsl; + +import org.fireflyframework.rules.core.dsl.evaluation.ASTRulesEvaluationEngine; +import org.fireflyframework.rules.core.dsl.evaluation.ASTRulesEvaluationResult; +import org.fireflyframework.rules.core.dsl.function.CustomFunctionRegistry; +import org.fireflyframework.rules.core.dsl.parser.ASTRulesDSLParser; +import org.fireflyframework.rules.core.dsl.parser.DSLParser; +import org.fireflyframework.rules.core.services.ConstantService; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import reactor.core.publisher.Flux; + +import java.math.BigDecimal; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Closes two specific test gaps identified by the phase-4 DSL gap audit: + * + *

    + *
  1. {@code do-while} loop semantics (the existing test suite covered {@code forEach} + * and {@code while} but not {@code do-while}).
    2. + *
  2. {@code CustomFunctionRegistry} usage from condition expressions, not + * just actions -- to confirm extension functions are reachable through the + * evaluator from both code paths.
    3. + *
+ */ +class DoWhileAndConditionFunctionTest { + + private ASTRulesEvaluationEngine engine; + private CustomFunctionRegistry registry; + + @BeforeEach + void setUp() { + DSLParser dslParser = new DSLParser(); + ASTRulesDSLParser parser = new ASTRulesDSLParser(dslParser); + ConstantService constants = Mockito.mock(ConstantService.class); + Mockito.when(constants.getConstantsByCodes(Mockito.anyList())).thenReturn(Flux.empty()); + registry = new CustomFunctionRegistry(); + engine = new ASTRulesEvaluationEngine(parser, constants, null, null, registry); + } + + @Test + @DisplayName("do-while executes body once then re-checks the condition each iteration") + void doWhileExecutesAtLeastOnce() { + String yaml = """ + inputs: + startValue: "number" + then: + - set counter to startValue + - do: add 1 to counter while counter < 5 + output: + counter: counter + """; + + // Start below cap: should loop until counter == 5 + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, Map.of("startValue", 2)); + assertThat(result.isSuccess()).isTrue(); + assertThat(new BigDecimal(result.getOutputData().get("counter").toString())) + .isEqualByComparingTo(new BigDecimal("5")); + } + + @Test + @DisplayName("do-while runs the body at least once even when the condition is already false") + void doWhileRunsBodyAtLeastOnceWhenConditionInitiallyFalse() { + String yaml = """ + inputs: + startValue: "number" + then: + - set counter to startValue + - do: add 1 to counter while counter < 5 + output: + counter: counter + """; + + // Start at 5: condition is false from the start, but do-while always runs the body once + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, Map.of("startValue", 5)); + assertThat(result.isSuccess()).isTrue(); + assertThat(new BigDecimal(result.getOutputData().get("counter").toString())) + .isEqualByComparingTo(new BigDecimal("6")); + } + + @Test + @DisplayName("Custom functions are callable from condition expressions, not just from actions") + void customFunctionReachableFromCondition() { + // Register a custom function that flags VIP customers + registry.register("is_vip", args -> { + String id = String.valueOf(args[0]); + return id.startsWith("VIP-"); + }); + + String yaml = """ + inputs: + customerId: "string" + when: + - is_vip(customerId) equals true + then: + - set tier to "PREMIUM" + else: + - set tier to "STANDARD" + output: + tier: tier + """; + + ASTRulesEvaluationResult vipResult = engine.evaluateRules(yaml, Map.of("customerId", "VIP-001")); + assertThat(vipResult.isSuccess()).isTrue(); + assertThat(vipResult.getOutputData()).containsEntry("tier", "PREMIUM"); + + ASTRulesEvaluationResult standardResult = engine.evaluateRules(yaml, Map.of("customerId", "STD-001")); + assertThat(standardResult.isSuccess()).isTrue(); + assertThat(standardResult.getOutputData()).containsEntry("tier", "STANDARD"); + } +} diff --git a/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/DslPrimitivesTest.java b/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/DslPrimitivesTest.java new file mode 100644 index 0000000..c786a25 --- /dev/null +++ b/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/DslPrimitivesTest.java @@ -0,0 +1,223 @@ +/* + * Copyright 2024-2026 Firefly Software Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.fireflyframework.rules.core.dsl; + +import org.fireflyframework.rules.core.dsl.evaluation.ASTRulesEvaluationEngine; +import org.fireflyframework.rules.core.dsl.evaluation.ASTRulesEvaluationResult; +import org.fireflyframework.rules.core.dsl.parser.ASTRulesDSLParser; +import org.fireflyframework.rules.core.dsl.parser.DSLParser; +import org.fireflyframework.rules.core.services.ConstantService; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import reactor.core.publisher.Flux; + +import java.math.BigDecimal; +import java.util.HashMap; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for new DSL function primitives added to round out the built-in catalog: + * {@code coalesce}, {@code if_else}, {@code is_in_range}, plus the documented-but- + * previously-missing {@code calculate_age}, {@code format_date}, {@code validate_email}, + * {@code validate_phone}. + */ +class DslPrimitivesTest { + + private ASTRulesEvaluationEngine engine; + + @BeforeEach + void setUp() { + DSLParser dslParser = new DSLParser(); + ASTRulesDSLParser parser = new ASTRulesDSLParser(dslParser); + ConstantService constantService = Mockito.mock(ConstantService.class); + Mockito.when(constantService.getConstantsByCodes(Mockito.anyList())).thenReturn(Flux.empty()); + engine = new ASTRulesEvaluationEngine(parser, constantService); + } + + @Test + @DisplayName("coalesce returns the first non-null argument") + void coalesceReturnsFirstNonNull() { + String yaml = """ + then: + - run preferred as coalesce(nickname, full_name, "Anonymous") + output: + preferred: preferred + """; + Map input = new HashMap<>(); + input.put("nickname", null); + input.put("full_name", "Jane Doe"); + + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, input); + + assertThat(result.isSuccess()).isTrue(); + assertThat(result.getOutputData()).containsEntry("preferred", "Jane Doe"); + } + + @Test + @DisplayName("coalesce falls all the way through to a literal default") + void coalesceFallsThroughToDefault() { + String yaml = """ + then: + - run preferred as coalesce(nickname, full_name, "Anonymous") + output: + preferred: preferred + """; + Map input = new HashMap<>(); + input.put("nickname", null); + input.put("full_name", null); + + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, input); + + assertThat(result.isSuccess()).isTrue(); + assertThat(result.getOutputData()).containsEntry("preferred", "Anonymous"); + } + + @Test + @DisplayName("if_else picks the then-branch when the condition is truthy") + void ifElseTruthyPicksThen() { + String yaml = """ + then: + - run category as if_else(age >= 18, "adult", "minor") + output: + category: category + """; + + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, Map.of("age", 21)); + + assertThat(result.isSuccess()).isTrue(); + assertThat(result.getOutputData()).containsEntry("category", "adult"); + } + + @Test + @DisplayName("if_else picks the else-branch when the condition is falsey") + void ifElseFalseyPicksElse() { + String yaml = """ + then: + - run category as if_else(age >= 18, "adult", "minor") + output: + category: category + """; + + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, Map.of("age", 12)); + + assertThat(result.isSuccess()).isTrue(); + assertThat(result.getOutputData()).containsEntry("category", "minor"); + } + + @Test + @DisplayName("is_in_range returns true within bounds, false outside (inclusive on both ends)") + void isInRangeBoundary() { + String yaml = """ + then: + - run lowOk as is_in_range(50, 50, 100) + - run highOk as is_in_range(100, 50, 100) + - run below as is_in_range(49, 50, 100) + - run above as is_in_range(101, 50, 100) + output: + lowOk: lowOk + highOk: highOk + below: below + above: above + """; + + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, Map.of()); + + assertThat(result.isSuccess()).isTrue(); + assertThat(result.getOutputData()).containsEntry("lowOk", true); + assertThat(result.getOutputData()).containsEntry("highOk", true); + assertThat(result.getOutputData()).containsEntry("below", false); + assertThat(result.getOutputData()).containsEntry("above", false); + } + + @Test + @DisplayName("calculate_age returns whole years between birth date and reference date") + void calculateAgeWithExplicitReferenceDate() { + String yaml = """ + then: + - run age as calculate_age("1990-06-15", "2024-06-14") + - run ageAtBirthday as calculate_age("1990-06-15", "2024-06-15") + output: + age: age + ageAtBirthday: ageAtBirthday + """; + + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, Map.of()); + + assertThat(result.isSuccess()).isTrue(); + assertThat(result.getOutputData()).containsEntry("age", new BigDecimal("33")); + assertThat(result.getOutputData()).containsEntry("ageAtBirthday", new BigDecimal("34")); + } + + @Test + @DisplayName("format_date applies a DateTimeFormatter pattern") + void formatDateWithPattern() { + String yaml = """ + then: + - run iso as format_date("2024-06-15") + - run pretty as format_date("2024-06-15", "dd MMM yyyy") + output: + iso: iso + pretty: pretty + """; + + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, Map.of()); + + assertThat(result.isSuccess()).isTrue(); + assertThat(result.getOutputData()).containsEntry("iso", "2024-06-15"); + // Locale-dependent month abbreviation; just check the structure + assertThat((String) result.getOutputData().get("pretty")).matches("15 \\w{3} 2024"); + } + + @Test + @DisplayName("validate_email function form matches the is_email operator") + void validateEmailFunctionForm() { + String yaml = """ + then: + - run good as validate_email("alice@example.com") + - run bad as validate_email("not-an-email") + output: + good: good + bad: bad + """; + + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, Map.of()); + + assertThat(result.isSuccess()).isTrue(); + assertThat(result.getOutputData()).containsEntry("good", true); + assertThat(result.getOutputData()).containsEntry("bad", false); + } + + @Test + @DisplayName("is_valid with unknown validation type surfaces a clear error") + void isValidUnknownTypeThrows() { + String yaml = """ + then: + - run check as is_valid(value, "no_such_type") + output: + check: check + """; + + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, Map.of("value", "x")); + + assertThat(result.isSuccess()).isFalse(); + assertThat(result.getError()).contains("no_such_type"); + } +} diff --git a/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/EndToEndScenarioTest.java b/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/EndToEndScenarioTest.java new file mode 100644 index 0000000..a75ea86 --- /dev/null +++ b/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/EndToEndScenarioTest.java @@ -0,0 +1,249 @@ +/* + * Copyright 2024-2026 Firefly Software Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.fireflyframework.rules.core.dsl; + +import org.fireflyframework.rules.core.dsl.evaluation.ASTRulesEvaluationEngine; +import org.fireflyframework.rules.core.dsl.evaluation.ASTRulesEvaluationResult; +import org.fireflyframework.rules.core.dsl.function.CustomFunctionRegistry; +import org.fireflyframework.rules.core.dsl.parser.ASTRulesDSLParser; +import org.fireflyframework.rules.core.dsl.parser.DSLParser; +import org.fireflyframework.rules.core.services.ConstantService; +import org.fireflyframework.rules.interfaces.dtos.crud.ConstantDTO; +import org.fireflyframework.rules.interfaces.enums.ValueType; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import reactor.core.publisher.Flux; + +import java.math.BigDecimal; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyList; + +/** + * End-to-end DSL integration test that exercises the full evaluation pipeline against a + * realistic, multi-step rule. Single failures here usually indicate a regression in one + * of: parser, AST cache, constant resolution, condition evaluation, action execution, + * arithmetic coercion, loops, sub-rules, circuit breaker, custom-function registry, or + * error propagation. + * + *

The scenario models a simplified loan-eligibility pipeline. It is wide rather than + * deep on purpose -- a "smoke test" that catches integration regressions a single-purpose + * unit test would miss.

+ */ +class EndToEndScenarioTest { + + private ASTRulesEvaluationEngine engine; + private CustomFunctionRegistry registry; + + @BeforeEach + void setUp() { + DSLParser dslParser = new DSLParser(); + ASTRulesDSLParser parser = new ASTRulesDSLParser(dslParser); + + ConstantService constants = Mockito.mock(ConstantService.class); + Mockito.when(constants.getConstantsByCodes(anyList())).thenReturn(Flux.just( + ConstantDTO.builder().code("MIN_CREDIT_SCORE").valueType(ValueType.NUMBER) + .currentValue(new BigDecimal("650")).build(), + ConstantDTO.builder().code("MIN_ANNUAL_INCOME").valueType(ValueType.NUMBER) + .currentValue(new BigDecimal("50000")).build(), + ConstantDTO.builder().code("MAX_DTI").valueType(ValueType.NUMBER) + .currentValue(new BigDecimal("0.4")).build() + )); + + registry = new CustomFunctionRegistry(); + registry.register("regional_risk_score", + args -> ("CA".equals(args[0]) || "NY".equals(args[0])) ? 10 : 0); + + engine = new ASTRulesEvaluationEngine(parser, constants, null, null, registry); + } + + /** Multi-stage loan-eligibility rule covering most DSL surface area. */ + private static final String LOAN_RULE = """ + name: "Loan Eligibility End-to-End" + description: "Exercises constants, sub-rules, loops, conditionals, and custom functions" + + inputs: + creditScore: "number" + annualIncome: "number" + existingDebtPayments: "list" + region: "string" + applicant_email: "string" + + rules: + - name: "Financial Validation" + when: + - creditScore at_least MIN_CREDIT_SCORE + - annualIncome at_least MIN_ANNUAL_INCOME + then: + - set financial_check to "PASSED" + - calculate monthly_income as annualIncome / 12 + else: + - set financial_check to "FAILED" + + - name: "Debt Aggregation" + then: + - set total_debt to 0 + - forEach payment in existingDebtPayments: add payment to total_debt + + - name: "DTI Computation" + then: + - calculate debt_to_income as total_debt / annualIncome + + - name: "Final Decision" + when: + - financial_check equals "PASSED" + - debt_to_income at_most MAX_DTI + then: + - run risk_adjustment as regional_risk_score(region) + - run validated_email as validate_email(applicant_email) + - run tier as if_else(creditScore at_least 750, "PRIME", "STANDARD") + - run display_name as coalesce(applicant_email, "anonymous") + - set decision to "APPROVED" + else: + - set decision to "DECLINED" + + output: + decision: decision + tier: tier + monthly_income: monthly_income + total_debt: total_debt + debt_to_income: debt_to_income + risk_adjustment: risk_adjustment + validated_email: validated_email + display_name: display_name + """; + + @Test + @DisplayName("Approves an applicant who satisfies every rule and computes all derived values") + void approvedApplicantEndToEnd() { + Map input = new HashMap<>(); + input.put("creditScore", 760); + input.put("annualIncome", new BigDecimal("120000")); + input.put("existingDebtPayments", List.of(new BigDecimal("400"), new BigDecimal("250"), new BigDecimal("150"))); + input.put("region", "CA"); + input.put("applicant_email", "alice@example.com"); + + ASTRulesEvaluationResult result = engine.evaluateRules(LOAN_RULE, input); + + assertThat(result.isSuccess()).isTrue(); + Map out = result.getOutputData(); + assertThat(out).containsEntry("decision", "APPROVED"); + assertThat(out).containsEntry("tier", "PRIME"); + assertThat(out).containsEntry("validated_email", true); + assertThat(out).containsEntry("display_name", "alice@example.com"); + assertThat(out).containsEntry("risk_adjustment", 10); + // monthly_income = 120000 / 12 = 10000 (BigDecimal with 10-scale division) + assertThat(new BigDecimal(out.get("monthly_income").toString())) + .isEqualByComparingTo(new BigDecimal("10000")); + // total_debt = 400 + 250 + 150 = 800 + assertThat(new BigDecimal(out.get("total_debt").toString())) + .isEqualByComparingTo(new BigDecimal("800")); + // debt_to_income = 800 / 120000 ≈ 0.0067 (well under MAX_DTI 0.4) + BigDecimal dti = new BigDecimal(out.get("debt_to_income").toString()); + assertThat(dti).isLessThan(new BigDecimal("0.4")); + } + + @Test + @DisplayName("Declines an applicant who fails the financial-validation gate") + void declinedOnLowCreditScore() { + Map input = new HashMap<>(); + input.put("creditScore", 600); // < MIN_CREDIT_SCORE + input.put("annualIncome", new BigDecimal("120000")); + input.put("existingDebtPayments", List.of(new BigDecimal("400"))); + input.put("region", "TX"); + input.put("applicant_email", "bob@example.com"); + + ASTRulesEvaluationResult result = engine.evaluateRules(LOAN_RULE, input); + + assertThat(result.isSuccess()).isTrue(); + Map out = result.getOutputData(); + assertThat(out).containsEntry("decision", "DECLINED"); + assertThat(out).containsEntry("financial_check", "FAILED"); + } + + @Test + @DisplayName("Picks STANDARD tier when credit score is below the PRIME cutoff") + void standardTierBelowPrimeCutoff() { + Map input = new HashMap<>(); + input.put("creditScore", 700); // qualifies but < 750 + input.put("annualIncome", new BigDecimal("80000")); + input.put("existingDebtPayments", List.of(new BigDecimal("500"))); + input.put("region", "TX"); + input.put("applicant_email", "carol@example.com"); + + ASTRulesEvaluationResult result = engine.evaluateRules(LOAN_RULE, input); + + assertThat(result.isSuccess()).isTrue(); + Map out = result.getOutputData(); + assertThat(out).containsEntry("decision", "APPROVED"); + assertThat(out).containsEntry("tier", "STANDARD"); + assertThat(out).containsEntry("risk_adjustment", 0); // non-CA/NY region + } + + @Test + @DisplayName("Handles an empty debt list without arithmetic errors") + void zeroDebtScenarioComputesZeroDti() { + Map input = new HashMap<>(); + input.put("creditScore", 800); + input.put("annualIncome", new BigDecimal("100000")); + input.put("existingDebtPayments", List.of()); // no debt + input.put("region", "NY"); + input.put("applicant_email", "dave@example.com"); + + ASTRulesEvaluationResult result = engine.evaluateRules(LOAN_RULE, input); + + assertThat(result.isSuccess()).isTrue(); + Map out = result.getOutputData(); + assertThat(out).containsEntry("decision", "APPROVED"); + assertThat(new BigDecimal(out.get("total_debt").toString())) + .isEqualByComparingTo(BigDecimal.ZERO); + assertThat(new BigDecimal(out.get("debt_to_income").toString())) + .isEqualByComparingTo(BigDecimal.ZERO); + } + + @Test + @DisplayName("Circuit-breaker action stops evaluation cleanly with structured metadata") + void circuitBreakerStopsEvaluation() { + String yaml = """ + inputs: + riskScore: "number" + when: + - riskScore at_least 0 + then: + - set started to true + - if riskScore at_least 90 then circuit_breaker "HIGH_RISK" + - set finished to true + output: + started: started + finished: finished + """; + + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, Map.of("riskScore", 95)); + + assertThat(result.isSuccess()).isTrue(); // circuit breaker is a controlled stop + assertThat(result.isCircuitBreakerTriggered()).isTrue(); + assertThat(result.getCircuitBreakerMessage()).contains("HIGH_RISK"); + // started was set before the break; finished should NOT be set + assertThat(result.getOutputData()).containsEntry("started", true); + assertThat(result.getOutputData()).doesNotContainKey("finished"); + } +} diff --git a/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/function/CustomFunctionRegistryTest.java b/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/function/CustomFunctionRegistryTest.java new file mode 100644 index 0000000..14cd184 --- /dev/null +++ b/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/function/CustomFunctionRegistryTest.java @@ -0,0 +1,156 @@ +/* + * Copyright 2024-2026 Firefly Software Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.fireflyframework.rules.core.dsl.function; + +import org.fireflyframework.rules.core.dsl.evaluation.ASTRulesEvaluationEngine; +import org.fireflyframework.rules.core.dsl.evaluation.ASTRulesEvaluationResult; +import org.fireflyframework.rules.core.dsl.parser.ASTRulesDSLParser; +import org.fireflyframework.rules.core.dsl.parser.DSLParser; +import org.fireflyframework.rules.core.services.ConstantService; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import reactor.core.publisher.Flux; + +import java.math.BigDecimal; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Behavioural tests for {@link CustomFunctionRegistry} and its integration with the + * evaluation engine. These cover the public extension contract: name resolution, + * shadowing of built-ins, error semantics, and unknown-function diagnostics. + */ +class CustomFunctionRegistryTest { + + private CustomFunctionRegistry registry; + private ASTRulesEvaluationEngine engine; + + @BeforeEach + void setUp() { + registry = new CustomFunctionRegistry(); + DSLParser dslParser = new DSLParser(); + ASTRulesDSLParser parser = new ASTRulesDSLParser(dslParser); + ConstantService constantService = Mockito.mock(ConstantService.class); + Mockito.when(constantService.getConstantsByCodes(Mockito.anyList())).thenReturn(Flux.empty()); + engine = new ASTRulesEvaluationEngine(parser, constantService, null, null, registry); + } + + @Test + @DisplayName("Registering a custom function makes it callable from `run`") + void registeredFunctionCallableFromRun() { + registry.register("triple", args -> ((Number) args[0]).intValue() * 3); + + String yaml = """ + when: + - amount at_least 0 + then: + - run result as triple(amount) + output: + result: result + """; + + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, Map.of("amount", 7)); + + assertThat(result.isSuccess()).isTrue(); + assertThat(result.getOutputData()).containsEntry("result", 21); + } + + @Test + @DisplayName("Custom function shadows built-in of the same name") + void customFunctionShadowsBuiltin() { + // The built-in `max` returns the largest argument. Shadow it with a no-op that returns 999. + registry.register("max", args -> BigDecimal.valueOf(999)); + + String yaml = """ + when: + - score at_least 0 + then: + - run winner as max(score, 1) + output: + winner: winner + """; + + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, Map.of("score", 5)); + + assertThat(result.isSuccess()).isTrue(); + assertThat(result.getOutputData()).containsEntry("winner", BigDecimal.valueOf(999)); + } + + @Test + @DisplayName("Unknown function name surfaces as a structured evaluation failure (not silent null)") + void unknownFunctionFailsEvaluation() { + String yaml = """ + when: + - amount at_least 0 + then: + - run result as no_such_function(amount) + output: + result: result + """; + + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, Map.of("amount", 1)); + + // The engine now fails the whole evaluation rather than silently swallowing the + // unknown-function error and continuing past the broken action. + assertThat(result.isSuccess()).isFalse(); + assertThat(result.getError()).contains("no_such_function"); + } + + @Test + @DisplayName("Lookup is case-insensitive") + void lookupIsCaseInsensitive() { + registry.register("MyFunc", args -> "called"); + + assertThat(registry.lookup("myfunc")).isPresent(); + assertThat(registry.lookup("MYFUNC")).isPresent(); + assertThat(registry.lookup("MyFunc")).isPresent(); + } + + @Test + @DisplayName("Re-registering a name replaces the previous function") + void reRegistrationReplaces() { + registry.register("f", args -> "v1"); + registry.register("f", args -> "v2"); + + assertThat(registry.lookup("f").orElseThrow().apply(new Object[0])).isEqualTo("v2"); + } + + @Test + @DisplayName("Unregister removes the function") + void unregisterRemoves() { + registry.register("temp", args -> 1); + assertThat(registry.contains("temp")).isTrue(); + assertThat(registry.unregister("temp")).isTrue(); + assertThat(registry.contains("temp")).isFalse(); + assertThat(registry.unregister("temp")).isFalse(); + } + + @Test + @DisplayName("Blank name and null function are rejected at registration time") + void rejectsInvalidInputs() { + assertThatThrownBy(() -> registry.register("", args -> null)) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> registry.register(null, args -> null)) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> registry.register("ok", null)) + .isInstanceOf(IllegalArgumentException.class); + } +} diff --git a/fireflyframework-rule-engine-interfaces/pom.xml b/fireflyframework-rule-engine-interfaces/pom.xml index 2444bc2..dffce6c 100644 --- a/fireflyframework-rule-engine-interfaces/pom.xml +++ b/fireflyframework-rule-engine-interfaces/pom.xml @@ -6,7 +6,7 @@ org.fireflyframework fireflyframework-rule-engine - 26.05.07 + 26.05.08 fireflyframework-rule-engine-interfaces diff --git a/fireflyframework-rule-engine-models/pom.xml b/fireflyframework-rule-engine-models/pom.xml index fab447d..7b024c3 100644 --- a/fireflyframework-rule-engine-models/pom.xml +++ b/fireflyframework-rule-engine-models/pom.xml @@ -6,7 +6,7 @@ org.fireflyframework fireflyframework-rule-engine - 26.05.07 + 26.05.08 fireflyframework-rule-engine-models diff --git a/fireflyframework-rule-engine-sdk/pom.xml b/fireflyframework-rule-engine-sdk/pom.xml index d86c3da..39eccdd 100644 --- a/fireflyframework-rule-engine-sdk/pom.xml +++ b/fireflyframework-rule-engine-sdk/pom.xml @@ -6,7 +6,7 @@ org.fireflyframework fireflyframework-rule-engine - 26.05.07 + 26.05.08 fireflyframework-rule-engine-sdk diff --git a/fireflyframework-rule-engine-web/pom.xml b/fireflyframework-rule-engine-web/pom.xml index 58189cf..c48c796 100644 --- a/fireflyframework-rule-engine-web/pom.xml +++ b/fireflyframework-rule-engine-web/pom.xml @@ -6,7 +6,7 @@ org.fireflyframework fireflyframework-rule-engine - 26.05.07 + 26.05.08 fireflyframework-rule-engine-web diff --git a/pom.xml b/pom.xml index 193d3ed..a10c50f 100644 --- a/pom.xml +++ b/pom.xml @@ -13,7 +13,7 @@ org.fireflyframework fireflyframework-rule-engine - 26.05.07 + 26.05.08 pom Firefly Framework - Rule Engine Library From ac49281c3a2749f1a83972ca948fdcb1eee67132 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Contreras=20Guill=C3=A9n?= Date: Sun, 24 May 2026 20:12:24 +0200 Subject: [PATCH 2/3] docs(dsl): full audit of YAML reference; add DocExamplesValidationTest guard Audits every YAML example in the docs against the actual parser source-of- truth, fixes a flurry of documentation bugs, and locks the docs to the implementation at build time via a new parameterised validation test. What was wrong -------------- - 12 examples in yaml-dsl-reference.md and 4 elsewhere used `calculate ... as (...)`. The DSL restricts `calculate` to pure-math expressions and raises IllegalArgumentException for function/REST/JSON calls; these examples couldn't actually be evaluated. Corrected to use `run`. - Arithmetic-action grammar was documented backwards. The parser is ` `, so the correct form is `multiply 1.5 by risk_factor`, not `multiply risk_factor by 1.5`. Updated the operator table and examples; added an explicit grammar note. - "Validation operators in expressions" example used C-style ternary `(... ? X : Y)` -- a syntax the parser doesn't have. Rewritten using the `if_else(cond, then, else)` built-in (which is documented in the same doc). - Several complete examples in docs/yaml-dsl-reference.md, common-patterns- guide.md, b2b-credit-scoring-tutorial.md, and quick-start-guide.md used unquoted YAML strings containing colons (e.g., `"Loan approved: " + amount`) or other patterns that the YAML / DSL parser rejects. Fixed where the rewrite was mechanical; tagged the rest with TODO skip markers (see below). New: DocExamplesValidationTest ------------------------------ - Extracts every fenced ```yaml block from README.md, docs/yaml-dsl- reference.md, docs/quick-start-guide.md, docs/common-patterns-guide.md, and docs/b2b-credit-scoring-tutorial.md (60 blocks total). - Skips blocks that don't look like complete rules (missing every top-level key), and skips blocks explicitly tagged with `` in the surrounding markdown (with an optional trailing rationale parenthetical). - Parses each remaining block through the real `ASTRulesDSLParser` and fails the build with the file:line of the offending block if parsing throws. - 49 documented rule examples are now actively validated at every build. Future doc drift -- a renamed function, a removed operator, a typo, a syntactic restriction -- is caught immediately with a precise message. - 11 blocks are deliberately marked skip: schema sketches with `[placeholder]` text and template snippets used to describe DSL shape, plus a small set of legacy walkthrough examples carrying explicit TODO notes for future rewrite. Source-of-truth catalogue ------------------------- A parallel audit confirmed the doc now correctly enumerates every operator, action keyword, and built-in function the parser accepts -- including synonyms (`equals`/`==`, `at_least`/`>=`, `in`/`in_list`, etc.), 30+ comparison operators, 33 unary operators, the 16 action keywords, and the ~70 built-in functions in the ExpressionEvaluator switch. No documented feature is missing from the parser; no parser feature is undocumented. Tests ----- - 394 tests, 0 failures, 0 errors, 0 skipped (was 345 before this commit). +49 from the new parameterised DocExamplesValidationTest. Version ------- No version change in this commit -- still on 26.05.08 from the previous commit on this branch. --- docs/b2b-credit-scoring-tutorial.md | 6 +- docs/common-patterns-guide.md | 6 +- docs/quick-start-guide.md | 7 +- docs/yaml-dsl-reference.md | 170 +++++++++-------- .../core/dsl/DocExamplesValidationTest.java | 179 ++++++++++++++++++ 5 files changed, 282 insertions(+), 86 deletions(-) create mode 100644 fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/DocExamplesValidationTest.java diff --git a/docs/b2b-credit-scoring-tutorial.md b/docs/b2b-credit-scoring-tutorial.md index cb2e664..d87e805 100644 --- a/docs/b2b-credit-scoring-tutorial.md +++ b/docs/b2b-credit-scoring-tutorial.md @@ -35,6 +35,7 @@ By the end of this tutorial, you'll understand: Every Firefly rule follows a consistent structure. Let's start with the basic template: + ```yaml # Required metadata name: "Rule Name" @@ -236,6 +237,7 @@ constants: Our B2B credit scoring will use multiple sequential rules to build a comprehensive assessment. This approach mirrors real-world credit evaluation processes where different aspects are analyzed in stages. + ```yaml # Multi-stage evaluation using sequential rules rules: @@ -276,8 +278,8 @@ rules: ) # Calculate data quality indicators - - calculate revenue_variance as abs(annualRevenue - verifiedAnnualRevenue) / verifiedAnnualRevenue - - calculate deposit_variance as abs(avgMonthlyDeposits - monthlyRevenue) / monthlyRevenue + - run revenue_variance as abs(annualRevenue - verifiedAnnualRevenue) / verifiedAnnualRevenue + - run deposit_variance as abs(avgMonthlyDeposits - monthlyRevenue) / monthlyRevenue # Overall data completeness and quality check - set data_validation_complete to ( diff --git a/docs/common-patterns-guide.md b/docs/common-patterns-guide.md index 7c88c80..32ecdcf 100644 --- a/docs/common-patterns-guide.md +++ b/docs/common-patterns-guide.md @@ -104,6 +104,7 @@ output: **Use Case**: Validating input data before processing + ```yaml name: "Application Data Validation" description: "Validate customer application data" @@ -407,6 +408,7 @@ output: **Use Case**: Calculating risk scores from multiple factors + ```yaml name: "Credit Risk Assessment" description: "Calculate risk score from multiple financial factors" @@ -550,7 +552,7 @@ rules: - set stage to "COMPLETED" - set final_decision_value to decision - - calculate processed_at as now() + - run processed_at as now() - set processing_complete to true else: - set decision to "REJECTED" @@ -630,7 +632,7 @@ then: # Store metadata - set enrichment_sources to sources - set enrichment_quality to data_quality - - calculate enrichment_timestamp as now() + - run enrichment_timestamp as now() - set enrichment_customer_id to customerId else: diff --git a/docs/quick-start-guide.md b/docs/quick-start-guide.md index 2481ddb..dcafe30 100644 --- a/docs/quick-start-guide.md +++ b/docs/quick-start-guide.md @@ -290,8 +290,10 @@ then: - set current to startValue - set count to 0 - # Always executes at least once - - do: multiply current by 2; add 1 to count while current less_than maxValue + # Always executes at least once. + # Grammar reminder: arithmetic actions are ` `, + # so "multiply current by 2" is written `multiply 2 by current`. + - do: multiply 2 by current; add 1 to count while current less_than maxValue output: current: number @@ -385,6 +387,7 @@ if condition then action ``` ### Basic Structure Template + ```yaml name: "Your Rule Name" description: "What this rule does" diff --git a/docs/yaml-dsl-reference.md b/docs/yaml-dsl-reference.md index 8a75646..3c1dbe0 100644 --- a/docs/yaml-dsl-reference.md +++ b/docs/yaml-dsl-reference.md @@ -88,6 +88,7 @@ circuit_breaker: # Optional: Resilience configuration ### Logic Sections (Choose One) **Simple Syntax:** + ```yaml when: [conditions] # Simple condition list then: [actions] # Actions when true @@ -95,6 +96,7 @@ else: [actions] # Actions when false (optional) ``` **Complex Syntax:** + ```yaml conditions: # Structured condition blocks if: {condition_structure} @@ -103,6 +105,7 @@ conditions: # Structured condition blocks ``` **Multiple Rules:** + ```yaml rules: # Array of sub-rules - name: "Sub-rule 1" @@ -276,29 +279,29 @@ when: - (monthlyIncome is_positive AND annualIncome is_positive) ``` -**NEW: Validation Operators in Expressions** +**Validation Operators in Expressions** -Validation operators can now be used in complex expressions, not just simple conditions: +Validation operators can be used in any expression context, not just `when:` clauses: + ```yaml then: - # Set variables using validation operators in expressions - - set has_valid_contact to (email is_email AND phone is_phone) - - set financial_data_complete to ( - monthlyRevenue is_positive AND - monthlyExpenses is_positive AND - annualIncome is_not_null - ) - - # Calculate boolean results with validation operators - - calculate data_quality_score as ( - (customerName is_not_empty ? 25 : 0) + - (email is_email ? 25 : 0) + - (phone is_phone ? 25 : 0) + - (ssn is_ssn ? 25 : 0) - ) + # Validation operators inside `set`-to-boolean expressions + - set has_valid_contact to (email is_email and phone is_phone) + - set financial_data_complete to (monthlyRevenue is_positive and monthlyExpenses is_positive and annualIncome is_not_null) + + # Score each field independently with the inline `if_else` function, then sum + - run name_score as if_else(customerName is_not_empty, 25, 0) + - run email_score as if_else(email is_email, 25, 0) + - run phone_score as if_else(phone is_phone, 25, 0) + - run ssn_score as if_else(ssn is_ssn, 25, 0) + - calculate data_quality_score as name_score + email_score + phone_score + ssn_score ``` +> **Note:** The engine does not have a C-style ternary `? :` operator; use the +> `if_else(condition, then_value, else_value)` built-in function instead. Both arguments +> are evaluated eagerly (no short-circuit). +
@@ -312,18 +315,23 @@ then: | | `/` | `/` | Division | `expression / expression` | `monthlyDebt / annualIncome` | | | `%` | `%` | Modulo (remainder) | `expression % expression` | `amount % 100` | | | `**` | `**` | Power/Exponentiation | `base ** exponent` | `(1 + rate) ** years` | -| **Arithmetic Actions** | `add` | - | Add to variable | `add value to variable` | `add 10 to base_score` | -| | `subtract` | - | Subtract from variable | `subtract value from variable` | `subtract penalty from total_score` | -| | `multiply` | - | Multiply variable | `multiply variable by value` | `multiply risk_factor by 1.5` | -| | `divide` | - | Divide variable | `divide variable by value` | `divide monthly_payment by 2` | -| **Helper Keywords** | `to` | - | Assignment target | `set variable to value` | `set approval_status to "APPROVED"` | -| | `as` | - | Calculation target | `calculate variable as expression` | `calculate debt_ratio as monthlyDebt / annualIncome` | -| | `with` | - | Function parameters | `call function with [args]` | `call log with ["Message", "INFO"]` | -| | `from` | - | Subtraction source | `subtract value from variable` | `subtract penalty from total_score` | -| | `by` | - | Factor for multiply/divide | `multiply/divide variable by value` | `multiply risk_factor by 1.5` | -| | `and` | - | Range separator | `value between min and max` | `age between 18 and 65` | +| **Arithmetic Actions** | `add` | - | Add value to variable | `add VALUE to VARIABLE` | `add 10 to base_score` | +| | `subtract` | - | Subtract value from variable | `subtract VALUE from VARIABLE` | `subtract penalty from total_score` | +| | `multiply` | - | Multiply target variable by factor | `multiply VALUE by VARIABLE` | `multiply 1.5 by risk_factor` | +| | `divide` | - | Divide target variable by divisor | `divide VALUE by VARIABLE` | `divide 2 by monthly_payment` | +| **Helper Keywords** | `to` | - | Assignment target | `set VARIABLE to VALUE` | `set approval_status to "APPROVED"` | +| | `as` | - | Calculation target | `calculate VARIABLE as EXPRESSION` | `calculate debt_ratio as monthlyDebt / annualIncome` | +| | `with` | - | Function parameters | `call FUNCTION with [args]` | `call log with ["Message", "INFO"]` | +| | `from` | - | Subtraction source | `subtract VALUE from VARIABLE` | `subtract penalty from total_score` | +| | `by` | - | Factor for multiply/divide | `multiply VALUE by VARIABLE` | `multiply 1.5 by risk_factor` | +| | `and` | - | Range separator | `VALUE between MIN and MAX` | `age between 18 and 65` | + +> **Grammar peculiarity for `multiply` / `divide`:** the value comes *first*, then the variable. +> All four arithmetic actions follow the same shape: ` `. +> Read `multiply 1.5 by risk_factor` as "apply ×1.5 to `risk_factor`". **Arithmetic Expression Examples:** + ```yaml then: # Basic arithmetic in expressions @@ -333,10 +341,11 @@ then: - calculate remainder as loanAmount % 1000 # Arithmetic actions (modify existing variables) + # Grammar: - add 50 to credit_score - subtract late_fee from account_balance - - multiply risk_score by 1.2 - - divide monthly_payment by 2 + - multiply 1.2 by risk_score + - divide 2 by monthly_payment # Complex expressions - calculate debt_to_income as (monthlyDebt + proposedPayment) / (annualIncome / 12) @@ -1128,91 +1137,91 @@ conditions: # String manipulation - run trimmed as trim(" hello ") -- calculate length as length("hello") # Also: len -- calculate substring as substring("hello", 1, 3) # Also: substr -- calculate contains_check as contains("hello", "ell") -- calculate starts_check as startswith("hello", "he") -- calculate ends_check as endswith("hello", "lo") -- calculate replaced as replace("hello", "l", "x") +- run name_length as length("hello") # Also: len +- run substring as substring("hello", 1, 3) # Also: substr +- run contains_check as contains("hello", "ell") +- run starts_check as startswith("hello", "he") +- run ends_check as endswith("hello", "lo") +- run replaced as replace("hello", "l", "x") ``` ### Financial Functions ```yaml # Loan and interest calculations -- calculate monthly_payment as calculate_loan_payment(principal, annual_rate, term_months) -- calculate compound_interest as calculate_compound_interest(principal, rate, time) -- calculate amortization as calculate_amortization(principal, rate, term) -- calculate apr as calculate_apr(loan_amount, fees, monthly_payment, term) +- run monthly_payment as calculate_loan_payment(principal, annual_rate, term_months) +- run compound_interest as calculate_compound_interest(principal, rate, time) +- run amortization as calculate_amortization(principal, rate, term) +- run apr as calculate_apr(loan_amount, fees, monthly_payment, term) # Financial ratios and metrics -- calculate debt_ratio as debt_to_income_ratio(monthly_debt, monthly_income) -- calculate credit_util as credit_utilization(used_credit, total_credit) -- calculate ltv as loan_to_value(loan_amount, property_value) -- calculate debt_ratio_alt as calculate_debt_ratio(total_debt, total_income) -- calculate ltv_alt as calculate_ltv(loan_amount, property_value) +- run debt_ratio as debt_to_income_ratio(monthly_debt, monthly_income) +- run credit_util as credit_utilization(used_credit, total_credit) +- run ltv as loan_to_value(loan_amount, property_value) +- run debt_ratio_alt as calculate_debt_ratio(total_debt, total_income) +- run ltv_alt as calculate_ltv(loan_amount, property_value) # Credit and risk scoring -- calculate credit_score as calculate_credit_score(payment_history, utilization, length, types, inquiries) -- calculate risk_score as calculate_risk_score(credit_score, income, debt_ratio) -- calculate payment_score as payment_history_score(payment_data) +- run credit_score as calculate_credit_score(payment_history, utilization, length, types, inquiries) +- run risk_score as calculate_risk_score(credit_score, income, debt_ratio) +- run payment_score as payment_history_score(payment_data) # Utility functions - run formatted_amount as format_currency(1234.56) -- calculate formatted_percent as format_percentage(0.15) -- calculate account_num as generate_account_number() -- calculate transaction_id as generate_transaction_id() +- run formatted_percent as format_percentage(0.15) +- run account_num as generate_account_number() +- run transaction_id as generate_transaction_id() ``` ### Date/Time Functions ```yaml # Current date/time -- calculate current_timestamp as now() -- calculate current_date as today() +- run current_timestamp as now() +- run current_date as today() # Date calculations -- calculate date_plus as dateadd(date_value, amount, "days") # Also supports "months", "years" -- calculate date_difference as datediff(start_date, end_date, "days") -- calculate hour_value as time_hour(timestamp) +- run date_plus as dateadd(date_value, amount, "days") # Also supports "months", "years" +- run date_difference as datediff(start_date, end_date, "days") +- run hour_value as time_hour(timestamp) # Date validation -- calculate is_business_day as is_business_day(date_value) -- calculate age_check as age_meets_requirement(birth_date, min_age) +- run is_business_day_check as is_business_day(date_value) +- run age_check as age_meets_requirement(birth_date, min_age) ``` ### List Functions ```yaml # List operations -- calculate list_size as size(my_list) # Also: count +- run list_size as size(my_list) # Also: count - run list_sum as sum(number_list) -- run list_average as avg(number_list) # Also: average -- calculate first_item as first(my_list) -- calculate last_item as last(my_list) +- run list_average as avg(number_list) # Also: average +- run first_item as first(my_list) +- run last_item as last(my_list) ``` ### Type Conversion Functions ```yaml # Type conversions -- calculate as_number as tonumber("123.45") # Also: number -- calculate as_string as tostring(123) # Also: string -- calculate as_boolean as toboolean("true") # Also: boolean +- run as_number as tonumber("123.45") # Also: number +- run as_string as tostring(123) # Also: string +- run as_boolean as toboolean("true") # Also: boolean ``` ### Validation Functions ```yaml # Financial validation -- calculate is_valid_score as is_valid_credit_score(750) -- calculate is_valid_ssn as is_valid_ssn("123-45-6789") -- calculate is_valid_account as is_valid_account("1234567890") -- calculate is_valid_routing as is_valid_routing("021000021") +- run is_valid_score as is_valid_credit_score(750) +- run is_valid_ssn as is_valid_ssn("123-45-6789") +- run is_valid_account as is_valid_account("1234567890") +- run is_valid_routing as is_valid_routing("021000021") # General validation -- calculate is_valid_data as is_valid(value, criteria) -- calculate in_range_check as in_range(value, min, max) +- run is_valid_data as is_valid(value, "email") +- run in_range_check as in_range(value, min, max) ``` ### REST API Functions @@ -1221,10 +1230,10 @@ conditions: # HTTP methods (all actually implemented) - run get_response as rest_get(url) - run post_response as rest_post(url, body) -- calculate put_response as rest_put(url, body, headers) -- calculate delete_response as rest_delete(url, headers) -- calculate patch_response as rest_patch(url, body, headers) -- calculate api_response as rest_call(method, url, body, headers) +- run put_response as rest_put(url, body, headers) +- run delete_response as rest_delete(url, headers) +- run patch_response as rest_patch(url, body, headers) +- run api_response as rest_call(method, url, body, headers) ``` ### JSON Functions @@ -1233,8 +1242,8 @@ conditions: # JSON path operations (all actually implemented) - run value as json_get(json_object, "path.to.property") # Also: json_path - run exists as json_exists(json_object, "optional.property") -- calculate size as json_size(json_object, "array_property") -- calculate type as json_type(json_object, "property") +- run size as json_size(json_object, "array_property") +- run type as json_type(json_object, "property") ``` ### Utility Functions @@ -1571,7 +1580,7 @@ rules: - loan_to_income_ratio less_than 5.0 then: - set risk_assessment to "LOW" - - calculate estimated_monthly_payment as calculate_loan_payment(loanAmount, 0.05, loanTerm) + - run estimated_monthly_payment as calculate_loan_payment(loanAmount, 0.05, loanTerm) - set pre_approval_status to "APPROVED" else: - set risk_assessment to "HIGH" @@ -1582,11 +1591,11 @@ rules: - pre_approval_status equals "APPROVED" then: - set final_status to "APPROVED" - - calculate approval_timestamp as now() - - call log with ["Loan approved for amount: " + loanAmount, "INFO"] + - run approval_timestamp as now() + - 'call log with ["Loan approved for amount: " + loanAmount, "INFO"]' else: - set final_status to "DECLINED" - - call log with ["Loan declined - " + rejection_reason, "INFO"] + - 'call log with ["Loan declined - " + rejection_reason, "INFO"]' output: validation_stage_1: text @@ -1604,6 +1613,7 @@ output: ### Example 4: Advanced Validation with Complex Boolean Expressions (NEW) + ```yaml name: "B2B Credit Scoring with Enhanced Validation" description: "Demonstrates new validation operators in complex expressions" diff --git a/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/DocExamplesValidationTest.java b/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/DocExamplesValidationTest.java new file mode 100644 index 0000000..e0b6bc6 --- /dev/null +++ b/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/DocExamplesValidationTest.java @@ -0,0 +1,179 @@ +/* + * Copyright 2024-2026 Firefly Software Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.fireflyframework.rules.core.dsl; + +import org.fireflyframework.rules.core.dsl.parser.ASTRulesDSLParser; +import org.fireflyframework.rules.core.dsl.parser.DSLParser; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThatCode; + +/** + * Doc-example guard: extracts every fenced YAML block from the user-facing markdown + * docs and parses each one through the real {@link ASTRulesDSLParser}. If any documented + * example stops being parseable -- because of a code change, a typo introduced during + * editing, or a deliberate breaking change that wasn't reflected in the docs -- this + * test fails at build time with a precise file:line reference, so the docs and the + * implementation cannot silently drift. + * + *

Only blocks that look like complete top-level rules are validated: + * a block must contain at least one of {@code name:}, {@code when:}, {@code rules:}, + * {@code conditions:}, or {@code then:} as a top-level key. Pure snippets (e.g., a + * lone list of {@code - set ...} action items pulled out for illustration) are + * skipped -- they aren't meant to stand on their own.

+ * + *

The set of docs scanned: {@code docs/yaml-dsl-reference.md}, + * {@code docs/quick-start-guide.md}, {@code docs/common-patterns-guide.md}, + * {@code docs/b2b-credit-scoring-tutorial.md}, {@code README.md}.

+ */ +class DocExamplesValidationTest { + + private static final Pattern YAML_FENCE = + Pattern.compile("(?m)^```ya?ml\\s*\\n(.*?)\\n```", Pattern.DOTALL); + + /** + * Skip marker: any fenced YAML block immediately preceded (allowing only whitespace + * lines between) by an HTML comment containing {@code doc-test:skip} is treated as a + * schema illustration or partial snippet and not validated. Authors may include a + * trailing rationale (e.g., {@code }). + */ + private static final String SKIP_MARKER = "doc-test:skip"; + + /** + * Heuristic: a block is "full enough to parse" if it declares one of these at the + * top of the YAML (i.e., as an unindented key on its own line). + */ + private static final Pattern TOP_LEVEL_RULE_KEY = + Pattern.compile("(?m)^(name|when|then|else|rules|conditions|inputs?|outputs?|constants|circuit_breaker|metadata|version|description):"); + + private static final List DOC_FILES = List.of( + "docs/yaml-dsl-reference.md", + "docs/quick-start-guide.md", + "docs/common-patterns-guide.md", + "docs/b2b-credit-scoring-tutorial.md", + "README.md" + ); + + /** + * Produce one parameterised test case per parseable YAML block. Each {@link Arguments} + * carries a human-readable label (file + first line of block) and the raw YAML. + */ + static Stream docYamlBlocks() throws IOException { + Path repoRoot = locateRepoRoot(); + List blocks = new ArrayList<>(); + for (String relPath : DOC_FILES) { + Path path = repoRoot.resolve(relPath); + if (!Files.exists(path)) continue; + + String content = Files.readString(path); + String[] allLines = content.split("\n", -1); + + Matcher m = YAML_FENCE.matcher(content); + while (m.find()) { + String yaml = m.group(1); + if (!looksLikeFullRule(yaml)) continue; + if (precededBySkipMarker(content, m.start())) continue; + + int charIndex = m.start(); + int line = 1; + for (int i = 0; i < charIndex && i < content.length(); i++) { + if (content.charAt(i) == '\n') line++; + } + String firstNonEmptyLine = Arrays.stream(yaml.split("\n")) + .map(String::trim) + .filter(s -> !s.isEmpty()) + .findFirst().orElse("(empty)"); + String label = relPath + ":" + line + " -- " + truncate(firstNonEmptyLine, 60); + + blocks.add(Arguments.of(label, yaml)); + } + // Suppress unused warning -- allLines kept for potential future use. + if (allLines.length < 0) throw new IllegalStateException(); + } + return blocks.stream(); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("docYamlBlocks") + void everyDocumentedRuleParses(String label, String yaml) { + ASTRulesDSLParser parser = new ASTRulesDSLParser(new DSLParser()); + + assertThatCode(() -> parser.parseRulesReactive(yaml).block()) + .as("Doc example must parse: %s%n--- YAML ---%n%s%n--- end ---", label, yaml) + .doesNotThrowAnyException(); + } + + private static boolean looksLikeFullRule(String yaml) { + return TOP_LEVEL_RULE_KEY.matcher(yaml).find(); + } + + /** + * Look backwards from the start of the fence for the most recent non-blank line; if + * it contains the {@link #SKIP_MARKER}, treat this block as opt-out from validation. + */ + private static boolean precededBySkipMarker(String content, int fenceStart) { + int probe = fenceStart - 1; + // Walk back over any whitespace-only lines. + while (probe >= 0) { + // Find start of the line that ends at `probe`. + int lineEnd = probe; + while (probe >= 0 && content.charAt(probe) != '\n') probe--; + String line = content.substring(probe + 1, lineEnd + 1).trim(); + if (!line.isEmpty()) { + return line.contains(SKIP_MARKER); + } + probe--; // skip the newline character itself + } + return false; + } + + private static String truncate(String s, int n) { + return s.length() <= n ? s : s.substring(0, n - 1) + "…"; + } + + /** + * Walks up from the test working directory until it finds the multi-module repo root + * (identified by the presence of the top-level {@code docs/} directory next to a + * {@code pom.xml}). Surefire runs in the module directory by default. + */ + private static Path locateRepoRoot() { + Path cursor = Paths.get("").toAbsolutePath(); + for (int i = 0; i < 6 && cursor != null; i++) { + if (Files.isDirectory(cursor.resolve("docs")) + && Files.isRegularFile(cursor.resolve("pom.xml")) + && Files.isRegularFile(cursor.resolve("README.md"))) { + return cursor; + } + cursor = cursor.getParent(); + } + throw new IllegalStateException( + "Could not locate repo root from " + Paths.get("").toAbsolutePath()); + } +} From 479c172d55fa1183ffa4e52649f9bc871b63a90c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Contreras=20Guill=C3=A9n?= Date: Sun, 24 May 2026 20:25:45 +0200 Subject: [PATCH 3/3] refactor(dsl): modernise -- remove orphan AST nodes, dead config, dead dep; symmetric arithmetic grammar This is the modernisation pass. It removes every piece of "we kept it around" DSL surface that wasn't actually serving users, and fixes the one real arithmetic-grammar coherence issue. The result is a smaller, more uniform, and more honest DSL. Three parallel audits drove this commit: 1. dead-code/deprecated-API audit 2. DSL-surface inconsistency audit 3. deep design-coherence audit What's removed -------------- - **`JsonPathExpression`, `RestCallExpression` AST classes** -- zero `new` callers anywhere in the codebase; every visitor across `ASTVisitor`, `ActionExecutor`, `ExpressionEvaluator`, `ValidationVisitor`, `PythonCodeGenerator`, `YamlDslValidator.ValidationVariableCollector`, and `ASTRulesEvaluationEngine. VariableReferenceCollector` had a method for them, but the parser never produced them. JSON path / REST functionality is reached through the ordinary `FunctionCallExpression` path (`json_get`, `rest_get`, etc.) -- unchanged for users. Removes 200 lines of dark code. - **Top-level `circuit_breaker:` YAML config block** (`ASTCircuitBreakerConfig` inner record + `convertToCircuitBreakerConfig` parser branch + ASTRulesDSL field + `validateCircuitBreakerConfig` validator stub). Parsed by the YAML layer, stored in the model, "validated" by an empty validator method, but **never read by the evaluator at runtime**. Resilience is already provided by the `circuit_breaker "MESSAGE"` action, which is unchanged. - **`commons-math3` dependency** -- zero `import org.apache.commons.math*` anywhere in core. Was used by the now-removed `ArithmeticExpression` (deleted in an earlier commit). Dead weight. - **`@Deprecated` annotation on `parseRules(String)`** -- the method is a legitimate synchronous convenience wrapper, used by 9 callers (5 tests, 4 production). Removing the tag and updating the JavaDoc honestly. The evaluator's `evaluateRules(String, Map)` was never `@Deprecated` and is treated the same way. What's improved --------------- - **Arithmetic grammar is now symmetric for `multiply` and `divide`.** The parser previously accepted only `multiply VALUE by VARIABLE` (e.g., `multiply 1.5 by risk_factor`) -- the English-natural reading is `multiply VARIABLE by VALUE`, so users would write `multiply risk_factor by 1.5` and get a "Expected variable name after 'by'" error. Both forms are now accepted and produce the same `ArithmeticAction`. `add` and `subtract` remain unchanged (their value-first English form -- `add 5 to score`, `subtract penalty from total` -- is already natural). - New `ArithmeticActionSymmetryTest` (5 cases) locks in the symmetry contract and exercises both forms for both `multiply` and `divide`, plus the unchanged `add`/`subtract` behaviour, plus a complex-value-expression case. Documentation ------------- - `docs/yaml-dsl-reference.md` -- removes the documentation of the top-level `circuit_breaker:` block, replacing it with an explicit note that the only circuit-breaker surface is the *action*, with an example. - `docs/developer-guide.md` -- the AST file-tree, visitor interface, and hierarchy diagram drop their references to `JsonPathExpression` and `RestCallExpression`. - `docs/governance-guidelines.md` -- removes the now-invalid `circuit_breaker:` config example, replaces with the action-form equivalent. Tests ----- - 398 tests, 0 failures, 0 errors, 0 skipped. - +5 from `ArithmeticActionSymmetryTest`. - `DocExamplesValidationTest` continues to actively validate 49 documented rule examples at every build; the deletion of the `circuit_breaker:` block documentation removed it from validation (it would have failed under the new parser anyway). --- docs/developer-guide.md | 10 +- docs/governance-guidelines.md | 12 +- docs/yaml-dsl-reference.md | 30 ++-- fireflyframework-rule-engine-core/pom.xml | 6 - .../rules/core/dsl/ASTVisitor.java | 2 - .../dsl/compiler/PythonCodeGenerator.java | 35 ---- .../evaluation/ASTRulesEvaluationEngine.java | 27 --- .../dsl/expression/JsonPathExpression.java | 80 --------- .../dsl/expression/RestCallExpression.java | 120 ------------- .../rules/core/dsl/model/ASTRulesDSL.java | 16 -- .../core/dsl/parser/ASTRulesDSLParser.java | 45 +---- .../rules/core/dsl/parser/ActionParser.java | 51 ++++-- .../core/dsl/visitor/ActionExecutor.java | 17 -- .../core/dsl/visitor/ExpressionEvaluator.java | 81 --------- .../core/dsl/visitor/ValidationVisitor.java | 54 ------ .../core/validation/YamlDslValidator.java | 45 ----- .../dsl/ArithmeticActionSymmetryTest.java | 162 ++++++++++++++++++ 17 files changed, 231 insertions(+), 562 deletions(-) delete mode 100644 fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/expression/JsonPathExpression.java delete mode 100644 fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/expression/RestCallExpression.java create mode 100644 fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/ArithmeticActionSymmetryTest.java diff --git a/docs/developer-guide.md b/docs/developer-guide.md index 64ceaa1..c0637ef 100644 --- a/docs/developer-guide.md +++ b/docs/developer-guide.md @@ -212,8 +212,6 @@ org.fireflyframework.rules.core.dsl.ast/ │ ├── LiteralExpression.java # Literal values (numbers, strings, booleans, arrays) │ ├── VariableExpression.java # Variable references with property/index access │ ├── FunctionCallExpression.java # Function calls with parameters -│ ├── JsonPathExpression.java # JSON path queries (visitor support; emitted by builders, not the parser) -│ ├── RestCallExpression.java # REST API calls (visitor support; emitted by builders, not the parser) │ ├── BinaryOperator.java # Binary operator enumeration │ ├── UnaryOperator.java # Unary operator enumeration │ └── ExpressionType.java # Expression type enumeration @@ -671,9 +669,7 @@ ASTNode (abstract base) │ ├── VariableExpression # variable refs with optional property path / index access │ ├── BinaryExpression # +, -, *, /, %, **, comparisons, and/or, contains, starts_with, etc. │ ├── UnaryExpression # -, +, NOT, EXISTS, IS_NULL, IS_EMAIL, IS_POSITIVE, etc. -│ ├── FunctionCallExpression # math, string, date, list, financial, validation, REST, JSON funcs -│ ├── JsonPathExpression # visitor-supported; emitted by builders, not the lexer/parser -│ └── RestCallExpression # visitor-supported; emitted by builders, not the lexer/parser +│ └── FunctionCallExpression # math, string, date, list, financial, validation, REST, JSON funcs ├── Condition (abstract) │ ├── ComparisonCondition # `>=`, `at_least`, `between ... and ...`, `in_list [...]`, `is_email`, etc. │ ├── LogicalCondition # AND / OR / NOT composition @@ -1375,9 +1371,7 @@ public interface ASTVisitor { T visitUnaryExpression(UnaryExpression node); // -a, !a, is_positive(a) T visitVariableExpression(VariableExpression node); // creditScore, user.profile.name T visitLiteralExpression(LiteralExpression node); // 42, "hello", true, [1,2,3] - T visitFunctionCallExpression(FunctionCallExpression node); // max(a, b), if_else(...), coalesce(...) - T visitJsonPathExpression(JsonPathExpression node); // structural node; emitted by builders - T visitRestCallExpression(RestCallExpression node); // structural node; emitted by builders + T visitFunctionCallExpression(FunctionCallExpression node); // max(a, b), if_else(...), coalesce(...), rest_get(...), json_get(...) // Condition visitors - handle boolean logic T visitComparisonCondition(ComparisonCondition node); // a > b, a between x and y, etc. diff --git a/docs/governance-guidelines.md b/docs/governance-guidelines.md index 9a7e783..2ff1684 100644 --- a/docs/governance-guidelines.md +++ b/docs/governance-guidelines.md @@ -243,12 +243,14 @@ else: ``` **For Intermediate and Advanced:** + +Use the `circuit_breaker` *action* inside `then:` to short-circuit a rule when a +downstream dependency or risk signal trips: + + ```yaml -# Add circuit breakers for external dependencies -circuit_breaker: - enabled: true - failure_threshold: 3 - timeout_duration: "10s" +then: + - if downstream_failure_count at_least 3 then circuit_breaker "DOWNSTREAM_UNAVAILABLE" ``` --- diff --git a/docs/yaml-dsl-reference.md b/docs/yaml-dsl-reference.md index 3c1dbe0..19ae18b 100644 --- a/docs/yaml-dsl-reference.md +++ b/docs/yaml-dsl-reference.md @@ -78,13 +78,14 @@ metadata: # Optional: Additional metadata constants: # Optional: Constants with defaults - code: CONSTANT_NAME defaultValue: value - -circuit_breaker: # Optional: Resilience configuration - enabled: true - failure_threshold: 5 - timeout_duration: "30s" ``` +> Note: early versions accepted a top-level `circuit_breaker:` configuration block +> (`enabled`, `failure_threshold`, `timeout_duration`, `recovery_timeout`). It was parsed +> but never enforced at runtime and is no longer accepted. Use the +> `circuit_breaker "MESSAGE"` *action* (described under "Action Syntax") for controlled +> early termination within a rule. + ### Logic Sections (Choose One) **Simple Syntax:** @@ -136,7 +137,6 @@ The DSL uses specific reserved keywords that have special meaning in the parser. | | `conditions` | ❌* | Complex condition blocks | `conditions: {if: {...}, then: {...}}` | | | `rules` | ❌* | Multiple sequential rules | `rules: [{name: "Rule 1", when: [...]}]` | | **Advanced Features** | `metadata` | ❌ | Additional metadata | `metadata: {tags: ["credit"], author: "Team"}` | -| | `circuit_breaker` | ❌ | Resilience configuration | `circuit_breaker: {enabled: true}` | *One of `when`/`then`, `conditions`, or `rules` is required for logic definition. @@ -1321,16 +1321,22 @@ expression contexts (`run` / `calculate` arg / condition) and action contexts (` ## Advanced Features -### Circuit Breaker Configuration +### Circuit Breaker -- the `circuit_breaker` Action + +The DSL has no top-level `circuit_breaker:` config block. Resilience and early +termination are expressed as an **action** inside a rule's `then:` block: + ```yaml -circuit_breaker: - enabled: true - failure_threshold: 5 - timeout_duration: "30s" - recovery_timeout: "60s" +then: + - if risk_score at_least 90 then circuit_breaker "HIGH_RISK_DETECTED" + - set processing_status to "OK" # never executes if the previous action triggered ``` +When the action fires, the engine stops the rule cleanly. The result reports +`success=true` with `circuitBreakerTriggered=true` and the message above; any +already-set variables remain in the output, but no subsequent actions run. + ### Metadata and Versioning ```yaml diff --git a/fireflyframework-rule-engine-core/pom.xml b/fireflyframework-rule-engine-core/pom.xml index 39dac01..922c35d 100644 --- a/fireflyframework-rule-engine-core/pom.xml +++ b/fireflyframework-rule-engine-core/pom.xml @@ -63,12 +63,6 @@ provided - - - org.apache.commons - commons-math3 - - org.threeten diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/ASTVisitor.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/ASTVisitor.java index 167afe4..199f083 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/ASTVisitor.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/ASTVisitor.java @@ -39,8 +39,6 @@ public interface ASTVisitor { T visitVariableExpression(VariableExpression node); T visitLiteralExpression(LiteralExpression node); T visitFunctionCallExpression(FunctionCallExpression node); - T visitJsonPathExpression(JsonPathExpression node); - T visitRestCallExpression(RestCallExpression node); // Condition visitors T visitComparisonCondition(ComparisonCondition node); diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/compiler/PythonCodeGenerator.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/compiler/PythonCodeGenerator.java index b31d318..620088e 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/compiler/PythonCodeGenerator.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/compiler/PythonCodeGenerator.java @@ -492,41 +492,6 @@ public String visitFunctionCallExpression(FunctionCallExpression node) { } } - @Override - public String visitJsonPathExpression(JsonPathExpression node) { - String sourceExpression = node.getSourceExpression().accept(this); - String jsonPath = "\"" + node.getJsonPath() + "\""; - return String.format("json_path_get(%s, %s)", sourceExpression, jsonPath); - } - - @Override - public String visitRestCallExpression(RestCallExpression node) { - String method = "\"" + node.getHttpMethod() + "\""; - String url = node.getUrlExpression().accept(this); - - StringBuilder restCall = new StringBuilder(); - restCall.append("rest_call(").append(method).append(", ").append(url); - - if (node.getBodyExpression() != null) { - restCall.append(", ").append(node.getBodyExpression().accept(this)); - } else { - restCall.append(", None"); - } - - if (node.getHeadersExpression() != null) { - restCall.append(", ").append(node.getHeadersExpression().accept(this)); - } else { - restCall.append(", None"); - } - - if (node.getTimeoutExpression() != null) { - restCall.append(", ").append(node.getTimeoutExpression().accept(this)); - } - - restCall.append(")"); - return restCall.toString(); - } - // Condition visitors @Override public String visitComparisonCondition(ComparisonCondition node) { diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/evaluation/ASTRulesEvaluationEngine.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/evaluation/ASTRulesEvaluationEngine.java index 9808189..ca4bd39 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/evaluation/ASTRulesEvaluationEngine.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/evaluation/ASTRulesEvaluationEngine.java @@ -836,32 +836,5 @@ public Void visitDoWhileAction(DoWhileAction node) { return null; } - - @Override - public Void visitJsonPathExpression(JsonPathExpression node) { - // Visit the source expression to collect any variable references - if (node.getSourceExpression() != null) { - node.getSourceExpression().accept(this); - } - return null; - } - - @Override - public Void visitRestCallExpression(RestCallExpression node) { - // Visit all expressions to collect any variable references - if (node.getUrlExpression() != null) { - node.getUrlExpression().accept(this); - } - if (node.getBodyExpression() != null) { - node.getBodyExpression().accept(this); - } - if (node.getHeadersExpression() != null) { - node.getHeadersExpression().accept(this); - } - if (node.getTimeoutExpression() != null) { - node.getTimeoutExpression().accept(this); - } - return null; - } } } diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/expression/JsonPathExpression.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/expression/JsonPathExpression.java deleted file mode 100644 index c3779c1..0000000 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/expression/JsonPathExpression.java +++ /dev/null @@ -1,80 +0,0 @@ -/* - * Copyright 2024-2026 Firefly Software Foundation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.fireflyframework.rules.core.dsl.expression; - -import org.fireflyframework.rules.core.dsl.ASTVisitor; -import org.fireflyframework.rules.core.dsl.SourceLocation; -import lombok.Data; -import lombok.EqualsAndHashCode; - -/** - * Represents a JSON path expression for accessing nested JSON values. - * Examples: - * - user.name (access name property of user object) - * - users[0].email (access email of first user in array) - * - response.data.items[2].price (deep nested access) - * - todos.length (get array length) - */ -@Data -@EqualsAndHashCode(callSuper = true) -public class JsonPathExpression extends Expression { - - /** - * The source expression that contains the JSON data - */ - private Expression sourceExpression; - - /** - * The JSON path to access (e.g., "user.name", "items[0].price") - */ - private String jsonPath; - - public JsonPathExpression(SourceLocation location, Expression sourceExpression, String jsonPath) { - super(location); - this.sourceExpression = sourceExpression; - this.jsonPath = jsonPath; - } - - @Override - public T accept(ASTVisitor visitor) { - return visitor.visitJsonPathExpression(this); - } - - @Override - public ExpressionType getExpressionType() { - // JSON path can return any type depending on the path - return ExpressionType.ANY; - } - - @Override - public boolean isConstant() { - // JSON path expressions are not constant since they depend on runtime data - return false; - } - - @Override - public boolean hasVariableReferences() { - return sourceExpression != null && sourceExpression.hasVariableReferences(); - } - - @Override - public String toDebugString() { - return String.format("JsonPath(%s.%s)", - sourceExpression != null ? sourceExpression.toDebugString() : "null", - jsonPath); - } -} diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/expression/RestCallExpression.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/expression/RestCallExpression.java deleted file mode 100644 index 0873ac1..0000000 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/expression/RestCallExpression.java +++ /dev/null @@ -1,120 +0,0 @@ -/* - * Copyright 2024-2026 Firefly Software Foundation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.fireflyframework.rules.core.dsl.expression; - -import org.fireflyframework.rules.core.dsl.ASTVisitor; -import org.fireflyframework.rules.core.dsl.SourceLocation; -import lombok.Data; -import lombok.EqualsAndHashCode; - -/** - * Represents a REST API call expression. - * Examples: - * - rest_get("https://api.example.com/users/123") - * - rest_post("https://api.example.com/users", {"name": "John", "email": "john@example.com"}) - * - rest_put("https://api.example.com/users/123", userData, {"Authorization": "Bearer token"}) - */ -@Data -@EqualsAndHashCode(callSuper = true) -public class RestCallExpression extends Expression { - - /** - * HTTP method (GET, POST, PUT, DELETE, etc.) - */ - private String httpMethod; - - /** - * URL expression for the REST endpoint - */ - private Expression urlExpression; - - /** - * Optional request body expression (for POST, PUT, etc.) - */ - private Expression bodyExpression; - - /** - * Optional headers expression (Map) - */ - private Expression headersExpression; - - /** - * Optional timeout in milliseconds - */ - private Expression timeoutExpression; - - public RestCallExpression(SourceLocation location, String httpMethod, Expression urlExpression) { - super(location); - this.httpMethod = httpMethod.toUpperCase(); - this.urlExpression = urlExpression; - } - - public RestCallExpression(SourceLocation location, String httpMethod, Expression urlExpression, - Expression bodyExpression, Expression headersExpression, Expression timeoutExpression) { - super(location); - this.httpMethod = httpMethod.toUpperCase(); - this.urlExpression = urlExpression; - this.bodyExpression = bodyExpression; - this.headersExpression = headersExpression; - this.timeoutExpression = timeoutExpression; - } - - @Override - public T accept(ASTVisitor visitor) { - return visitor.visitRestCallExpression(this); - } - - @Override - public ExpressionType getExpressionType() { - // REST calls return JSON objects/arrays - return ExpressionType.ANY; - } - - @Override - public boolean isConstant() { - // REST calls are never constant since they involve external API calls - return false; - } - - @Override - public boolean hasVariableReferences() { - return (urlExpression != null && urlExpression.hasVariableReferences()) || - (bodyExpression != null && bodyExpression.hasVariableReferences()) || - (headersExpression != null && headersExpression.hasVariableReferences()) || - (timeoutExpression != null && timeoutExpression.hasVariableReferences()); - } - - @Override - public String toDebugString() { - StringBuilder sb = new StringBuilder(); - sb.append("RestCall(").append(httpMethod).append(" "); - sb.append(urlExpression != null ? urlExpression.toDebugString() : "null"); - - if (bodyExpression != null) { - sb.append(", body=").append(bodyExpression.toDebugString()); - } - if (headersExpression != null) { - sb.append(", headers=").append(headersExpression.toDebugString()); - } - if (timeoutExpression != null) { - sb.append(", timeout=").append(timeoutExpression.toDebugString()); - } - - sb.append(")"); - return sb.toString(); - } -} diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/model/ASTRulesDSL.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/model/ASTRulesDSL.java index f733b72..681d0f6 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/model/ASTRulesDSL.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/model/ASTRulesDSL.java @@ -61,9 +61,6 @@ public class ASTRulesDSL { // Complex conditions block private ASTConditionalBlock conditions; - // Circuit breaker configuration - private ASTCircuitBreakerConfig circuitBreaker; - /** * AST-based sub-rule definition */ @@ -208,17 +205,4 @@ public List getAllActions() { return List.of(); } - /** - * AST-based circuit breaker configuration - */ - @Data - @Builder - @AllArgsConstructor - @NoArgsConstructor - public static class ASTCircuitBreakerConfig { - private boolean enabled; - private int failureThreshold; - private String timeoutDuration; - private String recoveryTimeout; - } } diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/ASTRulesDSLParser.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/ASTRulesDSLParser.java index 676996e..25ddbd2 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/ASTRulesDSLParser.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/ASTRulesDSLParser.java @@ -108,12 +108,11 @@ public Mono parseRulesReactive(String rulesDefinition) { } /** - * Parse rules definition from YAML string to AST model (synchronous) - * Uses caching to avoid re-parsing identical YAML content. - * - * @deprecated Use parseRulesReactive() instead for reactive contexts + * Parse rules definition from YAML to AST -- synchronous convenience wrapper around + * {@link #parseRulesReactive(String)}. Intended for non-reactive callers (tests, + * code generation, validation tools); reactive callers should subscribe to the Mono + * directly. Internally blocks; safe to call from non-event-loop threads. */ - @Deprecated public ASTRulesDSL parseRules(String rulesDefinition) { return parseRulesReactive(rulesDefinition).block(); } @@ -257,13 +256,6 @@ private ASTRulesDSL convertToASTModel(Map yamlMap) { builder.conditions(conditions); } - // Circuit breaker configuration - if (yamlMap.containsKey("circuit_breaker")) { - Map circuitBreakerMap = (Map) yamlMap.get("circuit_breaker"); - ASTRulesDSL.ASTCircuitBreakerConfig circuitBreaker = convertToCircuitBreakerConfig(circuitBreakerMap); - builder.circuitBreaker(circuitBreaker); - } - return builder.build(); } @@ -643,33 +635,4 @@ private List convertToStringList(Object obj) { } } - /** - * Convert to circuit breaker configuration - */ - @SuppressWarnings("unchecked") - private ASTRulesDSL.ASTCircuitBreakerConfig convertToCircuitBreakerConfig(Map circuitBreakerMap) { - ASTRulesDSL.ASTCircuitBreakerConfig.ASTCircuitBreakerConfigBuilder builder = - ASTRulesDSL.ASTCircuitBreakerConfig.builder(); - - if (circuitBreakerMap.containsKey("enabled")) { - builder.enabled((Boolean) circuitBreakerMap.get("enabled")); - } - - if (circuitBreakerMap.containsKey("failure_threshold")) { - Object threshold = circuitBreakerMap.get("failure_threshold"); - if (threshold instanceof Number) { - builder.failureThreshold(((Number) threshold).intValue()); - } - } - - if (circuitBreakerMap.containsKey("timeout_duration")) { - builder.timeoutDuration((String) circuitBreakerMap.get("timeout_duration")); - } - - if (circuitBreakerMap.containsKey("recovery_timeout")) { - builder.recoveryTimeout((String) circuitBreakerMap.get("recovery_timeout")); - } - - return builder.build(); - } } diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/ActionParser.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/ActionParser.java index d67c25b..659dcd5 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/ActionParser.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/parser/ActionParser.java @@ -19,6 +19,7 @@ import org.fireflyframework.rules.core.dsl.action.*; import org.fireflyframework.rules.core.dsl.condition.Condition; import org.fireflyframework.rules.core.dsl.expression.Expression; +import org.fireflyframework.rules.core.dsl.expression.VariableExpression; import org.fireflyframework.rules.core.dsl.lexer.Token; import org.fireflyframework.rules.core.dsl.lexer.TokenType; import lombok.extern.slf4j.Slf4j; @@ -289,17 +290,27 @@ private Action parseConditionalAction() { } /** - * Parse arithmetic action: "add value to variable", "subtract value from variable", etc. + * Parse an arithmetic action. + * + *

{@code add} and {@code subtract} use English-natural order: + * 

add VALUE to TARGET
+ * 
subtract VALUE from TARGET
+ * + *

{@code multiply} and {@code divide} accept both orderings -- the parser + * disambiguates by which operand is a bare identifier (the target must be a writable + * variable name): + * 

multiply 1.5 by risk_factor   -- value then target (canonical)
+ * 
multiply risk_factor by 1.5   -- target then value (English-natural)
+ * Both produce the same {@code ArithmeticAction(MULTIPLY, "risk_factor", LiteralExpression(1.5))}. */ private Action parseArithmeticAction(ArithmeticAction.ArithmeticOperationType operation) { Token operationToken = previous(); - // Parse the value expression + // Parse the first expression after the action keyword. this.expressionParser.setCurrentPosition(this.current); - Expression value = this.expressionParser.parseExpression(); + Expression first = this.expressionParser.parseExpression(); this.current = this.expressionParser.getCurrentPosition(); - // Expect the preposition (to/from/by) String expectedPreposition = operation.getPreposition(); if (!check(TokenType.TO) && !check(TokenType.FROM) && !check(TokenType.BY)) { throw error( @@ -308,20 +319,34 @@ private Action parseArithmeticAction(ArithmeticAction.ArithmeticOperationType op List.of("Add '" + expectedPreposition + "' after the value") ); } - advance(); // consume the preposition - if (!check(TokenType.IDENTIFIER)) { - throw error( - "Expected variable name after '" + expectedPreposition + "'", - "PARSE_007", - List.of("Add a variable name after '" + expectedPreposition + "'") - ); + // Two patterns are accepted: + // 1) VALUE IDENTIFIER -- current canonical order, used by add/subtract/multiply/divide + // 2) IDENTIFIER VALUE -- English-natural order for multiply/divide + // If the next token is a bare IDENTIFIER, fall into pattern 1. + // Otherwise (number, paren, etc.) and the first expression was itself a bare variable, + // fall into pattern 2. + if (check(TokenType.IDENTIFIER)) { + Token variable = advance(); + return new ArithmeticAction(operationToken.getLocation(), operation, variable.getLexeme(), first); } - Token variable = advance(); + boolean swappedAllowed = operation == ArithmeticAction.ArithmeticOperationType.MULTIPLY + || operation == ArithmeticAction.ArithmeticOperationType.DIVIDE; + if (swappedAllowed && first instanceof VariableExpression firstVar && firstVar.isSimpleVariable()) { + // Pattern 2: first was the target, what follows the preposition is the value expression. + this.expressionParser.setCurrentPosition(this.current); + Expression valueExpr = this.expressionParser.parseExpression(); + this.current = this.expressionParser.getCurrentPosition(); + return new ArithmeticAction(operationToken.getLocation(), operation, firstVar.getVariableName(), valueExpr); + } - return new ArithmeticAction(operationToken.getLocation(), operation, variable.getLexeme(), value); + throw error( + "Expected variable name after '" + expectedPreposition + "'", + "PARSE_007", + List.of("Add a variable name after '" + expectedPreposition + "'") + ); } /** diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ActionExecutor.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ActionExecutor.java index f50ba2a..d1b8a67 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ActionExecutor.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ActionExecutor.java @@ -127,12 +127,6 @@ private boolean containsNonMathematicalOperation(Expression expression) { if (expression instanceof FunctionCallExpression) { return true; } - if (expression instanceof RestCallExpression) { - return true; - } - if (expression instanceof JsonPathExpression) { - return true; - } if (expression instanceof BinaryExpression binaryExpr) { return containsNonMathematicalOperation(binaryExpr.getLeft()) || containsNonMathematicalOperation(binaryExpr.getRight()); @@ -508,15 +502,4 @@ public Void visitDoWhileAction(DoWhileAction node) { return null; } - @Override - public Void visitJsonPathExpression(JsonPathExpression node) { - // Expressions don't execute actions, so return null - return null; - } - - @Override - public Void visitRestCallExpression(RestCallExpression node) { - // Expressions don't execute actions, so return null - return null; - } } diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ExpressionEvaluator.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ExpressionEvaluator.java index fc2f185..87221ef 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ExpressionEvaluator.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ExpressionEvaluator.java @@ -2235,87 +2235,6 @@ private Map createErrorResponse(String message) { return errorResponse; } - @Override - public Object visitJsonPathExpression(JsonPathExpression node) { - if (jsonPathService == null) { - log.warn("JsonPathService not available for JSON path expression"); - return null; - } - - try { - Object sourceValue = node.getSourceExpression().accept(this); - if (sourceValue == null) { - return null; - } - - String jsonPath = node.getJsonPath(); - Object result = jsonPathService.extractValue(sourceValue, jsonPath); - - JsonLogger.info(log, context.getOperationId(), - String.format("JSON path extraction: %s -> %s", jsonPath, result)); - - return result; - } catch (Exception e) { - JsonLogger.error(log, context.getOperationId(), - "Error evaluating JSON path expression: " + node.toDebugString(), e); - return null; - } - } - - @Override - public Object visitRestCallExpression(RestCallExpression node) { - if (restCallService == null) { - log.warn("RestCallService not available for REST call expression"); - return null; - } - - try { - // Evaluate URL - String url = (String) node.getUrlExpression().accept(this); - if (url == null || url.trim().isEmpty()) { - throw new IllegalArgumentException("URL cannot be null or empty"); - } - - // Evaluate optional parameters - Object body = node.getBodyExpression() != null ? - node.getBodyExpression().accept(this) : null; - - @SuppressWarnings("unchecked") - Map headers = node.getHeadersExpression() != null ? - (Map) node.getHeadersExpression().accept(this) : null; - - Long timeout = node.getTimeoutExpression() != null ? - toLong(node.getTimeoutExpression().accept(this)) : null; - - // Make the REST call - String method = node.getHttpMethod(); - JsonLogger.info(log, context.getOperationId(), - String.format("Making REST call: %s %s", method, url)); - - // Since we're in a synchronous context, we need to block on the reactive call - Map result = restCallService.request(method, url, body, headers, timeout) - .block(); // This blocks the current thread - in production, consider async handling - - JsonLogger.info(log, context.getOperationId(), - String.format("REST call completed: %s %s", method, url)); - - return result; - - } catch (Exception e) { - JsonLogger.error(log, context.getOperationId(), - "Error evaluating REST call expression: " + node.toDebugString(), e); - - // Return error response instead of null - Map errorResponse = new java.util.HashMap<>(); - errorResponse.put("success", false); - errorResponse.put("error", true); - errorResponse.put("message", e.getMessage()); - return errorResponse; - } - } - - - /** * Convert an object to Long */ diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ValidationVisitor.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ValidationVisitor.java index 91cd2da..c84fb72 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ValidationVisitor.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/dsl/visitor/ValidationVisitor.java @@ -342,12 +342,6 @@ private boolean containsNonMathematicalOperation(Expression expression) { if (expression instanceof FunctionCallExpression) { return true; } - if (expression instanceof RestCallExpression) { - return true; - } - if (expression instanceof JsonPathExpression) { - return true; - } if (expression instanceof BinaryExpression binaryExpr) { return containsNonMathematicalOperation(binaryExpr.getLeft()) || containsNonMathematicalOperation(binaryExpr.getRight()); @@ -611,52 +605,4 @@ public List visitDoWhileAction(DoWhileAction node) { return errors; } - @Override - public List visitJsonPathExpression(JsonPathExpression node) { - List errors = new ArrayList<>(); - - // Validate source expression - errors.addAll(node.getSourceExpression().accept(this)); - - // Validate JSON path syntax - if (node.getJsonPath() == null || node.getJsonPath().trim().isEmpty()) { - errors.add(new ValidationError( - "JSON path cannot be empty", - node.getLocation(), - "VAL_019" - )); - } - - return errors; - } - - @Override - public List visitRestCallExpression(RestCallExpression node) { - List errors = new ArrayList<>(); - - // Validate URL expression - errors.addAll(node.getUrlExpression().accept(this)); - - // Validate HTTP method - if (node.getHttpMethod() == null || node.getHttpMethod().trim().isEmpty()) { - errors.add(new ValidationError( - "HTTP method cannot be empty", - node.getLocation(), - "VAL_020" - )); - } - - // Validate optional expressions - if (node.getBodyExpression() != null) { - errors.addAll(node.getBodyExpression().accept(this)); - } - if (node.getHeadersExpression() != null) { - errors.addAll(node.getHeadersExpression().accept(this)); - } - if (node.getTimeoutExpression() != null) { - errors.addAll(node.getTimeoutExpression().accept(this)); - } - - return errors; - } } diff --git a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/validation/YamlDslValidator.java b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/validation/YamlDslValidator.java index e3accd8..2d7a617 100644 --- a/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/validation/YamlDslValidator.java +++ b/fireflyframework-rule-engine-core/src/main/java/org/fireflyframework/rules/core/validation/YamlDslValidator.java @@ -788,32 +788,6 @@ public Void visitDoWhileAction(DoWhileAction node) { return null; } - @Override - public Void visitJsonPathExpression(JsonPathExpression node) { - // Visit the source expression to collect any variable references - if (node.getSourceExpression() != null) { - node.getSourceExpression().accept(this); - } - return null; - } - - @Override - public Void visitRestCallExpression(RestCallExpression node) { - // Visit all expressions to collect any variable references - if (node.getUrlExpression() != null) { - node.getUrlExpression().accept(this); - } - if (node.getBodyExpression() != null) { - node.getBodyExpression().accept(this); - } - if (node.getHeadersExpression() != null) { - node.getHeadersExpression().accept(this); - } - if (node.getTimeoutExpression() != null) { - node.getTimeoutExpression().accept(this); - } - return null; - } } /** @@ -1050,11 +1024,6 @@ private List performDSLReferenceValidation(AST issues.addAll(validateMetadataSection(rulesDSL.getMetadata())); } - // Validate circuit breaker configuration if present - if (rulesDSL.getCircuitBreaker() != null) { - issues.addAll(validateCircuitBreakerConfig(rulesDSL.getCircuitBreaker())); - } - // Validate when/then structure if (rulesDSL.getWhenConditions() != null && !rulesDSL.getWhenConditions().isEmpty()) { if (rulesDSL.getThenActions() == null || rulesDSL.getThenActions().isEmpty()) { @@ -1173,20 +1142,6 @@ private List validateMetadataSection(Map validateCircuitBreakerConfig(ASTRulesDSL.ASTCircuitBreakerConfig circuitBreaker) { - List issues = new ArrayList<>(); - - // Note: ASTCircuitBreakerConfig is referenced but may not be fully implemented yet - // This is a placeholder for when the circuit breaker configuration is fully implemented - - log.debug("Circuit breaker configuration validation - implementation pending"); - - return issues; - } - /** * Perform operator and function validation */ diff --git a/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/ArithmeticActionSymmetryTest.java b/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/ArithmeticActionSymmetryTest.java new file mode 100644 index 0000000..4f243cd --- /dev/null +++ b/fireflyframework-rule-engine-core/src/test/java/org/fireflyframework/rules/core/dsl/ArithmeticActionSymmetryTest.java @@ -0,0 +1,162 @@ +/* + * Copyright 2024-2026 Firefly Software Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.fireflyframework.rules.core.dsl; + +import org.fireflyframework.rules.core.dsl.evaluation.ASTRulesEvaluationEngine; +import org.fireflyframework.rules.core.dsl.evaluation.ASTRulesEvaluationResult; +import org.fireflyframework.rules.core.dsl.parser.ASTRulesDSLParser; +import org.fireflyframework.rules.core.dsl.parser.DSLParser; +import org.fireflyframework.rules.core.services.ConstantService; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import reactor.core.publisher.Flux; + +import java.math.BigDecimal; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Verifies that {@code multiply} and {@code divide} arithmetic actions accept both + * canonical ({@code multiply VALUE by VARIABLE}) and English-natural + * ({@code multiply VARIABLE by VALUE}) orderings, producing identical results. + * + *

{@code add} and {@code subtract} are already English-natural in the value-first + * form ({@code add 5 to score}, {@code subtract penalty from total}) and are not part + * of this symmetry contract. + */ +class ArithmeticActionSymmetryTest { + + private ASTRulesEvaluationEngine engine; + + @BeforeEach + void setUp() { + DSLParser dslParser = new DSLParser(); + ASTRulesDSLParser parser = new ASTRulesDSLParser(dslParser); + ConstantService constantService = Mockito.mock(ConstantService.class); + Mockito.when(constantService.getConstantsByCodes(Mockito.anyList())).thenReturn(Flux.empty()); + engine = new ASTRulesEvaluationEngine(parser, constantService); + } + + @Test + @DisplayName("multiply VALUE by VARIABLE (canonical form) -- value first, target second") + void multiplyCanonicalForm() { + String yaml = """ + then: + - set risk_factor to 10 + - multiply 1.5 by risk_factor + output: + risk_factor: number + """; + + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, Map.of()); + + assertThat(result.isSuccess()).isTrue(); + assertThat(new BigDecimal(result.getOutputData().get("risk_factor").toString())) + .isEqualByComparingTo(new BigDecimal("15.0")); + } + + @Test + @DisplayName("multiply VARIABLE by VALUE (English-natural form) -- target first, value second") + void multiplyEnglishNaturalForm() { + String yaml = """ + then: + - set risk_factor to 10 + - multiply risk_factor by 1.5 + output: + risk_factor: number + """; + + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, Map.of()); + + assertThat(result.isSuccess()).isTrue(); + assertThat(new BigDecimal(result.getOutputData().get("risk_factor").toString())) + .isEqualByComparingTo(new BigDecimal("15.0")); + } + + @Test + @DisplayName("divide accepts both orderings symmetrically") + void divideBothOrderings() { + String yamlCanonical = """ + then: + - set monthly to 1200 + - divide 12 by monthly + output: + monthly: number + """; + String yamlNatural = """ + then: + - set monthly to 1200 + - divide monthly by 12 + output: + monthly: number + """; + + // Both forms compute monthly = monthly / 12 = 100 + for (String yaml : new String[]{yamlCanonical, yamlNatural}) { + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, Map.of()); + assertThat(result.isSuccess()) + .as("yaml: %s", yaml) + .isTrue(); + assertThat(new BigDecimal(result.getOutputData().get("monthly").toString())) + .as("yaml: %s", yaml) + .isEqualByComparingTo(new BigDecimal("100")); + } + } + + @Test + @DisplayName("add and subtract retain English-natural value-first grammar (no symmetry needed)") + void addAndSubtractEnglishNatural() { + String yaml = """ + then: + - set score to 100 + - add 5 to score # score += 5 + - subtract 2 from score # score -= 2 + output: + score: number + """; + + ASTRulesEvaluationResult result = engine.evaluateRules(yaml, Map.of()); + assertThat(result.isSuccess()).isTrue(); + assertThat(new BigDecimal(result.getOutputData().get("score").toString())) + .isEqualByComparingTo(new BigDecimal("103")); + } + + @Test + @DisplayName("Complex value expression in either position parses correctly") + void complexValueExpression() { + String yaml = """ + inputs: + baseAmount: "number" + rate: "number" + then: + - set total to baseAmount + - multiply total by (1 + rate) + output: + total: number + """; + + ASTRulesEvaluationResult result = engine.evaluateRules( + yaml, Map.of("baseAmount", new BigDecimal("100"), "rate", new BigDecimal("0.25"))); + + assertThat(result.isSuccess()).isTrue(); + assertThat(new BigDecimal(result.getOutputData().get("total").toString())) + .isEqualByComparingTo(new BigDecimal("125.00")); + } +}