Merge branch 'bugfix/ini-writer-must-not-persist-section-keys-with-a-null-value-11743'

fixes #11743

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;
     }

application/forms/Config/UserGroup/UserGroupBackendForm.php

@@ -127,14 +127,7 @@ class UserGroupBackendForm extends ConfigForm
             unset($data['name']);
         }
 
-        $backendConfig->merge($data);
+        $this->config->setSection($name, $backendConfig->merge($data));
-        foreach ($backendConfig->toArray() as $k => $v) {
-            if ($v === null) {
-                unset($backendConfig->$k);
-            }
-        }
-
-        $this->config->setSection($name, $backendConfig);
         return $this;
     }
 

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

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');
                 }

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;
     }

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;
     }

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);
+    }
 }