Common: BigDecimal instead of Float for numeric EnrichedEvent fields (c…

…lose #654)
snowplow · Jul 21, 2022 · 665806a · 665806a
1 parent 4463419
commit 665806a
Show file tree

Hide file tree

Showing 15 changed files with 140 additions and 102 deletions.
diff --git a/...ich/common/fs2/blackbox/enrichments/CurrencyConversionEnrichmentTransactionItemSpec.scala b/...ich/common/fs2/blackbox/enrichments/CurrencyConversionEnrichmentTransactionItemSpec.scala
@@ -47,7 +47,7 @@ class CurrencyConversionEnrichmentTransactionItemSpec extends Specification with
         "ti_sku" -> "PBZ1001",
         "ti_quantity" -> "2",
         "ti_category" -> "APPAREL",
-        "ti_price" -> "2000.0",
+        "ti_price" -> "2000",
         "ti_price_base" -> "2240.45",
         "ti_name" -> "Blue t-shirt",
         "collector_tstamp" -> "2019-07-01 19:23:03.000"

diff --git a/.../enrich/common/fs2/blackbox/enrichments/CurrencyConversionEnrichmentTransactionSpec.scala b/.../enrich/common/fs2/blackbox/enrichments/CurrencyConversionEnrichmentTransactionSpec.scala
@@ -22,7 +22,7 @@ import cats.syntax.option._
 
 import com.snowplowanalytics.snowplow.enrich.common.fs2.blackbox.BlackBoxTesting
 
-class CurrencyConversionEnrichmentTransactionpec extends Specification with CatsIO {
+class CurrencyConversionEnrichmentTransactionSpec extends Specification with CatsIO {
 
   args(skipAll = !sys.env.contains("OER_KEY"))
 
@@ -44,12 +44,12 @@ class CurrencyConversionEnrichmentTransactionpec extends Specification with Cats
         "base_currency" -> "EUR",
         "tr_currency" -> "USD",
         "tr_affiliation" -> "pb",
-        "tr_total" -> "8000.0",
+        "tr_total" -> "8000",
         "tr_total_base" -> "7087.49",
         "tr_tax" -> "200.0",
         "tr_tax_base" -> "177.19",
-        "tr_shipping" -> "50.0",
-        "tr_shipping_base" -> "44.3",
+        "tr_shipping" -> "50",
+        "tr_shipping_base" -> "44.30",
         "tr_orderid" -> "order-123",
         "tr_state" -> "England",
         "txn_id" -> "28288",

diff --git a/.../com/snowplowanalytics/snowplow/enrich/common/fs2/blackbox/misc/TransactionItemSpec.scala b/.../com/snowplowanalytics/snowplow/enrich/common/fs2/blackbox/misc/TransactionItemSpec.scala
@@ -40,7 +40,7 @@ class TransactionItemSpec extends Specification with CatsIO {
         "ti_quantity" -> "2",
         "ti_currency" -> "",
         "ti_category" -> "APPAREL",
-        "ti_price" -> "2000.0",
+        "ti_price" -> "2000",
         "ti_price_base" -> ""
       )
       BlackBoxTesting.runTest(input, expected)

diff --git a/...cala/com/snowplowanalytics/snowplow/enrich/common/fs2/blackbox/misc/TransactionSpec.scala b/...cala/com/snowplowanalytics/snowplow/enrich/common/fs2/blackbox/misc/TransactionSpec.scala
@@ -38,11 +38,11 @@ class TransactionSpec extends Specification with CatsIO {
         "event_version" -> "1-0-0",
         "event" -> "transaction",
         "tr_affiliation" -> "pb",
-        "tr_total" -> "8000.0",
+        "tr_total" -> "8000",
         "tr_total_base" -> "",
-        "tr_tax" -> "200.0",
+        "tr_tax" -> "200",
         "tr_tax_base" -> "",
-        "tr_shipping" -> "50.0",
+        "tr_shipping" -> "50",
         "tr_shipping_base" -> "",
         "tr_orderid" -> "order-123",
         "tr_state" -> "England",

diff --git a/...in/scala/com.snowplowanalytics.snowplow.enrich/common/enrichments/EnrichmentManager.scala b/...in/scala/com.snowplowanalytics.snowplow.enrich/common/enrichments/EnrichmentManager.scala
@@ -484,40 +484,36 @@ object EnrichmentManager {
         currencyConversion match {
           case Some(currency) =>
             event.base_currency = currency.baseCurrency.getCode
-            // Note that jFloatToDouble is applied to either-valid-or-null event POJO
+            // Note that jBigDecimalToDouble is applied to either-valid-or-null event POJO
             // properties, so we don't expect any of these four vals to be a Failure
-            val trTax = CU.jFloatToDouble("tr_tx", event.tr_tax).toValidatedNel
-            val tiPrice = CU.jFloatToDouble("ti_pr", event.ti_price).toValidatedNel
-            val trTotal = CU.jFloatToDouble("tr_tt", event.tr_total).toValidatedNel
-            val trShipping = CU.jFloatToDouble("tr_sh", event.tr_shipping).toValidatedNel
-            (for {
-              convertedCu <- EitherT(
-                               (trTotal, trTax, trShipping, tiPrice)
-                                 .mapN {
-                                   currency.convertCurrencies(
-                                     Option(event.tr_currency),
-                                     _,
-                                     _,
-                                     _,
-                                     Option(event.ti_currency),
-                                     _,
-                                     timestamp
-                                   )
-                                 }
-                                 .toEither
-                                 .sequence
-                                 .map(_.flatMap(_.toEither))
-                             )
-              trTotalBase <- EitherT.fromEither[F](CU.doubleToJFloat("tr_total_base ", convertedCu._1).leftMap(e => NonEmptyList.one(e)))
-              _ = trTotalBase.map(t => event.tr_total_base = t)
-              trTaxBase <- EitherT.fromEither[F](CU.doubleToJFloat("tr_tax_base ", convertedCu._2).leftMap(e => NonEmptyList.one(e)))
-              _ = trTaxBase.map(t => event.tr_tax_base = t)
-              trShippingBase <-
-                EitherT.fromEither[F](CU.doubleToJFloat("tr_shipping_base ", convertedCu._3).leftMap(e => NonEmptyList.one(e)))
-              _ = trShippingBase.map(t => event.tr_shipping_base = t)
-              tiPriceBase <- EitherT.fromEither[F](CU.doubleToJFloat("ti_price_base ", convertedCu._4).leftMap(e => NonEmptyList.one(e)))
-              _ = tiPriceBase.map(t => event.ti_price_base = t)
-            } yield List.empty[SelfDescribingData[Json]]).value
+            val trTax = CU.jBigDecimalToDouble("tr_tx", event.tr_tax).toValidatedNel
+            val tiPrice = CU.jBigDecimalToDouble("ti_pr", event.ti_price).toValidatedNel
+            val trTotal = CU.jBigDecimalToDouble("tr_tt", event.tr_total).toValidatedNel
+            val trShipping = CU.jBigDecimalToDouble("tr_sh", event.tr_shipping).toValidatedNel
+            EitherT(
+              (trTotal, trTax, trShipping, tiPrice)
+                .mapN {
+                  currency.convertCurrencies(
+                    Option(event.tr_currency),
+                    _,
+                    _,
+                    _,
+                    Option(event.ti_currency),
+                    _,
+                    timestamp
+                  )
+                }
+                .toEither
+                .sequence
+                .map(_.flatMap(_.toEither))
+            ).map {
+              case (trTotalBase, trTaxBase, trShippingBase, tiPriceBase) =>
+                trTotalBase.foreach(v => event.tr_total_base = v)
+                trTaxBase.foreach(v => event.tr_tax_base = v)
+                trShippingBase.foreach(v => event.tr_shipping_base = v)
+                tiPriceBase.foreach(v => event.ti_price_base = v)
+                List.empty[SelfDescribingData[Json]]
+            }.value
           case None => Monad[F].pure(Nil.asRight)
         }
     }

diff --git a/...n/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/enrichments/Transform.scala b/...n/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/enrichments/Transform.scala
@@ -94,21 +94,21 @@ object Transform {
       ("ev_ac", (ME.toTsvSafe, "se_action")), // LEGACY tracker var. Leave for backwards compat
       ("ev_la", (ME.toTsvSafe, "se_label")), // LEGACY tracker var. Leave for backwards compat
       ("ev_pr", (ME.toTsvSafe, "se_property")), // LEGACY tracker var. Leave for backwards compat
-      ("ev_va", (CU.stringToJFloat2, "se_value")), // LEGACY tracker var. Leave for backwards compat
+      ("ev_va", (CU.stringToJBigDecimal2, "se_value")), // LEGACY tracker var. Leave for backwards compat
       ("se_ca", (ME.toTsvSafe, "se_category")),
       ("se_ac", (ME.toTsvSafe, "se_action")),
       ("se_la", (ME.toTsvSafe, "se_label")),
       ("se_pr", (ME.toTsvSafe, "se_property")),
-      ("se_va", (CU.stringToJFloat2, "se_value")),
+      ("se_va", (CU.stringToJBigDecimal2, "se_value")),
       // Custom unstructured events
       ("ue_pr", (JU.extractUnencJson, "unstruct_event")),
       ("ue_px", (JU.extractBase64EncJson, "unstruct_event")),
       // Ecommerce transactions
       ("tr_id", (ME.toTsvSafe, "tr_orderid")),
       ("tr_af", (ME.toTsvSafe, "tr_affiliation")),
-      ("tr_tt", (CU.stringToJFloat2, "tr_total")),
-      ("tr_tx", (CU.stringToJFloat2, "tr_tax")),
-      ("tr_sh", (CU.stringToJFloat2, "tr_shipping")),
+      ("tr_tt", (CU.stringToJBigDecimal2, "tr_total")),
+      ("tr_tx", (CU.stringToJBigDecimal2, "tr_tax")),
+      ("tr_sh", (CU.stringToJBigDecimal2, "tr_shipping")),
       ("tr_ci", (ME.toTsvSafe, "tr_city")),
       ("tr_st", (ME.toTsvSafe, "tr_state")),
       ("tr_co", (ME.toTsvSafe, "tr_country")),
@@ -118,7 +118,7 @@ object Transform {
       ("ti_na", (ME.toTsvSafe, "ti_name")), // ERROR in Tracker Protocol
       ("ti_nm", (ME.toTsvSafe, "ti_name")),
       ("ti_ca", (ME.toTsvSafe, "ti_category")),
-      ("ti_pr", (CU.stringToJFloat2, "ti_price")),
+      ("ti_pr", (CU.stringToJBigDecimal2, "ti_price")),
       ("ti_qu", (CU.stringToJInteger2, "ti_quantity")),
       // Page pings
       ("pp_mix", (CU.stringToJInteger2, "pp_xoffset_min")),

diff --git a/...wanalytics.snowplow.enrich/common/enrichments/registry/CurrencyConversionEnrichment.scala b/...wanalytics.snowplow.enrich/common/enrichments/registry/CurrencyConversionEnrichment.scala
@@ -14,6 +14,7 @@ package com.snowplowanalytics.snowplow.enrich.common.enrichments.registry
 
 import java.time.ZonedDateTime
 import java.io.{PrintWriter, StringWriter}
+import java.math.BigDecimal
 
 import cats.Monad
 import cats.data.{EitherT, NonEmptyList, ValidatedNel}
@@ -117,7 +118,7 @@ final case class CurrencyConversionEnrichment[F[_]: Monad](
     initialCurrency: Option[Either[FailureDetails.EnrichmentFailure, CurrencyUnit]],
     value: Option[Double],
     tstamp: ZonedDateTime
-  ): F[Either[FailureDetails.EnrichmentFailure, Option[Double]]] =
+  ): F[Either[FailureDetails.EnrichmentFailure, Option[BigDecimal]]] =
     (initialCurrency, value) match {
       case (Some(ic), Some(v)) =>
         (for {
@@ -132,7 +133,7 @@ final case class CurrencyConversionEnrichment[F[_]: Monad](
                      _.bimap(
                        l => mkEnrichmentFailure(Right(l)),
                        r =>
-                         Either.catchNonFatal(r.getAmount().doubleValue) match {
+                         Either.catchNonFatal(r.getAmount()) match {
                            case Left(e) =>
                              Left(mkEnrichmentFailure(Left(e)))
                            case Right(a) =>
@@ -166,7 +167,7 @@ final case class CurrencyConversionEnrichment[F[_]: Monad](
     collectorTstamp: Option[DateTime]
   ): F[ValidatedNel[
     FailureDetails.EnrichmentFailure,
-    (Option[Double], Option[Double], Option[Double], Option[Double])
+    (Option[BigDecimal], Option[BigDecimal], Option[BigDecimal], Option[BigDecimal])
   ]] =
     collectorTstamp match {
       case Some(tstamp) =>

diff --git a/...la/com.snowplowanalytics.snowplow.enrich/common/enrichments/registry/sqlquery/Input.scala b/...la/com.snowplowanalytics.snowplow.enrich/common/enrichments/registry/sqlquery/Input.scala
@@ -183,6 +183,7 @@ object Input {
     "java.lang.Integer" -> IntPlaceholder,
     "java.lang.Byte" -> BytePlaceholder,
     "java.lang.Float" -> FloatPlaceholder,
+    "java.math.BigDecimal" -> BigDecimalPlaceholder,
     // Just in case
     "String" -> StringPlaceholder,
     "scala.Int" -> IntPlaceholder,
@@ -369,4 +370,11 @@ object Input {
     def getSetter(preparedStatement: PreparedStatement) =
       preparedStatement.setLong
   }
+
+  object BigDecimalPlaceholder extends StatementPlaceholder {
+    type PlaceholderType = BigDecimal
+    def getSetter(preparedStatement: PreparedStatement) = { (i, v) =>
+      preparedStatement.setDouble(i, v.doubleValue)
+    }
+  }
 }
diff --git a/...n/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/outputs/EnrichedEvent.scala b/...n/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/outputs/EnrichedEvent.scala
@@ -15,6 +15,7 @@ package com.snowplowanalytics.snowplow.enrich.common.outputs
 import java.lang.{Integer => JInteger}
 import java.lang.{Float => JFloat}
 import java.lang.{Byte => JByte}
+import java.math.{BigDecimal => JBigDecimal}
 import java.time.format.DateTimeFormatter
 
 import scala.beans.BeanProperty
@@ -126,17 +127,17 @@ class EnrichedEvent extends Serializable {
   @BeanProperty var se_action: String = _
   @BeanProperty var se_label: String = _
   @BeanProperty var se_property: String = _
-  @BeanProperty var se_value: JFloat = _
+  @BeanProperty var se_value: JBigDecimal = _
 
   // Unstructured Event
   @BeanProperty var unstruct_event: String = _
 
   // Ecommerce transaction (from querystring)
   @BeanProperty var tr_orderid: String = _
   @BeanProperty var tr_affiliation: String = _
-  @BeanProperty var tr_total: JFloat = _
-  @BeanProperty var tr_tax: JFloat = _
-  @BeanProperty var tr_shipping: JFloat = _
+  @BeanProperty var tr_total: JBigDecimal = _
+  @BeanProperty var tr_tax: JBigDecimal = _
+  @BeanProperty var tr_shipping: JBigDecimal = _
   @BeanProperty var tr_city: String = _
   @BeanProperty var tr_state: String = _
   @BeanProperty var tr_country: String = _
@@ -146,7 +147,7 @@ class EnrichedEvent extends Serializable {
   @BeanProperty var ti_sku: String = _
   @BeanProperty var ti_name: String = _
   @BeanProperty var ti_category: String = _
-  @BeanProperty var ti_price: JFloat = _
+  @BeanProperty var ti_price: JBigDecimal = _
   @BeanProperty var ti_quantity: JInteger = _
 
   // Page Pings
@@ -203,11 +204,11 @@ class EnrichedEvent extends Serializable {
 
   // Currency
   @BeanProperty var tr_currency: String = _
-  @BeanProperty var tr_total_base: JFloat = _
-  @BeanProperty var tr_tax_base: JFloat = _
-  @BeanProperty var tr_shipping_base: JFloat = _
+  @BeanProperty var tr_total_base: JBigDecimal = _
+  @BeanProperty var tr_tax_base: JBigDecimal = _
+  @BeanProperty var tr_shipping_base: JBigDecimal = _
   @BeanProperty var ti_currency: String = _
-  @BeanProperty var ti_price_base: JFloat = _
+  @BeanProperty var ti_price_base: JBigDecimal = _
   @BeanProperty var base_currency: String = _
 
   // Geolocation
@@ -268,6 +269,7 @@ object EnrichedEvent {
   private def toKv(k: String, i: JInteger): Option[(String, Json)] = toKv(k, i, (jInt: JInteger) => Json.fromInt(jInt))
   private def toKv(k: String, f: JFloat): Option[(String, Json)] = toKv(k, f, (jFloat: JFloat) => Json.fromFloatOrNull(jFloat))
   private def toKv(k: String, b: JByte): Option[(String, Json)] = toKv(k, b, (jByte: JByte) => Json.fromBoolean(jByte != 0))
+  private def toKv(k: String, b: JBigDecimal): Option[(String, Json)] = toKv(k, b, (jNum: JBigDecimal) => Json.fromBigDecimal(jNum))
   private def toDateKv(k: String, s: String): Option[(String, Json)] =
     toKv(
       k,

diff --git a/...n/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/utils/ConversionUtils.scala b/...n/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/utils/ConversionUtils.scala
@@ -13,7 +13,7 @@
 package com.snowplowanalytics.snowplow.enrich.common
 package utils
 
-import java.lang.{Byte => JByte, Float => JFloat, Integer => JInteger}
+import java.lang.{Byte => JByte, Integer => JInteger}
 import java.lang.reflect.Field
 import java.math.{BigDecimal => JBigDecimal}
 import java.net.{InetAddress, URI, URLDecoder, URLEncoder}
@@ -345,21 +345,21 @@ object ConversionUtils {
         FailureDetails.EnrichmentFailure(None, f)
       }
 
-  val stringToJFloat: String => Either[String, JFloat] = str =>
+  val stringToJBigDecimal: String => Either[String, JBigDecimal] = str =>
     Option(str) match {
       case None =>
-        null.asInstanceOf[JFloat].asRight
+        null.asInstanceOf[JBigDecimal].asRight
       case Some(s) if s.toLowerCase == "null" =>
-        null.asInstanceOf[JFloat].asRight
+        null.asInstanceOf[JBigDecimal].asRight
       case Some(s) =>
         Either
-          .catchNonFatal(JFloat.valueOf(s))
-          .leftMap(e => s"cannot be converted to java.lang.Float. Error : ${e.getMessage}")
+          .catchNonFatal(new JBigDecimal(s))
+          .leftMap(e => s"cannot be converted to java.math.BigDecimal. Error : ${e.getMessage}")
     }
 
-  val stringToJFloat2: (String, String) => Either[FailureDetails.EnrichmentFailure, JFloat] =
+  val stringToJBigDecimal2: (String, String) => Either[FailureDetails.EnrichmentFailure, JBigDecimal] =
     (field, str) =>
-      stringToJFloat(str).leftMap { e =>
+      stringToJBigDecimal(str).leftMap { e =>
         val f = FailureDetails.EnrichmentFailureMessage.InputData(
           field,
           Option(str),
@@ -426,11 +426,11 @@ object ConversionUtils {
         )
       )
 
-  /** Convert a java Float a Double */
-  def jFloatToDouble(field: String, f: JFloat): Either[FailureDetails.EnrichmentFailure, Option[Double]] =
+  /** Convert a java BigDecimal a Double */
+  def jBigDecimalToDouble(field: String, f: JBigDecimal): Either[FailureDetails.EnrichmentFailure, Option[Double]] =
     Either
       .catchNonFatal {
-        Option(f).map(_.toDouble)
+        Option(f).map(_.doubleValue)
       }
       .leftMap(_ =>
         FailureDetails.EnrichmentFailure(
@@ -443,19 +443,19 @@ object ConversionUtils {
         )
       )
 
-  /** Convert a Double to a java Float */
-  def doubleToJFloat(field: String, d: Option[Double]): Either[FailureDetails.EnrichmentFailure, Option[JFloat]] =
+  /** Convert a Double to a java BigDecimal */
+  def doubleToJBigDecimal(field: String, d: Option[Double]): Either[FailureDetails.EnrichmentFailure, Option[JBigDecimal]] =
     Either
       .catchNonFatal {
-        d.map(dd => JFloat.valueOf(dd.toFloat))
+        d.map(dd => new JBigDecimal(dd))
       }
       .leftMap(_ =>
         FailureDetails.EnrichmentFailure(
           None,
           FailureDetails.EnrichmentFailureMessage.InputData(
             field,
             d.map(_.toString),
-            "cannot be converted to java Float"
+            "cannot be converted to java BigDecimal"
           )
         )
       )