Refactor comment command handling

- Refactored Comment class
- Dropped IComment interface
- Added AddCommentCommand class
- Updated CommentForm

refs #4580

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
      *

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  $author         The name of the comment's author
-     * @param string $comment   The comment text to use
+     * @param   string  $content        The text for this comment
-     * @param bool $persistent  Whether this comment should persist icinga restarts
+     * @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
+     * @param   bool    $ignorePersistentFlag   Whether the persistent flag should be included or not
-     *
+     * @return  array
-     * @return string                       The ADD_HOST_COMMENT or ADD_SVC_COMMENT command, without the timestamp
-     *
-     * @throws InvalidCommandException      When $type is unknown
      */
-    public function getFormatString($type)
+    public function getParameters($ignorePersistentFlag = false)
     {
-        $params = ';' . ($this->persistent ? '1' : '0') . ';' . $this->author . ';' . $this->comment;
+        if ($ignorePersistentFlag) {
-
+            return array($this->author, $this->content);
-        switch ($type) {
+        } else {
-            case CommandPipe::TYPE_HOST:
+            return array($this->persistent ? '1' : '0', $this->author, $this->content);
-                $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 ");
         }
-        return "ADD_{$typeVar}_COMMENT$params";
     }
 }

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;
     }
 
     /**

library/Icinga/Protocol/Commandpipe/IComment.php

@@ -1,37 +0,0 @@
-<?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\Protocol\Commandpipe;
-
-/**
- * Interface flagging a class as being a comment
- *
- */
-interface IComment
-{
-}

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);
         }
     }
 

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

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')
+            )
+        );
     }
 }

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

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
+     * Return this command's parameters properly arranged in an array
-     *
-     * @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 array
      */
     public function getParameters()
     {
-        $parameters = array(
+        $parameters = array_merge(
-            $this->sticky ? '2' : '0',
+            array(
-            $this->notify ? '1' : '0',
+                $this->sticky ? '2' : '0',
-            $this->comment->persistent ? '1' : '0',
+                $this->notify ? '1' : '0'
-            $this->comment->author,
+            ),
-            $this->comment->comment
+            $this->comment->getParameters()
         );
 
         if ($this->expireTime > -1) {

modules/monitoring/library/Monitoring/Command/AddCommentCommand.php

@@ -0,0 +1,80 @@
+<?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\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());
+    }
+}

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;

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 DateTimeZone;
-use \Icinga\Util\DateTimeFactory;
+use Icinga\Util\DateTimeFactory;
-use \Monitoring\Form\Command\ScheduleDowntimeForm; // Used by constant FORM_CLASS
+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

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);

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

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(
+        $cfg = new Zend_Config(
-            "path" => $tmpPipe,
+            array(
-            "name" => "test"
+                "path" => $tmpPipe,
-        ));
+                "name" => "test"
-        $comment = new Comment("Autor","Comment");
+            )
-        $pipe = new Commandpipe($cfg);
+        );
 
-        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(
+        $cfg = new Zend_Config(
-            "path" => $tmpPipe,
+            array(
-            "user"  => "vagrant",
+                "path"      => $tmpPipe,
-            "password" => "vagrant",
+                "user"      => "vagrant",
-            "host" => 'localhost',
+                "password"  => "vagrant",
-            "port"  => 22,
+                "host"      => 'localhost',
-            "name" => "test"
+                "port"      => 22,
-        ));
+                "name"      => "test"
-        $comment = new Comment("Autor","Comment");
+            )
-        $pipe = new Commandpipe($cfg);
+        );
 
-        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")),
+            $pipe->sendCommand(
-                new Comment("Autor","Comment")
+                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(