[icinga-checkins] icinga.org: icingaweb2-module-director/feature/service-sets-12891: IcingaObjectFieldLoader: allow exotic var names

git at icinga.org git at icinga.org
Wed Nov 9 11:37:06 CET 2016


Module: icingaweb2-module-director
Branch: feature/service-sets-12891
Commit: 63bf607a1df926d7bfe0b7f2868652c2b2ee06ad
URL:    https://git.icinga.org/?p=icingaweb2-module-director.git;a=commit;h=63bf607a1df926d7bfe0b7f2868652c2b2ee06ad

Author: Thomas Gelf <thomas at gelf.net>
Date:   Tue Nov  8 02:00:38 2016 +0100

IcingaObjectFieldLoader: allow exotic var names

fixes #12094
fixes #12962

---

 .../Director/Web/Controller/ObjectsController.php  |    9 ++++
 library/Director/Web/Form/DirectorObjectForm.php   |   38 +++++++--------
 .../Director/Web/Form/IcingaObjectFieldLoader.php  |   49 ++++++++++++++++----
 3 files changed, 68 insertions(+), 28 deletions(-)

diff --git a/library/Director/Web/Controller/ObjectsController.php b/library/Director/Web/Controller/ObjectsController.php
index 70ae4e0..bc80e24 100644
--- a/library/Director/Web/Controller/ObjectsController.php
+++ b/library/Director/Web/Controller/ObjectsController.php
@@ -2,6 +2,8 @@
 
 namespace Icinga\Module\Director\Web\Controller;
 
+use Icinga\Data\Filter\FilterChain;
+use Icinga\Data\Filter\FilterExpression;
 use Icinga\Exception\NotFoundError;
 use Icinga\Data\Filter\Filter;
 use Icinga\Module\Director\Objects\IcingaObject;
@@ -9,6 +11,7 @@ use Icinga\Module\Director\Web\Table\IcingaObjectTable;
 
 abstract class ObjectsController extends ActionController
 {
+    /** @var IcingaObject */
     protected $dummy;
 
     protected $isApified = true;
@@ -156,8 +159,11 @@ abstract class ObjectsController extends ActionController
         $dummy = $this->dummyObject();
         $objects = array();
         $db = $this->db();
+        /** @var $filter FilterChain */
         foreach ($filter->filters() as $sub) {
+            /** @var $sub FilterChain */
             foreach ($sub->filters() as $ex) {
+                /** @var $ex FilterChain|FilterExpression */
                 if ($ex->isExpression() && $ex->getColumn() === 'name') {
                     $name = $ex->getExpression();
                     $objects[$name] = $dummy::load($name, $db);
@@ -215,6 +221,9 @@ abstract class ObjectsController extends ActionController
         $this->setViewScript('objects/table');
     }
 
+    /**
+     * @return IcingaObject
+     */
     protected function dummyObject()
     {
         if ($this->dummy === null) {
diff --git a/library/Director/Web/Form/DirectorObjectForm.php b/library/Director/Web/Form/DirectorObjectForm.php
index f28c242..b50dbe2 100644
--- a/library/Director/Web/Form/DirectorObjectForm.php
+++ b/library/Director/Web/Form/DirectorObjectForm.php
@@ -220,7 +220,7 @@ abstract class DirectorObjectForm extends QuickForm
         $resolve = $this->assertResolvedImports();
         if ($this->hasBeenSent()) {
             foreach ($values as $key => $value) {
-                if ($key === 'imports') {
+                if ($key === 'imports' || substr($key, 0, 4) === 'var_') {
                     continue;
                 }
 
@@ -282,14 +282,21 @@ abstract class DirectorObjectForm extends QuickForm
         return $result;
     }
 
-    protected function handleCustomVars($object, & $values)
+    protected function loadFields($object)
     {
         if ($this->assertResolvedImports()) {
             $loader = $this->fieldLoader($object);
             $loader->addFieldsToForm($this);
-            if ($values) {
-                $loader->setValues($values, 'var_');
-            }
+        }
+
+        return $this;
+    }
+
+    protected function setCustomVarValues($object, & $values)
+    {
+        if ($this->assertResolvedImports()) {
+            $loader = $this->fieldLoader($object);
+            $loader->setValues($values, 'var_');
         }
 
         return $this;
@@ -580,6 +587,7 @@ abstract class DirectorObjectForm extends QuickForm
         $values = array();
 
         $object = $this->object();
+        $this->loadFields($object);
         if ($this->hasBeenSent()) {
 
             if ($this->shouldBeDeleted()) {
@@ -587,27 +595,19 @@ abstract class DirectorObjectForm extends QuickForm
             }
 
             $post = $this->getRequest()->getPost();
-            // ?? $this->populate($post);
+            $this->populate($post);
+            $values = $this->getValues();
 
-            foreach ($post as $key => $value) {
-                $el = $this->getElement($key);
-                if ($el) {
-                    if (! $el->getIgnore()) {
-                        $values[$key] = $el->setValue($value)->getValue();
-                    }
-                } elseif ($sub = $this->getSubForm($key)) {
-                    $values[$key] = $sub->populate($value)->getValues();
-                }
+            if ($object instanceof IcingaObject) {
+                $this->setCustomVarValues($object, $values);
             }
         }
 
         if ($object instanceof IcingaObject) {
-            $this->handleProperties($object, $values);
-            $this->handleCustomVars($object, $post);
             $this->handleRanges($object, $values);
-        } else {
-            $this->handleProperties($object, $values);
         }
+        $this->handleProperties($object, $values);
+
         /*
         // TODO: something like this could be used to remember unstored changes
         if ($object->hasBeenModified()) {
diff --git a/library/Director/Web/Form/IcingaObjectFieldLoader.php b/library/Director/Web/Form/IcingaObjectFieldLoader.php
index acf860e..94a8260 100644
--- a/library/Director/Web/Form/IcingaObjectFieldLoader.php
+++ b/library/Director/Web/Form/IcingaObjectFieldLoader.php
@@ -2,9 +2,9 @@
 
 namespace Icinga\Module\Director\Web\Form;
 
+use Icinga\Exception\IcingaException;
 use stdClass;
 use Icinga\Module\Director\Objects\IcingaObject;
-use Icinga\Module\Director\Objects\IcingaServiceSet;
 use Icinga\Module\Director\Objects\DirectorDatafield;
 use Zend_Form_Element as ZfElement;
 
@@ -18,6 +18,9 @@ class IcingaObjectFieldLoader
 
     protected $elements;
 
+    /** @var array Map element names to variable names 'elName' => 'varName' */
+    protected $nameMap = array();
+
     public function __construct(IcingaObject $object)
     {
         $this->object = $object;
@@ -86,16 +89,24 @@ class IcingaObjectFieldLoader
                 }
             }
 
-            if ($el = $this->getElement($key)) {
-                $el->setValue($value);
-                $value = $el->getValue();
+            $varName = $this->getElementVarName($prefix . $key);
+            if ($varName === null) {
+                throw new IcingaException(
+                    'Cannot set variable value for "%s", got no such element',
+                    $key
+                );
+            }
 
-                if ($value === '') {
-                    $value = null;
-                }
+            $el = $this->getElement($varName);
+            if ($el === null) {
+                throw new IcingaException('No such element %s', $key);
+            }
 
-                $vars->set($key, $value);
+            $value = $el->getValue();
+            if ($value === '') {
+                $value = null;
             }
+            $vars->set($varName, $value);
         }
 
         return $this;
@@ -151,6 +162,15 @@ class IcingaObjectFieldLoader
         }
     }
 
+    protected function getElementVarName($name)
+    {
+        if (array_key_exists($name, $this->nameMap)) {
+            return $this->nameMap[$name];
+        }
+
+        return null;
+    }
+
     /**
      * Get the form element for a specific field by it's variable name
      *
@@ -176,7 +196,18 @@ class IcingaObjectFieldLoader
         $elements = array();
 
         foreach ($this->getFields() as $name => $field) {
-            $elements[$name] = $field->getFormElement($form);
+            $el = $field->getFormElement($form);
+            $elName = $el->getName();
+            if (array_key_exists($elName, $this->nameMap)) {
+                $form->addErrorMessage(sprintf(
+                    'Form element name collision, "%s" resolves to "%s", but this is also used for "%s"',
+                    $name,
+                    $elName,
+                    $this->nameMap[$elName]
+                ));
+            }
+            $this->nameMap[$elName] = $name;
+            $elements[$name] = $el;
         }
 
         return $elements;



More information about the icinga-checkins mailing list