AssignRenderer: fix and test NOT on root level

fixes #1777

doc/82-Changelog.md

@@ -14,6 +14,7 @@ before switching to a new version.
 * FIX: restoring a basket fails when there is only one configured DB (#1716)
 * FIX: creating a new Basket with a "Custom Selection" failed with an error (#1733)
 * FIX: some new reserved keywords are now escaped correctly (#1765)
+* FIX: correctly render NOT used in apply rules (fixes #1777)
 
 1.6.0
 -----

library/Director/IcingaConfig/AssignRenderer.php

@@ -17,6 +17,7 @@ use Icinga\Data\Filter\FilterMatch;
 use Icinga\Data\Filter\FilterMatchNot;
 use Icinga\Data\Filter\FilterNotEqual;
 use Icinga\Exception\QueryException;
+use InvalidArgumentException;
 
 class AssignRenderer
 {
@@ -49,6 +50,14 @@ class AssignRenderer
 
     protected function renderFilter(Filter $filter)
     {
+        if ($filter instanceof FilterNot) {
+            $parts = [];
+            foreach ($filter->filters() as $sub) {
+                $parts[] = $this->renderFilter($sub);
+            }
+
+            return '!(' . implode(' && ', $parts) . ')';
+        }
         if ($filter->isChain()) {
             /** @var FilterChain $filter */
             return $this->renderFilterChain($filter);
@@ -235,31 +244,23 @@ class AssignRenderer
         } elseif ($filter instanceof FilterOr) {
             $op = ' || ';
         } elseif ($filter instanceof FilterNot) {
-            $op = ' !'; // TODO -> different
+            throw new InvalidArgumentException('renderFilterChain should never get a FilterNot instance');
         } else {
-            throw new QueryException('Cannot render filter: %s', $filter);
+            throw new InvalidArgumentException('Cannot render filter: %s', $filter);
         }
 
         $parts = array();
         if (! $filter->isEmpty()) {
             /** @var Filter $f */
             foreach ($filter->filters() as $f) {
-                if ($f->isChain()) {
+                if ($f instanceof FilterChain && $f->count() > 1) {
-                    if ($f instanceof FilterNot) {
+                    $parts[] = '(' . $this->renderFilter($f) . ')';
-                        $parts[] = '! (' . $this->renderFilter($f) . ')';
-                    } else {
-                        $parts[] = '(' . $this->renderFilter($f) . ')';
-                    }
                 } else {
                     $parts[] = $this->renderFilter($f);
                 }
             }
         }
 
-        if ($filter instanceof FilterNot) {
+        return implode($op, $parts);
-            return implode(' && ', $parts);
-        } else {
-            return implode($op, $parts);
-        }
     }
 }

test/php/library/Director/IcingaConfig/AssignRendererTest.php

@@ -18,6 +18,26 @@ class AssignRendererTest extends BaseTestCase
         );
     }
 
+    public function testNegationIsRenderedCorrectlyOnRootLevel()
+    {
+        $string = '!(host.name="one"&host.name="two")';
+        $expected = 'assign where !(host.name == "one" && host.name == "two")';
+        $this->assertEquals(
+            $expected,
+            $this->renderer($string)->renderAssign()
+        );
+    }
+
+    public function testNegationIsRenderedCorrectlyOnDeeperLevel()
+    {
+        $string = 'host.address="127.*"&!host.name="localhost"';
+        $expected = 'assign where match("127.*", host.address) && !(host.name == "localhost")';
+        $this->assertEquals(
+            $expected,
+            $this->renderer($string)->renderAssign()
+        );
+    }
+
     public function testWildcardsRenderAMatchMethod()
     {
         $string = 'host.address="127.0.0.*"';