From b2cd17e6a9e6efabfea4c2b9d84ce3fa2a4948fb Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Tue, 6 Feb 2018 12:11:48 +0800 Subject: [PATCH] MDL-36941 core: added 'convhash' field to quickly get conversations --- lib/db/install.xml | 4 +++ lib/db/upgrade.php | 20 ++++++++++++++ lib/messagelib.php | 8 +++--- message/classes/api.php | 43 +++++++++++------------------- message/classes/helper.php | 12 +++++++++ message/tests/api_test.php | 6 ++--- message/tests/externallib_test.php | 6 ++--- message/tests/messagelib_test.php | 6 ++--- version.php | 2 +- 9 files changed, 65 insertions(+), 42 deletions(-) diff --git a/lib/db/install.xml b/lib/db/install.xml index 192df05a358..ffc0d42c33b 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -612,11 +612,15 @@ + + + +
diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index e7f8cde2384..5da6ff7437e 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2131,5 +2131,25 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2018032200.01); } + if ($oldversion < 2018032200.04) { + // Define table 'message_conversations' to be updated. + $table = new xmldb_table('message_conversations'); + $field = new xmldb_field('convhash', XMLDB_TYPE_CHAR, '40', null, XMLDB_NOTNULL, null, 0, 'id'); + + // Conditionally launch add field 'convhash'. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Conditionally launch add index. + $index = new xmldb_index('convhash', XMLDB_INDEX_UNIQUE, array('convhash')); + if (!$dbman->index_exists($table, $index)) { + $dbman->add_index($table, $index); + } + + // Main savepoint reached. + upgrade_main_savepoint(true, 2018032200.04); + } + return true; } diff --git a/lib/messagelib.php b/lib/messagelib.php index 11f20695696..0b5d300609d 100644 --- a/lib/messagelib.php +++ b/lib/messagelib.php @@ -156,10 +156,10 @@ function message_send($eventdata) { } else { $table = 'messages'; - if (!$conversationid = \core_message\api::get_conversation_between_users($eventdata->userfrom->id, - $eventdata->userto->id)) { - $conversationid = \core_message\api::create_conversation_between_users($eventdata->userfrom->id, - $eventdata->userto->id); + if (!$conversationid = \core_message\api::get_conversation_between_users([$eventdata->userfrom->id, + $eventdata->userto->id])) { + $conversationid = \core_message\api::create_conversation_between_users([$eventdata->userfrom->id, + $eventdata->userto->id]); } $tabledata = new stdClass(); diff --git a/message/classes/api.php b/message/classes/api.php index df1ad671d29..2ff2a59bb89 100644 --- a/message/classes/api.php +++ b/message/classes/api.php @@ -637,7 +637,7 @@ class api { public static function delete_conversation($userid, $otheruserid) { global $DB, $USER; - $conversationid = self::get_conversation_between_users($userid, $otheruserid); + $conversationid = self::get_conversation_between_users([$userid, $otheruserid]); // If there is no conversation, there is nothing to do. if (!$conversationid) { @@ -1202,24 +1202,15 @@ class api { /** * Returns the conversation between two users. * - * @param int $userid1 The userid of the first user - * @param int $userid2 The userid of the second user + * @param array $userids * @return int|bool The id of the conversation, false if not found */ - public static function get_conversation_between_users($userid1, $userid2) { + public static function get_conversation_between_users(array $userids) { global $DB; - $sql = "SELECT DISTINCT mc.id - FROM {message_conversations} mc - INNER JOIN {message_conversation_members} mcm - ON mcm.conversationid = mc.id - INNER JOIN {message_conversation_members} mcm2 - ON mcm2.conversationid = mc.id - WHERE mcm.userid = :userid1 - AND mcm2.userid = :userid2 - AND mcm.id != mcm2.id"; + $hash = helper::get_conversation_hash($userids); - if ($conversation = $DB->get_record_sql($sql, ['userid1' => $userid1, 'userid2' => $userid2])) { + if ($conversation = $DB->get_record('message_conversations', ['convhash' => $hash])) { return $conversation->id; } @@ -1229,29 +1220,25 @@ class api { /** * Creates a conversation between two users. * - * @param int $userid1 The userid of the first user - * @param int $userid2 The userid of the second user + * @param array $userids * @return int The id of the conversation */ - public static function create_conversation_between_users($userid1, $userid2) { + public static function create_conversation_between_users(array $userids) { global $DB; $conversation = new \stdClass(); + $conversation->convhash = helper::get_conversation_hash($userids); $conversation->timecreated = time(); $conversation->id = $DB->insert_record('message_conversations', $conversation); // Add members to this conversation. - $member = new \stdClass(); - $member->conversationid = $conversation->id; - $member->userid = $userid1; - $member->timecreated = time(); - $DB->insert_record('message_conversation_members', $member); - - $member = new \stdClass(); - $member->conversationid = $conversation->id; - $member->userid = $userid2; - $member->timecreated = time(); - $DB->insert_record('message_conversation_members', $member); + foreach ($userids as $userid) { + $member = new \stdClass(); + $member->conversationid = $conversation->id; + $member->userid = $userid; + $member->timecreated = time(); + $DB->insert_record('message_conversation_members', $member); + } return $conversation->id; } diff --git a/message/classes/helper.php b/message/classes/helper.php index c780bd7afc0..dad58fcc32b 100644 --- a/message/classes/helper.php +++ b/message/classes/helper.php @@ -293,6 +293,18 @@ class helper { return $params; } + /** + * Returns the conversation hash between users for easy look-ups in the DB. + * + * @param array $userids + * @return string + */ + public static function get_conversation_hash(array $userids) { + sort($userids); + + return sha1(implode('-', $userids)); + } + /** * Returns the cache key for the time created value of the last message between two users. * diff --git a/message/tests/api_test.php b/message/tests/api_test.php index bdca5c3c05b..98fa368f9a2 100644 --- a/message/tests/api_test.php +++ b/message/tests/api_test.php @@ -1921,7 +1921,7 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $user1 = self::getDataGenerator()->create_user(); $user2 = self::getDataGenerator()->create_user(); - $this->assertFalse(\core_message\api::get_conversation_between_users($user1->id, $user2->id)); + $this->assertFalse(\core_message\api::get_conversation_between_users([$user1->id, $user2->id])); } /** @@ -1931,9 +1931,9 @@ class core_message_api_testcase extends core_message_messagelib_testcase { $user1 = self::getDataGenerator()->create_user(); $user2 = self::getDataGenerator()->create_user(); - $conversationid = \core_message\api::create_conversation_between_users($user1->id, $user2->id); + $conversationid = \core_message\api::create_conversation_between_users([$user1->id, $user2->id]); $this->assertEquals($conversationid, - \core_message\api::get_conversation_between_users($user1->id, $user2->id)); + \core_message\api::get_conversation_between_users([$user1->id, $user2->id])); } } diff --git a/message/tests/externallib_test.php b/message/tests/externallib_test.php index 83122b2d1da..f5475d79b8d 100644 --- a/message/tests/externallib_test.php +++ b/message/tests/externallib_test.php @@ -73,9 +73,9 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { return $DB->insert_record('notifications', $record); } - if (!$conversationid = \core_message\api::get_conversation_between_users($userfrom->id, $userto->id)) { - $conversationid = \core_message\api::create_conversation_between_users($userfrom->id, - $userto->id); + if (!$conversationid = \core_message\api::get_conversation_between_users([$userfrom->id, $userto->id])) { + $conversationid = \core_message\api::create_conversation_between_users([$userfrom->id, + $userto->id]); } // Ok, send the message. diff --git a/message/tests/messagelib_test.php b/message/tests/messagelib_test.php index dc47874c79b..231149aec8c 100644 --- a/message/tests/messagelib_test.php +++ b/message/tests/messagelib_test.php @@ -85,9 +85,9 @@ class core_message_messagelib_testcase extends advanced_testcase { return $DB->insert_record('notifications', $record); } - if (!$conversationid = \core_message\api::get_conversation_between_users($userfrom->id, $userto->id)) { - $conversationid = \core_message\api::create_conversation_between_users($userfrom->id, - $userto->id); + if (!$conversationid = \core_message\api::get_conversation_between_users([$userfrom->id, $userto->id])) { + $conversationid = \core_message\api::create_conversation_between_users([$userfrom->id, + $userto->id]); } // Ok, send the message. diff --git a/version.php b/version.php index c910bfd2f1f..cdca8f9f91e 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2018032200.03; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2018032200.04; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes.