Merge branch 'feature/resolve-runtime-macros-6392'

resolves #6392

modules/monitoring/application/controllers/HostsController.php

@@ -56,6 +56,7 @@ class Monitoring_HostsController extends Controller
             'host_icon_image',
             'host_icon_image_alt',
             'host_name',
+            'host_address',
             'host_state',
             'host_problem',
             'host_handled',
@@ -94,6 +95,7 @@ class Monitoring_HostsController extends Controller
             'host_icon_image',
             'host_icon_image_alt',
             'host_name',
+            'host_address',
             'host_state',
             'host_problem',
             'host_handled',

modules/monitoring/application/controllers/ServicesController.php

@@ -53,6 +53,7 @@ class Monitoring_ServicesController extends Controller
             'host_icon_image',
             'host_icon_image_alt',
             'host_name',
+            'host_address',
             'host_output',
             'host_state',
             'host_problem',
@@ -100,6 +101,7 @@ class Monitoring_ServicesController extends Controller
             'host_icon_image',
             'host_icon_image_alt',
             'host_name',
+            'host_address',
             'host_output',
             'host_state',
             'host_problem',

modules/monitoring/application/views/helpers/IconImage.php

@@ -1,6 +1,8 @@
 <?php
 /* Icinga Web 2 | (c) 2013-2015 Icinga Development Team | GPLv2+ */
 
+use Icinga\Module\Monitoring\Object\Macro;
+
 /**
  * Generate icons to describe a given hosts state
  */
@@ -26,7 +28,7 @@ class Zend_View_Helper_IconImage extends Zend_View_Helper_Abstract
     {
         if ($object->host_icon_image && ! preg_match('/[\'"]/', $object->host_icon_image)) {
             return $this->view->img(
-                'img/icons/' . $this->view->resolveMacros($object->host_icon_image, $object),
+                'img/icons/' . Macro::resolveMacros($object->host_icon_image, $object),
                 null,
                 array(
                     'alt' => $object->host_icon_image_alt,
@@ -48,7 +50,7 @@ class Zend_View_Helper_IconImage extends Zend_View_Helper_Abstract
     {
         if ($object->service_icon_image && ! preg_match('/[\'"]/', $object->service_icon_image)) {
             return $this->view->img(
-                'img/icons/' . $this->view->resolveMacros($object->service_icon_image, $object),
+                'img/icons/' . Macro::resolveMacros($object->service_icon_image, $object),
                 null,
                 array(
                     'alt' => $object->service_icon_image_alt,

modules/monitoring/application/views/scripts/show/components/actions.phtml

@@ -1,29 +1,16 @@
 <?php
 
-$links = array();
-
 // add warning to links that open in new tabs to improve accessibility, as recommended by WCAG20 G201
 $newTabInfo = sprintf('<span class="info-box display-on-hover"> %s </span>', $this->translate('opens in new window'));
 
-$linkText = '<a href="%s" target="_blank">%s ' . $newTabInfo . '</a>';
+$links = $object->getActionUrls();
-$localLinkText = '<a href="%s">%s</a>';
+foreach ($links as $i => $link) {
-
+    $links[$i] = sprintf('<a href="%s" target="_blank">%s ' . $newTabInfo . '</a>', $link, 'Action');
-if ($object->action_url) {
-    if (strpos($object->action_url, "' ") === false) {
-        $links[] = sprintf($linkText, $this->resolveMacros($object->action_url, $object), 'Action');
-    } else {
-        // TODO: We should find out document what's going on here. Looks strange :p
-        foreach(explode("' ", $object->action_url) as $url) {
-            $url = strpos($url, "'") === 0 ? substr($url, 1) : $url;
-            $url = strrpos($url, "'") === strlen($url) - 1 ? substr($url, 0, strlen($url) - 1) : $url;
-            $links[] = sprintf($linkText, $this->resolveMacros($url, $object), 'Action');
-        }
-    }
 }
 
 if (isset($this->actions)) {
     foreach ($this->actions as $id => $action) {
-        $links[] = sprintf($localLinkText, $action, $id);
+        $links[] = sprintf('<a href="%s">%s</a>', $action, $id);
     }
 }
 
@@ -34,5 +21,5 @@ if (empty($links)) {
 ?>
 <tr>
   <th><?= $this->translate('Actions') ?></th>
-  <td><?= implode("\n ", $links) . "\n" ?></td>
+  <td><?= implode("<br>", $links) ?></td>
 </tr>

modules/monitoring/application/views/scripts/show/components/notes.phtml

@@ -1,15 +1,13 @@
 <?php
 $notes = trim($object->getNotes());
 $links = $object->getNotesUrls();
-?>
 
-<?php if (! empty($links) || ! empty($notes)): ?>
+if (! empty($links) || ! empty($notes)): ?>
 <tr>
     <th><?= $this->translate('Notes') ?></th>
     <td>
         <?php
             if (! empty($notes)) {
-                $notes = $this->resolveMacros($notes, $object);
                 echo $notes . '<br>';
             }
             // add warning to links that open in new tabs to improve accessibility, as recommended by WCAG20 G201
@@ -19,8 +17,7 @@ $links = $object->getNotesUrls();
             );
             $linkText = '<a href="%s" target="_blank">%s ' . $newTabInfo . '</a>';
             foreach ($links as $i => $link) {
-                $resolved = $this->resolveMacros($link, $object);
+                $links[$i] = sprintf($linkText, $this->escape($link), $this->escape($link));
-                $links[$i] = sprintf($linkText, $this->escape($resolved), $this->escape($resolved));
             }
             echo implode('<br>', $links);
         ?>

modules/monitoring/library/Monitoring/Object/Host.php

@@ -7,7 +7,7 @@ use InvalidArgumentException;
 use Icinga\Module\Monitoring\Backend\MonitoringBackend;
 
 /**
- * A Icinga host
+ * An Icinga host
  */
 class Host extends MonitoredObject
 {
@@ -192,7 +192,9 @@ class Host extends MonitoredObject
 
     public function getNotesUrls()
     {
-        return MonitoredObject::parseAttributeUrls($this->host_notes_url);
+        return $this->resolveAllStrings(
+            MonitoredObject::parseAttributeUrls($this->host_notes_url)
+        );
     }
 
     public function getNotes()

modules/monitoring/library/Monitoring/Object/Macro.php

@@ -1,20 +1,25 @@
 <?php
 /* Icinga Web 2 | (c) 2013-2015 Icinga Development Team | GPLv2+ */
 
-use \Zend_View_Helper_Abstract;
+namespace Icinga\Module\Monitoring\Object;
-use Icinga\Module\Monitoring\Object\MonitoredObject;
 
-class Zend_View_Helper_ResolveMacros extends Zend_View_Helper_Abstract
+/**
+ * Expand macros in string in the context of MonitoredObjects
+ */
+class Macro
 {
     /**
      * Known icinga macros
      *
      * @var array
      */
-    private $icingaMacros = array(
+    private static $icingaMacros = array(
         'HOSTNAME'     => 'host_name',
         'HOSTADDRESS'  => 'host_address',
-        'SERVICEDESC'   => 'service_description'
+        'SERVICEDESC'  => 'service_description',
+        'host.name'    => 'host_name',
+        'host.address' => 'host_address',
+        'service.description' => 'service_description'
     );
 
     /**
@@ -25,12 +30,12 @@ class Zend_View_Helper_ResolveMacros extends Zend_View_Helper_Abstract
      *
      * @return  string                                  The substituted or unchanged string
      */
-    public function resolveMacros($input, $object)
+    public static function resolveMacros($input, $object)
     {
         $matches = array();
         if (preg_match_all('@\$([^\$\s]+)\$@', $input, $matches)) {
             foreach ($matches[1] as $key => $value) {
-                $newValue = $this->resolveMacro($value, $object);
+                $newValue = self::resolveMacro($value, $object);
                 if ($newValue !== $value) {
                     $input = str_replace($matches[0][$key], $newValue, $input);
                 }
@@ -48,10 +53,10 @@ class Zend_View_Helper_ResolveMacros extends Zend_View_Helper_Abstract
      *
      * @return  string                                  The new value or the macro if it cannot be resolved
      */
-    public function resolveMacro($macro, $object)
+    public static function resolveMacro($macro, $object)
     {
-        if (array_key_exists($macro, $this->icingaMacros) && $object->{$this->icingaMacros[$macro]} !== false) {
+        if (array_key_exists($macro, self::$icingaMacros) && $object->{self::$icingaMacros[$macro]} !== false) {
-            return $object->{$this->icingaMacros[$macro]};
+            return $object->{self::$icingaMacros[$macro]};
         }
         if (array_key_exists($macro, $object->customvars)) {
             return $object->customvars[$macro];

modules/monitoring/library/Monitoring/Object/MonitoredObject.php

@@ -569,18 +569,47 @@ abstract class MonitoredObject implements Filterable
      *
      * @return string   The notes as a string
      */
-    public function getNotes() {}
+    public abstract function getNotes();
 
     /**
      * Get all note urls configured for this monitored object
      *
      * @return array    All note urls as a string
      */
-    public function getNotesUrls() {}
+    public abstract function getNotesUrls();
+
+    /**
+     * Get all action urls configured for this monitored object
+     *
+     * @return array    All note urls as a string
+     */
+    public function getActionUrls()
+    {
+        return $this->resolveAllStrings(
+            MonitoredObject::parseAttributeUrls($this->action_url)
+        );
+    }
+
+    /**
+     * Resolve macros in all given strings in the current object context
+     *
+     * @param   array   $strs   An array of urls as string
+     * @return  type
+     */
+    protected function resolveAllStrings(array $strs)
+    {
+        foreach ($strs as $i => $str) {
+            $strs[$i] = Macro::resolveMacros($str, $this);
+        }
+        return $strs;
+    }
 
     /**
      * Parse the content of the action_url or notes_url attributes
      *
+     * Find all occurences of http links, separated by whitespaces and quoted
+     * by single or double-ticks.
+     *
      * @link http://docs.icinga.org/latest/de/objectdefinitions.html
      *
      * @param   string  $urlString  A string containing one or more urls

modules/monitoring/library/Monitoring/Object/Service.php

@@ -7,7 +7,7 @@ use InvalidArgumentException;
 use Icinga\Module\Monitoring\Backend\MonitoringBackend;
 
 /**
- * A Icinga service
+ * An Icinga service
  */
 class Service extends MonitoredObject
 {
@@ -202,7 +202,9 @@ class Service extends MonitoredObject
 
     public function getNotesUrls()
     {
-        return MonitoredObject::parseAttributeUrls($this->service_notes_url);
+        return $this->resolveAllStrings(
+            MonitoredObject::parseAttributeUrls($this->service_notes_url)
+        );
     }
 
     public function getNotes()

modules/monitoring/test/php/application/views/helpers/ResolveMacrosTest.php

@@ -1,78 +0,0 @@
-<?php
-/* Icinga Web 2 | (c) 2013-2015 Icinga Development Team | GPLv2+ */
-
-namespace Tests\Icinga\Modules\Monitoring\Application\Views\Helpers;
-
-use Mockery;
-use Zend_View_Helper_ResolveMacros;
-use Icinga\Test\BaseTestCase;
-
-require_once realpath(BaseTestCase::$moduleDir . '/monitoring/application/views/helpers/ResolveMacros.php');
-
-class ResolveMacrosTest extends BaseTestCase
-{
-    public function testHostMacros()
-    {
-        $hostMock = Mockery::mock('host');
-        $hostMock->host_name = 'test';
-        $hostMock->host_address = '1.1.1.1';
-
-        $helper = new Zend_View_Helper_ResolveMacros();
-        $this->assertEquals($helper->resolveMacros('$HOSTNAME$', $hostMock), $hostMock->host_name);
-        $this->assertEquals($helper->resolveMacros('$HOSTADDRESS$', $hostMock), $hostMock->host_address);
-    }
-
-    public function testServiceMacros()
-    {
-        $svcMock = Mockery::mock('service');
-        $svcMock->host_name = 'test';
-        $svcMock->host_address = '1.1.1.1';
-        $svcMock->service_description = 'a service';
-
-        $helper = new Zend_View_Helper_ResolveMacros();
-        $this->assertEquals($helper->resolveMacros('$HOSTNAME$', $svcMock), $svcMock->host_name);
-        $this->assertEquals($helper->resolveMacros('$HOSTADDRESS$', $svcMock), $svcMock->host_address);
-        $this->assertEquals($helper->resolveMacros('$SERVICEDESC$', $svcMock), $svcMock->service_description);
-    }
-
-    public function testCustomvars()
-    {
-        $objectMock = Mockery::mock('object');
-        $objectMock->customvars = array(
-            'CUSTOMVAR' => 'test'
-        );
-
-        $helper = new Zend_View_Helper_ResolveMacros();
-        $this->assertEquals($helper->resolveMacros('$CUSTOMVAR$', $objectMock), $objectMock->customvars['CUSTOMVAR']);
-    }
-
-    public function testFaultyMacros()
-    {
-        $hostMock = Mockery::mock('host');
-        $hostMock->host_name = 'test';
-        $hostMock->customvars = array(
-            'HOST' => 'te',
-            'NAME' => 'st'
-        );
-
-        $helper = new Zend_View_Helper_ResolveMacros();
-        $this->assertEquals(
-            $helper->resolveMacros('$$HOSTNAME$ $ HOSTNAME$ $HOST$NAME$', $hostMock),
-            '$test $ HOSTNAME$ teNAME$'
-        );
-    }
-
-    public function testMacrosWithSpecialCharacters()
-    {
-        $objectMock = Mockery::mock('object');
-        $objectMock->customvars = array(
-            'V€RY_SP3C|@L' => 'not too special!'
-        );
-
-        $helper = new Zend_View_Helper_ResolveMacros();
-        $this->assertEquals(
-            $helper->resolveMacros('$V€RY_SP3C|@L$', $objectMock),
-            $objectMock->customvars['V€RY_SP3C|@L']
-        );
-    }
-}

modules/monitoring/test/php/library/Monitoring/Object/MacroTest.php

@@ -0,0 +1,78 @@
+<?php
+/* Icinga Web 2 | (c) 2013-2015 Icinga Development Team | GPLv2+ */
+
+namespace Tests\Icinga\Modules\Monitoring\Application\Views\Helpers;
+
+use Mockery;
+use Icinga\Test\BaseTestCase;
+use Icinga\Module\Monitoring\Object\Macro;
+
+require_once realpath(BaseTestCase::$moduleDir . '/monitoring/library/Monitoring/Object/Macro.php');
+
+class MacroTest extends BaseTestCase
+{
+    public function testHostMacros()
+    {
+        $hostMock = Mockery::mock('host');
+        $hostMock->host_name = 'test';
+        $hostMock->host_address = '1.1.1.1';
+
+        $this->assertEquals(Macro::resolveMacros('$HOSTNAME$', $hostMock), $hostMock->host_name);
+        $this->assertEquals(Macro::resolveMacros('$HOSTADDRESS$', $hostMock), $hostMock->host_address);
+        $this->assertEquals(Macro::resolveMacros('$host.name$', $hostMock), $hostMock->host_name);
+        $this->assertEquals(Macro::resolveMacros('$host.address$', $hostMock), $hostMock->host_address);
+    }
+
+    public function testServiceMacros()
+    {
+        $svcMock = Mockery::mock('service');
+        $svcMock->host_name = 'test';
+        $svcMock->host_address = '1.1.1.1';
+        $svcMock->service_description = 'a service';
+
+        $this->assertEquals(Macro::resolveMacros('$HOSTNAME$', $svcMock), $svcMock->host_name);
+        $this->assertEquals(Macro::resolveMacros('$HOSTADDRESS$', $svcMock), $svcMock->host_address);
+        $this->assertEquals(Macro::resolveMacros('$SERVICEDESC$', $svcMock), $svcMock->service_description);
+        $this->assertEquals(Macro::resolveMacros('$host.name$', $svcMock), $svcMock->host_name);
+        $this->assertEquals(Macro::resolveMacros('$host.address$', $svcMock), $svcMock->host_address);
+        $this->assertEquals(Macro::resolveMacros('$service.description', $svcMock), $svcMock->service_description);
+    }
+
+    public function testCustomvars()
+    {
+        $objectMock = Mockery::mock('object');
+        $objectMock->customvars = array(
+            'CUSTOMVAR' => 'test'
+        );
+
+        $this->assertEquals(Macro::resolveMacros('$CUSTOMVAR$', $objectMock), $objectMock->customvars['CUSTOMVAR']);
+    }
+
+    public function testFaultyMacros()
+    {
+        $hostMock = Mockery::mock('host');
+        $hostMock->host_name = 'test';
+        $hostMock->customvars = array(
+            'HOST' => 'te',
+            'NAME' => 'st'
+        );
+
+        $this->assertEquals(
+            Macro::resolveMacros('$$HOSTNAME$ $ HOSTNAME$ $HOST$NAME$', $hostMock),
+            '$test $ HOSTNAME$ teNAME$'
+        );
+    }
+
+    public function testMacrosWithSpecialCharacters()
+    {
+        $objectMock = Mockery::mock('object');
+        $objectMock->customvars = array(
+            'V€RY_SP3C|@L' => 'not too special!'
+        );
+
+        $this->assertEquals(
+            Macro::resolveMacros('$V€RY_SP3C|@L$', $objectMock),
+            $objectMock->customvars['V€RY_SP3C|@L']
+        );
+    }
+}