tristaZero commented on a change in pull request #10206:
URL: https://github.com/apache/shardingsphere/pull/10206#discussion_r621064913
##########
File path:
shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/metadata/schema/refresher/type/CreateTableStatementSchemaRefresherTest.java
##########
@@ -31,90 +36,79 @@
import
org.apache.shardingsphere.sql.parser.sql.dialect.statement.sqlserver.ddl.SQLServerCreateTableStatement;
import org.junit.Test;
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
public final class CreateTableStatementSchemaRefresherTest {
+ private SchemaBuilderMaterials materials =
mock(SchemaBuilderMaterials.class);
+
@Test
public void refreshForMySQL() throws SQLException {
MySQLCreateTableStatement createTableStatement = new
MySQLCreateTableStatement();
createTableStatement.setNotExisted(false);
+ when(materials.getDatabaseType()).thenReturn(new MySQLDatabaseType());
refresh(createTableStatement);
}
@Test
public void refreshForOracle() throws SQLException {
OracleCreateTableStatement createTableStatement = new
OracleCreateTableStatement();
+ when(materials.getDatabaseType()).thenReturn(new OracleDatabaseType());
refresh(createTableStatement);
}
@Test
public void refreshForPostgreSQL() throws SQLException {
PostgreSQLCreateTableStatement createTableStatement = new
PostgreSQLCreateTableStatement();
createTableStatement.setNotExisted(false);
+ when(materials.getDatabaseType()).thenReturn(new
PostgreSQLDatabaseType());
refresh(createTableStatement);
}
@Test
public void refreshForSQL92() throws SQLException {
SQL92CreateTableStatement createTableStatement = new
SQL92CreateTableStatement();
+ when(materials.getDatabaseType()).thenReturn(new SQL92DatabaseType());
refresh(createTableStatement);
}
@Test
public void refreshForSQLServer() throws SQLException {
SQLServerCreateTableStatement createTableStatement = new
SQLServerCreateTableStatement();
+ when(materials.getDatabaseType()).thenReturn(new
SQLServerDatabaseType());
refresh(createTableStatement);
}
+ // TODO add more tests for tables with table rule
private void refresh(final CreateTableStatement createTableStatement)
throws SQLException {
- ShardingSphereSchema schema =
ShardingSphereSchemaBuildUtil.buildSchema();
createTableStatement.setTable(new SimpleTableSegment(new
TableNameSegment(1, 3, new IdentifierValue("t_order_0"))));
+ Map<String, DataSource> dataSourceMap = mock(HashMap.class);
+ when(materials.getDataSourceMap()).thenReturn(dataSourceMap);
+ DataSource dataSource = mock(DataSource.class);
+ when(dataSourceMap.get(eq("ds"))).thenReturn(dataSource);
+ Connection connection = mock(Connection.class);
+ when(dataSource.getConnection()).thenReturn(connection);
+ DatabaseMetaData metaData = mock(DatabaseMetaData.class);
+ when(connection.getMetaData()).thenReturn(metaData);
+ ResultSet resultSet = mock(ResultSet.class);
+ when(metaData.getTables(any(), any(), any(),
any())).thenReturn(resultSet);
+ when(resultSet.next()).thenReturn(false);
+ ShardingSphereSchema schema =
ShardingSphereSchemaBuildUtil.buildSchema();
SchemaRefresher<CreateTableStatement> schemaRefresher = new
CreateTableStatementSchemaRefresher();
- SchemaBuilderMaterials materials = mock(SchemaBuilderMaterials.class);
schemaRefresher.refresh(schema, Collections.singleton("ds"),
createTableStatement, materials);
assertTrue(schema.containsTable("t_order_0"));
}
-
- @Test
- public void refreshWithUnConfiguredForMySQL() throws SQLException {
- MySQLCreateTableStatement createTableStatement = new
MySQLCreateTableStatement();
- createTableStatement.setNotExisted(false);
- refreshWithUnConfigured(createTableStatement);
- }
-
- @Test
- public void refreshWithUnConfiguredForOracle() throws SQLException {
- OracleCreateTableStatement createTableStatement = new
OracleCreateTableStatement();
- refreshWithUnConfigured(createTableStatement);
- }
-
- @Test
- public void refreshWithUnConfiguredForPostgreSQL() throws SQLException {
- PostgreSQLCreateTableStatement createTableStatement = new
PostgreSQLCreateTableStatement();
- createTableStatement.setNotExisted(false);
- refreshWithUnConfigured(createTableStatement);
- }
-
- @Test
- public void refreshWithUnConfiguredForSQL92() throws SQLException {
- SQL92CreateTableStatement createTableStatement = new
SQL92CreateTableStatement();
- refreshWithUnConfigured(createTableStatement);
- }
-
- @Test
- public void refreshWithUnConfiguredForSQLServer() throws SQLException {
- SQLServerCreateTableStatement createTableStatement = new
SQLServerCreateTableStatement();
Review comment:
Do we add a UT to cover the branch of
`!containsInTableContainedRule(tableName, materials)` ?
##########
File path:
shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/refresher/type/AlterTableStatementSchemaRefresher.java
##########
@@ -38,7 +39,8 @@ public void refresh(final ShardingSphereSchema schema, final
Collection<String>
final AlterTableStatement sqlStatement, final
SchemaBuilderMaterials materials) throws SQLException {
String tableName =
sqlStatement.getTable().getTableName().getIdentifier().getValue();
if (!containsInTableContainedRule(tableName, materials)) {
- return;
+
TableMetaDataLoader.load(materials.getDataSourceMap().get(routeDataSourceNames.iterator().next()),
+ tableName,
materials.getDatabaseType()).ifPresent(tableMetaData -> schema.put(tableName,
tableMetaData));
}
if (null != schema && schema.containsTable(tableName)) {
TableMetaDataBuilder.build(tableName,
materials).ifPresent(tableMetaData -> schema.put(tableName, tableMetaData));
Review comment:
Do you think `if() {} else if() {}` is more appropriate?
##########
File path:
shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/refresher/type/CreateTableStatementSchemaRefresher.java
##########
@@ -44,7 +45,7 @@ public void refresh(final ShardingSphereSchema schema, final
Collection<String>
if (containsInTableContainedRule(tableName, materials)) {
tableMetaData = TableMetaDataBuilder.build(tableName,
materials).orElse(new TableMetaData());
} else {
- tableMetaData = new TableMetaData();
+ tableMetaData =
TableMetaDataLoader.load(materials.getDataSourceMap().get(routeDataSourceNames.iterator().next()),
tableName, materials.getDatabaseType()).orElse(new TableMetaData());
Review comment:
Is it necessary to check `routeDataSourceNames.size()`
##########
File path:
shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/refresher/type/AlterTableStatementSchemaRefresher.java
##########
@@ -38,7 +39,8 @@ public void refresh(final ShardingSphereSchema schema, final
Collection<String>
final AlterTableStatement sqlStatement, final
SchemaBuilderMaterials materials) throws SQLException {
String tableName =
sqlStatement.getTable().getTableName().getIdentifier().getValue();
if (!containsInTableContainedRule(tableName, materials)) {
- return;
+
TableMetaDataLoader.load(materials.getDataSourceMap().get(routeDataSourceNames.iterator().next()),
+ tableName,
materials.getDatabaseType()).ifPresent(tableMetaData -> schema.put(tableName,
tableMetaData));
}
if (null != schema && schema.containsTable(tableName)) {
TableMetaDataBuilder.build(tableName,
materials).ifPresent(tableMetaData -> schema.put(tableName, tableMetaData));
Review comment:
Beside, we still need to give a check on `routeDataSourceNames.size()`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
