L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'MDL-77555-400' of https://github.com/paulholden/moodle into MOODLE_400_STABLE

Browse Source
This commit is contained in:
Shamim Rezaie
2023-04-05 20:15:09 +10:00
parent 60ecd6cef5 551edbb9a6
commit df17b180f2
10 changed files with 149 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
reportbuilder/classes/local/filters/base.php
+3
View File
@@ -102,6 +102,9 @@ abstract class base {
/**
* Returns the filter clauses to be used with SQL where
*
* Ideally the field SQL should be included only once in the returned expression, however if that is unavoidable then
* use the {@see filter::get_field_sql_and_params} helper to ensure uniqueness of any parameters included within
*
* @param array $values
* @return array [$sql, [...$params]]
*/
reportbuilder/classes/local/filters/date.php
+17 -15
View File
@@ -164,30 +164,32 @@ class date extends base {
switch ($operator) {
case self::DATE_NOT_EMPTY:
$sql = "{$fieldsql} IS NOT NULL AND {$fieldsql} <> 0";
$sql = "COALESCE({$fieldsql}, 0) <> 0";
break;
case self::DATE_EMPTY:
$sql = "{$fieldsql} IS NULL OR {$fieldsql} = 0";
$sql = "COALESCE({$fieldsql}, 0) = 0";
break;
case self::DATE_RANGE:
$clauses = [];
$sql = '';
$datefrom = (int)($values["{$this->name}_from"] ?? 0);
if ($datefrom > 0) {
$paramdatefrom = database::generate_param_name();
$clauses[] = "{$fieldsql} >= :{$paramdatefrom}";
$params[$paramdatefrom] = $datefrom;
}
$dateto = (int)($values["{$this->name}_to"] ?? 0);
if ($dateto > 0) {
$paramdateto = database::generate_param_name();
$clauses[] = "{$fieldsql} < :{$paramdateto}";
$paramdatefrom = database::generate_param_name();
$paramdateto = database::generate_param_name();
if ($datefrom > 0 && $dateto > 0) {
$sql = "{$fieldsql} BETWEEN :{$paramdatefrom} AND :{$paramdateto}";
$params[$paramdatefrom] = $datefrom;
$params[$paramdateto] = $dateto;
} else if ($datefrom > 0) {
$sql = "{$fieldsql} >= :{$paramdatefrom}";
$params[$paramdatefrom] = $datefrom;
} else if ($dateto > 0) {
$sql = "{$fieldsql} < :{$paramdateto}";
$params[$paramdateto] = $dateto;
}
$sql = implode(' AND ', $clauses);
break;
// Relative helper method can handle these three cases.
case self::DATE_LAST:
@@ -202,7 +204,7 @@ class date extends base {
$paramdatefrom = database::generate_param_name();
$paramdateto = database::generate_param_name();
$sql = "{$fieldsql} >= :{$paramdatefrom} AND {$fieldsql} <= :{$paramdateto}";
$sql = "{$fieldsql} BETWEEN :{$paramdatefrom} AND :{$paramdateto}";
[
$params[$paramdatefrom],
$params[$paramdateto],
reportbuilder/classes/local/filters/number.php
+3 -3
View File
@@ -132,10 +132,10 @@ class number extends base {
case self::ANY_VALUE:
return ['', []];
case self::IS_NOT_EMPTY:
$res = "{$fieldsql} IS NOT NULL AND {$fieldsql} <> 0";
$res = "COALESCE({$fieldsql}, 0) <> 0";
break;
case self::IS_EMPTY:
$res = "{$fieldsql} IS NULL OR {$fieldsql} = 0";
$res = "COALESCE({$fieldsql}, 0) = 0";
break;
case self::LESS_THAN:
$res = "{$fieldsql} < :{$param}";
@@ -158,7 +158,7 @@ class number extends base {
$params[$param] = $value1;
break;
case self::RANGE:
$res = "({$fieldsql} >= :{$param} AND {$fieldsql} <= :{$param2})";
$res = "{$fieldsql} BETWEEN :{$param} AND :{$param2}";
$params[$param] = $value1;
$params[$param2] = $value2;
break;
reportbuilder/classes/local/helpers/database.php
+23
View File
@@ -76,6 +76,29 @@ class database {
return true;
}
/**
* Replace parameter names within given SQL expression, allowing caller to specify callback to handle their replacement
* primarily to ensure uniqueness when the expression is to be used as part of a larger query
*
* @param string $sql
* @param array $params
* @param callable $callback Method that takes a single string parameter, and returns another string
* @return string
*/
public static function sql_replace_parameter_names(string $sql, array $params, callable $callback): string {
foreach ($params as $param) {
// Pattern to look for param within the SQL.
$pattern = '/:(?<param>' . preg_quote($param) . ')\b/';
$sql = preg_replace_callback($pattern, function(array $matches) use ($callback): string {
return ':' . $callback($matches['param']);
}, $sql);
}
return $sql;
}
/**
* Generate SQL expression for sorting group concatenated fields
*
reportbuilder/classes/local/report/column.php
+6 -6
View File
@@ -367,12 +367,12 @@ final class column {
$fields = [];
foreach ($this->fields as $alias => $sql) {
// Ensure params within SQL are prefixed with column index.
foreach ($this->params as $name => $value) {
$sql = preg_replace_callback('/:(?<param>' . preg_quote($name, '\b/') . ')/', function(array $matches): string {
return ':' . $this->unique_param_name($matches['param']);
}, $sql);
}
// Ensure parameter names within SQL are prefixed with column index.
$params = array_keys($this->params);
$sql = database::sql_replace_parameter_names($sql, $params, function(string $param): string {
return $this->unique_param_name($param);
});
$fields[$alias] = [
'sql' => $sql,
reportbuilder/classes/local/report/filter.php
+33
View File
@@ -21,6 +21,7 @@ namespace core_reportbuilder\local\report;
use lang_string;
use moodle_exception;
use core_reportbuilder\local\filters\base;
use core_reportbuilder\local\helpers\database;
use core_reportbuilder\local\models\filter as filter_model;
/**
@@ -212,6 +213,38 @@ final class filter {
return $this->fieldparams;
}
/**
* Retrieve SQL expression and parameters for the field
*
* @param int $index
* @return array [$sql, [...$params]]
*/
public function get_field_sql_and_params(int $index = 0): array {
$fieldsql = $this->get_field_sql();
$fieldparams = $this->get_field_params();
// Shortcut if there aren't any parameters.
if (empty($fieldparams)) {
return [$fieldsql, $fieldparams];
}
// Simple callback for replacement of parameter names within filter SQL.
$transform = function(string $param) use ($index): string {
return "{$param}_{$index}";
};
$paramnames = array_keys($fieldparams);
$sql = database::sql_replace_parameter_names($fieldsql, $paramnames, $transform);
$params = [];
foreach ($paramnames as $paramname) {
$paramnametransform = $transform($paramname);
$params[$paramnametransform] = $fieldparams[$paramname];
}
return [$sql, $params];
}
/**
* Set the SQL expression for the field that is being filtered. It will be passed to the filter class
*
reportbuilder/tests/local/helpers/database_test.php
+27
View File
@@ -78,4 +78,31 @@ class database_test extends advanced_testcase {
$this->expectExceptionMessage('Invalid parameter names (invalidfoo, invalidbar)');
database::validate_params($params);
}
/**
* Test replacement of parameter names within SQL statements
*/
public function test_sql_replace_parameter_names(): void {
global $DB;
// Predefine parameter names, to ensure they don't overwrite each other.
[$param0, $param1, $param10] = ['rbparam0', 'rbparam1', 'rbparam10'];
$sql = "SELECT :{$param0} AS field0, :{$param1} AS field1, :{$param10} AS field10" . $DB->sql_null_from_clause();
$sql = database::sql_replace_parameter_names($sql, [$param0, $param1, $param10], static function(string $param): string {
return "prefix_{$param}";
});
$record = $DB->get_record_sql($sql, [
"prefix_{$param0}" => 'Zero',
"prefix_{$param1}" => 'One',
"prefix_{$param10}" => 'Ten',
]);
$this->assertEquals((object) [
'field0' => 'Zero',
'field1' => 'One',
'field10' => 'Ten',
], $record);
}
}
reportbuilder/tests/local/report/column_test.php
+18 -5
View File
@@ -173,19 +173,32 @@ class column_test extends advanced_testcase {
* Test adding params to field, and retrieving them
*/
public function test_add_field_with_params(): void {
$param = database::generate_param_name();
$param0 = database::generate_param_name();
$param1 = database::generate_param_name();
$column = $this->create_column('test')
->set_index(1)
->add_field(":{$param}", 'foo', [$param => 'bar']);
->add_field(":{$param0}", 'foo', [$param0 => 'foo'])
->add_field(":{$param1}", 'bar', [$param1 => 'bar']);
// Select will look like the following: "p<index>_rbparam<counter>", where index is the column index and counter is
// a static value of the report helper class.
$select = $column->get_fields();
preg_match('/:(?<paramname>p1_rbparam[\d]+) AS c1_foo/', $select[0], $matches);
$fields = $column->get_fields();
$this->assertCount(2, $fields);
preg_match('/:(?<paramname>p1_rbparam[\d]+) AS c1_foo/', $fields[0], $matches);
$this->assertArrayHasKey('paramname', $matches);
$this->assertEquals([$matches['paramname'] => 'bar'], $column->get_params());
$fieldparam0 = $matches['paramname'];
preg_match('/:(?<paramname>p1_rbparam[\d]+) AS c1_bar/', $fields[1], $matches);
$this->assertArrayHasKey('paramname', $matches);
$fieldparam1 = $matches['paramname'];
// Ensure column parameters have been renamed appropriately.
$this->assertEquals([
$fieldparam0 => 'foo',
$fieldparam1 => 'bar',
], $column->get_params());
}
/**
reportbuilder/tests/local/report/filter_test.php
+12
View File
@@ -108,6 +108,18 @@ class filter_test extends advanced_testcase {
$this->assertEquals(['foo' => 'bar'], $filter->get_field_params());
}
/**
* Test getting field SQL and params, while providing index for uniqueness
*/
public function test_get_field_sql_and_params(): void {
$filter = $this->create_filter('username', 'u.username = :username AND u.idnumber = :idnumber',
['username' => 'test', 'idnumber' => 'bar']);
[$sql, $params] = $filter->get_field_sql_and_params(1);
$this->assertEquals('u.username = :username_1 AND u.idnumber = :idnumber_1', $sql);
$this->assertEquals(['username_1' => 'test', 'idnumber_1' => 'bar'], $params);
}
/**
* Test adding single join
*/
reportbuilder/upgrade.txt
+7
View File
@@ -1,6 +1,13 @@
This file describes API changes in /reportbuilder/*
Information provided here is intended especially for developers.
=== 4.0.8 ===
* New database helper method `sql_replace_parameter_names` to help ensure uniqueness of parameters within an expression (where
that expression can be used multiple times as part of a larger query)
* The local report filter class has a new `get_field_sql_and_params` method which should be used by filter types that re-use
the filter field SQL within their generated expression, to ensure SQL containing parameters works correctly
=== 4.0.7 ===
* The schedule helper `create_schedule` method accepts a `$timenow` parameter to use for comparisons against current date