Merge pull request #20 from Revisor/small-fixes-in-manager

Fix small things in the Manager and Node
mariano · Oct 16, 2015 · 3ed352c · 3ed352c
2 parents bf24f94 + 9b8b3df
commit 3ed352c
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 8 deletions.
diff --git a/src/Connection/Manager.php b/src/Connection/Manager.php
@@ -133,7 +133,7 @@ public function getPriorityStrategy()
     /**
      * @inheritdoc
      */
-    public function setPriorityStrategy($priorityStrategy)
+    public function setPriorityStrategy(NodePrioritizerInterface $priorityStrategy)
     {
         $this->priorityStrategy = $priorityStrategy;
     }
@@ -166,7 +166,10 @@ public function execute(CommandInterface $command)
     {
         $this->shouldBeConnected();
         $command = $this->preprocessExecution($command);
-        $response = $this->nodes[$this->nodeId]->getConnection()->execute($command);
+
+        $currentNodeConnection = $this->nodes[$this->nodeId]->getConnection();
+        $response = $currentNodeConnection->execute($command);
+
         $response = $this->postprocessExecution($command, $response);
         return $response;
     }
@@ -278,7 +281,8 @@ protected function postprocessExecution(
     protected function updateNodeStats(array $jobs)
     {
         foreach ($jobs as $job) {
-            $nodeId = $this->getNodeIdFromJobId($job[JobsResponse::KEY_ID]);
+            $jobId = $job[JobsResponse::KEY_ID];
+            $nodeId = $this->getNodeIdFromJobId($jobId);
             if (!isset($nodeId) or !isset($this->nodes[$nodeId])) {
                 continue;
             }
@@ -449,7 +453,7 @@ private function revealNodeFromHello($nodeId, array $node)
         /**
          * Add the node prefix to the pool. We create the prefix manually
          * from the node ID rather than asking the Node object. Newly created
-         * Nodes aren't connected and thus don't know their ID nor prefix.
+         * Nodes aren't connected and thus don't know their ID or prefix.
          *
          * @see Node::sayHello()
          */

diff --git a/src/Connection/ManagerInterface.php b/src/Connection/ManagerInterface.php
@@ -49,7 +49,7 @@ public function getPriorityStrategy();
      *
      * @param NodePrioritizerInterface $priorityStrategy
      */
-    public function setPriorityStrategy($priorityStrategy);
+    public function setPriorityStrategy(NodePrioritizerInterface $priorityStrategy);
 
     /**
      * Tells if connection is established

diff --git a/src/Connection/Node/Node.php b/src/Connection/Node/Node.php
@@ -39,6 +39,12 @@ class Node
     const PRIORITY_POSSIBLE_FAILURE = 10;
     const PRIORITY_FAILURE = 100;
 
+    /**
+     * A fallback node priority if the HELLO response doesn't contain a priority
+     * This should not happen, but let's be sure.
+     */
+    const PRIORITY_FALLBACK = 2;
+
     /**
      * @var Credentials Credentials of this node - host, port, password
      */
@@ -330,8 +336,8 @@ private function readPriorityFromHello($hello, $id)
             }
         }
 
-        // Node not found in the HELLO? Strange, should not happen.
-        // Let's return a default value of 2.
-        return 2;
+        // Node not found in the HELLO? This should not happen.
+        // Return a fallback value
+        return self::PRIORITY_FALLBACK;
     }
 }