PropertyModifierCombine: implementation, tests...

...and related changes with some documentation

fixes #922

application/forms/ImportSourceForm.php

@@ -69,7 +69,10 @@ class ImportSourceForm extends DirectorObjectForm
                 . ' specified this will then be used as the object_name for the syncronized'
                 . ' Icinga object. Especially when getting started with director please make'
                 . ' sure to strictly follow this rule. Duplicate values for this column on different'
-                . ' rows will trigger a failure, your import run will not succeed'
+                . ' rows will trigger a failure, your import run will not succeed. Please pay attention'
+                . ' when synching services, as "purge" will only work correctly with a key_column'
+                . ' corresponding to host!name. Check the "Combine" property modifier in case your'
+                . ' data source cannot provide such a field'
             ),
             'placeholder' => $defaultKeyCol,
             'required'    => $defaultKeyCol === null,

library/Director/Hook/PropertyModifierHook.php

@@ -13,6 +13,20 @@ abstract class PropertyModifierHook
 
     private $db;
 
+    /**
+     * @var \stdClass
+     */
+    private $row;
+
+    /**
+     * Methode to transform the given value
+     *
+     * Your custom property modifier needs to implement this method.
+     *
+     * @return value
+     */
+    abstract public function transform($value);
+
     public function getName()
     {
         $parts = explode('\\', get_class($this));
@@ -28,22 +42,93 @@ abstract class PropertyModifierHook
         return $class;
     }
 
+    /**
+     * Whether this PropertyModifier wants to deal with array on it's own
+     *
+     * When true, the whole array value will be passed to transform(), otherwise
+     * transform() will be called for every single array member
+     *
+     * @return bool
+     */
     public function hasArraySupport()
     {
         return false;
     }
 
+    /**
+     * Whether this PropertyModifier wants access to the current row
+     *
+     * When true, the your modifier can access the current row via $this->getRow()
+     *
+     * @return bool
+     */
+    public function requiresRow()
+    {
+        return false;
+    }
+
+    /**
+     * Get the current row
+     *
+     * Will be null when requiresRow was not null. Please do not modify the
+     * row. It might work right now, as we pass in an object reference for
+     * performance reasons. However, modifying row properties is not supported,
+     * and the outcome of such operation might change without pre-announcement
+     * in any future version.
+     *
+     * @return \stdClass|null
+     */
+    public function getRow()
+    {
+        return $this->row;
+    }
+
+    /**
+     * Sets the current row
+     *
+     * Please see requiresRow/getRow for related details. This method is called
+     * by the Import implementation, you should never need to call this on your
+     * own - apart from writing tests of course.
+     *
+     * @param \stdClass $row
+     * @return $this
+     */
+    public function setRow($row)
+    {
+        $this->row = $row;
+        return $this;
+    }
+
+    /**
+     * The desired target property. Modifiers might want to have their outcome
+     * written to another property of the current row.
+     *
+     * @param $property
+     * @return $this
+     */
     public function setTargetProperty($property)
     {
         $this->targetProperty = $property;
         return $this;
     }
 
+    /**
+     * Whether the result of transform() should be written to a new property
+     *
+     * The Import implementation deals with this
+     *
+     * @return bool
+     */
     public function hasTargetProperty()
     {
         return $this->targetProperty !== null;
     }
 
+    /**
+     * Get the configured target property
+     *
+     * @return string
+     */
     public function getTargetProperty($default = null)
     {
         if ($this->targetProperty === null) {
@@ -85,13 +170,6 @@ abstract class PropertyModifierHook
         return $this;
     }
 
-    /**
-     * Methode to transform the given value
-     *
-     * @return value
-     */
-    abstract public function transform($value);
-
     /**
      * Override this method if you want to extend the settings form
      *

library/Director/Import/SyncUtils.php

@@ -23,6 +23,18 @@ class SyncUtils
         }
     }
 
+    /**
+     * Whether the given string contains variable names in the form ${var_name}
+     *
+     * @param  string $string
+     *
+     * @return bool
+     */
+    public static function hasVariables($string)
+    {
+        return preg_match('/\${([^}]+)}/', $string);
+    }
+
     /**
      * Recursively extract a value from a nested structure
      *
@@ -34,11 +46,11 @@ class SyncUtils
      * return { size => '255GB' }
      *
      * @param  string $val  The value to extract data from
-     * @param  object $keys A list of nested keys pointing to desired data
+     * @param  array  $keys A list of nested keys pointing to desired data
      *
      * @return mixed
      */
-    public static function getDeepValue($val, $keys)
+    public static function getDeepValue($val, array $keys)
     {
         $key = array_shift($keys);
         if (! property_exists($val, $key)) {

library/Director/Objects/ImportSource.php

@@ -6,6 +6,7 @@ use Icinga\Application\Benchmark;
 use Icinga\Exception\ConfigurationError;
 use Icinga\Exception\NotFoundError;
 use Icinga\Module\Director\Data\Db\DbObjectWithSettings;
+use Icinga\Module\Director\Hook\PropertyModifierHook;
 use Icinga\Module\Director\Import\Import;
 use Icinga\Module\Director\Import\SyncUtils;
 use Exception;
@@ -97,7 +98,11 @@ class ImportSource extends DbObjectWithSettings
         $modifiers = $this->getRowModifiers();
 
         foreach ($modifiers as $key => $mods) {
+            /** @var PropertyModifierHook $mod */
             foreach ($mods as $mod) {
+                if ($mod->requiresRow()) {
+                    $mod->setRow($row);
+                }
                 if (! property_exists($row, $key)) {
                     // Partial support for nested keys. Must write result to
                     // a dedicated flat key

library/Director/PropertyModifier/PropertyModifierCombine.php

@@ -0,0 +1,40 @@
+<?php
+
+namespace Icinga\Module\Director\PropertyModifier;
+
+use Icinga\Module\Director\Hook\PropertyModifierHook;
+use Icinga\Module\Director\Import\SyncUtils;
+use Icinga\Module\Director\Web\Form\QuickForm;
+
+class PropertyModifierCombine extends PropertyModifierHook
+{
+    public static function addSettingsFormFields(QuickForm $form)
+    {
+        $form->addElement('text', 'pattern', array(
+            'label'       => $form->translate('Pattern'),
+            'required'    => false,
+            'description' => $form->translate(
+                'This pattern will be evaluated, and variables like ${some_column}'
+                . ' will be filled accordingly. A typical use-case is generating'
+                . ' unique service idendifiers via ${host}!${service} in case your'
+                . ' data source doesn\'t allow you to ship such. The chosen "property"'
+                . ' has no effect here and will be ignored.'
+            )
+        ));
+    }
+
+    public function getName()
+    {
+        return 'Combine multiple properties';
+    }
+
+    public function requiresRow()
+    {
+        return true;
+    }
+
+    public function transform($value)
+    {
+        return SyncUtils::fillVariables($this->getSetting('pattern'), $this->getRow());
+    }
+}

run.php

@@ -40,6 +40,7 @@ $this->provideHook('director/PropertyModifier', $prefix . 'PropertyModifier\\Pro
 $this->provideHook('director/PropertyModifier', $prefix . 'PropertyModifier\\PropertyModifierToInt');
 $this->provideHook('director/PropertyModifier', $prefix . 'PropertyModifier\\PropertyModifierLConfCustomVar');
 $this->provideHook('director/PropertyModifier', $prefix . 'PropertyModifier\\PropertyModifierArrayFilter');
+$this->provideHook('director/PropertyModifier', $prefix . 'PropertyModifier\\PropertyModifierCombine');
 
 $this->provideHook('director/Job', $prefix . 'Job\\HousekeepingJob');
 $this->provideHook('director/Job', $prefix . 'Job\\ConfigJob');

test/php/library/Director/PropertyModifier/PropertyModifierCombineTest.php

@@ -0,0 +1,51 @@
+<?php
+
+namespace Tests\Icinga\Module\Director\Objects;
+
+use Icinga\Module\Director\PropertyModifier\PropertyModifierCombine;
+use Icinga\Module\Director\Test\BaseTestCase;
+
+class PropertyModifierCombineTest extends BaseTestCase
+{
+    public function testBuildsTypicalHostServiceCombination()
+    {
+        $row = (object) array('host' => 'localhost', 'service' => 'ping');
+        $modifier = new PropertyModifierCombine();
+        $modifier->setSettings(array('pattern' => '${host}!${service}'));
+
+        $this->assertEquals(
+            'localhost!ping',
+            $modifier->setRow($row)->transform('something')
+        );
+    }
+
+    public function testDoesNotFailForMissingProperties()
+    {
+        $row = (object) array('host' => 'localhost');
+        $modifier = new PropertyModifierCombine();
+        $modifier->setSettings(array('pattern' => '${host}!${service}'));
+
+        $this->assertEquals(
+            'localhost!',
+            $modifier->setRow($row)->transform('something')
+        );
+    }
+
+    public function testDoesNotEvaluateVariablesFromDataSource()
+    {
+        $row = (object) array('host' => '${service}', 'service' => 'ping');
+        $modifier = new PropertyModifierCombine();
+        $modifier->setSettings(array('pattern' => '${host}!${service}'));
+
+        $this->assertEquals(
+            '${service}!ping',
+            $modifier->setRow($row)->transform('something')
+        );
+    }
+
+    public function testRequiresRow()
+    {
+        $modifier = new PropertyModifierCombine();
+        $this->assertTrue($modifier->requiresRow());
+    }
+}