From 8826b9c29fd71217d2a400290bef7fd8f4fd6644 Mon Sep 17 00:00:00 2001 From: Adrian Haurat Date: Mon, 12 Oct 2020 01:05:21 -0700 Subject: [PATCH] Fix requirements to construct a valid object instance of the UserDataBag class (#1108) Co-authored-by: Adrian Haurat Co-authored-by: Alex Bouma --- CHANGELOG.md | 1 + src/UserDataBag.php | 43 ++++++++------------ tests/UserDataBagTest.php | 83 +++++++++++++++++++++------------------ 3 files changed, 62 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81f70974f..e97d7f748 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased - Fix missing source code excerpts for stacktrace frames whose absolute file path is equal to the file path (#1104) +- Fix requirements to construct a valid object instance of the `UserDataBag` class (#1108) ## 3.0.2 (2020-10-02) diff --git a/src/UserDataBag.php b/src/UserDataBag.php index 5849cbcaa..2237612be 100644 --- a/src/UserDataBag.php +++ b/src/UserDataBag.php @@ -34,8 +34,17 @@ final class UserDataBag */ private $metadata = []; - private function __construct() + /** + * UserDataBag constructor. + * + * @param string|int|null $id + */ + public function __construct($id = null, ?string $email = null, ?string $ipAddress = null, ?string $username = null) { + $this->setId($id); + $this->setEmail($email); + $this->setIpAddress($ipAddress); + $this->setUsername($username); } /** @@ -45,10 +54,7 @@ private function __construct() */ public static function createFromUserIdentifier($id): self { - $instance = new self(); - $instance->setId($id); - - return $instance; + return new self($id); } /** @@ -58,10 +64,7 @@ public static function createFromUserIdentifier($id): self */ public static function createFromUserIpAddress(string $ipAddress): self { - $instance = new self(); - $instance->setIpAddress($ipAddress); - - return $instance; + return new self(null, null, $ipAddress); } /** @@ -71,25 +74,21 @@ public static function createFromUserIpAddress(string $ipAddress): self */ public static function createFromArray(array $data): self { - if (!isset($data['id']) && !isset($data['ip_address'])) { - throw new \InvalidArgumentException('Either the "id" or the "ip_address" field must be set.'); - } - $instance = new self(); foreach ($data as $field => $value) { switch ($field) { case 'id': - $instance->setId($data['id']); + $instance->setId($value); break; case 'ip_address': - $instance->setIpAddress($data['ip_address']); + $instance->setIpAddress($value); break; case 'email': - $instance->setEmail($data['email']); + $instance->setEmail($value); break; case 'username': - $instance->setUsername($data['username']); + $instance->setUsername($value); break; default: $instance->setMetadata($field, $value); @@ -117,11 +116,7 @@ public function getId() */ public function setId($id): void { - if (null === $id && null === $this->ipAddress) { - throw new \BadMethodCallException('Either the IP address or the ID must be set.'); - } - - if (!\is_string($id) && !\is_int($id)) { + if (null !== $id && !\is_string($id) && !\is_int($id)) { throw new \UnexpectedValueException(sprintf('Expected an integer or string value for the $id argument. Got: "%s".', get_debug_type($id))); } @@ -183,10 +178,6 @@ public function setIpAddress(?string $ipAddress): void throw new \InvalidArgumentException(sprintf('The "%s" value is not a valid IP address.', $ipAddress)); } - if (null === $ipAddress && null === $this->id) { - throw new \BadMethodCallException('Either the IP address or the ID must be set.'); - } - $this->ipAddress = $ipAddress; } diff --git a/tests/UserDataBagTest.php b/tests/UserDataBagTest.php index ba9ace823..e4ca87a1c 100644 --- a/tests/UserDataBagTest.php +++ b/tests/UserDataBagTest.php @@ -9,17 +9,10 @@ final class UserDataBagTest extends TestCase { - public function testConstructorThrowsIfArgumentIsInvalid(): void - { - $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessage('The "foo" value is not a valid IP address.'); - - UserDataBag::createFromUserIpAddress('foo'); - } - public function testGettersAndSetters(): void { - $userDataBag = UserDataBag::createFromUserIdentifier('unique_id'); + $userDataBag = new UserDataBag(); + $userDataBag->setId('unique_id'); $userDataBag->setIpAddress('127.0.0.1'); $userDataBag->setEmail('foo@example.com'); $userDataBag->setUsername('my_user'); @@ -76,11 +69,8 @@ public function createFromArrayDataProvider(): iterable ]; yield [ - [ - 'id' => 'unique_id', - 'email' => 'foo@example.com', - ], - 'unique_id', + ['email' => 'foo@example.com'], + null, null, 'foo@example.com', null, @@ -88,11 +78,8 @@ public function createFromArrayDataProvider(): iterable ]; yield [ - [ - 'id' => 'unique_id', - 'username' => 'my_user', - ], - 'unique_id', + ['username' => 'my_user'], + null, null, null, 'my_user', @@ -100,11 +87,8 @@ public function createFromArrayDataProvider(): iterable ]; yield [ - [ - 'id' => 'unique_id', - 'subscription' => 'basic', - ], - 'unique_id', + ['subscription' => 'basic'], + null, null, null, null, @@ -113,17 +97,40 @@ public function createFromArrayDataProvider(): iterable } /** - * @dataProvider setIdThrowsIfValueIsUnexpectedValueDataProvider + * @dataProvider unexpectedValueForIdFieldDataProvider + */ + public function testConstructorThrowsIfIdValueIsUnexpectedValue($value, string $expectedExceptionMessage): void + { + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage($expectedExceptionMessage); + + new UserDataBag($value); + } + + /** + * @dataProvider unexpectedValueForIdFieldDataProvider */ public function testSetIdThrowsIfValueIsUnexpectedValue($value, string $expectedExceptionMessage): void { $this->expectException(\UnexpectedValueException::class); $this->expectExceptionMessage($expectedExceptionMessage); + $userDataBag = new UserDataBag(); + $userDataBag->setId($value); + } + + /** + * @dataProvider unexpectedValueForIdFieldDataProvider + */ + public function testCreateFromUserIdentifierThrowsIfArgumentIsInvalid($value, string $expectedExceptionMessage): void + { + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage($expectedExceptionMessage); + UserDataBag::createFromUserIdentifier($value); } - public function setIdThrowsIfValueIsUnexpectedValueDataProvider(): iterable + public function unexpectedValueForIdFieldDataProvider(): iterable { yield [ 12.34, @@ -136,31 +143,29 @@ public function setIdThrowsIfValueIsUnexpectedValueDataProvider(): iterable ]; } - public function testSetIdThrowsIfBothArgumentAndIpAddressAreNull(): void + public function testConstructorThrowsIfIpAddressArgumentIsInvalid(): void { - $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Either the IP address or the ID must be set.'); + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('The "foo" value is not a valid IP address.'); - $userDataBag = UserDataBag::createFromUserIdentifier('unique_id'); - $userDataBag->setId(null); + new UserDataBag(null, null, 'foo'); } - public function testSetIpAddressThrowsIfBothArgumentAndIdAreNull(): void + public function testSetIpAddressThrowsIfArgumentIsInvalid(): void { - $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Either the IP address or the ID must be set.'); + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('The "foo" value is not a valid IP address.'); - $userDataBag = UserDataBag::createFromUserIpAddress('127.0.0.1'); - $userDataBag->setIpAddress(null); + $userDataBag = new UserDataBag(); + $userDataBag->setIpAddress('foo'); } - public function testSetIpAddressThrowsIfArgumentIsInvalid(): void + public function testCreateFromIpAddressThrowsIfArgumentIsInvalid(): void { $this->expectException(\InvalidArgumentException::class); $this->expectExceptionMessage('The "foo" value is not a valid IP address.'); - $userDataBag = UserDataBag::createFromUserIpAddress('127.0.0.1'); - $userDataBag->setIpAddress('foo'); + UserDataBag::createFromUserIpAddress('foo'); } public function testMerge(): void