DirectorObjectForm: add unique error message...

...without adding an error in case we detect a loop. Also improve
resolve cache invalidation and clean up old imports-related code

refs #11803

library/Director/Objects/IcingaObject.php

@@ -117,6 +117,8 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
 
     private $cachedPlainUnmodified;
 
+    private $templateResolver;
+
     public function propertyIsBoolean($property)
     {
         return array_key_exists($property, $this->booleans);
@@ -774,8 +776,11 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
 
     public function templateResolver()
     {
-        // preserve object
+        if ($this->templateResolver === null) {
-        return new IcingaTemplateResolver($this);
+            $this->templateResolver = new IcingaTemplateResolver($this);
+        }
+
+        return $this->templateResolver;
     }
 
     public function getResolvedProperty($key, $default = null)
@@ -917,6 +922,10 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
     public function invalidateResolveCache()
     {
         $this->resolveCache = array();
+        if ($this->templateResolver) {
+            $this->templateResolver()->clearCache();
+        }
+
         return $this;
     }
 

library/Director/Objects/IcingaObjectImports.php

@@ -41,6 +41,12 @@ class IcingaObjectImports implements Iterator, Countable, IcingaConfigRenderer
         $this->position = 0;
     }
 
+    public function setModified()
+    {
+        $this->modified = true;
+        return $this;
+    }
+
     public function hasBeenModified()
     {
         return $this->modified;
@@ -83,7 +89,7 @@ class IcingaObjectImports implements Iterator, Countable, IcingaConfigRenderer
 
     public function set($import)
     {
-        if ($import === null) {
+        if (empty($import)) {
             if (empty($this->imports)) {
                 return $this;
             } else {
@@ -95,31 +101,33 @@ class IcingaObjectImports implements Iterator, Countable, IcingaConfigRenderer
             $import = array($import);
         }
 
-        $existing = array_keys($this->imports);
+        $existing = $this->listImportNames();
-        $new = array();
+        $new = $this->listNamesForGivenImports($import);
-        $class = $this->getImportClass();
-        foreach ($import as $i) {
-
-            if ($i instanceof $class) {
-                $this->objects[$i->object_name] = $i;
-                $new[] = $i->object_name;
-            } else {
-                $new[] = $i;
-            }
-        }
 
         if ($existing === $new) {
             return $this;
         }
 
-        if (count($new) === 0) {
-            return $this->clear();
-        }
-
         $this->imports = array();
         return $this->add($import);
     }
 
+    protected function listNamesForGivenImports($imports)
+    {
+        $list = array();
+        $class = $this->getImportClass();
+        foreach ($imports as $i) {
+
+            if ($i instanceof $class) {
+                $list[] = $i->object_name;
+            } else {
+                $list[] = $i;
+            }
+        }
+
+        return $list;
+    }
+
     /**
      * Magic isset check
      *
@@ -137,11 +145,9 @@ class IcingaObjectImports implements Iterator, Countable, IcingaConfigRenderer
         }
 
         $this->imports = array();
-
         $this->modified = true;
-        $this->refreshIndex();
 
-        return $this;
+        return $this->refreshIndex();
     }
 
     public function remove($import)
@@ -151,21 +157,20 @@ class IcingaObjectImports implements Iterator, Countable, IcingaConfigRenderer
         }
 
         $this->modified = true;
-        $this->refreshIndex();
 
-        return $this;
+        return $this->refreshIndex();
     }
 
     protected function refreshIndex()
     {
         $this->idx = array_keys($this->imports);
+        return $this;
     }
 
     public function add($import)
     {
         $class = $this->getImportClass();
 
-        // TODO: only one query when adding array
         if (is_array($import)) {
             foreach ($import as $i) {
                 // Gracefully ignore null members or empty strings
@@ -179,12 +184,13 @@ class IcingaObjectImports implements Iterator, Countable, IcingaConfigRenderer
         }
 
         if ($import instanceof $class) {
-            if (array_key_exists($import->object_name, $this->imports)) {
+            $name = $import->object_name;
+            if (array_key_exists($name, $this->imports)) {
                 return $this;
             }
 
-            $this->imports[$import->object_name] = $import->object_name;
+            $this->imports[$name] = $name;
-            $this->objects[$import->object_name] = $import;
+            $this->objects[$name] = $import;
         } elseif (is_string($import)) {
             if (array_key_exists($import, $this->imports)) {
                 return $this;
@@ -219,7 +225,13 @@ class IcingaObjectImports implements Iterator, Countable, IcingaConfigRenderer
         $class = $this->getImportClass();
         if (is_array($this->object->getKeyName())) {
             // Services only
-            $import = $class::load(array('object_name' => $name, 'object_type' => 'template'), $connection);
+            $import = $class::load(
+                array(
+                    'object_name' => $name,
+                    'object_type' => 'template'
+                ),
+                $connection
+            );
         } else {
             $import = $class::load($name, $connection);
         }
@@ -250,37 +262,17 @@ class IcingaObjectImports implements Iterator, Countable, IcingaConfigRenderer
     // TODO: prefetch
     protected function loadFromDb()
     {
-        $db = $this->object->getDb();
+        $resolver = $this->object->templateResolver();
-        $connection = $this->object->getConnection();
+        // Force nesting error
+        $resolver->listResolvedParentIds();
 
-        $type = $this->getType();
+        $this->objects = $resolver->fetchParents();
-
+        $this->imports = array();
-        $table = $this->object->getTableName();
-        $query = $db->select()->from(
-            array('o' => $table),
-            array()
-        )->join(
-            array('oi' => $table . '_inheritance'),
-            'oi.' . $type . '_id = o.id',
-            array()
-        )->join(
-            array('i' => $table),
-            'i.id = oi.parent_' . $type . '_id',
-            '*'
-        )->where('o.id = ?', (int) $this->object->id)
-        ->order('oi.weight');
-
-        $class = $this->getImportClass();
-        $this->objects = $class::loadAll($connection, $query, 'object_name');
         foreach ($this->objects as $k => $obj) {
             $this->imports[$k] = $k;
         }
 
-        $this->storedImports = array();
+        $this->cloneStored();
-        foreach ($this->objects as $k => $v) {
-            $this->storedImports[$k] = clone($v);
-        }
-
         return $this;
     }
 
@@ -297,19 +289,21 @@ class IcingaObjectImports implements Iterator, Countable, IcingaConfigRenderer
 
     public function store()
     {
-        $objectId = $this->object->id;
+        if (! $this->hasBeenModified()) {
+            return true;
+        }
+
+        $objectId = (int) $this->object->id;
         $type = $this->getType();
 
         $objectCol = $type . '_id';
         $importCol = 'parent_' . $type . '_id';
 
-        if (! $this->hasBeenModified()) {
-            return true;
-        }
-
         if ($this->object->hasBeenLoadedFromDb()) {
-
+            $this->object->db->delete(
-            $this->object->db->delete($this->getImportTableName(), $objectCol . ' = ' . $objectId);
+                $this->getImportTableName(),
+                $objectCol . ' = ' . $objectId
+            );
         }
 
         $weight = 1;
@@ -358,7 +352,7 @@ class IcingaObjectImports implements Iterator, Countable, IcingaConfigRenderer
     {
         $ret = '';
 
-        foreach ($this->imports as $name => $o) {
+        foreach ($this->listImportNames() as $name) {
             $ret .= '    import ' . c::renderString($name) . "\n";
         }
 
@@ -372,8 +366,8 @@ class IcingaObjectImports implements Iterator, Countable, IcingaConfigRenderer
     {
         $ret = '';
 
-        foreach ($this->imports as $name => $o) {
+        foreach ($this->listImportNames() as $name) {
-            $ret .= c1::renderKeyValue('use', c1::renderString($name));
+            $ret .= c1::renderKeyValue('use', c1::renderString($name)) . "\n";
         }
 
         if ($ret !== '') {

library/Director/Objects/IcingaTemplateResolver.php

@@ -49,8 +49,8 @@ class IcingaTemplateResolver
      */
     public function clearCache()
     {
-        $type = $object->getShortTableName();
+        unset(self::$templates[$this->type]);
-        unset(self::$templates[$type]);
+        return $this;
     }
 
     /**

library/Director/Web/Form/DirectorObjectForm.php

@@ -244,7 +244,7 @@ abstract class DirectorObjectForm extends QuickForm
                     false // Do not resolve IDs
                 );
             } catch (NestingError $e) {
-                $this->addUniqueError($e->getMessage());
+                $this->addUniqueErrorMessage($e->getMessage());
                 $props = $object->getProperties();
             }
 
@@ -273,7 +273,7 @@ abstract class DirectorObjectForm extends QuickForm
                 $inherited = $object->getInheritedProperties();
                 $origins   = $object->getOriginsProperties();
             } catch (NestingError $e) {
-                $this->addUniqueError($e->getMessage());
+                $this->addUniqueErrorMessage($e->getMessage());
             }
         }
 
@@ -968,7 +968,7 @@ abstract class DirectorObjectForm extends QuickForm
                 try {
                     $objectProperty = $object->getResolvedProperty($name);
                 } catch (NestingError $e) {
-                    $this->addUniqueError($e->getMessage());
+                    $this->addUniqueErrorMessage($e->getMessage());
                     $objectProperty = $object->$name;
                 }
             } else {
@@ -989,10 +989,10 @@ abstract class DirectorObjectForm extends QuickForm
         return $default;
     }
 
-    protected function addUniqueError($msg)
+    protected function addUniqueErrorMessage($msg)
     {
         if (! in_array($msg, $this->getErrorMessages())) {
-            $this->addError($msg);
+            $this->addErrorMessage($msg);
         }
 
         return $this;