From f273301753b1cc98af71c0cf934c085aa4373b44 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Tue, 25 Jun 2024 10:19:40 -0700 Subject: [PATCH] Separate build and validateAndBuild method in DataSourceMetadata (#2744) (#2752) (cherry picked from commit 7b40c2c6a1a83329a0805cb367a8a56f9fbe2c4b) Signed-off-by: Tomoyuki Morita Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- .../datasource/model/DataSourceMetadata.java | 8 +++++-- .../model/DataSourceMetadataTest.java | 24 ++++++++++++------- .../service/DataSourceServiceImpl.java | 6 +++-- .../utils/XContentParserUtils.java | 2 +- 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/datasource/model/DataSourceMetadata.java b/core/src/main/java/org/opensearch/sql/datasource/model/DataSourceMetadata.java index e3dd0e8ff7..6efc7c935c 100644 --- a/core/src/main/java/org/opensearch/sql/datasource/model/DataSourceMetadata.java +++ b/core/src/main/java/org/opensearch/sql/datasource/model/DataSourceMetadata.java @@ -128,10 +128,14 @@ public Builder setDataSourceStatus(DataSourceStatus status) { return this; } - public DataSourceMetadata build() { + public DataSourceMetadata validateAndBuild() { validateMissingAttributes(); validateName(); validateCustomResultIndex(); + return build(); + } + + public DataSourceMetadata build() { fillNullAttributes(); return new DataSourceMetadata(this); } @@ -239,6 +243,6 @@ public static DataSourceMetadata defaultOpenSearchDataSourceMetadata() { .setConnector(DataSourceType.OPENSEARCH) .setAllowedRoles(Collections.emptyList()) .setProperties(ImmutableMap.of()) - .build(); + .validateAndBuild(); } } diff --git a/core/src/test/java/org/opensearch/sql/datasource/model/DataSourceMetadataTest.java b/core/src/test/java/org/opensearch/sql/datasource/model/DataSourceMetadataTest.java index 24f830f18e..fe40fac868 100644 --- a/core/src/test/java/org/opensearch/sql/datasource/model/DataSourceMetadataTest.java +++ b/core/src/test/java/org/opensearch/sql/datasource/model/DataSourceMetadataTest.java @@ -36,7 +36,7 @@ public void testBuilderAndGetterMethods() { .setProperties(properties) .setResultIndex("query_execution_result_test123") .setDataSourceStatus(ACTIVE) - .build(); + .validateAndBuild(); assertEquals("test", metadata.getName()); assertEquals("test description", metadata.getDescription()); @@ -59,7 +59,10 @@ public void testDefaultDataSourceMetadata() { @Test public void testNameValidation() { try { - new DataSourceMetadata.Builder().setName("Invalid$$$Name").setConnector(PROMETHEUS).build(); + new DataSourceMetadata.Builder() + .setName("Invalid$$$Name") + .setConnector(PROMETHEUS) + .validateAndBuild(); fail("Should have thrown an IllegalArgumentException"); } catch (IllegalArgumentException e) { assertEquals( @@ -76,7 +79,7 @@ public void testResultIndexValidation() { .setName("test") .setConnector(PROMETHEUS) .setResultIndex("invalid_result_index") - .build(); + .validateAndBuild(); fail("Should have thrown an IllegalArgumentException"); } catch (IllegalArgumentException e) { assertEquals(DataSourceMetadata.INVALID_RESULT_INDEX_PREFIX, e.getMessage()); @@ -86,7 +89,7 @@ public void testResultIndexValidation() { @Test public void testMissingAttributes() { try { - new DataSourceMetadata.Builder().build(); + new DataSourceMetadata.Builder().validateAndBuild(); fail("Should have thrown an IllegalArgumentException due to missing attributes"); } catch (IllegalArgumentException e) { assertTrue(e.getMessage().contains("name")); @@ -97,7 +100,10 @@ public void testMissingAttributes() { @Test public void testFillAttributes() { DataSourceMetadata metadata = - new DataSourceMetadata.Builder().setName("test").setConnector(PROMETHEUS).build(); + new DataSourceMetadata.Builder() + .setName("test") + .setConnector(PROMETHEUS) + .validateAndBuild(); assertEquals("test", metadata.getName()); assertEquals(PROMETHEUS, metadata.getConnector()); @@ -115,7 +121,7 @@ public void testLengthyResultIndexName() { .setName("test") .setConnector(PROMETHEUS) .setResultIndex("query_execution_result_" + RandomStringUtils.randomAlphanumeric(300)) - .build(); + .validateAndBuild(); fail("Should have thrown an IllegalArgumentException"); } catch (IllegalArgumentException e) { assertEquals( @@ -131,7 +137,7 @@ public void testInbuiltLengthyResultIndexName() { new DataSourceMetadata.Builder() .setName(RandomStringUtils.randomAlphabetic(250)) .setConnector(PROMETHEUS) - .build(); + .validateAndBuild(); assertEquals(255, dataSourceMetadata.getResultIndex().length()); } @@ -150,8 +156,8 @@ public void testCopyFromAnotherMetadata() { .setProperties(properties) .setResultIndex("query_execution_result_test123") .setDataSourceStatus(ACTIVE) - .build(); - DataSourceMetadata copiedMetadata = new DataSourceMetadata.Builder(metadata).build(); + .validateAndBuild(); + DataSourceMetadata copiedMetadata = new DataSourceMetadata.Builder(metadata).validateAndBuild(); assertEquals(metadata.getResultIndex(), copiedMetadata.getResultIndex()); assertEquals(metadata.getProperties(), copiedMetadata.getProperties()); } diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java b/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java index 4fe42fbd5c..61f3c8cd5d 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java @@ -167,7 +167,7 @@ private DataSourceMetadata constructUpdatedDatasourceMetadata( break; } } - return metadataBuilder.build(); + return metadataBuilder.validateAndBuild(); } private DataSourceMetadata getRawDataSourceMetadata(String dataSourceName) { @@ -199,6 +199,8 @@ private DataSourceMetadata removeAuthInfo(DataSourceMetadata dataSourceMetadata) entry -> CONFIDENTIAL_AUTH_KEYS.stream() .anyMatch(confidentialKey -> entry.getKey().endsWith(confidentialKey))); - return new DataSourceMetadata.Builder(dataSourceMetadata).setProperties(safeProperties).build(); + return new DataSourceMetadata.Builder(dataSourceMetadata) + .setProperties(safeProperties) + .validateAndBuild(); } } diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java b/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java index 7c8c33b147..4c98b133a8 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java @@ -97,7 +97,7 @@ public static DataSourceMetadata toDataSourceMetadata(XContentParser parser) thr .setAllowedRoles(allowedRoles) .setResultIndex(resultIndex) .setDataSourceStatus(status) - .build(); + .validateAndBuild(); } public static Map toMap(XContentParser parser) throws IOException {