Separate build and validateAndBuild method in DataSourceMetadata (ope…

…nsearch-project#2744) Signed-off-by: Tomoyuki Morita <[email protected]>
noCharger · Jun 13, 2024 · 7b40c2c · 7b40c2c
1 parent 084a3c8
commit 7b40c2c
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 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
@@ -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
@@ -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
@@ -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
@@ -97,7 +97,7 @@ public static DataSourceMetadata toDataSourceMetadata(XContentParser parser) thr
         .setAllowedRoles(allowedRoles)
         .setResultIndex(resultIndex)
         .setDataSourceStatus(status)
-        .build();
+        .validateAndBuild();
   }
 
   public static Map<String, Object> toMap(XContentParser parser) throws IOException {