From dc0b6807763e6ea2fdee5b9ecb4ec6dc5dc9e529 Mon Sep 17 00:00:00 2001 From: Daniel Seifert Date: Mon, 18 Jul 2022 13:54:56 +0200 Subject: [PATCH] move recipient number check from list to itself --- Tests/RecipientsList/RecipientsListTest.php | 129 -------------------- Tests/ValueObject/RecipientTest.php | 25 ++-- src/RecipientsList/RecipientsList.php | 14 --- src/ValueObject/Recipient.php | 1 + 4 files changed, 18 insertions(+), 151 deletions(-) diff --git a/Tests/RecipientsList/RecipientsListTest.php b/Tests/RecipientsList/RecipientsListTest.php index e47a003..7383fa2 100644 --- a/Tests/RecipientsList/RecipientsListTest.php +++ b/Tests/RecipientsList/RecipientsListTest.php @@ -16,18 +16,13 @@ declare(strict_types=1); namespace D3\LinkmobilityClient\Tests\RecipientsList; use D3\LinkmobilityClient\Client; -use D3\LinkmobilityClient\LoggerHandler; use D3\LinkmobilityClient\RecipientsList\RecipientsList; use D3\LinkmobilityClient\Tests\ApiTestCase; use D3\LinkmobilityClient\ValueObject\Recipient; -use libphonenumber\NumberParseException; -use libphonenumber\PhoneNumber; use libphonenumber\PhoneNumberFormat; use libphonenumber\PhoneNumberType; use libphonenumber\PhoneNumberUtil; use PHPUnit\Framework\MockObject\MockObject; -use Psr\Log\AbstractLogger; -use Psr\Log\LoggerInterface; use ReflectionException; use stdClass; @@ -88,22 +83,6 @@ class RecipientsListTest extends ApiTestCase ); } - /** - * @test - * @throws ReflectionException - * @covers \D3\LinkmobilityClient\RecipientsList\RecipientsList::getPhoneNumberUtil - */ - public function testGetPhoneNumberUtil() - { - $this->assertInstanceOf( - PhoneNumberUtil::class, - $this->callMethod( - $this->recipientsList, - 'getPhoneNumberUtil' - ) - ); - } - /** * @test * @throws ReflectionException @@ -111,15 +90,6 @@ class RecipientsListTest extends ApiTestCase */ public function testAddValidNumber() { - /** @var PhoneNumberUtil|MockObject $phoneNumberUtilMock */ - $phoneNumberUtilMock = $this->getMockBuilder(PhoneNumberUtil::class) - ->onlyMethods(['parse', 'isValidNumber', 'getNumberType']) - ->disableOriginalConstructor() - ->getMock(); - $phoneNumberUtilMock->method('parse')->willReturn(new PhoneNumber()); - $phoneNumberUtilMock->method('isValidNumber')->willReturn(true); - $phoneNumberUtilMock->method('getNumberType')->willReturn(PhoneNumberType::MOBILE); - /** @var Recipient|MockObject $recipientMock */ $recipientMock = $this->getMockBuilder(Recipient::class) ->onlyMethods(['get', 'getCountryCode']) @@ -128,15 +98,6 @@ class RecipientsListTest extends ApiTestCase $recipientMock->method('get')->willReturn($this->phoneNumberFixture); $recipientMock->method('getCountryCode')->willReturn($this->phoneCountryFixture); - /** @var RecipientsList|MockObject $recListMock */ - $recListMock = $this->getMockBuilder(RecipientsList::class) - ->onlyMethods(['getPhoneNumberUtil']) - ->setConstructorArgs([$this->recipientsList->getClient()]) - ->getMock(); - $recListMock->method('getPhoneNumberUtil')->willReturn($phoneNumberUtilMock); - - $this->recipientsList = $recListMock; - $this->assertCount( 0, $this->callMethod($this->recipientsList, 'getRecipientsList') @@ -157,96 +118,6 @@ class RecipientsListTest extends ApiTestCase ); } - /** - * @test - * @throws ReflectionException - * @dataProvider addInvalidNumberDataProvider - * @covers \D3\LinkmobilityClient\RecipientsList\RecipientsList::add - */ - public function testAddInvalidNumber($unparsable, $invalidNumber, $invalidNumberType) - { - /** @var PhoneNumberUtil|MockObject $phoneNumberUtilMock */ - $phoneNumberUtilMock = $this->getMockBuilder(PhoneNumberUtil::class) - ->onlyMethods(['parse', 'isValidNumber', 'getNumberType']) - ->disableOriginalConstructor() - ->getMock(); - - if ($unparsable) { - $phoneNumberUtilMock->method('parse')->willThrowException(new NumberParseException(0, 'message')); - } else { - $phoneNumberUtilMock->method('parse')->willReturn(new PhoneNumber()); - } - $phoneNumberUtilMock->method('isValidNumber')->willReturn(!$invalidNumber); - $phoneNumberUtilMock->method('getNumberType')->willReturn($invalidNumberType ? PhoneNumberType::FIXED_LINE : PhoneNumberType::MOBILE); - - /** @var Recipient|MockObject $recipientMock */ - $recipientMock = $this->getMockBuilder(Recipient::class) - ->onlyMethods(['get', 'getCountryCode']) - ->setConstructorArgs([$this->phoneNumberFixture, $this->phoneCountryFixture]) - ->getMock(); - $recipientMock->method('get')->willReturn($this->phoneNumberFixture); - $recipientMock->method('getCountryCode')->willReturn($this->phoneCountryFixture); - - /** @var LoggerInterface|MockObject $loggerMock */ - $loggerMock = $this->getMockBuilder(AbstractLogger::class) - ->onlyMethods(['info', 'log']) - ->getMock(); - $loggerMock->expects($this->atLeastOnce())->method('info')->willReturn(true); - - /** @var LoggerHandler|MockObject $loggerHandlerMock */ - $loggerHandlerMock = $this->getMockBuilder(LoggerHandler::class) - ->onlyMethods(['getLogger']) - ->getMock(); - $loggerHandlerMock->method('getLogger')->willReturn($loggerMock); - - /** @var Client|MockObject $clientMock */ - $clientMock = $this->getMockBuilder(Client::class) - ->disableOriginalConstructor() - ->onlyMethods(['getLoggerHandler']) - ->getMock(); - $clientMock->method('getLoggerHandler')->willReturn($loggerHandlerMock); - - /** @var RecipientsList|MockObject $recListMock */ - $recListMock = $this->getMockBuilder(RecipientsList::class) - ->onlyMethods(['getPhoneNumberUtil']) - ->setConstructorArgs([$clientMock]) - ->getMock(); - $recListMock->method('getPhoneNumberUtil')->willReturn($phoneNumberUtilMock); - - $this->recipientsList = $recListMock; - - $this->assertCount( - 0, - $this->callMethod($this->recipientsList, 'getRecipientsList') - ); - - $this->assertSame( - $this->recipientsList, - $this->callMethod( - $this->recipientsList, - 'add', - [$recipientMock] - ) - ); - - $this->assertCount( - 0, - $this->callMethod($this->recipientsList, 'getRecipientsList') - ); - } - - /** - * @return array[] - */ - public function addInvalidNumberDataProvider(): array - { - return [ - 'unparsable' => [true, false, false], - 'invalid number' => [false, true, false], - 'invalid number type' => [false, false, true], - ]; - } - /** * @test * @throws ReflectionException diff --git a/Tests/ValueObject/RecipientTest.php b/Tests/ValueObject/RecipientTest.php index 51ea496..f8504a0 100644 --- a/Tests/ValueObject/RecipientTest.php +++ b/Tests/ValueObject/RecipientTest.php @@ -16,6 +16,7 @@ declare(strict_types=1); namespace D3\LinkmobilityClient\Tests\ValueObject; use Assert\InvalidArgumentException; +use D3\LinkmobilityClient\Exceptions\RecipientException; use D3\LinkmobilityClient\Tests\ApiTestCase; use D3\LinkmobilityClient\ValueObject\Recipient; use libphonenumber\NumberParseException; @@ -37,6 +38,7 @@ class RecipientTest extends ApiTestCase /** * @return void * @throws NumberParseException + * @throws RecipientException */ public function setUp(): void { @@ -63,6 +65,7 @@ class RecipientTest extends ApiTestCase * @test * @return void * @throws NumberParseException + * @throws RecipientException * @throws ReflectionException * @covers \D3\LinkmobilityClient\ValueObject\Recipient::__construct */ @@ -70,11 +73,13 @@ class RecipientTest extends ApiTestCase { /** @var PhoneNumberUtil|MockObject $phoneNumberUtilMock */ $phoneNumberUtilMock = $this->getMockBuilder(PhoneNumberUtil::class) - ->onlyMethods(['parse', 'format']) + ->onlyMethods(['parse', 'format', 'isValidNumber', 'getNumberType']) ->disableOriginalConstructor() ->getMock(); $phoneNumberUtilMock->method('parse')->willReturn(new PhoneNumber()); $phoneNumberUtilMock->method('format')->willReturn('+491527565839'); + $phoneNumberUtilMock->method('isValidNumber')->willReturn(true); + $phoneNumberUtilMock->method('getNumberType')->willReturn(PhoneNumberType::MOBILE); /** @var Recipient|MockObject $recipientMock */ $recipientMock = $this->getMockBuilder(Recipient::class) @@ -107,18 +112,20 @@ class RecipientTest extends ApiTestCase * @param $number * @param $country * @param $validNumber + * @param $numberType * @param $expectedException * * @return void * @throws NumberParseException + * @throws RecipientException * @dataProvider constructInvalidDataProvider - * @covers \D3\LinkmobilityClient\ValueObject\Recipient::__construct + * @covers \D3\LinkmobilityClient\ValueObject\Recipient::__construct */ - public function testConstructInvalid($number, $country, $validNumber, $expectedException) + public function testConstructInvalid($number, $country, $validNumber, $numberType, $expectedException) { /** @var PhoneNumberUtil|MockObject $phoneNumberUtilMock */ $phoneNumberUtilMock = $this->getMockBuilder(PhoneNumberUtil::class) - ->onlyMethods(['parse', 'format', 'isValidNumber']) + ->onlyMethods(['parse', 'format', 'isValidNumber', 'getNumberType']) ->disableOriginalConstructor() ->getMock(); if ($number === 'abc') { @@ -128,6 +135,7 @@ class RecipientTest extends ApiTestCase } $phoneNumberUtilMock->method('format')->willReturn($number); $phoneNumberUtilMock->method('isValidNumber')->willReturn($validNumber); + $phoneNumberUtilMock->method('getNumberType')->willReturn($numberType); /** @var Recipient|MockObject $recipientMock */ $recipientMock = $this->getMockBuilder(Recipient::class) @@ -150,10 +158,11 @@ class RecipientTest extends ApiTestCase $phoneNumberFixture = $phoneUtil->format($example, PhoneNumberFormat::NATIONAL); return [ - 'empty number' => ['', 'DE', true, InvalidArgumentException::class], - 'invalid country code' => [$phoneNumberFixture, 'DEX', true, InvalidArgumentException::class], - 'unparsable' => ['abc', 'DE', true, NumberParseException::class], - 'invalid number' => ['abc', 'DE', false, NumberParseException::class] + 'empty number' => ['', 'DE', true, PhoneNumberType::MOBILE, InvalidArgumentException::class], + 'invalid country code' => [$phoneNumberFixture, 'DEX', true, PhoneNumberType::MOBILE, InvalidArgumentException::class], + 'unparsable' => ['abc', 'DE', true, PhoneNumberType::MOBILE, NumberParseException::class], + 'invalid number' => ['abcd', 'DE', false, PhoneNumberType::MOBILE, RecipientException::class], + 'not mobile number' => ['abcd', 'DE', false, PhoneNumberType::FIXED_LINE, RecipientException::class] ]; } diff --git a/src/RecipientsList/RecipientsList.php b/src/RecipientsList/RecipientsList.php index d3f1a4c..c9a50f0 100644 --- a/src/RecipientsList/RecipientsList.php +++ b/src/RecipientsList/RecipientsList.php @@ -16,13 +16,8 @@ declare(strict_types=1); namespace D3\LinkmobilityClient\RecipientsList; use D3\LinkmobilityClient\Client; -use D3\LinkmobilityClient\Exceptions\ExceptionMessages; -use D3\LinkmobilityClient\Exceptions\RecipientException; use D3\LinkmobilityClient\ValueObject\Recipient; use Iterator; -use libphonenumber\NumberParseException; -use libphonenumber\PhoneNumberType; -use libphonenumber\PhoneNumberUtil; class RecipientsList implements RecipientsListInterface, Iterator { @@ -41,14 +36,6 @@ class RecipientsList implements RecipientsListInterface, Iterator $this->setClient($client); } - /** - * @return PhoneNumberUtil - */ - protected function getPhoneNumberUtil(): PhoneNumberUtil - { - return PhoneNumberUtil::getInstance(); - } - /** * @param Recipient $recipient * @@ -56,7 +43,6 @@ class RecipientsList implements RecipientsListInterface, Iterator */ public function add(Recipient $recipient): RecipientsListInterface { - $phoneUtil = $this->getPhoneNumberUtil(); $this->recipients[ md5(serialize($recipient)) ] = $recipient; return $this; diff --git a/src/ValueObject/Recipient.php b/src/ValueObject/Recipient.php index d978abc..d387249 100644 --- a/src/ValueObject/Recipient.php +++ b/src/ValueObject/Recipient.php @@ -39,6 +39,7 @@ class Recipient extends StringValueObject */ public function __construct(string $number, string $iso2CountryCode) { + Assert::that($number)->notEmpty(); Assert::that($iso2CountryCode)->string()->length(2); $phoneUtil = $this->getPhoneNumberUtil();