Import/Sync: cleanly rollback transactions

Helps to avoid side-effects

library/Director/Import/Import.php

@@ -2,6 +2,7 @@
 
 namespace Icinga\Module\Director\Import;
 
+use Exception;
 use Icinga\Exception\IcingaException;
 use Icinga\Module\Director\Hook\ImportSourceHook;
 use Icinga\Module\Director\Objects\ImportSource;
@@ -322,43 +323,49 @@ class Import
 
         $db->beginTransaction();
 
-        if ($this->isEmpty()) {
+        try {
-            $newRows = array();
-            $newProperties = array();
-        } else {
-            $newRows = $this->newChecksums('imported_row', $this->rowChecksums);
-            $newProperties = $this->newChecksums('imported_property', array_keys($this->properties));
-        }
 
-        $db->insert('imported_rowset', array('checksum' => $this->quoteBinary($rowset)));
+            if ($this->isEmpty()) {
-
+                $newRows = array();
-        foreach ($newProperties as $checksum) {
+                $newProperties = array();
-            $db->insert('imported_property', $this->properties[$checksum]);
+            } else {
-        }
+                $newRows = $this->newChecksums('imported_row', $this->rowChecksums);
-
+                $newProperties = $this->newChecksums('imported_property', array_keys($this->properties));
-        foreach ($newRows as $row) {
-            $db->insert('imported_row', $rows[$row]);
-            foreach ($this->rowProperties[$row] as $property) {
-                $db->insert('imported_row_property', array(
-                    'row_checksum'      => $this->quoteBinary($row),
-                    'property_checksum' => $property
-                ));
             }
+
+            $db->insert('imported_rowset', array('checksum' => $this->quoteBinary($rowset)));
+
+            foreach ($newProperties as $checksum) {
+                $db->insert('imported_property', $this->properties[$checksum]);
+            }
+
+            foreach ($newRows as $row) {
+                $db->insert('imported_row', $rows[$row]);
+                foreach ($this->rowProperties[$row] as $property) {
+                    $db->insert('imported_row_property', array(
+                        'row_checksum'      => $this->quoteBinary($row),
+                        'property_checksum' => $property
+                    ));
+                }
+            }
+
+            foreach (array_keys($rows) as $row) {
+                $db->insert(
+                    'imported_rowset_row',
+                    array(
+                        'rowset_checksum' => $this->quoteBinary($rowset),
+                        'row_checksum'    => $this->quoteBinary($row)
+                    )
+                );
+            }
+
+            $db->commit();
+
+            $this->rowsetExists = true;
+        } catch (Exception $e) {
+            $db->rollBack();
+            throw $e;
         }
-
-        foreach (array_keys($rows) as $row) {
-            $db->insert(
-                'imported_rowset_row',
-                array(
-                    'rowset_checksum' => $this->quoteBinary($rowset),
-                    'row_checksum'    => $this->quoteBinary($row)
-                )
-            );
-        }
-
-        $db->commit();
-
-        $this->rowsetExists = true;
     }
 
     /**

library/Director/Import/Sync.php

@@ -2,6 +2,7 @@
 
 namespace Icinga\Module\Director\Import;
 
+use Exception;
 use Icinga\Data\Filter\Filter;
 use Icinga\Module\Director\Objects\IcingaObject;
 use Icinga\Module\Director\Objects\ImportSource;
@@ -685,63 +686,70 @@ class Sync
         $db = $this->db;
         $dba = $db->getDbAdapter();
         $dba->beginTransaction();
-        $formerActivityChecksum = Util::hex2binary(
-            $db->getLastActivityChecksum()
-        );
-        $created = 0;
-        $modified = 0;
-        $deleted = 0;
-        foreach ($objects as $object) {
-            if ($object instanceof IcingaObject && $object->isTemplate()) {
-                // TODO: allow to sync templates
-                if ($object->hasBeenModified()) {
-                    throw new IcingaException(
-                        'Sync is not allowed to modify template "%s"',
-                        $object->$objectKey
-                    );
-                }
-                continue;
-            }
 
-            if ($object instanceof IcingaObject && $object->shouldBeRemoved()) {
+        try {
-                $object->delete($db);
+            $formerActivityChecksum = Util::hex2binary(
-                $deleted++;
-                continue;
-            }
-
-            if ($object->hasBeenModified()) {
-                if ($object->hasBeenLoadedFromDb()) {
-                    $modified++;
-                } else {
-                    $created++;
-                }
-                $object->store($db);
-            }
-        }
-
-        $runProperties = array(
-            'objects_created'  => $created,
-            'objects_deleted'  => $deleted,
-            'objects_modified' => $modified,
-        );
-
-        if ($created + $deleted + $modified > 0) {
-            // TODO: What if this has been the very first activity?
-            $runProperties['last_former_activity'] = $db->quoteBinary($formerActivityChecksum);
-            $runProperties['last_related_activity'] = $db->quoteBinary(Util::hex2binary(
                 $db->getLastActivityChecksum()
-            ));
+            );
+            $created = 0;
+            $modified = 0;
+            $deleted = 0;
+            foreach ($objects as $object) {
+                if ($object instanceof IcingaObject && $object->isTemplate()) {
+                    // TODO: allow to sync templates
+                    if ($object->hasBeenModified()) {
+                        throw new IcingaException(
+                            'Sync is not allowed to modify template "%s"',
+                            $object->$objectKey
+                        );
+                    }
+                    continue;
+                }
+
+                if ($object instanceof IcingaObject && $object->shouldBeRemoved()) {
+                    $object->delete($db);
+                    $deleted++;
+                    continue;
+                }
+
+                if ($object->hasBeenModified()) {
+                    if ($object->hasBeenLoadedFromDb()) {
+                        $modified++;
+                    } else {
+                        $created++;
+                    }
+                    $object->store($db);
+                }
+            }
+
+            $runProperties = array(
+                'objects_created'  => $created,
+                'objects_deleted'  => $deleted,
+                'objects_modified' => $modified,
+            );
+
+            if ($created + $deleted + $modified > 0) {
+                // TODO: What if this has been the very first activity?
+                $runProperties['last_former_activity'] = $db->quoteBinary($formerActivityChecksum);
+                $runProperties['last_related_activity'] = $db->quoteBinary(Util::hex2binary(
+                    $db->getLastActivityChecksum()
+                ));
+            }
+
+            $this->run->setProperties($runProperties)->store();
+
+            $dba->commit();
+
+            // Store duration after commit, as the commit might take some time
+            $this->run->set('duration_ms', (int) round(
+                (microtime(true) - $this->runStartTime) * 1000
+            ))->store();
+
+        } catch (Exception $e) {
+            $dba->rollBack();
+            throw $e;
         }
 
-        $this->run->setProperties($runProperties)->store();
-
-        $dba->commit();
-
-        // Store duration after commit, as the commit might take some time
-        $this->run->set('duration_ms', (int) round(
-            (microtime(true) - $this->runStartTime) * 1000
-        ))->store();
-
         return $this->run->id;
     }
 }