From b60518501f6aeedad5d579d90e470a5b42af4e18 Mon Sep 17 00:00:00 2001
From: Paul Holden <paulh@moodle.com>
Date: Mon, 27 Feb 2023 21:09:39 +0000
Subject: [PATCH] MDL-77321 reportbuilder: cache list of loaded reports per
 user.

Since 0188af39 we've cached loaded reports, however there was an
edge case in those report sources that relied on the current user
as part of their own initialization (e.g. checking capabilities).

Co-authored-by: Marina Glancy <marina@moodle.com>
---
 reportbuilder/classes/manager.php    |  8 +++--
 reportbuilder/tests/manager_test.php | 49 ++++++++++++++++++++++++++--
 2 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/reportbuilder/classes/manager.php b/reportbuilder/classes/manager.php
index 8fdc18f680d..0d30ff80adc 100644
--- a/reportbuilder/classes/manager.php
+++ b/reportbuilder/classes/manager.php
@@ -40,7 +40,7 @@ class manager {
     /**
      * Return an instance of a report class from the given report persistent
      *
-     * We statically cache the list of loaded reports during request lifecycle, to allow this method to be called
+     * We statically cache the list of loaded reports per user during request lifecycle, to allow this method to be called
      * repeatedly without potential performance problems initialising the same report multiple times
      *
      * @param report $report
@@ -50,7 +50,11 @@ class manager {
      * @throws source_unavailable_exception
      */
     public static function get_report_from_persistent(report $report, array $parameters = []): base {
-        $instancekey = $report->get('id');
+        global $USER;
+
+        // Cached instance per report/user, to account for initialization dependent on current user.
+        $instancekey = $report->get('id') . ':' . ($USER->id ?? 0);
+
         if (!array_key_exists($instancekey, static::$instances)) {
             $source = $report->get('source');
 
diff --git a/reportbuilder/tests/manager_test.php b/reportbuilder/tests/manager_test.php
index 72476af64f5..c6dc6399e9f 100644
--- a/reportbuilder/tests/manager_test.php
+++ b/reportbuilder/tests/manager_test.php
@@ -18,12 +18,19 @@ declare(strict_types=1);
 
 namespace core_reportbuilder;
 
-use advanced_testcase;
 use context_system;
+use core_reportbuilder_generator;
+use core_reportbuilder_testcase;
+use core_user\reportbuilder\datasource\users;
 use stdClass;
 use core_reportbuilder\local\models\report;
 use core_reportbuilder\local\report\base;
 
+defined('MOODLE_INTERNAL') || die();
+
+global $CFG;
+require_once("{$CFG->dirroot}/reportbuilder/tests/helpers.php");
+
 /**
  * Unit tests for the report manager class
  *
@@ -32,7 +39,7 @@ use core_reportbuilder\local\report\base;
  * @copyright   2020 Paul Holden <paulh@moodle.com>
  * @license     http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class manager_test extends advanced_testcase {
+class manager_test extends core_reportbuilder_testcase {
 
     /**
      * Test creating a report instance from persistent
@@ -52,6 +59,44 @@ class manager_test extends advanced_testcase {
         $this->assertInstanceOf(system_report::class, $systemreport);
     }
 
+    /**
+     * Test creating a report instance from persistent differs per-user, using a report source whose own initialization is
+     * dependent on the current user (the users report source, loading available user profile fields)
+     *
+     * Note: internally the {@see get_custom_report_content} test helper calls {@see manager::get_report_from_persistent}
+     */
+    public function test_get_report_from_persistent_per_user(): void {
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        // Custom profile field, visible only to the admin.
+        $this->getDataGenerator()->create_custom_profile_field([
+            'shortname' => 'text', 'name' => 'Text field', 'datatype' => 'text', 'visible' => 0]);
+        $user = $this->getDataGenerator()->create_user(['username' => 'usertwo', 'profile_field_text' => 'Hello']);
+
+        /** @var core_reportbuilder_generator $generator */
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
+
+        $report = $generator->create_report(['name' => 'Hidden profile field', 'source' => users::class, 'default' => 0]);
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:username', 'sortenabled' => 1]);
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:profilefield_text']);
+
+        $content = $this->get_custom_report_content($report->get('id'));
+        $this->assertEquals([
+            ['admin', ''],
+            ['usertwo', 'Hello'],
+        ], array_map('array_values', $content));
+
+        // Now switch to second, non-admin, user.
+        $this->setUser($user);
+
+        $content = $this->get_custom_report_content($report->get('id'));
+        $this->assertEquals([
+            ['admin'],
+            ['usertwo'],
+        ], array_map('array_values', $content));
+    }
+
     /**
      * Test creating a report instance from persistent with an invalid source
      */