MDL-78907 mod_bigbluebuttonbn: Refactor instance class

* Make the constructor private as it should only be instancied through
static methods
* Do not duplicate the code between get_from_instanceid,
get_from_cmid and get_all_instances_in_course

mod/bigbluebuttonbn/classes/instance.php

@@ -20,6 +20,7 @@ use cm_info;
 use context;
 use context_course;
 use context_module;
+use core\dml\table;
 use mod_bigbluebuttonbn\local\config;
 use mod_bigbluebuttonbn\local\helpers\files;
 use mod_bigbluebuttonbn\local\helpers\roles;
@@ -64,14 +65,16 @@ class instance {
     protected $groupid;
 
     /**
-     * instance constructor.
+     * Instance constructor.
+     *
+     * Never called directly. Use self::get_from_instanceid or self::get_from_cmid.
      *
      * @param cm_info $cm
      * @param stdClass $course
      * @param stdClass $instancedata
      * @param int|null $groupid
      */
-    public function __construct(cm_info $cm, stdClass $course, stdClass $instancedata, ?int $groupid = null) {
+    private function __construct(cm_info $cm, stdClass $course, stdClass $instancedata, ?int $groupid = null) {
         $this->cm = $cm;
         $this->course = $course;
         $this->instancedata = $instancedata;
@@ -101,42 +104,7 @@ class instance {
      * @return null|self
      */
     public static function get_from_instanceid(int $instanceid): ?self {
-        global $DB;
-
-        $coursetable = new \core\dml\table('course', 'c', 'c');
-        $courseselect = $coursetable->get_field_select();
-        $coursefrom = $coursetable->get_from_sql();
-
-        $cmtable = new \core\dml\table('course_modules', 'cm', 'cm');
-        $cmfrom = $cmtable->get_from_sql();
-
-        $bbbtable = new \core\dml\table('bigbluebuttonbn', 'bbb', 'b');
-        $bbbselect = $bbbtable->get_field_select();
-        $bbbfrom = $bbbtable->get_from_sql();
-
-        $sql = <<<EOF
-    SELECT {$courseselect}, {$bbbselect}
-      FROM {$cmfrom}
-INNER JOIN {$coursefrom} ON c.id = cm.course
-INNER JOIN {modules} m ON m.id = cm.module AND m.name = :modname
-INNER JOIN {$bbbfrom} ON cm.instance = bbb.id
-     WHERE bbb.id = :instanceid
-EOF;
-
-        $result = $DB->get_record_sql($sql, [
-            'modname' => 'bigbluebuttonbn',
-            'instanceid' => $instanceid,
-        ]);
-
-        if (empty($result)) {
-            return null;
-        }
-
-        $course = $coursetable->extract_from_result($result);
-        $instancedata = $bbbtable->extract_from_result($result);
-        $cm = get_fast_modinfo($course)->instances['bigbluebuttonbn'][$instancedata->id];
-
-        return new self($cm, $course, $instancedata);
+        return self::get_instance_info_retriever($instanceid, self::IDTYPE_INSTANCEID);
     }
 
     /**
@@ -146,32 +114,72 @@ EOF;
      * @return null|self
      */
     public static function get_from_cmid(int $cmid): ?self {
+        return self::get_instance_info_retriever($cmid, self::IDTYPE_CMID);
+    }
+
+    /**
+     * Get the instance information from a cmid.
+     */
+    const IDTYPE_CMID = 0;
+    /**
+     * Get the instance information from an id.
+     */
+    const IDTYPE_INSTANCEID = 1;
+
+    /**
+     * Helper to get the instance information from an id.
+     *
+     * Used by self::get_from_id and self::get_cmid.
+     *
+     * @param string $id The id to look for.
+     * @param int $idtype self::IDTYPE_CMID or self::IDTYPE_INSTANCEID
+     * @return null|self
+     * @throws \moodle_exception
+     */
+    private static function get_instance_info_retriever(string $id, int $idtype = self::IDTYPE_INSTANCEID): ?self {
         global $DB;
 
-        $coursetable = new \core\dml\table('course', 'c', 'c');
-        $courseselect = $coursetable->get_field_select();
-        $coursefrom = $coursetable->get_from_sql();
+        if (!in_array($idtype, [self::IDTYPE_CMID, self::IDTYPE_INSTANCEID])) {
+            throw new \moodle_exception('Invalid idtype');
+        }
 
-        $cmtable = new \core\dml\table('course_modules', 'cm', 'cm');
-        $cmfrom = $cmtable->get_from_sql();
+        [
+            'coursetable' => $coursetable,
+            'courseselect' => $courseselect,
+            'coursefrom' => $coursefrom,
+            'cmfrom' => $cmfrom,
+            'cmselect' => $cmselect,
+            'bbbtable' => $bbbtable,
+            'bbbselect' => $bbbselect,
+            'bbbfrom' => $bbbfrom,
+        ] = self::get_tables_info();
 
-        $bbbtable = new \core\dml\table('bigbluebuttonbn', 'bbb', 'b');
-        $bbbselect = $bbbtable->get_field_select();
-        $bbbfrom = $bbbtable->get_from_sql();
-
-        $sql = <<<EOF
-    SELECT {$courseselect}, {$bbbselect}
-      FROM {$cmfrom}
-INNER JOIN {$coursefrom} ON c.id = cm.course
-INNER JOIN {modules} m ON m.id = cm.module AND m.name = :modname
-INNER JOIN {$bbbfrom} ON cm.instance = bbb.id
-     WHERE cm.id = :cmid
-EOF;
-
-        $result = $DB->get_record_sql($sql, [
+        $select = implode(', ', [$courseselect, $bbbselect, $cmselect]);
+        $params = [
             'modname' => 'bigbluebuttonbn',
-            'cmid' => $cmid,
-        ]);
+            'bbbid' => $id,
+        ];
+        $where = 'bbb.id = :bbbid';
+        $from = <<<EOF
+                {$bbbfrom}
+                INNER JOIN {$cmfrom} ON cm.instance = bbb.id
+                INNER JOIN {$coursefrom} ON c.id = cm.course
+                INNER JOIN {modules} m ON m.id = cm.module AND m.name = :modname
+EOF;
+        if ($idtype == self::IDTYPE_CMID) {
+            $params['cmid'] = $id;
+            $where = 'cm.id = :cmid';
+            $from = <<<EOF
+                {$cmfrom}
+                INNER JOIN {$coursefrom} ON c.id = cm.course
+                INNER JOIN {modules} m ON m.id = cm.module AND m.name = :modname
+                INNER JOIN {$bbbfrom} ON cm.instance = bbb.id
+EOF;
+        }
+
+        $sql = "SELECT {$select} FROM {$from} WHERE {$where}";
+
+        $result = $DB->get_record_sql($sql, $params);
 
         if (empty($result)) {
             return null;
@@ -179,8 +187,13 @@ EOF;
 
         $course = $coursetable->extract_from_result($result);
         $instancedata = $bbbtable->extract_from_result($result);
-        $cm = get_fast_modinfo($course)->get_cm($cmid);
-
+        if ($idtype == self::IDTYPE_INSTANCEID) {
+            $cmid = $result->cmid;
+        } else {
+            $cmid = $id;
+        }
+        $modinfo = get_fast_modinfo($course);
+        $cm = $modinfo->get_cm($cmid);
         return new self($cm, $course, $instancedata);
     }
 
@@ -233,20 +246,19 @@ EOF;
      */
     public static function get_all_instances_in_course(int $courseid): array {
         global $DB;
+        [
+            'coursetable' => $coursetable,
+            'courseselect' => $courseselect,
+            'coursefrom' => $coursefrom,
+            'cmfrom' => $cmfrom,
+            'bbbtable' => $bbbtable,
+            'bbbselect' => $bbbselect,
+            'bbbfrom' => $bbbfrom
+        ] = self::get_tables_info();
 
-        $coursetable = new \core\dml\table('course', 'c', 'c');
-        $courseselect = $coursetable->get_field_select();
-        $coursefrom = $coursetable->get_from_sql();
-
-        $cmtable = new \core\dml\table('course_modules', 'cm', 'cm');
-        $cmfrom = $cmtable->get_from_sql();
-
-        $bbbtable = new \core\dml\table('bigbluebuttonbn', 'bbb', 'b');
-        $bbbselect = $bbbtable->get_field_select();
-        $bbbfrom = $bbbtable->get_from_sql();
-
+        $selects = implode(', ', [$courseselect, $bbbselect]);
         $sql = <<<EOF
-    SELECT cm.id as cmid, {$courseselect}, {$bbbselect}
+    SELECT cm.id as cmid, {$selects}
       FROM {$cmfrom}
 INNER JOIN {$coursefrom} ON c.id = cm.course
 INNER JOIN {modules} m ON m.id = cm.module AND m.name = :modname
@@ -269,7 +281,32 @@ EOF;
 
         return $instances;
     }
+    // Add an helper method that can be used in both self::get_all_instances_in_course and self::get_all_instances_in_course.
+    // This method will be used to get the additional tables returned from the subplugin.
+    /**
+     * Get the additional tables returned from the subplugin.
+     *
+     * @return array
+     */
+    private static function get_tables_info(): array {
+        $coursetable = new table('course', 'c', 'c');
+        $courseselect = $coursetable->get_field_select();
+        $coursefrom = $coursetable->get_from_sql();
 
+        $cmtable = new table('course_modules', 'cm', 'cm');
+        $cmselect = $cmtable->get_field_select();
+        $cmfrom = $cmtable->get_from_sql();
+
+        $bbbtable = new table('bigbluebuttonbn', 'bbb', 'b');
+        $bbbselect = $bbbtable->get_field_select();
+        $bbbfrom = $bbbtable->get_from_sql();
+
+        return compact(
+            'coursetable', 'courseselect', 'coursefrom',
+            'cmtable', 'cmselect', 'cmfrom',
+            'bbbtable', 'bbbselect', 'bbbfrom'
+        );
+    }
     /**
      * Set the current group id of the activity.
      *

mod/bigbluebuttonbn/tests/instance_test.php

@@ -588,6 +588,26 @@ class instance_test extends advanced_testcase {
         $this->assertFalse($instance->is_guest_allowed());
     }
 
+    /**
+     * Test private method get_instance_info_retriever
+     *
+     * @covers ::get_instance_info_retriever
+     */
+    public function test_get_instance_info_retriever() {
+        $this->resetAfterTest();
+        [
+            'record' => $record,
+            'cm' => $cm,
+        ] = $this->get_test_instance();
+        $instance = instance::get_from_instanceid($record->id);
+        $instancereflection = new \ReflectionClass($instance);
+        $getinstanceinforetriever = $instancereflection->getMethod('get_instance_info_retriever');
+        $getinstanceinforetriever->setAccessible(true);
+        $this->assertInstanceOf('\mod_bigbluebuttonbn\instance',
+            $getinstanceinforetriever->invoke($instance, $record->id, instance::IDTYPE_INSTANCEID));
+        $this->assertEquals($cm->id, $instance->get_cm_id());
+    }
+
     /**
      * Test guest access password
      *
@@ -601,5 +621,4 @@ class instance_test extends advanced_testcase {
         $instance = instance::get_from_instanceid($record->id);
         $this->assertNotEmpty($instance->get_guest_access_password());
     }
-
 }

mod/bigbluebuttonbn/upgrade.txt

@@ -1,4 +1,7 @@
 This files describes API changes in the bigbluebuttonbn code.
+=== 4.3 ===
+* Make instance class constructor private and use the factory method to create instances.
+
 === 4.2 ===
 * Simplify bigbluebutton_proxy get_join_url so to pass only parameters that cannot be deduced from the instance.
 * Add a new parameter for mod_bigbluebuttonbn\recording::sync_pending_recordings_from_server so sync only 'dismissed' recordings