From 94c225d0daead305394e83807626e7ee42f97ddf Mon Sep 17 00:00:00 2001 From: Hailong Cui Date: Tue, 4 Apr 2023 11:33:55 +0800 Subject: [PATCH] convert empty httpEntity to {} to avoid DeliveryStatus initialization exception (#648) * convert empty httpEntity to {} to avoid DeliveryStatus initialization exception Signed-off-by: Hailong Cui * add integTest case for webhook empty response Signed-off-by: Hailong Cui --------- Signed-off-by: Hailong Cui --- .../core/client/DestinationHttpClient.kt | 4 +- .../destinations/ChimeDestinationTests.kt | 2 +- .../CustomWebhookDestinationTests.kt | 15 +++- .../destinations/SlackDestinationTests.kt | 2 +- notifications/notifications/build.gradle | 6 ++ .../send/SendTestMessageWithMockServerIT.kt | 81 +++++++++++++++++++ 6 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 notifications/notifications/src/test/kotlin/org/opensearch/integtest/send/SendTestMessageWithMockServerIT.kt diff --git a/notifications/core/src/main/kotlin/org/opensearch/notifications/core/client/DestinationHttpClient.kt b/notifications/core/src/main/kotlin/org/opensearch/notifications/core/client/DestinationHttpClient.kt index de9751cd..9493d3d4 100644 --- a/notifications/core/src/main/kotlin/org/opensearch/notifications/core/client/DestinationHttpClient.kt +++ b/notifications/core/src/main/kotlin/org/opensearch/notifications/core/client/DestinationHttpClient.kt @@ -141,7 +141,9 @@ class DestinationHttpClient { @Throws(IOException::class) fun getResponseString(response: CloseableHttpResponse): String { val entity: HttpEntity = response.entity ?: return "{}" - return EntityUtils.toString(entity) + val responseString = EntityUtils.toString(entity) + // DeliveryStatus need statusText must not be empty, convert empty response to {} + return if (responseString.isNullOrEmpty()) "{}" else responseString } @Throws(IOException::class) diff --git a/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/ChimeDestinationTests.kt b/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/ChimeDestinationTests.kt index 607ee06e..209d7388 100644 --- a/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/ChimeDestinationTests.kt +++ b/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/ChimeDestinationTests.kt @@ -88,7 +88,7 @@ internal class ChimeDestinationTests { @Test fun `test chime message empty entity response`() { val mockHttpClient: CloseableHttpClient = EasyMock.createMock(CloseableHttpClient::class.java) - val expectedWebhookResponse = DestinationMessageResponse(RestStatus.OK.status, "") + val expectedWebhookResponse = DestinationMessageResponse(RestStatus.OK.status, "{}") val httpResponse = mockk() EasyMock.expect(mockHttpClient.execute(EasyMock.anyObject(HttpPost::class.java))).andReturn(httpResponse) diff --git a/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/CustomWebhookDestinationTests.kt b/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/CustomWebhookDestinationTests.kt index fc344252..c4db2ae0 100644 --- a/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/CustomWebhookDestinationTests.kt +++ b/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/CustomWebhookDestinationTests.kt @@ -104,7 +104,7 @@ internal class CustomWebhookDestinationTests { @MethodSource("methodToHttpRequestType") fun `test custom webhook message empty entity response`(method: String, expectedHttpClass: Class) { val mockHttpClient: CloseableHttpClient = EasyMock.createMock(CloseableHttpClient::class.java) - val expectedWebhookResponse = DestinationMessageResponse(RestStatus.OK.status, "") + val expectedWebhookResponse = DestinationMessageResponse(RestStatus.OK.status, "{}") val httpResponse = mockk() EasyMock.expect(mockHttpClient.execute(EasyMock.anyObject(HttpPost::class.java))).andReturn(httpResponse) @@ -218,4 +218,17 @@ internal class CustomWebhookDestinationTests { val actualRequestBody = httpClient.buildRequestBody(destination, message) assertEquals(messageText, actualRequestBody) } + + @Test + fun `test get response string`() { + val httpClient = DestinationHttpClient() + val response = mockk() + every { response.entity } returns null + var responseString = httpClient.getResponseString(response) + assertEquals(responseString, "{}") + + every { response.entity } returns StringEntity("") + responseString = httpClient.getResponseString(response) + assertEquals(responseString, "{}") + } } diff --git a/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/SlackDestinationTests.kt b/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/SlackDestinationTests.kt index 7bc642bf..e2d0216d 100644 --- a/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/SlackDestinationTests.kt +++ b/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/SlackDestinationTests.kt @@ -90,7 +90,7 @@ internal class SlackDestinationTests { @Test fun `test Slack message empty entity response`() { val mockHttpClient: CloseableHttpClient = EasyMock.createMock(CloseableHttpClient::class.java) - val expectedWebhookResponse = DestinationMessageResponse(RestStatus.OK.status, "") + val expectedWebhookResponse = DestinationMessageResponse(RestStatus.OK.status, "{}") val httpResponse = mockk() EasyMock.expect(mockHttpClient.execute(EasyMock.anyObject(HttpPost::class.java))).andReturn(httpResponse) diff --git a/notifications/notifications/build.gradle b/notifications/notifications/build.gradle index de3a4d4d..b3076350 100644 --- a/notifications/notifications/build.gradle +++ b/notifications/notifications/build.gradle @@ -230,6 +230,12 @@ integTest { excludeTestsMatching "org.opensearch.integtest.bwc.*IT" } } + + if (usingRemoteCluster) { + filter { + excludeTestsMatching "org.opensearch.integtest.send.SendTestMessageWithMockServerIT" + } + } } project.getTasks().getByName("bundlePlugin").dependsOn(findProject(":${rootProject.name}-core").tasks.getByPath(":${rootProject.name}-core:bundlePlugin")) diff --git a/notifications/notifications/src/test/kotlin/org/opensearch/integtest/send/SendTestMessageWithMockServerIT.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/send/SendTestMessageWithMockServerIT.kt new file mode 100644 index 00000000..4cbb03bd --- /dev/null +++ b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/send/SendTestMessageWithMockServerIT.kt @@ -0,0 +1,81 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.integtest.send + +import com.google.gson.JsonArray +import com.google.gson.JsonObject +import com.sun.net.httpserver.HttpServer +import org.junit.AfterClass +import org.junit.Assert +import org.junit.BeforeClass +import org.opensearch.integtest.PluginRestTestCase +import org.opensearch.notifications.NotificationPlugin.Companion.PLUGIN_BASE_URI +import org.opensearch.rest.RestRequest +import org.opensearch.rest.RestStatus +import java.net.InetAddress +import java.net.InetSocketAddress + +internal class SendTestMessageWithMockServerIT : PluginRestTestCase() { + + fun `test webhook return with empty entity`() { + val url = "http://${server.address.hostString}:${server.address.port}/webhook" + logger.info("webhook url = {}", url) + // Create webhook notification config + val createRequestJsonString = """ + { + "config":{ + "name":"this is a sample config name", + "description":"this is a sample config description", + "config_type":"webhook", + "is_enabled":true, + "webhook":{ + "url":"$url", + "header_params": { + "Content-type": "text/plain" + } + } + } + } + """.trimIndent() + val configId = createConfigWithRequestJsonString(createRequestJsonString) + Assert.assertNotNull(configId) + Thread.sleep(1000) + + // send test message + val sendResponse = executeRequest( + RestRequest.Method.POST.name, "$PLUGIN_BASE_URI/feature/test/$configId", "", RestStatus.OK.status + ) + + logger.info(sendResponse) + + // verify failure response is with message + val deliveryStatus = (sendResponse.get("status_list") as JsonArray).get(0).asJsonObject + .get("delivery_status") as JsonObject + Assert.assertEquals(deliveryStatus.get("status_code").asString, "200") + Assert.assertEquals(deliveryStatus.get("status_text").asString, "{}") + } + + companion object { + private lateinit var server: HttpServer + + @JvmStatic + @BeforeClass + fun setupWebhook() { + server = HttpServer.create(InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0) + + server.createContext("/webhook") { + it.sendResponseHeaders(200, -1) + } + server.start() + } + + @JvmStatic + @AfterClass + fun stopMockServer() { + server.stop(1) + } + } +}