From 2b9a06573e6390e457aa18cbd77e7028cebbfcc0 Mon Sep 17 00:00:00 2001 From: Lars Thoms Date: Mon, 24 Apr 2023 13:58:40 +0200 Subject: [PATCH 1/5] Fixes learnweb/moodle-mod_moodleoverflow#75 Split query into two parts due to a never ending database request --- classes/privacy/provider.php | 88 +++++++++++++++++++++--------------- 1 file changed, 51 insertions(+), 37 deletions(-) diff --git a/classes/privacy/provider.php b/classes/privacy/provider.php index 80f424b92f..c7a900ab92 100644 --- a/classes/privacy/provider.php +++ b/classes/privacy/provider.php @@ -137,48 +137,62 @@ public static function get_metadata(collection $collection) : collection { */ public static function get_contexts_for_userid(int $userid) : contextlist { // Fetch all Moodleoverflow discussions, moodleoverflow posts, ratings, tracking settings and subscriptions. - $sql = "SELECT c.id - FROM {context} c - INNER JOIN {course_modules} cm ON cm.id = c.instanceid AND c.contextlevel = :contextlevel - INNER JOIN {modules} m ON m.id = cm.module AND m.name = :modname - INNER JOIN {moodleoverflow} mof ON mof.id = cm.instance - LEFT JOIN {moodleoverflow_discussions} d ON d.moodleoverflow = mof.id - LEFT JOIN {moodleoverflow_posts} p ON p.discussion = d.id - LEFT JOIN {moodleoverflow_read} r ON r.moodleoverflowid = mof.id - LEFT JOIN {moodleoverflow_subscriptions} s ON s.moodleoverflow = mof.id - LEFT JOIN {moodleoverflow_discuss_subs} ds ON ds.moodleoverflow = mof.id - LEFT JOIN {moodleoverflow_ratings} ra ON ra.moodleoverflowid = mof.id - LEFT JOIN {moodleoverflow_tracking} track ON track.moodleoverflowid = mof.id - LEFT JOIN {moodleoverflow_grades} g ON g.moodleoverflowid = mof.id - WHERE ( - d.userid = :duserid OR - d.usermodified = :dmuserid OR - p.userid = :puserid OR - r.userid = :ruserid OR - s.userid = :suserid OR - ds.userid = :dsuserid OR - ra.userid = :rauserid OR - track.userid = :userid OR - g.userid = :guserid - ) - "; + // FIX: split query into two parts due to a never ending database request. + $sql1 = "SELECT c.id + FROM {context} c + INNER JOIN {course_modules} cm ON cm.id = c.instanceid AND c.contextlevel = :contextlevel + INNER JOIN {modules} m ON m.id = cm.module AND m.name = :modname + INNER JOIN {moodleoverflow} mof ON mof.id = cm.instance + LEFT JOIN {moodleoverflow_discussions} d ON d.moodleoverflow = mof.id + LEFT JOIN {moodleoverflow_posts} p ON p.discussion = d.id + LEFT JOIN {moodleoverflow_read} r ON r.moodleoverflowid = mof.id + LEFT JOIN {moodleoverflow_subscriptions} s ON s.moodleoverflow = mof.id + WHERE ( + d.userid = :duserid OR + d.usermodified = :dmuserid OR + p.userid = :puserid OR + r.userid = :ruserid OR + s.userid = :suserid + ) GROUP BY c.id"; + + $sql2 = "SELECT c.id + FROM {context} c + INNER JOIN {course_modules} cm ON cm.id = c.instanceid AND c.contextlevel = :contextlevel + INNER JOIN {modules} m ON m.id = cm.module AND m.name = :modname + INNER JOIN {moodleoverflow} mof ON mof.id = cm.instance + LEFT JOIN {moodleoverflow_discuss_subs} ds ON ds.moodleoverflow = mof.id + LEFT JOIN {moodleoverflow_ratings} ra ON ra.moodleoverflowid = mof.id + LEFT JOIN {moodleoverflow_tracking} track ON track.moodleoverflowid = mof.id + LEFT JOIN {moodleoverflow_grades} g ON g.moodleoverflowid = mof.id + WHERE ( + ds.userid = :dsuserid OR + ra.userid = :rauserid OR + track.userid = :userid OR + g.userid = :guserid + ) GROUP BY c.id"; + + $params1 = [ + 'modname' => 'moodleoverflow', + 'contextlevel' => CONTEXT_MODULE, + 'duserid' => $userid, + 'dmuserid' => $userid, + 'puserid' => $userid, + 'ruserid' => $userid, + 'suserid' => $userid + ]; - $params = [ - 'modname' => 'moodleoverflow', + $params2 = [ + 'modname' => 'moodleoverflow', 'contextlevel' => CONTEXT_MODULE, - 'duserid' => $userid, - 'dmuserid' => $userid, - 'puserid' => $userid, - 'ruserid' => $userid, - 'suserid' => $userid, - 'dsuserid' => $userid, - 'rauserid' => $userid, - 'userid' => $userid, - 'guserid' => $userid + 'dsuserid' => $userid, + 'rauserid' => $userid, + 'userid' => $userid, + 'guserid' => $userid ]; $contextlist = new \core_privacy\local\request\contextlist(); - $contextlist->add_from_sql($sql, $params); + $contextlist->add_from_sql($sql1, $params1); + $contextlist->add_from_sql($sql2, $params2); return $contextlist; } From db0a344bed9780cd40f005d587714c664ee24eda Mon Sep 17 00:00:00 2001 From: Lars Thoms Date: Mon, 24 Apr 2023 15:33:55 +0200 Subject: [PATCH 2/5] changed joins into multiple subqueries as suggested by @ziegenberg --- classes/privacy/provider.php | 75 ++++++++++++------------------------ 1 file changed, 25 insertions(+), 50 deletions(-) diff --git a/classes/privacy/provider.php b/classes/privacy/provider.php index c7a900ab92..304bb1ce83 100644 --- a/classes/privacy/provider.php +++ b/classes/privacy/provider.php @@ -137,62 +137,37 @@ public static function get_metadata(collection $collection) : collection { */ public static function get_contexts_for_userid(int $userid) : contextlist { // Fetch all Moodleoverflow discussions, moodleoverflow posts, ratings, tracking settings and subscriptions. - // FIX: split query into two parts due to a never ending database request. - $sql1 = "SELECT c.id - FROM {context} c - INNER JOIN {course_modules} cm ON cm.id = c.instanceid AND c.contextlevel = :contextlevel - INNER JOIN {modules} m ON m.id = cm.module AND m.name = :modname - INNER JOIN {moodleoverflow} mof ON mof.id = cm.instance - LEFT JOIN {moodleoverflow_discussions} d ON d.moodleoverflow = mof.id - LEFT JOIN {moodleoverflow_posts} p ON p.discussion = d.id - LEFT JOIN {moodleoverflow_read} r ON r.moodleoverflowid = mof.id - LEFT JOIN {moodleoverflow_subscriptions} s ON s.moodleoverflow = mof.id - WHERE ( - d.userid = :duserid OR - d.usermodified = :dmuserid OR - p.userid = :puserid OR - r.userid = :ruserid OR - s.userid = :suserid - ) GROUP BY c.id"; - - $sql2 = "SELECT c.id - FROM {context} c - INNER JOIN {course_modules} cm ON cm.id = c.instanceid AND c.contextlevel = :contextlevel - INNER JOIN {modules} m ON m.id = cm.module AND m.name = :modname - INNER JOIN {moodleoverflow} mof ON mof.id = cm.instance - LEFT JOIN {moodleoverflow_discuss_subs} ds ON ds.moodleoverflow = mof.id - LEFT JOIN {moodleoverflow_ratings} ra ON ra.moodleoverflowid = mof.id - LEFT JOIN {moodleoverflow_tracking} track ON track.moodleoverflowid = mof.id - LEFT JOIN {moodleoverflow_grades} g ON g.moodleoverflowid = mof.id - WHERE ( - ds.userid = :dsuserid OR - ra.userid = :rauserid OR - track.userid = :userid OR - g.userid = :guserid - ) GROUP BY c.id"; - - $params1 = [ - 'modname' => 'moodleoverflow', - 'contextlevel' => CONTEXT_MODULE, - 'duserid' => $userid, - 'dmuserid' => $userid, - 'puserid' => $userid, - 'ruserid' => $userid, - 'suserid' => $userid - ]; + $sql = "SELECT c.id + FROM {context} c + INNER JOIN {course_modules} cm ON cm.id = c.instanceid AND c.contextlevel = :contextlevel + INNER JOIN {modules} m ON m.id = cm.module AND m.name = :modname + INNER JOIN {moodleoverflow} mof ON mof.id = cm.instance + WHERE EXISTS ( + SELECT 1 FROM {moodleoverflow_discussions} d WHERE d.moodleoverflow = mof.id AND (d.userid = :userid OR d.usermodified = :userid) + ) OR EXISTS ( + SELECT 1 FROM {moodleoverflow_posts} p WHERE p.discussion IN (SELECT id FROM {moodleoverflow_discussions} WHERE moodleoverflow = mof.id) AND p.userid = :userid + ) OR EXISTS ( + SELECT 1 FROM {moodleoverflow_read} r WHERE r.moodleoverflowid = mof.id AND r.userid = :userid + ) OR EXISTS ( + SELECT 1 FROM {moodleoverflow_subscriptions} s WHERE s.moodleoverflow = mof.id AND s.userid = :userid + ) OR EXISTS ( + SELECT 1 FROM {moodleoverflow_discuss_subs} ds WHERE ds.moodleoverflow = mof.id AND ds.userid = :userid + ) OR EXISTS ( + SELECT 1 FROM {moodleoverflow_ratings} ra WHERE ra.moodleoverflowid = mof.id AND ra.userid = :userid + ) OR EXISTS ( + SELECT 1 FROM {moodleoverflow_tracking} track WHERE track.moodleoverflowid = mof.id AND track.userid = :userid + ) OR EXISTS ( + SELECT 1 FROM {moodleoverflow_grades} g WHERE g.moodleoverflowid = mof.id AND g.userid = :userid + );" - $params2 = [ + $params = [ 'modname' => 'moodleoverflow', 'contextlevel' => CONTEXT_MODULE, - 'dsuserid' => $userid, - 'rauserid' => $userid, - 'userid' => $userid, - 'guserid' => $userid + 'userid' => $userid ]; $contextlist = new \core_privacy\local\request\contextlist(); - $contextlist->add_from_sql($sql1, $params1); - $contextlist->add_from_sql($sql2, $params2); + $contextlist->add_from_sql($sql, $params); return $contextlist; } From 370eae8c479cef44986b2a4c49e8d37ec28564d3 Mon Sep 17 00:00:00 2001 From: Lars Thoms Date: Tue, 25 Apr 2023 07:31:46 +0200 Subject: [PATCH 3/5] fixed syntax error --- classes/privacy/provider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/classes/privacy/provider.php b/classes/privacy/provider.php index 304bb1ce83..c1b403dfeb 100644 --- a/classes/privacy/provider.php +++ b/classes/privacy/provider.php @@ -158,7 +158,7 @@ public static function get_contexts_for_userid(int $userid) : contextlist { SELECT 1 FROM {moodleoverflow_tracking} track WHERE track.moodleoverflowid = mof.id AND track.userid = :userid ) OR EXISTS ( SELECT 1 FROM {moodleoverflow_grades} g WHERE g.moodleoverflowid = mof.id AND g.userid = :userid - );" + );"; $params = [ 'modname' => 'moodleoverflow', From 5afcda0c5592cd5e445b6f3dfb88d205aa58a78f Mon Sep 17 00:00:00 2001 From: Lars Thoms Date: Tue, 25 Apr 2023 09:51:38 +0200 Subject: [PATCH 4/5] fixed number of query parameters to work as positional parameter --- classes/privacy/provider.php | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/classes/privacy/provider.php b/classes/privacy/provider.php index c1b403dfeb..95e3e7f69d 100644 --- a/classes/privacy/provider.php +++ b/classes/privacy/provider.php @@ -143,27 +143,35 @@ public static function get_contexts_for_userid(int $userid) : contextlist { INNER JOIN {modules} m ON m.id = cm.module AND m.name = :modname INNER JOIN {moodleoverflow} mof ON mof.id = cm.instance WHERE EXISTS ( - SELECT 1 FROM {moodleoverflow_discussions} d WHERE d.moodleoverflow = mof.id AND (d.userid = :userid OR d.usermodified = :userid) + SELECT 1 FROM {moodleoverflow_discussions} d WHERE d.moodleoverflow = mof.id AND (d.userid = :duserid OR d.usermodified = :dmuserid) ) OR EXISTS ( - SELECT 1 FROM {moodleoverflow_posts} p WHERE p.discussion IN (SELECT id FROM {moodleoverflow_discussions} WHERE moodleoverflow = mof.id) AND p.userid = :userid + SELECT 1 FROM {moodleoverflow_posts} p WHERE p.discussion IN (SELECT id FROM {moodleoverflow_discussions} WHERE moodleoverflow = mof.id) AND p.userid = :puserid ) OR EXISTS ( - SELECT 1 FROM {moodleoverflow_read} r WHERE r.moodleoverflowid = mof.id AND r.userid = :userid + SELECT 1 FROM {moodleoverflow_read} r WHERE r.moodleoverflowid = mof.id AND r.userid = :ruserid ) OR EXISTS ( - SELECT 1 FROM {moodleoverflow_subscriptions} s WHERE s.moodleoverflow = mof.id AND s.userid = :userid + SELECT 1 FROM {moodleoverflow_subscriptions} s WHERE s.moodleoverflow = mof.id AND s.userid = :suserid ) OR EXISTS ( - SELECT 1 FROM {moodleoverflow_discuss_subs} ds WHERE ds.moodleoverflow = mof.id AND ds.userid = :userid + SELECT 1 FROM {moodleoverflow_discuss_subs} ds WHERE ds.moodleoverflow = mof.id AND ds.userid = :dsuserid ) OR EXISTS ( - SELECT 1 FROM {moodleoverflow_ratings} ra WHERE ra.moodleoverflowid = mof.id AND ra.userid = :userid + SELECT 1 FROM {moodleoverflow_ratings} ra WHERE ra.moodleoverflowid = mof.id AND ra.userid = :rauserid ) OR EXISTS ( - SELECT 1 FROM {moodleoverflow_tracking} track WHERE track.moodleoverflowid = mof.id AND track.userid = :userid + SELECT 1 FROM {moodleoverflow_tracking} track WHERE track.moodleoverflowid = mof.id AND track.userid = :tuserid ) OR EXISTS ( - SELECT 1 FROM {moodleoverflow_grades} g WHERE g.moodleoverflowid = mof.id AND g.userid = :userid + SELECT 1 FROM {moodleoverflow_grades} g WHERE g.moodleoverflowid = mof.id AND g.userid = :guserid );"; $params = [ 'modname' => 'moodleoverflow', 'contextlevel' => CONTEXT_MODULE, - 'userid' => $userid + 'duserid' => $userid, + 'dmuserid' => $userid, + 'puserid' => $userid, + 'ruserid' => $userid, + 'suserid' => $userid, + 'dsuserid' => $userid, + 'rauserid' => $userid, + 'tuserid' => $userid, + 'guserid' => $userid ]; $contextlist = new \core_privacy\local\request\contextlist(); From 7b88e22d129dee21e0f1b95277987e38bd83f4ae Mon Sep 17 00:00:00 2001 From: Lars Thoms Date: Tue, 25 Apr 2023 10:31:32 +0200 Subject: [PATCH 5/5] Update classes/privacy/provider.php fixes phpunit errors Co-authored-by: Daniel Ziegenberg --- classes/privacy/provider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/classes/privacy/provider.php b/classes/privacy/provider.php index 95e3e7f69d..566ac160ea 100644 --- a/classes/privacy/provider.php +++ b/classes/privacy/provider.php @@ -158,7 +158,7 @@ public static function get_contexts_for_userid(int $userid) : contextlist { SELECT 1 FROM {moodleoverflow_tracking} track WHERE track.moodleoverflowid = mof.id AND track.userid = :tuserid ) OR EXISTS ( SELECT 1 FROM {moodleoverflow_grades} g WHERE g.moodleoverflowid = mof.id AND g.userid = :guserid - );"; + )"; $params = [ 'modname' => 'moodleoverflow',