Skip to content

Commit

Permalink
Common: BigDecimal instead of Float for numeric EnrichedEvent fields (c…
Browse files Browse the repository at this point in the history
…lose #654)
  • Loading branch information
istreeter committed Jul 21, 2022
1 parent 4463419 commit bc957f8
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")),
Expand All @@ -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")),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = _
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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(
Expand All @@ -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"
)
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -97,21 +98,21 @@ 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)
testField(_.ti_orderid = "ti_orderid", _.ti_orderid)
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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit bc957f8

Please sign in to comment.