You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@directory.apache.org by bd...@apache.org on 2023/01/11 01:26:24 UTC
[directory-scimple] 01/01: Phone number parsing is non-strict by default
This is an automated email from the ASF dual-hosted git repository.
bdemers pushed a commit to branch phone-lax-parsing
in repository https://gitbox.apache.org/repos/asf/directory-scimple.git
commit fad3df08cf236f95b9da9ae6e89e781845cdf165
Author: Brian Demers <bd...@apache.org>
AuthorDate: Tue Jan 10 20:26:16 2023 -0500
Phone number parsing is non-strict by default
Per SCIM RFC, phone numbers _should_ be formatted as defined in RFC3966, but this is not a requirement
The SCIM RFC has an example that uses phone number `555-555-5555`, this needs to be allowed as a valid phone number
---
.../directory/scim/spec/resources/PhoneNumber.java | 51 ++++++++++++----------
.../scim/spec/phonenumber/PhoneNumberTest.java | 16 ++++---
2 files changed, 37 insertions(+), 30 deletions(-)
diff --git a/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/resources/PhoneNumber.java b/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/resources/PhoneNumber.java
index 337038c4..b3925a3a 100644
--- a/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/resources/PhoneNumber.java
+++ b/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/resources/PhoneNumber.java
@@ -67,10 +67,8 @@ public class PhoneNumber extends KeyedResource implements Serializable, TypedAtt
private static final String VISUAL_SEPARATORS = "[\\(\\)\\-\\.]";
- @Getter
- @Setter
- private static boolean strict = true;
-
+ private static final Logger log = LoggerFactory.getLogger(PhoneNumber.class);
+
@ScimAttribute(description = "Phone number of the User")
String value;
@@ -137,12 +135,15 @@ public class PhoneNumber extends KeyedResource implements Serializable, TypedAtt
return value;
}
- public void setValue(String value) throws PhoneNumberParseException {
+ public PhoneNumber setValue(String value) throws PhoneNumberParseException {
+ return this.setValue(value, false);
+ }
+
+ public PhoneNumber setValue(String value, boolean strict) throws PhoneNumberParseException {
if (value == null) {
throw new PhoneNumberParseException("null values are illegal for phone numbers");
}
- if (strict) {
PhoneNumberLexer phoneNumberLexer = new PhoneNumberLexer(new ANTLRInputStream(value));
PhoneNumberParser p = new PhoneNumberParser(new CommonTokenStream(phoneNumberLexer));
p.setBuildParseTree(true);
@@ -158,23 +159,26 @@ public class PhoneNumber extends KeyedResource implements Serializable, TypedAtt
try {
ParseTree tree = p.phoneNumber();
ParseTreeWalker.DEFAULT.walk(tpl, tree);
+ PhoneNumber parsedPhoneNumber = tpl.getPhoneNumber();
+
+ this.value = parsedPhoneNumber.getValue();
+ this.number = parsedPhoneNumber.getNumber();
+ this.extension = parsedPhoneNumber.getExtension();
+ this.subAddress = parsedPhoneNumber.getSubAddress();
+ this.phoneContext = parsedPhoneNumber.getPhoneContext();
+ this.params = parsedPhoneNumber.getParams();
+ this.isGlobalNumber = parsedPhoneNumber.isGlobalNumber();
+ this.isDomainPhoneContext = parsedPhoneNumber.isDomainPhoneContext();
} catch (IllegalStateException e) {
- throw new PhoneNumberParseException(e);
+ // SCIM Core RFC section 4.1.2 states phone numbers SHOULD be formatted per RFC3966, e.g. 'tel:+1-201-555-0123'
+ // but this is not required, if exception is thrown while parsing, fall back to original value, unless `strict`
+ if (strict) {
+ throw new PhoneNumberParseException(e);
+ }
+ log.debug("Failed to parse phone number '{}'", value, e);
+ this.value = value;
}
-
- PhoneNumber parsedPhoneNumber = tpl.getPhoneNumber();
-
- this.value = parsedPhoneNumber.getValue();
- this.number = parsedPhoneNumber.getNumber();
- this.extension = parsedPhoneNumber.getExtension();
- this.subAddress = parsedPhoneNumber.getSubAddress();
- this.phoneContext = parsedPhoneNumber.getPhoneContext();
- this.params = parsedPhoneNumber.getParams();
- this.isGlobalNumber = parsedPhoneNumber.isGlobalNumber();
- this.isDomainPhoneContext = parsedPhoneNumber.isDomainPhoneContext();
- } else {
- this.value = value;
- }
+ return this;
}
/*
@@ -444,12 +448,11 @@ public class PhoneNumber extends KeyedResource implements Serializable, TypedAtt
}
PhoneNumber phoneNumber = new PhoneNumber();
-
String formattedValue = getFormattedValue();
- LOGGER.debug("" + formattedValue);
+ LOGGER.debug("Building phone number: '{}'", formattedValue);
if (validate) {
- phoneNumber.setValue(formattedValue);
+ phoneNumber.setValue(formattedValue, true);
} else {
phoneNumber.value = formattedValue;
phoneNumber.extension = this.extension;
diff --git a/scim-spec/scim-spec-schema/src/test/java/org/apache/directory/scim/spec/phonenumber/PhoneNumberTest.java b/scim-spec/scim-spec-schema/src/test/java/org/apache/directory/scim/spec/phonenumber/PhoneNumberTest.java
index 6e09a074..6468dfec 100644
--- a/scim-spec/scim-spec-schema/src/test/java/org/apache/directory/scim/spec/phonenumber/PhoneNumberTest.java
+++ b/scim-spec/scim-spec-schema/src/test/java/org/apache/directory/scim/spec/phonenumber/PhoneNumberTest.java
@@ -26,6 +26,8 @@ import org.junit.jupiter.params.provider.MethodSource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static org.assertj.core.api.Assertions.assertThat;
+
public class PhoneNumberTest {
private static final Logger LOGGER = LoggerFactory.getLogger(PhoneNumberTest.class);
@@ -159,18 +161,20 @@ public class PhoneNumberTest {
@ParameterizedTest
@MethodSource("getAllValidPhones")
public void test_parser_with_valid_phone_numbers(String phoneUri) throws Exception {
- LOGGER.info("valid phones (" + phoneUri + ") start");
PhoneNumber phoneNumber = new PhoneNumber();
- phoneNumber.setValue(phoneUri);
+ phoneNumber.setValue(phoneUri, true);
+ assertThat(phoneNumber.getValue()).isEqualTo(phoneUri);
}
- @ParameterizedTest
+ @ParameterizedTest
@MethodSource("getAllInvalidPhones")
public void test_parser_with_invalid_phone_numbers(String phoneUri) throws PhoneNumberParseException {
- LOGGER.info("invalid phones (" + phoneUri + ") start");
PhoneNumber phoneNumber = new PhoneNumber();
+ phoneNumber.setValue(phoneUri);
+ // non-strict parsing should work
+ assertThat(phoneNumber.getValue()).isEqualTo(phoneUri);
- Assertions.assertThrows(PhoneNumberParseException.class, () -> phoneNumber.setValue(phoneUri));
+ // strict parsing should fail
+ Assertions.assertThrows(PhoneNumberParseException.class, () -> new PhoneNumber().setValue(phoneUri, true));
}
-
}