Adding check for no updated settings

Signed-off-by: Joshua Palis <[email protected]>
joshpalis · Aug 4, 2024 · ad8ee5b · ad8ee5b
1 parent ce3d016
commit ad8ee5b
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 1 deletion.
diff --git a/src/main/java/org/opensearch/flowframework/workflow/UpdateIndexStep.java b/src/main/java/org/opensearch/flowframework/workflow/UpdateIndexStep.java
@@ -144,6 +144,11 @@ public PlainActionFuture<WorkflowData> execute(
                 }
             }
 
+            if (updateSettingsRequest.settings().size() == 0) {
+                String errorMessage = "Failed to update index settings for index " + indexName + ", no settings have been updated";
+                throw new FlowFrameworkException(errorMessage, RestStatus.BAD_REQUEST);
+            }
+
             client.admin().indices().updateSettings(updateSettingsRequest, ActionListener.wrap(acknowledgedResponse -> {
                 String resourceName = getResourceByWorkflowStep(getName());
                 logger.info("Updated index settings for index {}", indexName);
@@ -158,7 +163,7 @@ public PlainActionFuture<WorkflowData> execute(
                 updateIndexFuture.onFailure(new WorkflowStepException(errorMessage, ExceptionsHelper.status(e)));
             }));
         } catch (Exception e) {
-            updateIndexFuture.onFailure(e);
+            updateIndexFuture.onFailure(new WorkflowStepException(e.getMessage(), ExceptionsHelper.status(e)));
         }
 
         return updateIndexFuture;

diff --git a/src/test/java/org/opensearch/flowframework/workflow/UpdateIndexStepTests.java b/src/test/java/org/opensearch/flowframework/workflow/UpdateIndexStepTests.java
@@ -189,4 +189,54 @@ public void testMissingInputs() throws InterruptedException {
 
     }
 
+    public void testNoSettingsChanged() throws InterruptedException {
+        UpdateIndexStep updateIndexStep = new UpdateIndexStep(client);
+
+        String indexName = "test-index";
+
+        // Create existing settings for default pipelines
+        Settings.Builder builder = Settings.builder();
+        builder.put("index.number_of_shards", 2);
+        builder.put("index.number_of_replicas", 1);
+        builder.put("index.knn", true);
+        builder.put("index.default_pipeline", "ingest_pipeline_id");
+        builder.put("index.search.default_pipeline", "search_pipeline_id");
+        Map<String, Settings> indexToSettings = new HashMap<>();
+        indexToSettings.put(indexName, builder.build());
+
+        // Stub get index settings request/response
+        doAnswer(invocation -> {
+            ActionListener<GetSettingsResponse> getSettingsResponseListener = invocation.getArgument(1);
+            getSettingsResponseListener.onResponse(new GetSettingsResponse(indexToSettings, indexToSettings));
+            return null;
+        }).when(indicesAdminClient).getSettings(any(), any());
+
+        // validate update settings request content
+        @SuppressWarnings({ "unchecked" })
+        ArgumentCaptor<UpdateSettingsRequest> updateSettingsRequestCaptor = ArgumentCaptor.forClass(UpdateSettingsRequest.class);
+
+        // Configurations have no change
+        String configurations =
+            "{\"settings\":{\"index\":{\"knn\":true,\"number_of_shards\":2,\"number_of_replicas\":1,\"default_pipeline\":\"ingest_pipeline_id\",\"search\":{\"default_pipeline\":\"search_pipeline_id\"}}},\"mappings\":{\"properties\":{\"age\":{\"type\":\"integer\"}}},\"aliases\":{\"sample-alias1\":{}}}";
+        WorkflowData data = new WorkflowData(
+            Map.ofEntries(Map.entry(INDEX_NAME, indexName), Map.entry(CONFIGURATIONS, configurations)),
+            "test-id",
+            "test-node-id"
+        );
+        PlainActionFuture future = updateIndexStep.execute(
+            data.getNodeId(),
+            data,
+            Collections.emptyMap(),
+            Collections.emptyMap(),
+            Collections.emptyMap()
+        );
+
+        ExecutionException exception = assertThrows(ExecutionException.class, () -> future.get());
+        assertTrue(exception.getCause() instanceof Exception);
+        assertEquals(
+            "Failed to update index settings for index test-index, no settings have been updated",
+            exception.getCause().getMessage()
+        );
+    }
+
 }