From cd4099cf4b670f9d575c92abf6cb8e2ffbae88cc Mon Sep 17 00:00:00 2001 From: yuri Date: Thu, 15 Nov 2018 13:21:39 +0200 Subject: [PATCH] email import parallel fixes --- application/Espo/Core/Mail/Importer.php | 41 +++- .../Espo/Core/ORM/Repositories/RDB.php | 207 ++++++++++-------- application/Espo/Repositories/Email.php | 96 ++++---- tests/unit/Espo/Core/Mail/ImporterTest.php | 2 + 4 files changed, 195 insertions(+), 151 deletions(-) diff --git a/application/Espo/Core/Mail/Importer.php b/application/Espo/Core/Mail/Importer.php index 0d12f6a78f..6432de3e87 100644 --- a/application/Espo/Core/Mail/Importer.php +++ b/application/Espo/Core/Mail/Importer.php @@ -154,7 +154,6 @@ class Importer } if ($duplicate = $this->findDuplicate($email)) { - $duplicate = $this->getEntityManager()->getEntity('Email', $duplicate->id); $this->processDuplicate($duplicate, $assignedUserId, $userIdList, $folderData, $teamsIdList); return $duplicate; } @@ -284,7 +283,6 @@ class Importer if ($duplicate = $this->findDuplicate($email)) { $this->getEntityManager()->getPdo()->query('UNLOCK TABLES'); - $duplicate = $this->getEntityManager()->getEntity('Email', $duplicate->id); $this->processDuplicate($duplicate, $assignedUserId, $userIdList, $folderData, $teamsIdList); return $duplicate; } @@ -317,13 +315,15 @@ class Importer } } - $this->getEntityManager()->saveEntity($email); + $this->getEntityManager()->saveEntity($email, [ + 'isBeingImported' => true + ]); foreach ($inlineAttachmentList as $attachment) { - $attachment->set(array( + $attachment->set([ 'relatedId' => $email->id, 'relatedType' => 'Email' - )); + ]); $this->getEntityManager()->saveEntity($attachment); } @@ -370,9 +370,9 @@ class Importer protected function findDuplicate(Entity $email) { if ($email->get('messageId')) { - $duplicate = $this->getEntityManager()->getRepository('Email')->select(['id'])->where(array( + $duplicate = $this->getEntityManager()->getRepository('Email')->select(['id'])->where([ 'messageId' => $email->get('messageId') - ))->findOne(['skipAdditionalSelectParams' => true]); + ])->findOne(['skipAdditionalSelectParams' => true]); if ($duplicate) { return $duplicate; } @@ -381,25 +381,44 @@ class Importer protected function processDuplicate(Entity $duplicate, $assignedUserId, $userIdList, $folderData, $teamsIdList) { + $duplicate->loadLinkMultipleField('users'); + $fetchedUserIdList = $duplicate->getLinkMultipleIdList('users'); + $duplicate->setLinkMultipleIdList('users', []); + if ($assignedUserId) { - $duplicate->addLinkMultipleId('users', $assignedUserId); + if (!in_array($assignedUserId, $fetchedUserIdList)) { + $duplicate->addLinkMultipleId('users', $assignedUserId); + } $duplicate->addLinkMultipleId('assignedUsers', $assignedUserId); } if (!empty($userIdList)) { foreach ($userIdList as $uId) { - $duplicate->addLinkMultipleId('users', $uId); + if (!in_array($uId, $fetchedUserIdList)) { + $duplicate->addLinkMultipleId('users', $uId); + } } } if ($folderData) { foreach ($folderData as $uId => $folderId) { - $duplicate->setLinkMultipleColumn('users', 'folderId', $uId, $folderId); + if (!in_array($uId, $fetchedUserIdList)) { + $duplicate->setLinkMultipleColumn('users', 'folderId', $uId, $folderId); + } else { + $this->getEntityManager()->getRepository()->updateRelation($duplicate, 'users', $uId, [ + 'folderId' => $folderId + ]); + } } } $duplicate->set('isBeingImported', true); - $this->getEntityManager()->saveEntity($duplicate); + $this->getEntityManager()->saveEntity($duplicate, [ + 'skipLinkMultipleRemove' => true, + 'skipLinkMultipleUpdate' => true, + 'isBeingImported' => true, + 'isDuplicate' => true + ]); if (!empty($teamsIdList)) { foreach ($teamsIdList as $teamId) { diff --git a/application/Espo/Core/ORM/Repositories/RDB.php b/application/Espo/Core/ORM/Repositories/RDB.php index 566a05994c..c716508fa9 100644 --- a/application/Espo/Core/ORM/Repositories/RDB.php +++ b/application/Espo/Core/ORM/Repositories/RDB.php @@ -256,7 +256,7 @@ class RDB extends \Espo\ORM\Repositories\RDB implements Injectable } } - protected function afterSave(Entity $entity, array $options = array()) + protected function afterSave(Entity $entity, array $options = []) { if (!empty($this->restoreData)) { $entity->set($this->restoreData); @@ -267,7 +267,7 @@ class RDB extends \Espo\ORM\Repositories\RDB implements Injectable if (!$this->processFieldsAfterSaveDisabled) { $this->processEmailAddressSave($entity); $this->processPhoneNumberSave($entity); - $this->processSpecifiedRelationsSave($entity); + $this->processSpecifiedRelationsSave($entity, $options); $this->processFileFieldsSave($entity); $this->processArrayFieldsSave($entity); $this->processWysiwygFieldsSave($entity); @@ -454,112 +454,135 @@ class RDB extends \Espo\ORM\Repositories\RDB implements Injectable } } - protected function processSpecifiedRelationsSave(Entity $entity) + protected function processSpecifiedRelationsSave(Entity $entity, array $options = []) { $relationTypeList = [$entity::HAS_MANY, $entity::MANY_MANY, $entity::HAS_CHILDREN]; foreach ($entity->getRelations() as $name => $defs) { if (in_array($defs['type'], $relationTypeList)) { - $fieldName = $name . 'Ids'; - $columnsFieldsName = $name . 'Columns'; + $idListAttribute = $name . 'Ids'; + $columnsAttribute = $name . 'Columns'; - - if ($entity->has($fieldName) || $entity->has($columnsFieldsName)) { + if ($entity->has($idListAttribute) || $entity->has($columnsAttribute)) { if ($this->getMetadata()->get("entityDefs." . $entity->getEntityType() . ".fields.{$name}.noSave")) { continue; } - if ($entity->has($fieldName)) { - $specifiedIds = $entity->get($fieldName); + $skipCreate = false; + $skipRemove = false; + $skipUpdate = false; + if (!empty($options['skipLinkMultipleCreate'])) $skipCreate = true; + if (!empty($options['skipLinkMultipleRemove'])) $skipRemove = true; + if (!empty($options['skipLinkMultipleUpdate'])) $skipUpdate = true; + + if ($entity->isNew()) { + $skipRemove = true; + } + + if ($entity->has($idListAttribute)) { + $specifiedIdList = $entity->get($idListAttribute); + } else if ($entity->has($columnsAttribute)) { + $skipRemove = true; + $specifiedIdList = []; + foreach ($entity->get($columnsAttribute) as $id => $d) { + $specifiedIdList[] = $id; + } } else { - $specifiedIds = array(); - foreach ($entity->get($columnsFieldsName) as $id => $d) { - $specifiedIds[] = $id; + continue; + } + + if (!is_array($specifiedIdList)) continue; + + $toRemoveIdList = []; + $existingIdList = []; + $toUpdateIdList = []; + $toCreateIdList = []; + $existingColumnsData = (object)[]; + + $defs = []; + $columns = $this->getMetadata()->get("entityDefs." . $entity->getEntityType() . ".fields.{$name}.columns"); + if (!empty($columns)) { + $columnData = $entity->get($columnsAttribute); + $defs['additionalColumns'] = $columns; + } + + $foreignEntityList = $entity->get($name, $defs); + if ($foreignEntityList) { + foreach ($foreignEntityList as $foreignEntity) { + $existingIdList[] = $foreignEntity->id; + if (!empty($columns)) { + $data = (object)[]; + foreach ($columns as $columnName => $columnField) { + $foreignId = $foreignEntity->id; + $data->$columnName = $foreignEntity->get($columnField); + } + $existingColumnsData->$foreignId = $data; + if (!$entity->isNew()) { + $entity->setFetched($columnsAttribute, $existingColumnsData); + } + } } } - if (is_array($specifiedIds)) { - $toRemoveIds = array(); - $existingIds = array(); - $toUpdateIds = array(); - $existingColumnsData = new \stdClass(); - $defs = array(); - $columns = $this->getMetadata()->get("entityDefs." . $entity->getEntityType() . ".fields.{$name}.columns"); - if (!empty($columns)) { - $columnData = $entity->get($columnsFieldsName); - $defs['additionalColumns'] = $columns; + if (!$entity->isNew()) { + if ($entity->has($idListAttribute) && !$entity->hasFetched($idListAttribute)) { + $entity->setFetched($idListAttribute, $existingIdList); } - - $foreignCollection = $entity->get($name, $defs); - if ($foreignCollection) { - foreach ($foreignCollection as $foreignEntity) { - $existingIds[] = $foreignEntity->id; - if (!empty($columns)) { - $data = new \stdClass(); - foreach ($columns as $columnName => $columnField) { - $foreignId = $foreignEntity->id; - $data->$columnName = $foreignEntity->get($columnField); - } - $existingColumnsData->$foreignId = $data; - if (!$entity->isNew()) { - $entity->setFetched($columnsFieldsName, $existingColumnsData); - } - } - - } + if ($entity->has($columnsAttribute) && !empty($columns)) { + $entity->setFetched($columnsAttribute, $existingColumnsData); } + } - if (!$entity->isNew()) { - if ($entity->has($fieldName)) { - $entity->setFetched($fieldName, $existingIds); + foreach ($existingIdList as $id) { + if (!in_array($id, $specifiedIdList)) { + if (!$skipRemove) { + $toRemoveIdList[] = $id; } - if ($entity->has($columnsFieldsName) && !empty($columns)) { - $entity->setFetched($columnsFieldsName, $existingColumnsData); - } - } - - foreach ($existingIds as $id) { - if (!in_array($id, $specifiedIds)) { - $toRemoveIds[] = $id; - } else { - if (!empty($columns)) { - foreach ($columns as $columnName => $columnField) { - if (isset($columnData->$id) && is_object($columnData->$id)) { - if ( - property_exists($columnData->$id, $columnName) - && - ( - !property_exists($existingColumnsData->$id, $columnName) - || - $columnData->$id->$columnName !== $existingColumnsData->$id->$columnName - ) - ) { - $toUpdateIds[] = $id; - } + } else { + if (!$skipUpdate && !empty($columns)) { + foreach ($columns as $columnName => $columnField) { + if (isset($columnData->$id) && is_object($columnData->$id)) { + if ( + property_exists($columnData->$id, $columnName) + && + ( + !property_exists($existingColumnsData->$id, $columnName) + || + $columnData->$id->$columnName !== $existingColumnsData->$id->$columnName + ) + ) { + $toUpdateIdList[] = $id; } } } } } + } - foreach ($specifiedIds as $id) { - if (!in_array($id, $existingIds)) { - $data = null; - if (!empty($columns) && isset($columnData->$id)) { - $data = $columnData->$id; - } - $this->relate($entity, $name, $id, $data); - } - } - foreach ($toRemoveIds as $id) { - $this->unrelate($entity, $name, $id); - } - if (!empty($columns)) { - foreach ($toUpdateIds as $id) { - $data = $columnData->$id; - $this->updateRelation($entity, $name, $id, $data); + if (!$skipCreate) { + foreach ($specifiedIdList as $id) { + if (!in_array($id, $existingIdList)) { + $toCreateIdList[] = $id; } } } + + foreach ($toCreateIdList as $id) { + $data = null; + if (!empty($columns) && isset($columnData->$id)) { + $data = $columnData->$id; + } + $this->relate($entity, $name, $id, $data); + } + + foreach ($toRemoveIdList as $id) { + $this->unrelate($entity, $name, $id); + } + + foreach ($toUpdateIdList as $id) { + $data = $columnData->$id; + $this->updateRelation($entity, $name, $id, $data); + } + } } else if ($defs['type'] === $entity::HAS_ONE) { if (empty($defs['entity']) || empty($defs['foreignKey'])) continue; @@ -570,39 +593,37 @@ class RDB extends \Espo\ORM\Repositories\RDB implements Injectable $foreignEntityType = $defs['entity']; $foreignKey = $defs['foreignKey']; - $idFieldName = $name . 'Id'; - $nameFieldName = $name . 'Name'; + $idAttribute = $name . 'Id'; - if (!$entity->has($idFieldName)) continue; + if (!$entity->has($idAttribute)) continue; - $where = array(); + $where = []; $where[$foreignKey] = $entity->id; $previousForeignEntity = $this->getEntityManager()->getRepository($foreignEntityType)->where($where)->findOne(); if ($previousForeignEntity) { if (!$entity->isNew()) { - $entity->setFetched($idFieldName, $previousForeignEntity->id); + $entity->setFetched($idAttribute, $previousForeignEntity->id); } - if ($previousForeignEntity->id !== $entity->get($idFieldName)) { + if ($previousForeignEntity->id !== $entity->get($idAttribute)) { $previousForeignEntity->set($foreignKey, null); $this->getEntityManager()->saveEntity($previousForeignEntity); } } else { if (!$entity->isNew()) { - $entity->setFetched($idFieldName, null); + $entity->setFetched($idAttribute, null); } } - if ($entity->get($idFieldName)) { - $newForeignEntity = $this->getEntityManager()->getEntity($foreignEntityType, $entity->get($idFieldName)); + if ($entity->get($idAttribute)) { + $newForeignEntity = $this->getEntityManager()->getEntity($foreignEntityType, $entity->get($idAttribute)); if ($newForeignEntity) { $newForeignEntity->set($foreignKey, $entity->id); $this->getEntityManager()->saveEntity($newForeignEntity); } else { - $entity->set($idFieldName, null); + $entity->set($idAttribute, null); } } } } } } - diff --git a/application/Espo/Repositories/Email.php b/application/Espo/Repositories/Email.php index f171307d17..ffde900050 100644 --- a/application/Espo/Repositories/Email.php +++ b/application/Espo/Repositories/Email.php @@ -208,7 +208,7 @@ class Email extends \Espo\Core\ORM\Repositories\RDB $entity->set('idHash', $idHash); } - protected function beforeSave(Entity $entity, array $options = array()) + protected function beforeSave(Entity $entity, array $options = []) { if ($entity->isNew() && !$entity->get('messageId')) { $entity->setDummyMessageId(); @@ -223,47 +223,49 @@ class Email extends \Espo\Core\ORM\Repositories\RDB } } - if ($entity->has('from') || $entity->has('to') || $entity->has('cc') || $entity->has('bcc') || $entity->has('replyTo')) { - if (!$entity->has('usersIds')) { - $entity->loadLinkMultipleField('users'); - } + if (empty($options['isDuplicate'])) { + if ($entity->has('from') || $entity->has('to') || $entity->has('cc') || $entity->has('bcc') || $entity->has('replyTo')) { + if (!$entity->has('usersIds')) { + $entity->loadLinkMultipleField('users'); + } - if ($entity->has('from')) { - $from = trim($entity->get('from')); - if (!empty($from)) { - $ids = $eaRepository->getIds(array($from)); - if (!empty($ids)) { - $entity->set('fromEmailAddressId', $ids[0]); - $this->addUserByEmailAddressId($entity, $ids[0], true); + if ($entity->has('from')) { + $from = trim($entity->get('from')); + if (!empty($from)) { + $ids = $eaRepository->getIds([$from]); + if (!empty($ids)) { + $entity->set('fromEmailAddressId', $ids[0]); + $this->addUserByEmailAddressId($entity, $ids[0], true); - if (!$entity->get('sentById')) { - $user = $this->getEntityManager()->getRepository('EmailAddress')->getEntityByAddressId($entity->get('fromEmailAddressId'), 'User', true); - if ($user) { - $entity->set('sentById', $user->id); + if (!$entity->get('sentById')) { + $user = $this->getEntityManager()->getRepository('EmailAddress')->getEntityByAddressId($entity->get('fromEmailAddressId'), 'User', true); + if ($user) { + $entity->set('sentById', $user->id); + } } } + } else { + $entity->set('fromEmailAddressId', null); } - } else { - $entity->set('fromEmailAddressId', null); } - } - if ($entity->has('to')) { - $this->prepareAddressess($entity, 'to', true); - } - if ($entity->has('cc')) { - $this->prepareAddressess($entity, 'cc'); - } - if ($entity->has('bcc')) { - $this->prepareAddressess($entity, 'bcc'); - } - if ($entity->has('replyTo')) { - $this->prepareAddressess($entity, 'replyTo'); - } + if ($entity->has('to')) { + $this->prepareAddressess($entity, 'to', true); + } + if ($entity->has('cc')) { + $this->prepareAddressess($entity, 'cc'); + } + if ($entity->has('bcc')) { + $this->prepareAddressess($entity, 'bcc'); + } + if ($entity->has('replyTo')) { + $this->prepareAddressess($entity, 'replyTo'); + } - $assignedUserId = $entity->get('assignedUserId'); - if ($assignedUserId) { - $entity->addLinkMultipleId('users', $assignedUserId); + $assignedUserId = $entity->get('assignedUserId'); + if ($assignedUserId) { + $entity->addLinkMultipleId('users', $assignedUserId); + } } } @@ -300,12 +302,14 @@ class Email extends \Espo\Core\ORM\Repositories\RDB } } - if ($entity->get('isBeingImported')) { - if (!$entity->has('from')) { - $this->loadFromField($entity); - } - if (!$entity->has('to')) { - $this->loadToField($entity); + if (!empty($options['isBeingImported'])) { + if (empty($options['isDuplicate'])) { + if (!$entity->has('from')) { + $this->loadFromField($entity); + } + if (!$entity->has('to')) { + $this->loadToField($entity); + } } foreach ($entity->getLinkMultipleIdList('users') as $userId) { $filter = $this->getEmailFilterManager()->getMatchingFilter($entity, $userId); @@ -324,7 +328,7 @@ class Email extends \Espo\Core\ORM\Repositories\RDB } } - protected function afterSave(Entity $entity, array $options = array()) + protected function afterSave(Entity $entity, array $options = []) { parent::afterSave($entity, $options); if (!$entity->isNew()) { @@ -333,10 +337,10 @@ class Email extends \Espo\Core\ORM\Repositories\RDB foreach ($replyList as $reply) { if ($reply->id === $entity->id) continue; if (!$reply->get('parentId')) { - $reply->set(array( + $reply->set([ 'parentId' => $entity->get('parentId'), - 'parentType' => $entity->get('parentType'), - )); + 'parentType' => $entity->get('parentType') + ]); $this->getEntityManager()->saveEntity($reply); } } @@ -352,7 +356,7 @@ class Email extends \Espo\Core\ORM\Repositories\RDB $replied = $this->getEntityManager()->getEntity('Email', $entity->get('repliedId')); if ($replied && $replied->id !== $entity->id && !$replied->get('isReplied')) { $replied->set('isReplied', true); - $this->getEntityManager()->saveEntity($replied, array('silent' => true)); + $this->getEntityManager()->saveEntity($replied, ['silent' => true]); } } } @@ -366,6 +370,4 @@ class Email extends \Espo\Core\ORM\Repositories\RDB { return $this->getInjection('emailFilterManager'); } - } - diff --git a/tests/unit/Espo/Core/Mail/ImporterTest.php b/tests/unit/Espo/Core/Mail/ImporterTest.php index 936bff1ee2..3e634eed5b 100644 --- a/tests/unit/Espo/Core/Mail/ImporterTest.php +++ b/tests/unit/Espo/Core/Mail/ImporterTest.php @@ -36,6 +36,8 @@ class ImporterTest extends \PHPUnit\Framework\TestCase { function setUp() { + $GLOBALS['log'] = $this->getMockBuilder('\\Espo\\Core\\Utils\\Log')->disableOriginalConstructor()->getMock(); + $entityManager = $this->entityManager = $this->getMockBuilder('\\Espo\\Core\\ORM\\EntityManager')->disableOriginalConstructor()->getMock(); $config = $this->config = $this->getMockBuilder('\\Espo\\Core\\Utils\\Config')->disableOriginalConstructor()->getMock();