From ab20c52115e7b9d586e66a9eebc3d9e202e82ded Mon Sep 17 00:00:00 2001 From: Daniele Alessandri Date: Fri, 24 Jul 2015 14:59:23 +0200 Subject: [PATCH] Do not parse response to EXISTS into boolean value. Starting with Redis 3.0.3 the EXISTS command is variadic so that it is possible to check for the existence of multiple keys in one request, with the server returning the number of keys found. This change could break codebases relying on strict comparison (===) against a boolean value, but just doing $redis->exists('key') == TRUE is totally fine. --- src/Command/KeyExists.php | 8 --- tests/Predis/Command/ConnectionSelectTest.php | 2 +- tests/Predis/Command/KeyExistsTest.php | 50 ++++++++++++++++--- tests/Predis/Command/KeyExpireAtTest.php | 4 +- tests/Predis/Command/KeyExpireTest.php | 4 +- tests/Predis/Command/KeyMoveTest.php | 4 +- .../Predis/Command/KeyPreciseExpireAtTest.php | 4 +- tests/Predis/Command/KeyPreciseExpireTest.php | 4 +- .../Predis/Command/KeyRenamePreserveTest.php | 4 +- tests/Predis/Command/KeyRenameTest.php | 4 +- tests/Predis/Command/ListPushHeadXTest.php | 2 +- tests/Predis/Command/ListPushTailXTest.php | 2 +- .../Command/ServerFlushDatabaseTest.php | 2 +- .../Command/SetIntersectionStoreTest.php | 2 +- tests/Predis/Command/StringGetTest.php | 4 +- .../Command/StringPreciseSetExpireTest.php | 4 +- tests/Predis/Command/StringSetExpireTest.php | 4 +- .../Command/StringSetMultiplePreserveTest.php | 2 +- tests/Predis/Command/StringSetTest.php | 2 +- .../Predis/Command/TransactionDiscardTest.php | 2 +- tests/Predis/Pipeline/PipelineTest.php | 10 ++-- tests/Predis/Transaction/MultiExecTest.php | 6 +-- 22 files changed, 80 insertions(+), 50 deletions(-) diff --git a/src/Command/KeyExists.php b/src/Command/KeyExists.php index 1035baa8..aa4516dc 100644 --- a/src/Command/KeyExists.php +++ b/src/Command/KeyExists.php @@ -24,12 +24,4 @@ class KeyExists extends Command { return 'EXISTS'; } - - /** - * {@inheritdoc} - */ - public function parseResponse($data) - { - return (bool) $data; - } } diff --git a/tests/Predis/Command/ConnectionSelectTest.php b/tests/Predis/Command/ConnectionSelectTest.php index caa9f5e6..93abb45a 100644 --- a/tests/Predis/Command/ConnectionSelectTest.php +++ b/tests/Predis/Command/ConnectionSelectTest.php @@ -65,7 +65,7 @@ class ConnectionSelectTest extends PredisCommandTestCase $redis->set('foo', 'bar'); $this->assertEquals('OK', $redis->select(REDIS_SERVER_DBNUM - 1)); - $this->assertFalse($redis->exists('foo')); + $this->assertSame(0, $redis->exists('foo')); } /** diff --git a/tests/Predis/Command/KeyExistsTest.php b/tests/Predis/Command/KeyExistsTest.php index d1fe63b0..6d72deb7 100644 --- a/tests/Predis/Command/KeyExistsTest.php +++ b/tests/Predis/Command/KeyExistsTest.php @@ -47,6 +47,20 @@ class KeyExistsTest extends PredisCommandTestCase $this->assertSame($expected, $command->getArguments()); } + /** + * @group disconnected + */ + public function testFilterArgumentsMultipleKeys() + { + $arguments = array('key:1', 'key:2', 'key:3'); + $expected = array('key:1', 'key:2', 'key:3'); + + $command = $this->getCommand(); + $command->setArguments($arguments); + + $this->assertSame($expected, $command->getArguments()); + } + /** * @group disconnected */ @@ -54,28 +68,52 @@ class KeyExistsTest extends PredisCommandTestCase { $command = $this->getCommand(); - $this->assertTrue($command->parseResponse(1)); - $this->assertFalse($command->parseResponse(0)); + $this->assertSame(0, $command->parseResponse(0)); + $this->assertSame(1, $command->parseResponse(1)); + $this->assertSame(2, $command->parseResponse(2)); } /** * @group connected */ - public function testReturnsTrueIfKeyExists() + public function testReturnValueWhenKeyExists() { $redis = $this->getClient(); $redis->set('foo', 'bar'); - $this->assertTrue($redis->exists('foo')); + $this->assertSame(1, $redis->exists('foo')); } /** * @group connected */ - public function testReturnsFalseIfKeyDoesNotExist() + public function testReturnValueWhenKeyDoesNotExist() { $redis = $this->getClient(); - $this->assertFalse($redis->exists('foo')); + $this->assertSame(0, $redis->exists('foo')); + } + + /** + * @group connected + * @expectedRedisVersion >= 3.0.3 + */ + public function testReturnValueWhenKeysExist() + { + $redis = $this->getClient(); + + $redis->mset('foo', 'bar', 'hoge', 'piyo'); + $this->assertSame(2, $redis->exists('foo', 'hoge')); + } + + /** + * @group connected + * @expectedRedisVersion >= 3.0.3 + */ + public function testReturnValueWhenKeyDoNotExist() + { + $redis = $this->getClient(); + + $this->assertSame(0, $redis->exists('foo', 'bar')); } } diff --git a/tests/Predis/Command/KeyExpireAtTest.php b/tests/Predis/Command/KeyExpireAtTest.php index 6e5166ac..1567d7d8 100644 --- a/tests/Predis/Command/KeyExpireAtTest.php +++ b/tests/Predis/Command/KeyExpireAtTest.php @@ -85,7 +85,7 @@ class KeyExpireAtTest extends PredisCommandTestCase $this->sleep(2); - $this->assertFalse($redis->exists('foo')); + $this->assertSame(0, $redis->exists('foo')); } /** @@ -99,6 +99,6 @@ class KeyExpireAtTest extends PredisCommandTestCase $redis->set('foo', 'bar'); $this->assertTrue($redis->expireat('foo', $now - 100)); - $this->assertFalse($redis->exists('foo')); + $this->assertSame(0, $redis->exists('foo')); } } diff --git a/tests/Predis/Command/KeyExpireTest.php b/tests/Predis/Command/KeyExpireTest.php index f219ce6a..a27060eb 100644 --- a/tests/Predis/Command/KeyExpireTest.php +++ b/tests/Predis/Command/KeyExpireTest.php @@ -84,7 +84,7 @@ class KeyExpireTest extends PredisCommandTestCase $this->sleep(2.0); - $this->assertFalse($redis->exists('foo')); + $this->assertSame(0, $redis->exists('foo')); } /** @@ -113,6 +113,6 @@ class KeyExpireTest extends PredisCommandTestCase $redis->set('foo', 'bar'); $this->assertTrue($redis->expire('foo', -10)); - $this->assertFalse($redis->exists('foo')); + $this->assertSame(0, $redis->exists('foo')); } } diff --git a/tests/Predis/Command/KeyMoveTest.php b/tests/Predis/Command/KeyMoveTest.php index 22154d55..36d9bbd5 100644 --- a/tests/Predis/Command/KeyMoveTest.php +++ b/tests/Predis/Command/KeyMoveTest.php @@ -70,10 +70,10 @@ class KeyMoveTest extends PredisCommandTestCase $redis->set('foo', 'bar'); $this->assertTrue($redis->move('foo', $db)); - $this->assertFalse($redis->exists('foo')); + $this->assertSame(0, $redis->exists('foo')); $redis->select($db); - $this->assertTrue($redis->exists('foo')); + $this->assertSame(1, $redis->exists('foo')); $redis->del('foo'); } diff --git a/tests/Predis/Command/KeyPreciseExpireAtTest.php b/tests/Predis/Command/KeyPreciseExpireAtTest.php index eb2ce4b5..c2cb825b 100644 --- a/tests/Predis/Command/KeyPreciseExpireAtTest.php +++ b/tests/Predis/Command/KeyPreciseExpireAtTest.php @@ -75,7 +75,7 @@ class KeyPreciseExpireAtTest extends PredisCommandTestCase $this->sleep($ttl + 0.5); - $this->assertFalse($redis->exists('foo')); + $this->assertSame(0, $redis->exists('foo')); } /** @@ -88,6 +88,6 @@ class KeyPreciseExpireAtTest extends PredisCommandTestCase $redis->set('foo', 'bar'); $this->assertTrue($redis->expireat('foo', time() - 100000)); - $this->assertFalse($redis->exists('foo')); + $this->assertSame(0, $redis->exists('foo')); } } diff --git a/tests/Predis/Command/KeyPreciseExpireTest.php b/tests/Predis/Command/KeyPreciseExpireTest.php index a7f057d7..c0173949 100644 --- a/tests/Predis/Command/KeyPreciseExpireTest.php +++ b/tests/Predis/Command/KeyPreciseExpireTest.php @@ -83,7 +83,7 @@ class KeyPreciseExpireTest extends PredisCommandTestCase $this->assertTrue($redis->pexpire('foo', $ttl)); $this->sleep(1.2); - $this->assertFalse($redis->exists('foo')); + $this->assertSame(0, $redis->exists('foo')); } /** @@ -114,6 +114,6 @@ class KeyPreciseExpireTest extends PredisCommandTestCase $redis->set('foo', 'bar'); $this->assertTrue($redis->pexpire('foo', -10000)); - $this->assertFalse($redis->exists('foo')); + $this->assertSame(0, $redis->exists('foo')); } } diff --git a/tests/Predis/Command/KeyRenamePreserveTest.php b/tests/Predis/Command/KeyRenamePreserveTest.php index 4a2078da..90eb5eaa 100644 --- a/tests/Predis/Command/KeyRenamePreserveTest.php +++ b/tests/Predis/Command/KeyRenamePreserveTest.php @@ -66,8 +66,8 @@ class KeyRenamePreserveTest extends PredisCommandTestCase $redis->set('foo', 'bar'); $this->assertTrue($redis->renamenx('foo', 'foofoo')); - $this->assertFalse($redis->exists('foo')); - $this->assertTrue($redis->exists('foofoo')); + $this->assertSame(0, $redis->exists('foo')); + $this->assertSame(1, $redis->exists('foofoo')); } /** diff --git a/tests/Predis/Command/KeyRenameTest.php b/tests/Predis/Command/KeyRenameTest.php index c2f428fd..5517b04e 100644 --- a/tests/Predis/Command/KeyRenameTest.php +++ b/tests/Predis/Command/KeyRenameTest.php @@ -65,8 +65,8 @@ class KeyRenameTest extends PredisCommandTestCase $redis->set('foo', 'bar'); $this->assertEquals('OK', $redis->rename('foo', 'foofoo')); - $this->assertFalse($redis->exists('foo')); - $this->assertTrue($redis->exists('foofoo')); + $this->assertSame(0, $redis->exists('foo')); + $this->assertSame(1, $redis->exists('foofoo')); } /** diff --git a/tests/Predis/Command/ListPushHeadXTest.php b/tests/Predis/Command/ListPushHeadXTest.php index 55650192..6a9841f7 100644 --- a/tests/Predis/Command/ListPushHeadXTest.php +++ b/tests/Predis/Command/ListPushHeadXTest.php @@ -77,7 +77,7 @@ class ListPushHeadXTest extends PredisCommandTestCase $this->assertSame(0, $redis->lpushx('metavars', 'foo')); $this->assertSame(0, $redis->lpushx('metavars', 'hoge')); - $this->assertFalse($redis->exists('metavars')); + $this->assertSame(0, $redis->exists('metavars')); } /** diff --git a/tests/Predis/Command/ListPushTailXTest.php b/tests/Predis/Command/ListPushTailXTest.php index 898a19c2..b99957e9 100644 --- a/tests/Predis/Command/ListPushTailXTest.php +++ b/tests/Predis/Command/ListPushTailXTest.php @@ -77,7 +77,7 @@ class ListPushTailXTest extends PredisCommandTestCase $this->assertSame(0, $redis->rpushx('metavars', 'foo')); $this->assertSame(0, $redis->rpushx('metavars', 'hoge')); - $this->assertFalse($redis->exists('metavars')); + $this->assertSame(0, $redis->exists('metavars')); } /** diff --git a/tests/Predis/Command/ServerFlushDatabaseTest.php b/tests/Predis/Command/ServerFlushDatabaseTest.php index a86e9b9b..5f642580 100644 --- a/tests/Predis/Command/ServerFlushDatabaseTest.php +++ b/tests/Predis/Command/ServerFlushDatabaseTest.php @@ -62,6 +62,6 @@ class ServerFlushDatabaseTest extends PredisCommandTestCase $redis->set('foo', 'bar'); $this->assertEquals('OK', $redis->flushdb()); - $this->assertFalse($redis->exists('foo')); + $this->assertSame(0, $redis->exists('foo')); } } diff --git a/tests/Predis/Command/SetIntersectionStoreTest.php b/tests/Predis/Command/SetIntersectionStoreTest.php index 19de440f..44beeebd 100644 --- a/tests/Predis/Command/SetIntersectionStoreTest.php +++ b/tests/Predis/Command/SetIntersectionStoreTest.php @@ -92,7 +92,7 @@ class SetIntersectionStoreTest extends PredisCommandTestCase $redis->sadd('letters:1st', 'a', 'b', 'c', 'd', 'e', 'f', 'g'); $this->assertSame(0, $redis->sinterstore('letters:destination', 'letters:1st', 'letters:2nd')); - $this->assertFalse($redis->exists('letters:destination')); + $this->assertSame(0, $redis->exists('letters:destination')); } /** diff --git a/tests/Predis/Command/StringGetTest.php b/tests/Predis/Command/StringGetTest.php index c64fd507..0e99b4d0 100644 --- a/tests/Predis/Command/StringGetTest.php +++ b/tests/Predis/Command/StringGetTest.php @@ -75,7 +75,7 @@ class StringGetTest extends PredisCommandTestCase $redis->set('foo', ''); - $this->assertTrue($redis->exists('foo')); + $this->assertSame(1, $redis->exists('foo')); $this->assertSame('', $redis->get('foo')); } @@ -86,7 +86,7 @@ class StringGetTest extends PredisCommandTestCase { $redis = $this->getClient(); - $this->assertFalse($redis->exists('foo')); + $this->assertSame(0, $redis->exists('foo')); $this->assertNull($redis->get('foo')); } diff --git a/tests/Predis/Command/StringPreciseSetExpireTest.php b/tests/Predis/Command/StringPreciseSetExpireTest.php index c93744e6..b985e9b7 100644 --- a/tests/Predis/Command/StringPreciseSetExpireTest.php +++ b/tests/Predis/Command/StringPreciseSetExpireTest.php @@ -63,7 +63,7 @@ class StringPreciseSetExpireTest extends PredisCommandTestCase $redis = $this->getClient(); $this->assertEquals('OK', $redis->psetex('foo', 10000, 'bar')); - $this->assertTrue($redis->exists('foo')); + $this->assertSame(1, $redis->exists('foo')); $this->assertEquals(10, $redis->ttl('foo')); } @@ -77,7 +77,7 @@ class StringPreciseSetExpireTest extends PredisCommandTestCase $redis->psetex('foo', 50, 'bar'); $this->sleep(0.5); - $this->assertFalse($redis->exists('foo'));; + $this->assertSame(0, $redis->exists('foo'));; } /** diff --git a/tests/Predis/Command/StringSetExpireTest.php b/tests/Predis/Command/StringSetExpireTest.php index b52e9fab..e4197167 100644 --- a/tests/Predis/Command/StringSetExpireTest.php +++ b/tests/Predis/Command/StringSetExpireTest.php @@ -63,7 +63,7 @@ class StringSetExpireTest extends PredisCommandTestCase $redis = $this->getClient(); $this->assertEquals('OK', $redis->setex('foo', 10, 'bar')); - $this->assertTrue($redis->exists('foo')); + $this->assertSame(1, $redis->exists('foo')); $this->assertEquals(10, $redis->ttl('foo')); } @@ -78,7 +78,7 @@ class StringSetExpireTest extends PredisCommandTestCase $redis->setex('foo', 1, 'bar'); $this->sleep(2.0); - $this->assertFalse($redis->exists('foo'));; + $this->assertSame(0, $redis->exists('foo'));; } /** diff --git a/tests/Predis/Command/StringSetMultiplePreserveTest.php b/tests/Predis/Command/StringSetMultiplePreserveTest.php index f3a768f0..3a72aa95 100644 --- a/tests/Predis/Command/StringSetMultiplePreserveTest.php +++ b/tests/Predis/Command/StringSetMultiplePreserveTest.php @@ -92,6 +92,6 @@ class StringSetMultiplePreserveTest extends PredisCommandTestCase $this->assertFalse($redis->msetnx('foo', 'barbar', 'hoge', 'piyo')); $this->assertSame('bar', $redis->get('foo')); - $this->assertFalse($redis->exists('hoge')); + $this->assertSame(0, $redis->exists('hoge')); } } diff --git a/tests/Predis/Command/StringSetTest.php b/tests/Predis/Command/StringSetTest.php index aeaa1d40..3d2ba51d 100644 --- a/tests/Predis/Command/StringSetTest.php +++ b/tests/Predis/Command/StringSetTest.php @@ -63,7 +63,7 @@ class StringSetTest extends PredisCommandTestCase $redis = $this->getClient(); $this->assertEquals('OK', $redis->set('foo', 'bar')); - $this->assertTrue($redis->exists('foo')); + $this->assertSame(1, $redis->exists('foo')); $this->assertSame('bar', $redis->get('foo')); } } diff --git a/tests/Predis/Command/TransactionDiscardTest.php b/tests/Predis/Command/TransactionDiscardTest.php index ab89b5c9..a661e4de 100644 --- a/tests/Predis/Command/TransactionDiscardTest.php +++ b/tests/Predis/Command/TransactionDiscardTest.php @@ -63,7 +63,7 @@ class TransactionDiscardTest extends PredisCommandTestCase $this->assertEquals('QUEUED', $redis->set('foo', 'bar')); $this->assertEquals('OK', $redis->discard()); - $this->assertFalse($redis->exists('foo')); + $this->assertSame(0, $redis->exists('foo')); } /** diff --git a/tests/Predis/Pipeline/PipelineTest.php b/tests/Predis/Pipeline/PipelineTest.php index 492133ad..f8a1b0e9 100644 --- a/tests/Predis/Pipeline/PipelineTest.php +++ b/tests/Predis/Pipeline/PipelineTest.php @@ -390,7 +390,7 @@ class PipelineTest extends PredisTestCase }); $this->assertEquals(array('OK', 'bar'), $results); - $this->assertTrue($client->exists('foo')); + $this->assertSame(1, $client->exists('foo')); } /** @@ -409,7 +409,7 @@ class PipelineTest extends PredisTestCase $this->assertEquals(array('OK', 'bar'), $results); $this->assertSame('oob message', $oob); - $this->assertTrue($client->exists('foo')); + $this->assertSame(1, $client->exists('foo')); } /** @@ -432,7 +432,7 @@ class PipelineTest extends PredisTestCase $this->assertInstanceOf('Predis\ClientException', $exception); $this->assertSame('TEST', $exception->getMessage()); - $this->assertFalse($client->exists('foo')); + $this->assertSame(0, $client->exists('foo')); } /** @@ -457,8 +457,8 @@ class PipelineTest extends PredisTestCase } $this->assertInstanceOf('Predis\Response\ServerException', $exception); - $this->assertTrue($client->exists('foo')); - $this->assertTrue($client->exists('hoge')); + $this->assertSame(1, $client->exists('foo')); + $this->assertSame(1, $client->exists('hoge')); } /** diff --git a/tests/Predis/Transaction/MultiExecTest.php b/tests/Predis/Transaction/MultiExecTest.php index e54edfac..1711d839 100644 --- a/tests/Predis/Transaction/MultiExecTest.php +++ b/tests/Predis/Transaction/MultiExecTest.php @@ -662,7 +662,7 @@ class MultiExecTest extends PredisTestCase } $this->assertInstanceOf('RuntimeException', $exception); - $this->assertFalse($client->exists('foo')); + $this->assertSame(0, $client->exists('foo')); } /** @@ -720,8 +720,8 @@ class MultiExecTest extends PredisTestCase }); $this->assertSame(1, count($responses)); - $this->assertFalse($client->exists('foo')); - $this->assertTrue($client->exists('hoge')); + $this->assertSame(0, $client->exists('foo')); + $this->assertSame(1, $client->exists('hoge')); } /**