Findings API Enhancements changes and integ tests fix (#1464)

* solution to fix integ tests Signed-off-by: Riya Saxena <[email protected]> * fix flaky DocumentMonitor Runner tests Signed-off-by: Riya Saxena <[email protected]> * fix findings API enhancemnts Signed-off-by: Riya Saxena <[email protected]> --------- Signed-off-by: Riya Saxena <[email protected]>
opensearch-project · Mar 13, 2024 · ba84d04 · ba84d04
1 parent a93605f
commit ba84d04
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 76 deletions.
diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestGetFindingsAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestGetFindingsAction.kt
@@ -45,8 +45,6 @@ class RestGetFindingsAction : BaseRestHandler() {
         val size = request.paramAsInt("size", 20)
         val startIndex = request.paramAsInt("startIndex", 0)
         val searchString = request.param("searchString", "")
-        val severity: String? = request.param("severity", "ALL")
-        val detectionType: String? = request.param("detectionType", "rules")
 
         val table = Table(
             sortOrder,
@@ -59,9 +57,7 @@ class RestGetFindingsAction : BaseRestHandler() {
 
         val getFindingsSearchRequest = GetFindingsRequest(
             findingID,
-            table,
-            severity,
-            detectionType
+            table
         )
         return RestChannelConsumer {
                 channel ->

diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetFindingsAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetFindingsAction.kt
@@ -40,6 +40,7 @@ import org.opensearch.commons.alerting.model.FindingWithDocs
 import org.opensearch.commons.utils.recreateObject
 import org.opensearch.core.action.ActionListener
 import org.opensearch.core.common.Strings
+import org.opensearch.core.common.io.stream.NamedWriteableRegistry
 import org.opensearch.core.xcontent.NamedXContentRegistry
 import org.opensearch.core.xcontent.XContentParser
 import org.opensearch.core.xcontent.XContentParserUtils
@@ -62,7 +63,8 @@ class TransportGetFindingsSearchAction @Inject constructor(
     clusterService: ClusterService,
     actionFilters: ActionFilters,
     val settings: Settings,
-    val xContentRegistry: NamedXContentRegistry
+    val xContentRegistry: NamedXContentRegistry,
+    val namedWriteableRegistry: NamedWriteableRegistry
 ) : HandledTransportAction<ActionRequest, GetFindingsResponse> (
     AlertingActions.GET_FINDINGS_ACTION_NAME, transportService, actionFilters, ::GetFindingsRequest
 ),
@@ -80,11 +82,8 @@ class TransportGetFindingsSearchAction @Inject constructor(
         actionListener: ActionListener<GetFindingsResponse>
     ) {
         val getFindingsRequest = request as? GetFindingsRequest
-            ?: recreateObject(request) { GetFindingsRequest(it) }
+            ?: recreateObject(request, namedWriteableRegistry) { GetFindingsRequest(it) }
         val tableProp = getFindingsRequest.table
-        val severity = getFindingsRequest.severity
-        val detectionType = getFindingsRequest.detectionType
-        val searchString = tableProp.searchString
 
         val sortBuilder = SortBuilders
             .fieldSort(tableProp.sortString)
@@ -101,79 +100,16 @@ class TransportGetFindingsSearchAction @Inject constructor(
             .seqNoAndPrimaryTerm(true)
             .version(true)
 
-        val queryBuilder = QueryBuilders.boolQuery()
+        val queryBuilder = getFindingsRequest.boolQueryBuilder ?: QueryBuilders.boolQuery()
 
         if (!getFindingsRequest.findingId.isNullOrBlank())
             queryBuilder.filter(QueryBuilders.termQuery("_id", getFindingsRequest.findingId))
-
-        if (!getFindingsRequest.findingIds.isNullOrEmpty()) {
-            queryBuilder.filter(QueryBuilders.termsQuery("id", getFindingsRequest.findingIds))
-        }
-
         if (getFindingsRequest.monitorId != null) {
             queryBuilder.filter(QueryBuilders.termQuery("monitor_id", getFindingsRequest.monitorId))
         } else if (getFindingsRequest.monitorIds.isNullOrEmpty() == false) {
             queryBuilder.filter(QueryBuilders.termsQuery("monitor_id", getFindingsRequest.monitorIds))
         }
 
-        if (getFindingsRequest.startTime != null && getFindingsRequest.endTime != null) {
-            val startTime = getFindingsRequest.startTime!!.toEpochMilli()
-            val endTime = getFindingsRequest.endTime!!.toEpochMilli()
-            val timeRangeQuery = QueryBuilders.rangeQuery("timestamp")
-                .from(startTime) // Greater than or equal to start time
-                .to(endTime) // Less than or equal to end time
-            queryBuilder.filter(timeRangeQuery)
-        }
-
-        if (!detectionType.isNullOrBlank()) {
-            val nestedQueryBuilder = QueryBuilders.nestedQuery(
-                "queries",
-                when {
-                    detectionType.equals("threat", ignoreCase = true) -> {
-                        QueryBuilders.boolQuery().filter(
-                            QueryBuilders.prefixQuery("queries.id", "threat_intel_")
-                        )
-                    }
-                    else -> {
-                        QueryBuilders.boolQuery().mustNot(
-                            QueryBuilders.prefixQuery("queries.id", "threat_intel_")
-                        )
-                    }
-                },
-                ScoreMode.None
-            )
-
-            // Add the nestedQueryBuilder to the main queryBuilder
-            queryBuilder.must(nestedQueryBuilder)
-        }
-
-        if (!searchString.isNullOrBlank()) {
-            queryBuilder
-                .should(QueryBuilders.matchQuery("index", searchString))
-                .should(
-                    QueryBuilders.nestedQuery(
-                        "queries",
-                        QueryBuilders.matchQuery("queries.tags", searchString),
-                        ScoreMode.None
-                    )
-                )
-                .should(QueryBuilders.regexpQuery("monitor_name", searchString + ".*"))
-                .minimumShouldMatch(1)
-        }
-
-        if (!severity.isNullOrBlank()) {
-            queryBuilder
-                .must(
-                    QueryBuilders.nestedQuery(
-                        "queries",
-                        QueryBuilders.boolQuery().should(
-                            QueryBuilders.matchQuery("queries.tags", severity)
-                        ),
-                        ScoreMode.None
-                    )
-                )
-        }
-
         if (!tableProp.searchString.isNullOrBlank()) {
             queryBuilder
                 .should(

diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/DocumentMonitorRunnerIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/DocumentMonitorRunnerIT.kt
@@ -2119,8 +2119,10 @@ class DocumentMonitorRunnerIT : AlertingRestTestCase() {
 
         val findings = searchFindings(monitor)
         assertEquals("Findings saved for test monitor", 2, findings.size)
-        assertTrue("Findings saved for test monitor", findings[0].relatedDocIds.contains("1"))
-        assertTrue("Findings saved for test monitor", findings[1].relatedDocIds.contains("5"))
+        val findings0 = findings[0].relatedDocIds.contains("1") || findings[0].relatedDocIds.contains("5")
+        val findings1 = findings[1].relatedDocIds.contains("5") || findings[1].relatedDocIds.contains("1")
+        assertTrue("Findings saved for test monitor", findings0)
+        assertTrue("Findings saved for test monitor", findings1)
     }
 
     fun `test document-level monitor when index alias contain docs that do match a NOT EQUALS query and EXISTS query`() {