diff --git a/reportbuilder/classes/local/helpers/database.php b/reportbuilder/classes/local/helpers/database.php index a19694c12b7..f1753f1b62b 100644 --- a/reportbuilder/classes/local/helpers/database.php +++ b/reportbuilder/classes/local/helpers/database.php @@ -106,7 +106,7 @@ class database { * primarily to ensure uniqueness when the expression is to be used as part of a larger query * * @param string $sql - * @param array $params + * @param array $params Parameter names * @param callable $callback Method that takes a single string parameter, and returns another string * @return string */ @@ -124,6 +124,27 @@ class database { return $sql; } + /** + * Replace parameter names within given SQL expression, returning updated SQL and parameter elements + * + * {@see sql_replace_parameter_names} + * + * @param string $sql + * @param array $params Parameter name/values + * @param callable $callback + * @return array [$sql, $params] + */ + public static function sql_replace_parameters(string $sql, array $params, callable $callback): array { + $transformedsql = static::sql_replace_parameter_names($sql, array_keys($params), $callback); + + $transformedparams = []; + foreach ($params as $name => $value) { + $transformedparams[$callback($name)] = $value; + } + + return [$transformedsql, $transformedparams]; + } + /** * Generate SQL expression for sorting group concatenated fields * diff --git a/reportbuilder/classes/table/base_report_table.php b/reportbuilder/classes/table/base_report_table.php index 97e53f988d7..d33f8167bf1 100644 --- a/reportbuilder/classes/table/base_report_table.php +++ b/reportbuilder/classes/table/base_report_table.php @@ -74,10 +74,13 @@ abstract class base_report_table extends table_sql implements dynamic, renderabl $wheres[] = $where; } + // Track the index of conditions/filters as we iterate over them. + $conditionindex = $filterindex = 0; + // For each condition, we need to ensure their values are always accounted for in the report. $conditionvalues = $this->report->get_condition_values(); foreach ($this->report->get_active_conditions() as $condition) { - [$conditionsql, $conditionparams] = $this->get_filter_sql($condition, $conditionvalues); + [$conditionsql, $conditionparams] = $this->get_filter_sql($condition, $conditionvalues, 'c' . $conditionindex++); if ($conditionsql !== '') { $joins = array_merge($joins, $condition->get_joins()); $wheres[] = "({$conditionsql})"; @@ -89,7 +92,7 @@ abstract class base_report_table extends table_sql implements dynamic, renderabl if (!$this->editing) { $filtervalues = $this->report->get_filter_values(); foreach ($this->report->get_active_filters() as $filter) { - [$filtersql, $filterparams] = $this->get_filter_sql($filter, $filtervalues); + [$filtersql, $filterparams] = $this->get_filter_sql($filter, $filtervalues, 'f' . $filterindex++); if ($filtersql !== '') { $joins = array_merge($joins, $filter->get_joins()); $wheres[] = "({$filtersql})"; @@ -139,13 +142,24 @@ abstract class base_report_table extends table_sql implements dynamic, renderabl * * @param filter $filter * @param array $filtervalues + * @param string $paramprefix * @return array [$sql, $params] */ - private function get_filter_sql(filter $filter, array $filtervalues): array { + private function get_filter_sql(filter $filter, array $filtervalues, string $paramprefix): array { /** @var base $filterclass */ $filterclass = $filter->get_filter_class(); - return $filterclass::create($filter)->get_sql_filter($filtervalues); + // Retrieve SQL fragments from the filter instance, process parameters if required. + [$sql, $params] = $filterclass::create($filter)->get_sql_filter($filtervalues); + if ($paramprefix !== '' && count($params) > 0) { + return database::sql_replace_parameters( + $sql, + $params, + fn(string $param) => "{$paramprefix}_{$param}", + ); + } + + return [$sql, $params]; } /** diff --git a/reportbuilder/tests/local/helpers/database_test.php b/reportbuilder/tests/local/helpers/database_test.php index 22df8614a49..3ceccea50c5 100644 --- a/reportbuilder/tests/local/helpers/database_test.php +++ b/reportbuilder/tests/local/helpers/database_test.php @@ -30,7 +30,7 @@ use core_user; * @copyright 2020 Paul Holden * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class database_test extends advanced_testcase { +final class database_test extends advanced_testcase { /** * Test generating alias @@ -150,9 +150,11 @@ class database_test extends advanced_testcase { [$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}"; - }); + $sql = database::sql_replace_parameter_names( + $sql, + [$param0, $param1, $param10], + fn(string $param) => "prefix_{$param}", + ); $record = $DB->get_record_sql($sql, [ "prefix_{$param0}" => 'Zero', @@ -166,4 +168,29 @@ class database_test extends advanced_testcase { 'field10' => 'Ten', ], $record); } + + /** + * Test replacement of parameter names within query, returning both modified query and parameters + */ + public function test_sql_replace_parameters(): 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, $params] = database::sql_replace_parameters( + $sql, + [$param0 => 'Zero', $param1 => 'One', $param10 => 'Ten'], + fn(string $param) => "prefix_{$param}", + ); + + $record = $DB->get_record_sql($sql, $params); + + $this->assertEquals((object) [ + 'field0' => 'Zero', + 'field1' => 'One', + 'field10' => 'Ten', + ], $record); + } } diff --git a/reportbuilder/upgrade.txt b/reportbuilder/upgrade.txt index ba73dbc3963..b3f1163559e 100644 --- a/reportbuilder/upgrade.txt +++ b/reportbuilder/upgrade.txt @@ -1,6 +1,10 @@ This file describes API changes in /reportbuilder/* Information provided here is intended especially for developers. +=== 4.3.5 === + +* New database helper method `sql_replace_parameters` to help ensure uniqueness of parameters within a SQL expression + === 4.3.4 === * The `get_name` method has been moved to the base report class and can now be implemented for both custom and system reports, it