From 78b9e608e95d064455e4c75f2ab528bc61e1bb58 Mon Sep 17 00:00:00 2001 From: Yuri Kuznetsov Date: Thu, 27 Aug 2020 12:58:22 +0300 Subject: [PATCH] locker --- application/Espo/Core/CronManager.php | 119 +++++++++------- application/Espo/Core/Mail/Importer.php | 38 +++-- application/Espo/Core/Utils/Cron/Job.php | 29 +++- application/Espo/ORM/EntityManager.php | 28 +++- application/Espo/ORM/Locker/BaseLocker.php | 129 +++++++++++++++++ application/Espo/ORM/Locker/Locker.php | 62 +++++++++ application/Espo/ORM/Locker/MysqlLocker.php | 60 ++++++++ .../ORM/QueryComposer/BaseQueryComposer.php | 33 +++-- .../ORM/QueryComposer/MysqlQueryComposer.php | 37 +++++ .../Espo/ORM/QueryComposer/QueryComposer.php | 24 +++- .../Espo/ORM/QueryParams/LockTable.php | 54 ++++++++ .../Espo/ORM/QueryParams/LockTableBuilder.php | 85 ++++++++++++ .../Espo/ORM/Repository/RDBRepository.php | 35 +---- application/Espo/Repositories/User.php | 34 +++-- tests/unit/Espo/ORM/EntityTest.php | 2 + tests/unit/Espo/ORM/MapperTest.php | 5 + tests/unit/Espo/ORM/MetadataTest.php | 2 + tests/unit/Espo/ORM/MysqlLockerTest.php | 130 ++++++++++++++++++ .../unit/Espo/ORM/MysqlQueryComposerTest.php | 33 +++++ 19 files changed, 801 insertions(+), 138 deletions(-) create mode 100644 application/Espo/ORM/Locker/BaseLocker.php create mode 100644 application/Espo/ORM/Locker/Locker.php create mode 100644 application/Espo/ORM/Locker/MysqlLocker.php create mode 100644 application/Espo/ORM/QueryParams/LockTable.php create mode 100644 application/Espo/ORM/QueryParams/LockTableBuilder.php create mode 100644 tests/unit/Espo/ORM/MysqlLockerTest.php diff --git a/application/Espo/Core/CronManager.php b/application/Espo/Core/CronManager.php index b4d831a2b0..cb3c76fd1e 100644 --- a/application/Espo/Core/CronManager.php +++ b/application/Espo/Core/CronManager.php @@ -42,6 +42,7 @@ use Espo\Core\{ Utils\Cron\ScheduledJob as CronScheduledJob, Utils\Cron\Job as CronJob, ORM\EntityManager, + Utils\Cron\JobTask, }; use Espo\Entities\Job as JobEntity; @@ -196,7 +197,7 @@ class CronManager /** * Run a portion of pending jobs. */ - public function processPendingJobs($queue = null, $limit = null, $poolDisabled = false, $noLock = false) + public function processPendingJobs($queue = null, $limit = null, bool $poolDisabled = false, bool $noLock = false) { if (is_null($limit)) { $limit = intval($this->config->get('jobMaxPortion', 0)); @@ -210,51 +211,17 @@ class CronManager $useProcessPool = false; } + $pool = null; + if ($useProcessPool) { - $pool = \Spatie\Async\Pool::create() + $pool = AsyncPool::create() ->autoload(getcwd() . '/vendor/autoload.php') ->concurrency($this->config->get('jobPoolConcurrencyNumber')) ->timeout($this->config->get('jobPeriodForActiveProcess')); } foreach ($pendingJobList as $job) { - $skip = false; - if (!$noLock) $this->lockJobTable(); - if ($noLock || $this->getCronJobUtil()->isJobPending($job->id)) { - if ($job->get('scheduledJobId')) { - if ($this->getCronJobUtil()->isScheduledJobRunning( - $job->get('scheduledJobId'), $job->get('targetId'), $job->get('targetType')) - ) { - $skip = true; - } - } - } else { - $skip = true; - } - - if ($skip) { - if (!$noLock) $this->unlockTables(); - continue; - } - - $job->set('startedAt', date('Y-m-d H:i:s')); - - if ($useProcessPool) { - $job->set('status', self::READY); - } else { - $job->set('status', self::RUNNING); - $job->set('pid', \Espo\Core\Utils\System::getPid()); - } - - $this->entityManager->saveEntity($job); - if (!$noLock) $this->unlockTables(); - - if ($useProcessPool) { - $task = new \Espo\Core\Utils\Cron\JobTask($job->id); - $pool->add($task); - } else { - $this->runJob($job); - } + $this->processPendingJob($job, $pool, $noLock); } if ($useProcessPool) { @@ -262,14 +229,74 @@ class CronManager } } - protected function lockJobTable() + protected function processPendingJob(JobEntity $job, $pool = null, bool $noLock = false) { - $this->entityManager->getPDO()->query('LOCK TABLES `job` WRITE'); - } + $useProcessPool = (bool) $pool; - protected function unlockTables() - { - $this->entityManager->getPDO()->query('UNLOCK TABLES'); + $lockTable = (bool) $job->get('scheduledJobId') && !$noLock; + + $skip = false; + + if (!$noLock) { + if ($lockTable) { + // MySQL doesn't allow to lock non-existent rows. We resort to locking an entire table. + $this->entityManager->getLocker()->lockExclusive('Job'); + } else { + $this->entityManager->getTransactionManager()->start(); + } + } + + if ($noLock || $this->getCronJobUtil()->isJobPending($job->id)) { + if ($job->get('scheduledJobId')) { + if ($this->getCronJobUtil()->isScheduledJobRunning( + $job->get('scheduledJobId'), $job->get('targetId'), $job->get('targetType')) + ) { + $skip = true; + } + } + } else { + $skip = true; + } + + if ($skip) { + if (!$noLock) { + if ($lockTable) { + $this->entityManager->getLocker()->rollback(); + } else { + $this->entityManager->getTransactionManager()->rollback(); + } + } + + return; + } + + $job->set('startedAt', date('Y-m-d H:i:s')); + + if ($useProcessPool) { + $job->set('status', self::READY); + } else { + $job->set('status', self::RUNNING); + $job->set('pid', System::getPid()); + } + + $this->entityManager->saveEntity($job); + + if (!$noLock) { + if ($lockTable) { + $this->entityManager->getLocker()->commit(); + } else { + $this->entityManager->getTransactionManager()->commit(); + } + } + + if ($useProcessPool) { + $task = new JobTask($job->id); + $pool->add($task); + + return; + } + + $this->runJob($job); } /** @@ -292,7 +319,7 @@ class CronManager } $job->set('status', self::RUNNING); - $job->set('pid',System::getPid()); + $job->set('pid', System::getPid()); $this->entityManager->saveEntity($job); $this->runJob($job); diff --git a/application/Espo/Core/Mail/Importer.php b/application/Espo/Core/Mail/Importer.php index e229680a9b..aaea341144 100644 --- a/application/Espo/Core/Mail/Importer.php +++ b/application/Espo/Core/Mail/Importer.php @@ -326,12 +326,15 @@ class Importer } if (!$duplicate) { - $this->lockEmailTable(); + $this->getEntityManager()->getLocker()->lockExclusive('Email'); + if ($duplicate = $this->findDuplicate($email)) { - $this->unlockTables(); + $this->getEntityManager()->getLocker()->rollback(); + if ($duplicate->get('status') != 'Being Imported') { $duplicate = $this->getEntityManager()->getEntity('Email', $duplicate->id); $this->processDuplicate($duplicate, $assignedUserId, $userIdList, $folderData, $teamsIdList); + return $duplicate; } } @@ -355,23 +358,26 @@ class Importer 'fromString' => $email->get('fromString'), 'replyToString' => $email->get('replyToString'), ]); + $this->getEntityManager()->getRepository('Email')->fillAccount($duplicate); $this->processDuplicate($duplicate, $assignedUserId, $userIdList, $folderData, $teamsIdList); + return $duplicate; } if (!$email->get('messageId')) { $email->setDummyMessageId(); } + $email->set('status', 'Being Imported'); $this->getEntityManager()->saveEntity($email, [ 'skipAll' => true, - 'keepNew' => true + 'keepNew' => true, ]); - $this->unlockTables(); + $this->getEntityManager()->getLocker()->commit(); if ($parentFound) { $parentType = $email->get('parentType'); @@ -400,7 +406,7 @@ class Importer $attachment->set([ 'relatedId' => $email->id, 'relatedType' => 'Email', - 'field' => 'body' + 'field' => 'body', ]); $this->getEntityManager()->saveEntity($attachment); } @@ -408,21 +414,12 @@ class Importer return $email; } - protected function lockEmailTable() - { - $this->getEntityManager()->getPdo()->query('LOCK TABLES `email` WRITE'); - } - - protected function unlockTables() - { - $this->getEntityManager()->getPdo()->query('UNLOCK TABLES'); - } - protected function findParent(Email $email, $emailAddress) { $contact = $this->getEntityManager()->getRepository('Contact')->where([ 'emailAddress' => $emailAddress ])->findOne(); + if ($contact) { if (!$this->getConfig()->get('b2cMode')) { if ($contact->get('accountId')) { @@ -438,6 +435,7 @@ class Importer $account = $this->getEntityManager()->getRepository('Account')->where([ 'emailAddress' => $emailAddress ])->findOne(); + if ($account) { $email->set('parentType', 'Account'); $email->set('parentId', $account->id); @@ -507,7 +505,7 @@ class Importer $duplicate->setLinkMultipleColumn('users', 'folderId', $uId, $folderId); } else { $this->getEntityManager()->getRepository('Email')->updateRelation($duplicate, 'users', $uId, [ - 'folderId' => $folderId + 'folderId' => $folderId, ]); } } @@ -519,17 +517,17 @@ class Importer $this->getEntityManager()->getRepository('Email')->processLinkMultipleFieldSave($duplicate, 'users', [ 'skipLinkMultipleRemove' => true, - 'skipLinkMultipleUpdate' => true + 'skipLinkMultipleUpdate' => true, ]); $this->getEntityManager()->getRepository('Email')->processLinkMultipleFieldSave($duplicate, 'assignedUsers', [ 'skipLinkMultipleRemove' => true, - 'skipLinkMultipleUpdate' => true + 'skipLinkMultipleUpdate' => true, ]); if ($notificator = $this->getNotificator()) { $notificator->process($duplicate, [ - 'isBeingImported' => true + 'isBeingImported' => true, ]); } @@ -558,7 +556,7 @@ class Importer 'targetId' => $duplicate->id ], 'executeAt' => $executeAt, - 'queue' => 'q1' + 'queue' => 'q1', ]); $this->getEntityManager()->saveEntity($job); } diff --git a/application/Espo/Core/Utils/Cron/Job.php b/application/Espo/Core/Utils/Cron/Job.php index 7a97d5898e..d54fd8f987 100644 --- a/application/Espo/Core/Utils/Cron/Job.php +++ b/application/Espo/Core/Utils/Cron/Job.php @@ -75,12 +75,22 @@ class Job return $this->cronScheduledJob; } - public function isJobPending($id) + public function isJobPending(string $id) : bool { - return !!$this->getEntityManager()->getRepository('Job')->select(['id'])->where([ - 'id' => $id, - 'status' => CronManager::PENDING - ])->findOne(); + $job = $this->getEntityManager() + ->getRepository('Job') + ->select(['id', 'status']) + ->where([ + 'id' => $id, + ]) + ->forUpdate() + ->findOne(); + + if (!$job) { + return false; + } + + return $job->get('status') === CronManager::PENDING; } public function getPendingJobList($queue = null, $limit = 0) @@ -113,17 +123,22 @@ class Job return $this->getEntityManager()->getRepository('Job')->find($selectParams); } - public function isScheduledJobRunning($scheduledJobId, $targetId = null, $targetType = null) + public function isScheduledJobRunning(string $scheduledJobId, ?string $targetId = null, ?string $targetType = null) : bool { $where = [ 'scheduledJobId' => $scheduledJobId, 'status' => [CronManager::RUNNING, CronManager::READY], ]; + if ($targetId && $targetType) { $where['targetId'] = $targetId; $where['targetType'] = $targetType; } - return !!$this->getEntityManager()->getRepository('Job')->select(['id'])->where($where)->findOne(); + + return (booL) $this->getEntityManager()->getRepository('Job') + ->select(['id']) + ->where($where) + ->findOne(); } public function getRunningScheduledJobIdList() : array diff --git a/application/Espo/ORM/EntityManager.php b/application/Espo/ORM/EntityManager.php index aaa9e9a669..7ac7204fee 100644 --- a/application/Espo/ORM/EntityManager.php +++ b/application/Espo/ORM/EntityManager.php @@ -34,6 +34,7 @@ use Espo\ORM\{ QueryComposer\QueryComposer, Repository\RepositoryFactory, Repository\Repository, + Locker\Locker, }; use PDO; @@ -70,6 +71,8 @@ class EntityManager protected $transactionManager; + protected $locker; + protected $defaultMapperName = 'RDB'; protected $driverPlatformMap = [ @@ -114,6 +117,8 @@ class EntityManager $this->collectionFactory = new CollectionFactory($this); $this->transactionManager = new TransactionManager($this->getPDO(), $this->queryComposer); + + $this->initLocker(); } protected function initQueryComposer() @@ -126,12 +131,28 @@ class EntityManager } if (!$className || !class_exists($className)) { - throw new RuntimeException("Query composer {$name} could not be created."); + throw new RuntimeException("Query composer could not be created."); } $this->queryComposer = new $className($this->getPDO(), $this->entityFactory, $this->metadata); } + protected function initLocker() + { + $className = $this->params['lockerClassName'] ?? null; + + if (!$className) { + $platform = $this->params['platform']; + $className = 'Espo\\ORM\\Locker\\' . ucfirst($platform) . 'Locker'; + } + + if (!$className || !class_exists($className)) { + throw new RuntimeException("Locker could not be created."); + } + + $this->locker = new $className($this->getPDO(), $this->queryComposer, $this->transactionManager); + } + /** * @todo Remove in v7.0. * @deprecated @@ -151,6 +172,11 @@ class EntityManager return $this->transactionManager; } + public function getLocker() : Locker + { + return $this->locker; + } + protected function getMapperClassName(string $name) : string { $className = null; diff --git a/application/Espo/ORM/Locker/BaseLocker.php b/application/Espo/ORM/Locker/BaseLocker.php new file mode 100644 index 0000000000..a9a3a2f600 --- /dev/null +++ b/application/Espo/ORM/Locker/BaseLocker.php @@ -0,0 +1,129 @@ +pdo = $pdo; + $this->queryComposer = $queryComposer; + $this->transactionManager = $transactionManager; + } + + /** + * {@inheritdoc} + */ + public function isLocked() : bool + { + return $this->isLocked; + } + + /** + * {@inheritdoc} + */ + public function lockExclusive(string $entityType) + { + $this->isLocked = true; + + $this->transactionManager->start(); + + $query = (new LockTableBuilder()) + ->table($entityType) + ->inExclusiveMode() + ->build(); + + $sql = $this->queryComposer->compose($query); + + $this->pdo->exec($sql); + } + + /** + * {@inheritdoc} + */ + public function lockShare(string $entityType) + { + $this->isLocked = true; + + $this->transactionManager->start(); + + $query = (new LockTableBuilder()) + ->table($entityType) + ->inShareMode() + ->build(); + + $sql = $this->queryComposer->compose($query); + + $this->pdo->exec($sql); + } + + /** + * {@inheritdoc} + */ + public function commit() + { + if (!$this->isLocked) { + throw new RuntimeException("Can't commit, it was not locked."); + } + + $this->transactionManager->commit(); + + $this->isLocked = false; + } + + /** + * {@inheritdoc} + */ + public function rollback() + { + if (!$this->isLocked) { + throw new RuntimeException("Can't rollback, it was not locked."); + } + + $this->transactionManager->rollback(); + + $this->isLocked = false; + } +} diff --git a/application/Espo/ORM/Locker/Locker.php b/application/Espo/ORM/Locker/Locker.php new file mode 100644 index 0000000000..82ef0fe09b --- /dev/null +++ b/application/Espo/ORM/Locker/Locker.php @@ -0,0 +1,62 @@ +queryComposer->composeUnlockTables(); + + $this->pdo->exec($sql); + + } + + /** + * {@inheritdoc} + */ + public function rollback() + { + parent::rollback(); + + $sql = $this->queryComposer->composeUnlockTables(); + + $this->pdo->exec($sql); + } +} diff --git a/application/Espo/ORM/QueryComposer/BaseQueryComposer.php b/application/Espo/ORM/QueryComposer/BaseQueryComposer.php index 8797fc5723..b5ac6951c5 100644 --- a/application/Espo/ORM/QueryComposer/BaseQueryComposer.php +++ b/application/Espo/ORM/QueryComposer/BaseQueryComposer.php @@ -42,6 +42,7 @@ use Espo\ORM\{ QueryParams\Delete as DeleteQuery, QueryParams\Union as UnionQuery, QueryParams\Selecting as SelectingQuery, + QueryParams\LockTable as LockTableQuery, }; use PDO; @@ -208,28 +209,32 @@ abstract class BaseQueryComposer implements QueryComposer return $this->composeUnion($query); } + if ($query instanceof LockTableQuery) { + return $this->composeLockTable($query); + } + throw new RuntimeException("ORM Query: Unknown query type passed."); } - public function composeCreateSavepoint(string $name) : string + public function composeCreateSavepoint(string $savepointName) : string { - $name = $this->sanitize($name); + $savepointName = $this->sanitize($savepointName); - return 'SAVEPOINT ' . $name; + return 'SAVEPOINT ' . $savepointName; } - public function composeReleaseSavepoint(string $name) : string + public function composeReleaseSavepoint(string $savepointName) : string { - $name = $this->sanitize($name); + $savepointName = $this->sanitize($savepointName); - return 'RELEASE SAVEPOINT ' . $name; + return 'RELEASE SAVEPOINT ' . $savepointName; } - public function composeRollbackToSavepoint(string $name) : string + public function composeRollbackToSavepoint(string $savepointName) : string { - $name = $this->sanitize($name); + $savepointName = $this->sanitize($savepointName); - return 'ROLLBACK TO SAVEPOINT ' . $name; + return 'ROLLBACK TO SAVEPOINT ' . $savepointName; } protected function composeSelecting(SelectingQuery $queryParams) : string @@ -237,35 +242,35 @@ abstract class BaseQueryComposer implements QueryComposer return $this->compose($queryParams); } - protected function composeSelect(SelectQuery $queryParams) : string + public function composeSelect(SelectQuery $queryParams) : string { $params = $queryParams->getRawParams(); return $this->createSelectQueryInternal($params); } - protected function composeUpdate(UpdateQuery $queryParams) : string + public function composeUpdate(UpdateQuery $queryParams) : string { $params = $queryParams->getRawParams(); return $this->createUpdateQuery($params); } - protected function composeDelete(DeleteQuery $queryParams) : string + public function composeDelete(DeleteQuery $queryParams) : string { $params = $queryParams->getRawParams(); return $this->createDeleteQuery($params); } - protected function composeInsert(InsertQuery $queryParams) : string + public function composeInsert(InsertQuery $queryParams) : string { $params = $queryParams->getRawParams(); return $this->createInsertQuery($params); } - protected function composeUnion(UnionQuery $queryParams) : string + public function composeUnion(UnionQuery $queryParams) : string { $params = $queryParams->getRawParams(); diff --git a/application/Espo/ORM/QueryComposer/MysqlQueryComposer.php b/application/Espo/ORM/QueryComposer/MysqlQueryComposer.php index aa743b799f..9dcf01570f 100644 --- a/application/Espo/ORM/QueryComposer/MysqlQueryComposer.php +++ b/application/Espo/ORM/QueryComposer/MysqlQueryComposer.php @@ -29,8 +29,45 @@ namespace Espo\ORM\QueryComposer; +use Espo\ORM\{ + QueryParams\LockTable as LockTableQuery, +}; + class MysqlQueryComposer extends BaseQueryComposer { + public function composeLockTable(LockTableQuery $queryParams) : string + { + $params = $queryParams->getRawParams(); + + $table = $this->toDb($this->sanitize($params['table'])); + + $mode = $params['mode']; + + if (empty($table)) { + throw new LogicException(); + } + + if (!in_array($mode, [LockTableQuery::MODE_SHARE, LockTableQuery::MODE_EXCLUSIVE])) { + throw new LogicException(); + } + + $sql = "LOCK TABLES `{$table}` "; + + $modeMap = [ + LockTableQuery::MODE_SHARE => 'READ', + LockTableQuery::MODE_EXCLUSIVE => 'WRITE', + ]; + + $sql .= $modeMap[$mode]; + + return $sql; + } + + public function composeUnlockTables() : string + { + return "UNLOCK TABLES"; + } + protected function limit(string $sql, ?int $offset = null, ?int $limit = null) : string { if (!is_null($offset) && !is_null($limit)) { diff --git a/application/Espo/ORM/QueryComposer/QueryComposer.php b/application/Espo/ORM/QueryComposer/QueryComposer.php index 2689e532b4..5a6ee6cb2a 100644 --- a/application/Espo/ORM/QueryComposer/QueryComposer.php +++ b/application/Espo/ORM/QueryComposer/QueryComposer.php @@ -31,6 +31,12 @@ namespace Espo\ORM\QueryComposer; use Espo\ORM\{ QueryParams\Query as Query, + QueryParams\Select as SelectQuery, + QueryParams\Update as UpdateQuery, + QueryParams\Insert as InsertQuery, + QueryParams\Delete as DeleteQuery, + QueryParams\Union as UnionQuery, + QueryParams\LockTable as LockTableQuery, }; interface QueryComposer @@ -40,9 +46,21 @@ interface QueryComposer */ public function compose(Query $query) : string; - public function composeCreateSavepoint(string $name) : string; + public function composeSelect(SelectQuery $query) : string; - public function composeReleaseSavepoint(string $name) : string; + public function composeUpdate(UpdateQuery $query) : string; - public function composeRollbackToSavepoint(string $name) : string; + public function composeDelete(DeleteQuery $query) : string; + + public function composeInsert(InsertQuery $query) : string; + + public function composeUnion(UnionQuery $query) : string; + + public function composeLockTable(LockTableQuery $query) : string; + + public function composeCreateSavepoint(string $savepointName) : string; + + public function composeReleaseSavepoint(string $savepointName) : string; + + public function composeRollbackToSavepoint(string $savepointName) : string; } diff --git a/application/Espo/ORM/QueryParams/LockTable.php b/application/Espo/ORM/QueryParams/LockTable.php new file mode 100644 index 0000000000..784d0879b5 --- /dev/null +++ b/application/Espo/ORM/QueryParams/LockTable.php @@ -0,0 +1,54 @@ +params); + } + + /** + * Clone an existing query for a subsequent modifying and building. + */ + public function clone(LockTable $query) : self + { + $this->cloneInternal($query); + + return $this; + } + + /** + * What entity type to lock. + */ + public function table(string $entityType) : self + { + $this->params['table'] = $entityType; + + return $this; + } + + /** + * In SHARE mode. + */ + public function inShareMode() : self + { + $this->params['mode'] = LockTable::MODE_SHARE; + + return $this; + } + + /** + * In EXCLUSIVE mode. + */ + public function inExclusiveMode() : self + { + $this->params['mode'] = LockTable::MODE_EXCLUSIVE; + + return $this; + } +} diff --git a/application/Espo/ORM/Repository/RDBRepository.php b/application/Espo/ORM/Repository/RDBRepository.php index 3b2c5472e4..06a67216d6 100644 --- a/application/Espo/ORM/Repository/RDBRepository.php +++ b/application/Espo/ORM/Repository/RDBRepository.php @@ -46,8 +46,6 @@ class RDBRepository extends Repository { protected $mapper; - private $isTableLocked = false; - protected $hookMediator; protected $transactionManager; @@ -56,10 +54,8 @@ class RDBRepository extends Repository string $entityType, EntityManager $entityManager, EntityFactory $entityFactory, ?HookMediator $hookMediator = null ) { $this->entityType = $entityType; - $this->entityName = $entityType; - - $this->entityFactory = $entityFactory; $this->entityManager = $entityManager; + $this->entityFactory = $entityFactory; $this->seed = $this->entityFactory->create($entityType); @@ -757,35 +753,6 @@ class RDBRepository extends Repository return $this->entityManager->getPDO(); } - /** - * @deprecated - * @todo Remove. - */ - protected function lockTable() - { - $tableName = $this->entityManager->getQueryComposer()->toDb($this->entityType); - - // @todo Use SELECT ... FOR UPDATE with transaction. - $this->getPDO()->query("LOCK TABLES `{$tableName}` WRITE"); - $this->isTableLocked = true; - } - - /** - * @deprecated - * @todo Remove. - */ - protected function unlockTable() - { - // @todo Use SELECT ... FOR UPDATE with transaction. - $this->getPDO()->query("UNLOCK TABLES"); - $this->isTableLocked = false; - } - - protected function isTableLocked() - { - return $this->isTableLocked; - } - protected function createSelectBuilder() : RDBSelectBuilder { $builder = new RDBSelectBuilder($this->entityManager, $this->entityType); diff --git a/application/Espo/Repositories/User.php b/application/Espo/Repositories/User.php index daca569978..68627e7033 100644 --- a/application/Espo/Repositories/User.php +++ b/application/Espo/Repositories/User.php @@ -87,14 +87,18 @@ class User extends \Espo\Core\Repositories\Database implements throw new Error("Username can't be empty."); } - $this->lockTable(); + $this->getEntityManager()->getLocker()->lockExclusive($this->entityType); - $user = $this->select(['id'])->where([ - 'userName' => $userName - ])->findOne(); + $user = $this + ->select(['id']) + ->where([ + 'userName' => $userName, + ]) + ->findOne(); if ($user) { - $this->unlockTable(); + $this->getEntityManager()->getLocker()->rollback(); + throw new Conflict(json_encode(['reason' => 'userNameExists'])); } } else { @@ -104,15 +108,19 @@ class User extends \Espo\Core\Repositories\Database implements throw new Error("Username can't be empty."); } - $this->lockTable(); + $this->getEntityManager()->getLocker()->lockExclusive($this->entityType); - $user = $this->select(['id'])->where(array( - 'userName' => $userName, - 'id!=' => $entity->id - ))->findOne(); + $user = $this + ->select(['id']) + ->where([ + 'userName' => $userName, + 'id!=' => $entity->id, + ]) + ->findOne(); if ($user) { - $this->unlockTable(); + $this->getEntityManager()->getLocker()->rollback(); + throw new Conflict(json_encode(['reason' => 'userNameExists'])); } } @@ -121,8 +129,8 @@ class User extends \Espo\Core\Repositories\Database implements protected function afterSave(Entity $entity, array $options = []) { - if ($this->isTableLocked()) { - $this->unlockTable(); + if ($this->getEntityManager()->getLocker()->isLocked()) { + $this->getEntityManager()->getLocker()->commit(); } parent::afterSave($entity, $options); diff --git a/tests/unit/Espo/ORM/EntityTest.php b/tests/unit/Espo/ORM/EntityTest.php index 005238036d..d6537465de 100644 --- a/tests/unit/Espo/ORM/EntityTest.php +++ b/tests/unit/Espo/ORM/EntityTest.php @@ -27,6 +27,8 @@ * these Appropriate Legal Notices must retain the display of the "EspoCRM" word. ************************************************************************/ +namespace tests\unit\Espo\ORM; + use Espo\ORM\{ DB\MysqlMapper, DB\Query\Mysql as Query, diff --git a/tests/unit/Espo/ORM/MapperTest.php b/tests/unit/Espo/ORM/MapperTest.php index a4a32e7b65..f6ff3e6ac6 100644 --- a/tests/unit/Espo/ORM/MapperTest.php +++ b/tests/unit/Espo/ORM/MapperTest.php @@ -27,6 +27,8 @@ * these Appropriate Legal Notices must retain the display of the "EspoCRM" word. ************************************************************************/ +namespace tests\unit\Espo\ORM; + use Espo\ORM\{ Metadata, EntityManager, @@ -47,6 +49,9 @@ use Espo\Entities\{ Job, }; +use PDO; +use PDOStatement; + require_once 'tests/unit/testData/DB/Entities.php'; class DBMapperTest extends \PHPUnit\Framework\TestCase diff --git a/tests/unit/Espo/ORM/MetadataTest.php b/tests/unit/Espo/ORM/MetadataTest.php index 2c8a374ae0..6ff33cb4ff 100644 --- a/tests/unit/Espo/ORM/MetadataTest.php +++ b/tests/unit/Espo/ORM/MetadataTest.php @@ -27,6 +27,8 @@ * these Appropriate Legal Notices must retain the display of the "EspoCRM" word. ************************************************************************/ +namespace tests\unit\Espo\ORM; + use Espo\ORM\{ Metadata, }; diff --git a/tests/unit/Espo/ORM/MysqlLockerTest.php b/tests/unit/Espo/ORM/MysqlLockerTest.php new file mode 100644 index 0000000000..4cefed04f8 --- /dev/null +++ b/tests/unit/Espo/ORM/MysqlLockerTest.php @@ -0,0 +1,130 @@ +pdo = $this->getMockBuilder(PDO::class)->disableOriginalConstructor()->getMock(); + + $entityFactory = $this->getMockBuilder(EntityFactory::class)->disableOriginalConstructor()->getMock(); + + $metadata = $this->getMockBuilder(Metadata::class)->disableOriginalConstructor()->getMock(); + + $this->transactionManager = $this->getMockBuilder(TransactionManager::class)->disableOriginalConstructor()->getMock(); + + $composer = new MysqlQueryComposer($this->pdo, $entityFactory, $metadata); + + $this->locker = new MysqlLocker($this->pdo, $composer, $this->transactionManager); + } + + public function testLockCommit() + { + $this->pdo + ->expects($this->at(0)) + ->method('exec') + ->with('LOCK TABLES `account` WRITE'); + + $this->transactionManager + ->expects($this->at(0)) + ->method('start'); + + $this->pdo + ->expects($this->at(1)) + ->method('exec') + ->with('LOCK TABLES `contact` READ'); + + $this->transactionManager + ->expects($this->at(1)) + ->method('commit'); + + $this->pdo + ->expects($this->at(2)) + ->method('exec') + ->with('UNLOCK TABLES'); + + $this->locker->lockExclusive('Account'); + $this->locker->lockShare('Contact'); + + $this->assertTrue($this->locker->isLocked()); + + $this->locker->commit(); + + $this->assertFalse($this->locker->isLocked()); + } + + public function testLockRollback() + { + $this->pdo + ->expects($this->at(0)) + ->method('exec') + ->with('LOCK TABLES `account` WRITE'); + + $this->transactionManager + ->expects($this->at(0)) + ->method('start'); + + $this->pdo + ->expects($this->at(1)) + ->method('exec') + ->with('LOCK TABLES `contact` READ'); + + $this->transactionManager + ->expects($this->at(1)) + ->method('rollback'); + + $this->pdo + ->expects($this->at(2)) + ->method('exec') + ->with('UNLOCK TABLES'); + + $this->locker->lockExclusive('Account'); + $this->locker->lockShare('Contact'); + + $this->assertTrue($this->locker->isLocked()); + + $this->locker->commit(); + + $this->assertFalse($this->locker->isLocked()); + } +} diff --git a/tests/unit/Espo/ORM/MysqlQueryComposerTest.php b/tests/unit/Espo/ORM/MysqlQueryComposerTest.php index 00d9a098bc..17dd740d86 100644 --- a/tests/unit/Espo/ORM/MysqlQueryComposerTest.php +++ b/tests/unit/Espo/ORM/MysqlQueryComposerTest.php @@ -42,6 +42,7 @@ use Espo\ORM\QueryParams\{ Insert, Update, Delete, + LockTableBuilder, }; use Espo\Entities\{ @@ -2089,4 +2090,36 @@ class MysqlQueryComposerTest extends \PHPUnit\Framework\TestCase $this->assertEquals($expectedSql, $sql); } + + public function testLockTableShare() + { + $builder = new LockTableBuilder(); + + $query = $builder + ->table('Account') + ->inShareMode() + ->build(); + + $sql = $this->query->compose($query); + + $expectedSql = "LOCK TABLES `account` READ"; + + $this->assertEquals($expectedSql, $sql); + } + + public function testLockTableExclusive() + { + $builder = new LockTableBuilder(); + + $query = $builder + ->table('Account') + ->inExclusiveMode() + ->build(); + + $sql = $this->query->compose($query); + + $expectedSql = "LOCK TABLES `account` WRITE"; + + $this->assertEquals($expectedSql, $sql); + } }