Requires url to be defined in LegacyCustomWebhookMessage for use acro…

…ss transport wire and only writes the full url instead of each individual part Signed-off-by: Drew Baugher <[email protected]>
opensearch-project · Aug 13, 2021 · c530c44 · c530c44
1 parent 8875a87
commit c530c44
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 36 deletions.
diff --git a/src/main/java/org/opensearch/commons/destination/message/LegacyCustomWebhookMessage.java b/src/main/java/org/opensearch/commons/destination/message/LegacyCustomWebhookMessage.java
@@ -49,7 +49,7 @@ public class LegacyCustomWebhookMessage extends LegacyBaseMessage {
     private final String method;
     private final int port;
     private String path;
-    private Map<String, String> queryParams;
+    private final Map<String, String> queryParams;
     private Map<String, String> headerParams;
 
     private LegacyCustomWebhookMessage(
@@ -107,16 +107,12 @@ public LegacyCustomWebhookMessage(StreamInput streamInput) throws IOException {
         super(streamInput);
         this.message = super.getMessageContent();
         this.url = streamInput.readOptionalString();
-        this.scheme = streamInput.readOptionalString();
-        this.host = streamInput.readOptionalString();
+        this.scheme = null;
+        this.host = null;
         this.method = streamInput.readOptionalString();
-        this.port = streamInput.readOptionalInt();
-        this.path = streamInput.readOptionalString();
-        if (streamInput.readBoolean()) {
-            @SuppressWarnings("unchecked")
-            Map<String, String> queryParams = (Map<String, String>) (Map) streamInput.readMap();
-            this.queryParams = queryParams;
-        }
+        this.port = -1;
+        this.path = null;
+        this.queryParams = null;
         if (streamInput.readBoolean()) {
             @SuppressWarnings("unchecked")
             Map<String, String> headerParams = (Map<String, String>) (Map) streamInput.readMap();
@@ -261,18 +257,13 @@ public String getMessage() {
     @Override
     public void writeTo(StreamOutput streamOutput) throws IOException {
         super.writeTo(streamOutput);
+        // Making LegacyCustomWebhookMessage streamable is purely to support the new pass through API from ISM -> Notification plugin
+        // and it only supports LegacyCustomWebhookMessage when the url is already constructed by ISM.
+        if (Strings.isNullOrEmpty(getUrl())) {
+            throw new IllegalStateException("Cannot use LegacyCustomWebhookMessage across transport wire without defining full url.");
+        }
         streamOutput.writeOptionalString(url);
-        streamOutput.writeOptionalString(scheme);
-        streamOutput.writeOptionalString(host);
         streamOutput.writeOptionalString(method);
-        streamOutput.writeOptionalInt(port);
-        streamOutput.writeOptionalString(path);
-        streamOutput.writeBoolean(queryParams != null);
-        if (queryParams != null) {
-            @SuppressWarnings("unchecked")
-            Map<String, Object> queryParams = (Map<String, Object>) (Map) this.queryParams;
-            streamOutput.writeMap(queryParams);
-        }
         streamOutput.writeBoolean(headerParams != null);
         if (headerParams != null) {
             @SuppressWarnings("unchecked")

diff --git a/src/test/java/org/opensearch/commons/destination/message/LegacyCustomWebhookMessageTest.java b/src/test/java/org/opensearch/commons/destination/message/LegacyCustomWebhookMessageTest.java
@@ -73,7 +73,7 @@ public void testRoundTrippingLegacyCustomWebhookMessageWithUrl() throws IOExcept
     }
 
     @Test
-    public void testRoundTrippingLegacyCustomWebhookMessageWithHost() throws IOException {
+    public void testRoundTrippingLegacyCustomWebhookMessageWithHostFails() throws IOException {
         Map<String, String> queryParams = new HashMap<String, String>();
         queryParams.put("token", "sometoken");
         Map<String, String> headers = new HashMap<String, String>();
@@ -89,21 +89,12 @@ public void testRoundTrippingLegacyCustomWebhookMessageWithHost() throws IOExcep
             .withScheme("https")
             .build();
         BytesStreamOutput out = new BytesStreamOutput();
-        message.writeTo(out);
-
-        StreamInput in = StreamInput.wrap(out.bytes().toBytesRef().bytes);
-        LegacyCustomWebhookMessage newMessage = new LegacyCustomWebhookMessage(in);
-
-        assertEquals(newMessage.destinationName, message.destinationName);
-        assertEquals(newMessage.getChannelType(), message.getChannelType());
-        assertEquals(newMessage.getMessageContent(), message.getMessageContent());
-        assertEquals(newMessage.getHost(), message.getHost());
-        assertEquals(newMessage.getMethod(), message.getMethod());
-        assertEquals(newMessage.getPath(), message.getPath());
-        assertEquals(newMessage.getQueryParams(), message.getQueryParams());
-        assertEquals(newMessage.getHeaderParams(), message.getHeaderParams());
-        assertEquals(newMessage.getPort(), message.getPort());
-        assertEquals(newMessage.getScheme(), message.getScheme());
+        try {
+            message.writeTo(out);
+            fail("Writing LegacyCustomWebhookMessage with host instead of url to stream output should fail");
+        } catch (IllegalStateException e) {
+            assertEquals("Cannot use LegacyCustomWebhookMessage across transport wire without defining full url.", e.getMessage());
+        }
     }
 
     @Test