Skip to content

Separate build and validateAndBuild method in DataSourceMetadata #2744

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ykmr1224 merged 1 commit into from
Jun 13, 2024
Merged

Separate build and validateAndBuild method in DataSourceMetadata #2744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions core/src/main/java/org/opensearch/sql/datasource/model/DataSourceMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -239,6 +243,6 @@ public static DataSourceMetadata defaultOpenSearchDataSourceMetadata() {
.setConnector(DataSourceType.OPENSEARCH)
.setAllowedRoles(Collections.emptyList())
.setProperties(ImmutableMap.of())
.build();
.validateAndBuild();
}
}
24 changes: 15 additions & 9 deletions core/src/test/java/org/opensearch/sql/datasource/model/DataSourceMetadataTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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(
Expand All @@ -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());
Expand All @@ -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"));
Expand All @@ -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());
Expand All @@ -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(
Expand All @@ -131,7 +137,7 @@ public void testInbuiltLengthyResultIndexName() {
new DataSourceMetadata.Builder()
.setName(RandomStringUtils.randomAlphabetic(250))
.setConnector(PROMETHEUS)
.build();
.validateAndBuild();
assertEquals(255, dataSourceMetadata.getResultIndex().length());
}

Expand All @@ -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());
}
Expand Down
6 changes: 4 additions & 2 deletions datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ private DataSourceMetadata constructUpdatedDatasourceMetadata(
break;
}
}
return metadataBuilder.build();
return metadataBuilder.validateAndBuild();
}

private DataSourceMetadata getRawDataSourceMetadata(String dataSourceName) {
Expand Down Expand Up @@ -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();
}
}
2 changes: 1 addition & 1 deletion datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down