From 588758ed6d14a6c17eba9acf440164867c21cf25 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Tue, 28 Sep 2010 15:45:00 -0700 Subject: [PATCH] Made YammerImport more robust against errors; can now pause/resume/reset the import state from the admin interface. --- .../YammerImport/actions/yammeradminpanel.php | 13 ++++++ plugins/YammerImport/classes/Yammer_state.php | 3 ++ plugins/YammerImport/css/admin.css | 8 +++- plugins/YammerImport/lib/yammerimporter.php | 1 + .../YammerImport/lib/yammerprogressform.php | 39 +++++++++++++++-- .../YammerImport/lib/yammerqueuehandler.php | 23 +++++----- plugins/YammerImport/lib/yammerrunner.php | 42 ++++++++++++++++++- 7 files changed, 113 insertions(+), 16 deletions(-) diff --git a/plugins/YammerImport/actions/yammeradminpanel.php b/plugins/YammerImport/actions/yammeradminpanel.php index 13960d9051..3faf390ac1 100644 --- a/plugins/YammerImport/actions/yammeradminpanel.php +++ b/plugins/YammerImport/actions/yammeradminpanel.php @@ -73,6 +73,7 @@ class YammeradminpanelAction extends AdminPanelAction { // @fixme move this to saveSettings and friends? if ($_SERVER['REQUEST_METHOD'] == 'POST') { + StatusNet::setApi(true); // short error pages :P $this->checkSessionToken(); if ($this->subaction == 'change-apikey') { $form = new YammerApiKeyForm($this); @@ -97,6 +98,18 @@ class YammeradminpanelAction extends AdminPanelAction $this->runner->startBackgroundImport(); $form = new YammerProgressForm($this, $this->runner); + } else if ($this->subaction == 'pause-import') { + $this->runner->recordError(_m('Paused from admin panel.')); + $form = $this->statusForm(); + } else if ($this->subaction == 'continue-import') { + $this->runner->clearError(); + $this->runner->startBackgroundImport(); + $form = $this->statusForm(); + } else if ($this->subaction == 'abort-import') { + $this->runner->reset(); + $form = $this->statusForm(); + } else if ($this->subaction == 'progress') { + $form = $this->statusForm(); } else { throw new ClientException('Invalid POST'); } diff --git a/plugins/YammerImport/classes/Yammer_state.php b/plugins/YammerImport/classes/Yammer_state.php index 2f1fd7780b..88bd693bfd 100644 --- a/plugins/YammerImport/classes/Yammer_state.php +++ b/plugins/YammerImport/classes/Yammer_state.php @@ -36,6 +36,7 @@ class Yammer_state extends Memcached_DataObject public $__table = 'yammer_state'; // table name public $id; // int primary_key not_null public $state; // import state key + public $last_error; // text of last-encountered error, if any public $oauth_token; // actual oauth token! clear when import is done? public $oauth_secret; // actual oauth secret! clear when import is done? public $users_page; // last page of users we've fetched @@ -70,6 +71,7 @@ class Yammer_state extends Memcached_DataObject return array(new ColumnDef('id', 'int', null, false, 'PRI'), new ColumnDef('state', 'text'), + new ColumnDef('last_error', 'text'), new ColumnDef('oauth_token', 'text'), new ColumnDef('oauth_secret', 'text'), new ColumnDef('users_page', 'int'), @@ -93,6 +95,7 @@ class Yammer_state extends Memcached_DataObject { return array('id' => DB_DATAOBJECT_INT + DB_DATAOBJECT_NOTNULL, 'state' => DB_DATAOBJECT_STR, + 'last_error' => DB_DATAOBJECT_STR, 'oauth_token' => DB_DATAOBJECT_STR, 'oauth_secret' => DB_DATAOBJECT_STR, 'users_page' => DB_DATAOBJECT_INT, diff --git a/plugins/YammerImport/css/admin.css b/plugins/YammerImport/css/admin.css index 9c99a0b880..80e0e038a2 100644 --- a/plugins/YammerImport/css/admin.css +++ b/plugins/YammerImport/css/admin.css @@ -52,4 +52,10 @@ .magiclink { margin-left: 40px; -} \ No newline at end of file +} + +fieldset.import-error { + margin-top: 12px; + margin-bottom: 0px !important; + background-color: #fee !important; +} diff --git a/plugins/YammerImport/lib/yammerimporter.php b/plugins/YammerImport/lib/yammerimporter.php index ed11915259..80cbcff8e7 100644 --- a/plugins/YammerImport/lib/yammerimporter.php +++ b/plugins/YammerImport/lib/yammerimporter.php @@ -132,6 +132,7 @@ class YammerImporter if ($noticeId) { return Notice::staticGet('id', $noticeId); } else { + $notice = Notice::staticGet('uri', $data['options']['uri']); $content = $data['content']; $user = User::staticGet($data['profile']); diff --git a/plugins/YammerImport/lib/yammerprogressform.php b/plugins/YammerImport/lib/yammerprogressform.php index 776efa100f..add8d9ab2f 100644 --- a/plugins/YammerImport/lib/yammerprogressform.php +++ b/plugins/YammerImport/lib/yammerprogressform.php @@ -9,7 +9,7 @@ class YammerProgressForm extends Form */ function id() { - return 'yammer-progress'; + return 'yammer-progress-form'; } /** @@ -39,8 +39,11 @@ class YammerProgressForm extends Form */ function formData() { + $this->out->hidden('subaction', 'progress'); + $runner = YammerRunner::init(); + $error = $runner->lastError(); $userCount = $runner->countUsers(); $groupCount = $runner->countGroups(); $fetchedCount = $runner->countFetchedNotices(); @@ -86,7 +89,13 @@ class YammerProgressForm extends Form $steps = array_keys($labels); $currentStep = array_search($runner->state(), $steps); - $this->out->elementStart('fieldset', array('class' => 'yammer-import')); + $classes = array('yammer-import'); + if ($error) { + $classes[] = 'yammer-error'; + } else { + $classes[] = 'yammer-running'; + } + $this->out->elementStart('fieldset', array('class' => implode(' ', $classes))); $this->out->element('legend', array(), _m('Import status')); foreach ($steps as $step => $state) { if ($state == 'init') { @@ -104,7 +113,8 @@ class YammerProgressForm extends Form $this->progressBar($state, 'progress', $labels[$state]['label'], - $labels[$state]['progress']); + $labels[$state]['progress'], + $error); } else { // This step has not yet been done. $this->progressBar($state, @@ -116,13 +126,34 @@ class YammerProgressForm extends Form $this->out->elementEnd('fieldset'); } - private function progressBar($state, $class, $label, $status) + private function progressBar($state, $class, $label, $status, $error=null) { // @fixme prettify ;) $this->out->elementStart('div', array('class' => "import-step import-step-$state $class")); $this->out->element('div', array('class' => 'import-label'), $label); $this->out->element('div', array('class' => 'import-status'), $status); + if ($class == 'progress') { + if ($state == 'done') { + $this->out->submit('abort-import', _m('Reset import state')); + } else { + if ($error) { + $this->errorBox($error); + } else { + $this->out->submit('pause-import', _m('Pause import')); + } + } + } $this->out->elementEnd('div'); } + private function errorBox($msg) + { + $errline = sprintf(_m('Encountered error "%s"'), $msg); + $this->out->elementStart('fieldset', array('class' => 'import-error')); + $this->out->element('legend', array(), _m('Paused')); + $this->out->element('p', array(), $errline); + $this->out->submit('continue-import', _m('Continue')); + $this->out->submit('abort-import', _m('Abort import')); + $this->out->elementEnd('fieldset'); + } } diff --git a/plugins/YammerImport/lib/yammerqueuehandler.php b/plugins/YammerImport/lib/yammerqueuehandler.php index acc8073115..0c4e8aec1d 100644 --- a/plugins/YammerImport/lib/yammerqueuehandler.php +++ b/plugins/YammerImport/lib/yammerqueuehandler.php @@ -38,21 +38,24 @@ class YammerQueueHandler extends QueueHandler { $runner = YammerRunner::init(); if ($runner->hasWork()) { - if ($runner->iterate()) { - if ($runner->hasWork()) { - // More to do? Shove us back on the queue... - $runner->startBackgroundImport(); + try { + if ($runner->iterate()) { + if ($runner->hasWork()) { + // More to do? Shove us back on the queue... + $runner->startBackgroundImport(); + } + } + } catch (Exception $e) { + try { + $runner->recordError($e->getMessage()); + } catch (Exception $f) { + common_log(LOG_ERR, "Error while recording error in Yammer background import: " . $e->getMessage() . " " . $f->getMessage()); } - return true; - } else { - // Something failed? - // @fixme should we be trying again here, or should we give warning? - return false; } } else { // We're done! common_log(LOG_INFO, "Yammer import has no work to do at this time; discarding."); - return true; } + return true; } } diff --git a/plugins/YammerImport/lib/yammerrunner.php b/plugins/YammerImport/lib/yammerrunner.php index e0aec0d166..3e53f3361b 100644 --- a/plugins/YammerImport/lib/yammerrunner.php +++ b/plugins/YammerImport/lib/yammerrunner.php @@ -298,7 +298,10 @@ class YammerRunner $this->state->state = 'save-messages'; } else { foreach ($messages as $item) { - Yammer_notice_stub::record($item['id'], $item); + $stub = Yammer_notice_stub::staticGet($item['id']); + if (!$stub) { + Yammer_notice_stub::record($item['id'], $item); + } $oldest = $item['id']; } $this->state->messages_oldest = $oldest; @@ -395,4 +398,41 @@ class YammerRunner $qm->enqueue('YammerImport', 'yammer'); } + /** + * Record an error condition from a background run, which we should + * display in progress state for the admin. + * + * @param string $msg + */ + public function recordError($msg) + { + // HACK HACK HACK + try { + $temp = new Yammer_state(); + $temp->query('ROLLBACK'); + } catch (Exception $e) { + common_log(LOG_ERR, 'Exception while confirming rollback while recording error: ' . $e->getMessage()); + } + $old = clone($this->state); + $this->state->last_error = $msg; + $this->state->update($old); + } + + /** + * Clear the error state. + */ + public function clearError() + { + $this->recordError(''); + } + + /** + * Get the last recorded background error message, if any. + * + * @return string + */ + public function lastError() + { + return $this->state->last_error; + } }