From bb25de6126eebaf9b5349d2ee54c81751e391a1f Mon Sep 17 00:00:00 2001 From: Thomas Gelf Date: Wed, 30 Nov 2016 10:37:19 +0100 Subject: [PATCH 1/4] FieldLoader: fix field handling issues fixes #13241 fixes #13259 --- .../Director/Web/Form/DirectorObjectForm.php | 88 ++++++++++++++++--- .../Web/Form/IcingaObjectFieldLoader.php | 34 ++++++- 2 files changed, 104 insertions(+), 18 deletions(-) diff --git a/library/Director/Web/Form/DirectorObjectForm.php b/library/Director/Web/Form/DirectorObjectForm.php index 868d5679..68ff41b0 100644 --- a/library/Director/Web/Form/DirectorObjectForm.php +++ b/library/Director/Web/Form/DirectorObjectForm.php @@ -11,6 +11,7 @@ use Icinga\Module\Director\Exception\NestingError; use Icinga\Module\Director\IcingaConfig\StateFilterSet; use Icinga\Module\Director\IcingaConfig\TypeFilterSet; use Icinga\Module\Director\Objects\IcingaObject; +use Icinga\Module\Director\Util; use Zend_Form_Element as ZfElement; use Zend_Form_Element_Select as ZfSelect; @@ -38,6 +39,7 @@ abstract class DirectorObjectForm extends QuickForm protected $preferredObjectType; + /** @var IcingaObjectFieldLoader */ protected $fieldLoader; private $allowsExperimental; @@ -47,6 +49,16 @@ abstract class DirectorObjectForm extends QuickForm private $presetImports; + private $earlyProperties = array( + 'imports', + 'check_command', + 'check_command_id', + 'command', + 'command_id', + 'event_command', + 'event_command_id', + ); + public function setPreferredObjectType($type) { $this->preferredObjectType = $type; @@ -112,9 +124,23 @@ abstract class DirectorObjectForm extends QuickForm } if ($this->hasBeenSent()) { - if ($el = $this->getElement('imports')) { - $this->populate($this->getRequest()->getPost()); - $object->set('imports', $el->getValue()); + // prefill special properties, required to resolve fields and similar + $post = $this->getRequest()->getPost(); + foreach ($this->earlyProperties as $key) { + if ($el = $this->getElement($key)) { + if (array_key_exists($key, $post)) { + $this->populate(array($key => $post[$key])); + + try { + $old = $object->get($key); + $object->set($key, $el->getValue()); + $object->resolveUnresolvedRelatedProperties(); + } catch (Exception $e) { + $object->set($key, $old); + $this->addException($e, $key); + } + } + } } } @@ -235,7 +261,7 @@ abstract class DirectorObjectForm extends QuickForm $resolve = $this->assertResolvedImports(); if ($this->hasBeenSent()) { foreach ($values as $key => $value) { - if ($key === 'imports' || substr($key, 0, 4) === 'var_') { + if (in_array($key, $this->earlyProperties) || substr($key, 0, 4) === 'var_') { continue; } @@ -266,7 +292,11 @@ abstract class DirectorObjectForm extends QuickForm $this->setDefaults($this->removeEmptyProperties($props)); if ($resolve) { - $this->showInheritedProperties($object); + try { + $this->showInheritedProperties($object); + } catch (Exception $e) { + $this->addException($e); + } } } @@ -297,11 +327,11 @@ abstract class DirectorObjectForm extends QuickForm return $result; } - protected function loadFields($object) + protected function prepareFields($object) { if ($this->assertResolvedImports()) { - $loader = $this->fieldLoader($object); - $loader->addFieldsToForm($this); + $this->fieldLoader = new IcingaObjectFieldLoader($object); + $this->fieldLoader->prepareElements($this); } return $this; @@ -309,14 +339,21 @@ abstract class DirectorObjectForm extends QuickForm protected function setCustomVarValues($object, & $values) { - if ($this->assertResolvedImports()) { - $loader = $this->fieldLoader($object); - $loader->setValues($values, 'var_'); + if ($this->fieldLoader) { + $this->fieldLoader->setValues($values, 'var_'); } return $this; } + protected function addFields() + { + if ($this->fieldLoader) { + $this->fieldLoader->addFieldsToForm($this); + } + } + + // TODO: remove, used in sets I guess protected function fieldLoader($object) { if ($this->fieldLoader === null) { @@ -602,7 +639,7 @@ abstract class DirectorObjectForm extends QuickForm $values = array(); $object = $this->object(); - $this->loadFields($object); + $this->prepareFields($object); if ($this->hasBeenSent()) { if ($this->shouldBeDeleted()) { @@ -614,9 +651,10 @@ abstract class DirectorObjectForm extends QuickForm $values = $this->getValues(); if ($object instanceof IcingaObject) { - $this->setCustomVarValues($object, $values); + $this->setCustomVarValues($object, $post); } } + $this->addFields(); if ($object instanceof IcingaObject) { $this->handleRanges($object, $values); @@ -913,8 +951,21 @@ abstract class DirectorObjectForm extends QuickForm protected function addImportsElement($required = null) { + $required = $required !== null ? $required : !$this->isTemplate(); $enum = $this->enumAllowedTemplates(); if (empty($enum)) { + if ($required) { + if ($this->hasBeenSent()) { + $this->addError($this->translate('No template has been chosen')); + } else { + if ($this->hasPermission('director/admin')) { + $html = $this->translate('Please define a related template first'); + } else { + $html = $this->translate('No related template has been provided yet'); + } + $this->addHtml('

' . $html . '

'); + } + } return $this; } @@ -925,7 +976,7 @@ abstract class DirectorObjectForm extends QuickForm . ' matters when importing properties from multiple templates: last one' . ' wins' ), - 'required' => ($required !== null ? $required : !$this->isTemplate()), + 'required' => $required, 'multiOptions' => $this->optionallyAddFromEnum($enum), 'sorted' => true, 'value' => $this->presetImports, @@ -1293,6 +1344,15 @@ abstract class DirectorObjectForm extends QuickForm return $this; } + /** + * @param string $permission + * @return bool + */ + public function hasPermission($permission) + { + return Util::hasPermission($permission); + } + protected function allowsExperimental() { // NO, it is NOT a good idea to use this. You'll break your monitoring diff --git a/library/Director/Web/Form/IcingaObjectFieldLoader.php b/library/Director/Web/Form/IcingaObjectFieldLoader.php index 40d5454f..65a92cda 100644 --- a/library/Director/Web/Form/IcingaObjectFieldLoader.php +++ b/library/Director/Web/Form/IcingaObjectFieldLoader.php @@ -2,10 +2,11 @@ namespace Icinga\Module\Director\Web\Form; +use Exception; use Icinga\Exception\IcingaException; -use stdClass; use Icinga\Module\Director\Objects\IcingaObject; use Icinga\Module\Director\Objects\DirectorDatafield; +use stdClass; use Zend_Form_Element as ZfElement; class IcingaObjectFieldLoader @@ -102,10 +103,12 @@ class IcingaObjectFieldLoader throw new IcingaException('No such element %s', $key); } + $el->setValue($value); $value = $el->getValue(); - if ($value === '') { + if ($value === '' || $value === array()) { $value = null; } + $vars->set($varName, $value); } @@ -143,6 +146,22 @@ class IcingaObjectFieldLoader return $this->elements; } + /** + * Prepare the form elements for our fields + * + * @param QuickForm $form Optional + * + * @return self + */ + public function prepareElements(QuickForm $form = null) + { + if ($this->object->supportsFields()) { + $this->getElements($form); + } + + return $this; + } + /** * Attach our form fields to the given form * @@ -233,14 +252,20 @@ class IcingaObjectFieldLoader /** * Create the fields for our object * - * + * @param IcingaObject $object * @return DirectorDatafield[] */ protected function prepareObjectFields($object) { $fields = $this->loadResolvedFieldsForObject($object); if ($object->hasRelation('check_command')) { - $command = $object->getResolvedRelated('check_command'); + try { + $command = $object->getResolvedRelated('check_command'); + } catch (Exception $e) { + // Ignore failures + $command = null; + } + if ($command) { $cmdFields = $this->loadResolvedFieldsForObject($command); foreach ($cmdFields as $varname => $field) { @@ -336,6 +361,7 @@ class IcingaObjectFieldLoader foreach ($res as $r) { $id = $r->object_id; unset($r->object_id); + $r->object = $objects[$id]; if (! array_key_exists($id, $result)) { $result[$id] = new stdClass; From e207bb2bc92565f07baa78de4375e8ba779c0db1 Mon Sep 17 00:00:00 2001 From: Thomas Gelf Date: Wed, 30 Nov 2016 10:45:49 +0100 Subject: [PATCH 2/4] DirectorObjectForm: resolve ids --- library/Director/Web/Form/DirectorObjectForm.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/Director/Web/Form/DirectorObjectForm.php b/library/Director/Web/Form/DirectorObjectForm.php index 68ff41b0..b47a1d8e 100644 --- a/library/Director/Web/Form/DirectorObjectForm.php +++ b/library/Director/Web/Form/DirectorObjectForm.php @@ -282,7 +282,7 @@ abstract class DirectorObjectForm extends QuickForm false, false, null, - false // Do not resolve IDs + true // is default//false // Do not resolve IDs ); } else { $props = $object->getProperties(); From e60fdb31b810e8996607085301778f98e8b93f51 Mon Sep 17 00:00:00 2001 From: Thomas Gelf Date: Tue, 13 Dec 2016 16:57:05 +0100 Subject: [PATCH 3/4] DirectorObjectForm: move check_command to main fixes #13551 --- library/Director/Web/Form/DirectorObjectForm.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/Director/Web/Form/DirectorObjectForm.php b/library/Director/Web/Form/DirectorObjectForm.php index b47a1d8e..1a1bb9da 100644 --- a/library/Director/Web/Form/DirectorObjectForm.php +++ b/library/Director/Web/Form/DirectorObjectForm.php @@ -1034,7 +1034,8 @@ abstract class DirectorObjectForm extends QuickForm 'multiOptions' => $this->optionalEnum($this->db->enumCheckcommands()), 'class' => 'autosubmit', // This influences fields )); - $this->addToCheckExecutionDisplayGroup('check_command_id'); + $this->getDisplayGroup('object_definition') + ->addElement($this->getElement('check_command_id')); $eventCommands = $this->db->enumEventcommands(); From 2cfa78af14480ee92164631913ef953a16db8973 Mon Sep 17 00:00:00 2001 From: Thomas Gelf Date: Tue, 13 Dec 2016 16:57:32 +0100 Subject: [PATCH 4/4] FieldLoader: do not fail missing fields One might have toggled template or command, sent values for missing fields might therefore be perfectly legal and should be silently ignored refs #13241 --- .../Web/Form/IcingaObjectFieldLoader.php | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/library/Director/Web/Form/IcingaObjectFieldLoader.php b/library/Director/Web/Form/IcingaObjectFieldLoader.php index 65a92cda..6ca16e07 100644 --- a/library/Director/Web/Form/IcingaObjectFieldLoader.php +++ b/library/Director/Web/Form/IcingaObjectFieldLoader.php @@ -92,15 +92,21 @@ class IcingaObjectFieldLoader $varName = $this->getElementVarName($prefix . $key); if ($varName === null) { - throw new IcingaException( - 'Cannot set variable value for "%s", got no such element', - $key - ); + // throw new IcingaException( + // 'Cannot set variable value for "%s", got no such element', + // $key + // ); + + // Silently ignore additional fields. One might have switched + // template or command + continue; } $el = $this->getElement($varName); if ($el === null) { - throw new IcingaException('No such element %s', $key); + // throw new IcingaException('No such element %s', $key); + // Same here. + continue; } $el->setValue($value);