Address Ian's feedback

snowplow · Aug 10, 2023 · 902927e · 902927e
1 parent 73a23b1
commit 902927e
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 12 deletions.
diff --git a/...c/main/scala/com.snowplowanalytics.snowplow.collectors.scalastream/CollectorService.scala b/...c/main/scala/com.snowplowanalytics.snowplow.collectors.scalastream/CollectorService.scala
@@ -2,13 +2,15 @@ package com.snowplowanalytics.snowplow.collectors.scalastream
 
 import java.util.UUID
 
+import org.apache.commons.codec.binary.Base64
+
 import scala.concurrent.duration._
 import scala.collection.JavaConverters._
 
 import cats.effect.{Clock, Sync}
 import cats.implicits._
 
-import fs2.{Stream, text}
+import fs2.Stream
 
 import org.http4s._
 import org.http4s.headers._
@@ -40,16 +42,19 @@ trait Service[F[_]] {
   def determinePath(vendor: String, version: String): String
 }
 
+object CollectorService {
+  // Contains an invisible pixel to return for `/i` requests.
+  val pixel = Base64.decodeBase64("R0lGODlhAQABAPAAAP///wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==")
+}
+
 class CollectorService[F[_]: Sync](
   config: CollectorConfig,
   sinks: CollectorSinks[F],
   appName: String,
   appVersion: String
 ) extends Service[F] {
 
-  // Contains an invisible pixel to return for `/i` requests.
-  val pixelBase64 = "R0lGODlhAQABAPAAAP///wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw=="
-  val pixel       = Stream.emit[F, String](pixelBase64).through(text.base64.decode)
+  val pixelStream = Stream.iterable[F, Byte](CollectorService.pixel)
 
   // TODO: Add sink type as well
   private val collector = s"$appName-$appVersion"
@@ -152,7 +157,7 @@ class CollectorService[F[_]: Sync](
       case true =>
         Response[F](
           headers = headers.put(`Content-Type`(MediaType.image.gif)),
-          body    = pixel
+          body    = pixelStream
         )
       // See https://github.com/snowplow/snowplow-javascript-tracker/issues/482
       case false =>

diff --git a/...st/scala/com.snowplowanalytics.snowplow.collectors.scalastream/CollectorServiceSpec.scala b/...st/scala/com.snowplowanalytics.snowplow.collectors.scalastream/CollectorServiceSpec.scala
@@ -2,7 +2,6 @@ package com.snowplowanalytics.snowplow.collectors.scalastream
 
 import scala.concurrent.duration._
 import scala.collection.JavaConverters._
-import org.apache.commons.codec.binary.Base64
 import cats.effect.{Clock, IO}
 import cats.effect.unsafe.implicits.global
 import cats.data.NonEmptyList
@@ -168,6 +167,28 @@ class CollectorServiceSpec extends Specification {
           "image/gif"
         ).asJava
       }
+
+      "return necessary cache control headers and respond with pixel when pixelExpected is true" in {
+        val r = service
+          .cookie(
+            queryString = Some("nuid=12"),
+            body = IO.pure(Some("b")),
+            path = "p",
+            cookie = None,
+            userAgent = None,
+            refererUri = None,
+            hostname = IO.pure(Some("h")),
+            ip = None,
+            request = Request[IO](),
+            pixelExpected = true,
+            doNotTrack = false,
+            contentType = None,
+            spAnonymous = Some("*")
+          )
+          .unsafeRunSync()
+        r.headers.get[`Cache-Control`] shouldEqual Some(`Cache-Control`(CacheDirective.`no-cache`(), CacheDirective.`no-store`, CacheDirective.`must-revalidate`))
+        r.body.compile.toList.unsafeRunSync().toArray shouldEqual CollectorService.pixel
+      }
     }
 
     "buildEvent" in {
@@ -242,16 +263,11 @@ class CollectorServiceSpec extends Specification {
     }
 
     "buildHttpResponse" in {
-      // base64 decode function from apache commons library used in here
-      // because that is how pixel is created in the akka-collector and
-      // we want to test that base64 decode function from fs2 is doing
-      // same thing.
-      val pixel = Base64.decodeBase64("R0lGODlhAQABAPAAAP///wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==")
       "send back a gif if pixelExpected is true" in {
         val res = service.buildHttpResponse(hs, pixelExpected = true)
         res.status shouldEqual Status.Ok
         res.headers shouldEqual hs.put(`Content-Type`(MediaType.image.gif))
-        res.body.compile.toList.unsafeRunSync().toArray shouldEqual pixel
+        res.body.compile.toList.unsafeRunSync().toArray shouldEqual CollectorService.pixel
       }
       "send back ok otherwise" in {
         val res = service.buildHttpResponse(hs, pixelExpected = false)