Merge branch 'MDL-53481-31-uniqsqlparam' of git://github.com/mudrd8mz/moodle into MOODLE_31_STABLE

This commit is contained in:
David Monllao
2016-11-09 12:50:07 +08:00
2 changed files with 64 additions and 2 deletions
+7 -2
View File
@@ -34,6 +34,10 @@ defined('MOODLE_INTERNAL') || die();
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
abstract class tree_node {
/** @var int Counter to be used in {@link tree_node::unique_sql_parameter()}. */
protected static $uniquesqlparametercounter = 1;
/**
* Determines whether this particular item is currently available
* according to the availability criteria.
@@ -242,10 +246,11 @@ abstract class tree_node {
* @return SQL code for the parameter, e.g. ':pr1234'
*/
protected static function unique_sql_parameter(array &$params, $value) {
static $count = 1;
// Note we intentionally do not use self:: here.
$count = tree_node::$uniquesqlparametercounter++;
$unique = 'usp' . $count;
$params[$unique] = $value;
$count++;
return ':' . $unique;
}
}
+57
View File
@@ -675,6 +675,63 @@ class tree_testcase extends \advanced_testcase {
json_encode(tree::get_root_json(array($child, $child), tree::OP_AND, array(true, false))));
}
/**
* Tests the behaviour of the counter in unique_sql_parameter().
*
* There was a problem with static counters used to implement a sequence of
* parameter placeholders (MDL-53481). As always with static variables, it
* is a bit tricky to unit test the behaviour reliably as it depends on the
* actual tests executed and also their order.
*
* To minimise risk of false expected behaviour, this test method should be
* first one where {@link core_availability\tree::get_user_list_sql()} is
* used. We also use higher number of condition instances to increase the
* risk of the counter collision, should there remain a problem.
*/
public function test_unique_sql_parameter_behaviour() {
global $DB;
$this->resetAfterTest();
$generator = $this->getDataGenerator();
// Create a test course with multiple groupings and groups and a student in each of them.
$course = $generator->create_course();
$user = $generator->create_user();
$studentroleid = $DB->get_field('role', 'id', array('shortname' => 'student'));
$generator->enrol_user($user->id, $course->id, $studentroleid);
// The total number of groupings and groups must not be greater than 61.
// There is a limit in MySQL on the max number of joined tables.
$groups = [];
for ($i = 0; $i < 25; $i++) {
$group = $generator->create_group(array('courseid' => $course->id));
groups_add_member($group, $user);
$groups[] = $group;
}
$groupings = [];
for ($i = 0; $i < 25; $i++) {
$groupings[] = $generator->create_grouping(array('courseid' => $course->id));
}
foreach ($groupings as $grouping) {
foreach ($groups as $group) {
groups_assign_grouping($grouping->id, $group->id);
}
}
$info = new \core_availability\mock_info($course);
// Make a huge tree with 'AND' of all groups and groupings conditions.
$conditions = [];
foreach ($groups as $group) {
$conditions[] = \availability_group\condition::get_json($group->id);
}
foreach ($groupings as $groupingid) {
$conditions[] = \availability_grouping\condition::get_json($grouping->id);
}
shuffle($conditions);
$tree = new tree(tree::get_root_json($conditions));
list($sql, $params) = $tree->get_user_list_sql(false, $info, false);
$result = $DB->get_fieldset_sql($sql, $params);
$this->assertEquals(array($user->id), $result);
}
/**
* Tests get_user_list_sql.
*/