From d689caece8001ab98b3f060088f5f62dcdbf5756 Mon Sep 17 00:00:00 2001 From: Javier Spagnoletti Date: Wed, 19 Apr 2023 13:35:24 -0300 Subject: [PATCH] Deprecate passing timezone information to methods where it is ignored --- src/Types/DateImmutableType.php | 38 ++++++++- tests/Types/DateImmutableTypeTest.php | 111 +++++++++++++++++++++++++- 2 files changed, 145 insertions(+), 4 deletions(-) diff --git a/src/Types/DateImmutableType.php b/src/Types/DateImmutableType.php index da96b69d543..5f66ed8ee3a 100644 --- a/src/Types/DateImmutableType.php +++ b/src/Types/DateImmutableType.php @@ -35,6 +35,22 @@ public function convertToDatabaseValue($value, AbstractPlatform $platform) } if ($value instanceof DateTimeImmutable) { + $offset = $value->format('O'); + $defaultOffset = (new DateTimeImmutable())->format('O'); + + if ($offset !== $defaultOffset) { + Deprecation::triggerIfCalledFromOutside( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/6020', + 'Passing a timezone offset (%s) different than the default one (%s) is deprecated' + . ' as it will be lost, use %s::%s() instead.', + $offset, + $defaultOffset, + DateTimeTzImmutableType::class, + __FUNCTION__, + ); + } + return $value->format($platform->getDateFormatString()); } @@ -56,7 +72,27 @@ public function convertToDatabaseValue($value, AbstractPlatform $platform) */ public function convertToPHPValue($value, AbstractPlatform $platform) { - if ($value === null || $value instanceof DateTimeImmutable) { + if ($value === null) { + return null; + } + + if ($value instanceof DateTimeImmutable) { + $offset = $value->format('O'); + $defaultOffset = (new DateTimeImmutable())->format('O'); + + if ($offset !== $defaultOffset) { + Deprecation::triggerIfCalledFromOutside( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/6020', + 'Passing a timezone offset (%s) different than the default one (%s) is deprecated' + . ' as it may be lost, use %s::%s() instead.', + $offset, + $defaultOffset, + DateTimeTzImmutableType::class, + __FUNCTION__, + ); + } + return $value; } diff --git a/tests/Types/DateImmutableTypeTest.php b/tests/Types/DateImmutableTypeTest.php index f21c2802b12..e4504d80a24 100644 --- a/tests/Types/DateImmutableTypeTest.php +++ b/tests/Types/DateImmutableTypeTest.php @@ -4,10 +4,13 @@ use DateTime; use DateTimeImmutable; +use DateTimeZone; use Doctrine\DBAL\ParameterType; use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Types\ConversionException; use Doctrine\DBAL\Types\DateImmutableType; +use Doctrine\Deprecations\PHPUnit\VerifyDeprecations; +use InvalidArgumentException; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -15,6 +18,8 @@ class DateImmutableTypeTest extends TestCase { + use VerifyDeprecations; + /** @var AbstractPlatform&MockObject */ private AbstractPlatform $platform; @@ -48,10 +53,18 @@ public function testConvertsDateTimeImmutableInstanceToDatabaseValue(): void $this->platform->expects(self::once()) ->method('getDateFormatString') ->willReturn('Y-m-d'); - $date->expects(self::once()) + $date->expects(self::exactly(2)) ->method('format') - ->with('Y-m-d') - ->willReturn('2016-01-01'); + ->willReturnCallback(static function (string $format): string { + switch ($format) { + case 'O': + return 'UTC'; + case 'Y-m-d': + return '2016-01-01'; + default: + throw new InvalidArgumentException(); + } + }); self::assertSame( '2016-01-01', @@ -117,4 +130,96 @@ public function testRequiresSQLCommentHint(): void { self::assertTrue($this->type->requiresSQLCommentHint($this->platform)); } + + /** @dataProvider provideDateTimeInstancesWithTimezone */ + public function testTimezoneDeprecationFromConvertsToDatabaseValue( + string $defaultTimeZone, + DateTimeImmutable $date + ): void { + $this->iniSet('date.timezone', $defaultTimeZone); + + $defaultOffset = (new DateTimeImmutable())->format('O'); + + self::assertFalse($defaultOffset === $date->format('O')); + + $this->platform->expects(self::once()) + ->method('getDateFormatString') + ->willReturn('Y-m-d'); + + $this->expectDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/6020'); + + $this->type->convertToDatabaseValue($date, $this->platform); + } + + /** @dataProvider provideDateTimeInstancesWithTimezone */ + public function testTimezoneDeprecationFromConvertToPHPValue(string $defaultTimeZone, DateTimeImmutable $date): void + { + $this->iniSet('date.timezone', $defaultTimeZone); + + $defaultOffset = (new DateTimeImmutable())->format('O'); + + self::assertFalse($defaultOffset === $date->format('O')); + + $this->expectDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/6020'); + + $this->type->convertToPHPValue($date, $this->platform); + } + + /** @psalm-return iterable */ + public function provideDateTimeInstancesWithTimezone(): iterable + { + yield 'timezone_utc' => [ + 'UTC', + (new DateTimeImmutable())->setTimezone(new DateTimeZone('America/Argentina/Buenos_Aires')), + ]; + + yield 'timezone_amsterdam' => [ + 'Europe/Amsterdam', + (new DateTimeImmutable())->setTimezone(new DateTimeZone('America/Argentina/Buenos_Aires')), + ]; + + yield 'offset_utc' => [ + 'UTC', + (new DateTimeImmutable())->setTimezone(new DateTimeZone('-0300')), + ]; + } + + /** @dataProvider provideDateTimeInstancesWithNoTimezoneDiff */ + public function testNoTimezoneInValueConversion(string $defaultTimeZone, DateTimeImmutable $date): void + { + $this->iniSet('date.timezone', $defaultTimeZone); + + $this->platform->expects(self::once()) + ->method('getDateFormatString') + ->willReturn('Y-m-d'); + + $this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/6020'); + + $this->type->convertToDatabaseValue($date, $this->platform); + $this->type->convertToPHPValue($date, $this->platform); + } + + /** @psalm-return iterable */ + public function provideDateTimeInstancesWithNoTimezoneDiff(): iterable + { + yield 'timezone_utc' => [ + 'UTC', + (new DateTimeImmutable())->setTimezone(new DateTimeZone('UTC')), + ]; + + yield 'timezone_buenos_aires' => [ + 'America/Argentina/Buenos_Aires', + (new DateTimeImmutable())->setTimezone(new DateTimeZone('America/Argentina/Buenos_Aires')), + ]; + + yield 'same_offset_with_different_timezones' => [ + 'America/Sao_Paulo', + (new DateTimeImmutable())->setTimezone(new DateTimeZone('America/Argentina/Buenos_Aires')), + ]; + + yield 'offset_-0300' => [ + 'America/Argentina/Buenos_Aires', + (new DateTimeImmutable())->setTimezone(new DateTimeZone('-0300')), + ]; + } }