RulesEngine: add JSON Logic predicate evaluator#3482
Draft
ajpallares wants to merge 12 commits into
Draft
Conversation
First slice of the JSON Logic rules engine in the new `:rules-engine` module (built on top of the module skeleton from `pallares/rules-engine-skeleton`). Pure Kotlin, no exported API surface — every new declaration is module-`internal` so the engine can land incrementally without changing `rules-engine/api.txt` or pulling anything into the SDK's metalava signature. What's in: - `Logger.kt` — internal `RulesEngineLogger` interface + default `PrintlnLogger` (stderr) + test `CapturingLogger`. Kept internal so a future host-supplied logger can be adapted to the same interface without API churn. - `Value.kt` — typed `Value` sealed class (`Null` / `BoolValue` / `IntValue` / `FloatValue` / `StringValue` / `ArrayValue` / `ObjectValue`), JSON Logic truthiness + loose (`==`) / strict (`===`) equality with type coercion. - `RuleError.kt` — `RuleError` sealed `RuntimeException` (`Parse`, `TypeMismatch`, `UnsupportedOperator`). - `Evaluator.kt` — top-level dispatcher; takes a typed `Value` predicate (no JSON parsing in the engine) and returns `Boolean`. - `operators/` — MVP set: `var` (strict dot-path), `missing`, `==`, `!=`, `===`, `!==`, `!`, `!!`, `and`, `or`, `if`. - Test source set adds a `helpers/ValueJsonHelper.kt` (`org.json` + `BigDecimal` scale check to preserve int-vs-decimal intent) so test predicates can be expressed as JSON literals — production callers will construct `Value` trees from the host SDK's own JSON parser. Key decisions: - **JSON parsing lives outside the engine.** The engine accepts a typed `Value` tree (the JSON-shaped sealed class is what cross-language bridges can express across the boundary). The `org.json`-backed JSON helper is gated to the test source set only via a new `testImplementation(libs.json)` dep. - **Missing variables** resolve to `Null` and emit a warning, per JSON Logic spec. No `MissingVariable` subclass in v1; reserved for a future strict mode. - **`var` lookup** uses strict dot-path navigation. Flat-key fallback considered and deferred — pure addition if we need it later. Verification: - `./gradlew :rules-engine:test` — 63 tests, 0 failures. - `./gradlew :rules-engine:check` — `lint` + `metalavaCheckCompatibility` pass; `api.txt` unchanged because every new declaration is `internal`. - `./gradlew detektAll` — clean. Co-authored-by: Cursor <cursoragent@cursor.com>
This was referenced May 14, 2026
…ternal-api' into pallares/json-logic-evaluator
ajpallares
changed the base branch from
pallares/rules-engine-skeleton
to
pallares/rules-engine-enforce-internal-api
`opIf` already supports the 1-arg `{"if": [expr]}` (returns the evaluated
arg) and 2-arg `{"if": [cond, then]}` (returns evaluated `then` when
truthy, `Null` otherwise) forms via the trailing-else fall-through path.
Both were unpinned by tests — only the 3-arg, chained-else-if, even-arity
"no else" and empty cases were covered. Add focused tests so a future
tweak to the loop bounds in `opIf` can't silently change either form.
Co-authored-by: Cursor <cursoragent@cursor.com>
`looseEq` and `strictEq` of `FloatValue(Double.NaN)` against itself both return `false` (IEEE 754: `NaN == NaN` is always false). `FloatValue` of `Double.POSITIVE_INFINITY` against itself returns `true`. Both match the JS `==` / `===` semantics the engine targets. Pin the behavior so: 1. A future refactor can't accidentally special-case NaN-equals-NaN. 2. Cross-platform drift (Android's `String.toDoubleOrNull` vs iOS's `Double(String)`) stays visible. Mirrors the equivalent test in the iOS RulesEngine PR. Co-authored-by: Cursor <cursoragent@cursor.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## pallares/rules-engine-enforce-internal-api #3482 +/- ##
===========================================================================
Coverage 79.89% 79.89%
===========================================================================
Files 369 369
Lines 14871 14871
Branches 2048 2048
===========================================================================
Hits 11881 11881
Misses 2157 2157
Partials 833 833 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Threading a `RulesEngineLogger` parameter through every signature in `Evaluator` + every `Operators` helper added a lot of boilerplate for two warning sites (`var` missing-variable and `var` extra-args). Replace the parameter with a module-internal `RulesEngineLog` object that holds a swappable `sink: RulesEngineLogger` (defaulting to `PrintlnLogger`). Production callers — currently only the engine internals — call `RulesEngineLog.warn(...)` directly. A future host-supplied logger swaps the sink at startup. Mechanical changes: - New `RulesEngineLog` object in `Logger.kt`. - Drop `logger` param from `Evaluator.evaluate*`, `Operators.dispatch`, `Operators.evalArgs`, `Operators.evalTwo`, every `op*` in `AccessorOperators` / `EqualityOperators` / `LogicOperators`. - Move `CapturingLogger` to the test source set (no production caller). - Add `CapturingLoggerRule`: a JUnit `TestRule` that swaps the sink to a fresh `CapturingLogger` for the duration of the test and restores the previous sink afterwards. Tests assert via `loggerRule.warnings`. - Update all rules-engine tests to install the rule and drop the per-call logger argument. Co-authored-by: Cursor <cursoragent@cursor.com>
…Logic spec
Two reference-impl deviations the previous review surfaced. Pulling them
into spec compliance now while the engine has no consumers — cheaper to
fix here than after rule artifacts start depending on the deviations.
`missing`: per json-logic-js (`logic.js`'s `missing` op), a key is
"missing" when its resolved value is `null` OR `""`. Our `opMissing`
only checked for "absent from data" (Kotlin-null from `lookupPath`), so
data like `{"email": null}` or `{"email": ""}` reported `email` as
present. Now they're reported as missing. Other falsy-but-defined values
(`0`, `false`, `[]`, `{}`, `"0"`) stay non-missing per spec.
`and` / `or` empty-args: json-logic-js leaves its `current` accumulator
`undefined` when the loop body never runs and returns it; we returned
`BoolValue(true)` / `BoolValue(false)` (vacuous truth/falsity). Both
were falsy in boolean contexts but the JS-`undefined` shape only mapped
cleanly to `Value.Null` in our algebra, so visible-as-value contexts
like `{"if": [{"and": []}, "yes", "no"]}` were producing different
branches across engines. Now we return `Value.Null` for both empty
cases, matching the JS reference impl.
Tests:
- `missing reports null-valued keys as missing per spec`
- `missing reports empty-string-valued keys as missing per spec`
- `missing does not report falsy-but-defined values as missing`
(pins the negative side: `0`, `false`, `[]`, `{}`, `"0"`)
- `and empty is null per spec` / `or empty is null per spec`
(replacing the previous `BoolValue(true)` / `BoolValue(false)` asserts)
iOS PR #6789 has the same deviations on both fronts; will follow up
there to keep the two evaluators in lockstep.
Co-authored-by: Cursor <cursoragent@cursor.com>
`json-logic-js` recursively evaluates the argument(s) of `var` and
`missing` before using them as paths, so dynamic constructs like
`{"var": {"var": "active_path_key"}}` or
`{"missing": [{"var": "key_to_check"}]}` are valid. Our previous
implementation treated those args as literals, which silently rejected
otherwise-valid predicates.
`opVar` now routes its argument through `Evaluator.evaluateValue` before
parsing: the array form evaluates each element in place (path and
default both become dynamic), and the singleton form evaluates the lone
argument and treats the result as the path. `opMissing` evaluates each
key the same way, and additionally unpacks the first evaluated arg when
it resolves to an array — mirrors `Array.isArray(arguments[0])` so
constructs like `{"missing": {"merge": [["a"], ["b"]]}}` work as the
spec describes.
The one deliberate deviation: when the singleton form of `var`
evaluates to a non-primitive (e.g. an array), we throw `TypeMismatch`
instead of JS-stringifying it ("x,y") and looking that up. Pinned by
`var singleton expression resolving to array throws`.
Doc comments updated to describe the spec-aligned behavior. New tests
cover dynamic singleton paths, dynamic array-form paths, dynamic
defaults, dynamic `missing` keys, and the first-arg-array unpack rule.
Three float-path edge cases mirrored from iOS pin our `formatNumber`
behavior under integer-valued, fractional, and oversized doubles.
Ports purchases-ios `267947e60a` to keep the two evaluators in lockstep.
Co-authored-by: Cursor <cursoragent@cursor.com>
Mirrors the iOS RulesEngine's `Rules.logger` singleton: the host SDK installs an adapter once at integration time, and the engine routes diagnostic warnings through it. The default falls through to a stderr `PrintlnLogger` until the SDK wires it up. The static accessor and the logger interface are public-but-opt-in (`@InternalRulesEngineAPI`) so `:purchases` / `:ui:revenuecatui` / hybrid bridges can swap the sink without leaking new public API. The `InternalRulesEngineAPI` annotation is a metalava `hiddenAnnotation`, so the API dump still contains only the marker annotation itself — verified by `scripts/check-rules-engine-internal-only.sh`. Co-authored-by: Cursor <cursoragent@cursor.com>
Carry over four behavioral / cosmetic deltas the iOS counterpart PR
landed during review. None of these change semantics on Android — most
are documentation tightening or test-pin additions — but keeping the
two platforms in lockstep avoids future drift.
- Drop the `if (items.isEmpty()) return Value.Null` early guard from
`LogicOperators.opIf`. The loop / final return already fall through
to `Value.Null` for empty args, so the guard is dead. Mirrors iOS
`fed4baea5`.
- Add `empty and inside if takes else branch` and the `or` mirror to
`LogicOperatorsTest`. The existing tests pinned the return value of
`{"and": []}` / `{"or": []}` but nothing exercised the
cross-operator interaction the spec alignment is meant to preserve
(`{"if": [{"and": []}, …]}` must take the `else` branch). Mirrors
iOS `459f78f9b`.
- Tighten `ValueJsonHelper.convert` to throw `RuleError.Parse` for
unknown `JSONTokener` outputs (e.g. `Date`, custom `JSONString`
impls) instead of silently coercing to `Value.Null`. Better to fail
loudly than mask a parser misconfiguration with phantom `null`
operands. Mirrors iOS `4c4cc95fa`.
- Trim the verbose KDoc on the `Rules` namespace and drop the now
redundant doc on `Rules.logger`. The opt-in / cross-module
reasoning lives on `InternalRulesEngineAPI`. Mirrors iOS
`266415d91` + `d63dac156`.
Verified: `:rules-engine:testDefaultsDebugUnitTest`, `detektAll`, and
`scripts/check-rules-engine-internal-only.sh` all green.
Co-authored-by: Cursor <cursoragent@cursor.com>
…ternal-api' into pallares/json-logic-evaluator Pulls in the skeleton + enforce-side annotation drop. With `@InternalRulesEngineAPI` deleted upstream, every evaluator-side file that referenced it would fail to compile until the references are stripped, so the cleanup is folded into the merge commit: - `Logger.kt`: drop the file-level `@OptIn`, drop the `@InternalRulesEngineAPI` on `RulesEngineLogger`, trim its KDoc to drop the gating-mechanism explanation. The interface is now a plain `public interface`. - `Rules.kt`: drop the `@InternalRulesEngineAPI` on the namespace object. Stays plain `public object Rules`. - `operators/AccessorOperators.kt`: drop the file-level `@OptIn` and the `InternalRulesEngineAPI` import. Internal callsites of `Rules.logger.warn(...)` no longer need the opt-in dance. - `CapturingLogger.kt`, `CapturingLoggerRule.kt`, `LoggerTest.kt`: drop the file-level `@OptIn`. Test plumbing reaches `Rules.logger` and `RulesEngineLogger` directly now. The conflicts the merge produced were both modify/delete pairs on files the evaluator branch already deleted (`RulesEngine.kt` / `RulesEngineTest.kt` were renamed to `Rules.kt` + dropped here in commit `162574d20`); resolution is `git rm` to keep the deletions. The committed `rules-engine/api.txt` baseline is regenerated to reflect the new public surface — `Rules` (with its `logger` property) and `RulesEngineLogger`. Verified: `:rules-engine:testDefaultsDebugUnitTest`, `detektAll`, and `:rules-engine:metalavaCheckDefaultsRelease` all green. Co-authored-by: Cursor <cursoragent@cursor.com>
Locks in the observed runtime behavior for `{"==": [{"a":1}, {"a":1}]}`:
both our engine and json-logic-js dispatch single-key objects as
operators via `is_logic`/`evaluateValue`, so this literal predicate
throws `RuleError.UnsupportedOperator("a")` instead of doing a
structural compare. Pins the contrast with the existing multi-key
test (where the structural-vs-reference divergence still applies) so
a future operator-dispatch refactor can't quietly regress this.
Also tightens the doc comment on `multi-key object is a literal data
value` to mirror its iOS counterpart.
Co-authored-by: Cursor <cursoragent@cursor.com>
`looseEq` only had explicit cases for primitives and same-compound
operands, so `[1] == "1"` and `[1, 2] == "1,2"` returned `false` —
divergent from json-logic-js, which inherits JS abstract equality:
when one side is a compound (Array/Object) and the other is a
primitive, the compound goes through ToPrimitive (string hint) and
the comparison is retried.
Add four cross-type arms to `looseEq` that mirror exactly that
coercion: arrays render via `Array.prototype.toString()` (recursive
comma-join, with `null` elements as the empty string); objects render
as `"[object Object]"`. The recursive call falls through to the
existing primitive arms — string-vs-string for `[1, 2] == "1,2"`,
the numeric fallback for `[] == 0` / `[1] == 1`. Same-compound
comparisons keep their structural-eq behavior (deliberate divergence
from JS reference identity, which would make rule patterns like
`{"==": [{"var": "tags"}, ["a", "b"]]}` always false).
Sharpen the `looseEq` doc comment to spell out the four behaviors
(same-type, cross-numeric, same-compound structural, compound-vs-
primitive ToPrimitive) and the JS reference identity divergence.
Pin the new behavior with `ValueTest` cases covering each spec
example (`[1] == "1"`, `[1, 2] == "1,2"`, `[null, 1] == ",1"`,
`[] == ""`, nested arrays, the numeric fallback path, JS-specific
float rendering, `{...} == "[object Object]"`, and array-vs-object).
Also pin the deliberate same-compound structural rule in a separate
`looseEq objects structural` test, mirroring the array case.
Add two `EvaluatorTest` cases that drive the same coercion through
the full `Evaluator.evaluate` path so a future regression in either
layer is caught.
Mirrors purchases-ios c5d53ec71.
Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves SDK-4326
Checklist
Motivation
First slice of the JSON Logic rules engine, stacked on the no-public-APIs enforcement in #3480 (and through it, the skeleton in #3478). Resolves SDK-4320.
Description
Logger,RuleError,Value,Evaluator, andoperators/with the MVP set:var,missing,==,!=,===,!==,!,!!,and,or,if.internal, so Enforce :rules-engine has no public API outside @InternalRulesEngineAPI #3480's intrinsic "no non-internal public API" check stays clean and the engine can land incrementally.ValueJsonHelperlets predicates be authored the way they appear in rule artifacts; production callers will buildValuetrees from the host SDK's own JSON parser.Verification:
./gradlew :rules-engine:test(63/63 pass),./gradlew :rules-engine:check(lint + metalava clean),./gradlew detektAllclean.Made with Cursor