Merge branch 'bugfix/enable-module-exception-4604'

fixes #4604

application/controllers/ConfigController.php

@@ -141,12 +141,19 @@ class ConfigController extends BaseConfigController
      */
     public function moduleenableAction()
     {
-        $module = $this->_getParam('name');
+        $module = $this->getParam('name');
         $manager = Icinga::app()->getModuleManager();
-        $manager->enableModule($module);
+        try {
-        $manager->loadModule($module);
+            $manager->enableModule($module);
-        $this->view->successMessage = 'Module "' . $module . '" enabled';
+            $manager->loadModule($module);
-        $this->moduleoverviewAction();
+            $this->view->successMessage = 'Module "' . $module . '" enabled';
+            $this->moduleoverviewAction();
+        } catch (Exception $e) {
+            $this->view->exceptionMessage = $e->getMessage();
+            $this->view->moduleName = $module;
+            $this->view->action = 'enable';
+            $this->render('module-configuration-error');
+        }
     }
 
     /**
@@ -154,11 +161,18 @@ class ConfigController extends BaseConfigController
      */
     public function moduledisableAction()
     {
-        $module = $this->_getParam('name');
+        $module = $this->getParam('name');
         $manager = Icinga::app()->getModuleManager();
-        $manager->disableModule($module);
+        try {
-        $this->view->successMessage = 'Module "' . $module . '" disabled';
+            $manager->disableModule($module);
-        $this->moduleoverviewAction();
+            $this->view->successMessage = 'Module "' . $module . '" disabled';
+            $this->moduleoverviewAction();
+        } catch (Exception $e) {
+            $this->view->exceptionMessage = $e->getMessage();
+            $this->view->moduleName = $module;
+            $this->view->action = 'disable';
+            $this->render('module-configuration-error');
+        }
     }
 
     /**

application/views/scripts/config/module-configuration-error.phtml

@@ -0,0 +1,28 @@
+<?php
+  $action = (isset($this->action)) ? $this->action : 'do something with';
+  $moduleName = $this->moduleName;
+  $exceptionMessage = $this->exceptionMessage;
+?>
+<?= $this->tabs->render($this); ?>
+<br/>
+<div class="alert alert-error">
+  <h1>Could not <?= $action; ?> module "<?= $moduleName; ?>"</h1>
+  <p>
+    While operation the following error occurred:
+    <br />
+    <?= $exceptionMessage; ?>
+  </p>
+</div>
+
+<p>
+  This could have one or more of the following reasons:
+<ul>
+  <li>No file permissions to write into module directory</li>
+  <li>Errors on filesystems: Mount points, operational errors </li>
+  <li>General application error</li>
+</ul>
+</p>
+
+<p>
+  Details can be seen in your application log (if you don't have access to this file, call your administrator in this case).
+</p>

application/views/scripts/error/error.phtml

@@ -11,16 +11,6 @@
     <p>
       <b>Message:</b>
       <?= $this->exception->getMessage(); ?>
-
-      <?php if (isset($this->exception->action)): ?>
-      <br/>
-      <b>Action</b>: <?= $this->exception->action; ?>
-      <?php endif; ?>
-
-      <?php if (isset($this->exception->action)): ?>
-      <br/>
-      <b>Target</b>: <?= $this->exception->target; ?>
-      <?php endif; ?>
     </p>
 
     <h3>Stack trace:</h3>
@@ -29,8 +19,8 @@
     </pre>
 
     <h3>Request Parameters:</h3>
-      <pre><?= var_export(\Zend_Controller_Front::getInstance()->getParams(), true) ?>
+      <pre><?= var_export(\Zend_Controller_Front::getInstance()->getParams(), true); ?>
     </pre>
   </div>
-  <?php endif ?>
+  <?php endif; ?>
 </div>

library/Icinga/Application/ApplicationBootstrap.php

@@ -343,7 +343,14 @@ abstract class ApplicationBootstrap
     {
         $this->moduleManager = new ModuleManager($this, $this->configDir . '/enabledModules');
 
-        $this->moduleManager->loadEnabledModules();
+        try {
+            $this->moduleManager->loadEnabledModules();
+        } catch (Exception $e) {
+            Logger::fatal(
+                'Could not load modules. An exception was thrown during bootstrap: %s',
+                $e->getMessage()
+            );
+        }
 
         return $this;
     }

library/Icinga/Application/Modules/Manager.php

@@ -1,5 +1,29 @@
 <?php
 // {{{ICINGA_LICENSE_HEADER}}}
+/**
+ * This file is part of Icinga 2 Web.
+ *
+ * Icinga 2 Web - Head for multiple monitoring backends.
+ * Copyright (C) 2013 Icinga Development Team
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ * @copyright 2013 Icinga Development Team <info@icinga.org>
+ * @license   http://www.gnu.org/licenses/gpl-2.0.txt GPL, version 2
+ * @author    Icinga Development Team <info@icinga.org>
+ */
 // {{{ICINGA_LICENSE_HEADER}}}
 
 namespace Icinga\Application\Modules;
@@ -7,6 +31,8 @@ namespace Icinga\Application\Modules;
 use \Icinga\Application\ApplicationBootstrap;
 use \Icinga\Application\Icinga;
 use \Icinga\Application\Logger;
+use \Icinga\Data\ArrayDatasource;
+use \Icinga\Data\ArrayQuery;
 use \Icinga\Exception\ConfigurationError;
 use \Icinga\Exception\SystemPermissionException;
 use \Icinga\Exception\ProgrammingError;
@@ -25,11 +51,9 @@ class Manager
     /**
      * Array of all installed module's base directories
      *
-     * null if modules haven't been scanned yet
+     * @var array
-     *
-     * @var array|null
      */
-    private $installedBaseDirs = null;
+    private $installedBaseDirs = array();
 
     /**
      * Array of all enabled modules base dirs
@@ -78,53 +102,48 @@ class Manager
     {
         $this->app = $app;
         if (empty($availableDirs)) {
-            $availableDirs = array(ICINGA_APPDIR."/../modules");
+            $availableDirs = array(ICINGA_APPDIR . '/../modules');
         }
         $this->modulePaths = $availableDirs;
         if ($enabledDir === null) {
-            $enabledDir = $this->app->getConfig()->getConfigDir()
+            $enabledDir = $this->app->getConfigDir() . '/enabledModules';
-                         . '/enabledModules';
         }
-        $this->prepareEssentials($enabledDir);
-        $this->detectEnabledModules();
-    }
 
-    /**
+        $this->enableDir = $enabledDir;
-     * Set the module dir and checks for existence
-     *
-     * @param   string $moduleDir The module directory to set for the module manager
-     * @throws  \Icinga\Exception\ProgrammingError
-     */
-    private function prepareEssentials($moduleDir)
-    {
-        $this->enableDir = $moduleDir;
-
-        if (!file_exists($this->enableDir) || !is_dir($this->enableDir)) {
-            throw new ProgrammingError(
-                sprintf(
-                    'Missing module directory: %s',
-                    $this->enableDir
-                )
-            );
-        }
     }
 
     /**
      * Query interface for the module manager
      *
-     * @return \Icinga\Data\ArrayQuery
+     * @return ArrayQuery
      */
     public function select()
     {
-        $source = new \Icinga\Data\ArrayDatasource($this->getModuleInfo());
+        $source = new ArrayDatasource($this->getModuleInfo());
         return $source->select();
     }
 
     /**
-     * Check for enabled modules and update the internal $enabledDirs property with the enabled modules
+     * Check for enabled modules
+     *
+     * Update the internal $enabledDirs property with the enabled modules.
+     *
+     * @throws ConfigurationError If module dir is not a directory or not readable
      */
     private function detectEnabledModules()
     {
+        if (!is_dir($this->enableDir)) {
+            throw new ConfigurationError(
+                'Could not read enabled modules: Module directory is not a directory: ' . $this->enableDir
+            );
+        }
+
+        if (!is_readable($this->enableDir)) {
+            throw new ConfigurationError(
+                'Could not read enabled modules: Module directory is not readable: ' . $this->enableDir
+            );
+        }
+
         $fh = opendir($this->enableDir);
 
         $this->enabledDirs = array();
@@ -202,31 +221,29 @@ class Manager
     /**
      * Set the given module to the enabled state
      *
-     * @param   string $name The module to enable
+     * @param   string $name                The module to enable
      *
      * @return  self
-     * @throws  \Icinga\Exception\ConfigurationError        When trying to enable a module that is not installed
+     * @throws  ConfigurationError          When trying to enable a module that is not installed
-     * @throws  \Icinga\Exception\SystemPermissionException When insufficient permissions for the application exist
+     * @throws  SystemPermissionException   When insufficient permissions for the application exist
      */
     public function enableModule($name)
     {
         if (!$this->hasInstalled($name)) {
             throw new ConfigurationError(
                 sprintf(
-                    "Cannot enable module '%s' as it isn't installed",
+                    'Cannot enable module "%s". Module is not installed.',
                     $name
                 )
             );
-            return $this;
         }
         clearstatcache(true);
         $target = $this->installedBaseDirs[$name];
         $link = $this->enableDir . '/' . $name;
         if (!is_writable($this->enableDir)) {
             throw new SystemPermissionException(
-                "Insufficient system permissions for enabling modules",
+                'Can not enable module "' . $name . '". '
-                "write",
+                . 'Insufficient system permissions for enabling modules.'
-                $this->enableDir
             );
         }
         if (file_exists($link) && is_link($link)) {
@@ -235,7 +252,11 @@ class Manager
         if (!@symlink($target, $link)) {
             $error = error_get_last();
             if (strstr($error["message"], "File exists") === false) {
-                throw new SystemPermissionException($error["message"], "symlink", $link);
+                throw new SystemPermissionException(
+                    'Could not enable module "' . $name . '" due to file system errors. '
+                    . 'Please check path and mounting points because this is not a permission error. '
+                    . 'Primary error was: ' . $error['message']
+                );
             }
         }
         $this->enabledDirs[$name] = $link;
@@ -245,12 +266,12 @@ class Manager
     /**
      * Disable the given module and remove it's enabled state
      *
-     * @param   string $name The name of the module to disable
+     * @param   string $name                The name of the module to disable
      *
      * @return  self
      *
-     * @throws  \Icinga\Exception\ConfigurationError         When the module is not installed or it's not symlinked
+     * @throws  ConfigurationError          When the module is not installed or it's not a symlink
-     * @throws  \Icinga\Exception\SystemPermissionException  When the module can't be disabled
+     * @throws  SystemPermissionException   When the module can't be disabled
      */
     public function disableModule($name)
     {
@@ -258,16 +279,17 @@ class Manager
             return $this;
         }
         if (!is_writable($this->enableDir)) {
-            throw new SystemPermissionException("Can't write the module directory", "write", $this->enableDir);
+            throw new SystemPermissionException(
-            return $this;
+                'Could not disable module. Module path is not writable.'
+            );
         }
         $link = $this->enableDir . '/' . $name;
         if (!file_exists($link)) {
-            throw new ConfigurationError('The module ' . $name . ' could not be found, can\'t disable it');
+            throw new ConfigurationError('Could not disable module. The module ' . $name . ' was not found.');
         }
         if (!is_link($link)) {
             throw new ConfigurationError(
-                'The module ' . $name . ' can\'t be disabled as this would delete the whole module. '
+                'Could not disable module. The module "' . $name . '" is not a symlink. '
                 . 'It looks like you have installed this module manually and moved it to your module folder. '
                 . 'In order to dynamically enable and disable modules, you have to create a symlink to '
                 . 'the enabled_modules folder'
@@ -277,11 +299,14 @@ class Manager
         if (file_exists($link) && is_link($link)) {
             if (!@unlink($link)) {
                 $error = error_get_last();
-                throw new SystemPermissionException($error['message'], 'unlink', $link);
+                throw new SystemPermissionException(
+                    'Could not disable module "' . $name . '" due to file system errors. '
+                    . 'Please check path and mounting points because this is not a permission error. '
+                    . 'Primary error was: ' . $error['message']
+                );
             }
-        } else {
-
         }
+
         unset($this->enabledDirs[$name]);
         return $this;
     }
@@ -294,7 +319,7 @@ class Manager
      *
      * @return  string
      *
-     * @throws  \Icinga\Exception\ProgrammingError When the module is not installed or existing
+     * @throws  ProgrammingError When the module is not installed or existing
      */
     public function getModuleDir($name, $subdir = '')
     {
@@ -323,7 +348,7 @@ class Manager
      */
     public function hasInstalled($name)
     {
-        if ($this->installedBaseDirs === null) {
+        if (!count($this->installedBaseDirs)) {
             $this->detectInstalledModules();
         }
         return array_key_exists($name, $this->installedBaseDirs);
@@ -357,8 +382,7 @@ class Manager
      * Return an array containing all loaded modules
      *
      * @return  array
-     *
+     * @see     Module
-     * @see     \Icinga\Application\Modules\Module
      */
     public function getLoadedModules()
     {
@@ -368,11 +392,10 @@ class Manager
     /**
      * Return the module instance of the given module when it is loaded
      *
-     * @param   string $name The module name to return
+     * @param   string $name        The module name to return
      *
-     * @return  \Icinga\Application\Modules\Module
+     * @return  Module
-     *
+     * @throws  ProgrammingError    When the module hasn't been loaded
-     * @throws  \Icinga\Exception\ProgrammingError When the module hasn't been loaded
      */
     public function getModule($name)
     {
@@ -425,6 +448,10 @@ class Manager
      */
     public function listEnabledModules()
     {
+        if (count($this->enabledDirs) === 0) {
+            $this->detectEnabledModules();
+        }
+
         return array_keys($this->enabledDirs);
     }
 
@@ -439,7 +466,7 @@ class Manager
     }
 
     /**
-     * Return an array containing all installled module names as strings
+     * Return an array of module names from installed modules
      *
      * Calls detectInstalledModules() if no module discovery has been performed yet
      *
@@ -449,13 +476,15 @@ class Manager
      */
     public function listInstalledModules()
     {
-        if ($this->installedBaseDirs === null) {
+        if (!count($this->installedBaseDirs)) {
             $this->detectInstalledModules();
         }
 
-        if ($this->installedBaseDirs !== null) {
+        if (count($this->installedBaseDirs)) {
             return array_keys($this->installedBaseDirs);
         }
+
+        return array();
     }
 
     /**

library/Icinga/Exception/SystemPermissionException.php

@@ -28,19 +28,11 @@
 
 namespace Icinga\Exception;
 
-/**
+use \Exception;
- * Class ProgrammingError
- * @package Icinga\Exception
- */
-class SystemPermissionException extends \Exception
-{
-    public $action;
-    public $target;
 
-    public function __construct($message, $action, $target = "")
+/**
-    {
+ * Handle problems according to file system permissions
-        parent::__construct($message);
+ */
-        $this->action = $action;
+class SystemPermissionException extends Exception
-        $this->target = $target;
+{
-    }
 }