portfolio MDL-20872 elegant handling of cleanup/display race condition

lots of work to elegantly resolve the issue between interactive browser
sessions and "pull" portfolio plugins that cause a race condition where we lose
the ability to display information to the user if the pull cleans up the export
first.  this also improves the portfolio transfer log display for later

diff --git a/lang/en_utf8/portfolio.php b/lang/en_utf8/portfolio.php
index ec958abb5aac18dba1ee5e015a71a2d594c51d91..3cf2f806e0627a465150dbf2385264c039a28acf 100644 (file)
--- a/lang/en_utf8/portfolio.php
+++ b/lang/en_utf8/portfolio.php
@@ -29,6 +29,8 @@ $string['dontwait'] = 'Don\'t wait';
 $string['err_uniquename'] = 'Portfolio name must be unique (per plugin)';
 $string['enabled'] = 'Enable portfolios';
 $string['enableddesc'] = 'This will allow administrators to configure remote systems for users to export content to';
+$string['exportalreadyfinished']= 'Portfolio export complete!';
+$string['exportalreadyfinisheddesc'] = 'Portfolio export complete!';
 $string['exporting'] = 'Exporting to portfolio';
 $string['exportingcontentfrom'] = 'Exporting content from $a';
 $string['exportcomplete'] = 'Portfolio export complete!';

diff --git a/lib/portfolio/exporter.php b/lib/portfolio/exporter.php
index ae74fdce47a8cf5b986f8497546a408e1cd5d4d6..f542698118878a77c714d31773adc845dac4f6c3 100644 (file)
--- a/lib/portfolio/exporter.php
+++ b/lib/portfolio/exporter.php
@@ -117,6 +117,12 @@ class portfolio_exporter {
      */
     private $expirytime;
 
+    /**
+     * deleted - this is set during the cleanup routine
+     * so that subsequent save() calls can detect it
+     */
+    private $deleted = false;
+
     /**
     * construct a new exporter for use
     *
@@ -485,6 +491,7 @@ class portfolio_exporter {
         $DB->delete_records('portfolio_tempdata', array('id' => $this->id));
         $fs = get_file_storage();
         $fs->delete_area_files(SYSCONTEXTID, 'portfolio_exporter', $this->id);
+        $this->deleted = true;
         return true;
     }
 
@@ -523,11 +530,23 @@ class portfolio_exporter {
             'caller_file'    => $this->callerfile,
             'caller_sha1'    => $this->caller->get_sha1(),
             'caller_class'   => get_class($this->caller),
+            'continueurl'    => $this->instance->get_static_continue_url(),
+            'returnurl'      => $this->caller->get_return_url(),
+            'tempdataid'     => $this->id,
             'time'           => time(),
         );
         $DB->insert_record('portfolio_log', $l);
     }
 
+    /**
+     * in some cases (mahara) we need to update this after the log has been done
+     * because of MDL-20872
+     */
+    public function update_log_url($url) {
+        global $DB;
+        $DB->set_field('portfolio_log', 'continueurl', $url, array('tempdataid' => $this->id));
+    }
+
     /**
     * processes the 'finish' stage of the export
     *
@@ -536,7 +555,7 @@ class portfolio_exporter {
     public function process_stage_finished($queued=false) {
         global $OUTPUT;
         $returnurl = $this->caller->get_return_url();
-        $continueurl = $this->instance->get_continue_url();
+        $continueurl = $this->instance->get_interactive_continue_url();
         $extras = $this->instance->get_extra_finish_options();
 
         $key = 'exportcomplete';
@@ -616,7 +635,9 @@ class portfolio_exporter {
             $this->save(); // call again so that id gets added to the save data.
         } else {
             if (!$r = $DB->get_record('portfolio_tempdata', array('id' => $this->id))) {
-                debugging("tried to save current object, but failed - see  MDL-20872");
+                if (!$this->deleted) {
+                    debugging("tried to save current object, but failed - see MDL-20872");
+                }
                 return;
             }
             $r->data = base64_encode(serialize($this));
@@ -774,7 +795,7 @@ class portfolio_exporter {
     *
     * @return array of stored_file objects keyed by name
     */
-    public function get_tempfiles() {
+    public function get_tempfiles($skipfile='portfolio-export.zip') {
         $fs = get_file_storage();
         $files = $fs->get_area_files(SYSCONTEXTID, 'portfolio_exporter', $this->id, '', false);
         if (empty($files)) {
@@ -782,6 +803,9 @@ class portfolio_exporter {
         }
         $returnfiles = array();
         foreach ($files as $f) {
+            if ($f->get_filename() == $skipfile) {
+                continue;
+            }
             $returnfiles[$f->get_filepath() . '/' . $f->get_filename()] = $f;
         }
         return $returnfiles;
@@ -856,15 +880,18 @@ class portfolio_exporter {
         return $DB->get_records_sql_menu($sql, $values);
     }
 
-    public static function print_cleaned_export($log) {
+    public static function print_cleaned_export($log, $instance=null) {
         global $CFG, $OUTPUT, $PAGE;
+        if (empty($instance) || !$instance instanceof portfolio_plugin) {
+            $instance = portfolio_instance($log->portfolio);
+        }
         $title = get_string('exportalreadyfinished', 'portfolio');
         $PAGE->navbar->add($title);
         $PAGE->set_title($title);
         $PAGE->set_heading($title);
         echo $OUTPUT->header();
         echo $OUTPUT->notification(get_string('exportalreadyfinished', 'portfolio'));
-        self::print_finish_info($log->returnurl, $log->continueurl);
+        self::print_finish_info($log->returnurl, $instance->resolve_static_continue_url($log->continueurl));
         echo $OUTPUT->continue_button($CFG->wwwroot);
         echo $OUTPUT->footer();
         exit;

diff --git a/lib/portfolio/plugin.php b/lib/portfolio/plugin.php
index a8a91349bbb7294b9e73426303d7815d16acc183..fd0dbe9666433d2062776ab755cc760cbadb8924 100644 (file)
--- a/lib/portfolio/plugin.php
+++ b/lib/portfolio/plugin.php
@@ -325,10 +325,29 @@ abstract class portfolio_plugin_base {
 
     /**
     * the url for the user to continue to their portfolio
+    * during the lifecycle of the request
     *
     * @return string url or false.
     */
-    public abstract function get_continue_url();
+    public abstract function get_interactive_continue_url();
+
+    /**
+     * the url to save in the log as the continue url
+     * this is passed through resolve_static_continue_url()
+     * at display time to the user.
+     */
+    public function get_static_continue_url() {
+        return $this->get_interactive_continue_url();
+    }
+
+    /**
+     * override this function if you need to add something on to the url
+     * for post-export continues (eg from the log page)
+     * mahara does this, for example, to start a jump session
+     */
+    public function resolve_static_continue_url($url) {
+        return $url;
+    }
 
     /**
     * mform to display to the user in their profile

diff --git a/lib/simpletest/portfolio_testclass.php b/lib/simpletest/portfolio_testclass.php
index 40586d7d2c9ef471d2b1a48a792a314b676c66b4..29aa3db5922c4705403deab88276fc2d2c68234c 100644 (file)
--- a/lib/simpletest/portfolio_testclass.php
+++ b/lib/simpletest/portfolio_testclass.php
@@ -51,7 +51,7 @@ class portfolio_plugin_test extends portfolio_plugin_push_base {
         return true;
     }
 
-    public function get_continue_url() {
+    public function get_interactive_continue_url() {
         return '';
     }
 

diff --git a/portfolio/type/boxnet/lib.php b/portfolio/type/boxnet/lib.php
index f03278c4190da44c2d1bf2585e2cd79b545ec4d1..408c7faeeed3089524511709637bd6d8762f2405 100644 (file)
--- a/portfolio/type/boxnet/lib.php
+++ b/portfolio/type/boxnet/lib.php
@@ -60,7 +60,7 @@ class portfolio_plugin_boxnet extends portfolio_plugin_push_base {
         );
     }
 
-    public function get_continue_url() {
+    public function get_interactive_continue_url() {
         return 'http://box.net/files#0:f:' . $this->get_export_config('folder');
     }
 

diff --git a/portfolio/type/download/lib.php b/portfolio/type/download/lib.php
index 1f175c952bc5d5958a58c85c9f908f72f24e45b2..3ad63109a741dcc875680975d8a4a6f4a7c6dee2 100644 (file)
--- a/portfolio/type/download/lib.php
+++ b/portfolio/type/download/lib.php
@@ -48,7 +48,7 @@ class portfolio_plugin_download extends portfolio_plugin_pull_base {
         return true;
     }
 
-    public function get_continue_url() {
+    public function get_interactive_continue_url() {
         return false;
     }
 }

diff --git a/portfolio/type/flickr/lib.php b/portfolio/type/flickr/lib.php
index 2ce4df1817f633be9e448908069a307c536fd8c2..4fdd703ad834ac719def4efcef4c97a725ad052a 100755 (executable)
--- a/portfolio/type/flickr/lib.php
+++ b/portfolio/type/flickr/lib.php
@@ -55,7 +55,7 @@ class portfolio_plugin_flickr extends portfolio_plugin_push_base {
         return false;
     }
 
-    public function get_continue_url() {
+    public function get_interactive_continue_url() {
         // return $this->flickr->urls_getUserPhotos();
         return "http://www.flickr.com/tools/uploader_edit.gne?ids="; // Add ids of uploaded files
     }

diff --git a/portfolio/type/googledocs/lib.php b/portfolio/type/googledocs/lib.php
index adfcc5507cef5516fec83f5d09bc534519bab5f3..db02fd1313bdb35828fd5c27edbdb0a5f18ed796 100644 (file)
--- a/portfolio/type/googledocs/lib.php
+++ b/portfolio/type/googledocs/lib.php
@@ -23,7 +23,7 @@ class portfolio_plugin_googledocs extends portfolio_plugin_push_base {
         return true;
     }
        
-    public function get_continue_url(){
+    public function get_interactive_continue_url(){
         return 'http://docs.google.com/';
     }
 

diff --git a/portfolio/type/mahara/lib.php b/portfolio/type/mahara/lib.php
index 2105cbeafeac8fd25d266260387e06e16d75816f..8688644e18d3b870200d63b8e07881b42193299a 100644 (file)
--- a/portfolio/type/mahara/lib.php
+++ b/portfolio/type/mahara/lib.php
@@ -195,22 +195,40 @@ class portfolio_plugin_mahara extends portfolio_plugin_pull_base {
         if (isset($response->querystring)) {
             $this->continueurl = $response->querystring;
         }
+        // if we're not queuing the logging might have already happened
+        $this->exporter->update_log_url($this->get_static_continue_url());
     }
 
-    public function get_continue_url() {
-        $this->ensure_mnethost();
-        $this->ensure_environment();
-        $mnetauth = get_auth_plugin('mnet');
+    public function get_static_continue_url() {
         $remoteurl = '/artefact/file/';// @todo penny this might change later when we change formats.
         if (isset($this->continueurl)) {
             $remoteurl .= $this->continueurl;
         }
+        return $remoteurl;
+    }
+
+    public function resolve_static_continue_url($remoteurl) {
+        static $sessions = array();
+        // if this is called mutliple times for the same host, stuff breaks
+        // so we have to keep track and just replace the wantsurl bit
+        // in case things go to different plugins or whatever
+        if (array_key_exists($this->get_config('mnethostid'), $sessions)) {
+            return preg_replace('/wantsurl=[^&]*&/', 'wantsurl=' . urlencode($remoteurl) . '&', $sessions[$this->get_config('mnethostid')]);
+        }
+        $this->ensure_mnethost();
+        $this->ensure_environment();
+        $mnetauth = get_auth_plugin('mnet');
         if (!$url = $mnetauth->start_jump_session($this->get_config('mnethostid'), $remoteurl)) {
             return false;
         }
+        $sessions[$this->get_config('mnethostid')] = $url;
         return $url;
     }
 
+    public function get_interactive_continue_url() {
+        return $this->resolve_static_continue_url($this->get_static_continue_url());
+    }
+
     public function steal_control($stage) {
         if ($stage != PORTFOLIO_STAGE_CONFIG) {
             return false;

diff --git a/portfolio/type/picasa/lib.php b/portfolio/type/picasa/lib.php
index 763945a527b1248c33c863d84ea8039f99948925..0632a7a0ee884026909ce412298f2577fd81c9e1 100644 (file)
--- a/portfolio/type/picasa/lib.php
+++ b/portfolio/type/picasa/lib.php
@@ -24,7 +24,7 @@ class portfolio_plugin_picasa extends portfolio_plugin_push_base {
         return true;
     }
        
-    public function get_continue_url(){
+    public function get_interactive_continue_url(){
         return 'http://picasaweb.google.com/';
     }
 

diff --git a/user/portfoliologs.php b/user/portfoliologs.php
index 078b622efc02a4368c79623d8cda9ebeb3a78ea0..b059d9e6704eebab2e7e2ac78e696407388f67c2 100644 (file)
--- a/user/portfoliologs.php
+++ b/user/portfoliologs.php
@@ -130,14 +130,19 @@ if ($logcount > 0) {
         $pluginname = '';
         try {
             $plugin = portfolio_instance($log->portfolio);
-            $pluginname = $plugin->get('name');
+            $url = $plugin->resolve_static_continue_url($log->continueurl);
+            if ($url) {
+                $pluginname = '<a href="' . $url . '">' . $plugin->get('name') . '</a>';
+            } else {
+                $pluginname = $plugin->get('name');
+            }
         } catch (portfolio_exception $e) { // may have been deleted
             $pluginname = get_string('unknownplugin', 'portfolio');
         }
 
         $table->data[] = array(
             $pluginname,
-            call_user_func(array($class, 'display_name')),
+            '<a href="' . $log->returnurl . '">' . call_user_func(array($class, 'display_name')) . '</a>',
             userdate($log->time),
         );
     }