From 6124f984eecc92914bcbb0187aaf1a0be4c8d8e7 Mon Sep 17 00:00:00 2001 From: Matthias Jentsch Date: Tue, 14 Oct 2014 15:50:15 +0200 Subject: [PATCH 01/15] Adjust scroll position on container after triggering the 'rendered' event Prevent behaviors from altering the current scroll-position on page reload when refreshing or changing the focus. fixes #7269 --- public/js/icinga/loader.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/public/js/icinga/loader.js b/public/js/icinga/loader.js index 9847df4e6..dca88c394 100644 --- a/public/js/icinga/loader.js +++ b/public/js/icinga/loader.js @@ -723,9 +723,6 @@ } this.icinga.ui.assignUniqueContainerIds(); - if (scrollPos !== false) { - $container.scrollTop(scrollPos); - } if (origFocus) { $(origFocus).focus(); } @@ -733,6 +730,9 @@ // TODO: this.icinga.events.refreshContainer(container); $container.trigger('rendered'); + if (scrollPos !== false) { + $container.scrollTop(scrollPos); + } var icinga = this.icinga; //icinga.events.applyHandlers($container); icinga.ui.initializeControls($container); From 937dbe2c15f59132ceb12ea92a3ff47bab99f94d Mon Sep 17 00:00:00 2001 From: Matthias Jentsch Date: Tue, 14 Oct 2014 17:54:52 +0200 Subject: [PATCH 02/15] Add missing close buttons to views without tabs --- .../config/authentication/create.phtml | 3 ++ .../config/authentication/modify.phtml | 3 ++ .../config/authentication/remove.phtml | 3 ++ .../scripts/config/resource/create.phtml | 3 ++ .../scripts/config/resource/modify.phtml | 3 ++ .../scripts/config/resource/remove.phtml | 3 ++ library/Icinga/Web/Widget/Tabs.php | 34 ++++++++++++++++--- .../views/scripts/partials/command-form.phtml | 4 +++ .../process/disable-notifications.phtml | 3 ++ .../views/scripts/show/contact.phtml | 1 + .../views/scripts/timeline/index.phtml | 1 + 11 files changed, 56 insertions(+), 5 deletions(-) diff --git a/application/views/scripts/config/authentication/create.phtml b/application/views/scripts/config/authentication/create.phtml index 52fcbceb3..427758292 100644 --- a/application/views/scripts/config/authentication/create.phtml +++ b/application/views/scripts/config/authentication/create.phtml @@ -1,3 +1,6 @@ +
+ tabs->showOnlyCloseButton() ?> +

translate('Create New Authentication Backend'); ?>

translate( diff --git a/application/views/scripts/config/authentication/modify.phtml b/application/views/scripts/config/authentication/modify.phtml index 19901ae5b..b01d7095a 100644 --- a/application/views/scripts/config/authentication/modify.phtml +++ b/application/views/scripts/config/authentication/modify.phtml @@ -1,2 +1,5 @@ +

+ tabs->showOnlyCloseButton() ?> +

translate('Edit Backend'); ?>

\ No newline at end of file diff --git a/application/views/scripts/config/authentication/remove.phtml b/application/views/scripts/config/authentication/remove.phtml index e1050db0d..424aff9a0 100644 --- a/application/views/scripts/config/authentication/remove.phtml +++ b/application/views/scripts/config/authentication/remove.phtml @@ -1,2 +1,5 @@ +
+ tabs->showOnlyCloseButton() ?> +

translate('Remove Backend'); ?>

\ No newline at end of file diff --git a/application/views/scripts/config/resource/create.phtml b/application/views/scripts/config/resource/create.phtml index 5a07d832e..0750a5aa9 100644 --- a/application/views/scripts/config/resource/create.phtml +++ b/application/views/scripts/config/resource/create.phtml @@ -1,3 +1,6 @@ +
+ tabs->showOnlyCloseButton() ?> +

translate('Create A New Resource'); ?>

translate('Resources are entities that provide data to Icinga Web 2.'); ?>

\ No newline at end of file diff --git a/application/views/scripts/config/resource/modify.phtml b/application/views/scripts/config/resource/modify.phtml index 5fa2536bd..d355aeb4c 100644 --- a/application/views/scripts/config/resource/modify.phtml +++ b/application/views/scripts/config/resource/modify.phtml @@ -1,2 +1,5 @@ +
+ tabs->showOnlyCloseButton() ?> +

translate('Edit Existing Resource'); ?>

\ No newline at end of file diff --git a/application/views/scripts/config/resource/remove.phtml b/application/views/scripts/config/resource/remove.phtml index a43bcd74c..14829ab40 100644 --- a/application/views/scripts/config/resource/remove.phtml +++ b/application/views/scripts/config/resource/remove.phtml @@ -1,2 +1,5 @@ +
+ tabs->showOnlyCloseButton() ?> +

translate('Remove Existing Resource'); ?>

\ No newline at end of file diff --git a/library/Icinga/Web/Widget/Tabs.php b/library/Icinga/Web/Widget/Tabs.php index d79453a01..b2a34e0ec 100644 --- a/library/Icinga/Web/Widget/Tabs.php +++ b/library/Icinga/Web/Widget/Tabs.php @@ -74,6 +74,13 @@ EOT; */ private $dropdownTabs = array(); + /** + * Whether only the close-button should by rendered for this tab + * + * @var bool + */ + private $closeButtonOnly = false; + /** * Whether the tabs should contain a close-button * @@ -275,14 +282,19 @@ EOT; */ public function render() { - if (empty($this->tabs)) { - return ''; + if (empty($this->tabs) || true === $this->closeButtonOnly) { + $tabs = ''; + $drop = ''; + } else { + $tabs = $this->renderTabs(); + $drop = $this->renderDropdownTabs(); } + $close = $this->closeTab ? $this->renderCloseTab() : ''; $html = $this->baseTpl; - $html = str_replace('{TABS}', $this->renderTabs(), $html); - $html = str_replace('{DROPDOWN}', $this->renderDropdownTabs(), $html); - $html = str_replace('{CLOSE}', $this->closeTab ? $this->renderCloseTab() : '', $html); + $html = str_replace('{TABS}', $tabs, $html); + $html = str_replace('{DROPDOWN}', $drop, $html); + $html = str_replace('{CLOSE}', $close, $html); return $html; } @@ -318,6 +330,18 @@ EOT; return $this->tabs; } + /** + * Whether to hide all elements except of the close button + * + * @param bool $value + * @return Tabs fluent interface + */ + public function showOnlyCloseButton($value = true) + { + $this->closeButtonOnly = $value; + return $this; + } + /** * Apply a Tabextension on this tabs object * diff --git a/modules/monitoring/application/views/scripts/partials/command-form.phtml b/modules/monitoring/application/views/scripts/partials/command-form.phtml index 0aae10046..0e5a4777e 100644 --- a/modules/monitoring/application/views/scripts/partials/command-form.phtml +++ b/modules/monitoring/application/views/scripts/partials/command-form.phtml @@ -1,3 +1,7 @@ +
+ tabs->showOnlyCloseButton() ?> +
+

diff --git a/modules/monitoring/application/views/scripts/process/disable-notifications.phtml b/modules/monitoring/application/views/scripts/process/disable-notifications.phtml index 6b45a1f22..547b3ac37 100644 --- a/modules/monitoring/application/views/scripts/process/disable-notifications.phtml +++ b/modules/monitoring/application/views/scripts/process/disable-notifications.phtml @@ -1,3 +1,6 @@ +
+ tabs->showOnlyCloseButton() ?> +

notifications_enabled === false): ?> diff --git a/modules/monitoring/application/views/scripts/show/contact.phtml b/modules/monitoring/application/views/scripts/show/contact.phtml index a755266ea..cdb6af12c 100644 --- a/modules/monitoring/application/views/scripts/show/contact.phtml +++ b/modules/monitoring/application/views/scripts/show/contact.phtml @@ -1,5 +1,6 @@ getHelper('ContactFlags') ?>
+ tabs ?>

translate('Contact details') ?>

+ tabs ?>
From 64940198beb1797fd2de0d2fead4263aefe2f289 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Wed, 15 Oct 2014 11:18:16 +0200 Subject: [PATCH 03/15] Fix AlertSummary report cannot find EventHistory dataview We should start to think about how to prevent such issues ;) --- .../application/controllers/AlertsummaryController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/monitoring/application/controllers/AlertsummaryController.php b/modules/monitoring/application/controllers/AlertsummaryController.php index 8aeb27c76..4f6ced349 100644 --- a/modules/monitoring/application/controllers/AlertsummaryController.php +++ b/modules/monitoring/application/controllers/AlertsummaryController.php @@ -275,7 +275,7 @@ class Monitoring_AlertsummaryController extends Controller $interval = $this->getInterval(); $query = $this->backend->select()->from( - 'eventhistory', + 'EventHistory', array( 'host_name', 'service_description', From 92b4f4fbec96417f42905cbc0494f899fece9bb7 Mon Sep 17 00:00:00 2001 From: Marius Hein Date: Thu, 16 Oct 2014 09:50:15 +0200 Subject: [PATCH 04/15] AlertSummary: Avoid division by zero --- .../application/controllers/AlertsummaryController.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/monitoring/application/controllers/AlertsummaryController.php b/modules/monitoring/application/controllers/AlertsummaryController.php index 4f6ced349..02c8bb3db 100644 --- a/modules/monitoring/application/controllers/AlertsummaryController.php +++ b/modules/monitoring/application/controllers/AlertsummaryController.php @@ -259,7 +259,11 @@ class Monitoring_AlertsummaryController extends Controller } $out = new stdClass(); - $out->avg = sprintf('%.2f', array_sum($slots) / count($slots)); + if (! empty($slots)) { + $out->avg = sprintf('%.2f', array_sum($slots) / count($slots)); + } else { + $out->avg = '0.0'; + } $out->last = array_shift($slots); return $out; From f9e7e6d88816aa58456f53581c01081b3f268382 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 16 Oct 2014 11:20:16 +0200 Subject: [PATCH 05/15] general config: Don't keep unused configuration directives --- application/forms/Config/GeneralConfigForm.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/application/forms/Config/GeneralConfigForm.php b/application/forms/Config/GeneralConfigForm.php index 550fe4cfb..0b55704c2 100644 --- a/application/forms/Config/GeneralConfigForm.php +++ b/application/forms/Config/GeneralConfigForm.php @@ -40,13 +40,16 @@ class GeneralConfigForm extends ConfigForm */ public function onSuccess(Request $request) { + $sections = array(); foreach ($this->getValues() as $sectionAndPropertyName => $value) { list($section, $property) = explode('_', $sectionAndPropertyName); - if (isset($this->config->{$section})) { - $this->config->{$section}->{$property} = $value; - } else { - $this->config->{$section} = array($property => $value); + if (! isset($sections[$section])) { + $sections[$section] = array(); } + $sections[$section][$property] = $value; + } + foreach ($sections as $section => $config) { + $this->config->{$section} = $config; } if ($this->save()) { From d475ccd56954a844e070311e0e6c0fa916e2d4d4 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 16 Oct 2014 15:37:07 +0200 Subject: [PATCH 06/15] IcingaCommand: Fix strict standards violation: Only variables should be passed by reference --- .../monitoring/library/Monitoring/Command/IcingaCommand.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/monitoring/library/Monitoring/Command/IcingaCommand.php b/modules/monitoring/library/Monitoring/Command/IcingaCommand.php index d88f8dba0..3b569de8e 100644 --- a/modules/monitoring/library/Monitoring/Command/IcingaCommand.php +++ b/modules/monitoring/library/Monitoring/Command/IcingaCommand.php @@ -16,6 +16,7 @@ abstract class IcingaCommand */ public function getName() { - return substr_replace(end(explode('\\', get_called_class())), '', -7); // Remove 'Command' Suffix + $nsParts = explode('\\', get_called_class()); + return substr_replace(end($nsParts), '', -7); // Remove 'Command' Suffix } } From a2772a17a73e34514491306ef7f5fa9922b99f08 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 16 Oct 2014 15:47:50 +0200 Subject: [PATCH 07/15] File: Fix ErrorException::__construct()'s parameters --- library/Icinga/Util/File.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/Icinga/Util/File.php b/library/Icinga/Util/File.php index b7aff9743..9ce1ec893 100644 --- a/library/Icinga/Util/File.php +++ b/library/Icinga/Util/File.php @@ -131,7 +131,7 @@ class File extends SplFileObject set_error_handler( function ($errno, $errstr, $errfile, $errline) { restore_error_handler(); - throw new ErrorException($errno, $errstr, $errfile, $errline); + throw new ErrorException($errstr, 0, $errno, $errfile, $errline); }, E_WARNING ); From 985154d3d8c736b7276d7a8cb1342b50c946a389 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 16 Oct 2014 15:51:18 +0200 Subject: [PATCH 08/15] Throw an ErrorException on E_STRICT errors --- library/Icinga/Application/ApplicationBootstrap.php | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/library/Icinga/Application/ApplicationBootstrap.php b/library/Icinga/Application/ApplicationBootstrap.php index 0df6700bc..73aed966b 100644 --- a/library/Icinga/Application/ApplicationBootstrap.php +++ b/library/Icinga/Application/ApplicationBootstrap.php @@ -4,10 +4,10 @@ namespace Icinga\Application; +use ErrorException; use Exception; use Zend_Config; use Icinga\Application\Modules\Manager as ModuleManager; -use Icinga\Application\Config; use Icinga\Data\ResourceFactory; use Icinga\Exception\ConfigurationError; use Icinga\Exception\NotReadableError; @@ -386,6 +386,17 @@ abstract class ApplicationBootstrap error_reporting(E_ALL | E_STRICT); ini_set('display_startup_errors', 1); ini_set('display_errors', 1); + set_error_handler(function ($errno, $errstr, $errfile, $errline) { + if (error_reporting() === 0) { + // Error was suppressed with the @-operator + return false; // Continue with the normal error handler + } + switch($errno) { + case E_STRICT: + throw new ErrorException($errstr, 0, $errno, $errfile, $errline); + } + return false; // Continue with the normal error handler + }); return $this; } From 97677ee2c19777a3f4cf9187a149388e3810d4bb Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 16 Oct 2014 15:54:13 +0200 Subject: [PATCH 09/15] Fix our Logger not supporting named logging levels and requiring the 'enabled' directive --- library/Icinga/Logger/Logger.php | 154 +++++++++++++++++++++---------- 1 file changed, 104 insertions(+), 50 deletions(-) diff --git a/library/Icinga/Logger/Logger.php b/library/Icinga/Logger/Logger.php index b4f2b5f26..1cc820ef7 100644 --- a/library/Icinga/Logger/Logger.php +++ b/library/Icinga/Logger/Logger.php @@ -6,61 +6,110 @@ namespace Icinga\Logger; use Exception; use Zend_Config; -use LogicException; use Icinga\Exception\ConfigurationError; +use Icinga\Logger\Writer\FileWriter; +use Icinga\Logger\Writer\SyslogWriter; /** - * Singleton logger + * Logger */ class Logger { + /** + * Debug message + */ + const DEBUG = 1; + + /** + * Informational message + */ + const INFO = 2; + + /** + * Warning message + */ + const WARNING = 4; + + /** + * Error message + */ + const ERROR = 8; + + /** + * Log levels + * + * @var array + */ + public static $levels = array( + Logger::DEBUG => 'DEBUG', + Logger::INFO => 'INFO', + Logger::WARNING => 'WARNING', + Logger::ERROR => 'ERROR' + ); + /** * This logger's instance * - * @var Logger + * @var static */ protected static $instance; /** - * The log writer to use + * Log writer * * @var \Icinga\Logger\LogWriter */ protected $writer; /** - * The configured type - * - * @string Type (syslog, file) - */ - protected $type = 'none'; - - /** - * The maximum severity to emit + * Maximum level to emit * * @var int */ - protected $verbosity; - - /** - * The supported severities - */ - public static $NONE = 0; - public static $ERROR = 1; - public static $WARNING = 2; - public static $INFO = 3; - public static $DEBUG = 4; + protected $level; /** * Create a new logger object * - * @param Zend_Config $config + * @param Zend_Config $config + * + * @throws ConfigurationError If the logging configuration directive 'log' is missing or if the logging level is + * not defined */ public function __construct(Zend_Config $config) { - $this->verbosity = $config->level; + if ($config->log === null) { + throw new ConfigurationError('Required logging configuration directive \'log\' missing'); + } - if ($config->enable) { + if (($level = $config->level) !== null) { + if (is_numeric($level)) { + if (! isset(static::$levels[(int) $level])) { + throw new ConfigurationError( + 'Can\'t set logging level %d. Logging level is not defined. Use one of %s or one of the' + . ' Logger\'s constants.', + $level, + implode(', ', array_keys(static::$levels)) + ); + } + $this->level = static::$levels[(int) $level]; + } else { + $level = strtoupper($level); + $levels = array_flip(static::$levels); + if (! isset($levels[$level])) { + throw new ConfigurationError( + 'Can\'t set logging level "%s". Logging level is not defined. Use one of %s.', + $level, + implode(', ', array_keys($levels)) + ); + } + $this->level = $levels[$level]; + } + } else { + $this->level = static::ERROR; + } + + if (strtolower($config->get('log', 'syslog')) !== 'none') { $this->writer = $this->createWriter($config); } } @@ -69,14 +118,17 @@ class Logger * Create a new logger object * * @param Zend_Config $config + * + * @return static */ public static function create(Zend_Config $config) { static::$instance = new static($config); + return static::$instance; } /** - * Return a log writer + * Create a log writer * * @param Zend_Config $config The configuration to initialize the writer with * @@ -85,34 +137,26 @@ class Logger */ protected function createWriter(Zend_Config $config) { - $class = 'Icinga\\Logger\\Writer\\' . ucfirst(strtolower($config->type)) . 'Writer'; - if (!class_exists($class)) { + $class = 'Icinga\\Logger\\Writer\\' . ucfirst(strtolower($config->log)) . 'Writer'; + if (! class_exists($class)) { throw new ConfigurationError( 'Cannot find log writer of type "%s"', - $config->type + $config->log ); } - $this->type = $config->type; - return new $class($config); } /** - * Write a message to the log + * Log a message * - * @param string $message The message to write - * @param int $severity The severity to use - * - * @throws LogicException In case $severity equals self::$NONE + * @param int $level The logging level + * @param string $message The log message */ - public function log($message, $severity) + public function log($level, $message) { - if ($severity === static::$NONE) { - throw new LogicException("`None' (0) is not a valid severity to log messages"); - } - - if ($this->writer !== null && $this->verbosity >= $severity) { - $this->writer->log($severity, $message); + if ($this->writer !== null && $this->level >= $level) { + $this->writer->log($level, $message); } } @@ -170,7 +214,7 @@ class Logger public static function error() { if (static::$instance !== null && func_num_args() > 0) { - static::$instance->log(static::formatMessage(func_get_args()), static::$ERROR); + static::$instance->log(static::ERROR, static::formatMessage(func_get_args())); } } @@ -182,7 +226,7 @@ class Logger public static function warning() { if (static::$instance !== null && func_num_args() > 0) { - static::$instance->log(static::formatMessage(func_get_args()), static::$WARNING); + static::$instance->log(static::WARNING, static::formatMessage(func_get_args())); } } @@ -194,7 +238,7 @@ class Logger public static function info() { if (static::$instance !== null && func_num_args() > 0) { - static::$instance->log(static::formatMessage(func_get_args()), static::$INFO); + static::$instance->log(static::INFO, static::formatMessage(func_get_args())); } } @@ -206,7 +250,7 @@ class Logger public static function debug() { if (static::$instance !== null && func_num_args() > 0) { - static::$instance->log(static::formatMessage(func_get_args()), static::$DEBUG); + static::$instance->log(static::DEBUG, static::formatMessage(func_get_args())); } } @@ -220,20 +264,30 @@ class Logger return $this->writer; } + /** + * Is the logger writing to Syslog? + * + * @return bool + */ public static function writesToSyslog() { - return static::$instance && static::$instance->type === 'syslog'; + return static::$instance && static::$instance instanceof SyslogWriter; } + /** + * Is the logger writing to a file? + * + * @return bool + */ public static function writesToFile() { - return static::$instance && static::$instance->type === 'file'; + return static::$instance && static::$instance instanceof FileWriter; } /** * Get this' instance * - * @return Logger + * @return static */ public static function getInstance() { From 04a8df54cda0d30571821aae84d50ab5382f44c6 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 16 Oct 2014 15:55:24 +0200 Subject: [PATCH 10/15] Logger/FileWriter: Rename 'target' directive to 'file' --- library/Icinga/Logger/Writer/FileWriter.php | 87 +++++++-------------- 1 file changed, 27 insertions(+), 60 deletions(-) diff --git a/library/Icinga/Logger/Writer/FileWriter.php b/library/Icinga/Logger/Writer/FileWriter.php index 4ab04d85b..527358e1c 100644 --- a/library/Icinga/Logger/Writer/FileWriter.php +++ b/library/Icinga/Logger/Writer/FileWriter.php @@ -5,38 +5,43 @@ namespace Icinga\Logger\Writer; use Exception; -use Icinga\Exception\IcingaException; use Zend_Config; -use Icinga\Util\File; +use Icinga\Exception\ConfigurationError; use Icinga\Logger\Logger; use Icinga\Logger\LogWriter; -use Icinga\Exception\ConfigurationError; +use Icinga\Util\File; /** - * Class to write log messages to a file + * Log to a file */ class FileWriter extends LogWriter { /** - * The path to the file + * Path to the file * * @var string */ - protected $path; + protected $file; /** - * Create a new log writer initialized with the given configuration + * Create a new file log writer * - * @throws ConfigurationError In case the log path does not exist + * @param Zend_Config $config + * + * @throws ConfigurationError If the configuration directive 'file' is missing or if the path to 'file' does + * not exist or if writing to 'file' is not possible */ public function __construct(Zend_Config $config) { - $this->path = $config->target; + if ($config->file === null) { + throw new ConfigurationError('Required logging configuration directive \'file\' missing'); + } + $this->file = $config->file; - if (substr($this->path, 0, 6) !== 'php://' && false === file_exists(dirname($this->path))) { + if (substr($this->file, 0, 6) !== 'php://' && ! file_exists(dirname($this->file))) { throw new ConfigurationError( 'Log path "%s" does not exist', - dirname($this->path) + dirname($this->file) ); } @@ -45,70 +50,32 @@ class FileWriter extends LogWriter } catch (Exception $e) { throw new ConfigurationError( 'Cannot write to log file "%s" (%s)', - $this->path, + $this->file, $e->getMessage() ); } } /** - * Log a message with the given severity + * Log a message * - * @param int $severity The severity to use - * @param string $message The message to log + * @param int $level The logging level + * @param string $message The log message */ - public function log($severity, $message) + public function log($level, $message) { - $this->write(date('c') . ' ' . $this->getSeverityString($severity) . ' ' . $message . PHP_EOL); + $this->write(date('c') . ' - ' . Logger::$levels[$level] . ' - ' . $message . PHP_EOL); } /** - * Return a string representation for the given severity + * Write a message to the log * - * @param string $severity The severity to use - * - * @return string The string representation of the severity - * - * @throws IcingaException In case the given severity is unknown + * @param string $message */ - protected function getSeverityString($severity) + protected function write($message) { - switch ($severity) { - case Logger::$ERROR: - return '- ERROR -'; - case Logger::$WARNING: - return '- WARNING -'; - case Logger::$INFO: - return '- INFO -'; - case Logger::$DEBUG: - return '- DEBUG -'; - default: - throw new IcingaException( - 'Unknown severity "%s"', - $severity - ); - } - } - - /** - * Write a message to the path - * - * @param string $text The message to write - * - * @throws Exception In case write acess to the path failed - */ - protected function write($text) - { - $file = new File($this->path, 'a'); - $file->fwrite($text); + $file = new File($this->file, 'a'); + $file->fwrite($message); $file->fflush(); } - - /** - * @return string - */ - public function getPath() - { - return $this->path; - } } From d2d653209f0eff63fe9d275bccb9726027ab310c Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 16 Oct 2014 15:56:38 +0200 Subject: [PATCH 11/15] Logger/SyslogWriter: Define configuration defaults here Usage of closelog() is optional so I removed the explicit calls. --- library/Icinga/Logger/Writer/SyslogWriter.php | 98 ++++++------------- 1 file changed, 29 insertions(+), 69 deletions(-) diff --git a/library/Icinga/Logger/Writer/SyslogWriter.php b/library/Icinga/Logger/Writer/SyslogWriter.php index d176e1cbb..f67c618d2 100644 --- a/library/Icinga/Logger/Writer/SyslogWriter.php +++ b/library/Icinga/Logger/Writer/SyslogWriter.php @@ -4,27 +4,24 @@ namespace Icinga\Logger\Writer; -use Exception; use Zend_Config; use Icinga\Logger\Logger; use Icinga\Logger\LogWriter; -use Icinga\Exception\ConfigurationError; -use Icinga\Exception\IcingaException; /** - * Class to write messages to syslog + * Log to the syslog service */ class SyslogWriter extends LogWriter { /** - * The facility where to write messages to + * Syslog facility * - * @var string + * @var int */ protected $facility; /** - * The prefix to prepend to each message + * Prefix to prepend to each message * * @var string */ @@ -35,79 +32,42 @@ class SyslogWriter extends LogWriter * * @var array */ - protected $facilities = array( - 'LOG_USER' => LOG_USER + public static $facilities = array( + 'user' => LOG_USER ); /** - * Create a new log writer initialized with the given configuration + * Log level to syslog severity map + * + * @var array + */ + public static $severityMap = array( + Logger::ERROR => LOG_ERR, + Logger::WARNING => LOG_WARNING, + Logger::INFO => LOG_INFO, + Logger::DEBUG => LOG_DEBUG + ); + + /** + * Create a new syslog log writer + * + * @param Zend_Config $config */ public function __construct(Zend_Config $config) { - if (!array_key_exists($config->facility, $this->facilities)) { - throw new ConfigurationError( - 'Cannot create syslog writer with unknown facility "%s"', - $config->facility - ); - } - - $this->ident = $config->application; - $this->facility = $this->facilities[$config->facility]; + $this->ident = $config->get('application', 'icingaweb'); + $this->facility = static::$facilities['user']; } /** - * Log a message with the given severity + * Log a message * - * @param int $severity The severity to use - * @param string $message The message to log - * - * @throws Exception In case the given severity cannot be mapped to a valid syslog priority + * @param int $level The logging level + * @param string $message The log message */ - public function log($severity, $message) + public function log($level, $message) { - $priorities = array( - Logger::$ERROR => LOG_ERR, - Logger::$WARNING => LOG_WARNING, - Logger::$INFO => LOG_INFO, - Logger::$DEBUG => LOG_DEBUG - ); - - if (!array_key_exists($severity, $priorities)) { - throw new IcingaException( - 'Severity "%s" cannot be mapped to a valid syslog priority', - $severity - ); - } - - $this->open(); - $this->write($priorities[$severity], $message); - $this->close(); - } - - /** - * Open a new syslog connection - */ - protected function open() - { - openlog($this->ident, 0, $this->facility); - } - - /** - * Write a message to the syslog connection - * - * @param int $priority The priority to use - * @param string $message The message to write - */ - protected function write($priority, $message) - { - syslog($priority, $message); - } - - /** - * Close the syslog connection - */ - protected function close() - { - closelog(); + openlog($this->ident, LOG_PID, $this->facility); + syslog(static::$severityMap[$level], $message); } } From 501aca42b924fb51c73a9403c90438da4c2f9233 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 16 Oct 2014 15:58:37 +0200 Subject: [PATCH 12/15] Logger: Fix the StreamWriterTest --- .../php/library/Icinga/Logger/Writer/StreamWriterTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/php/library/Icinga/Logger/Writer/StreamWriterTest.php b/test/php/library/Icinga/Logger/Writer/StreamWriterTest.php index f20191418..11fec9740 100644 --- a/test/php/library/Icinga/Logger/Writer/StreamWriterTest.php +++ b/test/php/library/Icinga/Logger/Writer/StreamWriterTest.php @@ -9,7 +9,7 @@ use Icinga\Logger\Logger; use Icinga\Test\BaseTestCase; use Icinga\Logger\Writer\FileWriter; -class LoggerTest extends BaseTestCase +class StreamWriterTest extends BaseTestCase { public function setUp() { @@ -27,7 +27,7 @@ class LoggerTest extends BaseTestCase public function testWhetherStreamWriterCreatesMissingFiles() { - new FileWriter(new Zend_Config(array('target' => $this->target))); + new FileWriter(new Zend_Config(array('file' => $this->target))); $this->assertFileExists($this->target, 'StreamWriter does not create missing files on initialization'); } @@ -36,8 +36,8 @@ class LoggerTest extends BaseTestCase */ public function testWhetherStreamWriterWritesMessages() { - $writer = new FileWriter(new Zend_Config(array('target' => $this->target))); - $writer->log(Logger::$ERROR, 'This is a test error'); + $writer = new FileWriter(new Zend_Config(array('file' => $this->target))); + $writer->log(Logger::ERROR, 'This is a test error'); $log = file_get_contents($this->target); $this->assertContains('This is a test error', $log, 'StreamWriter does not write log messages'); } From 6dfefb0e7355ea659d0525733f2406ed1ee65c4d Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 16 Oct 2014 15:59:35 +0200 Subject: [PATCH 13/15] CLI: Fix logging setup --- library/Icinga/Application/Cli.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/library/Icinga/Application/Cli.php b/library/Icinga/Application/Cli.php index 58daf5f2f..d329ce439 100644 --- a/library/Icinga/Application/Cli.php +++ b/library/Icinga/Application/Cli.php @@ -51,10 +51,9 @@ class Cli extends ApplicationBootstrap Logger::create( new Zend_Config( array( - 'enable' => true, - 'level' => Logger::$INFO, - 'type' => 'file', - 'target' => 'php://stderr' + 'level' => Logger::INFO, + 'log' => 'file', + 'file' => 'php://stderr' ) ) ); From 6c59c220448aab651a7a6771ebf3e3a0a7716553 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 16 Oct 2014 15:59:56 +0200 Subject: [PATCH 14/15] Bootstrap: Remove logging configuration directives which do not override defaults --- library/Icinga/Application/ApplicationBootstrap.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/library/Icinga/Application/ApplicationBootstrap.php b/library/Icinga/Application/ApplicationBootstrap.php index 73aed966b..958642940 100644 --- a/library/Icinga/Application/ApplicationBootstrap.php +++ b/library/Icinga/Application/ApplicationBootstrap.php @@ -348,11 +348,7 @@ abstract class ApplicationBootstrap Logger::create( new Zend_Config( array( - 'enable' => true, - 'level' => Logger::$ERROR, - 'type' => 'syslog', - 'facility' => 'LOG_USER', - 'application' => 'icingaweb' + 'log' => 'syslog' ) ) ); From 5ca97c14d6b6e391f53ed90171cef1e97e7e9fb6 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 16 Oct 2014 16:13:00 +0200 Subject: [PATCH 15/15] logging config: Support 'log' 'none' --- .../Config/General/LoggingConfigForm.php | 87 ++++++++++--------- 1 file changed, 48 insertions(+), 39 deletions(-) diff --git a/application/forms/Config/General/LoggingConfigForm.php b/application/forms/Config/General/LoggingConfigForm.php index 135dd9e28..733c306b9 100644 --- a/application/forms/Config/General/LoggingConfigForm.php +++ b/application/forms/Config/General/LoggingConfigForm.php @@ -5,6 +5,7 @@ namespace Icinga\Form\Config\General; use Icinga\Application\Icinga; +use Icinga\Logger\Logger; use Icinga\Web\Form; use Icinga\Web\Form\Validator\WritablePathValidator; @@ -19,29 +20,14 @@ class LoggingConfigForm extends Form } /** - * @see Form::createElements() + * (non-PHPDoc) + * @see Form::createElements() For the method documentation. */ public function createElements(array $formData) { $this->addElement( 'select', - 'logging_level', - array( - 'required' => true, - 'label' => t('Logging Level'), - 'description' => t('The maximum loglevel to emit.'), - 'multiOptions' => array( - 0 => t('None'), - 1 => t('Error'), - 2 => t('Warning'), - 3 => t('Information'), - 4 => t('Debug') - ) - ) - ); - $this->addElement( - 'select', - 'logging_type', + 'logging_log', array( 'required' => true, 'class' => 'autosubmit', @@ -49,13 +35,32 @@ class LoggingConfigForm extends Form 'description' => t('The type of logging to utilize.'), 'multiOptions' => array( 'syslog' => 'Syslog', - 'file' => t('File') + 'file' => t('File'), + 'none' => t('None') ) ) ); - if (false === isset($formData['logging_type']) || $formData['logging_type'] === 'syslog') { - $this->addElement( + if (! isset($formData['logging_log']) || $formData['logging_log'] !== 'none') { + $this->addElement( + 'select', + 'logging_level', + array( + 'required' => true, + 'label' => t('Logging Level'), + 'description' => t('The maximum logging level to emit.'), + 'multiOptions' => array( + Logger::$levels[Logger::ERROR] => t('Error'), + Logger::$levels[Logger::WARNING] => t('Warning'), + Logger::$levels[Logger::INFO] => t('Information'), + Logger::$levels[Logger::DEBUG] => t('Debug') + ) + ) + ); + } + + if (isset($formData['logging_log']) && $formData['logging_log'] === 'syslog') { + $this->addElement( 'text', 'logging_application', array( @@ -77,26 +82,30 @@ class LoggingConfigForm extends Form ) ) ); - $this->addElement( - 'select', - 'logging_facility', - array( - 'required' => true, - 'label' => t('Facility'), - 'description' => t('The Syslog facility to utilize.'), - 'multiOptions' => array( - 'LOG_USER' => 'LOG_USER' - ) - ) - ); - } elseif ($formData['logging_type'] === 'file') { + /* + * Note(el): Since we provide only one possible value for the syslog facility, I opt against exposing + * this configuration. + */ +// $this->addElement( +// 'select', +// 'logging_facility', +// array( +// 'required' => true, +// 'label' => t('Facility'), +// 'description' => t('The syslog facility to utilize.'), +// 'multiOptions' => array( +// 'user' => 'LOG_USER' +// ) +// ) +// ); + } elseif (isset($formData['logging_log']) && $formData['logging_log'] === 'file') { $this->addElement( 'text', - 'logging_target', + 'logging_file', array( 'required' => true, - 'label' => t('Filepath'), - 'description' => t('The logfile to write messages to.'), + 'label' => t('File path'), + 'description' => t('The full path to the log file to write messages to.'), 'value' => $this->getDefaultLogDir(), 'validators' => array(new WritablePathValidator()) ) @@ -107,9 +116,9 @@ class LoggingConfigForm extends Form } /** - * Return the default logging directory for type "file" + * Return the default logging directory for type 'file' * - * @return string + * @return string */ protected function getDefaultLogDir() {