respond to PR comments

Author: seanzatzdev

server/src/main/java/org/elasticsearch/action/admin/indices/sampling/RestPutSampleConfigurationAction.java

@@ -77,6 +77,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
         // Set the target index
         putRequest.indices(indexNames);
 
+        // TODO: Make this cancellable e.g. RestGetSampleStatsAction
         return channel -> client.execute(PutSampleConfigurationAction.INSTANCE, putRequest, new RestToXContentListener<>(channel));
     }
 

server/src/main/java/org/elasticsearch/ingest/SamplingService.java

@@ -100,7 +100,7 @@ public SamplingService(
         this.updateSamplingConfigurationTaskQueue = clusterService.createTaskQueue(
             "update-sampling-configuration",
             Priority.NORMAL,
-            new UpdateSamplingConfigurationExecutor(projectResolver, this)
+            new UpdateSamplingConfigurationExecutor()
         );
     }
 
@@ -293,9 +293,10 @@ public void updateSampleConfiguration(
         ActionListener<AcknowledgedResponse> listener
     ) {
         // Early validation: check if adding a new configuration would exceed the limit
-        boolean maxConfigLimitBreached = checkMaxConfigLimitBreached(projectId, index);
+        ClusterState clusterState = clusterService.state();
+        boolean maxConfigLimitBreached = checkMaxConfigLimitBreached(projectId, index, clusterState);
         if (maxConfigLimitBreached) {
-            Integer maxConfigurations = MAX_CONFIGURATIONS_SETTING.get(clusterService.state().getMetadata().settings());
+            Integer maxConfigurations = MAX_CONFIGURATIONS_SETTING.get(clusterState.getMetadata().settings());
             listener.onFailure(
                 new IllegalStateException(
                     "Cannot add sampling configuration for index ["
@@ -349,8 +350,7 @@ private static Script getScript(String conditional) throws IOException {
 
     // Checks whether the maximum number of sampling configurations has been reached for the given project.
     // If the limit is breached, it notifies the listener with an IllegalStateException and returns true.
-    private boolean checkMaxConfigLimitBreached(ProjectId projectId, String index) {
-        ClusterState currentState = clusterService.state();
+    private static boolean checkMaxConfigLimitBreached(ProjectId projectId, String index, ClusterState currentState) {
         Metadata currentMetadata = currentState.metadata();
         ProjectMetadata projectMetadata = currentMetadata.getProject(projectId);
 
@@ -789,13 +789,8 @@ static class UpdateSamplingConfigurationTask extends AckedBatchedClusterStateUpd
 
     static class UpdateSamplingConfigurationExecutor extends SimpleBatchedAckListenerTaskExecutor<UpdateSamplingConfigurationTask> {
         private static final Logger logger = LogManager.getLogger(UpdateSamplingConfigurationExecutor.class);
-        private final ProjectResolver projectResolver;
-        private final SamplingService samplingService;
 
-        UpdateSamplingConfigurationExecutor(ProjectResolver projectResolver, SamplingService samplingService) {
-            this.projectResolver = projectResolver;
-            this.samplingService = samplingService;
-        }
+        UpdateSamplingConfigurationExecutor() {}
 
         @Override
         public Tuple<ClusterState, ClusterStateAckListener> executeTask(
@@ -836,9 +831,10 @@ public Tuple<ClusterState, ClusterStateAckListener> executeTask(
 
             Integer maxConfigurations = MAX_CONFIGURATIONS_SETTING.get(metadata.settings());
             // check if adding a new configuration would exceed the limit
-            boolean maxConfigLimitBreached = samplingService.checkMaxConfigLimitBreached(
+            boolean maxConfigLimitBreached = checkMaxConfigLimitBreached(
                 updateSamplingConfigurationTask.projectId,
-                updateSamplingConfigurationTask.indexName
+                updateSamplingConfigurationTask.indexName,
+                clusterState
             );
             if (maxConfigLimitBreached) {
                 throw new IllegalStateException(

server/src/test/java/org/elasticsearch/ingest/UpdateSamplingConfigurationExecutorTests.java

@@ -52,7 +52,6 @@ public class UpdateSamplingConfigurationExecutorTests extends ESTestCase {
     @Mock
     private Metadata metadata;
 
-    private SamplingService samplingService;
     private SamplingService.UpdateSamplingConfigurationExecutor executor;
 
     @Override
@@ -61,8 +60,7 @@ public void setUp() throws Exception {
         MockitoAnnotations.openMocks(this);
 
         // Create a real SamplingService with mocked dependencies
-        samplingService = new SamplingService(scriptService, clusterService, projectResolver, timeSupplier);
-        executor = new SamplingService.UpdateSamplingConfigurationExecutor(projectResolver, samplingService);
+        executor = new SamplingService.UpdateSamplingConfigurationExecutor();
 
         // Set up settings with MAX_CONFIGURATIONS_SETTING = 100 (the default value)
         Settings settings = Settings.builder().put("sampling.max_configurations", 100).build();