This is an automated email from the ASF dual-hosted git repository.
zhangliang 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 71970658a15 Remove duplication within DataNode Constructor (#36238)
71970658a15 is described below
commit 71970658a1594bd819119798159bbd4c590dcd0f
Author: NeatGuyCoding <[email protected]>
AuthorDate: Mon Aug 11 16:52:25 2025 +0800
Remove duplication within DataNode Constructor (#36238)
* Remove duplication within DataNode Constructor
* Remove duplication within DataNode Constructor(spotless apply)
---
.../shardingsphere/infra/datanode/DataNode.java | 54 +++++++++++--
.../infra/datanode/DataNodeTest.java | 93 ++++++++++++++++++++++
2 files changed, 142 insertions(+), 5 deletions(-)
diff --git
a/infra/common/src/main/java/org/apache/shardingsphere/infra/datanode/DataNode.java
b/infra/common/src/main/java/org/apache/shardingsphere/infra/datanode/DataNode.java
index 2a0186c59a2..021064cde52 100644
---
a/infra/common/src/main/java/org/apache/shardingsphere/infra/datanode/DataNode.java
+++
b/infra/common/src/main/java/org/apache/shardingsphere/infra/datanode/DataNode.java
@@ -57,11 +57,14 @@ public final class DataNode {
* @param dataNode string of data node. use {@code .} to split data source
name and table name.
*/
public DataNode(final String dataNode) {
- // TODO remove duplicated splitting?
- boolean isIncludeInstance =
isActualDataNodesIncludedDataSourceInstance(dataNode);
- ShardingSpherePreconditions.checkState(isIncludeInstance ||
isValidDataNode(dataNode, 2), () -> new
InvalidDataNodeFormatException(dataNode));
- ShardingSpherePreconditions.checkState(!isIncludeInstance ||
isValidDataNode(dataNode, 3), () -> new
InvalidDataNodeFormatException(dataNode));
+ // Validate data node format first
+ validateDataNodeFormat(dataNode);
+
+ // Split only once
List<String> segments = Splitter.on(DELIMITER).splitToList(dataNode);
+
+ // Determine if instance is included and set fields accordingly
+ boolean isIncludeInstance = segments.size() == 3;
dataSourceName = isIncludeInstance ? segments.get(0) + DELIMITER +
segments.get(1) : segments.get(0);
tableName = segments.get(isIncludeInstance ? 2 : 1);
}
@@ -95,13 +98,54 @@ public final class DataNode {
}
private boolean isValidDataNode(final String dataNodeStr, final Integer
tier) {
- return dataNodeStr.contains(DELIMITER) && tier ==
Splitter.on(DELIMITER).omitEmptyStrings().splitToList(dataNodeStr).size();
+ if (!dataNodeStr.contains(DELIMITER)) {
+ return false;
+ }
+
+ // Check for leading or trailing delimiter
+ if (dataNodeStr.startsWith(DELIMITER) ||
dataNodeStr.endsWith(DELIMITER)) {
+ return false;
+ }
+
+ // Check for consecutive delimiters (which would create empty segments)
+ if (dataNodeStr.contains(DELIMITER + DELIMITER)) {
+ return false;
+ }
+
+ // Check for whitespace around delimiters
+ if (dataNodeStr.contains(" " + DELIMITER) ||
dataNodeStr.contains(DELIMITER + " ")) {
+ return false;
+ }
+
+ List<String> segments =
Splitter.on(DELIMITER).splitToList(dataNodeStr);
+
+ // Check if any segment is empty or contains only whitespace
+ for (String segment : segments) {
+ if (segment.trim().isEmpty()) {
+ return false;
+ }
+ }
+
+ return tier == segments.size();
}
private boolean isActualDataNodesIncludedDataSourceInstance(final String
actualDataNodes) {
return isValidDataNode(actualDataNodes, 3);
}
+ /**
+ * Validates the data node format based on its structure.
+ *
+ * @param dataNode the data node string to validate
+ * @throws InvalidDataNodeFormatException if the format is invalid
+ */
+ private void validateDataNodeFormat(final String dataNode) {
+ // Check if it's a valid 2-segment or 3-segment format
+ if (!isValidDataNode(dataNode, 2) && !isValidDataNode(dataNode, 3)) {
+ throw new InvalidDataNodeFormatException(dataNode);
+ }
+ }
+
/**
* Format data node as string with schema.
*
diff --git
a/infra/common/src/test/java/org/apache/shardingsphere/infra/datanode/DataNodeTest.java
b/infra/common/src/test/java/org/apache/shardingsphere/infra/datanode/DataNodeTest.java
index a2a38df80af..1dd5b9d5754 100644
---
a/infra/common/src/test/java/org/apache/shardingsphere/infra/datanode/DataNodeTest.java
+++
b/infra/common/src/test/java/org/apache/shardingsphere/infra/datanode/DataNodeTest.java
@@ -109,4 +109,97 @@ class DataNodeTest {
void assertToStringIncludeInstance() {
assertThat(new DataNode("ds_0.db_0.tbl_0").toString(),
is("DataNode(dataSourceName=ds_0.db_0, tableName=tbl_0, schemaName=null)"));
}
+
+ @Test
+ void assertNewDataNodeWithOnlyOneSegment() {
+ assertThrows(InvalidDataNodeFormatException.class, () -> new
DataNode("ds_0"));
+ }
+
+ @Test
+ void assertNewDataNodeWithFourSegments() {
+ assertThrows(InvalidDataNodeFormatException.class, () -> new
DataNode("ds_0.db_0.tbl_0.col_0"));
+ }
+
+ @Test
+ void assertNewDataNodeWithEmptySegments() {
+ assertThrows(InvalidDataNodeFormatException.class, () -> new
DataNode("ds_0..tbl_0"));
+ }
+
+ @Test
+ void assertNewDataNodeWithLeadingDelimiter() {
+ assertThrows(InvalidDataNodeFormatException.class, () -> new
DataNode(".ds_0.tbl_0"));
+ }
+
+ @Test
+ void assertNewDataNodeWithTrailingDelimiter() {
+ assertThrows(InvalidDataNodeFormatException.class, () -> new
DataNode("ds_0.tbl_0."));
+ }
+
+ @Test
+ void assertNewDataNodeWithMultipleConsecutiveDelimiters() {
+ assertThrows(InvalidDataNodeFormatException.class, () -> new
DataNode("ds_0..tbl_0"));
+ }
+
+ @Test
+ void assertNewDataNodeWithWhitespaceInSegments() {
+ assertThrows(InvalidDataNodeFormatException.class, () -> new
DataNode("ds_0 . tbl_0"));
+ }
+
+ @Test
+ void assertNewDataNodeWithSpecialCharacters() {
+ DataNode dataNode = new DataNode("ds-0.tbl_0");
+ assertThat(dataNode.getDataSourceName(), is("ds-0"));
+ assertThat(dataNode.getTableName(), is("tbl_0"));
+ }
+
+ @Test
+ void assertNewDataNodeWithUnderscores() {
+ DataNode dataNode = new DataNode("data_source_0.table_name_0");
+ assertThat(dataNode.getDataSourceName(), is("data_source_0"));
+ assertThat(dataNode.getTableName(), is("table_name_0"));
+ }
+
+ @Test
+ void assertNewDataNodeWithNumbers() {
+ DataNode dataNode = new DataNode("ds123.tbl456");
+ assertThat(dataNode.getDataSourceName(), is("ds123"));
+ assertThat(dataNode.getTableName(), is("tbl456"));
+ }
+
+ @Test
+ void assertNewDataNodeWithMixedFormat() {
+ DataNode dataNode = new DataNode("prod-db-01.schema_01.users");
+ assertThat(dataNode.getDataSourceName(), is("prod-db-01.schema_01"));
+ assertThat(dataNode.getTableName(), is("users"));
+ }
+
+ @Test
+ void assertNewDataNodeWithLongNames() {
+ String longDataSource =
"very_long_data_source_name_that_exceeds_normal_length";
+ String longTable = "very_long_table_name_that_exceeds_normal_length";
+ DataNode dataNode = new DataNode(longDataSource + "." + longTable);
+ assertThat(dataNode.getDataSourceName(), is(longDataSource));
+ assertThat(dataNode.getTableName(), is(longTable));
+ }
+
+ @Test
+ void assertNewDataNodeWithSingleCharacterNames() {
+ DataNode dataNode = new DataNode("a.b");
+ assertThat(dataNode.getDataSourceName(), is("a"));
+ assertThat(dataNode.getTableName(), is("b"));
+ }
+
+ @Test
+ void assertNewDataNodeWithInstanceFormat() {
+ DataNode dataNode = new DataNode("instance1.database1.table1");
+ assertThat(dataNode.getDataSourceName(), is("instance1.database1"));
+ assertThat(dataNode.getTableName(), is("table1"));
+ }
+
+ @Test
+ void assertNewDataNodeWithComplexInstanceFormat() {
+ DataNode dataNode = new DataNode("prod-cluster-01.mysql-master.users");
+ assertThat(dataNode.getDataSourceName(),
is("prod-cluster-01.mysql-master"));
+ assertThat(dataNode.getTableName(), is("users"));
+ }
}