From 31aa080609f6ea8a561182eb5b3da46733bef313 Mon Sep 17 00:00:00 2001 From: Thomas Bruederli Date: Wed, 23 Apr 2014 13:21:51 +0200 Subject: [PATCH] Further refine cross-folder searching: - Store incomplete search results in session and re-send search requests to the server if returned before complete (this should avoid hitting request timeouts). - Display full folder path on mouseover in message list - Remove pthreads implementation stuff as this wasn't really working --- program/js/app.js | 16 +- program/lib/Roundcube/rcube_imap.php | 10 ++ program/lib/Roundcube/rcube_imap_search.php | 169 ++++-------------- program/lib/Roundcube/rcube_result_index.php | 2 + .../Roundcube/rcube_result_multifolder.php | 52 +++++- program/lib/Roundcube/rcube_result_thread.php | 2 + program/localization/en_US/messages.inc | 1 + program/steps/mail/func.inc | 2 +- program/steps/mail/search.inc | 26 ++- 9 files changed, 136 insertions(+), 144 deletions(-) diff --git a/program/js/app.js b/program/js/app.js index 6f61685d1..f586c1143 100644 --- a/program/js/app.js +++ b/program/js/app.js @@ -1992,6 +1992,9 @@ function rcube_webmail() else html = ' '; } + else if (c == 'folder') { + html = '' + cols[c] + ''; + } else html = cols[c]; @@ -4214,6 +4217,17 @@ function rcube_webmail() return false; }; + this.continue_search = function(request_id) + { + var lock = ref.set_busy(true, 'stillsearching'); + + setTimeout(function(){ + var url = ref.search_params(); + url._continue = request_id; + ref.env.qsearch = { lock: lock, request: ref.http_request('search', url, lock) }; + }, 100); + }; + // build URL params for search this.search_params = function(search, filter, smods) { @@ -7870,7 +7884,7 @@ rcube_webmail.long_subject_title = function(elem, indent) { if (!elem.title) { var $elem = $(elem); - if ($elem.width() + indent * 15 > $elem.parent().width()) + if ($elem.width() + (indent || 0) * 15 > $elem.parent().width()) elem.title = $elem.text(); } }; diff --git a/program/lib/Roundcube/rcube_imap.php b/program/lib/Roundcube/rcube_imap.php index f2ade9545..2038b94d8 100644 --- a/program/lib/Roundcube/rcube_imap.php +++ b/program/lib/Roundcube/rcube_imap.php @@ -1513,6 +1513,16 @@ class rcube_imap extends rcube_storage $this->threading = false; $searcher = new rcube_imap_search($this->options, $this->conn); + + // set limit to not exceed the client's request timeout + $searcher->set_timelimit(60); + + // continue existing incomplete search + if (!empty($this->search_set) && $this->search_set->incomplete && $str == $this->search_string) { + $searcher->set_results($this->search_set); + } + + // execute the search $results = $searcher->exec( $folder, $str, diff --git a/program/lib/Roundcube/rcube_imap_search.php b/program/lib/Roundcube/rcube_imap_search.php index 926a75d76..0c44daf1b 100644 --- a/program/lib/Roundcube/rcube_imap_search.php +++ b/program/lib/Roundcube/rcube_imap_search.php @@ -5,7 +5,7 @@ | This file is part of the Roundcube Webmail client | | | | Copyright (C) 2013, The Roundcube Dev Team | - | Copyright (C) 2013, Kolab Systems AG | + | Copyright (C) 2014, Kolab Systems AG | | | | Licensed under the GNU General Public License version 3 or | | any later version with exceptions for skins & plugins. | @@ -18,15 +18,8 @@ +-----------------------------------------------------------------------+ */ -// create classes defined by the pthreads module if that isn't installed -if (!defined('PTHREADS_INHERIT_ALL')) { - class Worker { } - class Stackable { } -} - /** * Class to control search jobs on multiple IMAP folders. - * This implement a simple threads pool using the pthreads extension. * * @package Framework * @subpackage Storage @@ -36,12 +29,10 @@ class rcube_imap_search { public $options = array(); - private $size = 10; - private $next = 0; - private $workers = array(); - private $states = array(); - private $jobs = array(); - private $conn; + protected $jobs = array(); + protected $timelimit = 0; + protected $results; + protected $conn; /** * Default constructor @@ -63,28 +54,32 @@ class rcube_imap_search */ public function exec($folders, $str, $charset = null, $sort_field = null, $threading=null) { - $pthreads = defined('PTHREADS_INHERIT_ALL'); - + $start = floor(microtime(true)); $results = new rcube_result_multifolder($folders); // start a search job for every folder to search in foreach ($folders as $folder) { - $job = new rcube_imap_search_job($folder, $str, $charset, $sort_field, $threading); - if ($pthreads && $this->submit($job)) { - $this->jobs[] = $job; + // a complete result for this folder already exists + $result = $this->results ? $this->results->get_set($folder) : false; + if ($result && !$result->incomplete) { + $results->add($result); } else { + $job = new rcube_imap_search_job($folder, $str, $charset, $sort_field, $threading); $job->worker = $this; - $job->run(); $this->jobs[] = $job; } } - // wait for all workers to be done - $this->shutdown(); - - // gather results + // execute jobs and gather results foreach ($this->jobs as $job) { + // only run search if within the configured time limit + // TODO: try to estimate the required time based on folder size and previous search performance + if (!$this->timelimit || floor(microtime(true)) - $start < $this->timelimit) { + $job->run(); + } + + // add result (may have ->incomplete flag set) $results->add($job->get_result()); } @@ -92,51 +87,21 @@ class rcube_imap_search } /** - * Assign the given job object to one of the worker threads for execution + * Setter for timelimt property */ - public function submit(Stackable $job) + public function set_timelimit($seconds) { - if (count($this->workers) < $this->size) { - $id = count($this->workers); - $this->workers[$id] = new rcube_imap_search_worker($id, $this->options); - $this->workers[$id]->start(PTHREADS_INHERIT_ALL); - - if ($this->workers[$id]->stack($job)) { - return $job; - } - else { - // trigger_error(sprintf("Failed to push Stackable onto %s", $id), E_USER_WARNING); - } - } - if (($worker = $this->workers[$this->next])) { - $this->next = ($this->next+1) % $this->size; - if ($worker->stack($job)) { - return $job; - } - else { - // trigger_error(sprintf("Failed to stack onto selected worker %s", $worker->id), E_USER_WARNING); - } - } - else { - // trigger_error(sprintf("Failed to select a worker for Stackable"), E_USER_WARNING); - } - - return false; + $this->timelimit = $seconds; } /** - * Shutdown the pool of threads cleanly, retaining exit status locally + * Setter for previous (potentially incomplete) search results */ - public function shutdown() + public function set_results($res) { - foreach ($this->workers as $worker) { - $this->states[$worker->getThreadId()] = $worker->shutdown(); - $worker->close(); - } - - # console('shutdown', $this->states); + $this->results = $res; } - + /** * Get connection to the IMAP server * (used for single-thread mode) @@ -151,7 +116,7 @@ class rcube_imap_search /** * Stackable item to run the search on a specific IMAP folder */ -class rcube_imap_search_job extends Stackable +class rcube_imap_search_job /* extends Stackable */ { private $folder; private $search; @@ -169,13 +134,14 @@ class rcube_imap_search_job extends Stackable $this->charset = $charset; $this->sort_field = $sort_field; $this->threading = $threading; + + $this->result = new rcube_result_index($folder); + $this->result->incomplete = true; } public function run() { - // trigger_error("Start search $this->folder", E_USER_NOTICE); $this->result = $this->search_index(); - // trigger_error("End search $this->folder: " . $this->result->count(), E_USER_NOTICE); } /** @@ -183,7 +149,6 @@ class rcube_imap_search_job extends Stackable */ protected function search_index() { - $pthreads = defined('PTHREADS_INHERIT_ALL'); $criteria = $this->search; $charset = $this->charset; @@ -193,10 +158,10 @@ class rcube_imap_search_job extends Stackable trigger_error("No IMAP connection for $this->folder", E_USER_WARNING); if ($this->threading) { - return new rcube_result_thread(); + return new rcube_result_thread($this->folder); } else { - return new rcube_result_index(); + return new rcube_result_index($this->folder); } } @@ -220,10 +185,6 @@ class rcube_imap_search_job extends Stackable rcube_imap::convert_criteria($criteria, $charset), true, 'US-ASCII'); } - // close IMAP connection again - if ($pthreads) - $imap->closeConnection(); - return $threads; } @@ -249,10 +210,6 @@ class rcube_imap_search_job extends Stackable } } - // close IMAP connection again - if ($pthreads) - $imap->closeConnection(); - return $messages; } @@ -271,67 +228,7 @@ class rcube_imap_search_job extends Stackable { return $this->result; } -} - - -/** - * Worker thread to run search jobs while maintaining a common context - */ -class rcube_imap_search_worker extends Worker -{ - public $id; - public $options; - private $conn; - private $counts = 0; - - /** - * Default constructor - */ - public function __construct($id, $options) - { - $this->id = $id; - $this->options = $options; - } - - /** - * Get a dedicated connection to the IMAP server - */ - public function get_imap() - { - // TODO: make this connection persistent for several jobs - // This doesn't seem to work. Socket connections don't survive serialization which is used in pthreads - - $conn = new rcube_imap_generic(); - # $conn->setDebug(true, function($conn, $message){ trigger_error($message, E_USER_NOTICE); }); - - if ($this->options['user'] && $this->options['password']) { - $this->options['ident']['command'] = 'search-' . $this->id . 't' . ++$this->counts; - $conn->connect($this->options['host'], $this->options['user'], $this->options['password'], $this->options); - } - - if ($conn->error) - trigger_error($conn->error, E_USER_WARNING); - - return $conn; - } - - /** - * @override - */ - public function run() - { - - } - - /** - * Close IMAP connection - */ - public function close() - { - if ($this->conn) { - $this->conn->close(); - } - } } + diff --git a/program/lib/Roundcube/rcube_result_index.php b/program/lib/Roundcube/rcube_result_index.php index 058f25c6f..ffc1ad78a 100644 --- a/program/lib/Roundcube/rcube_result_index.php +++ b/program/lib/Roundcube/rcube_result_index.php @@ -26,6 +26,8 @@ */ class rcube_result_index { + public $incomplete = false; + protected $raw_data; protected $mailbox; protected $meta = array(); diff --git a/program/lib/Roundcube/rcube_result_multifolder.php b/program/lib/Roundcube/rcube_result_multifolder.php index b5473b841..e5abead51 100644 --- a/program/lib/Roundcube/rcube_result_multifolder.php +++ b/program/lib/Roundcube/rcube_result_multifolder.php @@ -28,6 +28,7 @@ class rcube_result_multifolder { public $multi = true; public $sets = array(); + public $incomplete = false; public $folder; protected $meta = array(); @@ -54,8 +55,9 @@ class rcube_result_multifolder */ public function add($result) { + $this->sets[] = $result; + if ($count = $result->count()) { - $this->sets[] = $result; $this->meta['count'] += $count; // append UIDs to global index @@ -63,6 +65,9 @@ class rcube_result_multifolder $index = array_map(function($uid) use ($folder) { return $uid . '-' . $folder; }, $result->get()); $this->index = array_merge($this->index, $index); } + else if ($result->incomplete) { + $this->incomplete = true; + } } /** @@ -266,6 +271,22 @@ class rcube_result_multifolder return $params; } + /** + * Returns the stored result object for a particular folder + * + * @param string $folder Folder name + * @return false|obejct rcube_result_* instance of false if none found + */ + public function get_set($folder) + { + foreach ($this->sets as $set) { + if ($set->get_parameters('MAILBOX') == $folder) { + return $set; + } + } + + return false; + } /** * Returns length of internal data representation @@ -276,4 +297,33 @@ class rcube_result_multifolder { return $this->count(); } + + + /* Serialize magic methods */ + + public function __sleep() + { + return array('sets','folders','sorting','order'); + } + + public function __wakeup() + { + // restore index from saved result sets + $this->meta = array('count' => 0); + + foreach ($this->sets as $result) { + if ($count = $result->count()) { + $this->meta['count'] += $count; + + // append UIDs to global index + $folder = $result->get_parameters('MAILBOX'); + $index = array_map(function($uid) use ($folder) { return $uid . '-' . $folder; }, $result->get()); + $this->index = array_merge($this->index, $index); + } + else if ($result->incomplete) { + $this->incomplete = true; + } + } + } + } diff --git a/program/lib/Roundcube/rcube_result_thread.php b/program/lib/Roundcube/rcube_result_thread.php index ceaaf59a6..168761696 100644 --- a/program/lib/Roundcube/rcube_result_thread.php +++ b/program/lib/Roundcube/rcube_result_thread.php @@ -26,6 +26,8 @@ */ class rcube_result_thread { + public $incomplete = false; + protected $raw_data; protected $mailbox; protected $meta = array(); diff --git a/program/localization/en_US/messages.inc b/program/localization/en_US/messages.inc index 0a1824398..45c91e3d0 100644 --- a/program/localization/en_US/messages.inc +++ b/program/localization/en_US/messages.inc @@ -95,6 +95,7 @@ $messages['contactsearchsuccessful'] = '$nr contacts found.'; $messages['searchnomatch'] = 'Search returned no matches.'; $messages['searching'] = 'Searching...'; $messages['checking'] = 'Checking...'; +$messages['stillsearching'] = 'Still searching...'; $messages['nospellerrors'] = 'No spelling errors found.'; $messages['folderdeleted'] = 'Folder successfully deleted.'; $messages['foldersubscribed'] = 'Folder successfully subscribed.'; diff --git a/program/steps/mail/func.inc b/program/steps/mail/func.inc index 10d30a5f3..b9971ce0c 100644 --- a/program/steps/mail/func.inc +++ b/program/steps/mail/func.inc @@ -142,7 +142,7 @@ if (empty($RCMAIL->action) || $RCMAIL->action == 'list') { if (!$OUTPUT->ajax_call) { $OUTPUT->add_label('checkingmail', 'deletemessage', 'movemessagetotrash', 'movingmessage', 'copyingmessage', 'deletingmessage', 'markingmessage', - 'copy', 'move', 'quota', 'replyall', 'replylist'); + 'copy', 'move', 'quota', 'replyall', 'replylist', 'stillsearching'); } $pagetitle = $RCMAIL->localize_foldername($mbox_name, true); diff --git a/program/steps/mail/search.inc b/program/steps/mail/search.inc index 813e8c492..f52d1a084 100644 --- a/program/steps/mail/search.inc +++ b/program/steps/mail/search.inc @@ -38,6 +38,7 @@ $mbox = rcube_utils::get_input_value('_mbox', rcube_utils::INPUT_GET, true); $filter = rcube_utils::get_input_value('_filter', rcube_utils::INPUT_GET); $headers = rcube_utils::get_input_value('_headers', rcube_utils::INPUT_GET); $scope = rcube_utils::get_input_value('_scope', rcube_utils::INPUT_GET); +$continue = rcube_utils::get_input_value('_continue', rcube_utils::INPUT_GET); $subject = array(); $filter = trim($filter); @@ -110,6 +111,12 @@ if (!empty($subject)) { $search_str = trim($search_str); $sort_column = rcmail_sort_column(); +// set message set for already stored (but incomplete) search request +if (!empty($continue) && isset($_SESSION['search']) && $_SESSION['search_request'] == $continue) { + $RCMAIL->storage->set_search_set($_SESSION['search']); + $search_str = $_SESSION['search'][0]; +} + // execute IMAP search if ($search_str) { // search all, current or subfolders folders @@ -126,10 +133,6 @@ if ($search_str) { $result = $RCMAIL->storage->search($mboxes, $search_str, $imap_charset, $sort_column); } -// Get the headers -$result_h = $RCMAIL->storage->list_messages($mbox, 1, $sort_column, rcmail_sort_order()); -$count = $RCMAIL->storage->count($mbox, $RCMAIL->storage->get_threading() ? 'THREADS' : 'ALL'); - // save search results in session if (!is_array($_SESSION['search'])) { $_SESSION['search'] = array(); @@ -142,6 +145,13 @@ if ($search_str) { $_SESSION['search_request'] = $search_request; $_SESSION['search_scope'] = $scope; + +// Get the headers +if (!$result->incomplete) { + $result_h = $RCMAIL->storage->list_messages($mbox, 1, $sort_column, rcmail_sort_order()); + $count = $RCMAIL->storage->count($mbox, $RCMAIL->storage->get_threading() ? 'THREADS' : 'ALL'); +} + // Make sure we got the headers if (!empty($result_h)) { rcmail_js_message_list($result_h, false); @@ -157,9 +167,15 @@ if (!empty($result_h)) { } } // handle IMAP errors (e.g. #1486905) -else if ($err_code = $RCMAIL->storage->get_error_code()) { +else if ($err_code = $RCMAIL->storage->get_error_code()) { $RCMAIL->display_server_error(); } +// advice the client to re-send the (cross-folder) search request +else if ($result->incomplete) { + $count = 0; // keep UI locked + $OUTPUT->command('continue_search', $search_request); + console('search incomplete', strlen(serialize($result))); +} else { $OUTPUT->show_message('searchnomatch', 'notice'); $OUTPUT->set_env('multifolder_listing', (bool)$result->multi);