From b74e264f01000f8e018d732a8240d0595755a18b Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Wed, 4 Sep 2013 16:21:44 +0200 Subject: [PATCH] Refactor comment command handling - Refactored Comment class - Dropped IComment interface - Added AddCommentCommand class - Updated CommentForm refs #4580 --- .../Protocol/Commandpipe/CommandPipe.php | 20 ----- .../Icinga/Protocol/Commandpipe/Comment.php | 58 +++++--------- .../Icinga/Protocol/Commandpipe/Downtime.php | 2 +- .../Icinga/Protocol/Commandpipe/IComment.php | 37 --------- .../controllers/CommandController.php | 2 +- .../forms/Command/AcknowledgeForm.php | 2 +- .../application/forms/Command/CommentForm.php | 19 +++-- .../forms/Command/ScheduleDowntimeForm.php | 2 +- .../Monitoring/Command/AcknowledgeCommand.php | 52 ++++++------ .../Monitoring/Command/AddCommentCommand.php | 80 +++++++++++++++++++ .../Monitoring/Command/BaseCommand.php | 2 +- .../Command/ScheduleDowntimeFormTest.php | 8 +- .../Commandpipe/AcknowledgementTest.php | 5 +- .../Commandpipe/CommandPipeLoader.php | 2 +- .../Protocol/Commandpipe/CommandPipeTest.php | 66 ++++++++------- 15 files changed, 193 insertions(+), 164 deletions(-) delete mode 100644 library/Icinga/Protocol/Commandpipe/IComment.php create mode 100644 modules/monitoring/library/Monitoring/Command/AddCommentCommand.php diff --git a/library/Icinga/Protocol/Commandpipe/CommandPipe.php b/library/Icinga/Protocol/Commandpipe/CommandPipe.php index bdd73ad8c..54f59fc10 100644 --- a/library/Icinga/Protocol/Commandpipe/CommandPipe.php +++ b/library/Icinga/Protocol/Commandpipe/CommandPipe.php @@ -237,26 +237,6 @@ class CommandPipe } } - /** - * Add a comment to all submitted objects - * - * @param array $objects An array of hosts and services to add a comment for - * @param Comment $comment The comment object to add - */ - public function addComment(array $objects, Comment $comment) - { - foreach ($objects as $object) { - if (isset($object->service_description)) { - $format = $comment->getFormatString(self::TYPE_SERVICE); - $this->send(sprintf($format, $object->host_name, $object->service_description)); - } else { - $format = $comment->getFormatString(self::TYPE_HOST); - $this->send(sprintf($format, $object->host_name)); - } - } - - } - /** * Removes the submitted comments * diff --git a/library/Icinga/Protocol/Commandpipe/Comment.php b/library/Icinga/Protocol/Commandpipe/Comment.php index 4b4103d74..ac55bfd15 100644 --- a/library/Icinga/Protocol/Commandpipe/Comment.php +++ b/library/Icinga/Protocol/Commandpipe/Comment.php @@ -30,70 +30,56 @@ namespace Icinga\Protocol\Commandpipe; /** * Container for comment information that can be send to icinga's external command pipe - * */ -class Comment implements IComment +class Comment { /** - * Whether the persistent flag should be submitted with this command + * Whether this comment is persistent or not * * @var bool */ - public $persistent = false; + public $persistent; /** - * The author of this comment + * The author of this comment * - * @var string + * @var string */ - public $author = ""; + public $author; /** - * The comment text to use + * The text of this comment * - * @var string + * @var string */ - public $comment = ""; + public $content; /** * Create a new comment object * - * @param string $author The author name to use for this object - * @param string $comment The comment text to use - * @param bool $persistent Whether this comment should persist icinga restarts + * @param string $author The name of the comment's author + * @param string $content The text for this comment + * @param bool $persistent Whether this comment should be persistent or not */ - public function __construct($author, $comment, $persistent = false) + public function __construct($author, $content, $persistent = false) { $this->author = $author; - $this->comment = $comment; + $this->content = $content; $this->persistent = $persistent; } /** - * Return this comment as an ADD_?_COMMENT external command string that can directly be send to the command pipe + * Return this comment's properties as list of command parameters * - * @param string $type either CommandPipe::TYPE_HOST or CommandPipe::TYPE_SERVICE - * - * @return string The ADD_HOST_COMMENT or ADD_SVC_COMMENT command, without the timestamp - * - * @throws InvalidCommandException When $type is unknown + * @param bool $ignorePersistentFlag Whether the persistent flag should be included or not + * @return array */ - public function getFormatString($type) + public function getParameters($ignorePersistentFlag = false) { - $params = ';' . ($this->persistent ? '1' : '0') . ';' . $this->author . ';' . $this->comment; - - switch ($type) { - case CommandPipe::TYPE_HOST: - $typeVar = "HOST"; - $params = ";%s" . $params; - break; - case CommandPipe::TYPE_SERVICE: - $typeVar = "SVC"; - $params = ";%s;%s" . $params; - break; - default: - throw new InvalidCommandException("Acknowledgements can only apply on hosts and services "); + if ($ignorePersistentFlag) { + return array($this->author, $this->content); + } else { + return array($this->persistent ? '1' : '0', $this->author, $this->content); } - return "ADD_{$typeVar}_COMMENT$params"; } } diff --git a/library/Icinga/Protocol/Commandpipe/Downtime.php b/library/Icinga/Protocol/Commandpipe/Downtime.php index 3e5f7a3d7..fe0f246e2 100644 --- a/library/Icinga/Protocol/Commandpipe/Downtime.php +++ b/library/Icinga/Protocol/Commandpipe/Downtime.php @@ -148,7 +148,7 @@ class Downtime . $this->trigger_id . ';' . $this->duration . ';' . $this->comment->author . ';' - . $this->comment->comment; + . $this->comment->content; } /** diff --git a/library/Icinga/Protocol/Commandpipe/IComment.php b/library/Icinga/Protocol/Commandpipe/IComment.php deleted file mode 100644 index a374a4ff2..000000000 --- a/library/Icinga/Protocol/Commandpipe/IComment.php +++ /dev/null @@ -1,37 +0,0 @@ - - * @license http://www.gnu.org/licenses/gpl-2.0.txt GPL, version 2 - * @author Icinga Development Team - */ -// {{{ICINGA_LICENSE_HEADER}}} - -namespace Icinga\Protocol\Commandpipe; - -/** - * Interface flagging a class as being a comment - * - */ -interface IComment -{ -} diff --git a/modules/monitoring/application/controllers/CommandController.php b/modules/monitoring/application/controllers/CommandController.php index 277e9107e..b43e05ed3 100644 --- a/modules/monitoring/application/controllers/CommandController.php +++ b/modules/monitoring/application/controllers/CommandController.php @@ -651,7 +651,7 @@ class Monitoring_CommandController extends ActionController $this->setForm($form); if ($form->IsSubmittedAndValid() === true) { - $this->target->addComment($this->view->objects, $form->getComment()); + $this->target->sendCommand($form->createCommand(), $this->view->objects); } } diff --git a/modules/monitoring/application/forms/Command/AcknowledgeForm.php b/modules/monitoring/application/forms/Command/AcknowledgeForm.php index d4b8b372d..9b9dfdbdd 100644 --- a/modules/monitoring/application/forms/Command/AcknowledgeForm.php +++ b/modules/monitoring/application/forms/Command/AcknowledgeForm.php @@ -31,7 +31,7 @@ namespace Icinga\Module\Monitoring\Form\Command; use \Icinga\Web\Form\Element\DateTimePicker; use \Icinga\Protocol\Commandpipe\Comment; use \Icinga\Util\DateTimeFactory; -use \Monitoring\Command\AcknowledgeCommand; +use \Icinga\Module\Monitoring\Command\AcknowledgeCommand; /** * Form for problem acknowledgements diff --git a/modules/monitoring/application/forms/Command/CommentForm.php b/modules/monitoring/application/forms/Command/CommentForm.php index a9175d1cb..e8b60e74a 100644 --- a/modules/monitoring/application/forms/Command/CommentForm.php +++ b/modules/monitoring/application/forms/Command/CommentForm.php @@ -28,7 +28,8 @@ namespace Icinga\Module\Monitoring\Form\Command; -use \Icinga\Protocol\Commandpipe\Comment; +use Icinga\Protocol\Commandpipe\Comment; +use Icinga\Module\Monitoring\Command\AddCommentCommand; /** * Form for adding comment commands @@ -40,6 +41,8 @@ class CommentForm extends CommandForm */ protected function create() { + $this->setName('form_CommentForm'); + $this->addNote(t('This command is used to add a comment to hosts or services.')); $this->addElement($this->createAuthorField()); @@ -78,12 +81,18 @@ class CommentForm extends CommandForm } /** - * Create comment from request data + * Create the command object to add comments * - * @return \Icinga\Protocol\Commandpipe\Comment + * @return AddCommentCommand */ - public function getComment() + public function createCommand() { - return new Comment($this->getAuthorName(), $this->getValue('comment'), $this->getValue('persistent')); + return new AddCommentCommand( + new Comment( + $this->getAuthorName(), + $this->getValue('comment'), + $this->getValue('persistent') + ) + ); } } diff --git a/modules/monitoring/application/forms/Command/ScheduleDowntimeForm.php b/modules/monitoring/application/forms/Command/ScheduleDowntimeForm.php index 4848c2ba3..55075b4ed 100644 --- a/modules/monitoring/application/forms/Command/ScheduleDowntimeForm.php +++ b/modules/monitoring/application/forms/Command/ScheduleDowntimeForm.php @@ -35,7 +35,7 @@ use \Icinga\Web\Form\Element\DateTimePicker; use \Icinga\Protocol\Commandpipe\Downtime; use \Icinga\Protocol\Commandpipe\Comment; use \Icinga\Util\DateTimeFactory; -use \Monitoring\Backend; +use \Icinga\Module\Monitoring\Backend; /** * Form for scheduling downtimes diff --git a/modules/monitoring/library/Monitoring/Command/AcknowledgeCommand.php b/modules/monitoring/library/Monitoring/Command/AcknowledgeCommand.php index 8c3bc089a..2bff674d1 100644 --- a/modules/monitoring/library/Monitoring/Command/AcknowledgeCommand.php +++ b/modules/monitoring/library/Monitoring/Command/AcknowledgeCommand.php @@ -26,9 +26,9 @@ */ // {{{ICINGA_LICENSE_HEADER}}} -namespace Monitoring\Command; +namespace Icinga\Module\Monitoring\Command; -use \Icinga\Protocol\Commandpipe\Comment; +use Icinga\Protocol\Commandpipe\Comment; class AcknowledgeCommand extends BaseCommand { @@ -44,7 +44,7 @@ class AcknowledgeCommand extends BaseCommand * * @var Comment */ - protected $comment; + private $comment; /** * Whether to set the notify flag of this acknowledgment @@ -60,6 +60,22 @@ class AcknowledgeCommand extends BaseCommand */ private $sticky; + /** + * Initialise a new acknowledgement command object + * + * @param Comment $comment The comment to use for this acknowledgement + * @param int $expire The expire time or -1 of not expiring + * @param bool $notify Whether to set the notify flag + * @param bool $sticky Whether to set the sticky flag + */ + public function __construct(Comment $comment, $expire = -1, $notify = false, $sticky = false) + { + $this->expireTime = $expire; + $this->comment = $comment; + $this->notify = $notify; + $this->sticky = $sticky; + } + /** * Set the time when this acknowledgement should expire * @@ -109,34 +125,18 @@ class AcknowledgeCommand extends BaseCommand } /** - * Initialise a new acknowledgement command object - * - * @param Comment $comment The comment to use for this acknowledgement - * @param int $expire The expire time or -1 of not expiring - * @param bool $notify Whether to set the notify flag - * @param bool $sticky Whether to set the sticky flag - */ - public function __construct(Comment $comment, $expire = -1, $notify = false, $sticky = false) - { - $this->expireTime = $expire; - $this->comment = $comment; - $this->notify = $notify; - $this->sticky = $sticky; - } - - /** - * Return the parameters in the right order + * Return this command's parameters properly arranged in an array * * @return array */ public function getParameters() { - $parameters = array( - $this->sticky ? '2' : '0', - $this->notify ? '1' : '0', - $this->comment->persistent ? '1' : '0', - $this->comment->author, - $this->comment->comment + $parameters = array_merge( + array( + $this->sticky ? '2' : '0', + $this->notify ? '1' : '0' + ), + $this->comment->getParameters() ); if ($this->expireTime > -1) { diff --git a/modules/monitoring/library/Monitoring/Command/AddCommentCommand.php b/modules/monitoring/library/Monitoring/Command/AddCommentCommand.php new file mode 100644 index 000000000..2c546e85e --- /dev/null +++ b/modules/monitoring/library/Monitoring/Command/AddCommentCommand.php @@ -0,0 +1,80 @@ + + * @license http://www.gnu.org/licenses/gpl-2.0.txt GPL, version 2 + * @author Icinga Development Team + */ +// {{{ICINGA_LICENSE_HEADER}}} + +namespace Icinga\Module\Monitoring\Command; + +use Icinga\Protocol\Commandpipe\Comment; + +class AddCommentCommand extends BaseCommand +{ + /** + * The comment associated to this command + * + * @var Comment + */ + private $comment; + + /** + * Initialise a new command object to add comments + * + * @param Comment $comment The comment to use for this acknowledgement + */ + public function __construct(Comment $comment) + { + $this->comment = $comment; + } + + /** + * Set the comment for this command + * + * @param Comment $comment + * @return self + */ + public function setComment(Comment $comment) + { + $this->comment = $comment; + return $this; + } + + /** + * @see BaseCommand::getHostCommand() + */ + public function getHostCommand($hostname) + { + return sprintf('ADD_HOST_COMMENT;%s;', $hostname) . implode(';', $this->comment->getParameters()); + } + + /** + * @see BaseCommand::getServiceCommand() + */ + public function getServiceCommand($hostname, $servicename) + { + return sprintf('ADD_SVC_COMMENT;%s;%s;', $hostname, $servicename) + . implode(';', $this->comment->getParameters()); + } +} diff --git a/modules/monitoring/library/Monitoring/Command/BaseCommand.php b/modules/monitoring/library/Monitoring/Command/BaseCommand.php index 9a968d617..770c9878a 100644 --- a/modules/monitoring/library/Monitoring/Command/BaseCommand.php +++ b/modules/monitoring/library/Monitoring/Command/BaseCommand.php @@ -26,7 +26,7 @@ */ // {{{ICINGA_LICENSE_HEADER}}} -namespace Monitoring\Command; +namespace Icinga\Module\Monitoring\Command; use Icinga\Exception\NotImplementedError; use Icinga\Protocol\Commandpipe\CommandType; diff --git a/modules/monitoring/test/php/application/forms/Command/ScheduleDowntimeFormTest.php b/modules/monitoring/test/php/application/forms/Command/ScheduleDowntimeFormTest.php index bcb8e67fb..125f01ee0 100644 --- a/modules/monitoring/test/php/application/forms/Command/ScheduleDowntimeFormTest.php +++ b/modules/monitoring/test/php/application/forms/Command/ScheduleDowntimeFormTest.php @@ -42,13 +42,13 @@ require_once BaseTestCase::$libDir . '/Util/ConfigAwareFactory.php'; require_once BaseTestCase::$moduleDir . '/monitoring/application/forms/Command/ScheduleDowntimeForm.php'; // @codingStandardsIgnoreEnd -use \DateTimeZone; -use \Icinga\Util\DateTimeFactory; -use \Monitoring\Form\Command\ScheduleDowntimeForm; // Used by constant FORM_CLASS +use DateTimeZone; +use Icinga\Util\DateTimeFactory; +use Icinga\Module\Monitoring\Form\Command\ScheduleDowntimeForm; // Used by constant FORM_CLASS class ScheduleDowntimeFormTest extends BaseTestCase { - const FORM_CLASS = 'Monitoring\Form\Command\ScheduleDowntimeForm'; + const FORM_CLASS = 'Icinga\Module\Monitoring\Form\Command\ScheduleDowntimeForm'; /** * Set up the default time zone diff --git a/test/php/library/Icinga/Protocol/Commandpipe/AcknowledgementTest.php b/test/php/library/Icinga/Protocol/Commandpipe/AcknowledgementTest.php index a4e9bc190..3c676fd54 100644 --- a/test/php/library/Icinga/Protocol/Commandpipe/AcknowledgementTest.php +++ b/test/php/library/Icinga/Protocol/Commandpipe/AcknowledgementTest.php @@ -3,9 +3,8 @@ namespace Tests\Icinga\Protocol\Commandpipe; use Icinga\Protocol\Commandpipe\Comment; -use Monitoring\Command\AcknowledgeCommand; +use Icinga\Module\Monitoring\Command\AcknowledgeCommand; -require_once("../../library/Icinga/Protocol/Commandpipe/IComment.php"); require_once("../../library/Icinga/Protocol/Commandpipe/Comment.php"); require_once("../../library/Icinga/Protocol/Commandpipe/CommandType.php"); require_once("../../library/Icinga/Protocol/Commandpipe/CommandPipe.php"); @@ -25,7 +24,7 @@ class AcknowledgementTest extends \PHPUnit_Framework_TestCase public function testAcknowledgeServiceMessage() { - $ack = new AcknowledgeCommand(new Comment("author","commentdata")); + $ack = new AcknowledgeCommand(new Comment("author", "commentdata")); $this->assertEquals("ACKNOWLEDGE_SVC_PROBLEM;foo;bar;0;0;0;author;commentdata", $ack->getServiceCommand('foo', 'bar')); $ack->setExpire(1000); diff --git a/test/php/library/Icinga/Protocol/Commandpipe/CommandPipeLoader.php b/test/php/library/Icinga/Protocol/Commandpipe/CommandPipeLoader.php index 4aa06b745..0f446788b 100644 --- a/test/php/library/Icinga/Protocol/Commandpipe/CommandPipeLoader.php +++ b/test/php/library/Icinga/Protocol/Commandpipe/CommandPipeLoader.php @@ -16,7 +16,6 @@ class CommandPipeLoader extends LibraryLoader { require_once("Zend/Log.php"); require_once("../../library/Icinga/Application/Logger.php"); - require_once("../../library/Icinga/Protocol/Commandpipe/IComment.php"); require_once("../../library/Icinga/Protocol/Commandpipe/Comment.php"); require_once("../../library/Icinga/Protocol/Commandpipe/CommandType.php"); require_once("../../library/Icinga/Protocol/Commandpipe/CommandPipe.php"); @@ -29,6 +28,7 @@ class CommandPipeLoader extends LibraryLoader { require_once('../../library/Icinga/Protocol/Commandpipe/CustomNotification.php'); require_once('../../modules/monitoring/library/Monitoring/Command/BaseCommand.php'); require_once('../../modules/monitoring/library/Monitoring/Command/AcknowledgeCommand.php'); + require_once('../../modules/monitoring/library/Monitoring/Command/AddCommentCommand.php'); } } // @codingStandardsIgnoreEnd diff --git a/test/php/library/Icinga/Protocol/Commandpipe/CommandPipeTest.php b/test/php/library/Icinga/Protocol/Commandpipe/CommandPipeTest.php index 81e9809bd..c35c848f2 100644 --- a/test/php/library/Icinga/Protocol/Commandpipe/CommandPipeTest.php +++ b/test/php/library/Icinga/Protocol/Commandpipe/CommandPipeTest.php @@ -8,13 +8,15 @@ namespace Tests\Icinga\Protocol\Commandpipe; require_once(__DIR__.'/CommandPipeLoader.php'); CommandPipeLoader::requireLibrary(); -use Icinga\Protocol\Commandpipe\Comment as Comment; +use Zend_Config; +use Icinga\Protocol\Commandpipe\Comment; use Icinga\Protocol\Commandpipe\CustomNotification; use Icinga\Protocol\Commandpipe\Downtime as Downtime; use Icinga\Protocol\Commandpipe\Commandpipe as Commandpipe; use Icinga\Protocol\Commandpipe\PropertyModifier as MONFLAG; use Icinga\Protocol\Ldap\Exception; -use Monitoring\Command\AcknowledgeCommand; +use Icinga\Module\Monitoring\Command\AcknowledgeCommand; +use Icinga\Module\Monitoring\Command\AddCommentCommand; if(!defined("EXTCMD_TEST_BIN")) define("EXTCMD_TEST_BIN", "./bin/extcmd_test"); @@ -50,14 +52,14 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase $this->cleanup(); touch($tmpPipe); - $cfg = new \Zend_Config(array( - "path" => $tmpPipe, - "name" => "test" - )); - $comment = new Comment("Autor","Comment"); - $pipe = new Commandpipe($cfg); + $cfg = new Zend_Config( + array( + "path" => $tmpPipe, + "name" => "test" + ) + ); - return $pipe; + return new Commandpipe($cfg); } /** @@ -71,18 +73,18 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase $this->cleanup(); touch($tmpPipe); - $cfg = new \Zend_Config(array( - "path" => $tmpPipe, - "user" => "vagrant", - "password" => "vagrant", - "host" => 'localhost', - "port" => 22, - "name" => "test" - )); - $comment = new Comment("Autor","Comment"); - $pipe = new Commandpipe($cfg); + $cfg = new Zend_Config( + array( + "path" => $tmpPipe, + "user" => "vagrant", + "password" => "vagrant", + "host" => 'localhost', + "port" => 22, + "name" => "test" + ) + ); - return $pipe; + return new Commandpipe($cfg); } /** @@ -131,7 +133,7 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase { $pipe = $this->getLocalTestPipe(); try { - $ack = new AcknowledgeCommand(new Comment("I can","sends teh ack")); + $ack = new AcknowledgeCommand(new Comment("I can", "sends teh ack")); $pipe->sendCommand( $ack, array( @@ -157,7 +159,7 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase { $pipe = $this->getLocalTestPipe(); try { - $ack = new AcknowledgeCommand(new Comment("I can","sends teh ack")); + $ack = new AcknowledgeCommand(new Comment("I can", "sends teh ack")); $pipe->getTransport()->setOpenMode("a"); $pipe->sendCommand( $ack, @@ -193,14 +195,24 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase /** * Test whether a single host comment is correctly serialized and send to the command pipe * - * @throws \Exception|Exception + * @throws Exception */ public function testAddHostComment() { $pipe = $this->getLocalTestPipe(); try { - $pipe->addComment(array((object) array("host_name" => "hostA")), - new Comment("Autor","Comment") + $pipe->sendCommand( + new AddCommentCommand( + new Comment( + "Autor", + "Comment" + ) + ), + array( + (object) array( + "host_name" => "hostA" + ) + ) ); $this->assertCommandSucceeded("ADD_HOST_COMMENT;hostA;0;Autor;Comment"); } catch(Exception $e) { @@ -364,7 +376,7 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase { $pipe = $this->getLocalTestPipe(); try { - $downtime = new Downtime(25,26,new Comment("me","test")); + $downtime = new Downtime(25, 26, new Comment("me", "test")); $pipe->scheduleDowntime(array( (object) array( "host_name" => "Testhost" @@ -490,7 +502,7 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase } $pipe = $this->getSSHTestPipe(); try { - $ack = new AcknowledgeCommand(new Comment("I can","sends teh ack")); + $ack = new AcknowledgeCommand(new Comment("I can", "sends teh ack")); $pipe->sendCommand( $ack, array(