From 0d5e58a6d37b3887b3bcc1255fa8618a988c0da9 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Thu, 7 Mar 2024 17:20:06 -0800 Subject: [PATCH] adding BWC for connector config field (#2184) (#2185) Signed-off-by: Dhrubo Saha (cherry picked from commit ce6f7e11b92922a72023ee6ae8224854c19e0abd) Co-authored-by: Dhrubo Saha --- .../connector/MLCreateConnectorInput.java | 25 ++++--- .../MLCreateConnectorInputTests.java | 72 +++++++++++++++++++ 2 files changed, 88 insertions(+), 9 deletions(-) diff --git a/common/src/main/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorInput.java b/common/src/main/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorInput.java index 1c32982ca4..78b1ed4af2 100644 --- a/common/src/main/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorInput.java +++ b/common/src/main/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorInput.java @@ -7,6 +7,7 @@ import lombok.Builder; import lombok.Data; +import org.opensearch.Version; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.common.io.stream.Writeable; @@ -45,7 +46,7 @@ public class MLCreateConnectorInput implements ToXContentObject, Writeable { public static final String ACCESS_MODE_FIELD = "access_mode"; public static final String DRY_RUN_FIELD = "dry_run"; - + private static final Version MINIMAL_SUPPORTED_VERSION_FOR_CLIENT_CONFIG = Version.V_2_13_0; public static final String DRY_RUN_CONNECTOR_NAME = "dryRunConnector"; @@ -226,6 +227,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws @Override public void writeTo(StreamOutput output) throws IOException { + Version streamOutputVersion = output.getVersion(); output.writeOptionalString(name); output.writeOptionalString(description); output.writeOptionalString(version); @@ -266,16 +268,18 @@ public void writeTo(StreamOutput output) throws IOException { } output.writeBoolean(dryRun); output.writeBoolean(updateConnector); - if (connectorClientConfig != null) { - output.writeBoolean(true); - connectorClientConfig.writeTo(output); - } else { - output.writeBoolean(false); + if (streamOutputVersion.onOrAfter(MINIMAL_SUPPORTED_VERSION_FOR_CLIENT_CONFIG)) { + if (connectorClientConfig != null) { + output.writeBoolean(true); + connectorClientConfig.writeTo(output); + } else { + output.writeBoolean(false); + } } - } public MLCreateConnectorInput(StreamInput input) throws IOException { + Version streamInputVersion = input.getVersion(); name = input.readOptionalString(); description = input.readOptionalString(); version = input.readOptionalString(); @@ -302,8 +306,11 @@ public MLCreateConnectorInput(StreamInput input) throws IOException { } dryRun = input.readBoolean(); updateConnector = input.readBoolean(); - if (input.readBoolean()) { - this.connectorClientConfig = new ConnectorClientConfig(input); + if (streamInputVersion.onOrAfter(MINIMAL_SUPPORTED_VERSION_FOR_CLIENT_CONFIG)) { + if (input.readBoolean()) { + this.connectorClientConfig = new ConnectorClientConfig(input); + } } + } } diff --git a/common/src/test/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorInputTests.java b/common/src/test/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorInputTests.java index 7a706272ea..125bbaf393 100644 --- a/common/src/test/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorInputTests.java +++ b/common/src/test/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorInputTests.java @@ -9,6 +9,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.opensearch.Version; import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.LoggingDeprecationHandler; @@ -218,6 +219,77 @@ public void readInputStream_SuccessWithNullFields() throws IOException { }); } + @Test + public void testBuilder_NullActions_ShouldNotThrowException() { + // Actions can be null for a connector without any specific actions defined. + MLCreateConnectorInput input = MLCreateConnectorInput.builder() + .name("test_connector_name") + .description("this is a test connector") + .version("1") + .protocol("http") + .parameters(Map.of("input", "test input value")) + .credential(Map.of("key", "test_key_value")) + .actions(null) // Setting actions to null + .access(AccessMode.PUBLIC) + .backendRoles(Arrays.asList("role1", "role2")) + .addAllBackendRoles(false) + .build(); + + assertNull(input.getActions()); + } + + @Test + public void testParse_MissingNameField_ShouldThrowException() throws IOException { + String jsonMissingName = "{\"description\":\"this is a test connector\",\"version\":\"1\",\"protocol\":\"http\"}"; + XContentParser parser = createParser(jsonMissingName); + + exceptionRule.expect(IllegalArgumentException.class); + exceptionRule.expectMessage("Connector name is null"); + + MLCreateConnectorInput.parse(parser); + } + + @Test + public void testWriteToVersionCompatibility() throws IOException { + MLCreateConnectorInput input = mlCreateConnectorInput; // Assuming mlCreateConnectorInput is already initialized + + // Simulate an older version of OpenSearch that does not support connectorClientConfig + Version oldVersion = Version.fromString("2.12.0"); // Change this as per your old version + BytesStreamOutput output = new BytesStreamOutput(); + output.setVersion(oldVersion); + + input.writeTo(output); + + StreamInput streamInput = output.bytes().streamInput(); + streamInput.setVersion(oldVersion); + + MLCreateConnectorInput deserializedInput = new MLCreateConnectorInput(streamInput); + + // The ConnectorClientConfig should be null as it's not supported in the older version + assertNull(deserializedInput.getConnectorClientConfig()); + } + + @Test + public void testDryRunConnectorInput_IgnoreValidation() { + MLCreateConnectorInput input = MLCreateConnectorInput.builder() + .dryRun(true) // Set dryRun to true + .build(); + + // No exception for missing mandatory fields when dryRun is true + assertTrue(input.isDryRun()); + assertNull(input.getName()); // Name is not set, but no exception due to dryRun + } + + // Helper method to create XContentParser from a JSON string + private XContentParser createParser(String jsonString) throws IOException { + XContentParser parser = XContentType.JSON.xContent().createParser( + new NamedXContentRegistry(new SearchModule(Settings.EMPTY, Collections.emptyList()).getNamedXContents()), + LoggingDeprecationHandler.INSTANCE, + jsonString); + parser.nextToken(); // Move to the first token + return parser; + } + private void testParseFromJsonString(String expectedInputString, Consumer verify) throws Exception { XContentParser parser = XContentType.JSON.xContent().createParser(new NamedXContentRegistry(new SearchModule(Settings.EMPTY, Collections.emptyList()).getNamedXContents()), LoggingDeprecationHandler.INSTANCE, expectedInputString);