From 6f3543609f3f394e0ba67bd6724c76695ce761a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20Lepp=C3=A4nen?= Date: Sat, 21 May 2022 12:42:00 +0300 Subject: [PATCH 1/9] Added support for readonly properties --- tests/Integration/Entity/EntityTestCase.php | 83 ++++++++++++++------- 1 file changed, 57 insertions(+), 26 deletions(-) diff --git a/tests/Integration/Entity/EntityTestCase.php b/tests/Integration/Entity/EntityTestCase.php index c4f84b957..991fae3d7 100644 --- a/tests/Integration/Entity/EntityTestCase.php +++ b/tests/Integration/Entity/EntityTestCase.php @@ -15,6 +15,7 @@ use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Mapping\ClassMetadataInfo; +use ReflectionProperty; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use Throwable; use TypeError; @@ -81,8 +82,12 @@ public function testThatGetIdReturnsCorrectUuid(): void * * @testdox Test that `getter` and `setter` methods exists for `$type $property` property */ - public function testThatGetterAndSetterExists(string $property, string $type, array $meta, bool $readOnly): void - { + public function testThatGetterAndSetterExists( + string $property, + string $type, + array $meta, + bool $readOnlyClass, + ): void { $entity = $this->getEntity(); $getter = 'get' . ucfirst($property); $setter = 'set' . ucfirst($property); @@ -102,12 +107,29 @@ public function testThatGetterAndSetterExists(string $property, string $type, ar ); if (array_key_exists('columnName', $meta)) { - $message = $readOnly + $entity = $this->getEntity(); + + $readOnlyProperty = $this->isReadOnlyProperty($property); + + $message = $readOnlyClass || $readOnlyProperty ? "Entity '%s' has not expected setter '%s()' method for '%s' property." : "Entity '%s' does not have expected setter '%s()' method for '%s' property."; + if ($readOnlyProperty) { + static::assertFalse( + method_exists($entity, $setter), + sprintf($message, $this->entityName, $setter, $property) + ); + } else { + static::assertSame( + !$readOnlyClass, + method_exists($entity, $setter), + sprintf($message, $this->entityName, $setter, $property), + ); + } + self::assertSame( - !$readOnly, + !$readOnlyClass, method_exists($entity, $setter), sprintf($message, $this->entityName, $setter, $property), ); @@ -128,7 +150,9 @@ public function testThatSetterOnlyAcceptSpecifiedType( string $type, array $meta, ): void { - if (!array_key_exists('columnName', $meta) && !array_key_exists('joinColumns', $meta)) { + if ((!array_key_exists('columnName', $meta) && !array_key_exists('joinColumns', $meta)) + || $this->isReadOnlyProperty($property) + ) { self::markTestSkipped('No need to test this setter...'); } @@ -163,7 +187,7 @@ public function testThatSetterReturnsInstanceOfEntity( string $type, array $meta, ): void { - if (!array_key_exists('columnName', $meta)) { + if (!array_key_exists('columnName', $meta) || $this->isReadOnlyProperty($property)) { self::markTestSkipped('No need to test this setter...'); } @@ -207,26 +231,28 @@ public function testThatGetterReturnsExpectedValue(string $property, string $typ /** @var callable $callable */ $callable = [$entity, $getter]; - if (array_key_exists('columnName', $meta) || array_key_exists('joinColumns', $meta)) { - $value = PhpUnitUtil::getValidValueForType($type, $meta); - - $entity->{$setter}($value); - - self::assertSame( - $value, - $callable(), - sprintf( - 'Getter method of %s:%s did not return expected value type (%s) and it returned (%s)', - $this->entityName, - $getter, - gettype($value), - gettype($callable()), - ), - ); - } else { - $type = ArrayCollection::class; - - self::assertInstanceOf($type, $callable()); + if (!$this->isReadOnlyProperty($property)) { + if (array_key_exists('columnName', $meta) || array_key_exists('joinColumns', $meta)) { + $value = PhpUnitUtil::getValidValueForType($type, $meta); + + $entity->{$setter}($value); + + self::assertSame( + $value, + $callable(), + sprintf( + 'Getter method of %s:%s did not return expected value type (%s) and it returned (%s)', + $this->entityName, + $getter, + gettype($value), + gettype($callable()), + ), + ); + } else { + $type = ArrayCollection::class; + + self::assertInstanceOf($type, $callable()); + } } try { @@ -770,4 +796,9 @@ protected function createEntity(): EntityInterface return $entity; } + + private function isReadOnlyProperty(string $property): bool + { + return (new ReflectionProperty($this->getEntity(), $property))->isReadOnly(); + } } From 217b83465968056e08062c3fe9b4fd186c5b41f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20Lepp=C3=A4nen?= Date: Sat, 21 May 2022 12:42:53 +0300 Subject: [PATCH 2/9] Introduced readonly properties to `DateDimension` entity --- src/Entity/DateDimension.php | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Entity/DateDimension.php b/src/Entity/DateDimension.php index edd56b9d1..93a44f825 100644 --- a/src/Entity/DateDimension.php +++ b/src/Entity/DateDimension.php @@ -58,7 +58,7 @@ class DateDimension implements EntityInterface 'DateDimension', 'DateDimension.id', ])] - private UuidInterface $id; + private readonly UuidInterface $id; #[ORM\Column( name: 'year', @@ -71,7 +71,7 @@ class DateDimension implements EntityInterface 'DateDimension', 'DateDimension.year', ])] - private int $year; + private readonly int $year; #[ORM\Column( name: 'month', @@ -84,7 +84,7 @@ class DateDimension implements EntityInterface 'DateDimension', 'DateDimension.month', ])] - private int $month; + private readonly int $month; #[ORM\Column( name: 'day', @@ -97,7 +97,7 @@ class DateDimension implements EntityInterface 'DateDimension', 'DateDimension.day', ])] - private int $day; + private readonly int $day; #[ORM\Column( name: 'quarter', @@ -110,7 +110,7 @@ class DateDimension implements EntityInterface 'DateDimension', 'DateDimension.quarter', ])] - private int $quarter; + private readonly int $quarter; #[ORM\Column( name: 'week_number', @@ -123,7 +123,7 @@ class DateDimension implements EntityInterface 'DateDimension', 'DateDimension.weekNumber', ])] - private int $weekNumber; + private readonly int $weekNumber; #[ORM\Column( name: 'day_number_of_week', @@ -136,7 +136,7 @@ class DateDimension implements EntityInterface 'DateDimension', 'DateDimension.dayNumber', ])] - private int $dayNumberOfWeek; + private readonly int $dayNumberOfWeek; #[ORM\Column( name: 'day_number_of_year', @@ -149,7 +149,7 @@ class DateDimension implements EntityInterface 'DateDimension', 'DateDimension.dayNumberOfYear', ])] - private int $dayNumberOfYear; + private readonly int $dayNumberOfYear; #[ORM\Column( name: 'leap_year', @@ -162,7 +162,7 @@ class DateDimension implements EntityInterface 'DateDimension', 'DateDimension.leapYear', ])] - private bool $leapYear; + private readonly bool $leapYear; #[ORM\Column( name: 'week_numbering_year', @@ -175,7 +175,7 @@ class DateDimension implements EntityInterface 'DateDimension', 'DateDimension.weekNumberingYear', ])] - private int $weekNumberingYear; + private readonly int $weekNumberingYear; #[ORM\Column( name: 'unix_time', @@ -188,7 +188,7 @@ class DateDimension implements EntityInterface 'DateDimension', 'DateDimension.unixTime', ])] - private int $unixTime; + private readonly int $unixTime; public function __construct( #[ORM\Column( @@ -199,7 +199,7 @@ public function __construct( 'DateDimension', 'DateDimension.date', ])] - private DateTimeImmutable $date + private readonly DateTimeImmutable $date ) { $this->id = $this->createUuid(); From 7d6db8b1fa9b37c8380c114fb3114be099d9d977 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20Lepp=C3=A4nen?= Date: Sat, 21 May 2022 12:44:51 +0300 Subject: [PATCH 3/9] Refactored `DateDimensionTest` --- .../Integration/Entity/DateDimensionTest.php | 68 ++++--------------- 1 file changed, 12 insertions(+), 56 deletions(-) diff --git a/tests/Integration/Entity/DateDimensionTest.php b/tests/Integration/Entity/DateDimensionTest.php index 6a63e406d..b7c851ee6 100644 --- a/tests/Integration/Entity/DateDimensionTest.php +++ b/tests/Integration/Entity/DateDimensionTest.php @@ -9,11 +9,9 @@ namespace App\Tests\Integration\Entity; use App\Entity\DateDimension; -use App\Utils\Tests\PhpUnitUtil; +use DateTime; use DateTimeImmutable; -use Throwable; -use function in_array; -use function ucfirst; +use DateTimeZone; /** * Class DateDimensionTest @@ -29,59 +27,17 @@ class DateDimensionTest extends EntityTestCase * @var class-string */ protected string $entityName = DateDimension::class; - - /** @noinspection PhpMissingParentCallCommonInspection */ - /** - * @testdox No setter for `$property` property in read only entity - so cannot test this - */ - public function testThatSetterOnlyAcceptSpecifiedType( - ?string $property = null, - ?string $type = null, - ?array $meta = null, - ): void { - self::markTestSkipped('There is not setter in read only entity...'); - } - - /** @noinspection PhpMissingParentCallCommonInspection */ - /** - * @testdox No setter for `$property` property in read only entity - so cannot test this - */ - public function testThatSetterReturnsInstanceOfEntity( - ?string $property = null, - ?string $type = null, - ?array $meta = null - ): void { - self::markTestSkipped('There is not setter in read only entity...'); - } - - /** @noinspection PhpMissingParentCallCommonInspection */ - /** - * @dataProvider dataProviderTestThatSetterAndGettersWorks - * - * @throws Throwable - * - * @testdox Test that getter method for `$type $property` property returns expected - */ - public function testThatGetterReturnsExpectedValue(string $property, string $type, array $meta): void + + public function testThatGetCreatedAtMethodReturnsExpected(): void { - $getter = 'get' . ucfirst($property); - - if (in_array($type, [PhpUnitUtil::TYPE_BOOL, PhpUnitUtil::TYPE_BOOLEAN], true)) { - $getter = 'is' . ucfirst($property); - } - - $dateDimension = $this->createEntity(); - - try { - $method = 'assertIs' . ucfirst($type); - - self::$method($dateDimension->{$getter}()); - } catch (Throwable $error) { - /** - * @var class-string $type - */ - self::assertInstanceOf($type, $dateDimension->{$getter}(), $error->getMessage()); - } + $entity = $this->createEntity(); + $createdAt = $entity->getCreatedAt(); + + self::assertEqualsWithDelta( + (new DateTime('now', new DateTimeZone('utc')))->getTimestamp(), + $createdAt->getTimestamp(), + 1 + ); } /** From 32222b0bc2e3a478bc09281c4d8194e1f2c75eca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20Lepp=C3=A4nen?= Date: Sat, 21 May 2022 12:45:48 +0300 Subject: [PATCH 4/9] Make sure that kernel is booted in `ApiKeyTest` --- tests/Integration/Entity/ApiKeyTest.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/Integration/Entity/ApiKeyTest.php b/tests/Integration/Entity/ApiKeyTest.php index 756352050..6cc7ca372 100644 --- a/tests/Integration/Entity/ApiKeyTest.php +++ b/tests/Integration/Entity/ApiKeyTest.php @@ -31,6 +31,13 @@ class ApiKeyTest extends EntityTestCase */ protected string $entityName = ApiKey::class; + protected function setUp(): void + { + parent::setUp(); + + self::bootKernel(); + } + /** * @dataProvider dataProviderTestThatApiKeyHasExpectedRoles * From 037deece6c909fd773c83b929f389ef5b9e00497 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20Lepp=C3=A4nen?= Date: Sat, 21 May 2022 12:59:11 +0300 Subject: [PATCH 5/9] Refactored `LogLogin` entity and test --- src/Entity/LogLogin.php | 44 ++++++-------- src/Entity/Traits/LogEntityTrait.php | 10 ++-- tests/Integration/Entity/LogLoginTest.php | 71 ----------------------- 3 files changed, 24 insertions(+), 101 deletions(-) diff --git a/src/Entity/LogLogin.php b/src/Entity/LogLogin.php index a7731ca12..a1f6b8b72 100644 --- a/src/Entity/LogLogin.php +++ b/src/Entity/LogLogin.php @@ -68,7 +68,7 @@ class LogLogin implements EntityInterface 'LogLogin', 'LogLogin.id', ])] - private UuidInterface $id; + private readonly UuidInterface $id; #[ORM\Column( name: 'username', @@ -80,7 +80,7 @@ class LogLogin implements EntityInterface 'LogLogin', 'LogLogin.username', ])] - private string $username = ''; + private readonly string $username; #[ORM\Column( name: 'client_type', @@ -92,7 +92,7 @@ class LogLogin implements EntityInterface 'LogLogin', 'LogLogin.clientType', ])] - private ?string $clientType = null; + private readonly string $clientType; #[ORM\Column( name: 'client_name', @@ -104,7 +104,7 @@ class LogLogin implements EntityInterface 'LogLogin', 'LogLogin.clientName', ])] - private ?string $clientName = null; + private readonly string $clientName; #[ORM\Column( name: 'client_short_name', @@ -116,7 +116,7 @@ class LogLogin implements EntityInterface 'LogLogin', 'LogLogin.clientShortName', ])] - private ?string $clientShortName = null; + private readonly string $clientShortName; #[ORM\Column( name: 'client_version', @@ -128,7 +128,7 @@ class LogLogin implements EntityInterface 'LogLogin', 'LogLogin.clientVersion', ])] - private ?string $clientVersion = null; + private readonly string $clientVersion; #[ORM\Column( name: 'client_engine', @@ -140,7 +140,7 @@ class LogLogin implements EntityInterface 'LogLogin', 'LogLogin.clientEngine', ])] - private ?string $clientEngine = null; + private readonly string $clientEngine; #[ORM\Column( name: 'os_name', @@ -152,7 +152,7 @@ class LogLogin implements EntityInterface 'LogLogin', 'LogLogin.osName', ])] - private ?string $osName = null; + private readonly string $osName; #[ORM\Column( name: 'os_short_name', @@ -164,7 +164,7 @@ class LogLogin implements EntityInterface 'LogLogin', 'LogLogin.osShortName', ])] - private ?string $osShortName = null; + private readonly string $osShortName; #[ORM\Column( name: 'os_version', @@ -176,7 +176,7 @@ class LogLogin implements EntityInterface 'LogLogin', 'LogLogin.osVersion', ])] - private ?string $osVersion = null; + private readonly string $osVersion; #[ORM\Column( name: 'os_platform', @@ -188,7 +188,7 @@ class LogLogin implements EntityInterface 'LogLogin', 'LogLogin.osPlatform', ])] - private ?string $osPlatform = null; + private readonly string $osPlatform; #[ORM\Column( name: 'device_name', @@ -200,7 +200,7 @@ class LogLogin implements EntityInterface 'LogLogin', 'LogLogin.deviceName', ])] - private ?string $deviceName = null; + private readonly string $deviceName; #[ORM\Column( name: 'brand_name', @@ -212,7 +212,7 @@ class LogLogin implements EntityInterface 'LogLogin', 'LogLogin.brandName', ])] - private ?string $brandName = null; + private readonly string $brandName; #[ORM\Column( name: 'model', @@ -224,9 +224,7 @@ class LogLogin implements EntityInterface 'LogLogin', 'LogLogin.model', ])] - private ?string $model = null; - - private DeviceDetector $deviceDetector; + private readonly string $model; /** * LogLogin constructor. @@ -242,9 +240,9 @@ public function __construct( 'LogLogin', 'LogLogin.type', ])] - private string $type, - Request $request, - DeviceDetector $deviceDetector, + private readonly string $type, + private Request $request, + private readonly DeviceDetector $deviceDetector, #[ORM\ManyToOne( targetEntity: User::class, inversedBy: 'logsLogin', @@ -257,19 +255,15 @@ public function __construct( 'LogLogin', 'LogLogin.user', ])] - private ?User $user = null + private readonly ?User $user = null ) { $this->id = $this->createUuid(); - $this->deviceDetector = $deviceDetector; - $this->processTimeAndDate(); $this->processRequestData($request); $this->processClientData(); - if ($this->user !== null) { - $this->username = $this->user->getUsername(); - } + $this->username = $this->user?->getUsername() ?? ''; } public function getId(): string diff --git a/src/Entity/Traits/LogEntityTrait.php b/src/Entity/Traits/LogEntityTrait.php index ba7d479c7..2b36591ea 100644 --- a/src/Entity/Traits/LogEntityTrait.php +++ b/src/Entity/Traits/LogEntityTrait.php @@ -38,7 +38,7 @@ trait LogEntityTrait 'LogRequest', 'LogRequest.time', ])] - protected DateTimeImmutable $time; + protected readonly DateTimeImmutable $time; #[ORM\Column( name: '`date`', @@ -51,7 +51,7 @@ trait LogEntityTrait 'LogRequest', 'LogRequest.date', ])] - protected DateTimeImmutable $date; + protected readonly DateTimeImmutable $date; #[ORM\Column( name: 'agent', @@ -64,7 +64,7 @@ trait LogEntityTrait 'LogRequest', 'LogRequest.agent', ])] - protected string $agent = ''; + protected readonly string $agent; #[ORM\Column( name: 'http_host', @@ -78,7 +78,7 @@ trait LogEntityTrait 'LogRequest', 'LogRequest.httpHost', ])] - protected string $httpHost = ''; + protected readonly string $httpHost; #[ORM\Column( name: 'client_ip', @@ -92,7 +92,7 @@ trait LogEntityTrait 'LogRequest', 'LogRequest.clientIp', ])] - private string $clientIp = ''; + private readonly string $clientIp; public function getTime(): DateTimeImmutable { diff --git a/tests/Integration/Entity/LogLoginTest.php b/tests/Integration/Entity/LogLoginTest.php index e1c77d2ec..6a1ca6b5c 100644 --- a/tests/Integration/Entity/LogLoginTest.php +++ b/tests/Integration/Entity/LogLoginTest.php @@ -34,77 +34,6 @@ class LogLoginTest extends EntityTestCase */ protected string $entityName = LogLogin::class; - /** @noinspection PhpMissingParentCallCommonInspection */ - /** - * @testdox No setter for `$property` property in read only entity - so cannot test this - */ - public function testThatSetterOnlyAcceptSpecifiedType( - ?string $property = null, - ?string $type = null, - ?array $meta = null - ): void { - self::markTestSkipped('There is not setter in read only entity...'); - } - - /** @noinspection PhpMissingParentCallCommonInspection */ - /** - * @testdox No setter for `$property` property in read only entity - so cannot test this - */ - public function testThatSetterReturnsInstanceOfEntity( - ?string $property = null, - ?string $type = null, - ?array $meta = null - ): void { - self::markTestSkipped('There is not setter in read only entity...'); - } - - /** @noinspection PhpMissingParentCallCommonInspection */ - /** - * @dataProvider dataProviderTestThatSetterAndGettersWorks - * - * @throws Throwable - * - * @testdox Test that getter method for `$type $property` property returns expected - */ - public function testThatGetterReturnsExpectedValue(string $property, string $type, array $meta): void - { - $getter = 'get' . ucfirst($property); - - if (in_array($type, [PhpUnitUtil::TYPE_BOOL, PhpUnitUtil::TYPE_BOOLEAN], true)) { - $getter = 'is' . ucfirst($property); - } - - $request = Request::create(''); - - // Parse user agent data with device detector - $deviceDetector = new DeviceDetector((string)$request->headers->get('User-Agent')); - $deviceDetector->parse(); - - $logRequest = new LogLogin( - '', - $request, - $deviceDetector, - new User() - ); - - if (!(array_key_exists('columnName', $meta) || array_key_exists('joinColumns', $meta))) { - $type = ArrayCollection::class; - - self::assertInstanceOf($type, $logRequest->{$getter}()); - } - - try { - $method = 'assertIs' . ucfirst($type); - - self::$method($logRequest->{$getter}()); - } catch (Throwable $error) { - /** - * @var class-string $type - */ - self::assertInstanceOf($type, $logRequest->{$getter}(), $error->getMessage()); - } - } - /** * @noinspection PhpMissingParentCallCommonInspection * From 798dc554c21b887765ced481978210dc585a7263 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20Lepp=C3=A4nen?= Date: Mon, 30 May 2022 19:11:57 +0300 Subject: [PATCH 6/9] Changed entity properties to be `readonly` --- src/Entity/Healthz.php | 31 +++++++++++-------------------- src/Entity/LogLogin.php | 2 +- src/Entity/LogLoginFailure.php | 22 ++++++++++------------ src/Entity/LogRequest.php | 27 ++++++++++++--------------- 4 files changed, 34 insertions(+), 48 deletions(-) diff --git a/src/Entity/Healthz.php b/src/Entity/Healthz.php index 38b57b7ee..39e3b63bb 100644 --- a/src/Entity/Healthz.php +++ b/src/Entity/Healthz.php @@ -50,25 +50,23 @@ class Healthz implements EntityInterface ])] private UuidInterface $id; - #[ORM\Column( - name: 'timestamp', - type: Types::DATETIME_IMMUTABLE, - )] - #[Groups([ - 'Healthz', - 'Healthz.timestamp', - ])] - private DateTimeImmutable $timestamp; - /** * Healthz constructor. * * @throws Throwable */ - public function __construct() - { + public function __construct( + #[ORM\Column( + name: 'timestamp', + type: Types::DATETIME_IMMUTABLE, + )] + #[Groups([ + 'Healthz', + 'Healthz.timestamp', + ])] + private readonly DateTimeImmutable $timestamp = new DateTimeImmutable(timezone: new DateTimeZone('UTC')), + ) { $this->id = $this->createUuid(); - $this->timestamp = new DateTimeImmutable(timezone: new DateTimeZone('UTC')); } public function getId(): string @@ -81,13 +79,6 @@ public function getTimestamp(): DateTimeImmutable return $this->getCreatedAt(); } - public function setTimestamp(DateTimeImmutable $timestamp): self - { - $this->timestamp = $timestamp; - - return $this; - } - public function getCreatedAt(): DateTimeImmutable { return $this->timestamp; diff --git a/src/Entity/LogLogin.php b/src/Entity/LogLogin.php index a1f6b8b72..137862bb2 100644 --- a/src/Entity/LogLogin.php +++ b/src/Entity/LogLogin.php @@ -241,7 +241,7 @@ public function __construct( 'LogLogin.type', ])] private readonly string $type, - private Request $request, + readonly Request $request, private readonly DeviceDetector $deviceDetector, #[ORM\ManyToOne( targetEntity: User::class, diff --git a/src/Entity/LogLoginFailure.php b/src/Entity/LogLoginFailure.php index 1dceea401..db313da61 100644 --- a/src/Entity/LogLoginFailure.php +++ b/src/Entity/LogLoginFailure.php @@ -58,16 +58,6 @@ class LogLoginFailure implements EntityInterface ])] private UuidInterface $id; - #[ORM\Column( - name: 'timestamp', - type: Types::DATETIME_IMMUTABLE, - )] - #[Groups([ - 'LogLoginFailure', - 'LogLoginFailure.timestamp', - ])] - private DateTimeImmutable $timestamp; - /** * LogLoginFailure constructor. * @@ -87,10 +77,18 @@ public function __construct( 'LogLoginFailure', 'LogLoginFailure.user', ])] - private User $user + private readonly User $user, + #[ORM\Column( + name: 'timestamp', + type: Types::DATETIME_IMMUTABLE, + )] + #[Groups([ + 'LogLoginFailure', + 'LogLoginFailure.timestamp', + ])] + private readonly DateTimeImmutable $timestamp = new DateTimeImmutable(timezone: new DateTimeZone('UTC')), ) { $this->id = $this->createUuid(); - $this->timestamp = new DateTimeImmutable(timezone: new DateTimeZone('UTC')); } public function getId(): string diff --git a/src/Entity/LogRequest.php b/src/Entity/LogRequest.php index 30d997d0c..b3d333d4a 100644 --- a/src/Entity/LogRequest.php +++ b/src/Entity/LogRequest.php @@ -99,16 +99,6 @@ class LogRequest implements EntityInterface ])] private int $responseContentLength = 0; - #[ORM\Column( - name: 'is_main_request', - type: Types::BOOLEAN, - )] - #[Groups([ - 'LogRequest', - 'LogRequest.isMainRequest', - ])] - private bool $mainRequest; - /** * LogRequest constructor. * @@ -117,7 +107,7 @@ class LogRequest implements EntityInterface * @throws Throwable */ public function __construct( - private array $sensitiveProperties, + private readonly array $sensitiveProperties, ?Request $request = null, ?Response $response = null, #[ORM\ManyToOne( @@ -131,7 +121,7 @@ public function __construct( #[Groups([ 'LogRequest.user', ])] - private ?User $user = null, + private readonly ?User $user = null, #[ORM\ManyToOne( targetEntity: ApiKey::class, inversedBy: 'logsRequest', @@ -143,11 +133,18 @@ public function __construct( #[Groups([ 'LogRequest.apiKey', ])] - private ?ApiKey $apiKey = null, - ?bool $mainRequest = null + private readonly ?ApiKey $apiKey = null, + #[ORM\Column( + name: 'is_main_request', + type: Types::BOOLEAN, + )] + #[Groups([ + 'LogRequest', + 'LogRequest.isMainRequest', + ])] + private readonly bool $mainRequest = true, ) { $this->id = $this->createUuid(); - $this->mainRequest = $mainRequest ?? true; $this->processTimeAndDate(); From 9da18f7c7c8a70daea646ef0c4d5edbe51a107a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20Lepp=C3=A4nen?= Date: Mon, 30 May 2022 19:12:28 +0300 Subject: [PATCH 7/9] Refactored entity tests --- tests/Integration/Entity/DateDimensionTest.php | 2 +- tests/Integration/Entity/EntityTestCase.php | 8 +------- tests/Integration/Entity/LogLoginTest.php | 5 ----- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/tests/Integration/Entity/DateDimensionTest.php b/tests/Integration/Entity/DateDimensionTest.php index b7c851ee6..dab0a87a1 100644 --- a/tests/Integration/Entity/DateDimensionTest.php +++ b/tests/Integration/Entity/DateDimensionTest.php @@ -27,7 +27,7 @@ class DateDimensionTest extends EntityTestCase * @var class-string */ protected string $entityName = DateDimension::class; - + public function testThatGetCreatedAtMethodReturnsExpected(): void { $entity = $this->createEntity(); diff --git a/tests/Integration/Entity/EntityTestCase.php b/tests/Integration/Entity/EntityTestCase.php index 991fae3d7..40596d14d 100644 --- a/tests/Integration/Entity/EntityTestCase.php +++ b/tests/Integration/Entity/EntityTestCase.php @@ -127,12 +127,6 @@ public function testThatGetterAndSetterExists( sprintf($message, $this->entityName, $setter, $property), ); } - - self::assertSame( - !$readOnlyClass, - method_exists($entity, $setter), - sprintf($message, $this->entityName, $setter, $property), - ); } } @@ -153,7 +147,7 @@ public function testThatSetterOnlyAcceptSpecifiedType( if ((!array_key_exists('columnName', $meta) && !array_key_exists('joinColumns', $meta)) || $this->isReadOnlyProperty($property) ) { - self::markTestSkipped('No need to test this setter...'); + static::markTestSkipped('No need to test this setter...'); } $this->expectException(TypeError::class); diff --git a/tests/Integration/Entity/LogLoginTest.php b/tests/Integration/Entity/LogLoginTest.php index 6a1ca6b5c..9211e2f57 100644 --- a/tests/Integration/Entity/LogLoginTest.php +++ b/tests/Integration/Entity/LogLoginTest.php @@ -10,14 +10,9 @@ use App\Entity\LogLogin; use App\Entity\User; -use App\Utils\Tests\PhpUnitUtil; use DeviceDetector\DeviceDetector; -use Doctrine\Common\Collections\ArrayCollection; use Symfony\Component\HttpFoundation\Request; use Throwable; -use function array_key_exists; -use function in_array; -use function ucfirst; /** * Class LogLoginTest From 6a98f287be22c9bdb28e6ca0154b81ab59e722d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20Lepp=C3=A4nen?= Date: Wed, 1 Jun 2022 18:20:43 +0300 Subject: [PATCH 8/9] Some refactoring --- ...gEntityTrait.php => AbstractLogEntity.php} | 49 ++++++++----------- src/Entity/LogLogin.php | 39 ++++++--------- src/Entity/LogRequest.php | 48 +++++++++--------- .../Traits/LogRequestProcessRequestTrait.php | 48 ++++++------------ 4 files changed, 77 insertions(+), 107 deletions(-) rename src/Entity/{Traits/LogEntityTrait.php => AbstractLogEntity.php} (81%) diff --git a/src/Entity/Traits/LogEntityTrait.php b/src/Entity/AbstractLogEntity.php similarity index 81% rename from src/Entity/Traits/LogEntityTrait.php rename to src/Entity/AbstractLogEntity.php index 2b36591ea..5edc4b234 100644 --- a/src/Entity/Traits/LogEntityTrait.php +++ b/src/Entity/AbstractLogEntity.php @@ -1,14 +1,13 @@ */ -namespace App\Entity\Traits; +namespace App\Entity; -use App\Entity\User; use DateTimeImmutable; use DateTimeZone; use Doctrine\DBAL\Types\Types; @@ -18,14 +17,13 @@ use Throwable; /** - * Trait LogEntityTrait + * Class AbstractLogEntity * - * @package App\Entity\Traits + * @package App\Entity * @author TLe, Tarmo Leppänen - * - * @property User|null $user */ -trait LogEntityTrait +#[ORM\MappedSuperclass] +abstract class AbstractLogEntity { #[ORM\Column( name: 'time', @@ -92,7 +90,21 @@ trait LogEntityTrait 'LogRequest', 'LogRequest.clientIp', ])] - private readonly string $clientIp; + protected readonly string $clientIp; + + /** + * @throws Throwable + */ + public function __construct(?Request $request) + { + $now = new DateTimeImmutable(timezone: new DateTimeZone('UTC')); + + $this->time = $now; + $this->date = $now; + $this->clientIp = $request?->getClientIp() ?? ''; + $this->httpHost = $request?->getHttpHost() ?? ''; + $this->agent = $request?->headers->get('User-Agent') ?? ''; + } public function getTime(): DateTimeImmutable { @@ -123,23 +135,4 @@ public function getCreatedAt(): ?DateTimeImmutable { return $this->getDate(); } - - private function processRequestData(Request $request): void - { - $this->clientIp = $request->getClientIp() ?? ''; - $this->httpHost = $request->getHttpHost(); - $this->agent = $request->headers->get('User-Agent') ?? ''; - } - - /** - * @throws Throwable - */ - #[ORM\PrePersist] - private function processTimeAndDate(): void - { - $now = new DateTimeImmutable(timezone: new DateTimeZone('UTC')); - - $this->time = $now; - $this->date = $now; - } } diff --git a/src/Entity/LogLogin.php b/src/Entity/LogLogin.php index 137862bb2..329df8f30 100644 --- a/src/Entity/LogLogin.php +++ b/src/Entity/LogLogin.php @@ -10,7 +10,6 @@ use App\Doctrine\DBAL\Types\Types as AppTypes; use App\Entity\Interfaces\EntityInterface; -use App\Entity\Traits\LogEntityTrait; use App\Entity\Traits\Uuid; use DeviceDetector\DeviceDetector; use Doctrine\DBAL\Types\Types; @@ -50,9 +49,8 @@ )] #[ORM\HasLifecycleCallbacks] #[ORM\ChangeTrackingPolicy('DEFERRED_EXPLICIT')] -class LogLogin implements EntityInterface +class LogLogin extends AbstractLogEntity implements EntityInterface { - use LogEntityTrait; use Uuid; /** @@ -257,12 +255,21 @@ public function __construct( ])] private readonly ?User $user = null ) { - $this->id = $this->createUuid(); - - $this->processTimeAndDate(); - $this->processRequestData($request); - $this->processClientData(); + parent::__construct($request); + $this->id = $this->createUuid(); + $this->clientType = $this->getClientData('getClient', 'type'); + $this->clientName = $this->getClientData('getClient', 'name'); + $this->clientShortName = $this->getClientData('getClient', 'short_name'); + $this->clientVersion = $this->getClientData('getClient', 'version'); + $this->clientEngine = $this->getClientData('getClient', 'engine'); + $this->osName = $this->getClientData('getOs', 'name'); + $this->osShortName = $this->getClientData('getOs', 'short_name'); + $this->osVersion = $this->getClientData('getOs', 'version'); + $this->osPlatform = $this->getClientData('getOs', 'platform'); + $this->deviceName = $this->deviceDetector->getDeviceName(); + $this->brandName = $this->deviceDetector->getBrandName(); + $this->model = $this->deviceDetector->getModel(); $this->username = $this->user?->getUsername() ?? ''; } @@ -346,22 +353,6 @@ public function getModel(): ?string return $this->model; } - private function processClientData(): void - { - $this->clientType = $this->getClientData('getClient', 'type'); - $this->clientName = $this->getClientData('getClient', 'name'); - $this->clientShortName = $this->getClientData('getClient', 'short_name'); - $this->clientVersion = $this->getClientData('getClient', 'version'); - $this->clientEngine = $this->getClientData('getClient', 'engine'); - $this->osName = $this->getClientData('getOs', 'name'); - $this->osShortName = $this->getClientData('getOs', 'short_name'); - $this->osVersion = $this->getClientData('getOs', 'version'); - $this->osPlatform = $this->getClientData('getOs', 'platform'); - $this->deviceName = $this->deviceDetector->getDeviceName(); - $this->brandName = $this->deviceDetector->getBrandName(); - $this->model = $this->deviceDetector->getModel(); - } - private function getClientData(string $method, string $attribute): string { /** @var string|array $value */ diff --git a/src/Entity/LogRequest.php b/src/Entity/LogRequest.php index b3d333d4a..ac3c90e54 100644 --- a/src/Entity/LogRequest.php +++ b/src/Entity/LogRequest.php @@ -9,7 +9,6 @@ namespace App\Entity; use App\Entity\Interfaces\EntityInterface; -use App\Entity\Traits\LogEntityTrait; use App\Entity\Traits\LogRequestProcessRequestTrait; use App\Entity\Traits\Uuid; use Doctrine\DBAL\Types\Types; @@ -55,9 +54,8 @@ )] #[ORM\HasLifecycleCallbacks] #[ORM\ChangeTrackingPolicy('DEFERRED_EXPLICIT')] -class LogRequest implements EntityInterface +class LogRequest extends AbstractLogEntity implements EntityInterface { - use LogEntityTrait; use LogRequestProcessRequestTrait; use Uuid; @@ -87,7 +85,7 @@ class LogRequest implements EntityInterface 'LogRequest', 'LogRequest.statusCode', ])] - private int $statusCode = 0; + private readonly int $statusCode; #[ORM\Column( name: 'response_content_length', @@ -97,7 +95,7 @@ class LogRequest implements EntityInterface 'LogRequest', 'LogRequest.responseContentLength', ])] - private int $responseContentLength = 0; + private readonly int $responseContentLength; /** * LogRequest constructor. @@ -108,8 +106,8 @@ class LogRequest implements EntityInterface */ public function __construct( private readonly array $sensitiveProperties, - ?Request $request = null, - ?Response $response = null, + readonly ?Request $request = null, + readonly ?Response $response = null, #[ORM\ManyToOne( targetEntity: User::class, inversedBy: 'logsRequest', @@ -144,18 +142,32 @@ public function __construct( ])] private readonly bool $mainRequest = true, ) { - $this->id = $this->createUuid(); + parent::__construct($request); - $this->processTimeAndDate(); + $this->id = $this->createUuid(); + // Change this after - https://github.com/phpstan/phpstan/issues/6402 - is solved if ($request !== null) { - $this->processRequestData($request); - $this->processRequest($request); + $this->action = $this->determineAction($request); + $this->content = $this->cleanContent($request->getContent()); + $this->method = $request->getRealMethod(); + $this->scheme = $request->getScheme(); + $this->basePath = $request->getBasePath(); + $this->script = '/' . basename($request->getScriptName()); + $this->path = $request->getPathInfo(); + $this->queryString = $request->getRequestUri(); + $this->uri = $request->getUri(); + $this->controller = (string)$request->attributes->get('_controller', ''); + $this->contentType = (string)$request->getMimeType($request->getContentType() ?? ''); + $this->contentTypeShort = (string)$request->getContentType(); + $this->xmlHttpRequest = $request->isXmlHttpRequest(); + $this->headers = $this->processHeaders($request); + $this->parameters = $this->processParameters($request); } - if ($response !== null) { - $this->processResponse($response); - } + $content = $response?->getContent() ?? false; + $this->statusCode = $response?->getStatusCode() ?? 0; + $this->responseContentLength = $content === false ? 0 : mb_strlen($content); } public function getId(): string @@ -195,12 +207,4 @@ public function getSensitiveProperties(): array { return $this->sensitiveProperties; } - - private function processResponse(Response $response): void - { - $content = $response->getContent(); - - $this->statusCode = $response->getStatusCode(); - $this->responseContentLength = $content === false ? 0 : mb_strlen($content); - } } diff --git a/src/Entity/Traits/LogRequestProcessRequestTrait.php b/src/Entity/Traits/LogRequestProcessRequestTrait.php index e05ccf018..12995fd7e 100644 --- a/src/Entity/Traits/LogRequestProcessRequestTrait.php +++ b/src/Entity/Traits/LogRequestProcessRequestTrait.php @@ -17,7 +17,6 @@ use function array_key_exists; use function array_map; use function array_walk; -use function basename; use function explode; use function is_array; use function mb_strtolower; @@ -296,29 +295,27 @@ public function getContentTypeShort(): ?string return $this->contentTypeShort; } - protected function processRequest(Request $request): void - { - $this->processRequestBaseInfo($request); - $this->processHeadersAndParameters($request); - - $this->action = $this->determineAction($request); - $this->content = $this->cleanContent($request->getContent()); - } - - private function processHeadersAndParameters(Request $request): void + /** + * @return array>|array + */ + private function processHeaders(Request $request): array { $rawHeaders = $request->headers->all(); - // Clean possible sensitive data from parameters + // Clean possible sensitive data from headers array_walk( $rawHeaders, - function (mixed &$value, string|int $key): void { - $this->cleanParameters($value, (string)$key); - }, + fn (mixed &$value, string|int $key) => $this->cleanParameters($value, (string)$key) ); - $this->headers = $rawHeaders; + return $rawHeaders; + } + /** + * @return array + */ + private function processParameters(Request $request): array + { $rawParameters = $this->determineParameters($request); // Clean possible sensitive data from parameters @@ -327,22 +324,7 @@ function (mixed &$value, string|int $key): void { fn (mixed &$value, string $key) => $this->cleanParameters($value, $key), ); - $this->parameters = $rawParameters; - } - - private function processRequestBaseInfo(Request $request): void - { - $this->method = $request->getRealMethod(); - $this->scheme = $request->getScheme(); - $this->basePath = $request->getBasePath(); - $this->script = '/' . basename($request->getScriptName()); - $this->path = $request->getPathInfo(); - $this->queryString = $request->getRequestUri(); - $this->uri = $request->getUri(); - $this->controller = (string)$request->attributes->get('_controller', ''); - $this->contentType = (string)$request->getMimeType($request->getContentType() ?? ''); - $this->contentTypeShort = (string)$request->getContentType(); - $this->xmlHttpRequest = $request->isXmlHttpRequest(); + return $rawParameters; } private function determineAction(Request $request): string @@ -388,7 +370,7 @@ private function determineParameters(Request $request): array private function cleanParameters(mixed &$value, string $key): void { // What keys we should replace so that any sensitive data is not logged - $replacements = array_fill_keys($this->sensitiveProperties, $this->replaceValue); + $replacements = array_fill_keys($this->getSensitiveProperties(), $this->replaceValue); // Normalize current key $key = mb_strtolower($key); From 4647c5194ba99628b5c381c4237c00baf552acf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20Lepp=C3=A4nen?= Date: Sun, 5 Jun 2022 14:41:37 +0300 Subject: [PATCH 9/9] Fixed PHP Insight errors --- src/Entity/{AbstractLogEntity.php => LogEntity.php} | 6 +++--- src/Entity/LogLogin.php | 2 +- src/Entity/LogRequest.php | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) rename src/Entity/{AbstractLogEntity.php => LogEntity.php} (96%) diff --git a/src/Entity/AbstractLogEntity.php b/src/Entity/LogEntity.php similarity index 96% rename from src/Entity/AbstractLogEntity.php rename to src/Entity/LogEntity.php index 5edc4b234..69d30d547 100644 --- a/src/Entity/AbstractLogEntity.php +++ b/src/Entity/LogEntity.php @@ -1,7 +1,7 @@ */ @@ -17,13 +17,13 @@ use Throwable; /** - * Class AbstractLogEntity + * Class LogEntity * * @package App\Entity * @author TLe, Tarmo Leppänen */ #[ORM\MappedSuperclass] -abstract class AbstractLogEntity +abstract class LogEntity { #[ORM\Column( name: 'time', diff --git a/src/Entity/LogLogin.php b/src/Entity/LogLogin.php index 329df8f30..eb4fbb830 100644 --- a/src/Entity/LogLogin.php +++ b/src/Entity/LogLogin.php @@ -49,7 +49,7 @@ )] #[ORM\HasLifecycleCallbacks] #[ORM\ChangeTrackingPolicy('DEFERRED_EXPLICIT')] -class LogLogin extends AbstractLogEntity implements EntityInterface +class LogLogin extends LogEntity implements EntityInterface { use Uuid; diff --git a/src/Entity/LogRequest.php b/src/Entity/LogRequest.php index ac3c90e54..3ec6fe78e 100644 --- a/src/Entity/LogRequest.php +++ b/src/Entity/LogRequest.php @@ -54,7 +54,7 @@ )] #[ORM\HasLifecycleCallbacks] #[ORM\ChangeTrackingPolicy('DEFERRED_EXPLICIT')] -class LogRequest extends AbstractLogEntity implements EntityInterface +class LogRequest extends LogEntity implements EntityInterface { use LogRequestProcessRequestTrait; use Uuid; @@ -106,8 +106,8 @@ class LogRequest extends AbstractLogEntity implements EntityInterface */ public function __construct( private readonly array $sensitiveProperties, - readonly ?Request $request = null, - readonly ?Response $response = null, + readonly Request|null $request = null, + readonly Response|null $response = null, #[ORM\ManyToOne( targetEntity: User::class, inversedBy: 'logsRequest',