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 <[email protected]>

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');
 

reportbuilder/tests/manager_test.php

@@ -18,14 +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
  *
@@ -34,7 +39,7 @@ use core_reportbuilder\local\report\base;
  * @copyright   2020 Paul Holden <[email protected]>
  * @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
@@ -54,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
      */