From 45754515a5d4c177270503bd261d3a0cf314b8bc Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Fri, 17 Aug 2018 19:53:24 +0200 Subject: [PATCH 1/5] fix type error handling when writing values --- .../PropertyAccess/PropertyAccessor.php | 7 +++++- .../Tests/Fixtures/ReturnTyped.php | 5 ++++ .../Tests/PropertyAccessorTest.php | 23 +++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index 3d1885a857..d639506ef3 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -231,7 +231,12 @@ class PropertyAccessor implements PropertyAccessorInterface private static function throwInvalidArgumentException($message, $trace, $i) { - if (isset($trace[$i]['file']) && __FILE__ === $trace[$i]['file'] && isset($trace[$i]['args'][0])) { + // the type mismatch is not caused by invalid arguments (but e.g. by an incompatible return type hint of the writer method) + if (0 !== strpos($message, 'Argument ')) { + return; + } + + if (isset($trace[$i]['file']) && __FILE__ === $trace[$i]['file'] && array_key_exists(0, $trace[$i]['args'])) { $pos = strpos($message, $delim = 'must be of the type ') ?: (strpos($message, $delim = 'must be an instance of ') ?: strpos($message, $delim = 'must implement interface ')); $pos += \strlen($delim); $type = $trace[$i]['args'][0]; diff --git a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/ReturnTyped.php b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/ReturnTyped.php index b6a9852715..71c4a574c0 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/ReturnTyped.php +++ b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/ReturnTyped.php @@ -28,4 +28,9 @@ class ReturnTyped public function removeFoo(\DateTime $dateTime) { } + + public function setName($name): self + { + return 'This does not respect the return type on purpose.'; + } } diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php index ec314a6e69..b05672910a 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php @@ -538,6 +538,17 @@ class PropertyAccessorTest extends TestCase $this->propertyAccessor->setValue($object, 'date', 'This is a string, \DateTime expected.'); } + /** + * @expectedException \Symfony\Component\PropertyAccess\Exception\InvalidArgumentException + * @expectedExceptionMessage Expected argument of type "DateTime", "NULL" given + */ + public function testThrowTypeErrorWithNullArgument() + { + $object = new TypeHinted(); + + $this->propertyAccessor->setValue($object, 'date', null); + } + public function testSetTypeHint() { $date = new \DateTime(); @@ -579,4 +590,16 @@ class PropertyAccessorTest extends TestCase $this->propertyAccessor->setValue($object, 'foos', array(new \DateTime())); } + + /** + * @requires PHP 7 + * + * @expectedException \TypeError + */ + public function testDoNotDiscardReturnTypeErrorWhenWriterMethodIsMisconfigured() + { + $object = new ReturnTyped(); + + $this->propertyAccessor->setValue($object, 'name', 'foo'); + } } From 5bdc755d73a7d6a9304afac35f0576a10014209f Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Sun, 19 Aug 2018 18:51:04 +0200 Subject: [PATCH 2/5] fix data mapper return type in docblock --- src/Symfony/Component/Form/FormConfigInterface.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Form/FormConfigInterface.php b/src/Symfony/Component/Form/FormConfigInterface.php index e66570cf5f..b7083544a4 100644 --- a/src/Symfony/Component/Form/FormConfigInterface.php +++ b/src/Symfony/Component/Form/FormConfigInterface.php @@ -99,7 +99,7 @@ interface FormConfigInterface /** * Returns the data mapper of the form. * - * @return DataMapperInterface The data mapper + * @return DataMapperInterface|null The data mapper */ public function getDataMapper(); From 8b59d177db2373e28f5c73de38da168a5cc2fc11 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 23 Aug 2018 17:01:50 +0200 Subject: [PATCH 3/5] [Cache] enable Memcached::OPT_TCP_NODELAY to fix perf of misses --- .../Component/Cache/Tests/Adapter/MemcachedAdapterTest.php | 1 + src/Symfony/Component/Cache/Traits/MemcachedTrait.php | 1 + 2 files changed, 2 insertions(+) diff --git a/src/Symfony/Component/Cache/Tests/Adapter/MemcachedAdapterTest.php b/src/Symfony/Component/Cache/Tests/Adapter/MemcachedAdapterTest.php index 76e0608006..d1f8790340 100644 --- a/src/Symfony/Component/Cache/Tests/Adapter/MemcachedAdapterTest.php +++ b/src/Symfony/Component/Cache/Tests/Adapter/MemcachedAdapterTest.php @@ -89,6 +89,7 @@ class MemcachedAdapterTest extends AdapterTestCase $this->assertTrue($client->getOption(\Memcached::OPT_COMPRESSION)); $this->assertSame(1, $client->getOption(\Memcached::OPT_BINARY_PROTOCOL)); + $this->assertSame(1, $client->getOption(\Memcached::OPT_TCP_NODELAY)); $this->assertSame(1, $client->getOption(\Memcached::OPT_LIBKETAMA_COMPATIBLE)); } diff --git a/src/Symfony/Component/Cache/Traits/MemcachedTrait.php b/src/Symfony/Component/Cache/Traits/MemcachedTrait.php index 7e36164293..5983d9ebd1 100644 --- a/src/Symfony/Component/Cache/Traits/MemcachedTrait.php +++ b/src/Symfony/Component/Cache/Traits/MemcachedTrait.php @@ -134,6 +134,7 @@ trait MemcachedTrait $options = array_change_key_case($options, CASE_UPPER); $client->setOption(\Memcached::OPT_BINARY_PROTOCOL, true); $client->setOption(\Memcached::OPT_NO_BLOCK, true); + $client->setOption(\Memcached::OPT_TCP_NODELAY, true); if (!array_key_exists('LIBKETAMA_COMPATIBLE', $options) && !array_key_exists(\Memcached::OPT_LIBKETAMA_COMPATIBLE, $options)) { $client->setOption(\Memcached::OPT_LIBKETAMA_COMPATIBLE, true); } From f245df404f9ffbc3f2a647a660eeee6dc267b050 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 23 Aug 2018 23:19:02 +0200 Subject: [PATCH 4/5] [travis] enable Redis cluster --- .travis.yml | 7 ++++ .../Tests/Adapter/RedisClusterAdapterTest.php | 27 ++++++++++++++ .../Tests/Simple/RedisClusterCacheTest.php | 27 ++++++++++++++ .../Component/Lock/Store/RedisStore.php | 6 ++-- .../Tests/Store/RedisClusterStoreTest.php | 35 +++++++++++++++++++ 5 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 src/Symfony/Component/Cache/Tests/Adapter/RedisClusterAdapterTest.php create mode 100644 src/Symfony/Component/Cache/Tests/Simple/RedisClusterCacheTest.php create mode 100644 src/Symfony/Component/Lock/Tests/Store/RedisClusterStoreTest.php diff --git a/.travis.yml b/.travis.yml index 68b36579f4..9690c4a934 100644 --- a/.travis.yml +++ b/.travis.yml @@ -41,8 +41,15 @@ services: - memcached - mongodb - redis-server + - docker before_install: + - | + # Start Redis cluster + docker pull grokzen/redis-cluster:4.0.8 + docker run -d -p 7000:7000 -p 7001:7001 -p 7002:7002 -p 7003:7003 -p 7004:7004 -p 7005:7005 --name redis-cluster grokzen/redis-cluster:4.0.8 + export REDIS_CLUSTER_HOSTS='localhost:7000 localhost:7001 localhost:7002 localhost:7003 localhost:7004 localhost:7005' + - | # General configuration set -e diff --git a/src/Symfony/Component/Cache/Tests/Adapter/RedisClusterAdapterTest.php b/src/Symfony/Component/Cache/Tests/Adapter/RedisClusterAdapterTest.php new file mode 100644 index 0000000000..852079c00c --- /dev/null +++ b/src/Symfony/Component/Cache/Tests/Adapter/RedisClusterAdapterTest.php @@ -0,0 +1,27 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Cache\Tests\Adapter; + +class RedisClusterAdapterTest extends AbstractRedisAdapterTest +{ + public static function setupBeforeClass() + { + if (!class_exists('RedisCluster')) { + self::markTestSkipped('The RedisCluster class is required.'); + } + if (!$hosts = getenv('REDIS_CLUSTER_HOSTS')) { + self::markTestSkipped('REDIS_CLUSTER_HOSTS env var is not defined.'); + } + + self::$redis = new \RedisCluster(null, explode(' ', $hosts)); + } +} diff --git a/src/Symfony/Component/Cache/Tests/Simple/RedisClusterCacheTest.php b/src/Symfony/Component/Cache/Tests/Simple/RedisClusterCacheTest.php new file mode 100644 index 0000000000..99d4e518fb --- /dev/null +++ b/src/Symfony/Component/Cache/Tests/Simple/RedisClusterCacheTest.php @@ -0,0 +1,27 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Cache\Tests\Simple; + +class RedisClusterCacheTest extends AbstractRedisCacheTest +{ + public static function setupBeforeClass() + { + if (!class_exists('RedisCluster')) { + self::markTestSkipped('The RedisCluster class is required.'); + } + if (!$hosts = getenv('REDIS_CLUSTER_HOSTS')) { + self::markTestSkipped('REDIS_CLUSTER_HOSTS env var is not defined.'); + } + + self::$redis = new \RedisCluster(null, explode(' ', $hosts)); + } +} diff --git a/src/Symfony/Component/Lock/Store/RedisStore.php b/src/Symfony/Component/Lock/Store/RedisStore.php index a65e8cb8f5..e3b1500532 100644 --- a/src/Symfony/Component/Lock/Store/RedisStore.php +++ b/src/Symfony/Component/Lock/Store/RedisStore.php @@ -54,8 +54,10 @@ class RedisStore implements StoreInterface $script = ' if redis.call("GET", KEYS[1]) == ARGV[1] then return redis.call("PEXPIRE", KEYS[1], ARGV[2]) + elseif redis.call("SET", KEYS[1], ARGV[1], "NX", "PX", ARGV[2]) then + return 1 else - return redis.call("set", KEYS[1], ARGV[1], "NX", "PX", ARGV[2]) + return 0 end '; @@ -144,7 +146,7 @@ class RedisStore implements StoreInterface return \call_user_func_array(array($this->redis, 'eval'), array_merge(array($script, 1, $resource), $args)); } - throw new InvalidArgumentException(sprintf('%s() expects been initialized with a Redis, RedisArray, RedisCluster or Predis\Client, %s given', __METHOD__, \is_object($this->redis) ? \get_class($this->redis) : \gettype($this->redis))); + throw new InvalidArgumentException(sprintf('%s() expects being initialized with a Redis, RedisArray, RedisCluster or Predis\Client, %s given', __METHOD__, \is_object($this->redis) ? \get_class($this->redis) : \gettype($this->redis))); } /** diff --git a/src/Symfony/Component/Lock/Tests/Store/RedisClusterStoreTest.php b/src/Symfony/Component/Lock/Tests/Store/RedisClusterStoreTest.php new file mode 100644 index 0000000000..2ee17888eb --- /dev/null +++ b/src/Symfony/Component/Lock/Tests/Store/RedisClusterStoreTest.php @@ -0,0 +1,35 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Lock\Tests\Store; + +/** + * @author Jérémy Derussé + * + * @requires extension redis + */ +class RedisClusterStoreTest extends AbstractRedisStoreTest +{ + public static function setupBeforeClass() + { + if (!class_exists('RedisCluster')) { + self::markTestSkipped('The RedisCluster class is required.'); + } + if (!getenv('REDIS_CLUSTER_HOSTS')) { + self::markTestSkipped('REDIS_CLUSTER_HOSTS env var is not defined.'); + } + } + + protected function getRedisConnection() + { + return new \RedisCluster(null, explode(' ', getenv('REDIS_CLUSTER_HOSTS'))); + } +} From 2ac883a99bb543f776ce3dfc49d002fa853ab99b Mon Sep 17 00:00:00 2001 From: David Maicher Date: Thu, 23 Aug 2018 21:04:06 +0200 Subject: [PATCH 5/5] [DoctrineBridge] support __toString as documented for UniqueEntityValidator --- .../Tests/Validator/Constraints/UniqueEntityValidatorTest.php | 2 +- .../Doctrine/Validator/Constraints/UniqueEntityValidator.php | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Bridge/Doctrine/Tests/Validator/Constraints/UniqueEntityValidatorTest.php b/src/Symfony/Bridge/Doctrine/Tests/Validator/Constraints/UniqueEntityValidatorTest.php index a26ce98107..d8b55eb808 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/Validator/Constraints/UniqueEntityValidatorTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/Validator/Constraints/UniqueEntityValidatorTest.php @@ -482,7 +482,7 @@ class UniqueEntityValidatorTest extends ConstraintValidatorTestCase $this->buildViolation('myMessage') ->atPath('property.path.single') - ->setParameter('{{ value }}', 'object("Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdEntity") identified by (id => 1)') + ->setParameter('{{ value }}', 'foo') ->setInvalidValue($entity1) ->setCode(UniqueEntity::NOT_UNIQUE_ERROR) ->setCause(array($associated, $associated2)) diff --git a/src/Symfony/Bridge/Doctrine/Validator/Constraints/UniqueEntityValidator.php b/src/Symfony/Bridge/Doctrine/Validator/Constraints/UniqueEntityValidator.php index 2015e7fb3a..161a187ff9 100644 --- a/src/Symfony/Bridge/Doctrine/Validator/Constraints/UniqueEntityValidator.php +++ b/src/Symfony/Bridge/Doctrine/Validator/Constraints/UniqueEntityValidator.php @@ -186,6 +186,10 @@ class UniqueEntityValidator extends ConstraintValidator return $this->formatValue($value, self::PRETTY_DATE); } + if (\method_exists($value, '__toString')) { + return (string) $value; + } + if ($class->getName() !== $idClass = \get_class($value)) { // non unique value might be a composite PK that consists of other entity objects if ($em->getMetadataFactory()->hasMetadataFor($idClass)) {