From 88c6abc8b6e9d9aed9c27fad3212f5042656e1fe Mon Sep 17 00:00:00 2001 From: Vamsi Manohar Date: Thu, 2 Mar 2023 18:11:41 -0800 Subject: [PATCH 1/2] Added DataSourceMetadataStorage interface Signed-off-by: Vamsi Manohar --- .../org/opensearch/sql/analysis/Analyzer.java | 9 ++++--- .../datasource/DataSourceMetadataStorage.java | 25 +++++++++++++++++++ .../sql/datasource/DataSourceService.java | 2 +- .../sql/datasource/DataSourceServiceImpl.java | 10 ++++++-- .../datasource/model/DataSourceMetadata.java | 12 +++++---- .../datasource/DataSourceTableScan.java | 9 ++++--- .../sql/analysis/AnalyzerTestBase.java | 6 +++-- .../datasource/DataSourceServiceImplTest.java | 10 ++++---- .../datasource/DataSourceTableScanTest.java | 7 +++++- 9 files changed, 66 insertions(+), 24 deletions(-) create mode 100644 core/src/main/java/org/opensearch/sql/datasource/DataSourceMetadataStorage.java diff --git a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java index 228b54ba0c..472f038981 100644 --- a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java +++ b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java @@ -65,6 +65,7 @@ import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.datasource.DataSourceService; import org.opensearch.sql.datasource.model.DataSource; +import org.opensearch.sql.datasource.model.DataSourceMetadata; import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.expression.DSL; import org.opensearch.sql.expression.Expression; @@ -134,9 +135,9 @@ public LogicalPlan analyze(UnresolvedPlan unresolved, AnalysisContext context) { @Override public LogicalPlan visitRelation(Relation node, AnalysisContext context) { QualifiedName qualifiedName = node.getTableQualifiedName(); - Set allowedDataSourceNames = dataSourceService.getDataSources() + Set allowedDataSourceNames = dataSourceService.getMaskedDataSourceMetadataSet() .stream() - .map(DataSource::getName) + .map(DataSourceMetadata::getName) .collect(Collectors.toSet()); DataSourceSchemaIdentifierNameResolver dataSourceSchemaIdentifierNameResolver = new DataSourceSchemaIdentifierNameResolver(qualifiedName.getParts(), @@ -182,9 +183,9 @@ public LogicalPlan visitRelationSubquery(RelationSubquery node, AnalysisContext @Override public LogicalPlan visitTableFunction(TableFunction node, AnalysisContext context) { QualifiedName qualifiedName = node.getFunctionName(); - Set allowedDataSourceNames = dataSourceService.getDataSources() + Set allowedDataSourceNames = dataSourceService.getMaskedDataSourceMetadataSet() .stream() - .map(DataSource::getName) + .map(DataSourceMetadata::getName) .collect(Collectors.toSet()); DataSourceSchemaIdentifierNameResolver dataSourceSchemaIdentifierNameResolver = new DataSourceSchemaIdentifierNameResolver(qualifiedName.getParts(), diff --git a/core/src/main/java/org/opensearch/sql/datasource/DataSourceMetadataStorage.java b/core/src/main/java/org/opensearch/sql/datasource/DataSourceMetadataStorage.java new file mode 100644 index 0000000000..ed5b5770d0 --- /dev/null +++ b/core/src/main/java/org/opensearch/sql/datasource/DataSourceMetadataStorage.java @@ -0,0 +1,25 @@ +/* + * + * * Copyright OpenSearch Contributors + * * SPDX-License-Identifier: Apache-2.0 + * + */ + +package org.opensearch.sql.datasource; + +import java.util.List; +import java.util.Optional; +import org.opensearch.sql.datasource.model.DataSourceMetadata; + +/** + * Interface for DataSourceMetadata Storage. + */ +public interface DataSourceMetadataStorage { + + List getDataSourceMetadata(); + + Optional getDataSourceMetadata(String datasourceName); + + void createDataSourceMetadata(DataSourceMetadata dataSourceMetadata); + +} \ No newline at end of file diff --git a/core/src/main/java/org/opensearch/sql/datasource/DataSourceService.java b/core/src/main/java/org/opensearch/sql/datasource/DataSourceService.java index 37e6f8e085..3612a5edad 100644 --- a/core/src/main/java/org/opensearch/sql/datasource/DataSourceService.java +++ b/core/src/main/java/org/opensearch/sql/datasource/DataSourceService.java @@ -19,7 +19,7 @@ public interface DataSourceService { * * @return set of {@link DataSource}. */ - Set getDataSources(); + Set getMaskedDataSourceMetadataSet(); /** * Returns {@link DataSource} with corresponding to the DataSource name. diff --git a/core/src/main/java/org/opensearch/sql/datasource/DataSourceServiceImpl.java b/core/src/main/java/org/opensearch/sql/datasource/DataSourceServiceImpl.java index 274024e548..f39c4751fe 100644 --- a/core/src/main/java/org/opensearch/sql/datasource/DataSourceServiceImpl.java +++ b/core/src/main/java/org/opensearch/sql/datasource/DataSourceServiceImpl.java @@ -7,6 +7,8 @@ import com.google.common.base.Preconditions; import com.google.common.base.Strings; +import com.google.common.collect.ImmutableMap; +import java.util.HashMap; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -45,8 +47,12 @@ public DataSourceServiceImpl(Set dataSourceFactories) { } @Override - public Set getDataSources() { - return Set.copyOf(dataSourceMap.values()); + public Set getMaskedDataSourceMetadataSet() { + return dataSourceMap.values().stream() + .map(dataSource + -> new DataSourceMetadata(dataSource.getName(), + dataSource.getConnectorType(), ImmutableMap.of())) + .collect(Collectors.toSet()); } @Override 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 f97d272bb9..854e489acd 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 @@ -13,14 +13,19 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.collect.ImmutableMap; import java.util.Map; +import lombok.AllArgsConstructor; import lombok.EqualsAndHashCode; import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.RequiredArgsConstructor; import lombok.Setter; import org.opensearch.sql.datasource.DataSourceService; @JsonIgnoreProperties(ignoreUnknown = true) @Getter @Setter +@AllArgsConstructor +@NoArgsConstructor @EqualsAndHashCode public class DataSourceMetadata { @@ -39,10 +44,7 @@ public class DataSourceMetadata { * {@link DataSource} to {@link DataSourceService}. */ public static DataSourceMetadata defaultOpenSearchDataSourceMetadata() { - DataSourceMetadata dataSourceMetadata = new DataSourceMetadata(); - dataSourceMetadata.setName(DEFAULT_DATASOURCE_NAME); - dataSourceMetadata.setConnector(DataSourceType.OPENSEARCH); - dataSourceMetadata.setProperties(ImmutableMap.of()); - return dataSourceMetadata; + return new DataSourceMetadata(DEFAULT_DATASOURCE_NAME, + DataSourceType.OPENSEARCH, ImmutableMap.of()); } } diff --git a/core/src/main/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScan.java b/core/src/main/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScan.java index 14cd09e162..a628f678ef 100644 --- a/core/src/main/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScan.java +++ b/core/src/main/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScan.java @@ -18,7 +18,7 @@ import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.model.ExprValueUtils; import org.opensearch.sql.datasource.DataSourceService; -import org.opensearch.sql.datasource.model.DataSource; +import org.opensearch.sql.datasource.model.DataSourceMetadata; import org.opensearch.sql.storage.TableScanOperator; /** @@ -47,14 +47,15 @@ public String explain() { @Override public void open() { List exprValues = new ArrayList<>(); - Set dataSources = dataSourceService.getDataSources(); - for (DataSource dataSource : dataSources) { + Set dataSourceMetadataSet + = dataSourceService.getMaskedDataSourceMetadataSet(); + for (DataSourceMetadata dataSource : dataSourceMetadataSet) { exprValues.add( new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( "DATASOURCE_NAME", ExprValueUtils.stringValue(dataSource.getName()), "CONNECTOR_TYPE", - ExprValueUtils.stringValue(dataSource.getConnectorType().name()))))); + ExprValueUtils.stringValue(dataSource.getConnector().name()))))); } iterator = exprValues.iterator(); } diff --git a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTestBase.java b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTestBase.java index c68ba2653e..7c9acfd0cc 100644 --- a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTestBase.java +++ b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTestBase.java @@ -10,6 +10,7 @@ import static org.opensearch.sql.data.type.ExprCoreType.LONG; import static org.opensearch.sql.data.type.ExprCoreType.STRING; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import java.util.List; import java.util.Map; @@ -198,8 +199,9 @@ private class DefaultDataSourceService implements DataSourceService { @Override - public Set getDataSources() { - return ImmutableSet.of(dataSource); + public Set getMaskedDataSourceMetadataSet() { + return ImmutableSet.of(new DataSourceMetadata(dataSource.getName(), + dataSource.getConnectorType(), ImmutableMap.of())); } @Override diff --git a/core/src/test/java/org/opensearch/sql/datasource/DataSourceServiceImplTest.java b/core/src/test/java/org/opensearch/sql/datasource/DataSourceServiceImplTest.java index 2b40b32ee6..5c1bf643e4 100644 --- a/core/src/test/java/org/opensearch/sql/datasource/DataSourceServiceImplTest.java +++ b/core/src/test/java/org/opensearch/sql/datasource/DataSourceServiceImplTest.java @@ -81,19 +81,19 @@ void getNotExistDataSourceShouldFail() { @Test void getAddDataSourcesShouldSuccess() { - assertEquals(0, dataSourceService.getDataSources().size()); + assertEquals(0, dataSourceService.getMaskedDataSourceMetadataSet().size()); dataSourceService.addDataSource(metadata(NAME, DataSourceType.OPENSEARCH, ImmutableMap.of())); - assertEquals(1, dataSourceService.getDataSources().size()); + assertEquals(1, dataSourceService.getMaskedDataSourceMetadataSet().size()); } @Test void noDataSourceExistAfterClear() { dataSourceService.addDataSource(metadata(NAME, DataSourceType.OPENSEARCH, ImmutableMap.of())); - assertEquals(1, dataSourceService.getDataSources().size()); + assertEquals(1, dataSourceService.getMaskedDataSourceMetadataSet().size()); dataSourceService.clear(); - assertEquals(0, dataSourceService.getDataSources().size()); + assertEquals(0, dataSourceService.getMaskedDataSourceMetadataSet().size()); } @Test @@ -137,7 +137,7 @@ void metaDataMissingPropertiesShouldFail() { @Test void metaDataHasDuplicateNameShouldFail() { dataSourceService.addDataSource(metadata(NAME, DataSourceType.OPENSEARCH, ImmutableMap.of())); - assertEquals(1, dataSourceService.getDataSources().size()); + assertEquals(1, dataSourceService.getMaskedDataSourceMetadataSet().size()); IllegalArgumentException exception = assertThrows( diff --git a/core/src/test/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScanTest.java b/core/src/test/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScanTest.java index 2f7188a248..0bd5d60cdf 100644 --- a/core/src/test/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScanTest.java +++ b/core/src/test/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScanTest.java @@ -16,6 +16,7 @@ import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Set; +import java.util.stream.Collectors; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -25,6 +26,7 @@ import org.opensearch.sql.data.model.ExprValueUtils; import org.opensearch.sql.datasource.DataSourceService; import org.opensearch.sql.datasource.model.DataSource; +import org.opensearch.sql.datasource.model.DataSourceMetadata; import org.opensearch.sql.datasource.model.DataSourceType; import org.opensearch.sql.storage.StorageEngine; @@ -54,7 +56,10 @@ void testIterator() { Set dataSourceSet = new HashSet<>(); dataSourceSet.add(new DataSource("prometheus", DataSourceType.PROMETHEUS, storageEngine)); dataSourceSet.add(new DataSource("opensearch", DataSourceType.OPENSEARCH, storageEngine)); - when(dataSourceService.getDataSources()).thenReturn(dataSourceSet); + Set dataSourceMetadata = dataSourceSet.stream() + .map(dataSource -> new DataSourceMetadata(dataSource.getName(), + dataSource.getConnectorType(), ImmutableMap.of())).collect(Collectors.toSet()); + when(dataSourceService.getMaskedDataSourceMetadataSet()).thenReturn(dataSourceMetadata); assertFalse(dataSourceTableScan.hasNext()); dataSourceTableScan.open(); From d5b01dcb04b4e8e429bbc7108e8e621261761e79 Mon Sep 17 00:00:00 2001 From: Vamsi Manohar Date: Tue, 7 Mar 2023 12:34:45 -0800 Subject: [PATCH 2/2] Added new interface methods in DataSourceService and DataSourceMetadataStorage Signed-off-by: Vamsi Manohar --- .gitignore | 1 + .../org/opensearch/sql/analysis/Analyzer.java | 5 +- .../datasource/DataSourceMetadataStorage.java | 41 +++++++++++++- .../sql/datasource/DataSourceService.java | 39 +++++++++++--- .../sql/datasource/DataSourceServiceImpl.java | 19 ++++++- .../datasource/DataSourceTableScan.java | 8 +-- .../sql/analysis/AnalyzerTestBase.java | 19 ++++++- .../datasource/DataSourceServiceImplTest.java | 53 ++++++++++++++----- .../datasource/DataSourceTableScanTest.java | 17 ++++-- .../org/opensearch/sql/ppl/StandaloneIT.java | 2 +- .../org/opensearch/sql/plugin/SQLPlugin.java | 6 +-- .../datasource/DataSourceMetaDataTest.java | 4 +- 12 files changed, 170 insertions(+), 44 deletions(-) diff --git a/.gitignore b/.gitignore index 2e78185969..46d984ce4f 100644 --- a/.gitignore +++ b/.gitignore @@ -46,3 +46,4 @@ gen /.prom.pid.lock .java-version +.worktrees \ No newline at end of file diff --git a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java index 472f038981..6e87e5d663 100644 --- a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java +++ b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java @@ -64,7 +64,6 @@ import org.opensearch.sql.data.model.ExprMissingValue; import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.datasource.DataSourceService; -import org.opensearch.sql.datasource.model.DataSource; import org.opensearch.sql.datasource.model.DataSourceMetadata; import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.expression.DSL; @@ -135,7 +134,7 @@ public LogicalPlan analyze(UnresolvedPlan unresolved, AnalysisContext context) { @Override public LogicalPlan visitRelation(Relation node, AnalysisContext context) { QualifiedName qualifiedName = node.getTableQualifiedName(); - Set allowedDataSourceNames = dataSourceService.getMaskedDataSourceMetadataSet() + Set allowedDataSourceNames = dataSourceService.getDataSourceMetadataSet() .stream() .map(DataSourceMetadata::getName) .collect(Collectors.toSet()); @@ -183,7 +182,7 @@ public LogicalPlan visitRelationSubquery(RelationSubquery node, AnalysisContext @Override public LogicalPlan visitTableFunction(TableFunction node, AnalysisContext context) { QualifiedName qualifiedName = node.getFunctionName(); - Set allowedDataSourceNames = dataSourceService.getMaskedDataSourceMetadataSet() + Set allowedDataSourceNames = dataSourceService.getDataSourceMetadataSet() .stream() .map(DataSourceMetadata::getName) .collect(Collectors.toSet()); diff --git a/core/src/main/java/org/opensearch/sql/datasource/DataSourceMetadataStorage.java b/core/src/main/java/org/opensearch/sql/datasource/DataSourceMetadataStorage.java index ed5b5770d0..85ffd0a1b3 100644 --- a/core/src/main/java/org/opensearch/sql/datasource/DataSourceMetadataStorage.java +++ b/core/src/main/java/org/opensearch/sql/datasource/DataSourceMetadataStorage.java @@ -9,17 +9,56 @@ import java.util.List; import java.util.Optional; +import javax.xml.crypto.Data; +import org.opensearch.sql.datasource.model.DataSource; import org.opensearch.sql.datasource.model.DataSourceMetadata; /** - * Interface for DataSourceMetadata Storage. + * Interface for DataSourceMetadata Storage + * which will be only used by DataSourceService for Storage. */ public interface DataSourceMetadataStorage { + /** + * Returns all dataSource Metadata objects. The returned objects won't contain + * any of the credential info. + * + * @return list of {@link DataSourceMetadata}. + */ List getDataSourceMetadata(); + + /** + * Gets {@link DataSourceMetadata} corresponding to the + * datasourceName from underlying storage. + * + * @param datasourceName name of the {@link DataSource}. + */ Optional getDataSourceMetadata(String datasourceName); + + /** + * Stores {@link DataSourceMetadata} in underlying storage. + * + * @param dataSourceMetadata {@link DataSourceMetadata}. + */ void createDataSourceMetadata(DataSourceMetadata dataSourceMetadata); + + /** + * Updates {@link DataSourceMetadata} in underlying storage. + * + * @param dataSourceMetadata {@link DataSourceMetadata}. + */ + void updateDataSourceMetadata(DataSourceMetadata dataSourceMetadata); + + + /** + * Deletes {@link DataSourceMetadata} corresponding to the + * datasourceName from underlying storage. + * + * @param datasourceName name of the {@link DataSource}. + */ + void deleteDataSourceMetadata(String datasourceName); + } \ No newline at end of file diff --git a/core/src/main/java/org/opensearch/sql/datasource/DataSourceService.java b/core/src/main/java/org/opensearch/sql/datasource/DataSourceService.java index 3612a5edad..e45b9cd9c8 100644 --- a/core/src/main/java/org/opensearch/sql/datasource/DataSourceService.java +++ b/core/src/main/java/org/opensearch/sql/datasource/DataSourceService.java @@ -15,26 +15,49 @@ public interface DataSourceService { /** - * Returns all DataSource objects. + * Returns {@link DataSource} corresponding to the DataSource name. * - * @return set of {@link DataSource}. + * @param dataSourceName Name of the {@link DataSource}. + * @return {@link DataSource}. */ - Set getMaskedDataSourceMetadataSet(); + DataSource getDataSource(String dataSourceName); + /** - * Returns {@link DataSource} with corresponding to the DataSource name. + * Returns all dataSource Metadata objects. The returned objects won't contain + * any of the credential info. * - * @param dataSourceName Name of the {@link DataSource}. - * @return {@link DataSource}. + * @return set of {@link DataSourceMetadata}. */ - DataSource getDataSource(String dataSourceName); + Set getDataSourceMetadataSet(); /** * Register {@link DataSource} defined by {@link DataSourceMetadata}. * * @param metadatas list of {@link DataSourceMetadata}. */ - void addDataSource(DataSourceMetadata... metadatas); + void createDataSource(DataSourceMetadata... metadatas); + + /** + * Updates {@link DataSource} corresponding to dataSourceMetadata. + * + * @param dataSourceMetadata {@link DataSourceMetadata}. + */ + void updateDataSource(DataSourceMetadata dataSourceMetadata); + + + /** + * Deletes {@link DataSource} corresponding to the DataSource name. + * + * @param dataSourceName name of the {@link DataSource}. + */ + void deleteDataSource(String dataSourceName); + + /** + * This method is to bootstrap + * datasources during the startup of the plugin. + */ + void bootstrapDataSources(); /** * remove all the registered {@link DataSource}. diff --git a/core/src/main/java/org/opensearch/sql/datasource/DataSourceServiceImpl.java b/core/src/main/java/org/opensearch/sql/datasource/DataSourceServiceImpl.java index f39c4751fe..915e5aa287 100644 --- a/core/src/main/java/org/opensearch/sql/datasource/DataSourceServiceImpl.java +++ b/core/src/main/java/org/opensearch/sql/datasource/DataSourceServiceImpl.java @@ -47,7 +47,7 @@ public DataSourceServiceImpl(Set dataSourceFactories) { } @Override - public Set getMaskedDataSourceMetadataSet() { + public Set getDataSourceMetadataSet() { return dataSourceMap.values().stream() .map(dataSource -> new DataSourceMetadata(dataSource.getName(), @@ -65,7 +65,7 @@ public DataSource getDataSource(String dataSourceName) { } @Override - public void addDataSource(DataSourceMetadata... metadatas) { + public void createDataSource(DataSourceMetadata... metadatas) { for (DataSourceMetadata metadata : metadatas) { validateDataSourceMetaData(metadata); dataSourceMap.put( @@ -74,6 +74,21 @@ public void addDataSource(DataSourceMetadata... metadatas) { } } + @Override + public void updateDataSource(DataSourceMetadata dataSourceMetadata) { + throw new UnsupportedOperationException("will be supported in future"); + } + + @Override + public void deleteDataSource(String dataSourceName) { + throw new UnsupportedOperationException("will be supported in future"); + } + + @Override + public void bootstrapDataSources() { + throw new UnsupportedOperationException("will be supported in future"); + } + @Override public void clear() { dataSourceMap.clear(); diff --git a/core/src/main/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScan.java b/core/src/main/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScan.java index a628f678ef..60969f4d54 100644 --- a/core/src/main/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScan.java +++ b/core/src/main/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScan.java @@ -48,14 +48,14 @@ public String explain() { public void open() { List exprValues = new ArrayList<>(); Set dataSourceMetadataSet - = dataSourceService.getMaskedDataSourceMetadataSet(); - for (DataSourceMetadata dataSource : dataSourceMetadataSet) { + = dataSourceService.getDataSourceMetadataSet(); + for (DataSourceMetadata dataSourceMetadata : dataSourceMetadataSet) { exprValues.add( new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( "DATASOURCE_NAME", - ExprValueUtils.stringValue(dataSource.getName()), + ExprValueUtils.stringValue(dataSourceMetadata.getName()), "CONNECTOR_TYPE", - ExprValueUtils.stringValue(dataSource.getConnector().name()))))); + ExprValueUtils.stringValue(dataSourceMetadata.getConnector().name()))))); } iterator = exprValues.iterator(); } diff --git a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTestBase.java b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTestBase.java index 7c9acfd0cc..ea5c7d54eb 100644 --- a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTestBase.java +++ b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTestBase.java @@ -199,7 +199,7 @@ private class DefaultDataSourceService implements DataSourceService { @Override - public Set getMaskedDataSourceMetadataSet() { + public Set getDataSourceMetadataSet() { return ImmutableSet.of(new DataSourceMetadata(dataSource.getName(), dataSource.getConnectorType(), ImmutableMap.of())); } @@ -210,10 +210,25 @@ public DataSource getDataSource(String dataSourceName) { } @Override - public void addDataSource(DataSourceMetadata... metadatas) { + public void createDataSource(DataSourceMetadata... metadatas) { throw new UnsupportedOperationException(); } + @Override + public void updateDataSource(DataSourceMetadata dataSourceMetadata) { + + } + + @Override + public void deleteDataSource(String dataSourceName) { + + } + + @Override + public void bootstrapDataSources() { + + } + @Override public void clear() { throw new UnsupportedOperationException(); diff --git a/core/src/test/java/org/opensearch/sql/datasource/DataSourceServiceImplTest.java b/core/src/test/java/org/opensearch/sql/datasource/DataSourceServiceImplTest.java index 5c1bf643e4..b623313c96 100644 --- a/core/src/test/java/org/opensearch/sql/datasource/DataSourceServiceImplTest.java +++ b/core/src/test/java/org/opensearch/sql/datasource/DataSourceServiceImplTest.java @@ -6,6 +6,7 @@ package org.opensearch.sql.datasource; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.lenient; @@ -65,7 +66,7 @@ public void clear() { @Test void getDataSourceSuccess() { - dataSourceService.addDataSource(DataSourceMetadata.defaultOpenSearchDataSourceMetadata()); + dataSourceService.createDataSource(DataSourceMetadata.defaultOpenSearchDataSourceMetadata()); assertEquals( new DataSource(DEFAULT_DATASOURCE_NAME, DataSourceType.OPENSEARCH, storageEngine), @@ -81,19 +82,21 @@ void getNotExistDataSourceShouldFail() { @Test void getAddDataSourcesShouldSuccess() { - assertEquals(0, dataSourceService.getMaskedDataSourceMetadataSet().size()); + assertEquals(0, dataSourceService.getDataSourceMetadataSet().size()); - dataSourceService.addDataSource(metadata(NAME, DataSourceType.OPENSEARCH, ImmutableMap.of())); - assertEquals(1, dataSourceService.getMaskedDataSourceMetadataSet().size()); + dataSourceService.createDataSource(metadata(NAME, + DataSourceType.OPENSEARCH, ImmutableMap.of())); + assertEquals(1, dataSourceService.getDataSourceMetadataSet().size()); } @Test void noDataSourceExistAfterClear() { - dataSourceService.addDataSource(metadata(NAME, DataSourceType.OPENSEARCH, ImmutableMap.of())); - assertEquals(1, dataSourceService.getMaskedDataSourceMetadataSet().size()); + dataSourceService.createDataSource(metadata(NAME, + DataSourceType.OPENSEARCH, ImmutableMap.of())); + assertEquals(1, dataSourceService.getDataSourceMetadataSet().size()); dataSourceService.clear(); - assertEquals(0, dataSourceService.getMaskedDataSourceMetadataSet().size()); + assertEquals(0, dataSourceService.getDataSourceMetadataSet().size()); } @Test @@ -102,7 +105,7 @@ void metaDataMissingNameShouldFail() { assertThrows( IllegalArgumentException.class, () -> - dataSourceService.addDataSource( + dataSourceService.createDataSource( metadata(null, DataSourceType.OPENSEARCH, ImmutableMap.of()))); assertEquals( "Missing Name Field from a DataSource. Name is a required parameter.", @@ -115,7 +118,7 @@ void metaDataHasIllegalDataSourceNameShouldFail() { assertThrows( IllegalArgumentException.class, () -> - dataSourceService.addDataSource( + dataSourceService.createDataSource( metadata("prometheus.test", DataSourceType.OPENSEARCH, ImmutableMap.of()))); assertEquals( "DataSource Name: prometheus.test contains illegal characters. " @@ -128,7 +131,8 @@ void metaDataMissingPropertiesShouldFail() { IllegalArgumentException exception = assertThrows( IllegalArgumentException.class, - () -> dataSourceService.addDataSource(metadata(NAME, DataSourceType.OPENSEARCH, null))); + () -> dataSourceService.createDataSource(metadata(NAME, + DataSourceType.OPENSEARCH, null))); assertEquals( "Missing properties field in catalog configuration. Properties are required parameters.", exception.getMessage()); @@ -136,18 +140,41 @@ void metaDataMissingPropertiesShouldFail() { @Test void metaDataHasDuplicateNameShouldFail() { - dataSourceService.addDataSource(metadata(NAME, DataSourceType.OPENSEARCH, ImmutableMap.of())); - assertEquals(1, dataSourceService.getMaskedDataSourceMetadataSet().size()); + dataSourceService.createDataSource(metadata(NAME, + DataSourceType.OPENSEARCH, ImmutableMap.of())); + assertEquals(1, dataSourceService.getDataSourceMetadataSet().size()); IllegalArgumentException exception = assertThrows( IllegalArgumentException.class, - () -> dataSourceService.addDataSource(metadata(NAME, DataSourceType.OPENSEARCH, null))); + () -> dataSourceService.createDataSource(metadata(NAME, + DataSourceType.OPENSEARCH, null))); assertEquals( String.format("Datasource name should be unique, Duplicate datasource found %s.", NAME), exception.getMessage()); } + @Test + void testUpdateDatasource() { + assertThrows( + UnsupportedOperationException.class, + () -> dataSourceService.updateDataSource(new DataSourceMetadata())); + } + + @Test + void testDeleteDatasource() { + assertThrows( + UnsupportedOperationException.class, + () -> dataSourceService.deleteDataSource(NAME)); + } + + @Test + void testLoadDatasource() { + assertThrows( + UnsupportedOperationException.class, + () -> dataSourceService.bootstrapDataSources()); + } + DataSourceMetadata metadata(String name, DataSourceType type, Map properties) { DataSourceMetadata dataSourceMetadata = new DataSourceMetadata(); dataSourceMetadata.setName(name); diff --git a/core/src/test/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScanTest.java b/core/src/test/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScanTest.java index 0bd5d60cdf..1c45807245 100644 --- a/core/src/test/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScanTest.java +++ b/core/src/test/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScanTest.java @@ -23,6 +23,7 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.opensearch.sql.data.model.ExprTupleValue; +import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.model.ExprValueUtils; import org.opensearch.sql.datasource.DataSourceService; import org.opensearch.sql.datasource.model.DataSource; @@ -59,17 +60,23 @@ void testIterator() { Set dataSourceMetadata = dataSourceSet.stream() .map(dataSource -> new DataSourceMetadata(dataSource.getName(), dataSource.getConnectorType(), ImmutableMap.of())).collect(Collectors.toSet()); - when(dataSourceService.getMaskedDataSourceMetadataSet()).thenReturn(dataSourceMetadata); + when(dataSourceService.getDataSourceMetadataSet()).thenReturn(dataSourceMetadata); assertFalse(dataSourceTableScan.hasNext()); dataSourceTableScan.open(); assertTrue(dataSourceTableScan.hasNext()); + Set exprTupleValues = new HashSet<>(); + while (dataSourceTableScan.hasNext()) { + exprTupleValues.add(dataSourceTableScan.next()); + } + + Set expectedExprTupleValues = new HashSet<>(); for (DataSource dataSource : dataSourceSet) { - assertEquals(new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( - "DATASOURCE_NAME", ExprValueUtils.stringValue(dataSource.getName()), - "CONNECTOR_TYPE", ExprValueUtils.stringValue(dataSource.getConnectorType().name())))), - dataSourceTableScan.next()); + expectedExprTupleValues.add(new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( + "DATASOURCE_NAME", ExprValueUtils.stringValue(dataSource.getName()), + "CONNECTOR_TYPE", ExprValueUtils.stringValue(dataSource.getConnectorType().name()))))); } + assertEquals(expectedExprTupleValues, exprTupleValues); } } diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/StandaloneIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/StandaloneIT.java index 33050e7200..1ec28f4193 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/StandaloneIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/StandaloneIT.java @@ -81,7 +81,7 @@ public void init() { new ImmutableSet.Builder() .add(new OpenSearchDataSourceFactory(client, defaultSettings())) .build()); - dataSourceService.addDataSource(defaultOpenSearchDataSourceMetadata()); + dataSourceService.createDataSource(defaultOpenSearchDataSourceMetadata()); context.registerBean(DataSourceService.class, () -> dataSourceService); context.register(StandaloneConfig.class); context.register(PPLServiceConfig.class); diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java index fab14966d8..1237a8bdee 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java @@ -167,7 +167,7 @@ public Collection createComponents( new OpenSearchNodeClient(this.client), pluginSettings)) .add(new PrometheusStorageFactory()) .build()); - dataSourceService.addDataSource(defaultOpenSearchDataSourceMetadata()); + dataSourceService.createDataSource(defaultOpenSearchDataSourceMetadata()); loadDataSources(dataSourceService, clusterService.getSettings()); LocalClusterState.state().setClusterService(clusterService); LocalClusterState.state().setPluginSettings((OpenSearchSettings) pluginSettings); @@ -221,7 +221,7 @@ public ScriptEngine getScriptEngine(Settings settings, Collection metadataList = objectMapper.readValue(inputStream, new TypeReference<>() {}); - dataSourceService.addDataSource(metadataList.toArray(new DataSourceMetadata[0])); + dataSourceService.createDataSource(metadataList.toArray(new DataSourceMetadata[0])); } catch (IOException e) { LOG.error( "DataSource Configuration File uploaded is malformed. Verify and re-upload.", e); diff --git a/plugin/src/test/java/org/opensearch/sql/plugin/datasource/DataSourceMetaDataTest.java b/plugin/src/test/java/org/opensearch/sql/plugin/datasource/DataSourceMetaDataTest.java index e7e4c677b6..3ccb1cd403 100644 --- a/plugin/src/test/java/org/opensearch/sql/plugin/datasource/DataSourceMetaDataTest.java +++ b/plugin/src/test/java/org/opensearch/sql/plugin/datasource/DataSourceMetaDataTest.java @@ -96,7 +96,7 @@ public void testLoadConnectorsWithMalformedJson() { Settings settings = getDataSourceSettings("malformed_datasources.json"); loadConnectors(settings); - verify(dataSourceService, never()).addDataSource(any()); + verify(dataSourceService, never()).createDataSource(any()); } private Settings getDataSourceSettings(String filename) throws URISyntaxException, IOException { @@ -114,7 +114,7 @@ void loadConnectors(Settings settings) { void verifyAddDataSourceWithMetadata(List metadataList) { ArgumentCaptor metadataCaptor = ArgumentCaptor.forClass(DataSourceMetadata[].class); - verify(dataSourceService, times(1)).addDataSource(metadataCaptor.capture()); + verify(dataSourceService, times(1)).createDataSource(metadataCaptor.capture()); List actualValues = Arrays.asList(metadataCaptor.getValue()); assertEquals(metadataList.size(), actualValues.size()); assertEquals(metadataList, actualValues);