Merge branch 'MDL-72329-master' of https://github.com/dravek/moodle

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);

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'));
     }
 

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]);
     }
 
     /**

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']);
     }
 
     /**

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 <davidmc@moodle.com>
+ * @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,
+            ]
+        ];
+    }
+}

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.

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
      *