update the response code to 404 when deleting a memory that is not al…

…lowed to access (opensearch-project#2212) Signed-off-by: Xun Zhang <[email protected]>
rbhavna · Mar 19, 2024 · af539dc · af539dc
1 parent b037032
commit af539dc
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 4 deletions.
diff --git a/...y/src/main/java/org/opensearch/ml/memory/index/OpenSearchConversationalMemoryHandler.java b/...y/src/main/java/org/opensearch/ml/memory/index/OpenSearchConversationalMemoryHandler.java
@@ -21,6 +21,7 @@
 import java.util.List;
 import java.util.Map;
 
+import org.opensearch.OpenSearchStatusException;
 import org.opensearch.action.StepListener;
 import org.opensearch.action.search.SearchRequest;
 import org.opensearch.action.search.SearchResponse;
@@ -32,6 +33,7 @@
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.action.ActionFuture;
 import org.opensearch.core.action.ActionListener;
+import org.opensearch.core.rest.RestStatus;
 import org.opensearch.ml.common.conversation.ConversationMeta;
 import org.opensearch.ml.common.conversation.ConversationalIndexConstants;
 import org.opensearch.ml.common.conversation.Interaction;
@@ -325,7 +327,14 @@ public void deleteConversation(String conversationId, ActionListener<Boolean> li
                 }, listener::onFailure);
 
             } else {
-                listener.onResponse(false);
+                log.error("No access to delete the memory for " + conversationId);
+                listener
+                    .onFailure(
+                        new OpenSearchStatusException(
+                            "Resources not found. Failed to delete the memory for " + conversationId,
+                            RestStatus.NOT_FOUND
+                        )
+                    );
             }
         }, listener::onFailure);
     }

diff --git a/.../org/opensearch/ml/memory/action/conversation/DeleteConversationTransportActionTests.java b/.../org/opensearch/ml/memory/action/conversation/DeleteConversationTransportActionTests.java
@@ -31,13 +31,15 @@
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.Mockito;
+import org.opensearch.OpenSearchStatusException;
 import org.opensearch.action.support.ActionFilters;
 import org.opensearch.client.Client;
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.settings.ClusterSettings;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.util.concurrent.ThreadContext;
 import org.opensearch.core.action.ActionListener;
+import org.opensearch.core.rest.RestStatus;
 import org.opensearch.core.xcontent.NamedXContentRegistry;
 import org.opensearch.ml.common.conversation.ConversationalIndexConstants;
 import org.opensearch.ml.memory.MemoryTestUtil;
@@ -118,6 +120,21 @@ public void testDeleteConversation() {
         assert (argCaptor.getValue().wasSuccessful());
     }
 
+    public void testDeleteConversation_NoAccess() {
+        doAnswer(invocation -> {
+            ActionListener<Boolean> al = invocation.getArgument(1);
+            al
+                .onFailure(
+                    new OpenSearchStatusException("Resources not found. Failed to delete the memory for " + "test", RestStatus.NOT_FOUND)
+                );
+            return null;
+        }).when(cmHandler).deleteConversation(any(), any());
+        action.doExecute(null, request, actionListener);
+        ArgumentCaptor<OpenSearchStatusException> argCaptor = ArgumentCaptor.forClass(OpenSearchStatusException.class);
+        verify(actionListener).onFailure(argCaptor.capture());
+        assert (argCaptor.getValue().getMessage().equals("Resources not found. Failed to delete the memory for test"));
+    }
+
     public void testDeleteFails_thenFail() {
         doAnswer(invocation -> {
             ActionListener<Boolean> al = invocation.getArgument(1);

diff --git a/.../test/java/org/opensearch/ml/memory/index/OpenSearchConversationalMemoryHandlerTests.java b/.../test/java/org/opensearch/ml/memory/index/OpenSearchConversationalMemoryHandlerTests.java
@@ -33,6 +33,7 @@
 import org.junit.Before;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
+import org.opensearch.OpenSearchStatusException;
 import org.opensearch.action.DocWriteResponse;
 import org.opensearch.action.search.SearchRequest;
 import org.opensearch.action.search.SearchResponse;
@@ -201,9 +202,9 @@ public void testDelete_NoAccess() {
         @SuppressWarnings("unchecked")
         ActionListener<Boolean> deleteListener = mock(ActionListener.class);
         cmHandler.deleteConversation("cid", deleteListener);
-        ArgumentCaptor<Boolean> argCaptor = ArgumentCaptor.forClass(Boolean.class);
-        verify(deleteListener, times(1)).onResponse(argCaptor.capture());
-        assert (!argCaptor.getValue());
+        ArgumentCaptor<OpenSearchStatusException> argCaptor = ArgumentCaptor.forClass(OpenSearchStatusException.class);
+        verify(deleteListener).onFailure(argCaptor.capture());
+        assert (argCaptor.getValue().getMessage().equals("Resources not found. Failed to delete the memory for cid"));
     }
 
     public void testDelete_ConversationMetaDeleteFalse_ThenFalse() {