fix container_acl and tree_node_acl tables
authorPaul Mehrer <p.mehrer@metaways.de>
Wed, 12 Jul 2017 13:15:27 +0000 (15:15 +0200)
committerPaul Mehrer <p.mehrer@metaways.de>
Thu, 13 Jul 2017 11:37:45 +0000 (13:37 +0200)
no primary key on id
no proper unique key on right columns

Change-Id: Iae241bb059f3850c57fb196cc28223ac19f5dc53
Reviewed-on: http://gerrit.tine20.com/customers/5123
Tested-by: Jenkins CI (http://ci.tine20.com/)
Reviewed-by: Paul Mehrer <p.mehrer@metaways.de>
Tested-by: Paul Mehrer <p.mehrer@metaways.de>
tests/tine20/Tinebase/Timemachine/ModificationLogTest.php
tine20/Tinebase/Container.php
tine20/Tinebase/Controller/Record/Grants.php
tine20/Tinebase/Model/Grants.php
tine20/Tinebase/Record/Abstract.php
tine20/Tinebase/Record/Interface.php
tine20/Tinebase/Setup/Update/Release10.php
tine20/Tinebase/Setup/setup.xml

index c79e8ac..0df3731 100644 (file)
@@ -880,7 +880,12 @@ class Tinebase_Timemachine_ModificationLogTest extends PHPUnit_Framework_TestCas
         $testPathNode = $filesystem->stat(Tinebase_Model_Tree_Node_Path::createFromPath($fmController->addBasePath($testPath . '/subfolder'))->statpath);
         static::assertEquals($testPathNode->getId(), $testPathNode->acl_node, 'grants not set');
         Tinebase_Tree_NodeGrants::getInstance()->getGrantsForRecord($testPathNode);
-        static::assertEquals($testNodeGrants->grants, $testPathNode->grants);
+        static::assertEquals($testNodeGrants->grants->count(), $testPathNode->grants->count());
+        $oldGrants = $testNodeGrants->grants->getFirstRecord()->toArray();
+        unset($oldGrants['id']);
+        $newGrants = $testPathNode->grants->getFirstRecord()->toArray();
+        unset($newGrants['id']);
+        static::assertEquals($oldGrants, $newGrants);
 
         // unset grants
         $mod = $modifications->getFirstRecord();
index 916bffe..9d3e725 100644 (file)
@@ -293,12 +293,10 @@ class Tinebase_Container extends Tinebase_Backend_Sql_Abstract implements Tineba
         $containerGrants->addIndices(array('account_type', 'account_id'));
         $existingGrants = $containerGrants->filter('account_type', $_accountType)->filter('account_id', $_accountId)->getFirstRecord();
         
-        $id = Tinebase_Record_Abstract::generateUID();
-        
         foreach($_grants as $grant) {
             if ($existingGrants === NULL || ! $existingGrants->{$grant}) {
                 $data = array(
-                    'id'            => $id,
+                    'id'            => Tinebase_Record_Abstract::generateUID(),
                     'container_id'  => $containerId,
                     'account_type'  => $_accountType,
                     'account_id'    => $accountId,
@@ -1546,17 +1544,14 @@ class Tinebase_Container extends Tinebase_Backend_Sql_Abstract implements Tineba
             
             foreach ($_grants as $recordGrants) {
                 $data = array(
-                    'id'            => $recordGrants->getId(),
                     'container_id'  => $containerId,
                     'account_id'    => $recordGrants['account_id'],
                     'account_type'  => $recordGrants['account_type'],
                 );
-                if (empty($data['id'])) {
-                    $data['id'] = $recordGrants->generateUID();
-                }
                 
                 foreach ($recordGrants as $grantName => $grant) {
                     if (in_array($grantName, $recordGrants->getAllGrants()) && $grant === TRUE) {
+                        $data['id'] = $recordGrants->generateUID();
                         $this->_getContainerAclTable()->insert($data + array('account_grant' => $grantName));
                     }
                 }
index ef9d687..d1d07dd 100644 (file)
@@ -185,6 +185,7 @@ abstract class Tinebase_Controller_Record_Grants extends Tinebase_Controller_Rec
             foreach ($record->grants as $newGrant) {
                 foreach (call_user_func($this->_grantsModel . '::getAllGrants') as $grant) {
                     if ($newGrant->{$grant}) {
+                        $newGrant->id = null;
                         $newGrant->account_grant = $grant;
                         $newGrant->record_id = $recordId;
                         $this->_grantsBackend->create($newGrant);
index 0bad779..2ca3f3e 100644 (file)
  * 
  * @package     Tinebase
  * @subpackage  Record
- *  
+ * @property string         id
+ * @property string         record_id
+ * @property string         account_grant
+ * @property string         account_id
+ * @property string         account_type
  */
 class Tinebase_Model_Grants extends Tinebase_Record_Abstract
 {
@@ -301,4 +305,92 @@ class Tinebase_Model_Grants extends Tinebase_Record_Abstract
             'account_type'   => Tinebase_Acl_Rights::ACCOUNT_TYPE_USER,
         ), $grants)));
     }
+
+    /**
+     * @param Tinebase_Record_RecordSet $_recordSet
+     * @param Tinebase_Record_RecordSetDiff $_recordSetDiff
+     * @return bool
+     * @throws Tinebase_Exception_InvalidArgument
+     */
+    public static function applyRecordSetDiff(Tinebase_Record_RecordSet $_recordSet, Tinebase_Record_RecordSetDiff $_recordSetDiff)
+    {
+        $model = $_recordSetDiff->model;
+        if ($_recordSet->getRecordClassName() !== $model) {
+            throw new Tinebase_Exception_InvalidArgument('try to apply record set diff on a record set of different model!' .
+                'record set model: ' . $_recordSet->getRecordClassName() . ', record set diff model: ' . $model);
+        }
+
+        /** @var Tinebase_Record_Abstract $modelInstance */
+        $modelInstance = new $model(array(), true);
+        $idProperty = $modelInstance->getIdProperty();
+
+        foreach($_recordSetDiff->removed as $data) {
+            if (!isset($data[$idProperty])) {
+                throw new Tinebase_Exception_InvalidArgument('failed to apply record set diff because removed data contained bad data, id property missing (' . $idProperty . '): ' . print_r($data, true));
+            }
+            if (false !== ($record = $_recordSet->getById($data[$idProperty]))) {
+                $_recordSet->removeRecord($record);
+            }
+            $found = false;
+            /** @var Tinebase_Model_Grants $record */
+            foreach ($_recordSet as $record) {
+                if (    $record->record_id      === $data['record_id']      &&
+                        $record->account_id     === $data['account_id']     &&
+                        $record->account_type   === $data['account_type']) {
+                    $found = true;
+                    break;
+                }
+            }
+            if (true === $found) {
+                $_recordSet->removeRecord($record);
+            }
+        }
+
+        foreach($_recordSetDiff->modified as $data) {
+            $diff = new Tinebase_Record_Diff();
+            $diff->id = $data[$idProperty];
+            $diff->diff = $data;
+            if (false !== ($record = $_recordSet->getById($diff->getId()))) {
+                $record->applyDiff($diff);
+            } else {
+                $found = false;
+                /** @var Tinebase_Model_Grants $record */
+                foreach ($_recordSet as $record) {
+                    if (    $record->record_id      === $data['record_id']      &&
+                            $record->account_id     === $data['account_id']     &&
+                            $record->account_type   === $data['account_type']) {
+                        $found = true;
+                        break;
+                    }
+                }
+                if (true === $found) {
+                    $record->applyDiff($diff);
+                } else {
+                    Tinebase_Core::getLogger()->err(__METHOD__ . '::' . __LINE__
+                        . ' Did not find the record supposed to be modified with id: ' . $data[$idProperty]);
+                    throw new Tinebase_Exception_InvalidArgument('Did not find the record supposed to be modified with id: ' . $data[$idProperty]);
+                }
+            }
+        }
+
+        foreach($_recordSetDiff->added as $data) {
+            $found = false;
+            /** @var Tinebase_Model_Grants $record */
+            foreach ($_recordSet as $record) {
+                if (    $record->record_id      === $data['record_id']      &&
+                        $record->account_id     === $data['account_id']     &&
+                        $record->account_type   === $data['account_type']) {
+                    $found = true;
+                    break;
+                }
+            }
+            if (true === $found) {
+                $_recordSet->removeRecord($record);
+            }
+            $newRecord = new $model($data);
+            $_recordSet->addRecord($newRecord);
+        }
+
+        return true;
+    }
 }
index 59a7dc5..645c743 100644 (file)
@@ -1357,7 +1357,11 @@ abstract class Tinebase_Record_Abstract implements Tinebase_Record_Interface
                         is_array($this->$property)?$this->$property:array());
                 }
 
-                $this->$property->applyRecordSetDiff($recordSetDiff);
+                /** @var Tinebase_Record_Abstract $model */
+                $model = $recordSetDiff->model;
+                if (true !== $model::applyRecordSetDiff($this->$property, $recordSetDiff)) {
+                    $this->$property->applyRecordSetDiff($recordSetDiff);
+                }
             } else {
                 if (in_array($property, $this->_datetimeFields) && ! is_object($oldValue)) {
                     $oldValue = new Tinebase_DateTime($oldValue);
@@ -1368,6 +1372,16 @@ abstract class Tinebase_Record_Abstract implements Tinebase_Record_Interface
     }
 
     /**
+     * @param Tinebase_Record_RecordSet $_recordSet
+     * @param Tinebase_Record_RecordSetDiff $_recordSetDiff
+     * @return bool
+     */
+    public static function applyRecordSetDiff(Tinebase_Record_RecordSet $_recordSet, Tinebase_Record_RecordSetDiff $_recordSetDiff)
+    {
+        return false;
+    }
+
+    /**
      * returns true if this record should be replicated
      *
      * @return boolean
index b78ccf2..3bfe79a 100644 (file)
@@ -302,4 +302,11 @@ interface Tinebase_Record_Interface extends ArrayAccess, IteratorAggregate
      * @return array
      */
     public function getModlogOmitFields();
+
+    /**
+     * @param Tinebase_Record_RecordSet $_recordSet
+     * @param Tinebase_Record_RecordSetDiff $_recordSetDiff
+     * @return bool
+     */
+    public static function applyRecordSetDiff(Tinebase_Record_RecordSet $_recordSet, Tinebase_Record_RecordSetDiff $_recordSetDiff);
 }
index 3e9c87d..86cbe87 100644 (file)
@@ -1785,4 +1785,135 @@ class Tinebase_Setup_Update_Release10 extends Setup_Update_Abstract
 
         $this->setApplicationVersion('Tinebase', '10.36');
     }
+
+    /**
+     * update to 10.37
+     *
+     * add quota column to tree_nodes
+     */
+    public function update_36()
+    {
+        $result = $this->_db->select()->from(SQL_TABLE_PREFIX . 'container_acl')->query(Zend_DB::FETCH_ASSOC);
+        $quotedId = $this->_db->quoteIdentifier('id');
+        $quotedContainerId = $this->_db->quoteIdentifier('container_id');
+        $quotedAccountType = $this->_db->quoteIdentifier('account_type');
+        $quotedAccountId = $this->_db->quoteIdentifier('account_id');
+        $quotedAccountGrant = $this->_db->quoteIdentifier('account_grant');
+        foreach ($result->fetchAll() as $row) {
+            $this->_db->update(SQL_TABLE_PREFIX . 'container_acl',
+                array('id' => Tinebase_Record_Abstract::generateUID()),
+                $quotedId           . ' = ' . $this->_db->quote($row['id'])           . ' AND ' .
+                $quotedContainerId  . ' = ' . $this->_db->quote($row['container_id']) . ' AND ' .
+                $quotedAccountType  . ' = ' . $this->_db->quote($row['account_type']) . ' AND ' .
+                $quotedAccountId    . ' = ' . $this->_db->quote($row['account_id'])   . ' AND ' .
+                $quotedAccountGrant . ' = ' . $this->_db->quote($row['account_grant'])
+            );
+        }
+
+        $result = $this->_db->select()->from(SQL_TABLE_PREFIX . 'tree_node_acl')->query(Zend_DB::FETCH_ASSOC);
+        $quotedRecordId = $this->_db->quoteIdentifier('record_id');
+        foreach ($result->fetchAll() as $row) {
+            $this->_db->update(SQL_TABLE_PREFIX . 'tree_node_acl',
+                array('id' => Tinebase_Record_Abstract::generateUID()),
+                $quotedId           . ' = ' . $this->_db->quote($row['id'])           . ' AND ' .
+                $quotedRecordId     . ' = ' . $this->_db->quote($row['record_id'])    . ' AND ' .
+                $quotedAccountType  . ' = ' . $this->_db->quote($row['account_type']) . ' AND ' .
+                $quotedAccountId    . ' = ' . $this->_db->quote($row['account_id'])   . ' AND ' .
+                $quotedAccountGrant . ' = ' . $this->_db->quote($row['account_grant'])
+            );
+        }
+
+        /** @var Tinebase_Backend_Sql_Command_Interface $command */
+        $command = Tinebase_Backend_Sql_Command::factory($this->_db);
+        $result = $this->_db->select()->from(SQL_TABLE_PREFIX . 'container_acl', array($command->getAggregate('id'),
+            new Zend_Db_Expr('count(*) AS c')))
+            ->group(array('container_id', 'account_type', 'account_id', 'account_grant'))
+            ->having('c > 1')->query(Zend_DB::FETCH_NUM);
+        foreach ($result->fetchAll() as $row) {
+            $ids = explode(',', ltrim(rtrim($row[0], '}'), '{'));
+            array_pop($ids);
+            $this->_db->delete(SQL_TABLE_PREFIX . 'container_acl', $this->_db->quoteInto($quotedId . ' in (?)', $ids));
+        }
+
+        $result = $this->_db->select()->from(SQL_TABLE_PREFIX . 'tree_node_acl', array($command->getAggregate('id'),
+            new Zend_Db_Expr('count(*) AS c')))
+            ->group(array('record_id', 'account_type', 'account_id', 'account_grant'))
+            ->having('c > 1')->query(Zend_DB::FETCH_NUM);
+        foreach ($result->fetchAll() as $row) {
+            $ids = explode(',', ltrim(rtrim($row[0], '}'), '{'));
+            array_pop($ids);
+            $this->_db->delete(SQL_TABLE_PREFIX . 'tree_node_acl', $this->_db->quoteInto($quotedId . ' in (?)', $ids));
+        }
+
+        if ($this->getTableVersion('container_acl') < 5) {
+            if ($this->_db instanceof Zend_Db_Adapter_Pdo_Pgsql) {
+                $this->_backend->dropIndex('container_acl', 'container_id-account-type-account_id-account_grant');
+            } else {
+                $this->_backend->dropIndex('container_acl', 'PRIMARY');
+            }
+            $this->_backend->addIndex('container_acl', new Setup_Backend_Schema_Index_Xml('<index>
+                    <name>id</name>
+                    <primary>true</primary>
+                    <field>
+                        <name>id</name>
+                    </field>
+                </index>
+            '));
+            $this->_backend->addIndex('container_acl', new Setup_Backend_Schema_Index_Xml('<index>
+                    <name>container_id-account_type-account_id-acount_grant</name>
+                    <unique>true</unique>
+                    <field>
+                        <name>container_id</name>
+                    </field>
+                    <field>
+                        <name>account_type</name>
+                    </field>
+                    <field>
+                        <name>account_id</name>
+                    </field>
+                    <field>
+                        <name>account_grant</name>
+                    </field>
+                </index>
+            '));
+            $this->_backend->dropIndex('container_acl', 'id-account_type-account_id');
+            $this->setTableVersion('container_acl', 5);
+        }
+
+        if ($this->getTableVersion('tree_node_acl') < 2) {
+            if ($this->_db instanceof Zend_Db_Adapter_Pdo_Pgsql) {
+                $this->_backend->dropIndex('tree_node_acl', 'record_id-account-type-account_id-account_grant');
+            } else {
+                $this->_backend->dropIndex('tree_node_acl', 'PRIMARY');
+            }
+            $this->_backend->addIndex('tree_node_acl', new Setup_Backend_Schema_Index_Xml('<index>
+                    <name>id</name>
+                    <primary>true</primary>
+                    <field>
+                        <name>id</name>
+                    </field>
+                </index>
+            '));
+            $this->_backend->addIndex('tree_node_acl', new Setup_Backend_Schema_Index_Xml('<index>
+                    <name>record_id-account-type-account_id-account_grant</name>
+                    <unique>true</unique>
+                    <field>
+                        <name>record_id</name>
+                    </field>
+                    <field>
+                        <name>account_type</name>
+                    </field>
+                    <field>
+                        <name>account_id</name>
+                    </field>
+                    <field>
+                        <name>account_grant</name>
+                    </field>
+                </index>
+            '));
+            $this->_backend->dropIndex('tree_node_acl', 'id-account_type-account_id');
+            $this->setTableVersion('tree_node_acl', 2);
+        }
+        $this->setApplicationVersion('Tinebase', '10.37');
+    }
 }
index 0e25e3b..c22d2e2 100644 (file)
@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="utf-8"?>
 <application>
     <name>Tinebase</name>
-    <version>10.36</version>
+    <version>10.37</version>
     <tables>
         <table>
             <name>applications</name>
         </table>
         <table>
             <name>container_acl</name>
-            <version>4</version>
+            <version>5</version>
             <declaration>
                 <field>
                     <name>id</name>
                     <length>40</length>
                     <notnull>true</notnull>
                 </field>
-
                 <index>
-                    <name>container_id-account-type-account_id-account_grant</name>
+                    <name>id</name>
                     <primary>true</primary>
                     <field>
                         <name>id</name>
                     </field>
+                </index>
+                <index>
+                    <name>container_id-account_type-account_id-acount_grant</name>
+                    <unique>true</unique>
                     <field>
                         <name>container_id</name>
                     </field>
                     </field>
                 </index>
                 <index>
-                    <name>id-account_type-account_id</name>
-                    <field>
-                        <name>container_id</name>
-                    </field>
-                    <field>
-                        <name>account_type</name>
-                    </field>
-                    <field>
-                        <name>account_id</name>
-                    </field>
-                </index>
-                <index>
                     <name>container_acl::container_id--container::id</name>
                     <field>
                         <name>container_id</name>
         </table>
         <table>
             <name>tree_node_acl</name>
-            <version>1</version>
+            <version>2</version>
             <declaration>
                 <field>
                     <name>id</name>
                     <length>40</length>
                     <notnull>true</notnull>
                 </field>
-
                 <index>
-                    <name>record_id-account-type-account_id-account_grant</name>
+                    <name>id</name>
                     <primary>true</primary>
                     <field>
                         <name>id</name>
                     </field>
+                </index>
+                <index>
+                    <name>record_id-account-type-account_id-account_grant</name>
+                    <unique>true</unique>
                     <field>
                         <name>record_id</name>
                     </field>
                     </field>
                 </index>
                 <index>
-                    <name>id-account_type-account_id</name>
-                    <field>
-                        <name>record_id</name>
-                    </field>
-                    <field>
-                        <name>account_type</name>
-                    </field>
-                    <field>
-                        <name>account_id</name>
-                    </field>
-                </index>
-                <index>
                     <name>tree_node_acl::record_id--tree_nodes::id</name>
                     <field>
                         <name>record_id</name>