Arguments: externalize, improve code, fix issues

library/Director/Objects/Extension/Arguments.php

@@ -0,0 +1,61 @@
+<?php
+
+namespace Icinga\Module\Director\Objects\Extension;
+
+use Icinga\Module\Director\Objects\IcingaArguments;
+use Icinga\Module\Director\Objects\IcingaObject;
+
+trait Arguments
+{
+    private $arguments;
+
+    public function arguments()
+    {
+        /** @var IcingaObject $this */
+        if ($this->arguments === null) {
+            if ($this->hasBeenLoadedFromDb()) {
+                $this->arguments = IcingaArguments::loadForStoredObject($this);
+            } else {
+                $this->arguments = new IcingaArguments($this);
+            }
+        }
+
+        return $this->arguments;
+    }
+
+    public function gotArguments()
+    {
+        return null !== $this->arguments;
+    }
+
+    public function unsetArguments()
+    {
+        unset($this->arguments);
+    }
+
+    /**
+     * @return string
+     */
+    protected function renderArguments()
+    {
+        return $this->arguments()->toConfigString();
+    }
+
+    /**
+     * @param $value
+     * @return $this
+     */
+    protected function setArguments($value)
+    {
+        $this->arguments()->setArguments($value);
+        return $this;
+    }
+
+    /**
+     * @return array
+     */
+    protected function getArguments()
+    {
+        return $this->arguments()->toPlainObject();
+    }
+}

library/Director/Objects/IcingaArguments.php

@@ -7,12 +7,12 @@ use Iterator;
 use Countable;
 use Icinga\Module\Director\IcingaConfig\IcingaConfigRenderer;
 use Icinga\Module\Director\IcingaConfig\IcingaConfigHelper as c;
-use Icinga\Module\Director\Objects\IcingaCommandArgument;
 
 class IcingaArguments implements Iterator, Countable, IcingaConfigRenderer
 {
     protected $storedArguments = array();
 
+    /** @var IcingaCommandArgument[] */
     protected $arguments = array();
 
     protected $modified = false;
@@ -291,6 +291,12 @@ class IcingaArguments implements Iterator, Countable, IcingaConfigRenderer
         array $chosenProperties = null,
         $resolveIds = true
     ) {
+        if ($chosenProperties !== null) {
+            throw new ProgrammingError(
+                'IcingaArguments does not support chosenProperties[]'
+            );
+        }
+
         $args = array();
         foreach ($this->arguments as $arg) {
             if ($arg->shouldBeRemoved()) {

library/Director/Objects/IcingaCommand.php

@@ -4,9 +4,12 @@ namespace Icinga\Module\Director\Objects;
 
 use Icinga\Module\Director\IcingaConfig\IcingaConfigHelper as c;
 use Icinga\Module\Director\IcingaConfig\IcingaLegacyConfigHelper as c1;
+use Icinga\Module\Director\Objects\Extension\Arguments;
 
-class IcingaCommand extends IcingaObject
+class IcingaCommand extends IcingaObject implements ObjectWithArguments
 {
+    use Arguments;
+
     protected $table = 'icinga_command';
 
     protected $type = 'CheckCommand';
@@ -28,8 +31,6 @@ class IcingaCommand extends IcingaObject
 
     protected $supportsImports = true;
 
-    protected $supportsArguments = true;
-
     protected $supportedInLegacy = true;
 
     protected $intervalProperties = array(

library/Director/Objects/IcingaCommandArgument.php

@@ -2,7 +2,7 @@
 
 namespace Icinga\Module\Director\Objects;
 
-use Icinga\Module\Director\Objects\IcingaObject;
+use Icinga\Exception\ProgrammingError;
 use Icinga\Module\Director\IcingaConfig\IcingaConfigHelper as c;
 
 class IcingaCommandArgument extends IcingaObject
@@ -11,6 +11,8 @@ class IcingaCommandArgument extends IcingaObject
 
     protected $table = 'icinga_command_argument';
 
+    protected $supportsImports = false;
+
     protected $booleans = array(
         'skip_key'   => 'skip_key',
         'repeat_key' => 'repeat_key',
@@ -65,67 +67,108 @@ class IcingaCommandArgument extends IcingaObject
         return $this;
     }
 
+    protected function makePlainArgumentValue($value, $format)
+    {
+        if ($format === 'expression') {
+            return (object) [
+                'type' => 'Function',
+                // TODO: Not for dummy comment
+                'body' => $value
+            ];
+        } else {
+            // json or string
+            return $value;
+        }
+    }
+
+    protected function extractValueFromPlain($plain)
+    {
+        if ($plain->argument_value) {
+            return $this->makePlainArgumentValue(
+                $plain->argument_value,
+                $plain->argument_format
+            );
+        } else {
+            return null;
+        }
+    }
+
+    protected function transformPlainArgumentValue($plain)
+    {
+        if (property_exists($plain, 'argument_value')) {
+            $plain->value = $this->makePlainArgumentValue(
+                $plain->argument_value,
+                $plain->argument_format
+            );
+            unset($plain->argument_value);
+            unset($plain->argument_format);
+        }
+    }
+
+    public function toCompatPlainObject($skipDefaults = false)
+    {
+        $plain = parent::toPlainObject(
+            false,
+            $skipDefaults,
+            null,
+            false
+        );
+
+        unset($plain->id);
+
+        $this->transformPlainArgumentValue($plain);
+
+        // Will happen only combined with $skipDefaults
+        if (array_keys((array) $plain) === ['value']) {
+            return $plain->value;
+        } else {
+            if (property_exists($plain, 'sort_order') && $plain->sort_order !== null) {
+                $plain->order = $plain->sort_order;
+                unset($plain->sort_order);
+            }
+
+            return $plain;
+        }
+    }
+
+    public function toFullPlainObject($skipDefaults = false)
+    {
+        $plain = parent::toPlainObject(
+            false,
+            $skipDefaults,
+            null,
+            false
+        );
+
+        unset($plain->id);
+        unset($plain->argument_format);
+
+        return $plain;
+    }
+
     public function toPlainObject(
         $resolved = false,
         $skipDefaults = false,
         array $chosenProperties = null,
         $resolveIds = true
     ) {
-        // TODO: skipDefaults?
+        if ($resolved) {
-
+            throw new ProgrammingError(
-        $data = array();
+                'A single CommandArgument cannot be resolved'
-        if ($this->argument_value) {
+            );
-            switch ($this->argument_format) {
-                case 'string':
-                case 'json':
-                    $data['value'] = $this->argument_value;
-                    break;
-                case 'expression':
-                    $data['value'] = (object) array(
-                        'type' => 'Function',
-                        // TODO: Not for dummy comment
-                        'body' => $this->argument_value
-                    );
-                    break;
-            }
         }
 
-        if ($this->sort_order !== null) {
+        if ($chosenProperties) {
-            $data['order'] = $this->sort_order;
+            throw new ProgrammingError(
-        }
+                'IcingaCommandArgument does not support chosenProperties[]'
-
+            );
-        if ($this->set_if) {
-            $data['set_if'] = $this->set_if;
-        }
-
-        if ($this->required !== null) {
-            $data['required'] = $this->required === 'y';
-        }
-
-        if ($this->repeat_key !== null) {
-            $data['repeat_key'] = $this->repeat_key === 'y';
-        }
-
-        if ($this->description) {
-            $data['description'] = $this->description;
         }
 
+        // $resolveIds is misused here
         if ($resolveIds) {
-            if (array_keys($data) === array('value')) {
+            return $this->toCompatPlainObject($skipDefaults);
-                return $data['value'];
-            } else {
-                return (object) $data;
-            }
         } else {
-            unset($data['value']);
+            return $this->toFullPlainObject($skipDefaults);
-            unset($data['order']);
-            $data['sort_order'] = $this->sort_order;
-            $data['command_id']    = $this->command_id;
-            $data['argument_name'] = $this->argument_name;
-            $data['argument_value'] = $this->argument_value;
-            $data['argument_format'] = $this->argument_format;
-            $data['set_if_format'] = $this->set_if_format;
-            return (object) $data;
         }
     }
 

library/Director/Objects/IcingaObject.php

@@ -37,9 +37,6 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
     /** @var bool Whether this Object makes use of (time) ranges */
     protected $supportsRanges = false;
 
-    /** @var bool Whether this object supports (command) Arguments */
-    protected $supportsArguments = false;
-
     /** @var bool Whether inheritance via "imports" property is supported */
     protected $supportsImports = false;
 
@@ -121,8 +118,6 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
     /** @var  IcingaTimePeriodRanges - TODO: generic ranges */
     private $ranges;
 
-    private $arguments;
-
     private $shouldBeRemoved = false;
 
     private $resolveCache = array();
@@ -342,7 +337,6 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
         $dummy->prefetchAllRelatedTypes();
     }
 
-
     /**
      * Whether this Object supports custom variables
      *
@@ -380,7 +374,7 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
      */
     public function supportsArguments()
     {
-        return $this->supportsArguments;
+        return $this instanceof ObjectWithArguments;
     }
 
     /**
@@ -515,6 +509,9 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
 
     public function hasBeenModified()
     {
+        if (parent::hasBeenModified()) {
+            return true;
+        }
         $this->resolveUnresolvedRelatedProperties();
 
         if ($this->supportsCustomVars() && $this->vars !== null && $this->vars()->hasBeenModified()) {
@@ -533,7 +530,10 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
             return true;
         }
 
-        if ($this->supportsArguments() && $this->arguments !== null && $this->arguments()->hasBeenModified()) {
+        if ($this instanceof ObjectWithArguments
+            && $this->gotArguments()
+            && $this->arguments()->hasBeenModified()
+        ) {
             return true;
         }
 
@@ -549,7 +549,7 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
             }
         }
 
-        return parent::hasBeenModified();
+        return false;
     }
 
     protected function hasUnresolvedRelatedProperty($name)
@@ -648,7 +648,9 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
             //TODO: allow for deep keys
             $this->vars()->set(substr($key, 5), $value);
             return $this;
-        } elseif (substr($key, 0, 10) === 'arguments.') {
+        } elseif ($this instanceof ObjectWithArguments
+            && substr($key, 0, 10) === 'arguments.'
+        ) {
             $this->arguments()->set(substr($key, 10), $value);
             return $this;
         }
@@ -709,17 +711,6 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
         return $this;
     }
 
-    protected function setArguments($value)
-    {
-        $this->arguments()->setArguments($value);
-        return $this;
-    }
-
-    protected function getArguments()
-    {
-        return $this->arguments()->toPlainObject();
-    }
-
     protected function getRanges()
     {
         return $this->ranges()->getValues();
@@ -804,20 +795,6 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
         return $this->rangeClass;
     }
 
-    public function arguments()
-    {
-        $this->assertArgumentsSupport();
-        if ($this->arguments === null) {
-            if ($this->hasBeenLoadedFromDb()) {
-                $this->arguments = IcingaArguments::loadForStoredObject($this);
-            } else {
-                $this->arguments = new IcingaArguments($this);
-            }
-        }
-
-        return $this->arguments;
-    }
-
     /**
      * @return IcingaObjectImports
      */
@@ -1218,18 +1195,6 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
         return $this;
     }
 
-    protected function assertArgumentsSupport()
-    {
-        if (! $this->supportsArguments()) {
-            throw new ProgrammingError(
-                'Objects of type "%s" have no arguments',
-                $this->getType()
-            );
-        }
-
-        return $this;
-    }
-
     protected function assertImportsSupport()
     {
         if (! $this->supportsImports()) {
@@ -1406,8 +1371,8 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
      */
     protected function storeArguments()
     {
-        if ($this->supportsArguments()) {
+        if ($this instanceof ObjectWithArguments) {
-            $this->arguments !== null && $this->arguments()->store();
+            $this->gotArguments() && $this->arguments()->store();
         }
 
         return $this;
@@ -1999,11 +1964,7 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
      */
     protected function renderArguments()
     {
-        if ($this->supportsArguments()) {
+        return '';
-            return $this->arguments()->toConfigString();
-        } else {
-            return '';
-        }
     }
 
     protected function renderRelatedSets()
@@ -2554,8 +2515,7 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
             }
         }
 
-        if ($this->supportsArguments()) {
+        if ($this instanceof ObjectWithArguments) {
-            // TODO: resolve
             $props['arguments'] = $this->arguments()->toPlainObject(
                 $resolved,
                 $skipDefaults
@@ -2766,7 +2726,7 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
             }
         }
 
-        if ($this->supportsArguments()) {
+        if ($this instanceof ObjectWithArguments) {
             $args = $this->arguments()->toUnmodifiedPlainObject();
             if (! empty($args)) {
                 $props['arguments'] = $args;
@@ -2825,7 +2785,9 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
         unset($this->groups);
         unset($this->imports);
         unset($this->ranges);
-        unset($this->arguments);
+        if ($this instanceof ObjectWithArguments) {
+            $this->unsetArguments();
+        }
 
         parent::__destruct();
     }

library/Director/Objects/ObjectWithArguments.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace Icinga\Module\Director\Objects;
+
+interface ObjectWithArguments
+{
+    /**
+     * @return boolean
+     */
+    public function gotArguments();
+
+    /**
+     * @return IcingaArguments
+     */
+    public function arguments();
+
+    public function unsetArguments();
+}