MDL-63658 core_favourites: properly define interface methods and cleanup
authorJake Dallimore <jake@moodle.com>
Thu, 18 Oct 2018 07:31:55 +0000 (15:31 +0800)
committerJake Dallimore <jake@moodle.com>
Thu, 18 Oct 2018 07:39:57 +0000 (15:39 +0800)
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.

favourites/classes/local/repository/favourite_repository.php
favourites/classes/local/repository/favourite_repository_interface.php
favourites/tests/repository_test.php

index 2b94f91..d0236b7 100644 (file)
@@ -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.
      *
@@ -130,31 +168,31 @@ class favourite_repository implements favourite_repository_interface {
     }
 
     /**
-     * Return all items matching the supplied criteria (a [key => value,..] list).
+     * Return all items in this repository, as an array, indexed by id.
      *
-     * @param array $criteria the list of key/value criteria pairs.
      * @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 favourites matching the criteria.
+     * @return array the list of all favourites stored within this repository.
      * @throws \dml_exception if any database errors are encountered.
      */
-    public function find_by(array $criteria, int $limitfrom = 0, int $limitnum = 0) : array {
+    public function find_all(int $limitfrom = 0, int $limitnum = 0) : array {
         global $DB;
-        $records = $DB->get_records($this->favouritetable, $criteria, '', '*', $limitfrom, $limitnum);
+        $records = $DB->get_records($this->favouritetable, null, '', '*', $limitfrom, $limitnum);
         return $this->get_list_of_favourites_from_records($records);
     }
 
     /**
-     * Return all items in this repository, as an array, indexed by id.
+     * Return all items matching the supplied criteria (a [key => value,..] list).
      *
+     * @param array $criteria the list of key/value criteria pairs.
      * @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.
+     * @return array the list of favourites matching the criteria.
      * @throws \dml_exception if any database errors are encountered.
      */
-    public function find_all(int $limitfrom = 0, int $limitnum = 0) : array {
+    public function find_by(array $criteria, int $limitfrom = 0, int $limitnum = 0) : array {
         global $DB;
-        $records = $DB->get_records($this->favouritetable, null, '', '*', $limitfrom, $limitnum);
+        $records = $DB->get_records($this->favouritetable, $criteria, '', '*', $limitfrom, $limitnum);
         return $this->get_list_of_favourites_from_records($records);
     }
 
@@ -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.
      *
@@ -223,60 +273,25 @@ class favourite_repository implements favourite_repository_interface {
     }
 
     /**
-     * Return the total number of favourites in this repository.
-     *
-     * @return int the total number of items.
-     * @throws \dml_exception if any database errors are encountered.
-     */
-    public function count() : int {
-        global $DB;
-        return $DB->count_records($this->favouritetable);
-    }
-
-    /**
-     * Check for the existence of a favourite item in the specified area.
+     * Delete all favourites matching the specified criteria.
      *
-     * 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.
+     * @param array $criteria the list of key/value criteria pairs.
      * @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 {
+    public function delete_by(array $criteria) {
         global $DB;
-        return $DB->record_exists($this->favouritetable,
-            [
-                'userid' => $userid,
-                'component' => $component,
-                'itemtype' => $itemtype,
-                'itemid' => $itemid,
-                'contextid' => $contextid
-            ]
-        );
+        $DB->delete_records($this->favouritetable, $criteria);
     }
 
     /**
-     * Delete all favourites within the component/itemtype.
+     * Return the total number of favourites in this repository.
      *
-     * @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.
+     * @return int the total number of items.
      * @throws \dml_exception if any database errors are encountered.
      */
-    public function delete_by_area(int $userid, string $component, string $itemtype) {
+    public function count() : int {
         global $DB;
-        $DB->delete_records($this->favouritetable,
-            [
-                'userid' => $userid,
-                'component' => $component,
-                'itemtype' => $itemtype
-            ]
-        );
+        return $DB->count_records($this->favouritetable);
     }
 
     /**
@@ -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) . "'.");
-        }
-    }
 }
index 138872a..e0d36c1 100644 (file)
@@ -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.
      *
index 60c8ed3..7245261 100644 (file)
@@ -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);
+    }
 }