Skip to content

Commit 74a310e

Browse files
authored
fix ut (#4455)
Signed-off-by: Yaliang Wu <[email protected]>
1 parent 4fac96e commit 74a310e

File tree

8 files changed

+167
-104
lines changed

8 files changed

+167
-104
lines changed

plugin/src/main/java/org/opensearch/ml/action/memorycontainer/TransportDeleteMemoryContainerAction.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ protected void doExecute(Task task, ActionRequest request, ActionListener<Delete
117117
MemoryConfiguration configuration = container.getConfiguration();
118118
String indexPrefix = configuration.getIndexPrefix();
119119

120-
memoryContainerHelper.countContainersWithPrefix(indexPrefix, tenantId, ActionListener.wrap(count -> {
120+
memoryContainerHelper.countContainersWithPrefix(indexPrefix, ActionListener.wrap(count -> {
121121
if (count > 1) {
122122
// Multiple containers share this prefix - cannot delete indices
123123
String error = String

plugin/src/main/java/org/opensearch/ml/action/memorycontainer/memory/TransportSearchMemoriesAction.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,10 @@ private void searchMemories(
118118
memoryContainerHelper.addContainerIdFilter(input.getMemoryContainerId(), input.getSearchSourceBuilder());
119119

120120
// Add owner filter for non-admin users
121-
// if (!memoryContainerHelper.isAdminUser(user)) {
122-
// memoryContainerHelper.addOwnerIdFilter(user, input.getSearchSourceBuilder());
123-
// }
121+
// TODO: now we don't support owner id in remote store, check if it's necessary to add
122+
if (container.getConfiguration().getRemoteStore() == null && !memoryContainerHelper.isAdminUser(user)) {
123+
memoryContainerHelper.addOwnerIdFilter(user, input.getSearchSourceBuilder());
124+
}
124125

125126
SearchRequest searchRequest = new SearchRequest(indexName).source(input.getSearchSourceBuilder());
126127
// TODO: add search pipeline support in SearchRequest

plugin/src/main/java/org/opensearch/ml/helper/MemoryContainerHelper.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -722,10 +722,11 @@ public SearchSourceBuilder addUserBackendRolesFilter(User user, SearchSourceBuil
722722
}
723723

724724
public SearchSourceBuilder addOwnerIdFilter(User user, SearchSourceBuilder searchSourceBuilder) {
725-
BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder();
726-
if (user != null) {
727-
boolQueryBuilder.should(QueryBuilders.termsQuery(OWNER_ID_FIELD, user.getName()));
725+
if (user == null) {
726+
return searchSourceBuilder;
728727
}
728+
BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder();
729+
boolQueryBuilder.should(QueryBuilders.termsQuery(OWNER_ID_FIELD, user.getName()));
729730
return applyFilterToSearchSource(searchSourceBuilder, boolQueryBuilder);
730731
}
731732

@@ -799,10 +800,9 @@ public String getOwnerId(User user) {
799800
* Count memory containers with the specified index prefix
800801
*
801802
* @param indexPrefix the index prefix to search for
802-
* @param tenantId the tenant ID (optional)
803803
* @param listener action listener returning the count
804804
*/
805-
public void countContainersWithPrefix(String indexPrefix, String tenantId, ActionListener<Long> listener) {
805+
public void countContainersWithPrefix(String indexPrefix, ActionListener<Long> listener) {
806806
if (indexPrefix == null || indexPrefix.isBlank()) {
807807
listener.onResponse(0L);
808808
return;

plugin/src/test/java/org/opensearch/ml/action/memorycontainer/TransportDeleteMemoryContainerActionTests.java

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -393,10 +393,10 @@ public void testHandleDeleteResponse_WithDeleteAllMemories_Success() {
393393

394394
// Mock countContainersWithPrefix to return 1 (sole owner - safe to delete indices)
395395
doAnswer(invocation -> {
396-
ActionListener<Long> listener = invocation.getArgument(2);
396+
ActionListener<Long> listener = invocation.getArgument(1);
397397
listener.onResponse(1L);
398398
return null;
399-
}).when(memoryContainerHelper).countContainersWithPrefix(any(), any(), any());
399+
}).when(memoryContainerHelper).countContainersWithPrefix(any(), any());
400400

401401
// Mock successful deleteIndex operation
402402
doAnswer(invocation -> {
@@ -466,10 +466,10 @@ public void testHandleDeleteResponse_WithDeleteAllMemories_DeleteIndexFailure()
466466

467467
// Mock countContainersWithPrefix to return 1 (sole owner - safe to delete indices)
468468
doAnswer(invocation -> {
469-
ActionListener<Long> listener = invocation.getArgument(2);
469+
ActionListener<Long> listener = invocation.getArgument(1);
470470
listener.onResponse(1L);
471471
return null;
472-
}).when(memoryContainerHelper).countContainersWithPrefix(any(), any(), any());
472+
}).when(memoryContainerHelper).countContainersWithPrefix(any(), any());
473473

474474
// Mock failed deleteIndex operation
475475
doAnswer(invocation -> {
@@ -613,10 +613,10 @@ public void testDeleteMemoryContainer_WithSelectiveMemories_Success() {
613613

614614
// Mock countContainersWithPrefix to return 1 (sole owner - safe to delete indices)
615615
doAnswer(invocation -> {
616-
ActionListener<Long> listener = invocation.getArgument(2);
616+
ActionListener<Long> listener = invocation.getArgument(1);
617617
listener.onResponse(1L);
618618
return null;
619-
}).when(memoryContainerHelper).countContainersWithPrefix(any(), any(), any());
619+
}).when(memoryContainerHelper).countContainersWithPrefix(any(), any());
620620

621621
// Mock successful deleteIndex operation
622622
doAnswer(invocation -> {
@@ -684,10 +684,10 @@ public void testDeleteMemoryContainer_WithSelectiveMemories_UnknownType() {
684684

685685
// Mock countContainersWithPrefix to return 1 (sole owner - safe to delete indices)
686686
doAnswer(invocation -> {
687-
ActionListener<Long> listener = invocation.getArgument(2);
687+
ActionListener<Long> listener = invocation.getArgument(1);
688688
listener.onResponse(1L);
689689
return null;
690-
}).when(memoryContainerHelper).countContainersWithPrefix(any(), any(), any());
690+
}).when(memoryContainerHelper).countContainersWithPrefix(any(), any());
691691

692692
// Mock successful deleteIndex operation
693693
doAnswer(invocation -> {
@@ -789,10 +789,10 @@ public void testDeleteMemoryContainer_WithSharedPrefix_PreventsDeletion() {
789789

790790
// Mock countContainersWithPrefix to return 2 (shared prefix - unsafe to delete indices)
791791
doAnswer(invocation -> {
792-
ActionListener<Long> listener = invocation.getArgument(2);
792+
ActionListener<Long> listener = invocation.getArgument(1);
793793
listener.onResponse(2L);
794794
return null;
795-
}).when(memoryContainerHelper).countContainersWithPrefix(any(), any(), any());
795+
}).when(memoryContainerHelper).countContainersWithPrefix(any(), any());
796796

797797
// Execute
798798
transportDeleteMemoryContainerAction.doExecute(null, deleteRequest, actionListener);
@@ -847,7 +847,7 @@ public void testDeleteMemoryContainer_CountCheckFailure_AbortsOperation() {
847847
ActionListener<Long> listener = invocation.getArgument(2);
848848
listener.onFailure(new RuntimeException("Failed to count containers"));
849849
return null;
850-
}).when(memoryContainerHelper).countContainersWithPrefix(any(), any(), any());
850+
}).when(memoryContainerHelper).countContainersWithPrefix(any(), any());
851851

852852
// Execute
853853
transportDeleteMemoryContainerAction.doExecute(null, deleteRequest, actionListener);
@@ -903,7 +903,7 @@ public void testDeleteMemoryContainer_WithoutIndexDeletion_SkipsCountCheck() {
903903
transportDeleteMemoryContainerAction.doExecute(null, deleteRequest, actionListener);
904904

905905
// Verify countContainersWithPrefix was NEVER called (no index deletion requested)
906-
verify(memoryContainerHelper, timeout(1000).times(0)).countContainersWithPrefix(any(), any(), any());
906+
verify(memoryContainerHelper, timeout(1000).times(0)).countContainersWithPrefix(any(), any());
907907

908908
// Verify deleteIndex was NEVER called
909909
verify(memoryContainerHelper, timeout(1000).times(0)).deleteIndex(any(), any(), any());

0 commit comments

Comments
 (0)