Skip to content

Commit

Permalink
MDL-36741 mod_forum: fixed SQL that was generated when either timed p…
Browse files Browse the repository at this point in the history
…osts or groups were enabled
  • Loading branch information
mdjnelson committed Nov 23, 2012
1 parent 2d7c5ee commit 5fe65fd
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 52 deletions.
15 changes: 13 additions & 2 deletions lib/rsslib.php
Expand Up @@ -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 stdClass $instance the instance of the source of the RSS feed
* @param string $sql the SQL used to produce 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 * @return string the name of the RSS file
*/ */
function rss_get_file_name($instance, $sql) { function rss_get_file_name($instance, $sql, $params = array()) {
return $instance->id.'_'.md5($sql); 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);
}
} }


/** /**
Expand Down
89 changes: 39 additions & 50 deletions mod/forum/rsslib.php
Expand Up @@ -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 //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. // 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 $filename = rss_get_file_name($forum, $sql, $params);
// 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);
}
$cachedfilepath = rss_get_file_full_name('mod_forum', $filename); $cachedfilepath = rss_get_file_full_name('mod_forum', $filename);


//Is the cache out of date? //Is the cache out of date?
Expand All @@ -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 //if the cache is more than 60 seconds old and there's new stuff
$dontrecheckcutoff = time()-60; $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 //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)) { if (!empty($result)) {
$status = rss_save_file('mod_forum',$filename,$result); $status = rss_save_file('mod_forum',$filename,$result);
} }
Expand Down Expand Up @@ -111,10 +105,12 @@ function forum_rss_delete_file($forum) {
function forum_rss_newstuff($forum, $cm, $time) { function forum_rss_newstuff($forum, $cm, $time) {
global $DB; 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 false;
return ($recs && !empty($recs));
} }


/** /**
Expand All @@ -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 * @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) { function forum_rss_get_sql($forum, $cm, $time=0) {
$sql = null; if ($forum->rsstype == 1) { // Discussion RSS

return forum_rss_feed_discussions_sql($forum, $cm, $time);
if (!empty($forum->rsstype)) { } else { // Post RSS
if ($forum->rsstype == 1) { //Discussion RSS return forum_rss_feed_posts_sql($forum, $cm, $time);
$sql = forum_rss_feed_discussions_sql($forum, $cm, $time);
} else { //Post RSS
$sql = forum_rss_feed_posts_sql($forum, $cm, $time);
}
} }

return $sql;
} }


/** /**
Expand All @@ -155,7 +145,7 @@ function forum_rss_feed_discussions_sql($forum, $cm, $newsince=0) {
$modcontext = null; $modcontext = null;


$now = round(time(), -2); $now = round(time(), -2);
$params = array($cm->instance); $params = array();


$modcontext = context_module::instance($cm->id); $modcontext = context_module::instance($cm->id);


Expand All @@ -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) { if ($newsince) {
$newsince = " AND p.modified > '$newsince'"; $params['newsince'] = $newsince;
$newsince = " AND p.modified > :newsince";
} else { } else {
$newsince = ''; $newsince = '';
} }


//get group enforcing SQL // Get group enforcing SQL.
$groupmode = groups_get_activity_groupmode($cm); $groupmode = groups_get_activity_groupmode($cm);
$currentgroup = groups_get_activity_group($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) { // Add the groupparams to the params array.
$params['groupid'] = $currentgroup; $params = array_merge($params, $groupparams);
}


$forumsort = "d.timemodified DESC"; $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"; $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";
Expand All @@ -199,7 +189,7 @@ function forum_rss_feed_discussions_sql($forum, $cm, $newsince=0) {
WHERE d.forum = {$forum->id} AND p.parent = 0 WHERE d.forum = {$forum->id} AND p.parent = 0
$timelimit $groupselect $newsince $timelimit $groupselect $newsince
ORDER BY $forumsort"; ORDER BY $forumsort";
return $sql; return array($sql, $params);
} }


/** /**
Expand All @@ -213,19 +203,20 @@ function forum_rss_feed_discussions_sql($forum, $cm, $newsince=0) {
function forum_rss_feed_posts_sql($forum, $cm, $newsince=0) { function forum_rss_feed_posts_sql($forum, $cm, $newsince=0) {
$modcontext = context_module::instance($cm->id); $modcontext = context_module::instance($cm->id);


//get group enforcement SQL // Get group enforcement SQL.
$groupmode = groups_get_activity_groupmode($cm); $groupmode = groups_get_activity_groupmode($cm);
$currentgroup = groups_get_activity_group($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) { // Add the groupparams to the params array.
$params['groupid'] = $currentgroup; $params = array_merge($params, $groupparams);
}


//do we only want new posts? // Do we only want new posts?
if ($newsince) { if ($newsince) {
$newsince = " AND p.modified > '$newsince'"; $params['newsince'] = $newsince;
$newsince = " AND p.modified > :newsince";
} else { } else {
$newsince = ''; $newsince = '';
} }
Expand All @@ -250,7 +241,7 @@ function forum_rss_feed_posts_sql($forum, $cm, $newsince=0) {
$groupselect $groupselect
ORDER BY p.created desc"; ORDER BY p.created desc";


return $sql; return array($sql, $params);
} }


/** /**
Expand All @@ -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) { function forum_rss_get_group_sql($cm, $groupmode, $currentgroup, $modcontext=null) {
$groupselect = ''; $groupselect = '';
$params = array();


if ($groupmode) { if ($groupmode) {
if ($groupmode == VISIBLEGROUPS or has_capability('moodle/site:accessallgroups', $modcontext)) { if ($groupmode == VISIBLEGROUPS or has_capability('moodle/site:accessallgroups', $modcontext)) {
Expand All @@ -272,7 +264,7 @@ function forum_rss_get_group_sql($cm, $groupmode, $currentgroup, $modcontext=nul
$params['groupid'] = $currentgroup; $params['groupid'] = $currentgroup;
} }
} else { } else {
//seprate groups without access all // Separate groups without access all.
if ($currentgroup) { if ($currentgroup) {
$groupselect = "AND (d.groupid = :groupid OR d.groupid = -1)"; $groupselect = "AND (d.groupid = :groupid OR d.groupid = -1)";
$params['groupid'] = $currentgroup; $params['groupid'] = $currentgroup;
Expand All @@ -282,29 +274,27 @@ function forum_rss_get_group_sql($cm, $groupmode, $currentgroup, $modcontext=nul
} }
} }


return $groupselect; return array($groupselect, $params);
} }


/** /**
* This function return the XML rss contents about the forum * This function return the XML rss contents about the forum
* It returns false if something is wrong * It returns false if something is wrong
* *
* @param stdClass $forum the forum object * @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 * @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 * @return bool|string false if the contents is empty, otherwise the contents of the feed is returned
* *
* @Todo MDL-31129 implement post attachment handling * @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; global $CFG, $DB, $USER;



$status = true; $status = true;


$params = array();
//$params['forumid'] = $forum->id;
$recs = $DB->get_recordset_sql($sql, $params, 0, $forum->rssarticles); $recs = $DB->get_recordset_sql($sql, $params, 0, $forum->rssarticles);


//set a flag. Are we displaying discussions or posts? //set a flag. Are we displaying discussions or posts?
Expand All @@ -316,7 +306,6 @@ function forum_rss_feed_contents($forum, $sql) {
if (!$cm = get_coursemodule_from_instance('forum', $forum->id, $forum->course)) { if (!$cm = get_coursemodule_from_instance('forum', $forum->id, $forum->course)) {
print_error('invalidcoursemodule'); print_error('invalidcoursemodule');
} }
$context = context_module::instance($cm->id);


$formatoptions = new stdClass(); $formatoptions = new stdClass();
$items = array(); $items = array();
Expand Down

0 comments on commit 5fe65fd

Please sign in to comment.