Merge pull request #49 from brianlmoon/check_server_version_for_timeout

Check server version for timeout unit
brianlmoon · Dec 1, 2021 · a092a78 · a092a78
2 parents 92a23bd + e9efc05
commit a092a78
Show file tree

Hide file tree

Showing 2 changed files with 99 additions and 5 deletions.
diff --git a/Net/Gearman/Connection.php b/Net/Gearman/Connection.php
@@ -107,6 +107,13 @@ class Net_Gearman_Connection
 
     public $socket;
 
+    /**
+     * Gearmand Server Version
+     *
+     * @var        string
+     */
+    protected $serverVersion;
+
     public function __construct($host=null, $timeout=250) {
         if ($host) {
             $this->connect($host, $timeout);
@@ -123,8 +130,8 @@ public function __destruct() {
      * Opens the socket to the Gearman Job server. It throws an exception if
      * a socket error occurs. Also populates Net_Gearman_Connection::$magic.
      *
-     * @param string $host    e.g. 127.0.0.1 or 127.0.0.1:7003
-     * @param int    $timeout Timeout in milliseconds
+     * @param string              $host    e.g. 127.0.0.1 or 127.0.0.1:7003
+     * @param int                 $timeout Timeout in milliseconds
      *
      * @return resource A connection to a Gearman server
      * @throws Net_Gearman_Exception when it can't connect to server
@@ -203,6 +210,8 @@ public function connect($host, $timeout = 250)
 
             // socket_set_option($this->socket, SOL_TCP, SO_DEBUG, 1); // Debug
 
+            $this->setServerVersion($host);
+
          } else {
 
             $errno = @socket_last_error($this->socket);
@@ -259,6 +268,10 @@ public function send($command, array $params = array())
             }
         }
 
+        if ($command === 'can_do_timeout') {
+            $params = $this->fixTimeout($params);
+        }
+
         $d = implode("\x00", $data);
 
         $cmd = "\0REQ" . pack("NN",
@@ -548,4 +561,30 @@ public static function subString($str, $start, $length)
             return substr($str, $start, $length);
         }
     }
+
+    /**
+     * Sets the server version.
+     *
+     * @param string              $host     The host
+     * @param Net_Gearman_Manager $manager Optional manager object
+     */
+    protected function setServerVersion($host, $manager = null)
+    {
+        if (empty($manager)) {
+            $manager = new \Net_Gearman_Manager($host);
+        }
+        $this->serverVersion = $manager->version();
+        unset($manager);
+    }
+
+    protected function fixTimeout($params) {
+        // In gearmand version 1.1.19 and greater, the timeout is
+        // expected to be in milliseconds. Before that version, it
+        // is expected to be in seconds.
+        // https://github.com/gearman/gearmand/issues/196
+        if (version_compare('1.1.18', $this->serverVersion)) {
+            $params['timeout'] *= 1000;
+        }
+        return $params;
+    }
 }
diff --git a/tests/Net/Gearman/ConnectionTest.php b/tests/Net/Gearman/ConnectionTest.php
@@ -2,13 +2,68 @@
 
 /**
  * Net_Gearman_ConnectionTest
- * @group functional
  */
 class Net_Gearman_ConnectionTest extends \PHPUnit\Framework\TestCase
 {
+    /**
+     * @group unit
+     */
+    public function testSetServerVersion() {
+        $mock_manager = new class extends \Net_Gearman_Manager {
+            public $mock_version;
+            public function __construct() {
+                // noop
+            }
+            public function version() {
+                return $this->mock_version;
+            }
+        };
+
+        $connection = new class extends Net_Gearman_Connection {
+            public function setServerVersion($host, $manager = null) {
+                parent::setServerVersion($host, $manager);
+                return $this->serverVersion;
+            }
+        };
+
+        $mock_manager->mock_version = '1.1.18';
+
+        $result = $connection->setServerVersion('localhost:4730', $mock_manager);
+        $this->assertEquals('1.1.18', $result);
+
+        $mock_manager->mock_version = '1.1.19';
+
+        $result = $connection->setServerVersion('localhost:4730', $mock_manager);
+        $this->assertEquals('1.1.19', $result);
+    }
+
+    /**
+     * @group unit
+     */
+    public function testFixTimeout() {
+        $connection = new class extends Net_Gearman_Connection {
+            public $serverVersion;
+            public function fixTimeout($params) {
+                return parent::fixTimeout($params);
+            }
+        };
+
+        $connection->serverVersion = '1.1.18';
+        $result = $connection->fixTimeout(['timeout' => 10]);
+        $this->assertEquals(['timeout' => 10], $result);
+
+        $connection->serverVersion = '1.1.19';
+        $result = $connection->fixTimeout(['timeout' => 10]);
+        $this->assertEquals(['timeout' => 10000], $result);
+
+        $connection->serverVersion = '1.1.19.1';
+        $result = $connection->fixTimeout(['timeout' => 10]);
+        $this->assertEquals(['timeout' => 10000], $result);
+    }
+
     /**
      * When no server is supplied, it should connect to localhost:4730.
-     *
+     * @group functional
      * @return void
      */
     public function testDefaultConnect()
@@ -25,7 +80,7 @@ public function testDefaultConnect()
 
     /**
      * 001-echo_req.phpt
-     *
+     * @group functional
      * @return void
      */
     public function testSend()