diff --git a/application/forms/Config/UserBackendConfigForm.php b/application/forms/Config/UserBackendConfigForm.php index 421313820..f7d4d1572 100644 --- a/application/forms/Config/UserBackendConfigForm.php +++ b/application/forms/Config/UserBackendConfigForm.php @@ -200,12 +200,6 @@ class UserBackendConfigForm extends ConfigForm } $backendConfig->merge($data); - foreach ($backendConfig->toArray() as $k => $v) { - if ($v === null) { - unset($backendConfig->$k); - } - } - $this->config->setSection($name, $backendConfig); return $this; } diff --git a/application/forms/Config/UserGroup/UserGroupBackendForm.php b/application/forms/Config/UserGroup/UserGroupBackendForm.php index 84c1cb4f8..42e76e347 100644 --- a/application/forms/Config/UserGroup/UserGroupBackendForm.php +++ b/application/forms/Config/UserGroup/UserGroupBackendForm.php @@ -127,14 +127,7 @@ class UserGroupBackendForm extends ConfigForm unset($data['name']); } - $backendConfig->merge($data); - foreach ($backendConfig->toArray() as $k => $v) { - if ($v === null) { - unset($backendConfig->$k); - } - } - - $this->config->setSection($name, $backendConfig); + $this->config->setSection($name, $backendConfig->merge($data)); return $this; } diff --git a/application/forms/Navigation/NavigationConfigForm.php b/application/forms/Navigation/NavigationConfigForm.php index 158875c87..9bfc3a02a 100644 --- a/application/forms/Navigation/NavigationConfigForm.php +++ b/application/forms/Navigation/NavigationConfigForm.php @@ -426,11 +426,6 @@ class NavigationConfigForm extends ConfigForm } $itemConfig->merge($data); - foreach ($itemConfig->toArray() as $k => $v) { - if ($v === null) { - unset($itemConfig->$k); - } - } if ($shared) { // Share all descendant children diff --git a/library/Icinga/File/Ini/IniWriter.php b/library/Icinga/File/Ini/IniWriter.php index 83410599f..f00040e39 100644 --- a/library/Icinga/File/Ini/IniWriter.php +++ b/library/Icinga/File/Ini/IniWriter.php @@ -149,6 +149,10 @@ class IniWriter $domSection = $doc->getSection($section); } foreach ($directives as $key => $value) { + if ($value === null) { + continue; + } + if ($value instanceof ConfigObject) { throw new ProgrammingError('Cannot diff recursive configs'); } diff --git a/modules/monitoring/application/forms/Config/BackendConfigForm.php b/modules/monitoring/application/forms/Config/BackendConfigForm.php index 78cb049de..e7fdba67c 100644 --- a/modules/monitoring/application/forms/Config/BackendConfigForm.php +++ b/modules/monitoring/application/forms/Config/BackendConfigForm.php @@ -147,12 +147,6 @@ class BackendConfigForm extends ConfigForm } $backendConfig->merge($data); - foreach ($backendConfig->toArray() as $k => $v) { - if ($v === null) { - unset($backendConfig->$k); - } - } - $this->config->setSection($name, $backendConfig); return $this; } diff --git a/modules/monitoring/application/forms/Config/TransportConfigForm.php b/modules/monitoring/application/forms/Config/TransportConfigForm.php index 0e5ad4f49..79ac1741b 100644 --- a/modules/monitoring/application/forms/Config/TransportConfigForm.php +++ b/modules/monitoring/application/forms/Config/TransportConfigForm.php @@ -167,12 +167,6 @@ class TransportConfigForm extends ConfigForm } $transportConfig->merge($data); - foreach ($transportConfig->toArray() as $k => $v) { - if ($v === null) { - unset($transportConfig->$k); - } - } - $this->config->setSection($name, $transportConfig); return $this; } diff --git a/test/php/library/Icinga/File/Ini/IniWriterTest.php b/test/php/library/Icinga/File/Ini/IniWriterTest.php index 3148779ef..9e38aa9bd 100644 --- a/test/php/library/Icinga/File/Ini/IniWriterTest.php +++ b/test/php/library/Icinga/File/Ini/IniWriterTest.php @@ -400,4 +400,53 @@ EOD; file_put_contents($this->tempFile, $config); return $this->tempFile; } + + public function testWhetherNullValuesGetPersisted() + { + $config = Config::fromArray(array()); + $section = $config->getSection('garbage'); + $section->foobar = null; + $config->setSection('garbage', $section); + + $iniWriter = new IniWriter($config, '/dev/null'); + $this->assertNotContains( + 'foobar', + $iniWriter->render(), + 'IniWriter persists section keys with null values' + ); + } + + public function testWhetherEmptyValuesGetPersisted() + { + $config = Config::fromArray(array()); + $section = $config->getSection('garbage'); + $section->foobar = ''; + $config->setSection('garbage', $section); + + $iniWriter = new IniWriter($config, '/dev/null'); + $this->assertContains( + 'foobar', + $iniWriter->render(), + 'IniWriter doesn\'t persist section keys with empty values' + ); + } + + public function testExplicitRemove() + { + $filename = tempnam(sys_get_temp_dir(), 'iw2'); + $config = Config::fromArray(array('garbage' => array('foobar' => 'lolcat'))); + $iniWriter = new IniWriter($config, $filename); + $iniWriter->write(); + + $section = $config->getSection('garbage'); + $section->foobar = null; + $iniWriter = new IniWriter($config, $filename); + $this->assertNotContains( + 'foobar', + $iniWriter->render(), + 'IniWriter doesn\'t remove section keys with null values' + ); + + unlink($filename); + } }