0011590: improve concurrent update check performance
authorPaul Mehrer <p.mehrer@metaways.de>
Mon, 8 Feb 2016 13:45:43 +0000 (14:45 +0100)
committerPhilipp Schüle <p.schuele@metaways.de>
Wed, 10 Feb 2016 17:06:06 +0000 (18:06 +0100)
Tinebase_Timemachine_ModificationLog::getModificationsBySeq did
not use the application id to filter, so index could not be used
fixed that

https://forge.tine20.org/view.php?id=11590

Change-Id: Ia4bdc28fc356acb655935dde5a0fc7cfc8988674
Reviewed-on: http://gerrit.tine20.com/customers/2676
Tested-by: Jenkins CI (http://ci.tine20.com/)
Reviewed-by: Philipp Schüle <p.schuele@metaways.de>
tests/tine20/Calendar/JsonTests.php
tests/tine20/Tinebase/Timemachine/ModificationLogTest.php
tine20/Tinebase/Controller/Record/Abstract.php
tine20/Tinebase/Timemachine/ModificationLog.php

index e01c0b7..024fed6 100644 (file)
@@ -1434,7 +1434,9 @@ class Calendar_JsonTests extends Calendar_TestCase
         $eventData['attendee'][$adminIndex]['status'] = Calendar_Model_Attender::STATUS_TENTATIVE;
         $event = $this->_uit->saveEvent($eventData);
         
-        $loggedMods = Tinebase_Timemachine_ModificationLog::getInstance()->getModificationsBySeq(new Calendar_Model_Attender($eventData['attendee'][$adminIndex]), 2);
+        $loggedMods = Tinebase_Timemachine_ModificationLog::getInstance()->getModificationsBySeq(
+            Tinebase_Application::getInstance()->getApplicationByName('Calendar')->getId(),
+            new Calendar_Model_Attender($eventData['attendee'][$adminIndex]), 2);
         $this->assertEquals(1, count($loggedMods), 'attender modification has not been logged');
         
         $eventData['attendee'] = $currentAttendee;
index b6148d1..458ca09 100644 (file)
@@ -327,7 +327,9 @@ class Tinebase_Timemachine_ModificationLogTest extends PHPUnit_Framework_TestCas
         $updatedTask = Tasks_Controller_Task::getInstance()->update($task);
         
         $task->seq = 1;
-        $modlog = $this->_modLogClass->getModificationsBySeq($task, 2);
+        $modlog = $this->_modLogClass->getModificationsBySeq(
+            Tinebase_Application::getInstance()->getApplicationByName('Tasks')->getId(),
+            $task, 2);
         
         $this->assertEquals(1, count($modlog));
         $this->assertEquals((string) $task->due, (string) $modlog->getFirstRecord()->new_value, 'new value mismatch: ' . print_r($modlog->toArray(), TRUE));
index b0140d3..2326cb9 100644 (file)
@@ -919,7 +919,9 @@ abstract class Tinebase_Controller_Record_Abstract
             . ' Doing concurrency check ...');
 
         $modLog = Tinebase_Timemachine_ModificationLog::getInstance();
-        $modLog->manageConcurrentUpdates($_record, $_currentRecord);
+        $modLog->manageConcurrentUpdates(
+            Tinebase_Application::getInstance()->getApplicationByName($this->_applicationName)->getId(),
+            $_record, $_currentRecord);
         $modLog->setRecordMetaData($_record, 'update', $_currentRecord);
     }
     
index 92ef101..1cfd9aa 100644 (file)
@@ -94,6 +94,13 @@ class Tinebase_Timemachine_ModificationLog
      * @var Tinebase_Timemachine_ModificationLog
      */
     private static $instance = NULL;
+
+    /**
+     * holds the applicationId of the current context temporarily.
+     *
+     * @var string
+     */
+    protected $_applicationId = NULL;
     
     /**
      * the singleton pattern
@@ -183,18 +190,20 @@ class Tinebase_Timemachine_ModificationLog
 
     /**
      * get modifications by seq
-     * 
+     *
+     * @param string $applicationId
      * @param Tinebase_Record_Abstract $newRecord
      * @param integer $currentSeq
      * @return Tinebase_Record_RecordSet RecordSet of Tinebase_Model_ModificationLog
      */
-    public function getModificationsBySeq(Tinebase_Record_Abstract $newRecord, $currentSeq)
+    public function getModificationsBySeq($applicationId, Tinebase_Record_Abstract $newRecord, $currentSeq)
     {
         $filter = new Tinebase_Model_ModificationLogFilter(array(
             array('field' => 'seq',            'operator' => 'greater', 'value' => $newRecord->seq),
             array('field' => 'seq',            'operator' => 'less',    'value' => $currentSeq + 1),
             array('field' => 'record_type',    'operator' => 'equals',  'value' => get_class($newRecord)),
             array('field' => 'record_id',      'operator' => 'equals',  'value' => $newRecord->getId()),
+            array('field' => 'application_id', 'operator' => 'equals',  'value' => $applicationId),
         ));
         $paging = new Tinebase_Model_Pagination(array(
             'sort' => 'seq'
@@ -292,18 +301,21 @@ class Tinebase_Timemachine_ModificationLog
     
     /**
      * merges changes made to local storage on concurrent updates into the new record 
-     * 
+     *
+     * @param string $applicationId
      * @param  Tinebase_Record_Interface $newRecord record from user data
      * @param  Tinebase_Record_Interface $curRecord record from storage
      * @return Tinebase_Record_RecordSet with resolved concurrent updates (Tinebase_Model_ModificationLog records)
      * @throws Tinebase_Timemachine_Exception_ConcurrencyConflict
      */
-    public function manageConcurrentUpdates(Tinebase_Record_Interface $newRecord, Tinebase_Record_Interface $curRecord)
+    public function manageConcurrentUpdates($applicationId, Tinebase_Record_Interface $newRecord, Tinebase_Record_Interface $curRecord)
     {
         if (! $newRecord->has('seq')) {
             return $this->manageConcurrentUpdatesByTimestamp($newRecord, $curRecord, get_class($newRecord), 'Sql', $newRecord->getId());
         }
-        
+
+        $this->_applicationId = $applicationId;
+
         $resolved = new Tinebase_Record_RecordSet('Tinebase_Model_ModificationLog');
         if ($curRecord->seq != $newRecord->seq) {
             
@@ -315,7 +327,7 @@ class Tinebase_Timemachine_ModificationLog
                     ($curRecord->creation_time instanceof DateTime ? $curRecord->creation_time : 'unknown')) .
                 "' / current sequence: " . $curRecord->seq . " - new record sequence: " . $newRecord->seq);
             
-            $loggedMods = $this->getModificationsBySeq($newRecord, $curRecord->seq);
+            $loggedMods = $this->getModificationsBySeq($applicationId, $newRecord, $curRecord->seq);
             
             // effective modifications made to the record after current user got his record
             $diffs = $this->computeDiff($loggedMods);
@@ -441,8 +453,11 @@ class Tinebase_Timemachine_ModificationLog
                     . ' new record: ' . print_r($newRecordsRecord->toArray(), TRUE));
                 if (Tinebase_Core::isLogLevel(Zend_Log::TRACE)) Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__
                     . ' modified record: ' . print_r($modifiedRecord->toArray(), TRUE));
-                
-                $this->manageConcurrentUpdates($newRecordsRecord, $modifiedRecord);
+
+                if (null === $this->_applicationId) {
+                    throw new Tinebase_Exception_UnexpectedValue('application_id needs to be set here');
+                }
+                $this->manageConcurrentUpdates($this->_applicationId, $newRecordsRecord, $modifiedRecord);
             } else {
                 throw new Tinebase_Timemachine_Exception_ConcurrencyConflict('concurrency conflict - modified record changes could not be merged!');
             }