fix: add additional tests for including query params and update accor…

…ding to comments on PR Signed-off-by: vikhy-aws <[email protected]>
opensearch-project · Feb 1, 2025 · 98c0f98 · 98c0f98
1 parent d712404
commit 98c0f98
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 8 deletions.
diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestToggleMonitorAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestToggleMonitorAction.kt
@@ -58,8 +58,8 @@ class RestToggleMonitorAction : BaseRestHandler() {
     @Throws(IOException::class)
     override fun prepareRequest(request: RestRequest, client: NodeClient): RestChannelConsumer {
         val monitorId = request.param("monitorID", Monitor.NO_ID)
-        if (request.method() == PUT && Monitor.NO_ID == monitorId) {
-            throw AlertingException.wrap(IllegalArgumentException("Missing monitor ID"))
+        if (Monitor.NO_ID == monitorId) {
+            throw AlertingException.wrap(IllegalArgumentException("Missing monitor ID."))
         }
 
         // Check if the request is being made to enable the monitor

diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportToggleMonitorAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportToggleMonitorAction.kt
@@ -62,15 +62,15 @@ class TransportToggleMonitorAction @Inject constructor(
                     try {
                         if (getMonitorResponse.monitor == null) {
                             actionListener.onFailure(
-                                OpenSearchStatusException("Monitor $monitorId not found", RestStatus.NOT_FOUND)
+                                OpenSearchStatusException("Monitor $monitorId not found.", RestStatus.NOT_FOUND)
                             )
                             return
                         }
 
                         if (getMonitorResponse.monitor!!.enabled == enabled) {
                             actionListener.onFailure(
                                 OpenSearchStatusException(
-                                    "Monitor $monitorId is already ${if (enabled) "enabled" else "disabled"}",
+                                    "Monitor $monitorId is already ${if (enabled) "enabled." else "disabled."}",
                                     RestStatus.BAD_REQUEST
                                 )
                             )

diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt
@@ -1365,6 +1365,38 @@ class MonitorRestApiIT : AlertingRestTestCase() {
         assertNull(enabledTime)
     }
 
+    fun `test enable monitor with query params`() {
+        val monitorId = createMonitor(randomQueryLevelMonitor(enabled = false, enabledTime = null), refresh = true).id
+        val response = client().makeRequest(
+            "PUT",
+            "${AlertingPlugin.MONITOR_BASE_URI}/$monitorId/_enable?pretty",
+            emptyMap(),
+            null
+        )
+        val jsonResponse = createParser(XContentType.JSON.xContent(), response.entity.content).map()
+        val monitor = jsonResponse["Monitor"] as Map<*, *>
+        val enabled = monitor["enabled"]
+        // Monitor should be enabled
+        assertEquals(true, enabled)
+    }
+
+    fun `test disable monitor with query params`() {
+        val monitorId = createMonitor(randomQueryLevelMonitor(enabled = true, enabledTime = Instant.now()), refresh = true).id
+        val response = client().makeRequest(
+            "PUT",
+            "${AlertingPlugin.MONITOR_BASE_URI}/$monitorId/_disable?pretty",
+            emptyMap(),
+            null
+        )
+        val jsonResponse = createParser(XContentType.JSON.xContent(), response.entity.content).map()
+        val monitor = jsonResponse["Monitor"] as Map<*, *>
+        val enabled = monitor["enabled"]
+        val enabledTime = monitor["enabledTime"]
+        // Monitor should be disabled and the enabledTime should be null
+        assertEquals(false, enabled)
+        assertNull(enabledTime)
+    }
+
     fun `test enable monitor when already enabled`() {
         val monitorId = createMonitor(randomQueryLevelMonitor(enabled = true, enabledTime = Instant.now()), refresh = true).id
         val exception = assertThrows(ResponseException::class.java) {
@@ -1382,11 +1414,11 @@ class MonitorRestApiIT : AlertingRestTestCase() {
                 "root_cause" to listOf(
                     mapOf(
                         "type" to "status_exception",
-                        "reason" to "Monitor $monitorId is already enabled"
+                        "reason" to "Monitor $monitorId is already enabled."
                     )
                 ),
                 "type" to "status_exception",
-                "reason" to "Monitor $monitorId is already enabled"
+                "reason" to "Monitor $monitorId is already enabled."
             ),
             "status" to 400
         )
@@ -1410,11 +1442,11 @@ class MonitorRestApiIT : AlertingRestTestCase() {
                 "root_cause" to listOf(
                     mapOf(
                         "type" to "status_exception",
-                        "reason" to "Monitor $monitorId is already disabled"
+                        "reason" to "Monitor $monitorId is already disabled."
                     )
                 ),
                 "type" to "status_exception",
-                "reason" to "Monitor $monitorId is already disabled"
+                "reason" to "Monitor $monitorId is already disabled."
             ),
             "status" to 400
         )