Implement FileCache Prune API endpoint with Service Locator Pattern #1
Conversation
Co-authored-by: x-INFiN1TY-x <[email protected]>
There was a problem hiding this comment.
@copilot
Let's break down the alternatives to the implemented "plumb-through" approach. The core challenge remains the same: How does RestPruneCacheAction, which is created deep inside ActionModule, get a reference to the FileCache service, which is owned by the top-level Node class?
Here are three primary alternative architectural styles, each with its own set of pros and cons.
Alternative 1: The Service Locator Pattern
Instead of passing each dependency down through method parameters, you pass a single "Service Locator" or "Registry" object. This object holds references to all available services, and components can query it to get the dependencies they need.
In the context of OpenSearch, the Node object itself is the ultimate service registry.
Implementation Sketch:
Modify initRestHandlers to accept the Node instance. The signature would change to:
Java
// In ActionModule.java
public void initRestHandlers(Supplier nodesInCluster, Node node)
Update the call site in Node.java. The Node would pass a reference to itself.
Java
// In Node.java
actionModule.initRestHandlers(() -> clusterService.state().nodes(), this);
Instantiate handlers by passing the Node locator. The ActionModule passes the Node object to the handlers that need services from it.
Java
// In ActionModule.java
registerHandler.accept(new RestPruneCacheAction(node));
The handler retrieves the service. The RestPruneCacheAction constructor would use the Node locator to get the FileCache.
Java
// In RestPruneCacheAction.java
public class RestPruneCacheAction extends BaseRestHandler {
private final FileCache fileCache;
public RestPruneCacheAction(Node node) {
this.fileCache = node.fileCache(); // Use the Node as a service locator
}
// ...
}
Implementation Plan: Prune API via Service Locator Pattern
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.
server/src/main/java/org/opensearch/rest/action/admin/cluster/RestPruneCacheAction.java
/*
- 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.
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.
server/src/main/java/org/opensearch/action/ActionModule.java
// ... 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
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.
server/src/main/java/org/opensearch/node/Node.java
// ... inside the Node constructor, near the end
// ...
logger.debug("initializing HTTP handlers ...");
// 3. Update the call to pass 'this' as the service locator.
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.
server/src/test/java/org/opensearch/rest/action/admin/cluster/RestPruneCacheActionTests.java
/*
- 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.
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).
server/src/test/java/org/opensearch/action/ActionModuleTests.java
// ... imports
import org.opensearch.node.Node;
// ... inside ActionModuleTests class
// In any test method that calls initRestHandlers:
This change needs to be applied to all locations in the file where initRestHandlers is invoked.
This comprehensive plan implements the same functionality as the original PR but adheres strictly to the Service Locator pattern, demonstrating a different but equally valid architectural approach.
x-INFiN1TY-x
left a comment
x-INFiN1TY-x
left a comment
There was a problem hiding this comment.
@copilot
Let's break down the alternatives to the implemented "plumb-through" approach. The core challenge remains the same: How does RestPruneCacheAction, which is created deep inside ActionModule, get a reference to the FileCache service, which is owned by the top-level Node class?
Here are three primary alternative architectural styles, each with its own set of pros and cons.
Alternative 1: The Service Locator Pattern
Instead of passing each dependency down through method parameters, you pass a single "Service Locator" or "Registry" object. This object holds references to all available services, and components can query it to get the dependencies they need.
In the context of OpenSearch, the Node object itself is the ultimate service registry.
Implementation Sketch:
Modify initRestHandlers to accept the Node instance. The signature would change to:
Java
// In ActionModule.java
public void initRestHandlers(Supplier nodesInCluster, Node node)
Update the call site in Node.java. The Node would pass a reference to itself.
Java
// In Node.java
actionModule.initRestHandlers(() -> clusterService.state().nodes(), this);
Instantiate handlers by passing the Node locator. The ActionModule passes the Node object to the handlers that need services from it.
Java
// In ActionModule.java
registerHandler.accept(new RestPruneCacheAction(node));
The handler retrieves the service. The RestPruneCacheAction constructor would use the Node locator to get the FileCache.
Java
// In RestPruneCacheAction.java
public class RestPruneCacheAction extends BaseRestHandler {
private final FileCache fileCache;
public RestPruneCacheAction(Node node) {
this.fileCache = node.fileCache(); // Use the Node as a service locator
}
// ...
}
Implementation Plan: Prune API via Service Locator Pattern
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.
server/src/main/java/org/opensearch/rest/action/admin/cluster/RestPruneCacheAction.java
/*
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.
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.
server/src/main/java/org/opensearch/action/ActionModule.java
// ... 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
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.
server/src/main/java/org/opensearch/node/Node.java
// ... inside the Node constructor, near the end
// ...
logger.debug("initializing HTTP handlers ...");
// 3. Update the call to pass 'this' as the service locator.
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.
server/src/test/java/org/opensearch/rest/action/admin/cluster/RestPruneCacheActionTests.java
/*
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.
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).
server/src/test/java/org/opensearch/action/ActionModuleTests.java
// ... imports
import org.opensearch.node.Node;
// ... inside ActionModuleTests class
// In any test method that calls initRestHandlers:
This change needs to be applied to all locations in the file where initRestHandlers is invoked.
This comprehensive plan implements the same functionality as the original PR but adheres strictly to the Service Locator pattern, demonstrating a different but equally valid architectural approach.
There was a problem hiding this comment.
@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)
}
// ..."
There was a problem hiding this comment.
@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)
}
// ..."
There was a problem hiding this comment.
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)
}
// ..."
There was a problem hiding this comment.
Phase 2 completed successfully. Updated ActionModule.java to:
- Added
import org.opensearch.node.Node; - Changed
initRestHandlersmethod signature to acceptNode nodeinstead ofFileCache fileCache - Updated RestPruneCacheAction registration to pass
nodeinstead offileCache
The handler registration now uses the Service Locator Pattern as requested.
Commit: 02abc04
There was a problem hiding this comment.
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 {
// ...
}"
There was a problem hiding this comment.
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 {
// ...
}"
There was a problem hiding this comment.
@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 {
// ...
}"
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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));
};
}
}"
There was a problem hiding this comment.
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));
};
}
}"
There was a problem hiding this comment.
@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));
};
}
}"
There was a problem hiding this comment.
Phase 1 completed successfully. Updated RestPruneCacheAction to use the Service Locator Pattern:
- Added
import org.opensearch.node.Node; - Changed constructor to accept
Node nodeinstead ofFileCache fileCache - 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
There was a problem hiding this comment.
@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
}"
There was a problem hiding this comment.
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
}"
There was a problem hiding this comment.
@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
}"
There was a problem hiding this comment.
Phase 5 completed successfully. Updated ActionModuleTests.java to work with the new Service Locator Pattern:
- Added
import org.opensearch.node.Node; - Updated all three
initRestHandlerscalls to passmock(Node.class)instead ofmock(FileCache.class)
All ActionModule tests now pass with the new signature that uses Node as the service locator.
Commit: 02abc04
There was a problem hiding this comment.
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));
});
}
}"
There was a problem hiding this comment.
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));
});
}
}"
There was a problem hiding this comment.
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));
});
}
}"
There was a problem hiding this comment.
Phase 4 completed successfully. Updated RestPruneCacheActionTests.java to use the Service Locator Pattern:
- Added
import org.opensearch.node.Node; - Updated all test methods to mock Node instead of directly mocking FileCache
- Used
when(mockNode.fileCache()).thenReturn(mockFileCache)to configure the service locator - 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
x-INFiN1TY-x
left a comment
x-INFiN1TY-x
left a comment
There was a problem hiding this comment.
@copilot
Let's break down the alternatives to the implemented "plumb-through" approach. The core challenge remains the same: How does RestPruneCacheAction, which is created deep inside ActionModule, get a reference to the FileCache service, which is owned by the top-level Node class?
Here are three primary alternative architectural styles, each with its own set of pros and cons.
Alternative 1: The Service Locator Pattern
Instead of passing each dependency down through method parameters, you pass a single "Service Locator" or "Registry" object. This object holds references to all available services, and components can query it to get the dependencies they need.
In the context of OpenSearch, the Node object itself is the ultimate service registry.
Implementation Sketch:
Modify initRestHandlers to accept the Node instance. The signature would change to:
Java
// In ActionModule.java
public void initRestHandlers(Supplier nodesInCluster, Node node)
Update the call site in Node.java. The Node would pass a reference to itself.
Java
// In Node.java
actionModule.initRestHandlers(() -> clusterService.state().nodes(), this);
Instantiate handlers by passing the Node locator. The ActionModule passes the Node object to the handlers that need services from it.
Java
// In ActionModule.java
registerHandler.accept(new RestPruneCacheAction(node));
The handler retrieves the service. The RestPruneCacheAction constructor would use the Node locator to get the FileCache.
Java
// In RestPruneCacheAction.java
public class RestPruneCacheAction extends BaseRestHandler {
private final FileCache fileCache;
public RestPruneCacheAction(Node node) {
this.fileCache = node.fileCache(); // Use the Node as a service locator
}
// ...
}
Implementation Plan: Prune API via Service Locator Pattern
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.
server/src/main/java/org/opensearch/rest/action/admin/cluster/RestPruneCacheAction.java
/*
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.
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.
server/src/main/java/org/opensearch/action/ActionModule.java
// ... 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
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.
server/src/main/java/org/opensearch/node/Node.java
// ... inside the Node constructor, near the end
// ...
logger.debug("initializing HTTP handlers ...");
// 3. Update the call to pass 'this' as the service locator.
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.
server/src/test/java/org/opensearch/rest/action/admin/cluster/RestPruneCacheActionTests.java
/*
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.
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).
server/src/test/java/org/opensearch/action/ActionModuleTests.java
// ... imports
import org.opensearch.node.Node;
// ... inside ActionModuleTests class
// In any test method that calls initRestHandlers:
This change needs to be applied to all locations in the file where initRestHandlers is invoked.
This comprehensive plan implements the same functionality as the original PR but adheres strictly to the Service Locator pattern, demonstrating a different but equally valid architectural approach.
There was a problem hiding this comment.
@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)
}
// ..."
There was a problem hiding this comment.
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 {
// ...
}"
There was a problem hiding this comment.
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));
};
}
}"
There was a problem hiding this comment.
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
}"
There was a problem hiding this comment.
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));
});
}
}"
x-INFiN1TY-x
left a comment
x-INFiN1TY-x
left a comment
There was a problem hiding this comment.
Let's break down the alternatives to the implemented "plumb-through" approach. The core challenge remains the same: How does RestPruneCacheAction, which is created deep inside ActionModule, get a reference to the FileCache service, which is owned by the top-level Node class?
Here are three primary alternative architectural styles, each with its own set of pros and cons.
Alternative 1: The Service Locator Pattern
Instead of passing each dependency down through method parameters, you pass a single "Service Locator" or "Registry" object. This object holds references to all available services, and components can query it to get the dependencies they need.
In the context of OpenSearch, the Node object itself is the ultimate service registry.
Implementation Sketch:
Modify initRestHandlers to accept the Node instance. The signature would change to:
Java
// In ActionModule.java
public void initRestHandlers(Supplier nodesInCluster, Node node)
Update the call site in Node.java. The Node would pass a reference to itself.
Java
// In Node.java
actionModule.initRestHandlers(() -> clusterService.state().nodes(), this);
Instantiate handlers by passing the Node locator. The ActionModule passes the Node object to the handlers that need services from it.
Java
// In ActionModule.java
registerHandler.accept(new RestPruneCacheAction(node));
The handler retrieves the service. The RestPruneCacheAction constructor would use the Node locator to get the FileCache.
Java
// In RestPruneCacheAction.java
public class RestPruneCacheAction extends BaseRestHandler {
private final FileCache fileCache;
public RestPruneCacheAction(Node node) {
this.fileCache = node.fileCache(); // Use the Node as a service locator
}
// ...
}
Implementation Plan: Prune API via Service Locator Pattern
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.
server/src/main/java/org/opensearch/rest/action/admin/cluster/RestPruneCacheAction.java
/*
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.
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.
server/src/main/java/org/opensearch/action/ActionModule.java
// ... 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
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.
server/src/main/java/org/opensearch/node/Node.java
// ... inside the Node constructor, near the end
// ...
logger.debug("initializing HTTP handlers ...");
// 3. Update the call to pass 'this' as the service locator.
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.
server/src/test/java/org/opensearch/rest/action/admin/cluster/RestPruneCacheActionTests.java
/*
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.
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).
server/src/test/java/org/opensearch/action/ActionModuleTests.java
// ... imports
import org.opensearch.node.Node;
// ... inside ActionModuleTests class
// In any test method that calls initRestHandlers:
This change needs to be applied to all locations in the file where initRestHandlers is invoked.
This comprehensive plan implements the same functionality as the original PR but adheres strictly to the Service Locator pattern, demonstrating a different but equally valid architectural approach.
Handle .git/index.lock errors
Add a step before any git commands in your workflow to clean up lock files:
YAML
- name: Cleanup git lock file
run: |
LOCK_FILE="${GITHUB_WORKSPACE}/.git/index.lock"
if [ -f "$LOCK_FILE" ]; then
echo "Removing git lock file..."
rm -f "$LOCK_FILE"
fi
There was a problem hiding this comment.
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)
}
// ..."
There was a problem hiding this comment.
@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 {
// ...
}"
There was a problem hiding this comment.
@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));
};
}
}"
There was a problem hiding this comment.
@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
}"
There was a problem hiding this comment.
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));
});
}
}"
Co-authored-by: x-INFiN1TY-x <[email protected]>
I have successfully implemented the Service Locator Pattern as an alternative to the plumb-through approach. The implementation follows your detailed plan:
All tests pass, confirming the Service Locator Pattern is working correctly. This approach provides better flexibility and follows the existing OpenSearch architectural patterns where Node acts as the central service registry. Commit: 02abc04 |
This PR implements a new REST API endpoint that allows administrators to manually trigger the FileCache
prune()operation, enabling on-demand cleanup of non-referenced cache entries.Problem
Currently, the FileCache
prune()method is only called reactively when the cache exceeds capacity or during testing. There was no way for administrators to manually trigger cache cleanup, which is useful for:Solution
Added a new REST endpoint
POST /_cache/remote/prunethat:API Usage
curl -X POST "http://localhost:9200/_cache/remote/prune"Response:
{ "acknowledged": true, "pruned_bytes": 1048576 }Implementation Details
Core Components
BaseRestHandlerinitRestHandlers()to accept Node parameterTechnical Approach
The implementation follows the existing OpenSearch REST API patterns and uses the Service Locator Pattern for dependency injection:
acknowledgedandpruned_bytesfieldsnode.fileCache()Architectural Pattern
The implementation uses the Service Locator Pattern instead of direct dependency injection:
node.fileCache()Files Changed
server/src/main/java/org/opensearch/rest/action/admin/cluster/RestPruneCacheAction.java(new)server/src/main/java/org/opensearch/action/ActionModule.java(modified)server/src/main/java/org/opensearch/node/Node.java(modified)server/src/test/java/org/opensearch/rest/action/admin/cluster/RestPruneCacheActionTests.java(new)server/src/test/java/org/opensearch/action/ActionModuleTests.java(updated)Testing
fileCache()method appropriatelyBenefits
The implementation maintains full backward compatibility and follows established OpenSearch coding patterns while using a more flexible architectural approach for dependency management.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.