From 875a7f644ca119ce6d82f71425f24eb75730867f Mon Sep 17 00:00:00 2001
From: brharrington <brharrington@gmail.com>
Date: Wed, 3 Apr 2024 07:30:43 -0500
Subject: [PATCH] lwcapi: fix status code for event without id (#1649)

If an event is sent to `/evaluate` without an id, respond
with a 400 error instead of 500.
---
 .../netflix/atlas/eval/model/LwcEvent.scala   |  2 ++
 .../atlas/lwcapi/EvaluateApiSuite.scala       | 22 +++++++++++++------
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/atlas-eval/src/main/scala/com/netflix/atlas/eval/model/LwcEvent.scala b/atlas-eval/src/main/scala/com/netflix/atlas/eval/model/LwcEvent.scala
index 134da8cdf..32c70ac0f 100644
--- a/atlas-eval/src/main/scala/com/netflix/atlas/eval/model/LwcEvent.scala
+++ b/atlas-eval/src/main/scala/com/netflix/atlas/eval/model/LwcEvent.scala
@@ -27,5 +27,7 @@ import com.netflix.atlas.json.JsonSupport
   *     Raw event payload.
   */
 case class LwcEvent(id: String, payload: JsonNode) extends JsonSupport {
+
+  require(id != null, "id cannot be null")
   val `type`: String = "event"
 }
diff --git a/atlas-lwcapi/src/test/scala/com/netflix/atlas/lwcapi/EvaluateApiSuite.scala b/atlas-lwcapi/src/test/scala/com/netflix/atlas/lwcapi/EvaluateApiSuite.scala
index b87048f02..5f64c2d42 100644
--- a/atlas-lwcapi/src/test/scala/com/netflix/atlas/lwcapi/EvaluateApiSuite.scala
+++ b/atlas-lwcapi/src/test/scala/com/netflix/atlas/lwcapi/EvaluateApiSuite.scala
@@ -24,6 +24,7 @@ import com.netflix.atlas.eval.model.LwcEvent
 import com.netflix.atlas.json.Json
 import com.netflix.atlas.lwcapi.EvaluateApi.*
 import com.netflix.atlas.pekko.DiagnosticMessage
+import com.netflix.atlas.pekko.RequestHandler
 import com.netflix.atlas.pekko.testkit.MUnitRouteSuite
 import com.netflix.spectator.api.NoopRegistry
 
@@ -34,11 +35,11 @@ class EvaluateApiSuite extends MUnitRouteSuite {
   private implicit val routeTestTimeout: RouteTestTimeout = RouteTestTimeout(5.second)
 
   private val sm = new StreamSubscriptionManager(new NoopRegistry)
-  private val endpoint = new EvaluateApi(new NoopRegistry, sm)
+  private val routes = RequestHandler.standardOptions(new EvaluateApi(new NoopRegistry, sm).routes)
 
   test("post empty payload") {
     val json = EvaluateRequest(1234L, Nil, Nil).toJson
-    Post("/lwc/api/v1/evaluate", json) ~> endpoint.routes ~> check {
+    Post("/lwc/api/v1/evaluate", json) ~> routes ~> check {
       assertEquals(response.status, StatusCodes.OK)
     }
   }
@@ -46,7 +47,7 @@ class EvaluateApiSuite extends MUnitRouteSuite {
   test("post metrics") {
     val metrics = List(Item("abc", SortedTagMap("a" -> "1"), 42.0))
     val json = EvaluateRequest(1234L, metrics, Nil, Nil).toJson
-    Post("/lwc/api/v1/evaluate", json) ~> endpoint.routes ~> check {
+    Post("/lwc/api/v1/evaluate", json) ~> routes ~> check {
       assertEquals(response.status, StatusCodes.OK)
     }
   }
@@ -54,29 +55,36 @@ class EvaluateApiSuite extends MUnitRouteSuite {
   test("post events") {
     val events = List(LwcEvent("abc", Json.decode[JsonNode]("42.0")))
     val json = EvaluateRequest(1234L, Nil, events, Nil).toJson
-    Post("/lwc/api/v1/evaluate", json) ~> endpoint.routes ~> check {
+    Post("/lwc/api/v1/evaluate", json) ~> routes ~> check {
       assertEquals(response.status, StatusCodes.OK)
     }
   }
 
+  test("post events, missing event id") {
+    val json = """{"timestamp":1234,"events":[{"payload":42.0,"type":"event"}]}"""
+    Post("/lwc/api/v1/evaluate", json) ~> routes ~> check {
+      assertEquals(response.status, StatusCodes.BadRequest)
+    }
+  }
+
   test("post diagnostic message") {
     val msgs = List(LwcDiagnosticMessage("abc", DiagnosticMessage.error("bad expression")))
     val json = EvaluateRequest(1234L, Nil, Nil, msgs).toJson
-    Post("/lwc/api/v1/evaluate", json) ~> endpoint.routes ~> check {
+    Post("/lwc/api/v1/evaluate", json) ~> routes ~> check {
       assertEquals(response.status, StatusCodes.OK)
     }
   }
 
   test("post missing messages field") {
     val json = """{"timestamp":12345,"metrics":[]}"""
-    Post("/lwc/api/v1/evaluate", json) ~> endpoint.routes ~> check {
+    Post("/lwc/api/v1/evaluate", json) ~> routes ~> check {
       assertEquals(response.status, StatusCodes.OK)
     }
   }
 
   test("post missing metrics field") {
     val json = """{"timestamp":12345,"messages":[]}"""
-    Post("/lwc/api/v1/evaluate", json) ~> endpoint.routes ~> check {
+    Post("/lwc/api/v1/evaluate", json) ~> routes ~> check {
       assertEquals(response.status, StatusCodes.OK)
     }
   }