Common: skip the SQL enrichment on null placeholders (close #315)

snowplow · Sep 6, 2020 · e520911 · e520911
1 parent f937ba6
commit e520911
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 17 deletions.
diff --git a/...m.snowplowanalytics.snowplow.enrich/common/enrichments/registry/sqlquery/DbExecutor.scala b/...m.snowplowanalytics.snowplow.enrich/common/enrichments/registry/sqlquery/DbExecutor.scala
@@ -358,12 +358,12 @@ object DbExecutor {
     connectionRef: ConnectionRef[F],
     sql: String,
     placeholderMap: Input.PlaceholderMap
-  ): F[Either[String, Unit]] =
+  ): F[Either[String, Input.PlaceholderMap]] =
     getPlaceholderCount(rdbms, connectionRef, sql).map {
       case Right(placeholderCount) =>
         placeholderMap match {
-          case Some(intMap) if intMap.keys.size == placeholderCount => ().asRight
-          case _ => s"The placeholder map error. The map: $placeholderMap, where count is: $placeholderCount".asLeft
+          case Some(intMap) if intMap.keys.size == placeholderCount => placeholderMap.asRight
+          case _ => None.asRight
         }
       case Left(error) =>
         error.getMessage.asLeft

diff --git a/...owanalytics.snowplow.enrich/common/enrichments/registry/sqlquery/SqlQueryEnrichment.scala b/...owanalytics.snowplow.enrich/common/enrichments/registry/sqlquery/SqlQueryEnrichment.scala
@@ -134,12 +134,12 @@ final case class SqlQueryEnrichment[F[_]: Monad: DbExecutor](
     unstructEvent: Option[SelfDescribingData[Json]]
   ): F[ValidatedNel[FailureDetails.EnrichmentFailure, List[SelfDescribingData[Json]]]] = {
     val contexts = for {
-      map <- Input
-               .buildPlaceholderMap(inputs, event, derivedContexts, customContexts, unstructEvent)
-               .toEitherT[F]
-      _ <- EitherT(DbExecutor.allPlaceholdersFilled(db, connection, query.sql, map))
-             .leftMap(NonEmptyList.one)
-      result <- map match {
+      placeholders <- Input
+                        .buildPlaceholderMap(inputs, event, derivedContexts, customContexts, unstructEvent)
+                        .toEitherT[F]
+      verifiedPlaceholders <- EitherT(DbExecutor.allPlaceholdersFilled(db, connection, query.sql, placeholders))
+                                .leftMap(NonEmptyList.one)
+      result <- verifiedPlaceholders match {
                   case Some(m) =>
                     EitherT(get(m)).leftMap(NonEmptyList.one)
                   case None =>

diff --git a/...wplow.enrich.common/enrichments/registry/sqlquery/SqlQueryEnrichmentIntegrationTest.scala b/...wplow.enrich.common/enrichments/registry/sqlquery/SqlQueryEnrichmentIntegrationTest.scala
@@ -22,8 +22,6 @@ import com.snowplowanalytics.iglu.core.{SchemaKey, SchemaVer, SelfDescribingData
 import org.specs2.Specification
 
 import outputs.EnrichedEvent
-import com.snowplowanalytics.snowplow.badrows.FailureDetails.EnrichmentFailure
-import com.snowplowanalytics.snowplow.badrows.FailureDetails
 
 object SqlQueryEnrichmentIntegrationTest {
   def continuousIntegration: Boolean =
@@ -407,12 +405,7 @@ class SqlQueryEnrichmentIntegrationTest extends Specification {
     val config = SqlQueryEnrichment.parse(configuration, SCHEMA_KEY).map(_.enrichment)
     val context = config.toEither.flatMap(_.lookup(event, Nil, Nil, None).toEither)
 
-    context must beLeft.like {
-      case nel =>
-        nel.head.asInstanceOf[EnrichmentFailure].message must beEqualTo(
-          FailureDetails.EnrichmentFailureMessage.Simple("The placeholder map error. The map: Some(IntMap()), where count is: 1")
-        )
-    }
+    context must beRight(Nil)
   }
 
 }