[NU-1932] Big decimal uses default scale (#7356)

Co-authored-by: Pawel Czajka <[email protected]>

build.sbt

@@ -1130,7 +1130,7 @@ lazy val mathUtils = (project in utils("math-utils"))
       "org.springframework" % "spring-expression" % springV,
     )
   )
-  .dependsOn(componentsApi, testUtils % Test)
+  .dependsOn(commonUtils, componentsApi, testUtils % Test)
 
 lazy val defaultHelpers = (project in utils("default-helpers"))
   .settings(commonSettings)

docs/Changelog.md

@@ -30,6 +30,8 @@
 * [#7346](https://github.com/TouK/nussknacker/pull/7346) OpenAPI enricher: ability to configure common secret for any security scheme
 * [#7307](https://github.com/TouK/nussknacker/pull/7307) Added StddevPop, StddevSamp, VarPop and VarSamp aggregators
 * [#7341](https://github.com/TouK/nussknacker/pull/7341) Added possibility to choose presets and define lists for Integer typed parameter inputs in fragments.
+* [#7356](https://github.com/TouK/nussknacker/pull/7356) Integers converted to BigDecimals have scale 18,
+  this fixes issue with unexpected low scale when performing division on BigDecimals which were created in such conversion.
 
 ## 1.18
 

engine/flink/executor/src/test/scala/pl/touk/nussknacker/engine/process/functional/FragmentSpec.scala

@@ -10,18 +10,42 @@ import pl.touk.nussknacker.engine.canonicalgraph.{CanonicalProcess, canonicalnod
 import pl.touk.nussknacker.engine.compile.FragmentResolver
 import pl.touk.nussknacker.engine.graph.evaluatedparam.BranchParameters
 import pl.touk.nussknacker.engine.graph.evaluatedparam.{Parameter => NodeParameter}
+import pl.touk.nussknacker.engine.graph.expression.Expression
 import pl.touk.nussknacker.engine.graph.node.FragmentInputDefinition.{FragmentClazzRef, FragmentParameter}
 import pl.touk.nussknacker.engine.graph.node._
 import pl.touk.nussknacker.engine.graph.sink.SinkRef
+import pl.touk.nussknacker.engine.graph.variable.Field
 import pl.touk.nussknacker.engine.process.helpers.ProcessTestHelpers
 import pl.touk.nussknacker.engine.process.helpers.SampleNodes._
+import pl.touk.nussknacker.springframework.util.BigDecimalScaleEnsurer
 
 import java.util.Date
 
 class FragmentSpec extends AnyFunSuite with Matchers with ProcessTestHelpers {
 
   import pl.touk.nussknacker.engine.spel.SpelExtension._
 
+  test("should properly convert fragment input to BigDecimal") {
+
+    val process = resolve(
+      ScenarioBuilder
+        .streaming("proc1")
+        .source("id", "input")
+        .fragmentOneOut("sub", "fragmentWithBigDecimalInput", "output", "fragmentResult", "param" -> "1".spel)
+        .processorEnd("end1", "logService", "all" -> "#fragmentResult.a".spel)
+    )
+
+    val data = List(
+      SimpleRecord("1", 12, "a", new Date(0))
+    )
+
+    processInvoker.invokeWithSampleData(process, data)
+    val results = ProcessTestHelpers.logServiceResultsHolder.results
+
+    results.size shouldBe 1
+    results(0).asInstanceOf[java.math.BigDecimal].scale() shouldBe BigDecimalScaleEnsurer.DEFAULT_BIG_DECIMAL_SCALE
+  }
+
   test("should accept same id in fragment and main process ") {
 
     val process = resolve(
@@ -101,6 +125,20 @@ class FragmentSpec extends AnyFunSuite with Matchers with ProcessTestHelpers {
   }
 
   private def resolve(scenario: CanonicalProcess) = {
+    val fragmentWithBigDecimalInput = CanonicalProcess(
+      MetaData("fragmentWithBigDecimalInput", FragmentSpecificData()),
+      List(
+        canonicalnode.FlatNode(
+          FragmentInputDefinition(
+            "start",
+            List(FragmentParameter(ParameterName("param"), FragmentClazzRef[java.math.BigDecimal]))
+          )
+        ),
+        canonicalnode.FlatNode(FragmentOutputDefinition("outB1", "output", List(Field("a", Expression.spel("#param")))))
+      ),
+      List.empty
+    )
+
     val fragment = CanonicalProcess(
       MetaData("fragment1", FragmentSpecificData()),
       List(
@@ -182,7 +220,9 @@ class FragmentSpec extends AnyFunSuite with Matchers with ProcessTestHelpers {
     )
 
     val resolved =
-      FragmentResolver(List(fragmentWithSplit, fragment, fragmentWithGlobalVar, diamondFragment)).resolve(scenario)
+      FragmentResolver(
+        List(fragmentWithBigDecimalInput, fragmentWithSplit, fragment, fragmentWithGlobalVar, diamondFragment)
+      ).resolve(scenario)
 
     resolved shouldBe Symbol("valid")
     resolved.toOption.get

engine/lite/components/request-response-tests/src/test/scala/pl/touk/nussknacker/engine/lite/components/requestresponse/CollectTransformerTest.scala

@@ -55,7 +55,7 @@ class CollectTransformerTest
   test("should collect elements after for-each") {
     val scenario = scenarioBuilderWithSchemas
       .customNode("for-each", "outForEach", "for-each", "Elements" -> "#input".spel)
-      .buildSimpleVariable("someVar", "ourVar", "'x = ' + (#outForEach * 2)".spel)
+      .buildSimpleVariable("someVar", "ourVar", "'x = ' + (#outForEach.intValue() * 2)".spel)
       .customNode("collect", "outCollector", "collect", "Input expression" -> "#ourVar".spel)
       .emptySink("response", "response", SinkRawEditorParamName.value -> "true".spel, "Value" -> "#outCollector".spel)
     val requestElements = (0 to 3).toList

scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/extension/Conversion.scala

@@ -6,6 +6,7 @@ import org.springframework.util.NumberUtils
 import pl.touk.nussknacker.engine.api.generics.GenericFunctionTypingError
 import pl.touk.nussknacker.engine.api.typed.typing.{Typed, TypingResult}
 import pl.touk.nussknacker.engine.util.classes.Extensions.ClassExtensions
+import pl.touk.nussknacker.springframework.util.BigDecimalScaleEnsurer
 
 import java.lang.{
   Boolean => JBoolean,
@@ -68,11 +69,14 @@ object ToBigDecimalConversion extends ToNumericConversion[JBigDecimal] {
 
   override def convertEither(value: Any): Either[Throwable, JBigDecimal] =
     value match {
-      case v: JBigDecimal => Right(v)
-      case v: JBigInteger => Right(new JBigDecimal(v))
+      case v: JBigDecimal => Right(BigDecimalScaleEnsurer.ensureBigDecimalScale(v))
+      case v: JBigInteger => Right(BigDecimalScaleEnsurer.ensureBigDecimalScale(new JBigDecimal(v)))
       case v: Number      => Try(NumberUtils.convertNumberToTargetClass(v, resultTypeClass)).toEither
-      case v: String      => Try(new JBigDecimal(v)).toEither
-      case _              => Left(new IllegalArgumentException(s"Cannot convert: $value to BigDecimal"))
+      case v: String =>
+        Try({
+          BigDecimalScaleEnsurer.ensureBigDecimalScale(new JBigDecimal(v))
+        }).toEither
+      case _ => Left(new IllegalArgumentException(s"Cannot convert: $value to BigDecimal"))
     }
 
 }

scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala

@@ -52,6 +52,7 @@ import pl.touk.nussknacker.engine.spel.SpelExpressionParseError.{
 }
 import pl.touk.nussknacker.engine.spel.SpelExpressionParser.{Flavour, Standard}
 import pl.touk.nussknacker.engine.testing.ModelDefinitionBuilder
+import pl.touk.nussknacker.springframework.util.BigDecimalScaleEnsurer
 import pl.touk.nussknacker.test.ValidatedValuesDetailedMessage
 
 import java.lang.{
@@ -358,6 +359,31 @@ class SpelExpressionSpec extends AnyFunSuite with Matchers with ValidatedValuesD
     }
   }
 
+  test("should set large enough scale when converting to big decimal so that division by 2 works as expected") {
+    val result = evaluate[Any]("""(1).toBigDecimal / 2""".stripMargin)
+    BigDecimal(result.asInstanceOf[java.math.BigDecimal]) shouldBe BigDecimal(0.5) +- BigDecimal(0.001)
+  }
+
+  test("should set scale at least default when creating big decimal from int") {
+    val result = evaluate[Any]("""(1).toBigDecimal""".stripMargin)
+    result.asInstanceOf[java.math.BigDecimal].scale() shouldBe BigDecimalScaleEnsurer.DEFAULT_BIG_DECIMAL_SCALE
+  }
+
+  test("should set scale at least default when creating big decimal from big int") {
+    val result = evaluate[Any]("""#a.toBigDecimal""".stripMargin, Context("asd", Map("a" -> JBigInteger.ONE)))
+    result.asInstanceOf[java.math.BigDecimal].scale() shouldBe BigDecimalScaleEnsurer.DEFAULT_BIG_DECIMAL_SCALE
+  }
+
+  test("should set scale at least default when creating big decimal from string with low scale") {
+    val result = evaluate[Any]("""("1.23").toBigDecimal""".stripMargin)
+    result.asInstanceOf[java.math.BigDecimal].scale() shouldBe BigDecimalScaleEnsurer.DEFAULT_BIG_DECIMAL_SCALE
+  }
+
+  test("should set high scale when creating big decimal from string with high scale") {
+    val result = evaluate[Any]("""("1.345345345345345345345345345345").toBigDecimal""".stripMargin)
+    result.asInstanceOf[java.math.BigDecimal].scale() shouldBe 30
+  }
+
   test("indexer access on unknown - array like case") {
     parse[Any](
       "#containerWithUnknownArray.value[0]"
@@ -1789,17 +1815,45 @@ class SpelExpressionSpec extends AnyFunSuite with Matchers with ValidatedValuesD
         ("#unknownDouble.value.to('Double')", doubleTyping, 1.1),
         ("#unknownDoubleString.value.to('Double')", doubleTyping, 1.1),
         ("#unknownBoolean.value.toOrNull('Double')", doubleTyping, null),
-        ("1.to('BigDecimal')", bigDecimalTyping, BigDecimal(1).bigDecimal),
+        (
+          "1.to('BigDecimal')",
+          bigDecimalTyping,
+          BigDecimalScaleEnsurer.ensureBigDecimalScale(BigDecimal(1).bigDecimal)
+        ),
         ("1.1.to('BigDecimal')", bigDecimalTyping, convertedDoubleToBigDecimal),
-        ("'1'.to('BigDecimal')", bigDecimalTyping, BigDecimal(1).bigDecimal),
-        ("1.toBigDecimalOrNull()", bigDecimalTyping, BigDecimal(1).bigDecimal),
+        (
+          "'1'.to('BigDecimal')",
+          bigDecimalTyping,
+          BigDecimalScaleEnsurer.ensureBigDecimalScale(BigDecimal(1).bigDecimal)
+        ),
+        (
+          "1.toBigDecimalOrNull()",
+          bigDecimalTyping,
+          BigDecimalScaleEnsurer.ensureBigDecimalScale(BigDecimal(1).bigDecimal)
+        ),
         ("1.1.toOrNull('BigDecimal')", bigDecimalTyping, convertedDoubleToBigDecimal),
-        ("'1'.toOrNull('BigDecimal')", bigDecimalTyping, BigDecimal(1).bigDecimal),
+        (
+          "'1'.toOrNull('BigDecimal')",
+          bigDecimalTyping,
+          BigDecimalScaleEnsurer.ensureBigDecimalScale(BigDecimal(1).bigDecimal)
+        ),
         ("'a'.toOrNull('BigDecimal')", bigDecimalTyping, null),
-        ("#unknownLong.value.to('BigDecimal')", bigDecimalTyping, BigDecimal(11).bigDecimal),
-        ("#unknownLongString.value.to('BigDecimal')", bigDecimalTyping, BigDecimal(11).bigDecimal),
+        (
+          "#unknownLong.value.to('BigDecimal')",
+          bigDecimalTyping,
+          BigDecimalScaleEnsurer.ensureBigDecimalScale(BigDecimal(11).bigDecimal)
+        ),
+        (
+          "#unknownLongString.value.to('BigDecimal')",
+          bigDecimalTyping,
+          BigDecimalScaleEnsurer.ensureBigDecimalScale(BigDecimal(11).bigDecimal)
+        ),
         ("#unknownDouble.value.to('BigDecimal')", bigDecimalTyping, convertedDoubleToBigDecimal),
-        ("#unknownDoubleString.value.to('BigDecimal')", bigDecimalTyping, BigDecimal(1.1).bigDecimal),
+        (
+          "#unknownDoubleString.value.to('BigDecimal')",
+          bigDecimalTyping,
+          BigDecimalScaleEnsurer.ensureBigDecimalScale(BigDecimal(1.1).bigDecimal)
+        ),
         ("#unknownBoolean.value.toOrNull('BigDecimal')", bigDecimalTyping, null),
         ("'true'.to('Boolean')", booleanTyping, true),
         ("#unknownInteger.value.toOrNull('Boolean')", booleanTyping, null),
@@ -1874,17 +1928,41 @@ class SpelExpressionSpec extends AnyFunSuite with Matchers with ValidatedValuesD
         ("#unknownDouble.value.toDouble()", doubleTyping, 1.1),
         ("#unknownDoubleString.value.toDouble()", doubleTyping, 1.1),
         ("#unknownBoolean.value.toDoubleOrNull()", doubleTyping, null),
-        ("1.toBigDecimal()", bigDecimalTyping, BigDecimal(1).bigDecimal),
+        ("1.toBigDecimal()", bigDecimalTyping, BigDecimalScaleEnsurer.ensureBigDecimalScale(BigDecimal(1).bigDecimal)),
         ("1.1.toBigDecimal()", bigDecimalTyping, convertedDoubleToBigDecimal),
-        ("'1'.toBigDecimal()", bigDecimalTyping, BigDecimal(1).bigDecimal),
-        ("1.toBigDecimalOrNull()", bigDecimalTyping, BigDecimal(1).bigDecimal),
+        (
+          "'1'.toBigDecimal()",
+          bigDecimalTyping,
+          BigDecimalScaleEnsurer.ensureBigDecimalScale(BigDecimal(1).bigDecimal)
+        ),
+        (
+          "1.toBigDecimalOrNull()",
+          bigDecimalTyping,
+          BigDecimalScaleEnsurer.ensureBigDecimalScale(BigDecimal(1).bigDecimal)
+        ),
         ("1.1.toBigDecimalOrNull()", bigDecimalTyping, convertedDoubleToBigDecimal),
-        ("'1'.toBigDecimalOrNull()", bigDecimalTyping, BigDecimal(1).bigDecimal),
+        (
+          "'1'.toBigDecimalOrNull()",
+          bigDecimalTyping,
+          BigDecimalScaleEnsurer.ensureBigDecimalScale(BigDecimal(1).bigDecimal)
+        ),
         ("'a'.toBigDecimalOrNull()", bigDecimalTyping, null),
-        ("#unknownLong.value.toBigDecimal()", bigDecimalTyping, BigDecimal(11).bigDecimal),
-        ("#unknownLongString.value.toBigDecimal()", bigDecimalTyping, BigDecimal(11).bigDecimal),
+        (
+          "#unknownLong.value.toBigDecimal()",
+          bigDecimalTyping,
+          BigDecimalScaleEnsurer.ensureBigDecimalScale(BigDecimal(11).bigDecimal)
+        ),
+        (
+          "#unknownLongString.value.toBigDecimal()",
+          bigDecimalTyping,
+          BigDecimalScaleEnsurer.ensureBigDecimalScale(BigDecimal(11).bigDecimal)
+        ),
         ("#unknownDouble.value.toBigDecimal()", bigDecimalTyping, convertedDoubleToBigDecimal),
-        ("#unknownDoubleString.value.toBigDecimal()", bigDecimalTyping, BigDecimal(1.1).bigDecimal),
+        (
+          "#unknownDoubleString.value.toBigDecimal()",
+          bigDecimalTyping,
+          BigDecimalScaleEnsurer.ensureBigDecimalScale(BigDecimal(1.1).bigDecimal)
+        ),
         ("#unknownBoolean.value.toBigDecimalOrNull()", bigDecimalTyping, null),
         ("'true'.toBoolean()", booleanTyping, true),
         ("#unknownInteger.value.toBooleanOrNull()", booleanTyping, null),

utils/math-utils/src/test/scala/pl/touk/nussknacker/engine/util/MathUtilsSpec.scala

@@ -2,6 +2,7 @@ package pl.touk.nussknacker.engine.util
 
 import org.scalatest.funsuite.AnyFunSuite
 import org.scalatest.matchers.should.Matchers
+import pl.touk.nussknacker.springframework.util.BigDecimalScaleEnsurer
 
 class MathUtilsSpec extends AnyFunSuite with Matchers {
 
@@ -23,7 +24,9 @@ class MathUtilsSpec extends AnyFunSuite with Matchers {
 
     val minForIntAndBigDecimal = MathUtils.min(1, java.math.BigDecimal.valueOf(2))
     minForIntAndBigDecimal.getClass shouldEqual classOf[java.math.BigDecimal]
-    minForIntAndBigDecimal shouldEqual java.math.BigDecimal.valueOf(1)
+    BigDecimal(minForIntAndBigDecimal.asInstanceOf[java.math.BigDecimal]) shouldEqual BigDecimal(
+      java.math.BigDecimal.valueOf(1)
+    ) +- BigDecimal("0.000001")
   }
 
   test("max") {
@@ -49,11 +52,18 @@ class MathUtilsSpec extends AnyFunSuite with Matchers {
 
     val sumForIntAndBigDecimal = MathUtils.sum(1, java.math.BigDecimal.valueOf(2))
     sumForIntAndBigDecimal.getClass shouldEqual classOf[java.math.BigDecimal]
-    sumForIntAndBigDecimal shouldEqual java.math.BigDecimal.valueOf(3)
+    BigDecimal(sumForIntAndBigDecimal.asInstanceOf[java.math.BigDecimal]) shouldEqual BigDecimal(
+      java.math.BigDecimal.valueOf(3)
+    ) +- BigDecimal("0.00001")
 
     val sumForBytes = MathUtils.sum(1.byteValue(), 2.byteValue())
     sumForBytes.getClass shouldEqual classOf[java.lang.Integer]
     sumForBytes shouldEqual 3
   }
 
+  test("max should use big decimal conversion which sets default scale") {
+    val result = MathUtils.max(1, new java.math.BigDecimal(0))
+    result.asInstanceOf[java.math.BigDecimal].scale() shouldBe BigDecimalScaleEnsurer.DEFAULT_BIG_DECIMAL_SCALE
+  }
+
 }

utils/utils/src/main/java/org/springframework/util/NumberUtils.java

@@ -16,9 +16,10 @@
 
 package org.springframework.util;
 
+import pl.touk.nussknacker.springframework.util.BigDecimalScaleEnsurer;
+
 import java.math.BigDecimal;
 import java.math.BigInteger;
-import java.math.MathContext;
 import java.math.RoundingMode;
 import java.text.DecimalFormat;
 import java.text.NumberFormat;
@@ -142,7 +143,7 @@ else if (BigDecimal.class == targetClass) {
 			} else if (number instanceof Float) {
 				return (T) bigDecimal.setScale(FLOAT_SCALE, RoundingMode.UNNECESSARY);
 			} else {
-				return (T) bigDecimal;
+				return (T) BigDecimalScaleEnsurer.ensureBigDecimalScale(bigDecimal);
 			}
 		}
 		else {

utils/utils/src/main/java/pl/touk/nussknacker/springframework/util/BigDecimalScaleEnsurer.scala

@@ -0,0 +1,32 @@
+package pl.touk.nussknacker.springframework.util
+
+import java.math.RoundingMode;
+
+/*
+In spel division of two big decimals uses divide operator org.springframework.expression.spel.ast.OpDivide.
+In this operator if one of the divided numbers is BigDecimal then second number is also converted to BigDecimal using org.springframework.util.NumberUtils class
+and resulting BigDecimal will have scale (that is number of stored digits to the right of decimal point) equal to max of the scales of two divided numbers.
+The behaviour of org.springframework.util.NumberUtils is to convert integers to BigDecimals in such a way, that they have scale equal to 0.
+This may lead to unexpected behaviour. For instance
+"(1).toBigDecimal / 2"
+would evaluate to BigDecimal("0") (we use org.springframework.util.NumberUtils to convert 1 to BigDecimal).
+This is not an acceptable result because
+"(1).toDouble / 2"
+will always evaluate to "0.5" and user would expect higher computation precision from BigDecimal.
+
+To avoid such issues we use this class to make sure that BigDecimals which are created always have scale of at least DEFAULT_BIG_DECIMAL_SCALE.
+BigDecimals can be created in org.springframework.util.NumberUtils conversion, or in our .toBigDecimal spel extension.
+
+There is the risk that big decimals enter process in other ways (for instance from jdbc controllers or from scenario input),
+and they may have small scales. This may again lead to unexpected
+behaviour when using division operator. This issue can be solved by using our own version of OpDivide class, but for now we decided
+not to do it.
+*/
+object BigDecimalScaleEnsurer {
+  // visible for testing
+  val DEFAULT_BIG_DECIMAL_SCALE = 18
+
+  def ensureBigDecimalScale(value: java.math.BigDecimal): java.math.BigDecimal = {
+    value.setScale(Math.max(value.scale(), BigDecimalScaleEnsurer.DEFAULT_BIG_DECIMAL_SCALE), RoundingMode.UNNECESSARY)
+  }
+}