MDL-21951, improve comments performance

diff --git a/blocks/comments/block_comments.php b/blocks/comments/block_comments.php
index 1ed7932..56c3bb9 100644 (file)
--- a/blocks/comments/block_comments.php
+++ b/blocks/comments/block_comments.php
@@ -40,20 +40,20 @@ class block_comments extends block_base {
         $this->content->text = '';
         //TODO: guest and not-logged-in shoudl be able to read comments, right?
         if (isloggedin() && !isguestuser()) {   // Show the block
+            list($context, $course, $cm) = get_context_info_array($this->context->id);
             $cmt = new stdclass;
-            $cmt->context   = $this->context;
+            $cmt->context   = $context;
+            $cmt->course    = $course;
             $cmt->area      = 'block_comments';
             $cmt->itemid    = $this->instance->id;
-            $cmt->course    = $this->page->course;
-            // this is a hack to adjust commenting UI
-            // in block_comments
+            // this is a hack to adjust commenting UI in block_comments
             $cmt->env       = 'block_comments';
             $cmt->linktext  = get_string('showcomments');
             $comment = new comment($cmt);
+
             $this->content = new stdClass;
             $this->content->text = $comment->output(true);
             $this->content->footer = '';
-
         }
         return $this->content;
     }

diff --git a/blog/locallib.php b/blog/locallib.php
index 15307e1..04de513 100644 (file)
--- a/blog/locallib.php
+++ b/blog/locallib.php
@@ -100,7 +100,8 @@ class blog_entry {
         $user = $DB->get_record('user', array('id'=>$this->userid));
         // Comments
         $cmt = new stdClass();
-        $cmt->contextid = get_context_instance(CONTEXT_USER, $user->id)->id;
+        $cmt->context = get_context_instance(CONTEXT_USER, $user->id);
+        $cmt->courseid = $COURSE->id;
         $cmt->area = 'format_blog';
         $cmt->env = 'blog';
         $cmt->itemid = $this->id;

diff --git a/comment/comment_ajax.php b/comment/comment_ajax.php
index e5cd555..08d948a 100644 (file)
--- a/comment/comment_ajax.php
+++ b/comment/comment_ajax.php
@@ -39,20 +39,22 @@ $page      = optional_param('page',      0,      PARAM_INT);
 
 if (!empty($client_id)) {
     $cmt = new stdclass;
-    $cmt->contextid = $contextid;
-    if (!empty($course)) {
-        $cmt->courseid  = $course->id;
-    }
+    $cmt->context   = $context;
+    $cmt->course    = $course;
+    $cmt->cm        = $cm;
     $cmt->area      = $area;
     $cmt->itemid    = $itemid;
     $cmt->client_id = $client_id;
     $comment = new comment($cmt);
+} else {
+    die;
 }
+
 switch ($action) {
     case 'add':
         $cmt = $comment->add($content);
-        $cmt->count = $comment->count();
         if (!empty($cmt) && is_object($cmt)) {
+            $cmt->count = $comment->count();
             $cmt->client_id = $client_id;
             echo json_encode($cmt);
         }

diff --git a/comment/lib.php b/comment/lib.php
index befe16f..3d1afd9 100644 (file)
--- a/comment/lib.php
+++ b/comment/lib.php
@@ -54,13 +54,18 @@ class comment {
      */
     private $template;
     private $context;
-    private $course;
+    private $courseid;
     /**
-     * course module object
+     * course module object, only be used to help find pluginname automatically
+     * if pluginname is specified, it won't be used at all
      * @var string
      */
     private $cm;
     private $plugintype;
+    /**
+     * When used in module, it is recommended to use it
+     * @var string
+     */
     private $pluginname;
     private $viewcap;
     private $postcap;
@@ -74,14 +79,15 @@ class comment {
      * @var string
      */
     private $linktext;
+
+    // static variable will be used by non-js comments UI
     private static $nonjs = false;
-    // will be used by non-js comments UI
     private static $comment_itemid = null;
     private static $comment_context = null;
     private static $comment_area = null;
        private static $comment_page = null;
     /**
-     * Construct function of comment class, initilize
+     * Construct function of comment class, initialise
      * class members
      * @param object $options
      */
@@ -95,12 +101,14 @@ class comment {
         $this->viewcap = false;
         $this->postcap = false;
 
+        // setup client_id
         if (!empty($options->client_id)) {
             $this->cid = $options->client_id;
         } else {
             $this->cid = uniqid();
         }
-
+        
+        // setup context
         if (!empty($options->context)) {
             $this->context = $options->context;
             $this->contextid = $this->context->id;
@@ -111,32 +119,71 @@ class comment {
             print_error('invalidcontext');
         }
 
+        // setup course
+        // course will be used to generate user profile link
+        if (!empty($options->course)) {
+            $this->courseid = $options->course->id;
+        } else if (!empty($options->courseid)) {
+            $this->courseid = $options->courseid;
+        }
+
+        if (!empty($options->pluginname)) {
+            $this->pluginname = $options->pluginname;
+        }
+        
+        // setup coursemodule
+        if (!empty($options->cm)) {
+            $this->cm = $options->cm;
+        } else {
+            $this->cm = null;
+        }
+
+        // setup commentarea
         if (!empty($options->area)) {
             $this->commentarea = $options->area;
         }
 
+        // setup itemid
         if (!empty($options->itemid)) {
             $this->itemid = $options->itemid;
         }
 
+        // setup env
         if (!empty($options->env)) {
             $this->env = $options->env;
         } else {
             $this->env = '';
         }
 
+        // setup customized linktext
         if (!empty($options->linktext)) {
             $this->linktext = $options->linktext;
         } else {
             $this->linktext = get_string('comments');
         }
+        // setting post and view permissions
+        $this->check_permissions();
+
+        if (!empty($options->showcount)) {
+            $count = $this->count();
+            if (empty($count)) {
+                $this->count = '';
+            } else {
+                $this->count = '('.$count.')';
+            }
+        } else {
+            $this->count = '';
+        }
 
         $this->setup_plugin();
 
-        $this->options = new stdclass;
-        $this->options->context     = $this->context;
-        $this->options->commentarea = $this->commentarea;
-        $this->options->itemid      = $this->itemid;
+        // setup options for callback functions
+        $this->args = new stdclass;
+        $this->args->context     = $this->context;
+        $this->args->courseid    = $this->courseid;
+        $this->args->cm          = $this->cm;
+        $this->args->commentarea = $this->commentarea;
+        $this->args->itemid      = $this->itemid;
 
         // load template
         $this->template = <<<EOD
@@ -147,32 +194,7 @@ class comment {
 </div>
 EOD;
         if (!empty($this->plugintype)) {
-            $this->template = plugin_callback($this->plugintype, $this->pluginname, FEATURE_COMMENT, 'template', $this->options, $this->template);
-        }
-
-        // setting post and view permissions
-        $this->check_permissions();
-
-        // setup course
-        if (!empty($options->course)) {
-            $this->course   = $options->course;
-        } else if (!empty($options->courseid)) {
-            $courseid = $options->courseid;
-            $this->setup_course($courseid);
-        } else {
-            $courseid = SITEID;
-            $this->setup_course($courseid);
-        }
-
-        if (!empty($options->showcount)) {
-            $count = $this->count();
-            if (empty($count)) {
-                $this->count = '';
-            } else {
-                $this->count = '('.$count.')';
-            }
-        } else {
-            $this->count = '';
+            $this->template = plugin_callback($this->plugintype, $this->pluginname, FEATURE_COMMENT, 'template', $this->args, $this->template);
         }
 
         unset($options);
@@ -195,22 +217,8 @@ EOD;
         $PAGE->requires->string_for_js('comments', 'moodle');
     }
 
-    private function setup_course($courseid) {
-        global $PAGE, $DB;
-        if (!empty($this->course)) {
-            // already set, stop
-            return;
-        }
-        if ($courseid == $PAGE->course->id) {
-            $this->course = $PAGE->course;
-        } else if (!$this->course = $DB->get_record('course', array('id'=>$courseid))) {
-            $this->course = null;
-        }
-    }
-
     /**
-     * Setting module info
-     *
+     * Setup plugin type and plugin name
      */
     private function setup_plugin() {
         global $DB;
@@ -230,12 +238,18 @@ EOD;
                 $this->pluginname = $block->blockname;
             }
         }
+
         if ($this->context->contextlevel == CONTEXT_MODULE) {
             $this->plugintype = 'mod';
-            $this->cm = get_coursemodule_from_id('', $this->context->instanceid);
-            $this->setup_course($this->cm->course);
-            $this->modinfo = get_fast_modinfo($this->course);
-            $this->pluginname = $this->modinfo->cms[$this->cm->id]->modname;
+            // to improve performance, pluginname should be assigned before initilise comment object
+            // if it is empty, we will try to guess, it will rarely be used.
+            if (empty($this->pluginname)) {
+                if (empty($this->course)) {
+                    $this->course = $DB->get_record('course', array('id'=>$this->courseid), '*', MUST_EXIST);
+                }
+                $this->modinfo = get_fast_modinfo($this->course);
+                $this->pluginname = $this->modinfo->cms[$this->cm->id]->modname;
+            }
         }
     }
 
@@ -250,7 +264,7 @@ EOD;
         $this->postcap = has_capability('moodle/comment:post', $this->context);
         $this->viewcap = has_capability('moodle/comment:view', $this->context);
         if (!empty($this->plugintype)) {
-            $permissions = plugin_callback($this->plugintype, $this->pluginname, FEATURE_COMMENT, 'permissions', $this->options, array('post'=>true, 'view'=>true));
+            $permissions = plugin_callback($this->plugintype, $this->pluginname, FEATURE_COMMENT, 'permissions', $this->args, array('post'=>true, 'view'=>true));
             $this->postcap = $this->postcap && $permissions['post'];
             $this->viewcap = $this->viewcap && $permissions['view'];
         }
@@ -269,9 +283,9 @@ EOD;
         $murl = new moodle_url($this->link);
         $murl->remove_params('nonjscomment');
         $murl->param('nonjscomment', 'true');
-        $murl->param('comment_itemid', $this->options->itemid);
-        $murl->param('comment_context', $this->options->context->id);
-        $murl->param('comment_area', $this->options->commentarea);
+        $murl->param('comment_itemid', $this->itemid);
+        $murl->param('comment_context', $this->context->id);
+        $murl->param('comment_area', $this->commentarea);
         $murl->remove_params('comment_page');
         $this->link = $murl->out();
 
@@ -280,7 +294,7 @@ EOD;
         $options->commentarea = $this->commentarea;
         $options->itemid = $this->itemid;
         $options->page   = 0;
-        $options->courseid = $this->course->id;
+        $options->courseid = $this->courseid;
         $options->contextid = $this->contextid;
         $options->env = $this->env;
         if ($this->env == 'block_comments') {
@@ -396,7 +410,7 @@ EOD;
         $candelete = has_capability('moodle/comment:delete', $this->context);
         if ($records = $DB->get_records_sql($sql, $params, $start, $CFG->commentsperpage)) {
             foreach ($records as &$c) {
-                $url = $CFG->httpswwwroot.'/user/view.php?id='.$c->userid.'&amp;course='.$this->course->id;
+                $url = $CFG->httpswwwroot.'/user/view.php?id='.$c->userid.'&amp;course='.$this->courseid;
                 $c->username = '<a href="'.$url.'">'.fullname($c).'</a>';
                 $c->time = userdate($c->timecreated, get_string('strftimerecent', 'langconfig'));
                 $user = new stdclass;
@@ -413,10 +427,9 @@ EOD;
                 $comments[] = $c;
             }
         }
-
         if (!empty($this->plugintype)) {
             // moodle module will filter comments
-            $comments = plugin_callback($this->plugintype, $this->pluginname, FEATURE_COMMENT, 'display', array($comments, $this->options));
+            $comments = plugin_callback($this->plugintype, $this->pluginname, FEATURE_COMMENT, 'display', array($comments, $this->args), $comments);
         }
 
         return $comments;
@@ -480,7 +493,7 @@ EOD;
 
         if (!empty($this->plugintype)) {
             // moodle module will check content
-            $ret = plugin_callback($this->plugintype, $this->pluginname, FEATURE_COMMENT, 'add', array(&$newcmt, $this->options), true);
+            $ret = plugin_callback($this->plugintype, $this->pluginname, FEATURE_COMMENT, 'add', array(&$newcmt, $this->args), true);
             if (!$ret) {
                 throw new comment_exception('modulererejectcomment');
             }
@@ -541,9 +554,9 @@ EOD;
     public function print_comments($page = 0, $return = true, $nonjs = true) {
         global $DB, $CFG, $PAGE;
         $html = '';
-        if (!(self::$comment_itemid == $this->options->itemid &&
-            self::$comment_context == $this->options->context->id &&
-            self::$comment_area == $this->options->commentarea)) {
+        if (!(self::$comment_itemid == $this->itemid &&
+            self::$comment_context == $this->context->id &&
+            self::$comment_area == $this->commentarea)) {
             $page = 0;
         }
         $comments = $this->get_comments($page);
@@ -575,7 +588,7 @@ EOD;
 <input type="hidden" name="action" value="add" />
 <input type="hidden" name="area" value="$this->commentarea" />
 <input type="hidden" name="itemid" value="$this->itemid" />
-<input type="hidden" name="courseid" value="{$this->course->id}" />
+<input type="hidden" name="courseid" value="{$this->courseid}" />
 <input type="hidden" name="sesskey" value="{$sesskey}" />
 <input type="hidden" name="returnurl" value="{$returnurl}" />
 <input type="submit" value="{$strsubmit}" />

diff --git a/lib/moodlelib.php b/lib/moodlelib.php
index 8b0da79..51681ea 100644 (file)
--- a/lib/moodlelib.php
+++ b/lib/moodlelib.php
@@ -439,7 +439,6 @@ function optional_param($parname, $default=NULL, $type=PARAM_CLEAN) {
     } else {
         return $default;
     }
-
     return clean_param($param, $type);
 }
 
@@ -520,7 +519,6 @@ function validate_param($param, $type, $allownull=NULL_NOT_ALLOWED, $debuginfo='
 function clean_param($param, $type) {
 
     global $CFG;
-
     if (is_array($param)) {              // Let's loop
         $newparam = array();
         foreach ($param as $key => $value) {
@@ -4883,6 +4881,7 @@ function send_password_change_confirmation_email($user) {
     $data->lastname  = $user->lastname;
     $data->sitename  = format_string($site->fullname);
     $data->link      = $CFG->httpswwwroot .'/login/forgot_password.php?p='. $user->secret .'&s='. urlencode($user->username);
+    debug($data);
     $data->admin     = generate_email_signoff();
 
     $message = get_string('emailpasswordconfirmation', '', $data);
@@ -9638,3 +9637,16 @@ function mnet_get_idp_jump_url($user) {
     }
     return $mnetjumps[$user->mnethostid];
 }
+
+function echo_fb($var, $label='info') {
+    require_once('FirePHPCore/FirePHP.class.php');
+    $firephp = FirePHP::getInstance(true);
+    $firephp->log($var, $label);
+}
+
+function debug($text) {
+    $str = time();
+    $str .= var_export($text, true);
+    $str .= "\n==\n";
+    file_put_contents('/Library/WebServer/moodledata/output.log', $str, FILE_APPEND);
+}

diff --git a/mod/data/db/upgrade.php b/mod/data/db/upgrade.php
index 58a73b5..50e5eba 100644 (file)
--- a/mod/data/db/upgrade.php
+++ b/mod/data/db/upgrade.php
@@ -199,7 +199,7 @@ function xmldb_data_upgrade($oldversion) {
                 FROM {data_comments} c, {data_records} r, {data} d
                 WHERE c.recordid=r.id AND r.dataid=d.id ORDER BY dataid, courseid';
 
-            /// move data comments to new comments table
+            /// move data comments to comments table
             $lastdataid = null;
             $lastcourseid = null;
             $modcontext = null;
@@ -214,7 +214,8 @@ function xmldb_data_upgrade($oldversion) {
                         $lastcourseid = $res->courseid;
                     }
                     $cmt = new stdclass;
-                    $cmt->contextid = $modcontext->id;
+                    $cmt->pluginname = 'data';
+                    $cmt->context   = $modcontext;
                     $cmt->courseid  = $res->courseid;
                     $cmt->area      = 'database_entry';
                     $cmt->itemid    = $res->itemid;

diff --git a/mod/data/lib.php b/mod/data/lib.php
index f91b280..bcb7ef0 100755 (executable)
--- a/mod/data/lib.php
+++ b/mod/data/lib.php
@@ -1278,10 +1278,13 @@ function data_print_template($template, $records, $data, $search='', $page=0, $r
 
             if (!empty($CFG->usecomments)) {
                 require_once($CFG->dirroot  . '/comment/lib.php');
+                list($context, $course, $cm) = get_context_info_array($context->id);
                 $cmt = new stdclass;
-                $modcontext = get_context_instance(CONTEXT_MODULE, $cm->id);
-                $cmt->area    = 'database_entry';
+                $cmt->pluginname = 'data';
                 $cmt->context = $context;
+                $cmt->course  = $course;
+                $cmt->cm      = $cm;
+                $cmt->area    = 'database_entry';
                 $cmt->itemid  = $record->id;
                 $cmt->showcount = true;
                 $comment = new comment($cmt);
@@ -1314,10 +1317,13 @@ function data_print_template($template, $records, $data, $search='', $page=0, $r
             if (($template == 'singletemplate') && ($data->comments)) {    //prints ratings options
                 if (!empty($CFG->usecomments)) {
                     require_once($CFG->dirroot . '/comment/lib.php');
+                    list($context, $course, $cm) = get_context_info_array($context->id);
                     $cmt = new stdclass;
-                    $modcontext = get_context_instance(CONTEXT_MODULE, $cm->id);
-                    $cmt->area    = 'database_entry';
+                    $cmt->pluginname = 'data';
                     $cmt->context = $context;
+                    $cmt->course  = $course;
+                    $cmt->cm      = $cm;
+                    $cmt->area    = 'database_entry';
                     $cmt->itemid  = $record->id;
                     $cmt->showcount = true;
                     $comment = new comment($cmt);

diff --git a/mod/glossary/db/upgrade.php b/mod/glossary/db/upgrade.php
index 168b91e..8d9f225 100644 (file)
--- a/mod/glossary/db/upgrade.php
+++ b/mod/glossary/db/upgrade.php
@@ -227,7 +227,7 @@ function xmldb_glossary_upgrade($oldversion) {
             $lastcourseid   = null;
             $modcontext     = null;
 
-        /// move glossary comments to new comments table
+        /// move glossary comments to comments table
             if ($rs = $DB->get_recordset_sql($sql)) {
                 foreach($rs as $res) {
                     if ($res->glossaryid != $lastglossaryid || $res->courseid != $lastcourseid) {
@@ -238,11 +238,14 @@ function xmldb_glossary_upgrade($oldversion) {
                         $lastglossaryid = $res->glossaryid;
                         $lastcourseid   = $res->courseid;
                     }
+                    list($context, $course, $cm) = get_context_info_array($contextid);
                     $cmt = new stdclass;
-                    $cmt->contextid = $modcontext->id;
-                    $cmt->courseid  = $res->courseid;
-                    $cmt->area      = 'glossary_entry';
-                    $cmt->itemid    = $res->itemid;
+                    $cmt->pluginname = 'glossary';
+                    $cmt->context  = $modcontext;
+                    $cmt->courseid = $res->courseid;
+                    $cmt->cm       = $cm;
+                    $cmt->area     = 'glossary_entry';
+                    $cmt->itemid   = $res->itemid;
                     $comment = new comment($cmt);
                     $cmt = $comment->add($res->commentcontent, $res->format);
                     if (!empty($cmt)) {

diff --git a/mod/glossary/lib.php b/mod/glossary/lib.php
index 5878206..0e2ca93 100644 (file)
--- a/mod/glossary/lib.php
+++ b/mod/glossary/lib.php
@@ -1009,13 +1009,15 @@ function glossary_print_entry_icons($course, $cm, $glossary, $entry, $mode='',$h
         if (!empty($CFG->usecomments)) {
             require_once($CFG->dirroot . '/comment/lib.php');
             $cmt = new stdclass;
-            $modcontext = get_context_instance(CONTEXT_MODULE, $cm->id);
-            $cmt->area    = 'glossary_entry';
-            $cmt->context = $modcontext;
-            $cmt->itemid  = $entry->id;
+            $cmt->pluginname = 'glossary';
+            $cmt->context  = $context;
+            $cmt->course   = $course->id;
+            $cmt->cm       = $cm;
+            $cmt->area     = 'glossary_entry';
+            $cmt->itemid   = $entry->id;
             $cmt->showcount = true;
             $comment = new comment($cmt);
-            $return .= '<div style="width:500px">'.$comment->output(true).'</div>';
+            $return .= '<div>'.$comment->output(true).'</div>';
         }
     }