This is an automated email from the ASF dual-hosted git repository.

jianglongtao 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 6238dba6e99 Improve properties verification of ShardingAlgorithm 
(#27302)
6238dba6e99 is described below

commit 6238dba6e998770ea342fe033c65e54d4004b3b1
Author: ChenJiaHao <[email protected]>
AuthorDate: Thu Jul 20 14:15:07 2023 +0800

    Improve properties verification of ShardingAlgorithm (#27302)
    
    * Improve properties verification of ShardingAlgorithm
    
    * Fix code style
---
 .../sharding/classbased/ClassBasedShardingAlgorithm.java  | 15 ++++++++++-----
 .../sharding/datetime/IntervalShardingAlgorithm.java      | 10 ++++++----
 .../sharding/inline/ComplexInlineShardingAlgorithm.java   |  4 +++-
 .../sharding/inline/InlineShardingAlgorithm.java          |  6 ++++--
 .../classbased/ClassBasedShardingAlgorithmTest.java       |  9 ++++++++-
 .../inline/ComplexInlineShardingAlgorithmTest.java        | 14 ++++++++++++++
 .../sharding/inline/InlineShardingAlgorithmTest.java      | 13 +++++++++++++
 7 files changed, 58 insertions(+), 13 deletions(-)

diff --git 
a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/classbased/ClassBasedShardingAlgorithm.java
 
b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/classbased/ClassBasedShardingAlgorithm.java
index c6506df0940..da1e86daaf3 100644
--- 
a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/classbased/ClassBasedShardingAlgorithm.java
+++ 
b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/classbased/ClassBasedShardingAlgorithm.java
@@ -17,6 +17,7 @@
 
 package org.apache.shardingsphere.sharding.algorithm.sharding.classbased;
 
+import com.google.common.base.Strings;
 import 
org.apache.shardingsphere.infra.util.exception.ShardingSpherePreconditions;
 import 
org.apache.shardingsphere.sharding.api.sharding.complex.ComplexKeysShardingAlgorithm;
 import 
org.apache.shardingsphere.sharding.api.sharding.complex.ComplexKeysShardingValue;
@@ -27,6 +28,7 @@ import 
org.apache.shardingsphere.sharding.api.sharding.standard.RangeShardingVal
 import 
org.apache.shardingsphere.sharding.api.sharding.standard.StandardShardingAlgorithm;
 import 
org.apache.shardingsphere.sharding.exception.algorithm.sharding.ShardingAlgorithmInitializationException;
 
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Properties;
 
@@ -59,15 +61,18 @@ public final class ClassBasedShardingAlgorithm implements 
StandardShardingAlgori
     
     private ClassBasedShardingAlgorithmStrategyType getStrategy(final 
Properties props) {
         String strategy = props.getProperty(STRATEGY_KEY);
-        ShardingSpherePreconditions.checkNotNull(strategy,
-                () -> new ShardingAlgorithmInitializationException(getType(), 
String.format("Properties `%s` can not be null when uses class based sharding 
strategy.", STRATEGY_KEY)));
-        return 
ClassBasedShardingAlgorithmStrategyType.valueOf(strategy.toUpperCase().trim());
+        
ShardingSpherePreconditions.checkState(!Strings.isNullOrEmpty(strategy),
+                () -> new ShardingAlgorithmInitializationException(getType(), 
String.format("Properties `%s` can not be null or empty when uses class based 
sharding strategy", STRATEGY_KEY)));
+        String shardingAlgorithmStrategyType = strategy.toUpperCase().trim();
+        ShardingSpherePreconditions.checkState(
+                
Arrays.stream(ClassBasedShardingAlgorithmStrategyType.values()).anyMatch(each 
-> each.name().equals(shardingAlgorithmStrategyType)),
+                () -> new ShardingAlgorithmInitializationException(getType(), 
String.format("Unsupported sharding strategy `%s`", strategy)));
+        return 
ClassBasedShardingAlgorithmStrategyType.valueOf(shardingAlgorithmStrategyType);
     }
     
     private String getAlgorithmClassName(final Properties props) {
         String result = props.getProperty(ALGORITHM_CLASS_NAME_KEY);
-        ShardingSpherePreconditions.checkNotNull(result,
-                () -> new ShardingAlgorithmInitializationException(getType(), 
String.format("Properties `%s` can not be null when uses class based sharding 
strategy.", ALGORITHM_CLASS_NAME_KEY)));
+        ShardingSpherePreconditions.checkState(!Strings.isNullOrEmpty(result), 
() -> new ShardingAlgorithmInitializationException(getType(), "Sharding 
algorithm class name can not be null or empty"));
         return result;
     }
     
diff --git 
a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/datetime/IntervalShardingAlgorithm.java
 
b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/datetime/IntervalShardingAlgorithm.java
index 0274d5523e4..a232e4bc797 100644
--- 
a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/datetime/IntervalShardingAlgorithm.java
+++ 
b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/datetime/IntervalShardingAlgorithm.java
@@ -17,6 +17,7 @@
 
 package org.apache.shardingsphere.sharding.algorithm.sharding.datetime;
 
+import com.google.common.base.Strings;
 import com.google.common.collect.BoundType;
 import com.google.common.collect.Range;
 import 
org.apache.shardingsphere.infra.util.exception.ShardingSpherePreconditions;
@@ -94,7 +95,7 @@ public final class IntervalShardingAlgorithm implements 
StandardShardingAlgorith
     
     private String getDateTimePattern(final Properties props) {
         
ShardingSpherePreconditions.checkState(props.containsKey(DATE_TIME_PATTERN_KEY),
-                () -> new ShardingAlgorithmInitializationException(getType(), 
String.format("%s can not be null.", DATE_TIME_PATTERN_KEY)));
+                () -> new ShardingAlgorithmInitializationException(getType(), 
String.format("%s can not be null", DATE_TIME_PATTERN_KEY)));
         return props.getProperty(DATE_TIME_PATTERN_KEY);
     }
     
@@ -117,9 +118,10 @@ public final class IntervalShardingAlgorithm implements 
StandardShardingAlgorith
     }
     
     private DateTimeFormatter getTableSuffixPattern(final Properties props) {
-        
ShardingSpherePreconditions.checkState(props.containsKey(SHARDING_SUFFIX_FORMAT_KEY),
-                () -> new ShardingAlgorithmInitializationException(getType(), 
String.format("%s can not be null.", SHARDING_SUFFIX_FORMAT_KEY)));
-        return 
DateTimeFormatter.ofPattern(props.getProperty(SHARDING_SUFFIX_FORMAT_KEY));
+        String suffix = props.getProperty(SHARDING_SUFFIX_FORMAT_KEY);
+        ShardingSpherePreconditions.checkState(!Strings.isNullOrEmpty(suffix),
+                () -> new ShardingAlgorithmInitializationException(getType(), 
String.format("%s can not be null or empty.", SHARDING_SUFFIX_FORMAT_KEY)));
+        return DateTimeFormatter.ofPattern(suffix);
     }
     
     private ChronoUnit getStepUnit(final String stepUnit) {
diff --git 
a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/inline/ComplexInlineShardingAlgorithm.java
 
b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/inline/ComplexInlineShardingAlgorithm.java
index 3b1f2c5ca9e..89ded491e1d 100644
--- 
a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/inline/ComplexInlineShardingAlgorithm.java
+++ 
b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/inline/ComplexInlineShardingAlgorithm.java
@@ -17,6 +17,7 @@
 
 package org.apache.shardingsphere.sharding.algorithm.sharding.inline;
 
+import com.google.common.base.Strings;
 import groovy.lang.Closure;
 import groovy.util.Expando;
 import org.apache.shardingsphere.infra.expr.core.InlineExpressionParserFactory;
@@ -64,7 +65,8 @@ public final class ComplexInlineShardingAlgorithm implements 
ComplexKeysSharding
     
     private String getAlgorithmExpression(final Properties props) {
         String algorithmExpression = 
props.getProperty(ALGORITHM_EXPRESSION_KEY);
-        ShardingSpherePreconditions.checkNotNull(algorithmExpression, () -> 
new ShardingAlgorithmInitializationException(getType(), "Inline sharding 
algorithm expression can not be null."));
+        
ShardingSpherePreconditions.checkState(!Strings.isNullOrEmpty(algorithmExpression),
+                () -> new ShardingAlgorithmInitializationException(getType(), 
"Inline sharding algorithm expression can not be null."));
         return 
InlineExpressionParserFactory.newInstance().handlePlaceHolder(algorithmExpression.trim());
     }
     
diff --git 
a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/inline/InlineShardingAlgorithm.java
 
b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/inline/InlineShardingAlgorithm.java
index d92901855a6..1d8e813033e 100644
--- 
a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/inline/InlineShardingAlgorithm.java
+++ 
b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/inline/InlineShardingAlgorithm.java
@@ -17,6 +17,7 @@
 
 package org.apache.shardingsphere.sharding.algorithm.sharding.inline;
 
+import com.google.common.base.Strings;
 import groovy.lang.Closure;
 import groovy.lang.MissingMethodException;
 import groovy.util.Expando;
@@ -54,8 +55,9 @@ public final class InlineShardingAlgorithm implements 
StandardShardingAlgorithm<
     }
     
     private String getAlgorithmExpression(final Properties props) {
-        String expression = props.getProperty(ALGORITHM_EXPRESSION_KEY, "");
-        ShardingSpherePreconditions.checkState(!expression.isEmpty(), () -> 
new ShardingAlgorithmInitializationException(getType(), "Inline sharding 
algorithm expression cannot be null or empty"));
+        String expression = props.getProperty(ALGORITHM_EXPRESSION_KEY);
+        
ShardingSpherePreconditions.checkState(!Strings.isNullOrEmpty(expression),
+                () -> new ShardingAlgorithmInitializationException(getType(), 
"Inline sharding algorithm expression cannot be null or empty"));
         return 
InlineExpressionParserFactory.newInstance().handlePlaceHolder(expression.trim());
     }
     
diff --git 
a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/sharding/classbased/ClassBasedShardingAlgorithmTest.java
 
b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/sharding/classbased/ClassBasedShardingAlgorithmTest.java
index cf1f2b7a35a..f7c0627e763 100644
--- 
a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/sharding/classbased/ClassBasedShardingAlgorithmTest.java
+++ 
b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/sharding/classbased/ClassBasedShardingAlgorithmTest.java
@@ -50,7 +50,8 @@ class ClassBasedShardingAlgorithmTest {
     
     @Test
     void assertInitWithWrongStrategy() {
-        assertThrows(IllegalArgumentException.class, () -> 
TypedSPILoader.getService(ShardingAlgorithm.class, "CLASS_BASED", 
PropertiesBuilder.build(new Property("strategy", "wrong"))));
+        assertThrows(ShardingAlgorithmInitializationException.class,
+                () -> TypedSPILoader.getService(ShardingAlgorithm.class, 
"CLASS_BASED", PropertiesBuilder.build(new Property("strategy", "wrong"))));
     }
     
     @Test
@@ -59,6 +60,12 @@ class ClassBasedShardingAlgorithmTest {
                 () -> TypedSPILoader.getService(ShardingAlgorithm.class, 
"CLASS_BASED", PropertiesBuilder.build(new Property("strategy", "standard"))));
     }
     
+    @Test
+    void assertInitWithEmptyClassName() {
+        assertThrows(ShardingAlgorithmInitializationException.class,
+                () -> TypedSPILoader.getService(ShardingAlgorithm.class, 
"CLASS_BASED", PropertiesBuilder.build(new Property("strategy", "standard"), 
new Property("algorithmClassName", ""))));
+    }
+    
     @Test
     void assertInitWithUndefinedClass() {
         assertThrows(ClassNotFoundException.class,
diff --git 
a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/sharding/inline/ComplexInlineShardingAlgorithmTest.java
 
b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/sharding/inline/ComplexInlineShardingAlgorithmTest.java
index 508a37c5ee0..9cb915f6684 100644
--- 
a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/sharding/inline/ComplexInlineShardingAlgorithmTest.java
+++ 
b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/sharding/inline/ComplexInlineShardingAlgorithmTest.java
@@ -20,6 +20,7 @@ package 
org.apache.shardingsphere.sharding.algorithm.sharding.inline;
 import com.google.common.collect.Range;
 import org.apache.shardingsphere.infra.util.spi.type.typed.TypedSPILoader;
 import 
org.apache.shardingsphere.sharding.api.sharding.complex.ComplexKeysShardingValue;
+import 
org.apache.shardingsphere.sharding.exception.algorithm.sharding.ShardingAlgorithmInitializationException;
 import org.apache.shardingsphere.sharding.spi.ShardingAlgorithm;
 import org.apache.shardingsphere.test.util.PropertiesBuilder;
 import org.apache.shardingsphere.test.util.PropertiesBuilder.Property;
@@ -33,10 +34,23 @@ import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 class ComplexInlineShardingAlgorithmTest {
     
+    @Test
+    void assertInitWithNullClass() {
+        assertThrows(ShardingAlgorithmInitializationException.class,
+                () -> TypedSPILoader.getService(ShardingAlgorithm.class, 
"COMPLEX_INLINE", PropertiesBuilder.build(new Property("wrong", ""))));
+    }
+    
+    @Test
+    void assertInitWithEmptyClassName() {
+        assertThrows(ShardingAlgorithmInitializationException.class,
+                () -> TypedSPILoader.getService(ShardingAlgorithm.class, 
"COMPLEX_INLINE", PropertiesBuilder.build(new Property("algorithm-expression", 
""))));
+    }
+    
     @Test
     void assertDoSharding() {
         Properties props = PropertiesBuilder.build(new 
Property("algorithm-expression", "t_order_${type % 2}_${order_id % 2}"), new 
Property("sharding-columns", "type,order_id"));
diff --git 
a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/sharding/inline/InlineShardingAlgorithmTest.java
 
b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/sharding/inline/InlineShardingAlgorithmTest.java
index 72c837594ca..0a155acd540 100644
--- 
a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/sharding/inline/InlineShardingAlgorithmTest.java
+++ 
b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/sharding/inline/InlineShardingAlgorithmTest.java
@@ -24,6 +24,7 @@ import 
org.apache.shardingsphere.infra.util.spi.type.typed.TypedSPILoader;
 import 
org.apache.shardingsphere.sharding.api.sharding.standard.PreciseShardingValue;
 import 
org.apache.shardingsphere.sharding.api.sharding.standard.RangeShardingValue;
 import 
org.apache.shardingsphere.sharding.exception.algorithm.sharding.MismatchedInlineShardingAlgorithmExpressionAndColumnException;
+import 
org.apache.shardingsphere.sharding.exception.algorithm.sharding.ShardingAlgorithmInitializationException;
 import org.apache.shardingsphere.sharding.spi.ShardingAlgorithm;
 import org.apache.shardingsphere.test.util.PropertiesBuilder;
 import org.apache.shardingsphere.test.util.PropertiesBuilder.Property;
@@ -61,6 +62,18 @@ class InlineShardingAlgorithmTest {
                 PropertiesBuilder.build(new Property("algorithm-expression", 
"t_order_$->{(order_id % 4).abs()}"), new 
Property("allow-range-query-with-inline-sharding", Boolean.TRUE.toString())));
     }
     
+    @Test
+    void assertInitWithNullClass() {
+        assertThrows(ShardingAlgorithmInitializationException.class,
+                () -> TypedSPILoader.getService(ShardingAlgorithm.class, 
"INLINE", PropertiesBuilder.build(new Property("wrong", ""))));
+    }
+    
+    @Test
+    void assertInitWithEmptyClassName() {
+        assertThrows(ShardingAlgorithmInitializationException.class,
+                () -> TypedSPILoader.getService(ShardingAlgorithm.class, 
"INLINE", PropertiesBuilder.build(new Property("algorithm-expression", ""))));
+    }
+    
     @Test
     void assertDoSharding() {
         List<String> availableTargetNames = Arrays.asList("t_order_0", 
"t_order_1", "t_order_2", "t_order_3");

Reply via email to