From b1905362053cf733bfcee7ca5d0b17a1a30f5c53 Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Tue, 28 May 2019 16:21:01 +0100 Subject: [PATCH 1/3] Check phpunit configuration for listeners The bridge listener can be registered via configuration by the user. In that case, we do not want to add it again to the list of listeners. Closes #31649 --- .../Bridge/PhpUnit/Legacy/CommandForV5.php | 17 ++++++++++++++--- .../Bridge/PhpUnit/Legacy/CommandForV6.php | 18 +++++++++++++++--- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Bridge/PhpUnit/Legacy/CommandForV5.php b/src/Symfony/Bridge/PhpUnit/Legacy/CommandForV5.php index 95dcb1e554..ed4128482f 100644 --- a/src/Symfony/Bridge/PhpUnit/Legacy/CommandForV5.php +++ b/src/Symfony/Bridge/PhpUnit/Legacy/CommandForV5.php @@ -23,8 +23,6 @@ class CommandForV5 extends \PHPUnit_TextUI_Command */ protected function createRunner() { - $listener = new SymfonyTestsListenerForV5(); - $this->arguments['listeners'] = isset($this->arguments['listeners']) ? $this->arguments['listeners'] : array(); $registeredLocally = false; @@ -37,8 +35,21 @@ class CommandForV5 extends \PHPUnit_TextUI_Command } } + if (isset($this->arguments['configuration'])) { + $configuration = $this->arguments['configuration']; + if (!$configuration instanceof \PHPUnit_Util_Configuration) { + $configuration = \PHPUnit_Util_Configuration::getInstance($this->arguments['configuration']); + } + foreach ($configuration->getListenerConfiguration() as $registeredListener) { + if ('Symfony\Bridge\PhpUnit\SymfonyTestsListener' === ltrim($registeredListener['class'], '\\')) { + $registeredLocally = true; + break; + } + } + } + if (!$registeredLocally) { - $this->arguments['listeners'][] = $listener; + $this->arguments['listeners'][] = new SymfonyTestsListenerForV5(); } return parent::createRunner(); diff --git a/src/Symfony/Bridge/PhpUnit/Legacy/CommandForV6.php b/src/Symfony/Bridge/PhpUnit/Legacy/CommandForV6.php index f8f75bb09a..93e1ad975b 100644 --- a/src/Symfony/Bridge/PhpUnit/Legacy/CommandForV6.php +++ b/src/Symfony/Bridge/PhpUnit/Legacy/CommandForV6.php @@ -13,6 +13,7 @@ namespace Symfony\Bridge\PhpUnit\Legacy; use PHPUnit\TextUI\Command as BaseCommand; use PHPUnit\TextUI\TestRunner as BaseRunner; +use PHPUnit\Util\Configuration; use Symfony\Bridge\PhpUnit\SymfonyTestsListener; /** @@ -27,8 +28,6 @@ class CommandForV6 extends BaseCommand */ protected function createRunner(): BaseRunner { - $listener = new SymfonyTestsListener(); - $this->arguments['listeners'] = isset($this->arguments['listeners']) ? $this->arguments['listeners'] : []; $registeredLocally = false; @@ -41,8 +40,21 @@ class CommandForV6 extends BaseCommand } } + if (isset($this->arguments['configuration'])) { + $configuration = $this->arguments['configuration']; + if (!$configuration instanceof Configuration) { + $configuration = Configuration::getInstance($this->arguments['configuration']); + } + foreach ($configuration->getListenerConfiguration() as $registeredListener) { + if ('Symfony\Bridge\PhpUnit\SymfonyTestsListener' === ltrim($registeredListener['class'], '\\')) { + $registeredLocally = true; + break; + } + } + } + if (!$registeredLocally) { - $this->arguments['listeners'][] = $listener; + $this->arguments['listeners'][] = new SymfonyTestsListener(); } return parent::createRunner(); From 28d7d9444f6ee23bc094a28c53b6beced936549d Mon Sep 17 00:00:00 2001 From: Thomas Calvet Date: Tue, 3 Sep 2019 10:55:46 +0200 Subject: [PATCH 2/3] [Validator] Sync string to date behavior and throw a better exception --- .../AbstractComparisonValidator.php | 16 ++++++------ .../Validator/Constraints/RangeValidator.php | 19 ++++++++++++-- .../AbstractComparisonValidatorTestCase.php | 26 +++++++++++++++++++ .../Tests/Constraints/RangeValidatorTest.php | 26 +++++++++++++++++++ 4 files changed, 77 insertions(+), 10 deletions(-) diff --git a/src/Symfony/Component/Validator/Constraints/AbstractComparisonValidator.php b/src/Symfony/Component/Validator/Constraints/AbstractComparisonValidator.php index 7e9934d133..cd96bc9df7 100644 --- a/src/Symfony/Component/Validator/Constraints/AbstractComparisonValidator.php +++ b/src/Symfony/Component/Validator/Constraints/AbstractComparisonValidator.php @@ -65,14 +65,14 @@ abstract class AbstractComparisonValidator extends ConstraintValidator // This allows to compare with any date/time value supported by // the DateTime constructor: // https://php.net/datetime.formats - if (\is_string($comparedValue)) { - if ($value instanceof \DateTimeImmutable) { - // If $value is immutable, convert the compared value to a - // DateTimeImmutable too - $comparedValue = new \DateTimeImmutable($comparedValue); - } elseif ($value instanceof \DateTimeInterface) { - // Otherwise use DateTime - $comparedValue = new \DateTime($comparedValue); + if (\is_string($comparedValue) && $value instanceof \DateTimeInterface) { + // If $value is immutable, convert the compared value to a DateTimeImmutable too, otherwise use DateTime + $dateTimeClass = $value instanceof \DateTimeImmutable ? \DateTimeImmutable::class : \DateTime::class; + + try { + $comparedValue = new $dateTimeClass($comparedValue); + } catch (\Exception $e) { + throw new ConstraintDefinitionException(sprintf('The compared value "%s" could not be converted to a "%s" instance in the "%s" constraint.', $comparedValue, $dateTimeClass, \get_class($constraint))); } } diff --git a/src/Symfony/Component/Validator/Constraints/RangeValidator.php b/src/Symfony/Component/Validator/Constraints/RangeValidator.php index c7cb859a5a..ea7d277808 100644 --- a/src/Symfony/Component/Validator/Constraints/RangeValidator.php +++ b/src/Symfony/Component/Validator/Constraints/RangeValidator.php @@ -13,6 +13,7 @@ namespace Symfony\Component\Validator\Constraints; use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\ConstraintValidator; +use Symfony\Component\Validator\Exception\ConstraintDefinitionException; use Symfony\Component\Validator\Exception\UnexpectedTypeException; /** @@ -50,12 +51,26 @@ class RangeValidator extends ConstraintValidator // the DateTime constructor: // https://php.net/datetime.formats if ($value instanceof \DateTimeInterface) { + $dateTimeClass = null; + if (\is_string($min)) { - $min = new \DateTime($min); + $dateTimeClass = $value instanceof \DateTimeImmutable ? \DateTimeImmutable::class : \DateTime::class; + + try { + $min = new $dateTimeClass($min); + } catch (\Exception $e) { + throw new ConstraintDefinitionException(sprintf('The min value "%s" could not be converted to a "%s" instance in the "%s" constraint.', $min, $dateTimeClass, \get_class($constraint))); + } } if (\is_string($max)) { - $max = new \DateTime($max); + $dateTimeClass = $dateTimeClass ?: ($value instanceof \DateTimeImmutable ? \DateTimeImmutable::class : \DateTime::class); + + try { + $max = new $dateTimeClass($max); + } catch (\Exception $e) { + throw new ConstraintDefinitionException(sprintf('The max value "%s" could not be converted to a "%s" instance in the "%s" constraint.', $max, $dateTimeClass, \get_class($constraint))); + } } } diff --git a/src/Symfony/Component/Validator/Tests/Constraints/AbstractComparisonValidatorTestCase.php b/src/Symfony/Component/Validator/Tests/Constraints/AbstractComparisonValidatorTestCase.php index cb10c06164..b02e57cfa2 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/AbstractComparisonValidatorTestCase.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/AbstractComparisonValidatorTestCase.php @@ -13,6 +13,7 @@ namespace Symfony\Component\Validator\Tests\Constraints; use Symfony\Component\Intl\Util\IntlTestHelper; use Symfony\Component\Validator\Constraint; +use Symfony\Component\Validator\Constraints\AbstractComparison; use Symfony\Component\Validator\Exception\ConstraintDefinitionException; use Symfony\Component\Validator\Test\ConstraintValidatorTestCase; @@ -211,6 +212,31 @@ abstract class AbstractComparisonValidatorTestCase extends ConstraintValidatorTe ->assertRaised(); } + /** + * @dataProvider throwsOnInvalidStringDatesProvider + */ + public function testThrowsOnInvalidStringDates(AbstractComparison $constraint, $expectedMessage, $value) + { + $this->expectException(ConstraintDefinitionException::class); + $this->expectExceptionMessage($expectedMessage); + + $this->validator->validate($value, $constraint); + } + + public function throwsOnInvalidStringDatesProvider() + { + $constraint = $this->createConstraint([ + 'value' => 'foo', + ]); + + $constraintClass = \get_class($constraint); + + return [ + [$constraint, sprintf('The compared value "foo" could not be converted to a "DateTimeImmutable" instance in the "%s" constraint.', $constraintClass), new \DateTimeImmutable()], + [$constraint, sprintf('The compared value "foo" could not be converted to a "DateTime" instance in the "%s" constraint.', $constraintClass), new \DateTime()], + ]; + } + /** * @return array */ diff --git a/src/Symfony/Component/Validator/Tests/Constraints/RangeValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/RangeValidatorTest.php index df33d2c422..83e32d8d97 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/RangeValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/RangeValidatorTest.php @@ -14,6 +14,7 @@ namespace Symfony\Component\Validator\Tests\Constraints; use Symfony\Component\Intl\Util\IntlTestHelper; use Symfony\Component\Validator\Constraints\Range; use Symfony\Component\Validator\Constraints\RangeValidator; +use Symfony\Component\Validator\Exception\ConstraintDefinitionException; use Symfony\Component\Validator\Test\ConstraintValidatorTestCase; class RangeValidatorTest extends ConstraintValidatorTestCase @@ -389,4 +390,29 @@ class RangeValidatorTest extends ConstraintValidatorTestCase ->setCode(Range::INVALID_CHARACTERS_ERROR) ->assertRaised(); } + + /** + * @dataProvider throwsOnInvalidStringDatesProvider + */ + public function testThrowsOnInvalidStringDates($expectedMessage, $value, $min, $max) + { + $this->expectException(ConstraintDefinitionException::class); + $this->expectExceptionMessage($expectedMessage); + + $this->validator->validate($value, new Range([ + 'min' => $min, + 'max' => $max, + ])); + } + + public function throwsOnInvalidStringDatesProvider() + { + return [ + ['The min value "foo" could not be converted to a "DateTimeImmutable" instance in the "Symfony\Component\Validator\Constraints\Range" constraint.', new \DateTimeImmutable(), 'foo', null], + ['The min value "foo" could not be converted to a "DateTime" instance in the "Symfony\Component\Validator\Constraints\Range" constraint.', new \DateTime(), 'foo', null], + ['The max value "foo" could not be converted to a "DateTimeImmutable" instance in the "Symfony\Component\Validator\Constraints\Range" constraint.', new \DateTimeImmutable(), null, 'foo'], + ['The max value "foo" could not be converted to a "DateTime" instance in the "Symfony\Component\Validator\Constraints\Range" constraint.', new \DateTime(), null, 'foo'], + ['The min value "bar" could not be converted to a "DateTimeImmutable" instance in the "Symfony\Component\Validator\Constraints\Range" constraint.', new \DateTimeImmutable(), 'bar', 'ccc'], + ]; + } } From b688aa31ec9e533aa85593c728316f1ed491cfee Mon Sep 17 00:00:00 2001 From: Thomas Calvet Date: Tue, 3 Sep 2019 13:59:35 +0200 Subject: [PATCH 3/3] [Validator] Add ConstraintValidator::formatValue() tests --- .../Validator/ConstraintValidator.php | 6 +- .../Tests/ConstraintValidatorTest.php | 65 +++++++++++++++++++ 2 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 src/Symfony/Component/Validator/Tests/ConstraintValidatorTest.php diff --git a/src/Symfony/Component/Validator/ConstraintValidator.php b/src/Symfony/Component/Validator/ConstraintValidator.php index 35f41889ea..93cca2ea74 100644 --- a/src/Symfony/Component/Validator/ConstraintValidator.php +++ b/src/Symfony/Component/Validator/ConstraintValidator.php @@ -85,12 +85,10 @@ abstract class ConstraintValidator implements ConstraintValidatorInterface */ protected function formatValue($value, $format = 0) { - $isDateTime = $value instanceof \DateTimeInterface; - - if (($format & self::PRETTY_DATE) && $isDateTime) { + if (($format & self::PRETTY_DATE) && $value instanceof \DateTimeInterface) { if (class_exists('IntlDateFormatter')) { $locale = \Locale::getDefault(); - $formatter = new \IntlDateFormatter($locale, \IntlDateFormatter::MEDIUM, \IntlDateFormatter::SHORT); + $formatter = new \IntlDateFormatter($locale, \IntlDateFormatter::MEDIUM, \IntlDateFormatter::SHORT, $value->getTimezone()); // neither the native nor the stub IntlDateFormatter support // DateTimeImmutable as of yet diff --git a/src/Symfony/Component/Validator/Tests/ConstraintValidatorTest.php b/src/Symfony/Component/Validator/Tests/ConstraintValidatorTest.php new file mode 100644 index 0000000000..96af6f13eb --- /dev/null +++ b/src/Symfony/Component/Validator/Tests/ConstraintValidatorTest.php @@ -0,0 +1,65 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Validator\Tests; + +use PHPUnit\Framework\TestCase; +use Symfony\Component\Validator\Constraint; +use Symfony\Component\Validator\ConstraintValidator; + +final class ConstraintValidatorTest extends TestCase +{ + /** + * @dataProvider formatValueProvider + */ + public function testFormatValue($expected, $value, $format = 0) + { + $this->assertSame($expected, (new TestFormatValueConstraintValidator())->formatValueProxy($value, $format)); + } + + public function formatValueProvider() + { + $data = [ + ['true', true], + ['false', false], + ['null', null], + ['resource', fopen('php://memory', 'r')], + ['"foo"', 'foo'], + ['array', []], + ['object', $toString = new TestToStringObject()], + ['ccc', $toString, ConstraintValidator::OBJECT_TO_STRING], + ['object', $dateTime = (new \DateTimeImmutable('@0'))->setTimezone(new \DateTimeZone('UTC'))], + [class_exists(\IntlDateFormatter::class) ? 'Jan 1, 1970, 12:00 AM' : '1970-01-01 00:00:00', $dateTime, ConstraintValidator::PRETTY_DATE], + ]; + + return $data; + } +} + +final class TestFormatValueConstraintValidator extends ConstraintValidator +{ + public function validate($value, Constraint $constraint) + { + } + + public function formatValueProxy($value, $format) + { + return $this->formatValue($value, $format); + } +} + +final class TestToStringObject +{ + public function __toString() + { + return 'ccc'; + } +}