0013330: Tinebase Records / TimeMachine - make all changes revertable
authorPaul Mehrer <p.mehrer@metaways.de>
Tue, 4 Jul 2017 08:14:28 +0000 (10:14 +0200)
committerPhilipp Schüle <p.schuele@metaways.de>
Mon, 10 Jul 2017 12:53:15 +0000 (14:53 +0200)
* implemented for
* * Addressbook_Model_Contact
* * Calendar_Model_Event
* * Tasks_Model_Task

0013330: Tinebase Records / TimeMachine - make all changes revertable

Change-Id: I8b3e3575b9052a6f6d364ecec789ab7a6384b0e6
Reviewed-on: http://gerrit.tine20.com/customers/5013
Reviewed-by: Philipp Schüle <p.schuele@metaways.de>
Tested-by: Philipp Schüle <p.schuele@metaways.de>
tests/tine20/Addressbook/ControllerTest.php
tine20/Tinebase/Controller/Record/Abstract.php
tine20/Tinebase/FileSystem.php
tine20/Tinebase/Model/ModificationLogFilter.php
tine20/Tinebase/Model/Tree/FileObject.php
tine20/Tinebase/Notes.php
tine20/Tinebase/Record/Abstract.php
tine20/Tinebase/Relation/Backend/Sql.php
tine20/Tinebase/Relations.php
tine20/Tinebase/Timemachine/ModificationLog.php

index 0227e8d..02cd7ff 100644 (file)
@@ -40,6 +40,8 @@ class Addressbook_ControllerTest extends TestCase
         parent::setUp();
         $this->_instance = Addressbook_Controller_Contact::getInstance();
 
+        $this->_oldFileSystemConfig = clone Tinebase_Config::getInstance()->{Tinebase_Config::FILESYSTEM};
+
         $personalContainer = Tinebase_Container::getInstance()->getPersonalContainer(
             Zend_Registry::get('currentAccount'), 
             'Addressbook', 
@@ -154,6 +156,8 @@ class Addressbook_ControllerTest extends TestCase
             $this->_instance->delete($this->objects['contact']);
         }
 
+        Tinebase_Config::getInstance()->{Tinebase_Config::FILESYSTEM} = $this->_oldFileSystemConfig;
+
         parent::tearDown();
     }
     
@@ -165,7 +169,7 @@ class Addressbook_ControllerTest extends TestCase
     protected function _addContact()
     {
         $contact = $this->objects['initialContact'];
-        $contact->notes = new Tinebase_Record_RecordSet('Tinebase_Model_Note', array($this->objects['note']));
+        $contact->notes = array($this->objects['note']);
         $contact = $this->_instance->create($contact);
         $this->objects['contact'] = $contact;
         
@@ -501,4 +505,140 @@ class Addressbook_ControllerTest extends TestCase
 
         $this->assertTrue(Addressbook_Config::getInstance()->featureEnabled(Addressbook_Config::FEATURE_LIST_VIEW));
     }
+
+    public function testModLogUndo()
+    {
+        // move this code to \Tinebase_Timemachine_ModificationLogTest::testFileManagerReplication etc.
+        //$instance_seq = Tinebase_Timemachine_ModificationLog::getInstance()->getMaxInstanceSeq();
+
+        // activate ModLog in FileSystem!
+        Tinebase_Config::getInstance()->{Tinebase_Config::FILESYSTEM}
+            ->{Tinebase_Config::FILESYSTEM_MODLOGACTIVE} = true;
+        $filesystem = Tinebase_FileSystem::getInstance();
+        $filesystem->resetBackends();
+        Tinebase_Core::clearAppInstanceCache();
+
+        $cField1 = Tinebase_CustomField::getInstance()->addCustomField(new Tinebase_Model_CustomField_Config(array(
+            'application_id'    => Tinebase_Application::getInstance()->getApplicationByName('Addressbook')->getId(),
+            'name'              => Tinebase_Record_Abstract::generateUID(),
+            'model'             => 'Addressbook_Model_Contact',
+            'definition'        => array(
+                'label' => Tinebase_Record_Abstract::generateUID(),
+                'type'  => 'string',
+                'uiconfig' => array(
+                    'xtype'  => Tinebase_Record_Abstract::generateUID(),
+                    'length' => 10,
+                    'group'  => 'unittest',
+                    'order'  => 100,
+                )
+            )
+        )));
+        $cField2 = Tinebase_CustomField::getInstance()->addCustomField(new Tinebase_Model_CustomField_Config(array(
+            'application_id'    => Tinebase_Application::getInstance()->getApplicationByName('Addressbook')->getId(),
+            'name'              => Tinebase_Record_Abstract::generateUID(),
+            'model'             => 'Addressbook_Model_Contact',
+            'definition'        => array(
+                'label' => Tinebase_Record_Abstract::generateUID(),
+                'type'  => 'string',
+                'uiconfig' => array(
+                    'xtype'  => Tinebase_Record_Abstract::generateUID(),
+                    'length' => 10,
+                    'group'  => 'unittest',
+                    'order'  => 100,
+                )
+            )
+        )));
+        $user = Tinebase_Core::getUser();
+        /** @var Addressbook_Model_Contact $contact */
+        $contact = $this->objects['initialContact'];
+
+        // create contact with notes, relations, tags, attachments, customfield
+        $contact->notes = array($this->objects['note']);
+        $contact->relations = array(array(
+            'related_id'        => $user->contact_id,
+            'related_model'     => 'Addressbook_Model_Contact',
+            'related_degree'    => Tinebase_Model_Relation::DEGREE_SIBLING,
+            'related_backend'   => Tinebase_Model_Relation::DEFAULT_RECORD_BACKEND,
+            'type'              => 'foo'
+        ));
+        $contact->tags = array(array('name' => 'testtag1'));
+        $path = Tinebase_TempFile::getTempPath();
+        file_put_contents($path, 'testAttachementData');
+        $contact->attachments = new Tinebase_Record_RecordSet('Tinebase_Model_Tree_Node', array(
+            array(
+                'name'      => 'testAttachementData.txt',
+                'tempFile'  => Tinebase_TempFile::getInstance()->createTempFile($path)
+            )
+        ), true);
+        $contact->customfields = array(
+            $cField1->name => 'test field1'
+        );
+
+        $createdContact = $this->_instance->create($contact);
+
+        // update contact, add more notes, relations, tags, attachements, customfields
+        /** @var Addressbook_Model_Contact $updateContact */
+        $updateContact = $this->objects['updatedContact'];
+        $updateContact->setId($createdContact->getId());
+        $notes = $createdContact->notes->toArray();
+        $notes[] = array(
+            'note_type_id'      => 1,
+            'note'              => 'phpunit test note 2',
+        );
+        $updateContact->notes = $notes;
+        $relations = $createdContact->relations->toArray();
+        $relations[] = array(
+            'related_id'        => $user->contact_id,
+            'related_model'     => 'Addressbook_Model_Contact',
+            'related_degree'    => Tinebase_Model_Relation::DEGREE_CHILD,
+            'related_backend'   => Tinebase_Model_Relation::DEFAULT_RECORD_BACKEND,
+            'type'              => 'bar'
+        );
+        $updateContact->relations = $relations;
+        $updateContact->tags = clone $createdContact->tags;
+        $updateContact->tags->addRecord(new Tinebase_Model_Tag(array('name' => 'testtag2'), true));
+        $updateContact->attachments = clone $createdContact->attachments;
+        $path = Tinebase_TempFile::getTempPath();
+        file_put_contents($path, 'moreTestAttachementData');
+        $updateContact->attachments->addRecord(new Tinebase_Model_Tree_Node(array(
+                'name'      => 'moreTestAttachementData.txt',
+                'tempFile'  => Tinebase_TempFile::getInstance()->createTempFile($path)
+        ), true));
+        $updateContact->xprops('customfields')[$cField2->name] = 'test field2';
+
+        $contact = $this->_instance->update($updateContact);
+
+        // delete it
+        $this->_instance->delete($contact->getId());
+
+        $contact->seq = 0;
+        $modifications = Tinebase_Timemachine_ModificationLog::getInstance()->getModificationsBySeq(
+            Tinebase_Application::getInstance()->getApplicationById('Addressbook')->getId(), $contact, 10000);
+
+        // undelete it
+        $mod = $modifications->getLastRecord();
+        $modifications->removeRecord($mod);
+        Tinebase_Timemachine_ModificationLog::getInstance()->undo(new Tinebase_Model_ModificationLogFilter(array(
+            array('field' => 'id', 'operator' => 'in', 'value' => array($mod->getId()))
+        )));
+        $undeletedContact = $this->_instance->get($contact->getId());
+        static::assertEquals(2, $undeletedContact->notes->count());
+        static::assertEquals(2, $undeletedContact->relations->count());
+        static::assertEquals(2, $undeletedContact->tags->count());
+        static::assertEquals(2, $undeletedContact->attachments->count());
+        static::assertEquals(2, count($undeletedContact->customfields));
+
+        // undo update
+        $mod = $modifications->getLastRecord();
+        $modifications->removeRecord($mod);
+        Tinebase_Timemachine_ModificationLog::getInstance()->undo(new Tinebase_Model_ModificationLogFilter(array(
+            array('field' => 'id', 'operator' => 'in', 'value' => array($mod->getId()))
+        )));
+        $undidContact = $this->_instance->get($contact->getId());
+        static::assertEquals(1, $undidContact->notes->count());
+        static::assertEquals(1, $undidContact->relations->count());
+        static::assertEquals(1, $undidContact->tags->count());
+        static::assertEquals(1, $undidContact->attachments->count());
+        static::assertEquals(1, count($undidContact->customfields));
+    }
 }
index 4d8b8cb..7440212 100644 (file)
@@ -427,10 +427,6 @@ abstract class Tinebase_Controller_Record_Abstract
             // get related data only on request (defaults to TRUE)
             if ($_getRelatedData) {
                 $this->_getRelatedData($record);
-                
-                if ($record->has('notes')) {
-                    $record->notes = Tinebase_Notes::getInstance()->getNotesOfRecord($this->_modelName, $record->getId());
-                }
             }
         }
         
@@ -484,6 +480,9 @@ abstract class Tinebase_Controller_Record_Abstract
         if ($record->has('attachments') && Tinebase_Core::isFilesystemAvailable()) {
             Tinebase_FileSystem_RecordAttachments::getInstance()->getRecordAttachments($record);
         }
+        if ($record->has('notes')) {
+            $record->notes = Tinebase_Notes::getInstance()->getNotesOfRecord($this->_modelName, $record->getId());
+        }
     }
 
     /**
@@ -587,8 +586,8 @@ abstract class Tinebase_Controller_Record_Abstract
             $this->_inspectAfterCreate($createdRecord, $_record);
             $this->_setRelatedData($createdRecord, $_record, null, TRUE);
             $this->_writeModLog($createdRecord, null);
-            $this->_setNotes($createdRecord, $_record);
-            
+            $this->_setSystemNotes($createdRecord);
+
             if ($this->sendNotifications()) {
                 $this->doSendNotifications($createdRecord, Tinebase_Core::getUser(), 'created');
             }
@@ -992,9 +991,9 @@ abstract class Tinebase_Controller_Record_Abstract
             $updatedRecordWithRelatedData = $this->_setRelatedData($updatedRecord, $_record, $currentRecord, TRUE);
             if (Tinebase_Core::isLogLevel(Zend_Log::TRACE)) Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__
                 . ' Updated record with related data: ' . print_r($updatedRecordWithRelatedData->toArray(), TRUE));
-            
+
             $currentMods = $this->_writeModLog($updatedRecordWithRelatedData, $currentRecord);
-            $this->_setNotes($updatedRecordWithRelatedData, $_record, Tinebase_Model_Note::SYSTEM_NOTE_NAME_CHANGED, $currentMods);
+            $this->_setSystemNotes($updatedRecordWithRelatedData, Tinebase_Model_Note::SYSTEM_NOTE_NAME_CHANGED, $currentMods);
             
             if ($this->_sendNotifications && count($currentMods) > 0) {
                 $this->doSendNotifications($updatedRecordWithRelatedData, Tinebase_Core::getUser(), 'changed', $currentRecord);
@@ -1105,6 +1104,12 @@ abstract class Tinebase_Controller_Record_Abstract
             $updatedRecord->attachments = $record->attachments;
             Tinebase_FileSystem_RecordAttachments::getInstance()->setRecordAttachments($updatedRecord);
         }
+        if ($record->has('notes') && $this->_setNotes !== false) {
+            if (isset($record->notes) && is_array($record->notes)) {
+                $updatedRecord->notes = $record->notes;
+                Tinebase_Notes::getInstance()->setNotesOfRecord($updatedRecord);
+            }
+        }
         
         if ($returnUpdatedRelatedData) {
             $this->_getRelatedData($updatedRecord);
@@ -1118,25 +1123,20 @@ abstract class Tinebase_Controller_Record_Abstract
         return $updatedRecord;
     }
 
-
     /**
-     * set notes
-     * 
+     * set system notes
+     *
      * @param   Tinebase_Record_Interface $_updatedRecord   the just updated record
      * @param   Tinebase_Record_Interface $_record          the update record
      * @param   string $_systemNoteType
      * @param   Tinebase_Record_RecordSet $_currentMods
      */
-    protected function _setNotes($_updatedRecord, $_record, $_systemNoteType = Tinebase_Model_Note::SYSTEM_NOTE_NAME_CREATED, $_currentMods = NULL)
+    protected function _setSystemNotes($_updatedRecord, $_systemNoteType = Tinebase_Model_Note::SYSTEM_NOTE_NAME_CREATED, $_currentMods = NULL)
     {
-        if (! $_record->has('notes') || $this->_setNotes === false) {
+        if (! $_updatedRecord->has('notes') || $this->_setNotes === false) {
             return;
         }
 
-        if (isset($_record->notes) && is_array($_record->notes)) {
-            $_updatedRecord->notes = $_record->notes;
-            Tinebase_Notes::getInstance()->setNotesOfRecord($_updatedRecord);
-        }
         Tinebase_Notes::getInstance()->addSystemNote($_updatedRecord, Tinebase_Core::getUser(), $_systemNoteType, $_currentMods);
     }
     
@@ -1486,9 +1486,7 @@ abstract class Tinebase_Controller_Record_Abstract
             $this->_checkRight('delete');
             
             foreach ($records as $record) {
-                if ($this->sendNotifications()) {
-                    $this->_getRelatedData($record);
-                }
+                $this->_getRelatedData($record);
                 $this->_deleteRecord($record);
             }
 
@@ -1632,7 +1630,7 @@ abstract class Tinebase_Controller_Record_Abstract
 
         $originalRecord = clone $_record;
 
-        //$this->_unDeleteLinkedObjects($_record);
+        $this->_unDeleteLinkedObjects($_record);
 
         Tinebase_Timemachine_ModificationLog::setRecordMetaData($_record, 'undelete', $_record);
         $this->_backend->update($_record);
@@ -1657,22 +1655,22 @@ abstract class Tinebase_Controller_Record_Abstract
 
         $this->_deleteLinkedObjects($_record);
 
-        // we do this here, before the record MetaData will be set. As we need the unchanged record!
-        // TODO Maybe we even should do it before _deleteLinkedObjects above... though decision
-        $this->_writeModLog(null, $_record);
-
         if (! $this->_purgeRecords && $_record->has('created_by')) {
             Tinebase_Timemachine_ModificationLog::setRecordMetaData($_record, 'delete', $_record);
             $this->_backend->update($_record);
         } else {
             $this->_backend->delete($_record);
         }
+
+        // needs to be done after setRecordMetaData so the sequence increase is done, though this tampers with the
+        // meta data. But better that than having two modlog entries withe the same sequence...
+        $this->_writeModLog(null, $_record);
         
         $this->_increaseContainerContentSequence($_record, Tinebase_Model_ContainerContent::ACTION_DELETE);
     }
 
     /**
-     * delete linked objects (notes, relations, ...) of record
+     * delete linked objects (notes, relations, attachments) of record
      *
      * @param Tinebase_Record_Interface $_record
      */
@@ -1692,6 +1690,32 @@ abstract class Tinebase_Controller_Record_Abstract
             Tinebase_FileSystem_RecordAttachments::getInstance()->deleteRecordAttachments($_record);
         }
     }
+
+    /**
+     * unDelete linked objects (notes, relations, attachments) of record
+     *
+     * @param Tinebase_Record_Interface $_record
+     */
+    protected function _unDeleteLinkedObjects(Tinebase_Record_Interface $_record)
+    {
+        if ($_record->has('notes') && count($_record->notes) > 0) {
+            $ids = array();
+            foreach($_record['notes'] as $val) {
+                $ids[] = $val['id'];
+            }
+            Tinebase_Notes::getInstance()->unDeleteNotes($ids);
+        }
+
+        if ($_record->has('relations') && count($_record->relations) > 0) {
+            Tinebase_Relations::getInstance()->undeleteRelations($_record->relations);
+        }
+
+        if ($_record->has('attachments') && count($_record->attachments) > 0 && Tinebase_Core::isFilesystemAvailable()) {
+            foreach ($_record->attachments as $attachment) {
+                Tinebase_FileSystem::getInstance()->unDeleteFileNode($attachment['id']);
+            }
+        }
+    }
     
     /**
      * delete linked relations
index 6654ca2..94d0841 100644 (file)
@@ -1393,6 +1393,56 @@ class Tinebase_FileSystem implements
     /**
      * delete file node
      *
+     * @param string $id
+     * @param bool $updateDirectoryNodesHash
+     * @throws Tinebase_Exception_InvalidArgument
+     */
+    public function unDeleteFileNode($id, $updateDirectoryNodesHash = true)
+    {
+        if (false === $this->_modLogActive ) {
+            return;
+        }
+
+        $transactionId = Tinebase_TransactionManager::getInstance()->startTransaction(Tinebase_Core::getDb());
+        try {
+            $node = $this->get($id, true);
+
+            Tinebase_Timemachine_ModificationLog::setRecordMetaData($node, 'undelete', $node);
+            $this->_getTreeNodeBackend()->update($node);
+
+            /** @var Tinebase_Model_Tree_FileObject $object */
+            $object = $this->_fileObjectBackend->get($node->object_id, true);
+
+            if ($object->is_deleted) {
+                Tinebase_Timemachine_ModificationLog::setRecordMetaData($object, 'undelete', $object);
+                $object->indexed_hash = '';
+                $this->_fileObjectBackend->update($object);
+            }
+
+            if (true === $updateDirectoryNodesHash) {
+                $this->_updateFolderSizesUpToRoot(new Tinebase_Record_RecordSet('Tinebase_Model_Tree_Node', array($node)),
+                    (int)$node->size, (int)$node->revision_size);
+
+                try {
+                    $path = Tinebase_Model_Tree_Node_Path::createFromPath($this->getPathOfNode($node, true));
+                    $this->_updateDirectoryNodesHash(dirname($path->statpath));
+
+                    // Tinebase_Model_Tree_Node_Path::_getContainerType may find that is not a personal or shared container (for example it may be a records container)
+                } catch (Tinebase_Exception_InvalidArgument $teia) {}
+            }
+
+            Tinebase_TransactionManager::getInstance()->commitTransaction($transactionId);
+            $transactionId = null;
+        } finally {
+            if (null !== $transactionId) {
+                Tinebase_TransactionManager::getInstance()->rollBack();
+            }
+        }
+    }
+
+    /**
+     * delete file node
+     *
      * @param Tinebase_Model_Tree_Node $node
      * @param bool $updateDirectoryNodesHash
      * @throws Tinebase_Exception_InvalidArgument
index c0dfa5c..5b9ac04 100644 (file)
@@ -39,6 +39,7 @@ class Tinebase_Model_ModificationLogFilter extends Tinebase_Model_Filter_FilterG
      * @var array filter model fieldName => definition
      */
     protected $_filterModel = array(
+        'id'                   => array('filter' => 'Tinebase_Model_Filter_Id'),
         'application_id'       => array('filter' => 'Tinebase_Model_Filter_Id'),
         'record_id'            => array('filter' => 'Tinebase_Model_Filter_Id'),
         'modification_account' => array('filter' => 'Tinebase_Model_Filter_Id'),
index d5f25c9..fdaa4d6 100644 (file)
@@ -18,6 +18,7 @@
  * @property  string   contenttype
  * @property  integer  size
  * @property  integer  revision_size
+ * @property  string   indexed_hash
  * @property  string   hash
  * @property  string   type
  * @property  integer  preview_count
index 6233fcf..36a77ad 100644 (file)
@@ -356,8 +356,8 @@ class Tinebase_Notes implements Tinebase_Backend_Sql_Interface
                         }
                     }
                 }
-                
             }
+            $_record->$_notesProperty = $notesToSet;
         }
         
         //$toAttach = array_diff($notesToSet->getArrayOfIds(), $currentNotesIds);
@@ -577,6 +577,27 @@ class Tinebase_Notes implements Tinebase_Backend_Sql_Interface
     }
 
     /**
+     * undelete notes
+     *
+     * @param array $ids
+     */
+    public function unDeleteNotes(array $ids)
+    {
+        $sqlBackend = new Tinebase_Backend_Sql(
+            array(
+                'tableName' => $this->getTableName(),
+                'modelName' => 'Tinebase_Model_Note'
+            ),
+            $this->getAdapter());
+
+        $notes = $sqlBackend->getMultiple($ids);
+        foreach($notes as $note) {
+            Tinebase_Timemachine_ModificationLog::setRecordMetaData($note, 'undelete', $note);
+            $sqlBackend->update($note);
+        }
+    }
+
+    /**
      * delete notes
      *
      * @param  string $_model     model of record
index 9391a78..59a7dc5 100644 (file)
@@ -1312,7 +1312,18 @@ abstract class Tinebase_Record_Abstract implements Tinebase_Record_Interface
 
         foreach((array)($diff->oldData) as $property => $oldValue)
         {
-            if (in_array($property, $this->_datetimeFields) && ! is_object($oldValue)) {
+            if ('customfields' === $property) {
+                if (!is_array($oldValue)) {
+                    $oldValue = array();
+                }
+                if (isset($diff->diff['customfields']) && is_array($diff->diff['customfields'])) {
+                    foreach (array_keys($diff->diff['customfields']) as $unSetProperty) {
+                        if (!isset($oldValue[$unSetProperty])) {
+                            $oldValue[$unSetProperty] = null;
+                        }
+                    }
+                }
+            } elseif (in_array($property, $this->_datetimeFields) && ! is_object($oldValue)) {
                 $oldValue = new Tinebase_DateTime($oldValue);
             }
             $this->$property = $oldValue;
index dcb6381..616b0dc 100644 (file)
@@ -70,7 +70,7 @@ class Tinebase_Relation_Backend_Sql extends Tinebase_Backend_Sql_Abstract
     /**
      * adds a new relation
      * 
-     * @param  Tinebase_Model_Relation $_relation 
+     * @param  Tinebase_Model_Relation $_relation
      * @return Tinebase_Model_Relation the new relation
      * 
      * @todo    move check existance and update / modlog to controller?
index 603ee91..33fafa3 100644 (file)
@@ -52,6 +52,42 @@ class Tinebase_Relations
         }
         return self::$instance;
     }
+
+    /**
+     * set all relations of a given record
+     *
+     * NOTE: given relation data is expected to be an array atm.
+     *
+     * @param  array  $_relationData    data for relations to create
+
+     * @return void
+     */
+    public function undeleteRelations($_relationData)
+    {
+        foreach((array) $_relationData as $relationData) {
+            if ($relationData instanceof Tinebase_Model_Relation) {
+                $relation = $relationData;
+            } else {
+                $relation = new Tinebase_Model_Relation($relationData, true);
+            }
+
+            $relation->related_record = null;
+
+            try {
+                $appController = Tinebase_Core::getApplicationInstance($relation->related_model);
+                try {
+                    $appController->getBackend()->get($relation->related_id);
+                } catch (Tinebase_Exception_NotFound $tenf) {
+                    continue;
+                }
+            } catch(Tinebase_Exception_AccessDenied $tead) {
+                // we just undelete it...
+            }
+
+            Tinebase_Timemachine_ModificationLog::setRecordMetaData($relation, 'undelete', $relation);
+            $this->_updateRelation($relation);
+        }
+    }
     
     /**
      * set all relations of a given record
@@ -669,7 +705,7 @@ class Tinebase_Relations
     /**
      * adds a new relation
      * 
-     * @param   Tinebase_Model_Relation $_relation 
+     * @param   Tinebase_Model_Relation $_relation
      * @return  Tinebase_Model_Relation|NULL the new relation
      * @throws  Tinebase_Exception_Record_Validation
      */
index 811de7c..4064f03 100644 (file)
@@ -78,9 +78,7 @@ class Tinebase_Timemachine_ModificationLog implements Tinebase_Controller_Interf
         'deleted_time',
         'deleted_by',
         'seq',
-    // @todo allow notes modlog
-        'notes',
-    // record specific properties / no meta properties 
+    // record specific properties / no meta properties
     // @todo to be moved to (contact) record definition
         'jpegphoto',
     );
@@ -781,12 +779,21 @@ class Tinebase_Timemachine_ModificationLog implements Tinebase_Controller_Interf
             $notNullRecord = $_newRecord;
         } else {
             if (null !== $_newRecord) {
-                $diff = new Tinebase_Record_Diff(array('diff' => $_newRecord->toArray()));
                 $notNullRecord = $_newRecord;
+                $diffProp = 'diff';
             } else {
-                $diff = new Tinebase_Record_Diff(array('oldData' => $_curRecord->toArray()));
                 $notNullRecord = $_curRecord;
+                $diffProp = 'oldData';
             }
+            $diffData = $notNullRecord->toArray();
+
+            foreach (array_merge($this->_metaProperties, $notNullRecord->getModlogOmitFields()) as $omit) {
+                if (isset($diffData[$omit])) {
+                    unset($diffData[$omit]);
+                }
+            }
+
+            $diff = new Tinebase_Record_Diff(array($diffProp => $diffData));
         }
 
         if (! $diff->isEmpty()) {
@@ -988,10 +995,6 @@ class Tinebase_Timemachine_ModificationLog implements Tinebase_Controller_Interf
             /* TODO $overwrite check in new code path! */
 
             $modifiedAttribute = $modlog->modified_attribute;
-            $isDeleted = false;
-            if (empty($modifiedAttribute)) {
-                $isDeleted = Tinebase_Timemachine_ModificationLog::DELETED === $modlog->change_type;
-            }
 
             try {
 
@@ -1004,18 +1007,19 @@ class Tinebase_Timemachine_ModificationLog implements Tinebase_Controller_Interf
                         $controller->undoReplicationModificationLog($modlog, $dryrun);
                     } else {
 
-                        $record = $controller->get($modlog->record_id, NULL, TRUE, $isDeleted);
-
                         if (Tinebase_Timemachine_ModificationLog::CREATED === $modlog->change_type) {
                             if (!$dryrun) {
-                                $controller->delete($record->getId());
+                                $controller->delete($modlog->record_id);
                             }
-                        } elseif (true === $isDeleted) {
+                        } elseif (Tinebase_Timemachine_ModificationLog::DELETED === $modlog->change_type) {
+                            $diff = new Tinebase_Record_Diff(json_decode($modlog->new_value, true));
+                            $model = $modlog->record_type;
+                            $record = new $model($diff->oldData, true);
                             if (!$dryrun) {
                                 $controller->unDelete($record);
                             }
                         } else {
-                            /** TODO this is not finished YET! see $notUndoableFields below etc.! */
+                            $record = $controller->get($modlog->record_id, null, true, true);
                             $diff = new Tinebase_Record_Diff(json_decode($modlog->new_value, true));
                             $record->undo($diff);
 
@@ -1028,7 +1032,7 @@ class Tinebase_Timemachine_ModificationLog implements Tinebase_Controller_Interf
                     // this is the legacy code for old data in existing installations
                 } else {
 
-                    $record = $controller->get($modlog->record_id, NULL, TRUE, $isDeleted);
+                    $record = $controller->get($modlog->record_id);
 
                     if (!in_array($modlog->modified_attribute, $notUndoableFields) && ($overwrite || $record->seq === $modlog->seq)) {
                         if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG)) Tinebase_Core::getLogger()->debug(__METHOD__ . '::' . __LINE__ .
@@ -1113,7 +1117,7 @@ class Tinebase_Timemachine_ModificationLog implements Tinebase_Controller_Interf
             case 'undelete':
                 $_newRecord->deleted_by   = null;
                 $_newRecord->deleted_time = null;
-                $_newRecord->is_deleted   = false;
+                $_newRecord->is_deleted   = 0;
                 self::increaseRecordSequence($_newRecord, $_curRecord);
                 break;
             default: