Skip to content

Commit

Permalink
Rework handling of phone URNs
Browse files Browse the repository at this point in the history
  • Loading branch information
rowanseymour committed May 7, 2024
1 parent 6caa19e commit e8695b3
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 91 deletions.
67 changes: 42 additions & 25 deletions urns/phone.go
Original file line number Diff line number Diff line change
@@ -1,49 +1,66 @@
package urns

import (
"regexp"
"strconv"
"strings"

"github.com/nyaruka/gocommon/i18n"
"github.com/nyaruka/phonenumbers"
"github.com/pkg/errors"
)

// FromLocalPhone returns a validated tel URN
func FromLocalPhone(number string, country string) (URN, error) {
path, err := ParsePhone(number, country)
var nonTelCharsRegex = regexp.MustCompile(`[^0-9A-Za-z]`)

// ParsePhone returns a validated phone URN. If it can parse a possible number then that is used.. otherwise any value
// that validates as a phone URN is used.
func ParsePhone(raw string, country i18n.Country) (URN, error) {
// strip all non-tel characters.. only preserving an optional leading +
raw = strings.TrimSpace(raw)
hasPlus := strings.HasPrefix(raw, "+")
raw = nonTelCharsRegex.ReplaceAllString(raw, "")
if hasPlus {
raw = "+" + raw
}

number, err := parsePhoneOrShortcode(raw, country)
if err != nil {
return NilURN, err
if err == phonenumbers.ErrInvalidCountryCode {
return NilURN, errors.New("invalid country code")
}

return NewFromParts(Phone, raw, "", "")
}

return NewURNFromParts(Phone, path, "", "")
return NewFromParts(Phone, number, "", "")
}

// ToLocalPhone converts a phone URN to a local number in the given country
func ToLocalPhone(u URN, country string) string {
_, path, _, _ := u.ToParts()
// tries to extract a valid phone number or shortcode from the given string
func parsePhoneOrShortcode(raw string, country i18n.Country) (string, error) {
parsed, err := phonenumbers.Parse(raw, string(country))
if err != nil {
return "", err
}

parsed, err := phonenumbers.Parse(path, country)
if err == nil {
return strconv.FormatUint(parsed.GetNationalNumber(), 10)
if phonenumbers.IsPossibleNumberWithReason(parsed) == phonenumbers.IS_POSSIBLE {
return phonenumbers.Format(parsed, phonenumbers.E164), nil
}
return path
}

// ParsePhone tries to parse the given string as a phone number and if successful returns it as E164
func ParsePhone(s, country string) (string, error) {
parsed, err := phonenumbers.Parse(s, country)
if err != nil {
return "", errors.Wrap(err, "unable to parse number")
if phonenumbers.IsPossibleShortNumberForRegion(parsed, string(country)) {
return phonenumbers.Format(parsed, phonenumbers.NATIONAL), nil
}

if phonenumbers.IsPossibleNumberWithReason(parsed) != phonenumbers.IS_POSSIBLE {
// if it's not a possible number, try adding a + and parsing again
if !strings.HasPrefix(s, "+") {
return ParsePhone("+"+s, country)
}
return "", errors.New("unable to parse phone number or shortcode")
}

return "", errors.New("not a possible number")
// ToLocalPhone converts a phone URN to a local phone number.. without any leading zeros
func ToLocalPhone(u URN, country i18n.Country) string {
_, path, _, _ := u.ToParts()

parsed, err := phonenumbers.Parse(path, string(country))
if err != nil {
return path
}

return phonenumbers.Format(parsed, phonenumbers.E164), nil
return strconv.FormatUint(parsed.GetNationalNumber(), 10)
}
96 changes: 59 additions & 37 deletions urns/phone_test.go
Original file line number Diff line number Diff line change
@@ -1,68 +1,90 @@
package urns_test
package urns

import (
"testing"

"github.com/nyaruka/gocommon/urns"
"github.com/nyaruka/gocommon/i18n"
"github.com/stretchr/testify/assert"
)

func TestFromLocalPhone(t *testing.T) {
func TestParsePhone(t *testing.T) {
testCases := []struct {
number string
country string
expected urns.URN
hasError bool
input string
country i18n.Country
expected URN
}{
{"tel:0788383383", "RW", "tel:+250788383383", false},
{"tel: +250788383383 ", "KE", "tel:+250788383383", false}, // already has country code
{"tel:(917)992-5253", "US", "tel:+19179925253", false},
{"tel:800-CABBAGE", "US", "tel:+18002222243", false},
{"tel:+62877747666", "ID", "tel:+62877747666", false},
{"tel:0877747666", "ID", "tel:+62877747666", false},
{"tel:07531669965", "GB", "tel:+447531669965", false},
{"tel:263780821000", "ZW", "tel:+263780821000", false},
{" 0788383383 ", "RW", "tel:+250788383383"},
{"+250788383383 ", "RW", "tel:+250788383383"}, // already has country code and leading +
{"250788383383 ", "RW", "tel:+250788383383"}, // already has country code and no leading +
{"+250788383383 ", "KE", "tel:+250788383383"}, // already has a different country code
{"(917)992-5253", "US", "tel:+19179925253"},
{"800-CABBAGE", "US", "tel:+18002222243"},
{"+62877747666", "ID", "tel:+62877747666"},
{"0877747666", "ID", "tel:+62877747666"},
{"07531669965", "GB", "tel:+447531669965"},
{"263780821000", "ZW", "tel:+263780821000"},

{"1", "RW", "tel:1"},
{"123456", "RW", "tel:123456"},
{"mtn", "RW", "tel:mtn"},
{"!mtn!", "RW", "tel:mtn"}, // non tel chars stripped

{"0788383383", "ZZ", urns.NilURN, true}, // invalid country code
{"1", "RW", urns.NilURN, true},
{"123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890", "RW", urns.NilURN, true},
{"0788383383", "ZZ", NilURN}, // invalid country code
{"1234567890123456789012345678901234567890123456789012345678901234567890123456789", "RW", NilURN}, // too long
}

for i, tc := range testCases {
urn, err := urns.FromLocalPhone(tc.number, tc.country)
urn, err := ParsePhone(tc.input, tc.country)

if tc.hasError {
assert.Error(t, err, "%d: expected error for %s, %s", i, tc.number, tc.country)
if tc.expected == NilURN {
assert.Error(t, err, "%d: expected error for %s, %s", i, tc.input, tc.country)
} else {
assert.NoError(t, err, "%d: unexpected error for %s, %s", i, tc.number, tc.country)
assert.Equal(t, tc.expected, urn, "%d: created URN mismatch for %s, %s", i, tc.number, tc.country)
assert.NoError(t, err, "%d: unexpected error for %s, %s", i, tc.input, tc.country)
assert.Equal(t, tc.expected, urn, "%d: created URN mismatch for %s, %s", i, tc.input, tc.country)
}
}
}

func TestParsePhone(t *testing.T) {
func TestParsePhoneOrShortcode(t *testing.T) {
tcs := []struct {
input string
country string
parsed string
input string
country i18n.Country
expected string
}{
{"+250788123123", "", "+250788123123"}, // international number fine without country
{"+250 788 123-123", "", "+250788123123"}, // fine if not E164 formatted
{"0788123123", "RW", "+250788123123"},
{"206 555 1212", "US", "+12065551212"},
{"12065551212", "US", "+12065551212"}, // country code but no +
{"5912705", "US", ""}, // is only possible as a local number so ignored
{"10000", "US", ""},
{"+250 788 123-123", "", "+250788123123"}, // still fine if not E164 formatted

{"0788123123", "RW", "+250788123123"}, // country code added
{" (206)555-1212 ", "US", "+12065551212"}, // punctiation removed
{"800-CABBAGE", "US", "+18002222243"}, // letters converted to numbers
{"12065551212", "US", "+12065551212"}, // country code but no +
{"10000", "US", "10000"}, // valid short code for US

{"5912705", "US", ""}, // is only possible as a local number so ignored
}

for _, tc := range tcs {
if tc.parsed != "" {
parsed, err := urns.ParsePhone(tc.input, tc.country)
parsed, err := parsePhoneOrShortcode(tc.input, tc.country)

if tc.expected != "" {
assert.NoError(t, err, "unexpected error for '%s'", tc.input)
assert.Equal(t, parsed, tc.parsed, "result mismatch for '%s'", tc.input)
assert.Equal(t, tc.expected, parsed, "result mismatch for '%s'", tc.input)
} else {
_, err := urns.ParsePhone(tc.input, tc.country)
assert.Error(t, err, "expected error for '%s'", tc.input)
}
}
}
func TestToLocalPhone(t *testing.T) {
tcs := []struct {
urn URN
country i18n.Country
expected string
}{
{"tel:+250788123123", "", "788123123"},
{"tel:123123", "", "123123"},
}

for _, tc := range tcs {
assert.Equal(t, tc.expected, ToLocalPhone(tc.urn, tc.country), "local mismatch for '%s'", tc.urn)
}
}
10 changes: 2 additions & 8 deletions urns/schemes.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
)

var allDigitsRegex = regexp.MustCompile(`^[0-9]+$`)
var nonTelCharsRegex = regexp.MustCompile(`[^0-9A-Z]`)

var emailRegex = regexp.MustCompile(`^[^\s@]+@[^\s@]+$`)
var freshchatRegex = regexp.MustCompile(`^[0-9a-fA-F]{8}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{12}/[0-9a-fA-F]{8}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{12}$`)
Expand Down Expand Up @@ -115,13 +114,8 @@ var Line = &Scheme{
var Phone = &Scheme{
Prefix: "tel",
Normalize: func(path string) string {
e164, err := ParsePhone(path, "")
if err != nil {
// could be a short code so uppercase and remove non alphanumeric characters
return nonTelCharsRegex.ReplaceAllString(strings.ToUpper(path), "")
}

return e164
// might have alpha characters in it
return strings.ToUpper(path)
},
Validate: func(path string) bool { return telRegex.MatchString(path) },
Format: func(path string) string {
Expand Down
20 changes: 11 additions & 9 deletions urns/urns.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type URN string
const NilURN = URN("")

// returns a new URN for the given scheme, path, query and display
func newURNFromParts(scheme, path, query, display string) URN {
func newFromParts(scheme, path, query, display string) URN {
u := &parsedURN{
scheme: scheme,
path: path,
Expand All @@ -39,9 +39,9 @@ func newURNFromParts(scheme, path, query, display string) URN {
return URN(u.String())
}

// NewURNFromParts returns a validated URN for the given scheme, path, query and display
func NewURNFromParts(typ *Scheme, path string, query string, display string) (URN, error) {
urn := newURNFromParts(typ.Prefix, path, query, display)
// NewFromParts returns a validated URN for the given scheme, path, query and display
func NewFromParts(typ *Scheme, path string, query string, display string) (URN, error) {
urn := newFromParts(typ.Prefix, path, query, display)

if err := urn.Validate(); err != nil {
return NilURN, err
Expand Down Expand Up @@ -80,7 +80,7 @@ func (u URN) Normalize() URN {
path = s.Normalize(path)
}

return newURNFromParts(scheme, path, query, display)
return newFromParts(scheme, path, query, display)
}

// Validate returns whether this URN is considered valid
Expand Down Expand Up @@ -144,7 +144,7 @@ func (u URN) Query() (url.Values, error) {
// Identity returns the URN with any query or display attributes stripped
func (u URN) Identity() URN {
scheme, path, _, _ := u.ToParts()
return newURNFromParts(scheme, path, "", "")
return newFromParts(scheme, path, "", "")
}

// String returns the string representation of this URN
Expand All @@ -154,13 +154,15 @@ func (u URN) String() string { return string(u) }
func (u URN) Format() string {
scheme, path, _, display := u.ToParts()

// display always takes precedence
if display != "" {
return display
}

s := schemes[scheme]
if s != nil && s.Format != nil {
return s.Format(path)
}

if display != "" {
return display
}
return path
}
16 changes: 4 additions & 12 deletions urns/urns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ func TestURNProperties(t *testing.T) {
query url.Values
}{
{"tel:+250788383383", "0788 383 383", "", "", map[string][]string{}},
{"tel:+250788383383#my-phone", "my-phone", "my-phone", "", map[string][]string{}},
{"twitter:85114#billy_bob", "billy_bob", "billy_bob", "", map[string][]string{}},
{"twitter:billy_bob", "billy_bob", "", "", map[string][]string{}},
{"tel:not-a-number", "not-a-number", "", "", map[string][]string{}},
Expand Down Expand Up @@ -69,7 +70,7 @@ func TestNewFromParts(t *testing.T) {
}

for _, tc := range testCases {
urn, err := urns.NewURNFromParts(tc.scheme, tc.path, "", tc.display)
urn, err := urns.NewFromParts(tc.scheme, tc.path, "", tc.display)
identity := urn.Identity()

assert.Equal(t, tc.expected, urn, "from parts mismatch for: %s, %s, %s", tc.scheme, tc.path, tc.display)
Expand All @@ -88,24 +89,15 @@ func TestNormalize(t *testing.T) {
rawURN urns.URN
expected urns.URN
}{
// valid tel numbers
{"tel: +250788383383 ", "tel:+250788383383"},
// valid tel numbers left as they are
{"tel:+250788383383", "tel:+250788383383"},
{"tel:250788383383", "tel:+250788383383"},
{"tel:(917)992-5253", "tel:+19179925253"},
{"tel:19179925253", "tel:+19179925253"},
{"tel:+62877747666", "tel:+62877747666"},
{"tel:62877747666", "tel:+62877747666"},
{"tel:0877747666", "tel:+62877747666"},
{"tel:07531669965", "tel:+447531669965"},
{"tel:22658125926", "tel:+22658125926"},
{"tel:263780821000", "tel:+263780821000"},
{"tel:+2203693333", "tel:+2203693333"},

// non-standard phone numbers
{"tel:12345", "tel:12345"},
{"tel:mtn", "tel:MTN"},
{"tel:+12345678901234567890", "tel:12345678901234567890"},
{"tel:+12345678901234567890", "tel:+12345678901234567890"},

// twitter handles remove @
{"twitter: @jimmyJO", "twitter:jimmyjo"},
Expand Down

0 comments on commit e8695b3

Please sign in to comment.