feature(Felamimail): save email user id in account xprops

- add facade functions to Tinebase_EmailUser_XpropsFacade - support account add, update and delete Change-Id: I2444c460ff4ce7b4ecc3392209f12c4fe29df9ff Reviewed-on: http://gerrit.tine20.com/customers/14871 Tested-by: Jenkins CI (http://ci.tine20.com/) <tine20-jenkins@metaways.de> Reviewed-by: Philipp Schüle <p.schuele@metaways.de>
tine20 · Dec 9, 2019 · b92ef91 · b92ef91
1 parent 4e6c21e
commit b92ef91
Show file tree

Hide file tree

Showing 11 changed files with 192 additions and 54 deletions.
diff --git a/tests/tine20/Addressbook/Controller/ListTest.php b/tests/tine20/Addressbook/Controller/ListTest.php
@@ -200,7 +200,7 @@ public function testListAsMailinglist()
         static::assertSame($list->email, $account->name);
 
         // test change email
-        $list->email = 'shoo@foo.bar';
+        $list->email = 'shoo@' . $domain;
         $this->_instance->update($list);
 
         $account = $accountCtrl->search(Tinebase_Model_Filter_FilterGroup::getFilterForModel(

diff --git a/tests/tine20/Admin/JsonTest.php b/tests/tine20/Admin/JsonTest.php
@@ -1647,9 +1647,10 @@ public function testEmailAccountApi()
         $account = $this->_testSimpleRecordApi(
             'EmailAccount', // use non-existant model to make simple api test work
             'name',
-            'email',
+            null,
             true,
-            ['type' => Felamimail_Model_Account::TYPE_SHARED, 'password' => '123', 'email' => 'a@' . TestServer::getPrimaryMailDomain()]
+            ['type' => Felamimail_Model_Account::TYPE_SHARED, 'password' => '123', 'email' => 'a@' . TestServer::getPrimaryMailDomain()],
+            false
         );
         self::assertEquals('Templates', $account['templates_folder'], print_r($account, true));
 

diff --git a/tests/tine20/Felamimail/AllTests.php b/tests/tine20/Felamimail/AllTests.php
@@ -46,6 +46,7 @@ public static function suite()
             $suite->addTestSuite(Felamimail_Controller_FolderTest::class);
             $suite->addTestSuite(Felamimail_Controller_MessageTest::class);
             $suite->addTestSuite(Felamimail_Controller_AccountTest::class);
+            $suite->addTestSuite(Felamimail_Controller_AccountTestWithXprops::class);
             $suite->addTestSuite(Felamimail_Controller_SieveTest::class);
             $suite->addTestSuite(Felamimail_Model_MessageTest::class);
             $suite->addTestSuite(Felamimail_Model_AccountTest::class);

diff --git a/tests/tine20/Felamimail/Controller/AccountTest.php b/tests/tine20/Felamimail/Controller/AccountTest.php
@@ -94,7 +94,8 @@ protected function _setCredentials($_username, $_password)
      */
     public function testDefaultAccountPreference()
     {
-        $this->assertEquals($this->_account->getId(), Tinebase_Core::getPreference('Felamimail')->{Felamimail_Preference::DEFAULTACCOUNT}, 'current account is not the default account');
+        $this->assertEquals($this->_account->getId(), Tinebase_Core::getPreference(
+            'Felamimail')->{Felamimail_Preference::DEFAULTACCOUNT}, 'current account is not the default account');
 
         $userAccount = clone($this->_account);
         unset($userAccount->id);
@@ -103,7 +104,8 @@ public function testDefaultAccountPreference()
 
         // deleting original account and check if user account is new default account
         $this->_controller->delete($this->_account->getId());
-        $this->assertEquals($userAccount->getId(), Tinebase_Core::getPreference('Felamimail')->{Felamimail_Preference::DEFAULTACCOUNT}, 'other account is not default account');
+        $this->assertEquals($userAccount->getId(), Tinebase_Core::getPreference(
+            'Felamimail')->{Felamimail_Preference::DEFAULTACCOUNT}, 'other account is not default account');
     }
 
     /**
@@ -325,6 +327,31 @@ public function testSearchMailsInSharedAccount()
         self::assertContains('aaaaaä', $messageViaGet->body);
     }
 
+    public function testUpdateSharedAccount()
+    {
+        // change email address and check if email user is updated, too
+        $this->_testNeedsTransaction();
+        $account = $this->_createSharedAccount();
+        $account->email = 'shared' . Tinebase_Record_Abstract::generateUID(10) . '@' . TestServer::getPrimaryMailDomain();
+        Felamimail_Controller_Account::getInstance()->update($account);
+        $emailUser = Tinebase_EmailUser_XpropsFacade::getEmailUserFromRecord($account);
+        $emailUserBackend = Tinebase_EmailUser::getInstance(Tinebase_Config::SMTP);
+        $userInBackend = $emailUserBackend->getRawUserById($emailUser);
+        self::assertEquals($account->email, $userInBackend['email'], 'email was not updated');
+    }
+
+    public function testDeleteSharedAccount()
+    {
+        $this->_testNeedsTransaction();
+        $account = $this->_createSharedAccount();
+        Felamimail_Controller_Account::getInstance()->delete($account->getId());
+        $emailUser = Tinebase_EmailUser_XpropsFacade::getEmailUserFromRecord($account);
+        // make sure email user is deleted, too
+        $emailUserBackend = Tinebase_EmailUser::getInstance(Tinebase_Config::IMAP);
+        $userInBackend = $emailUserBackend->getRawUserById($emailUser);
+        self::assertFalse($userInBackend, 'user should be deleted from backend');
+    }
+
     public function testChangeAccountFromByUserUpdate()
     {
         // change name of user
@@ -426,4 +453,20 @@ public function testCreateNewUserAccountWithINBOX()
         self::assertEquals(5, $folders['totalcount'], 'should find 5 initial folders. got: '
             . print_r($folders, true));
     }
+
+    public function testConvertAccountsToSaveUserIdInXprops()
+    {
+        // switch xprops in user off
+        Tinebase_Config::getInstance()->{Tinebase_Config::EMAIL_USER_ID_IN_XPROPS} = false;
+
+        $account = $this->_createSharedAccount();
+
+        Felamimail_Controller_Account::getInstance()->convertAccountsToSaveUserIdInXprops();
+
+        $convertedAccount = Felamimail_Controller_Account::getInstance()->get($account->getId());
+        self::assertNotEmpty($convertedAccount->xprops[Felamimail_Model_Account::XPROP_EMAIL_USERID_IMAP],
+            'XPROP_EMAIL_USERID_IMAP empty ' . print_r($convertedAccount->toArray(), true));
+        self::assertNotEmpty($convertedAccount->xprops[Felamimail_Model_Account::XPROP_EMAIL_USERID_SMTP],
+            'XPROP_EMAIL_USERID_SMTP empty ' . print_r($convertedAccount->toArray(), true));
+    }
 }
diff --git a/tests/tine20/Felamimail/Controller/AccountTestWithXprops.php b/tests/tine20/Felamimail/Controller/AccountTestWithXprops.php
@@ -0,0 +1,29 @@
+<?php
+/**
+ * Tine 2.0 - http://www.tine20.org
+ * 
+ * @package     Felamimail
+ * @license     http://www.gnu.org/licenses/agpl.html
+ * @copyright   Copyright (c) 2009-2019 Metaways Infosystems GmbH (http://www.metaways.de)
+ * @author      Philipp Schüle <p.schuele@metaways.de>
+ * 
+ */
+
+/**
+ * Test class for Felamimail_Controller_Account with enabled
+ */
+class Felamimail_Controller_AccountTestWithXprops extends Felamimail_Controller_AccountTest
+{
+    /**
+     * Sets up the fixture.
+     * This method is called before a test is executed.
+     *
+     * @access protected
+     */
+    protected function setUp()
+    {
+        parent::setUp();
+
+        Tinebase_Config::getInstance()->{Tinebase_Config::EMAIL_USER_ID_IN_XPROPS} = true;
+    }
+}
diff --git a/tests/tine20/Felamimail/Controller/SieveTest.php b/tests/tine20/Felamimail/Controller/SieveTest.php
@@ -24,6 +24,7 @@ public function testAdbMailinglistPutSieveRule()
         $this->_testNeedsTransaction();
 
         $mailinglist = $this->_createMailinglist();
+        self::assertInstanceOf(Addressbook_Model_List::class, $mailinglist);
         $result = Felamimail_Sieve_AdbList::setScriptForList($mailinglist);
         self::assertTrue($result);
     }

diff --git a/tine20/Felamimail/Controller/Account.php b/tine20/Felamimail/Controller/Account.php
@@ -276,17 +276,8 @@ protected function _createSharedEmailUserAndCredentials($_record)
         if (! $_record->email) {
             throw new Tinebase_Exception_UnexpectedValue($translation->_('shared / adb_list accounts need to have an email set'));
         }
-        $emailUserBackend = Tinebase_EmailUser::getInstance(Tinebase_Config::IMAP);
-        $userId = $_record->user_id ?: ($_record->user_id = Tinebase_Record_Abstract::generateUID());
-        $_record->user = $emailUserBackend->getLoginName($userId, $_record->email, $_record->email);
-
-        Felamimail_Controller_Account::getInstance()->addSystemAccountConfigValues($_record);
-
-        $user = $this->_getEmailUserFromAccount($_record);
-        $this->_checkIfEmailUserExists($user, $emailUserBackend);
 
-        $emailUserBackend->inspectAddUser($user, $user);
-        Tinebase_EmailUser::getInstance(Tinebase_Config::SMTP)->inspectAddUser($user, $user);
+        $this->_createSharedEmailUser($_record);
 
         $_record->credentials_id = $this->_createSharedCredentials($_record->user, $_record->password);
         if ($_record->smtp_user && $_record->smtp_password) {
@@ -299,6 +290,36 @@ protected function _createSharedEmailUserAndCredentials($_record)
         }
     }
 
+    /**
+     * @param Felamimail_Model_Account $_record
+     * @throws Tinebase_Exception_SystemGeneric
+     */
+    protected function _createSharedEmailUser($_record)
+    {
+        $userId = $_record->user_id ? $_record->user_id : Tinebase_Record_Abstract::generateUID();
+        if (Tinebase_Config::getInstance()->{Tinebase_Config::EMAIL_USER_ID_IN_XPROPS}) {
+            Tinebase_EmailUser_XpropsFacade::setXprops($_record, $userId);
+            $_record->user_id = null;
+        } else {
+            $_record->user_id = $userId;
+        }
+
+        $emailUserBackend = Tinebase_EmailUser::getInstance(Tinebase_Config::IMAP);
+        $_record->user = $emailUserBackend->getLoginName($userId, $_record->email, $_record->email);
+
+        Felamimail_Controller_Account::getInstance()->addSystemAccountConfigValues($_record);
+
+        $user = Tinebase_EmailUser_XpropsFacade::getEmailUserFromRecord($_record);
+        $this->_checkIfEmailUserExists($user, $emailUserBackend);
+
+        $emailUserBackend->inspectAddUser($user, $user);
+        Tinebase_EmailUser::getInstance(Tinebase_Config::SMTP)->inspectAddUser($user, $user);
+    }
+
+    /**
+     * @param Felamimail_Model_Account $_record
+     * @throws Tinebase_Exception_NotImplemented
+     */
     protected function _createUserInternalEmailUser($_record)
     {
         $translation = Tinebase_Translation::getTranslation($this->_applicationName);
@@ -348,10 +369,7 @@ protected function _deleteLinkedObjects(Tinebase_Record_Interface $_record)
                 Felamimail_Model_Account::TYPE_SHARED || $_record->type ===
                 Felamimail_Model_Account::TYPE_USER_INTERNAL) {
             $_record->resolveCredentials(false);
-            $user = new Tinebase_Model_FullUser([], true);
-            $user->setId($_record->user_id);
-            Tinebase_EmailUser::getInstance(Tinebase_Config::IMAP)->inspectDeleteUser($user);
-            Tinebase_EmailUser::getInstance(Tinebase_Config::SMTP)->inspectDeleteUser($user);
+            Tinebase_EmailUser_XpropsFacade::deleteEmailUsers($_record);
         }
     }
 
@@ -446,21 +464,6 @@ protected function _inspectAfterCreate($_createdRecord, Tinebase_Record_Interfac
         }
     }
 
-    protected function _getEmailUserFromAccount($account, $setUserId = true)
-    {
-        $user = new Tinebase_Model_FullUser([
-            'accountLoginName' => $account->email,
-            'accountEmailAddress' => $account->email,
-        ], true);
-        $emailData = $account->password ? ['emailPassword' => $account->password] : [];
-        $user->imapUser = new Tinebase_Model_EmailUser($emailData);
-        $user->smtpUser = new Tinebase_Model_EmailUser($emailData);
-        if ($setUserId) {
-            $user->setId($account->user_id);
-        }
-        return $user;
-    }
-
     protected function _checkIfEmailUserExists($user, $emailUserBackend = null)
     {
         $emailUserBackend = $emailUserBackend ? $emailUserBackend : Tinebase_EmailUser::getInstance(Tinebase_Config::IMAP);
@@ -507,8 +510,10 @@ protected function _beforeUpdateSharedAccount($_record, $_oldRecord)
         }
 
         if ($_oldRecord->email !== $_record->email) {
-            $user = $this->_getEmailUserFromAccount($_record, false);
+            // check only with email address!
+            $user = Tinebase_EmailUser_XpropsFacade::getEmailUserFromRecord($_record, [], false);
             $this->_checkIfEmailUserExists($user);
+            Tinebase_EmailUser_XpropsFacade::updateEmailUsers($_record);
         }
     }
 
@@ -1733,14 +1738,28 @@ public function approveMigration($accountId)
      */
     public function convertAccountsToSaveUserIdInXprops()
     {
-        $this->doContainerACLChecks(false);
+        $checks = $this->doContainerACLChecks(false);
         // get all shared email accounts
         $sharedAccounts = $this->search(Tinebase_Model_Filter_FilterGroup::getFilterForModel(
             Felamimail_Model_Account::class, [
-            ['field' => 'type', 'operator' => 'equals', Felamimail_Model_Account::TYPE_SHARED]
+            ['field' => 'type', 'operator' => 'equals', 'value' => Felamimail_Model_Account::TYPE_SHARED]
         ]));
+        $this->doContainerACLChecks($checks);
 
-        $xpropsFacade = new Tinebase_EmailUser_XpropsFacade();
-        $xpropsFacade->convertExistingUsers($sharedAccounts, $this);
+        // TODO needed?
+        // $xpropsFacade = new Tinebase_EmailUser_XpropsFacade();
+        // $xpropsFacade->convertExistingUsers($sharedAccounts, $this);
+
+        foreach ($sharedAccounts as $account) {
+//            $emailUserBackend = Tinebase_EmailUser::getInstance(Tinebase_Config::IMAP);
+//            $userId = $_record->user_id;
+//            $_record->user = $emailUserBackend->getLoginName($userId, $_record->email, $_record->email);
+
+            // save in record
+            $account->xprops()[Felamimail_Model_Account::XPROP_EMAIL_USERID_IMAP] = $account->user_id;
+            $account->xprops()[Felamimail_Model_Account::XPROP_EMAIL_USERID_SMTP] = $account->user_id;
+
+            $this->_backend->update($account);
+        }
     }
 }
diff --git a/tine20/Tinebase/EmailUser/Smtp/Postfix.php b/tine20/Tinebase/EmailUser/Smtp/Postfix.php
@@ -615,8 +615,6 @@ protected function _recordToRawData(Tinebase_Model_FullUser $_user, Tinebase_Mod
     {
         $rawData = array();
 
-        if (Tinebase_Core::isLogLevel(Zend_Log::TRACE)) Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__ . ' ' . print_r($_newUserProperties->toArray(), true));
-
         if (isset($_newUserProperties->smtpUser)) {
             foreach ($_newUserProperties->smtpUser as $key => $value) {
                 $property = (isset($this->_propertyMapping[$key]) || array_key_exists($key, $this->_propertyMapping)) ? $this->_propertyMapping[$key] : false;

diff --git a/tine20/Tinebase/EmailUser/XpropsFacade.php b/tine20/Tinebase/EmailUser/XpropsFacade.php
@@ -23,18 +23,62 @@ class Tinebase_EmailUser_XpropsFacade
     const XPROP_EMAIL_USERID_IMAP = 'emailUserIdImap';
     const XPROP_EMAIL_USERID_SMTP = 'emailUserIdSmtp';
 
-    /**
-     * @param $records
-     * @param $controller
-     *
-     * TODO finish implementation
-     */
-    public function convertExistingUsers($records, $controller)
+    public static function getEmailUserFromRecord($record, $propertyConfig = [], $setUserId = true)
+    {
+        $emailUserProperties = [
+            'email' => null,
+            'password' => null,
+            'user_id' => null,
+        ];
+
+        foreach ($emailUserProperties as $property => &$value) {
+            if ($property === 'user_id' && Tinebase_Config::getInstance()->{Tinebase_Config::EMAIL_USER_ID_IN_XPROPS}) {
+                $value = $record->xprops()[self::XPROP_EMAIL_USERID_IMAP];
+            } else {
+                $value = isset($propertyConfig[$property])
+                    ? ($record->has($propertyConfig[$property]) ? $record->{$propertyConfig[$property]} : null)
+                    : ($record->has($property) ? $record->{$property} : null);
+            }
+        }
+
+        $user = new Tinebase_Model_FullUser([
+            'accountLoginName' => $emailUserProperties['email'],
+            'accountEmailAddress' => $emailUserProperties['email'],
+        ], true);
+
+        $emailData = $emailUserProperties['password'] ? [
+            'emailPassword' => $emailUserProperties['password']
+        ] : [];
+        $user->imapUser = new Tinebase_Model_EmailUser($emailData);
+        $user->smtpUser = new Tinebase_Model_EmailUser($emailData);
+
+        if ($setUserId) {
+            $user->setId($emailUserProperties['user_id']);
+        }
+        return $user;
+    }
+
+    public static function setXprops($record, $userId = null)
     {
-        foreach ($records as $record) {
-            //-- get imap user
-            //-- get smtp user
-            //-- save in record
+        if (! $userId) {
+            $userId = Tinebase_Record_Abstract::generateUID();
         }
+
+        $record->xprops()[Felamimail_Model_Account::XPROP_EMAIL_USERID_IMAP] = $userId;
+        $record->xprops()[Felamimail_Model_Account::XPROP_EMAIL_USERID_SMTP] = $userId;
+    }
+
+    public static function deleteEmailUsers($record)
+    {
+        $user = self::getEmailUserFromRecord($record);
+        Tinebase_EmailUser::getInstance(Tinebase_Config::IMAP)->inspectDeleteUser($user);
+        Tinebase_EmailUser::getInstance(Tinebase_Config::SMTP)->inspectDeleteUser($user);
+    }
+
+    public static function updateEmailUsers($record)
+    {
+        $user = self::getEmailUserFromRecord($record);
+        Tinebase_EmailUser::getInstance(Tinebase_Config::IMAP)->inspectUpdateUser($user, $user);
+        Tinebase_EmailUser::getInstance(Tinebase_Config::SMTP)->inspectUpdateUser($user, $user);
     }
 }
diff --git a/tine20/Tinebase/Frontend/Cli.php b/tine20/Tinebase/Frontend/Cli.php
@@ -1920,14 +1920,14 @@ public function emailUserIdInXprops()
     {
         $this->_checkAdminRight();
 
-        // convert fmail accounts
         if (Tinebase_Application::getInstance()->isInstalled('Felamimail')) {
             Felamimail_Controller_Account::getInstance()->convertAccountsToSaveUserIdInXprops();
         }
 
         // TODO convert users
         // TODO convert lists
 
-        // TODO activate config
+        // activate config
+        Tinebase_Config::getInstance()->{Tinebase_Config::EMAIL_USER_ID_IN_XPROPS} = true;
     }
 }
diff --git a/tine20/Tinebase/User/Plugin/Abstract.php b/tine20/Tinebase/User/Plugin/Abstract.php
@@ -228,6 +228,7 @@ abstract protected function _updateUser(Tinebase_Model_FullUser $_updatedUser, T
      * check if user exists already in plugin user table
      *
      * @param Tinebase_Model_FullUser $_user
+     * @return boolean
      */
     public function userExists(Tinebase_Model_FullUser $_user)
     {
@@ -238,6 +239,7 @@ public function userExists(Tinebase_Model_FullUser $_user)
      * check if user exists already in plugin user table
      * 
      * @param Tinebase_Model_FullUser $_user
+     * @return boolean
      */
     abstract protected function _userExists(Tinebase_Model_FullUser $_user);