Basket: fix various little issues

fixes #1691

application/controllers/BasketController.php

@@ -282,6 +282,14 @@ class BasketController extends ActionController
                     }
                     $currentExport = $current->export();
                     $fieldResolver->tweakTargetIds($currentExport);
+
+                    // Ignore originalId
+                    if (isset($currentExport->originalId)) {
+                        unset($currentExport->originalId);
+                    }
+                    if (isset($object->originalId)) {
+                        unset($object->originalId);
+                    }
                     $hasChanged = Json::encode($currentExport) !== Json::encode($object);
                     $table->addNameValueRow(
                         $key,

library/Director/DirectorObject/Automation/Basket.php

@@ -24,6 +24,8 @@ class Basket extends DbObject implements ExportInterface
 
     protected $chosenObjects = [];
 
+    protected $protectedFormerChosenObjects;
+
     protected $defaultProperties = [
         'uuid'        => null,
         'basket_name' => null,
@@ -118,6 +120,7 @@ class Basket extends DbObject implements ExportInterface
         if (empty($objects)) {
             $this->chosenObjects = [];
         } else {
+            $this->protectedFormerChosenObjects = $this->chosenObjects;
             $this->chosenObjects = [];
             foreach ((array) $objects as $type => $object) {
                 $this->addObjects($type, $object);
@@ -142,13 +145,17 @@ class Basket extends DbObject implements ExportInterface
         } elseif ($objects === null || $objects === 'IGNORE') {
             return;
         } elseif ($objects === '[]' || is_array($objects)) {
-            if (isset($this->chosenObjects[$type])) {
             if (! is_array($this->chosenObjects[$type])) {
                 $this->chosenObjects[$type] = [];
             }
+            if (isset($this->protectedFormerChosenObjects[$type])) {
+                if (is_array($this->protectedFormerChosenObjects[$type])) {
+                    $this->chosenObjects[$type] = $this->protectedFormerChosenObjects;
                 } else {
                     $this->chosenObjects[$type] = [];
                 }
+            }
+
             if ($objects === '[]') {
                 $objects = [];
             }

library/Director/DirectorObject/Automation/BasketSnapshot.php

@@ -54,7 +54,6 @@ class BasketSnapshot extends DbObject
         'ServiceSet',
         'Notification',
         'Dependency',
-        'DataList',
         'ImportSource',
         'SyncRule',
         'DirectorJob',
@@ -213,10 +212,36 @@ class BasketSnapshot extends DbObject
         $db = $connection->getDbAdapter();
         $db->beginTransaction();
         $fieldResolver = new BasketSnapshotFieldResolver($all, $connection);
+        $this->restoreType($all, 'DataList', $fieldResolver, $connection, $replace);
         $fieldResolver->storeNewFields();
         foreach ($this->restoreOrder as $typeName) {
+            $this->restoreType($all, $typeName, $fieldResolver, $connection, $replace);
+        }
+        $db->commit();
+    }
+
+    /**
+     * @param $all
+     * @param $typeName
+     * @param BasketSnapshotFieldResolver $fieldResolver
+     * @param Db $connection
+     * @param $replace
+     * @throws \Icinga\Exception\NotFoundError
+     * @throws \Icinga\Module\Director\Exception\DuplicateKeyException
+     * @throws \Zend_Db_Adapter_Exception
+     */
+    protected function restoreType(
+        & $all,
+        $typeName,
+        BasketSnapshotFieldResolver $fieldResolver,
+        Db $connection,
+        $replace
+    ) {
         if (isset($all->$typeName)) {
             $objects = (array) $all->$typeName;
+        } else {
+            return;
+        }
         $class = static::getClassForType($typeName);
 
         $changed = [];
@@ -250,15 +275,12 @@ class BasketSnapshot extends DbObject
         }
         foreach ($changed as $key => $new) {
             // Store related fields. As objects might have formerly been
-                    // unstored, let's to it right here
+            // un-stored, let's to it right here
             if ($new instanceof IcingaObject) {
                 $fieldResolver->relinkObjectFields($new, $objects[$key]);
             }
         }
     }
-        }
-        $db->commit();
-    }
 
     /**
      * @param IcingaObject $object

library/Director/Objects/DirectorDatafield.php

@@ -93,12 +93,13 @@ class DirectorDatafield extends DbObjectWithSettings
         }
 
         if (isset($properties['settings']->datalist)) {
+            // Just try to load the list, import should fail if missing
             $list = DirectorDatalist::load(
                 $properties['settings']->datalist,
                 $db
             );
-            $properties['settings']->datalist_id = $list->get('id');
+        } else {
-            unset($properties['settings']->datalist);
+            $list = null;
         }
 
         $encoded = Json::encode($properties);
@@ -113,6 +114,11 @@ class DirectorDatafield extends DbObjectWithSettings
             }
         }
 
+        if ($list) {
+            unset($properties['settings']->datalist);
+            $properties['settings']->datalist_id = $list->get('id');
+        }
+
         $dba = $db->getDbAdapter();
         $query = $dba->select()
             ->from('director_datafield')

library/Director/Objects/DirectorDatalist.php

@@ -21,6 +21,9 @@ class DirectorDatalist extends DbObject implements ExportInterface
         'owner'         => null
     );
 
+    /** @var DirectorDatalistEntry[] */
+    protected $storedEntries;
+
     public function getUniqueIdentifier()
     {
         return $this->get('list_name');
@@ -38,7 +41,6 @@ class DirectorDatalist extends DbObject implements ExportInterface
     {
         $properties = (array) $plain;
         if (isset($properties['originalId'])) {
-            $id = $properties['originalId'];
             unset($properties['originalId']);
         } else {
             $id = null;
@@ -56,13 +58,83 @@ class DirectorDatalist extends DbObject implements ExportInterface
             $object = static::create([], $db);
         }
         $object->setProperties($properties);
-        if ($id !== null) {
-            $object->reallySet('id', $id);
-        }
 
         return $object;
     }
 
+    public function setEntries($entries)
+    {
+        $existing = $this->getStoredEntries();
+
+        $new = [];
+        $seen = [];
+        $modified = false;
+
+        foreach ($entries as $entry) {
+            $name = $entry->entry_name;
+            $entry = DirectorDatalistEntry::create((array) $entry);
+            $seen[$name] = true;
+            if (isset($existing[$name])) {
+                $existing[$name]->replaceWith($entry);
+                if (! $modified && $existing[$name]->hasBeenModified()) {
+                    $modified = true;
+                }
+            } else {
+                $modified = true;
+                $new[] = $entry;
+            }
+        }
+
+        foreach (array_keys($existing) as $key) {
+            if (! isset($seen[$key])) {
+                $existing[$key]->markForRemoval();
+                $modified = true;
+            }
+        }
+
+        foreach ($new as $entry) {
+            $existing[$entry->get('entry_name')] = $entry;
+        }
+
+        if ($modified) {
+            $this->hasBeenModified = true;
+        }
+
+        $this->storedEntries = $existing;
+        ksort($this->storedEntries);
+
+        return $this;
+    }
+
+    /**
+     * @throws DuplicateKeyException
+     */
+    public function onStore()
+    {
+        if ($this->storedEntries) {
+            $db = $this->getConnection();
+            $removedKeys = [];
+            $myId = $this->get('id');
+
+            foreach ($this->storedEntries as $key => $entry) {
+                if ($entry->shouldBeRemoved()) {
+                    $entry->delete();
+                    $removedKeys[] = $key;
+                } else {
+                    if (! $entry->hasBeenLoadedFromDb()) {
+                        $entry->set('list_id', $myId);
+                    }
+                    $entry->set('list_id', $myId);
+                    $entry->store($db);
+                }
+            }
+
+            foreach ($removedKeys as $key) {
+                unset($this->storedEntries[$key]);
+            }
+        }
+    }
+
     public function export()
     {
         $plain = (object) $this->getProperties();
@@ -70,8 +142,10 @@ class DirectorDatalist extends DbObject implements ExportInterface
         unset($plain->id);
 
         $plain->entries = [];
-        $entries = DirectorDatalistEntry::loadAllForList($this);
+        foreach ($this->getStoredEntries() as $key => $entry) {
-        foreach ($entries as $key => $entry) {
+            if ($entry->shouldBeRemoved()) {
+                continue;
+            }
             $plainEntry = (object) $entry->getProperties();
             unset($plainEntry->list_id);
 
@@ -80,4 +154,18 @@ class DirectorDatalist extends DbObject implements ExportInterface
 
         return $plain;
     }
+
+    protected function getStoredEntries()
+    {
+        if ($this->storedEntries === null) {
+            if ($id = $this->get('id')) {
+                $this->storedEntries = DirectorDatalistEntry::loadAllForList($this);
+                ksort($this->storedEntries);
+            } else {
+                $this->storedEntries = [];
+            }
+        }
+
+        return $this->storedEntries;
+    }
 }