MDL-15666 - change all the portfolio plugins and callers to use exceptions
rather than relying on return values (send_package and prepare_package) move zipping of temporary files into the exporter class so it can be overridden during unit tests fix a small todo in mahara plugin
This commit is contained in:
@@ -6,7 +6,7 @@ $string['addtoportfolio'] = 'Add to portfolio';
|
||||
$string['addalltoportfolio'] = 'Add all to portfolio';
|
||||
$string['alreadyexporting'] = 'You already have an active portfolio export in this session. Please <a href=\"$a->finish\">complete that first</a>, or <a href=\"$a->cancel\">click here</a> to cancel it.';
|
||||
$string['availableformats'] = 'Available export formats';
|
||||
$string['callercouldnotpackage'] = 'Failed to package up your data for export';
|
||||
$string['callercouldnotpackage'] = 'Failed to package up your data for export: original error was $a';
|
||||
$string['cannotsetvisible'] = 'Cannot set this to visible - the plugin has been completely disabled because of a misconfiguration';
|
||||
$string['configexport'] = 'Configure exported data';
|
||||
$string['configplugin'] = 'Configure portfolio plugin';
|
||||
@@ -28,7 +28,8 @@ $string['exportqueued'] = 'Portfolio export has been successfully queued for tra
|
||||
$string['exportqueuedforced'] = 'Portfolio export has been successfully queued for transfer (the remote system has enforced queued transfers)';
|
||||
$string['exportedpreviously'] = 'Previous exports';
|
||||
$string['exportexceptionnoexporter'] = 'A portfolio_export_exception was thrown with an active session but no exporter object';
|
||||
$string['failedtosendpackage'] = 'Failed to send your data to the selected portfolio system!';
|
||||
$string['failedtosendpackage'] = 'Failed to send your data to the selected portfolio system: original error was $a';
|
||||
$string['failedtopackage'] = 'Could not find files to package';
|
||||
$string['filedenied'] = 'Access denied to this file';
|
||||
$string['filenotfound'] = 'File not found';
|
||||
$string['format_file'] = 'File';
|
||||
@@ -79,7 +80,7 @@ $string['nouploaddirectory'] = 'Could not create a temporary directory to packag
|
||||
$string['portfolio'] = 'Portfolio';
|
||||
$string['portfolios'] = 'Portfolios';
|
||||
$string['plugin'] = 'Portfolio Plugin';
|
||||
$string['plugincouldnotpackage'] = 'Failed to package up your data for export';
|
||||
$string['plugincouldnotpackage'] = 'Failed to package up your data for export: original error was $a';
|
||||
$string['returntowhereyouwere'] = 'Return to where you were';
|
||||
$string['save'] = 'Save';
|
||||
$string['selectedformat'] = 'Selected export format';
|
||||
|
||||
@@ -19,5 +19,6 @@ $string['sharefolder'] = 'Share this new folder?';
|
||||
$string['targetfolder'] = 'Target folder';
|
||||
$string['tobecreated'] = 'To be created';
|
||||
$string['username'] = 'Your box.net username (will not be stored)';
|
||||
$string['sendfailed'] = 'Failed to send content to box.net: $a';
|
||||
|
||||
?>
|
||||
|
||||
@@ -426,11 +426,22 @@ class portfolio_exporter {
|
||||
// now we've agreed on a format,
|
||||
// the caller is given control to package it up however it wants
|
||||
// and then the portfolio plugin is given control to do whatever it wants.
|
||||
if (!$this->caller->prepare_package()) {
|
||||
throw new portfolio_export_exception($this, 'callercouldnotpackage', 'portfolio');
|
||||
try {
|
||||
$this->caller->prepare_package();
|
||||
} catch (portfolio_exception $e) {
|
||||
throw new portfolio_export_exception($this, 'callercouldnotpackage', 'portfolio', null, $e->getMessage());
|
||||
}
|
||||
if (!$package = $this->instance->prepare_package()) {
|
||||
throw new portfolio_export_exception($this, 'plugincouldnotpackage', 'portfolio');
|
||||
catch (file_exception $e) {
|
||||
throw new portfolio_export_exception($this, 'callercouldnotpackage', 'portfolio', null, $e->getMessage());
|
||||
}
|
||||
try {
|
||||
$this->instance->prepare_package();
|
||||
}
|
||||
catch (portfolio_exception $e) {
|
||||
throw new portfolio_export_exception($this, 'plugincouldnotpackage', 'portfolio', null, $e->getMessage());
|
||||
}
|
||||
catch (file_exception $e) {
|
||||
throw new portfolio_export_exception($this, 'plugincouldnotpackage', 'portfolio', null, $e->getMessage());
|
||||
}
|
||||
return true;
|
||||
}
|
||||
@@ -465,8 +476,13 @@ class portfolio_exporter {
|
||||
*/
|
||||
public function process_stage_send() {
|
||||
// send the file
|
||||
if (!$this->instance->send_package()) {
|
||||
throw new portfolio_export_exception($this, 'failedtosendpackage', 'portfolio');
|
||||
try {
|
||||
$this->instance->send_package();
|
||||
}
|
||||
catch (portfolio_plugin_exception $e) {
|
||||
// not catching anything more general here. plugins with dependencies on other libraries that throw exceptions should catch and rethrow.
|
||||
// eg curl exception
|
||||
throw new portfolio_export_exception($this, 'failedtosendpackage', 'portfolio', null, $e->getMessage());
|
||||
}
|
||||
// log the transfer
|
||||
global $DB;
|
||||
@@ -493,7 +509,7 @@ class portfolio_exporter {
|
||||
$extras = $this->instance->get_extra_finish_options();
|
||||
|
||||
$key = 'exportcomplete';
|
||||
if ($queued) {
|
||||
if ($queued || $this->forcequeue) {
|
||||
$key = 'exportqueued';
|
||||
if ($this->forcequeue) {
|
||||
$key = 'exportqueuedforced';
|
||||
@@ -660,6 +676,17 @@ class portfolio_exporter {
|
||||
return $fs->create_file_from_string($file_record, $content);
|
||||
}
|
||||
|
||||
public function zip_tempfiles($filename='portfolio-export.zip', $filepath='/final/') {
|
||||
$zipper = new zip_packer();
|
||||
|
||||
list ($contextid, $filearea, $itemid) = array_values($this->get_base_filearea());
|
||||
if ($newfile = $zipper->archive_to_storage($files, $contextid, $filearea, $itemid, $filepath, $filename, $this->user->id)) {
|
||||
return $newfile;
|
||||
}
|
||||
return false;
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
* returns an arary of files in the temporary working directory
|
||||
* for this export
|
||||
|
||||
@@ -3200,7 +3200,6 @@ class assignment_portfolio_caller extends portfolio_module_caller_base {
|
||||
foreach ($this->files as $file) {
|
||||
$this->exporter->copy_existing_file($file);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
public function get_sha1() {
|
||||
|
||||
@@ -275,7 +275,7 @@ class assignment_online extends assignment_base {
|
||||
|
||||
function portfolio_prepare_package($exporter, $userid=0) {
|
||||
$submission = $this->get_submission($userid);
|
||||
return $exporter->write_new_file(format_text($submission->data1, $submission->data2), 'assignment.html');
|
||||
$exporter->write_new_file(format_text($submission->data1, $submission->data2), 'assignment.html');
|
||||
}
|
||||
|
||||
function portfolio_supported_formats() {
|
||||
|
||||
+1
-1
@@ -899,7 +899,7 @@ class chat_portfolio_caller extends portfolio_module_caller_base {
|
||||
}
|
||||
$content = preg_replace('/\<img[^>]*\>/', '', $content);
|
||||
|
||||
return $this->exporter->write_new_file($content, clean_filename($this->cm->name . '-session.html'));
|
||||
$this->exporter->write_new_file($content, clean_filename($this->cm->name . '-session.html'));
|
||||
}
|
||||
|
||||
public static function display_name() {
|
||||
|
||||
+7
-9
@@ -7301,16 +7301,14 @@ class forum_portfolio_caller extends portfolio_module_caller_base {
|
||||
$content .= '<br /><br />' . $this->prepare_post($post);
|
||||
$this->copy_files($this->allfiles[$post->id]);
|
||||
}
|
||||
$this->get('exporter')->write_new_file($content, 'discussion.html');
|
||||
} else {
|
||||
$this->copy_files($this->postfiles, $this->attachment);
|
||||
if ($this->attachment) {
|
||||
return true; // all we need to do
|
||||
}
|
||||
$post = $this->prepare_post($this->post);
|
||||
$this->get('exporter')->write_new_file($post, 'post.html');
|
||||
return $this->get('exporter')->write_new_file($content, 'discussion.html');
|
||||
}
|
||||
return true;
|
||||
$this->copy_files($this->postfiles, $this->attachment);
|
||||
if ($this->attachment) {
|
||||
return; // all we need to do
|
||||
}
|
||||
$post = $this->prepare_post($this->post);
|
||||
$this->get('exporter')->write_new_file($post, 'post.html');
|
||||
}
|
||||
|
||||
private function copy_files($files, $justone=false) {
|
||||
|
||||
@@ -23,11 +23,10 @@ class portfolio_plugin_boxnet extends portfolio_plugin_push_base {
|
||||
$this->folders[$created['folder_id']] = $created['folder_name'];
|
||||
$this->set_export_config(array('folder' => $created['folder_id']));
|
||||
}
|
||||
return true; // don't do anything else for this plugin, we want to send all files as they are.
|
||||
// don't do anything else for this plugin, we want to send all files as they are.
|
||||
}
|
||||
|
||||
public function send_package() {
|
||||
$ret = array();
|
||||
foreach ($this->exporter->get_tempfiles() as $file) {
|
||||
$return = $this->boxclient->uploadFile(
|
||||
array(
|
||||
@@ -38,14 +37,13 @@ class portfolio_plugin_boxnet extends portfolio_plugin_push_base {
|
||||
);
|
||||
if (array_key_exists('status', $return) && $return['status'] == 'upload_ok'
|
||||
&& array_key_exists('id', $return) && count($return['id']) == 1) {
|
||||
$return['rename'] = $this->rename_file($return['id'][array_pop(array_keys($return['id']))], $file->get_filename());
|
||||
$ret[] = $return;
|
||||
$this->rename_file($return['id'][array_pop(array_keys($return['id']))], $file->get_filename());
|
||||
// if this fails, the file was sent but not renamed - this triggers a warning but is not fatal.
|
||||
}
|
||||
}
|
||||
if ($this->boxclient->isError()) {
|
||||
return false;
|
||||
throw new portfolio_plugin_exception('sendfailed', 'portfolio_boxnet', $this->boxclient->getErrorMsg());
|
||||
}
|
||||
return is_array($ret) && !empty($ret);
|
||||
}
|
||||
|
||||
public function get_export_summary() {
|
||||
|
||||
@@ -25,18 +25,9 @@ class portfolio_plugin_download extends portfolio_plugin_pull_base {
|
||||
|
||||
if (count($files) == 1) {
|
||||
$this->set('file', array_shift($files));
|
||||
return true;
|
||||
} else {
|
||||
$this->set('file', $this->exporter->zip_tempfiles()); // this will throw a file_exception which the exporter catches separately.
|
||||
}
|
||||
|
||||
$zipper = new zip_packer();
|
||||
|
||||
$filename = 'portfolio-export.zip';
|
||||
list ($contextid, $filearea, $itemid) = array_values($this->get('exporter')->get_base_filearea());
|
||||
if ($newfile = $zipper->archive_to_storage($files, $contextid, $filearea, $itemid, '/final/', $filename, $this->user->id)) {
|
||||
$this->set('file', $newfile);
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
public function get_extra_finish_options() {
|
||||
@@ -44,9 +35,7 @@ class portfolio_plugin_download extends portfolio_plugin_pull_base {
|
||||
return array($this->get_base_file_url() => get_string('downloadfile', 'portfolio_download'));
|
||||
}
|
||||
|
||||
public function send_package() {
|
||||
return true;
|
||||
}
|
||||
public function send_package() {}
|
||||
|
||||
public function verify_file_request_params($params) {
|
||||
// for download plugin the only thing we need to verify is that
|
||||
@@ -63,4 +52,3 @@ class portfolio_plugin_download extends portfolio_plugin_pull_base {
|
||||
}
|
||||
}
|
||||
|
||||
?>
|
||||
|
||||
@@ -16,11 +16,10 @@ class portfolio_plugin_flickr extends portfolio_plugin_push_base {
|
||||
|
||||
public function prepare_package() {
|
||||
$this->flickr = new phpFlickr($this->get_config('apikey'), $this->get_config('sharedsecret'));
|
||||
return true; // don't do anything else for this plugin, we want to send all files as they are.
|
||||
}
|
||||
|
||||
public function send_package() {
|
||||
|
||||
throw new portfolio_plugin_exception('notimplemented', 'portfolio', null, 'flickr');
|
||||
}
|
||||
|
||||
public static function allows_multiple() {
|
||||
|
||||
@@ -145,14 +145,8 @@ class portfolio_plugin_mahara extends portfolio_plugin_pull_base {
|
||||
);
|
||||
$this->totalsize += $f->get_filesize();
|
||||
}
|
||||
$zipper = new zip_packer();
|
||||
|
||||
$filename = 'portfolio-export.zip';
|
||||
if ($newfile = $zipper->archive_to_storage($files, SYSCONTEXTID, 'portfolio_exporter', $this->exporter->get('id'), '/final/', $filename, $this->user->id)) {
|
||||
$this->set('file', $newfile);
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
$this->set('file', $this->exporter->zip_tempfiles()); // this will throw a file_exception which the exporter catches separately.
|
||||
}
|
||||
|
||||
private function ensure_environment() {
|
||||
@@ -194,9 +188,9 @@ class portfolio_plugin_mahara extends portfolio_plugin_pull_base {
|
||||
if (!$response->status) {
|
||||
throw new portfolio_export_exception($this->get('exporter'), 'failedtoping', 'portfolio_mahara');
|
||||
}
|
||||
// @todo penny we should check $response->type here, it will tell us 'queued' or 'complete'
|
||||
// and we might want to tell the user if it's queued.
|
||||
return true;
|
||||
if ($response->type =='queued') {
|
||||
$this->exporter->set('forcequeue', true);
|
||||
}
|
||||
}
|
||||
|
||||
public function get_continue_url() {
|
||||
|
||||
Reference in New Issue
Block a user