IcingaTemplateResolver: detect and throw nesting..

...errors

fixes #11803

library/Director/Exception/NestingError.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Icinga\Module\Director\Exception;
+
+use Icinga\Exception\IcingaException;
+
+class NestingError extends IcingaException {}

library/Director/Objects/IcingaObject.php

@@ -6,6 +6,7 @@ use Icinga\Module\Director\CustomVariable\CustomVariables;
 use Icinga\Module\Director\Data\Db\DbObject;
 use Icinga\Module\Director\Db\Cache\PrefetchCache;
 use Icinga\Module\Director\Db;
+use Icinga\Module\Director\Exception\NestingError;
 use Icinga\Module\Director\IcingaConfig\IcingaConfig;
 use Icinga\Module\Director\IcingaConfig\IcingaConfigRenderer;
 use Icinga\Module\Director\IcingaConfig\IcingaConfigHelper as c;
@@ -753,7 +754,14 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
             $imports = array($imports);
         }
 
+        try {
             $this->imports()->set($imports);
+        } catch (NestingError $e) {
+            $this->imports = new IcingaObjectImports($this);
+            // Force modification, otherwise it won't be stored when empty
+            $this->imports->setModified()->set($imports);
+        }
+
         if ($this->imports()->hasBeenModified()) {
             $this->invalidateResolveCache();
         }
@@ -926,12 +934,20 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
         return $db->fetchOne($query);
     }
 
+    protected function triggerLoopDetection()
+    {
+        $this->templateResolver()->listResolvedParentIds();
+    }
+
     protected function resolve($what)
     {
         if ($this->hasResolveCached($what)) {
             return $this->getResolveCached($what);
         }
 
+        // Force exception
+        $this->triggerLoopDetection();
+
         $vals = array();
         $vals['_MERGED_']    = (object) array();
         $vals['_INHERITED_'] = (object) array();
@@ -1283,6 +1299,30 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
         DirectorActivityLog::logRemoval($this, $this->connection);
     }
 
+    public function toSingleIcingaConfig()
+    {
+        $config = new IcingaConfig($this->connection);
+        $object = $this;
+        if ($object->isExternal()) {
+            $object = clone($object);
+            $object->object_type = 'object';
+        }
+
+        try {
+            $object->renderToConfig($config);
+        } catch (Exception $e) {
+            $config->configFile(
+                'failed-to-render'
+            )->prepend(
+                "/** Failed to render this object **/\n"
+                . '/*  ' . $e->getMessage() . ' */'
+            );
+        }
+
+        return $config;
+    }
+
+
     public function renderToLegacyConfig(IcingaConfig $config)
     {
         if ($this->isExternal()) {

library/Director/Objects/IcingaTemplateResolver.php

@@ -3,6 +3,7 @@
 namespace Icinga\Module\Director\Objects;
 
 use Icinga\Module\Director\Db;
+use Icinga\Module\Director\Exception\NestingError;
 
 class IcingaTemplateResolver
 {
@@ -20,11 +21,16 @@ class IcingaTemplateResolver
 
     protected static $nameIdx = array();
 
+    protected static $idToName = array();
+
     public function __construct(IcingaObject $object)
     {
         $this->setObject($object);
     }
 
+    /**
+     * Set a specific object for this resolver instance
+     */
     public function setObject(IcingaObject $object)
     {
         $this->object     = $object;
@@ -47,6 +53,11 @@ class IcingaTemplateResolver
         unset(self::$templates[$type]);
     }
 
+    /**
+     * Fetch direct parents
+     *
+     * return IcingaObject[]
+     */
     public function fetchParents()
     {
         // TODO: involve lookup cache
@@ -96,16 +107,7 @@ class IcingaTemplateResolver
 
     public function fetchResolvedParents()
     {
-        // TODO: involve lookup cache
+        return $this->fetchObjectsById($this->listResolvedParentIds());
-        $res = array();
-        $class = $this->object;
-        $connection = $this->connection;
-
-        foreach ($this->listResolvedParentIds() as $id) {
-            $res[] = $class::loadWithAutoIncId($id, $connection);
-        }
-
-        return $res;
     }
 
     public function listResolvedParentIds()
@@ -117,44 +119,89 @@ class IcingaTemplateResolver
     public function listResolvedParentNames()
     {
         $this->requireTemplates();
+        if (array_key_exists($name, self::$nameIdx[$type])) {
+            return array_keys(self::$nameIdx[$type][$name]);
+        }
+
         return $this->resolveParentNames($this->object->object_name);
     }
 
-    public function resolveParentIds($id)
+    public function listParentsById($id)
     {
+        return $this->getNamesForIds($this->resolveParentIds($id));
+    }
+
+    public function listParentsByName($name)
+    {
+        return $this->resolveParentNames($name);
+    }
+
+    protected function resolveParentNames($name, &$list = array())
+    {
+        foreach ($this->listParentNames($name) as $parent) {
+            $this->assertNotInList($parent, $list, $id);
+            $list[$parent] = true;
+            $this->resolveParentIds($parent, $list);
+            unset($list[$parent]);
+            $list[$parent] = true;
+        }
+
+        return array_keys($list);
+    }
+
+    protected function resolveParentIds($id, &$list = array())
+    {
+        foreach ($this->listParentIds($id) as $parent) {
+            $this->assertNotInList($parent, $list, $id);
+            $list[$parent] = true;
+            $this->resolveParentIds($parent, $list);
+            unset($list[$parent]);
+            $list[$parent] = true;
+        }
+
+        return array_keys($list);
+    }
+
+    protected function assertNotInList($id, & $list, $root)
+    {
+        if (array_key_exists($id, $list)) {
+            $list = array_keys($list);
+            array_unshift($list, $root);
+            throw new NestingError(
+                'Loop detected: %s',
+                implode(' -> ', $this->getNamesForIds($list))
+            );
+        }
+    }
+
+    protected function getNamesForIds($ids)
+    {
+        $names = array();
+        foreach ($ids as $id) {
+            $names[] = $this->getNameForId($id);
+        }
+
+        return $names;
+    }
+
+    protected function getNameForId($id)
+    {
+        return self::$idToName[$this->type][$id];
+    }
+
+    protected function fetchObjectsById($ids)
+    {
+        $class = $this->object;
+        $connection = $this->connection;
         $res = array();
 
-        foreach ($this->listParentIds($id) as $parentId) {
+        foreach ($ids as $id) {
-            foreach ($this->resolveParentIds($parentId) as $gpId) {
+            $res[] = $class::loadWithAutoIncId($id, $connection);
-                $res[] = $gpId;
-            }
-            $res[] = $parentId;
         }
 
         return $res;
     }
 
-    public function resolveParentNames($name)
-    {
-        $res = array();
-        foreach ($this->listParentNames($name) as $parentName) {
-            foreach ($this->resolveParentNames($parentName) as $gpName) {
-                $res[] = $gpName;
-            }
-            $res[] = $parentName;
-        }
-
-        return $res;
-    }
-
-    /*
-    public function listChildren()
-    {
-    }
-
-    public function listChildrenIds
-    */
-
     protected function requireTemplates()
     {
         if (! array_key_exists($this->type, self::$templates)) {
@@ -168,15 +215,15 @@ class IcingaTemplateResolver
     {
         $type = $this->type;
 
-        $templates = static::fetchTemplates(
+        $templates = $this->fetchTemplates();
-            $this->db,
-            $type
-        );
 
         $ids = array();
         $names = array();
+        $idToName = array();
 
         foreach ($templates as $row) {
+            $idToName[$row->id] = $row->name;
+
             if ($row->parent_id === null) {
                 continue;
             }
@@ -197,6 +244,7 @@ class IcingaTemplateResolver
         self::$idIdx[$type]     = $ids;
         self::$nameIdx[$type]   = $names;
         self::$templates[$type] = $templates;
+        self::$idToName[$type]  = $idToName;
     }
 
     protected function fetchTemplates()
@@ -221,9 +269,7 @@ class IcingaTemplateResolver
             array('p' => $table),
             'p.id = i.parent_' . $type . '_id',
             array()
-        )//->where("o.object_type = 'template'")
+        )->order('o.id')->order('i.weight');
-         ->order('o.id')
-         ->order('i.weight');
 
         return $db->fetchAll($query);
     }

library/Director/Web/Controller/ObjectController.php

@@ -6,6 +6,7 @@ use Exception;
 use Icinga\Exception\IcingaException;
 use Icinga\Exception\InvalidPropertyException;
 use Icinga\Exception\NotFoundError;
+use Icinga\Module\Director\Exception\NestingError;
 use Icinga\Module\Director\IcingaConfig\IcingaConfig;
 use Icinga\Module\Director\Objects\IcingaObject;
 use Icinga\Web\Url;
@@ -117,7 +118,7 @@ abstract class ObjectController extends ActionController
                 array('class' => 'icon-resize-small state-warning')
             );
         } else {
-
+            try {
                 if ($object->supportsImports() && $object->imports()->count() > 0) {
                     $this->view->actionLinks = $this->view->qlink(
                         $this->translate('Show resolved'),
@@ -126,15 +127,13 @@ abstract class ObjectController extends ActionController
                         array('class' => 'icon-resize-full')
                     );
                 }
+            } catch (NestingError $e) {
+                // No resolve link with nesting errors
             }
-
-        if ($this->view->isExternal) {
-            $object->object_type = 'object';
         }
 
         $this->view->object = $object;
-        $this->view->config = new IcingaConfig($this->db());
+        $this->view->config = $object->toSingleIcingaConfig();
-        $object->renderToConfig($this->view->config);
 
         $this->view->title = sprintf(
             $this->translate('Config preview: %s'),

library/Director/Web/Form/DirectorObjectForm.php

@@ -3,6 +3,7 @@
 namespace Icinga\Module\Director\Web\Form;
 
 use Exception;
+use Icinga\Module\Director\Exception\NestingError;
 use Icinga\Module\Director\IcingaConfig\StateFilterSet;
 use Icinga\Module\Director\IcingaConfig\TypeFilterSet;
 use Icinga\Module\Director\Objects\IcingaObject;
@@ -235,12 +236,18 @@ abstract class DirectorObjectForm extends QuickForm
         }
 
         if ($object instanceof IcingaObject) {
+            try {
                 $props = (array) $object->toPlainObject(
                     false,
                     false,
                     null,
                     false // Do not resolve IDs
                 );
+            } catch (NestingError $e) {
+                $this->addUniqueError($e->getMessage());
+                $props = $object->getProperties();
+            }
+
         } else {
             $props = $object->getProperties();
             unset($props['vars']);
@@ -258,12 +265,16 @@ abstract class DirectorObjectForm extends QuickForm
             return $this;
         }
 
-        if ($object->supportsImports()) {
-            $inherited = $object->getInheritedProperties();
-            $origins   = $object->getOriginsProperties();
-        } else {
         $inherited = (object) array();
         $origins   = (object) array();
+
+        if ($object->supportsImports()) {
+            try {
+                $inherited = $object->getInheritedProperties();
+                $origins   = $object->getOriginsProperties();
+            } catch (NestingError $e) {
+                $this->addUniqueError($e->getMessage());
+            }
         }
 
         foreach ($props as $k => $v) {
@@ -284,9 +295,16 @@ abstract class DirectorObjectForm extends QuickForm
             return;
         }
 
+        try {
             $fields   = $object->getResolvedFields();
             $inherits = $object->getInheritedVars();
             $origins  = $object->getOriginsVars();
+        } catch (NestingError $e) {
+            $fields   = $object->getFields();
+            $inherits = (object) array();
+            $origins  = (object) array();
+        }
+        try {
             if ($object->hasCheckCommand()) {
                 $checkCommand = $object->getCheckCommand();
                 $checkFields = $checkCommand->getResolvedFields();
@@ -294,6 +312,9 @@ abstract class DirectorObjectForm extends QuickForm
             } else {
                 $checkFields = (object) array();
             }
+        } catch (NestingError $e) {
+            $checkFields  = (object) array();
+        }
 
         if ($this->hasBeenSent()) {
             $vars = array();
@@ -944,7 +965,12 @@ abstract class DirectorObjectForm extends QuickForm
         if ($object->hasProperty($name)) {
             if ($resolved && $object->supportsImports()) {
                 $this->assertResolvedImports();
+                try {
                     $objectProperty = $object->getResolvedProperty($name);
+                } catch (NestingError $e) {
+                    $this->addUniqueError($e->getMessage());
+                    $objectProperty = $object->$name;
+                }
             } else {
                 $objectProperty = $object->$name;
             }
@@ -963,6 +989,15 @@ abstract class DirectorObjectForm extends QuickForm
         return $default;
     }
 
+    protected function addUniqueError($msg)
+    {
+        if (! in_array($msg, $this->getErrorMessages())) {
+            $this->addError($msg);
+        }
+
+        return $this;
+    }
+
     public function loadObject($id)
     {
         $class = $this->getObjectClassname();