From a196f37088152c246f9d85ad17012c22655d477b Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Sun, 16 Aug 2015 18:49:52 +0800 Subject: [PATCH 1/2] MDL-27859 tags: unit test for correlated tags --- tag/lib.php | 25 +++++----- tag/tests/taglib_test.php | 96 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 12 deletions(-) diff --git a/tag/lib.php b/tag/lib.php index 102704cbc2b..2ab484b2cc5 100644 --- a/tag/lib.php +++ b/tag/lib.php @@ -546,10 +546,8 @@ function tag_get_related_tags($tagid, $type=TAG_RELATED_ALL, $limitnum=10) { if ( $type == TAG_RELATED_ALL || $type == TAG_RELATED_CORRELATED ) { //gets the correlated tags - $automatic_related_tags = tag_get_correlated($tagid, $limitnum); - if (is_array($automatic_related_tags)) { - $related_tags = array_merge($related_tags, $automatic_related_tags); - } + $automatic_related_tags = tag_get_correlated($tagid); + $related_tags = array_merge($related_tags, $automatic_related_tags); } // Remove duplicated tags (multiple instances of the same tag). @@ -1378,13 +1376,21 @@ function tag_get_name($tagids) { * Returns the correlated tags of a tag, retrieved from the tag_correlation table. Make sure cron runs, otherwise the table will be * empty and this function won't return anything. * + * Correlated tags are calculated in cron based on existing tag instances. + * + * This function will return as many entries as there are existing tag instances, + * which means that there will be duplicates for each tag. + * + * If you need only one record for each correlated tag please call: + * tag_get_related_tags($tag_id, TAG_RELATED_CORRELATED); + * * @package core_tag * @access private * @param int $tag_id is a single tag id - * @param int $limitnum this parameter does not appear to have any function??? + * @param int $notused this argument is no longer used * @return array an array of tag objects or an empty if no correlated tags are found */ -function tag_get_correlated($tag_id, $limitnum=null) { +function tag_get_correlated($tag_id, $notused = null) { global $DB; $tag_correlation = $DB->get_record('tag_correlation', array('tagid'=>$tag_id)); @@ -1399,12 +1405,7 @@ function tag_get_correlated($tag_id, $limitnum=null) { INNER JOIN {tag_instance} ti ON tg.id = ti.tagid WHERE tg.id IN ({$tag_correlation->correlatedtags}) ORDER BY ti.ordering ASC"; - $result = $DB->get_records_sql($sql); - if (!$result) { - return array(); - } - - return $result; + return $DB->get_records_sql($sql); } /** diff --git a/tag/tests/taglib_test.php b/tag/tests/taglib_test.php index 1e0bc74b50a..353f1a4e94f 100644 --- a/tag/tests/taglib_test.php +++ b/tag/tests/taglib_test.php @@ -256,4 +256,100 @@ class core_tag_taglib_testcase extends advanced_testcase { $instancecount = $DB->count_records('tag_instance'); $this->assertEquals(0, $instancecount); } + + /** + * Test for function tag_compute_correlations() that is part of tag cron + */ + public function test_correlations() { + global $DB; + $user = $this->getDataGenerator()->create_user(); + $this->setUser($user); + + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + $user3 = $this->getDataGenerator()->create_user(); + $user4 = $this->getDataGenerator()->create_user(); + $user5 = $this->getDataGenerator()->create_user(); + $user6 = $this->getDataGenerator()->create_user(); + + // Several records have both 'cat' and 'cats' tags attached to them. + // This will make those tags automatically correlated. + // Same with 'dog', 'dogs' and 'puppy. + tag_set('user', $user1->id, array('cat', 'cats'), + 'core', context_user::instance($user1->id)->id); + tag_set('user', $user2->id, array('cat', 'cats', 'kitten'), + 'core', context_user::instance($user2->id)->id); + tag_set('user', $user3->id, array('cat', 'cats'), + 'core', context_user::instance($user3->id)->id); + tag_set('user', $user4->id, array('dog', 'dogs', 'puppy'), + 'core', context_user::instance($user4->id)->id); + tag_set('user', $user5->id, array('dog', 'dogs', 'puppy'), + 'core', context_user::instance($user5->id)->id); + tag_set('user', $user6->id, array('dog', 'dogs', 'puppy'), + 'core', context_user::instance($user6->id)->id); + + $tags = tag_get_id(array('cat', 'cats', 'dog', 'dogs', 'kitten', 'puppy')); + + // Add manual relation between tags 'cat' and 'kitten'. + tag_set('tag', $tags['cat'], array('kitten'), 'core', context_system::instance()->id); + + tag_compute_correlations(); + + $this->assertEquals($tags['cats'], + $DB->get_field_select('tag_correlation', 'correlatedtags', + 'tagid = ?', array($tags['cat']))); + $this->assertEquals($tags['cat'], + $DB->get_field_select('tag_correlation', 'correlatedtags', + 'tagid = ?', array($tags['cats']))); + $this->assertEquals($tags['dogs'] . ',' . $tags['puppy'], + $DB->get_field_select('tag_correlation', 'correlatedtags', + 'tagid = ?', array($tags['dog']))); + $this->assertEquals($tags['dog'] . ',' . $tags['puppy'], + $DB->get_field_select('tag_correlation', 'correlatedtags', + 'tagid = ?', array($tags['dogs']))); + $this->assertEquals($tags['dog'] . ',' . $tags['dogs'], + $DB->get_field_select('tag_correlation', 'correlatedtags', + 'tagid = ?', array($tags['puppy']))); + + // Make sure tag_get_correlated() returns 'cats' as the only correlated tag to the 'cat'. + $correlatedtags = array_values(tag_get_correlated($tags['cat'])); + $this->assertCount(3, $correlatedtags); // This will return all existing instances but they all point to the same tag. + $this->assertEquals('cats', $correlatedtags[0]->rawname); + $this->assertEquals('cats', $correlatedtags[1]->rawname); + $this->assertEquals('cats', $correlatedtags[2]->rawname); + + $correlatedtags = array_values(tag_get_related_tags($tags['cat'], TAG_RELATED_CORRELATED)); + $this->assertCount(1, $correlatedtags); // Duplicates are filtered out here. + $this->assertEquals('cats', $correlatedtags[0]->rawname); + + // Make sure tag_get_correlated() returns 'dogs' and 'puppy' as the correlated tags to the 'dog'. + $correlatedtags = array_values(tag_get_correlated($tags['dog'])); + $this->assertCount(6, $correlatedtags); // 2 tags times 3 instances. + + $correlatedtags = array_values(tag_get_related_tags($tags['dog'], TAG_RELATED_CORRELATED)); + $this->assertCount(2, $correlatedtags); + $this->assertEquals('dogs', $correlatedtags[0]->rawname); + $this->assertEquals('puppy', $correlatedtags[1]->rawname); + + // Function tag_get_related_tags() with default argument will return both related and correlated tags. + $relatedtags = array_values(tag_get_related_tags($tags['cat'])); + $this->assertCount(2, $relatedtags); + $this->assertEquals('kitten', $relatedtags[0]->rawname); + $this->assertEquals('cats', $relatedtags[1]->rawname); + + // If we then manually set 'cat' and 'cats' as related, tag_get_related_tags() will filter out duplicates. + tag_set('tag', $tags['cat'], array('kitten', 'cats'), 'core', context_system::instance()->id); + + $relatedtags = array_values(tag_get_related_tags($tags['cat'])); + $this->assertCount(2, $relatedtags); + $this->assertEquals('kitten', $relatedtags[0]->rawname); + $this->assertEquals('cats', $relatedtags[1]->rawname); + + // Make sure tag_get_correlated() and tag_get_tags() return the same set of fields. + $relatedtags = tag_get_tags('tag', $tags['cat']); + $relatedtag = reset($relatedtags); + $correlatedtags = tag_get_correlated($tags['cat']); + $correlatedtag = reset($correlatedtags); + $this->assertEquals(array_keys((array)$relatedtag), array_keys((array)$correlatedtag)); + } } From 46f708ea17085ba3af0cd7c2f0bebfed2fd7d734 Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Sun, 16 Aug 2015 19:52:27 +0800 Subject: [PATCH 2/2] MDL-27859 tags: unit test for tag cleanup --- tag/tests/taglib_test.php | 69 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/tag/tests/taglib_test.php b/tag/tests/taglib_test.php index 353f1a4e94f..24afde7b18a 100644 --- a/tag/tests/taglib_test.php +++ b/tag/tests/taglib_test.php @@ -352,4 +352,73 @@ class core_tag_taglib_testcase extends advanced_testcase { $correlatedtag = reset($correlatedtags); $this->assertEquals(array_keys((array)$relatedtag), array_keys((array)$correlatedtag)); } + + /** + * Test for function tag_cleanup() that is part of tag cron + */ + public function test_cleanup() { + global $DB; + $user = $this->getDataGenerator()->create_user(); + + // Setting tags will create non-official tags 'cat', 'dog' and 'fish'. + tag_set('user', $user->id, array('cat', 'dog', 'fish'), + 'core', context_user::instance($user->id)->id); + + $this->assertTrue($DB->record_exists('tag', array('name' => 'cat'))); + $this->assertTrue($DB->record_exists('tag', array('name' => 'dog'))); + $this->assertTrue($DB->record_exists('tag', array('name' => 'fish'))); + + // Make tag 'dog' official. + $dogtag = tag_get('name', 'dog'); + $fishtag = tag_get('name', 'fish'); + tag_type_set($dogtag->id, 'official'); + + // Manually remove the instances pointing on tags 'dog' and 'fish'. + $DB->execute('DELETE FROM {tag_instance} WHERE tagid in (?,?)', array($dogtag->id, $fishtag->id)); + + // Call tag_cleanup(). + tag_cleanup(); + + // Tag 'cat' is still present because it's used. Tag 'dog' is present because it's official. + // Tag 'fish' was removed because it is not official and it is no longer used by anybody. + $this->assertTrue($DB->record_exists('tag', array('name' => 'cat'))); + $this->assertTrue($DB->record_exists('tag', array('name' => 'dog'))); + $this->assertFalse($DB->record_exists('tag', array('name' => 'fish'))); + + // Delete user without using API function. + $DB->update_record('user', array('id' => $user->id, 'deleted' => 1)); + + // Call tag_cleanup(). + tag_cleanup(); + + // Tag 'cat' was now deleted too. + $this->assertFalse($DB->record_exists('tag', array('name' => 'cat'))); + + // Assign tag to non-existing record. Make sure tag was created in the DB. + tag_set('course', 1231231, array('bird'), 'core', context_system::instance()->id); + $this->assertTrue($DB->record_exists('tag', array('name' => 'bird'))); + + // Call tag_cleanup(). + tag_cleanup(); + + // Tag 'bird' was now deleted because the related record does not exist in the DB. + $this->assertFalse($DB->record_exists('tag', array('name' => 'bird'))); + + // Now we have a tag instance pointing on 'sometag' tag. + $user = $this->getDataGenerator()->create_user(); + tag_set('user', $user->id, array('sometag'), + 'core', context_user::instance($user->id)->id); + $sometag = tag_get('name', 'sometag'); + + $this->assertTrue($DB->record_exists('tag_instance', array('tagid' => $sometag->id))); + + // Some hacker removes the tag without using API. + $DB->delete_records('tag', array('id' => $sometag->id)); + + // Call tag_cleanup(). + tag_cleanup(); + + // The tag instances were also removed. + $this->assertFalse($DB->record_exists('tag_instance', array('tagid' => $sometag->id))); + } }