Skip to content
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion server/src/main/java/org/opensearch/action/ActionModule.java
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot
Prune API via Service Locator Pattern:
The Node object will serve as our "Service Locator."
The core idea is to pass the top-level Node object, which "owns" all services, down to the point of REST handler creation. The handler then uses the Node object to locate and retrieve the specific service it needs (FileCache).

Phase 2: Modify the Handler Registration Point (ActionModule.java)
This is the "middleman" that connects the provider and the consumer. We will modify its initRestHandlers method to accept the Node service locator.

File to Modify: server/src/main/java/org/opensearch/action/ActionModule.java

Action: Change the method signature of initRestHandlers to accept Node and use it when instantiating RestPruneCacheAction.

draft code context : "// ... imports
import org.opensearch.node.Node; // <-- Import the Node class
// ... other imports
import org.opensearch.rest.action.admin.cluster.RestPruneCacheAction; // <-- Import the new action

// ... inside the ActionModule class

// 1. Change the method signature to accept 'Node'.
public void initRestHandlers(Supplier nodesInCluster, Node node) {
List catActions = new ArrayList<>();
List listActions = new ArrayList<>();
Consumer registerHandler = handler -> {
// ... (no changes here)
};

// ... (all existing handler registrations remain unchanged)

registerHandler.accept(new RestNodesStatsAction());
// ... etc.

// 2. Add the new handler registration, passing the 'node' locator.
// FileCache API
registerHandler.accept(new RestPruneCacheAction(node));

// ... (rest of the method)

}
// ..."

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot
Prune API via Service Locator Pattern:
The Node object will serve as our "Service Locator."
The core idea is to pass the top-level Node object, which "owns" all services, down to the point of REST handler creation. The handler then uses the Node object to locate and retrieve the specific service it needs (FileCache).

Phase 2: Modify the Handler Registration Point (ActionModule.java)
This is the "middleman" that connects the provider and the consumer. We will modify its initRestHandlers method to accept the Node service locator.

File to Modify: server/src/main/java/org/opensearch/action/ActionModule.java

Action: Change the method signature of initRestHandlers to accept Node and use it when instantiating RestPruneCacheAction.

draft code context : "// ... imports
import org.opensearch.node.Node; // <-- Import the Node class
// ... other imports
import org.opensearch.rest.action.admin.cluster.RestPruneCacheAction; // <-- Import the new action

// ... inside the ActionModule class

// 1. Change the method signature to accept 'Node'.
public void initRestHandlers(Supplier nodesInCluster, Node node) {
List catActions = new ArrayList<>();
List listActions = new ArrayList<>();
Consumer registerHandler = handler -> {
// ... (no changes here)
};

// ... (all existing handler registrations remain unchanged)

registerHandler.accept(new RestNodesStatsAction());
// ... etc.

// 2. Add the new handler registration, passing the 'node' locator.
// FileCache API
registerHandler.accept(new RestPruneCacheAction(node));

// ... (rest of the method)
}
// ..."

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot

Prune API via Service Locator Pattern:
The Node object will serve as our "Service Locator."
The core idea is to pass the top-level Node object, which "owns" all services, down to the point of REST handler creation. The handler then uses the Node object to locate and retrieve the specific service it needs (FileCache).

Phase 2: Modify the Handler Registration Point (ActionModule.java)
This is the "middleman" that connects the provider and the consumer. We will modify its initRestHandlers method to accept the Node service locator.

File to Modify: server/src/main/java/org/opensearch/action/ActionModule.java

Action: Change the method signature of initRestHandlers to accept Node and use it when instantiating RestPruneCacheAction.

draft code context : "// ... imports
import org.opensearch.node.Node; // <-- Import the Node class
// ... other imports
import org.opensearch.rest.action.admin.cluster.RestPruneCacheAction; // <-- Import the new action

// ... inside the ActionModule class

// 1. Change the method signature to accept 'Node'.
public void initRestHandlers(Supplier nodesInCluster, Node node) {
List catActions = new ArrayList<>();
List listActions = new ArrayList<>();
Consumer registerHandler = handler -> {
// ... (no changes here)
};

// ... (all existing handler registrations remain unchanged)

registerHandler.accept(new RestNodesStatsAction());
// ... etc.

// 2. Add the new handler registration, passing the 'node' locator.
// FileCache API
registerHandler.accept(new RestPruneCacheAction(node));

// ... (rest of the method)
}
// ..."

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Phase 2 completed successfully. Updated ActionModule.java to:

  1. Added import org.opensearch.node.Node;
  2. Changed initRestHandlers method signature to accept Node node instead of FileCache fileCache
  3. Updated RestPruneCacheAction registration to pass node instead of fileCache

The handler registration now uses the Service Locator Pattern as requested.

Commit: 02abc04

Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@
import org.opensearch.extensions.rest.RestSendToExtensionAction;
import org.opensearch.identity.IdentityService;
import org.opensearch.index.seqno.RetentionLeaseActions;
import org.opensearch.index.store.remote.filecache.FileCache;
import org.opensearch.indices.SystemIndices;
import org.opensearch.persistent.CompletionPersistentTaskAction;
import org.opensearch.persistent.RemovePersistentTaskAction;
Expand Down Expand Up @@ -378,6 +379,7 @@
import org.opensearch.rest.action.admin.cluster.RestNodesStatsAction;
import org.opensearch.rest.action.admin.cluster.RestNodesUsageAction;
import org.opensearch.rest.action.admin.cluster.RestPendingClusterTasksAction;
import org.opensearch.rest.action.admin.cluster.RestPruneCacheAction;
import org.opensearch.rest.action.admin.cluster.RestPutRepositoryAction;
import org.opensearch.rest.action.admin.cluster.RestPutStoredScriptAction;
import org.opensearch.rest.action.admin.cluster.RestReloadSecureSettingsAction;
Expand Down Expand Up @@ -839,7 +841,7 @@ private ActionFilters setupActionFilters(List<ActionPlugin> actionPlugins) {
);
}

public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster) {
public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster, FileCache fileCache) {
List<AbstractCatAction> catActions = new ArrayList<>();
List<AbstractListAction> listActions = new ArrayList<>();
Consumer<RestHandler> registerHandler = handler -> {
Expand Down Expand Up @@ -1064,6 +1066,9 @@ public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster) {
registerHandler.accept(new RestRestoreRemoteStoreAction());
registerHandler.accept(new RestRemoteStoreMetadataAction());

// FileCache API
registerHandler.accept(new RestPruneCacheAction(fileCache));

// pull-based ingestion API
registerHandler.accept(new RestPauseIngestionAction());
registerHandler.accept(new RestResumeIngestionAction());
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/opensearch/node/Node.java
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot

THINK IN DEPTH AND DETAIL , READ AND UNDERSTAND THE COMMENT AND THE CONTEXT , THEN USE YOUR OWN THINKING AND UNDERSTANDING TO IMPLEMENT THE CODE CHANGES.

Prune API via Service Locator Pattern:
The Node object will serve as our "Service Locator."
The core idea is to pass the top-level Node object, which "owns" all services, down to the point of REST handler creation. The handler then uses the Node object to locate and retrieve the specific service it needs (FileCache).

Phase 3: Update the Call Site (Node.java)
This is the top-level "provider" of the service locator. The Node class will now pass a reference to itself to the ActionModule.

File to Modify: server/src/main/java/org/opensearch/node/Node.java

Action: Update the call to actionModule.initRestHandlers to match the new signature.

code context : "// ... inside the Node constructor, near the end

    // ...
    
    logger.debug("initializing HTTP handlers ...");
    // 3. Update the call to pass 'this' as the service locator.
    actionModule.initRestHandlers(() -> clusterService.state().nodes(), this);
    logger.info("initialized");

    success = true;
} finally {
    // ...
}"

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot

THINK IN DEPTH AND DETAIL , READ AND UNDERSTAND THE COMMENT AND THE CONTEXT , THEN USE YOUR OWN THINKING AND UNDERSTANDING TO IMPLEMENT THE CODE CHANGES.

Prune API via Service Locator Pattern:
The Node object will serve as our "Service Locator."
The core idea is to pass the top-level Node object, which "owns" all services, down to the point of REST handler creation. The handler then uses the Node object to locate and retrieve the specific service it needs (FileCache).

Phase 3: Update the Call Site (Node.java)
This is the top-level "provider" of the service locator. The Node class will now pass a reference to itself to the ActionModule.

File to Modify: server/src/main/java/org/opensearch/node/Node.java

Action: Update the call to actionModule.initRestHandlers to match the new signature.

code context : "// ... inside the Node constructor, near the end

// ...

logger.debug("initializing HTTP handlers ...");
// 3. Update the call to pass 'this' as the service locator.
actionModule.initRestHandlers(() -> clusterService.state().nodes(), this);
logger.info("initialized");

success = true;

} finally {
// ...
}"

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot
THINK IN DEPTH AND DETAIL , READ AND UNDERSTAND THE COMMENT AND THE CONTEXT , THEN USE YOUR OWN THINKING AND UNDERSTANDING TO IMPLEMENT THE CODE CHANGES.

Prune API via Service Locator Pattern:
The Node object will serve as our "Service Locator."
The core idea is to pass the top-level Node object, which "owns" all services, down to the point of REST handler creation. The handler then uses the Node object to locate and retrieve the specific service it needs (FileCache).

Phase 3: Update the Call Site (Node.java)
This is the top-level "provider" of the service locator. The Node class will now pass a reference to itself to the ActionModule.

File to Modify: server/src/main/java/org/opensearch/node/Node.java

Action: Update the call to actionModule.initRestHandlers to match the new signature.

code context : "// ... inside the Node constructor, near the end

// ...

logger.debug("initializing HTTP handlers ...");
// 3. Update the call to pass 'this' as the service locator.
actionModule.initRestHandlers(() -> clusterService.state().nodes(), this);
logger.info("initialized");

success = true;
} finally {
// ...
}"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Phase 3 completed successfully. Updated Node.java call site to pass the Node instance as the service locator:

Changed from:

actionModule.initRestHandlers(() -> clusterService.state().nodes(), fileCache());

To:

actionModule.initRestHandlers(() -> clusterService.state().nodes(), this);

The Node now passes itself as the service locator to ActionModule as planned.

Commit: 02abc04

Original file line number Diff line number Diff line change
Expand Up @@ -1718,7 +1718,7 @@ protected Node(final Environment initialEnvironment, Collection<PluginInfo> clas
this.namedWriteableRegistry = namedWriteableRegistry;

logger.debug("initializing HTTP handlers ...");
actionModule.initRestHandlers(() -> clusterService.state().nodes());
actionModule.initRestHandlers(() -> clusterService.state().nodes(), fileCache());
logger.info("initialized");

success = true;
Expand Down
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot

Prune API via Service Locator Pattern:
The Node object will serve as our "Service Locator."
The core idea is to pass the top-level Node object, which "owns" all services, down to the point of REST handler creation. The handler then uses the Node object to locate and retrieve the specific service it needs (FileCache).

Phase 1: Implement the REST Handler (RestPruneCacheAction.java)
This is the component that consumes the service. Its constructor will accept the Node service locator.

File to Create: server/src/main/java/org/opensearch/rest/action/admin/cluster/RestPruneCacheAction.java

Action: Create the new REST handler class. It will extend BaseRestHandler and use the Node object passed into its constructor to get a reference to the FileCache.

draft version of code : "public class RestPruneCacheAction extends BaseRestHandler {

private final FileCache fileCache;

// The constructor now accepts the 'Node' object as the Service Locator.
public RestPruneCacheAction(Node node) {
// The handler uses the locator to retrieve the specific service it needs.
this.fileCache = node.fileCache();
}

@OverRide
public String getName() {
return "prune_cache_action";
}

@OverRide
public List routes() {
return singletonList(new Route(Method.POST, "/_cache/remote/prune"));
}

@OverRide
protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {
return channel -> {
// The core logic remains unchanged.
long prunedBytes = fileCache.prune();

    // Build the JSON response as specified.
    XContentBuilder builder = channel.newBuilder();
    builder.startObject();
    builder.field("acknowledged", true);
    builder.field("pruned_bytes", prunedBytes);
    builder.endObject();

    channel.sendResponse(new BytesRestResponse(RestStatus.OK, builder));
};

}
}"

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot

THINK IN DEPTH AND DETAIL , READ AND UNDERSTAND THE COMMENT AND THE CONTEXT , THEN USE YOUR OWN THINKING AND UNDERSTANDING TO IMPLEMENT THE CODE CHANGES.

Prune API via Service Locator Pattern:
The Node object will serve as our "Service Locator."
The core idea is to pass the top-level Node object, which "owns" all services, down to the point of REST handler creation. The handler then uses the Node object to locate and retrieve the specific service it needs (FileCache).

Phase 1: Implement the REST Handler (RestPruneCacheAction.java)
This is the component that consumes the service. Its constructor will accept the Node service locator.

File to Create: server/src/main/java/org/opensearch/rest/action/admin/cluster/RestPruneCacheAction.java

Action: Create the new REST handler class. It will extend BaseRestHandler and use the Node object passed into its constructor to get a reference to the FileCache.

draft version of code : "public class RestPruneCacheAction extends BaseRestHandler {

private final FileCache fileCache;

// The constructor now accepts the 'Node' object as the Service Locator.
public RestPruneCacheAction(Node node) {
// The handler uses the locator to retrieve the specific service it needs.
this.fileCache = node.fileCache();
}

@OverRide
public String getName() {
return "prune_cache_action";
}

@OverRide
public List routes() {
return singletonList(new Route(Method.POST, "/_cache/remote/prune"));
}

@OverRide
protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {
return channel -> {
// The core logic remains unchanged.
long prunedBytes = fileCache.prune();

// Build the JSON response as specified.
XContentBuilder builder = channel.newBuilder();
builder.startObject();
builder.field("acknowledged", true);
builder.field("pruned_bytes", prunedBytes);
builder.endObject();

channel.sendResponse(new BytesRestResponse(RestStatus.OK, builder));

};
}
}"

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot
THINK IN DEPTH AND DETAIL , READ AND UNDERSTAND THE COMMENT AND THE CONTEXT , THEN USE YOUR OWN THINKING AND UNDERSTANDING TO IMPLEMENT THE CODE CHANGES.

Prune API via Service Locator Pattern:
The Node object will serve as our "Service Locator."
The core idea is to pass the top-level Node object, which "owns" all services, down to the point of REST handler creation. The handler then uses the Node object to locate and retrieve the specific service it needs (FileCache).

Phase 1: Implement the REST Handler (RestPruneCacheAction.java)
This is the component that consumes the service. Its constructor will accept the Node service locator.

File to Create: server/src/main/java/org/opensearch/rest/action/admin/cluster/RestPruneCacheAction.java

Action: Create the new REST handler class. It will extend BaseRestHandler and use the Node object passed into its constructor to get a reference to the FileCache.

draft version of code : "public class RestPruneCacheAction extends BaseRestHandler {

private final FileCache fileCache;

// The constructor now accepts the 'Node' object as the Service Locator.
public RestPruneCacheAction(Node node) {
// The handler uses the locator to retrieve the specific service it needs.
this.fileCache = node.fileCache();
}

@OverRide
public String getName() {
return "prune_cache_action";
}

@OverRide
public List routes() {
return singletonList(new Route(Method.POST, "/_cache/remote/prune"));
}

@OverRide
protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {
return channel -> {
// The core logic remains unchanged.
long prunedBytes = fileCache.prune();

// Build the JSON response as specified.
XContentBuilder builder = channel.newBuilder();
builder.startObject();
builder.field("acknowledged", true);
builder.field("pruned_bytes", prunedBytes);
builder.endObject();

channel.sendResponse(new BytesRestResponse(RestStatus.OK, builder));
};
}
}"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Phase 1 completed successfully. Updated RestPruneCacheAction to use the Service Locator Pattern:

  1. Added import org.opensearch.node.Node;
  2. Changed constructor to accept Node node instead of FileCache fileCache
  3. Constructor now retrieves FileCache via this.fileCache = node.fileCache();

The REST handler now uses the Node as a service locator to retrieve the specific service it needs, exactly as outlined in your implementation plan.

Commit: 02abc04

Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* 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.
*/

package org.opensearch.rest.action.admin.cluster;

import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.index.store.remote.filecache.FileCache;
import org.opensearch.rest.BaseRestHandler;
import org.opensearch.rest.BytesRestResponse;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestRequest.Method;
import org.opensearch.transport.client.node.NodeClient;

import java.io.IOException;
import java.util.List;

import static java.util.Collections.singletonList;

/**
* REST action to manually trigger FileCache prune operation.
* This endpoint allows administrators to clear out non-referenced cache entries on demand.
*
* @opensearch.api
*/
public class RestPruneCacheAction extends BaseRestHandler {

private final FileCache fileCache;

public RestPruneCacheAction(FileCache fileCache) {
this.fileCache = fileCache;
}

@Override
public List<Route> routes() {
return singletonList(new Route(Method.POST, "/_cache/remote/prune"));
}

@Override
public String getName() {
return "prune_cache_action";
}

@Override
protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {
return channel -> {
// Execute the prune operation
long prunedBytes = fileCache.prune();

// Build JSON response
XContentBuilder builder = channel.newBuilder();
builder.startObject();
builder.field("acknowledged", true);
builder.field("pruned_bytes", prunedBytes);
builder.endObject();

channel.sendResponse(new BytesRestResponse(RestStatus.OK, builder));
};
}
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot
Prune API via Service Locator Pattern:
The Node object will serve as our "Service Locator."
The core idea is to pass the top-level Node object, which "owns" all services, down to the point of REST handler creation. The handler then uses the Node object to locate and retrieve the specific service it needs (FileCache).

Phase 5: Update Existing Tests (ActionModuleTests.java)
Finally, we must fix the existing tests that broke due to the signature change in ActionModule.initRestHandlers.

File to Modify: server/src/test/java/org/opensearch/action/ActionModuleTests.java

Action: Update all calls to initRestHandlers to pass a mock(Node.class).

DRAFT CONTEXT : "// ... imports
import org.opensearch.node.Node;

// ... inside ActionModuleTests class

// In any test method that calls initRestHandlers:
public void testSomeExistingTest() {
// ... setup code
Node mockNode = mock(Node.class);
// Before: actionModule.initRestHandlers(null);
// After:
actionModule.initRestHandlers(null, mockNode);
// ... assertions
}"

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot

THINK IN DEPTH AND DETAIL , READ AND UNDERSTAND THE COMMENT AND THE CONTEXT , THEN USE YOUR OWN THINKING AND UNDERSTANDING TO IMPLEMENT THE CODE CHANGES.
Prune API via Service Locator Pattern:
The Node object will serve as our "Service Locator."
The core idea is to pass the top-level Node object, which "owns" all services, down to the point of REST handler creation. The handler then uses the Node object to locate and retrieve the specific service it needs (FileCache).

Phase 5: Update Existing Tests (ActionModuleTests.java)
Finally, we must fix the existing tests that broke due to the signature change in ActionModule.initRestHandlers.

File to Modify: server/src/test/java/org/opensearch/action/ActionModuleTests.java

Action: Update all calls to initRestHandlers to pass a mock(Node.class).

DRAFT CONTEXT : "// ... imports
import org.opensearch.node.Node;

// ... inside ActionModuleTests class

// In any test method that calls initRestHandlers:
public void testSomeExistingTest() {
// ... setup code
Node mockNode = mock(Node.class);
// Before: actionModule.initRestHandlers(null);
// After:
actionModule.initRestHandlers(null, mockNode);
// ... assertions
}"

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot
THINK IN DEPTH AND DETAIL , READ AND UNDERSTAND THE COMMENT AND THE CONTEXT , THEN USE YOUR OWN THINKING AND UNDERSTANDING TO IMPLEMENT THE CODE CHANGES.
Prune API via Service Locator Pattern:
The Node object will serve as our "Service Locator."
The core idea is to pass the top-level Node object, which "owns" all services, down to the point of REST handler creation. The handler then uses the Node object to locate and retrieve the specific service it needs (FileCache).

Phase 5: Update Existing Tests (ActionModuleTests.java)
Finally, we must fix the existing tests that broke due to the signature change in ActionModule.initRestHandlers.

File to Modify: server/src/test/java/org/opensearch/action/ActionModuleTests.java

Action: Update all calls to initRestHandlers to pass a mock(Node.class).

DRAFT CONTEXT : "// ... imports
import org.opensearch.node.Node;

// ... inside ActionModuleTests class

// In any test method that calls initRestHandlers:
public void testSomeExistingTest() {
// ... setup code
Node mockNode = mock(Node.class);
// Before: actionModule.initRestHandlers(null);
// After:
actionModule.initRestHandlers(null, mockNode);
// ... assertions
}"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Phase 5 completed successfully. Updated ActionModuleTests.java to work with the new Service Locator Pattern:

  1. Added import org.opensearch.node.Node;
  2. Updated all three initRestHandlers calls to pass mock(Node.class) instead of mock(FileCache.class)

All ActionModule tests now pass with the new signature that uses Node as the service locator.

Commit: 02abc04

Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.opensearch.core.action.ActionResponse;
import org.opensearch.extensions.ExtensionsManager;
import org.opensearch.identity.IdentityService;
import org.opensearch.index.store.remote.filecache.FileCache;
import org.opensearch.plugins.ActionPlugin;
import org.opensearch.plugins.ActionPlugin.ActionHandler;
import org.opensearch.rest.RestChannel;
Expand Down Expand Up @@ -146,7 +147,7 @@ public void testSetupRestHandlerContainsKnownBuiltin() throws IOException {
new IdentityService(Settings.EMPTY, mock(ThreadPool.class), new ArrayList<>()),
new ExtensionsManager(Set.of(), new IdentityService(Settings.EMPTY, mock(ThreadPool.class), List.of()))
);
actionModule.initRestHandlers(null);
actionModule.initRestHandlers(null, mock(FileCache.class));
// At this point the easiest way to confirm that a handler is loaded is to try to register another one on top of it and to fail
Exception e = expectThrows(
IllegalArgumentException.class,
Expand Down Expand Up @@ -204,7 +205,7 @@ public String getName() {
null,
null
);
Exception e = expectThrows(IllegalArgumentException.class, () -> actionModule.initRestHandlers(null));
Exception e = expectThrows(IllegalArgumentException.class, () -> actionModule.initRestHandlers(null, mock(FileCache.class)));
assertThat(e.getMessage(), startsWith("Cannot replace existing handler for [/] for method: GET"));
} finally {
threadPool.shutdown();
Expand Down Expand Up @@ -255,7 +256,7 @@ public List<RestHandler> getRestHandlers(
null,
null
);
actionModule.initRestHandlers(null);
actionModule.initRestHandlers(null, mock(FileCache.class));
// At this point the easiest way to confirm that a handler is loaded is to try to register another one on top of it and to fail
Exception e = expectThrows(
IllegalArgumentException.class,
Expand Down
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot

Prune API via Service Locator Pattern:
The Node object will serve as our "Service Locator."
The core idea is to pass the top-level Node object, which "owns" all services, down to the point of REST handler creation. The handler then uses the Node object to locate and retrieve the specific service it needs (FileCache).

Phase 4: Implement Unit Tests (RestPruneCacheActionTests.java)
We must create tests for our new handler. This involves mocking the Node object and the FileCache it's expected to provide.

File to Create: server/src/test/java/org/opensearch/rest/action/admin/cluster/RestPruneCacheActionTests.java

Action: Create a new test class that extends RestActionTestCase.

Draft code context : "/*

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.
*/
package org.opensearch.rest.action.admin.cluster;

import org.opensearch.core.rest.RestStatus;
import org.opensearch.index.store.remote.filecache.FileCache;
import org.opensearch.node.Node;
import org.opensearch.rest.RestRequest;
import org.opensearch.test.rest.FakeRestRequest;
import org.opensearch.test.rest.RestActionTestCase;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class RestPruneCacheActionTests extends RestActionTestCase {

public void testGetName() {
Node mockNode = mock(Node.class);
RestPruneCacheAction action = new RestPruneCacheAction(mockNode);
assertEquals("prune_cache_action", action.getName());
}

public void testSuccessfulPrune() throws Exception {
// 1. Mock the FileCache service
FileCache mockFileCache = mock(FileCache.class);
final long bytesPruned = 12345678L;
when(mockFileCache.prune()).thenReturn(bytesPruned);

// 2. Mock the Node (Service Locator) to return the mocked service
Node mockNode = mock(Node.class);
when(mockNode.fileCache()).thenReturn(mockFileCache);

// 3. Create the action with the mocked locator
RestPruneCacheAction action = new RestPruneCacheAction(mockNode);
RestRequest request = new FakeRestRequest.Builder(xContentRegistry())
.withMethod(RestRequest.Method.POST)
.withPath("/_cache/remote/prune")
.build();

// 4. Dispatch the request and verify the response
verifying(action, request, (channel) -> {
assertEquals(RestStatus.OK, channel.capturedResponse().status());
String responseBody = channel.capturedResponse().content().utf8ToString();
assertTrue(responseBody.contains(""acknowledged":true"));
assertTrue(responseBody.contains(""pruned_bytes":" + bytesPruned));
});
}
}"

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot

Prune API via Service Locator Pattern:
The Node object will serve as our "Service Locator."
The core idea is to pass the top-level Node object, which "owns" all services, down to the point of REST handler creation. The handler then uses the Node object to locate and retrieve the specific service it needs (FileCache).

Phase 4: Implement Unit Tests (RestPruneCacheActionTests.java)
We must create tests for our new handler. This involves mocking the Node object and the FileCache it's expected to provide.

File to Create: server/src/test/java/org/opensearch/rest/action/admin/cluster/RestPruneCacheActionTests.java

Action: Create a new test class that extends RestActionTestCase.

Draft code context : "/*

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.
*/
package org.opensearch.rest.action.admin.cluster;

import org.opensearch.core.rest.RestStatus;
import org.opensearch.index.store.remote.filecache.FileCache;
import org.opensearch.node.Node;
import org.opensearch.rest.RestRequest;
import org.opensearch.test.rest.FakeRestRequest;
import org.opensearch.test.rest.RestActionTestCase;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class RestPruneCacheActionTests extends RestActionTestCase {

public void testGetName() {
Node mockNode = mock(Node.class);
RestPruneCacheAction action = new RestPruneCacheAction(mockNode);
assertEquals("prune_cache_action", action.getName());
}

public void testSuccessfulPrune() throws Exception {
// 1. Mock the FileCache service
FileCache mockFileCache = mock(FileCache.class);
final long bytesPruned = 12345678L;
when(mockFileCache.prune()).thenReturn(bytesPruned);

// 2. Mock the Node (Service Locator) to return the mocked service
Node mockNode = mock(Node.class);
when(mockNode.fileCache()).thenReturn(mockFileCache);

// 3. Create the action with the mocked locator
RestPruneCacheAction action = new RestPruneCacheAction(mockNode);
RestRequest request = new FakeRestRequest.Builder(xContentRegistry())
.withMethod(RestRequest.Method.POST)
.withPath("/_cache/remote/prune")
.build();

// 4. Dispatch the request and verify the response
verifying(action, request, (channel) -> {
assertEquals(RestStatus.OK, channel.capturedResponse().status());
String responseBody = channel.capturedResponse().content().utf8ToString();
assertTrue(responseBody.contains(""acknowledged":true"));
assertTrue(responseBody.contains(""pruned_bytes":" + bytesPruned));
});
}
}"

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot

Prune API via Service Locator Pattern:
The Node object will serve as our "Service Locator."
The core idea is to pass the top-level Node object, which "owns" all services, down to the point of REST handler creation. The handler then uses the Node object to locate and retrieve the specific service it needs (FileCache).

Phase 4: Implement Unit Tests (RestPruneCacheActionTests.java)
We must create tests for our new handler. This involves mocking the Node object and the FileCache it's expected to provide.

File to Create: server/src/test/java/org/opensearch/rest/action/admin/cluster/RestPruneCacheActionTests.java

Action: Create a new test class that extends RestActionTestCase.

Draft code context : "/*

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.
*/
package org.opensearch.rest.action.admin.cluster;

import org.opensearch.core.rest.RestStatus;
import org.opensearch.index.store.remote.filecache.FileCache;
import org.opensearch.node.Node;
import org.opensearch.rest.RestRequest;
import org.opensearch.test.rest.FakeRestRequest;
import org.opensearch.test.rest.RestActionTestCase;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class RestPruneCacheActionTests extends RestActionTestCase {

public void testGetName() {
Node mockNode = mock(Node.class);
RestPruneCacheAction action = new RestPruneCacheAction(mockNode);
assertEquals("prune_cache_action", action.getName());
}

public void testSuccessfulPrune() throws Exception {
// 1. Mock the FileCache service
FileCache mockFileCache = mock(FileCache.class);
final long bytesPruned = 12345678L;
when(mockFileCache.prune()).thenReturn(bytesPruned);

// 2. Mock the Node (Service Locator) to return the mocked service
Node mockNode = mock(Node.class);
when(mockNode.fileCache()).thenReturn(mockFileCache);

// 3. Create the action with the mocked locator
RestPruneCacheAction action = new RestPruneCacheAction(mockNode);
RestRequest request = new FakeRestRequest.Builder(xContentRegistry())
.withMethod(RestRequest.Method.POST)
.withPath("/_cache/remote/prune")
.build();

// 4. Dispatch the request and verify the response
verifying(action, request, (channel) -> {
assertEquals(RestStatus.OK, channel.capturedResponse().status());
String responseBody = channel.capturedResponse().content().utf8ToString();
assertTrue(responseBody.contains(""acknowledged":true"));
assertTrue(responseBody.contains(""pruned_bytes":" + bytesPruned));
});
}
}"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Phase 4 completed successfully. Updated RestPruneCacheActionTests.java to use the Service Locator Pattern:

  1. Added import org.opensearch.node.Node;
  2. Updated all test methods to mock Node instead of directly mocking FileCache
  3. Used when(mockNode.fileCache()).thenReturn(mockFileCache) to configure the service locator
  4. All tests now create RestPruneCacheAction with the Node mock

The comprehensive unit tests now properly validate the Service Locator Pattern implementation, with the Node object providing access to the FileCache service.

Commit: 02abc04

Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* 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.
*/

package org.opensearch.rest.action.admin.cluster;

import org.opensearch.core.rest.RestStatus;
import org.opensearch.index.store.remote.filecache.FileCache;
import org.opensearch.rest.RestRequest;
import org.opensearch.test.rest.FakeRestRequest;
import org.opensearch.test.rest.RestActionTestCase;
import org.opensearch.threadpool.TestThreadPool;
import org.opensearch.threadpool.ThreadPool;
import org.junit.After;
import org.junit.Before;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class RestPruneCacheActionTests extends RestActionTestCase {

private ThreadPool threadPool;

@Before
public void setup() {
threadPool = new TestThreadPool(this.getClass().getSimpleName());
}

@After
public void tearDown() throws Exception {
super.tearDown();
terminate(threadPool);
}

public void testRoutes() {
RestPruneCacheAction action = new RestPruneCacheAction(mock(FileCache.class));
assertEquals(1, action.routes().size());
assertEquals(RestRequest.Method.POST, action.routes().get(0).getMethod());
assertEquals("/_cache/remote/prune", action.routes().get(0).getPath());
}

public void testGetName() {
RestPruneCacheAction action = new RestPruneCacheAction(mock(FileCache.class));
assertEquals("prune_cache_action", action.getName());
}

public void testPrepareRequest() throws Exception {
FileCache mockFileCache = mock(FileCache.class);
when(mockFileCache.prune()).thenReturn(12345678L);

RestPruneCacheAction action = new RestPruneCacheAction(mockFileCache);
RestRequest request = new FakeRestRequest();

// Test that the action prepares correctly without throwing exceptions
assertNotNull(action.prepareRequest(request, null));
}
}
Loading