From 68eaa3150eadece016c5504e0e8a7a885ffbf992 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Thu, 18 Oct 2018 15:31:55 +0800 Subject: [PATCH] MDL-63658 core_favourites: properly define interface methods and cleanup This gets rid of specific repo functions which were unused, and makes sure the following methods are defined on the interface, implemented and tested: - exists_by($criteria) - find_by($criteria) - delete_by($crtieria) Also, added missing tests for find_favourite() repo method. --- .../local/repository/favourite_repository.php | 173 ++++++++---------- .../favourite_repository_interface.php | 24 +++ favourites/tests/repository_test.php | 112 ++++++++++-- 3 files changed, 200 insertions(+), 109 deletions(-) diff --git a/favourites/classes/local/repository/favourite_repository.php b/favourites/classes/local/repository/favourite_repository.php index 2b94f91caec..d0236b7999d 100644 --- a/favourites/classes/local/repository/favourite_repository.php +++ b/favourites/classes/local/repository/favourite_repository.php @@ -74,6 +74,44 @@ class favourite_repository implements favourite_repository_interface { return $list; } + /** + * Basic validation, confirming we have the minimum field set needed to save a record to the store. + * + * @param favourite $favourite the favourite record to validate. + * @throws \moodle_exception if the supplied favourite has missing or unsupported fields. + */ + protected function validate(favourite $favourite) { + + $favourite = (array)$favourite; + + // The allowed fields, and whether or not each is required to create a record. + // The timecreated, timemodified and id fields are generated during create/update. + $allowedfields = [ + 'userid' => true, + 'component' => true, + 'itemtype' => true, + 'itemid' => true, + 'contextid' => true, + 'ordering' => false, + 'timecreated' => false, + 'timemodified' => false, + 'id' => false + ]; + + $requiredfields = array_filter($allowedfields, function($field) { + return $field; + }); + + if ($missingfields = array_keys(array_diff_key($requiredfields, $favourite))) { + throw new \moodle_exception("Missing object property(s) '" . join(', ', $missingfields) . "'."); + } + + // If the record contains fields we don't allow, throw an exception. + if ($unsupportedfields = array_keys(array_diff_key($favourite, $allowedfields))) { + throw new \moodle_exception("Unexpected object property(s) '" . join(', ', $unsupportedfields) . "'."); + } + } + /** * Add a favourite to the repository. * @@ -129,6 +167,20 @@ class favourite_repository implements favourite_repository_interface { return $this->get_favourite_from_record($record); } + /** + * Return all items in this repository, as an array, indexed by id. + * + * @param int $limitfrom optional pagination control for returning a subset of records, starting at this point. + * @param int $limitnum optional pagination control for returning a subset comprising this many records. + * @return array the list of all favourites stored within this repository. + * @throws \dml_exception if any database errors are encountered. + */ + public function find_all(int $limitfrom = 0, int $limitnum = 0) : array { + global $DB; + $records = $DB->get_records($this->favouritetable, null, '', '*', $limitfrom, $limitnum); + return $this->get_list_of_favourites_from_records($records); + } + /** * Return all items matching the supplied criteria (a [key => value,..] list). * @@ -144,20 +196,6 @@ class favourite_repository implements favourite_repository_interface { return $this->get_list_of_favourites_from_records($records); } - /** - * Return all items in this repository, as an array, indexed by id. - * - * @param int $limitfrom optional pagination control for returning a subset of records, starting at this point. - * @param int $limitnum optional pagination control for returning a subset comprising this many records. - * @return array the list of all favourites stored within this repository. - * @throws \dml_exception if any database errors are encountered. - */ - public function find_all(int $limitfrom = 0, int $limitnum = 0) : array { - global $DB; - $records = $DB->get_records($this->favouritetable, null, '', '*', $limitfrom, $limitnum); - return $this->get_list_of_favourites_from_records($records); - } - /** * Find a specific favourite, based on the properties known to identify it. * @@ -196,6 +234,18 @@ class favourite_repository implements favourite_repository_interface { return $DB->record_exists($this->favouritetable, ['id' => $id]); } + /** + * Check whether an item exists in this repository, based on the specified criteria. + * + * @param array $criteria the list of key/value criteria pairs. + * @return bool true if the favourite exists, false otherwise. + * @throws \dml_exception if any database errors are encountered. + */ + public function exists_by(array $criteria) : bool { + global $DB; + return $DB->record_exists($this->favouritetable, $criteria); + } + /** * Update a favourite. * @@ -222,6 +272,17 @@ class favourite_repository implements favourite_repository_interface { $DB->delete_records($this->favouritetable, ['id' => $id]); } + /** + * Delete all favourites matching the specified criteria. + * + * @param array $criteria the list of key/value criteria pairs. + * @throws \dml_exception if any database errors are encountered. + */ + public function delete_by(array $criteria) { + global $DB; + $DB->delete_records($this->favouritetable, $criteria); + } + /** * Return the total number of favourites in this repository. * @@ -233,52 +294,6 @@ class favourite_repository implements favourite_repository_interface { return $DB->count_records($this->favouritetable); } - /** - * Check for the existence of a favourite item in the specified area. - * - * A favourite item is identified by the itemid/contextid pair. - * An area is identified by the component/itemtype pair. - * - * @param int $userid the id of user to whom the favourite belongs. - * @param string $component the frankenstyle component name. - * @param string $itemtype the type of the favourited item. - * @param int $itemid the id of the item which was favourited (not the favourite's id). - * @param int $contextid the contextid of the item which was favourited. - * @return bool true if the favourited item exists, false otherwise. - * @throws \dml_exception if any database errors are encountered. - */ - public function exists_by_area(int $userid, string $component, string $itemtype, int $itemid, int $contextid) : bool { - global $DB; - return $DB->record_exists($this->favouritetable, - [ - 'userid' => $userid, - 'component' => $component, - 'itemtype' => $itemtype, - 'itemid' => $itemid, - 'contextid' => $contextid - ] - ); - } - - /** - * Delete all favourites within the component/itemtype. - * - * @param int $userid the id of the user to whom the favourite belongs. - * @param string $component the frankenstyle component name. - * @param string $itemtype the type of the favourited item. - * @throws \dml_exception if any database errors are encountered. - */ - public function delete_by_area(int $userid, string $component, string $itemtype) { - global $DB; - $DB->delete_records($this->favouritetable, - [ - 'userid' => $userid, - 'component' => $component, - 'itemtype' => $itemtype - ] - ); - } - /** * Return the number of user favourites matching the specified criteria. * @@ -290,42 +305,4 @@ class favourite_repository implements favourite_repository_interface { global $DB; return $DB->count_records($this->favouritetable, $criteria); } - - /** - * Basic validation, confirming we have the minimum field set needed to save a record to the store. - * - * @param favourite $favourite the favourite record to validate. - * @throws \moodle_exception if the supplied favourite has missing or unsupported fields. - */ - protected function validate(favourite $favourite) { - - $favourite = (array)$favourite; - - // The allowed fields, and whether or not each is required to create a record. - // The timecreated, timemodified and id fields are generated during create/update. - $allowedfields = [ - 'userid' => true, - 'component' => true, - 'itemtype' => true, - 'itemid' => true, - 'contextid' => true, - 'ordering' => false, - 'timecreated' => false, - 'timemodified' => false, - 'id' => false - ]; - - $requiredfields = array_filter($allowedfields, function($field) { - return $field; - }); - - if ($missingfields = array_keys(array_diff_key($requiredfields, $favourite))) { - throw new \moodle_exception("Missing object property(s) '" . join(', ', $missingfields) . "'."); - } - - // If the record contains fields we don't allow, throw an exception. - if ($unsupportedfields = array_keys(array_diff_key($favourite, $allowedfields))) { - throw new \moodle_exception("Unexpected object property(s) '" . join(', ', $unsupportedfields) . "'."); - } - } } diff --git a/favourites/classes/local/repository/favourite_repository_interface.php b/favourites/classes/local/repository/favourite_repository_interface.php index 138872a974a..e0d36c16fe0 100644 --- a/favourites/classes/local/repository/favourite_repository_interface.php +++ b/favourites/classes/local/repository/favourite_repository_interface.php @@ -80,6 +80,14 @@ interface favourite_repository_interface { */ public function exists(int $id) : bool; + /** + * Check whether an item exists in this repository, based on the specified criteria. + * + * @param array $criteria the list of key/value criteria pairs. + * @return bool true if the favourite exists, false otherwise. + */ + public function exists_by(array $criteria) : bool; + /** * Return the total number of items in this repository. * @@ -87,6 +95,14 @@ interface favourite_repository_interface { */ public function count() : int; + /** + * Return the number of favourites matching the specified criteria. + * + * @param array $criteria the list of key/value criteria pairs. + * @return int the number of favourites matching the criteria. + */ + public function count_by(array $criteria) : int; + /** * Update an item within this repository. * @@ -103,6 +119,14 @@ interface favourite_repository_interface { */ public function delete(int $id); + /** + * Delete all favourites matching the specified criteria. + * + * @param array $criteria the list of key/value criteria pairs. + * @return void. + */ + public function delete_by(array $criteria); + /** * Find a single favourite, based on it's unique identifiers. * diff --git a/favourites/tests/repository_test.php b/favourites/tests/repository_test.php index 60c8ed31fd4..724526197f7 100644 --- a/favourites/tests/repository_test.php +++ b/favourites/tests/repository_test.php @@ -393,6 +393,9 @@ class favourite_repository_testcase extends advanced_testcase { 'itemtype' => 'nonexistenttype'])); } + /** + * Test the exists() function. + */ public function test_exists() { list($user1context, $user2context, $course1context, $course2context) = $this->setup_users_and_courses(); @@ -414,7 +417,10 @@ class favourite_repository_testcase extends advanced_testcase { $this->assertFalse($favouritesrepo->exists(1)); } - public function test_exists_by_area() { + /** + * Test the exists_by() method. + */ + public function test_exists_by() { list($user1context, $user2context, $course1context, $course2context) = $this->setup_users_and_courses(); // Create a favourites repository and favourite two courses, in different areas. @@ -437,14 +443,35 @@ class favourite_repository_testcase extends advanced_testcase { $favourite2 = $favouritesrepo->add($favourite2); // Verify the existence of the favourites. - $this->assertTrue($favouritesrepo->exists_by_area($user1context->instanceid, 'core_course', 'course', $favourite1->itemid, - $favourite1->contextid)); - $this->assertTrue($favouritesrepo->exists_by_area($user1context->instanceid, 'core_course', 'anothertype', - $favourite2->itemid, $favourite2->contextid)); + $this->assertTrue($favouritesrepo->exists_by( + [ + 'userid' => $user1context->instanceid, + 'component' => 'core_course', + 'itemtype' => 'course', + 'itemid' => $favourite1->itemid, + 'contextid' => $favourite1->contextid + ] + )); + $this->assertTrue($favouritesrepo->exists_by( + [ + 'userid' => $user1context->instanceid, + 'component' => 'core_course', + 'itemtype' => 'anothertype', + 'itemid' => $favourite2->itemid, + 'contextid' => $favourite2->contextid + ] + )); // Verify that we can't find a favourite from one area, in another. - $this->assertFalse($favouritesrepo->exists_by_area($user1context->instanceid, 'core_course', 'anothertype', - $favourite1->itemid, $favourite1->contextid)); + $this->assertFalse($favouritesrepo->exists_by( + [ + 'userid' => $user1context->instanceid, + 'component' => 'core_course', + 'itemtype' => 'anothertype', + 'itemid' => $favourite1->itemid, + 'contextid' => $favourite1->contextid + ] + )); } /** @@ -494,7 +521,10 @@ class favourite_repository_testcase extends advanced_testcase { $this->assertFalse($favouritesrepo->exists($favourite->id)); } - public function test_delete_by_area() { + /** + * Test the delete_by() method. + */ + public function test_delete_by() { list($user1context, $user2context, $course1context, $course2context) = $this->setup_users_and_courses(); // Create a favourites repository and favourite two courses, in different areas. @@ -520,17 +550,77 @@ class favourite_repository_testcase extends advanced_testcase { $this->assertEquals(2, $favouritesrepo->count()); // Try to delete by a non-existent area, and confirm it doesn't remove anything. - $favouritesrepo->delete_by_area($user1context->instanceid, 'core_course', 'donaldduck'); + $favouritesrepo->delete_by( + [ + 'userid' => $user1context->instanceid, + 'component' => 'core_course', + 'itemtype' => 'donaldduck' + ] + ); $this->assertEquals(2, $favouritesrepo->count()); // Try to delete by a non-existent area, and confirm it doesn't remove anything. - $favouritesrepo->delete_by_area($user1context->instanceid, 'core_course', 'cat'); + $favouritesrepo->delete_by( + [ + 'userid' => $user1context->instanceid, + 'component' => 'core_course', + 'itemtype' => 'cat' + ] + ); $this->assertEquals(2, $favouritesrepo->count()); // Delete by area, and confirm we have one record left, from the 'core_course/anothertype' area. - $favouritesrepo->delete_by_area($user1context->instanceid, 'core_course', 'course'); + $favouritesrepo->delete_by( + [ + 'userid' => $user1context->instanceid, + 'component' => 'core_course', + 'itemtype' => 'course' + ] + ); $this->assertEquals(1, $favouritesrepo->count()); $this->assertFalse($favouritesrepo->exists($favourite1->id)); $this->assertTrue($favouritesrepo->exists($favourite2->id)); } + + /** + * Test the find_favourite() method for an existing favourite. + */ + public function test_find_favourite_basic() { + list($user1context, $user2context, $course1context, $course2context) = $this->setup_users_and_courses(); + + // Create a favourites repository and favourite two courses, in different areas. + $favouritesrepo = new favourite_repository($user1context); + $favourite = new favourite( + 'core_course', + 'course', + $course1context->instanceid, + $course1context->id, + $user1context->instanceid + ); + $favourite2 = new favourite( + 'core_course', + 'anothertype', + $course1context->instanceid, + $course1context->id, + $user1context->instanceid + ); + $favourite1 = $favouritesrepo->add($favourite); + $favourite2 = $favouritesrepo->add($favourite2); + + $fav = $favouritesrepo->find_favourite($user1context->instanceid, 'core_course', 'course', $course1context->instanceid, + $course1context->id); + $this->assertInstanceOf(\core_favourites\local\entity\favourite::class, $fav); + } + + /** + * Test confirming the repository throws an exception in find_favourite if the favourite can't be found. + */ + public function test_find_favourite_nonexistent_favourite() { + list($user1context, $user2context, $course1context, $course2context) = $this->setup_users_and_courses(); + + // Confirm we get an exception. + $favouritesrepo = new favourite_repository($user1context); + $this->expectException(\dml_exception::class); + $favouritesrepo->find_favourite($user1context->instanceid, 'core_course', 'course', 0, $course1context->id); + } }