add inClassCache for Tinebase_Acl_Roles class
authorLars Kneschke <l.kneschke@metaways.de>
Fri, 19 Dec 2014 21:14:32 +0000 (22:14 +0100)
committerPhilipp Schüle <p.schuele@metaways.de>
Mon, 29 Dec 2014 09:18:23 +0000 (10:18 +0100)
- read all rights of given account at once, cache them in class cache
and check for requested right in PHP

Change-Id: I8f85acf0accd16ebb32cba994887fcf212c049f4
Reviewed-on: http://gerrit.tine20.com/customers/1476
Tested-by: Jenkins CI (http://ci.tine20.com/)
Reviewed-by: Philipp Schüle <p.schuele@metaways.de>
tests/tine20/Calendar/TestCase.php
tests/tine20/Timetracker/AbstractTest.php
tests/tine20/Timetracker/ControllerTest.php
tests/tine20/Tinebase/Acl/RolesTest.php
tine20/Tinebase/Acl/Roles.php

index 9d32ffd..6eb32c2 100644 (file)
@@ -87,6 +87,8 @@ abstract class Calendar_TestCase extends TestCase
         
         Calendar_Controller_Event::getInstance()->sendNotifications(false);
         
+        Tinebase_Acl_Roles::getInstance()->resetClassCache();
+        
         if (! $this->_transactionId) {
             if ($this->_backend != NULL) {
                 $events = $this->_backend->search(new Calendar_Model_EventFilter(array(
index c22da7c..3260129 100644 (file)
@@ -44,6 +44,7 @@ abstract class Timetracker_AbstractTest extends PHPUnit_Framework_TestCase
      */
     protected function setUp()
     {
+        Tinebase_Acl_Roles::getInstance()->resetClassCache();
         Tinebase_TransactionManager::getInstance()->startTransaction(Tinebase_Core::getDb());
         $this->_json = new Timetracker_Frontend_Json();
     }
@@ -57,6 +58,7 @@ abstract class Timetracker_AbstractTest extends PHPUnit_Framework_TestCase
     protected function tearDown()
     {
         Tinebase_TransactionManager::getInstance()->rollBack();
+        Tinebase_Acl_Roles::getInstance()->resetClassCache();
     }
 
     /************ protected helper funcs *************/
index 5fbc866..c68bd19 100644 (file)
@@ -6,21 +6,16 @@
  * 
  * @package     Timetracker
  * @license     http://www.gnu.org/licenses/agpl.html
- * @copyright   Copyright (c) 2008-2012 Metaways Infosystems GmbH (http://www.metaways.de)
+ * @copyright   Copyright (c) 2008-2014 Metaways Infosystems GmbH (http://www.metaways.de)
  * @author      Philipp Schüle <p.schuele@metaways.de>
  * 
  * @todo        add test for manage_billable
  */
 
 /**
- * Test helper
+ * Test class for Timetracker_ControllerTest
  */
-require_once dirname(dirname(__FILE__)) . DIRECTORY_SEPARATOR . 'TestHelper.php';
-
-/**
- * Test class for Tinebase_Group
- */
-class Timetracker_ControllerTest extends PHPUnit_Framework_TestCase
+class Timetracker_ControllerTest extends TestCase
 {
     /**
      * @var Timetracker_Controller_Timeaccount
@@ -47,18 +42,6 @@ class Timetracker_ControllerTest extends PHPUnit_Framework_TestCase
     protected $_objects = array();
     
     /**
-     * Runs the test methods of this class.
-     *
-     * @access public
-     * @static
-     */
-    public static function main()
-    {
-        $suite  = new PHPUnit_Framework_TestSuite('Tine 2.0 Timetracker Controller Tests');
-        PHPUnit_TextUI_TestRunner::run($suite);
-    }
-
-    /**
      * Sets up the fixture.
      * This method is called before a test is executed.
      *
@@ -66,6 +49,8 @@ class Timetracker_ControllerTest extends PHPUnit_Framework_TestCase
      */
     protected function setUp()
     {
+        parent::setUp();
+        
         $this->_timeaccountController = Timetracker_Controller_Timeaccount::getInstance();
         $this->_timesheetController = Timetracker_Controller_Timesheet::getInstance();
 
@@ -73,6 +58,8 @@ class Timetracker_ControllerTest extends PHPUnit_Framework_TestCase
         $this->_objects['timesheet'] = $this->_getTimesheet();
         $this->_objects['timeaccount'] = $this->_timeaccountController->get($this->_objects['timesheet']->timeaccount_id);
         
+        Tinebase_Acl_Roles::getInstance()->resetClassCache();
+        
         $this->_roleRights = self::removeManageAllRight();
     }
     
@@ -107,11 +94,9 @@ class Timetracker_ControllerTest extends PHPUnit_Framework_TestCase
      */
     protected function tearDown()
     {
-        // reset old admin role rights
-        Tinebase_Acl_Roles::getInstance()->setRoleRights(Tinebase_Acl_Roles::getInstance()->getRoleByName('admin role')->getId(), $this->_roleRights);
+        parent::tearDown();
         
-        // delete timeaccount
-        $this->_timeaccountController->delete($this->_objects['timeaccount']->getId());
+        Tinebase_Acl_Roles::getInstance()->resetClassCache();
     }
     
     /************ test functions follow **************/
index 380678b..d109e3a 100644 (file)
@@ -60,7 +60,7 @@ class Tinebase_Acl_RolesTest extends TestCase
         $this->objects['user'] = Tinebase_User::getInstance()->addUser($this->objects['user']);
         Tinebase_Group::getInstance()->addGroupMember($this->objects['user']->accountPrimaryGroup, $this->objects['user']);
         
-        return;
+        Tinebase_Acl_Roles::getInstance()->resetClassCache();
     }
 
     /**
@@ -72,7 +72,7 @@ class Tinebase_Acl_RolesTest extends TestCase
     protected function tearDown()
     {
         Tinebase_User::getInstance()->deleteUser($this->objects['user']->accountId);
-        
+        Tinebase_Acl_Roles::getInstance()->resetClassCache();
         parent::tearDown();
     }
 
index fa5e99f..0c57b64 100644 (file)
@@ -49,6 +49,11 @@ class Tinebase_Acl_Roles
      * @var Tinebase_Db_Table
      */
     protected $_roleRightsTable;
+        
+    protected $_classCache = array(
+        'getRoleMemberships' => array(),
+        'hasRight'           => array(),
+    );
     
     /**
      * holdes the _instance of the singleton
@@ -98,7 +103,8 @@ class Tinebase_Acl_Roles
 
     /**
      * check if one of the roles the user is in has a given right for a given application
-     * - admin right includes all other rights
+     * 
+     * we read all right for the given user at once and cache them in the internal class cache
      *
      * @param   string|Tinebase_Model_Application $_application the application (one of: app name, id or record)
      * @param   int $_accountId the numeric id of a user account
@@ -133,24 +139,33 @@ class Tinebase_Acl_Roles
             return false;
         }
         
-        $select = $this->_db->select()
-            ->distinct()
-            ->from(array('role_rights' => SQL_TABLE_PREFIX . 'role_rights'), array('right'))->where($this->_db->quoteInto($this->_db->quoteIdentifier('role_id') . ' IN (?)', $roleMemberships))
-            ->where('(' .    $this->_db->quoteInto($this->_db->quoteIdentifier('right') . ' = ?', $_right) 
-                . ' OR ' . $this->_db->quoteInto($this->_db->quoteIdentifier('right') . ' = ?', Tinebase_Acl_Rights::ADMIN) . ')')
-            ->where($this->_db->quoteInto($this->_db->quoteIdentifier('application_id') . ' = ?', $application->getId()));
-               
-        if (Tinebase_Core::isLogLevel(Zend_Log::TRACE)) Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__ . ' ' . $select->__toString());
-        
-        $stmt = $this->_db->query($select);
-        
-        $rows = $stmt->fetchAll(Zend_Db::FETCH_COLUMN);
+        $classCacheId = convertCacheId(implode('', $roleMemberships));
         
-        if (!$rows) {
-            return false;
+        if (!isset($this->_classCache[__FUNCTION__][$classCacheId])) {
+            $select = $this->_db->select()
+                ->distinct()
+                ->from(array('role_rights' => SQL_TABLE_PREFIX . 'role_rights'), array('application_id', 'right'))
+                ->where($this->_db->quoteIdentifier('role_id') . ' IN (?)', $roleMemberships);
+                
+            if (Tinebase_Core::isLogLevel(Zend_Log::TRACE)) Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__ . ' ' . $select->__toString());
+            
+            $stmt = $this->_db->query($select);
+            $rows = $stmt->fetchAll(Zend_Db::FETCH_ASSOC);
+            
+            $rights = array();
+            
+            foreach ($rows as $row) {
+                $rights[$row['application_id']][$row['right']] = true;
+            }
+            
+            $this->_classCache[__FUNCTION__][$classCacheId] = $rights;
+        } else {
+            $rights = $this->_classCache[__FUNCTION__][$classCacheId];
         }
         
-        return true;
+        $applicationId = $application->getId();
+        
+        return isset($rights[$applicationId]) && (isset($rights[$applicationId][$_right]) || isset($rights[$applicationId][Tinebase_Acl_Rights::ADMIN]));
     }
     
     /**
@@ -172,8 +187,6 @@ class Tinebase_Acl_Roles
             return new Tinebase_Record_RecordSet('Tinebase_Model_Application');
         }
 
-        $rightIdentifier = $this->_db->quoteIdentifier(SQL_TABLE_PREFIX . 'role_rights.right');
-        
         $select = $this->_db->select()
             ->distinct()
             ->from(array('role_rights' => SQL_TABLE_PREFIX . 'role_rights'), array())
@@ -181,14 +194,14 @@ class Tinebase_Acl_Roles
                 /* table  */ array('applications' => SQL_TABLE_PREFIX . 'applications'), 
                 /* on     */ $this->_db->quoteIdentifier('role_rights.application_id') . ' = ' . $this->_db->quoteIdentifier('applications.id')
             )
-            ->where($this->_db->quoteInto($this->_db->quoteIdentifier('role_id') . ' IN (?)', $roleMemberships))
-            ->where($this->_db->quoteInto($this->_db->quoteIdentifier('applications.status') . ' = ?', Tinebase_Application::ENABLED))
+            ->where($this->_db->quoteIdentifier('role_id') . ' IN (?)',          $roleMemberships)
+            ->where($this->_db->quoteIdentifier('applications.status') . ' = ?', Tinebase_Application::ENABLED)
             ->order('order ASC');
         
         if ($_anyRight) {
             $select->where($this->_db->quoteIdentifier('role_rights.right') . " IS NOT NULL");
         } else {
-            $select->where($this->_db->quoteInto($this->_db->quoteIdentifier('role_rights.right') . ' = ?', Tinebase_Acl_Rights::RUN));
+            $select->where($this->_db->quoteIdentifier('role_rights.right') . ' = ?', Tinebase_Acl_Rights::RUN);
         }
         
         if (Tinebase_Core::isLogLevel(Zend_Log::TRACE)) Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__
@@ -222,8 +235,8 @@ class Tinebase_Acl_Roles
         $select = $this->_db->select()
             ->distinct()
             ->from(SQL_TABLE_PREFIX . 'role_rights', array('account_rights' => SQL_TABLE_PREFIX . 'role_rights.right'))
-            ->where($this->_db->quoteInto($this->_db->quoteIdentifier(SQL_TABLE_PREFIX . 'role_rights.application_id') . ' = ?', $application->getId()))
-            ->where($this->_db->quoteInto($this->_db->quoteIdentifier('role_id') . ' IN (?)', $roleMemberships));
+            ->where($this->_db->quoteIdentifier(SQL_TABLE_PREFIX . 'role_rights.application_id') . ' = ?', $application->getId())
+            ->where($this->_db->quoteIdentifier('role_id') . ' IN (?)', $roleMemberships);
         
         $stmt = $this->_db->query($select);
 
@@ -349,6 +362,9 @@ class Tinebase_Acl_Roles
         }
         
         $role = $this->getRoleById($newId);
+        
+        $this->resetClassCache();
+        
         return $role;
     }
     
@@ -367,7 +383,10 @@ class Tinebase_Acl_Roles
         $where = $this->_db->quoteInto($this->_db->quoteIdentifier('id') . ' = ?', $_role->getId());
         $this->_rolesTable->update($data, $where);
         
+        $this->resetClassCache();
+        
         $role = $this->getRoleById($_role->getId());
+        
         return $role;
     }
     
@@ -391,6 +410,8 @@ class Tinebase_Acl_Roles
             
             Tinebase_TransactionManager::getInstance()->commitTransaction($transactionId);
             
+            $this->resetClassCache();
+            
         } catch (Exception $e) {
             Tinebase_Core::getLogger()->err(__METHOD__ . '::' . __LINE__ . ' error while deleting role ' . $e->__toString());
             Tinebase_TransactionManager::getInstance()->rollBack();
@@ -434,7 +455,7 @@ class Tinebase_Acl_Roles
         $members = array();
         
         $select = $this->_roleMembersTable->select();
-        $select->where($this->_db->quoteInto($this->_db->quoteIdentifier('role_id') . ' = ?', $_roleId));
+        $select->where($this->_db->quoteIdentifier('role_id') . ' = ?', $_roleId);
         
         $rows = $this->_roleMembersTable->fetchAll($select)->toArray();
         
@@ -452,17 +473,25 @@ class Tinebase_Acl_Roles
     public function getRoleMemberships($accountId, $type = Tinebase_Acl_Rights::ACCOUNT_TYPE_USER)
     {
         if ($type === Tinebase_Acl_Rights::ACCOUNT_TYPE_USER) {
-            $accountId = Tinebase_Model_User::convertUserIdToInt($accountId);
+            $accountId        = Tinebase_Model_User::convertUserIdToInt($accountId);
             $groupMemberships = Tinebase_Group::getInstance()->getGroupMemberships($accountId);
             if (empty($groupMemberships)) {
                 throw new Tinebase_Exception_NotFound('Any account must belong to at least one group. The account with accountId ' . $accountId . ' does not belong to any group.');
             }
+            
+            $classCacheId = convertCacheId ($accountId . implode('', $groupMemberships) . $type);
         } else if ($type === Tinebase_Acl_Rights::ACCOUNT_TYPE_GROUP) {
             $accountId = Tinebase_Model_Group::convertGroupIdToInt($accountId);
+            
+            $classCacheId = convertCacheId ($accountId . $type);
         } else {
             throw new Tinebase_Exception_InvalidArgument('Invalid type: ' . $type);
         }
         
+        if (isset($this->_classCache[__FUNCTION__][$classCacheId])) {
+            return $this->_classCache[__FUNCTION__][$classCacheId];
+        }
+        
         $select = $this->_db->select()
             ->distinct()
             ->from(array('role_accounts' => SQL_TABLE_PREFIX . 'role_accounts'), array('role_id'))
@@ -478,6 +507,8 @@ class Tinebase_Acl_Roles
         
         $memberships = $stmt->fetchAll(Zend_Db::FETCH_COLUMN);
         
+        $this->_classCache[__FUNCTION__][$classCacheId] = $memberships;
+        
         return $memberships;
     }
 
@@ -514,6 +545,8 @@ class Tinebase_Acl_Roles
             );
             $this->_roleMembersTable->insert($data);
         }
+        
+        $this->resetClassCache();
     }
     
     /**
@@ -589,6 +622,8 @@ class Tinebase_Acl_Roles
         } catch (Zend_Db_Statement_Exception $e) {
             // account is already member of this group
         }
+        
+        $this->resetClassCache();
     }
     
     /**
@@ -611,6 +646,25 @@ class Tinebase_Acl_Roles
         );
          
         $this->_roleMembersTable->delete($where);
+        
+        $this->resetClassCache();
+    }
+    
+    /**
+     * reset class cache
+     * 
+     * @param string $key
+     * @return Tinebase_Acl_Roles
+     */
+    public function resetClassCache($key = null)
+    {
+        foreach ($this->_classCache as $cacheKey => $cacheValue) {
+            if ($key === null || $key === $cacheKey) {
+                $this->_classCache[$cacheKey] = array();
+            }
+        }
+        
+        return $this;
     }
     
     /**
@@ -630,7 +684,7 @@ class Tinebase_Acl_Roles
         $select = $this->_db->select()
             ->distinct()
             ->from(array('role_rights' => SQL_TABLE_PREFIX . 'role_rights'), array('application_id', 'right'))
-            ->where($this->_db->quoteInto($this->_db->quoteIdentifier('role_id') . ' = ?', $_roleId));
+            ->where($this->_db->quoteIdentifier('role_id') . ' = ?', $_roleId);
         
         $stmt = $this->_db->query($select);
         
@@ -698,6 +752,8 @@ class Tinebase_Acl_Roles
             'application_id'    => $applicationId,
             'right'             => $right,
         ));
+        
+        $this->resetClassCache();
     }
     
     /**
@@ -716,6 +772,8 @@ class Tinebase_Acl_Roles
         );
         
         $this->_roleRightsTable->delete($where);
+        
+        $this->resetClassCache();
     }
     
     /**
@@ -750,6 +808,8 @@ class Tinebase_Acl_Roles
                 Tinebase_Core::getCache()->remove($cacheId);
             }
         }
+        
+        $this->resetClassCache();
     }
     
     /**
@@ -765,13 +825,15 @@ class Tinebase_Acl_Roles
     {
         // check if already in
         $select = $this->_roleRightsTable->select();
-        $select->where($this->_db->quoteInto($this->_db->quoteIdentifier('role_id') . ' = ?',        $_roleId))
-               ->where($this->_db->quoteInto($this->_db->quoteIdentifier('right') . ' = ?',          $_right))
-               ->where($this->_db->quoteInto($this->_db->quoteIdentifier('application_id') . ' = ?', $_applicationId));
+        $select->where($this->_db->quoteIdentifier('role_id') . ' = ?',        $_roleId)
+               ->where($this->_db->quoteIdentifier('right') . ' = ?',          $_right)
+               ->where($this->_db->quoteIdentifier('application_id') . ' = ?', $_applicationId);
         
         if (!$row = $this->_roleRightsTable->fetchRow($select)) {
             $this->addRoleRight($_roleId, $_applicationId, $_right);
         }
+        
+        $this->resetClassCache();
     }
     
     /**
@@ -812,5 +874,7 @@ class Tinebase_Acl_Roles
                 'type'  => Tinebase_Acl_Rights::ACCOUNT_TYPE_GROUP, 
             )
         ));
+        
+        $this->resetClassCache();
     }
 }