From c85f8d3f934e8364f846ed10c37e907b8f047cde Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Mudr=C3=A1k?= <david@moodle.com>
Date: Wed, 22 Feb 2017 13:42:05 +0100
Subject: [PATCH 1/2] MDL-58050 message: Fix the DB transaction handling

Consumer code is not supposed to call commit_delegated_transaction()
method directly, this was incorrect usage of the delegated transactions.
---
 message/classes/api.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/message/classes/api.php b/message/classes/api.php
index 3a629548b67..44526714d89 100644
--- a/message/classes/api.php
+++ b/message/classes/api.php
@@ -439,7 +439,7 @@ class api {
         $unreadcounts = $DB->get_records_sql($unreadcountssql, [$userid]);
 
         // We can close off the transaction now.
-        $DB->commit_delegated_transaction($transaction);
+        $transaction->allow_commit();
 
         // Now we need to order the messages back into the same order of the conversations.
         $orderedconvosigs = array_keys($conversationrecords);

From 36916193d2ca8f041913579170230421954e7dad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Mudr=C3=A1k?= <david@moodle.com>
Date: Wed, 22 Feb 2017 13:45:16 +0100
Subject: [PATCH 2/2] MDL-58050 message: Commit the transaction before the
 early return

If there was no conversation found and the method returned early, the
active transaction was not marked as committable and the error was
reported by the request shutdown handler.

Attaching a unit test for the case just because it did not seem to be
covered. I was trying to reproduce the thrown error in the unit test but
it can't be used in this case. The shutdown handler puts the warning
directly into the error_log and it is not guaranteed where such messages
go (depending on the PHP configuration). And we do not even raise it
during the unit test execution (presumably due to noise it would
produce) anyway.
---
 message/classes/api.php    | 1 +
 message/tests/api_test.php | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/message/classes/api.php b/message/classes/api.php
index 44526714d89..98a96fcba09 100644
--- a/message/classes/api.php
+++ b/message/classes/api.php
@@ -314,6 +314,7 @@ class api {
 
         // This user has no conversations so we can return early here.
         if (empty($conversationrecords)) {
+            $transaction->allow_commit();
             return [];
         }
 
diff --git a/message/tests/api_test.php b/message/tests/api_test.php
index d90a48af1e9..f77c1b2fbca 100644
--- a/message/tests/api_test.php
+++ b/message/tests/api_test.php
@@ -333,6 +333,9 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         // The person doing the search.
         $this->setUser($user1);
 
+        // No conversations yet.
+        $this->assertEquals([], \core_message\api::get_conversations($user1->id));
+
         // Send some messages back and forth, have some different conversations with different users.
         $time = 1;
         $this->send_fake_message($user1, $user2, 'Yo!', 0, $time + 1);