Fix plugins check_config usability and make it optionnal

- check_config is now optionnal
- a TOBECONFIGURED plugin will now be loaded (for autoload purpose), in order to be configurable
- hooks will now be called only for activated plugins, instead of loaded plugins

CHANGELOG.md

@@ -77,6 +77,10 @@ The present file will list all changes made to the project; according to the
 - `Toolbox::userErrorHandlerNormal()`
 - `Html::jsDisable()`
 - `Html::jsEnable()`
+- `Plugin::setLoaded()`
+- `Plugin::setUnloaded()`
+- `Plugin::setUnloadedByName()`
+- Usage of `$LOADED_PLUGINS` global variable
 
 #### Removed
 

inc/autoload.function.php

@@ -288,9 +288,8 @@ function glpi_autoload($classname) {
       $plugname = strtolower($plug['plugin']);
       $dir      = GLPI_ROOT . "/plugins/$plugname/inc/";
       $item     = str_replace('\\', '/', strtolower($plug['class']));
-      // Is the plugin active?
       if (!Plugin::isPluginLoaded($plugname)) {
-         // Plugin not activated
+         // Do not load plugin class if plugin is not loaded
          return false;
       }
    } else {

inc/commonitilobject.class.php

@@ -4941,7 +4941,7 @@ abstract class CommonITILObject extends CommonDBTM {
       //Types of the plugins (keep the plugin hook for right check)
       if (isset($PLUGIN_HOOKS['assign_to_ticket'])) {
          foreach (array_keys($PLUGIN_HOOKS['assign_to_ticket']) as $plugin) {
-            if (!Plugin::isPluginLoaded($plugin)) {
+            if (!Plugin::isPluginActive($plugin)) {
                continue;
             }
             $ptypes = Plugin::doOneHook($plugin, 'AssignToTicket', $ptypes);

inc/config.class.php

@@ -94,7 +94,7 @@ class Config extends CommonDBTM {
    function canViewItem() {
       if (isset($this->fields['context']) &&
          ($this->fields['context'] == 'core' ||
-         Plugin::isPluginLoaded($this->fields['context']))) {
+         Plugin::isPluginActive($this->fields['context']))) {
          return true;
       }
       return false;

inc/console/application.class.php

@@ -408,16 +408,7 @@ class Application extends BaseApplication {
       }
 
       $plugin = new Plugin();
-      $plugin->init();
-
-      $plugins_list = $plugin->getPlugins();
-      if (count($plugins_list) > 0) {
-         foreach ($plugins_list as $name) {
-            Plugin::load($name);
-         }
-         // For plugins which require action after all plugin init
-         Plugin::doHook("post_init");
-      }
+      $plugin->init(true);
    }
 
    /**

inc/console/plugin/installcommand.class.php

@@ -295,20 +295,18 @@ class InstallCommand extends AbstractPluginCommand {
       Plugin::load($directory, true);
 
       // Check that required functions exists
-      foreach (['install', 'check_config'] as $fct_suffix) {
-         $function = 'plugin_' . $directory . '_' . $fct_suffix;
-         if (!function_exists($function)) {
-            $message = sprintf(
-               __('Plugin "%s" function "%s" is missing.'),
-               $directory,
-               $function
-            );
-            $this->output->writeln(
-               '<error>' . $message . '</error>',
-               OutputInterface::VERBOSITY_QUIET
-            );
-            return false;
-         }
+      $function = 'plugin_' . $directory . '_install';
+      if (!function_exists($function)) {
+         $message = sprintf(
+            __('Plugin "%s" function "%s" is missing.'),
+            $directory,
+            $function
+         );
+         $this->output->writeln(
+            '<error>' . $message . '</error>',
+            OutputInterface::VERBOSITY_QUIET
+         );
+         return false;
       }
 
       // Check prerequisites

inc/html.class.php

@@ -1335,7 +1335,7 @@ class Html {
       if (isset($PLUGIN_HOOKS['add_css']) && count($PLUGIN_HOOKS['add_css'])) {
 
          foreach ($PLUGIN_HOOKS["add_css"] as $plugin => $files) {
-            if (!Plugin::isPluginLoaded($plugin)) {
+            if (!Plugin::isPluginActive($plugin)) {
                continue;
             }
 
@@ -1495,7 +1495,7 @@ JAVASCRIPT;
          if (isset($PLUGIN_HOOKS["menu_toadd"]) && count($PLUGIN_HOOKS["menu_toadd"])) {
 
             foreach ($PLUGIN_HOOKS["menu_toadd"] as $plugin => $items) {
-               if (!Plugin::isPluginLoaded($plugin)) {
+               if (!Plugin::isPluginActive($plugin)) {
                   continue;
                }
                if (count($items)) {
@@ -6416,7 +6416,7 @@ JAVASCRIPT;
       if (isset($PLUGIN_HOOKS['add_javascript']) && count($PLUGIN_HOOKS['add_javascript'])) {
 
          foreach ($PLUGIN_HOOKS["add_javascript"] as $plugin => $files) {
-            if (!Plugin::isPluginLoaded($plugin)) {
+            if (!Plugin::isPluginActive($plugin)) {
                continue;
             }
             $version = Plugin::getInfo($plugin, 'version');
@@ -6845,7 +6845,7 @@ JAVASCRIPT;
             && count($PLUGIN_HOOKS["helpdesk_menu_entry"])) {
 
             foreach ($PLUGIN_HOOKS["helpdesk_menu_entry"] as $plugin => $active) {
-               if (!Plugin::isPluginLoaded($plugin)) {
+               if (!Plugin::isPluginActive($plugin)) {
                   continue;
                }
                if ($active) {

inc/includes.php

@@ -98,16 +98,7 @@ if (!isset($PLUGINS_INCLUDED)) {
    $PLUGINS_INCLUDED = 1;
    $LOADED_PLUGINS   = [];
    $plugin           = new Plugin();
-   $plugin->init();
-
-   $plugins_list = $plugin->getPlugins();
-   if (count($plugins_list)) {
-      foreach ($plugins_list as $name) {
-         Plugin::load($name);
-      }
-      // For plugins which require action after all plugin init
-      Plugin::doHook("post_init");
-   }
+   $plugin->init(true);
 }
 
 

inc/massiveaction.class.php

@@ -576,7 +576,7 @@ class MassiveAction {
          // Plugin Specific actions
          if (isset($PLUGIN_HOOKS['use_massive_action'])) {
             foreach (array_keys($PLUGIN_HOOKS['use_massive_action']) as $plugin) {
-               if (!Plugin::isPluginLoaded($plugin)) {
+               if (!Plugin::isPluginActive($plugin)) {
                   continue;
                }
                $plug_actions = Plugin::doOneHook($plugin, 'MassiveActions', $itemtype);

inc/report.class.php

@@ -124,7 +124,7 @@ class Report extends CommonGLPI{
       $optgroup = [];
       if (isset($PLUGIN_HOOKS["reports"]) && is_array($PLUGIN_HOOKS["reports"])) {
          foreach ($PLUGIN_HOOKS["reports"] as $plug => $pages) {
-            if (!Plugin::isPluginLoaded($plug)) {
+            if (!Plugin::isPluginActive($plug)) {
                continue;
             }
             if (is_array($pages) && count($pages)) {

inc/rule.class.php

@@ -1712,7 +1712,7 @@ class Rule extends CommonDBTM {
       $input = $this->prepareInputDataForProcess($input, $params);
       if (isset($PLUGIN_HOOKS['use_rules'])) {
          foreach ($PLUGIN_HOOKS['use_rules'] as $plugin => $val) {
-            if (!Plugin::isPluginLoaded($plugin)) {
+            if (!Plugin::isPluginActive($plugin)) {
                continue;
             }
             if (is_array($val) && in_array($this->getType(), $val)) {
@@ -1751,7 +1751,7 @@ class Rule extends CommonDBTM {
          $params['criterias_results'] = $this->criterias_results;
          $params['rule_itemtype']     = $this->getType();
          foreach ($PLUGIN_HOOKS['use_rules'] as $plugin => $val) {
-            if (!Plugin::isPluginLoaded($plugin)) {
+            if (!Plugin::isPluginActive($plugin)) {
                continue;
             }
             if (is_array($val) && in_array($this->getType(), $val)) {
@@ -2585,7 +2585,7 @@ class Rule extends CommonDBTM {
          $params['criterias_results'] = $this->criterias_results;
          $params['rule_itemtype']     = $this->getType();
          foreach ($PLUGIN_HOOKS['use_rules'] as $plugin => $val) {
-            if (!Plugin::isPluginLoaded($plugin)) {
+            if (!Plugin::isPluginActive($plugin)) {
                continue;
             }
             if (is_array($val) && in_array($this->getType(), $val)) {
@@ -2690,7 +2690,7 @@ class Rule extends CommonDBTM {
       $toreturn = $params;
       if (isset($PLUGIN_HOOKS['use_rules'])) {
          foreach ($PLUGIN_HOOKS['use_rules'] as $plugin => $val) {
-            if (!Plugin::isPluginLoaded($plugin)) {
+            if (!Plugin::isPluginActive($plugin)) {
                continue;
             }
             if (is_array($val) && in_array($itemtype, $val)) {

inc/rulecollection.class.php

@@ -1662,7 +1662,7 @@ class RuleCollection extends CommonDBTM {
       $input = $this->prepareInputDataForProcess($input, $params);
       if (isset($PLUGIN_HOOKS['use_rules'])) {
          foreach ($PLUGIN_HOOKS['use_rules'] as $plugin => $val) {
-            if (!Plugin::isPluginLoaded($plugin)) {
+            if (!Plugin::isPluginActive($plugin)) {
                continue;
             }
             if (is_array($val) && in_array($this->getRuleClassName(), $val)) {
@@ -1845,7 +1845,7 @@ class RuleCollection extends CommonDBTM {
       if (isset($PLUGIN_HOOKS['use_rules'])) {
          $params['rule_itemtype'] = $this->getType();
          foreach ($PLUGIN_HOOKS['use_rules'] as $plugin => $val) {
-            if (!Plugin::isPluginLoaded($plugin)) {
+            if (!Plugin::isPluginActive($plugin)) {
                continue;
             }
             if (is_array($val) && in_array($this->getType(), $val)) {

inc/ruleimportcomputer.class.php

@@ -254,7 +254,7 @@ class RuleImportComputer extends Rule {
       //Add plugin global criteria
       if (isset($PLUGIN_HOOKS['use_rules'])) {
          foreach ($PLUGIN_HOOKS['use_rules'] as $plugin => $val) {
-            if (!Plugin::isPluginLoaded($plugin)) {
+            if (!Plugin::isPluginActive($plugin)) {
                continue;
             }
             if (is_array($val) && in_array($this->getType(), $val)) {
@@ -354,7 +354,7 @@ class RuleImportComputer extends Rule {
 
       if (isset($PLUGIN_HOOKS['use_rules'])) {
          foreach ($PLUGIN_HOOKS['use_rules'] as $plugin => $val) {
-            if (!Plugin::isPluginLoaded($plugin)) {
+            if (!Plugin::isPluginActive($plugin)) {
                continue;
             }
             if (is_array($val) && in_array($this->getType(), $val)) {

inc/ruleimportentity.class.php

@@ -121,7 +121,7 @@ class RuleImportEntity extends Rule {
       if ($criteria['field'] == '_source') {
          $tab = [];
          foreach ($PLUGIN_HOOKS['import_item'] as $plug => $types) {
-            if (!Plugin::isPluginLoaded($plug)) {
+            if (!Plugin::isPluginActive($plug)) {
                continue;
             }
             $tab[$plug] = Plugin::getInfo($plug, 'name');

inc/stat.class.php

@@ -1515,7 +1515,7 @@ class Stat extends CommonGLPI {
       $optgroup = [];
       if (isset($PLUGIN_HOOKS["stats"]) && is_array($PLUGIN_HOOKS["stats"])) {
          foreach ($PLUGIN_HOOKS["stats"] as $plug => $pages) {
-            if (!Plugin::isPluginLoaded($plug)) {
+            if (!Plugin::isPluginActive($plug)) {
                continue;
             }
             if (is_array($pages) && count($pages)) {

tests/functionnal/Plugin.php

@@ -574,6 +574,40 @@ class Plugin extends DbTestCase {
 
    /**
     * Test state checking on a valid directory corresponding to a known inactive plugin with no modifications
+    * but not validating config.
+    * Should results in changing plugin state to "TOBECONFIGURED".
+    */
+   public function testCheckPluginStateForInactiveAndNotUpdatedPluginNotValidatingConfig() {
+
+      $initial_data = [
+         'directory' => $this->test_plugin_directory,
+         'name'      => 'Test plugin',
+         'version'   => '1.0',
+         'state'     => \Plugin::NOTACTIVATED,
+      ];
+      $setup_informations = [
+         'name'    => 'Test plugin',
+         'version' => '1.0',
+      ];
+      $expected_data = array_merge(
+         $initial_data,
+         [
+            'state' => \Plugin::TOBECONFIGURED,
+         ]
+      );
+
+      $this->function->plugin_test_check_config = false;
+
+      $this->doTestCheckPluginState(
+         $initial_data,
+         $setup_informations,
+         $expected_data,
+         'Plugin "' . $this->test_plugin_directory . '" must be configured.'
+      );
+   }
+
+   /**
+    * Test state checking on a valid directory corresponding to a known active plugin with no modifications
     * but not matching versions.
     * Should results in changing plugin state to "NOTACTIVATED".
     */
@@ -610,7 +644,7 @@ class Plugin extends DbTestCase {
    }
 
    /**
-    * Test state checking on a valid directory corresponding to a known inactive plugin with no modifications
+    * Test state checking on a valid directory corresponding to a known active plugin with no modifications
     * but not matching prerequisites.
     * Should results in changing plugin state to "NOTACTIVATED".
     */
@@ -644,11 +678,11 @@ class Plugin extends DbTestCase {
    }
 
    /**
-    * Test state checking on a valid directory corresponding to a known inactive plugin with no modifications
+    * Test state checking on a valid directory corresponding to a known active plugin with no modifications
     * but not validating config.
-    * Should results in changing plugin state to "NOTACTIVATED".
+    * Should results in changing plugin state to "TOBECONFIGURED".
     */
-   public function testCheckPluginStateForActiveAndNotUpdatedPluginNotValidationConfig() {
+   public function testCheckPluginStateForActiveAndNotUpdatedPluginNotValidatingConfig() {
 
       $initial_data = [
          'directory' => $this->test_plugin_directory,
@@ -663,7 +697,7 @@ class Plugin extends DbTestCase {
       $expected_data = array_merge(
          $initial_data,
          [
-            'state' => \Plugin::NOTACTIVATED,
+            'state' => \Plugin::TOBECONFIGURED,
          ]
       );
 
@@ -673,12 +707,12 @@ class Plugin extends DbTestCase {
          $initial_data,
          $setup_informations,
          $expected_data,
-         'Plugin "' . $this->test_plugin_directory . '" prerequisites are not matched. It has been deactivated.'
+         'Plugin "' . $this->test_plugin_directory . '" must be configured.'
       );
    }
 
    /**
-    * Test state checking on a valid directory corresponding to a known inactive plugin with no modifications,
+    * Test state checking on a valid directory corresponding to a known active plugin with no modifications,
     * matching prerequisites and validating config.
     * Should results in no changes.
     */
@@ -706,6 +740,32 @@ class Plugin extends DbTestCase {
       );
    }
 
+   /**
+    * Test state checking on a valid directory corresponding to a known active plugin with no modifications
+    * having nor check_prerequisites nor check_config function.
+    * Should results in no changes.
+    */
+   public function testCheckPluginStateForActiveAndNotUpdatedPluginHavingNoCheckFunctions() {
+
+      $initial_data = [
+         'directory' => $this->test_plugin_directory,
+         'name'      => 'Test plugin',
+         'version'   => '1.0',
+         'state'     => \Plugin::ACTIVATED,
+      ];
+      $setup_informations = [
+         'name'    => 'Test plugin',
+         'version' => '1.0',
+      ];
+      $expected_data = $initial_data;
+
+      $this->doTestCheckPluginState(
+         $initial_data,
+         $setup_informations,
+         $expected_data
+      );
+   }
+
    /**
     * Test that state checking on a plugin directory.
     *