Persistent tests now pass;

boenrobot · boenrobot · commit c6262a878b74 · 2012-06-05T19:26:17.000+03:00
!done response is now always given to callbacks when canceling.
diff --git a/nbproject/private/private.xml b/nbproject/private/private.xml
@@ -2,13 +2,4 @@
 <project-private xmlns="http://www.netbeans.org/ns/project-private/1">
     <coverage xmlns="http://www.netbeans.org/ns/code-coverage/1" enabled="true"/>
     <editor-bookmarks xmlns="http://www.netbeans.org/ns/editor-bookmarks/1"/>
-    <open-files xmlns="http://www.netbeans.org/ns/projectui-open-files/1">
-        <file>file:/D:/Projects/PEAR2_Net_RouterOS.git/src/PEAR2/Net/RouterOS/SocketException.php</file>
-        <file>file:/D:/Projects/PEAR2_Net_RouterOS.git/src/PEAR2/Net/RouterOS/Client.php</file>
-        <file>file:/D:/Projects/PEAR2_Net_RouterOS.git/src/PEAR2/Net/RouterOS/Communicator.php</file>
-        <file>file:/D:/Projects/PEAR2_Net_RouterOS.git/src/PEAR2/Net/RouterOS/Query.php</file>
-        <file>file:/D:/Projects/PEAR2_Net_RouterOS.git/src/PEAR2/Net/RouterOS/Request.php</file>
-        <file>file:/D:/Projects/PEAR2_Net_RouterOS.git/src/PEAR2/Net/RouterOS/Registry.php</file>
-        <file>file:/D:/Projects/PEAR2_Net_RouterOS.git/src/PEAR2/Net/RouterOS/PersistentRegistry.php</file>
-    </open-files>
 </project-private>
diff --git a/src/PEAR2/Net/RouterOS/Client.php b/src/PEAR2/Net/RouterOS/Client.php
@@ -438,7 +438,7 @@ public function completeRequest($tag = null)
      */
     public function extractNewResponses($tag = null)
     {
-        if ('' == $tag) {
+        if (null === $tag) {
             $result = array();
             foreach (array_keys($this->responseBuffer) as $tag) {
                 $result = array_merge(
@@ -529,40 +529,43 @@ public function cancelRequest($tag = null)
             $tags = array_merge(
                 array_keys($this->responseBuffer), array_keys($this->callbacks)
             );
+            $this->registry->setTaglessMode(true);
             foreach ($tags as $t) {
-                $this->cancelRequest($t);
+                $cancelRequest->setArgument(
+                    'tag', $this->registry->getOwnershipTag() . $t
+                );
+                $this->sendSync($cancelRequest);
             }
-            return $this;
-        }
-        
-        if ($hasTag) {
-            if ($this->isRequestActive($tag)) {
-                if ($hasReg) {
-                    $cancelRequest->setArgument(
-                        'tag', $this->registry->getOwnershipTag() . $tag
-                    );
+            $this->registry->setTaglessMode(false);
+        } else {
+            if ($hasTag) {
+                if ($this->isRequestActive($tag)) {
+                    if ($hasReg) {
+                        $this->registry->setTaglessMode(true);
+                        $cancelRequest->setArgument(
+                            'tag', $this->registry->getOwnershipTag() . $tag
+                        );
+                    } else {
+                        $cancelRequest->setArgument('tag', $tag);
+                    }
                 } else {
-                    $cancelRequest->setArgument('tag', $tag);
+                    throw new DataFlowException(
+                        'No such request. Canceling aborted.', 11200
+                    );
                 }
-            } else {
-                throw new DataFlowException(
-                    'No such request. Canceling aborted.', 11200
-                );
+            }
+            $this->sendSync($cancelRequest);
+            if ($hasReg) {
+                $this->registry->setTaglessMode(false);
             }
         }
-        
-        $regBackup = $this->registry;
-        $this->registry = null;
-        $this->sendSync($cancelRequest);
-        $this->registry = $regBackup;
 
         if ($hasTag) {
             if ($this->isRequestActive($tag, self::FILTER_BUFFER)) {
-                unset($this->responseBuffer[$tag]);
-            } elseif ($this->isRequestActive($tag, self::FILTER_CALLBACK)) {
-                unset($this->callbacks[$tag]);
+                $this->responseBuffer[$tag] = $this->completeRequest($tag);
+            } else {
+                $this->completeRequest($tag);
             }
-            $this->pendingRequestsCount--;
         } else {
             $this->responseBuffer = array();
             $this->callbacks = array();
@@ -615,19 +618,27 @@ public function isStreamingResponses()
      */
     public function close()
     {
-        $result = false;
+        $result = true;
         try {
-            $response = $this->sendSync(new Request('/quit'));
-            $result = $this->com->close()
-                && $response->getType() === Response::TYPE_FATAL;
+            if (0 !== $this->pendingRequestsCount) {
+                if (null !== $this->registry) {
+                    $this->registry->setTaglessMode(true);
+                }
+                $response = $this->sendSync(new Request('/quit'));
+                if (null !== $this->registry) {
+                    $this->registry->setTaglessMode(false);
+                }
+                $result = $response->getType() === Response::TYPE_FATAL;
+            }
+            $result = $result && $this->com->close();
         } catch (SocketException $e) {
             $result = $e->getCode() === 205;
         }
         $this->callbacks = array();
         $this->pendingRequestsCount = 0;
-        if (null !== $this->registry) {
-            $this->registry->close();
-        }
+        //if (null !== $this->registry) {
+        //    $this->registry->close();
+        //}
         return $result;
     }
     
@@ -636,7 +647,11 @@ public function close()
      */
     public function __destruct()
     {
-        if (!$this->com->getTransmitter()->isPersistent()) {
+        if ($this->com->getTransmitter()->isPersistent()) {
+            if (0 !== $this->pendingRequestsCount) {
+                $this->cancelRequest();
+            }
+        } else {
             $this->close();
         }
     }
diff --git a/src/PEAR2/Net/RouterOS/Message.php b/src/PEAR2/Net/RouterOS/Message.php
@@ -41,7 +41,7 @@ abstract class Message
     /**
      * @var string An optional tag to associate the message with.
      */
-    private $_tag = null;
+    protected $_tag = null;
     
     /**
      * A shorthand gateway.
diff --git a/src/PEAR2/Net/RouterOS/Registry.php b/src/PEAR2/Net/RouterOS/Registry.php
@@ -98,6 +98,49 @@ public static function parseTag($tag)
         return $result;
     }
     
+    /**
+     * Checks if any instance is in tagless mode right now.
+     * 
+     * @return bool TRUE if an instance is in tagless mode, FALSE otherwise.
+     */
+    public function isTaglessModeOwner()
+    {
+        $this->shm->lock('taglessModeOwner');
+        $result = $this->shm->exists('taglessModeOwner')
+            && $this->getOwnershipTag() === $this->shm->get('taglessModeOwner');
+        $this->shm->unlock('taglessModeOwner');
+        return $result;
+    }
+    
+    /**
+     * Sets the "tagless mode" setting.
+     * 
+     * While in tagless mode, this instance will claim owhership of any
+     * responses without a tag. While not in this mode, any requests without a
+     * tag will be given to all instances.
+     * 
+     * Regardless of mode, if the type of the response is
+     * {@link Response::TYPE_FATAL}, it will be given to all instances.
+     * 
+     * @param bool $taglessMode TRUE to claim tagless ownership, FALSE to
+     * release such ownership, if taken.
+     * 
+     * @return bool TRUE on success, FALSE on failure. 
+     */
+    public function setTaglessMode($taglessMode)
+    {
+        return $taglessMode 
+            ?   ($this->shm->lock('taglessMode')
+                && $this->shm->lock('taglessModeOwner')
+                && $this->shm->add('taglessModeOwner', $this->getOwnershipTag())
+                && $this->shm->unlock('taglessModeOwner'))
+            :   ($this->isTaglessModeOwner()
+                && $this->shm->lock('taglessModeOwner')
+                && $this->shm->delete('taglessModeOwner')
+                && $this->shm->unlock('taglessModeOwner')
+                && $this->shm->unlock('taglessMode'));
+    }
+    
     /**
      * Get the ownership tag for this instance.
      * 
@@ -123,16 +166,28 @@ public function getOwnershipTag()
      */
     public function add(Response $response, $ownershipTag)
     {
-        if ($this->getOwnershipTag() === $ownershipTag) {
+        if ($this->getOwnershipTag() === $ownershipTag
+            || ($this->isTaglessModeOwner()
+            && $response->getType() !== Response::TYPE_FATAL)
+        ) {
             return false;
         }
         
         if (null === $ownershipTag) {
-            foreach ($this->shm->getIterator('/^(responseBuffer\_)/', true)
-                as $targetBufferName) {
-                $this->_add($response, $targetBufferName);
+            $this->shm->lock('taglessModeOwner');
+            if ($this->shm->exists('taglessModeOwner')
+                && $response->getType() !== Response::TYPE_FATAL
+            ) {
+                $ownershipTag = $this->shm->get('taglessModeOwner');
+                $this->shm->unlock('taglessModeOwner');
+            } else {
+                $this->shm->unlock('taglessModeOwner');
+                foreach ($this->shm->getIterator('/^(responseBuffer\_)/', true)
+                    as $targetBufferName) {
+                    $this->_add($response, $targetBufferName);
+                }
+                return true;
             }
-            return true;
         }
         
         $this->_add($response, 'responseBuffer_' . $ownershipTag);
@@ -189,10 +244,14 @@ public function getNextResponse()
      */
     public function close()
     {
-        foreach ($this->shm->getIterator('/^(responseBuffer\_)/', true)
-            as $targetBufferName) {
-            $this->_close($targetBufferName);
-        }
+        self::$requestId = -1;
+        self::$instanceIdSeed = -1;
+        $this->shm->clear();
+        
+        //foreach ($this->shm->getIterator('/^(responseBuffer\_)/', true)
+        //    as $targetBufferName) {
+        //    $this->_close($targetBufferName);
+        //}
     }
     
     /**
diff --git a/src/PEAR2/Net/RouterOS/Request.php b/src/PEAR2/Net/RouterOS/Request.php
@@ -245,7 +245,9 @@ public function removeAllArguments()
      */
     public function send(Communicator $com, Registry $reg = null)
     {
-        if (null !== $reg) {
+        if (null !== $reg
+            && (null != $this->getTag() || !$reg->isTaglessModeOwner())
+        ) {
             $originalTag = $this->getTag();
             $this->setTag($reg->getOwnershipTag() . $originalTag);
             $bytes = $this->send($com);
diff --git a/tests/ClientFeaturesTest.php b/tests/ClientFeaturesTest.php
@@ -334,8 +334,8 @@ function($response, $client) use (&$responseCount) {
             "No responses for '" . HOSTNAME . "' in 2 seconds."
         );
         $this->assertGreaterThanOrEqual(
+            $finalRepliesCount + 2/* The !trap and !done */,
             $responseCount,
-            $finalRepliesCount + 1/* The !trap */,
             "Insufficient callbacks during second loop."
         );
     }
@@ -370,7 +370,7 @@ function($response, $client) use (&$repliesCount, $limit) {
 
         $this->object->loop();
         $this->assertEquals(
-            $limit + 1/* The !trap */, $repliesCount,
+            $limit + 2/* The !trap and !done*/, $repliesCount,
             "Extra callbacks were executed during second loop."
         );
     }
@@ -435,12 +435,12 @@ function($response, $client) use (&$repliesCount) {
                 $repliesCount++;
             }
         );
+        sleep(1);
 
 
         $arpPrint = new Request('/ip/arp/print');
         $arpPrint->setTag('arp');
         $this->object->sendAsync($arpPrint);
-
         $list = $this->object->completeRequest('arp');
 
         
@@ -513,8 +513,12 @@ public function testSendAsyncAndCompleteRequestWithCallback()
 
         $arpPrint = new Request('/ip/arp/print');
         $arpPrint->setTag('arp');
-        $list1 = $this->object->sendSync($arpPrint);
-        $list2 = array();
+        $list1 = $list2 = array();
+        foreach ($this->object->sendSync($arpPrint) as $response) {
+            $list1[(string) $response->getArgument('.id')] = $response;
+        }
+        ksort($list1);
+        
         $this->object->sendAsync(
             $arpPrint, 
             function($response, $client) use (&$list2) {
@@ -531,12 +535,13 @@ function($response, $client) use (&$list2) {
                     'arp', $response->getTag(),
                     'The callback must only receive responses meant for it.'
                 );
-                $list2[] = $response;
+                $list2[(string) $response->getArgument('.id')] = $response;
             }
         );
 
         $this->assertEmpty($this->object->completeRequest('arp')->toArray());
-        $this->assertEquals($list1->toArray(), $list2);
+        ksort($list2);
+        $this->assertEquals($list1, $list2);
     }
 
     public function testCompleteRequestEmptyQueue()
@@ -623,7 +628,10 @@ public function testClientInvokability()
         $obj(new Request('/ip/arp/print', null, 'arp'));
         $this->assertEquals(1, $obj->getPendingRequestsCount());
         $obj();
-        $arpResponses2 = $obj('arp');
+        $this->assertEquals(0, $obj->getPendingRequestsCount());
+        $arpResponses2 = $obj->extractNewResponses('arp');
+        //$arpResponses2 = $obj('arp');
+        $this->assertEquals(0, $obj->getPendingRequestsCount());
         $this->assertGreaterThan(0, count($arpResponses2));
         $this->assertEquals(count($arpResponses1), count($arpResponses2));
         $this->assertInstanceOf(__NAMESPACE__ . '\Response', $arpResponses1(0));
