Code Review followup: Use Optional<> and create a new Map for length

Created new class PhoneNumberRequiredLengths.java with a Map lookup for the
requiredlength of phone numbers in select countries that are well-known to
have uniform fixed-length phone number throughout. Country codes missing
will continue to use just the previous length-range algorithms.  Lookup
into this Map is integrated into PhoneNumberValidator.java, no longer
needed an overloaded constructor to provide this functionality.
This commit is contained in:
Chris Parker 2024-09-22 04:30:31 -04:00
parent 1f3cd6765f
commit 824997d337
No known key found for this signature in database
GPG Key ID: C924FE487A9DA755
2 changed files with 40 additions and 55 deletions

View File

@ -7,6 +7,8 @@ import lombok.Getter;
import javax.annotation.Nullable;
import java.util.Optional;
/**
* Performs lenient validation of international phone numbers, and transforms given
* input numbers into E.164 international form. The E.164 normalized phone number
@ -44,7 +46,7 @@ public class PhoneNumberValidator extends InputValidator {
* Required length of phone number (excluding country code).
*/
@Getter
private int requiredLength;
private Optional<Integer> requiredLength;
///////////////////////////////////////////////////////////////////////////////////////////
// Constructors
@ -59,11 +61,7 @@ public class PhoneNumberValidator extends InputValidator {
this.isoCountryCode = isoCountryCode;
this.callingCode = CountryCallingCodes.getCallingCode(isoCountryCode);
this.normalizedCallingCode = CountryCallingCodes.getNormalizedCallingCode(isoCountryCode);
}
public PhoneNumberValidator(String isoCountryCode, int requiredLength) {
this(isoCountryCode);
this.requiredLength = requiredLength;
this.requiredLength = Optional.ofNullable(PhoneNumberRequiredLengths.getRequiredLength(isoCountryCode));
}
///////////////////////////////////////////////////////////////////////////////////////////
@ -107,9 +105,11 @@ public class PhoneNumberValidator extends InputValidator {
}
// If the 'requiredLength' was set, there's still one more check to apply on the normalizedNumber itself
result = validateRequiredLength(normalizedPhoneNumber, normalizedCallingCode);
if (!result.isValid) {
normalizedPhoneNumber = null;
if (requiredLength.isPresent()) {
result = validateRequiredLength(normalizedPhoneNumber, normalizedCallingCode);
if (!result.isValid) {
normalizedPhoneNumber = null;
}
}
}
return result;
@ -180,9 +180,9 @@ public class PhoneNumberValidator extends InputValidator {
private ValidationResult validateRequiredLength(String normalizedPhoneNumber, String normalizedCallingCode) {
try {
return ((0 == requiredLength) || ((normalizedPhoneNumber.length() - normalizedCallingCode.length() - 1) == requiredLength))
return ((normalizedPhoneNumber.length() - normalizedCallingCode.length() - 1) == requiredLength.get())
? new ValidationResult(true)
: new ValidationResult(false, Res.get("validation.phone.incorrectLength", requiredLength));
: new ValidationResult(false, Res.get("validation.phone.incorrectLength", requiredLength.get()));
} catch (Throwable t) {
return new ValidationResult(false, Res.get("validation.invalidInput", t.getMessage()));
}

View File

@ -285,70 +285,35 @@ public class PhoneNumberValidatorTest {
validator = new PhoneNumberValidator("US");
assertEquals(validator.getCallingCode(), "1");
validationResult = validator.validate("1 (1) 253 0000");
assertTrue(validationResult.isValid);
assertEquals("+112530000", validator.getNormalizedPhoneNumber());
assertFalse(validationResult.isValid);
assertNull(validator.getNormalizedPhoneNumber());
validationResult = validator.validate("1-120-253 0000");
assertTrue(validationResult.isValid);
assertNull(validationResult.errorMessage);
assertEquals("+11202530000", validator.getNormalizedPhoneNumber());
validationResult = validator.validate("(120) 253 0000");
assertTrue(validationResult.isValid);
// TODO validator incorrectly treats input as if it were +1 (202) 53-0000
/// assertEquals("+1202530000", validator.getNormalizedPhoneNumber());
assertFalse(validationResult.isValid);
assertNull(validator.getNormalizedPhoneNumber());
}
@Test
public void testPhoneNumberLength() {
// Construct the validator to be used in these test cases
validator = new PhoneNumberValidator("US"); // No requiredLength passed in these tests
// Most important, does the validator work for correct phone numbers?
validationResult = validator.validate("+1 512 888 0150");
assertTrue(validationResult.isValid, "+1 512 888 0150 should be a valid number in US");
assertNotNull(validator.getNormalizedPhoneNumber());
assertEquals(validator.getNormalizedPhoneNumber(), "+15128880150");
// If no country code provided by user, normalized number should have it added
validationResult = validator.validate("5128880150");
assertTrue(validationResult.isValid, "5128880150 should be a valid number in US");
assertNotNull(validator.getNormalizedPhoneNumber());
assertEquals(validator.getNormalizedPhoneNumber(), "+15128880150");
// If no phone number too short, there's a message for that
validationResult = validator.validate("+15121");
assertFalse(validationResult.isValid, "+15121 should be too short");
assertNull(validator.getNormalizedPhoneNumber());
assertEquals(Res.get("validation.phone.insufficientDigits", "+15121"), validationResult.errorMessage);
// If no phone number too long, there's a message for that
validationResult = validator.validate("51288801505128880150");
assertFalse(validationResult.isValid, "51288801505128880150 should be too long");
assertNull(validator.getNormalizedPhoneNumber());
assertEquals(Res.get("validation.phone.tooManyDigits", "51288801505128880150"), validationResult.errorMessage);
// If phone number is not exactly the requiredLength, it still may be okay if length in the range 4-12
validationResult = validator.validate("123456");
assertTrue(validationResult.isValid, "123456 should be a considered a valid number");
assertNotNull(validator.getNormalizedPhoneNumber());
assertEquals(validator.getNormalizedPhoneNumber(), "+123456");
}
@Test
public void testPhoneNumberRequiredLength() {
// Construct the validator to be used in these test cases
final int REQUIRED_LENGTH = 10;
validator = new PhoneNumberValidator("US", REQUIRED_LENGTH); // requiredLength passed for these tests
validator = new PhoneNumberValidator("US"); // requiredLength=10 to pass these tests
// Most important, does the validator work for correct phone numbers?
validationResult = validator.validate("+1 512 888 0150");
assertTrue(validationResult.isValid, "+1 512 888 0150 should be a valid number in US");
assertNull(validationResult.errorMessage);
assertNotNull(validator.getNormalizedPhoneNumber());
assertEquals(validator.getNormalizedPhoneNumber(), "+15128880150");
// If no country code provided by user, normalized number should have it added
validationResult = validator.validate("5128880150");
assertTrue(validationResult.isValid, "5128880150 should be a valid number in US");
assertNull(validationResult.errorMessage);
assertNotNull(validator.getNormalizedPhoneNumber());
assertEquals(validator.getNormalizedPhoneNumber(), "+15128880150");
@ -368,7 +333,27 @@ public class PhoneNumberValidatorTest {
validationResult = validator.validate("+1 888 123 456");
assertFalse(validationResult.isValid, "+1 888 123 456 should not be a valid number in US");
assertNull(validator.getNormalizedPhoneNumber());
assertEquals(Res.get("validation.phone.incorrectLength", validator.getRequiredLength()), validationResult.errorMessage);
assertEquals(Res.get("validation.phone.incorrectLength", validator.getRequiredLength().get()), validationResult.errorMessage);
}
@Test
public void testVariablePhoneNumberLengthCountry() {
// Construct the validator to be used in these test cases
validator = new PhoneNumberValidator("KP"); // requiredLength not defined for this country (North Korea +850)
assertFalse(validator.getRequiredLength().isPresent()); // If this fails, find another country to test with
// If phone number requiredLength is not defined, it is considered okay if length in the range 4-12
validationResult = validator.validate("12345678");
assertTrue(validationResult.isValid, "12345678 should be a considered a valid number");
assertNull(validationResult.errorMessage);
assertNotNull(validator.getNormalizedPhoneNumber());
assertEquals(validator.getNormalizedPhoneNumber(), "+85012345678");
validationResult = validator.validate("12345678901234");
assertTrue(validationResult.isValid, "12345678901234 should be a considered a valid number");
assertNull(validationResult.errorMessage);
assertNotNull(validator.getNormalizedPhoneNumber());
assertEquals(validator.getNormalizedPhoneNumber(), "+85012345678901234");
}
}