[Connector API] Make update configuration action non-additive

x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/335_connector_update_configuration.yml

@@ -57,7 +57,7 @@ setup:
         connector_id: test-connector
         body:
           configuration:
-            some_field:
+            some_new_field:
               default_value: null
               depends_on:
                 - field: some_field
@@ -92,20 +92,22 @@ setup:
       connector.get:
         connector_id: test-connector
 
-  - match: { configuration.some_field.value: 456 }
+  - is_false:  configuration.some_field  # configuration.some_field doesn't exist
+
+  - match: { configuration.some_new_field.value: 456 }
   - match: { status: configured }
-  - match: { configuration.some_field.validations.0.constraint: [123, 456, 789] }
-  - match: { configuration.some_field.validations.0.type: included_in }
-  - match: { configuration.some_field.validations.1.constraint: ["string 1", "string 2", "string 3"] }
-  - match: { configuration.some_field.validations.1.type: included_in }
-  - match: { configuration.some_field.validations.2.constraint: 0 }
-  - match: { configuration.some_field.validations.2.type: greater_than }
-  - match: { configuration.some_field.validations.3.constraint: 42 }
-  - match: { configuration.some_field.validations.3.type: less_than }
-  - match: { configuration.some_field.validations.4.constraint: int }
-  - match: { configuration.some_field.validations.4.type: list_type }
-  - match: { configuration.some_field.validations.5.constraint: "\\d+" }
-  - match: { configuration.some_field.validations.5.type: regex }
+  - match: { configuration.some_new_field.validations.0.constraint: [123, 456, 789] }
+  - match: { configuration.some_new_field.validations.0.type: included_in }
+  - match: { configuration.some_new_field.validations.1.constraint: ["string 1", "string 2", "string 3"] }
+  - match: { configuration.some_new_field.validations.1.type: included_in }
+  - match: { configuration.some_new_field.validations.2.constraint: 0 }
+  - match: { configuration.some_new_field.validations.2.type: greater_than }
+  - match: { configuration.some_new_field.validations.3.constraint: 42 }
+  - match: { configuration.some_new_field.validations.3.type: less_than }
+  - match: { configuration.some_new_field.validations.4.constraint: int }
+  - match: { configuration.some_new_field.validations.4.type: list_type }
+  - match: { configuration.some_new_field.validations.5.constraint: "\\d+" }
+  - match: { configuration.some_new_field.validations.5.type: regex }
 
 ---
 "Update Connector Configuration with null tooltip":

x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java

@@ -26,6 +26,8 @@
 import org.elasticsearch.client.internal.OriginSettingClient;
 import org.elasticsearch.index.IndexNotFoundException;
 import org.elasticsearch.index.query.MatchAllQueryBuilder;
+import org.elasticsearch.script.Script;
+import org.elasticsearch.script.ScriptType;
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.search.builder.SearchSourceBuilder;
 import org.elasticsearch.search.sort.SortOrder;
@@ -45,6 +47,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 import java.util.function.BiConsumer;
@@ -267,26 +270,41 @@ public void onFailure(Exception e) {
 
     /**
      * Updates the {@link ConnectorConfiguration} property of a {@link Connector}.
+     * The update process is non-additive; it completely replaces all existing configuration fields with the new configuration mapping,
+     * thereby deleting any old configurations.
      *
      * @param request   Request for updating connector configuration property.
      * @param listener  Listener to respond to a successful response or an error.
      */
     public void updateConnectorConfiguration(UpdateConnectorConfigurationAction.Request request, ActionListener<UpdateResponse> listener) {
         try {
             String connectorId = request.getConnectorId();
-            final UpdateRequest updateRequest = new UpdateRequest(CONNECTOR_INDEX_NAME, connectorId).doc(
-                new IndexRequest(CONNECTOR_INDEX_NAME).opType(DocWriteRequest.OpType.INDEX)
-                    .id(connectorId)
-                    .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
-                    .source(
-                        Map.of(
-                            Connector.CONFIGURATION_FIELD.getPreferredName(),
-                            request.getConfiguration(),
-                            Connector.STATUS_FIELD.getPreferredName(),
-                            ConnectorStatus.CONFIGURED.toString()
-                        )
-                    )
+
+            String updateConfigurationScript = String.format(
+                Locale.ROOT,
+                """
+                    ctx._source.%s = params.%s;
+                    ctx._source.%s = params.%s;
+                    """,
+                Connector.CONFIGURATION_FIELD.getPreferredName(),
+                Connector.CONFIGURATION_FIELD.getPreferredName(),
+                Connector.STATUS_FIELD.getPreferredName(),
+                Connector.STATUS_FIELD.getPreferredName()
+            );
+            Script script = new Script(
+                ScriptType.INLINE,
+                "painless",
+                updateConfigurationScript,
+                Map.of(
+                    Connector.CONFIGURATION_FIELD.getPreferredName(),
+                    request.getConfiguration(),
+                    Connector.STATUS_FIELD.getPreferredName(),
+                    ConnectorStatus.CONFIGURED.toString()
+                )
             );
+            final UpdateRequest updateRequest = new UpdateRequest(CONNECTOR_INDEX_NAME, connectorId).script(script)
+                .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE);
+
             clientWithOrigin.update(
                 updateRequest,
                 new DelegatingIndexNotFoundActionListener<>(connectorId, listener, (l, updateResponse) -> {

x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/ConnectorIndexServiceTests.java

@@ -12,7 +12,14 @@
 import org.elasticsearch.action.DocWriteResponse;
 import org.elasticsearch.action.delete.DeleteResponse;
 import org.elasticsearch.action.update.UpdateResponse;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.rest.RestStatus;
+import org.elasticsearch.script.MockScriptEngine;
+import org.elasticsearch.script.MockScriptPlugin;
+import org.elasticsearch.script.ScriptContext;
+import org.elasticsearch.script.ScriptEngine;
+import org.elasticsearch.script.UpdateScript;
 import org.elasticsearch.test.ESSingleNodeTestCase;
 import org.elasticsearch.xpack.application.connector.action.PostConnectorAction;
 import org.elasticsearch.xpack.application.connector.action.PutConnectorAction;
@@ -27,11 +34,14 @@
 import org.junit.Before;
 
 import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
@@ -49,6 +59,13 @@ public void setup() {
         this.connectorIndexService = new ConnectorIndexService(client());
     }
 
+    @Override
+    protected Collection<Class<? extends Plugin>> getPlugins() {
+        List<Class<? extends Plugin>> plugins = new ArrayList<>(super.getPlugins());
+        plugins.add(MockPainlessScriptEngine.TestPlugin.class);
+        return plugins;
+    }
+
     public void testPutConnector() throws Exception {
         Connector connector = ConnectorTestUtils.getRandomConnector();
         String connectorId = randomUUID();
@@ -90,21 +107,16 @@ public void testUpdateConnectorConfiguration() throws Exception {
         DocWriteResponse resp = buildRequestAndAwaitPutConnector(connectorId, connector);
         assertThat(resp.status(), anyOf(equalTo(RestStatus.CREATED), equalTo(RestStatus.OK)));
 
-        Map<String, ConnectorConfiguration> connectorConfiguration = connector.getConfiguration()
-            .entrySet()
-            .stream()
-            .collect(Collectors.toMap(Map.Entry::getKey, entry -> ConnectorTestUtils.getRandomConnectorConfigurationField()));
-
         UpdateConnectorConfigurationAction.Request updateConfigurationRequest = new UpdateConnectorConfigurationAction.Request(
             connectorId,
-            connectorConfiguration
+            connector.getConfiguration()
         );
 
         DocWriteResponse updateResponse = awaitUpdateConnectorConfiguration(updateConfigurationRequest);
         assertThat(updateResponse.status(), equalTo(RestStatus.OK));
-        Connector indexedConnector = awaitGetConnector(connectorId);
-        assertThat(connectorConfiguration, equalTo(indexedConnector.getConfiguration()));
-        assertThat(indexedConnector.getStatus(), equalTo(ConnectorStatus.CONFIGURED));
+
+        // Configuration update is handled via painless script. ScriptEngine is mocked for unit tests.
+        // More comprehensive tests are defined in yamlRestTest.
     }
 
     public void testUpdateConnectorPipeline() throws Exception {
@@ -608,4 +620,44 @@ public void onFailure(Exception e) {
         return resp.get();
     }
 
+    /**
+     * Update configuration action is handled via painless script. This implementation mocks the painless script engine
+     * for unit tests.
+     */
+    private static class MockPainlessScriptEngine extends MockScriptEngine {
+
+        public static final String NAME = "painless";
+
+        public static class TestPlugin extends MockScriptPlugin {
+            @Override
+            public ScriptEngine getScriptEngine(Settings settings, Collection<ScriptContext<?>> contexts) {
+                return new ConnectorIndexServiceTests.MockPainlessScriptEngine();
+            }
+
+            @Override
+            protected Map<String, Function<Map<String, Object>, Object>> pluginScripts() {
+                return Collections.emptyMap();
+            }
+        }
+
+        @Override
+        public String getType() {
+            return NAME;
+        }
+
+        @Override
+        public <T> T compile(String name, String script, ScriptContext<T> context, Map<String, String> options) {
+            if (context.instanceClazz.equals(UpdateScript.class)) {
+                UpdateScript.Factory factory = (params, ctx) -> new UpdateScript(params, ctx) {
+                    @Override
+                    public void execute() {
+
+                    }
+                };
+                return context.factoryClazz.cast(factory);
+            }
+            throw new IllegalArgumentException("mock painless does not know how to handle context [" + context.name + "]");
+        }
+    }
+
 }