Prevent useless optional/out of context requirement checks

glpi-project · Dec 18, 2023 · 33c1964 · 33c1964
1 parent 8d8f4d2
commit 33c1964
Show file tree

Hide file tree

Showing 22 changed files with 177 additions and 85 deletions.
diff --git a/src/System/Requirement/AbstractRequirement.php b/src/System/Requirement/AbstractRequirement.php
@@ -50,35 +50,35 @@ abstract class AbstractRequirement implements RequirementInterface
     /**
      * Flag that indicates if requirement is considered as optional.
      *
-     * @var bool
+     * @var bool|null
      */
-    protected $optional = false;
+    protected $optional;
 
     /**
      * Flag that indicates if requirement is recommended for security reasons.
      *
-     * @var bool
+     * @var bool|null
      */
-    protected bool $recommended_for_security = false;
+    protected ?bool $recommended_for_security;
 
     /**
      * Flag that indicates if requirement is considered as out of context.
      *
-     * @var bool
+     * @var bool|null
      */
-    protected $out_of_context = false;
+    protected $out_of_context;
 
     /**
      * Requirement title.
      *
-     * @var string
+     * @var string|null
      */
     protected $title;
 
     /**
      * Requirement description.
      *
-     * @var string
+     * @var string|null
      */
     protected $description;
 
@@ -96,6 +96,20 @@ abstract class AbstractRequirement implements RequirementInterface
      */
     protected $validation_messages = [];
 
+    public function __construct(
+        ?string $title,
+        ?string $description = null,
+        ?bool $optional = false,
+        ?bool $recommended_for_security = false,
+        ?bool $out_of_context = false
+    ) {
+        $this->title = $title;
+        $this->description = $description;
+        $this->optional = $optional;
+        $this->recommended_for_security = $recommended_for_security;
+        $this->out_of_context = $out_of_context;
+    }
+
     /**
      * Check requirement.
      *
@@ -121,13 +135,23 @@ private function doCheck()
 
     public function getTitle(): string
     {
+        if ($this->title !== null) {
+            // No need to run checks if variable is defined by constructor.
+            return $this->title;
+        }
+
         $this->doCheck();
 
-        return $this->title;
+        return $this->title ?? '';
     }
 
     public function getDescription(): ?string
     {
+        if ($this->description !== null) {
+            // No need to run checks if variable is defined by constructor.
+            return $this->description;
+        }
+
         $this->doCheck();
 
         return $this->description;
@@ -149,23 +173,38 @@ public function isMissing(): bool
 
     public function isOptional(): bool
     {
+        if ($this->optional !== null) {
+            // No need to run checks if variable is defined by constructor.
+            return $this->optional;
+        }
+
         $this->doCheck();
 
-        return $this->optional;
+        return $this->optional ?? false;
     }
 
     public function isRecommendedForSecurity(): bool
     {
+        if ($this->recommended_for_security !== null) {
+            // No need to run checks if variable is defined by constructor.
+            return $this->recommended_for_security;
+        }
+
         $this->doCheck();
 
-        return $this->recommended_for_security;
+        return $this->recommended_for_security ?? false;
     }
 
     public function isOutOfContext(): bool
     {
+        if ($this->out_of_context !== null) {
+            // No need to run checks if variable is defined by constructor.
+            return $this->out_of_context;
+        }
+
         $this->doCheck();
 
-        return $this->out_of_context;
+        return $this->out_of_context ?? false;
     }
 
     public function isValidated(): bool

diff --git a/src/System/Requirement/DataDirectoriesProtectedPath.php b/src/System/Requirement/DataDirectoriesProtectedPath.php
@@ -71,10 +71,12 @@ public function __construct(
         string $var_root_constant = 'GLPI_VAR_DIR',
         string $glpi_root_directory = GLPI_ROOT
     ) {
-        $this->title = __('Safe path for data directories');
-        $this->description = __('GLPI data directories should be placed outside web root directory. It can be achieved by redefining corresponding constants. See installation documentation for more details.');
-        $this->optional = true;
-        $this->recommended_for_security = true;
+        parent::__construct(
+            __('Safe path for data directories'),
+            __('GLPI data directories should be placed outside web root directory. It can be achieved by redefining corresponding constants. See installation documentation for more details.'),
+            true,
+            true
+        );
 
         $this->directories_constants = $directories_constants;
         $this->var_root_constant     = $var_root_constant;

diff --git a/src/System/Requirement/DbConfiguration.php b/src/System/Requirement/DbConfiguration.php
@@ -51,7 +51,10 @@ class DbConfiguration extends AbstractRequirement
 
     public function __construct(DBmysql $db)
     {
-        $this->title = __('DB configuration');
+        parent::__construct(
+            __('DB configuration')
+        );
+
         $this->db = $db;
     }
 

diff --git a/src/System/Requirement/DbEngine.php b/src/System/Requirement/DbEngine.php
@@ -49,7 +49,10 @@ class DbEngine extends AbstractRequirement
 
     public function __construct(\DBmysql $db)
     {
-        $this->title = __('DB engine version');
+        parent::__construct(
+            __('DB engine version')
+        );
+
         $this->db = $db;
     }
 

diff --git a/src/System/Requirement/DbTimezones.php b/src/System/Requirement/DbTimezones.php
@@ -49,10 +49,13 @@ class DbTimezones extends AbstractRequirement
 
     public function __construct(\DBmysql $db)
     {
-        $this->title = __('DB timezone data');
-        $this->description = __('Enable usage of timezones.');
+        parent::__construct(
+            __('DB timezone data'),
+            __('Enable usage of timezones.'),
+            true
+        );
+
         $this->db = $db;
-        $this->optional = true;
     }
 
     protected function check()

diff --git a/src/System/Requirement/DirectoriesWriteAccess.php b/src/System/Requirement/DirectoriesWriteAccess.php
@@ -54,9 +54,9 @@ class DirectoriesWriteAccess extends AbstractRequirement
      */
     public function __construct(string $title, array $paths, bool $optional = false)
     {
-        $this->title = $title;
+        parent::__construct($title, null, $optional);
+
         $this->paths = $paths;
-        $this->optional = $optional;
     }
 
     protected function check()

diff --git a/src/System/Requirement/DirectoryWriteAccess.php b/src/System/Requirement/DirectoryWriteAccess.php
@@ -54,57 +54,57 @@ class DirectoryWriteAccess extends AbstractRequirement
      */
     public function __construct(string $path, bool $optional = false, ?string $description = null)
     {
-        $this->path = $path;
-        $this->optional = $optional;
-        $this->description = $description;
-
-        switch (realpath($this->path)) {
+        switch (realpath($path)) {
             case realpath(GLPI_CACHE_DIR):
-                $this->title = __('Permissions for cache files');
+                $title = __('Permissions for cache files');
                 break;
             case realpath(GLPI_CONFIG_DIR):
-                $this->title = __('Permissions for setting files');
+                $title = __('Permissions for setting files');
                 break;
             case realpath(GLPI_CRON_DIR):
-                $this->title = __('Permissions for automatic actions files');
+                $title = __('Permissions for automatic actions files');
                 break;
             case realpath(GLPI_DOC_DIR):
-                $this->title = __('Permissions for document files');
+                $title = __('Permissions for document files');
                 break;
             case realpath(GLPI_DUMP_DIR):
-                $this->title = __('Permissions for dump files');
+                $title = __('Permissions for dump files');
                 break;
             case realpath(GLPI_GRAPH_DIR):
-                $this->title = __('Permissions for graphic files');
+                $title = __('Permissions for graphic files');
                 break;
             case realpath(GLPI_LOCK_DIR):
-                $this->title = __('Permissions for lock files');
+                $title = __('Permissions for lock files');
                 break;
             case realpath(GLPI_MARKETPLACE_DIR):
-                $this->title = __('Permissions for marketplace directory');
+                $title = __('Permissions for marketplace directory');
                 break;
             case realpath(GLPI_PLUGIN_DOC_DIR):
-                $this->title = __('Permissions for plugins document files');
+                $title = __('Permissions for plugins document files');
                 break;
             case realpath(GLPI_PICTURE_DIR):
-                $this->title = __('Permissions for pictures files');
+                $title = __('Permissions for pictures files');
                 break;
             case realpath(GLPI_RSS_DIR):
-                $this->title = __('Permissions for rss files');
+                $title = __('Permissions for rss files');
                 break;
             case realpath(GLPI_SESSION_DIR):
-                $this->title = __('Permissions for session files');
+                $title = __('Permissions for session files');
                 break;
             case realpath(GLPI_TMP_DIR):
-                $this->title = __('Permissions for temporary files');
+                $title = __('Permissions for temporary files');
                 break;
             case realpath(GLPI_UPLOAD_DIR):
-                $this->title = __('Permissions for upload files');
+                $title = __('Permissions for upload files');
                 break;
             default:
-                $this->title = sprintf(__('Permissions for directory %s'), $this->path);
+                $title = sprintf(__('Permissions for directory %s'), $path);
                 break;
         }
+
+        parent::__construct($title, $description, $optional);
+
+        $this->path = $path;
     }
 
     protected function check()

diff --git a/src/System/Requirement/Extension.php b/src/System/Requirement/Extension.php
@@ -54,10 +54,13 @@ class Extension extends AbstractRequirement
      */
     public function __construct(string $name, bool $optional = false, ?string $description = null)
     {
-        $this->title = sprintf(__('%s extension'), $name);
+        parent::__construct(
+            sprintf(__('%s extension'), $name),
+            $description,
+            $optional
+        );
+
         $this->name = $name;
-        $this->optional = $optional;
-        $this->description = $description;
     }
 
     protected function check()

diff --git a/src/System/Requirement/ExtensionConstant.php b/src/System/Requirement/ExtensionConstant.php
@@ -55,10 +55,13 @@ class ExtensionConstant extends AbstractRequirement
      */
     public function __construct(string $title, string $name, bool $optional = false, string $description = '')
     {
-        $this->title = $title;
+        parent::__construct(
+            $title,
+            $description,
+            $optional
+        );
+
         $this->name = $name;
-        $this->optional = $optional;
-        $this->description = $description;
     }
 
     protected function check()

diff --git a/src/System/Requirement/ExtensionGroup.php b/src/System/Requirement/ExtensionGroup.php
@@ -55,10 +55,13 @@ class ExtensionGroup extends AbstractRequirement
      */
     public function __construct(string $title, array $extensions, bool $optional = false, ?string $description = null)
     {
-        $this->title = $title;
+        parent::__construct(
+            $title,
+            $description,
+            $optional
+        );
+
         $this->extensions = $extensions;
-        $this->optional = $optional;
-        $this->description = $description;
     }
 
     protected function check()

diff --git a/src/System/Requirement/InstallationNotOverriden.php b/src/System/Requirement/InstallationNotOverriden.php
@@ -58,11 +58,16 @@ final class InstallationNotOverriden extends AbstractRequirement
 
     public function __construct(?DBmysql $db, string $version_dir = GLPI_ROOT . '/version')
     {
+        parent::__construct(
+            __('Previous GLPI version files detection'),
+            __('The presence of source files from previous versions of GLPI can lead to security issues or bugs.'),
+            false,
+            false,
+            null // $out_of_context will be computed on check
+        );
+
         $this->db = $db;
         $this->version_dir = $version_dir;
-
-        $this->title = __('Previous GLPI version files detection');
-        $this->description = __('The presence of source files from previous versions of GLPI can lead to security issues or bugs.');
     }
 
     protected function check()

diff --git a/src/System/Requirement/IntegerSize.php b/src/System/Requirement/IntegerSize.php
@@ -39,9 +39,11 @@ final class IntegerSize extends AbstractRequirement
 {
     public function __construct()
     {
-        $this->title = __('PHP maximal integer size');
-        $this->description = __('Support of 64 bits integers is required for IP addresses related operations (network inventory, API clients IP filtering, ...).');
-        $this->optional = true;
+        parent::__construct(
+            __('PHP maximal integer size'),
+            __('Support of 64 bits integers is required for IP addresses related operations (network inventory, API clients IP filtering, ...).'),
+            true
+        );
     }
 
     protected function check()

diff --git a/src/System/Requirement/LogsWriteAccess.php b/src/System/Requirement/LogsWriteAccess.php
@@ -55,8 +55,11 @@ class LogsWriteAccess extends AbstractRequirement
      */
     public function __construct(LoggerInterface $logger)
     {
+        parent::__construct(
+            __('Permissions for log files')
+        );
+
         $this->logger = $logger;
-        $this->title = __('Permissions for log files');
     }
 
     protected function check()

diff --git a/src/System/Requirement/MemoryLimit.php b/src/System/Requirement/MemoryLimit.php
@@ -52,7 +52,10 @@ class MemoryLimit extends AbstractRequirement
      */
     public function __construct(int $min)
     {
-        $this->title = __('Allocated memory');
+        parent::__construct(
+            __('Allocated memory')
+        );
+
         $this->min = $min;
     }