From bc957f845f2c6310f3071fcf6f3577b9d77a8508 Mon Sep 17 00:00:00 2001 From: Ian Streeter Date: Thu, 21 Jul 2022 15:25:43 +0100 Subject: [PATCH] Common: BigDecimal instead of Float for numeric EnrichedEvent fields (close #654) --- .../enrichments/EnrichmentManager.scala | 18 ++++++----- .../common/enrichments/Transform.scala | 12 ++++---- .../common/outputs/EnrichedEvent.scala | 20 +++++++------ .../common/utils/ConversionUtils.scala | 30 +++++++++---------- .../enrichments/EnrichmentManagerSpec.scala | 2 +- .../outputs/EnrichedEventSpec.scala | 19 ++++++------ 6 files changed, 53 insertions(+), 48 deletions(-) diff --git a/modules/common/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/enrichments/EnrichmentManager.scala b/modules/common/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/enrichments/EnrichmentManager.scala index b3ed81f3c..4b34d4ba4 100644 --- a/modules/common/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/enrichments/EnrichmentManager.scala +++ b/modules/common/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/enrichments/EnrichmentManager.scala @@ -486,10 +486,10 @@ object EnrichmentManager { event.base_currency = currency.baseCurrency.getCode // Note that jFloatToDouble 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 + 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 (for { convertedCu <- EitherT( (trTotal, trTax, trShipping, tiPrice) @@ -508,14 +508,16 @@ object EnrichmentManager { .sequence .map(_.flatMap(_.toEither)) ) - trTotalBase <- EitherT.fromEither[F](CU.doubleToJFloat("tr_total_base ", convertedCu._1).leftMap(e => NonEmptyList.one(e))) + trTotalBase <- + EitherT.fromEither[F](CU.doubleToJBigDecimal("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 <- EitherT.fromEither[F](CU.doubleToJBigDecimal("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))) + EitherT.fromEither[F](CU.doubleToJBigDecimal("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 <- + EitherT.fromEither[F](CU.doubleToJBigDecimal("ti_price_base ", convertedCu._4).leftMap(e => NonEmptyList.one(e))) _ = tiPriceBase.map(t => event.ti_price_base = t) } yield List.empty[SelfDescribingData[Json]]).value case None => Monad[F].pure(Nil.asRight) diff --git a/modules/common/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/enrichments/Transform.scala b/modules/common/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/enrichments/Transform.scala index 3c1596519..b4be9006b 100644 --- a/modules/common/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/enrichments/Transform.scala +++ b/modules/common/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/modules/common/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/outputs/EnrichedEvent.scala b/modules/common/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/outputs/EnrichedEvent.scala index aa4db0c56..5d0cb8c49 100644 --- a/modules/common/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/outputs/EnrichedEvent.scala +++ b/modules/common/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,7 +127,7 @@ 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 = _ @@ -134,9 +135,9 @@ class EnrichedEvent extends Serializable { // 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/modules/common/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/utils/ConversionUtils.scala b/modules/common/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/utils/ConversionUtils.scala index cf3ebf3ef..3970f2764 100644 --- a/modules/common/src/main/scala/com.snowplowanalytics.snowplow.enrich/common/utils/ConversionUtils.scala +++ b/modules/common/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,11 +443,11 @@ 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( @@ -455,7 +455,7 @@ object ConversionUtils { FailureDetails.EnrichmentFailureMessage.InputData( field, d.map(_.toString), - "cannot be converted to java Float" + "cannot be converted to java BigDecimal" ) ) ) diff --git a/modules/common/src/test/scala/com.snowplowanalytics.snowplow.enrich.common/enrichments/EnrichmentManagerSpec.scala b/modules/common/src/test/scala/com.snowplowanalytics.snowplow.enrich.common/enrichments/EnrichmentManagerSpec.scala index 0289949d8..53350deb8 100644 --- a/modules/common/src/test/scala/com.snowplowanalytics.snowplow.enrich.common/enrichments/EnrichmentManagerSpec.scala +++ b/modules/common/src/test/scala/com.snowplowanalytics.snowplow.enrich.common/enrichments/EnrichmentManagerSpec.scala @@ -627,7 +627,7 @@ class EnrichmentManagerSpec extends Specification with EitherMatchers { } "emit an EnrichedEvent for valid float fields" >> { - val floats = List("42", "42.5", "null") + val floats = List("42", "42.5", "null") // TODO: bigger numbers val fields = List("ev_va", "se_va", "tr_tt", "tr_tx", "tr_sh", "ti_pr") floats diff --git a/modules/common/src/test/scala/com.snowplowanalytics.snowplow.enrich.common/outputs/EnrichedEventSpec.scala b/modules/common/src/test/scala/com.snowplowanalytics.snowplow.enrich.common/outputs/EnrichedEventSpec.scala index 004321499..a96018424 100644 --- a/modules/common/src/test/scala/com.snowplowanalytics.snowplow.enrich.common/outputs/EnrichedEventSpec.scala +++ b/modules/common/src/test/scala/com.snowplowanalytics.snowplow.enrich.common/outputs/EnrichedEventSpec.scala @@ -18,6 +18,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 com.snowplowanalytics.snowplow.badrows.Payload.PartiallyEnrichedEvent import org.specs2.mutable.Specification @@ -97,13 +98,13 @@ class EnrichedEventSpec extends Specification { testField(_.se_action = "se_action", _.se_action) testField(_.se_label = "se_label", _.se_label) testField(_.se_property = "se_property", _.se_property) - testField(_.se_value = JFloat.valueOf("0.0"), _.se_value) + testField(_.se_value = new JBigDecimal("0.0"), _.se_value) testField(_.unstruct_event = "unstruct_event", _.unstruct_event) testField(_.tr_orderid = "tr_orderid", _.tr_orderid) testField(_.tr_affiliation = "tr_affiliation", _.tr_affiliation) - testField(_.tr_total = JFloat.valueOf("0.0"), _.tr_total) - testField(_.tr_tax = JFloat.valueOf("0.0"), _.tr_tax) - testField(_.tr_shipping = JFloat.valueOf("0.0"), _.tr_shipping) + testField(_.tr_total = new JBigDecimal("0.0"), _.tr_total) + testField(_.tr_tax = new JBigDecimal("0.0"), _.tr_tax) + testField(_.tr_shipping = new JBigDecimal("0.0"), _.tr_shipping) testField(_.tr_city = "tr_city", _.tr_city) testField(_.tr_state = "tr_state", _.tr_state) testField(_.tr_country = "tr_country", _.tr_country) @@ -111,7 +112,7 @@ class EnrichedEventSpec extends Specification { testField(_.ti_sku = "ti_sku", _.ti_sku) testField(_.ti_name = "ti_name", _.ti_name) testField(_.ti_category = "ti_category", _.ti_category) - testField(_.ti_price = JFloat.valueOf("0.0"), _.ti_price) + testField(_.ti_price = new JBigDecimal("0.0"), _.ti_price) testField(_.ti_quantity = JInteger.valueOf(0), _.ti_quantity) testField(_.pp_xoffset_min = JInteger.valueOf(0), _.pp_xoffset_min) testField(_.pp_xoffset_max = JInteger.valueOf(0), _.pp_xoffset_max) @@ -149,11 +150,11 @@ class EnrichedEventSpec extends Specification { testField(_.doc_width = JInteger.valueOf(0), _.doc_width) testField(_.doc_height = JInteger.valueOf(0), _.doc_height) testField(_.tr_currency = "tr_currency", _.tr_currency) - testField(_.tr_total_base = JFloat.valueOf("0.0"), _.tr_total_base) - testField(_.tr_tax_base = JFloat.valueOf("0.0"), _.tr_tax_base) - testField(_.tr_shipping_base = JFloat.valueOf("0.0"), _.tr_shipping_base) + testField(_.tr_total_base = new JBigDecimal("0.0"), _.tr_total_base) + testField(_.tr_tax_base = new JBigDecimal("0.0"), _.tr_tax_base) + testField(_.tr_shipping_base = new JBigDecimal("0.0"), _.tr_shipping_base) testField(_.ti_currency = "ti_currency", _.ti_currency) - testField(_.ti_price_base = JFloat.valueOf("0.0"), _.ti_price_base) + testField(_.ti_price_base = new JBigDecimal("0.0"), _.ti_price_base) testField(_.base_currency = "base_currency", _.base_currency) testField(_.geo_timezone = "geo_timezone", _.geo_timezone) testField(_.mkt_clickid = "mkt_clickid", _.mkt_clickid)