From beba2a25d0e1f89236702a9a2c739e4ca521e0de Mon Sep 17 00:00:00 2001
From: Mikael Nordfeldth <mmn@hethane.se>
Date: Sat, 18 Jul 2015 01:09:50 +0200
Subject: [PATCH] Don't retry unhandled transports in OpportunisticQM

It'd continue trying xmpp transports forever, for example...
---
 classes/Queue_item.php                               |  7 ++++++-
 lib/dbqueuemanager.php                               |  2 +-
 lib/queuemanager.php                                 | 12 ++++++++++++
 .../lib/opportunisticqueuemanager.php                |  9 ++++++++-
 4 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/classes/Queue_item.php b/classes/Queue_item.php
index 3a7d05adef..d41c53e0e0 100644
--- a/classes/Queue_item.php
+++ b/classes/Queue_item.php
@@ -40,7 +40,7 @@ class Queue_item extends Managed_DataObject
      * @param mixed $transports name of a single queue or array of queues to pull from
      *                          If not specified, checks all queues in the system.
      */
-    static function top($transports=null) {
+    static function top($transports=null, array $ignored_transports=array()) {
 
         $qi = new Queue_item();
         if ($transports) {
@@ -52,6 +52,11 @@ class Queue_item extends Managed_DataObject
                 $qi->transport = $transports;
             }
         }
+        if (!empty($ignored_transports)) {
+            // @fixme use safer escaping
+            $list = implode("','", array_map(array($qi, 'escape'), $ignored_transports));
+            $qi->whereAdd("transport NOT IN ('$list')");
+        }
         $qi->orderBy('created');
         $qi->whereAdd('claimed is null');
 
diff --git a/lib/dbqueuemanager.php b/lib/dbqueuemanager.php
index 45c4b694d2..9fb77eed96 100644
--- a/lib/dbqueuemanager.php
+++ b/lib/dbqueuemanager.php
@@ -72,7 +72,7 @@ class DBQueueManager extends QueueManager
     public function poll()
     {
         //$this->_log(LOG_DEBUG, 'Checking for notices...');
-        $qi = Queue_item::top($this->activeQueues());
+        $qi = Queue_item::top($this->activeQueues(), $this->getIgnoredTransports());
         if (!$qi instanceof Queue_item) {
             //$this->_log(LOG_DEBUG, 'No notices waiting; idling.');
             return false;
diff --git a/lib/queuemanager.php b/lib/queuemanager.php
index 45fe1e4ab4..487104099a 100644
--- a/lib/queuemanager.php
+++ b/lib/queuemanager.php
@@ -43,6 +43,7 @@ abstract class QueueManager extends IoManager
     protected $handlers = array();
     protected $groups = array();
     protected $activeGroups = array();
+    protected $ignoredTransports = array();
 
     /**
      * Factory function to pull the appropriate QueueManager object
@@ -255,6 +256,17 @@ abstract class QueueManager extends IoManager
         return array_keys($queues);
     }
 
+    function getIgnoredTransports()
+    {
+        return array_keys($this->ignoredTransports);
+    }
+
+    function ignoreTransport($transport)
+    {
+        // key is used for uniqueness, value doesn't mean anything
+        $this->ignoredTransports[$transport] = true;
+    }
+
     /**
      * Initialize the list of queue handlers for the current site.
      *
diff --git a/plugins/OpportunisticQM/lib/opportunisticqueuemanager.php b/plugins/OpportunisticQM/lib/opportunisticqueuemanager.php
index 4b2b679b58..b2dc61e15f 100644
--- a/plugins/OpportunisticQM/lib/opportunisticqueuemanager.php
+++ b/plugins/OpportunisticQM/lib/opportunisticqueuemanager.php
@@ -83,10 +83,17 @@ class OpportunisticQueueManager extends DBQueueManager
     // OpportunisticQM shouldn't discard items it can't handle, we're
     // only here to take care of what we _can_ handle!
     protected function noHandlerFound(Queue_item $qi, $rep=null) {
-        $this->_log(LOG_WARNING, "[{$qi->transport}:item {$qi->id}] Releasing claim for queue item without a handler");
+        $this->_log(LOG_WARNING, "[{$qi->transport}:item {$qi->id}] Releasing claim for queue item without a handler");              
         $this->_fail($qi, true);    // true here means "releaseOnly", so no error statistics since it's not an _error_
     }
 
+    protected function _fail(Queue_item $qi, $releaseOnly=false)
+    {
+        parent::_fail($qi, $releaseOnly);
+        $this->_log(LOG_DEBUG, "[{$qi->transport}:item {$qi->id}] Ignoring this transport for the rest of this execution");
+        $this->ignoreTransport($qi->transport);
+    }
+
     /**
      * Takes care of running through the queue items, returning when
      * the limits setup in __construct are met.