From 6fdc20b01a611f5d23d2ef81c685fb997e3ad19f 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/outputs/EnrichedEvent.scala | 20 ++++++++++--------- .../common/utils/ConversionUtils.scala | 14 ++++++------- .../outputs/EnrichedEventSpec.scala | 19 +++++++++--------- 4 files changed, 38 insertions(+), 33 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/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..94dff2c1d 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 @@ -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/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)