You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "kyooosukedn (via GitHub)" <gi...@apache.org> on 2023/05/19 23:20:00 UTC
[GitHub] [shardingsphere] kyooosukedn opened a new pull request, #25801: [#24755] Improve property verification of MaskAlgorithm
kyooosukedn opened a new pull request, #25801:
URL: https://github.com/apache/shardingsphere/pull/25801
Fixes #24755 .
Changes proposed in this pull request:
- SubTask:
- [x] MaskBeforeSpecialCharsAlgorithm
- [x] MaskFirstNLastMMaskAlgorithm
- [x] MaskAfterSpecialCharsAlgorithm
- [x] MaskFromXToYMaskAlgorithm
- [x] KeepFromXToYMaskAlgorithm
- [x] MilitaryIdentityNumberRandomReplaceAlgorithm
- [x] UnifiedCreditCodeRandomReplaceAlgorithm
- [x] LandlineNumberRandomAlgorithm
- [x] PersonalIdentityNumberRandomReplaceAlgorithm
- [x] TelephoneRandomReplaceAlgorithm
- [x] GenericTableRandomReplaceAlgorithm
---
Before committing this PR, I'm sure that I have checked the following options:
- [x] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
- [x] I have self-reviewed the commit code.
- [x] I have (or in comment I request) added corresponding labels for the pull request.
- [x] I have passed maven check locally : `./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e`.
- [ ] I have made corresponding changes to the documentation.
- [x] I have added corresponding unit tests for my changes.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] RaigorJiang commented on a diff in pull request #25801: [#24755] Improve property verification of MaskAlgorithm
Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang commented on code in PR #25801:
URL: https://github.com/apache/shardingsphere/pull/25801#discussion_r1203506865
##########
features/mask/core/src/test/java/org/apache/shardingsphere/mask/algorithm/replace/LandlineNumberRandomAlgorithmTest.java:
##########
@@ -42,4 +44,16 @@ void assertMask() {
assertThat(maskAlgorithm.mask("0251234567"), not("0251234567"));
assertThat(maskAlgorithm.mask("03101234567"), not("03101234567"));
}
+
+ @Test
+ void assertInitWhenConfigIsEmpty() {
+ assertThrows(MaskAlgorithmInitializationException.class, () -> maskAlgorithm.init(PropertiesBuilder.build()));
+ }
+
+ @Test
+ void assertMaskWithInvalidConfig() {
+ assertThrows(MaskAlgorithmInitializationException.class,
+ () -> maskAlgorithm.init(PropertiesBuilder.build(new Property("landline-numbers", ""))));
+ }
+
Review Comment:
@Pace2Car Please help to fix style, thank you!
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] RaigorJiang commented on a diff in pull request #25801: [#24755] Improve property verification of MaskAlgorithm
Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang commented on code in PR #25801:
URL: https://github.com/apache/shardingsphere/pull/25801#discussion_r1203497288
##########
features/mask/core/src/test/java/org/apache/shardingsphere/mask/algorithm/replace/LandlineNumberRandomAlgorithmTest.java:
##########
@@ -42,4 +44,16 @@ void assertMask() {
assertThat(maskAlgorithm.mask("0251234567"), not("0251234567"));
assertThat(maskAlgorithm.mask("03101234567"), not("03101234567"));
}
+
+ @Test
+ void assertInitWhenConfigIsEmpty() {
+ assertThrows(MaskAlgorithmInitializationException.class, () -> maskAlgorithm.init(PropertiesBuilder.build()));
+ }
+
+ @Test
+ void assertMaskWithInvalidConfig() {
+ assertThrows(MaskAlgorithmInitializationException.class,
+ () -> maskAlgorithm.init(PropertiesBuilder.build(new Property("landline-numbers", ""))));
+ }
+
Review Comment:
Useless blank line.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] RaigorJiang merged pull request #25801: [#24755] Improve property verification of MaskAlgorithm
Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang merged PR #25801:
URL: https://github.com/apache/shardingsphere/pull/25801
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] Pace2Car commented on a diff in pull request #25801: [#24755] Improve property verification of MaskAlgorithm
Posted by "Pace2Car (via GitHub)" <gi...@apache.org>.
Pace2Car commented on code in PR #25801:
URL: https://github.com/apache/shardingsphere/pull/25801#discussion_r1199911834
##########
features/mask/core/src/main/java/org/apache/shardingsphere/mask/algorithm/MaskAlgorithmPropsChecker.java:
##########
@@ -55,30 +55,46 @@ public static void checkSingleCharConfig(final Properties props, final String si
* @throws MaskAlgorithmInitializationException mask algorithm initialization exception
*/
public static void checkAtLeastOneCharConfig(final Properties props, final String atLeastOneCharConfigKey, final String maskType) {
- if (!props.containsKey(atLeastOneCharConfigKey)) {
- throw new MaskAlgorithmInitializationException(maskType, String.format("%s can not be null", atLeastOneCharConfigKey));
- }
- if (0 == props.getProperty(atLeastOneCharConfigKey).length()) {
- throw new MaskAlgorithmInitializationException(maskType, String.format("%s's length must be at least one", atLeastOneCharConfigKey));
- }
+ ShardingSpherePreconditions.checkState(props.containsKey(atLeastOneCharConfigKey),
+ () -> new MaskAlgorithmInitializationException(maskType, String.format("%s can not be null", atLeastOneCharConfigKey)));
+ ShardingSpherePreconditions.checkState(props.getProperty(atLeastOneCharConfigKey).length() > 0,
+ () -> new MaskAlgorithmInitializationException(maskType, String.format("%s's length must be at least one", atLeastOneCharConfigKey)));
}
/**
- * Check integer type config.
+ * Check required property config.
*
* @param props props
- * @param integerTypeConfigKey integer type config key
+ * @param requiredPropertyConfigKey required property config key
* @param maskType mask type
* @throws MaskAlgorithmInitializationException mask algorithm initialization exception
*/
- public static void checkIntegerTypeConfig(final Properties props, final String integerTypeConfigKey, final String maskType) {
- if (!props.containsKey(integerTypeConfigKey)) {
- throw new MaskAlgorithmInitializationException(maskType, String.format("%s can not be null", integerTypeConfigKey));
+ public static void checkRequiredPropertyConfig(final Properties props, final String requiredPropertyConfigKey, final String maskType) {
+ if (!props.containsKey(requiredPropertyConfigKey)) {
+ throw new MaskAlgorithmInitializationException(maskType, String.format("%s is required", requiredPropertyConfigKey));
}
+ }
+
+ /**
+ * Check required property config.
+ *
+ * @param props props
+ * @param positiveIntegerTypeConfigKey positive integer type config key
+ * @param maskType mask type
+ * @throws MaskAlgorithmInitializationException mask algorithm initialization exception
+ */
+ public static void checkPositiveIntegerConfig(final Properties props, final String positiveIntegerTypeConfigKey, final String maskType) {
+ ShardingSpherePreconditions.checkState(props.containsKey(positiveIntegerTypeConfigKey),
+ () -> new MaskAlgorithmInitializationException(maskType, String.format("%s can not be null", positiveIntegerTypeConfigKey)));
try {
- Integer.parseInt(props.getProperty(integerTypeConfigKey));
- } catch (final NumberFormatException ignored) {
- throw new MaskAlgorithmInitializationException(maskType, String.format("%s must be a valid integer number", integerTypeConfigKey));
+ Integer.parseInt(props.getProperty(positiveIntegerTypeConfigKey));
+ } catch (final NumberFormatException ex) {
+ throw new MaskAlgorithmInitializationException(maskType, String.format("%s must be a valid integer number", positiveIntegerTypeConfigKey));
+ }
+ if (!Strings.isNullOrEmpty(positiveIntegerTypeConfigKey)) {
Review Comment:
Here it can be combined like this
```java
try {
int integerValue = Integer.parseInt(props.getProperty(positiveIntegerTypeConfigKey));
ShardingSpherePreconditions.checkState(integerValue > 0,
() -> new MaskAlgorithmInitializationException(maskType, String.format("%s must be a positive integer.", positiveIntegerTypeConfigKey)));
} catch (final NumberFormatException ex) {
throw new MaskAlgorithmInitializationException(maskType, String.format("%s must be a valid integer number", positiveIntegerTypeConfigKey));
}
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] RaigorJiang commented on a diff in pull request #25801: [#24755] Improve property verification of MaskAlgorithm
Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang commented on code in PR #25801:
URL: https://github.com/apache/shardingsphere/pull/25801#discussion_r1203500308
##########
features/mask/core/src/main/java/org/apache/shardingsphere/mask/algorithm/MaskAlgorithmPropsChecker.java:
##########
@@ -55,30 +54,43 @@ public static void checkSingleCharConfig(final Properties props, final String si
* @throws MaskAlgorithmInitializationException mask algorithm initialization exception
*/
public static void checkAtLeastOneCharConfig(final Properties props, final String atLeastOneCharConfigKey, final String maskType) {
- if (!props.containsKey(atLeastOneCharConfigKey)) {
- throw new MaskAlgorithmInitializationException(maskType, String.format("%s can not be null", atLeastOneCharConfigKey));
- }
- if (0 == props.getProperty(atLeastOneCharConfigKey).length()) {
- throw new MaskAlgorithmInitializationException(maskType, String.format("%s's length must be at least one", atLeastOneCharConfigKey));
- }
+ ShardingSpherePreconditions.checkState(props.containsKey(atLeastOneCharConfigKey),
+ () -> new MaskAlgorithmInitializationException(maskType, String.format("%s can not be null", atLeastOneCharConfigKey)));
+ ShardingSpherePreconditions.checkState(props.getProperty(atLeastOneCharConfigKey).length() > 0,
+ () -> new MaskAlgorithmInitializationException(maskType, String.format("%s's length must be at least one", atLeastOneCharConfigKey)));
}
/**
- * Check integer type config.
+ * Check required property config.
*
* @param props props
- * @param integerTypeConfigKey integer type config key
+ * @param requiredPropertyConfigKey required property config key
* @param maskType mask type
* @throws MaskAlgorithmInitializationException mask algorithm initialization exception
*/
- public static void checkIntegerTypeConfig(final Properties props, final String integerTypeConfigKey, final String maskType) {
- if (!props.containsKey(integerTypeConfigKey)) {
- throw new MaskAlgorithmInitializationException(maskType, String.format("%s can not be null", integerTypeConfigKey));
+ public static void checkRequiredPropertyConfig(final Properties props, final String requiredPropertyConfigKey, final String maskType) {
+ if (!props.containsKey(requiredPropertyConfigKey)) {
+ throw new MaskAlgorithmInitializationException(maskType, String.format("%s is required", requiredPropertyConfigKey));
}
+ }
+
+ /**
+ * Check required property config.
+ *
+ * @param props props
+ * @param positiveIntegerTypeConfigKey positive integer type config key
+ * @param maskType mask type
+ * @throws MaskAlgorithmInitializationException mask algorithm initialization exception
Review Comment:
`MaskAlgorithmInitializationException` is a `RuntimeException` that is not defined in the method declaration, so this `@throws` comment is not needed.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kyooosukedn commented on pull request #25801: [#24755] Improve property verification of MaskAlgorithm
Posted by "kyooosukedn (via GitHub)" <gi...@apache.org>.
kyooosukedn commented on PR #25801:
URL: https://github.com/apache/shardingsphere/pull/25801#issuecomment-1561978938
@Pace2Car Thanks for your help
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] Pace2Car commented on a diff in pull request #25801: [#24755] Improve property verification of MaskAlgorithm
Posted by "Pace2Car (via GitHub)" <gi...@apache.org>.
Pace2Car commented on code in PR #25801:
URL: https://github.com/apache/shardingsphere/pull/25801#discussion_r1199915299
##########
features/mask/core/src/main/java/org/apache/shardingsphere/mask/algorithm/replace/TelephoneRandomReplaceAlgorithm.java:
##########
@@ -50,7 +51,9 @@ public void init(final Properties props) {
}
private List<String> createNetworkNumbers(final Properties props) {
- String networkNumbers = props.containsKey(NETWORK_NUMBERS) && !Strings.isNullOrEmpty(props.getProperty(NETWORK_NUMBERS)) ? props.getProperty(NETWORK_NUMBERS) : initDefaultNetworkNumbers();
+ ShardingSpherePreconditions.checkState(!Strings.isNullOrEmpty(NETWORK_NUMBERS),
Review Comment:
Unnecessary check, just keep it as it is
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] Pace2Car commented on a diff in pull request #25801: [#24755] Improve property verification of MaskAlgorithm
Posted by "Pace2Car (via GitHub)" <gi...@apache.org>.
Pace2Car commented on code in PR #25801:
URL: https://github.com/apache/shardingsphere/pull/25801#discussion_r1200358051
##########
features/mask/core/src/main/java/org/apache/shardingsphere/mask/algorithm/replace/TelephoneRandomReplaceAlgorithm.java:
##########
@@ -50,7 +50,7 @@ public void init(final Properties props) {
}
private List<String> createNetworkNumbers(final Properties props) {
- String networkNumbers = props.containsKey(NETWORK_NUMBERS) && !Strings.isNullOrEmpty(props.getProperty(NETWORK_NUMBERS)) ? props.getProperty(NETWORK_NUMBERS) : initDefaultNetworkNumbers();
+ String networkNumbers = props.getProperty(NETWORK_NUMBERS, initDefaultNetworkNumbers());
Review Comment:
No need to modify here
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org