From 6d5c0190e2d09986ffc35bde659ae7423e7fbfb9 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Mon, 14 Mar 2022 09:41:04 +0000 Subject: [PATCH] MDL-74192 core: add strictness to persistent get_record method. Allow developer to specify the given record MUST_EXIST, and the method to throw the underlying database exception. --- lib/classes/persistent.php | 6 ++- lib/tests/persistent_test.php | 72 +++++++++++++++++++++++++++++++++-- lib/upgrade.txt | 3 ++ 3 files changed, 75 insertions(+), 6 deletions(-) diff --git a/lib/classes/persistent.php b/lib/classes/persistent.php index de9419c72ae..96402a6fee4 100644 --- a/lib/classes/persistent.php +++ b/lib/classes/persistent.php @@ -854,12 +854,14 @@ abstract class persistent { * Load a single record. * * @param array $filters Filters to apply. + * @param int $strictness Similar to the internal DB get_record call, indicate whether a missing record should be + * ignored/return false ({@see IGNORE_MISSING}) or should cause an exception to be thrown ({@see MUST_EXIST}) * @return false|static */ - public static function get_record($filters = array()) { + public static function get_record(array $filters = [], int $strictness = IGNORE_MISSING) { global $DB; - $record = $DB->get_record(static::TABLE, $filters); + $record = $DB->get_record(static::TABLE, $filters, '*', $strictness); return $record ? new static(0, $record) : false; } diff --git a/lib/tests/persistent_test.php b/lib/tests/persistent_test.php index 4b61e134225..151db755130 100644 --- a/lib/tests/persistent_test.php +++ b/lib/tests/persistent_test.php @@ -22,8 +22,15 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ +namespace core; + +use advanced_testcase; +use coding_exception; +use dml_missing_record_exception; +use lang_string; +use xmldb_table; + defined('MOODLE_INTERNAL') || die(); -global $CFG; /** * Persistent testcase. @@ -31,8 +38,9 @@ global $CFG; * @package core * @copyright 2015 Frédéric Massart - FMCorz.net * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \core\persistent */ -class core_persistent_testcase extends advanced_testcase { +class persistent_test extends advanced_testcase { public function setUp(): void { $this->make_persistent_table(); @@ -165,6 +173,29 @@ class core_persistent_testcase extends advanced_testcase { $this->assertEquals($expected, core_testable_persistent::properties_definition()); } + /** + * Test creating persistent instance by specifying record ID in constructor + */ + public function test_constructor() : void { + $persistent = (new core_testable_persistent(0, (object) [ + 'idnumber' => '123', + 'sortorder' => 1, + ]))->create(); + + // Now create a new instance, passing the original instance ID in the constructor. + $another = new core_testable_persistent($persistent->get('id')); + $this->assertEquals($another->to_record(), $persistent->to_record()); + } + + /** + * Test creating persistent instance by specifying non-existing record ID in constructor throws appropriate exception + */ + public function test_constructor_invalid(): void { + $this->expectException(dml_missing_record_exception::class); + $this->expectExceptionMessage('Can\'t find data record in database table phpunit_persistent.'); + new core_testable_persistent(42); + } + public function test_to_record() { $p = new core_testable_persistent(); $expected = (object) array( @@ -547,6 +578,39 @@ class core_persistent_testcase extends advanced_testcase { $this->assertEquals($json, $record->path); } + /** + * Test get_record method for creating persistent instance + */ + public function test_get_record(): void { + $persistent = (new core_testable_persistent(0, (object) [ + 'idnumber' => '123', + 'sortorder' => 1, + ]))->create(); + + $another = core_testable_persistent::get_record(['id' => $persistent->get('id')]); + + // Assert we got back a persistent instance, and it matches original. + $this->assertInstanceOf(core_testable_persistent::class, $another); + $this->assertEquals($another->to_record(), $persistent->to_record()); + } + + /** + * Test get_record method for creating persistent instance, ignoring a non-existing record + */ + public function test_get_record_ignore_missing(): void { + $persistent = core_testable_persistent::get_record(['id' => 42]); + $this->assertFalse($persistent); + } + + /** + * Test get_record method for creating persistent instance, throws appropriate exception for non-existing record + */ + public function test_get_record_must_exist(): void { + $this->expectException(dml_missing_record_exception::class); + $this->expectExceptionMessage('Can\'t find data record in database table phpunit_persistent.'); + core_testable_persistent::get_record(['id' => 42], MUST_EXIST); + } + public function test_record_exists() { global $DB; $this->assertFalse($DB->record_exists(core_testable_persistent::TABLE, array('idnumber' => 'abc'))); @@ -625,7 +689,7 @@ class core_persistent_testcase extends advanced_testcase { * @copyright 2015 Frédéric Massart - FMCorz.net * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class core_testable_persistent extends \core\persistent { +class core_testable_persistent extends persistent { const TABLE = 'phpunit_persistent'; @@ -726,7 +790,7 @@ class core_testable_persistent extends \core\persistent { * @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 { +class core_testable_second_persistent extends persistent { /** Table name for the persistent. */ const TABLE = 'phpunit_second_persistent'; diff --git a/lib/upgrade.txt b/lib/upgrade.txt index ffaaf8cc7b2..1d74f4c9477 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -1,6 +1,9 @@ This files describes API changes in core libraries and APIs, information provided here is intended especially for developers. +=== 4.1 === +* Added $strictness parameter to persistent `get_record` method, optionally allowing caller to ensure record exists + === 4.0 === * To better detect wrong floats (like, for example, unformatted, using local-dependent separators ones) a number of