Implement units.parse and units.parse_bytes builtins - #152
Implement units.parse and units.parse_bytes builtins#152glasses-and-hat wants to merge 1 commit into
Conversation
|
@glasses-and-hat can you sign you commit please for the DCO to pass? https://github.com/open-policy-agent/java-opa-sdk/pull/152/checks?check_run_id=86619295459 |
Adds the units.parse and units.parse_bytes OPA builtins, which were
previously unimplemented and raised FunctionNotFoundError. Both accept
strings like "10K", "1.5Gi", or "100m" and convert them to a number,
supporting the standard SI decimal (k/K, M, G, T, P, E) and binary
(Ki, Mi, Gi, Ti, Pi, Ei) suffixes, plus the milli ("m") suffix for
units.parse.
Arithmetic is done with BigDecimal to avoid floating-point precision
loss, mirroring the reference Go implementation's use of big.Rat/
big.Float: units.parse_bytes truncates towards zero, while
units.parse produces an exact integer when possible and otherwise
rounds to 10 decimal places.
Fixes open-policy-agent#133
Signed-off-by: Rahul Yellapragada <Yellapragada.Tech@gmail.com>
0da6af6 to
42e5663
Compare
@sspaink Done, amended the commit with a DCO sign-off and force-pushed. Should be green now, thanks for flagging! |
| HexBuiltins.class, | ||
| StringBuiltins.class, | ||
| PrintBuiltins.class, | ||
| UnitsBuiltins.class, |
There was a problem hiding this comment.
This is what's failing CI (cannot find symbol: class UnitsBuiltins). This branch still has the impls.* wildcard import, but #171 replaced it on main with explicit per-class imports — so the merge keeps main's import list (no UnitsBuiltins) alongside this new entry, and nothing imports the class. Needs a rebase on main plus the explicit import:
import io.github.open_policy_agent.opa.ast.builtin.impls.UnitsBuiltins;
Summary
Implements the
units.parseandunits.parse_bytesOPA builtins (closes #133), which were previously unimplemented and raisedFunctionNotFoundErrorfor every call."10K","1.5Gi","100m"into a number, supporting the standard SI decimal (k/K,M,G,T,P,E) and binary (Ki,Mi,Gi,Ti,Pi,Ei) suffixes, plus them(milli) suffix forunits.parse.extractNumAndUnitintopdown/parse_bytes.go/topdown/parse_units.go), including scientific-notation handling and theexponent too largeguard (max 6 exponent digits).BigDecimalfor exact arithmetic rather thandouble, to avoid floating-point precision loss on suffix multiplication (mirrors the reference implementation's use ofbig.Rat/big.Float):units.parse_bytestruncates the product towards zero (matchesbig.Float.Int()).units.parsereturns an exact integer when the product has no fractional part, otherwise rounds to 10 decimal places (matchesbig.Rat.FloatString(10)).no amount provided,could not parse amount to a number,spaces not allowed in resource strings,exponent too large,unit X not recognized) match the reference implementation's wording.BuiltinRegistry.BUILTIN_CLASSES(no external dependencies needed, consistent withArithmeticBuiltins/HexBuiltins).Test plan
opa-evaluator/src/test/resources/compliance/.../TestData/v1/units/(covering both happy-path and error cases, including the precision-regression cases intest-units-precision.jsonandtest-issue-4856.json) now pass — previously they were silently skipped via the compliance suite's "missing builtin" allowance.units.parse/units.parse_bytesno longer appear in the compliance suite's "Missing Builtin Report".opa-evaluatortest suite passes (./gradlew :opa-evaluator:test), no regressions../gradlew checkstyleMain checkstyleTest pmdMain pmdTestclean for the new file../gradlew compileJavasucceeds across all modules.