Skip to content

Commit

Permalink
MDL-79397 reportbuilder: always auto-generate entity table aliases.
Browse files Browse the repository at this point in the history
As the number of report entity classes has grown, each having their own
manually defined table aliases, it becomes harder to ensure each of the
aliases are always unique across report sources.

We can remove that burden by ensuring table aliases are automatically
generated upon request.
  • Loading branch information
paulholden committed Dec 7, 2023
1 parent a891866 commit fb53d08
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 21 deletions.
66 changes: 49 additions & 17 deletions reportbuilder/classes/local/entities/base.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@
namespace core_reportbuilder\local\entities;

use coding_exception;
use lang_string;
use core_reportbuilder\local\helpers\database;
use core_reportbuilder\local\report\column;
use core_reportbuilder\local\report\filter;
use lang_string;

/**
* Base class for all report entities
Expand Down Expand Up @@ -57,14 +58,41 @@ abstract class base {
private $conditions = [];

/**
* Database tables that this entity uses and their default aliases
* Database tables that this entity uses
*
* Must be overridden by the entity to list all database tables that it expects to be present in the main
* SQL or in JOINs added to this entity
*
* @todo in Moodle 4.8 - make abstract when support for {@see get_default_table_aliases} is finally removed
*
* @return string[]
*/
protected function get_default_tables(): array {
static $debuggingshown;

// The default implementation falls back to retrieving deprecated table aliases to determine our table names.
$tablenamealiases = $this->get_default_table_aliases();
if (!empty($tablenamealiases) && !$debuggingshown) {
debugging('The function get_default_table_aliases() is deprecated, please define the entity' .
' tables with get_default_tables() in ' . static::class, DEBUG_DEVELOPER);

// Don't be too spammy with the debugging, this method is called multiple times per entity load.
$debuggingshown = true;
}

return array_keys($tablenamealiases);
}

/**
* Database tables that this entity uses and their default aliases (note that these aliases are now ignored)
*
* @return string[] Array of $tablename => $alias
*
* @deprecated since Moodle 4.4 - aliases are now autogenerated, please implement {@see get_default_tables} instead
*/
abstract protected function get_default_table_aliases(): array;
protected function get_default_table_aliases(): array {
return [];
}

/**
* The default title for this entity
Expand Down Expand Up @@ -137,16 +165,17 @@ final public function get_entity_title(): lang_string {
}

/**
* Override the default alias for given database table used in entity queries, to avoid table alias clashes that may occur
* if multiple entities of a report each define the same default alias for one of their tables
* Override the default alias for given database table used in entity queries, for instance when the same table is used
* by multiple entities and you want them each to refer to it by the same alias
*
* @param string $tablename
* @param string $tablename One of the tables set by {@see get_default_tables}
* @param string $alias
* @return self
* @throws coding_exception
* @throws coding_exception For invalid table name
*/
final public function set_table_alias(string $tablename, string $alias): self {
if (!array_key_exists($tablename, $this->get_default_table_aliases())) {
$tablenames = $this->get_default_tables();
if (array_search($tablename, $tablenames) === false) {
throw new coding_exception('Invalid table name', $tablename);
}

Expand All @@ -155,9 +184,7 @@ final public function set_table_alias(string $tablename, string $alias): self {
}

/**
* Override multiple default database table aliases used in entity queries as per {@see set_table_alias}, typically when
* you're adding an entity multiple times to a report you'd want to override the table aliases in the second instance to
* avoid clashes with the first
* Override multiple default database table aliases used in entity queries as per {@see set_table_alias}
*
* @param array $aliases Array of tablename => alias values
* @return self
Expand All @@ -172,17 +199,22 @@ final public function set_table_aliases(array $aliases): self {
/**
* Returns an alias used in the queries for a given table
*
* @param string $tablename
* @param string $tablename One of the tables set by {@see get_default_tables}
* @return string
* @throws coding_exception
* @throws coding_exception For invalid table name
*/
final public function get_table_alias(string $tablename): string {
$defaulttablealiases = $this->get_default_table_aliases();
if (!array_key_exists($tablename, $defaulttablealiases)) {
$tablenames = $this->get_default_tables();
if (array_search($tablename, $tablenames) === false) {
throw new coding_exception('Invalid table name', $tablename);
}

return $this->tablealiases[$tablename] ?? $defaulttablealiases[$tablename];
// We don't have the alias yet, generate a new one.
if (!array_key_exists($tablename, $this->tablealiases)) {
$this->set_table_alias($tablename, database::generate_alias());
}

return $this->tablealiases[$tablename];
}

/**
Expand Down Expand Up @@ -246,7 +278,7 @@ final public function get_joins(): array {
/**
* Helper method for returning joins necessary for retrieving tags related to the current entity
*
* Both 'tag' and 'tag_instance' aliases must be returned by the entity {@see get_default_table_aliases} method
* Both 'tag' and 'tag_instance' aliases must be returned by the entity {@see get_default_tables} method
*
* @param string $component
* @param string $itemtype
Expand Down
20 changes: 16 additions & 4 deletions reportbuilder/tests/local/entities/base_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,19 @@ class base_test extends advanced_testcase {
*/
public function test_get_table_alias(): void {
$entity = new base_test_entity();
$this->assertEquals('m', $entity->get_table_alias('mytable'));

$mytablealias = $entity->get_table_alias('mytable');
$this->assertMatchesRegularExpression('/^rbalias(\d+)$/', $mytablealias);

$myothertablealias = $entity->get_table_alias('myothertable');
$this->assertMatchesRegularExpression('/^rbalias(\d+)$/', $myothertablealias);

// They must differ.
$this->assertNotEquals($mytablealias, $myothertablealias);

// Re-request both, ensure they are identical to what we previously received.
$this->assertEquals($mytablealias, $entity->get_table_alias('mytable'));
$this->assertEquals($myothertablealias, $entity->get_table_alias('myothertable'));
}

/**
Expand Down Expand Up @@ -314,10 +326,10 @@ class base_test_entity extends base {
*
* @return array
*/
protected function get_default_table_aliases(): array {
protected function get_default_tables(): array {
return [
'mytable' => 'm',
'myothertable' => 'o',
'mytable',
'myothertable',
];
}

Expand Down
2 changes: 2 additions & 0 deletions reportbuilder/upgrade.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ Information provided here is intended especially for developers.
=== 4.4 ===

* New methods `get_identity_[columns|filters]` in user entity, for retrieving all user identity field report elements
* Entity table aliases are now auto-generated, hence usage of the `get_default_table_aliases` method is now deprecated. Instead,
entities should implement the `get_default_tables` method to define the tables they use
* The database helper `generate_alias[es]` and `generate_param_name[s]` methods now accept an optional `$suffix` argument for
appending additional string to the generated value
* New report filter types:
Expand Down

0 comments on commit fb53d08

Please sign in to comment.