From a08756b7cc51f44bb8fa6e0e304f45e19220fb20 Mon Sep 17 00:00:00 2001 From: Dennis Toepker Date: Fri, 7 Jun 2024 14:48:46 -0700 Subject: [PATCH] fixed lint issues and other restructuring Signed-off-by: Dennis Toepker --- .../TransportDeleteAlertingCommentAction.kt | 2 +- .../TransportIndexAlertingCommentAction.kt | 131 ++++++++++-------- .../TransportSearchAlertingCommentAction.kt | 2 +- 3 files changed, 72 insertions(+), 63 deletions(-) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportDeleteAlertingCommentAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportDeleteAlertingCommentAction.kt index 9396a0b9e..2b9a5913a 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportDeleteAlertingCommentAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportDeleteAlertingCommentAction.kt @@ -159,7 +159,7 @@ class TransportDeleteAlertingCommentAction @Inject constructor( if (searchResponse.hits.totalHits.value == 0L) { actionListener.onFailure( AlertingException.wrap( - OpenSearchStatusException("Comment with $commentId is not found", RestStatus.NOT_FOUND) + OpenSearchStatusException("Comment with ID $commentId is not found", RestStatus.NOT_FOUND) ) ) } diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexAlertingCommentAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexAlertingCommentAction.kt index 76dc59571..2a75c739e 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexAlertingCommentAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexAlertingCommentAction.kt @@ -19,6 +19,7 @@ import org.opensearch.action.search.SearchRequest import org.opensearch.action.search.SearchResponse import org.opensearch.action.support.ActionFilters import org.opensearch.action.support.HandledTransportAction +import org.opensearch.action.support.WriteRequest import org.opensearch.alerting.alerts.AlertIndices import org.opensearch.alerting.comments.CommentsIndices import org.opensearch.alerting.comments.CommentsIndices.Companion.COMMENTS_HISTORY_WRITE_INDEX @@ -58,7 +59,6 @@ import org.opensearch.tasks.Task import org.opensearch.transport.TransportService import java.lang.IllegalArgumentException import java.time.Instant -import org.opensearch.action.support.WriteRequest private val log = LogManager.getLogger(TransportIndexMonitorAction::class.java) private val scope: CoroutineScope = CoroutineScope(Dispatchers.IO) @@ -153,38 +153,8 @@ constructor( } private suspend fun indexComment() { - // need to validate the existence of the Alert that user is trying to add Comment to. - // Also need to check if user has permissions to add a Comment to the passed in Alert. To do this, - // we retrieve the Alert to get its associated monitor user, and use that to - // check if they have permissions to the Monitor that generated the Alert - val queryBuilder = QueryBuilders.boolQuery().must(QueryBuilders.termsQuery("_id", listOf(request.entityId))) - val searchSourceBuilder = - SearchSourceBuilder() - .version(true) - .seqNoAndPrimaryTerm(true) - .query(queryBuilder) - - // search all alerts, since user might want to create a comment - // on a completed alert - val searchRequest = - SearchRequest() - .indices(AlertIndices.ALL_ALERT_INDEX_PATTERN) - .source(searchSourceBuilder) - - val searchResponse: SearchResponse = client.suspendUntil { search(searchRequest, it) } - val alerts = searchResponse.hits.map { hit -> - val xcp = XContentHelper.createParser( - NamedXContentRegistry.EMPTY, - LoggingDeprecationHandler.INSTANCE, - hit.sourceRef, - XContentType.JSON - ) - XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, xcp.nextToken(), xcp) - val alert = Alert.parse(xcp, hit.id, hit.version) - alert - } - - if (alerts.isEmpty()) { + val alert = getAlert() + if (alert == null) { actionListener.onFailure( AlertingException.wrap( OpenSearchStatusException("Alert with ID ${request.entityId} is not found", RestStatus.NOT_FOUND), @@ -193,8 +163,6 @@ constructor( return } - val alert = alerts[0] // there should only be 1 Alert that matched the request alert ID - val numCommentsOnThisAlert = CommentsUtils.getCommentIDsByAlertIDs(client, listOf(alert.id)).size if (numCommentsOnThisAlert >= maxCommentsPerAlert) { actionListener.onFailure( @@ -243,34 +211,16 @@ constructor( } private suspend fun updateComment() { - val getRequest = GetRequest(COMMENTS_HISTORY_WRITE_INDEX, request.commentId) - try { - val getResponse: GetResponse = client.suspendUntil { client.get(getRequest, it) } - if (!getResponse.isExists) { - actionListener.onFailure( - AlertingException.wrap( - OpenSearchStatusException("Comment with ${request.commentId} is not found", RestStatus.NOT_FOUND), - ), - ) - return - } - val xcp = - XContentHelper.createParser( - xContentRegistry, - LoggingDeprecationHandler.INSTANCE, - getResponse.sourceAsBytesRef, - XContentType.JSON, - ) - xcp.nextToken() - val comment = Comment.parse(xcp, getResponse.id) - log.info("comment to be updated: $comment") - onGetCommentResponse(comment) - } catch (t: Exception) { - actionListener.onFailure(AlertingException.wrap(t)) + val currentComment = getComment() + if (currentComment == null) { + actionListener.onFailure( + AlertingException.wrap( + OpenSearchStatusException("Comment with ID ${request.commentId} is not found", RestStatus.NOT_FOUND), + ), + ) + return } - } - private suspend fun onGetCommentResponse(currentComment: Comment) { // check that the user has permissions to edit the comment. user can edit comment if // - user is Admin // - user is the author of the comment @@ -329,6 +279,65 @@ constructor( } } + private suspend fun getAlert(): Alert? { + // need to validate the existence of the Alert that user is trying to add Comment to. + // Also need to check if user has permissions to add a Comment to the passed in Alert. To do this, + // we retrieve the Alert to get its associated monitor user, and use that to + // check if they have permissions to the Monitor that generated the Alert + val queryBuilder = QueryBuilders.boolQuery().must(QueryBuilders.termsQuery("_id", listOf(request.entityId))) + val searchSourceBuilder = + SearchSourceBuilder() + .version(true) + .seqNoAndPrimaryTerm(true) + .query(queryBuilder) + + // search all alerts, since user might want to create a comment + // on a completed alert + val searchRequest = + SearchRequest() + .indices(AlertIndices.ALL_ALERT_INDEX_PATTERN) + .source(searchSourceBuilder) + + val searchResponse: SearchResponse = client.suspendUntil { search(searchRequest, it) } + val alerts = searchResponse.hits.map { hit -> + val xcp = XContentHelper.createParser( + NamedXContentRegistry.EMPTY, + LoggingDeprecationHandler.INSTANCE, + hit.sourceRef, + XContentType.JSON + ) + XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, xcp.nextToken(), xcp) + val alert = Alert.parse(xcp, hit.id, hit.version) + alert + } + + if (alerts.isEmpty()) return null + + return alerts[0] // there should only be 1 Alert that matched the request alert ID + } + + private suspend fun getComment(): Comment? { + val getRequest = GetRequest(COMMENTS_HISTORY_WRITE_INDEX, request.commentId) + try { + val getResponse: GetResponse = client.suspendUntil { client.get(getRequest, it) } + if (!getResponse.isExists) return null + val xcp = + XContentHelper.createParser( + xContentRegistry, + LoggingDeprecationHandler.INSTANCE, + getResponse.sourceAsBytesRef, + XContentType.JSON, + ) + xcp.nextToken() + val comment = Comment.parse(xcp, getResponse.id) + log.info("comment to be updated: $comment") + return comment + } catch (t: Exception) { + actionListener.onFailure(AlertingException.wrap(t)) + return null + } + } + private fun checkShardsFailure(response: IndexResponse): String? { val failureReasons = StringBuilder() if (response.shardInfo.failed > 0) { diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchAlertingCommentAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchAlertingCommentAction.kt index 60770b874..525aa25a7 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchAlertingCommentAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchAlertingCommentAction.kt @@ -163,7 +163,7 @@ class TransportSearchAlertingCommentAction @Inject constructor( val searchRequest = SearchRequest() .source(searchSourceBuilder) .indices(ALL_ALERT_INDEX_PATTERN) - // .preference(Preference.PRIMARY_FIRST.type()) // expensive, be careful + // .preference(Preference.PRIMARY_FIRST.type()) // expensive, be careful val searchResponse: SearchResponse = client.suspendUntil { search(searchRequest, it) } val alertIDs = searchResponse.hits.map { hit ->