diff --git a/admin/tool/dataprivacy/classes/external.php b/admin/tool/dataprivacy/classes/external.php index 30d2e6dc0ac..1db296f2e26 100644 --- a/admin/tool/dataprivacy/classes/external.php +++ b/admin/tool/dataprivacy/classes/external.php @@ -107,7 +107,7 @@ class external extends external_api { $request = reset($requests); $datasubject = $request->get('userid'); - if ($datasubject !== $USER->id) { + if ($datasubject !== (int) $USER->id) { // The user is not the subject. Check that they can cancel this request. if (!api::can_create_data_request_for_user($datasubject)) { $forusercontext = \context_user::instance($datasubject); diff --git a/admin/tool/dataprivacy/tests/expired_contexts_test.php b/admin/tool/dataprivacy/tests/expired_contexts_test.php index 863cb827e56..5c22f8ef61a 100644 --- a/admin/tool/dataprivacy/tests/expired_contexts_test.php +++ b/admin/tool/dataprivacy/tests/expired_contexts_test.php @@ -713,7 +713,7 @@ class tool_dataprivacy_expired_contexts_testcase extends advanced_testcase { $unexpiredroles = $expiredrecord->get('unexpiredroles'); $this->assertCount(1, $unexpiredroles); - $this->assertContains($role->id, $unexpiredroles); + $this->assertContainsEquals($role->id, $unexpiredroles); } /** @@ -820,7 +820,7 @@ class tool_dataprivacy_expired_contexts_testcase extends advanced_testcase { // The teacher is not expired. $unexpiredroles = $expiredrecord->get('unexpiredroles'); $this->assertCount(1, $unexpiredroles); - $this->assertContains($role->id, $unexpiredroles); + $this->assertContainsEquals($role->id, $unexpiredroles); $this->assertTrue((bool) $expiredrecord->get('defaultexpired')); } diff --git a/competency/tests/api_test.php b/competency/tests/api_test.php index e003ed8cebe..8b3584da9aa 100644 --- a/competency/tests/api_test.php +++ b/competency/tests/api_test.php @@ -185,7 +185,7 @@ class core_competency_api_testcase extends advanced_testcase { // Trying to change the context. $this->expectException(coding_exception::class); - api::update_template((object) array('id' => $template->get('id'), 'contextid' => context_coursecat::instance($cat->id))); + api::update_template((object) ['id' => $template->get('id'), 'contextid' => context_coursecat::instance($cat->id)->id]); } /** diff --git a/lib/classes/persistent.php b/lib/classes/persistent.php index 16095d6bed3..e7b09a2f299 100644 --- a/lib/classes/persistent.php +++ b/lib/classes/persistent.php @@ -143,7 +143,18 @@ abstract class persistent { if (method_exists($this, $methodname)) { return $this->$methodname(); } - return $this->raw_get($property); + + $properties = static::properties_definition(); + // If property can be NULL and value is NULL it needs to return null. + if ($properties[$property]['null'] === NULL_ALLOWED && $this->raw_get($property) === null) { + return null; + } + // Deliberately cast boolean types as such, because clean_param will cast them to integer. + if ($properties[$property]['type'] === PARAM_BOOL) { + return (bool)$this->raw_get($property); + } + + return clean_param($this->raw_get($property), $properties[$property]['type']); } /** diff --git a/lib/tests/persistent_test.php b/lib/tests/persistent_test.php index dbddd3d03c4..2f62e195879 100644 --- a/lib/tests/persistent_test.php +++ b/lib/tests/persistent_test.php @@ -36,6 +36,7 @@ class core_persistent_testcase extends advanced_testcase { public function setUp(): void { $this->make_persistent_table(); + $this->make_second_persistent_table(); $this->resetAfterTest(); } @@ -69,6 +70,35 @@ class core_persistent_testcase extends advanced_testcase { $dbman->create_table($table); } + /** + * Make the second table for the persistent. + */ + protected function make_second_persistent_table() { + global $DB; + $dbman = $DB->get_manager(); + + $table = new xmldb_table(core_testable_second_persistent::TABLE); + $table->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null); + $table->add_field('int', XMLDB_TYPE_INTEGER, '10', null, null, null, null); + $table->add_field('intnull', XMLDB_TYPE_INTEGER, '10', null, null, null, null); + $table->add_field('float', XMLDB_TYPE_FLOAT, '10', null, null, null, null); + $table->add_field('text', XMLDB_TYPE_TEXT, null, null, null, null, null); + $table->add_field('raw', XMLDB_TYPE_CHAR, '100', null, null, null, null); + $table->add_field('booltrue', XMLDB_TYPE_INTEGER, '1', null, XMLDB_NOTNULL, null, '0'); + $table->add_field('boolfalse', XMLDB_TYPE_INTEGER, '1', null, XMLDB_NOTNULL, null, '0'); + $table->add_field('timecreated', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null); + $table->add_field('timemodified', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null); + $table->add_field('usermodified', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, '0'); + + $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id')); + + if ($dbman->table_exists($table)) { + $dbman->drop_table($table); + } + + $dbman->create_table($table); + } + public function test_properties_definition() { $expected = array( 'shortname' => array( @@ -442,6 +472,39 @@ class core_persistent_testcase extends advanced_testcase { $this->expectExceptionMessageMatches('/The alias .+ exceeds 30 characters/'); core_testable_persistent::get_sql_fields('c'); } + + public function test_get(): void { + $data = [ + 'int' => 123, + 'intnull' => null, + 'float' => 33.44, + 'text' => 'Hello', + 'raw' => '/dev/hello', + 'booltrue' => true, + 'boolfalse' => false, + ]; + $p = new core_testable_second_persistent(0, (object)$data); + $p->create(); + + $this->assertSame($data['intnull'], $p->get('intnull')); + $this->assertSame($data['int'], $p->get('int')); + $this->assertSame($data['float'], $p->get('float')); + $this->assertSame($data['text'], $p->get('text')); + $this->assertSame($data['raw'], $p->get('raw')); + $this->assertSame($data['booltrue'], $p->get('booltrue')); + $this->assertSame($data['boolfalse'], $p->get('boolfalse')); + + // Ensure that types are correct after reloading data from database. + $p->read(); + + $this->assertSame($data['int'], $p->get('int')); + $this->assertSame($data['intnull'], $p->get('intnull')); + $this->assertSame($data['float'], $p->get('float')); + $this->assertSame($data['text'], $p->get('text')); + $this->assertSame($data['raw'], $p->get('raw')); + $this->assertSame($data['booltrue'], $p->get('booltrue')); + $this->assertSame($data['boolfalse'], $p->get('boolfalse')); + } } /** @@ -544,3 +607,51 @@ class core_testable_persistent extends \core\persistent { } } + +/** + * Example persistent class to test types. + * + * @package core + * @copyright 2021 David Matamoros + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class core_testable_second_persistent extends \core\persistent { + + /** Table name for the persistent. */ + const TABLE = 'phpunit_second_persistent'; + + /** + * Return the list of properties. + * + * @return array + */ + protected static function define_properties(): array { + return [ + 'int' => [ + 'type' => PARAM_INT, + ], + 'intnull' => [ + 'type' => PARAM_INT, + 'null' => NULL_ALLOWED, + 'default' => null, + ], + 'float' => [ + 'type' => PARAM_FLOAT, + ], + 'text' => [ + 'type' => PARAM_TEXT, + 'default' => '' + ], + 'raw' => [ + 'type' => PARAM_RAW, + 'default' => '' + ], + 'booltrue' => [ + 'type' => PARAM_BOOL, + ], + 'boolfalse' => [ + 'type' => PARAM_BOOL, + ] + ]; + } +} diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 1e558667196..7f89044137f 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -91,10 +91,11 @@ information provided here is intended especially for developers. * As the message_jabber notification plugin has been moved to the plugins database, the XMPPHP library (aka Jabber) has been completely removed from Moodle core too. * The SWF media player has been completely removed (The Flash Player was deprecated in 2017 and officially discontinued -on 31 December 2020). + on 31 December 2020). * The display_size function has been improved to add new optional parameters (decimal places, fixed units), to always include a non-breaking space between the number and unit, and to use consistent rounding (always 1 decimal place by default). +* The persistent method get() now returns the correct type for each property defined in the persistent class. === 3.11.2 === * For security reasons, filelib has been updated so all requests now use emulated redirects. diff --git a/reportbuilder/classes/local/models/report.php b/reportbuilder/classes/local/models/report.php index 0c874e3bcfa..c0c03d31d06 100644 --- a/reportbuilder/classes/local/models/report.php +++ b/reportbuilder/classes/local/models/report.php @@ -81,15 +81,6 @@ class report extends persistent { ]; } - /** - * Return report ID - * - * @return int - */ - protected function get_id(): int { - return (int) $this->raw_get('id'); - } - /** * Return report context, used by exporters *