Issue#708 Add PhoneNumber Regex for SMS/Voice and USSD #1806

croufay · 2017-02-12T22:19:19Z

Issue#708 Add PhoneNumber Regex for SMS/Voice and Ussd

The code does a normal exact match and if that fails, it checks if there is a relevant Regex match

example regex

tested

^*77...33#$ = matches any number that starts with *77 and and the three dots matches any character and it must end with a # sign

^[12]2233#$ = matches any number that starts with 1 or 2 and ends with 2233#

^[]76[]88[456]#$ = matches any number that starts with 76884# or 76886# or 76886#

7777|8888 = matches 7777 or 8888

[45]234[23] = matches 42342, 42343, 52342 and 52343,

All tested with Voice, SMS or USSD where relevant.

…umbersDao.java

croufay · 2017-03-08T17:30:02Z

I;m assigning to @maria-farooq

maria-farooq · 2017-03-09T07:28:39Z

Hi @croufay i reviewed other 2 PRs

This one i think should be reviewed by @gvagenas

Thanks
Maria

gvagenas

@croufay there are no test cases and no documentation. I could provide the test cases but I don't know what to test for.

gvagenas · 2017-04-10T13:41:35Z

...tcomm.dao/src/main/java/org/restcomm/connect/dao/mybatis/MybatisIncomingPhoneNumbersDao.java

-        } finally {
+            }//}
+            //check if there is a Regex match only if parameter is a String aka phone Number
+            if(!(parameter instanceof Sid)){


@croufay why you check if object parameter is not an instance of Sid? What is the point here?

@croufay what is your comment on that?

gvagenas · 2017-04-10T13:42:39Z

...tcomm.dao/src/main/java/org/restcomm/connect/dao/mybatis/MybatisIncomingPhoneNumbersDao.java

+            }//}
+            //check if there is a Regex match only if parameter is a String aka phone Number
+            if(!(parameter instanceof Sid)){
+               inboundPhoneNumber = parameter.toString().replace("+1", "");


@croufay why you always remove the +1 sign? What if we have +1512*, what if we have +331554*?
The + sign should be removed after we check the DB and there is no match

@gvagenas, that shouldn't be a problem because the first part always checks for exact match. ex. +331554*, If there is no exact match, it does a regex match. You have to remove the + sign as it has a special meaning and it will cause the regex search to fail

@croufay here you don't remove just the + sign but the +1 so I guess you have to fix that, right?
Also, you mentioned that the first part always checks for exact match. ex. +331554*, If there is no exact match, it does a regex match, I don't get that. From the code I can see that if the parameter contains * or # you try a REGEX search by first removing the +1, can you please point me to the part of the code you refer?

We should remove the

Restcomm-Connect/restcomm/restcomm.telephony/src/main/java/org/restcomm/connect/telephony/CallManager.java

Line 953 in e1f4290

formatedPhone = phoneNumberUtil.format(phoneNumberUtil.parse(phone, "US"), PhoneNumberFormat.E164);

line that adds the +1 to all number that are not formated with internation access code.
@deruelle please confirm if you agree

gvagenas · 2017-04-10T13:43:00Z

...tcomm.dao/src/main/java/org/restcomm/connect/dao/mybatis/MybatisIncomingPhoneNumbersDao.java

+            //check if there is a Regex match only if parameter is a String aka phone Number
+            if(!(parameter instanceof Sid)){
+               inboundPhoneNumber = parameter.toString().replace("+1", "");
+               logger.warn(" Inbound phoneNumber : " + inboundPhoneNumber );


@croufay no need for WARN log here. Just make it INFO

OK, I can update this to INFO

ok @croufay

gvagenas · 2017-04-10T13:44:01Z

...tcomm.dao/src/main/java/org/restcomm/connect/dao/mybatis/MybatisIncomingPhoneNumbersDao.java

+               String phoneRegexPattern = null;
+               try {
+                    List<IncomingPhoneNumber> listPhones = getAllIncomingPhoneNumbers();
+                   for (IncomingPhoneNumber listPhone : listPhones){


@croufay I don't get the reason to go through all the stored numbers to build the Regex pattern. Can you elaborate? This doesn't seem to be an efficient way.

@gvagenas, that is true but I didn't want to mess with the exact match we already have in place. Another thing is that the exact match is done using predefined SQL that we have defined in the https://github.com/RestComm/Restcomm-Connect/blob/master/restcomm/restcomm.application/src/main/webapp/WEB-INF/sql/incoming-phone-numbers.xml

The other option would be the parse the Regex using mysql but that might be problematic if mysql or mariadb version changed as we have seen when we do DB upgrade.

@croufay can you please explain the steps of your implementation here, I think I am missing the reason and I don't understand it.

@croufay we need a design document here

In order to avoid iterating over ALL IncomingPhoneNumbers we should add a new column at the IncomingPhoneNumbers table to flag REGEX true/false when we create a DID.
There must be a validation method to tell if the number is DID, USSD ShortCode or Regex

croufay · 2017-04-10T14:41:23Z

@gvagenas , the test case are similar to what we already do to match any numbers. We just need to add a few of the scenarios below

^*77...33#$ = matches any number that starts with *77 and and the three dots matches any character and it must end with a # sign

^[12]2233#$ = matches any number that starts with 1 or 2 and ends with 2233#

^[]76[]88[456]#$ = matches any number that starts with 76884# or 76886# or 76886#

7777|8888 = matches 7777 or 8888

[45]234[23] = matches 42342, 42343, 52342 and 52343,

All tested with Voice, SMS or USSD where relevant.

gvagenas · 2017-04-10T18:25:40Z

@croufay please provide test cases for the REGEX rules we support here. You can copy CallLifeCycleTest or TestDialVerb* and the DB file it uses, to provide REGEX rules in DB instead of full numbers.

gvagenas · 2017-04-19T14:29:32Z

Provide java regex validation at Admin UI and IncomingPhoneNumber REST API

gvagenas · 2017-04-19T14:43:50Z

For the testsuite you should create a new test class similar to CallLifecycleTest, create a new DB based on restcomm.script_callLifecycleTest but instead of specific DID numbers, provide the regex patterns.
In the test class provide a simple test case similar to org.restcomm.connect.testsuite.telephony.CallLifecycleTest#testDialClientAlice but every time dial a number that should match the patterns provided in the DB

into issue#708

… issue#708

…nd try to match the dialed number against them This refer to #708

…x). Improved log messages This refer to #708

This refer to #708

gvagenas · 2017-05-07T11:57:14Z

@croufay the PR is ready to merge, code is ok, tests are passing, all good.
We are missing documentation, when docs are ready we can merge to master

croufay · 2017-05-12T01:30:43Z

@gvagenas ,

Doc is here, https://github.com/RestComm/Restcomm-Connect/blob/master/restcomm/restcomm.docs/sources-asciidoc/src/main/asciidoc/api/incoming-phone-numbers-api.adoc#incoming-phone-number-regex-support

gvagenas · 2017-05-12T08:29:27Z

Great, thanks @croufay , I will merge today

croufay added 2 commits January 25, 2017 16:39

add regex feature for IncomingPhoneNumbers

b745d17

updated callManager.java to deal with Regex and MybatisIncomingPhoneN…

b661702

…umbersDao.java

croufay added the Peer Review label Feb 12, 2017

croufay assigned maria-farooq Mar 8, 2017

maria-farooq assigned gvagenas and unassigned maria-farooq Mar 9, 2017

deruelle added this to the 8.2.0 milestone Mar 24, 2017

deruelle added the High-Priority label Mar 24, 2017

deruelle requested a review from gvagenas March 24, 2017 15:06

deruelle added Critical_Importance and removed High-Priority labels Mar 30, 2017

Merge branch 'master' into issue#708

e1f4290

gvagenas suggested changes Apr 10, 2017

View reviewed changes

gvagenas modified the milestones: 8.2.0, George 8.2.0 Sprint 1 Apr 20, 2017

croufay and others added 6 commits April 23, 2017 05:04

Merge branch 'issue#708' of https://github.com/RestComm/Restcomm-Connect

0770b1e

into issue#708

update regex feature and add test case

bd25730

Merge branch 'issue#708' of github.com:RestComm/Restcomm-Connect into…

80a9a84

… issue#708

Sql script improvements to select only REGEX incoming phone numbers a…

b8c3dd9

…nd try to match the dialed number against them This refer to #708

added regex test scenario

09aae4f

Provided test cases for Ussd pull and dialing regular number (no rege…

4e6eb8e

…x). Improved log messages This refer to #708

gvagenas removed this from the George 8.2.0 Sprint 1 milestone May 2, 2017

gvagenas modified the milestones: George 8.2.0 Sprint 2, George 8.2.0 Sprint 1 May 2, 2017

gvagenas added 4 commits May 5, 2017 15:53

Merge branch 'master' into issue#708

58134bb

Added more tests for REGEX support.

315248e

This refer to #708

Added more tests for REGEX support.

1cead83

This refer to #708

Minor cleanup

6363367

This refer to #708

gvagenas merged commit 6363367 into master May 12, 2017

gvagenas added highlight and removed Peer Review labels May 12, 2017

jaimecasero deleted the issue#708 branch November 22, 2017 09:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue#708 Add PhoneNumber Regex for SMS/Voice and USSD #1806

Issue#708 Add PhoneNumber Regex for SMS/Voice and USSD #1806

croufay commented Feb 12, 2017

croufay commented Mar 8, 2017

maria-farooq commented Mar 9, 2017

gvagenas left a comment

gvagenas Apr 10, 2017

gvagenas Apr 10, 2017

gvagenas Apr 10, 2017

croufay Apr 10, 2017

gvagenas Apr 10, 2017

gvagenas Apr 19, 2017

gvagenas Apr 10, 2017

croufay Apr 10, 2017

gvagenas Apr 10, 2017

gvagenas Apr 10, 2017

croufay Apr 10, 2017

gvagenas Apr 10, 2017

gvagenas Apr 10, 2017

gvagenas Apr 19, 2017

croufay commented Apr 10, 2017

gvagenas commented Apr 10, 2017

gvagenas commented Apr 19, 2017

gvagenas commented Apr 19, 2017

gvagenas commented May 7, 2017

croufay commented May 12, 2017

gvagenas commented May 12, 2017

Issue#708 Add PhoneNumber Regex for SMS/Voice and USSD #1806

Issue#708 Add PhoneNumber Regex for SMS/Voice and USSD #1806

Conversation

croufay commented Feb 12, 2017

croufay commented Mar 8, 2017

maria-farooq commented Mar 9, 2017

gvagenas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

croufay commented Apr 10, 2017

gvagenas commented Apr 10, 2017

gvagenas commented Apr 19, 2017

gvagenas commented Apr 19, 2017

gvagenas commented May 7, 2017

croufay commented May 12, 2017

gvagenas commented May 12, 2017