From ae131b036793b466928b8ea88b7a46f03f14700a Mon Sep 17 00:00:00 2001 From: Martin Georgiev Date: Wed, 26 Mar 2025 23:10:17 +0000 Subject: [PATCH 1/3] refactor: modernise the validation in active code and the associated tests for dealing with integer arrays --- .../Doctrine/DBAL/Types/BaseIntegerArray.php | 74 +++++++++++++------ .../Doctrine/DBAL/Types/BigIntArray.php | 8 +- ...idIntegerArrayItemForDatabaseException.php | 45 +++++++++++ ...InvalidIntegerArrayItemForPHPException.php | 35 +++++++++ .../Doctrine/DBAL/Types/IntegerArray.php | 8 +- .../Doctrine/DBAL/Types/SmallIntArray.php | 8 +- .../DBAL/Types/BaseIntegerArrayTestCase.php | 34 +++++++-- .../Doctrine/DBAL/Types/BigIntArrayTest.php | 61 ++++++++++++--- .../DBAL/Types/DoublePrecisionArrayTest.php | 4 - .../Doctrine/DBAL/Types/IntegerArrayTest.php | 52 +++++++++---- .../Doctrine/DBAL/Types/SmallIntArrayTest.php | 47 +++++++++--- 11 files changed, 298 insertions(+), 78 deletions(-) create mode 100644 src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidIntegerArrayItemForDatabaseException.php create mode 100644 src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidIntegerArrayItemForPHPException.php diff --git a/src/MartinGeorgiev/Doctrine/DBAL/Types/BaseIntegerArray.php b/src/MartinGeorgiev/Doctrine/DBAL/Types/BaseIntegerArray.php index a99bd3eb..9ff66306 100644 --- a/src/MartinGeorgiev/Doctrine/DBAL/Types/BaseIntegerArray.php +++ b/src/MartinGeorgiev/Doctrine/DBAL/Types/BaseIntegerArray.php @@ -4,7 +4,8 @@ namespace MartinGeorgiev\Doctrine\DBAL\Types; -use Doctrine\DBAL\Types\ConversionException; +use MartinGeorgiev\Doctrine\DBAL\Types\Exceptions\InvalidIntegerArrayItemForDatabaseException; +use MartinGeorgiev\Doctrine\DBAL\Types\Exceptions\InvalidIntegerArrayItemForPHPException; /** * @see https://www.postgresql.org/docs/9.4/static/arrays.html @@ -14,45 +15,74 @@ */ abstract class BaseIntegerArray extends BaseArray { - abstract protected function getMinValue(): string; + private const INTEGER_REGEX = '/^-?\d+$/'; - abstract protected function getMaxValue(): string; + abstract protected function getMinValue(): int; - /** - * @param mixed $item - */ - public function isValidArrayItemForDatabase($item): bool + abstract protected function getMaxValue(): int; + + public function isValidArrayItemForDatabase(mixed $item): bool { - if (!\is_int($item) && !\is_string($item)) { + try { + $this->throwIfInvalidArrayItemForDatabase($item); + } catch (InvalidIntegerArrayItemForDatabaseException) { return false; } - if (!(bool) \preg_match('/^-?\d+$/', (string) $item)) { - return false; + return true; + } + + private function throwIfInvalidArrayItemForDatabase(mixed $item): void + { + $isNotANumber = !\is_int($item) && !\is_string($item); + if ($isNotANumber) { + throw InvalidIntegerArrayItemForDatabaseException::isNotANumber($item); } - if ((string) $item < $this->getMinValue()) { - return false; + $stringValue = (string) $item; + if (!\preg_match(self::INTEGER_REGEX, $stringValue)) { + throw InvalidIntegerArrayItemForDatabaseException::doesNotMatchRegex($item); + } + + $doesNotFitIntoPHPInteger = $stringValue !== (string) (int) $stringValue; + if ($doesNotFitIntoPHPInteger) { + throw InvalidIntegerArrayItemForDatabaseException::isOutOfRange($item); } - return (string) $item <= $this->getMaxValue(); + $integerValue = (int) $item; + if ($integerValue < $this->getMinValue()) { + throw InvalidIntegerArrayItemForDatabaseException::isBelowMinValue($item); + } + + if ($integerValue > $this->getMaxValue()) { + throw InvalidIntegerArrayItemForDatabaseException::isAboveMaxValue($item); + } } - /** - * @param int|string|null $item Whole number - */ - public function transformArrayItemForPHP($item): ?int + public function transformArrayItemForPHP(mixed $item): ?int { if ($item === null) { return null; } + $isNotANumberCandidate = !\is_int($item) && !\is_string($item); + if ($isNotANumberCandidate) { + throw InvalidIntegerArrayItemForPHPException::forValueThatIsNotAValidPHPInteger($item, static::TYPE_NAME); + } + $stringValue = (string) $item; - $isInvalidPHPInt = !(bool) \preg_match('/^-?\d+$/', $stringValue) - || $stringValue < $this->getMinValue() - || $stringValue > $this->getMaxValue(); - if ($isInvalidPHPInt) { - throw new ConversionException(\sprintf('Given value of %s content cannot be transformed to valid PHP integer.', \var_export($item, true))); + if (!\preg_match(self::INTEGER_REGEX, $stringValue)) { + throw InvalidIntegerArrayItemForPHPException::forValueThatIsNotAValidPHPInteger($item, static::TYPE_NAME); + } + + $doesNotFitIntoPHPInteger = $stringValue !== (string) (int) $stringValue; + if ($doesNotFitIntoPHPInteger) { + throw InvalidIntegerArrayItemForPHPException::forValueOutOfRangeInPHP($item, static::TYPE_NAME); + } + + $doesNotFitIntoDatabaseInteger = $stringValue < $this->getMinValue() || $stringValue > $this->getMaxValue(); + if ($doesNotFitIntoDatabaseInteger) { + throw InvalidIntegerArrayItemForPHPException::forValueOutOfRangeInDatabaseType($item, static::TYPE_NAME); } return (int) $item; diff --git a/src/MartinGeorgiev/Doctrine/DBAL/Types/BigIntArray.php b/src/MartinGeorgiev/Doctrine/DBAL/Types/BigIntArray.php index 315fac30..c4931433 100644 --- a/src/MartinGeorgiev/Doctrine/DBAL/Types/BigIntArray.php +++ b/src/MartinGeorgiev/Doctrine/DBAL/Types/BigIntArray.php @@ -19,13 +19,13 @@ class BigIntArray extends BaseIntegerArray */ protected const TYPE_NAME = 'bigint[]'; - protected function getMinValue(): string + protected function getMinValue(): int { - return '-9223372036854775807'; + return PHP_INT_MIN; } - protected function getMaxValue(): string + protected function getMaxValue(): int { - return '9223372036854775807'; + return PHP_INT_MAX; } } diff --git a/src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidIntegerArrayItemForDatabaseException.php b/src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidIntegerArrayItemForDatabaseException.php new file mode 100644 index 00000000..c97478cb --- /dev/null +++ b/src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidIntegerArrayItemForDatabaseException.php @@ -0,0 +1,45 @@ + + */ +class InvalidIntegerArrayItemForDatabaseException extends ConversionException +{ + private static function create(string $message, mixed $value): self + { + return new self(\sprintf($message, \var_export($value, true))); + } + + public static function isNotANumber(mixed $value): self + { + return self::create('Given value of %s is not a number.', $value); + } + + public static function doesNotMatchRegex(mixed $value): self + { + return self::create('Given value of %s does not match integer regex.', $value); + } + + public static function isBelowMinValue(mixed $value): self + { + return self::create('Given value of %s is below minimum value.', $value); + } + + public static function isAboveMaxValue(mixed $value): self + { + return self::create('Given value of %s is above maximum value.', $value); + } + + public static function isOutOfRange(mixed $value): self + { + return self::create('Given value of %s is out of range.', $value); + } +} diff --git a/src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidIntegerArrayItemForPHPException.php b/src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidIntegerArrayItemForPHPException.php new file mode 100644 index 00000000..75c2c8fc --- /dev/null +++ b/src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidIntegerArrayItemForPHPException.php @@ -0,0 +1,35 @@ + + */ +class InvalidIntegerArrayItemForPHPException extends ConversionException +{ + private static function create(string $message, mixed $value, string $type): self + { + return new self(\sprintf($message, \var_export($value, true), $type)); + } + + public static function forValueThatIsNotAValidPHPInteger(mixed $value, string $type): self + { + return self::create('Given value of %s content cannot be transformed to valid PHP integer from PostgreSQL %s type', $value, $type); + } + + public static function forValueOutOfRangeInPHP(mixed $value, string $type): self + { + return self::create('Given value of %s is out of range for PHP integer but appears valid for PostgreSQL %s type', $value, $type); + } + + public static function forValueOutOfRangeInDatabaseType(mixed $value, string $type): self + { + return self::create('Given value of %s is out of range for PostgreSQL %s type', $value, $type); + } +} diff --git a/src/MartinGeorgiev/Doctrine/DBAL/Types/IntegerArray.php b/src/MartinGeorgiev/Doctrine/DBAL/Types/IntegerArray.php index 77fc5d2d..9546662f 100644 --- a/src/MartinGeorgiev/Doctrine/DBAL/Types/IntegerArray.php +++ b/src/MartinGeorgiev/Doctrine/DBAL/Types/IntegerArray.php @@ -19,13 +19,13 @@ class IntegerArray extends BaseIntegerArray */ protected const TYPE_NAME = 'integer[]'; - protected function getMinValue(): string + protected function getMinValue(): int { - return '-2147483648'; + return -2147483648; } - protected function getMaxValue(): string + protected function getMaxValue(): int { - return '2147483647'; + return 2147483647; } } diff --git a/src/MartinGeorgiev/Doctrine/DBAL/Types/SmallIntArray.php b/src/MartinGeorgiev/Doctrine/DBAL/Types/SmallIntArray.php index e238044d..8beca272 100644 --- a/src/MartinGeorgiev/Doctrine/DBAL/Types/SmallIntArray.php +++ b/src/MartinGeorgiev/Doctrine/DBAL/Types/SmallIntArray.php @@ -19,13 +19,13 @@ class SmallIntArray extends BaseIntegerArray */ protected const TYPE_NAME = 'smallint[]'; - protected function getMinValue(): string + protected function getMinValue(): int { - return '-32768'; + return -32768; } - protected function getMaxValue(): string + protected function getMaxValue(): int { - return '32767'; + return 32767; } } diff --git a/tests/MartinGeorgiev/Doctrine/DBAL/Types/BaseIntegerArrayTestCase.php b/tests/MartinGeorgiev/Doctrine/DBAL/Types/BaseIntegerArrayTestCase.php index 6f84245e..71fb7f97 100644 --- a/tests/MartinGeorgiev/Doctrine/DBAL/Types/BaseIntegerArrayTestCase.php +++ b/tests/MartinGeorgiev/Doctrine/DBAL/Types/BaseIntegerArrayTestCase.php @@ -5,14 +5,11 @@ namespace Tests\MartinGeorgiev\Doctrine\DBAL\Types; use MartinGeorgiev\Doctrine\DBAL\Types\BaseIntegerArray; -use PHPUnit\Framework\MockObject\MockObject; +use MartinGeorgiev\Doctrine\DBAL\Types\Exceptions\InvalidIntegerArrayItemForPHPException; use PHPUnit\Framework\TestCase; abstract class BaseIntegerArrayTestCase extends TestCase { - /** - * @var BaseIntegerArray&MockObject - */ protected BaseIntegerArray $fixture; /** @@ -33,10 +30,14 @@ public static function provideInvalidTransformations(): array return [ [true], [null], - [-0.1], ['string'], [[]], [new \stdClass()], + ['1.23'], + ['not_a_number'], + ['1e2'], + ['0xFF'], + ['123abc'], ]; } @@ -61,7 +62,28 @@ public function can_transform_to_php_value(int $phpValue, string $postgresValue) } /** - * @return list> + * @return list */ abstract public static function provideValidTransformations(): array; + + /** + * @test + * + * @dataProvider provideOutOfRangeValues + */ + public function throws_domain_exception_when_value_exceeds_range(string $outOfRangeValue): void + { + $this->expectException(InvalidIntegerArrayItemForPHPException::class); + $this->expectExceptionMessage('is out of range for PostgreSQL'); + + $this->fixture->transformArrayItemForPHP($outOfRangeValue); + } + + /** + * @return list + */ + abstract protected function provideOutOfRangeValues(): array; } diff --git a/tests/MartinGeorgiev/Doctrine/DBAL/Types/BigIntArrayTest.php b/tests/MartinGeorgiev/Doctrine/DBAL/Types/BigIntArrayTest.php index 143b2ee7..ee6b34cd 100644 --- a/tests/MartinGeorgiev/Doctrine/DBAL/Types/BigIntArrayTest.php +++ b/tests/MartinGeorgiev/Doctrine/DBAL/Types/BigIntArrayTest.php @@ -5,14 +5,13 @@ namespace Tests\MartinGeorgiev\Doctrine\DBAL\Types; use MartinGeorgiev\Doctrine\DBAL\Types\BigIntArray; +use MartinGeorgiev\Doctrine\DBAL\Types\Exceptions\InvalidIntegerArrayItemForPHPException; class BigIntArrayTest extends BaseIntegerArrayTestCase { protected function setUp(): void { - parent::setUp(); - - $this->fixture = new BigIntArray(); // @phpstan-ignore-line + $this->fixture = new BigIntArray(); } /** @@ -25,21 +24,39 @@ public function has_name(): void public static function provideInvalidTransformations(): array { - return \array_merge(parent::provideInvalidTransformations(), [['-9223372036854775807.01'], [-9_223_372_036_854_775_809.0]]); + return \array_merge(parent::provideInvalidTransformations(), [ + ['9223372036854775808'], // Greater than PHP_INT_MAX + ['-9223372036854775809'], // Less than PHP_INT_MIN + ['1.23e10'], // Scientific notation + ['12345.67890'], // Decimal number + ]); } /** - * @return list + * @return array */ public static function provideValidTransformations(): array { return [ [ - 'phpValue' => -9_223_372_036_854_775_807, - 'postgresValue' => '-9223372036854775807', + 'phpValue' => PHP_INT_MAX, + 'postgresValue' => (string) PHP_INT_MAX, + ], + [ + 'phpValue' => PHP_INT_MIN, + 'postgresValue' => (string) PHP_INT_MIN, + ], + [ + 'phpValue' => 0, + 'postgresValue' => '0', + ], + [ + 'phpValue' => 1, + 'postgresValue' => '1', + ], + [ + 'phpValue' => -1, + 'postgresValue' => '-1', ], [ 'phpValue' => 9_223_372_036_854_775_807, @@ -47,4 +64,28 @@ public static function provideValidTransformations(): array ], ]; } + + /** + * @test + * + * @dataProvider provideOutOfRangeValues + */ + public function throws_domain_exception_when_value_exceeds_range(string $outOfRangeValue): void + { + $this->expectException(InvalidIntegerArrayItemForPHPException::class); + $this->expectExceptionMessage('is out of range for PHP integer but appears valid for PostgreSQL'); + + $this->fixture->transformArrayItemForPHP($outOfRangeValue); + } + + /** + * @return array + */ + protected function provideOutOfRangeValues(): array + { + return [ + ['9223372036854775808'], // PHP_INT_MAX + 1 + ['-9223372036854775809'], // PHP_INT_MIN - 1 + ]; + } } diff --git a/tests/MartinGeorgiev/Doctrine/DBAL/Types/DoublePrecisionArrayTest.php b/tests/MartinGeorgiev/Doctrine/DBAL/Types/DoublePrecisionArrayTest.php index aa6579c4..1f5a1b11 100644 --- a/tests/MartinGeorgiev/Doctrine/DBAL/Types/DoublePrecisionArrayTest.php +++ b/tests/MartinGeorgiev/Doctrine/DBAL/Types/DoublePrecisionArrayTest.php @@ -31,10 +31,6 @@ public static function provideInvalidTransformations(): array ['1.123456789012345678'], // Too many decimal places (>15) ['2.2250738585072014E-309'], // Too close to zero ['-2.2250738585072014E-309'], // Too close to zero (negative) - ['not_a_number'], - ['1.23.45'], - ['1e'], // Invalid scientific notation - ['e1'], // Invalid scientific notation ]); } diff --git a/tests/MartinGeorgiev/Doctrine/DBAL/Types/IntegerArrayTest.php b/tests/MartinGeorgiev/Doctrine/DBAL/Types/IntegerArrayTest.php index 5d2279d6..e674b228 100644 --- a/tests/MartinGeorgiev/Doctrine/DBAL/Types/IntegerArrayTest.php +++ b/tests/MartinGeorgiev/Doctrine/DBAL/Types/IntegerArrayTest.php @@ -10,9 +10,7 @@ class IntegerArrayTest extends BaseIntegerArrayTestCase { protected function setUp(): void { - parent::setUp(); - - $this->fixture = new IntegerArray(); // @phpstan-ignore-line + $this->fixture = new IntegerArray(); } /** @@ -23,31 +21,57 @@ public function has_name(): void self::assertEquals('integer[]', $this->fixture->getName()); } - /** - * @return array - */ public static function provideInvalidTransformations(): array { - return \array_merge(parent::provideInvalidTransformations(), [['-2147483647.01'], [2_147_483_649]]); + return \array_merge(parent::provideInvalidTransformations(), [ + ['2147483648'], // Greater than max integer + ['-2147483649'], // Less than min integer + ['1.23e6'], // Scientific notation + ['123456.789'], // Decimal number + ]); } /** - * @return list + * @return array */ public static function provideValidTransformations(): array { return [ [ - 'phpValue' => -2_147_483_648, + 'phpValue' => 2147483647, + 'postgresValue' => '2147483647', + ], + [ + 'phpValue' => -2147483648, 'postgresValue' => '-2147483648', ], [ - 'phpValue' => 2_147_483_647, - 'postgresValue' => '2147483647', + 'phpValue' => 0, + 'postgresValue' => '0', + ], + [ + 'phpValue' => 1, + 'postgresValue' => '1', + ], + [ + 'phpValue' => -1, + 'postgresValue' => '-1', + ], + [ + 'phpValue' => 999999999, + 'postgresValue' => '999999999', ], ]; } + + /** + * @return array + */ + protected function provideOutOfRangeValues(): array + { + return [ + ['2147483648'], // MAX_INTEGER + 1 + ['-2147483649'], // MIN_INTEGER - 1 + ]; + } } diff --git a/tests/MartinGeorgiev/Doctrine/DBAL/Types/SmallIntArrayTest.php b/tests/MartinGeorgiev/Doctrine/DBAL/Types/SmallIntArrayTest.php index 3dfa6146..cf3b6fd5 100644 --- a/tests/MartinGeorgiev/Doctrine/DBAL/Types/SmallIntArrayTest.php +++ b/tests/MartinGeorgiev/Doctrine/DBAL/Types/SmallIntArrayTest.php @@ -10,9 +10,7 @@ class SmallIntArrayTest extends BaseIntegerArrayTestCase { protected function setUp(): void { - parent::setUp(); - - $this->fixture = new SmallIntArray(); // @phpstan-ignore-line + $this->fixture = new SmallIntArray(); } /** @@ -25,26 +23,55 @@ public function has_name(): void public static function provideInvalidTransformations(): array { - return \array_merge(parent::provideInvalidTransformations(), [['-32767.01'], [-32769]]); + return \array_merge(parent::provideInvalidTransformations(), [ + ['32768'], // Greater than max smallint + ['-32769'], // Less than min smallint + ['1.23e4'], // Scientific notation + ['123.45'], // Decimal number + ]); } /** - * @return list + * @return array */ public static function provideValidTransformations(): array { return [ + [ + 'phpValue' => 32767, + 'postgresValue' => '32767', + ], [ 'phpValue' => -32768, 'postgresValue' => '-32768', ], [ - 'phpValue' => 32767, - 'postgresValue' => '32767', + 'phpValue' => 0, + 'postgresValue' => '0', + ], + [ + 'phpValue' => 1, + 'postgresValue' => '1', + ], + [ + 'phpValue' => -1, + 'postgresValue' => '-1', + ], + [ + 'phpValue' => 9999, + 'postgresValue' => '9999', ], ]; } + + /** + * @return array + */ + protected function provideOutOfRangeValues(): array + { + return [ + ['32768'], // MAX_SMALLINT + 1 + ['-32769'], // MIN_SMALLINT - 1 + ]; + } } From 1c4d3d4ed635993144e0891ac01fdbd95b47ee64 Mon Sep 17 00:00:00 2001 From: Martin Georgiev Date: Wed, 26 Mar 2025 23:13:06 +0000 Subject: [PATCH 2/3] oh-no: "someone" forgot to run PHPStan --- .../Doctrine/DBAL/Types/BaseIntegerArrayTestCase.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/MartinGeorgiev/Doctrine/DBAL/Types/BaseIntegerArrayTestCase.php b/tests/MartinGeorgiev/Doctrine/DBAL/Types/BaseIntegerArrayTestCase.php index 71fb7f97..e9edba99 100644 --- a/tests/MartinGeorgiev/Doctrine/DBAL/Types/BaseIntegerArrayTestCase.php +++ b/tests/MartinGeorgiev/Doctrine/DBAL/Types/BaseIntegerArrayTestCase.php @@ -83,7 +83,7 @@ public function throws_domain_exception_when_value_exceeds_range(string $outOfRa } /** - * @return list + * @return array */ abstract protected function provideOutOfRangeValues(): array; } From ade068e8765225641168b8ca13823da87b2abcb8 Mon Sep 17 00:00:00 2001 From: Martin Georgiev Date: Wed, 26 Mar 2025 23:19:21 +0000 Subject: [PATCH 3/3] ensure we compare integers explicitly --- src/MartinGeorgiev/Doctrine/DBAL/Types/BaseIntegerArray.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/MartinGeorgiev/Doctrine/DBAL/Types/BaseIntegerArray.php b/src/MartinGeorgiev/Doctrine/DBAL/Types/BaseIntegerArray.php index 9ff66306..e8eef5e0 100644 --- a/src/MartinGeorgiev/Doctrine/DBAL/Types/BaseIntegerArray.php +++ b/src/MartinGeorgiev/Doctrine/DBAL/Types/BaseIntegerArray.php @@ -80,11 +80,12 @@ public function transformArrayItemForPHP(mixed $item): ?int throw InvalidIntegerArrayItemForPHPException::forValueOutOfRangeInPHP($item, static::TYPE_NAME); } - $doesNotFitIntoDatabaseInteger = $stringValue < $this->getMinValue() || $stringValue > $this->getMaxValue(); + $integerValue = (int) $item; + $doesNotFitIntoDatabaseInteger = $integerValue < $this->getMinValue() || $integerValue > $this->getMaxValue(); if ($doesNotFitIntoDatabaseInteger) { throw InvalidIntegerArrayItemForPHPException::forValueOutOfRangeInDatabaseType($item, static::TYPE_NAME); } - return (int) $item; + return $integerValue; } }