From 52ba575e336315de213b511d506cb1379ac68ddc Mon Sep 17 00:00:00 2001 From: Christian Kraus Date: Tue, 11 Aug 2020 22:46:34 +0200 Subject: [PATCH 1/2] Use libphonenumber to properly format phone numbers --- composer.json | 3 +- config.example.php | 18 ++++++------ src/FritzBox/Converter.php | 17 ++++++++---- tests/ConverterTest.php | 56 +++++++++++++++++++++++--------------- tests/RestorerTest.php | 9 +----- 5 files changed, 56 insertions(+), 47 deletions(-) diff --git a/composer.json b/composer.json index a5ae9323..8392a4de 100644 --- a/composer.json +++ b/composer.json @@ -14,7 +14,8 @@ "symfony/console": "^4.3|^5.0", "guzzlehttp/guzzle": "^7.0", "ringcentral/psr7": "^1.2", - "sabre/vobject": "^4.0" + "sabre/vobject": "^4.0", + "giggsey/libphonenumber-for-php": "^8.12" }, "autoload": { "psr-4": { diff --git a/config.example.php b/config.example.php index 8aaeda21..d678070c 100644 --- a/config.example.php +++ b/config.example.php @@ -65,6 +65,14 @@ ], 'conversions' => [ + /** + * Two-letter country code where this FRITZ!Box is located. + * + * Phone numbers from this country will be formatted as a national number (i.e. no country code) + * whereas numbers from other countries will use the international format appropriate for this country. + */ + 'country' => 'DE', + 'vip' => [ 'category' => [ 'vip1' @@ -98,15 +106,5 @@ 'WORK' => 'work', 'HOME' => 'home' ], - /** - * 'phoneReplaceCharacters' conversions are processed consecutively. Order decides! - */ - 'phoneReplaceCharacters' => [ - '+49' => '', // router is usually operated in 'DE; '0049' could also be part of a phone number - '(' => '', - ')' => '', - '/' => '', - '-' => '' - ] ] ]; diff --git a/src/FritzBox/Converter.php b/src/FritzBox/Converter.php index 5dc92045..76eff627 100644 --- a/src/FritzBox/Converter.php +++ b/src/FritzBox/Converter.php @@ -3,6 +3,8 @@ namespace Andig\FritzBox; use Andig; +use libphonenumber\NumberParseException; +use libphonenumber\PhoneNumberUtil; use \SimpleXMLElement; class Converter @@ -79,13 +81,16 @@ public function convertPhonenumber($number) if (filter_var($number, FILTER_VALIDATE_EMAIL) || substr($number, 0, 2) == '**') { return $number; } - if (count($this->config['phoneReplaceCharacters'])) { - $number = str_replace("\xc2\xa0", "\x20", $number); - $number = strtr($number, $this->config['phoneReplaceCharacters']); - $number = trim(preg_replace('/\s+/', ' ', $number)); - } - return $number; + $numberUtil = PhoneNumberUtil::getInstance(); + try { + $countryCode = $this->config['defaultCountry']; + $parsedNumber = $numberUtil->parse($number, $countryCode); + return $numberUtil->formatOutOfCountryCallingNumber($parsedNumber, $countryCode); + } catch (NumberParseException $exception) { + // If parsing the phone number fails, it's probably garbage anyway. Just pass it on verbatim. + return $number; + } } diff --git a/tests/ConverterTest.php b/tests/ConverterTest.php index 63a61d00..2121c0d3 100644 --- a/tests/ConverterTest.php +++ b/tests/ConverterTest.php @@ -28,14 +28,7 @@ private function defaultConfig(): array 'CELL' => 'mobile', 'FAX' => 'fax_work' ], - 'phoneReplaceCharacters' => [ - '+49' => '', - '(' => '', - ')' => '', - '/' => '', - '@' => '', - '-' => '' - ], + 'defaultCountry' => 'DE', 'realName' => [], ], ]; @@ -211,24 +204,43 @@ public function testPhoneWithoutType() $this->assertEquals('other', (string)$numberType['type']); } - public function testPhonenumberConversionType() + public function testSipNumbersDoNotGetChanged() { - unset($this->contact->TEL); - $this->contact->add('TEL', 'foo@sip.de', ['type' => 'work']); - $this->contact->add('TEL', '(0511)12345/678-890', ['type' => 'home']); + $this->checkConversionResult('foo@sip.de', 'foo@sip.de'); + } - $res = $this->converter->convert($this->contact); - $this->assertCount(1, $res); + public function testNationalNumbersGetNoCountryCode() + { + $this->checkConversionResult('+49(0511)12345/678-890', '0511 12345678890'); + } - $contact = $res[0]; - $this->assertCount(2, $contact->telephony->children()); + public function testInternationalNumbersGetCountryCode() + { + $this->checkConversionResult('+1234567890', '00 1 234567890'); + } + + public function testGarbageNumbersAreNotChanged() + { + $this->checkConversionResult('garbage', 'garbage'); + } + + public function testInteralNumbersAreNotChanged() + { + $this->checkConversionResult('**123', '**123'); + } + + private function checkConversionResult($number, $expectedResult) + { + unset($this->contact->TEL); + $this->contact->add('TEL', $number, ['type' => 'other']); + + $result = $this->converter->convert($this->contact); + $this->assertCount(1, $result); - // no number conversion - $number = $contact->telephony->children()[0]; - $this->assertEquals('foo@sip.de', (string)$number); + $resultingContact = $result[0]; + $this->assertCount(1, $resultingContact->telephony->children()); - // number conversion - $number = $contact->telephony->children()[1]; - $this->assertEquals('051112345678890', (string)$number); + $convertedNumber = (string)$resultingContact->telephony->children()[0]; + $this->assertEquals($expectedResult, $convertedNumber); } } diff --git a/tests/RestorerTest.php b/tests/RestorerTest.php index 51c5929e..5011b23e 100644 --- a/tests/RestorerTest.php +++ b/tests/RestorerTest.php @@ -23,14 +23,7 @@ private function defaultConfig(): array 'CELL' => 'mobile', 'FAX' => 'fax_work', ], - 'phoneReplaceCharacters' => [ - '+49' => '', - '(' => '', - ')' => '', - '@' => '', - '/' => '', - '-' => '', - ], + 'defaultCountry' => 'DE' ], ]; } From b361953925bb9d488baf0bf413a29886b86ec989 Mon Sep 17 00:00:00 2001 From: Christian Kraus Date: Mon, 28 Dec 2020 14:10:16 +0100 Subject: [PATCH 2/2] Rename `defaultCountry` setting to `country` --- src/FritzBox/Converter.php | 2 +- tests/ConverterTest.php | 2 +- tests/RestorerTest.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/FritzBox/Converter.php b/src/FritzBox/Converter.php index 76eff627..051f39c7 100644 --- a/src/FritzBox/Converter.php +++ b/src/FritzBox/Converter.php @@ -84,7 +84,7 @@ public function convertPhonenumber($number) $numberUtil = PhoneNumberUtil::getInstance(); try { - $countryCode = $this->config['defaultCountry']; + $countryCode = $this->config['country']; $parsedNumber = $numberUtil->parse($number, $countryCode); return $numberUtil->formatOutOfCountryCallingNumber($parsedNumber, $countryCode); } catch (NumberParseException $exception) { diff --git a/tests/ConverterTest.php b/tests/ConverterTest.php index 2121c0d3..b1fa3357 100644 --- a/tests/ConverterTest.php +++ b/tests/ConverterTest.php @@ -28,7 +28,7 @@ private function defaultConfig(): array 'CELL' => 'mobile', 'FAX' => 'fax_work' ], - 'defaultCountry' => 'DE', + 'country' => 'DE', 'realName' => [], ], ]; diff --git a/tests/RestorerTest.php b/tests/RestorerTest.php index 5011b23e..1c377605 100644 --- a/tests/RestorerTest.php +++ b/tests/RestorerTest.php @@ -23,7 +23,7 @@ private function defaultConfig(): array 'CELL' => 'mobile', 'FAX' => 'fax_work', ], - 'defaultCountry' => 'DE' + 'country' => 'DE' ], ]; }