0011172: optimize getGroupmemberships in Principalbackend
authorPhilipp Schüle <p.schuele@metaways.de>
Fri, 14 Aug 2015 10:02:20 +0000 (12:02 +0200)
committerPhilipp Schüle <p.schuele@metaways.de>
Fri, 4 Sep 2015 10:05:30 +0000 (12:05 +0200)
backports fix to 2013.10

* Tinebase_Container: added functionality to AND connect ACL checks
* Tinebase_WebDav_PrincipalBackend: makes use of AND connected ACL checks
* improve test for delegations

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

Change-Id: I036276c7e718d961539a6af14ba84077d34896aa
Reviewed-on: http://gerrit.tine20.com/customers/2165
Tested-by: Jenkins CI (http://ci.tine20.com/)
Reviewed-by: Philipp Schüle <p.schuele@metaways.de>
tests/tine20/Calendar/Frontend/CalDAV/ProxyTest.php
tine20/Tinebase/Container.php
tine20/Tinebase/WebDav/PrincipalBackend.php

index b41e19c..ffeb92a 100644 (file)
@@ -34,7 +34,31 @@ class Calendar_Frontend_CalDAV_ProxyTest extends TestCase
     protected function setUp()
     {
         parent::setUp();
-        
+
+        // create shared folder and other users folder
+        $this->sharedContainer = Tinebase_Container::getInstance()->addContainer(new Tinebase_Model_Container(array(
+            'name'           => __CLASS__ . Tinebase_Record_Abstract::generateUID(),
+            'type'           => Tinebase_Model_Container::TYPE_SHARED,
+            'application_id' => Tinebase_Application::getInstance()->getApplicationByName('Calendar')->getId(),
+            'backend'        => 'Sql'
+        )));
+
+        $sclever = array_value('sclever', Zend_Registry::get('personas'));
+        $this->otherUsersContainer = Tinebase_Container::getInstance()->addContainer(new Tinebase_Model_Container(array(
+            'name'           => __CLASS__ . Tinebase_Record_Abstract::generateUID(),
+            'type'           => Tinebase_Model_Container::TYPE_PERSONAL,
+            'owner_id'       => $sclever->getId(),
+            'application_id' => Tinebase_Application::getInstance()->getApplicationByName('Calendar')->getId(),
+            'backend'        => 'Sql'
+        )));
+        Tinebase_Container::getInstance()->addGrants($this->otherUsersContainer, Tinebase_Acl_Rights::ACCOUNT_TYPE_USER, Tinebase_Core::getUser(), array(
+            Tinebase_Model_Grants::GRANT_READ,
+            Tinebase_Model_Grants::GRANT_SYNC,
+        ), true);
+
+        // clear container caches (brute force)
+        Tinebase_Core::getCache()->clean(Zend_Cache::CLEANING_MODE_ALL);
+
         $this->server = new Sabre\DAV\Server(new Tinebase_WebDav_Root());
         $this->server->debugExceptions = true;
         $this->server->addPlugin(new \Sabre\CalDAV\Plugin());
@@ -77,7 +101,7 @@ class Calendar_Frontend_CalDAV_ProxyTest extends TestCase
         $this->server->httpRequest = $request;
         $this->server->exec();
         //var_dump($this->response->body);
-        
+
         $this->assertEquals('HTTP/1.1 207 Multi-Status', $this->response->status);
         
         $responseDoc = new DOMDocument();
@@ -94,6 +118,13 @@ class Calendar_Frontend_CalDAV_ProxyTest extends TestCase
         $nodes = $xpath->query('//d:multistatus/d:response/d:propstat/d:prop/cs:calendar-proxy-write-for');
         $this->assertEquals(1, $nodes->length, $responseDoc->saveXML());
         #$this->assertEmpty($nodes->item(0)->nodeValue, $responseDoc->saveXML());
+
+        $nodes = $xpath->query('///d:response/d:href[text()="/principals/users/shared/"]');
+        $this->assertEquals(1, $nodes->length, "shared principal is missing in \n" . $responseDoc->saveXML());
+
+        $sclever = array_value('sclever', Zend_Registry::get('personas'));
+        $nodes = $xpath->query('///d:response/d:href[text()="/principals/users/'. $sclever->contact_id .'/"]');
+        $this->assertEquals(1, $nodes->length, "sclevers principal is missing in \n" .$responseDoc->saveXML());
     }
     
     /**
index 760e39c..520f493 100644 (file)
@@ -647,9 +647,10 @@ class Tinebase_Container extends Tinebase_Backend_Sql_Abstract
      * @param  String            $_accountId
      * @param  Array|String      $_grant
      * @param  String            $_aclTableName
+     * @param  bool              $_andGrants
      * @return void
      */
-    public static function addGrantsSql($_select, $_accountId, $_grant, $_aclTableName = 'container_acl')
+    public static function addGrantsSql($_select, $_accountId, $_grant, $_aclTableName = 'container_acl', $_andGrants = FALSE, $joinCallBack = NULL)
     {
         $accountId = $_accountId instanceof Tinebase_Record_Abstract
             ? $_accountId->getId()
@@ -659,10 +660,7 @@ class Tinebase_Container extends Tinebase_Backend_Sql_Abstract
         
         $grants = is_array($_grant) ? $_grant : array($_grant);
         
-        // admin grant includes all other grants
-        if (! in_array(Tinebase_Model_Grants::GRANT_ADMIN, $grants)) {
-            $grants[] = Tinebase_Model_Grants::GRANT_ADMIN;
-        }
+
 
         $groupMemberships   = Tinebase_Group::getInstance()->getGroupMemberships($accountId);
         
@@ -684,12 +682,29 @@ class Tinebase_Container extends Tinebase_Backend_Sql_Abstract
             // @todo fetch wildcard from specific db adapter
             $grants = str_replace('*', '%', $grants);
         
-            $quotedGrant   = $db->quoteIdentifier("{$_aclTableName}.account_grant");
-            
+            $quotedGrant   = $db->quoteIdentifier($_aclTableName . '.account_grant');
+
+            $iteration = 0;
             $grantsSelect = new Tinebase_Backend_Sql_Filter_GroupSelect($_select);
             foreach ($grants as $grant) {
-                $grantsSelect->orWhere("{$quotedGrant} LIKE ?", $grant);
+                if ($_andGrants) {
+                    if ($iteration > 0) {
+                        $callbackIdentifier = call_user_func($joinCallBack, $_select, $iteration);
+                        $grantsSelect->where($db->quoteIdentifier($callbackIdentifier . '.account_grant') . ' LIKE ?', $grant);
+                    } else {
+                        $grantsSelect->where($quotedGrant . ' LIKE ?', $grant);
+                    }
+                    ++$iteration;
+                } else {
+                    $grantsSelect->orWhere($quotedGrant . ' LIKE ?', $grant);
+                }
             }
+
+            // admin grant includes all other grants
+            if (! in_array(Tinebase_Model_Grants::GRANT_ADMIN, $grants)) {
+                $grantsSelect->orWhere($quotedGrant . ' LIKE ?', Tinebase_Model_Grants::GRANT_ADMIN);
+            }
+
             $grantsSelect->appendWhere(Zend_Db_Select::SQL_AND);
         }
     }
@@ -761,10 +776,11 @@ class Tinebase_Container extends Tinebase_Backend_Sql_Abstract
      * @param   string|Tinebase_Model_Application   $_application
      * @param   array|string                        $_grant
      * @param   bool                                $_ignoreACL
+     * @param   bool                                $_andGrants
      * @return  Tinebase_Record_RecordSet set of Tinebase_Model_Container
      * @throws  Tinebase_Exception_NotFound
      */
-    public function getSharedContainer($_accountId, $_application, $_grant, $_ignoreACL = FALSE)
+    public function getSharedContainer($_accountId, $_application, $_grant, $_ignoreACL = FALSE, $_andGrants = FALSE)
     {
         $accountId   = Tinebase_Model_User::convertUserIdToInt($_accountId);
         $application = Tinebase_Application::getInstance()->getApplicationByName($_application);
@@ -774,7 +790,8 @@ class Tinebase_Container extends Tinebase_Backend_Sql_Abstract
             $accountId .
             $application->getId() .
             implode('', (array)$grant) .
-            (int)$_ignoreACL
+            (int)$_ignoreACL .
+            (int)$_andGrants
         );
         
         try {
@@ -796,7 +813,7 @@ class Tinebase_Container extends Tinebase_Backend_Sql_Abstract
             
             ->order('container.name');
         
-        $this->addGrantsSql($select, $accountId, $grant);
+        $this->addGrantsSql($select, $accountId, $grant, 'container_acl', $_andGrants, __CLASS__ . '::addGrantsSqlCallback');
         
         $stmt = $this->_db->query('/*' . __FUNCTION__ . '*/' . $select);
         
@@ -818,18 +835,37 @@ class Tinebase_Container extends Tinebase_Backend_Sql_Abstract
      * @param   string|Tinebase_Model_Application   $_application
      * @param   array|string                        $_grant
      * @param   bool                                $_ignoreACL
+     * @param   bool                                $_andGrants
      * @return  Tinebase_Record_RecordSet set of Tinebase_Model_User
      */
-    public function getOtherUsers($_accountId, $_application, $_grant, $_ignoreACL = FALSE)
+    public function getOtherUsers($_accountId, $_application, $_grant, $_ignoreACL = FALSE, $_andGrants = FALSE)
     {
-        $userIds = $this->_getOtherAccountIds($_accountId, $_application, $_grant, $_ignoreACL);
-        
+        $userIds = $this->_getOtherAccountIds($_accountId, $_application, $_grant, $_ignoreACL, $_andGrants);
+
         $users = Tinebase_User::getInstance()->getMultiple($userIds);
         $users->sort('accountDisplayName');
         
         return $users;
     }
-    
+
+    /**
+     * appends container_acl sql
+     *
+     * @param  Zend_Db_Select    $_select
+     * @param  integer           $iteration
+     * @return string table identifier to work on
+     */
+    public static function addGrantsSqlCallback($_select, $iteration)
+    {
+        $db = $_select->getAdapter();
+        $_select->join(array(
+            /* table  */ 'container_acl' . $iteration => SQL_TABLE_PREFIX . 'container_acl'),
+            /* on     */ $db->quoteIdentifier('container_acl' . $iteration . '.container_id') . ' = ' . $db->quoteIdentifier('container.id'),
+            array()
+        );
+        return 'container_acl' . $iteration;
+    }
+
     /**
      * return account ids of accounts which made personal container accessible to given account
      *
@@ -837,15 +873,16 @@ class Tinebase_Container extends Tinebase_Backend_Sql_Abstract
      * @param   string|Tinebase_Model_Application   $_application
      * @param   array|string                        $_grant
      * @param   bool                                $_ignoreACL
+     * @param   bool                                $_andGrants
      * @return  array of array of containerData
      */
-    protected function _getOtherAccountIds($_accountId, $_application, $_grant, $_ignoreACL = FALSE)
+    protected function _getOtherAccountIds($_accountId, $_application, $_grant, $_ignoreACL = FALSE, $_andGrants = FALSE)
     {
         $accountId   = Tinebase_Model_User::convertUserIdToInt($_accountId);
         $application = Tinebase_Application::getInstance()->getApplicationByName($_application);
         $grant       = $_ignoreACL ? '*' : $_grant;
 
-        $classCacheId = convertCacheId($accountId . $application->getId() . implode('', (array)$grant) .(int)$_ignoreACL);
+        $classCacheId = convertCacheId($accountId . $application->getId() . implode('', (array)$grant) .(int)$_ignoreACL . (int)$_andGrants);
         try {
             return $this->loadFromClassCache(__FUNCTION__, $classCacheId);
         } catch (Tinebase_Exception_NotFound $tenf) {
@@ -864,9 +901,9 @@ class Tinebase_Container extends Tinebase_Backend_Sql_Abstract
             ->where("{$this->_db->quoteIdentifier('container.application_id')} = ?", $application->getId())
             ->where("{$this->_db->quoteIdentifier('container.type')} = ?", Tinebase_Model_Container::TYPE_PERSONAL)
             ->where("{$this->_db->quoteIdentifier('container.is_deleted')} = ?", 0, Zend_Db::INT_TYPE);
-            
-        $this->addGrantsSql($select, $accountId, $grant);
-        
+
+        $this->addGrantsSql($select, $accountId, $grant, 'container_acl', $_andGrants, __CLASS__ . '::addGrantsSqlCallback');
+
         $stmt = $this->_db->query('/*' . __FUNCTION__ . '*/' . $select);
         $containerIds = $stmt->fetchAll(Zend_Db::FETCH_COLUMN);
         
index fd2d3ee..f41655f 100644 (file)
@@ -360,30 +360,18 @@ class Tinebase_WebDav_PrincipalBackend implements \Sabre\DAVACL\PrincipalBackend
                         }
                     }
                     
-                    // return user only, if the containers have the sync AND read grant set
-                    $otherUsersSync = Tinebase_Container::getInstance()->getOtherUsers($user, 'Calendar', Tinebase_Model_Grants::GRANT_SYNC);
-                    
-                    if ($otherUsersSync->count() > 0) {
-                        $otherUsersRead = Tinebase_Container::getInstance()->getOtherUsers($user, 'Calendar', Tinebase_Model_Grants::GRANT_READ);
-                        
-                        $otherUsersIds = array_intersect($otherUsersSync->getArrayOfIds(), $otherUsersRead->getArrayOfIds());
-                        
-                        foreach ($otherUsersIds as $userId) {
-                            if ($otherUsersSync->getById($userId)->contact_id && $otherUsersSync->getById($userId)->visibility == Tinebase_Model_User::VISIBILITY_DISPLAYED) {
-                                $result[] = self::PREFIX_USERS . '/' . $otherUsersSync->getById($userId)->contact_id . '/calendar-proxy-write';
+                    if (Tinebase_Core::getUser()->hasRight('Calendar', Tinebase_Acl_Rights::RUN)) {
+                        // return users only, if they have the sync AND read grant set
+                        $otherUsers = Tinebase_Container::getInstance()->getOtherUsers($user, 'Calendar', array(Tinebase_Model_Grants::GRANT_SYNC, Tinebase_Model_Grants::GRANT_READ), false, true);
+                        foreach ($otherUsers as $u) {
+                            if ($u->contact_id && $u->visibility == Tinebase_Model_User::VISIBILITY_DISPLAYED) {
+                                $result[] = self::PREFIX_USERS . '/' . $u->contact_id . '/calendar-proxy-write';
                             }
                         }
-                    }
-                    
-                    // return user only, if the containers have the sync AND read grant set
-                    $sharedContainersSync = Tinebase_Container::getInstance()->getSharedContainer($user, 'Calendar', Tinebase_Model_Grants::GRANT_SYNC);
-                    
-                    if ($sharedContainersSync->count() > 0) {
-                        $sharedContainersRead = Tinebase_Container::getInstance()->getSharedContainer($user, 'Calendar', Tinebase_Model_Grants::GRANT_READ);
-                        
-                        $sharedContainerIds = array_intersect($sharedContainersSync->getArrayOfIds(), $sharedContainersRead->getArrayOfIds());
-                        
-                        if (count($sharedContainerIds) > 0) {
+
+                        // return containers only, if the user has the sync AND read grant set
+                        $sharedContainers = Tinebase_Container::getInstance()->getSharedContainer($user, 'Calendar', array(Tinebase_Model_Grants::GRANT_SYNC, Tinebase_Model_Grants::GRANT_READ), false, true);
+                        if ($sharedContainers->count() > 0) {
                             $result[] = self::PREFIX_USERS . '/' . self::SHARED . '/calendar-proxy-write';
                         }
                     }