MDL-67818 check: Improved Check API to reduce boilerplate

moodle · Apr 6, 2020 · 26bb3c2 · 26bb3c2
1 parent 71576a5
commit 26bb3c2
Show file tree

Hide file tree

Showing 21 changed files with 233 additions and 125 deletions.
diff --git a/lib/classes/check/access/defaultuserrole.php b/lib/classes/check/access/defaultuserrole.php
@@ -41,13 +41,22 @@
 class defaultuserrole extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
+        return get_string('check_defaultuserrole_name', 'report_security');
+    }
+
+    /**
+     * A link to a place to action this
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
         global $CFG;
-        $this->id = 'defaultuserrole';
-        $this->name = get_string('check_defaultuserrole_name', 'report_security');
-        $this->actionlink = new \action_link(
+        return new \action_link(
             new \moodle_url('/admin/roles/define.php?action=view&roleid=' . $CFG->defaultuserroleid),
             get_string('userpolicies', 'admin'));
     }

diff --git a/lib/classes/check/access/frontpagerole.php b/lib/classes/check/access/frontpagerole.php
@@ -41,12 +41,21 @@
 class frontpagerole extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
-        $this->id = 'frontpagerole';
-        $this->name = get_string('check_frontpagerole_name', 'report_security');
-        $this->actionlink = new \action_link(
+    public function get_name(): string {
+        return get_string('check_frontpagerole_name', 'report_security');
+    }
+
+    /**
+     * A link to a place to action this
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/settings.php?section=frontpagesettings#admin-defaultfrontpageroleid'),
             get_string('frontpagesettings', 'admin'));
     }

diff --git a/lib/classes/check/access/guestrole.php b/lib/classes/check/access/guestrole.php
@@ -41,13 +41,21 @@
 class guestrole extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
+        return get_string('check_guestrole_name', 'report_security');
+    }
 
-        $this->id = 'guestrole';
-        $this->name = get_string('check_guestrole_name', 'report_security');
-        $this->actionlink = new \action_link(
+    /**
+     * A link to a place to action this
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/settings.php?section=userpolicies'),
             get_string('userpolicies', 'admin'));
     }

diff --git a/lib/classes/check/access/riskadmin.php b/lib/classes/check/access/riskadmin.php
@@ -41,12 +41,21 @@
 class riskadmin extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
-        $this->id = 'riskadmin';
-        $this->name = get_string('check_riskadmin_name', 'report_security');
-        $this->actionlink = new \action_link(
+    public function get_name(): string {
+        return get_string('check_riskadmin_name', 'report_security');
+    }
+
+    /**
+     * A link to a place to action this
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/roles/admins.php'),
             get_string('siteadministrators', 'role'));
     }

diff --git a/lib/classes/check/access/riskbackup.php b/lib/classes/check/access/riskbackup.php
@@ -39,13 +39,21 @@
 class riskbackup extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
+        return get_string('check_riskbackup_name', 'report_security');
+    }
 
-        $this->id = 'riskbackup';
-        $this->name = get_string('check_riskbackup_name', 'report_security');
-        $this->actionlink = new \action_link(
+    /**
+     * A link to a place to action this
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/roles/manage.php'),
             get_string('manageroles', 'role'));
     }

diff --git a/lib/classes/check/access/riskxss.php b/lib/classes/check/access/riskxss.php
@@ -46,13 +46,21 @@
 class riskxss extends \core\check\check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
+        return get_string('check_riskxss_name', 'report_security');
+    }
 
-        $this->id = 'riskxss';
-        $this->name = get_string('check_riskxss_name', 'report_security');
-        $this->actionlink = new \action_link(
+    /**
+     * A link to a place to action this
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/roles/manage.php'),
             get_string('manageroles', 'role'));
     }

diff --git a/lib/classes/check/check.php b/lib/classes/check/check.php
@@ -41,21 +41,6 @@ abstract class check {
      */
     protected $component = 'core';
 
-    /**
-     * @var string $id - Should be unique identifier within a component.
-     */
-    protected $id = '';
-
-    /**
-     * @var string $name - Name for the check, should be the same regardless of state.
-     */
-    protected $name = '';
-
-    /**
-     * @var action_link - an optional link to a place to address the check.
-     */
-    protected $actionlink = null;
-
     /**
      * Get the frankenstyle component name
      *
@@ -77,10 +62,16 @@ public function set_component(string $component) {
     /**
      * Get the check's id
      *
-     * @return string must be unique for it's component
+     * This defaults to the base name of the class which is ok in the most
+     * cases but if you have a check which can have multiple instances then
+     * you should override this to be unique.
+     *
+     * @return string must be unique within a component
      */
     public function get_id(): string {
-        return $this->id;
+        $class = get_class($this);
+        $id = explode("\\", $class);
+        return end($id);
     }
 
     /**
@@ -103,7 +94,8 @@ public function get_ref(): string {
      * @return string
      */
     public function get_name(): string {
-        return $this->name;
+        $id = $this->get_id();
+        return get_string("check{$id}", $this->get_component());
     }
 
     /**
@@ -112,7 +104,7 @@ public function get_name(): string {
      * @return action_link|null
      */
     public function get_action_link(): ?\action_link {
-        return $this->actionlink;
+        return null;
     }
 
     /**

diff --git a/lib/classes/check/environment/configrw.php b/lib/classes/check/environment/configrw.php
@@ -41,12 +41,12 @@
 class configrw extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
-        global $CFG;
-        $this->id = 'configrw';
-        $this->name = get_string('check_configrw_name', 'report_security');
+    public function get_name(): string {
+        return get_string('check_configrw_name', 'report_security');
     }
 
     /**

diff --git a/lib/classes/check/environment/displayerrors.php b/lib/classes/check/environment/displayerrors.php
@@ -47,12 +47,12 @@
 class displayerrors extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
-
-        $this->id = 'displayerrors';
-        $this->name = get_string('check_displayerrors_name', 'report_security');
+    public function get_name(): string {
+        return get_string('check_displayerrors_name', 'report_security');
     }
 
     /**

diff --git a/lib/classes/check/environment/nodemodules.php b/lib/classes/check/environment/nodemodules.php
@@ -41,12 +41,12 @@
 class nodemodules extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
-        global $CFG;
-        $this->id = 'nodemodules';
-        $this->name = get_string('check_nodemodules_name', 'report_security');
+    public function get_name(): string {
+        return get_string('check_nodemodules_name', 'report_security');
     }
 
     /**

diff --git a/lib/classes/check/environment/preventexecpath.php b/lib/classes/check/environment/preventexecpath.php
@@ -41,13 +41,12 @@
 class preventexecpath extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
-
-        global $CFG;
-        $this->id = 'preventexecpath';
-        $this->name = get_string('check_preventexecpath_name', 'report_security');
+    public function get_name(): string {
+        return get_string('check_preventexecpath_name', 'report_security');
     }
 
     /**

diff --git a/lib/classes/check/environment/unsecuredataroot.php b/lib/classes/check/environment/unsecuredataroot.php
@@ -40,14 +40,12 @@
 class unsecuredataroot extends \core\check\check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
-
-        global $CFG;
-
-        $this->id = 'unsecuredataroot';
-        $this->name = get_string('check_unsecuredataroot_name', 'report_security');
+    public function get_name(): string {
+        return get_string('check_unsecuredataroot_name', 'report_security');
     }
 
     /**

diff --git a/lib/classes/check/environment/vendordir.php b/lib/classes/check/environment/vendordir.php
@@ -41,12 +41,12 @@
 class vendordir extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
-        global $CFG;
-        $this->id = 'vendordir';
-        $this->name = get_string('check_vendordir_name', 'report_security');
+    public function get_name(): string {
+        return get_string('check_vendordir_name', 'report_security');
     }
 
     /**

diff --git a/lib/classes/check/http/cookiesecure.php b/lib/classes/check/http/cookiesecure.php
@@ -45,13 +45,21 @@
 class cookiesecure extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
+        return get_string('check_cookiesecure_name', 'report_security');
+    }
 
-        $this->id = 'cookiesecure';
-        $this->name = get_string('check_cookiesecure_name', 'report_security');
-        $this->actionlink = new \action_link(
+    /**
+     * A link to a place to action this
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/settings.php?section=httpsecurity#admin-cookiesecure'),
             get_string('httpsecurity', 'admin'));
     }

diff --git a/lib/classes/check/security/crawlers.php b/lib/classes/check/security/crawlers.php
@@ -47,13 +47,21 @@
 class crawlers extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
+        return get_string('check_crawlers_name', 'report_security');
+    }
 
-        $this->id = 'crawlers';
-        $this->name = get_string('check_crawlers_name', 'report_security');
-        $this->actionlink = new \action_link(
+    /**
+     * A link to a place to action this
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/settings.php?section=sitepolicies#admin-opentowebcrawlers'),
             get_string('sitepolicies', 'admin'));
     }