[icinga-checkins] icinga.org: icingaweb2/master: ConfigForm: Introduce and utilize method transformEmptyValuesToNull()

git at icinga.org git at icinga.org
Thu Dec 1 10:56:18 CET 2016


Module: icingaweb2
Branch: master
Commit: 2fa854b0a814a13cdd93fed1836c4a41d3194e61
URL:    https://git.icinga.org/?p=icingaweb2.git;a=commit;h=2fa854b0a814a13cdd93fed1836c4a41d3194e61

Author: Johannes Meyer <johannes.meyer at netways.de>
Date:   Thu Dec  1 10:54:09 2016 +0100

ConfigForm: Introduce and utilize method transformEmptyValuesToNull()

This utility method serves as alternative for all previous custom solutions to prevent empty values from being persisted to INI files.

Since the IniWriter now handles NULL correctly, we're able to refrain from using array_filter for this purpose which was the actual cause for the referenced bug.

fixes #13357

---

 application/controllers/ConfigController.php             |    9 ++-------
 application/controllers/NavigationController.php         |    9 ++-------
 application/controllers/UsergroupbackendController.php   |    9 ++-------
 application/forms/Config/ResourceConfigForm.php          |    4 ++--
 application/forms/ConfigForm.php                         |   14 +++++++++++++-
 application/forms/Security/RoleForm.php                  |    2 +-
 .../application/controllers/ConfigController.php         |    4 ++--
 7 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/application/controllers/ConfigController.php b/application/controllers/ConfigController.php
index d22e3a6..997c6a7 100644
--- a/application/controllers/ConfigController.php
+++ b/application/controllers/ConfigController.php
@@ -215,7 +215,7 @@ class ConfigController extends Controller
 
         $form->setOnSuccess(function (UserBackendConfigForm $form) {
             try {
-                $form->add(array_filter($form->getValues()));
+                $form->add($form::transformEmptyValuesToNull($form->getValues()));
             } catch (Exception $e) {
                 $form->error($e->getMessage());
                 return false;
@@ -246,12 +246,7 @@ class ConfigController extends Controller
         $form->setIniConfig(Config::app('authentication'));
         $form->setOnSuccess(function (UserBackendConfigForm $form) use ($backendName) {
             try {
-                $form->edit($backendName, array_map(
-                    function ($v) {
-                        return $v !== '' ? $v : null;
-                    },
-                    $form->getValues()
-                ));
+                $form->edit($backendName, $form::transformEmptyValuesToNull($form->getValues()));
             } catch (Exception $e) {
                 $form->error($e->getMessage());
                 return false;
diff --git a/application/controllers/NavigationController.php b/application/controllers/NavigationController.php
index 221ff39..1d2d39f 100644
--- a/application/controllers/NavigationController.php
+++ b/application/controllers/NavigationController.php
@@ -219,7 +219,7 @@ class NavigationController extends Controller
         $form->setDefaultUrl(rawurldecode($this->params->get('url', '')));
 
         $form->setOnSuccess(function (NavigationConfigForm $form) {
-            $data = array_filter($form->getValues());
+            $data = $form::transformEmptyValuesToNull($form->getValues());
 
             try {
                 $form->add($data);
@@ -266,12 +266,7 @@ class NavigationController extends Controller
         $form->setUserConfig(Config::navigation($itemType, $itemOwner));
         $form->setRedirectUrl($referrer === 'shared' ? 'navigation/shared' : 'navigation');
         $form->setOnSuccess(function (NavigationConfigForm $form) use ($itemName) {
-            $data = array_map(
-                function ($v) {
-                    return $v !== '' ? $v : null;
-                },
-                $form->getValues()
-            );
+            $data = $form::transformEmptyValuesToNull($form->getValues());
 
             try {
                 $form->edit($itemName, $data);
diff --git a/application/controllers/UsergroupbackendController.php b/application/controllers/UsergroupbackendController.php
index 14456fc..a9e411c 100644
--- a/application/controllers/UsergroupbackendController.php
+++ b/application/controllers/UsergroupbackendController.php
@@ -43,7 +43,7 @@ class UsergroupbackendController extends Controller
         $form->setIniConfig(Config::app('groups'));
         $form->setOnSuccess(function (UserGroupBackendForm $form) {
             try {
-                $form->add(array_filter($form->getValues()));
+                $form->add($form::transformEmptyValuesToNull($form->getValues()));
             } catch (Exception $e) {
                 $form->error($e->getMessage());
                 return false;
@@ -73,12 +73,7 @@ class UsergroupbackendController extends Controller
         $form->setIniConfig(Config::app('groups'));
         $form->setOnSuccess(function (UserGroupBackendForm $form) use ($backendName) {
             try {
-                $form->edit($backendName, array_map(
-                    function ($v) {
-                        return $v !== '' ? $v : null;
-                    },
-                    $form->getValues()
-                ));
+                $form->edit($backendName, $form::transformEmptyValuesToNull($form->getValues()));
             } catch (Exception $e) {
                 $form->error($e->getMessage());
                 return false;
diff --git a/application/forms/Config/ResourceConfigForm.php b/application/forms/Config/ResourceConfigForm.php
index dfa8852..bbf54cb 100644
--- a/application/forms/Config/ResourceConfigForm.php
+++ b/application/forms/Config/ResourceConfigForm.php
@@ -181,10 +181,10 @@ class ResourceConfigForm extends ConfigForm
                         return false;
                     }
                 }
-                $this->add(array_filter($this->getValues()));
+                $this->add(static::transformEmptyValuesToNull($this->getValues()));
                 $message = $this->translate('Resource "%s" has been successfully created');
             } else { // edit existing resource
-                $this->edit($resource, array_filter($this->getValues()));
+                $this->edit($resource, static::transformEmptyValuesToNull($this->getValues()));
                 $message = $this->translate('Resource "%s" has been successfully changed');
             }
         } catch (InvalidArgumentException $e) {
diff --git a/application/forms/ConfigForm.php b/application/forms/ConfigForm.php
index 74164be..b69da5a 100644
--- a/application/forms/ConfigForm.php
+++ b/application/forms/ConfigForm.php
@@ -58,7 +58,7 @@ class ConfigForm extends Form
     {
         $sections = array();
         foreach ($this->getValues() as $sectionAndPropertyName => $value) {
-            if ($value === '') {
+            if (empty($value)) {
                 $value = null; // Causes the config writer to unset it
             }
 
@@ -127,4 +127,16 @@ class ConfigForm extends Form
     {
         $config->saveIni();
     }
+
+    /**
+     * Transform all empty values of the given array to null
+     *
+     * @param   array   $values
+     *
+     * @return  array
+     */
+    public static function transformEmptyValuesToNull(array $values)
+    {
+        return array_map(function ($v) { return empty($v) ? null : $v; }, $values);
+    }
 }
diff --git a/application/forms/Security/RoleForm.php b/application/forms/Security/RoleForm.php
index 2833013..de3f98a 100644
--- a/application/forms/Security/RoleForm.php
+++ b/application/forms/Security/RoleForm.php
@@ -289,7 +289,7 @@ class RoleForm extends ConfigForm
      */
     public function getValues($suppressArrayNotation = false)
     {
-        $values = array_filter(parent::getValues($suppressArrayNotation));
+        $values = static::transformEmptyValuesToNull(parent::getValues($suppressArrayNotation));
         if (isset($values['permissions'])) {
             $values['permissions'] = implode(', ', $values['permissions']);
         }
diff --git a/modules/monitoring/application/controllers/ConfigController.php b/modules/monitoring/application/controllers/ConfigController.php
index 09aeb7d..33ddaea 100644
--- a/modules/monitoring/application/controllers/ConfigController.php
+++ b/modules/monitoring/application/controllers/ConfigController.php
@@ -106,7 +106,7 @@ class ConfigController extends Controller
 
         $form->setOnSuccess(function (BackendConfigForm $form) {
             try {
-                $form->add(array_filter($form->getValues()));
+                $form->add($form::transformEmptyValuesToNull($form->getValues()));
             } catch (Exception $e) {
                 $form->error($e->getMessage());
                 return false;
@@ -258,7 +258,7 @@ class ConfigController extends Controller
         );
         $form->setOnSuccess(function (TransportConfigForm $form) {
             try {
-                $form->add(array_filter($form->getValues()));
+                $form->add($form::transformEmptyValuesToNull($form->getValues()));
             } catch (Exception $e) {
                 $form->error($e->getMessage());
                 return false;



More information about the icinga-checkins mailing list