This is an automated email from the ASF dual-hosted git repository. zhaojinchao pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/shardingsphere.git
The following commit(s) were added to refs/heads/master by this push: new 14a3f1e9245 Refactor SingleTableSegment (#32953) 14a3f1e9245 is described below commit 14a3f1e9245aef78b38a235d4425f2ad4f146bee Author: Liang Zhang <zhangli...@apache.org> AuthorDate: Sun Sep 22 12:38:24 2024 +0800 Refactor SingleTableSegment (#32953) * Refactor SingleTableSegment * Refactor SingleTableSegment * Refactor SingleTableSegment * Add more test cases on SingleTableDataNodeLoader * Add more test cases on SingleTableDataNodeLoader --- .../datanode/SingleTableDataNodeLoaderTest.java | 18 ++++-- .../handler/update/LoadSingleTableExecutor.java | 4 +- .../update/LoadSingleTableExecutorTest.java | 10 ++-- .../parser/core/SingleDistSQLStatementVisitor.java | 6 +- .../single/distsql/segment/SingleTableSegment.java | 65 ++++++++++++---------- .../distsql/segment/SingleTableSegmentTest.java | 48 ++++++++++++++++ 6 files changed, 107 insertions(+), 44 deletions(-) diff --git a/kernel/single/core/src/test/java/org/apache/shardingsphere/single/datanode/SingleTableDataNodeLoaderTest.java b/kernel/single/core/src/test/java/org/apache/shardingsphere/single/datanode/SingleTableDataNodeLoaderTest.java index 927baa2ac3d..b223fb979d6 100644 --- a/kernel/single/core/src/test/java/org/apache/shardingsphere/single/datanode/SingleTableDataNodeLoaderTest.java +++ b/kernel/single/core/src/test/java/org/apache/shardingsphere/single/datanode/SingleTableDataNodeLoaderTest.java @@ -64,14 +64,11 @@ class SingleTableDataNodeLoaderTest { private Map<String, DataSource> dataSourceMap; - private Collection<String> configuredSingleTables; - @BeforeEach void setUp() throws SQLException { dataSourceMap = new LinkedHashMap<>(2, 1F); dataSourceMap.put("ds0", mockDataSource("ds0", Arrays.asList("employee", "dept", "salary"))); dataSourceMap.put("ds1", mockDataSource("ds1", Arrays.asList("student", "teacher", "class", "salary"))); - configuredSingleTables = Collections.singletonList("*.*"); } private DataSource mockDataSource(final String dataSourceName, final List<String> tableNames) throws SQLException { @@ -100,7 +97,8 @@ class SingleTableDataNodeLoaderTest { TableMapperRuleAttribute ruleAttribute = mock(TableMapperRuleAttribute.class, RETURNS_DEEP_STUBS); when(ruleAttribute.getDistributedTableNames()).thenReturn(Arrays.asList("salary", "employee", "student")); when(builtRule.getAttributes()).thenReturn(new RuleAttributes(ruleAttribute)); - Map<String, Collection<DataNode>> actual = SingleTableDataNodeLoader.load(DefaultDatabase.LOGIC_NAME, databaseType, dataSourceMap, Collections.singleton(builtRule), configuredSingleTables); + Map<String, Collection<DataNode>> actual = SingleTableDataNodeLoader.load( + DefaultDatabase.LOGIC_NAME, databaseType, dataSourceMap, Collections.singleton(builtRule), Collections.singleton("*.*")); assertFalse(actual.containsKey("employee")); assertFalse(actual.containsKey("salary")); assertFalse(actual.containsKey("student")); @@ -114,7 +112,7 @@ class SingleTableDataNodeLoaderTest { @Test void assertLoadWithConflictTables() { - Map<String, Collection<DataNode>> actual = SingleTableDataNodeLoader.load(DefaultDatabase.LOGIC_NAME, databaseType, dataSourceMap, Collections.emptyList(), configuredSingleTables); + Map<String, Collection<DataNode>> actual = SingleTableDataNodeLoader.load(DefaultDatabase.LOGIC_NAME, databaseType, dataSourceMap, Collections.emptyList(), Collections.singleton("*.*.*")); assertTrue(actual.containsKey("employee")); assertTrue(actual.containsKey("salary")); assertTrue(actual.containsKey("student")); @@ -128,4 +126,14 @@ class SingleTableDataNodeLoaderTest { assertThat(actual.get("teacher").iterator().next().getDataSourceName(), is("ds1")); assertThat(actual.get("class").iterator().next().getDataSourceName(), is("ds1")); } + + @Test + void assertLoadWithEmptyConfiguredTables() { + ShardingSphereRule builtRule = mock(ShardingSphereRule.class); + TableMapperRuleAttribute ruleAttribute = mock(TableMapperRuleAttribute.class, RETURNS_DEEP_STUBS); + when(ruleAttribute.getDistributedTableNames()).thenReturn(Arrays.asList("salary", "employee", "student")); + when(builtRule.getAttributes()).thenReturn(new RuleAttributes(ruleAttribute)); + Map<String, Collection<DataNode>> actual = SingleTableDataNodeLoader.load(DefaultDatabase.LOGIC_NAME, databaseType, dataSourceMap, Collections.singleton(builtRule), Collections.emptyList()); + assertTrue(actual.isEmpty()); + } } diff --git a/kernel/single/distsql/handler/src/main/java/org/apache/shardingsphere/single/distsql/handler/update/LoadSingleTableExecutor.java b/kernel/single/distsql/handler/src/main/java/org/apache/shardingsphere/single/distsql/handler/update/LoadSingleTableExecutor.java index e01d3c86980..a04e870f81e 100644 --- a/kernel/single/distsql/handler/src/main/java/org/apache/shardingsphere/single/distsql/handler/update/LoadSingleTableExecutor.java +++ b/kernel/single/distsql/handler/src/main/java/org/apache/shardingsphere/single/distsql/handler/update/LoadSingleTableExecutor.java @@ -87,10 +87,10 @@ public final class LoadSingleTableExecutor implements DatabaseRuleCreateExecutor return; } if (isSchemaSupportedDatabaseType) { - ShardingSpherePreconditions.checkState(singleTableSegment.getSchemaName().isPresent(), + ShardingSpherePreconditions.checkState(singleTableSegment.containsSchema(), () -> new InvalidDataNodeFormatException(singleTableSegment.toString(), "Current database is schema required, please use format `db.schema.table`")); } else { - ShardingSpherePreconditions.checkState(!singleTableSegment.getSchemaName().isPresent(), + ShardingSpherePreconditions.checkState(!singleTableSegment.containsSchema(), () -> new InvalidDataNodeFormatException(singleTableSegment.toString(), "Current database does not support schema, please use format `db.table`")); } } diff --git a/kernel/single/distsql/handler/src/test/java/org/apache/shardingsphere/single/distsql/handler/update/LoadSingleTableExecutorTest.java b/kernel/single/distsql/handler/src/test/java/org/apache/shardingsphere/single/distsql/handler/update/LoadSingleTableExecutorTest.java index 94f930e0262..5a90b41f4af 100644 --- a/kernel/single/distsql/handler/src/test/java/org/apache/shardingsphere/single/distsql/handler/update/LoadSingleTableExecutorTest.java +++ b/kernel/single/distsql/handler/src/test/java/org/apache/shardingsphere/single/distsql/handler/update/LoadSingleTableExecutorTest.java @@ -71,7 +71,7 @@ class LoadSingleTableExecutorTest { when(database.getName()).thenReturn("foo_db"); when(schema.containsTable("foo")).thenReturn(true); when(database.getResourceMetaData().getNotExistedDataSources(any())).thenReturn(Collections.emptyList()); - LoadSingleTableStatement sqlStatement = new LoadSingleTableStatement(Collections.singleton(new SingleTableSegment("ds_0", null, "foo"))); + LoadSingleTableStatement sqlStatement = new LoadSingleTableStatement(Collections.singleton(new SingleTableSegment("ds_0", "foo"))); executor.setDatabase(database); assertThrows(TableExistsException.class, () -> executor.checkBeforeUpdate(sqlStatement)); } @@ -81,7 +81,7 @@ class LoadSingleTableExecutorTest { when(database.getName()).thenReturn("foo_db"); when(database.getResourceMetaData().getStorageUnits().isEmpty()).thenReturn(true); executor.setDatabase(database); - LoadSingleTableStatement sqlStatement = new LoadSingleTableStatement(Collections.singleton(new SingleTableSegment("*", null, "*"))); + LoadSingleTableStatement sqlStatement = new LoadSingleTableStatement(Collections.singleton(new SingleTableSegment("*", "*"))); assertThrows(EmptyStorageUnitException.class, () -> executor.checkBeforeUpdate(sqlStatement)); } @@ -89,13 +89,13 @@ class LoadSingleTableExecutorTest { void assertCheckWithInvalidStorageUnit() { when(database.getName()).thenReturn("foo_db"); executor.setDatabase(database); - LoadSingleTableStatement sqlStatement = new LoadSingleTableStatement(Collections.singleton(new SingleTableSegment("ds_0", null, "foo"))); + LoadSingleTableStatement sqlStatement = new LoadSingleTableStatement(Collections.singleton(new SingleTableSegment("ds_0", "foo"))); assertThrows(MissingRequiredStorageUnitsException.class, () -> executor.checkBeforeUpdate(sqlStatement)); } @Test void assertBuild() { - LoadSingleTableStatement sqlStatement = new LoadSingleTableStatement(Collections.singletonList(new SingleTableSegment("ds_0", null, "foo"))); + LoadSingleTableStatement sqlStatement = new LoadSingleTableStatement(Collections.singletonList(new SingleTableSegment("ds_0", "foo"))); SingleRule rule = mock(SingleRule.class); when(rule.getConfiguration()).thenReturn(new SingleRuleConfiguration()); executor.setRule(rule); @@ -107,7 +107,7 @@ class LoadSingleTableExecutorTest { void assertUpdate() { Collection<String> currentTables = new LinkedList<>(Collections.singletonList("ds_0.foo")); SingleRuleConfiguration currentConfig = new SingleRuleConfiguration(currentTables, null); - LoadSingleTableStatement sqlStatement = new LoadSingleTableStatement(Collections.singletonList(new SingleTableSegment("ds_0", null, "bar"))); + LoadSingleTableStatement sqlStatement = new LoadSingleTableStatement(Collections.singletonList(new SingleTableSegment("ds_0", "bar"))); SingleRule rule = mock(SingleRule.class); when(rule.getConfiguration()).thenReturn(currentConfig); executor.setRule(rule); diff --git a/kernel/single/distsql/parser/src/main/java/org/apache/shardingsphere/single/distsql/parser/core/SingleDistSQLStatementVisitor.java b/kernel/single/distsql/parser/src/main/java/org/apache/shardingsphere/single/distsql/parser/core/SingleDistSQLStatementVisitor.java index 9f1b53fe3eb..391e17e261e 100644 --- a/kernel/single/distsql/parser/src/main/java/org/apache/shardingsphere/single/distsql/parser/core/SingleDistSQLStatementVisitor.java +++ b/kernel/single/distsql/parser/src/main/java/org/apache/shardingsphere/single/distsql/parser/core/SingleDistSQLStatementVisitor.java @@ -94,7 +94,7 @@ public final class SingleDistSQLStatementVisitor extends SingleDistSQLStatementB private SingleTableSegment getSingleTableSegment(final TableIdentifierContext ctx) { if (ctx instanceof AllTablesFromStorageUnitContext) { - return new SingleTableSegment(getIdentifierValue(((AllTablesFromStorageUnitContext) ctx).storageUnitName()), null, "*"); + return new SingleTableSegment(getIdentifierValue(((AllTablesFromStorageUnitContext) ctx).storageUnitName()), "*"); } if (ctx instanceof AllTablesFromSchemaContext) { AllTablesFromSchemaContext tableContext = (AllTablesFromSchemaContext) ctx; @@ -102,14 +102,14 @@ public final class SingleDistSQLStatementVisitor extends SingleDistSQLStatementB } if (ctx instanceof TableFromStorageUnitContext) { TableFromStorageUnitContext tableContext = (TableFromStorageUnitContext) ctx; - return new SingleTableSegment(getIdentifierValue(tableContext.storageUnitName()), null, getIdentifierValue(tableContext.tableName())); + return new SingleTableSegment(getIdentifierValue(tableContext.storageUnitName()), getIdentifierValue(tableContext.tableName())); } if (ctx instanceof TableFromSchemaContext) { TableFromSchemaContext tableContext = (TableFromSchemaContext) ctx; return new SingleTableSegment(getIdentifierValue(tableContext.storageUnitName()), getIdentifierValue(tableContext.schemaName()), getIdentifierValue(tableContext.tableName())); } if (ctx instanceof AllTablesContext) { - return new SingleTableSegment("*", null, "*"); + return new SingleTableSegment("*", "*"); } if (ctx instanceof AllSchamesAndTablesFromStorageUnitContext) { return new SingleTableSegment(getIdentifierValue(((AllSchamesAndTablesFromStorageUnitContext) ctx).storageUnitName()), "*", "*"); diff --git a/kernel/single/distsql/statement/src/main/java/org/apache/shardingsphere/single/distsql/segment/SingleTableSegment.java b/kernel/single/distsql/statement/src/main/java/org/apache/shardingsphere/single/distsql/segment/SingleTableSegment.java index 21629d49adb..cc6f7ada2a3 100644 --- a/kernel/single/distsql/statement/src/main/java/org/apache/shardingsphere/single/distsql/segment/SingleTableSegment.java +++ b/kernel/single/distsql/statement/src/main/java/org/apache/shardingsphere/single/distsql/segment/SingleTableSegment.java @@ -17,56 +17,63 @@ package org.apache.shardingsphere.single.distsql.segment; -import com.google.common.base.Objects; -import lombok.Getter; +import lombok.EqualsAndHashCode; import lombok.RequiredArgsConstructor; import org.apache.shardingsphere.distsql.segment.DistSQLSegment; - -import java.util.Optional; +import org.apache.shardingsphere.infra.metadata.caseinsensitive.CaseInsensitiveIdentifier; /** * Single table segment. */ @RequiredArgsConstructor -@Getter +@EqualsAndHashCode public final class SingleTableSegment implements DistSQLSegment { - private final String storageUnitName; + private final CaseInsensitiveIdentifier storageUnitName; + + private final CaseInsensitiveIdentifier schemaName; - private final String schemaName; + private final CaseInsensitiveIdentifier tableName; + + public SingleTableSegment(final String storageUnitName, final String tableName) { + this(storageUnitName, null, tableName); + } - private final String tableName; + public SingleTableSegment(final String storageUnitName, final String schemaName, final String tableName) { + this.storageUnitName = new CaseInsensitiveIdentifier(storageUnitName); + this.schemaName = null == schemaName ? null : new CaseInsensitiveIdentifier(schemaName); + this.tableName = new CaseInsensitiveIdentifier(tableName); + } /** - * Get schema name. + * Get storage unit name. * - * @return schema name + * @return storage unit name */ - public Optional<String> getSchemaName() { - return Optional.ofNullable(schemaName); + public String getStorageUnitName() { + return storageUnitName.toString(); } - @Override - public String toString() { - return null == schemaName ? storageUnitName + "." + tableName : storageUnitName + "." + schemaName + "." + tableName; + /** + * Get table name. + * + * @return table name + */ + public String getTableName() { + return tableName.toString(); } - @Override - public boolean equals(final Object object) { - if (this == object) { - return true; - } - if (null == object || getClass() != object.getClass()) { - return false; - } - SingleTableSegment segment = (SingleTableSegment) object; - return Objects.equal(storageUnitName.toUpperCase(), segment.storageUnitName.toUpperCase()) - && Objects.equal(tableName.toUpperCase(), segment.tableName.toUpperCase()) - && Objects.equal(null == schemaName ? null : schemaName.toUpperCase(), null == segment.schemaName ? null : segment.schemaName.toUpperCase()); + /** + * Whether to contain schema. + * + * @return contains schema or not + */ + public boolean containsSchema() { + return null != schemaName; } @Override - public int hashCode() { - return Objects.hashCode(storageUnitName.toUpperCase(), tableName.toUpperCase(), null == schemaName ? null : schemaName.toUpperCase()); + public String toString() { + return null == schemaName ? String.join(".", getStorageUnitName(), getTableName()) : String.join(".", getStorageUnitName(), schemaName.toString(), getTableName()); } } diff --git a/kernel/single/distsql/statement/src/test/java/org/apache/shardingsphere/single/distsql/segment/SingleTableSegmentTest.java b/kernel/single/distsql/statement/src/test/java/org/apache/shardingsphere/single/distsql/segment/SingleTableSegmentTest.java new file mode 100644 index 00000000000..7d9797d08dc --- /dev/null +++ b/kernel/single/distsql/statement/src/test/java/org/apache/shardingsphere/single/distsql/segment/SingleTableSegmentTest.java @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shardingsphere.single.distsql.segment; + +import org.junit.jupiter.api.Test; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class SingleTableSegmentTest { + + @Test + void assertContainsSchema() { + assertTrue(new SingleTableSegment("foo_ds", "foo_schema", "foo_tbl").containsSchema()); + } + + @Test + void assertDoesNotContainSchema() { + assertFalse(new SingleTableSegment("foo_ds", "foo_tbl").containsSchema()); + } + + @Test + void assertToStringWithoutSchemaName() { + assertThat(new SingleTableSegment("foo_ds", "foo_tbl").toString(), is("foo_ds.foo_tbl")); + } + + @Test + void assertToStringWithSchemaName() { + assertThat(new SingleTableSegment("foo_ds", "foo_schema", "foo_tbl").toString(), is("foo_ds.foo_schema.foo_tbl")); + } +}