diff --git a/CHANGELOG.md b/CHANGELOG.md index 55298d2e..41e129b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,12 @@ v0.8.0 (201x-xx-xx) Redis are not generated by connection classes anymore but are thrown by the client class and other abstractions such as pipeline contexts. +- `Predis\Command\ScriptedCommand` internally relies on `EVALSHA` instead of + `EVAL` thus avoiding to send the Lua script body on each request. The client + automatically resends the command falling back to `EVAL` when Redis returns a + `-NOSCRIPT` error. Automatic fallback to `EVAL` does not work with pipelines + or when inside a MULTI / EXEC context or with plain `EVALSHA` commands. + - Cluster and replication connections now extend a new common interface, `Predis\Connection\AggregatedConnectionInterface`. diff --git a/lib/Predis/Client.php b/lib/Predis/Client.php index 1d12485a..b990d51d 100644 --- a/lib/Predis/Client.php +++ b/lib/Predis/Client.php @@ -21,6 +21,7 @@ use Predis\PubSub\PubSubContext; use Predis\Monitor\MonitorContext; use Predis\Pipeline\PipelineContext; use Predis\Transaction\MultiExecContext; +use Predis\Command\ScriptedCommand; /** * Main class that exposes the most high-level interface to interact with Redis. @@ -209,7 +210,7 @@ class Client implements ClientInterface $response = $this->connection->executeCommand($command); if ($response instanceof ResponseErrorInterface) { - $this->onResponseError($command, $response); + $response = $this->onResponseError($command, $response); } return $response; @@ -231,7 +232,7 @@ class Client implements ClientInterface $response = $this->connection->executeCommand($command); if ($response instanceof ResponseErrorInterface) { - $this->onResponseError($command, $response); + return $this->onResponseError($command, $response); } return $response; @@ -242,13 +243,23 @@ class Client implements ClientInterface * * @param CommandInterface $command The command that generated the error. * @param ResponseErrorInterface $response The error response instance. + * @return mixed */ protected function onResponseError(CommandInterface $command, ResponseErrorInterface $response) { + if ($command instanceof ScriptedCommand && $response->getErrorType() === 'NOSCRIPT') { + $eval = $this->createCommand('eval'); + $eval->setArguments($command->getEvalArguments()); + + return $this->executeCommand($eval); + } + if ($this->options->exceptions === true) { $message = $response->getMessage(); throw new ServerException($message); } + + return $response; } /** diff --git a/lib/Predis/Command/ScriptedCommand.php b/lib/Predis/Command/ScriptedCommand.php index bd5500a1..a9ccac94 100644 --- a/lib/Predis/Command/ScriptedCommand.php +++ b/lib/Predis/Command/ScriptedCommand.php @@ -18,7 +18,7 @@ namespace Predis\Command; * @link http://redis.io/commands/eval * @author Daniele Alessandri */ -abstract class ScriptedCommand extends ServerEval +abstract class ScriptedCommand extends ServerEvalSHA { /** * Gets the body of a Lua script. @@ -67,6 +67,17 @@ abstract class ScriptedCommand extends ServerEval $numkeys = count($arguments); } - return array_merge(array($this->getScript(), $numkeys), $arguments); + return array_merge(array(sha1($this->getScript()), $numkeys), $arguments); + } + + /** + * @return array + */ + public function getEvalArguments() + { + $arguments = $this->getArguments(); + $arguments[0] = $this->getScript(); + + return $arguments; } } diff --git a/tests/Predis/ClientTest.php b/tests/Predis/ClientTest.php index c6731d04..549432fb 100644 --- a/tests/Predis/ClientTest.php +++ b/tests/Predis/ClientTest.php @@ -643,6 +643,31 @@ class ClientTest extends StandardTestCase $this->assertInstanceOf('Predis\Monitor\MonitorContext', $monitor = $client->monitor()); } + /** + * @group disconnected + */ + public function testClientResendScriptedCommandUsingEvalOnNoScriptErrors() + { + $command = $this->getMockForAbstractClass('Predis\Command\ScriptedCommand'); + $command->expects($this->once()) + ->method('getScript') + ->will($this->returnValue('return redis.call(\'exists\', KEYS[1])')); + + $connection = $this->getMock('Predis\Connection\SingleConnectionInterface'); + $connection->expects($this->at(0)) + ->method('executeCommand') + ->with($command) + ->will($this->returnValue(new ResponseError('NOSCRIPT'))); + $connection->expects($this->at(1)) + ->method('executeCommand') + ->with($this->isInstanceOf('Predis\Command\ServerEval')) + ->will($this->returnValue(true)); + + $client = new Client($connection); + + $this->assertTrue($client->executeCommand($command)); + } + // ******************************************************************** // // ---- HELPER METHODS ------------------------------------------------ // // ******************************************************************** // diff --git a/tests/Predis/Command/ScriptedCommandTest.php b/tests/Predis/Command/ScriptedCommandTest.php index ecd32c30..918ac86d 100644 --- a/tests/Predis/Command/ScriptedCommandTest.php +++ b/tests/Predis/Command/ScriptedCommandTest.php @@ -19,6 +19,7 @@ use \PHPUnit_Framework_TestCase as StandardTestCase; class ScriptedCommandTest extends StandardTestCase { const LUA_SCRIPT = 'return { KEYS[1], KEYS[2], ARGV[1], ARGV[2] }'; + const LUA_SCRIPT_SHA1 = '6e07f61f502e36d123fe28523076af588f5c315e'; /** * @group disconnected @@ -36,7 +37,8 @@ class ScriptedCommandTest extends StandardTestCase ->will($this->returnValue(2)); $command->setArguments($arguments); - $this->assertSame(array_merge(array(self::LUA_SCRIPT, 2), $arguments), $command->getArguments()); + + $this->assertSame(array_merge(array(self::LUA_SCRIPT_SHA1, 2), $arguments), $command->getArguments()); } /** @@ -55,7 +57,7 @@ class ScriptedCommandTest extends StandardTestCase ->will($this->returnValue(-2)); $command->setArguments($arguments); - $this->assertSame(array_merge(array(self::LUA_SCRIPT, 2), $arguments), $command->getArguments()); + $this->assertSame(array_merge(array(self::LUA_SCRIPT_SHA1, 2), $arguments), $command->getArguments()); } /** @@ -156,6 +158,6 @@ class ScriptedCommandTest extends StandardTestCase ->will($this->returnValue(2)); $command->setArguments($arguments); - $this->assertSame(sha1(self::LUA_SCRIPT), $command->getScriptHash()); + $this->assertSame(self::LUA_SCRIPT_SHA1, $command->getScriptHash()); } }