bncriju commented on code in PR #6299:
URL:
https://github.com/apache/incubator-kie-drools/pull/6299#discussion_r2038930033
##########
kie-dmn/kie-dmn-feel/src/test/java/org/kie/dmn/feel/util/NumberEvalHelperTest.java:
##########
@@ -34,4 +35,46 @@ void getBigDecimalOrNull() {
assertThat(NumberEvalHelper.getBigDecimalOrNull(10000000000.5D)).isEqualTo(new
BigDecimal("10000000000.5"));
}
+ @Test
+ void coerceIntegerNumber_withBigDecimal() {
Review Comment:
Add a note that it truncates instead of rounding, to clarify behavior
##########
kie-dmn/kie-dmn-feel/src/test/java/org/kie/dmn/feel/util/NumberEvalHelperTest.java:
##########
@@ -34,4 +35,46 @@ void getBigDecimalOrNull() {
assertThat(NumberEvalHelper.getBigDecimalOrNull(10000000000.5D)).isEqualTo(new
BigDecimal("10000000000.5"));
}
+ @Test
+ void coerceIntegerNumber_withBigDecimal() {
+ Object result = NumberEvalHelper.coerceIntegerNumber(new
BigDecimal("99.99"));
+ assertThat(result).isNotNull();
+ assertThat(result).isEqualTo(99);
+ }
+
+ @Test
+ void coerceIntegerNumber_withBigInteger() {
+ Object result = NumberEvalHelper.coerceIntegerNumber(new
BigInteger("1000"));
+ assertThat(result).isNotNull();
+ assertThat(result).isEqualTo(1000);
+ }
+
+ @Test
+ void coerceIntegerNumber_withDouble() {
+ Object result = NumberEvalHelper.coerceIntegerNumber(042.50);
+ assertThat(result).isNotNull();
+ assertThat(result).isEqualTo(42);
+ }
+
+ @Test
+ void coerceIntegerNumber_withString() {
+ Object result = NumberEvalHelper.coerceIntegerNumber("123");
+ assertThat(result).isNotNull();
+ assertThat(result).isEqualTo("123");
+ }
+
+ @Test
+ void coerceIntegerNumber_withInteger() {
+ Object result = NumberEvalHelper.coerceIntegerNumber(42);
+ assertThat(result).isNotNull();
+ assertThat(result).isEqualTo(42);
+ }
+
+ @Test
+ void coerceIntegerNumber_withLong() {
+ Object result = NumberEvalHelper.coerceIntegerNumber(423L);
+ assertThat(result).isNotNull();
+ assertThat(result).isEqualTo(423);
+ }
Review Comment:
Add a test for null input. Add boundary tests for the scale validation logic
(e.g., exactly -6111, 6176, just outside the bounds).
##########
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/functions/CeilingFunction.java:
##########
@@ -45,11 +46,14 @@ public FEELFnResult<BigDecimal> invoke(@ParameterName( "n"
) BigDecimal n, @Para
if ( scale == null ) {
return FEELFnResult.ofError(new
InvalidParametersEvent(Severity.ERROR, "scale", "cannot be null"));
}
+ Object scaleObj = NumberEvalHelper.coerceIntegerNumber(scale);
// Based on Table 76: Semantics of numeric functions, the scale is in
range −6111 .. 6176
- if (scale.compareTo(BigDecimal.valueOf(-6111)) < 0 ||
scale.compareTo(BigDecimal.valueOf(6176)) > 0) {
- return FEELFnResult.ofError(new
InvalidParametersEvent(Severity.ERROR, "scale", "must be in range between -6111
to 6176."));
+ if (scaleObj instanceof Integer) {
+ int scaleInt = (Integer) scaleObj;
+ if (scaleInt < -6111 || scaleInt > 6176) {
+ return FEELFnResult.ofError(new
InvalidParametersEvent(Severity.ERROR, "scale", "must be in range between -6111
to 6176."));
+ }
Review Comment:
else case could be written here? (i.e., if scaleObj is not an Integer).
currently, the flow silently skips validation.
##########
kie-dmn/kie-dmn-feel/src/test/java/org/kie/dmn/feel/util/NumberEvalHelperTest.java:
##########
@@ -34,4 +35,46 @@ void getBigDecimalOrNull() {
assertThat(NumberEvalHelper.getBigDecimalOrNull(10000000000.5D)).isEqualTo(new
BigDecimal("10000000000.5"));
}
+ @Test
+ void coerceIntegerNumber_withBigDecimal() {
+ Object result = NumberEvalHelper.coerceIntegerNumber(new
BigDecimal("99.99"));
+ assertThat(result).isNotNull();
+ assertThat(result).isEqualTo(99);
+ }
+
+ @Test
+ void coerceIntegerNumber_withBigInteger() {
+ Object result = NumberEvalHelper.coerceIntegerNumber(new
BigInteger("1000"));
+ assertThat(result).isNotNull();
+ assertThat(result).isEqualTo(1000);
+ }
+
+ @Test
+ void coerceIntegerNumber_withDouble() {
+ Object result = NumberEvalHelper.coerceIntegerNumber(042.50);
Review Comment:
042.50 is invalid syntax in Java (leading 0 implies octal, and .50 isn't
valid in octal). it could be corrected to 42.50
##########
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/functions/DateFunction.java:
##########
@@ -72,16 +73,10 @@ public FEELFnResult<TemporalAccessor>
invoke(@ParameterName( "from" ) String val
}
public FEELFnResult<TemporalAccessor> invoke(@ParameterName( "year" )
Number year, @ParameterName( "month" ) Number month, @ParameterName( "day" )
Number day) {
- if ( year == null ) {
- return FEELFnResult.ofError(new
InvalidParametersEvent(Severity.ERROR, "year", "cannot be null"));
- }
- if ( month == null ) {
- return FEELFnResult.ofError(new
InvalidParametersEvent(Severity.ERROR, "month", "cannot be null"));
- }
- if ( day == null ) {
- return FEELFnResult.ofError(new
InvalidParametersEvent(Severity.ERROR, "day", "cannot be null"));
- }
-
+ FEELFnResult<TemporalAccessor> result;
+ if ((result = checkNullParam("year", year)) != null) return result;
Review Comment:
checkNullParam could be renamed to checkParamIfNull
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
