From 5fe65fd4382f1f3975c46902a6c380abdf7a2f70 Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Fri, 23 Nov 2012 16:58:35 +0800 Subject: [PATCH] MDL-36741 mod_forum: fixed SQL that was generated when either timed posts or groups were enabled --- lib/rsslib.php | 15 +++++++-- mod/forum/rsslib.php | 89 +++++++++++++++++++++++----------------------------- 2 files changed, 52 insertions(+), 52 deletions(-) diff --git a/lib/rsslib.php b/lib/rsslib.php index dda87148396..6046e66b225 100644 --- a/lib/rsslib.php +++ b/lib/rsslib.php @@ -207,10 +207,21 @@ function rss_get_file_full_name($componentname, $filename) { * * @param stdClass $instance the instance of the source of the RSS feed * @param string $sql the SQL used to produce the RSS feed + * @param array $params the parameters used in the SQL query * @return string the name of the RSS file */ -function rss_get_file_name($instance, $sql) { - return $instance->id.'_'.md5($sql); +function rss_get_file_name($instance, $sql, $params = array()) { + if ($params) { + // If a parameters array is passed, then we want to + // serialize it and then concatenate it with the sql. + // The reason for this is to generate a unique filename + // for queries using the same sql but different parameters. + asort($parms); + $serializearray = serialize($params); + return $instance->id.'_'.md5($sql . $serializearray); + } else { + return $instance->id.'_'.md5($sql); + } } /** diff --git a/mod/forum/rsslib.php b/mod/forum/rsslib.php index 86bf31b7e92..bb463624ef5 100644 --- a/mod/forum/rsslib.php +++ b/mod/forum/rsslib.php @@ -56,16 +56,10 @@ function forum_rss_get_feed($context, $args) { } //the sql that will retreive the data for the feed and be hashed to get the cache filename - $sql = forum_rss_get_sql($forum, $cm); + list($sql, $params) = forum_rss_get_sql($forum, $cm); // Hash the sql to get the cache file name. - // If the forum is Q and A then we need to cache the files per user. This can - // have a large impact on performance, so we want to only do it on this type of forum. - if ($forum->type == 'qanda') { - $filename = rss_get_file_name($forum, $sql . $USER->id); - } else { - $filename = rss_get_file_name($forum, $sql); - } + $filename = rss_get_file_name($forum, $sql, $params); $cachedfilepath = rss_get_file_full_name('mod_forum', $filename); //Is the cache out of date? @@ -75,9 +69,9 @@ function forum_rss_get_feed($context, $args) { } //if the cache is more than 60 seconds old and there's new stuff $dontrecheckcutoff = time()-60; - if ( $dontrecheckcutoff > $cachedfilelastmodified && forum_rss_newstuff($forum, $cm, $cachedfilelastmodified)) { + if ($dontrecheckcutoff > $cachedfilelastmodified && forum_rss_newstuff($forum, $cm, $cachedfilelastmodified)) { //need to regenerate the cached version - $result = forum_rss_feed_contents($forum, $sql, $modcontext); + $result = forum_rss_feed_contents($forum, $sql, $params, $modcontext); if (!empty($result)) { $status = rss_save_file('mod_forum',$filename,$result); } @@ -111,10 +105,12 @@ function forum_rss_delete_file($forum) { function forum_rss_newstuff($forum, $cm, $time) { global $DB; - $sql = forum_rss_get_sql($forum, $cm, $time); + list($sql, $params) = forum_rss_get_sql($forum, $cm, $time); + if ($DB->count_records_sql($sql, $params) > 0) { + return true; + } - $recs = $DB->get_records_sql($sql, null, 0, 1);//limit of 1. If we get even 1 back we have new stuff - return ($recs && !empty($recs)); + return false; } /** @@ -126,17 +122,11 @@ function forum_rss_newstuff($forum, $cm, $time) { * @return string the SQL query to be used to get the Discussion/Post details from the forum table of the database */ function forum_rss_get_sql($forum, $cm, $time=0) { - $sql = null; - - if (!empty($forum->rsstype)) { - if ($forum->rsstype == 1) { //Discussion RSS - $sql = forum_rss_feed_discussions_sql($forum, $cm, $time); - } else { //Post RSS - $sql = forum_rss_feed_posts_sql($forum, $cm, $time); - } + if ($forum->rsstype == 1) { // Discussion RSS + return forum_rss_feed_discussions_sql($forum, $cm, $time); + } else { // Post RSS + return forum_rss_feed_posts_sql($forum, $cm, $time); } - - return $sql; } /** @@ -155,7 +145,7 @@ function forum_rss_feed_discussions_sql($forum, $cm, $newsince=0) { $modcontext = null; $now = round(time(), -2); - $params = array($cm->instance); + $params = array(); $modcontext = context_module::instance($cm->id); @@ -172,21 +162,21 @@ function forum_rss_feed_discussions_sql($forum, $cm, $newsince=0) { } } - //do we only want new posts? + // Do we only want new posts? if ($newsince) { - $newsince = " AND p.modified > '$newsince'"; + $params['newsince'] = $newsince; + $newsince = " AND p.modified > :newsince"; } else { $newsince = ''; } - //get group enforcing SQL - $groupmode = groups_get_activity_groupmode($cm); + // Get group enforcing SQL. + $groupmode = groups_get_activity_groupmode($cm); $currentgroup = groups_get_activity_group($cm); - $groupselect = forum_rss_get_group_sql($cm, $groupmode, $currentgroup, $modcontext); + list($groupselect, $groupparams) = forum_rss_get_group_sql($cm, $groupmode, $currentgroup, $modcontext); - if ($groupmode && $currentgroup) { - $params['groupid'] = $currentgroup; - } + // Add the groupparams to the params array. + $params = array_merge($params, $groupparams); $forumsort = "d.timemodified DESC"; $postdata = "p.id AS postid, p.subject, p.created as postcreated, p.modified, p.discussion, p.userid, p.message as postmessage, p.messageformat AS postformat, p.messagetrust AS posttrust"; @@ -199,7 +189,7 @@ function forum_rss_feed_discussions_sql($forum, $cm, $newsince=0) { WHERE d.forum = {$forum->id} AND p.parent = 0 $timelimit $groupselect $newsince ORDER BY $forumsort"; - return $sql; + return array($sql, $params); } /** @@ -213,19 +203,20 @@ function forum_rss_feed_discussions_sql($forum, $cm, $newsince=0) { function forum_rss_feed_posts_sql($forum, $cm, $newsince=0) { $modcontext = context_module::instance($cm->id); - //get group enforcement SQL - $groupmode = groups_get_activity_groupmode($cm); + // Get group enforcement SQL. + $groupmode = groups_get_activity_groupmode($cm); $currentgroup = groups_get_activity_group($cm); + $params = array(); - $groupselect = forum_rss_get_group_sql($cm, $groupmode, $currentgroup, $modcontext); + list($groupselect, $groupparams) = forum_rss_get_group_sql($cm, $groupmode, $currentgroup, $modcontext); - if ($groupmode && $currentgroup) { - $params['groupid'] = $currentgroup; - } + // Add the groupparams to the params array. + $params = array_merge($params, $groupparams); - //do we only want new posts? + // Do we only want new posts? if ($newsince) { - $newsince = " AND p.modified > '$newsince'"; + $params['newsince'] = $newsince; + $newsince = " AND p.modified > :newsince"; } else { $newsince = ''; } @@ -250,7 +241,7 @@ function forum_rss_feed_posts_sql($forum, $cm, $newsince=0) { $groupselect ORDER BY p.created desc"; - return $sql; + return array($sql, $params); } /** @@ -264,6 +255,7 @@ function forum_rss_feed_posts_sql($forum, $cm, $newsince=0) { */ function forum_rss_get_group_sql($cm, $groupmode, $currentgroup, $modcontext=null) { $groupselect = ''; + $params = array(); if ($groupmode) { if ($groupmode == VISIBLEGROUPS or has_capability('moodle/site:accessallgroups', $modcontext)) { @@ -272,7 +264,7 @@ function forum_rss_get_group_sql($cm, $groupmode, $currentgroup, $modcontext=nul $params['groupid'] = $currentgroup; } } else { - //seprate groups without access all + // Separate groups without access all. if ($currentgroup) { $groupselect = "AND (d.groupid = :groupid OR d.groupid = -1)"; $params['groupid'] = $currentgroup; @@ -282,7 +274,7 @@ function forum_rss_get_group_sql($cm, $groupmode, $currentgroup, $modcontext=nul } } - return $groupselect; + return array($groupselect, $params); } /** @@ -290,21 +282,19 @@ function forum_rss_get_group_sql($cm, $groupmode, $currentgroup, $modcontext=nul * It returns false if something is wrong * * @param stdClass $forum the forum object - * @param string $sql The SQL used to retrieve the contents from the database + * @param string $sql the SQL used to retrieve the contents from the database + * @param array $params the SQL parameters used * @param object $context the context this forum relates to * @return bool|string false if the contents is empty, otherwise the contents of the feed is returned * * @Todo MDL-31129 implement post attachment handling */ -function forum_rss_feed_contents($forum, $sql) { +function forum_rss_feed_contents($forum, $sql, $params, $context) { global $CFG, $DB, $USER; - $status = true; - $params = array(); - //$params['forumid'] = $forum->id; $recs = $DB->get_recordset_sql($sql, $params, 0, $forum->rssarticles); //set a flag. Are we displaying discussions or posts? @@ -316,7 +306,6 @@ function forum_rss_feed_contents($forum, $sql) { if (!$cm = get_coursemodule_from_instance('forum', $forum->id, $forum->course)) { print_error('invalidcoursemodule'); } - $context = context_module::instance($cm->id); $formatoptions = new stdClass(); $items = array(); -- 2.11.4.GIT