From c92a4bcf36bccdbf5bfcdcb32bddb17612a2a4bf Mon Sep 17 00:00:00 2001 From: Yuri Kuznetsov Date: Mon, 3 Aug 2020 10:54:04 +0300 Subject: [PATCH] orm changes --- .../Espo/Modules/Crm/Services/MassEmail.php | 30 ++++-- application/Espo/ORM/DB/BaseMapper.php | 97 ++++++++----------- application/Espo/ORM/EntityManager.php | 12 +-- application/Espo/ORM/RDBSelectParams.php | 8 +- tests/unit/Espo/ORM/DB/MapperTest.php | 39 ++++---- 5 files changed, 94 insertions(+), 92 deletions(-) diff --git a/application/Espo/Modules/Crm/Services/MassEmail.php b/application/Espo/Modules/Crm/Services/MassEmail.php index 23880810ad..d219e6d191 100644 --- a/application/Espo/Modules/Crm/Services/MassEmail.php +++ b/application/Espo/Modules/Crm/Services/MassEmail.php @@ -86,20 +86,34 @@ class MassEmail extends \Espo\Services\Record implements { parent::afterDeleteEntity($massEmail); - $this->getEntityManager()->getMapper('RDB')->massDeleteFromDb('EmailQueueItem', [ - 'massEmailId' => $massEmail->id, - ]); + $selectParams = $this->getEntityManager()->createSelectBuilder() + ->from('EmailQueueItem') + ->where([ + 'massEmailId' => $massEmail->id, + ]) + ->build(); + + $sql = $this->getEntityManager()->getQuery()->createDeleteQuery('EmailQueueItem', $selectParams->getRaw()); + + $this->getEntityManager()->runQuery($sql); } protected function cleanupQueueItems(Entity $massEmail) { - $this->getEntityManager()->getMapper('RDB')->massDeleteFromDb('EmailQueueItem', [ - 'massEmailId' => $massEmail->id, - 'status' => ['Pending', 'Failed'], - ]); + $selectParams = $this->getEntityManager()->createSelectBuilder() + ->from('EmailQueueItem') + ->where([ + 'massEmailId' => $massEmail->id, + 'status' => ['Pending', 'Failed'], + ]) + ->build(); + + $sql = $this->getEntityManager()->getQuery()->createDeleteQuery('EmailQueueItem', $selectParams->getRaw()); + + $this->getEntityManager()->runQuery($sql); } - public function createQueue(Entity $massEmail, $isTest = false, $additionalTargetList = []) + public function createQueue(Entity $massEmail, bool $isTest = false, $additionalTargetList = []) { if (!$isTest && $massEmail->get('status') !== 'Pending') { throw new Error("Mass Email '".$massEmail->id."' should be 'Pending'."); diff --git a/application/Espo/ORM/DB/BaseMapper.php b/application/Espo/ORM/DB/BaseMapper.php index 011d4e0409..b867609b93 100644 --- a/application/Espo/ORM/DB/BaseMapper.php +++ b/application/Espo/ORM/DB/BaseMapper.php @@ -29,6 +29,8 @@ namespace Espo\ORM\DB; +use Espo\Core\Exceptions\Error; + use Espo\ORM\{ Entity, Collection, @@ -40,6 +42,8 @@ use Espo\ORM\{ }; use PDO; +use Exception; +use LogicException; /** * Abstraction for DB. Mapping of Entity to DB. Supposed to be used only internally. Use repositories instead. @@ -206,15 +210,15 @@ abstract class BaseMapper implements Mapper $relDefs = $entity->relations[$relationName]; if (!isset($relDefs['type'])) { - throw new \LogicException( - "Missing 'type' in definition for relationship {$relationName} in " . $entity->getEntityType() . " entity" + throw new LogicException( + "Missing 'type' in definition for relationship {$relationName} in " . $entity->getEntityType() . " entity." ); } if ($relDefs['type'] !== Entity::BELONGS_TO_PARENT) { if (!isset($relDefs['entity'])) { - throw new \LogicException( - "Missing 'entity' in definition for relationship {$relationName} in " . $entity->getEntityType() . " entity" + throw new LogicException( + "Missing 'entity' in definition for relationship {$relationName} in " . $entity->getEntityType() . " entity." ); } @@ -552,7 +556,7 @@ abstract class BaseMapper implements Mapper $relDefs = $entity->relations[$relationName]; if (!isset($relDefs['entity']) || !isset($relDefs['type'])) { - throw new \LogicException("Not appropriate definition for relationship {$relationName} in " . $entity->getEntityType() . " entity"); + throw new LogicException("Not appropriate definition for relationship {$relationName} in " . $entity->getEntityType() . " entity."); } $relType = $relDefs['type']; @@ -611,18 +615,13 @@ abstract class BaseMapper implements Mapper { try { return $this->pdo->query($query); - } catch (\Exception $e) { + } catch (Exception $e) { if ($rerunIfDeadlock) { - if ( - isset($e->errorInfo) && - $e->errorInfo[0] == 40001 && - $e->errorInfo[1] == 1213 - ) { + if (isset($e->errorInfo) && $e->errorInfo[0] == 40001 && $e->errorInfo[1] == 1213) { return $this->pdo->query($query); - } else { - throw $e; } } + throw $e; } } @@ -645,7 +644,7 @@ abstract class BaseMapper implements Mapper $foreignEntityType = $entity->getRelationParam($relationName, 'entity'); if (!$relType || !$foreignEntityType && $relType !== Entity::BELONGS_TO_PARENT) { - throw new \LogicException("Not appropriate definition for relationship {$relationName} in " . $entity->getEntityType() . " entity"); + throw new LogicException("Not appropriate definition for relationship {$relationName} in " . $entity->getEntityType() . " entity."); } $className = (!empty($relDefs['class'])) ? $relDefs['class'] : $foreignEntityType; @@ -876,8 +875,8 @@ abstract class BaseMapper implements Mapper $foreignEntityType = $entity->getRelationParam($relationName, 'entity'); if (!$relType || !$foreignEntityType && $relType !== Entity::BELONGS_TO_PARENT) { - throw new \LogicException( - "Not appropriate definition for relationship {$relationName} in " . $entity->getEntityType() . " entity" + throw new LogicException( + "Not appropriate definition for relationship {$relationName} in " . $entity->getEntityType() . " entity." ); } @@ -1184,44 +1183,23 @@ abstract class BaseMapper implements Mapper */ public function deleteFromDb(string $entityType, string $id, bool $onlyDeleted = false) { - if (empty($entityType) || empty($id)) return false; + if (empty($entityType) || empty($id)) { + throw new Error("Can't delete an empty entity type or ID from DB."); + } - $table = $this->toDb($entityType); + $whereClause = [ + 'id' => $id, + ]; - $sql = "DELETE FROM `{$table}` WHERE id = " . $this->quote($id); if ($onlyDeleted) { - $sql .= " AND deleted = 1"; + $whereClause['deleted'] = true; } - $this->pdo->query($sql); - } + $sql = $this->query->createDeleteQuery($entityType, [ + 'whereClause' => $whereClause, + ]); - /** - * Mass delete from DB by specified whereClause. - * - * @return Number of deleted records or null if failure. - */ - public function massDeleteFromDb(string $entityType, array $whereClause) : ?int - { - $table = $this->toDb($entityType); - - $sql = "DELETE FROM `{$table}`"; - - $entity = $this->entityFactory->create($entityType); - if (!$entity) return null; - - $wherePart = $this->query->buildWherePart($entity->getEntityType(), $whereClause); - if ($wherePart) { - $sql .= ' WHERE ' . $wherePart; - } - - $sth = $this->pdo->prepare($sql); - - if ($sth->execute()) { - return $sth->rowCount(); - } - - return null; + $this->runQuery($sql); } /** @@ -1229,12 +1207,17 @@ abstract class BaseMapper implements Mapper */ public function restoreDeleted(string $entityType, string $id) { - if (empty($entityType) || empty($id)) return false; + if (empty($entityType) || empty($id)) { + throw new Error("Can't restore an empty entity type or ID."); + } - $table = $this->toDb($entityType); - $sql = "UPDATE `{$table}` SET `deleted` = 0 WHERE id = " . $this->quote($id); + $whereClause = [ + 'id' => $id, + ]; - $this->pdo->query($sql); + $sql = $this->query->createUpdateQuery($entityType, ['whereClause' => $whereClause], ['deleted' => false]); + + $this->runQuery($sql); } /** @@ -1243,12 +1226,14 @@ abstract class BaseMapper implements Mapper public function delete(Entity $entity) : bool { $entity->set('deleted', true); + return (booL) $this->update($entity); } - protected function toValueMap(Entity $entity, bool $onlyStorable = true) + protected function toValueMap(Entity $entity, bool $onlyStorable = true) : array { $data = []; + foreach ($entity->getAttributes() as $attribute => $defs) { if ($entity->has($attribute)) { if ($onlyStorable) { @@ -1264,16 +1249,18 @@ abstract class BaseMapper implements Mapper $data[$attribute] = $entity->get($attribute); } } + return $data; } - protected function fromRow(Entity $entity, $data) + protected function fromRow(Entity $entity, $data) : Entity { $entity->set($data); + return $entity; } - protected function getMMJoin(Entity $entity, $relationName, $keySet = false, $conditions = []) + protected function getMMJoin(Entity $entity, string $relationName, $keySet = false, $conditions = []) : string { $relDefs = $entity->relations[$relationName]; diff --git a/application/Espo/ORM/EntityManager.php b/application/Espo/ORM/EntityManager.php index e7a78eee1c..30f66b3733 100644 --- a/application/Espo/ORM/EntityManager.php +++ b/application/Espo/ORM/EntityManager.php @@ -37,6 +37,7 @@ use Espo\ORM\DB\{ }; use PDO; +use Exception; /** * A central access point to ORM functionality. @@ -76,11 +77,11 @@ class EntityManager if (empty($this->params['platform'])) { if (empty($this->params['driver'])) { - throw new \Exception('No database driver specified.'); + throw new Exception('No database driver specified.'); } $driver = $this->params['driver']; if (empty($this->driverPlatformMap[$driver])) { - throw new \Exception("Database driver '{$driver}' is not supported."); + throw new Exception("Database driver '{$driver}' is not supported."); } $this->params['platform'] = $this->driverPlatformMap[$this->params['driver']]; } @@ -313,14 +314,13 @@ class EntityManager { try { return $this->getPDO()->query($query); - } catch (\Exception $e) { + } catch (Exception $e) { if ($rerunIfDeadlock) { - if ($e->errorInfo[0] == 40001 && $e->errorInfo[1] == 1213) { + if (isset($e->errorInfo) && $e->errorInfo[0] == 40001 && $e->errorInfo[1] == 1213) { return $this->getPDO()->query($query); - } else { - throw $e; } } + throw $e; } } } diff --git a/application/Espo/ORM/RDBSelectParams.php b/application/Espo/ORM/RDBSelectParams.php index 379b992224..4b94e97cca 100644 --- a/application/Espo/ORM/RDBSelectParams.php +++ b/application/Espo/ORM/RDBSelectParams.php @@ -44,12 +44,18 @@ class RDBSelectParams $this->params = $params; } + /** + * Get an entity type. + */ public function getEntityType() : string { return $this->entityType; } - public function getRawParams() : array + /** + * Get parameters in RAW format. + */ + public function getRaw() : array { return $this->params; } diff --git a/tests/unit/Espo/ORM/DB/MapperTest.php b/tests/unit/Espo/ORM/DB/MapperTest.php index ce63857dac..3f92863e94 100644 --- a/tests/unit/Espo/ORM/DB/MapperTest.php +++ b/tests/unit/Espo/ORM/DB/MapperTest.php @@ -104,7 +104,7 @@ class DBMapperTest extends \PHPUnit\Framework\TestCase unset($this->pdo, $this->db, $this->post, $this->comment); } - protected function mockQuery($query, $return, $any = false) + protected function mockQuery(string $query, $return = true, $any = false) { if ($any) { $expects = $this->any(); @@ -544,32 +544,27 @@ class DBMapperTest extends \PHPUnit\Framework\TestCase )); } - public function testMassDeleteFromDb() + public function testDeleteFromDb1() { - $query = "DELETE FROM `post` WHERE post.name = 'test' AND post.deleted = 1"; + $query = "DELETE FROM `comment` WHERE comment.id = '1'"; + $this->mockQuery($query); - $sth = $this->getMockBuilder('\\PDOStatement')->disableOriginalConstructor()->getMock(); + $this->db->deleteFromDb('Comment', '1'); + } - $this->pdo - ->expects($this->once()) - ->method('prepare') - ->with($query) - ->will($this->returnValue($sth)); + public function testDeleteFromDb2() + { + $query = "DELETE FROM `comment` WHERE comment.id = '1' AND comment.deleted = 1"; + $this->mockQuery($query); - $sth - ->expects($this->once()) - ->method('execute') - ->will($this->returnValue(true)); + $this->db->deleteFromDb('Comment', '1', true); + } - $sth - ->expects($this->once()) - ->method('rowCount') - ->will($this->returnValue(1)); + public function testRestoreDeleted() + { + $query = "UPDATE `comment` SET comment.deleted = 0 WHERE comment.id = '1'"; + $this->mockQuery($query); - - $this->db->massDeleteFromDb('Post', [ - 'name' => 'test', - 'deleted' => true, - ]); + $this->db->restoreDeleted('Comment', '1'); } }