Ver código fonte

Porperly handle -ERR instead of +QUEUED inside MULTI ... EXEC.

Error responses such as -OOM or -ERR on invalid arguments in commands
are returned immediatly instead of +QUEUED when using a transaction,
which is a condition that had not been tested enough. This condition
led to a bug in which Predis was not invalidating the transaction, so
when trying to create a new transaction Redis returned a "-ERR MULTI
calls can not be nested".

This commit fixes #187.
Daniele Alessandri 10 anos atrás
pai
commit
74817b5596

+ 9 - 6
lib/Predis/Transaction/MultiExecContext.php

@@ -206,17 +206,20 @@ class MultiExecContext implements BasicClientInterface, ExecutableContextInterfa
     public function executeCommand(CommandInterface $command)
     {
         $this->initialize();
-        $response = $this->client->executeCommand($command);
 
         if ($this->checkState(self::STATE_CAS)) {
-            return $response;
+            return $this->client->executeCommand($command);
         }
 
-        if (!$response instanceof ResponseQueued) {
-            $this->onProtocolError('The server did not respond with a QUEUED status reply');
-        }
+        $response = $this->client->getConnection()->executeCommand($command);
 
-        $this->commands->enqueue($command);
+        if ($response instanceof ResponseQueued) {
+            $this->commands->enqueue($command);
+        } else if ($response instanceof ResponseErrorInterface) {
+            throw new AbortedMultiExecException($this, $response->getMessage());
+        } else {
+            $this->onProtocolError('The server did not return a +QUEUED status response.');
+        }
 
         return $this;
     }

+ 45 - 0
tests/Predis/Transaction/MultiExecContextTest.php

@@ -14,6 +14,7 @@ namespace Predis\Transaction;
 use PredisTestCase;
 use Predis\Client;
 use Predis\ResponseQueued;
+use Predis\ResponseError;
 use Predis\ServerException;
 use Predis\Command\CommandInterface;
 
@@ -473,6 +474,50 @@ class MultiExecContextTest extends PredisTestCase
         $this->assertSame(array('MULTI', 'SET', 'ECHO', 'DISCARD'), self::commandsToIDs($commands));
     }
 
+    /**
+     * @group disconnected
+     */
+    public function testProperlyDiscardsTransactionAfterServerExceptionInBlock()
+    {
+        $connection = $this->getMockedConnection(function (CommandInterface $command) {
+            switch ($command->getId()) {
+                case 'MULTI':
+                    return true;
+
+                case 'ECHO':
+                    return new ResponseError('ERR simulated failure on ECHO');
+
+                case 'EXEC':
+                    return new ResponseError('EXECABORT Transaction discarded because of previous errors.');
+
+                default:
+                    return new ResponseQueued();
+            }
+        });
+
+        $client = new Client($connection);
+
+        // First attempt
+        $tx = new MultiExecContext($client);
+
+        try {
+            $tx->multi()->set('foo', 'bar')->echo('simulated failure')->exec();
+        } catch (\Exception $exception) {
+            $this->assertInstanceOf('Predis\Transaction\AbortedMultiExecException', $exception);
+            $this->assertSame('ERR simulated failure on ECHO', $exception->getMessage());
+        }
+
+        // Second attempt
+        $tx = new MultiExecContext($client);
+
+        try {
+            $tx->multi()->set('foo', 'bar')->echo('simulated failure')->exec();
+        } catch (\Exception $exception) {
+            $this->assertInstanceOf('Predis\Transaction\AbortedMultiExecException', $exception);
+            $this->assertSame('ERR simulated failure on ECHO', $exception->getMessage());
+        }
+    }
+
     // ******************************************************************** //
     // ---- INTEGRATION TESTS --------------------------------------------- //
     // ******************************************************************** //