From ee644bdc4044a6c1489b9b93035c32bec4b8eb36 Mon Sep 17 00:00:00 2001
From: Vlad Rozov <vrozov@users.noreply.github.com>
Date: Fri, 30 Jul 2021 11:38:14 -0700
Subject: [PATCH] Add unit test for RestActionListener. Validate that
 onFailure() sends response even when BytesRestResponse can not be constructed
 using passed exception. Follow up on #923. (#1024)

Signed-off-by: Vlad Rozov <vrozov@users.noreply.github.com>
---
 .../ExceptionSerializationTests.java          | 11 +++--
 .../rest/action/RestActionListenerTests.java  | 49 +++++++++++++++++++
 2 files changed, 55 insertions(+), 5 deletions(-)
 create mode 100644 server/src/test/java/org/opensearch/rest/action/RestActionListenerTests.java

diff --git a/server/src/test/java/org/opensearch/ExceptionSerializationTests.java b/server/src/test/java/org/opensearch/ExceptionSerializationTests.java
index c55ab234098..52fdc4223fc 100644
--- a/server/src/test/java/org/opensearch/ExceptionSerializationTests.java
+++ b/server/src/test/java/org/opensearch/ExceptionSerializationTests.java
@@ -150,10 +150,11 @@ public class ExceptionSerializationTests extends OpenSearchTestCase {
         final String path = "/org/opensearch";
         final Path startPath = PathUtils.get(OpenSearchException.class.getProtectionDomain().getCodeSource().getLocation().toURI())
                 .resolve("org").resolve("opensearch");
-        final Set<? extends Class<?>> ignore = Sets.newHashSet(
-                CancellableThreadsTests.CustomException.class,
-                org.opensearch.rest.BytesRestResponseTests.WithHeadersException.class,
-                AbstractClientHeadersTestCase.InternalException.class);
+        final Set<String> ignore = Sets.newHashSet(
+                CancellableThreadsTests.CustomException.class.getName(),
+                org.opensearch.rest.BytesRestResponseTests.WithHeadersException.class.getName(),
+                AbstractClientHeadersTestCase.InternalException.class.getName(),
+                "org.opensearch.rest.action.RestActionListenerTests$2");
         FileVisitor<Path> visitor = new FileVisitor<Path>() {
             private Path pkgPrefix = PathUtils.get(path).getParent();
 
@@ -181,7 +182,7 @@ public class ExceptionSerializationTests extends OpenSearchTestCase {
             }
 
             private void checkClass(Class<?> clazz) {
-                if (ignore.contains(clazz) || isAbstract(clazz.getModifiers()) || isInterface(clazz.getModifiers())) {
+                if (ignore.contains(clazz.getName()) || isAbstract(clazz.getModifiers()) || isInterface(clazz.getModifiers())) {
                     return;
                 }
                 if (isEsException(clazz) == false) {
diff --git a/server/src/test/java/org/opensearch/rest/action/RestActionListenerTests.java b/server/src/test/java/org/opensearch/rest/action/RestActionListenerTests.java
new file mode 100644
index 00000000000..562428baa11
--- /dev/null
+++ b/server/src/test/java/org/opensearch/rest/action/RestActionListenerTests.java
@@ -0,0 +1,49 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+/*
+ * Modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
+ */
+
+package org.opensearch.rest.action;
+
+import org.opensearch.OpenSearchException;
+import org.opensearch.rest.RestStatus;
+import org.opensearch.test.OpenSearchTestCase;
+import org.opensearch.test.rest.FakeRestChannel;
+import org.opensearch.test.rest.FakeRestRequest;
+
+public class RestActionListenerTests extends OpenSearchTestCase {
+
+    /**
+     * Validate that response is sent even when BytesRestResponse can not be constructed from the exception
+     * see https://github.com/opensearch-project/OpenSearch/pull/923
+     */
+    public void testExceptionInByteRestResponse() throws Exception {
+        FakeRestChannel channel = new FakeRestChannel(new FakeRestRequest(), true, 1);
+        RestActionListener listener = new RestActionListener(channel) {
+            @Override protected void processResponse(Object o) {
+                fail("call to processResponse is not expected");
+            }
+        };
+
+        // TODO: it will be better to mock BytesRestResponse() and throw exception from it's ctor, but the current version of
+        //  mockito does not support mocking static methods and ctor.
+        listener.onFailure(new OpenSearchException("mock status() call") {
+            @Override
+            public RestStatus status() {
+                throw new OpenSearchException("call to status failed");
+            }
+        });
+
+        assertEquals(0, channel.responses().get());
+        assertEquals(1, channel.errors().get());
+        assertEquals(RestStatus.INTERNAL_SERVER_ERROR, channel.capturedResponse().status());
+    }
+}